Revamp interop API to align it with Android and make it reusable #1489

elijah-semyonov · 2024-08-05T14:27:17Z

Refactor interop API on iOS and Desktop to make it aligned with AndroidView

Make an InteropViewHolder a type responsible for configuring and emitting a LayoutNode associated with interop view.
Introduce a TypedInteropViewHolder that allows type-bound user-provided callbacks to run and be correctly updated in the scope of InteropViewHolder.
Introduce an InteropView composable function as an entry point for all interop implementations.
Merge SwingInteropViewHolder and SwingInteropViewHolder2 and wire them with TypedInteropViewHolder.
Delete InteropViewUpdater and use interop container resident SnapshotObserver to track updates read by InteropViewHolder.
Change Swing update scheduling mechanics to batch them and then synchronously execute along with rendering sequence.
Rewire iOS implementation to use of InteropView with UIKitInteropViewControllerHolder, UIKitInteropViewHolder, and UIKitInteropElementHolder containing common parts of former two.
Tie UIViewController containment calls with place/unplace.

Release notes

iOS - Breaking changes

Actual of expected InteropView on iOS is UIResponder now instead of UIView. It's the first common ancestor for UIViewController and UIView, both of which can be integrated using iOS interop APIs.

compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/awt/SwingPanel.desktop.kt

.../ui/src/desktopMain/kotlin/androidx/compose/ui/viewinterop/SwingInteropViewHolder.desktop.kt

MatkovIvan · 2024-08-05T14:39:54Z

compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/viewinterop/InteropView.skiko.kt

+import androidx.compose.ui.platform.LocalDensity
+import androidx.compose.ui.unit.Density
+
+private val NoOp: Any.() -> Unit = {}


Why is it explicitly scoped to Any. ?

Just copied this:

private val STUB_CALLBACK_WITH_RECEIVER: Any.() -> Unit = {}

Is there a better way?

compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/viewinterop/InteropView.skiko.kt

compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/viewinterop/InteropView.uikit.kt

...se/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/viewinterop/UIKitInteropContainer.uikit.kt

...i/ui/src/uikitMain/kotlin/androidx/compose/ui/viewinterop/UIKitInteropElementHolder.uikit.kt

compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/window/InteractionUIView.uikit.kt

compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/viewinterop/InteropViewHolder.skiko.kt

compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/awt/SwingPanel.desktop.kt

...i/ui/src/desktopMain/kotlin/androidx/compose/ui/viewinterop/SwingInteropContainer.desktop.kt

compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/viewinterop/InteropViewHolder.skiko.kt

compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/interop/UIKitView.uikit.kt

compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/scene/ComposeSceneMediator.uikit.kt

compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/viewinterop/InteropView.uikit.kt

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

igordmn · 2024-08-09T11:29:14Z

The scheduling code LGTM 👍

elijah-semyonov · 2024-08-09T11:34:48Z

Waiting for CI then

elijah-semyonov · 2024-08-09T12:14:02Z

CI is green.
@igordmn @MatkovIvan
Awaiting an approval then and let's merge it.

...i/ui/src/desktopMain/kotlin/androidx/compose/ui/viewinterop/SwingInteropContainer.desktop.kt

MatkovIvan · 2024-08-12T08:50:32Z

...i/ui/src/desktopMain/kotlin/androidx/compose/ui/viewinterop/SwingInteropContainer.desktop.kt

