Repository navigation
FIX: [Port] [UUM-155740] [Input System] Fixed UI Toolkit receiving an extra PointerDownEvent for touch drags - #2502
benoitalain wants to merge 1 commit into
Conversation
… extra PointerDownEvent for touch drags A touch's press control reads its state from the touch phase, so the pass-through UI/Click action is performed again when the phase changes from Began to Moved. InputSystemProvider dispatched a ButtonPressed on every callback, so a single-finger drag produced two PointerDownEvents. The provider now tracks pressed inputs per binding and only dispatches a button event when the pressed state changes. Presses whose release was missed while the click action was disabled are dropped in Update. Unity.InputSystem.ForUI now allows unsafe code, to read the binding state of the callback. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
![u-pr[bot]](https://avatars.githubusercontent.com/in/1201017?s=60&v=4){kind=small}
| bindingIndex = ctx.bindingIndex, | ||
| control = ctx.control, | ||
| }; | ||
| if (!m_PressedInputs.TryAdd(key, pressedInput)) |
There was a problem hiding this comment.
A new press is swallowed when a disabled release and subsequent press occur before the next InputSystemProvider.Update. After delivering a mouse press, disable the UI map, process the release, then re-enable the map and process another press. The disabled release never reaches OnClickPerformed, and enabling an initial-state check skips released controls. Because ForgetReleasedInputs has not observed the release, TryAdd finds the old key and suppresses the new press. Multiple input updates are supported in manual update mode. Reconcile tracked entries across click-action disable and enable transitions. Add a regression test without a provider update between release and repress.
🤖 Helpful? 👍/👎
There was a problem hiding this comment.
This is the same edge case that was raised and accepted on the Editor PR (unity/unity#130695). It needs a release to be processed while the UI map is disabled, then the map to be re-enabled and pressed again before the provider's next Update. Manual update mode allows several input updates within that window, but the disable and re-enable must still happen between the release and the press. Disabling a pass-through action fires no canceled, so the only hook available is the global InputSystem.onActionChange. Subscribing to it from the always-on provider would add a callback to every phase change of every action in the game, and disable the no-listener early-out. We're accepting this edge case, and keeping the package identical to the Editor fix.
— Written by Claude Code (AI agent) on behalf of @benoit-alain.
| if (!m_PressedInputs.TryAdd(key, pressedInput)) | ||
| return; | ||
| } | ||
| else if (!m_PressedInputs.Remove(key)) |
There was a problem hiding this comment.
Erasing an earlier click binding while a button is held can suppress its release and leave m_MouseState.ButtonsState pressed. Configure pass-through UI/Click with initial-state checks off and bindings for right then left mouse buttons. Hold left, disable the map, erase binding zero, and re-enable. Binding resolution reuses InputActionState, but the left binding moves from index one to zero. ForgetReleasedInputs drops the saved index-one entry. OnClickPerformed then rejects the release because the index-zero key was never recorded. Re-find saved bindings using their stable InputBinding.id before polling or matching callbacks. Add a held-button binding-erasure regression test.
🤖 Helpful? 👍/👎
There was a problem hiding this comment.
Confirmed: re-resolution reuses the InputActionState, so erasing an earlier UI/Click binding shifts the held binding's index. The tracked press is then dropped, and its release is suppressed. The pointer state stays pressed until the next click on that button, which dispatches normally and restores it. Keying presses on InputBinding.id would add a Guid parse per click callback and a search per held press per frame, to cover editing the UI click bindings while one of them is held. We're accepting this edge case. The Editor fix has the same behavior, and the package stays identical to it.
— Written by Claude Code (AI agent) on behalf of @benoit-alain.
Codecov ReportAll modified and coverable lines are covered by tests ✅ @@ Coverage Diff @@
## develop #2502 +/- ##
===========================================
+ Coverage 78.96% 79.05% +0.08%
===========================================
Files 767 768 +1
Lines 140949 141398 +449
===========================================
+ Hits 111305 111782 +477
+ Misses 29644 29616 -28
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Description
Port of the Editor fix for UUM-155740. On iOS and Android, a single-finger drag on a UI Toolkit runtime UI dispatched
PointerDownEventtwice: Down, Move, Down, Move, Up.UI/Clickis a pass-through action bound to<Touchscreen>/touch*/press, and a touch'spresscontrol reads its state from the touch phase. When the phase changes fromBegantoMoved, the press value stays 1 but the action is performed again, andInputSystemProviderdispatched a secondButtonPressed.The provider now only dispatches a button event when the pressed state of an input changes:
Updatedrops tracked presses that the action no longer evaluates as pressed, or whose device was removed, so they don't swallow the next press.Testing status & QA
Added
InputForUITestsfor touch drags, two fingers, two touchscreens, releases while the UI map is disabled, and click bindings with processors and composites. Ran locally in 6000.0.51f1 batchmode, PlayMode,InputForUITests:TouchDrag_DispatchesOneButtonPressedAndOneButtonReleasedandTwoFingerDrag_DispatchesButtonEventsForEachFinger.I haven't run the full suites locally.
Overall Product Risks
Comments to reviewers
Unity.InputSystem.ForUInow allows unsafe code. The provider reads the callback's binding state, and in the Editor the package is built with unsafe code allowed, so the fix didn't need it there.presscontrol notifies listeners on any phase change even when the press value doesn't change. That could be worth a look on its own, but this PR doesn't change it.Written by Claude Code (AI agent) on behalf of @benoit-alain.
Checklist
Before review:
Changed,Fixed,Addedsections.Area_CanDoX,Area_CanDoX_EvenIfYIsTheCase,Area_WhenIDoX_AndYHappens_ThisIsTheResult.During merge:
NEW: ___.FIX: ___.DOCS: ___.CHANGE: ___.RELEASE: 1.1.0-preview.3.