Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hover during scrolling disappears with one frame lag #1480

Open
igordmn opened this issue Nov 27, 2021 · 2 comments
Open

Hover during scrolling disappears with one frame lag #1480

igordmn opened this issue Nov 27, 2021 · 2 comments
Assignees
Labels
desktop scroll ui glitch Visual glitch in UI, usually not blocking normal usage

Comments

@igordmn
Copy link
Collaborator

igordmn commented Nov 27, 2021

Compose 1.0.0-rc6

import androidx.compose.foundation.clickable
import androidx.compose.foundation.lazy.LazyColumn
import androidx.compose.material.Text
import androidx.compose.ui.Modifier
import androidx.compose.ui.window.singleWindowApplication

fun main() = singleWindowApplication {
    LazyColumn {
        items(count = 100) { i ->
            Text(
                text = "Item number $i",
                modifier = Modifier.clickable {}
            )
        }
    }
}
2021-11-27-71.mp4

When we scroll, we don't immediately cancel hover of the previous item - we draw it hovered for one frame.

@igordmn igordmn added ui glitch Visual glitch in UI, usually not blocking normal usage scroll desktop labels Nov 27, 2021
@igordmn igordmn self-assigned this Nov 27, 2021
@igordmn
Copy link
Collaborator Author

igordmn commented Nov 27, 2021

If we don't use hoverable, and subscribe to the events directly, there is no lag:

import androidx.compose.foundation.background
import androidx.compose.foundation.lazy.LazyColumn
import androidx.compose.material.Text
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.setValue
import androidx.compose.ui.ExperimentalComposeUiApi
import androidx.compose.ui.Modifier
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.input.pointer.PointerEventType
import androidx.compose.ui.input.pointer.onPointerEvent
import androidx.compose.ui.window.singleWindowApplication

@OptIn(ExperimentalComposeUiApi::class)
fun main() = singleWindowApplication {
    LazyColumn {
        items(count = 100) { i ->
            var isHovered by remember { mutableStateOf(false) }
            Text(
                text = "Item number $i",
                modifier = Modifier
                    .background(if (isHovered) Color.LightGray else Color.Transparent)
                    .onPointerEvent(PointerEventType.Exit) { isHovered = false }
                    .onPointerEvent(PointerEventType.Enter) { isHovered = true }
            )
        }
    }
}

I suppose this is because we launch a new coroutine in hoverable:

PointerEventType.Enter -> outerScope.launch { emitEnter() }
PointerEventType.Exit -> outerScope.launch { emitExit() }

igordmn added a commit to JetBrains/compose-multiplatform-core that referenced this issue Nov 27, 2021
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
igordmn added a commit to JetBrains/compose-multiplatform-core that referenced this issue Nov 27, 2021
@igordmn
Copy link
Collaborator Author

igordmn commented Nov 27, 2021

This commit fixes the issue, but it is fragile. Will try to improve it later

igordmn added a commit to JetBrains/compose-multiplatform-core that referenced this issue Jan 14, 2022
Call enter/exit events if we hover another popup

Fixes JetBrains/compose-multiplatform#841

Consume key events

Don't send event to focusable popup if there is no hovered popup

Don't scroll outside of focusable popup

Fixes JetBrains/compose-multiplatform#1346

Fix crash when we press right mouse button during dragging with left button

Fixes JetBrains/compose-multiplatform#1426
Fixes JetBrains/compose-multiplatform#1176

