Skip to content

Commit 5a17425

Browse files
author
varinotmUnity
committed
Fixed issue when selecting a face that was grouped in the window, and then moving it. this was due to bad handle position set, which would then overwrite the uv center badly. this only happens in the editor, not with the API
1 parent 56d0738 commit 5a17425

File tree

2 files changed

+195
-11
lines changed

2 files changed

+195
-11
lines changed

Editor/EditorCore/UVEditor.cs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -92,14 +92,11 @@ static Color DRAG_BOX_COLOR
9292
static readonly Color SELECTED_COLOR_MANUAL = new Color(1f, .68f, 0f, .39f);
9393
static readonly Color SELECTED_COLOR_AUTO = new Color(0f, .785f, 1f, .39f);
9494

95-
#if UNITY_STANDALONE_OSX
96-
public bool ControlKey { get { return Event.current.modifiers == EventModifiers.Command; } }
97-
#else
9895
public bool ControlKey
9996
{
100-
get { return Event.current.modifiers == EventModifiers.Control; }
97+
get { return EditorGUI.actionKey; }
10198
}
102-
#endif
99+
103100
public bool ShiftKey
104101
{
105102
get { return Event.current.modifiers == EventModifiers.Shift; }
@@ -547,13 +544,17 @@ internal void OnBeginUVModification()
547544
GUI.FocusControl(string.Empty);
548545
bool update = false;
549546

547+
Vector2 originalHandlePosition = handlePosition;
548+
550549
// Make sure all TextureGroups are auto-selected
551550
for (int i = 0; i < selection.Length; i++)
552551
{
553552
if (selection[i].selectedFaceCount > 0)
554553
{
555554
int fc = selection[i].selectedFaceCount;
556-
selection[i].SetSelectedFaces(SelectTextureGroups(selection[i], selection[i].selectedFacesInternal));
555+
selection[i].SetSelectedFaces(
556+
SelectTextureGroups(selection[i], selection[i].selectedFacesInternal)
557+
);
557558

558559
// kinda lame... this will cause setSelectedUVsWithSceneView to be called again.
559560
if (fc != selection[i].selectedFaceCount)
@@ -567,9 +568,8 @@ internal void OnBeginUVModification()
567568
if (update)
568569
{
569570
// UpdateSelection clears handlePosition
570-
Vector2 storedHandlePosition = handlePosition;
571571
ProBuilderEditor.Refresh();
572-
SetHandlePosition(storedHandlePosition, true);
572+
SetHandlePosition(originalHandlePosition, true);
573573
}
574574

575575
CopySelectionUVs(out uv_origins);

Tests/Editor/Editor/UVEditorTest.cs

Lines changed: 187 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,8 @@
99
using UnityEngine.ProBuilder.Shapes;
1010
using EditorUtility = UnityEditor.ProBuilder.EditorUtility;
1111

12-
public class UVEditorWindow
12+
[TestFixture]
13+
public class UVEditorTests
1314
{
1415
ProBuilderMesh m_cube;
1516

@@ -18,7 +19,7 @@ public void Setup()
1819
{
1920
m_cube = ShapeFactory.Instantiate<Cube>();
2021
EditorUtility.InitObject(m_cube);
21-
// Unsure UV bounds origin is not at (0,0) lower left
22+
// Unsure UV bounds origin is not at (0,0) lower left
2223
foreach (var face in m_cube.facesInternal)
2324
face.uv = new AutoUnwrapSettings(face.uv) { anchor = AutoUnwrapSettings.Anchor.UpperLeft, offset = new Vector2(-0.5f, -0.5f) };
2425
m_cube.RefreshUV(m_cube.faces);
@@ -31,8 +32,29 @@ public void Setup()
3132
[TearDown]
3233
public void Cleanup()
3334
{
35+
// Close the UV Editor window first
36+
if (UVEditor.instance != null)
37+
{
38+
UVEditor.instance.Close();
39+
}
40+
41+
// Clear ProBuilder selections
42+
MeshSelection.ClearElementSelection();
43+
Selection.activeGameObject = null;
44+
45+
// Reset tool context
3446
ToolManager.SetActiveContext<GameObjectToolContext>();
35-
UObject.DestroyImmediate(m_cube.gameObject);
47+
48+
// Destroy the cube
49+
if (m_cube != null && m_cube.gameObject != null)
50+
{
51+
UObject.DestroyImmediate(m_cube.gameObject);
52+
}
53+
54+
m_cube = null;
55+
56+
// Clear undo to prevent resurrection
57+
Undo.ClearAll();
3658
}
3759

3860
[Test]
@@ -103,4 +125,166 @@ public void Manual_PlanarProjection()
103125
minimalUV = UVEditor.instance.UVSelectionMinimalUV();
104126
Assert.That(minimalUV, Is.EqualTo(UVEditor.LowerLeft));
105127
}
128+
129+
/// <summary>
130+
/// Test that moving a single unconnected face doesn't cause distortion
131+
/// </summary>
132+
[Test]
133+
public void MoveSingleFace_PreservesRelativePositions()
134+
{
135+
// Setup: One face in manual UV mode, NOT in a texture group
136+
var face0 = m_cube.facesInternal[0];
137+
138+
face0.manualUV = false;
139+
face0.textureGroup = -1; // No texture group (isolated face)
140+
141+
m_cube.ToMesh();
142+
m_cube.Refresh();
143+
144+
// Select the face
145+
MeshSelection.SetSelection(m_cube.gameObject);
146+
m_cube.SetSelectedFaces(new Face[] { face0 });
147+
MeshSelection.OnObjectSelectionChanged();
148+
149+
// Capture initial UV positions
150+
var face0InitialUVs = UnityEngine.ProBuilder.ArrayUtility.ValuesWithIndexes(
151+
m_cube.texturesInternal, face0.distinctIndexesInternal);
152+
153+
// Calculate initial relative offsets within the face
154+
Vector2[] face0InitialOffsets = new Vector2[face0InitialUVs.Length];
155+
for (int i = 0; i < face0InitialUVs.Length; i++)
156+
face0InitialOffsets[i] = face0InitialUVs[i] - face0InitialUVs[0];
157+
158+
// Simulate a move operation
159+
Vector2 moveDelta = new Vector2(0.1f, 0.2f);
160+
UVEditor.instance.SceneMoveTool(moveDelta);
161+
162+
// Get final UV positions
163+
var face0FinalUVs = UnityEngine.ProBuilder.ArrayUtility.ValuesWithIndexes(
164+
m_cube.texturesInternal, face0.distinctIndexesInternal);
165+
166+
// TEST 1: Verify each vertex moved by the delta
167+
for (int i = 0; i < face0InitialUVs.Length; i++)
168+
{
169+
Assert.That(face0FinalUVs[i].x, Is.EqualTo(face0InitialUVs[i].x + moveDelta.x).Within(0.0001f),
170+
$"Vertex {i} X should move by delta");
171+
Assert.That(face0FinalUVs[i].y, Is.EqualTo(face0InitialUVs[i].y + moveDelta.y).Within(0.0001f),
172+
$"Vertex {i} Y should move by delta");
173+
}
174+
175+
// TEST 2: Verify relative offsets within the face are preserved (no distortion)
176+
for (int i = 0; i < face0FinalUVs.Length; i++)
177+
{
178+
Vector2 finalOffset = face0FinalUVs[i] - face0FinalUVs[0];
179+
Assert.That(finalOffset.x, Is.EqualTo(face0InitialOffsets[i].x).Within(0.0001f),
180+
$"Vertex {i} relative X offset changed - face was distorted!");
181+
Assert.That(finalOffset.y, Is.EqualTo(face0InitialOffsets[i].y).Within(0.0001f),
182+
$"Vertex {i} relative Y offset changed - face was distorted!");
183+
}
184+
185+
// Cleanup
186+
UVEditor.instance.OnFinishUVModification();
187+
}
188+
189+
[Test]
190+
public void MoveConnectedFaces_PreservesRelativePositions()
191+
{
192+
// Setup: Two faces in the same texture group (Auto UV mode)
193+
var face0 = m_cube.facesInternal[0];
194+
var face1 = m_cube.facesInternal[1];
195+
196+
face0.manualUV = false;
197+
face1.manualUV = false;
198+
face0.textureGroup = 1;
199+
face1.textureGroup = 1;
200+
201+
m_cube.ToMesh();
202+
m_cube.Refresh();
203+
204+
// Select ONLY face0
205+
MeshSelection.SetSelection(m_cube.gameObject);
206+
m_cube.SetSelectedFaces(new Face[] { face0 });
207+
MeshSelection.OnObjectSelectionChanged();
208+
209+
// Capture initial UV positions of BOTH faces
210+
var face0InitialUVs = UnityEngine.ProBuilder.ArrayUtility.ValuesWithIndexes(
211+
m_cube.texturesInternal, face0.distinctIndexesInternal);
212+
var face1InitialUVs = UnityEngine.ProBuilder.ArrayUtility.ValuesWithIndexes(
213+
m_cube.texturesInternal, face1.distinctIndexesInternal);
214+
215+
// Calculate initial relative offsets within each face
216+
Vector2[] face0InitialOffsets = new Vector2[face0InitialUVs.Length];
217+
for (int i = 0; i < face0InitialUVs.Length; i++)
218+
face0InitialOffsets[i] = face0InitialUVs[i] - face0InitialUVs[0];
219+
220+
Vector2[] face1InitialOffsets = new Vector2[face1InitialUVs.Length];
221+
for (int i = 0; i < face1InitialUVs.Length; i++)
222+
face1InitialOffsets[i] = face1InitialUVs[i] - face1InitialUVs[0];
223+
224+
// Calculate initial distance between the two faces
225+
Vector2 face0InitialCenter = Bounds2D.Center(face0InitialUVs);
226+
Vector2 face1InitialCenter = Bounds2D.Center(face1InitialUVs);
227+
Vector2 initialCenterDistance = face1InitialCenter - face0InitialCenter;
228+
229+
// Simulate a move operation
230+
Vector2 moveDelta = new Vector2(0.1f, 0.2f);
231+
UVEditor.instance.SceneMoveTool(moveDelta);
232+
233+
// Get final UV positions
234+
var face0FinalUVs = UnityEngine.ProBuilder.ArrayUtility.ValuesWithIndexes(
235+
m_cube.texturesInternal, face0.distinctIndexesInternal);
236+
var face1FinalUVs = UnityEngine.ProBuilder.ArrayUtility.ValuesWithIndexes(
237+
m_cube.texturesInternal, face1.distinctIndexesInternal);
238+
239+
// TEST 1: Verify each vertex in face0 moved by the delta
240+
for (int i = 0; i < face0InitialUVs.Length; i++)
241+
{
242+
Assert.That(face0FinalUVs[i].x, Is.EqualTo(face0InitialUVs[i].x + moveDelta.x).Within(0.0001f),
243+
$"Face0 vertex {i} X should move by delta");
244+
Assert.That(face0FinalUVs[i].y, Is.EqualTo(face0InitialUVs[i].y + moveDelta.y).Within(0.0001f),
245+
$"Face0 vertex {i} Y should move by delta");
246+
}
247+
248+
// TEST 2: Verify each vertex in face1 moved by the delta (auto-selected)
249+
for (int i = 0; i < face1InitialUVs.Length; i++)
250+
{
251+
Assert.That(face1FinalUVs[i].x, Is.EqualTo(face1InitialUVs[i].x + moveDelta.x).Within(0.0001f),
252+
$"Face1 vertex {i} X should move by delta");
253+
Assert.That(face1FinalUVs[i].y, Is.EqualTo(face1InitialUVs[i].y + moveDelta.y).Within(0.0001f),
254+
$"Face1 vertex {i} Y should move by delta");
255+
}
256+
257+
// TEST 3: Verify relative offsets within face0 are preserved (no distortion)
258+
for (int i = 0; i < face0FinalUVs.Length; i++)
259+
{
260+
Vector2 finalOffset = face0FinalUVs[i] - face0FinalUVs[0];
261+
Assert.That(finalOffset.x, Is.EqualTo(face0InitialOffsets[i].x).Within(0.0001f),
262+
$"Face0 vertex {i} relative X offset changed - face was distorted!");
263+
Assert.That(finalOffset.y, Is.EqualTo(face0InitialOffsets[i].y).Within(0.0001f),
264+
$"Face0 vertex {i} relative Y offset changed - face was distorted!");
265+
}
266+
267+
// TEST 4: Verify relative offsets within face1 are preserved (no distortion)
268+
for (int i = 0; i < face1FinalUVs.Length; i++)
269+
{
270+
Vector2 finalOffset = face1FinalUVs[i] - face1FinalUVs[0];
271+
Assert.That(finalOffset.x, Is.EqualTo(face1InitialOffsets[i].x).Within(0.0001f),
272+
$"Face1 vertex {i} relative X offset changed - face was distorted!");
273+
Assert.That(finalOffset.y, Is.EqualTo(face1InitialOffsets[i].y).Within(0.0001f),
274+
$"Face1 vertex {i} relative Y offset changed - face was distorted!");
275+
}
276+
277+
// TEST 5: Verify the distance between face centers is preserved
278+
Vector2 face0FinalCenter = Bounds2D.Center(face0FinalUVs);
279+
Vector2 face1FinalCenter = Bounds2D.Center(face1FinalUVs);
280+
Vector2 finalCenterDistance = face1FinalCenter - face0FinalCenter;
281+
282+
Assert.That(finalCenterDistance.x, Is.EqualTo(initialCenterDistance.x).Within(0.0001f),
283+
"Distance between face centers X changed - faces were recentered!");
284+
Assert.That(finalCenterDistance.y, Is.EqualTo(initialCenterDistance.y).Within(0.0001f),
285+
"Distance between face centers Y changed - faces were recentered!");
286+
287+
// Cleanup
288+
UVEditor.instance.OnFinishUVModification();
289+
}
106290
}

0 commit comments

Comments
 (0)