+
+            body()
+        } finally {
+            synchronized(lock) {


mutex for bool seems as overkill. Can it be just atomic?

No, access to both needsRequestRedrawOnUpdateScheduled, scheduled is the same critical section.

MatkovIvan · 2024-08-12T08:59:45Z

compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/viewinterop/InteropContainer.skiko.kt

+     * Callback to be invoked when the layout of the interop view changes to notify the system
+     * that something has changed.
+     */
+    fun onInteropViewLayoutChange(holder: InteropViewHolder)


Looking at a way how scheduleUpdate is written on Desktop, having this separate method is questionable again. I doubt that this method is required even on Android. Do you know if implementation of scheduleUpdate will be enough for Android?

Since it's no-op for all platforms, let's comment it out with TODO (please restore TODO about Owner and merge with it)

Looking at a way how scheduleUpdate is written on Desktop, having this separate method is questionable again. I doubt that this method is required even on Android. Do you know if implementation of scheduleUpdate will be enough for Android?

Yes, I think so. If it's not enough, we will iterate on it.

Since it's no-op for all platforms, let's comment it out with TODO (please restore TODO about Owner and merge with it)

Done.

MatkovIvan · 2024-08-12T09:02:39Z

compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/viewinterop/InteropView.uikit.kt

-internal fun Modifier.interopViewSemantics(enabled: Boolean, interopViewHolder: InteropViewHolder) =
-    if (enabled) {
-        this.semantics { interopView = interopViewHolder.group as UIKitInteropViewGroup }
+internal fun Modifier.nativeAccessibility(isEnabled: Boolean, interopWrappingView: InteropWrappingView) =


TODO: align "platform" vs "native" naming

Refactor interop API on iOS and Desktop to make it aligned with `AndroidView` - Make an `InteropViewHolder` a type responsible for configuring and emitting a `LayoutNode` associated with interop view. - Introduce a `TypedInteropViewHolder` that allows type-bound user-provided callbacks to run and be correctly updated in the scope of `InteropViewHolder`. - Introduce an `InteropView` composable function as an entry point for all interop implementations. - Merge `SwingInteropViewHolder` and `SwingInteropViewHolder2` and wire them with `TypedInteropViewHolder`. - Delete `InteropViewUpdater` and use interop container resident `SnapshotObserver` to track updates read by `InteropViewHolder`. - Change Swing update scheduling mechanics to batch them and then synchronously execute along with rendering sequence. - Rewire iOS implementation to use of `InteropView` with `UIKitInteropViewControllerHolder`, `UIKitInteropViewHolder`, and `UIKitInteropElementHolder` containing common parts of former two. - Tie `UIViewController` containment calls with `place`/`unplace`. ## Release notes ### iOS - Breaking changes - Actual of expected `InteropView` on iOS is `UIResponder` now instead of `UIView`. It's the first common ancestor for `UIViewController` and `UIView`, both of which can be integrated using iOS interop APIs. --------- Co-authored-by: Ivan Matkov <ivan.matkov@jetbrains.com>

elijah-semyonov requested review from MatkovIvan, m-sasha and igordmn August 5, 2024 14:27

elijah-semyonov self-assigned this Aug 5, 2024

igordmn removed their request for review August 5, 2024 14:50

MatkovIvan reviewed Aug 5, 2024

View reviewed changes

MatkovIvan reviewed Aug 6, 2024

View reviewed changes

compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/viewinterop/InteropViewHolder.skiko.kt Outdated Show resolved Hide resolved

elijah-semyonov marked this pull request as ready for review August 6, 2024 15:18

MatkovIvan reviewed Aug 6, 2024

View reviewed changes

elijah-semyonov force-pushed the es/commonize-interop-factory-API branch from 87a1a45 to ff72fe1 Compare August 7, 2024 11:23

elijah-semyonov mentioned this pull request Aug 7, 2024

Add executeAWTQueueScheduledCommands to SkiaLayer and expose a constructor. JetBrains/skiko#974

Closed

elijah-semyonov added 19 commits August 8, 2024 11:04

Add changes

32d9cc0

Prepare stubs for integrating with iOS

5394dea

Add a couple of changes

6072c9a

Fix compilation bug

7b2e377

Wire (almost) iOS implementation

3358b7d

Wire (almost) iOS implementation

be52170

Wire (almost) iOS implementation

904992a

Wire (almost) iOS implementation

95ac1a9

Initial wiring for Swing

968f265

Roll back stuff

2bd0804

Wire UIKitView

f211306

Wire UIKitViewController

fab364c

Fix desktop red code

d31fe2a

Refactor

3d5a324

Refactor

ab03781

Make blue thing not interactive

5dbb74c

Add swing integration

212cb1e

Fix stuff

fa5ae87

Fix incorrect containment

5368451

elijah-semyonov added 12 commits August 8, 2024 11:04

Update doc

82a6c79

Rename

9549221

Add comment

ac3f7f3

Detach view controller from mediator.

4015974

Make bounds change synchronous

d6365d0

Fix formatting

97d6d72

Make name more meaningful, add comments

1a60479

Add TODO

7b32436

Separate concerns of update and onInteropViewLayoutChange

ea24b79

Rename FocusSwitcher to InteropFocusSwitcher

35daf0e

Remove invokeLater

75a5778

Adopt update scheduling strategy on Swing

a06a6a1

elijah-semyonov force-pushed the es/commonize-interop-factory-API branch from 0da8553 to a06a6a1 Compare August 8, 2024 09:04

igordmn reviewed Aug 8, 2024

View reviewed changes

compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/scene/ComposeSceneMediator.desktop.kt Outdated Show resolved Hide resolved

elijah-semyonov added 2 commits August 9, 2024 13:02

Rearrange update execution on Swing.

9d3e630

Rename

2e64754

elijah-semyonov changed the title ~~Commonize interop API to align it with Android and make it reusable~~ Revamp interop API to align it with Android and make it reusable Aug 9, 2024

MatkovIvan reviewed Aug 12, 2024

View reviewed changes

elijah-semyonov added the changes in API label Aug 12, 2024

elijah-semyonov added 4 commits August 12, 2024 11:32

Remove ScheduledUpdate

0d885d3

Add TODO about owner merge

829468d

Add missing TODO

567f8e7

Add TODO: Align "platform" vs "native" naming

2b0a312

MatkovIvan approved these changes Aug 12, 2024

View reviewed changes

elijah-semyonov merged commit 7a2dea8 into jb-main Aug 12, 2024
6 checks passed

elijah-semyonov deleted the es/commonize-interop-factory-API branch August 12, 2024 11:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Revamp interop API to align it with Android and make it reusable #1489

Revamp interop API to align it with Android and make it reusable #1489

elijah-semyonov commented Aug 5, 2024 •

edited

Loading

MatkovIvan Aug 5, 2024

elijah-semyonov Aug 6, 2024

igordmn commented Aug 9, 2024

elijah-semyonov commented Aug 9, 2024

elijah-semyonov commented Aug 9, 2024

MatkovIvan Aug 12, 2024

elijah-semyonov Aug 12, 2024 •

edited

Loading

MatkovIvan Aug 12, 2024

elijah-semyonov Aug 12, 2024

MatkovIvan Aug 12, 2024

elijah-semyonov Aug 12, 2024

Revamp interop API to align it with Android and make it reusable #1489

Revamp interop API to align it with Android and make it reusable #1489

Conversation

elijah-semyonov commented Aug 5, 2024 • edited Loading

Release notes

iOS - Breaking changes

MatkovIvan Aug 5, 2024

Choose a reason for hiding this comment

elijah-semyonov Aug 6, 2024

Choose a reason for hiding this comment

igordmn commented Aug 9, 2024

elijah-semyonov commented Aug 9, 2024

elijah-semyonov commented Aug 9, 2024

MatkovIvan Aug 12, 2024

Choose a reason for hiding this comment

elijah-semyonov Aug 12, 2024 • edited Loading

Choose a reason for hiding this comment

MatkovIvan Aug 12, 2024

Choose a reason for hiding this comment

elijah-semyonov Aug 12, 2024

Choose a reason for hiding this comment

MatkovIvan Aug 12, 2024

Choose a reason for hiding this comment

elijah-semyonov Aug 12, 2024

Choose a reason for hiding this comment

elijah-semyonov commented Aug 5, 2024 •

edited

Loading

elijah-semyonov Aug 12, 2024 •

edited

Loading