The weird behaviour doesn't reproduce anymore after cherry-picks
(this fix was reverted a few days ago here: #89)

Fix lazy scrollbar

Fixes JetBrains/compose-multiplatform#1430

Fix ScrollbarTest

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.

Fix build

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

Get rid of mousePressed from ComposeScene

mousePressed is unreliable on Windows (we can miss the release event), and doesn't work well with multiple buttons.

After this fix, mouseClickable only reacts to the first pressed button. Right button click doesn't trigger callback, if there is already left mouse button is pressed.

`Clickable` shouldn't be able to handle these cases. If users would want simultaneously handle multiple buttons, they have to use low-level api:
```
Modifier.pointerInput(Unit) {
    while (true) {
        val event = awaitPointerEventScope { awaitPointerEvent() }

        if (event.type == PointerEventType.Press && event.buttons.isPrimaryPressed) {
            // do something
        } else if (event.type == PointerEventType.Press && event.buttons.isSecondaryPressed) {
            // do something
        }
    }
}
```
(it is verbose, there is a field for improvement)

Also pass PointerButtons and PointerKeyboardModifiers to ComposeScene, instead of reading them from AWT event.

Test: ./gradlew jvmTest desktopTest -Pandroidx.compose.multiplatformEnabled=true
Test: the snippet from JetBrains/compose-multiplatform#1149, because changed the code for that fix

Revert "Remove pointerId from ComposeScene"

Remove pointerId from ComposeScene

pointerId was indroduced in https://android-review.googlesource.com/c/platform/frameworks/support/+/1402607, because double click didn't work (see https://jetbrains.slack.com/archives/GT449QBCK/p1597328095373000)

But it messes with hover and clicking multiple mouse buttons at the same time.

Double clicking still works after removing it:
```
import androidx.compose.foundation.ExperimentalFoundationApi
import androidx.compose.foundation.background
import androidx.compose.foundation.combinedClickable
import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.size
import androidx.compose.ui.Modifier
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.unit.dp
import androidx.compose.ui.window.singleWindowApplication

@OptIn(ExperimentalFoundationApi::class)
fun main() = singleWindowApplication {
    Box(
        Modifier
        .size(300.dp)
        .background(Color.Red)
            .combinedClickable(onDoubleClick = {
                println("onDoubleClick")
            }, onClick = {
                println("onClick")
            })
    ) {
    }
}
```

Fixes JetBrains/compose-multiplatform#1176

Test: ./gradlew jvmTest desktopTest -Pandroidx.compose.multiplatformEnabled=true
Test: manual (see the snippet)

Events

Fix sending events from AWT

Fix disposing window in event callback

Fixes JetBrains/compose-multiplatform#1448

The sequence of calls:
mouseReleased
  window.dispose
    scene.dispose
  cancelRippleEffect
    scene.invalidate
      layer.needRedraw

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

Fix losing hover events, speed up scroll

Fixes JetBrains/compose-multiplatform#1324

Fix crash when AWT event is sent after the window is disposed

Fixes JetBrains/compose-multiplatform#1448 (comment)

AWT can send event even after calling `window.dipose`

I checked, and it nothing to do with scheduleSyntheticMoveEvent - the crash still reproducible without it.

Remove async events

Async events is cause why sometimes interaction is so clunky:
- when we resize a heavy undecorated window from #124, it continue resizing, even if we release the mouse
- when we scroll CodeViewer, it continues to scroll for 1-2 seconds

Async events removed freezes for us for scrolling heavy lazy lists, but instead it adds unresponsive UI that lives its own live.

Also, i checked heavy lazy list (codeviewer), and it doesn't freeze anymore during scrolling

Partially fixes JetBrains/compose-multiplatform#1345

Without that we can't merge #124

1
igordmn added a commit to JetBrains/compose-multiplatform-core that referenced this issue Jan 18, 2022
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
igordmn added a commit to JetBrains/compose-multiplatform-core that referenced this issue Jan 20, 2022
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
igordmn added a commit to JetBrains/compose-multiplatform-core that referenced this issue Jan 20, 2022
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
igordmn added a commit to JetBrains/compose-multiplatform-core that referenced this issue Feb 14, 2022
`launch` will dispatch change hover state to the next coroutine frame, which can lead to some UI glitches

See, for example scroll of hoverable elements:
JetBrains/compose-multiplatform#1480

The CL won't completely fix this issue though. To completely fix the issue we need multiple fixes on Desktop/Android side.
igordmn added a commit to JetBrains/compose-multiplatform-core that referenced this issue Feb 18, 2022
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

https: //github.com/JetBrains/compose-multiplatform/issues/1324
Change-Id: I3bba1a9878626a3a8bd52d86671e3260229425fa
@igordmn igordmn removed the p:normal label Dec 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
desktop scroll ui glitch Visual glitch in UI, usually not blocking normal usage
Projects
None yet
Development

No branches or pull requests

1 participant