Skip to content

Commit

Permalink
synth
Browse files Browse the repository at this point in the history
Make real synthetic move events

I encountered code, where users doesn't use Compose events at all. They just listen to awtEvent, and check its type.

The feature "send synthetic move event on relayout", which we recently implemented, will break such use case, as synthetic event type would not reflect reality. For example, we would resend the latest native event, which was MousePressed. The application checks it and determines, that that cursor is over some button and presses it.

To fix that, the platform should provide a factory-method to create synthetic move events.

Desktop. Fix problems with Enter/Exit events

Resend events similar to Android:
https://android-review.googlesource.com/c/platform/frameworks/support/+/1856856

On desktop we don't change the native event, as we don't know the nature of it, it can be not only MouseEvent in the future.
We change only PointerEvent.
This can lead to some confusion, if user reads the native event type instead of Compose event type.

Fixes JetBrains/compose-multiplatform#523
Fixes JetBrains/compose-multiplatform#594

Manual test:
```
import androidx.compose.foundation.background
import androidx.compose.foundation.gestures.scrollBy
import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.height
import androidx.compose.foundation.layout.padding
import androidx.compose.foundation.layout.size
import androidx.compose.foundation.rememberScrollState
import androidx.compose.foundation.verticalScroll
import androidx.compose.material.Text
import androidx.compose.runtime.Composable
import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.setValue
import androidx.compose.ui.Modifier
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.input.pointer.PointerEventType
import androidx.compose.ui.input.pointer.pointerInput
import androidx.compose.ui.platform.LocalDensity
import androidx.compose.ui.unit.dp
import androidx.compose.ui.window.singleWindowApplication
import kotlinx.coroutines.delay

@composable
private fun App() {
    val state = rememberScrollState()
    Column(Modifier.size(300.dp).verticalScroll(state)) {
        repeat(20) {
            Item(it)
        }
    }
}

@composable
private fun Item(num: Int) {
    var isHovered by remember { mutableStateOf(false) }
    Box(
        Modifier
            .fillMaxWidth()
            .height(50.dp)
            .padding(8.dp)
            .background(if (isHovered) Color.Red else Color.Green)
            .pointerInput(Unit) {
                while (true) {
                    awaitPointerEventScope<Unit> {
                        val event = awaitPointerEvent()
                        when (event.type) {
                            PointerEventType.Enter -> {
                                isHovered = true
                            }
                            PointerEventType.Exit -> {
                                isHovered = false
                            }
                        }
                    }
                }
            }
    ) {
        Text(num.toString())
    }
}

fun main() {
    singleWindowApplication {
        App()
    }
}
```

Change-Id: I667c206bd08568fa0cb78208037c797bb8298702
Test: manual and ./gradlew jvmTest desktopTest -Pandroidx.compose.multiplatformEnabled=true

# Conflicts:
#	compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/input/pointer/PointerEvent.desktop.kt
#	compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/ComposeScene.skiko.kt

Fix losing Exit event on scroll

Fixes JetBrains/compose-multiplatform#1324 (comment)

Compose doesn't work well if we send an event with different coordinates without sending Move event before it:
```
Column {
  Box(size=10)
  Box(size=10)
}
```
If we send these events:
Move(5,5) -> Scroll(5,15) -> Move(5,15)

Then during the scroll event, HitPathTracker forgets about the first Box, and never send Exit to it. Instead it sends the Scroll event to it.

We should send events in this order:
Move(5,5) -> Move(5,15) -> Scroll(5,15) -> Move(5,15)

With synthetic events things more complicated, as we always send events with the same position. I suppose the proper fix should be in Compose core itself, but it would a very huge fix.

