Fixed issue with edge selection. (#625)

varinotmUnity · web-flow · commit 42261481ddd8 · 2025-09-18T10:36:20.000-04:00
* Fixed issue with edge selection.

* add changelog

* Update following PR comments
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 ### Fixed
 
 - [PBLD-240] Fixed a bug where buttons for "Create Cube" and "Create PolyShape" appeared incorrectly on Light theme.
+- [PBLD-258] Fixed an bug where clicking a highlighted edge might select a hidden edge instead.
 
 ## [6.0.7] - 2025-08-28
 
diff --git a/Editor/EditorCore/EditorSceneViewPicker.cs b/Editor/EditorCore/EditorSceneViewPicker.cs
@@ -639,13 +639,38 @@ static float EdgeRaycast(Vector3 mousePosition, ScenePickerPreferences pickerPre
             bool highlightedEdgeExists = ProBuilderEditor.instance.hovering.edges.Count > 0;
 
             // if there is an edge already highlighted, we don't want to select a different game object
-            if (!highlightedEdgeExists)
+            if (highlightedEdgeExists)
             {
-                selection.gameObject = HandleUtility.PickGameObject(mousePosition, false);
+                ProBuilderMesh hoveredMeshFromHighlight = ProBuilderEditor.instance.hovering.mesh;
+                if (hoveredMeshFromHighlight != null && ProBuilderEditor.instance.hovering.edges.Count > 0)
+                {
+                    Edge highlightedEdge = ProBuilderEditor.instance.hovering.edges[0]; // Assuming single edge hover
+
+                    // Calculate the screen distance for this specific highlighted edge.
+                    Vector3[] positions = hoveredMeshFromHighlight.positionsInternal;
+                    Vector3 worldPosA = hoveredMeshFromHighlight.transform.TransformPoint(positions[highlightedEdge.a]);
+                    Vector3 worldPosB = hoveredMeshFromHighlight.transform.TransformPoint(positions[highlightedEdge.b]);
+
+                    if (ProcessEdgePoints(Camera.current, worldPosA, worldPosB, out Vector3 guiPointA, out Vector3 guiPointB))
+                    {
+                        float distToHighlightedEdge = HandleUtility.DistancePointLine(mousePosition, guiPointA, guiPointB);
+
+                        // If the highlighted edge is within the maximum picking distance, select it and return early.
+                        if (distToHighlightedEdge <= ScenePickerPreferences.maxPointerDistance)
+                        {
+                            selection.gameObject = hoveredMeshFromHighlight.gameObject;
+                            selection.mesh = hoveredMeshFromHighlight;
+                            selection.SetSingleEdge(highlightedEdge);
+                            return distToHighlightedEdge;
+                        }
+                    }
+                }
             }
-            var hoveredMesh = selection.gameObject != null ? selection.gameObject.GetComponent<ProBuilderMesh>() : null;
 
             float bestDistance = Mathf.Infinity;
+            selection.gameObject = HandleUtility.PickGameObject(mousePosition, false);
+            var hoveredMesh = selection.gameObject != null ? selection.gameObject.GetComponent<ProBuilderMesh>() : null;
+
             bool hoveredIsInSelection = MeshSelection.topInternal.Contains(hoveredMesh);
 
             if (hoveredMesh != null && (allowUnselected || hoveredIsInSelection))
@@ -659,7 +684,8 @@ static float EdgeRaycast(Vector3 mousePosition, ScenePickerPreferences pickerPre
                     selection.SetSingleEdge(tup.edge);
                     bestDistance = tup.distance;
 
-                    // If the nearest edge was acquired by a raycast, then the distance to mesh is 0f.
+                    // If the nearest edge was acquired by a raycast on an already selected mesh,
+                    // return early to prioritize it.
                     if (hoveredIsInSelection)
                         return tup.distance;
                 }
diff --git a/Tests/Editor/Picking/EdgePickerTests.cs b/Tests/Editor/Picking/EdgePickerTests.cs
@@ -1,4 +1,5 @@
 using System;
+using System.Collections;
 using System.Linq;
 using UnityEngine;
 using UObject = UnityEngine.Object;
@@ -25,13 +26,15 @@ public void Setup()
         window.Focus();
 
         m_Camera = new GameObject("TestCamera", typeof(Camera)).GetComponent<Camera>();
-        m_Camera.transform.position = new Vector3(0, 0, -10);
+        m_Camera.transform.position = new Vector3(0, 3, 0);
         m_Camera.transform.LookAt(Vector3.zero);
         m_Camera.orthographic = false;
         m_Camera.cullingMask = ~0;
+        m_Camera.fieldOfView = 60f;
+        MatchSceneViewToCamera(m_Camera);
 
-        m_Mesh = ShapeFactory.Instantiate<UnityEngine.ProBuilder.Shapes.Plane>();
-        m_Mesh.name = "TestPlane";
+        m_Mesh = ShapeFactory.Instantiate<UnityEngine.ProBuilder.Shapes.Cube>();
+        m_Mesh.name = "TestCube";
         m_Mesh.transform.position = Vector3.zero;
         m_Mesh.transform.rotation = Quaternion.identity;
         m_Mesh.Refresh();
@@ -83,23 +86,28 @@ private Event CreateMouseEvent(Vector3 mousePosition, EventType type = EventType
         return evt;
     }
 
-    [Test]
-    public void EdgePicker_PicksVisibleEdge()
+    [UnityTest]
+    public IEnumerator EdgePicker_PicksVisibleEdge()
     {
-        Edge edgeToPick = m_Mesh.facesInternal[0].edgesInternal[0];
+        Edge edgeToPick = m_Mesh.facesInternal[4].edgesInternal[0];
 
         Vector3 pA_world = m_Mesh.transform.TransformPoint(m_Mesh.positionsInternal[edgeToPick.a]);
         Vector3 pB_world = m_Mesh.transform.TransformPoint(m_Mesh.positionsInternal[edgeToPick.b]);
 
         Vector3 centerOfEdge_world = (pA_world + pB_world) / 2f;
         Vector2 mousePos = UnityEditor.HandleUtility.WorldToGUIPoint(centerOfEdge_world);
 
+        yield return null;
+
         UnityEngine.TestTools.LogAssert.ignoreFailingMessages = true;
         EditorSceneViewPicker.DoMouseClick(
             CreateMouseEvent(mousePos, EventType.MouseDown, EventModifiers.None),
             SelectMode.Edge,
             m_PickerPreferences);
 
+
+        yield return null;
+
         SceneSelection currentSelection = EditorSceneViewPicker.selection;
 
         Assert.IsNotNull(currentSelection.mesh, "A mesh should be selected.");
@@ -109,8 +117,39 @@ public void EdgePicker_PicksVisibleEdge()
         Assert.AreEqual(edgeToPick, currentSelection.edges.First(), "The expected edge should be picked.");
     }
 
-    [Test]
-    public void EdgePicker_DoesNotPickWhenNotHovering()
+    [UnityTest]
+    public IEnumerator EdgePicker_DoesntPickHiddenEdge()
+    {
+        Edge edgeToPick = m_Mesh.facesInternal[0].edgesInternal[0];
+
+        Vector3 pA_world = m_Mesh.transform.TransformPoint(m_Mesh.positionsInternal[edgeToPick.a]);
+        Vector3 pB_world = m_Mesh.transform.TransformPoint(m_Mesh.positionsInternal[edgeToPick.b]);
+
+        Vector3 centerOfEdge_world = (pA_world + pB_world) / 2f;
+        Vector2 mousePos = UnityEditor.HandleUtility.WorldToGUIPoint(centerOfEdge_world);
+
+        yield return null;
+
+        UnityEngine.TestTools.LogAssert.ignoreFailingMessages = true;
+        EditorSceneViewPicker.DoMouseClick(
+            CreateMouseEvent(mousePos, EventType.MouseDown, EventModifiers.None),
+            SelectMode.Edge,
+            m_PickerPreferences);
+
+
+        yield return null;
+
+        SceneSelection currentSelection = EditorSceneViewPicker.selection;
+
+        Assert.IsNotNull(currentSelection.mesh, "A mesh should be selected.");
+        Assert.AreEqual(m_Mesh, currentSelection.mesh, "The correct mesh should be selected.");
+        Assert.IsNotNull(currentSelection.edges, "Edges collection should not be null.");
+        Assert.AreEqual(1, currentSelection.edges.Count, "Exactly one edge should be selected.");
+        Assert.AreEqual(m_Mesh.facesInternal[4].edgesInternal[0], currentSelection.edges.First(), "The expected edge should be picked.");
+    }
+
+    [UnityTest]
+    public IEnumerator EdgePicker_DoesNotPickWhenNotHovering()
     {
         Vector2 mousePos = new Vector2(Screen.width / 2f, Screen.height / 2f + 500f);
 
@@ -120,31 +159,32 @@ public void EdgePicker_DoesNotPickWhenNotHovering()
             SelectMode.Edge,
             m_PickerPreferences);
 
+        yield return null;
+
         SceneSelection currentSelection = EditorSceneViewPicker.selection;
 
         Assert.IsNull(currentSelection.mesh, "No mesh should be selected.");
         Assert.IsEmpty(currentSelection.edges, "No edges should be selected.");
     }
 
-    [Test]
-    public void EdgePicker_PicksCorrectEdgeWithMultipleOverlapping()
+    [UnityTest]
+    public IEnumerator EdgePicker_PicksCorrectEdgeWithMultipleOverlapping()
     {
-        m_Camera.transform.position = new Vector3(0.5f, 0.5f, -10);
-        m_Camera.transform.LookAt(Vector3.zero);
+        MatchSceneViewToCamera(m_Camera);
 
-        ProBuilderMesh mesh2 = ShapeFactory.Instantiate<UnityEngine.ProBuilder.Shapes.Plane>();
-        mesh2.name = "TestPlane2";
-        mesh2.transform.position = new Vector3(0.1f, 0.1f, 0.5f);
+        ProBuilderMesh mesh2 = ShapeFactory.Instantiate<UnityEngine.ProBuilder.Shapes.Cube>();
+        mesh2.name = "TestCube2";
+        mesh2.transform.position = new Vector3(0f, 1f, 0f);
         mesh2.Refresh();
 
         var meshCollider2 = mesh2.gameObject.GetComponent<MeshCollider>();
         if (meshCollider2 == null) meshCollider2 = mesh2.gameObject.AddComponent<MeshCollider>();
         meshCollider2.sharedMesh = mesh2.mesh;
         meshCollider2.enabled = true;
 
-        MeshSelection.AddToSelection(mesh2.gameObject);
+        yield return null;
 
-        Edge edge1 = m_Mesh.facesInternal[0].edgesInternal[0];
+        Edge edge1 = m_Mesh.facesInternal[0].edgesInternal[3];
         Vector3 pA1_world = m_Mesh.transform.TransformPoint(m_Mesh.positionsInternal[edge1.a]);
         Vector3 pB1_world = m_Mesh.transform.TransformPoint(m_Mesh.positionsInternal[edge1.b]);
         Vector3 center1_world = (pA1_world + pB1_world) / 2f;
@@ -157,6 +197,8 @@ public void EdgePicker_PicksCorrectEdgeWithMultipleOverlapping()
             SelectMode.Edge,
             m_PickerPreferences);
 
+        yield return null;
+
         SceneSelection currentSelection = EditorSceneViewPicker.selection;
 
         Assert.IsNotNull(currentSelection.mesh, "An edge should be picked.");
@@ -168,49 +210,51 @@ public void EdgePicker_PicksCorrectEdgeWithMultipleOverlapping()
         UObject.DestroyImmediate(mesh2.gameObject);
     }
 
