[Android] Check all pointers in move events#4010
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes Android multi-touch event routing where ACTION_MOVE batches all pointers but actionIndex stays 0, causing handlers tracking a non-zero pointer to stop receiving move updates (issue #3995).
Changes:
- Update
GestureHandler.wantsEvent(MotionEvent)to treat an event as relevant if any pointer in the event is tracked by the handler.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...ve-gesture-handler/android/src/main/java/com/swmansion/gesturehandler/core/GestureHandler.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return if (event.actionMasked == MotionEvent.ACTION_MOVE) { | ||
| (0 until event.pointerCount).any { i -> | ||
| isTrackingPointer(event.getPointerId(i)) | ||
| } | ||
| } else { | ||
| isTrackingPointer(event.getPointerId(event.actionIndex)) |
There was a problem hiding this comment.
wantsEvent is on a hot path (called for every handler on every MotionEvent). For the MOVE case, (0 until event.pointerCount).any { ... } allocates an iterator/lambda per call. To match the rest of this codebase (e.g. dispatchTouchMoveEvent uses a for loop over pointerCount) and minimize overhead, consider switching this to a plain for loop with an early return true when a tracked pointer is found.
| return if (event.actionMasked == MotionEvent.ACTION_MOVE) { | |
| (0 until event.pointerCount).any { i -> | |
| isTrackingPointer(event.getPointerId(i)) | |
| } | |
| } else { | |
| isTrackingPointer(event.getPointerId(event.actionIndex)) | |
| if (event.actionMasked == MotionEvent.ACTION_MOVE) { | |
| val pointerCount = event.pointerCount | |
| for (i in 0 until pointerCount) { | |
| if (isTrackingPointer(event.getPointerId(i))) { | |
| return true | |
| } | |
| } | |
| return false | |
| } else { | |
| return isTrackingPointer(event.getPointerId(event.actionIndex)) |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
## Description When one pointer is placed on the background, handlers don't to other pointers. Instead, they fail on [this line](https://github.com/software-mansion/react-native-gesture-handler/blob/41fbd374a85def52eb7051f96e3d4c63e6eb0724/packages/react-native-gesture-handler/android/src/main/java/com/swmansion/gesturehandler/core/GestureHandlerOrchestrator.kt#L295). This is because on Android events with `ACTION_MOVE` are batched and all pointers are contained within one event. As [Android docs say](https://developer.android.com/reference/android/view/MotionEvent#getActionIndex()), `actionIndex` is fine to check for up and down actions. However, we use it for move action and it always returns `0`. To solve this problem, I've added loop that checks whether any pointer from event is tracked by the handler. Fixes #3995 ## Test plan Tested on example code from #3995
## Description When one pointer is placed on the background, handlers don't to other pointers. Instead, they fail on [this line](https://github.com/software-mansion/react-native-gesture-handler/blob/41fbd374a85def52eb7051f96e3d4c63e6eb0724/packages/react-native-gesture-handler/android/src/main/java/com/swmansion/gesturehandler/core/GestureHandlerOrchestrator.kt#L295). This is because on Android events with `ACTION_MOVE` are batched and all pointers are contained within one event. As [Android docs say](https://developer.android.com/reference/android/view/MotionEvent#getActionIndex()), `actionIndex` is fine to check for up and down actions. However, we use it for move action and it always returns `0`. To solve this problem, I've added loop that checks whether any pointer from event is tracked by the handler. Fixes #3995 ## Test plan Tested on example code from #3995 Co-authored-by: m-bert <63123542+m-bert@users.noreply.github.com>
## Description Cherry pick thread for release 2.31.0 ## List of PRs - #3983 - #3987 - #3989 - #3991 - #4010 - #4015 - #4020 - #4029 - [Already existing] #4039 - #4047 ## Needs double-check - #3964 - [Original commit](560d1b5#diff-3a70a725140527b922181806417d7510bb1c7a19ee9243a0dce4ce7d826ab235) - [Cherry-picked commit](058addc) - Follow up: [85c364a](85c364a) - #4003 - [Original commit](7406046) - [Cherry-picked commit](92b3bca) - #4012 - [Original commit](58c4641) - [Cherry-picked commit](4225660) - This is only TS check so I think it shouldn't break anything - #4023 - [Original commit](3918d8a) - [Cherry-picked commit](e7e8506) - I've checked that only formatting part was skipped, so up to you if you want to check that as well ## Test plan 🚀 --------- Co-authored-by: Janic Duplessis <janicduplessis@gmail.com> Co-authored-by: Wojciech Lewicki <wojciech.lewicki@swmansion.com> Co-authored-by: Andreas Högström <andreas.hogstrom@navigraph.com> Co-authored-by: YevheniiKotyrlo <44449990+YevheniiKotyrlo@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Jakub Piasecki <jakub.piasecki@swmansion.com> Co-authored-by: David Duarte <34779165+DavidDuarte22@users.noreply.github.com>
Description
When one pointer is placed on the background, handlers don't respond to other pointers. Instead, they fail on this line. This is because on Android events with
ACTION_MOVEare batched and all pointers are contained within one event. As Android docs say,actionIndexis fine to check for up and down actions. However, we use it for move action and it always returns0.To solve this problem, I've added loop that checks whether any pointer from event is tracked by the handler.
Fixes #3995
Test plan
Tested on example code from #3995