From 55e6e05b22e4ce3aec58409d62b212d0a8eacd1c Mon Sep 17 00:00:00 2001 From: Alexander Maryanovsky Date: Mon, 13 May 2024 14:22:59 +0300 Subject: [PATCH] Fix RootNodeOwner.OwnerImpl.onRequestRelayout to only schedule layout, not measure. --- .../compose/ui/node/RootNodeOwner.skiko.kt | 18 +++-- .../node/SnapshotInvalidationTracker.skiko.kt | 10 +-- .../compose/ui/platform/RenderPhasesTest.kt | 79 +++++++++++++++++++ 3 files changed, 97 insertions(+), 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 index fb06553c89a17..fd53047b13f3a 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 @@ -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/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 index 275a8360acfb8..935c6f4e8e6fa 100644 --- 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 @@ -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 index 64d2036ba9bbf..73865a548e781 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 @@ -20,11 +20,14 @@ import androidx.compose.foundation.Canvas import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.size +import androidx.compose.material.Text 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 @@ -32,6 +35,7 @@ import androidx.compose.ui.geometry.Offset import androidx.compose.ui.input.pointer.PointerEventType import androidx.compose.ui.input.pointer.onPointerEvent import androidx.compose.ui.layout.Layout +import androidx.compose.ui.layout.RootMeasurePolicy.measure import androidx.compose.ui.scene.BaseComposeScene import androidx.compose.ui.test.ExperimentalTestApi import androidx.compose.ui.test.InternalTestApi @@ -41,6 +45,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 +219,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 = { measurables, constraints -> + measureCount += 1 + measure(measurables, constraints) + 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) + var layoutCount = 0 + setContent { + Layout( + measurePolicy = { measurables, constraints -> + val measurable = measurables.first() + val placeable = measurable.measure(constraints) + val prevValue = Snapshot.withoutReadObservation { + state.value + } + state.value = prevValue+1 + layout(100, 100) { + state.value // Read the state value! + layoutCount++ + placeable.placeRelative(0, 0) + } + }, + content = { + Text("Hello") + } + ) + } + + mainClock.advanceTimeByFrame() + assertEquals(1, state.value) + layoutCount = 0 + + // Check that no more measure or layout has happened + mainClock.advanceTimeByFrame() + assertEquals(1, state.value) + assertEquals(0, layoutCount) + } + } } \ No newline at end of file