Reproducer:
```
import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.size
import androidx.compose.ui.ExperimentalComposeUiApi
import androidx.compose.ui.ImageComposeScene
import androidx.compose.ui.Modifier
import androidx.compose.ui.geometry.Offset
import androidx.compose.ui.input.pointer.PointerEventType
import androidx.compose.ui.input.pointer.onPointerEvent
import androidx.compose.ui.unit.Density
import androidx.compose.ui.unit.dp
import androidx.compose.ui.use
import com.google.common.truth.Truth.assertThat
import org.junit.Test
import org.junit.runner.RunWith
import org.junit.runners.JUnit4

@OptIn(ExperimentalComposeUiApi::class)
@RunWith(JUnit4::class)
class MouseHoverFilterTest {
    // bug: JetBrains/compose-multiplatform#1324 (comment)
    @test
    fun `move from one component to another, between another non-move event`() = ImageComposeScene(
        width = 100,
        height = 100,
        density = Density(1f)
    ).use { scene ->
        var enterCount1 = 0
        var exitCount1 = 0
        var enterCount2 = 0
        var exitCount2 = 0

        scene.setContent {
            Column {
                Box(
                    modifier = Modifier
                        .pointerMove(
                            onEnter = { enterCount1++ },
                            onExit = { exitCount1++ }
                        )
                        .size(10.dp, 10.dp)
                )
                Box(
                    modifier = Modifier
                        .pointerMove(
                            onEnter = { enterCount2++ },
                            onExit = { exitCount2++ }
                        )
                        .size(10.dp, 10.dp)
                )
            }
        }

        scene.sendPointerEvent(PointerEventType.Enter, Offset(5f, 5f))
        assertThat(enterCount1).isEqualTo(1)
        assertThat(exitCount1).isEqualTo(0)
        assertThat(enterCount2).isEqualTo(0)
        assertThat(exitCount2).isEqualTo(0)

        // Compose doesn't work well if we send an event with different type and different coordinates, without sending move event before it
        scene.sendPointerEvent(PointerEventType.Scroll, Offset(5f, 15f))
        scene.sendPointerEvent(PointerEventType.Move, Offset(5f, 15f))
        assertThat(enterCount1).isEqualTo(1)
        assertThat(exitCount1).isEqualTo(1)
        assertThat(enterCount2).isEqualTo(1)
        assertThat(exitCount2).isEqualTo(0)
    }
}
```

Another issue:
JetBrains/compose-multiplatform#1480

JetBrains/compose-multiplatform#1324
  • Loading branch information
igordmn committed Jan 18, 2022
1 parent e54bf93 commit b7b32ae
Show file tree
Hide file tree
Showing 5 changed files with 101 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ import java.awt.event.MouseWheelEvent
import java.awt.im.InputMethodRequests
import javax.accessibility.Accessible
import javax.accessibility.AccessibleContext
import javax.swing.SwingUtilities
import androidx.compose.ui.input.key.KeyEvent as ComposeKeyEvent

internal class ComposeLayer {
Expand Down Expand Up @@ -145,12 +146,21 @@ internal class ComposeLayer {
density = (this as SkiaLayer).density
this@ComposeLayer.scene.density = density
}

override fun scheduleSyntheticMoveEvent() {
needSendSyntheticMove = true
SwingUtilities.invokeLater {
if (isDisposed) return@invokeLater
flushSyntheticMoveEvent()
}
}
}

