Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix RootNodeOwner.OwnerImpl.onRequestRelayout to only schedule layout, not measure #1355

Merged
merged 1 commit into from
May 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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() {
m-sasha marked this conversation as resolved.
Show resolved Hide resolved
needMeasureAndLayout = true
invalidate()
}

fun onLayout() {
needLayout = false
fun onMeasureAndLayout() {
needMeasureAndLayout = false
}

fun requestDraw() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ internal abstract class BaseComposeScene(
protected val snapshotInvalidationTracker = SnapshotInvalidationTracker(::invalidateIfNeeded)
protected val inputHandler: ComposeSceneInputHandler =
ComposeSceneInputHandler(
prepareForPointerInputEvent = ::doLayout,
prepareForPointerInputEvent = ::doMeasureAndLayout,
processPointerInputEvent = ::processPointerInputEvent,
processKeyEvent = ::processKeyEvent
)
Expand Down Expand Up @@ -159,7 +159,7 @@ internal abstract class BaseComposeScene(
recomposer.performScheduledRecomposerTasks()
frameClock.sendFrame(nanoTime) // withFrameMillis/Nanos and recomposition

doLayout() // Layout
doMeasureAndLayout() // Layout

// Schedule synthetic events to be sent after `render` completes
if (inputHandler.needUpdatePointerPosition) {
Expand Down Expand Up @@ -223,8 +223,8 @@ internal abstract class BaseComposeScene(
inputHandler.onKeyEvent(keyEvent)
}

private fun doLayout() {
snapshotInvalidationTracker.onLayout()
private fun doMeasureAndLayout() {
snapshotInvalidationTracker.onMeasureAndLayout()
measureAndLayout()
}

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

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