FIX: PointerEventData.pointerId is the same when simultaneously releasing and then pressing with another finger (ISXB-845) (#2033)

jfreire-unity · web-flow · commit c52fff9a4a58 · 2024-10-24T18:29:21.000+02:00
* Check if pointer state touchID matches cached TouchControl touchID

Covers the edge case of releasing one finger and pressing with another in the same frame.

* Add unit test to avoid regression

* Add pointerId to the asserts

* Update CHANGELOG.md

* Apply changes based on review

* Get pointer state index based on pointerIds instead of touch controls cache

The commit also:
- Expands unit test case
- Removes m_PointerTouchControls as they were not being used anymore

* Fix compiling error

* Fix again compile errors

* Fix CHANGELOG.md

* Removed wrong changelog entry
diff --git a/Assets/Tests/InputSystem/Plugins/UITests.cs b/Assets/Tests/InputSystem/Plugins/UITests.cs
@@ -1693,6 +1693,201 @@ public IEnumerator UI_CanDriveUIFromMultipleTouches()
         Assert.That(scene.leftChildReceiver.events, Is.Empty);
     }
 
+    // https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-845
+    // This tests that we can release and press touches on the same frame with the expected events and touchIds.
+    [UnityTest]
+    [Category("UI")]
+    public IEnumerator UI_CanReleaseAndPressTouchesOnSameFrame()
+    {
+        var touchScreen = InputSystem.AddDevice<Touchscreen>();
+
+        // Prevent default selection of left object. This means that we will not have to contend with selections at all
+        // in this test as they are driven from UI objects and not by the input module itself.
+        var scene = CreateTestUI(noFirstSelected: true);
+
+        var asset = ScriptableObject.CreateInstance<InputActionAsset>();
+        var map = asset.AddActionMap("map");
+        var pointAction = map.AddAction("point", type: InputActionType.PassThrough, binding: "<Touchscreen>/touch*/position");
+        var leftClickAction = map.AddAction("leftClick", type: InputActionType.PassThrough, binding: "<Touchscreen>/touch*/press");
+
+        scene.uiModule.point = InputActionReference.Create(pointAction);
+        scene.uiModule.leftClick = InputActionReference.Create(leftClickAction);
+
+        map.Enable();
+
+        yield return null;
+
+        scene.leftChildReceiver.events.Clear();
+
+        Assert.That(scene.eventSystem.IsPointerOverGameObject(1), Is.False);
+        Assert.That(scene.eventSystem.IsPointerOverGameObject(2), Is.False);
+
+        // Touch left object.
+        var firstPosition = scene.From640x480ToScreen(100, 100);
+        BeginTouch(1, firstPosition);
+        yield return null;
+
+        var pointerIdTouch1 = ExtendedPointerEventData.MakePointerIdForTouch(touchScreen.deviceId, 1);
+        var pointerIdTouch2 = ExtendedPointerEventData.MakePointerIdForTouch(touchScreen.deviceId, 2);
+        var pointerIdTouch3 = ExtendedPointerEventData.MakePointerIdForTouch(touchScreen.deviceId, 3);
+
+        Assert.That(scene.eventSystem.IsPointerOverGameObject(1), Is.True);
+        Assert.That(scene.eventSystem.IsPointerOverGameObject(2), Is.False);
+
+        Assert.That(scene.leftChildReceiver.events,
+            Has.Exactly(1).With.Property("type").EqualTo(EventType.PointerEnter).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.device == touchScreen).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.touchId == 1).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.pointerId == pointerIdTouch1).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.pointerType == UIPointerType.Touch).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.position == firstPosition));
+        Assert.That(scene.leftChildReceiver.events,
+            Has.Exactly(1).With.Property("type").EqualTo(EventType.PointerDown).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.device == touchScreen).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.touchId == 1).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.pointerId == pointerIdTouch1).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.pointerType == UIPointerType.Touch).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.position == firstPosition));
+
+        Assert.That(scene.rightChildReceiver.events, Is.Empty);
+        Assert.That(scene.uiModule.m_PointerStates.length, Is.EqualTo(1));
+
+        scene.leftChildReceiver.events.Clear();
+        scene.rightChildReceiver.events.Clear();
+
+        // Release left object and Touch right object on the same frame.
+        var secondPosition = scene.From640x480ToScreen(350, 200);
+        EndTouch(1, firstPosition);
+        BeginTouch(2, secondPosition);
+        BeginTouch(3, secondPosition);
+        MoveTouch(2, secondPosition);
+        yield return null;
+
+        Assert.That(scene.eventSystem.IsPointerOverGameObject(1), Is.True);
+        Assert.That(scene.eventSystem.IsPointerOverGameObject(2), Is.True);
+        Assert.That(scene.eventSystem.IsPointerOverGameObject(3), Is.True);
+
+        // Pointer 1
+        Assert.That(scene.leftChildReceiver.events,
+            Has.Exactly(1).With.Property("type").EqualTo(EventType.PointerUp).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.device == touchScreen).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.touchId == 1).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.pointerId == pointerIdTouch1).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.pointerType == UIPointerType.Touch).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.position == firstPosition));
+
+        // Pointer 2
+        Assert.That(scene.rightChildReceiver.events,
+            Has.Exactly(1).With.Property("type").EqualTo(EventType.PointerEnter).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.device == touchScreen).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.touchId == 2).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.pointerId == pointerIdTouch2).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.pointerType == UIPointerType.Touch).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.position == secondPosition));
+
+        Assert.That(scene.rightChildReceiver.events,
+            Has.Exactly(1).With.Property("type").EqualTo(EventType.PointerDown).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.device == touchScreen).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.touchId == 2).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.pointerId == pointerIdTouch2).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.pointerType == UIPointerType.Touch).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.position == secondPosition));
+
+        Assert.That(scene.rightChildReceiver.events,
+            Has.Exactly(1).With.Property("type").EqualTo(EventType.PointerMove).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.device == touchScreen).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.touchId == 2).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.pointerId == pointerIdTouch2).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.pointerType == UIPointerType.Touch).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.position == secondPosition));
+
+        // Pointer 3
+        Assert.That(scene.rightChildReceiver.events,
+            Has.Exactly(1).With.Property("type").EqualTo(EventType.PointerEnter).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.device == touchScreen).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.touchId == 3).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.pointerId == pointerIdTouch3).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.pointerType == UIPointerType.Touch).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.position == secondPosition));
+
+        Assert.That(scene.rightChildReceiver.events,
+            Has.Exactly(1).With.Property("type").EqualTo(EventType.PointerDown).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.device == touchScreen).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.touchId == 3).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.pointerId == pointerIdTouch3).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.pointerType == UIPointerType.Touch).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.position == secondPosition));
+
+        Assert.That(scene.uiModule.m_PointerStates.length, Is.EqualTo(3));
+
+        scene.leftChildReceiver.events.Clear();
+        scene.rightChildReceiver.events.Clear();
+
+        // End second touch.
+        EndTouch(2, secondPosition);
+        EndTouch(3, secondPosition);
+        yield return null;
+
+        Assert.That(scene.eventSystem.IsPointerOverGameObject(1), Is.True);
+        Assert.That(scene.eventSystem.IsPointerOverGameObject(2), Is.True);
+        Assert.That(scene.eventSystem.IsPointerOverGameObject(3), Is.True);
+
+        Assert.That(scene.leftChildReceiver.events,
+            Has.Exactly(1).With.Property("type").EqualTo(EventType.PointerExit).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.device == touchScreen).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.touchId == 1).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.pointerId == pointerIdTouch1).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.pointerType == UIPointerType.Touch).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.position == firstPosition));
+
+        Assert.That(scene.rightChildReceiver.events,
+            Has.Exactly(1).With.Property("type").EqualTo(EventType.PointerUp).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.device == touchScreen).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.touchId == 2).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.pointerId == pointerIdTouch2).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.pointerType == UIPointerType.Touch).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.position == secondPosition));
+
+        Assert.That(scene.rightChildReceiver.events,
+            Has.Exactly(1).With.Property("type").EqualTo(EventType.PointerUp).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.device == touchScreen).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.touchId == 3).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.pointerId == pointerIdTouch3).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.pointerType == UIPointerType.Touch).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.position == secondPosition));
+
+        Assert.That(scene.uiModule.m_PointerStates.length, Is.EqualTo(2));
+
+        scene.leftChildReceiver.events.Clear();
+        scene.rightChildReceiver.events.Clear();
+
+        // Next frame
+        yield return null;
+
+        Assert.That(scene.eventSystem.IsPointerOverGameObject(1), Is.False);
+        Assert.That(scene.eventSystem.IsPointerOverGameObject(2), Is.False);
+        Assert.That(scene.eventSystem.IsPointerOverGameObject(3), Is.False);
+
+        Assert.That(scene.leftChildReceiver.events, Is.Empty);
+        Assert.That(scene.rightChildReceiver.events,
+            Has.Exactly(1).With.Property("type").EqualTo(EventType.PointerExit).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.device == touchScreen).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.touchId == 2).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.pointerId == pointerIdTouch2).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.pointerType == UIPointerType.Touch).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.position == secondPosition));
+
+        Assert.That(scene.rightChildReceiver.events,
+            Has.Exactly(1).With.Property("type").EqualTo(EventType.PointerExit).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.device == touchScreen).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.touchId == 3).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.pointerId == pointerIdTouch3).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.pointerType == UIPointerType.Touch).And
+                .Matches((UICallbackReceiver.Event e) => e.pointerData.position == secondPosition));
+
+        Assert.That(scene.uiModule.m_PointerStates.length, Is.Zero);
+    }
+
     // https://fogbugz.unity3d.com/f/cases/1190150/
     [UnityTest]
     [Category("UI")]
