Skip to content

Commit 9665e6b

Browse files
authored
Redispatch unconsumed mouse wheel events (#2425) (#2438)
This is a cherry-pick (with some adaptations) of #2425 Redispatch mouse wheel events which the Compose scene did not consume. ### Difference with Swing behavior This fix introduces a slight difference with how Swing handles nested mouse-wheel scrolling. In Swing, there is essentially no nested scrolling. In a scenario with a `JScrollPane` inside another `JScrollPane`, only the inner one can be scrolled when the mouse is over it. With this PR, a `ComposePanel` with a scrollable element within a `JScrollPane` will scroll until it reaches the start/end and then the outer `JScrollPane` will scroll. This behavior seems to be an improvement, so we thought it was ok to have this difference. ### Danger, Will Robinson Note that this includes a potentially dangerous hack of whose consequences I'm not yet sure. If it proves too problematic, we will need to take a different approach (documented in the source). The hack is that in order to allow AWT to find the correct target for the redispatched event, we temporarily unregister `ComposeSceneMediator`'s mouse listeners. ### Feature flag I've put the behavior behind `ComposeFeatureFlags.redispatchUnconsumedMouseWheelEvents`. The flag will be: - `false` by default in 1.9.1 - `true` by default in 1.10 - Removed in 1.11, if no major problems are discovered. In this PR, it's `true`, as it merges into `jb-main`. In the cherry-pick to 1.9.1, we should change it to `false`. When it's approved I will add a ticket to remove it for 1.11 ### Other mouse events Note that this PR only deals with mouse wheel events, but at least in the case of a lightweight skia layer (i.e. `SkiaSwingLayer`), we should probably do the same for other mouse events. Fixes https://youtrack.jetbrains.com/issue/CMP-4601 ## Testing Added unit tests, and also tested manually with ``` import androidx.compose.foundation.* import androidx.compose.foundation.layout.* import androidx.compose.material.* import androidx.compose.runtime.Composable import androidx.compose.ui.* import androidx.compose.ui.awt.* import androidx.compose.ui.graphics.* import androidx.compose.ui.unit.* import java.awt.* import javax.swing.* fun main() = SwingUtilities.invokeLater { val frame = JFrame("CfD repro") frame.defaultCloseOperation = JFrame.EXIT_ON_CLOSE frame.minimumSize = Dimension(500, 400) System.setProperty("compose.swing.render.on.graphics", "true") val mainPanel = JPanel(null).apply { val container = JPanel(null).apply { layout = BoxLayout(this, BoxLayout.Y_AXIS) addChildren() } val scrollPane = JScrollPane(container) scrollPane.setBounds(0, 0, 500, 500) add(scrollPane) } frame.contentPane.add(mainPanel, BorderLayout.CENTER) frame.isVisible = true } private fun JPanel.addChildren() { repeat(10) { repeat(5) { add(JLabel("<html>" + "Very long text here ".repeat(10) + "</html>")) add(Box.createVerticalStrut(10)) } add( ComposePanel().apply { setContent { ComposeBox() } } ) add(Box.createVerticalStrut(10)) } } @composable fun ComposeBox() { Box( modifier = Modifier .height(100.dp) .background(color = Color(180, 180, 180)) .padding(10.dp) ) { val stateVertical = rememberScrollState(0) Box( modifier = Modifier .verticalScroll(stateVertical) .padding(end = 12.dp, bottom = 12.dp) ) { Column { for (item in 0..10) { TextBox("Item #$item") } } } } } @composable fun TextBox(text: String = "Item") { Box( modifier = Modifier.height(32.dp) .width(400.dp) .background(color = Color(0, 0, 0, 20)) .padding(start = 10.dp), contentAlignment = Alignment.CenterStart ) { Text(text = text) } } ``` This should be tested by QA ## Release Notes ### Fixes - Desktop - ComposePanel can now re-dispatch unconsumed mouse wheel events, allowing scrollable components beneath to be scrolled. To enable this behavior, set the system property `"compose.swing.redispatchMouseWheelEvents"` to `"true"`.
2 parents aada99a + cbc3c43 commit 9665e6b

File tree

4 files changed

+215
-17
lines changed

4 files changed

+215
-17
lines changed

compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/ComposeFeatureFlags.desktop.kt

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,4 +88,14 @@ internal object ComposeFeatureFlags {
8888
val useInteropBlending: Boolean by lazy {
8989
System.getProperty("compose.interop.blending").toBoolean()
9090
}
91+
92+
/**
93+
* Whether to redispatch unconsumed mouse wheel events to the parent heavyweight component.
94+
* This allows any scrollable components beneath Compose to be scrolled.
95+
*
96+
* This flag will be removed in the future, and the default behavior will correspond to a value
97+
* of `true`.
98+
*/
99+
val redispatchUnconsumedMouseWheelEvents: Boolean
100+
get() = System.getProperty("compose.swing.redispatchMouseWheelEvents", "false").toBoolean()
91101
}

compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/scene/ComposeSceneMediator.desktop.kt

Lines changed: 68 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -376,7 +376,7 @@ internal class ComposeSceneMediator(
376376

377377
contentComponent.focusTraversalKeysEnabled = false
378378

379-
subscribe(contentComponent)
379+
subscribeToInputEvents()
380380
}
381381

382382
private inline fun catchExceptions(block: () -> Unit) {
@@ -394,18 +394,22 @@ internal class ComposeSceneMediator(
394394
}
395395
}
396396

397-
private fun subscribe(component: Component) {
398-
component.addInputMethodListener(inputMethodListener)
399-
component.addFocusListener(focusListener)
400-
component.addKeyListener(keyListener)
401-
component.subscribeToMouseEvents(mouseListener)
397+
private fun subscribeToInputEvents() {
398+
with(contentComponent) {
399+
addInputMethodListener(inputMethodListener)
400+
addFocusListener(focusListener)
401+
addKeyListener(keyListener)
402+
subscribeToMouseEvents(mouseListener)
403+
}
402404
}
403405

404-
private fun unsubscribe(component: Component) {
405-
component.removeInputMethodListener(inputMethodListener)
406-
component.removeFocusListener(focusListener)
407-
component.removeKeyListener(keyListener)
408-
component.unsubscribeFromMouseEvents(mouseListener)
406+
private fun unsubscribeFromInputEvents() {
407+
with(contentComponent) {
408+
removeInputMethodListener(inputMethodListener)
409+
removeFocusListener(focusListener)
410+
removeKeyListener(keyListener)
411+
unsubscribeFromMouseEvents(mouseListener)
412+
}
409413
}
410414

411415
private var isMouseEventProcessing = false
@@ -459,8 +463,57 @@ internal class ComposeSceneMediator(
459463
if (eventListener.onMouseEvent(event)) {
460464
return
461465
}
466+
462467
processMouseEvent {
463-
scene.onMouseWheelEvent(event.position, event)
468+
val processingResult = scene.onMouseWheelEvent(event.position, event)
469+
if (!processingResult.anyChangeConsumed) {
470+
if (ComposeFeatureFlags.redispatchUnconsumedMouseWheelEvents) {
471+
redispatchUnconsumedMouseEvent(event)
472+
}
473+
}
474+
}
475+
}
476+
477+
/**
478+
* Returns the first heavyweight ancestor of the given component.
479+
*/
480+
private fun Component.heavyWeightAncestorOrNull() : Component? {
481+
var parent = parent
482+
while (parent != null) {
483+
if (!parent.isLightweight) return parent
484+
parent = parent.parent
485+
}
486+
return null
487+
}
488+
489+
/**
490+
* (Re)Dispatches the given mouse event to the component that would have received it had
491+
* this [ComposeSceneMediator] not been listening to the corresponding type of mouse events.
492+
*
493+
* The problem this attempts to solve is that [ComposeSceneMediator] has to register listeners
494+
* for all types of mouse events, even if there is nothing in the scene that listens to them.
495+
* AWT/Swing, however, interprets listening to mouse events as "interest" in them and sends them
496+
* only to the "interested" component "under" the mouse pointer.
497+
*/
498+
private fun redispatchUnconsumedMouseEvent(event: MouseEvent) {
499+
// Redispatch the event to the heavyweight ancestor, which in turn will try to find the
500+
// correct target component and send the event to it. Unregistering from mouse events
501+
// during this call allows the event to be sent to the component it would've been sent to
502+
// if ComposeSceneMediator wasn't listening to the corresponding type of mouse events.
503+
//
504+
// This is possibly a dangerous hack. If it breaks something, the alternative is to dispatch
505+
// only to the parent of the source. This isn't ideal because the "right" component may not
506+
// be the parent/ancestor, but a sibling of the source component.
507+
// With that approach, there would probably also be a need to add a flag (or multiple flags)
508+
// to ComposePanel that would control which types of events should be listened to.
509+
val source = event.component ?: return // Should be contentComponent
510+
val target = source.heavyWeightAncestorOrNull() ?: return
511+
try {
512+
unsubscribeFromInputEvents()
513+
val retargetedEvent = SwingUtilities.convertMouseEvent(source, event, target)
514+
target.dispatchEvent(retargetedEvent)
515+
} finally {
516+
subscribeToInputEvents()
464517
}
465518
}
466519

@@ -488,7 +541,7 @@ internal class ComposeSceneMediator(
488541
check(!isDisposed) { "ComposeSceneMediator is already disposed" }
489542
isDisposed = true
490543

491-
unsubscribe(contentComponent)
544+
unsubscribeFromInputEvents()
492545

493546
container.remove(contentComponent)
494547
container.remove(invisibleComponent)
@@ -814,8 +867,8 @@ internal val MouseEvent.composePointerButton: PointerButton? get() {
814867
private fun ComposeScene.onMouseWheelEvent(
815868
position: Offset,
816869
event: MouseWheelEvent
817-
) {
818-
sendPointerEvent(
870+
) : PointerEventResult {
871+
return sendPointerEvent(
819872
eventType = PointerEventType.Scroll,
820873
position = position,
821874
scrollDelta = if (event.isShiftDown) {

compose/ui/ui/src/desktopTest/kotlin/androidx/compose/ui/TestUtils.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -186,8 +186,8 @@ fun Container.sendMouseEvent(
186186
}
187187

188188
fun Container.sendMouseWheelEvent(
189-
x: Int,
190-
y: Int,
189+
x: Int = width / 2,
190+
y: Int = height / 2,
191191
scrollType: Int = MouseWheelEvent.WHEEL_UNIT_SCROLL,
192192
wheelRotation: Double = 0.0,
193193
modifiers: Int = 0,

compose/ui/ui/src/desktopTest/kotlin/androidx/compose/ui/awt/ComposePanelTest.kt

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,17 @@
1515
*/
1616
package androidx.compose.ui.awt
1717

18+
import androidx.compose.foundation.ScrollState
1819
import androidx.compose.foundation.layout.Box
20+
import androidx.compose.foundation.layout.Column
1921
import androidx.compose.foundation.layout.fillMaxSize
22+
import androidx.compose.foundation.layout.fillMaxWidth
23+
import androidx.compose.foundation.layout.height
2024
import androidx.compose.foundation.layout.requiredSize
2125
import androidx.compose.foundation.layout.size
2226
import androidx.compose.foundation.layout.sizeIn
2327
import androidx.compose.foundation.lazy.LazyColumn
28+
import androidx.compose.foundation.verticalScroll
2429
import androidx.compose.material.Text
2530
import androidx.compose.runtime.Composable
2631
import androidx.compose.runtime.LaunchedEffect
@@ -31,12 +36,15 @@ import androidx.compose.runtime.saveable.rememberSaveable
3136
import androidx.compose.runtime.setValue
3237
import androidx.compose.ui.ExperimentalComposeUiApi
3338
import androidx.compose.ui.Modifier
39+
import androidx.compose.ui.background
3440
import androidx.compose.ui.geometry.Size
41+
import androidx.compose.ui.graphics.Color
3542
import androidx.compose.ui.input.pointer.PointerEventType
3643
import androidx.compose.ui.input.pointer.onPointerEvent
3744
import androidx.compose.ui.layout.layout
3845
import androidx.compose.ui.layout.onGloballyPositioned
3946
import androidx.compose.ui.sendMouseEvent
47+
import androidx.compose.ui.sendMouseWheelEvent
4048
import androidx.compose.ui.unit.Constraints
4149
import androidx.compose.ui.unit.dp
4250
import androidx.compose.ui.unit.toSize
@@ -49,8 +57,11 @@ import java.awt.BorderLayout
4957
import java.awt.Dimension
5058
import java.awt.GraphicsEnvironment
5159
import java.awt.event.MouseEvent
60+
import javax.swing.BoxLayout
5261
import javax.swing.JFrame
5362
import javax.swing.JPanel
63+
import javax.swing.JScrollPane
64+
import javax.swing.ScrollPaneConstants
5465
import junit.framework.TestCase.assertTrue
5566
import kotlin.test.assertEquals
5667
import kotlin.test.assertFalse
@@ -545,4 +556,128 @@ class ComposePanelTest {
545556
window.dispose()
546557
}
547558
}
559+
560+
@Test
561+
fun `ComposePanel propagates unconsumed mouse wheel scroll events to parent`() {
562+
val originalFlagValue = System.setProperty("compose.swing.redispatchMouseWheelEvents", "true")
563+
try {
564+
runApplicationTest {
565+
val composePanel = ComposePanel()
566+
composePanel.preferredSize = Dimension(200, 200)
567+
val scrollState = ScrollState(0)
568+
composePanel.setContent {
569+
Box(Modifier.size(200.dp).verticalScroll(scrollState).background(Color.Yellow)) {
570+
Column(Modifier.fillMaxWidth().height(400.dp)) {
571+
Text("Hello World")
572+
Text("Hello World")
573+
Text("Hello World")
574+
Text("Hello World")
575+
Text("Hello World")
576+
}
577+
}
578+
}
579+
580+
val window = JFrame()
581+
try {
582+
window.size = Dimension(200, 200)
583+
val scrollPane = JScrollPane(
584+
JPanel().apply {
585+
layout = BoxLayout(this, BoxLayout.Y_AXIS)
586+
add(composePanel)
587+
add(javax.swing.Box.createVerticalStrut(1000), BorderLayout.CENTER)
588+
}
589+
)
590+
scrollPane.horizontalScrollBarPolicy = ScrollPaneConstants.HORIZONTAL_SCROLLBAR_NEVER
591+
window.contentPane.add(scrollPane, BorderLayout.CENTER)
592+
window.isVisible = true
593+
594+
awaitIdle()
595+
596+
// Scroll a little and check that compose content was scrolled
597+
composePanel.sendMouseWheelEvent(wheelRotation = 1.0)
598+
awaitIdle()
599+
assertThat(scrollState.value).isGreaterThan(0)
600+
601+
// Scroll a lot and check that the Swing JScrollPane was scrolled
602+
// Note that we need two scroll events for now because Compose can't partially consume
603+
// scroll events. So one event is needed to scroll Compose content to the end, and
604+
// another one to scroll JScrollPane.
605+
window.sendMouseWheelEvent(wheelRotation = 1000.0)
606+
awaitIdle()
607+
window.sendMouseWheelEvent(wheelRotation = 1000.0)
608+
assertThat(scrollPane.viewport.viewPosition.y).isGreaterThan(0)
609+
} finally {
610+
window.dispose()
611+
}
612+
}
613+
} finally {
614+
System.setProperty("compose.swing.redispatchMouseWheelEvents", originalFlagValue ?: "false")
615+
}
616+
}
617+
618+
@Test
619+
fun `ComposePanel propagates unconsumed mouse wheel scroll events to sibling`() {
620+
val originalFlagValue = System.setProperty("compose.swing.redispatchMouseWheelEvents", "true")
621+
try {
622+
runApplicationTest {
623+
val composePanel = ComposePanel()
624+
val scrollState = ScrollState(0)
625+
composePanel.setContent {
626+
Box(Modifier.size(200.dp).verticalScroll(scrollState).background(Color.Green)) {
627+
Column(Modifier.fillMaxWidth().height(400.dp)) {
628+
Text("Hello World")
629+
Text("Hello World")
630+
Text("Hello World")
631+
Text("Hello World")
632+
Text("Hello World")
633+
}
634+
}
635+
}
636+
637+
val container = JPanel(null)
638+
container.size = Dimension(200, 200)
639+
640+
val scrollPane = JScrollPane(
641+
JPanel().apply {
642+
layout = BoxLayout(this, BoxLayout.Y_AXIS)
643+
add(javax.swing.Box.createVerticalStrut(1000), BorderLayout.CENTER)
644+
}
645+
)
646+
scrollPane.horizontalScrollBarPolicy = ScrollPaneConstants.HORIZONTAL_SCROLLBAR_NEVER
647+
648+
composePanel.size = Dimension(200, 200)
649+
scrollPane.size = Dimension(200, 400)
650+
651+
val window = JFrame()
652+
try {
653+
window.size = Dimension(200, 400)
654+
container.add(composePanel)
655+
container.add(scrollPane)
656+
657+
window.contentPane.add(container, BorderLayout.CENTER)
658+
window.isVisible = true
659+
660+
awaitIdle()
661+
662+
// Scroll a little and check that compose content was scrolled
663+
composePanel.sendMouseWheelEvent(wheelRotation = 1.0)
664+
awaitIdle()
665+
assertThat(scrollState.value).isGreaterThan(0)
666+
667+
// Scroll a lot and check that the Swing JScrollPane was scrolled
668+
// Note that we need two scroll events for now because Compose can't partially consume
669+
// scroll events. So one event is needed to scroll Compose content to the end, and
670+
// another one to scroll JScrollPane.
671+
composePanel.sendMouseWheelEvent(wheelRotation = 1000.0)
672+
awaitIdle()
673+
window.sendMouseWheelEvent(wheelRotation = 1000.0)
674+
assertThat(scrollPane.viewport.viewPosition.y).isGreaterThan(0)
675+
} finally {
676+
window.dispose()
677+
}
678+
}
679+
} finally {
680+
System.setProperty("compose.swing.redispatchMouseWheelEvents", originalFlagValue ?: "false")
681+
}
682+
}
548683
}

0 commit comments

Comments
 (0)