From 4ca7439c11417d13b170b44aeb94fee5a2b185da Mon Sep 17 00:00:00 2001 From: Thomas Lopez Date: Fri, 21 Aug 2026 10:56:34 -0400 Subject: [PATCH 1/5] [UUM-149791] Fix offset pivot gizmo when duplicating shapes with First Vertex pivot previewPivotPosition normalized the drag-captured corner offset and then rescaled it by its own original magnitude, which is a no-op and always reconstructs the offset from the size of the shape that was last dragged out rather than the size of the shape currently being placed. Resizing a shape (e.g. via Shape Settings) before shift-duplicating it with Pivot = First Vertex therefore placed the new shape's pivot using the previous shape's dimensions instead of its own. Now only the corner (sign per axis) is kept from the last drag, and the offset magnitude is scaled from the current bounds size. --- Editor/EditorCore/DrawShapeTool.cs | 23 ++++++-- .../Editor/DrawShapeToolPivotOffsetTests.cs | 55 +++++++++++++++++++ .../DrawShapeToolPivotOffsetTests.cs.meta | 2 + 3 files changed, 75 insertions(+), 5 deletions(-) create mode 100644 Tests/Editor/Editor/DrawShapeToolPivotOffsetTests.cs create mode 100644 Tests/Editor/Editor/DrawShapeToolPivotOffsetTests.cs.meta diff --git a/Editor/EditorCore/DrawShapeTool.cs b/Editor/EditorCore/DrawShapeTool.cs index 7b532be53..38bd3099a 100644 --- a/Editor/EditorCore/DrawShapeTool.cs +++ b/Editor/EditorCore/DrawShapeTool.cs @@ -446,13 +446,17 @@ internal Vector3 previewPivotPosition { if (pivotLocation == PivotLocation.FirstVertex && instance != null) { + // Only the corner (sign per axis) of the last non-duplicate shape is reused here. + // The offset magnitude must come from the size of the shape currently being + // previewed/duplicated (m_Bounds.size), not from the size it was originally drawn at. var lastCenterToOrigin = instance.m_LastNonDuplicateCenterToOrigin; - var lastCenterToOriginNorm = lastCenterToOrigin.normalized; + var cornerSign = new Vector3( + SignOrZero(lastCenterToOrigin.x), + SignOrZero(lastCenterToOrigin.y), + SignOrZero(lastCenterToOrigin.z)); - var deltaRot = instance.m_PlaneRotation; - lastCenterToOriginNorm = deltaRot * lastCenterToOriginNorm; - - var pivotOffset = lastCenterToOriginNorm * lastCenterToOrigin.magnitude; + var currentCenterToOrigin = Vector3.Scale(cornerSign, instance.m_Bounds.size * 0.5f); + var pivotOffset = instance.m_PlaneRotation * currentCenterToOrigin; return pivotOffset + instance.m_Bounds.center; } @@ -460,6 +464,15 @@ internal Vector3 previewPivotPosition } } + static float SignOrZero(float value) + { + if (value > 0f) + return 1f; + if (value < 0f) + return -1f; + return 0f; + } + int m_ControlID; internal float minSnapSize diff --git a/Tests/Editor/Editor/DrawShapeToolPivotOffsetTests.cs b/Tests/Editor/Editor/DrawShapeToolPivotOffsetTests.cs new file mode 100644 index 000000000..08ebac70b --- /dev/null +++ b/Tests/Editor/Editor/DrawShapeToolPivotOffsetTests.cs @@ -0,0 +1,55 @@ +using NUnit.Framework; +using UnityEditor; +using UnityEditor.EditorTools; +using UnityEditor.ProBuilder; +using UnityEngine; +using ToolManager = UnityEditor.EditorTools.ToolManager; + +public class DrawShapeToolPivotOffsetTests +{ + Pref m_PivotPref = new Pref("ShapeBuilder.PivotLocation.Cube", PivotLocation.Center); + PivotLocation m_PreviousPivot; + + [SetUp] + public void SetUp() + { + ToolManager.SetActiveContext(); + ToolManager.SetActiveTool(); + + m_PreviousPivot = m_PivotPref.value; + m_PivotPref.SetValue(PivotLocation.FirstVertex); + } + + [TearDown] + public void TearDown() + { + m_PivotPref.SetValue(m_PreviousPivot); + ToolManager.RestorePreviousPersistentTool(); + } + + // Reproduces: create a shape with Pivot = First Vertex at one size (e.g. drag out a 4x4x4 cube), + // then place a duplicate (shift-click) at a different, smaller size (e.g. 1x1x1 after editing + // Shape Settings). The duplicate's pivot offset from its own bounds center must scale with the + // size of the duplicate being placed, not the size of the shape it was copied from. + [Test] + public void PreviewPivotPosition_ScalesWithCurrentBoundsSize_NotStaleDragSize() + { + var tool = DrawShapeTool.instance; + Assume.That(tool, Is.Not.Null); + + tool.m_PlaneRotation = Quaternion.identity; + + // Corner-to-center offset captured from a previously drawn 4x4x4 shape. + tool.m_LastNonDuplicateCenterToOrigin = new Vector3(-2f, -2f, -2f); + + // Bounds for the shape currently being previewed/duplicated: size has since been changed to 1x1x1. + tool.m_Bounds = new Bounds(new Vector3(5f, 0.5f, 5f), Vector3.one); + + var pivot = tool.previewPivotPosition; + var offset = pivot - tool.m_Bounds.center; + + Assert.That(offset.x, Is.EqualTo(-0.5f).Within(0.0001f)); + Assert.That(offset.y, Is.EqualTo(-0.5f).Within(0.0001f)); + Assert.That(offset.z, Is.EqualTo(-0.5f).Within(0.0001f)); + } +} diff --git a/Tests/Editor/Editor/DrawShapeToolPivotOffsetTests.cs.meta b/Tests/Editor/Editor/DrawShapeToolPivotOffsetTests.cs.meta new file mode 100644 index 000000000..2be35a860 --- /dev/null +++ b/Tests/Editor/Editor/DrawShapeToolPivotOffsetTests.cs.meta @@ -0,0 +1,2 @@ +fileFormatVersion: 2 +guid: 8d737f6e725194841a350eb63842ae47 \ No newline at end of file From b22ea7bbde77e03fcdc1df9ef80c1262f7855104 Mon Sep 17 00:00:00 2001 From: Thomas Lopez Date: Fri, 21 Aug 2026 11:35:37 -0400 Subject: [PATCH 2/5] Fix missing using directive for PivotLocation in pivot offset test --- Tests/Editor/Editor/DrawShapeToolPivotOffsetTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/Tests/Editor/Editor/DrawShapeToolPivotOffsetTests.cs b/Tests/Editor/Editor/DrawShapeToolPivotOffsetTests.cs index 08ebac70b..cf96b6eb1 100644 --- a/Tests/Editor/Editor/DrawShapeToolPivotOffsetTests.cs +++ b/Tests/Editor/Editor/DrawShapeToolPivotOffsetTests.cs @@ -3,6 +3,7 @@ using UnityEditor.EditorTools; using UnityEditor.ProBuilder; using UnityEngine; +using UnityEngine.ProBuilder; using ToolManager = UnityEditor.EditorTools.ToolManager; public class DrawShapeToolPivotOffsetTests From aa36ccd6e58f1df08de208dfeb798f0d5fe406e4 Mon Sep 17 00:00:00 2001 From: Thomas Lopez Date: Fri, 21 Aug 2026 14:47:20 -0400 Subject: [PATCH 3/5] [UUM-149791] Fix First Vertex pivot offset when resizing shapes or shift-duplicating ProBuilderShape.UpdateShape() rebuilt a resized shape around its stale center (valid only for the previous size) instead of the pivot corner, so resizing a shape via Shape Settings left the mesh floating away from its pivot. The first attempt at this re-derived the pivot corner from the sign of the stale center, but that breaks for shapes with a negative size axis (e.g. Stairs dragged backwards): center = pivot + size/2 holds regardless of size's sign, so a corner "sign" derived independently of size can end up flipped relative to it. Fixed by recording, per axis, the ratio of the pivot-to-center offset to the half-size (0 for Center, 1 for First Vertex - a value immune to size's sign) once at rebuild time, and re-deriving the center from that ratio and the current size on every resize. DrawShapeTool.previewPivotPosition had the same class of bug: it reconstructed the shift-duplicate placement offset from the sign of the last-drawn shape's recorded corner vector and that shape's own (possibly stale or differently-signed) size. Since a First Vertex pivot is always exactly bounds.center - size/2, that reconstruction is unnecessary - the fix uses the current preview bounds directly. --- Editor/EditorCore/DrawShapeTool.cs | 25 ++-------- Runtime/Shapes/ProBuilderShape.cs | 24 +++++++++- .../Editor/DrawShapeToolPivotOffsetTests.cs | 40 ++++++++++------ .../Editor/ProBuilderShapeResizePivotTests.cs | 47 +++++++++++++++++++ .../ProBuilderShapeResizePivotTests.cs.meta | 2 + 5 files changed, 102 insertions(+), 36 deletions(-) create mode 100644 Tests/Editor/Editor/ProBuilderShapeResizePivotTests.cs create mode 100644 Tests/Editor/Editor/ProBuilderShapeResizePivotTests.cs.meta diff --git a/Editor/EditorCore/DrawShapeTool.cs b/Editor/EditorCore/DrawShapeTool.cs index 38bd3099a..4549722d5 100644 --- a/Editor/EditorCore/DrawShapeTool.cs +++ b/Editor/EditorCore/DrawShapeTool.cs @@ -446,17 +446,11 @@ internal Vector3 previewPivotPosition { if (pivotLocation == PivotLocation.FirstVertex && instance != null) { - // Only the corner (sign per axis) of the last non-duplicate shape is reused here. - // The offset magnitude must come from the size of the shape currently being - // previewed/duplicated (m_Bounds.size), not from the size it was originally drawn at. - var lastCenterToOrigin = instance.m_LastNonDuplicateCenterToOrigin; - var cornerSign = new Vector3( - SignOrZero(lastCenterToOrigin.x), - SignOrZero(lastCenterToOrigin.y), - SignOrZero(lastCenterToOrigin.z)); - - var currentCenterToOrigin = Vector3.Scale(cornerSign, instance.m_Bounds.size * 0.5f); - var pivotOffset = instance.m_PlaneRotation * currentCenterToOrigin; + // A First Vertex pivot always sits at bounds.center - size/2: the sign of `size` + // itself already encodes which side the shape extends toward, so the size (and + // corner) of whatever shape was last drawn has no bearing on this - only the + // shape currently being previewed/duplicated (instance.m_Bounds) does. + var pivotOffset = instance.m_PlaneRotation * (instance.m_Bounds.size * -0.5f); return pivotOffset + instance.m_Bounds.center; } @@ -464,15 +458,6 @@ internal Vector3 previewPivotPosition } } - static float SignOrZero(float value) - { - if (value > 0f) - return 1f; - if (value < 0f) - return -1f; - return 0f; - } - int m_ControlID; internal float minSnapSize diff --git a/Runtime/Shapes/ProBuilderShape.cs b/Runtime/Shapes/ProBuilderShape.cs index 8855407de..061a3a046 100644 --- a/Runtime/Shapes/ProBuilderShape.cs +++ b/Runtime/Shapes/ProBuilderShape.cs @@ -69,6 +69,14 @@ public Bounds editionBounds [SerializeField] Vector3 m_LocalCenter; + + // Per axis, how far the pivot sits from the center as a fraction of the half-size: 0 for a + // Center pivot, 1 for a First Vertex pivot. Captured whenever the shape is fully rebuilt so + // that resizing (which only knows the new size, not what it was built with) can re-derive the + // center from the current size instead of keeping the previous size's stale center fixed. + [SerializeField] + Vector3 m_PivotRatio; + public Bounds shapeLocalBounds => new Bounds(m_LocalCenter, size); public Bounds shapeWorldBounds => new Bounds(shapeWorldCenter, size); @@ -104,7 +112,10 @@ internal void UpdateShape() if(gameObject == null || gameObject.hideFlags == HideFlags.HideAndDontSave) return; - Rebuild(mesh.transform.position, mesh.transform.rotation, new Bounds(shapeWorldCenter, size)); + var newLocalCenter = Vector3.Scale(m_PivotRatio, size * 0.5f); + var newWorldCenter = mesh.transform.TransformPoint(newLocalCenter); + + Rebuild(mesh.transform.position, mesh.transform.rotation, new Bounds(newWorldCenter, size)); } internal void UpdateBounds(Bounds bounds) @@ -121,10 +132,21 @@ internal void Rebuild(Vector3 pivotPosition, Quaternion rotation, Bounds bounds) Rebuild(); mesh.SetPivot(pivotPosition); m_LocalCenter = mesh.transform.InverseTransformPoint(bounds.center); + m_PivotRatio = new Vector3( + RatioOrZero(m_LocalCenter.x, bounds.size.x), + RatioOrZero(m_LocalCenter.y, bounds.size.y), + RatioOrZero(m_LocalCenter.z, bounds.size.z)); m_UnmodifiedMeshVersion = mesh.versionIndex; } + static float RatioOrZero(float localCenterComponent, float sizeComponent) + { + if (Mathf.Abs(sizeComponent) < 0.0001f) + return 0f; + return Mathf.Clamp(localCenterComponent / (sizeComponent * 0.5f), -1f, 1f); + } + internal void Rebuild(Bounds bounds, Quaternion rotation) { var trs = transform; diff --git a/Tests/Editor/Editor/DrawShapeToolPivotOffsetTests.cs b/Tests/Editor/Editor/DrawShapeToolPivotOffsetTests.cs index cf96b6eb1..8ffded3af 100644 --- a/Tests/Editor/Editor/DrawShapeToolPivotOffsetTests.cs +++ b/Tests/Editor/Editor/DrawShapeToolPivotOffsetTests.cs @@ -1,3 +1,4 @@ +using System.Collections.Generic; using NUnit.Framework; using UnityEditor; using UnityEditor.EditorTools; @@ -28,29 +29,38 @@ public void TearDown() ToolManager.RestorePreviousPersistentTool(); } - // Reproduces: create a shape with Pivot = First Vertex at one size (e.g. drag out a 4x4x4 cube), - // then place a duplicate (shift-click) at a different, smaller size (e.g. 1x1x1 after editing - // Shape Settings). The duplicate's pivot offset from its own bounds center must scale with the - // size of the duplicate being placed, not the size of the shape it was copied from. - [Test] - public void PreviewPivotPosition_ScalesWithCurrentBoundsSize_NotStaleDragSize() + // For a First Vertex pivot, the pivot always sits at bounds.center - size/2: the sign of `size` + // itself already encodes which side the shape extends toward, so the previous drag's recorded + // corner-to-center vector should have no bearing on where a same-or-different-sized duplicate's + // pivot lands. Both cases below place a duplicate of the same currentSize/expectedOffset, only + // the stale drag data differs, to prove that stale data can't leak into the result. + static IEnumerable PivotOffsetCases() + { + // Drawn previously as a 4x4x4 cube, now duplicating at 1x1x1 (Shape Settings edited down). + yield return new TestCaseData(new Vector3(-2f, -2f, -2f), Vector3.one, new Vector3(-0.5f, -0.5f, -0.5f)) + .SetName("PreviewPivotPosition_AllPositiveOldSize_ScalesToCurrentSize"); + + // Drawn previously as an all-negative-size shape (e.g. dragged backwards on every axis), now + // duplicating at 1x1x-1 (matching a Stairs-style shape with a negative Z size). + yield return new TestCaseData(new Vector3(2f, 2f, 2f), new Vector3(1f, 1f, -1f), new Vector3(-0.5f, -0.5f, 0.5f)) + .SetName("PreviewPivotPosition_NegativeAxisInOldSize_DoesNotFlipCurrentAxis"); + } + + [TestCaseSource(nameof(PivotOffsetCases))] + public void PreviewPivotPosition_ScalesWithCurrentBoundsSize_NotStaleDragSize(Vector3 lastCenterToOrigin, Vector3 currentSize, Vector3 expectedOffset) { var tool = DrawShapeTool.instance; Assume.That(tool, Is.Not.Null); tool.m_PlaneRotation = Quaternion.identity; - - // Corner-to-center offset captured from a previously drawn 4x4x4 shape. - tool.m_LastNonDuplicateCenterToOrigin = new Vector3(-2f, -2f, -2f); - - // Bounds for the shape currently being previewed/duplicated: size has since been changed to 1x1x1. - tool.m_Bounds = new Bounds(new Vector3(5f, 0.5f, 5f), Vector3.one); + tool.m_LastNonDuplicateCenterToOrigin = lastCenterToOrigin; + tool.m_Bounds = new Bounds(new Vector3(5f, 0.5f, 5f), currentSize); var pivot = tool.previewPivotPosition; var offset = pivot - tool.m_Bounds.center; - Assert.That(offset.x, Is.EqualTo(-0.5f).Within(0.0001f)); - Assert.That(offset.y, Is.EqualTo(-0.5f).Within(0.0001f)); - Assert.That(offset.z, Is.EqualTo(-0.5f).Within(0.0001f)); + Assert.That(offset.x, Is.EqualTo(expectedOffset.x).Within(0.0001f)); + Assert.That(offset.y, Is.EqualTo(expectedOffset.y).Within(0.0001f)); + Assert.That(offset.z, Is.EqualTo(expectedOffset.z).Within(0.0001f)); } } diff --git a/Tests/Editor/Editor/ProBuilderShapeResizePivotTests.cs b/Tests/Editor/Editor/ProBuilderShapeResizePivotTests.cs new file mode 100644 index 000000000..c484b6121 --- /dev/null +++ b/Tests/Editor/Editor/ProBuilderShapeResizePivotTests.cs @@ -0,0 +1,47 @@ +using NUnit.Framework; +using UnityEngine; +using UnityEngine.ProBuilder; +using UnityEngine.ProBuilder.Shapes; +using UObject = UnityEngine.Object; + +public class ProBuilderShapeResizePivotTests +{ + ProBuilderMesh m_PBMesh; + + [TearDown] + public void TearDown() + { + if (m_PBMesh != null) + UObject.DestroyImmediate(m_PBMesh.gameObject); + } + + // Reproduces: draw a shape with Pivot = First Vertex (pivot sits at a corner, not the center), + // then edit its size directly in Shape Settings (e.g. 2x2x2 -> 1x1x1). The pivot/gizmo must stay + // where it was, and the shape must be rebuilt against that same pivot corner at the new size - + // not left floating at the old size's center. Both cases resize to the same (1,1,1) target, so + // both must land on the exact same expected center - proving the old size's sign (e.g. a Stairs + // shape drawn with a negative Z size) can't leak into the new center's placement. + [TestCase(2f, 2f, 2f, TestName = "AllPositiveInitialSize")] + [TestCase(2f, 2f, -2f, TestName = "NegativeAxisInInitialSize")] + public void UpdateShape_AfterResizeWithFirstVertexPivot_KeepsShapeAnchoredAtPivot(float initialX, float initialY, float initialZ) + { + m_PBMesh = ShapeFactory.Instantiate(); + var shapeComponent = m_PBMesh.GetComponent(); + + var pivotPosition = Vector3.zero; + var initialSize = new Vector3(initialX, initialY, initialZ); + var initialBounds = new Bounds(pivotPosition + initialSize * 0.5f, initialSize); + shapeComponent.Rebuild(pivotPosition, Quaternion.identity, initialBounds); + + // Simulate editing the size field in the Shape Settings inspector down to 1x1x1. + shapeComponent.size = Vector3.one; + shapeComponent.UpdateShape(); + + Assert.That(shapeComponent.transform.position, Is.EqualTo(pivotPosition)); + + var expectedCenter = pivotPosition + Vector3.one * 0.5f; + Assert.That(shapeComponent.shapeWorldCenter.x, Is.EqualTo(expectedCenter.x).Within(0.0001f)); + Assert.That(shapeComponent.shapeWorldCenter.y, Is.EqualTo(expectedCenter.y).Within(0.0001f)); + Assert.That(shapeComponent.shapeWorldCenter.z, Is.EqualTo(expectedCenter.z).Within(0.0001f)); + } +} diff --git a/Tests/Editor/Editor/ProBuilderShapeResizePivotTests.cs.meta b/Tests/Editor/Editor/ProBuilderShapeResizePivotTests.cs.meta new file mode 100644 index 000000000..5f83d39cb --- /dev/null +++ b/Tests/Editor/Editor/ProBuilderShapeResizePivotTests.cs.meta @@ -0,0 +1,2 @@ +fileFormatVersion: 2 +guid: 1687fe7e7fdacaf4fb39640e9566a107 \ No newline at end of file From 45ec05730b29f7c8b895a4b718a0ce46615fa670 Mon Sep 17 00:00:00 2001 From: Thomas Lopez Date: Fri, 21 Aug 2026 15:29:23 -0400 Subject: [PATCH 4/5] [UUM-149791] Add changelog entry for shape pivot offset fix --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 520deb005..d6fc55c30 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Fixed +- [UUM-149791] Fixed the First Vertex pivot ending up offset from the shape after resizing an existing shape or shift-duplicating one with the Shape tool. - [UUM-148237] Fixed the "Lightmap UVs Settings" foldout in ProBuilder Preferences not opening when clicking its title. - [UUM-133528] Fixed an issue where using some UV Editor actions would clear a mesh's Lightmap UVs without regenerating them. - Fixed the Select Hidden (Select Back Faces) toggle icon missing from the Tool Settings overlay due to a filename casing mismatch. From a901f490aadf81c1f12281504edb789e3d8063bc Mon Sep 17 00:00:00 2001 From: Thomas Lopez Date: Fri, 21 Aug 2026 15:47:36 -0400 Subject: [PATCH 5/5] [UUM-149791] Add regression test for negative-drag-direction pivot placement Covers the reviewer-reported scenario directly: a shape drawn toward negative X (positive m_LastNonDuplicateCenterToOrigin.x, negative current size.x). Already passes - previewPivotPosition stopped using m_LastNonDuplicateCenterToOrigin entirely as of aa36ccd6e, in favor of computing the offset straight from the current (signed) bounds size. --- Tests/Editor/Editor/DrawShapeToolPivotOffsetTests.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Tests/Editor/Editor/DrawShapeToolPivotOffsetTests.cs b/Tests/Editor/Editor/DrawShapeToolPivotOffsetTests.cs index 8ffded3af..5370d951a 100644 --- a/Tests/Editor/Editor/DrawShapeToolPivotOffsetTests.cs +++ b/Tests/Editor/Editor/DrawShapeToolPivotOffsetTests.cs @@ -44,6 +44,13 @@ static IEnumerable PivotOffsetCases() // duplicating at 1x1x-1 (matching a Stairs-style shape with a negative Z size). yield return new TestCaseData(new Vector3(2f, 2f, 2f), new Vector3(1f, 1f, -1f), new Vector3(-0.5f, -0.5f, 0.5f)) .SetName("PreviewPivotPosition_NegativeAxisInOldSize_DoesNotFlipCurrentAxis"); + + // Drawn toward negative X (size.x negative), then duplicated unchanged: this gives a + // *positive* m_LastNonDuplicateCenterToOrigin.x alongside a *negative* current size.x - the + // exact combination that reverses the pivot onto the wrong corner if that stale vector's sign + // is multiplied against the current (still negative) size instead of driving off size alone. + yield return new TestCaseData(new Vector3(1.5f, -0.5f, -0.5f), new Vector3(-3f, 1f, 1f), new Vector3(1.5f, -0.5f, -0.5f)) + .SetName("PreviewPivotPosition_NegativeDragDirection_DoesNotReversePivotCorner"); } [TestCaseSource(nameof(PivotOffsetCases))]