From 6b1cbe1bb844c7a0c0602031289b273431e58f28 Mon Sep 17 00:00:00 2001 From: Ivan Matkov Date: Fri, 1 Mar 2024 00:05:18 +0100 Subject: [PATCH] Fix clipping bounds of `SwingPanel` (#1147) ## Proposed Changes - Fix setting bounds - currently it relys on clipped size - Use clipped bounds for actual clipping, but not clipped for interop layout - Based on #1145 ## Testing Test: try to use `SwingPanel` inside scroll ```kt Column(Modifier.size(250.dp).verticalScroll(rememberScrollState())) { repeat(10) { SwingPanel( factory = { JButton() }, modifier = Modifier.size(100.dp) ) } } ```` Before: https://github.com/JetBrains/compose-multiplatform-core/assets/1836384/362198c2-f8fa-4004-881b-5a7a6867f0c0 After: https://github.com/JetBrains/compose-multiplatform-core/assets/1836384/546968e6-dcff-4f70-9935-0465dcb2a6e4 ## Issues Fixed Fixes https://github.com/JetBrains/compose-multiplatform/issues/3207 --- .../ui/awt/SwingInteropContainer.desktop.kt | 59 +++++++++++++------ .../compose/ui/awt/SwingPanel.desktop.kt | 54 +++++++++++++---- .../ui/scene/ComposeSceneMediator.desktop.kt | 5 +- .../compose/ui/node/InteropContainer.kt | 17 +++--- .../ui/interop/UIKitInteropContainer.uikit.kt | 4 ++ .../ui/scene/ComposeSceneMediator.uikit.kt | 1 - 6 files changed, 101 insertions(+), 39 deletions(-) diff --git a/compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/awt/SwingInteropContainer.desktop.kt b/compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/awt/SwingInteropContainer.desktop.kt index 8b903d80d42c7..4c7ee026b18e8 100644 --- a/compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/awt/SwingInteropContainer.desktop.kt +++ b/compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/awt/SwingInteropContainer.desktop.kt @@ -27,8 +27,10 @@ import androidx.compose.ui.node.TrackInteropModifierElement import androidx.compose.ui.node.TrackInteropModifierNode import androidx.compose.ui.node.countInteropComponentsBefore import androidx.compose.ui.scene.ComposeSceneMediator +import androidx.compose.ui.unit.IntRect import java.awt.Component import java.awt.Container +import org.jetbrains.skiko.ClipRectangle /** * Providing interop container as composition local, so [SwingPanel] can use it to add @@ -50,30 +52,28 @@ internal val LocalSwingInteropContainer = staticCompositionLocalOf { +): InteropContainer { /** - * Represents the count of interop components in [container]. - * - * This variable is required to add interop components to right indexes independently of - * already existing children of [container]. - * * @see SwingInteropContainer.addInteropView * @see SwingInteropContainer.removeInteropView */ - private var interopComponentsCount = 0 + private var interopComponents = mutableMapOf() - override var rootModifier: TrackInteropModifierNode? = null + override var rootModifier: TrackInteropModifierNode? = null + override val interopViews: Set + get() = interopComponents.values.toSet() - override fun addInteropView(nativeView: Component) { - val nonInteropComponents = container.componentCount - interopComponentsCount + override fun addInteropView(nativeView: InteropComponent) { + val component = nativeView.container + val nonInteropComponents = container.componentCount - interopComponents.size // AWT uses the reverse order for drawing and events, so index = size - count - val index = interopComponentsCount - countInteropComponentsBefore(nativeView) - container.add(nativeView, if (placeInteropAbove) { + val index = interopComponents.size - countInteropComponentsBefore(nativeView) + interopComponents[component] = nativeView + container.add(component, if (placeInteropAbove) { index } else { index + nonInteropComponents }) - interopComponentsCount++ // Sometimes Swing displays the rest of interop views in incorrect order after removing, // so we need to force re-validate it. @@ -81,9 +81,10 @@ internal class SwingInteropContainer( container.repaint() } - override fun removeInteropView(nativeView: Component) { - interopComponentsCount-- - container.remove(nativeView) + override fun removeInteropView(nativeView: InteropComponent) { + val component = nativeView.container + container.remove(component) + interopComponents.remove(component) // Sometimes Swing displays the rest of interop views in incorrect order after removing, // so we need to force re-validate it. @@ -91,13 +92,15 @@ internal class SwingInteropContainer( container.repaint() } + fun getClipRectForComponent(component: Component): ClipRectangle = + requireNotNull(interopComponents[component]) + @Composable operator fun invoke(content: @Composable () -> Unit) { CompositionLocalProvider( LocalSwingInteropContainer provides this, ) { TrackInteropContainer( - container = container, content = content ) } @@ -110,7 +113,27 @@ internal class SwingInteropContainer( * @param component The Swing component that matches the current node. */ internal fun Modifier.trackSwingInterop( - component: Component + component: InteropComponent ): Modifier = this then TrackInteropModifierElement( nativeView = component ) + +/** + * Provides clipping bounds for skia canvas. + * + * @param container The container that holds the component. + * @param clipBounds The rectangular region to clip skia canvas. It's relative to Compose root + */ +internal open class InteropComponent( + val container: Container, + var clipBounds: IntRect? = null +) : ClipRectangle { + override val x: Float + get() = (clipBounds?.left ?: container.x).toFloat() + override val y: Float + get() = (clipBounds?.top ?: container.y).toFloat() + override val width: Float + get() = (clipBounds?.width ?: container.width).toFloat() + override val height: Float + get() = (clipBounds?.height ?: container.height).toFloat() +} diff --git a/compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/awt/SwingPanel.desktop.kt b/compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/awt/SwingPanel.desktop.kt index 7b43558959ad9..de44524ea243a 100644 --- a/compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/awt/SwingPanel.desktop.kt +++ b/compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/awt/SwingPanel.desktop.kt @@ -41,7 +41,7 @@ import androidx.compose.ui.input.pointer.changedToDownIgnoreConsumed import androidx.compose.ui.input.pointer.changedToUpIgnoreConsumed import androidx.compose.ui.layout.EmptyLayout import androidx.compose.ui.layout.OverlayLayout -import androidx.compose.ui.layout.boundsInRoot +import androidx.compose.ui.layout.findRootCoordinates import androidx.compose.ui.layout.onGloballyPositioned import androidx.compose.ui.platform.LocalDensity import androidx.compose.ui.platform.LocalFocusManager @@ -50,7 +50,6 @@ import androidx.compose.ui.unit.IntRect import androidx.compose.ui.unit.IntSize import androidx.compose.ui.util.fastAny import androidx.compose.ui.util.fastForEach -import java.awt.BorderLayout import java.awt.Component import java.awt.Container import java.awt.event.FocusEvent @@ -105,14 +104,36 @@ public fun SwingPanel( OverlayLayout( modifier = modifier.onGloballyPositioned { coordinates -> - val bounds = coordinates.boundsInRoot().round(density) - componentInfo.container.setBounds(bounds.left, bounds.top, bounds.width, bounds.height) + val rootCoordinates = coordinates.findRootCoordinates() + val clipedBounds = rootCoordinates + .localBoundingBoxOf(coordinates, clipBounds = true).round(density) + val bounds = rootCoordinates + .localBoundingBoxOf(coordinates, clipBounds = false).round(density) + + // Take care about clipped bounds + componentInfo.clipBounds = clipedBounds // Clipping area for skia canvas + componentInfo.container.isVisible = !clipedBounds.isEmpty // Hide if it's fully clipped + // Swing clips children based on parent's bounds, so use our container for clipping + componentInfo.container.setBounds( + /* x = */ clipedBounds.left, + /* y = */ clipedBounds.top, + /* width = */ clipedBounds.width, + /* height = */ clipedBounds.height + ) + + // The real size and position should be based on not-clipped bounds + componentInfo.component.setBounds( + /* x = */ bounds.left - clipedBounds.left, // Local position relative to container + /* y = */ bounds.top - clipedBounds.top, + /* width = */ bounds.width, + /* height = */ bounds.height + ) componentInfo.container.validate() componentInfo.container.repaint() }.drawBehind { // Clear interop area to make visible the component under our canvas. drawRect(Color.Transparent, blendMode = BlendMode.Clear) - }.trackSwingInterop(componentInfo.container) + }.trackSwingInterop(componentInfo) .then(InteropPointerInputModifier(componentInfo)) ) { focusSwitcher.Content() @@ -133,9 +154,9 @@ public fun SwingPanel( override fun focusLost(e: FocusEvent) = Unit } interopContainer.container.addFocusListener(focusListener) - interopContainer.addInteropView(componentInfo.container) + interopContainer.addInteropView(componentInfo) onDispose { - interopContainer.removeInteropView(componentInfo.container) + interopContainer.removeInteropView(componentInfo) interopContainer.container.removeFocusListener(focusListener) } } @@ -156,13 +177,19 @@ public fun SwingPanel( } } +/** + * A container for [SwingPanel]'s component. Takes care about focus and clipping. + * + * @param key The unique identifier for the panel container. + * @param focusComponent The component that should receive focus. + */ private class SwingPanelContainer( key: Int, private val focusComponent: Component ): JPanel() { init { - name = "SwingPanel #$key" - layout = BorderLayout(0, 0) + name = "SwingPanel #${key.toString(MaxSupportedRadix)}" + layout = null focusTraversalPolicy = object : LayoutFocusTraversalPolicy() { override fun getComponentAfter(aContainer: Container?, aComponent: Component?): Component? { return if (aComponent == getLastComponent(aContainer)) { @@ -252,8 +279,8 @@ private class FocusSwitcher( } private class ComponentInfo( - val container: Container -) { + container: SwingPanelContainer +): InteropComponent(container) { lateinit var component: T lateinit var updater: Updater } @@ -371,3 +398,8 @@ private class InteropPointerInputModifier( return SwingUtilities.getDeepestComponentAt(parent, point.x, point.y) } } + +/** + * The maximum radix available for conversion to and from strings. + */ +private val MaxSupportedRadix = 36 diff --git a/compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/scene/ComposeSceneMediator.desktop.kt b/compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/scene/ComposeSceneMediator.desktop.kt index 3bd2a2c07449e..a40f613b3a45e 100644 --- a/compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/scene/ComposeSceneMediator.desktop.kt +++ b/compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/scene/ComposeSceneMediator.desktop.kt @@ -78,6 +78,7 @@ import kotlin.coroutines.CoroutineContext import kotlin.math.roundToInt import org.jetbrains.skia.Canvas import org.jetbrains.skiko.ClipComponent +import org.jetbrains.skiko.ClipRectangle import org.jetbrains.skiko.ExperimentalSkikoApi import org.jetbrains.skiko.GraphicsApi import org.jetbrains.skiko.SkikoInput @@ -151,7 +152,7 @@ internal class ComposeSceneMediator( ) private val containerListener = object : ContainerListener { - private val clipMap = mutableMapOf() + private val clipMap = mutableMapOf() override fun componentAdded(e: ContainerEvent) { val component = e.child @@ -176,7 +177,7 @@ internal class ComposeSceneMediator( } private fun addClipComponent(component: Component) { - val clipRectangle = ClipComponent(component) + val clipRectangle = interopContainer.getClipRectForComponent(component) clipMap[component] = clipRectangle skiaLayerComponent.clipComponents.add(clipRectangle) } diff --git a/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/node/InteropContainer.kt b/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/node/InteropContainer.kt index 0fe58b4f5e51e..b761528452f94 100644 --- a/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/node/InteropContainer.kt +++ b/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/node/InteropContainer.kt @@ -29,6 +29,7 @@ import androidx.compose.ui.node.TraversableNode.Companion.TraverseDescendantsAct */ internal interface InteropContainer { var rootModifier: TrackInteropModifierNode? + val interopViews: Set fun addInteropView(nativeView: T) fun removeInteropView(nativeView: T) @@ -44,7 +45,11 @@ internal fun InteropContainer.countInteropComponentsBefore(nativeView: T) var componentsBefore = 0 rootModifier?.traverseDescendants { if (it.nativeView != nativeView) { - componentsBefore++ + // It might be inside Compose tree before adding in InteropContainer in case + // if it was initiated out of scroll visible bounds for example. + if (it.nativeView in interopViews) { + componentsBefore++ + } ContinueTraversal } else { CancelTraversal @@ -58,11 +63,9 @@ internal fun InteropContainer.countInteropComponentsBefore(nativeView: T) * that allows to traverse interop views in the tree with the right order. */ @Composable -internal fun InteropContainer.TrackInteropContainer(container: T, content: @Composable () -> Unit) { +internal fun InteropContainer.TrackInteropContainer(content: @Composable () -> Unit) { OverlayLayout( - modifier = TrackInteropModifierElement( - nativeView = container - ) { rootModifier = it }, + modifier = TrackInteropModifierElement { rootModifier = it }, content = content ) } @@ -79,7 +82,7 @@ internal fun InteropContainer.TrackInteropContainer(container: T, content * @see ModifierNodeElement */ internal data class TrackInteropModifierElement( - var nativeView: T, + var nativeView: T? = null, val onModifierNodeCreated: ((TrackInteropModifierNode) -> Unit)? = null ) : ModifierNodeElement>() { override fun create() = TrackInteropModifierNode( @@ -105,7 +108,7 @@ private const val TRAVERSAL_NODE_KEY = * @see TraversableNode */ internal class TrackInteropModifierNode( - var nativeView: T + var nativeView: T? ) : Modifier.Node(), TraversableNode { override val traverseKey = TRAVERSAL_NODE_KEY } diff --git a/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/interop/UIKitInteropContainer.uikit.kt b/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/interop/UIKitInteropContainer.uikit.kt index ccfd9b2c6674c..94dad704884a3 100644 --- a/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/interop/UIKitInteropContainer.uikit.kt +++ b/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/interop/UIKitInteropContainer.uikit.kt @@ -44,14 +44,18 @@ internal val LocalUIKitInteropContainer = staticCompositionLocalOf { val containerView: UIView = UIKitInteropContainerView() override var rootModifier: TrackInteropModifierNode? = null + override var interopViews = mutableSetOf() + private set override fun addInteropView(nativeView: UIView) { val index = countInteropComponentsBefore(nativeView) + interopViews.add(nativeView) containerView.insertSubview(nativeView, index.toLong()) } override fun removeInteropView(nativeView: UIView) { nativeView.removeFromSuperview() + interopViews.remove(nativeView) } } diff --git a/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/scene/ComposeSceneMediator.uikit.kt b/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/scene/ComposeSceneMediator.uikit.kt index a40829fc7efe0..8e4b3ef610766 100644 --- a/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/scene/ComposeSceneMediator.uikit.kt +++ b/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/scene/ComposeSceneMediator.uikit.kt @@ -448,7 +448,6 @@ internal class ComposeSceneMediator( if (renderingView.isReadyToShowContent.value) { ProvideComposeSceneMediatorCompositionLocals { interopViewContainer.TrackInteropContainer( - container = interopViewContainer.containerView, content = content ) }