diff --git a/Assets/Tests/InputSystem/Plugins/UITests.cs b/Assets/Tests/InputSystem/Plugins/UITests.cs index cc9d066098..288ecfd75a 100644 --- a/Assets/Tests/InputSystem/Plugins/UITests.cs +++ b/Assets/Tests/InputSystem/Plugins/UITests.cs @@ -1460,33 +1460,132 @@ 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] + [Category("UI")] + [Description("Tests that disabling the UI module during a Button click event works correctly with touch pointers." + + "ISXB-687")] + public IEnumerator UI_DisablingEventSystemOnClickEventWorksWithTouchPointers() + { + 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] 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 diff --git a/Packages/com.unity.inputsystem/InputSystem/Plugins/UI/InputSystemUIInputModule.cs b/Packages/com.unity.inputsystem/InputSystem/Plugins/UI/InputSystemUIInputModule.cs index 2ce58a635a..667a833494 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Plugins/UI/InputSystemUIInputModule.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Plugins/UI/InputSystemUIInputModule.cs @@ -2070,15 +2070,12 @@ private bool SendPointerExitEventsAndRemovePointer(int index) 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 + // 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; - } + + Debug.Assert(m_PointerStates[index].eventData.pointerEnter == null, "Pointer should have exited all objects before being removed"); // 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 +2347,7 @@ 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 (m_PointerStates[i].pointerType != UIPointerType.MouseOrPen) { if (SendPointerExitEventsAndRemovePointer(i)) --i; @@ -2452,8 +2443,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; }