Skip to content

FIX: Pen touch input triggers UI/Click action two times (ISXB-1456) #2201

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 2 commits into
base: develop
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 55 additions & 15 deletions Assets/Tests/InputSystem/Plugins/UITests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<UICallbackReceiver.Event, bool> 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]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
// }
Comment on lines +2075 to +2081
Copy link
Preview

Copilot AI Jul 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The double-commented block clutters the cleanup logic. Consider removing these dead code lines or extracting them behind a clearly named feature flag to improve readability.

Suggested change
// // 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;
// }
// Removed dead code block for clarity and readability.

Copilot uses AI. Check for mistakes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will be removed, it's just to help with testing at the moment.


// Retain event data so that we can reuse the event the next time we allocate a PointerModel record.
var eventData = m_PointerStates[index].eventData;
Expand Down Expand Up @@ -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;
Expand Down