diff --git a/compose/ui/ui-test/src/skikoMain/kotlin/androidx/compose/ui/test/ComposeUiTest.skiko.kt b/compose/ui/ui-test/src/skikoMain/kotlin/androidx/compose/ui/test/ComposeUiTest.skiko.kt index f8e83abec3b35..656c18fcb0667 100644 --- a/compose/ui/ui-test/src/skikoMain/kotlin/androidx/compose/ui/test/ComposeUiTest.skiko.kt +++ b/compose/ui/ui-test/src/skikoMain/kotlin/androidx/compose/ui/test/ComposeUiTest.skiko.kt @@ -387,7 +387,6 @@ open class SkikoComposeUiTest @InternalTestApi constructor( return !Snapshot.current.hasPendingChanges() && !Snapshot.isApplyObserverNotificationPending && !scene.hasPendingMeasureOrLayout - && !scene.hasPendingSnapshotCommands && areAllResourcesIdle() } diff --git a/compose/ui/ui/src/desktopTest/kotlin/androidx/compose/ui/ComposeSceneTest.kt b/compose/ui/ui/src/desktopTest/kotlin/androidx/compose/ui/ComposeSceneTest.kt index 69ee6b0b2a5e2..c1b1e6a604e33 100644 --- a/compose/ui/ui/src/desktopTest/kotlin/androidx/compose/ui/ComposeSceneTest.kt +++ b/compose/ui/ui/src/desktopTest/kotlin/androidx/compose/ui/ComposeSceneTest.kt @@ -108,6 +108,7 @@ import org.junit.Assert.assertFalse import org.junit.Ignore import org.junit.Rule import org.junit.Test +import org.junit.rules.Timeout @OptIn(InternalTestApi::class, ExperimentalComposeUiApi::class) class ComposeSceneTest { @@ -117,6 +118,9 @@ class ComposeSceneTest { @get:Rule val composeRule = createComposeRule() + @get:Rule // A timeout inside @Test annotation does not always work + val timeout: Timeout = Timeout.seconds(60) + private fun ScreenshotTestRule.snap(surface: Surface, idSuffix: String? = null) { assertImageAgainstGolden(surface.makeImageSnapshot(), idSuffix) } diff --git a/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/layout/OffsetToFocusedRect.skiko.kt b/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/layout/OffsetToFocusedRect.skiko.kt index 7ff300e2e6863..eeb0d1a4b1aa1 100644 --- a/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/layout/OffsetToFocusedRect.skiko.kt +++ b/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/layout/OffsetToFocusedRect.skiko.kt @@ -97,6 +97,10 @@ internal fun OffsetToFocusedRect( // Intentionally update state within composition to trigger second measure and // layout because focus rect may be miscalculated due to simultaneous offset and // window insets changes. + // + // FIXME: this "second measure" only settles in-frame because BaseComposeScene.draw() + // currently calls Snapshot.sendApplyNotifications() between the measure and draw phases - + // a temporary, un-Android workaround kept solely for this code path. currentOffset = startOffset + (endOffset - startOffset) * offsetProgress val placeables = measurables.fastMap { it.measure(constraints) } diff --git a/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/node/RootNodeOwner.skiko.kt b/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/node/RootNodeOwner.skiko.kt index 08954ec7b4343..fd030804235ac 100644 --- a/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/node/RootNodeOwner.skiko.kt +++ b/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/node/RootNodeOwner.skiko.kt @@ -99,6 +99,7 @@ import androidx.compose.ui.util.trace import androidx.compose.ui.viewinterop.InteropPointerInputModifier import androidx.compose.ui.viewinterop.InteropView import androidx.compose.ui.viewinterop.pointerInteropFilter +import kotlin.concurrent.Volatile import kotlin.coroutines.CoroutineContext import kotlin.math.max import kotlin.math.min @@ -122,14 +123,15 @@ internal class RootNodeOwner( size: IntSize?, coroutineContext: CoroutineContext, val platformContext: PlatformContext, - private val snapshotInvalidationTracker: SnapshotInvalidationTracker, private val inputHandler: ComposeSceneInputHandler, + private val invalidate: () -> Unit, + onChangedExecutor: (callback: () -> Unit) -> Unit, ) { val focusOwner: FocusOwner get() = _owner.focusOwner val dragAndDropOwner = DragAndDropOwner(platformContext.dragAndDropManager) private val rootSemanticsNode = EmptySemanticsModifier() - private val snapshotObserver = snapshotInvalidationTracker.snapshotObserver() + private val snapshotObserver = OwnerSnapshotObserver(onChangedExecutor) private val graphicsContext = SkiaGraphicsContext(platformContext.measureDrawLayerBounds) private val coroutineScope = CoroutineScope(coroutineContext + Job(parent = coroutineContext[Job])) @@ -155,6 +157,14 @@ internal class RootNodeOwner( owner.root.layoutDirection = value } + @Volatile + var hasPendingMeasureOrLayout: Boolean = true + private set + + @Volatile + var hasPendingDraw: Boolean = true + private set + private val rootForTest by lazy(LazyThreadSafetyMode.NONE) { PlatformRootForTestImpl() } @@ -233,6 +243,7 @@ internal class RootNodeOwner( fun measureAndLayout() { require(!isDisposed) { "RootNodeOwner is already disposed" } + hasPendingMeasureOrLayout = false owner.measureAndLayout(sendPointerUpdate = true) updatePositionCacheAndDispatch() } @@ -293,12 +304,23 @@ internal class RootNodeOwner( fun draw(canvas: Canvas) { require(!isDisposed) { "RootNodeOwner is already disposed" } trace("RootNodeOwner:draw") { + hasPendingDraw = false ownedLayerManager.draw(canvas) clearInvalidObservations() owner.rectManager.dispatchCallbacks() } } + private fun requestMeasureAndLayout() { + hasPendingMeasureOrLayout = true + invalidate() + } + + private fun requestDraw() { + hasPendingDraw = true + invalidate() + } + fun setRootModifier(modifier: Modifier) { owner.root.modifier = _owner.rootModifier then modifier } @@ -306,7 +328,7 @@ internal class RootNodeOwner( private fun onRootSizeChanged(size: IntSize?) { measureAndLayoutDelegate.updateRootConstraints(size.toMaxConstraints()) if (measureAndLayoutDelegate.hasPendingMeasureOrLayout) { - snapshotInvalidationTracker.requestMeasureAndLayout() + requestMeasureAndLayout() } } @@ -573,7 +595,7 @@ internal class RootNodeOwner( val resend = if (sendPointerUpdate) onPointerUpdateCallback else null val rootNodeResized = measureAndLayoutDelegate.measureAndLayout(resend) if (rootNodeResized) { - snapshotInvalidationTracker.requestDraw() + requestDraw() } measureAndLayoutDelegate.dispatchOnPositionedCallbacks() rectManager.dispatchCallbacks() @@ -609,12 +631,12 @@ internal class RootNodeOwner( if (measureAndLayoutDelegate.requestLookaheadRemeasure(layoutNode, forceRequest) && scheduleMeasureAndLayout ) { - snapshotInvalidationTracker.requestMeasureAndLayout() + requestMeasureAndLayout() } } else if (measureAndLayoutDelegate.requestRemeasure(layoutNode, forceRequest) && scheduleMeasureAndLayout ) { - snapshotInvalidationTracker.requestMeasureAndLayout() + requestMeasureAndLayout() } } @@ -625,18 +647,18 @@ internal class RootNodeOwner( ) { if (affectsLookahead) { if (measureAndLayoutDelegate.requestLookaheadRelayout(layoutNode, forceRequest)) { - snapshotInvalidationTracker.requestMeasureAndLayout() + requestMeasureAndLayout() } } else { if (measureAndLayoutDelegate.requestRelayout(layoutNode, forceRequest)) { - snapshotInvalidationTracker.requestMeasureAndLayout() + requestMeasureAndLayout() } } } override fun requestOnPositionedCallback(layoutNode: LayoutNode) { measureAndLayoutDelegate.requestOnPositionedCallback(layoutNode) - snapshotInvalidationTracker.requestMeasureAndLayout() + requestMeasureAndLayout() } override fun createLayer( @@ -703,13 +725,6 @@ internal class RootNodeOwner( private val endApplyChangesListeners = mutableVectorOf<(() -> Unit)?>() override fun onEndApplyChanges() { - // Android's OwnerSnapshotObserver runs callbacks immediately when apply changes - // happens on the view handler thread. Non-Android queues off-thread owner callbacks in - // the scene-local tracker, so drain them here before clearing invalid observations and - // invoking end-apply listeners. - // This preserves the previous render-time synchronous observer ordering - // after recomposition moved to FrameRecomposer. - snapshotInvalidationTracker.performSnapshotChanges() clearInvalidObservations() // Listeners can add more items to the list and we want to ensure that they @@ -736,7 +751,7 @@ internal class RootNodeOwner( override fun registerOnLayoutCompletedListener(listener: Owner.OnLayoutCompletedListener) { measureAndLayoutDelegate.registerOnLayoutCompletedListener(listener) - snapshotInvalidationTracker.requestMeasureAndLayout() + requestMeasureAndLayout() } override fun voteFrameRate(frameRate: Float) { @@ -942,7 +957,7 @@ internal class RootNodeOwner( } override fun invalidate() { - snapshotInvalidationTracker.requestDraw() + requestDraw() } private var currentFrameRate = Float.NaN diff --git a/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/node/SnapshotInvalidationTracker.skiko.kt b/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/node/SnapshotInvalidationTracker.skiko.kt deleted file mode 100644 index 55a146751b98e..0000000000000 --- a/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/node/SnapshotInvalidationTracker.skiko.kt +++ /dev/null @@ -1,151 +0,0 @@ -/* - * Copyright 2023 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.ui.platform.makeSynchronizedObject -import androidx.compose.ui.internal.getCurrentThreadId -import androidx.compose.ui.platform.synchronized -import androidx.compose.ui.util.fastForEach -import kotlinx.atomicfu.atomic - -/** - * SnapshotCommandList is a class that manages commands and invalidations for snapshot-based recomposition. - * It allows postponing execution of commands and performing them in the future. - * - * @param invalidate a function that is called whenever an invalidation is requested - */ -internal class SnapshotInvalidationTracker( - private val invalidate: () -> Unit = {} -) { - private val snapshotChanges = CommandList(invalidate) - - /** - * The id of the thread currently inside [performSnapshotChangesSynchronously]. - * - * Note that it's not valid to have more than one thread calling it at the same time. - */ - private var renderingThreadId: Long? by atomic(null) - - val hasPendingSnapshotCommands: Boolean - get() = snapshotChanges.hasCommands - - var hasPendingMeasureOrLayout: Boolean = true - private set - - var hasPendingDraw: Boolean = true - private set - - fun requestMeasureAndLayout() { - hasPendingMeasureOrLayout = true - invalidate() - } - - fun onMeasureAndLayout() { - hasPendingMeasureOrLayout = false - } - - fun requestDraw() { - hasPendingDraw = true - invalidate() - } - - fun onDraw() { - hasPendingDraw = false - } - - /** - * Creates an observer for monitoring changes in the snapshot of an owner. - * - * @return the observer for monitoring snapshot changes - */ - fun snapshotObserver() = OwnerSnapshotObserver { command -> - if (renderingThreadId == getCurrentThreadId()) - command() - else - snapshotChanges.add(command) - } - - /** - * Performs pending snapshot observer callbacks without sending new apply notifications. - */ - fun performSnapshotChanges() { - snapshotChanges.perform() - } - - /** - * Runs [block], performing any snapshot changes it generates synchronously. - * - * See [OwnerSnapshotObserverTest.observeReadsChangedBeforeDisposeEffect] for more details. - */ - inline fun performSnapshotChangesSynchronously(block: () -> T): T { - return try { - renderingThreadId = getCurrentThreadId() - block() - } finally { - renderingThreadId = null - } - } -} - -/** - * Allows postponing execution of some code (command), adding it to the list via [add], - * and performing all added commands in some time in the future via [perform] - */ -private class CommandList( - private var onNewCommand: () -> Unit -) { - private val lock = makeSynchronizedObject() - private val list = mutableListOf<() -> Unit>() - private val listCopy = mutableListOf<() -> Unit>() - - /** - * true if there are any commands added. - * - * Can be called concurrently from multiple threads. - */ - val hasCommands: Boolean - get() = synchronized(lock) { - list.isNotEmpty() - } - - /** - * Add command to the list, and notify observer via [onNewCommand]. - * - * Can be called concurrently from multiple threads. - */ - fun add(command: () -> Unit) { - synchronized(lock) { - list.add(command) - } - onNewCommand() - } - - /** - * Clear added commands and perform them. - * - * Doesn't support multiple [perform]'s from different threads. But does support concurrent [perform] - * and concurrent [add]. - */ - fun perform() { - synchronized(lock) { - listCopy.addAll(list) - list.clear() - } - listCopy.fastForEach { it.invoke() } - listCopy.clear() - } -} diff --git a/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/platform/FrameRecomposer.skiko.kt b/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/platform/FrameRecomposer.skiko.kt index 850b543dcd615..180dc4d2b2112 100644 --- a/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/platform/FrameRecomposer.skiko.kt +++ b/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/platform/FrameRecomposer.skiko.kt @@ -22,8 +22,11 @@ import androidx.compose.runtime.MonotonicFrameClock import androidx.compose.runtime.Recomposer import androidx.compose.runtime.snapshots.Snapshot import androidx.compose.ui.InternalComposeUiApi +import androidx.compose.ui.internal.getCurrentThreadId import androidx.compose.ui.util.trace +import kotlin.coroutines.ContinuationInterceptor import kotlin.coroutines.CoroutineContext +import kotlinx.atomicfu.atomic import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.CoroutineStart import kotlinx.coroutines.Job @@ -32,14 +35,25 @@ import kotlinx.coroutines.launch import kotlinx.coroutines.withContext /** - * Owns a recomposer and frame clock shared by one or more scenes hosted by the same platform - * container. + * Owns a [Recomposer] and frame clock shared by one or more scenes hosted by the same platform + * container - the non-Android analog of Android's host-side recomposer/frame-clock machinery + * (`AndroidComposeView` + the host recomposer + `Choreographer`). * - * This is an equivalent of the Android host-side recomposer/frame-clock machinery: Android drives - * global snapshot notifications through `GlobalSnapshotManager`, drains dispatcher work on - * the UI thread, then lets the recomposer resume frame-clock awaiters and apply changes. - * Non-Android platforms do not have a shared Android-style View/Choreographer integration point, + * Two work queues mirror `AndroidUiDispatcher`'s two queues: + * - [trampolineDispatcher] (Android's `toRunTrampolined`): coroutine dispatch, composition effects + * (`LaunchedEffect`, `rememberCoroutineScope` launches) and the recomposer's effect context; + * - [frameDispatcher] (Android's `toRunOnFrame`), together with [frameClock]: `withFrameNanos` + * awaiters and recomposition (the recomposition loop runs on `frameDispatcher + frameClock`). + * + * Both are [FlushCoroutineDispatcher]s layered over the host's real dispatcher, so on a host with + * a live native loop they drain automatically; [performFrame] and the scene phases also roll them + * synchronously via [performTrampolineDispatch] / [performFrameDispatch]. + * + * Android drives frames through `Choreographer.doFrame`; non-Android platforms have no such hook, * so the host calls [performFrame] explicitly before driving scene measure/layout and draw. + * + * The host dispatcher must be confined to a single thread, so [composeThreadId] is stable. + * It is recorded whenever the recomposer runs on the host thread (via [performFrameDispatch]). */ @InternalComposeUiApi class FrameRecomposer( @@ -48,10 +62,35 @@ class FrameRecomposer( ) : AutoCloseable { private val job = Job() private val coroutineScope = CoroutineScope(coroutineContext + job) + + /** + * Trampoline queue (Android's `toRunTrampolined`): + * - Coroutine dispatch + * - Composition effects + * - Scheduled apply notifications + * Rolled synchronously by [performTrampolineDispatch]. + */ + private val trampolineDispatcher = FlushCoroutineDispatcher(coroutineScope) + + /** + * Frame queue (Android's `toRunOnFrame`): `withFrameNanos` awaiters and recomposition tasks. + * Rolled synchronously by [performFrameDispatch]. + */ + private val frameDispatcher = FlushCoroutineDispatcher(coroutineScope) + + /** + * The clock that drives the recomposition loop. + * Its `withFrameNanos` awaiters are resumed by [performFrame]. + */ private val frameClock = BroadcastFrameClock(::onNewAwaiters) - private val effectDispatcher = FlushCoroutineDispatcher(coroutineScope) - private val recomposeDispatcher = FlushCoroutineDispatcher(coroutineScope) - private val recomposer = Recomposer(coroutineContext + job + effectDispatcher) + + private val recomposer = Recomposer(coroutineContext + job + trampolineDispatcher) + + /** + * Id of the host (compose) thread. Snapshot-observer callbacks run inline when on this thread, + * otherwise they are posted to the shared [effectDispatcher]. + */ + private var composeThreadId: Long? by atomic(null) /** * Registers `coroutineContext` with the shared [GlobalSnapshotManager] so ambient global writes @@ -61,8 +100,14 @@ class FrameRecomposer( private val globalSnapshotRegistration = GlobalSnapshotManager.register(coroutineContext) init { + // The host must carry a (single-thread) continuation interceptor that work is dispatched + // through. It need not be a CoroutineDispatcher directly - e.g. tests wrap it with an + // ApplyingContinuationInterceptor that delegates to the test dispatcher. + requireNotNull(coroutineContext[ContinuationInterceptor]) { + "FrameRecomposer requires a ContinuationInterceptor in its coroutineContext" + } coroutineScope.launch( - recomposeDispatcher + frameClock, + frameDispatcher + frameClock, start = CoroutineStart.UNDISPATCHED ) { recomposer.runRecomposeAndApplyChanges() @@ -94,34 +139,12 @@ class FrameRecomposer( } /** - * Performs one host frame. Platforms should call this once from their native frame callback - * before running scene measure/layout and draw phases. - * - * The snapshot checkpoints are deliberate behavior parity with the old combined render call - * and with Android's flow: - * - the first call observes global snapshot writes that were scheduled before this native - * frame, like Android's `GlobalSnapshotManager` running on the UI dispatcher; - * - [recomposeFrame] then flushes effects/recomposer tasks and sends the frame clock, matching - * the recomposer's frame-aligned work; - * - the second call mirrors the runtime recomposer checkpoint after `sendFrame`, so state - * changes produced by frame awaiters are visible before platform layout/draw phases run. + * Performs one host frame. Platforms call this once from their native frame callback before + * running [androidx.compose.ui.scene.ComposeScene] measure/layout and draw phases. */ fun performFrame(frameTimeNanos: Long) { - Snapshot.sendApplyNotifications() - recomposeFrame(frameTimeNanos) - Snapshot.sendApplyNotifications() - } - - /** - * Advances only the host recomposer and frame clock by one frame at [frameTimeNanos]. - */ - private fun recomposeFrame(frameTimeNanos: Long) { postponeFrameInvalidation { - // Flush composition effects (e.g. LaunchedEffect, coroutines launched in - // rememberCoroutineScope()) queued by the previous turn must run before - // recomposition tasks and frame-clock awaiters. - performScheduledEffects() - performScheduledRecomposerTasks() + performFrameDispatch() frameClock.sendFrame(frameTimeNanos) } @@ -131,12 +154,12 @@ class FrameRecomposer( } /** - * Returns whether the host still has recomposition or frame-clock work to process. + * Returns whether the host still has recomposition or loop work to process. */ fun hasPendingWork(): Boolean = recomposer.hasPendingWork || - effectDispatcher.hasImmediateTasks() || - recomposeDispatcher.hasImmediateTasks() || + trampolineDispatcher.hasImmediateTasks() || + frameDispatcher.hasImmediateTasks() || frameClock.hasAwaiters /** @@ -160,19 +183,42 @@ class FrameRecomposer( } /** - * Enqueues host-owned work to run later in the current turn, before the next frame. + * Runs [block] on the compose thread: inline when already on it, otherwise [dispatch]ed onto + * the shared trampoline queue. + */ + internal fun runOnComposeThread(block: () -> Unit) { + if (composeThreadId == getCurrentThreadId()) block() else dispatch(block) + } + + /** + * Enqueues [block] onto the trampoline queue; it runs on the next loop turn or the next + * [performTrampolineDispatch]. */ internal fun dispatch(block: () -> Unit) { - effectDispatcher.dispatch(job, Runnable(block)) + trampolineDispatcher.dispatch(job, Runnable(block)) } - internal fun performScheduledRecomposerTasks(): Unit = - trace("FrameRecomposer:performScheduledRecomposerTasks") { - recomposeDispatcher.flush() + /** + * Synchronously rolls the frame loop: drains the [frameDispatcher] queue (pending + * `withFrameNanos` / recompose tasks) after first rolling the trampoline loop via + * [performTrampolineDispatch]. + */ + internal fun performFrameDispatch(): Unit = + trace("FrameRecomposer:performFrameDispatch") { + composeThreadId = getCurrentThreadId() + performTrampolineDispatch() + frameDispatcher.flush() } - internal fun performScheduledEffects(): Unit = - trace("FrameRecomposer:performScheduledEffects") { - effectDispatcher.flush() + /** + * Synchronously rolls the trampoline loop: first flushes pending snapshot apply notifications + * (so writes made since the last turn are visible to the queued work), then drains the + * [trampolineDispatcher] queue (coroutine dispatch / composition effects). + */ + internal fun performTrampolineDispatch(): Unit = + trace("FrameRecomposer:performTrampolineDispatch") { + Snapshot.sendApplyNotifications() + + trampolineDispatcher.flush() } } diff --git a/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/scene/BaseComposeScene.skiko.kt b/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/scene/BaseComposeScene.skiko.kt index 082a9dad92090..185ad51451510 100644 --- a/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/scene/BaseComposeScene.skiko.kt +++ b/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/scene/BaseComposeScene.skiko.kt @@ -35,11 +35,9 @@ import androidx.compose.ui.input.pointer.PointerInputEvent import androidx.compose.ui.input.pointer.PointerKeyboardModifiers import androidx.compose.ui.input.pointer.PointerType import androidx.compose.ui.input.rotary.RotaryScrollEvent -import androidx.compose.ui.node.SnapshotInvalidationTracker import androidx.compose.ui.platform.FrameRecomposer import androidx.compose.ui.platform.ProvidePlatformCompositionLocals import androidx.compose.ui.util.trace -import kotlin.concurrent.Volatile /** * BaseComposeScene is an internal abstract class that implements the ComposeScene interface. @@ -54,10 +52,9 @@ internal abstract class BaseComposeScene( private val invalidateLayout: () -> Unit, private val invalidateDraw: () -> Unit, ) : ComposeScene { - protected val snapshotInvalidationTracker = SnapshotInvalidationTracker(::updateInvalidations) protected val inputHandler: ComposeSceneInputHandler = ComposeSceneInputHandler( - prepareForPointerInputEvent = ::runMeasureAndLayout, + prepareForPointerInputEvent = ::doMeasureAndLayout, processPointerInputEvent = ::onPointerInputEvent, cancelPointerInput = ::processCancelPointerInput, processKeyEvent = ::processKeyEvent, @@ -77,43 +74,21 @@ internal abstract class BaseComposeScene( if (isInvalidationDisabled) return block() isInvalidationDisabled = true return try { - // Keep the same scene-boundary snapshot behavior the previous combined render path had - // via SnapshotInvalidationTracker.sendAndPerformSnapshotChanges(): first send global - // apply notifications, then run only this scene's queued owner-observer callbacks. - // This makes snapshot reads that affect layout/draw visible before the phase starts, - // but keeps the tracker scene-local; - Snapshot.sendApplyNotifications() - - // Try to get see the up-to-date state before running block - // Note that this doesn't guarantee it, if sendApplyNotifications is called concurrently - // in a different thread than this code. - snapshotInvalidationTracker.performSnapshotChanges() - snapshotInvalidationTracker.performSnapshotChangesSynchronously(block) + block() } finally { - // This is the previous wrapper's trailing checkpoint written out explicitly. - // It lets state writes produced during the phase enqueue layout/draw invalidations - // before the native platform decides whether another layout or draw pass is needed. - Snapshot.sendApplyNotifications() - snapshotInvalidationTracker.performSnapshotChanges() isInvalidationDisabled = false }.also { - updateInvalidations() + invokeInvalidationCallbacks() } } - protected fun updateInvalidations() { - hasPendingMeasureOrLayout = snapshotInvalidationTracker.hasPendingMeasureOrLayout - hasPendingDraw = snapshotInvalidationTracker.hasPendingDraw - if (!isInvalidationDisabled && !isClosed && composition != null) { - if (hasPendingMeasureOrLayout) { - invalidateLayout() - } - // Snapshot-observer commands queued on this scene need a future host turn to be - // performed (they're drained inside measureAndLayout/draw's postponeInvalidation), so - // request a draw invalidation without flipping the scene's own hasPendingDraw flag. - if (hasPendingDraw || hasPendingSnapshotCommands) { - invalidateDraw() - } + protected fun invokeInvalidationCallbacks() { + if (isInvalidationDisabled || isClosed || composition == null) return + if (hasPendingMeasureOrLayout) { + invalidateLayout() + } + if (hasPendingDraw) { + invalidateDraw() } } @@ -133,17 +108,6 @@ internal abstract class BaseComposeScene( composition?.dispose() } - @Volatile - override var hasPendingMeasureOrLayout: Boolean = true - protected set - - @Volatile - override var hasPendingDraw: Boolean = true - protected set - - override val hasPendingSnapshotCommands: Boolean - get() = snapshotInvalidationTracker.hasPendingSnapshotCommands - override fun setContent( parentCompositionContext: CompositionContext?, content: @Composable () -> Unit, @@ -157,7 +121,7 @@ internal abstract class BaseComposeScene( * changed parameters can be applied in a separate turn and trigger double * recomposition when new content is installed. */ - frameRecomposer.performScheduledRecomposerTasks() + frameRecomposer.performFrameDispatch() composition?.dispose() composition = createComposition( parentCompositionContext = parentCompositionContext ?: frameRecomposer.compositionContext, @@ -170,17 +134,14 @@ internal abstract class BaseComposeScene( content = content ) } - frameRecomposer.performScheduledRecomposerTasks() + frameRecomposer.performFrameDispatch() } override fun measureAndLayout() { if (isClosed) return postponeInvalidation("BaseComposeScene:measureAndLayout") { - // Android runs owner measure/layout from AndroidComposeView.measureAndLayout() during - // the host layout traversal. Skiko exposes that phase imperatively so platforms can - // call it from their native layout pass instead of hiding it inside draw/render. - runMeasureAndLayout() + doMeasureAndLayout() // Schedule synthetic events to be sent after measure/layout completes. if (inputHandler.needUpdatePointerPosition) { @@ -195,11 +156,23 @@ internal abstract class BaseComposeScene( if (isClosed) return postponeInvalidation("BaseComposeScene:draw") { + // FIXME: Remove applying the global snapshot here. + // Android never applies the snapshot *between* the layout and draw phases + // (applies happen once per frame on the main looper, not between phases). + // This between-phase apply is a temporary workaround kept only to preserve current + // behavior for OffsetToFocusedRect (iOS FocusableAboveKeyboard). + Snapshot.sendApplyNotifications() + // AndroidComposeView.dispatchDraw() begins with measureAndLayout() so layout changes // discovered after the host layout traversal are still settled before drawing. Keep // that trailing layout pass here even though measureAndLayout() is also a public phase. - runMeasureAndLayout() - snapshotInvalidationTracker.onDraw() + doMeasureAndLayout() + + // Advance the global snapshot before drawing so writes made since the last pass + // including state objects created during a prior draw are recorded as modified and + // visible to this draw. Lighter than sendApplyNotifications, matches what Android does. + Snapshot.notifyObjectsInitialized() + doDraw(canvas) } } @@ -232,7 +205,7 @@ internal abstract class BaseComposeScene( scaleGestureFactor = scaleGestureFactor, panGestureOffset = panGestureOffset, ).also { - frameRecomposer.performScheduledEffects() + frameRecomposer.performTrampolineDispatch() } } @@ -263,7 +236,7 @@ internal abstract class BaseComposeScene( scaleGestureFactor = scaleGestureFactor, panGestureOffset = panGestureOffset, ).also { - frameRecomposer.performScheduledEffects() + frameRecomposer.performTrampolineDispatch() } } @@ -274,7 +247,7 @@ internal abstract class BaseComposeScene( override fun sendKeyEvent(keyEvent: KeyEvent): Boolean = postponeInvalidation("BaseComposeScene:sendKeyEvent") { inputHandler.onKeyEvent(keyEvent).also { - frameRecomposer.performScheduledEffects() + frameRecomposer.performTrampolineDispatch() } } @@ -289,15 +262,10 @@ internal abstract class BaseComposeScene( uptimeMillis = timeMillis ) processRotaryScrollEvent(event).also { - frameRecomposer.performScheduledEffects() + frameRecomposer.performTrampolineDispatch() } } - protected fun runMeasureAndLayout() { - snapshotInvalidationTracker.onMeasureAndLayout() - doMeasureAndLayout() - } - protected abstract fun createComposition( parentCompositionContext: CompositionContext, content: @Composable () -> Unit diff --git a/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/scene/CanvasLayersComposeScene.skiko.kt b/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/scene/CanvasLayersComposeScene.skiko.kt index aa4abceae4817..09d9958245c7e 100644 --- a/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/scene/CanvasLayersComposeScene.skiko.kt +++ b/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/scene/CanvasLayersComposeScene.skiko.kt @@ -55,7 +55,6 @@ import androidx.compose.ui.viewinterop.InteropView import androidx.compose.ui.window.getDialogScrimBlendMode import kotlin.coroutines.CoroutineContext import kotlin.math.max -import kotlinx.coroutines.Dispatchers /** * Constructs a multi-layer [ComposeScene] using the specified parameters. Unlike @@ -119,8 +118,9 @@ private class CanvasLayersComposeSceneImpl( size = size, coroutineContext = frameRecomposer.compositionContext.effectCoroutineContext, platformContext = composeSceneContext.platformContext, - snapshotInvalidationTracker = snapshotInvalidationTracker, inputHandler = inputHandler, + invalidate = ::invokeInvalidationCallbacks, + onChangedExecutor = frameRecomposer::runOnComposeThread, ) override val composeSceneContext: ComposeSceneContext @@ -226,6 +226,14 @@ private class CanvasLayersComposeSceneImpl( mainOwner.invalidatePositionOnScreen() } + override val hasPendingMeasureOrLayout: Boolean + get() = mainOwner.hasPendingMeasureOrLayout + || layers.fastAny { it.owner.hasPendingMeasureOrLayout } + + override val hasPendingDraw: Boolean + get() = mainOwner.hasPendingDraw + || layers.fastAny { it.owner.hasPendingDraw } + override fun createComposition( parentCompositionContext: CompositionContext, content: @Composable () -> Unit, @@ -509,7 +517,7 @@ private class CanvasLayersComposeSceneImpl( onOwnerAppended(layer.owner) inputHandler.onPointerUpdate() - updateInvalidations() + invokeInvalidationCallbacks() } private fun detachLayer(layer: AttachedComposeSceneLayer) { @@ -520,7 +528,7 @@ private class CanvasLayersComposeSceneImpl( onOwnerRemoved(layer.owner) inputHandler.onPointerUpdate() - updateInvalidations() + invokeInvalidationCallbacks() } private fun requestFocus(layer: AttachedComposeSceneLayer) { @@ -562,8 +570,9 @@ private class CanvasLayersComposeSceneImpl( // TODO: Figure out why real requestFocus is required // even with empty parentFocusManager }, - snapshotInvalidationTracker = snapshotInvalidationTracker, inputHandler = inputHandler, + invalidate = ::invokeInvalidationCallbacks, + onChangedExecutor = frameRecomposer::runOnComposeThread, ) private var composition: Composition? = null private var outsidePointerCallback: (( @@ -598,7 +607,7 @@ private class CanvasLayersComposeSceneImpl( releaseFocus(this) } inputHandler.onPointerUpdate() - updateInvalidations() + invokeInvalidationCallbacks() } private val background: Modifier diff --git a/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/scene/ComposeScene.skiko.kt b/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/scene/ComposeScene.skiko.kt index e20ff20d34c5c..a799887d28f54 100644 --- a/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/scene/ComposeScene.skiko.kt +++ b/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/scene/ComposeScene.skiko.kt @@ -168,15 +168,6 @@ sealed interface ComposeScene : AutoCloseable { */ val hasPendingDraw: Boolean - /** - * Returns whether the scene has queued snapshot-observer callbacks that have not been - * performed yet. The scene drains these synchronously inside [measureAndLayout] and [draw], - * so this is mainly useful for test harnesses that decide when to drive the next frame after - * snapshot writes happen outside the scene's input/render paths. - * Can be called from any thread. - */ - val hasPendingSnapshotCommands: Boolean - /** * Update the composition with the content described by the [content] composable. After this * has been called the changes to produce the initial composition has been calculated and @@ -317,7 +308,7 @@ sealed interface ComposeScene : AutoCloseable { */ @InternalComposeUiApi fun ComposeScene.hasInvalidations(): Boolean = - hasPendingMeasureOrLayout || hasPendingDraw || hasPendingSnapshotCommands + hasPendingMeasureOrLayout || hasPendingDraw /** * Returns the current content size (in pixels) in infinity constraints. diff --git a/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/scene/PlatformLayersComposeScene.skiko.kt b/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/scene/PlatformLayersComposeScene.skiko.kt index bb4c71ba4645a..7c45c259d7df3 100644 --- a/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/scene/PlatformLayersComposeScene.skiko.kt +++ b/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/scene/PlatformLayersComposeScene.skiko.kt @@ -98,8 +98,9 @@ private class PlatformLayersComposeSceneImpl( coroutineContext = frameRecomposer.compositionContext.effectCoroutineContext, size = size, platformContext = composeSceneContext.platformContext, - snapshotInvalidationTracker = snapshotInvalidationTracker, inputHandler = inputHandler, + invalidate = ::invokeInvalidationCallbacks, + onChangedExecutor = frameRecomposer::runOnComposeThread, ) } @@ -160,6 +161,12 @@ private class PlatformLayersComposeSceneImpl( mainOwner.invalidatePositionOnScreen() } + override val hasPendingMeasureOrLayout: Boolean + get() = mainOwner.hasPendingMeasureOrLayout + + override val hasPendingDraw: Boolean + get() = mainOwner.hasPendingDraw + override fun createComposition( parentCompositionContext: CompositionContext, content: @Composable () -> Unit, diff --git a/compose/ui/ui/src/skikoTest/kotlin/androidx/compose/ui/node/RootNodeOwnerTest.kt b/compose/ui/ui/src/skikoTest/kotlin/androidx/compose/ui/node/RootNodeOwnerTest.kt index 0a84a843356d9..39cbbf6c41580 100644 --- a/compose/ui/ui/src/skikoTest/kotlin/androidx/compose/ui/node/RootNodeOwnerTest.kt +++ b/compose/ui/ui/src/skikoTest/kotlin/androidx/compose/ui/node/RootNodeOwnerTest.kt @@ -166,9 +166,7 @@ class RootNodeOwnerTest { var invalidationCount = 0 val owner = RootNodeOwner( - snapshotInvalidationTracker = SnapshotInvalidationTracker { - invalidationCount++ - } + invalidate = { invalidationCount++ } ) // Set the initial size @@ -199,20 +197,21 @@ class RootNodeOwnerTest { private fun RootNodeOwner( coroutineContext: CoroutineContext = EmptyCoroutineContext, platformContext: PlatformContext = PlatformContext.Empty(), - snapshotInvalidationTracker: SnapshotInvalidationTracker = SnapshotInvalidationTracker {}, + invalidate: () -> Unit = {}, ) = RootNodeOwner( density = Density(1f), layoutDirection = LayoutDirection.Ltr, size = null, coroutineContext = coroutineContext, platformContext = platformContext, - snapshotInvalidationTracker = snapshotInvalidationTracker, inputHandler = ComposeSceneInputHandler( prepareForPointerInputEvent = {}, processPointerInputEvent = { PointerEventResult(false) }, cancelPointerInput = {}, processKeyEvent = { false }, - ) + ), + invalidate = invalidate, + onChangedExecutor = { it() }, ) @ExperimentalComposeUiApi diff --git a/compose/ui/ui/src/skikoTest/kotlin/androidx/compose/ui/node/VoteFrameRateTest.kt b/compose/ui/ui/src/skikoTest/kotlin/androidx/compose/ui/node/VoteFrameRateTest.kt index 369cbc4bb87d6..7dfb9d31a97d1 100644 --- a/compose/ui/ui/src/skikoTest/kotlin/androidx/compose/ui/node/VoteFrameRateTest.kt +++ b/compose/ui/ui/src/skikoTest/kotlin/androidx/compose/ui/node/VoteFrameRateTest.kt @@ -388,11 +388,12 @@ private fun RootNodeOwner( size = null, coroutineContext = EmptyCoroutineContext, platformContext = platformContext, - snapshotInvalidationTracker = SnapshotInvalidationTracker {}, inputHandler = ComposeSceneInputHandler( prepareForPointerInputEvent = {}, processPointerInputEvent = { PointerEventResult(false) }, cancelPointerInput = {}, processKeyEvent = { false }, - ) + ), + invalidate = {}, + onChangedExecutor = { it() }, ) \ No newline at end of file diff --git a/compose/ui/ui/src/skikoTest/kotlin/androidx/compose/ui/platform/RenderPhasesTest.kt b/compose/ui/ui/src/skikoTest/kotlin/androidx/compose/ui/platform/RenderPhasesTest.kt index 81e9b1720886d..d1cc1dc29d2f1 100644 --- a/compose/ui/ui/src/skikoTest/kotlin/androidx/compose/ui/platform/RenderPhasesTest.kt +++ b/compose/ui/ui/src/skikoTest/kotlin/androidx/compose/ui/platform/RenderPhasesTest.kt @@ -55,6 +55,7 @@ import androidx.compose.ui.test.ExperimentalTestApi import androidx.compose.ui.test.InternalTestApi import androidx.compose.ui.test.onNodeWithTag import androidx.compose.ui.test.performMouseInput +import androidx.compose.ui.test.runSkikoComposeUiTest import androidx.compose.ui.test.v2.runInternalSkikoComposeUiTest import androidx.compose.ui.touch import androidx.compose.ui.unit.dp @@ -306,7 +307,7 @@ class RenderPhasesTest { } @Test - fun measureAndLayoutRunsAgainBeforeDraw() = runInternalSkikoComposeUiTest { + fun measureAndLayoutRunsAgainBeforeDraw() = runSkikoComposeUiTest { // Android runs measureAndLayout again right before drawing; validate this behavior. val state = mutableStateOf(0) val events = mutableListOf() @@ -363,7 +364,7 @@ class RenderPhasesTest { } @Test - fun scrollPointerEventHandlesScrollUpdatesSynchronously() = runInternalSkikoComposeUiTest { + fun scrollPointerEventHandlesScrollUpdatesSynchronously() = runSkikoComposeUiTest { val scrollState = ScrollState(0) setContent { Box(modifier = Modifier.size(100.dp).verticalScroll(scrollState)) { @@ -385,7 +386,7 @@ class RenderPhasesTest { } @Test - fun panPointerEventHandlesScrollUpdatesSynchronously() = runInternalSkikoComposeUiTest { + fun panPointerEventHandlesScrollUpdatesSynchronously() = runSkikoComposeUiTest { val scrollState = ScrollState(0) setContent { Box(modifier = Modifier.size(100.dp).verticalScroll(scrollState)) {