Skip to content

Commit 0bf1cbc

Browse files
authored
compose/ui/web: fix incorrect interpretation of ontouchend events (#2490)
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.
1 parent 72b4dda commit 0bf1cbc

File tree

2 files changed

+148
-27
lines changed

2 files changed

+148
-27
lines changed

compose/ui/ui/src/webCommonW3C/kotlin/androidx/compose/ui/window/ComposeWindow.w3c.kt

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ import org.w3c.dom.events.Event
106106
import org.w3c.dom.events.KeyboardEvent
107107
import org.w3c.dom.events.MouseEvent
108108
import org.w3c.dom.events.WheelEvent
109+
import org.w3c.dom.pointerevents.PointerEventInit
109110

110111
private val actualDensity
111112
get() = window.devicePixelRatio
@@ -510,27 +511,23 @@ internal class ComposeWindow(
510511
}
511512

512513
/**
513-
* We use both targetTouches and changedTouches:
514-
* - targetTouches is empty when a last pointer is released, but changedTouches won't be empty;
515-
* - changedTouches contains only a Touch of a changed pointer, but compose needs all pointers,
516-
* therefore we take targetTouches in this case;
514+
* The set of touches needed for compose are:
515+
* - targetTouches: contains all pressed touches for the current target element
516+
* - changedTouches when the event is 'touchend' or 'touchcancel': contains released touches
517517
*/
518-
val touches = if (event.targetTouches.length > event.changedTouches.length) {
519-
event.targetTouches.asList()
520-
} else {
521-
event.changedTouches.asList()
518+
val touches = event.targetTouches.asList().fastMap { it to true }.toMutableList()
519+
if (eventType == PointerEventType.Release) {
520+
touches.addAll(event.changedTouches.asList().fastMap { it to false } )
522521
}
523-
val pointers = touches.fastMap { touch ->
522+
523+
val pointers = touches.fastMap { (touch, pressed) ->
524524
ComposeScenePointer(
525525
id = PointerId(touch.identifier.toLong()),
526526
position = Offset(
527527
x = touch.clientX - offset.x,
528528
y = touch.clientY - offset.y
529529
) * density.density,
530-
pressed = when (eventType) {
531-
PointerEventType.Press, PointerEventType.Move -> true
532-
else -> false
533-
},
530+
pressed = pressed,
534531
type = PointerType.Touch,
535532
pressure = touch.unsafeCast<ExtendedTouchEvent>().force.toFloat()
536533
)

compose/ui/ui/src/webTest/kotlin/androidx/compose/ui/input/GesturesTest.kt

Lines changed: 138 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,10 @@ import androidx.compose.ui.events.TouchEventInit
2626
import androidx.compose.ui.geometry.Offset
2727
import androidx.compose.ui.input.pointer.PointerEvent
2828
import androidx.compose.ui.input.pointer.PointerEventType
29+
import androidx.compose.ui.input.pointer.changedToDown
30+
import androidx.compose.ui.input.pointer.changedToDownIgnoreConsumed
31+
import androidx.compose.ui.input.pointer.changedToUp
32+
import androidx.compose.ui.input.pointer.changedToUpIgnoreConsumed
2933
import androidx.compose.ui.input.pointer.pointerInput
3034
import androidx.compose.ui.platform.LocalDensity
3135
import androidx.compose.ui.unit.Density
@@ -54,11 +58,11 @@ class GesturesTest : OnCanvasTests {
5458
}
5559

5660
dispatchEvents(
57-
TouchEvent("touchstart", touchEventInit(createTouch(0, getCanvas()))),
61+
TouchEvent("touchstart", touchEventWithChangedTouchesInit(createTouch(0, getCanvas()))),
5862
// first move to exceed the touch slop
59-
TouchEvent("touchmove", touchEventInit(createTouch(0, getCanvas(), clientX = 10.0, clientY = 10.0))),
60-
TouchEvent("touchmove", touchEventInit(createTouch(0, getCanvas(), clientX = 10.0, clientY = 20.0))),
61-
TouchEvent("touchmove", touchEventInit(createTouch(0, getCanvas(), clientX = 20.0, clientY = 20.0))),
63+
TouchEvent("touchmove", touchEventWithChangedTouchesInit(createTouch(0, getCanvas(), clientX = 10.0, clientY = 10.0))),
64+
TouchEvent("touchmove", touchEventWithChangedTouchesInit(createTouch(0, getCanvas(), clientX = 10.0, clientY = 20.0))),
65+
TouchEvent("touchmove", touchEventWithChangedTouchesInit(createTouch(0, getCanvas(), clientX = 20.0, clientY = 20.0))),
6266
)
6367

6468
val actualPan = 10f * currentDensity.density
@@ -87,45 +91,45 @@ class GesturesTest : OnCanvasTests {
8791
// Simulate two touch points starting fairly close together
8892
TouchEvent(
8993
"touchstart",
90-
touchEventInit(
94+
touchEventWithChangedTouchesInit(
9195
createTouch(0, getCanvas(), clientX = 50.0, clientY = 50.0),
9296
createTouch(1, getCanvas(), clientX = 60.0, clientY = 50.0)
9397
)
9498
),
9599
// first move to exceed the touch slop
96100
TouchEvent(
97101
"touchmove",
98-
touchEventInit(
102+
touchEventWithChangedTouchesInit(
99103
createTouch(0, getCanvas(), clientX = 45.0, clientY = 60.0),
100104
createTouch(1, getCanvas(), clientX = 65.0, clientY = 50.0)
101105
)
102106
),
103107
// Zoom in, zoom > 1
104108
TouchEvent(
105109
"touchmove",
106-
touchEventInit(
110+
touchEventWithChangedTouchesInit(
107111
createTouch(0, getCanvas(), clientX = 40.0, clientY = 50.0),
108112
createTouch(1, getCanvas(), clientX = 70.0, clientY = 50.0)
109113
)
110114
),
111115
TouchEvent(
112116
"touchmove",
113-
touchEventInit(
117+
touchEventWithChangedTouchesInit(
114118
createTouch(0, getCanvas(), clientX = 30.0, clientY = 50.0),
115119
createTouch(1, getCanvas(), clientX = 80.0, clientY = 50.0)
116120
)
117121
),
118122
// and now zoom out, zoom < 1
119123
TouchEvent(
120124
"touchmove",
121-
touchEventInit(
125+
touchEventWithChangedTouchesInit(
122126
createTouch(0, getCanvas(), clientX = 35.0, clientY = 50.0),
123127
createTouch(1, getCanvas(), clientX = 75.0, clientY = 50.0)
124128
)
125129
),
126130
TouchEvent(
127131
"touchmove",
128-
touchEventInit(
132+
touchEventWithChangedTouchesInit(
129133
createTouch(0, getCanvas(), clientX = 37.0, clientY = 50.0),
130134
createTouch(1, getCanvas(), clientX = 73.0, clientY = 50.0)
131135
)
@@ -157,8 +161,8 @@ class GesturesTest : OnCanvasTests {
157161
assertNull(lastPointerEvent)
158162

159163
dispatchEvents(
160-
TouchEvent("touchstart", touchEventInit(createTouch(0, getCanvas(), clientX = 50.0, clientY = 50.0))),
161-
TouchEvent("touchmove", touchEventInit(createTouch(0, getCanvas(), clientX = 60.0, clientY = 60.0)))
164+
TouchEvent("touchstart", touchEventWithChangedTouchesInit(createTouch(0, getCanvas(), clientX = 50.0, clientY = 50.0))),
165+
TouchEvent("touchmove", touchEventWithChangedTouchesInit(createTouch(0, getCanvas(), clientX = 60.0, clientY = 60.0)))
162166
)
163167

164168
awaitIdle()
@@ -181,6 +185,99 @@ class GesturesTest : OnCanvasTests {
181185
assertEquals(PointerEventType.Move, lastPointerEvent!!.type)
182186
}
183187

188+
@Test
189+
fun threeTouchesWithTouchEnd() = runApplicationTest {
190+
val pointerEvents = mutableListOf<PointerEvent>()
191+
192+
createComposeWindow {
193+
Box(modifier = Modifier.fillMaxSize().pointerInput(Unit) {
194+
awaitPointerEventScope {
195+
while (coroutineContext.isActive) {
196+
pointerEvents.add(awaitPointerEvent())
197+
}
198+
}
199+
})
200+
}
201+
202+
assertTrue(pointerEvents.isEmpty())
203+
204+
val touch1 = createTouch(1, getCanvas(), clientX = 10.0, clientY = 10.0)
205+
val touch2 = createTouch(2, getCanvas(), clientX = 20.0, clientY = 20.0)
206+
val touch3 = createTouch(3, getCanvas(), clientX = 30.0, clientY = 30.0)
207+
208+
dispatchEvents(
209+
// +1
210+
TouchEvent(
211+
"touchstart",
212+
touchEventInit(
213+
changedTouches = listOf(touch1),
214+
targetTouches = listOf(touch1),
215+
)
216+
),
217+
// +2
218+
TouchEvent(
219+
"touchstart",
220+
touchEventInit(
221+
changedTouches = listOf(touch2),
222+
targetTouches = listOf(touch1, touch2),
223+
)
224+
),
225+
// +3
226+
TouchEvent(
227+
"touchstart",
228+
touchEventInit(
229+
changedTouches = listOf(touch3),
230+
targetTouches = listOf(touch1, touch2, touch3),
231+
)
232+
),
233+
// -3
234+
TouchEvent(
235+
"touchend",
236+
touchEventInit(
237+
changedTouches = listOf(touch3),
238+
targetTouches = listOf(touch1, touch2),
239+
)
240+
),
241+
// -2
242+
TouchEvent(
243+
"touchend",
244+
touchEventInit(
245+
changedTouches = listOf(touch2),
246+
targetTouches = listOf(touch1),
247+
)
248+
),
249+
// -1
250+
TouchEvent(
251+
"touchend",
252+
touchEventInit(
253+
changedTouches = listOf(touch1),
254+
targetTouches = listOf(),
255+
)
256+
)
257+
)
258+
259+
awaitIdle()
260+
261+
val expected = """
262+
+ 1
263+
+ 1; + 2
264+
+ 1; + 2; + 3
265+
+ 1; + 2; - 3
266+
+ 1; - 2
267+
- 1
268+
""".trimIndent()
269+
270+
val actual = pointerEvents.joinToString("\n") { event ->
271+
event.changes.sortedBy { it.id.value }.joinToString("; ") {
272+
if (it.pressed) {
273+
"+ ${it.id.value}"
274+
} else {
275+
"- ${it.id.value}"
276+
}
277+
}
278+
}
279+
assertEquals(expected, actual)
280+
}
184281
}
185282

186283
external interface Touch
@@ -206,14 +303,14 @@ private fun createTouch(
206303
"""
207304
)
208305

209-
private fun touchEventInit(vararg touches: Touch): TouchEventInit = js(
306+
private fun touchEventWithChangedTouchesInit(vararg touches: Touch): TouchEventInit = js(
210307
"""
211308
({
212309
bubbles: true,
213310
cancelable: true,
214311
composed: true,
215312
changedTouches: touches,
216-
targetTouches: [],
313+
targetTouches: touches,
217314
touches: []
218315
})
219316
"""
@@ -232,3 +329,30 @@ private fun touchEventWithTargetTouchesInit(vararg touches: Touch): TouchEventIn
232329
"""
233330
)
234331

332+
@OptIn(ExperimentalJsCollectionsApi::class)
333+
private fun touchEventInit(
334+
changedTouchesBeforeIndex: Int,
335+
vararg touches: Touch,
336+
): TouchEventInit = js(
337+
"""
338+
({
339+
bubbles: true,
340+
cancelable: true,
341+
composed: true,
342+
changedTouches: touches.slice(0, changedTouchesBeforeIndex),
343+
targetTouches: touches.slice(changedTouchesBeforeIndex),
344+
touches: []
345+
})
346+
"""
347+
)
348+
349+
@OptIn(ExperimentalJsCollectionsApi::class, ExperimentalJsExport::class)
350+
private fun touchEventInit(
351+
changedTouches: List<Touch>,
352+
targetTouches: List<Touch>,
353+
): TouchEventInit = touchEventInit(
354+
changedTouchesBeforeIndex = changedTouches.size,
355+
*arrayOf(*changedTouches.toTypedArray(), *targetTouches.toTypedArray())
356+
)
357+
358+

0 commit comments

Comments
 (0)