Don't send synthetic Move events before Press/Release for touch (#870)

These kind of synthetics were added to fix missed Enter/Exit events: Box1 | Box2 If we have this chain of native events: ``` Move on Box1 Press on Box1 Release on Box2 Move on Box2 ``` Without synthetic events, Compose receives: ``` Enter on Box1 Press on Box1 Release on Box1 (because Box1 takes ownership of the all events) Enter on Box2 ``` With synthetic events: ``` Enter on Box1 Press on Box1 Exit on Box1 Release on Box1 Enter on Box2 ``` Touch doesn't have Enter/Exit, so we don't have to send them. Furthermore, such kind of synthetics can bring issues when calculating the end fling velocity (the end position should be excluded from the velocity calculation on iOS). This isn't an issue for mouse. ## Testing - run existing tests - run iOS demo and check that click, and scrolling works ## Issues Fixed https://youtrack.jetbrains.com/issue/COMPOSE-474/Fix-synthetic-events-before-end-event-with-different-position
JetBrains · Dec 7, 2023 · 3dd999d · 3dd999d
1 parent 8d1e0f0
commit 3dd999d
Show file tree

Hide file tree

Showing 6 changed files with 60 additions and 111 deletions.
diff --git a/...n/foundation/src/skikoTest/kotlin/androidx/compose/foundation/gestures/DragGestureTest.kt b/...n/foundation/src/skikoTest/kotlin/androidx/compose/foundation/gestures/DragGestureTest.kt
@@ -313,6 +313,11 @@ class DragGestureTest {
             assertFalse(dragCanceled)
             assertFalse(dragEnded)
 
+            scene.sendPointerEvent(
+                eventType = PointerEventType.Move,
+                position = Offset(5f + viewConfiguration.touchSlop, 15f),
+                type = PointerType.Touch
+            )
             scene.sendPointerEvent(
                 eventType = PointerEventType.Release,
                 position = Offset(5f + viewConfiguration.touchSlop, 15f),

diff --git a/compose/ui/ui/src/desktopTest/kotlin/androidx/compose/ui/ComposeSceneInputTest.kt b/compose/ui/ui/src/desktopTest/kotlin/androidx/compose/ui/ComposeSceneInputTest.kt
@@ -444,16 +444,11 @@ class ComposeSceneInputTest {
             touch(1f, 1f, pressed = false, id = 1),
             touch(1f, 2f, pressed = false, id = 2),
         )
-        // Position is changed, we need to generate a synthetic Move for this position
-        background.events.assertReceived(
-            PointerEventType.Move,
-            touch(1f, 1f, pressed = true, id = 1),
-            touch(1f, 2f, pressed = true, id = 2),
-        )
+
         background.events.assertReceived(
             PointerEventType.Release,
-            touch(1f, 1f, pressed = false, id = 1),
-            touch(1f, 2f, pressed = true, id = 2),
+            touch(10f, 55f, pressed = false, id = 1),
+            touch(1f, 55f, pressed = true, id = 2),
         )
         background.events.assertReceivedLast(
             PointerEventType.Release,

diff --git a/compose/ui/ui/src/desktopTest/kotlin/androidx/compose/ui/SyntheticEventSenderTest.kt b/compose/ui/ui/src/desktopTest/kotlin/androidx/compose/ui/SyntheticEventSenderTest.kt
@@ -76,6 +76,23 @@ class SyntheticEventSenderTest {
         )
     }
 
+    @Test
+    fun `touch, shouldn't generate new move before non-move if position isn't the same`() {
+        eventsSentBy(
+            event(Press, 1 to touch(10f, 25f, pressed = true)),
+            event(Move, 1 to touch(10f, 30f, pressed = true)),
+            event(Release, 1 to touch(10f, 35f, pressed = false)),
+            event(Press, 2 to touch(10f, 45f, pressed = true)),
+            event(Release, 2 to touch(10f, 50f, pressed = false)),
+        ) positionAndDownShouldEqual listOf(
+            event(Press, 1 to touch(10f, 25f, pressed = true)),
+            event(Move, 1 to touch(10f, 30f, pressed = true)),
+            event(Release, 1 to touch(10f, 35f, pressed = false)),
+            event(Press, 2 to touch(10f, 45f, pressed = true)),
+            event(Release, 2 to touch(10f, 50f, pressed = false)),
+        )
+    }
+
     @Test
     fun `touch, shouldn't generate new events if order is correct, without moves`() {
         eventsSentBy(
@@ -114,26 +131,6 @@ class SyntheticEventSenderTest {
         )
     }
 
-    @Test
-    fun `touch, should generate new move before non-move if position isn't the same`() {
-        eventsSentBy(
-            event(Press, 1 to touch(1f, 2f, pressed = true)),
-            event(Press, 1 to touch(1f, 3f, pressed = true), 2 to touch(10f, 20f, pressed = true)),
-            event(Move, 1 to touch(1f, 3f, pressed = true), 2 to touch(10f, 25f, pressed = true)),
-            event(Release, 1 to touch(1f, 4f, pressed = false), 2 to touch(10f, 30f, pressed = true)),
-            event(Release, 2 to touch(10f, 40f, pressed = false)),
-        ) positionAndDownShouldEqual listOf(
-            event(Press, 1 to touch(1f, 2f, pressed = true)),
-            event(Move, 1 to touch(1f, 3f, pressed = true)),
-            event(Press, 1 to touch(1f, 3f, pressed = true), 2 to touch(10f, 20f, pressed = true)),
-            event(Move, 1 to touch(1f, 3f, pressed = true), 2 to touch(10f, 25f, pressed = true)),
-            event(Move, 1 to touch(1f, 4f, pressed = true), 2 to touch(10f, 30f, pressed = true)),
-            event(Release, 1 to touch(1f, 4f, pressed = false), 2 to touch(10f, 30f, pressed = true)),
-            event(Move, 1 to touch(1f, 4f, pressed = false), 2 to touch(10f, 40f, pressed = true)),
-            event(Release, 2 to touch(10f, 40f, pressed = false)),
-        )
-    }
-
     @Test
     fun `touch, should generate one press or release at a time`() {
         eventsSentBy(
@@ -181,71 +178,6 @@ class SyntheticEventSenderTest {
         )
     }
 
-    @Test
-    fun `touch, should generate one press or release at a time, with moves and changed position`() {
-        eventsSentBy(
-            event(
-                Press,
-                1 to touch(1f, 3f, pressed = true),
-                2 to touch(10f, 20f, pressed = true),
-                3 to touch(100f, 200f, pressed = true),
-            ),
-            event(
-                Move,
-                1 to touch(1f, 3f, pressed = true),
-                2 to touch(1f, 2f, pressed = true),
-                3 to touch(1f, 4f, pressed = true),
-            ),
-            event(
-                Release,
-                2 to touch(10f, 20f, pressed = false),
-                3 to touch(100f, 200f, pressed = true),
-            ),
-        ) positionAndDownShouldEqual listOf(
-            event(
-                Press,
-                1 to touch(1f, 3f, pressed = true),
-                2 to touch(10f, 20f, pressed = false),
-                3 to touch(100f, 200f, pressed = false),
-            ),
-            event(
-                Press,
-                1 to touch(1f, 3f, pressed = true),
-                2 to touch(10f, 20f, pressed = true),
-                3 to touch(100f, 200f, pressed = false),
-            ),
-            event(
-                Press,
-                1 to touch(1f, 3f, pressed = true),
-                2 to touch(10f, 20f, pressed = true),
-                3 to touch(100f, 200f, pressed = true),
-            ),
-            event(
-                Move,
-                1 to touch(1f, 3f, pressed = true),
-                2 to touch(1f, 2f, pressed = true),
-                3 to touch(1f, 4f, pressed = true),
-            ),
-            event(
-                Move,
-                1 to touch(1f, 3f, pressed = true),
-                2 to touch(10f, 20f, pressed = true),
-                3 to touch(100f, 200f, pressed = true),
-            ),
-            event(
-                Release,
-                1 to touch(1f, 3f, pressed = false),
-                2 to touch(10f, 20f, pressed = true),
-                3 to touch(100f, 200f, pressed = true),
-            ),
-            event(
-                Release,
-                2 to touch(10f, 20f, pressed = false),
-                3 to touch(100f, 200f, pressed = true),
-            ),
-        )
-    }
-
     private fun eventsSentBy(
         vararg inputEvents: PointerInputEvent
     ): List<PointerInputEvent> {

diff --git a/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/ComposeScene.skiko.kt b/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/ComposeScene.skiko.kt
@@ -972,6 +972,7 @@ private fun pointerInputEvent(
                 it.pressed,
                 it.pressure,
                 it.type,
+                issuesEnterExit = it.type == PointerType.Mouse,
                 historical = it.historical,
                 scrollDelta = scrollDelta
             )

diff --git a/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/SyntheticEventSender.kt b/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/SyntheticEventSender.kt
@@ -27,13 +27,30 @@ import androidx.compose.ui.util.fastAny
 /**
  * Compose or user code can't work well if we miss some events.
  *
- * For example:
- * - if we miss Move before Press event with a different position
- * - if we send one event with 2 pressed touches without sending 1 pressed touch first
+ * This class generates new synthetic events based on the previous event, if something is missing.
  *
- * Platforms can send events this way.
+ * Synthetic events:
+ * 1. Synthetic Move, if we miss Move before Press/Release events with a different position
+ * for Mouse.
  *
- * This class generates new synthetic events based on the previous event, if something is missing
+ * Reason: Compose can receive a native Move and send it as Enter/Exit to the nodes.
+ * If we don't have some Move's before Press/Release, we can miss Enter/Exit.
+ *
+ * The alternative of sending synthetic moves is to send a native press/release as
+ * Enter/Exit separately from Press/Release.
+ * But this approach requires more changes - we need a separate HitPathTracker for Enter/Exit.
+ * The user code  won't see anything new with this approach
+ * (besides that Enter/Exit event will have nativeEvent.type == Release/Press)
+ *
+ * We don't send synthetic events for touch, as it doesn't have Enter/Exit, and it will be
+ * useless to send them.
+ * Besides, a Release of touch is different from a Release of mouse.
+ * Touch can be released in a different position
+ * (the finger is lifted, but we can still detect its position),
+ * Mouse can't be released in different position - we should move the cursor to this position.
+ *
+ * 2. Synthetic Press/Release if we send one event with 2 pressed touches without sending 1 pressed
+ * touch first. For example, iOS simulator can send 2 touches simultaneously.
  */
 internal class SyntheticEventSender(
     send: (PointerInputEvent) -> Unit
@@ -60,7 +77,7 @@ internal class SyntheticEventSender(
      * Send [event] and synthetic events before it if needed. On each sent event we just call [send]
      */
     fun send(event: PointerInputEvent) {
-        sendMissingMove(event)
+        sendMissingMoveForHover(event)
         sendMissingReleases(event)
         sendMissingPresses(event)
         sendInternal(event)
@@ -92,8 +109,11 @@ internal class SyntheticEventSender(
         )
     }
 
-    private fun sendMissingMove(currentEvent: PointerInputEvent) {
-        if (isMoveEventMissing(previousEvent, currentEvent)) {
+    private fun sendMissingMoveForHover(currentEvent: PointerInputEvent) {
+        // issuesEnterExit means that the pointer can issues hover events (enter/exit), and so we
+        // should generate a synthetic Move (see why we need to do that in the class description)
+        if (currentEvent.pointers.any { it.issuesEnterExit } &&
+            isMoveEventMissing(previousEvent, currentEvent)) {
             sendSyntheticMove(currentEvent)
         }
     }
@@ -116,11 +136,10 @@ internal class SyntheticEventSender(
                     type = PointerEventType.Release,
                     copyPointer = {
                         it.copySynthetic(
-                            down = if (it.id in sendingAsUp) {
-                                !sendingAsUp.contains(it.id)
-                            } else {
-                                it.down
-                            }
+                            // TODO is this a typo and it should be `it.id in newReleased`, as in sendMissingPresses?
+                            //  or maybe we can even write `down = !sendingAsUp.contains(it.id)` and `down = sendingAsDown.contains(it.id)`
+                            //  The test pass in both cases
+                            down = !sendingAsUp.contains(it.id)
                         )
                     }
                 )
@@ -145,11 +164,7 @@ internal class SyntheticEventSender(
                     type = PointerEventType.Press,
                     copyPointer = {
                         it.copySynthetic(
-                            down = if (it.id in newPressed) {
-                                sendingAsDown.contains(it.id)
-                            } else {
-                                it.down
-                            }
+                            down = sendingAsDown.contains(it.id)
                         )
                     }
                 )

diff --git a/compose/ui/ui/src/skikoTest/kotlin/androidx/compose/ui/EventTestUtils.kt b/compose/ui/ui/src/skikoTest/kotlin/androidx/compose/ui/EventTestUtils.kt
@@ -253,6 +253,7 @@ internal fun mouseEvent(
             pressure = 1f,
             type = PointerType.Mouse,
             scrollDelta = Offset.Zero,
+            issuesEnterExit = true,
             historical = emptyList()
         )
     ),