Move the effects and synthetic events dispatching to after the draw p…

…hase in the render loop. (#1260)

compose/ui/ui/src/desktopTest/kotlin/androidx/compose/ui/ComposeSceneTest.kt

@@ -73,31 +73,23 @@ import androidx.compose.ui.layout.Layout
 import androidx.compose.ui.layout.MeasurePolicy
 import androidx.compose.ui.layout.RootMeasurePolicy.measure
 import androidx.compose.ui.platform.renderingTest
-import androidx.compose.ui.platform.testTag
-import androidx.compose.ui.test.ExperimentalTestApi
 import androidx.compose.ui.test.InternalTestApi
 import androidx.compose.ui.test.junit4.DesktopScreenshotTestRule
 import androidx.compose.ui.test.junit4.createComposeRule
-import androidx.compose.ui.test.onNodeWithTag
 import androidx.compose.ui.test.onRoot
 import androidx.compose.ui.test.performKeyPress
-import androidx.compose.ui.test.performMouseInput
-import androidx.compose.ui.test.runComposeUiTest
 import androidx.compose.ui.unit.dp
-import androidx.compose.ui.window.Window
-import androidx.compose.ui.window.runApplicationTest
 import com.google.common.truth.Truth.assertThat
 import kotlin.test.assertEquals
 import kotlin.test.assertTrue
 import kotlinx.coroutines.Dispatchers
-import kotlinx.coroutines.launch
 import kotlinx.coroutines.runBlocking
 import org.junit.Assert.assertFalse
 import org.junit.Ignore
 import org.junit.Rule
 import org.junit.Test
 
-@OptIn(InternalTestApi::class, ExperimentalComposeUiApi::class, ExperimentalTestApi::class)
+@OptIn(InternalTestApi::class, ExperimentalComposeUiApi::class)
 class ComposeSceneTest {
     @get:Rule
     val screenshotRule = DesktopScreenshotTestRule("compose/ui/ui-desktop")
@@ -704,125 +696,6 @@ class ComposeSceneTest {
             assertThat(field2FocusState!!.isFocused).isFalse()
         }
     }
-
-    // Test that we're draining the effect dispatcher completely before starting the draw phase
-    @Test
-    fun allEffectsRunBeforeDraw() = runApplicationTest {
-        var flag by mutableStateOf(false)
-        val events = mutableListOf<String>()
-        launchTestApplication {
-            Window(onCloseRequest = {}) {
-                LaunchedEffect(flag) {
-                    if (flag) {
-                        launch {
-                            launch {
-                                launch {
-                                    launch {
-                                        launch {
-                                            launch {
-                                                events.add("effect")
-                                            }
-                                        }
-                                    }
-                                }
-                            }
-                        }
-                    }
-                }
-
-                Canvas(Modifier.size(100.dp)) {
-                    if (flag) {
-                        events.add("draw")
-                    }
-                }
-            }
-        }
-        awaitIdle()
-
-        flag = true
-        awaitIdle()
-        assertEquals(listOf("effect", "draw"), events)
-    }
-
-    // Test that effects are run before synthetic events are delivered.
-    // This is important because the effects may be registering to receive the events. For example
-    // when a new element appears under the mouse pointer and registers to mouse-enter events.
-    @Test
-    fun effectsRunBeforeSyntheticEvents() = runComposeUiTest {
-        var flag by mutableStateOf(false)
-        val events = mutableListOf<String>()
-
-        setContent {
-            Box(
-                modifier = Modifier
-                    .size(100.dp)
-                    .testTag("container"),
-                contentAlignment = Alignment.Center
-            ) {
-                if (flag) {
-                    Box(
-                        modifier = Modifier
-                            .size(50.dp)
-                            .onPointerEvent(eventType = PointerEventType.Enter) {
-                                events.add("mouse-enter")
-                            }
-                    )
-
-                    LaunchedEffect(Unit) {
-                        events.add("effect")
-                    }
-                }
-            }
-        }
-
-        onNodeWithTag("container").performMouseInput {
-            moveTo(Offset(50f, 50f))
-        }
-        flag = true
-        waitForIdle()
-
-        assertEquals(listOf("effect", "mouse-enter"), events)
-    }
-
-    // Test that synthetic events are dispatched before the drawing phase.
-    @Test
-    fun syntheticEventsDispatchedBeforeDraw() = runComposeUiTest {
-        var flag by mutableStateOf(false)
-        val events = mutableListOf<String>()
-
-        setContent {
-            Box(
-                modifier = Modifier
-                    .size(100.dp)
-                    .testTag("container"),
-                contentAlignment = Alignment.Center
-            ) {
-                if (flag) {
-                    Box(
-                        modifier = Modifier
-                            .size(50.dp)
-                            .onPointerEvent(eventType = PointerEventType.Enter) {
-                                events.add("mouse-enter")
-                            }
-                    )
-
-                    Canvas(Modifier.size(100.dp)) {
-                        if (flag) {
-                            events.add("draw")
-                        }
-                    }
-                }
-            }
-        }
-
-        onNodeWithTag("container").performMouseInput {
-            moveTo(Offset(50f, 50f))
-        }
-        flag = true
-        waitForIdle()
-
-        assertEquals(listOf("mouse-enter", "draw"), events)
-    }
 }
 
 private object PressTestIndication : Indication {

compose/ui/ui/src/desktopTest/kotlin/androidx/compose/ui/platform/RenderingTestScope.kt

@@ -17,19 +17,20 @@
 package androidx.compose.ui.platform
 
 import androidx.compose.runtime.Composable
-import androidx.compose.ui.ComposeScene
 import androidx.compose.ui.graphics.Color
+import androidx.compose.ui.graphics.asComposeCanvas
+import androidx.compose.ui.graphics.nativeCanvas
 import androidx.compose.ui.graphics.toArgb
-import androidx.compose.ui.unit.Constraints
+import androidx.compose.ui.scene.MultiLayerComposeScene
 import androidx.compose.ui.unit.Density
+import androidx.compose.ui.unit.IntSize
+import kotlin.coroutines.CoroutineContext
 import kotlinx.coroutines.CompletableDeferred
 import kotlinx.coroutines.runBlocking
 import kotlinx.coroutines.yield
-import org.jetbrains.skia.Canvas
 import org.jetbrains.skia.Surface
 import org.jetbrains.skiko.FrameDispatcher
 import org.jetbrains.skiko.MainUIDispatcher
-import kotlin.coroutines.CoroutineContext
 
 internal fun renderingTest(
     width: Int,
@@ -57,12 +58,12 @@ internal class RenderingTestScope(
     }
 
     val surface: Surface = Surface.makeRasterN32Premul(width, height)
-    private val canvas: Canvas = surface.canvas
-    val scene = ComposeScene(
+    private val canvas = surface.canvas.asComposeCanvas()
+    val scene = MultiLayerComposeScene(
         coroutineContext = coroutineContext,
         invalidate = frameDispatcher::scheduleFrame
     ).apply {
-        constraints = Constraints(maxWidth = width, maxHeight = height)
+        size = IntSize(width = width, height = height)
     }
 
     var density: Float
@@ -85,7 +86,7 @@ internal class RenderingTestScope(
     }
 
     private fun onRender(timeNanos: Long) {
-        canvas.clear(Color.Transparent.toArgb())
+        canvas.nativeCanvas.clear(Color.Transparent.toArgb())
         scene.render(canvas, timeNanos)
         onRender.complete(Unit)
     }

compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/platform/FlushCoroutineDispatcher.skiko.kt

@@ -40,7 +40,7 @@ internal class FlushCoroutineDispatcher(
 ) : CoroutineDispatcher(), Delay {
     // Dispatcher should always be alive, even if Job is cancelled. Otherwise coroutines which
     // use this dispatcher won't be properly cancelled.
-    // TODO replace it by scope.coroutineContext[Dispatcher] when it will be no longer experimental
+    // TODO replace it by scope.coroutineContext[CoroutineDispatcher] when it will be no longer experimental
     private val scope = CoroutineScope(scope.coroutineContext.minusKey(Job))
     private val tasks = mutableSetOf<Runnable>()
     private val delayedTasks = mutableSetOf<Runnable>()

compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/scene/BaseComposeScene.skiko.kt

@@ -63,6 +63,9 @@ internal abstract class BaseComposeScene(
             processKeyEvent = ::processKeyEvent
         )
 
+    // Store this to avoid creating a lambda every frame
+    private val updatePointerPosition = inputHandler::updatePointerPosition
+
     private val frameClock = BroadcastFrameClock(onNewAwaiters = ::invalidateIfNeeded)
     private val recomposer: ComposeSceneRecomposer =
         ComposeSceneRecomposer(coroutineContext, frameClock)
@@ -95,8 +98,7 @@ internal abstract class BaseComposeScene(
     private var hasPendingDraws = true
     protected fun invalidateIfNeeded() {
         hasPendingDraws = frameClock.hasAwaiters ||
-            snapshotInvalidationTracker.hasInvalidations ||
-            inputHandler.hasInvalidations
+            snapshotInvalidationTracker.hasInvalidations
         if (hasPendingDraws && !isInvalidationDisabled && !isClosed && composition != null) {
             invalidate()
         }
@@ -133,7 +135,7 @@ internal abstract class BaseComposeScene(
          * It's required before setting content to apply changed parameters
          * before first recomposition. Otherwise, it can lead to double recomposition.
          */
-        recomposer.performScheduledTasks()
+        recomposer.performScheduledRecomposerTasks()
 
         composition?.dispose()
         composition = createComposition {
@@ -143,31 +145,30 @@ internal abstract class BaseComposeScene(
             )
         }
 
-        recomposer.performScheduledTasks()
+        recomposer.performScheduledRecomposerTasks()
     }
 
     override fun render(canvas: Canvas, nanoTime: Long) =
         postponeInvalidation("BaseComposeScene:render") {
-            // Note that on Android the order is slightly different:
-            // - Recomposition
-            // - Layout
-            // - Draw
-            // - Composition effects
-            // - Synthetic events
-            // We do this differently in order to be able to observe changes made by synthetic events
-            // in the drawing phase, thus reducing the time before they are visible on screen.
-            //
-            // It is important, however, to run the composition effects before the synthetic events are
-            // dispatched, in order to allow registering for these events before they are sent.
-            // Otherwise, events like a synthetic mouse-enter sent due to a new element appearing under
-            // the pointer would be missed by e.g. InteractionSource.collectHoverAsState
-            recomposer.performScheduledTasks()
-            frameClock.sendFrame(nanoTime)           // Recomposition
-            doLayout()                               // Layout
-            recomposer.performScheduledEffects()     // Composition effects (e.g. LaunchedEffect)
-            inputHandler.updatePointerPosition()     // Synthetic move event
+            // We try to run the phases here in the same order Android does.
+
+            // Flush composition effects (e.g. LaunchedEffect, coroutines launched in
+            // rememberCoroutineScope()) before everything else
+            recomposer.performScheduledEffects()
+
+            recomposer.performScheduledRecomposerTasks()
+            frameClock.sendFrame(nanoTime) // withFrameMillis/Nanos and recomposition
+
+            doLayout()  // Layout
+
+            // Schedule synthetic events to be sent after `render` completes
+            if (inputHandler.needUpdatePointerPosition) {
+                recomposer.scheduleAsEffect(updatePointerPosition)
+            }
+
+            // Draw
             snapshotInvalidationTracker.onDraw()
-            draw(canvas)                             // Draw
+            draw(canvas)
         }
 
     override fun sendPointerEvent(

compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/scene/ComposeSceneInputHandler.skiko.kt

@@ -64,10 +64,9 @@ internal class ComposeSceneInputHandler(
         get() = pointerPositions.values.firstOrNull()
 
     /**
-     * Indicates if there were invalidations and triggering [BaseComposeScene.measureAndLayout]
-     * is now required.
+     * Indicates whether [updatePointerPosition] needs to be called.
      */
-    val hasInvalidations: Boolean
+    val needUpdatePointerPosition: Boolean
         get() = syntheticEventSender.needUpdatePointerPosition
 
     fun onPointerEvent(

compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/scene/ComposeSceneRecomposer.skiko.kt

@@ -25,6 +25,7 @@ import kotlin.coroutines.CoroutineContext
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.CoroutineStart
 import kotlinx.coroutines.Job
+import kotlinx.coroutines.Runnable
 import kotlinx.coroutines.launch
 
 /**
@@ -44,8 +45,8 @@ internal class ComposeSceneRecomposer(
     private val coroutineScope = CoroutineScope(coroutineContext + job)
 
     /**
-     * We use [FlushCoroutineDispatcher] not because we need [flush] for
-     * LaunchEffect tasks, but because we need to know if it is idle (hasn't scheduled tasks)
+     * We use [FlushCoroutineDispatcher] not only because we need [FlushCoroutineDispatcher.flush]
+     * for LaunchEffect tasks, but also to know whether it is idle (has no scheduled tasks)
      */
     private val effectDispatcher = FlushCoroutineDispatcher(coroutineScope)
     private val recomposeDispatcher = FlushCoroutineDispatcher(coroutineScope)
@@ -75,12 +76,13 @@ internal class ComposeSceneRecomposer(
     }
 
     /**
-     * Perform all scheduled tasks and wait for the tasks which are already
+     * Perform all scheduled recomposer tasks and wait for the tasks which are already
      * performing in the recomposition scope.
      */
-    fun performScheduledTasks() = trace("ComposeSceneRecomposer:performScheduledTasks") {
-        recomposeDispatcher.flush()
-    }
+    fun performScheduledRecomposerTasks() =
+        trace("ComposeSceneRecomposer:performScheduledRecomposerTasks") {
+            recomposeDispatcher.flush()
+        }
 
     /**
      * Perform all scheduled effects.
@@ -90,7 +92,14 @@ internal class ComposeSceneRecomposer(
     }
 
     /**
-     * Permanently shut down this [Recomposer] for future use.
+     * Schedules the given [block] to run with the effects dispatcher.
+     */
+    fun scheduleAsEffect(block: () -> Unit) {
+        effectDispatcher.dispatch(job, Runnable(block))
+    }
+
+    /**
+     * Permanently shut down this [ComposeSceneRecomposer] for future use.
      *
      * @see Recomposer.cancel
      */

compose/ui/ui/src/skikoTest/kotlin/androidx/compose/ui/platform/FlushCoroutineDispatcherTest.kt

@@ -17,14 +17,12 @@
 package androidx.compose.ui.platform
 
 import kotlinx.coroutines.test.runTest
-import kotlin.random.Random
 import kotlin.test.Test
 import kotlin.test.assertEquals
 import kotlin.test.assertFalse
 import kotlin.test.assertTrue
 import kotlinx.coroutines.*
 
-@OptIn(ExperimentalCoroutinesApi::class)
 class FlushCoroutineDispatcherTest {
 
     @Test