Skip to content

Commit

Permalink
Fix RootNodeOwner.OwnerImpl.onRequestRelayout to only schedule layout…
Browse files Browse the repository at this point in the history
…, not measure.
  • Loading branch information
m-sasha committed May 13, 2024
1 parent 383239d commit 55e6e05
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
}

Expand All @@ -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(
Expand Down Expand Up @@ -434,7 +442,7 @@ internal class RootNodeOwner(

override fun registerOnLayoutCompletedListener(listener: Owner.OnLayoutCompletedListener) {
measureAndLayoutDelegate.registerOnLayoutCompletedListener(listener)
snapshotInvalidationTracker.requestLayout()
snapshotInvalidationTracker.requestMeasureAndLayout()
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

/**
Expand All @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,22 @@ 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
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
Expand All @@ -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

Expand Down Expand Up @@ -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)
}
}
}

0 comments on commit 55e6e05

Please # to comment.