diff --git a/CHANGELOG.md b/CHANGELOG.md index 49d845d7d..dd91fb38b 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-150702] Fixed the ProBuilderDefault and Checker materials referencing a stale shader ID, causing ProBuilder meshes to render magenta when a scene was opened. - [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. diff --git a/Editor/EditorCore/DrawShapeTool.cs b/Editor/EditorCore/DrawShapeTool.cs index 7b532be53..4549722d5 100644 --- a/Editor/EditorCore/DrawShapeTool.cs +++ b/Editor/EditorCore/DrawShapeTool.cs @@ -446,13 +446,11 @@ internal Vector3 previewPivotPosition { if (pivotLocation == PivotLocation.FirstVertex && instance != null) { - var lastCenterToOrigin = instance.m_LastNonDuplicateCenterToOrigin; - var lastCenterToOriginNorm = lastCenterToOrigin.normalized; - - var deltaRot = instance.m_PlaneRotation; - lastCenterToOriginNorm = deltaRot * lastCenterToOriginNorm; - - var pivotOffset = lastCenterToOriginNorm * lastCenterToOrigin.magnitude; + // 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; } 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 new file mode 100644 index 000000000..5370d951a --- /dev/null +++ b/Tests/Editor/Editor/DrawShapeToolPivotOffsetTests.cs @@ -0,0 +1,73 @@ +using System.Collections.Generic; +using NUnit.Framework; +using UnityEditor; +using UnityEditor.EditorTools; +using UnityEditor.ProBuilder; +using UnityEngine; +using UnityEngine.ProBuilder; +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(); + } + + // 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"); + + // 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))] + 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; + 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(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/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 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