From 355e947a857acf1c9632006e2208b2cc3728f3ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Freire?= Date: Mon, 7 Jul 2025 12:14:31 +0300 Subject: [PATCH 1/6] Revert #1982 but leave commented code --- .../Plugins/UI/InputSystemUIInputModule.cs | 34 +++++++++++-------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/Packages/com.unity.inputsystem/InputSystem/Plugins/UI/InputSystemUIInputModule.cs b/Packages/com.unity.inputsystem/InputSystem/Plugins/UI/InputSystemUIInputModule.cs index 2ce58a635a..375f2546c9 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Plugins/UI/InputSystemUIInputModule.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Plugins/UI/InputSystemUIInputModule.cs @@ -2072,13 +2072,13 @@ private bool RemovePointerAtIndex(int index) { Debug.Assert(m_PointerStates[index].eventData.pointerEnter == null, "Pointer should have exited all objects before being removed"); - // We don't want to release touch pointers on the same frame they are released (unpressed). They get cleaned up one frame later in Process() - ref var state = ref GetPointerStateForIndex(index); - if (state.pointerType == UIPointerType.Touch && (state.leftButton.isPressed || state.leftButton.wasReleasedThisFrame)) - { - // The pointer was not removed - return false; - } + // // We don't want to release touch pointers on the same frame they are released (unpressed). They get cleaned up one frame later in Process() + // ref var state = ref GetPointerStateForIndex(index); + // if (state.pointerType == UIPointerType.Touch && (state.leftButton.isPressed || state.leftButton.wasReleasedThisFrame)) + // { + // // The pointer was not removed + // return false; + // } // Retain event data so that we can reuse the event the next time we allocate a PointerModel record. var eventData = m_PointerStates[index].eventData; @@ -2350,13 +2350,19 @@ private void FilterPointerStatesByType() // We have input on a mouse or pen. Kill all touch and tracked pointers we may have. for (var i = 0; i < m_PointerStates.length; ++i) { - ref var state = ref GetPointerStateForIndex(i); - // Touch pointers need to get forced to no longer be pressed otherwise they will not get released in subsequent frames. - if (m_PointerStates[i].pointerType == UIPointerType.Touch) - { - state.leftButton.isPressed = false; - } - if (m_PointerStates[i].pointerType != UIPointerType.MouseOrPen && m_PointerStates[i].pointerType != UIPointerType.Touch || (m_PointerStates[i].pointerType == UIPointerType.Touch && !state.leftButton.isPressed && !state.leftButton.wasReleasedThisFrame)) + // ref var state = ref GetPointerStateForIndex(i); + // // Touch pointers need to get forced to no longer be pressed otherwise they will not get released in subsequent frames. + // if (m_PointerStates[i].pointerType == UIPointerType.Touch) + // { + // state.leftButton.isPressed = false; + // } + // if (m_PointerStates[i].pointerType != UIPointerType.MouseOrPen && m_PointerStates[i].pointerType != UIPointerType.Touch || (m_PointerStates[i].pointerType == UIPointerType.Touch && !state.leftButton.isPressed && !state.leftButton.wasReleasedThisFrame)) + // { + // if (SendPointerExitEventsAndRemovePointer(i)) + // --i; + // } + + if (m_PointerStates[i].pointerType != UIPointerType.MouseOrPen) { if (SendPointerExitEventsAndRemovePointer(i)) --i; From 96b62918598bc426bd44704edd6a6bbc08f9698b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Freire?= Date: Fri, 11 Jul 2025 16:33:56 +0300 Subject: [PATCH 2/6] Change existing test so that it catches the ISXB-1456 regression --- Assets/Tests/InputSystem/Plugins/UITests.cs | 70 ++++++++++++++++----- 1 file changed, 55 insertions(+), 15 deletions(-) diff --git a/Assets/Tests/InputSystem/Plugins/UITests.cs b/Assets/Tests/InputSystem/Plugins/UITests.cs index cc9d066098..bd026bc480 100644 --- a/Assets/Tests/InputSystem/Plugins/UITests.cs +++ b/Assets/Tests/InputSystem/Plugins/UITests.cs @@ -1460,33 +1460,73 @@ public IEnumerator UI_CanDriveUIFromMultiplePointers(UIPointerBehavior pointerBe scene.leftChildReceiver.events.Clear(); scene.rightChildReceiver.events.Clear(); - // Test if creating Pointer events from different devices at the same time results in only one event - BeginTouch(0, firstPosition, screen: touch1, queueEventOnly: true); + // End previous touches that started so that we can do a cleanup from the last test. + EndTouch(1, secondPosition, screen: touch1); + yield return null; + EndTouch(1, firstPosition, screen: touch2); + yield return null; + // Set a mouse position without any clicks to "emulate" a real movement before a button press. + Set(mouse1.position, secondPosition + new Vector2(-10, 0)); + yield return null; + + scene.leftChildReceiver.events.Clear(); + scene.rightChildReceiver.events.Clear(); + + // Test a press and release from both a Mouse and Touchscreen at the same time + // This is to simulate some platforms that always send Mouse/Pen and Touches (e.g. Android). + // Also, this mostly assets the expected behavior for the options SingleMouseOrPenButMultiTouchAndTrack. + var touchId = 2; + BeginTouch(touchId, secondPosition, screen: touch1, queueEventOnly: true); + Set(mouse1.position, secondPosition, queueEventOnly: true); Press(mouse1.leftButton); + yield return null; - EndTouch(0, firstPosition, screen: touch1, queueEventOnly: true); + + EndTouch(touchId, secondPosition, screen: touch1, queueEventOnly: true); Release(mouse1.leftButton); yield return null; + Func eventDeviceCondition = null; + var expectedCount = 0; switch (pointerBehavior) { + case UIPointerBehavior.SingleMouseOrPenButMultiTouchAndTrack: + // Expects only mouse events for PointerClick, PointerDown, and PointerUp + eventDeviceCondition = (e) => e.pointerData.device == mouse1; + expectedCount = 1; + // Make sure that the touch does not generate a UI events. + Assert.That(scene.rightChildReceiver.events, Has.None.Matches((UICallbackReceiver.Event e) => + e.pointerData != null && e.pointerData.device == touch1)); + break; + case UIPointerBehavior.SingleUnifiedPointer: - //// Getting "Drop" event even if using only one type of input device for Press/Release. - //// E.g. the following test would also produce only a Drop event: - //// Press(mouse1.leftButton); - //// yield return null; - //// Release(mouse1.leftButton); - //// yield return null; + // Expects only single UI events with touch source since they are the first events in the queue + eventDeviceCondition = (e) => e.pointerData.device == touch1; + expectedCount = 1; break; - case UIPointerBehavior.SingleMouseOrPenButMultiTouchAndTrack: + case UIPointerBehavior.AllPointersAsIs: - // Single pointer click on the left object - Assert.That(scene.leftChildReceiver.events, - Has.Exactly(1).With.Property("type").EqualTo(EventType.PointerClick).And - .Matches((UICallbackReceiver.Event e) => e.pointerData.device == mouse1).And - .Matches((UICallbackReceiver.Event e) => e.pointerData.position == firstPosition)); + // Expects both pointer devices to generate PointerClick, PointerDown, and PointerUp events + eventDeviceCondition = (e) => e.pointerData.device == mouse1 || e.pointerData.device == touch1; + expectedCount = 2; break; + + default: + throw new ArgumentOutOfRangeException(nameof(pointerBehavior), pointerBehavior, null); } + + Assert.That(scene.rightChildReceiver.events, + Has.Exactly(expectedCount).With.Property("type").EqualTo(EventType.PointerClick).And + .Matches((UICallbackReceiver.Event e) => eventDeviceCondition(e)).And + .Matches((UICallbackReceiver.Event e) => e.pointerData.position == secondPosition)); + Assert.That(scene.rightChildReceiver.events, + Has.Exactly(expectedCount).With.Property("type").EqualTo(EventType.PointerDown).And + .Matches((UICallbackReceiver.Event e) => eventDeviceCondition(e)).And + .Matches((UICallbackReceiver.Event e) => e.pointerData.position == secondPosition)); + Assert.That(scene.rightChildReceiver.events, + Has.Exactly(expectedCount).With.Property("type").EqualTo(EventType.PointerUp).And + .Matches((UICallbackReceiver.Event e) => eventDeviceCondition(e)).And + .Matches((UICallbackReceiver.Event e) => e.pointerData.position == secondPosition)); } [UnityTest] From 16cd8202ca8af09aa034d5389d3cde4d8db66dba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Freire?= Date: Mon, 14 Jul 2025 16:55:46 +0300 Subject: [PATCH 3/6] Check for empty pointer states before removal This fixes ISXB-687 and adds a test. --- Assets/Tests/InputSystem/Plugins/UITests.cs | 59 +++++++++++++++++++ .../Plugins/UI/InputSystemUIInputModule.cs | 9 ++- 2 files changed, 66 insertions(+), 2 deletions(-) diff --git a/Assets/Tests/InputSystem/Plugins/UITests.cs b/Assets/Tests/InputSystem/Plugins/UITests.cs index bd026bc480..be97d1589f 100644 --- a/Assets/Tests/InputSystem/Plugins/UITests.cs +++ b/Assets/Tests/InputSystem/Plugins/UITests.cs @@ -1529,6 +1529,65 @@ public IEnumerator UI_CanDriveUIFromMultiplePointers(UIPointerBehavior pointerBe .Matches((UICallbackReceiver.Event e) => e.pointerData.position == secondPosition)); } + [UnityTest] + [Category("UI")] + [Description("Tests that disabling the UI module during a Button click event works correctly with touch pointers." + + "ISXB-687")] + public IEnumerator UI_DisablingEventSystemOnClickEventWorksWithTouchPointersWorks() + { + var touch = InputSystem.AddDevice(); + var scene = CreateTestUI(); + + var actions = ScriptableObject.CreateInstance(); + var uiActions = actions.AddActionMap("UI"); + var pointAction = uiActions.AddAction("point", type: InputActionType.PassThrough); + var clickAction = uiActions.AddAction("click", type: InputActionType.PassThrough); + + pointAction.AddBinding("/touch*/position"); + clickAction.AddBinding("/touch*/press"); + + pointAction.Enable(); + clickAction.Enable(); + + scene.uiModule.point = InputActionReference.Create(pointAction); + scene.uiModule.pointerBehavior = UIPointerBehavior.SingleMouseOrPenButMultiTouchAndTrack; + scene.uiModule.leftClick = InputActionReference.Create(clickAction); + + // Turn left object into a button. + var button = scene.leftGameObject.AddComponent(); + var clicked = false; + + // Add a listener to the button to disable the UI module when clicked. + // This calls InputSystemUIInputModule.OnDisable() which will reset the pointer data during + // InputSystemUIInputModule.Process() and ProcessPointer(). It will allow us to test that removing + // a pointer once the UI module is disabled (all pointers are removed) works correctly. + button.onClick.AddListener(() => + { + clicked = true; + scene.uiModule.enabled = false; // Disable the UI module to test pointer reset. + }); + + yield return null; + + var firstPosition = scene.From640x480ToScreen(100, 100); + + // This will allocate a pointer for the touch and set the first touch position and press + BeginTouch(1, firstPosition, screen: touch); + yield return null; + + Assert.That(clicked, Is.False, "Button was clicked when it should not have been yet."); + Assert.That(scene.uiModule.m_PointerStates.length, Is.EqualTo(1), + "A pointer states was not allocated for the touch pointer."); + + // Release the touch to make sure we have a Click event that calls the button listener. + EndTouch(1, firstPosition, screen: touch); + yield return null; + + Assert.That(clicked, Is.True, "Button was not clicked when it should have been."); + Assert.That(scene.uiModule.m_PointerStates.length, Is.EqualTo(0), + "Pointer states were not cleared when the UI module was disabled after a click event."); + } + [UnityTest] [Category("UI")] public IEnumerator UI_CanDriveUIFromMultipleTouches() diff --git a/Packages/com.unity.inputsystem/InputSystem/Plugins/UI/InputSystemUIInputModule.cs b/Packages/com.unity.inputsystem/InputSystem/Plugins/UI/InputSystemUIInputModule.cs index 375f2546c9..16b1efc9c7 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Plugins/UI/InputSystemUIInputModule.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Plugins/UI/InputSystemUIInputModule.cs @@ -2070,6 +2070,11 @@ private bool SendPointerExitEventsAndRemovePointer(int index) private bool RemovePointerAtIndex(int index) { + // Pointers can have be reset before (e.g. when calling OnDisable) which would make m_PointerStates + // empty (ISXB-687). + if (m_PointerStates.length == 0) + return false; + Debug.Assert(m_PointerStates[index].eventData.pointerEnter == null, "Pointer should have exited all objects before being removed"); // // We don't want to release touch pointers on the same frame they are released (unpressed). They get cleaned up one frame later in Process() @@ -2458,8 +2463,8 @@ public override void Process() // stays true for the touch in the frame of release (see UI_TouchPointersAreKeptForOneFrameAfterRelease). if (state.pointerType == UIPointerType.Touch && !state.leftButton.isPressed && !state.leftButton.wasReleasedThisFrame) { - RemovePointerAtIndex(i); - --i; + if (RemovePointerAtIndex(i)) + --i; continue; } From 43434636f9abf3316c8787d4badaf3c3e6b8a0ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Freire?= Date: Mon, 14 Jul 2025 16:57:53 +0300 Subject: [PATCH 4/6] Remove commented code --- .../Plugins/UI/InputSystemUIInputModule.cs | 20 ------------------- 1 file changed, 20 deletions(-) diff --git a/Packages/com.unity.inputsystem/InputSystem/Plugins/UI/InputSystemUIInputModule.cs b/Packages/com.unity.inputsystem/InputSystem/Plugins/UI/InputSystemUIInputModule.cs index 16b1efc9c7..dce300d15d 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Plugins/UI/InputSystemUIInputModule.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Plugins/UI/InputSystemUIInputModule.cs @@ -2077,14 +2077,6 @@ private bool RemovePointerAtIndex(int index) Debug.Assert(m_PointerStates[index].eventData.pointerEnter == null, "Pointer should have exited all objects before being removed"); - // // We don't want to release touch pointers on the same frame they are released (unpressed). They get cleaned up one frame later in Process() - // ref var state = ref GetPointerStateForIndex(index); - // if (state.pointerType == UIPointerType.Touch && (state.leftButton.isPressed || state.leftButton.wasReleasedThisFrame)) - // { - // // The pointer was not removed - // return false; - // } - // Retain event data so that we can reuse the event the next time we allocate a PointerModel record. var eventData = m_PointerStates[index].eventData; Debug.Assert(eventData != null, "Pointer state should have an event instance!"); @@ -2355,18 +2347,6 @@ private void FilterPointerStatesByType() // We have input on a mouse or pen. Kill all touch and tracked pointers we may have. for (var i = 0; i < m_PointerStates.length; ++i) { - // ref var state = ref GetPointerStateForIndex(i); - // // Touch pointers need to get forced to no longer be pressed otherwise they will not get released in subsequent frames. - // if (m_PointerStates[i].pointerType == UIPointerType.Touch) - // { - // state.leftButton.isPressed = false; - // } - // if (m_PointerStates[i].pointerType != UIPointerType.MouseOrPen && m_PointerStates[i].pointerType != UIPointerType.Touch || (m_PointerStates[i].pointerType == UIPointerType.Touch && !state.leftButton.isPressed && !state.leftButton.wasReleasedThisFrame)) - // { - // if (SendPointerExitEventsAndRemovePointer(i)) - // --i; - // } - if (m_PointerStates[i].pointerType != UIPointerType.MouseOrPen) { if (SendPointerExitEventsAndRemovePointer(i)) From 01cf85de87cbd48f01bcc717e124259a8ab7fab9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Freire?= Date: Tue, 15 Jul 2025 10:54:21 +0300 Subject: [PATCH 5/6] Update based on Copilot suggestions --- Assets/Tests/InputSystem/Plugins/UITests.cs | 2 +- .../InputSystem/Plugins/UI/InputSystemUIInputModule.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Assets/Tests/InputSystem/Plugins/UITests.cs b/Assets/Tests/InputSystem/Plugins/UITests.cs index be97d1589f..288ecfd75a 100644 --- a/Assets/Tests/InputSystem/Plugins/UITests.cs +++ b/Assets/Tests/InputSystem/Plugins/UITests.cs @@ -1533,7 +1533,7 @@ public IEnumerator UI_CanDriveUIFromMultiplePointers(UIPointerBehavior pointerBe [Category("UI")] [Description("Tests that disabling the UI module during a Button click event works correctly with touch pointers." + "ISXB-687")] - public IEnumerator UI_DisablingEventSystemOnClickEventWorksWithTouchPointersWorks() + public IEnumerator UI_DisablingEventSystemOnClickEventWorksWithTouchPointers() { var touch = InputSystem.AddDevice(); var scene = CreateTestUI(); diff --git a/Packages/com.unity.inputsystem/InputSystem/Plugins/UI/InputSystemUIInputModule.cs b/Packages/com.unity.inputsystem/InputSystem/Plugins/UI/InputSystemUIInputModule.cs index dce300d15d..667a833494 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Plugins/UI/InputSystemUIInputModule.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Plugins/UI/InputSystemUIInputModule.cs @@ -2070,7 +2070,7 @@ private bool SendPointerExitEventsAndRemovePointer(int index) private bool RemovePointerAtIndex(int index) { - // Pointers can have be reset before (e.g. when calling OnDisable) which would make m_PointerStates + // Pointers might have been reset before (e.g. when calling OnDisable) which would make m_PointerStates // empty (ISXB-687). if (m_PointerStates.length == 0) return false; From 408cc444df644b753c80e44d237894b1c2f05e73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Freire?= Date: Tue, 15 Jul 2025 12:08:46 +0300 Subject: [PATCH 6/6] Update CHANGELOG.md --- Packages/com.unity.inputsystem/CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Packages/com.unity.inputsystem/CHANGELOG.md b/Packages/com.unity.inputsystem/CHANGELOG.md index 7ef854808d..876a0ccfcb 100644 --- a/Packages/com.unity.inputsystem/CHANGELOG.md +++ b/Packages/com.unity.inputsystem/CHANGELOG.md @@ -10,6 +10,9 @@ however, it has to be formatted properly to pass verification tests. ## [Unreleased] - yyyy-mm-dd +### Fixed +- Fixed an issue where using Pen devices on Android tablets would result in double clicks for UI interactions. [ISXB-1456](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1456) + ## [1.14.1] - 2025-07-10