From 9ba813e62c0b575fb8dff19974475919d4236463 Mon Sep 17 00:00:00 2001 From: Igor Demin Date: Tue, 18 Jan 2022 13:50:51 +0300 Subject: [PATCH] Remove pointerId and isMousePressed states from ComposeScene 1. Remove pointerId pointerId was introduced in https://android-review.googlesource.com/c/platform/frameworks/support/+/1402607, because double click didn't work But it messes with hover and clicking multiple mouse buttons at the same time. Double clicking still works after removing it. Fixes https://github.com/JetBrains/compose-jb/issues/1176 2. Remove isMousePressed mousePressed is unreliable on Windows (we can miss the release event), and doesn't work well with multiple buttons (the second pressed button resets the state). Fixes https://github.com/JetBrains/compose-jb/issues/1426 Test: ./gradlew jvmTest desktopTest -Pandroidx.compose.multiplatformEnabled=true Change-Id: Idd9d17302f41dfcb6eaa7d1259034200d107bcca # Conflicts: # compose/ui/ui/src/desktopTest/kotlin/androidx/compose/ui/awt/ComposeWindowTest.kt --- .../compose/ui/awt/ComposeWindowTest.kt | 41 ++++++----- .../ui/window/window/WindowInputEventTest.kt | 73 +++++++++++-------- .../androidx/compose/ui/ComposeScene.skiko.kt | 17 +---- 3 files changed, 68 insertions(+), 63 deletions(-) diff --git a/compose/ui/ui/src/desktopTest/kotlin/androidx/compose/ui/awt/ComposeWindowTest.kt b/compose/ui/ui/src/desktopTest/kotlin/androidx/compose/ui/awt/ComposeWindowTest.kt index 226bc0aad2ebc..2501c2daa8e19 100644 --- a/compose/ui/ui/src/desktopTest/kotlin/androidx/compose/ui/awt/ComposeWindowTest.kt +++ b/compose/ui/ui/src/desktopTest/kotlin/androidx/compose/ui/awt/ComposeWindowTest.kt @@ -45,27 +45,30 @@ import org.junit.Test class ComposeWindowTest { // bug https://github.com/JetBrains/compose-jb/issues/1448 @Test - fun `dispose window inside event handler`() = runApplicationTest { - var isClickHappened = false - + fun `dispose window in event handler`() = runApplicationTest { val window = ComposeWindow() - window.isUndecorated = true - window.size = Dimension(200, 200) - window.setContent { - Box(modifier = Modifier.fillMaxSize().background(Color.Blue).clickable { - isClickHappened = true - window.dispose() - }) + try { + var isClickHappened = false + window.size = Dimension(300, 400) + window.setContent { + Box(modifier = Modifier.fillMaxSize().background(Color.Blue).clickable { + isClickHappened = true + window.dispose() + }) + } + window.isVisible = true + window.sendMouseEvent(MOUSE_ENTERED, 100, 50) + awaitIdle() + window.sendMouseEvent(MOUSE_MOVED, 100, 50) + awaitIdle() + window.sendMouseEvent(MOUSE_PRESSED, 100, 50, modifiers = BUTTON1_DOWN_MASK) + awaitIdle() + window.sendMouseEvent(MOUSE_RELEASED, 100, 50) + awaitIdle() + assertThat(isClickHappened).isTrue() + } finally { + window.dispose() } - - window.isVisible = true - awaitIdle() - - window.sendMouseEvent(MOUSE_PRESSED, x = 100, y = 50) - window.sendMouseEvent(MOUSE_RELEASED, x = 100, y = 50) - awaitIdle() - - assertThat(isClickHappened).isTrue() } @Test diff --git a/compose/ui/ui/src/desktopTest/kotlin/androidx/compose/ui/window/window/WindowInputEventTest.kt b/compose/ui/ui/src/desktopTest/kotlin/androidx/compose/ui/window/window/WindowInputEventTest.kt index 0e7f02c779d8f..5418388d8d477 100644 --- a/compose/ui/ui/src/desktopTest/kotlin/androidx/compose/ui/window/window/WindowInputEventTest.kt +++ b/compose/ui/ui/src/desktopTest/kotlin/androidx/compose/ui/window/window/WindowInputEventTest.kt @@ -14,8 +14,6 @@ * limitations under the License. */ -@file:Suppress("DEPRECATION") // https://github.com/JetBrains/compose-jb/issues/1514 - package androidx.compose.ui.window.window import androidx.compose.foundation.layout.Box @@ -49,11 +47,19 @@ import androidx.compose.ui.window.launchApplication import androidx.compose.ui.window.rememberWindowState import androidx.compose.ui.window.runApplicationTest import com.google.common.truth.Truth.assertThat -import org.junit.Test import java.awt.Toolkit import java.awt.event.KeyEvent import java.awt.event.MouseEvent -import java.awt.event.MouseWheelEvent +import java.awt.event.MouseEvent.BUTTON1_DOWN_MASK +import java.awt.event.MouseEvent.BUTTON3_DOWN_MASK +import java.awt.event.MouseEvent.CTRL_DOWN_MASK +import java.awt.event.MouseEvent.MOUSE_DRAGGED +import java.awt.event.MouseEvent.MOUSE_PRESSED +import java.awt.event.MouseEvent.MOUSE_RELEASED +import java.awt.event.MouseEvent.MOUSE_WHEEL +import java.awt.event.MouseEvent.SHIFT_DOWN_MASK +import java.awt.event.MouseWheelEvent.WHEEL_UNIT_SCROLL +import org.junit.Test @OptIn(ExperimentalComposeUiApi::class) class WindowInputEventTest { @@ -215,8 +221,8 @@ class WindowInputEventTest { Box( Modifier.fillMaxSize().pointerInput(events) { - while (true) { - awaitPointerEventScope { + awaitPointerEventScope { + while (true) { events += awaitPointerEvent() } } @@ -229,21 +235,28 @@ class WindowInputEventTest { awaitIdle() assertThat(events.size).isEqualTo(0) - window.sendMouseEvent(MouseEvent.MOUSE_PRESSED, x = 100, y = 50) + window.sendMouseEvent(MouseEvent.MOUSE_ENTERED, x = 100, y = 50) awaitIdle() assertThat(events.size).isEqualTo(1) - assertThat(events.last().pressed).isEqualTo(true) + assertThat(events.last().pressed).isEqualTo(false) assertThat(events.last().position).isEqualTo(Offset(100 * density, 50 * density)) - window.sendMouseEvent(MouseEvent.MOUSE_DRAGGED, x = 90, y = 40) + window.sendMouseEvent(MOUSE_PRESSED, 100, 50, modifiers = BUTTON1_DOWN_MASK) awaitIdle() assertThat(events.size).isEqualTo(2) assertThat(events.last().pressed).isEqualTo(true) - assertThat(events.last().position).isEqualTo(Offset(90 * density, 40 * density)) + assertThat(events.last().position).isEqualTo(Offset(100 * density, 50 * density)) - window.sendMouseEvent(MouseEvent.MOUSE_RELEASED, x = 80, y = 30) + window.sendMouseEvent(MOUSE_DRAGGED, 90, 40, modifiers = BUTTON1_DOWN_MASK) awaitIdle() assertThat(events.size).isEqualTo(3) + assertThat(events.last().pressed).isEqualTo(true) + assertThat(events.last().position).isEqualTo(Offset(90 * density, 40 * density)) + + window.sendMouseEvent(MOUSE_RELEASED, 80, 30) + awaitIdle() + assertThat(events.size).isEqualTo(4) + assertThat(events.last().type).isEqualTo(PointerEventType.Release) assertThat(events.last().pressed).isEqualTo(false) assertThat(events.last().position).isEqualTo(Offset(80 * density, 30 * density)) @@ -296,9 +309,9 @@ class WindowInputEventTest { assertThat(onEnters).isEqualTo(1) assertThat(onExits).isEqualTo(0) - window.sendMouseEvent(MouseEvent.MOUSE_PRESSED, x = 90, y = 50) - window.sendMouseEvent(MouseEvent.MOUSE_DRAGGED, x = 80, y = 50) - window.sendMouseEvent(MouseEvent.MOUSE_RELEASED, x = 80, y = 50) + window.sendMouseEvent(MOUSE_PRESSED, x = 90, y = 50, modifiers = BUTTON1_DOWN_MASK) + window.sendMouseEvent(MOUSE_DRAGGED, x = 80, y = 50, modifiers = BUTTON1_DOWN_MASK) + window.sendMouseEvent(MOUSE_RELEASED, x = 80, y = 50) awaitIdle() assertThat(onMoves.size).isEqualTo(2) assertThat(onMoves.last()).isEqualTo(Offset(80 * density, 50 * density)) @@ -342,10 +355,10 @@ class WindowInputEventTest { assertThat(deltas.size).isEqualTo(0) window.sendMouseWheelEvent( - MouseEvent.MOUSE_WHEEL, + MOUSE_WHEEL, x = 100, y = 50, - scrollType = MouseWheelEvent.WHEEL_UNIT_SCROLL, + scrollType = WHEEL_UNIT_SCROLL, wheelRotation = 1 ) awaitIdle() @@ -353,10 +366,10 @@ class WindowInputEventTest { assertThat(deltas.last()).isEqualTo(Offset(0f, 1f)) window.sendMouseWheelEvent( - MouseEvent.MOUSE_WHEEL, + MOUSE_WHEEL, x = 100, y = 50, - scrollType = MouseWheelEvent.WHEEL_UNIT_SCROLL, + scrollType = WHEEL_UNIT_SCROLL, wheelRotation = -1 ) awaitIdle() @@ -396,10 +409,10 @@ class WindowInputEventTest { repeat(eventCount) { window.sendMouseWheelEvent( - MouseEvent.MOUSE_WHEEL, + MOUSE_WHEEL, x = 100, y = 50, - scrollType = MouseWheelEvent.WHEEL_UNIT_SCROLL, + scrollType = WHEEL_UNIT_SCROLL, wheelRotation = 1 ) } @@ -440,10 +453,10 @@ class WindowInputEventTest { repeat(eventCount) { window.sendMouseWheelEvent( - MouseEvent.MOUSE_WHEEL, + MOUSE_WHEEL, x = 100, y = 50, - scrollType = MouseWheelEvent.WHEEL_UNIT_SCROLL, + scrollType = WHEEL_UNIT_SCROLL, wheelRotation = 1 ) } @@ -486,11 +499,11 @@ class WindowInputEventTest { awaitIdle() window.sendMouseEvent( - MouseEvent.MOUSE_PRESSED, + MOUSE_PRESSED, x = 100, y = 50, - modifiers = MouseEvent.SHIFT_DOWN_MASK or MouseEvent.CTRL_DOWN_MASK or - MouseEvent.BUTTON1_DOWN_MASK or MouseEvent.BUTTON3_DOWN_MASK + modifiers = SHIFT_DOWN_MASK or CTRL_DOWN_MASK or + BUTTON1_DOWN_MASK or BUTTON3_DOWN_MASK ) awaitIdle() @@ -513,13 +526,13 @@ class WindowInputEventTest { ) window.sendMouseWheelEvent( - MouseEvent.MOUSE_WHEEL, + MOUSE_WHEEL, x = 100, y = 50, - scrollType = MouseWheelEvent.WHEEL_UNIT_SCROLL, + scrollType = WHEEL_UNIT_SCROLL, wheelRotation = 1, - modifiers = MouseEvent.SHIFT_DOWN_MASK or MouseEvent.CTRL_DOWN_MASK or - MouseEvent.BUTTON1_DOWN_MASK or MouseEvent.BUTTON3_DOWN_MASK + modifiers = SHIFT_DOWN_MASK or CTRL_DOWN_MASK or + BUTTON1_DOWN_MASK or BUTTON3_DOWN_MASK ) awaitIdle() @@ -596,4 +609,4 @@ class WindowInputEventTest { private val PointerEvent.pressed get() = changes.first().pressed private val PointerEvent.position get() = changes.first().position -} \ No newline at end of file +} 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 index ac9584456afa9..888c4e0876c51 100644 --- 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 @@ -35,6 +35,7 @@ import androidx.compose.ui.input.pointer.PointerInputEvent import androidx.compose.ui.input.pointer.PointerInputEventData import androidx.compose.ui.input.pointer.PointerKeyboardModifiers import androidx.compose.ui.input.pointer.PointerType +import androidx.compose.ui.input.pointer.areAnyPressed import androidx.compose.ui.node.LayoutNode import androidx.compose.ui.platform.AccessibilityController import androidx.compose.ui.platform.PlatformComponent @@ -166,9 +167,6 @@ class ComposeScene internal constructor( private val defaultPointerStateTracker = DefaultPointerStateTracker() - private var pointerId = 0L - private var isMousePressed = false - private val job = Job() private val coroutineScope = CoroutineScope(coroutineContext + job) // We use FlushCoroutineDispatcher for effectDispatcher not because we need `flush` for @@ -403,18 +401,12 @@ class ComposeScene internal constructor( val actualKeyboardModifiers = keyboardModifiers ?: defaultPointerStateTracker.keyboardModifiers - when (eventType) { - PointerEventType.Press -> isMousePressed = true - PointerEventType.Release -> isMousePressed = false - } val event = pointerInputEvent( eventType, position, timeMillis, nativeEvent, type, - isMousePressed, - pointerId, scrollDelta, actualButtons, actualKeyboardModifiers @@ -455,7 +447,6 @@ class ComposeScene internal constructor( private fun onMouseReleased(event: PointerInputEvent) { val owner = hoveredOwner ?: focusedOwner owner?.processPointerInput(event) - pointerId += 1 } private var pointLocation = Offset.Zero @@ -494,8 +485,6 @@ private fun pointerInputEvent( timeMillis: Long, nativeEvent: Any?, type: PointerType, - isMousePressed: Boolean, - pointerId: Long, scrollDelta: Offset, buttons: PointerButtons, keyboardModifiers: PointerKeyboardModifiers @@ -505,11 +494,11 @@ private fun pointerInputEvent( timeMillis, listOf( PointerInputEventData( - PointerId(pointerId), + PointerId(0), timeMillis, position, position, - isMousePressed, + buttons.areAnyPressed, type, scrollDelta = scrollDelta )