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 JetBrains/compose-multiplatform#3207
JetBrains · Feb 29, 2024 · 6b1cbe1 · 6b1cbe1
1 parent 339dbd3
commit 6b1cbe1
Show file tree

Hide file tree

Showing 6 changed files with 101 additions and 39 deletions.
diff --git a/...ose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/awt/SwingInteropContainer.desktop.kt b/...ose/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,54 +52,55 @@ internal val LocalSwingInteropContainer = staticCompositionLocalOf<SwingInteropC
 internal class SwingInteropContainer(
     val container: Container,
     private val placeInteropAbove: Boolean
-): InteropContainer<Component> {
+): InteropContainer<InteropComponent> {
     /**
-     * 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<Component, InteropComponent>()
 
-    override var rootModifier: TrackInteropModifierNode<Component>? = null
+    override var rootModifier: TrackInteropModifierNode<InteropComponent>? = null
+    override val interopViews: Set<InteropComponent>
+        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.
         container.validate()
         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.
         container.validate()
         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
@@ -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 <T : Component> 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 <T : Component> 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 <T : Component> 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<T : Component>(
 }
 
 private class ComponentInfo<T : Component>(
-    val container: Container
-) {
+    container: SwingPanelContainer
+): InteropComponent(container) {
     lateinit var component: T
     lateinit var updater: Updater<T>
 }
@@ -371,3 +398,8 @@ private class InteropPointerInputModifier<T : Component>(
         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/...se/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/scene/ComposeSceneMediator.desktop.kt b/...se/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<Component, ClipComponent>()
+        private val clipMap = mutableMapOf<Component, ClipRectangle>()
 
         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
@@ -29,6 +29,7 @@ import androidx.compose.ui.node.TraversableNode.Companion.TraverseDescendantsAct
  */
 internal interface InteropContainer<T> {
     var rootModifier: TrackInteropModifierNode<T>?
+    val interopViews: Set<T>
 
     fun addInteropView(nativeView: T)
     fun removeInteropView(nativeView: T)
@@ -44,7 +45,11 @@ internal fun <T> InteropContainer<T>.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 <T> InteropContainer<T>.countInteropComponentsBefore(nativeView: T)
  * that allows to traverse interop views in the tree with the right order.
  */
 @Composable
-internal fun <T> InteropContainer<T>.TrackInteropContainer(container: T, content: @Composable () -> Unit) {
+internal fun <T> InteropContainer<T>.TrackInteropContainer(content: @Composable () -> Unit) {
     OverlayLayout(
-        modifier = TrackInteropModifierElement(
-            nativeView = container
-        ) { rootModifier = it },
+        modifier = TrackInteropModifierElement { rootModifier = it },
         content = content
     )
 }
@@ -79,7 +82,7 @@ internal fun <T> InteropContainer<T>.TrackInteropContainer(container: T, content
  * @see ModifierNodeElement
  */
 internal data class TrackInteropModifierElement<T>(
-    var nativeView: T,
+    var nativeView: T? = null,
     val onModifierNodeCreated: ((TrackInteropModifierNode<T>) -> Unit)? = null
 ) : ModifierNodeElement<TrackInteropModifierNode<T>>() {
     override fun create() = TrackInteropModifierNode(
@@ -105,7 +108,7 @@ private const val TRAVERSAL_NODE_KEY =
  * @see TraversableNode
  */
 internal class TrackInteropModifierNode<T>(
-    var nativeView: T
+    var nativeView: T?
 ) : Modifier.Node(), TraversableNode {
     override val traverseKey = TRAVERSAL_NODE_KEY
 }
diff --git a/...ose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/interop/UIKitInteropContainer.uikit.kt b/...ose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/interop/UIKitInteropContainer.uikit.kt
@@ -44,14 +44,18 @@ internal val LocalUIKitInteropContainer = staticCompositionLocalOf<UIKitInteropC
 internal class UIKitInteropContainer: InteropContainer<UIView> {
     val containerView: UIView = UIKitInteropContainerView()
     override var rootModifier: TrackInteropModifierNode<UIView>? = null
+    override var interopViews = mutableSetOf<UIView>()
+        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
@@ -448,7 +448,6 @@ internal class ComposeSceneMediator(
                 if (renderingView.isReadyToShowContent.value) {
                     ProvideComposeSceneMediatorCompositionLocals {
                         interopViewContainer.TrackInteropContainer(
-                            container = interopViewContainer.containerView,
                             content = content
                         )
                     }