@@ -1724,8 +1919,6 @@ public IEnumerator UI_CanUseTouchSimulationWithUI()
 
             Assert.That(scene.uiModule.m_CurrentPointerType, Is.EqualTo(UIPointerType.Touch));
             Assert.That(scene.uiModule.m_PointerIds.length, Is.EqualTo(1));
-            Assert.That(scene.uiModule.m_PointerTouchControls.length, Is.EqualTo(1));
-            Assert.That(scene.uiModule.m_PointerTouchControls[0], Is.SameAs(Touchscreen.current.touches[0]));
             Assert.That(scene.leftChildReceiver.events,
                 EventSequence(
                     AllEvents("pointerType", UIPointerType.Touch),
@@ -1749,8 +1942,6 @@ public IEnumerator UI_CanUseTouchSimulationWithUI()
 
             Assert.That(scene.uiModule.m_CurrentPointerType, Is.EqualTo(UIPointerType.Touch));
             Assert.That(scene.uiModule.m_PointerIds.length, Is.EqualTo(1));
-            Assert.That(scene.uiModule.m_PointerTouchControls.length, Is.EqualTo(1));
-            Assert.That(scene.uiModule.m_PointerTouchControls[0], Is.SameAs(Touchscreen.current.touches[0]));
             Assert.That(scene.leftChildReceiver.events,
                 EventSequence(
                     AllEvents("pointerType", UIPointerType.Touch),
@@ -1767,7 +1958,6 @@ public IEnumerator UI_CanUseTouchSimulationWithUI()
 
             Assert.That(scene.uiModule.m_CurrentPointerType, Is.EqualTo(UIPointerType.None));
             Assert.That(scene.uiModule.m_PointerIds.length, Is.Zero);
-            Assert.That(scene.uiModule.m_PointerTouchControls.length, Is.Zero);
             Assert.That(scene.leftChildReceiver.events,
                 EventSequence(
                     AllEvents("pointerType", UIPointerType.Touch),
diff --git a/Packages/com.unity.inputsystem/CHANGELOG.md b/Packages/com.unity.inputsystem/CHANGELOG.md
@@ -13,6 +13,7 @@ however, it has to be formatted properly to pass verification tests.
 ### Fixed
 - Fixed an issue causing the Action context menu to not show on right click when right clicking an action in the Input Action Editor [ISXB-1134](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1134).
 - Fixed `ArgumentNullException: Value cannot be null.` during the migration of Project-wide Input Actions from `InputManager.asset` to `InputSystem_Actions.inputactions` asset which lead do the lost of the configuration [ISXB-1105](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1105)
+- Fixed pointerId staying the same when simultaneously releasing and then pressing in the same frame on mobile using touch. [ISXB-1006](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-845)
 
 ### Changed
 - Added back the InputManager to InputSystem project-wide asset migration code with performance improvement (ISX-2086)
diff --git a/Packages/com.unity.inputsystem/InputSystem/Plugins/UI/InputSystemUIInputModule.cs b/Packages/com.unity.inputsystem/InputSystem/Plugins/UI/InputSystemUIInputModule.cs
@@ -1721,26 +1721,9 @@ private int GetPointerStateIndexFor(InputControl control, bool createIfNotExists
             ////REVIEW: Any way we can cut down on the hops all over memory that we're doing here?
             var device = control.device;
 
-            ////TODO: We're repeatedly inspecting the control setup here. Do this once and only redo it if the control setup changes.
-
-            ////REVIEW: It seems wrong that we are picking up an input here that is *NOT* reflected in our actions. We just end
-            ////        up reading a touchId control implicitly instead of allowing actions to deliver IDs to us. On the other hand,
-            ////        making that setup explicit in actions may be quite awkward and not nearly as robust.
             // Determine the pointer (and touch) ID. We default the pointer ID to the device
             // ID of the InputDevice.
             var controlParent = control.parent;
-            var touchControlIndex = m_PointerTouchControls.IndexOfReference(controlParent);
-            if (touchControlIndex != -1)
-            {
-                // For touches, we cache a reference to the control of a pointer so that we don't
-                // have to continuously do ReadValue() on the touch ID control.
-                m_CurrentPointerId = m_PointerIds[touchControlIndex];
-                m_CurrentPointerIndex = touchControlIndex;
-                m_CurrentPointerType = UIPointerType.Touch;
-
-                return touchControlIndex;
-            }
-
             var pointerId = device.deviceId;
             var touchId = 0;
             var touchPosition = Vector2.zero;
@@ -1773,18 +1756,15 @@ private int GetPointerStateIndexFor(InputControl control, bool createIfNotExists
             // NOTE: This is a linear search but m_PointerIds is only IDs and the number of concurrent pointers
             //       should be very low at any one point (in fact, we don't generally expect to have more than one
             //       which is why we are using InlinedArrays).
-            if (touchId == 0) // Not necessary for touches; see above.
+            for (var i = 0; i < m_PointerIds.length; i++)
             {
-                for (var i = 0; i < m_PointerIds.length; i++)
+                if (m_PointerIds[i] == pointerId)
                 {
-                    if (m_PointerIds[i] == pointerId)
-                    {
-                        // Existing entry found. Make it the current pointer.
-                        m_CurrentPointerId = pointerId;
-                        m_CurrentPointerIndex = i;
-                        m_CurrentPointerType = m_PointerStates[i].pointerType;
-                        return i;
-                    }
+                    // Existing entry found. Make it the current pointer.
+                    m_CurrentPointerId = pointerId;
+                    m_CurrentPointerIndex = i;
+                    m_CurrentPointerType = m_PointerStates[i].pointerType;
+                    return i;
                 }
             }
 
@@ -1930,7 +1910,6 @@ private int AllocatePointer(int pointerId, int displayIndex, int touchId, UIPoin
 
             // Allocate state.
             m_PointerIds.AppendWithCapacity(pointerId);
-            m_PointerTouchControls.AppendWithCapacity(touchControl);
             return m_PointerStates.AppendWithCapacity(new PointerModel(eventData));
         }
 
@@ -1975,7 +1954,6 @@ private void RemovePointerAtIndex(int index)
             // Remove. Note that we may change the order of pointers here. This can save us needless copying
             // and m_CurrentPointerIndex should be the only index we get around for longer.
             m_PointerIds.RemoveAtByMovingTailWithCapacity(index);
-            m_PointerTouchControls.RemoveAtByMovingTailWithCapacity(index);
             m_PointerStates.RemoveAtByMovingTailWithCapacity(index);
             Debug.Assert(m_PointerIds.length == m_PointerStates.length, "Pointer ID array should match state array in length");
 
@@ -2475,7 +2453,6 @@ private struct InputActionReferenceState
         [NonSerialized] private int m_CurrentPointerIndex = -1;
         [NonSerialized] internal UIPointerType m_CurrentPointerType = UIPointerType.None;
         internal InlinedArray<int> m_PointerIds; // Index in this array maps to index in m_PointerStates. Separated out to make searching more efficient (we do a linear search).
-        internal InlinedArray<InputControl> m_PointerTouchControls;
         internal InlinedArray<PointerModel> m_PointerStates;
 
         // Navigation-type input.