-    [Test]
-    public void EdgePicker_PicksClippedEdge_OnePointBehindCamera()
+    [UnityTest]
+    public IEnumerator EdgePicker_PicksClippedEdge_OnePointBehindCamera()
     {
-
-        m_Camera.nearClipPlane = 0.1f;
-        m_Camera.transform.position = new Vector3(0, 0, -0.5f);
-        m_Camera.transform.LookAt(Vector3.zero);
-
-        Edge targetEdge = m_Mesh.facesInternal[0].edgesInternal[0];
-
-        Vector3[] currentLocalPositions = m_Mesh.positionsInternal;
-        currentLocalPositions[targetEdge.a] = new Vector3(currentLocalPositions[targetEdge.a].x, currentLocalPositions[targetEdge.a].y, -0.6f);
-
+        m_Mesh.transform.position = new Vector3(0, 0, 0);
+        m_Mesh.transform.localScale = new Vector3(1, 30, 1);
         m_Mesh.Rebuild();
 
         m_Mesh.GetComponent<MeshCollider>().sharedMesh = m_Mesh.mesh;
         m_Mesh.GetComponent<MeshCollider>().enabled = true;
 
-        Vector3 testPointOnClippedEdge_world = m_Mesh.transform.TransformPoint(-0.5f, 0, -0.3f);
-        Vector2 mousePos = UnityEditor.HandleUtility.WorldToGUIPoint(testPointOnClippedEdge_world);
+        yield return null;
+
+        Edge edgeToTest = m_Mesh.facesInternal[3].edgesInternal[2];
+        Vector3 pA_world = m_Mesh.transform.TransformPoint(m_Mesh.positionsInternal[edgeToTest.a]);
+        Vector3 pB_world = m_Mesh.transform.TransformPoint(m_Mesh.positionsInternal[edgeToTest.b]);
+
+        Vector3 centerOfEdge_world = (pA_world + pB_world) / 2f;
+        Vector2 mousePos = UnityEditor.HandleUtility.WorldToGUIPoint(centerOfEdge_world);
 
         UnityEngine.TestTools.LogAssert.ignoreFailingMessages = true;
         EditorSceneViewPicker.DoMouseClick(
             CreateMouseEvent(mousePos, EventType.MouseDown, EventModifiers.None),
             SelectMode.Edge,
             m_PickerPreferences);
 
+        yield return null;
+
         SceneSelection currentSelection = EditorSceneViewPicker.selection;
 
         Assert.IsNotNull(currentSelection.mesh, "Clipped edge should be picked.");
         Assert.AreEqual(m_Mesh, currentSelection.mesh, "The correct mesh should be selected.");
         Assert.IsNotNull(currentSelection.edges, "Edges collection should not be null.");
         Assert.AreEqual(1, currentSelection.edges.Count, "Exactly one edge should be selected.");
-        Assert.AreEqual(targetEdge, currentSelection.edges.First(), "The expected clipped edge should be picked.");
+        Assert.AreEqual(edgeToTest, currentSelection.edges.First(), "The expected clipped edge should be picked.");
     }
 
