Skip to content

Conversation

TheLortex
Copy link

In ComposeWindow.w3c.kt, the touch handler does not correctly handle the touchend event. Indeed the logic selecting between targetTouches and changedTouches is flawed. For touchend both should be taken into account, also changedTouches contains the un-pressed pointer and targetTouches contains the rest of the remaining pressed pointers.

The fix consists in removing the logic that compares the size of both targetTouches and changedTouches to pick the largest one.
Instead, it checks the pointer event type to determine whether only targetTouches or both targetTouches and changedTouches are needed while correctly accounting which touches are pressed depending on the source.

Fixes: https://youtrack.jetbrains.com/issue/CMP-9030/wasm-canvas-incorrect-handling-of-multi-touch-input

Testing

I have added a failing test in GestureTest.kt which is subsequently fixed by the implementation commit. The test consists in using three touches that are progressively pressed (1, 2, 3) and unpressed (3, 2, 1), logging and comparing the behavior of the pointerInput modifier with the expected events.

Release Notes

Fixes - Web

  • Fix incorrect interpretation of ontouchend events.

@eymar eymar self-requested a review October 15, 2025 14:41
Copy link
Member

@eymar eymar left a comment

Choose a reason for hiding this comment

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

} else {
event.changedTouches.asList()
event.changedTouches.asList().fastMap { it to false } + event.targetTouches.asList()
.fastMap { it to true }
Copy link
Member

@eymar eymar Oct 16, 2025

Choose a reason for hiding this comment

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

As I see the final touches list always contains event.targetTouches.

In this case, wdyt about changing this to:

val touches = event.targetTouches.asList().fastMap { it to true }.toMutableList()
if (eventType == PointerEventType.Release) {
    touches.addAll(event.changedTouches.asList().fastMap { it to false })
}

Copy link
Author

Choose a reason for hiding this comment

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

yes, that's cleaner

@TheLortex
Copy link
Author

Thank you for your review. I have applied your suggestion.

Would you like to add a demo that you have created in the bug report?

I'd like to keep the changeset small if that's fine for you ! I don't think the demo is needed in this case

@TheLortex TheLortex requested a review from eymar October 20, 2025 08:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants