Fix RootNodeOwner.OwnerImpl.onRequestRelayout to only schedule layout…

…, not measure.
JetBrains · May 13, 2024 · 99e91a7 · 99e91a7
1 parent 383239d
commit 99e91a7
Show file tree

Hide file tree

Showing 3 changed files with 95 additions and 10 deletions.
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
@@ -338,12 +338,12 @@ internal class RootNodeOwner(
                 if (measureAndLayoutDelegate.requestLookaheadRemeasure(layoutNode, forceRequest) &&
                     scheduleMeasureAndLayout
                 ) {
-                    snapshotInvalidationTracker.requestLayout()
+                    snapshotInvalidationTracker.requestMeasureAndLayout()
                 }
             } else if (measureAndLayoutDelegate.requestRemeasure(layoutNode, forceRequest) &&
                 scheduleMeasureAndLayout
             ) {
-                snapshotInvalidationTracker.requestLayout()
+                snapshotInvalidationTracker.requestMeasureAndLayout()
             }
         }
 
@@ -352,12 +352,20 @@ internal class RootNodeOwner(
             affectsLookahead: Boolean,
             forceRequest: Boolean
         ) {
-            this.onRequestMeasure(layoutNode, affectsLookahead, forceRequest, scheduleMeasureAndLayout = true)
+            if (affectsLookahead) {
+                if (measureAndLayoutDelegate.requestLookaheadRelayout(layoutNode, forceRequest)) {
+                    snapshotInvalidationTracker.requestMeasureAndLayout()
+                }
+            } else {
+                if (measureAndLayoutDelegate.requestRelayout(layoutNode, forceRequest)) {
+                    snapshotInvalidationTracker.requestMeasureAndLayout()
+                }
+            }
         }
 
         override fun requestOnPositionedCallback(layoutNode: LayoutNode) {
             measureAndLayoutDelegate.requestOnPositionedCallback(layoutNode)
-            snapshotInvalidationTracker.requestLayout()
+            snapshotInvalidationTracker.requestMeasureAndLayout()
         }
 
         override fun createLayer(
@@ -434,7 +442,7 @@ internal class RootNodeOwner(
 
         override fun registerOnLayoutCompletedListener(listener: Owner.OnLayoutCompletedListener) {
             measureAndLayoutDelegate.registerOnLayoutCompletedListener(listener)
-            snapshotInvalidationTracker.requestLayout()
+            snapshotInvalidationTracker.requestMeasureAndLayout()
         }
     }
 

diff --git a/.../ui/ui/src/skikoMain/kotlin/androidx/compose/ui/node/SnapshotInvalidationTracker.skiko.kt b/.../ui/ui/src/skikoMain/kotlin/androidx/compose/ui/node/SnapshotInvalidationTracker.skiko.kt
@@ -32,7 +32,7 @@ internal class SnapshotInvalidationTracker(
     private val invalidate: () -> Unit = {}
 ) {
     private val snapshotChanges = CommandList(invalidate)
-    private var needLayout = true
+    private var needMeasureAndLayout = true
     private var needDraw = true
 
     /**
@@ -43,15 +43,15 @@ internal class SnapshotInvalidationTracker(
     private var renderingThreadId: Long? by atomic(null)
 
     val hasInvalidations: Boolean
-        get() = needLayout || needDraw || snapshotChanges.hasCommands
+        get() = needMeasureAndLayout || needDraw || snapshotChanges.hasCommands
 
-    fun requestLayout() {
-        needLayout = true
+    fun requestMeasureAndLayout() {
+        needMeasureAndLayout = true
         invalidate()
     }
 
     fun onLayout() {
-        needLayout = false
+        needMeasureAndLayout = false
     }
 
     fun requestDraw() {

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
@@ -23,8 +23,10 @@ import androidx.compose.foundation.layout.size
 import androidx.compose.runtime.LaunchedEffect
 import androidx.compose.runtime.getValue
 import androidx.compose.runtime.mutableStateOf
+import androidx.compose.runtime.neverEqualPolicy
 import androidx.compose.runtime.rememberCoroutineScope
 import androidx.compose.runtime.setValue
+import androidx.compose.runtime.snapshots.Snapshot
 import androidx.compose.runtime.withFrameMillis
 import androidx.compose.runtime.withFrameNanos
 import androidx.compose.ui.Modifier
@@ -41,6 +43,8 @@ import androidx.compose.ui.test.runInternalSkikoComposeUiTest
 import androidx.compose.ui.unit.dp
 import kotlin.test.Test
 import kotlin.test.assertContentEquals
+import kotlin.test.assertEquals
+import kotlin.test.assertTrue
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.test.StandardTestDispatcher
 
@@ -213,4 +217,77 @@ class RenderPhasesTest {
             actual = phases
         )
     }
+
+    @Test
+    fun layoutScopeInvalidationDoesNotCauseRemeasure() = runInternalSkikoComposeUiTest(
+        coroutineDispatcher = StandardTestDispatcher()
+    ) {
+        var measureCount = 0
+        var layoutCount = 0
+
+        var layoutScopeInvalidation by mutableStateOf(Unit, neverEqualPolicy())
+
+        setContent {
+            Layout(
+                measurePolicy = { _, _ ->
+                    measureCount += 1
+                    layout(100, 100) {
+                        @Suppress("UNUSED_EXPRESSION")
+                        layoutScopeInvalidation
+                        layoutCount += 1
+                    }
+                }
+            )
+        }
+
+        assertEquals(measureCount, layoutCount)
+
+        measureCount = 0
+        layoutCount = 0
+        layoutScopeInvalidation = Unit
+        waitForIdle()
+
+        assertEquals(0, measureCount)
+        assertTrue(layoutCount > 0)
+    }
+
+    @Test
+    fun readingStateInLayoutModifiedByMeasureDoesNotCauseInfiniteRemeasureAndLayout() {
+        // https://github.com/JetBrains/compose-multiplatform/issues/4760
+        runInternalSkikoComposeUiTest(coroutineDispatcher = StandardTestDispatcher()) {
+            mainClock.autoAdvance = false
+            val state = mutableStateOf(0)
+            // Don't read the state initially so that the test fails rather than getting stuck
+            var readStateInLayout by mutableStateOf(false)
+            var layoutCount = 0
+            setContent {
+                Layout(
+                    measurePolicy = { _, _ ->
+                        val prevValue = Snapshot.withoutReadObservation {
+                            state.value
+                        }
+                        state.value = prevValue+1
+                        layout(100, 100) {
+                            if (readStateInLayout) {
+                                state.value  // Read the state value!
+                            }
+                            layoutCount++
+                        }
+                    },
+                )
+            }
+
+            mainClock.advanceTimeByFrame()
+            assertEquals(1, state.value)
+
+            readStateInLayout = true
+            layoutCount = 0
+            mainClock.advanceTimeByFrame()
+            assertEquals(1, layoutCount)
+
+            // Check that no more layout happens
+            mainClock.advanceTimeByFrame()
+            assertEquals(1, layoutCount)
+        }
+    }
 }