-    [Test]
-    public void EdgePicker_DoesNotPickEdge_BothPointsBehindCamera()
+    [UnityTest]
+    public IEnumerator EdgePicker_DoesNotPickEdge_BothPointsBehindCamera()
     {
         m_Camera.nearClipPlane = 0.1f;
         m_Camera.transform.position = new Vector3(0, 0, -0.5f);
         m_Camera.transform.LookAt(Vector3.zero);
 
+        MatchSceneViewToCamera(m_Camera);
+
         m_Mesh.transform.position = new Vector3(0, 0, -1.0f);
         m_Mesh.Refresh();
 
@@ -224,16 +268,36 @@ public void EdgePicker_DoesNotPickEdge_BothPointsBehindCamera()
         Vector3 centerOfEdge_world = (pA_world + pB_world) / 2f;
         Vector2 mousePos = UnityEditor.HandleUtility.WorldToGUIPoint(centerOfEdge_world);
 
+        yield return null;
+
         UnityEngine.TestTools.LogAssert.Expect("Handles.GetClosestPickingID called outside an editor OnGUI");
         UnityEngine.TestTools.LogAssert.Expect("Assertion failed on expression: 'device.IsInsideFrame()'");
         EditorSceneViewPicker.DoMouseClick(
             CreateMouseEvent(mousePos, EventType.MouseDown, EventModifiers.None),
             SelectMode.Edge,
             m_PickerPreferences);
 
+        yield return null;
+
         SceneSelection currentSelection = EditorSceneViewPicker.selection;
 
         Assert.IsNull(currentSelection.mesh, "No mesh should be selected.");
         Assert.IsEmpty(currentSelection.edges, "No edges should be selected.");
     }
+
+    private void MatchSceneViewToCamera(Camera cam)
+    {
+        var sceneView = SceneView.lastActiveSceneView;
+        if (sceneView == null)
+            sceneView = EditorWindow.GetWindow<SceneView>();
+
+        sceneView.in2DMode = false;
+        sceneView.orthographic = cam.orthographic;
+        sceneView.size = cam.orthographicSize;
+        sceneView.pivot = cam.transform.position + cam.transform.forward * 10f; // pivot is center of view
+        sceneView.rotation = cam.transform.rotation;
+        sceneView.cameraSettings.fieldOfView = cam.fieldOfView;
+
+        sceneView.Repaint();
+    }
 }
