From 523b51d625cadb21e001f1e16a00998aae416827 Mon Sep 17 00:00:00 2001 From: Ivan Matkov Date: Tue, 23 Apr 2024 10:55:03 +0200 Subject: [PATCH 1/6] Use z-sorted traverse in `InteropContainer` --- .../androidx/compose/ui/node/InteropContainer.kt | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) 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 b761528452f94..1f8f17e9f0283 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 @@ -18,9 +18,8 @@ package androidx.compose.ui.node import androidx.compose.runtime.Composable import androidx.compose.ui.Modifier +import androidx.compose.ui.areObjectsOfSameType import androidx.compose.ui.layout.OverlayLayout -import androidx.compose.ui.node.TraversableNode.Companion.TraverseDescendantsAction.CancelTraversal -import androidx.compose.ui.node.TraversableNode.Companion.TraverseDescendantsAction.ContinueTraversal /** * An interface for container that controls interop views/components. @@ -43,17 +42,19 @@ internal interface InteropContainer { */ internal fun InteropContainer.countInteropComponentsBefore(nativeView: T): Int { var componentsBefore = 0 - rootModifier?.traverseDescendants { - if (it.nativeView != nativeView) { + rootModifier?.visitSubtreeIf(Nodes.Traversable, zOrder = true) { + if (TRAVERSAL_NODE_KEY == it.traverseKey && areObjectsOfSameType(this, it)) { + @Suppress("UNCHECKED_CAST") + val interopModifierNode = it as TrackInteropModifierNode + if (interopModifierNode.nativeView == nativeView) return 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 } + true } return componentsBefore } From 2adab035507d579310ee9ae7bc7feefaaadc4fc1 Mon Sep 17 00:00:00 2001 From: Ivan Matkov Date: Wed, 22 May 2024 12:05:58 +0200 Subject: [PATCH 2/6] Add native interop views only after placement --- .../ui/awt/SwingInteropContainer.desktop.kt | 20 ++- .../compose/ui/awt/SwingPanel.desktop.kt | 123 ++++++++++-------- .../compose/ui/node/InteropContainer.kt | 71 +++++++--- .../ui/interop/UIKitInteropContainer.uikit.kt | 19 ++- .../compose/ui/interop/UIKitView.uikit.kt | 6 +- .../ui/scene/ComposeSceneMediator.uikit.kt | 13 +- 6 files changed, 162 insertions(+), 90 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 cb6ab5922c437..62f17aef1a15c 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 @@ -30,6 +30,10 @@ import androidx.compose.ui.scene.ComposeSceneMediator import androidx.compose.ui.unit.IntRect import java.awt.Component import java.awt.Container +import java.awt.event.ContainerAdapter +import java.awt.event.ContainerEvent +import java.awt.event.ContainerListener +import javax.swing.SwingUtilities import org.jetbrains.skiko.ClipRectangle /** @@ -54,7 +58,7 @@ internal class SwingInteropContainer( private val placeInteropAbove: Boolean ): InteropContainer { /** - * @see SwingInteropContainer.addInteropView + * @see SwingInteropContainer.placeInteropView * @see SwingInteropContainer.removeInteropView */ private var interopComponents = mutableMapOf() @@ -63,12 +67,13 @@ internal class SwingInteropContainer( override val interopViews: Set get() = interopComponents.values.toSet() - override fun addInteropView(nativeView: InteropComponent) { + override fun placeInteropView(nativeView: InteropComponent) = SwingUtilities.invokeLater { 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 = interopComponents.size - countInteropComponentsBefore(nativeView) interopComponents[component] = nativeView + container.remove(component) container.add(component, if (placeInteropAbove) { index } else { @@ -81,7 +86,7 @@ internal class SwingInteropContainer( container.repaint() } - override fun removeInteropView(nativeView: InteropComponent) { + override fun removeInteropView(nativeView: InteropComponent) = SwingUtilities.invokeLater { val component = nativeView.container container.remove(component) interopComponents.remove(component) @@ -92,6 +97,11 @@ internal class SwingInteropContainer( container.repaint() } + fun validateComponentsOrder() = SwingUtilities.invokeLater { + container.validate() + container.repaint() + } + fun getClipRectForComponent(component: Component): ClipRectangle = requireNotNull(interopComponents[component]) @@ -113,8 +123,10 @@ internal class SwingInteropContainer( * @param component The Swing component that matches the current node. */ internal fun Modifier.trackSwingInterop( + container: SwingInteropContainer, component: InteropComponent ): Modifier = this then TrackInteropModifierElement( + container = container, nativeView = component ) @@ -126,7 +138,7 @@ internal fun Modifier.trackSwingInterop( */ internal open class InteropComponent( val container: Container, - var clipBounds: IntRect? = null + protected var clipBounds: IntRect? = null ) : ClipRectangle { override val x: Float get() = (clipBounds?.left ?: container.x).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 de44524ea243a..f2258c37bf89f 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 @@ -89,52 +89,35 @@ public fun SwingPanel( ) { val interopContainer = LocalSwingInteropContainer.current val compositeKey = currentCompositeKeyHash - val componentInfo = remember { - ComponentInfo( + val interopComponent = remember { + SwingInteropComponent( container = SwingPanelContainer( key = compositeKey, focusComponent = interopContainer.container, - ) + ), + update = update, ) } val density = LocalDensity.current val focusManager = LocalFocusManager.current - val focusSwitcher = remember { FocusSwitcher(componentInfo, focusManager) } + val focusSwitcher = remember { FocusSwitcher(interopComponent, focusManager) } OverlayLayout( modifier = modifier.onGloballyPositioned { coordinates -> val rootCoordinates = coordinates.findRootCoordinates() - val clipedBounds = rootCoordinates + val clippedBounds = 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() + interopComponent.setBounds(bounds, clippedBounds) + interopContainer.validateComponentsOrder() }.drawBehind { // Clear interop area to make visible the component under our canvas. drawRect(Color.Transparent, blendMode = BlendMode.Clear) - }.trackSwingInterop(componentInfo) - .then(InteropPointerInputModifier(componentInfo)) + }.trackSwingInterop(interopContainer, interopComponent) + .then(InteropPointerInputModifier(interopComponent)) ) { focusSwitcher.Content() } @@ -142,7 +125,7 @@ public fun SwingPanel( DisposableEffect(Unit) { val focusListener = object : FocusListener { override fun focusGained(e: FocusEvent) { - if (componentInfo.container.isParentOf(e.oppositeComponent)) { + if (interopComponent.container.isParentOf(e.oppositeComponent)) { when (e.cause) { FocusEvent.Cause.TRAVERSAL_FORWARD -> focusSwitcher.moveForward() FocusEvent.Cause.TRAVERSAL_BACKWARD -> focusSwitcher.moveBackward() @@ -154,26 +137,22 @@ public fun SwingPanel( override fun focusLost(e: FocusEvent) = Unit } interopContainer.container.addFocusListener(focusListener) - interopContainer.addInteropView(componentInfo) onDispose { - interopContainer.removeInteropView(componentInfo) + interopContainer.removeInteropView(interopComponent) interopContainer.container.removeFocusListener(focusListener) } } DisposableEffect(factory) { - componentInfo.component = factory() - componentInfo.container.add(componentInfo.component) - componentInfo.updater = Updater(componentInfo.component, update) + interopComponent.setComponent(factory()) onDispose { - componentInfo.container.remove(componentInfo.component) - componentInfo.updater.dispose() + interopComponent.dispose() } } SideEffect { - componentInfo.container.background = background.toAwtColor() - componentInfo.updater.update = update + interopComponent.container.background = background.toAwtColor() + interopComponent.update = update } } @@ -212,7 +191,7 @@ private class SwingPanelContainer( } private class FocusSwitcher( - private val info: ComponentInfo, + private val interopComponent: SwingInteropComponent, private val focusManager: FocusManager, ) { private val backwardRequester = FocusRequester() @@ -248,7 +227,9 @@ private class FocusSwitcher( if (it.isFocused && !isRequesting) { focusManager.clearFocus(force = true) - val component = info.container.focusTraversalPolicy.getFirstComponent(info.container) + val component = interopComponent.container.let { container -> + container.focusTraversalPolicy.getFirstComponent(container) + } if (component != null) { component.requestFocus(FocusEvent.Cause.TRAVERSAL_FORWARD) } else { @@ -265,7 +246,7 @@ private class FocusSwitcher( if (it.isFocused && !isRequesting) { focusManager.clearFocus(force = true) - val component = info.container.focusTraversalPolicy.getLastComponent(info.container) + val component = interopComponent.container.focusTraversalPolicy.getLastComponent(interopComponent.container) if (component != null) { component.requestFocus(FocusEvent.Cause.TRAVERSAL_BACKWARD) } else { @@ -278,11 +259,54 @@ private class FocusSwitcher( } } -private class ComponentInfo( - container: SwingPanelContainer +private class SwingInteropComponent( + container: SwingPanelContainer, + var update: (T) -> Unit ): InteropComponent(container) { - lateinit var component: T - lateinit var updater: Updater + private var component: T? = null + private var updater: Updater? = null + + fun dispose() { + container.remove(component) + updater?.dispose() + component = null + updater = null + } + + fun setComponent(component: T) { + this.component = component + container.add(component) + updater = Updater(component, update) + } + + fun setBounds( + bounds: IntRect, + clippedBounds: IntRect = bounds + ) { + clipBounds = clippedBounds // Clipping area for skia canvas + container.isVisible = !clippedBounds.isEmpty // Hide if it's fully clipped + // Swing clips children based on parent's bounds, so use our container for clipping + container.setBounds( + /* x = */ clippedBounds.left, + /* y = */ clippedBounds.top, + /* width = */ clippedBounds.width, + /* height = */ clippedBounds.height + ) + + // The real size and position should be based on not-clipped bounds + component?.setBounds( + /* x = */ bounds.left - clippedBounds.left, // Local position relative to container + /* y = */ bounds.top - clippedBounds.top, + /* width = */ bounds.width, + /* height = */ bounds.height + ) + } + + fun getDeepestComponentForEvent(event: MouseEvent): Component? { + if (component == null) return null + val point = SwingUtilities.convertPoint(event.component, event.point, component) + return SwingUtilities.getDeepestComponentAt(component, point.x, point.y) + } } private class Updater( @@ -343,7 +367,7 @@ private fun Rect.round(density: Density): IntRect { } private class InteropPointerInputModifier( - private val componentInfo: ComponentInfo, + private val interopComponent: SwingInteropComponent, ) : PointerInputFilter(), PointerInputModifier { override val pointerInputFilter: PointerInputFilter = this @@ -380,11 +404,11 @@ private class InteropPointerInputModifier( // to original component. MouseEvent.MOUSE_ENTERED, MouseEvent.MOUSE_EXITED -> return } - if (SwingUtilities.isDescendingFrom(e.component, componentInfo.container)) { + if (SwingUtilities.isDescendingFrom(e.component, interopComponent.container)) { // Do not redispatch the event if it originally from this interop view. return } - val component = getDeepestComponentForEvent(componentInfo.component, e) + val component = interopComponent.getDeepestComponentForEvent(e) if (component != null) { component.dispatchEvent(SwingUtilities.convertMouseEvent(e.component, e, component)) pointerEvent.changes.fastForEach { @@ -392,11 +416,6 @@ private class InteropPointerInputModifier( } } } - - private fun getDeepestComponentForEvent(parent: Component, event: MouseEvent): Component? { - val point = SwingUtilities.convertPoint(event.component, event.point, parent) - return SwingUtilities.getDeepestComponentAt(parent, point.x, point.y) - } } /** 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 1f8f17e9f0283..48e3b47c68198 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 @@ -19,6 +19,7 @@ package androidx.compose.ui.node import androidx.compose.runtime.Composable import androidx.compose.ui.Modifier import androidx.compose.ui.areObjectsOfSameType +import androidx.compose.ui.layout.LayoutCoordinates import androidx.compose.ui.layout.OverlayLayout /** @@ -30,7 +31,7 @@ internal interface InteropContainer { var rootModifier: TrackInteropModifierNode? val interopViews: Set - fun addInteropView(nativeView: T) + fun placeInteropView(nativeView: T) fun removeInteropView(nativeView: T) } @@ -42,57 +43,83 @@ internal interface InteropContainer { */ internal fun InteropContainer.countInteropComponentsBefore(nativeView: T): Int { var componentsBefore = 0 - rootModifier?.visitSubtreeIf(Nodes.Traversable, zOrder = true) { - if (TRAVERSAL_NODE_KEY == it.traverseKey && areObjectsOfSameType(this, it)) { - @Suppress("UNCHECKED_CAST") - val interopModifierNode = it as TrackInteropModifierNode - if (interopModifierNode.nativeView == nativeView) return componentsBefore - + rootModifier?.traverseDescendantsInDrawOrder { + if (it.nativeView != nativeView) { // 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++ } + true + } else { + false } - true } return componentsBefore } +private fun T.traverseDescendantsInDrawOrder(block: (T) -> Boolean) where T : TraversableNode { + visitSubtreeIf(Nodes.Traversable, zOrder = true) { + if (this.traverseKey == it.traverseKey && areObjectsOfSameType(this, it)) { + @Suppress("UNCHECKED_CAST") + if (!block(it as T)) return + } + true + } +} + /** * Wrapper of Compose content that might contain interop views. It adds a helper modifier to root - * that allows to traverse interop views in the tree with the right order. + * that allows traversing interop views in the tree with the right order. */ @Composable internal fun InteropContainer.TrackInteropContainer(content: @Composable () -> Unit) { OverlayLayout( - modifier = TrackInteropModifierElement { rootModifier = it }, + modifier = RootTrackInteropModifierElement { rootModifier = it }, content = content ) } /** - * A helper modifier element that tracks an interop view inside a [LayoutNode] hierarchy. + * A helper modifier element to track interop views inside a [LayoutNode] hierarchy. * - * @property nativeView The native view associated with this modifier element. * @property onModifierNodeCreated An optional block of code that to receive the reference to * [TrackInteropModifierNode]. + */ +private data class RootTrackInteropModifierElement( + val onModifierNodeCreated: (TrackInteropModifierNode) -> Unit +) : ModifierNodeElement>() { + override fun create() = TrackInteropModifierNode( + container = null, + nativeView = null + ).also { + onModifierNodeCreated.invoke(it) + } + + override fun update(node: TrackInteropModifierNode) { + } +} + +/** + * A helper modifier element that tracks an interop view inside a [LayoutNode] hierarchy. + * + * @property nativeView The native view associated with this modifier element. * @param T The type of the native view. * * @see TrackInteropModifierNode * @see ModifierNodeElement */ internal data class TrackInteropModifierElement( - var nativeView: T? = null, - val onModifierNodeCreated: ((TrackInteropModifierNode) -> Unit)? = null + var container: InteropContainer, + var nativeView: T, ) : ModifierNodeElement>() { override fun create() = TrackInteropModifierNode( + container = container, nativeView = nativeView - ).also { - onModifierNodeCreated?.invoke(it) - } + ) override fun update(node: TrackInteropModifierNode) { + node.container = container node.nativeView = nativeView } } @@ -109,7 +136,13 @@ private const val TRAVERSAL_NODE_KEY = * @see TraversableNode */ internal class TrackInteropModifierNode( - var nativeView: T? -) : Modifier.Node(), TraversableNode { + var container: InteropContainer?, + var nativeView: T?, +) : Modifier.Node(), TraversableNode, LayoutAwareModifierNode { override val traverseKey = TRAVERSAL_NODE_KEY + + override fun onPlaced(coordinates: LayoutCoordinates) { + val nativeView = nativeView ?: return + container?.placeInteropView(nativeView) + } } 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 94dad704884a3..a4e8b847720ac 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 @@ -41,15 +41,22 @@ internal val LocalUIKitInteropContainer = staticCompositionLocalOf { +internal class UIKitInteropContainer( + private val interopContext: UIKitInteropContext +): InteropContainer { val containerView: UIView = UIKitInteropContainerView() override var rootModifier: TrackInteropModifierNode? = null override var interopViews = mutableSetOf() private set - override fun addInteropView(nativeView: UIView) { + override fun placeInteropView(nativeView: UIView) = interopContext.deferAction { val index = countInteropComponentsBefore(nativeView) - interopViews.add(nativeView) + if (nativeView in interopViews) { + // Place might be called multiple times + nativeView.removeFromSuperview() + } else { + interopViews.add(nativeView) + } containerView.insertSubview(nativeView, index.toLong()) } @@ -61,8 +68,8 @@ internal class UIKitInteropContainer: InteropContainer { private class UIKitInteropContainerView: UIView(CGRectZero.readValue()) { /** - * We used simple solution to make only this view not touchable. - * Other view added to this container will be touchable. + * We used a simple solution to make only this view not touchable. + * Another view added to this container will be touchable. */ override fun hitTest(point: CValue, withEvent: UIEvent?): UIView? = super.hitTest(point, withEvent).takeIf { @@ -76,7 +83,9 @@ private class UIKitInteropContainerView: UIView(CGRectZero.readValue()) { * @param view The [UIView] that matches the current node. */ internal fun Modifier.trackUIKitInterop( + container: UIKitInteropContainer, view: UIView ): Modifier = this then TrackInteropModifierElement( + container = container, nativeView = view ) diff --git a/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/interop/UIKitView.uikit.kt b/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/interop/UIKitView.uikit.kt index 9f20adc8a6907..a178a8d30233c 100644 --- a/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/interop/UIKitView.uikit.kt +++ b/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/interop/UIKitView.uikit.kt @@ -182,7 +182,7 @@ fun UIKitView( }.drawBehind { // Clear interop area to make visible the component under our canvas. drawRect(Color.Transparent, blendMode = BlendMode.Clear) - }.trackUIKitInterop(embeddedInteropComponent.wrappingView).let { + }.trackUIKitInterop(interopContainer, embeddedInteropComponent.wrappingView).let { if (interactive) { it.then(InteropViewCatchPointerModifier()) } else { @@ -301,7 +301,7 @@ fun UIKitViewController( }.drawBehind { // Clear interop area to make visible the component under our canvas. drawRect(Color.Transparent, blendMode = BlendMode.Clear) - }.trackUIKitInterop(embeddedInteropComponent.wrappingView).let { + }.trackUIKitInterop(interopContainer, embeddedInteropComponent.wrappingView).let { if (interactive) { it.then(InteropViewCatchPointerModifier()) } else { @@ -359,7 +359,7 @@ private abstract class EmbeddedInteropComponent( protected fun addViewToHierarchy(view: UIView) { wrappingView.addSubview(view) - interopContainer.addInteropView(wrappingView) + // wrappingView will be added to the container from [onPlaced] } protected fun removeViewFromHierarchy(view: UIView) { 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 bbefb225ec976..0e467f3e40075 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 @@ -266,10 +266,15 @@ internal class ComposeSceneMediator( */ private val rootView = ComposeSceneMediatorRootUIView() + + private val interopContext = UIKitInteropContext( + requestRedraw = ::onComposeSceneInvalidate + ) + /** * Container for UIKitView and UIKitViewController */ - private val interopViewContainer = UIKitInteropContainer() + private val interopViewContainer = UIKitInteropContainer(interopContext) private val interactionBounds: IntRect get() { val boundsLayout = _layout as? SceneLayout.Bounds @@ -293,12 +298,6 @@ internal class ComposeSceneMediator( ) } - private val interopContext: UIKitInteropContext by lazy { - UIKitInteropContext( - requestRedraw = ::onComposeSceneInvalidate - ) - } - @OptIn(ExperimentalComposeApi::class) private val semanticsOwnerListener by lazy { SemanticsOwnerListenerImpl( From 1702c7f21968f822ea77e0adacfb93a75f067d7c Mon Sep 17 00:00:00 2001 From: Ivan Matkov Date: Wed, 22 May 2024 19:37:37 +0200 Subject: [PATCH 3/6] Copy DelegatableNode.visitSubtreeIf --- ...Container.kt => InteropContainer.skiko.kt} | 11 --- .../node/TraversableNodeInDrawOrder.skiko.kt | 89 +++++++++++++++++++ 2 files changed, 89 insertions(+), 11 deletions(-) rename compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/node/{InteropContainer.kt => InteropContainer.skiko.kt} (91%) create mode 100644 compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/node/TraversableNodeInDrawOrder.skiko.kt 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.skiko.kt similarity index 91% rename from compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/node/InteropContainer.kt rename to compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/node/InteropContainer.skiko.kt index 48e3b47c68198..cb20de18a73de 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.skiko.kt @@ -18,7 +18,6 @@ package androidx.compose.ui.node import androidx.compose.runtime.Composable import androidx.compose.ui.Modifier -import androidx.compose.ui.areObjectsOfSameType import androidx.compose.ui.layout.LayoutCoordinates import androidx.compose.ui.layout.OverlayLayout @@ -58,16 +57,6 @@ internal fun InteropContainer.countInteropComponentsBefore(nativeView: T) return componentsBefore } -private fun T.traverseDescendantsInDrawOrder(block: (T) -> Boolean) where T : TraversableNode { - visitSubtreeIf(Nodes.Traversable, zOrder = true) { - if (this.traverseKey == it.traverseKey && areObjectsOfSameType(this, it)) { - @Suppress("UNCHECKED_CAST") - if (!block(it as T)) return - } - true - } -} - /** * Wrapper of Compose content that might contain interop views. It adds a helper modifier to root * that allows traversing interop views in the tree with the right order. diff --git a/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/node/TraversableNodeInDrawOrder.skiko.kt b/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/node/TraversableNodeInDrawOrder.skiko.kt new file mode 100644 index 0000000000000..4c32b1ce93ec1 --- /dev/null +++ b/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/node/TraversableNodeInDrawOrder.skiko.kt @@ -0,0 +1,89 @@ +/* + * Copyright 2024 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package androidx.compose.ui.node + +import androidx.compose.runtime.collection.MutableVector +import androidx.compose.runtime.collection.mutableVectorOf +import androidx.compose.ui.Modifier +import androidx.compose.ui.areObjectsOfSameType + +internal fun T.traverseDescendantsInDrawOrder(block: (T) -> Boolean) where T : TraversableNode { + visitSubtreeIf(Nodes.Traversable, zOrder = true) { + if (this.traverseKey == it.traverseKey && areObjectsOfSameType(this, it)) { + @Suppress("UNCHECKED_CAST") + if (!block(it as T)) return + } + true + } +} + +// TODO: Remove a copy once aosp/3045224 merged + +private inline fun DelegatableNode.visitSubtreeIf( + type: NodeKind, + zOrder: Boolean = false, + block: (T) -> Boolean +) = visitSubtreeIf(type.mask, zOrder) foo@{ node -> + node.dispatchForKind(type) { + if (!block(it)) return@foo false + } + true +} + +private fun LayoutNode.getChildren(zOrder: Boolean) = + if (zOrder) { + zSortedChildren + } else { + _children + } + +private fun MutableVector.addLayoutNodeChildren( + node: Modifier.Node, + zOrder: Boolean, +) { + node.requireLayoutNode().getChildren(zOrder).forEachReversed { + add(it.nodes.head) + } +} + +private inline fun DelegatableNode.visitSubtreeIf( + mask: Int, + zOrder: Boolean, + block: (Modifier.Node) -> Boolean +) { + check(node.isAttached) { "visitSubtreeIf called on an unattached node" } + val branches = mutableVectorOf() + val child = node.child + if (child == null) + branches.addLayoutNodeChildren(node, zOrder) + else + branches.add(child) + outer@ while (branches.isNotEmpty()) { + val branch = branches.removeAt(branches.size - 1) + if (branch.aggregateChildKindSet and mask != 0) { + var node: Modifier.Node? = branch + while (node != null) { + if (node.kindSet and mask != 0) { + val diveDeeper = block(node) + if (!diveDeeper) continue@outer + } + node = node.child + } + } + branches.addLayoutNodeChildren(branch, zOrder) + } +} From b9618697af151f213863e062f64a953b0e5b4100 Mon Sep 17 00:00:00 2001 From: Ivan Matkov Date: Fri, 24 May 2024 09:10:21 +0200 Subject: [PATCH 4/6] Remove `invokeLater` from `SwingInteropContainer` --- .../compose/ui/awt/SwingInteropContainer.desktop.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 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 62f17aef1a15c..5646f006d1f2f 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 @@ -67,7 +67,7 @@ internal class SwingInteropContainer( override val interopViews: Set get() = interopComponents.values.toSet() - override fun placeInteropView(nativeView: InteropComponent) = SwingUtilities.invokeLater { + override fun placeInteropView(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 @@ -86,7 +86,7 @@ internal class SwingInteropContainer( container.repaint() } - override fun removeInteropView(nativeView: InteropComponent) = SwingUtilities.invokeLater { + override fun removeInteropView(nativeView: InteropComponent) { val component = nativeView.container container.remove(component) interopComponents.remove(component) @@ -97,7 +97,7 @@ internal class SwingInteropContainer( container.repaint() } - fun validateComponentsOrder() = SwingUtilities.invokeLater { + fun validateComponentsOrder() { container.validate() container.repaint() } From acdcd9693dbfa158ef9e8d5f3930baf01373f9d9 Mon Sep 17 00:00:00 2001 From: Ivan Matkov Date: Mon, 27 May 2024 09:22:21 +0200 Subject: [PATCH 5/6] Address feedback --- .../ui/awt/SwingInteropContainer.desktop.kt | 16 ++++---- .../compose/ui/awt/SwingPanel.desktop.kt | 38 +++++++++---------- 2 files changed, 28 insertions(+), 26 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 5646f006d1f2f..a3e91feda5ca2 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 @@ -71,14 +71,16 @@ internal class SwingInteropContainer( 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 = interopComponents.size - countInteropComponentsBefore(nativeView) - interopComponents[component] = nativeView - container.remove(component) - container.add(component, if (placeInteropAbove) { - index + var index = interopComponents.size - countInteropComponentsBefore(nativeView) + if (!placeInteropAbove) { + index += nonInteropComponents + } + if (component in interopComponents) { + container.setComponentZOrder(component, index) } else { - index + nonInteropComponents - }) + interopComponents[component] = nativeView + container.add(component, index) + } // Sometimes Swing displays the rest of interop views in incorrect order after adding, // so we need to force re-validate it. 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 f2258c37bf89f..a148292621bc4 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 @@ -144,9 +144,9 @@ public fun SwingPanel( } DisposableEffect(factory) { - interopComponent.setComponent(factory()) + interopComponent.setupUserComponent(factory()) onDispose { - interopComponent.dispose() + interopComponent.cleanUserComponent() } } @@ -227,9 +227,8 @@ private class FocusSwitcher( if (it.isFocused && !isRequesting) { focusManager.clearFocus(force = true) - val component = interopComponent.container.let { container -> - container.focusTraversalPolicy.getFirstComponent(container) - } + val container = interopComponent.container + val component = container.focusTraversalPolicy.getFirstComponent(container) if (component != null) { component.requestFocus(FocusEvent.Cause.TRAVERSAL_FORWARD) } else { @@ -263,22 +262,23 @@ private class SwingInteropComponent( container: SwingPanelContainer, var update: (T) -> Unit ): InteropComponent(container) { - private var component: T? = null + private var userComponent: T? = null private var updater: Updater? = null - fun dispose() { - container.remove(component) - updater?.dispose() - component = null - updater = null - } - - fun setComponent(component: T) { - this.component = component + fun setupUserComponent(component: T) { + check(userComponent == null) + userComponent = component container.add(component) updater = Updater(component, update) } + fun cleanUserComponent() { + container.remove(userComponent) + updater?.dispose() + userComponent = null + updater = null + } + fun setBounds( bounds: IntRect, clippedBounds: IntRect = bounds @@ -294,7 +294,7 @@ private class SwingInteropComponent( ) // The real size and position should be based on not-clipped bounds - component?.setBounds( + userComponent?.setBounds( /* x = */ bounds.left - clippedBounds.left, // Local position relative to container /* y = */ bounds.top - clippedBounds.top, /* width = */ bounds.width, @@ -303,9 +303,9 @@ private class SwingInteropComponent( } fun getDeepestComponentForEvent(event: MouseEvent): Component? { - if (component == null) return null - val point = SwingUtilities.convertPoint(event.component, event.point, component) - return SwingUtilities.getDeepestComponentAt(component, point.x, point.y) + if (userComponent == null) return null + val point = SwingUtilities.convertPoint(event.component, event.point, userComponent) + return SwingUtilities.getDeepestComponentAt(userComponent, point.x, point.y) } } From 1c194479eaf856cdac9e1ac7bfef947fff6775cd Mon Sep 17 00:00:00 2001 From: Ivan Matkov Date: Mon, 27 May 2024 10:47:17 +0200 Subject: [PATCH 6/6] Fix AWT index calculation during re-placement --- .../ui/awt/SwingInteropContainer.desktop.kt | 50 +++++++++++++------ .../compose/ui/node/InteropContainer.skiko.kt | 4 +- .../ui/interop/UIKitInteropContainer.uikit.kt | 4 +- 3 files changed, 40 insertions(+), 18 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 a3e91feda5ca2..c4041a2ae9456 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 @@ -25,15 +25,11 @@ import androidx.compose.ui.node.LayoutNode import androidx.compose.ui.node.TrackInteropContainer import androidx.compose.ui.node.TrackInteropModifierElement import androidx.compose.ui.node.TrackInteropModifierNode -import androidx.compose.ui.node.countInteropComponentsBefore +import androidx.compose.ui.node.countInteropComponentsBelow import androidx.compose.ui.scene.ComposeSceneMediator import androidx.compose.ui.unit.IntRect import java.awt.Component import java.awt.Container -import java.awt.event.ContainerAdapter -import java.awt.event.ContainerEvent -import java.awt.event.ContainerListener -import javax.swing.SwingUtilities import org.jetbrains.skiko.ClipRectangle /** @@ -67,19 +63,45 @@ internal class SwingInteropContainer( override val interopViews: Set get() = interopComponents.values.toSet() + /** + * Index of last interop component in [container]. + * + * [ComposeSceneMediator] might keep extra components in the same container. + * So based on [placeInteropAbove] they should go below or under all interop views. + * + * @see ComposeSceneMediator.contentComponent + * @see ComposeSceneMediator.invisibleComponent + */ + private val lastInteropIndex: Int + get() { + var lastInteropIndex = interopComponents.size - 1 + if (!placeInteropAbove) { + val nonInteropComponents = container.componentCount - interopComponents.size + lastInteropIndex += nonInteropComponents + } + return lastInteropIndex + } + override fun placeInteropView(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 - var index = interopComponents.size - countInteropComponentsBefore(nativeView) - if (!placeInteropAbove) { - index += nonInteropComponents + + // Add this component to [interopComponents] to track count and clip rects + val alreadyAdded = component in interopComponents + if (!alreadyAdded) { + interopComponents[component] = nativeView } - if (component in interopComponents) { - container.setComponentZOrder(component, index) + + // Iterate through a Compose layout tree in draw order and count interop view below this one + val countBelow = countInteropComponentsBelow(nativeView) + + // AWT/Swing uses the **REVERSE ORDER** for drawing and events + val awtIndex = lastInteropIndex - countBelow + + // Update AWT/Swing hierarchy + if (alreadyAdded) { + container.setComponentZOrder(component, awtIndex) } else { - interopComponents[component] = nativeView - container.add(component, index) + container.add(component, awtIndex) } // Sometimes Swing displays the rest of interop views in incorrect order after adding, diff --git a/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/node/InteropContainer.skiko.kt b/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/node/InteropContainer.skiko.kt index cb20de18a73de..dd54b1dbf121e 100644 --- a/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/node/InteropContainer.skiko.kt +++ b/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/node/InteropContainer.skiko.kt @@ -40,11 +40,11 @@ internal interface InteropContainer { * @param nativeView The native view to count interop components before. * @return The number of interop components before the given native view. */ -internal fun InteropContainer.countInteropComponentsBefore(nativeView: T): Int { +internal fun InteropContainer.countInteropComponentsBelow(nativeView: T): Int { var componentsBefore = 0 rootModifier?.traverseDescendantsInDrawOrder { if (it.nativeView != nativeView) { - // It might be inside Compose tree before adding in InteropContainer in case + // It might be inside a 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++ 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 a4e8b847720ac..985d818fabc86 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 @@ -22,7 +22,7 @@ import androidx.compose.ui.node.InteropContainer import androidx.compose.ui.node.LayoutNode import androidx.compose.ui.node.TrackInteropModifierElement import androidx.compose.ui.node.TrackInteropModifierNode -import androidx.compose.ui.node.countInteropComponentsBefore +import androidx.compose.ui.node.countInteropComponentsBelow import kotlinx.cinterop.CValue import kotlinx.cinterop.readValue import platform.CoreGraphics.CGPoint @@ -50,7 +50,7 @@ internal class UIKitInteropContainer( private set override fun placeInteropView(nativeView: UIView) = interopContext.deferAction { - val index = countInteropComponentsBefore(nativeView) + val index = countInteropComponentsBelow(nativeView) if (nativeView in interopViews) { // Place might be called multiple times nativeView.removeFromSuperview()