init {
_component.skikoView = object : SkikoView {
override fun onRender(canvas: Canvas, width: Int, height: Int, nanoTime: Long) {
try {
flushSyntheticMoveEvent()
scene.render(canvas, nanoTime)
} catch (e: Throwable) {
if (System.getProperty("compose.desktop.render.ignore.errors") == null) {
Expand Down Expand Up @@ -199,14 +209,85 @@ internal class ComposeLayer {
private fun onMouseEvent(event: MouseEvent) {
// AWT can send events after the window is disposed
if (isDisposed) return
checkSyntheticEvents(event)
scene.onMouseEvent(density, event)
}

private fun onMouseWheelEvent(event: MouseWheelEvent) {
if (isDisposed) return
checkSyntheticEvents(event)
scene.onMouseWheelEvent(density, event)
}

private var lastMouseEvent: MouseEvent? = null
private var needSendSyntheticMove = false

private fun flushSyntheticMoveEvent() {
val lastMouseEvent = lastMouseEvent ?: return
if (needSendSyntheticMove) {
needSendSyntheticMove = false
val source = lastMouseEvent.source as Component
val event = MouseEvent(
source,
MouseEvent.MOUSE_MOVED,
System.nanoTime(),
lastMouseEvent.modifiersEx,
lastMouseEvent.x,
lastMouseEvent.y,
0,
false
)
scene.onMouseEvent(density, event)
}
}

/**
* Compose can't work well if we miss Move event before, for example, Scroll event.
*
* This is because of the implementation of [HitPathTracker].
*
* Imaging two boxes:
* ```
* Column {
* Box(size=10)
* Box(size=10)
* }
* ```
*
* - we send Move's in the right order:
* 1. Move(5,5) -> box1 receives Enter(5,5)
* 2. Move(5,15) -> box1 receives Exit(5,15), box2 receives Enter(5,15)
* 3. Scroll(5,15) -> box2 receives Scroll(5,15)
*
* - we skip some Move's (AWT can skip them):
* 1. Move(5,5) -> box1 receives Enter(5,5)
* 2. Scroll(5,15) -> box1 receives Scroll(5,15), box2 receives Scroll(5,15)
* 3. Move(5,16) -> box2 receives Enter(5,16)
*
* You can see that box1 loses the Exit event.
*/
private fun checkSyntheticEvents(event: MouseEvent) {
val lastMouseEvent = lastMouseEvent

val isMove = event.id == MouseEvent.MOUSE_MOVED
|| event.id == MouseEvent.MOUSE_DRAGGED
|| event.id == MouseEvent.MOUSE_ENTERED
|| event.id == MouseEvent.MOUSE_EXITED

val isMoved = lastMouseEvent?.isSamePosition(event) == false

if (!isMove && isMoved) {
needSendSyntheticMove = true
}

this.lastMouseEvent = event

flushSyntheticMoveEvent()
}

private fun MouseEvent.isSamePosition(other: MouseEvent) =
x == other.x && y == other.y

private fun onKeyEvent(event: KeyEvent) {
if (isDisposed) return
if (scene.sendKeyEvent(ComposeKeyEvent(event))) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ import java.awt.Cursor
import java.awt.Point
import java.awt.im.InputMethodRequests

internal actual interface PlatformComponent : PlatformInputComponent, PlatformComponentWithCursor
internal actual interface PlatformComponent : PlatformInputComponent, PlatformComponentWithCursor {
actual fun scheduleSyntheticMoveEvent()
}

internal actual interface PlatformComponentWithCursor {
var desiredCursor: Cursor
Expand All @@ -39,4 +41,5 @@ internal actual object DummyPlatformComponent : PlatformComponent {
override val locationOnScreen = Point(0, 0)
override val density: Density
get() = Density(1f, 1f)
override fun scheduleSyntheticMoveEvent() = Unit
}
Original file line number Diff line number Diff line change
Expand Up @@ -352,10 +352,15 @@ class ComposeScene internal constructor(
) = list.indexOf(this) > list.indexOf(targetOwner)

// TODO(demin): return Boolean (when it is consumed).
// see ComposeLayer todo about AWTDebounceEventQueue
/**
* Send pointer event to the content.
*
* Don't send non-Move event with a different position without sending Move event first.
* Otherwise hover can lose Exit/Enter events.
*
* Do: Move(5,5) -> Move(15,5) -> Scroll(15,5) -> Press(15,5) -> Move(20,5) -> Release(20,5)
* Don't: Move(5,5) -> Scroll(15,5) -> Press(15,5) -> Release(20,5)
*
* @param eventType Indicates the primary reason that the event was sent.
* @param position The [Offset] of the current pointer event, relative to the content.
* @param scrollDelta scroll delta for the PointerEventType.Scroll event
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@
*/
package androidx.compose.ui.platform

internal expect interface PlatformComponent : PlatformInputComponent, PlatformComponentWithCursor
internal expect interface PlatformComponent : PlatformInputComponent, PlatformComponentWithCursor {
fun scheduleSyntheticMoveEvent()
}

internal expect interface PlatformComponentWithCursor

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,11 @@ internal class SkiaBasedOwner(

override fun measureAndLayout(sendPointerUpdate: Boolean) {
measureAndLayoutDelegate.updateRootConstraints(constraints)
if (measureAndLayoutDelegate.measureAndLayout()) {
if (
measureAndLayoutDelegate.measureAndLayout(
scheduleSyntheticEvents.takeIf { sendPointerUpdate }
)
) {
requestDraw()
}
measureAndLayoutDelegate.dispatchOnPositionedCallbacks()
Expand Down Expand Up @@ -336,6 +340,8 @@ internal class SkiaBasedOwner(
root.draw(canvas.asComposeCanvas())
}

private val scheduleSyntheticEvents = component::scheduleSyntheticMoveEvent

internal fun processPointerInput(event: PointerInputEvent): ProcessResult {
measureAndLayout()
return pointerInputEventProcessor.process(
Expand Down

0 comments on commit b7b32ae

Please sign in to comment.