Revert "Reuse nodes across subcompose layout boundary"
Partial revert of aosp/2247915 (benchmarks are left in place), as the original change caused perf regression.
Change-Id: I47f1d4704636fc7965d4a68de1580cc299e60435
diff --git a/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/grid/LazyGridSlotsReuseTest.kt b/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/grid/LazyGridSlotsReuseTest.kt
index f1b9c98..cfd4977 100644
--- a/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/grid/LazyGridSlotsReuseTest.kt
+++ b/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/grid/LazyGridSlotsReuseTest.kt
@@ -21,14 +21,13 @@
import androidx.compose.foundation.layout.height
import androidx.compose.foundation.layout.width
import androidx.compose.runtime.Composable
+import androidx.compose.runtime.remember
import androidx.compose.ui.Modifier
-import androidx.compose.ui.layout.layout
import androidx.compose.ui.platform.testTag
import androidx.compose.ui.test.assertIsDisplayed
import androidx.compose.ui.test.assertIsNotDisplayed
import androidx.compose.ui.test.junit4.createComposeRule
import androidx.compose.ui.test.onNodeWithTag
-import androidx.compose.ui.unit.IntOffset
import androidx.compose.ui.unit.dp
import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.filters.LargeTest
@@ -320,24 +319,9 @@
fun scrollingBackReusesTheSameSlot() {
lateinit var state: LazyGridState
var counter0 = 0
- var counter1 = 0
-
- val measureCountModifier0 = Modifier.layout { measurable, constraints ->
- counter0++
- val placeable = measurable.measure(constraints)
- layout(placeable.width, placeable.height) {
- placeable.place(IntOffset.Zero)
- }
- }
-
- val measureCountModifier1 = Modifier.layout { measurable, constraints ->
- counter1++
- val placeable = measurable.measure(constraints)
- layout(placeable.width, placeable.height) {
- placeable.place(IntOffset.Zero)
- }
- }
-
+ var counter1 = 10
+ var rememberedValue0 = -1
+ var rememberedValue1 = -1
rule.setContent {
state = rememberLazyGridState()
LazyVerticalGrid(
@@ -346,17 +330,13 @@
state
) {
items(100) {
- val modifier = when (it) {
- 0 -> measureCountModifier0
- 1 -> measureCountModifier1
- else -> Modifier
+ if (it == 0) {
+ rememberedValue0 = remember { counter0++ }
}
- Spacer(
- Modifier
- .height(itemsSizeDp)
- .testTag("$it")
- .then(modifier)
- )
+ if (it == 1) {
+ rememberedValue1 = remember { counter1++ }
+ }
+ Spacer(Modifier.height(itemsSizeDp).fillMaxWidth().testTag("$it"))
}
}
}
@@ -368,10 +348,10 @@
}
rule.runOnIdle {
- Truth.assertWithMessage("Item 0 measured $counter0 times, expected 1.")
- .that(counter0).isEqualTo(1)
- Truth.assertWithMessage("Item 1 measured $counter1 times, expected 1.")
- .that(counter1).isEqualTo(1)
+ Truth.assertWithMessage("Item 0 restored remembered value is $rememberedValue0")
+ .that(rememberedValue0).isEqualTo(0)
+ Truth.assertWithMessage("Item 1 restored remembered value is $rememberedValue1")
+ .that(rememberedValue1).isEqualTo(10)
}
rule.onNodeWithTag("0")
diff --git a/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/layout/LazyLayoutTest.kt b/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/layout/LazyLayoutTest.kt
index 836604f..d3e9d7a 100644
--- a/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/layout/LazyLayoutTest.kt
+++ b/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/layout/LazyLayoutTest.kt
@@ -19,7 +19,6 @@
import android.os.Parcelable
import androidx.compose.foundation.ExperimentalFoundationApi
import androidx.compose.foundation.layout.Box
-import androidx.compose.foundation.layout.BoxWithConstraints
import androidx.compose.foundation.layout.fillMaxSize
import androidx.compose.runtime.Composable
import androidx.compose.runtime.DisposableEffect
@@ -31,8 +30,6 @@
import androidx.compose.ui.layout.AlignmentLine
import androidx.compose.ui.layout.MeasureResult
import androidx.compose.ui.layout.Placeable
-import androidx.compose.ui.layout.Remeasurement
-import androidx.compose.ui.layout.RemeasurementModifier
import androidx.compose.ui.layout.layout
import androidx.compose.ui.platform.testTag
import androidx.compose.ui.test.assertIsDisplayed
@@ -97,10 +94,7 @@
@Test
fun measureAndPlaceTwoItems() {
val itemProvider = itemProvider({ 2 }) { index ->
- Box(
- Modifier
- .fillMaxSize()
- .testTag("$index"))
+ Box(Modifier.fillMaxSize().testTag("$index"))
}
rule.setContent {
LazyLayout(itemProvider) {
@@ -124,14 +118,8 @@
@Test
fun measureAndPlaceMultipleLayoutsInOneItem() {
val itemProvider = itemProvider({ 1 }) { index ->
- Box(
- Modifier
- .fillMaxSize()
- .testTag("${index}x0"))
- Box(
- Modifier
- .fillMaxSize()
- .testTag("${index}x1"))
+ Box(Modifier.fillMaxSize().testTag("${index}x0"))
+ Box(Modifier.fillMaxSize().testTag("${index}x1"))
}
rule.setContent {
@@ -155,10 +143,7 @@
@Test
fun updatingitemProvider() {
var itemProvider by mutableStateOf(itemProvider({ 1 }) { index ->
- Box(
- Modifier
- .fillMaxSize()
- .testTag("$index"))
+ Box(Modifier.fillMaxSize().testTag("$index"))
})
rule.setContent {
@@ -181,10 +166,7 @@
rule.runOnIdle {
itemProvider = itemProvider({ 2 }) { index ->
- Box(
- Modifier
- .fillMaxSize()
- .testTag("$index"))
+ Box(Modifier.fillMaxSize().testTag("$index"))
}
}
@@ -196,10 +178,7 @@
fun stateBaseditemProvider() {
var itemCount by mutableStateOf(1)
val itemProvider = itemProvider({ itemCount }) { index ->
- Box(
- Modifier
- .fillMaxSize()
- .testTag("$index"))
+ Box(Modifier.fillMaxSize().testTag("$index"))
}
rule.setContent {
@@ -249,11 +228,7 @@
}
}
val itemProvider = itemProvider({ 1 }) { index ->
- Box(
- Modifier
- .fillMaxSize()
- .testTag("$index")
- .then(modifier))
+ Box(Modifier.fillMaxSize().testTag("$index").then(modifier))
}
var needToCompose by mutableStateOf(false)
val prefetchState = LazyLayoutPrefetchState()
@@ -360,15 +335,13 @@
fun nodeIsReusedWithoutExtraRemeasure() {
var indexToCompose by mutableStateOf<Int?>(0)
var remeasuresCount = 0
- val modifier = Modifier
- .layout { measurable, constraints ->
- val placeable = measurable.measure(constraints)
- remeasuresCount++
- layout(placeable.width, placeable.height) {
- placeable.place(0, 0)
- }
+ val modifier = Modifier.layout { measurable, constraints ->
+ val placeable = measurable.measure(constraints)
+ remeasuresCount++
+ layout(placeable.width, placeable.height) {
+ placeable.place(0, 0)
}
- .fillMaxSize()
+ }.fillMaxSize()
val itemProvider = itemProvider({ 2 }) {
Box(modifier)
}
@@ -402,55 +375,6 @@
}
}
- @Test
- fun subcomposeNodeContentIsResetWhenReused() {
- var indexToCompose by mutableStateOf(0)
- var remeasurement: Remeasurement? = null
- val itemProvider = itemProvider({ 3 }) {
- BoxWithConstraints(
- Modifier.testTag("Box $it")
- ) {
- Box(Modifier.testTag("$it"))
- }
- }
-
- rule.setContent {
- LazyLayout(
- itemProvider = itemProvider,
- modifier = object : RemeasurementModifier {
- @Suppress("PARAMETER_NAME_CHANGED_ON_OVERRIDE")
- override fun onRemeasurementAvailable(value: Remeasurement) {
- remeasurement = value
- }
- }
- ) { constraints ->
- val node = measure(indexToCompose, constraints).first()
- layout(node.width, node.height) {
- node.place(0, 0)
- }
- }
- }
-
- rule.runOnIdle {
- indexToCompose = 1
- remeasurement?.forceRemeasure()
- indexToCompose = 2
- remeasurement?.forceRemeasure()
- }
-
- rule.onNodeWithTag("Box 0")
- .assertDoesNotExist()
-
- rule.onNodeWithTag("0")
- .assertDoesNotExist()
-
- rule.onNodeWithTag("Box 2")
- .assertExists()
-
- rule.onNodeWithTag("2")
- .assertExists()
- }
-
private fun itemProvider(
itemCount: () -> Int,
itemContent: @Composable (Int) -> Unit
diff --git a/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/list/LazyColumnTest.kt b/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/list/LazyColumnTest.kt
index 2856359..967acf3 100644
--- a/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/list/LazyColumnTest.kt
+++ b/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/list/LazyColumnTest.kt
@@ -48,7 +48,6 @@
import androidx.compose.ui.geometry.Size
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.graphics.graphicsLayer
-import androidx.compose.ui.layout.layout
import androidx.compose.ui.platform.testTag
import androidx.compose.ui.test.assertCountEquals
import androidx.compose.ui.test.assertIsDisplayed
@@ -504,49 +503,6 @@
}
}
- @Test
- fun nestedLazyRowChildrenAreReused() {
- lateinit var state: LazyListState
- var remeasuresCount = 0
- val measureModifier = Modifier.layout { _, constraints ->
- remeasuresCount++
- layout(constraints.maxWidth, constraints.maxHeight) {}
- }
- rule.setContentWithTestViewConfiguration {
- state = rememberLazyListState()
- LazyColumn(
- Modifier
- .fillMaxWidth()
- .height(10.dp),
- state = state
- ) {
- items(100) {
- LazyRow {
- item {
- Box(Modifier.size(25.dp).then(measureModifier))
- }
- }
- }
- }
- }
-
- rule.runOnIdle {
- state.prefetchingEnabled = false
- runBlocking {
- state.scrollToItem(1) // now item 0 should be kept for reuse
- assertThat(remeasuresCount).isEqualTo(2)
- remeasuresCount = 0
- state.scrollToItem(2) // item 2 should reuse item 0 slot
- }
- }
-
- rule.runOnIdle {
- // no remeasures are expected as the LayoutNode should be reused and modifiers
- // didn't change.
- assertThat(remeasuresCount).isEqualTo(0)
- }
- }
-
@Composable
private fun LazyRowWrapped(content: @Composable () -> Unit) {
LazyRow {
diff --git a/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/list/LazyListSlotsReuseTest.kt b/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/list/LazyListSlotsReuseTest.kt
index 97c63ce..867fd32 100644
--- a/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/list/LazyListSlotsReuseTest.kt
+++ b/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/list/LazyListSlotsReuseTest.kt
@@ -25,14 +25,13 @@
import androidx.compose.foundation.lazy.items
import androidx.compose.foundation.lazy.rememberLazyListState
import androidx.compose.runtime.Composable
+import androidx.compose.runtime.remember
import androidx.compose.ui.Modifier
-import androidx.compose.ui.layout.layout
import androidx.compose.ui.platform.testTag
import androidx.compose.ui.test.assertIsDisplayed
import androidx.compose.ui.test.assertIsNotDisplayed
import androidx.compose.ui.test.junit4.createComposeRule
import androidx.compose.ui.test.onNodeWithTag
-import androidx.compose.ui.unit.IntOffset
import androidx.compose.ui.unit.dp
import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.filters.LargeTest
@@ -317,24 +316,9 @@
fun scrollingBackReusesTheSameSlot() {
lateinit var state: LazyListState
var counter0 = 0
- var counter1 = 0
-
- val measureCountModifier0 = Modifier.layout { measurable, constraints ->
- counter0++
- val placeable = measurable.measure(constraints)
- layout(placeable.width, placeable.height) {
- placeable.place(IntOffset.Zero)
- }
- }
-
- val measureCountModifier1 = Modifier.layout { measurable, constraints ->
- counter1++
- val placeable = measurable.measure(constraints)
- layout(placeable.width, placeable.height) {
- placeable.place(IntOffset.Zero)
- }
- }
-
+ var counter1 = 10
+ var rememberedValue0 = -1
+ var rememberedValue1 = -1
rule.setContent {
state = rememberLazyListState()
LazyColumn(
@@ -342,18 +326,13 @@
state
) {
items(100) {
- val modifier = when (it) {
- 0 -> measureCountModifier0
- 1 -> measureCountModifier1
- else -> Modifier
+ if (it == 0) {
+ rememberedValue0 = remember { counter0++ }
}
- Spacer(
- Modifier
- .height(itemsSizeDp)
- .fillParentMaxWidth()
- .testTag("$it")
- .then(modifier)
- )
+ if (it == 1) {
+ rememberedValue1 = remember { counter1++ }
+ }
+ Spacer(Modifier.height(itemsSizeDp).fillParentMaxWidth().testTag("$it"))
}
}
}
@@ -365,10 +344,10 @@
}
rule.runOnIdle {
- Truth.assertWithMessage("Item 0 measured $counter0 times, expected 1.")
- .that(counter0).isEqualTo(1)
- Truth.assertWithMessage("Item 1 measured $counter1 times, expected 1.")
- .that(counter1).isEqualTo(1)
+ Truth.assertWithMessage("Item 0 restored remembered value is $rememberedValue0")
+ .that(rememberedValue0).isEqualTo(0)
+ Truth.assertWithMessage("Item 1 restored remembered value is $rememberedValue1")
+ .that(rememberedValue1).isEqualTo(10)
}
rule.onNodeWithTag("0")
diff --git a/compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/layout/SubcomposeLayoutTest.kt b/compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/layout/SubcomposeLayoutTest.kt
index 52f73d2..f3a63ae 100644
--- a/compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/layout/SubcomposeLayoutTest.kt
+++ b/compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/layout/SubcomposeLayoutTest.kt
@@ -2035,27 +2035,13 @@
}
}
- var precomposedSlotActive = false
-
val handle = rule.runOnIdle {
state.precompose(1) {
- Box(
- modifier = Modifier
- .size(10.dp)
- .testTag("1")
- )
-
- DisposableEffect(Unit) {
- precomposedSlotActive = true
- onDispose {
- precomposedSlotActive = false
- }
- }
+ Box(modifier = Modifier.size(10.dp).testTag("1"))
}
}
rule.runOnIdle {
- assertThat(precomposedSlotActive).isTrue()
needSlot = false
}
@@ -2063,10 +2049,7 @@
handle.dispose()
}
- assertThat(precomposedSlotActive).isFalse()
-
- // Both slots inside subcompose are reused, as parent was detached with these nodes active
- rule.onNodeWithTag("1").assertIsNotDisplayed()
+ rule.onNodeWithTag("1").assertDoesNotExist()
rule.onNodeWithTag("0").assertIsNotDisplayed()
}
@@ -2185,149 +2168,6 @@
rule.waitUntil { isActive }
}
- @Test
- fun reusingNestedSubcompose_nestedChildrenAreResetAndReused() {
- val slotState = mutableStateOf(0)
-
- val activeChildren = mutableSetOf<Int>()
- var remeasureCount = 0
- val measureCountModifier = Modifier.layout { measurable, constraints ->
- remeasureCount++
- val placeable = measurable.measure(constraints)
- layout(placeable.width, placeable.height) {
- placeable.place(0, 0)
- }
- }
-
- rule.setContent {
- SubcomposeLayout(
- remember { SubcomposeLayoutState(SubcomposeSlotReusePolicy(1)) }
- ) { constraints ->
- val slot = slotState.value
- val child = measure(slot, constraints) {
- Box {
- SubcomposeLayout { constraints ->
- val placeable = measure(Unit, constraints) {
- Box(
- modifier = Modifier
- .size(10.dp)
- .then(measureCountModifier)
- )
-
- DisposableEffect(Unit) {
- activeChildren += slot
- onDispose {
- activeChildren -= slot
- }
- }
- }
- layout(placeable.width, placeable.height) {
- placeable.place(0, 0)
- }
- }
- }
- }
- layout(child.width, child.height) {
- child.place(0, 0)
- }
- }
- }
-
- rule.runOnIdle {
- assertThat(activeChildren).containsExactly(0)
-
- slotState.value = 1
- }
-
- rule.runOnIdle {
- assertThat(activeChildren).containsExactly(1)
- assertThat(remeasureCount).isEqualTo(2)
-
- remeasureCount = 0
-
- slotState.value = 2
- }
-
- rule.runOnIdle {
- assertThat(activeChildren).containsExactly(2)
- assertThat(remeasureCount).isEqualTo(0)
- }
- }
-
- @Test
- fun reusingNestedSubcompose_nestedContentIsResetWhenReusedOnNextFrame() {
- var contentActive by mutableStateOf(true)
- var slotId by mutableStateOf(0)
- val activeChildren = mutableSetOf<Int>()
- var remeasureCount = 0
- val measureCountModifier = Modifier.layout { measurable, constraints ->
- remeasureCount++
- val placeable = measurable.measure(constraints)
- layout(placeable.width, placeable.height) {
- placeable.place(0, 0)
- }
- }
-
- rule.setContent {
- SubcomposeLayout(
- remember { SubcomposeLayoutState(SubcomposeSlotReusePolicy(1)) }
- ) { constraints ->
- if (contentActive) {
- val child = measure(slotId, constraints) {
- Box {
- SubcomposeLayout { constraints ->
- val placeable = measure(Unit, constraints) {
- Box(modifier = Modifier.size(10.dp).then(measureCountModifier))
-
- DisposableEffect(Unit) {
- val capturedSlotId = slotId
- activeChildren += slotId
- onDispose {
- activeChildren -= capturedSlotId
- }
- }
- }
- layout(placeable.width, placeable.height) {
- placeable.place(0, 0)
- }
- }
- }
-
- DisposableEffect(Unit) {
- onDispose {
- // schedule remeasure / compose when child is reset
- contentActive = true
- slotId++
- }
- }
- }
- layout(child.width, child.height) {
- child.place(0, 0)
- }
- } else {
- layout(0, 0) { }
- }
- }
- }
-
- rule.runOnIdle {
- assertThat(activeChildren).containsExactly(0)
-
- contentActive = false
- }
-
- rule.runOnIdle {
- assertThat(activeChildren).containsExactly(1)
- }
- }
-
- private fun SubcomposeMeasureScope.measure(
- slotId: Any,
- constraints: Constraints,
- content: @Composable () -> Unit
- ): Placeable =
- subcompose(slotId, content).first().measure(constraints)
-
private fun composeItems(
state: SubcomposeLayoutState,
items: MutableState<List<Int>>
diff --git a/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/layout/SubcomposeLayout.kt b/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/layout/SubcomposeLayout.kt
index 1196016..575a9ba 100644
--- a/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/layout/SubcomposeLayout.kt
+++ b/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/layout/SubcomposeLayout.kt
@@ -18,13 +18,12 @@
import androidx.compose.runtime.Applier
import androidx.compose.runtime.Composable
+import androidx.compose.runtime.ComposeNode
import androidx.compose.runtime.Composition
import androidx.compose.runtime.CompositionContext
import androidx.compose.runtime.DisposableEffect
-import androidx.compose.runtime.ReusableComposeNode
import androidx.compose.runtime.ReusableContentHost
import androidx.compose.runtime.SideEffect
-import androidx.compose.runtime.collection.mutableVectorOf
import androidx.compose.runtime.currentComposer
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
@@ -34,8 +33,8 @@
import androidx.compose.runtime.setValue
import androidx.compose.runtime.snapshots.Snapshot
import androidx.compose.ui.Modifier
-import androidx.compose.ui.UiComposable
import androidx.compose.ui.layout.SubcomposeLayoutState.PrecomposedSlotHandle
+import androidx.compose.ui.UiComposable
import androidx.compose.ui.materialize
import androidx.compose.ui.node.ComposeUiNode
import androidx.compose.ui.node.LayoutNode
@@ -112,7 +111,7 @@
val density = LocalDensity.current
val layoutDirection = LocalLayoutDirection.current
val viewConfiguration = LocalViewConfiguration.current
- ReusableComposeNode<LayoutNode, Applier<Any>>(
+ ComposeNode<LayoutNode, Applier<Any>>(
factory = LayoutNode.Constructor,
update = {
set(state, state.setRoot)
@@ -131,9 +130,8 @@
}
val stateHolder = rememberUpdatedState(state)
DisposableEffect(Unit) {
- stateHolder.value.keepDetachedNodes()
onDispose {
- stateHolder.value.disposeOrReuseChildren()
+ stateHolder.value.disposeCurrentNodes()
}
}
}
@@ -227,11 +225,9 @@
fun precompose(slotId: Any?, content: @Composable () -> Unit): PrecomposedSlotHandle =
state.precompose(slotId, content)
- internal fun keepDetachedNodes() = state.keepDetachedNodes()
-
internal fun forceRecomposeChildren() = state.forceRecomposeChildren()
- internal fun disposeOrReuseChildren() = state.disposeOrReuseChildren()
+ internal fun disposeCurrentNodes() = state.disposeCurrentNodes()
/**
* Instance of this interface is returned by [precompose] function.
@@ -368,16 +364,14 @@
if (field !== value) {
field = value
// apply the new policy
- disposeOrReuseStartingFromIndex(0, retainAllSlots = true)
+ disposeOrReuseStartingFromIndex(0)
}
}
private var currentIndex = 0
- private var disposeOnDetach = false
private val nodeToNodeState = mutableMapOf<LayoutNode, NodeState>()
// this map contains active slotIds (without precomposed or reusable nodes)
private val slotIdToNode = mutableMapOf<Any?, LayoutNode>()
- private val detachedNodes = mutableVectorOf<NodeState>()
private val scope = Scope()
private val precomposeMap = mutableMapOf<Any?, LayoutNode>()
private val reusableSlotIdsSet = SubcomposeSlotReusePolicy.SlotIdsSet()
@@ -394,20 +388,6 @@
private var reusableCount = 0
private var precomposedCount = 0
- init {
- root.>
- // UiApplier.clear removes a single level of nodes, but dispatches detach recursively.
- // SubcomposeLayout traverses children manually, so explicitly removing children from
- // it here whenever root is detached.
- // As parent detach happens before children, onDetach for children nodes will only be
- // called once.
- disposeCurrentNodes()
- root.>
- throw IllegalStateException("Disposed subcompose root shouldn't be reattached")
- }
- }
- }
-
fun subcompose(slotId: Any?, content: @Composable () -> Unit): List<Measurable> {
makeSureStateIsConsistent()
val layoutState = root.layoutState
@@ -422,8 +402,7 @@
precomposedCount--
precomposed
} else {
- takeNodeFromReusables(slotId)
- ?: createNodeAt(currentIndex)
+ takeNodeFromReusables(slotId) ?: createNodeAt(currentIndex)
}
}
@@ -447,65 +426,47 @@
val nodeState = nodeToNodeState.getOrPut(node) {
NodeState(slotId, {})
}
-
val hasPendingChanges = nodeState.composition?.hasInvalidations ?: true
if (nodeState.content !== content || hasPendingChanges || nodeState.forceRecompose) {
- subcompose(node, nodeState, content)
+ nodeState.content = content
+ subcompose(node, nodeState)
nodeState.forceRecompose = false
}
}
- private fun subcompose(
- node: LayoutNode,
- nodeState: NodeState,
- content: @Composable () -> Unit
- ) {
+ private fun subcompose(node: LayoutNode, nodeState: NodeState) {
Snapshot.withoutReadObservation {
- if (nodeState.forceResetContent) {
- require(nodeState.active) { "Node must be active to reset content" }
- nodeState.active = false
- subcomposeNode(node, nodeState)
- nodeState.active = true
+ ignoreRemeasureRequests {
+ val content = nodeState.content
+ nodeState.composition = subcomposeInto(
+ existing = nodeState.composition,
+ container = node,
+ parent = compositionContext ?: error("parent composition reference not set"),
+ // Do not optimize this by passing nodeState.content directly; the additional
+ // composable function call from the lambda expression affects the scope of
+ // recomposition and recomposition of siblings.
+ composable = {
+ ReusableContentHost(nodeState.active, content)
+ }
+ )
}
-
- require(nodeState.active && !nodeState.forceResetContent) {
- "Node must be active with content cleared for correct subcompose"
- }
-
- nodeState.content = content
- subcomposeNode(node, nodeState)
}
}
- private fun subcomposeNode(
- node: LayoutNode,
- nodeState: NodeState
- ) {
- ignoreRemeasureRequests {
- val content = nodeState.content
- val existing = nodeState.composition
- val parentContext = compositionContext ?: error("parent composition reference not set")
-
- nodeState.composition =
- if (existing == null || existing.isDisposed) {
- createSubcomposition(node, parentContext)
- } else {
- existing
- }
- .apply {
- setContent {
- val active = nodeState.active
-
- SideEffect {
- if (!active) {
- nodeState.forceResetContent = false
- }
- }
-
- ReusableContentHost(active, content)
- }
- }
+ private fun subcomposeInto(
+ existing: Composition?,
+ container: LayoutNode,
+ parent: CompositionContext,
+ composable: @Composable () -> Unit
+ ): Composition {
+ return if (existing == null || existing.isDisposed) {
+ createSubcomposition(container, parent)
+ } else {
+ existing
}
+ .apply {
+ setContent(composable)
+ }
}
private fun getSlotIdAtIndex(index: Int): Any? {
@@ -513,33 +474,29 @@
return nodeToNodeState[node]!!.slotId
}
- fun disposeOrReuseStartingFromIndex(startIndex: Int, retainAllSlots: Boolean = false) {
+ fun disposeOrReuseStartingFromIndex(startIndex: Int) {
reusableCount = 0
val lastReusableIndex = root.foldedChildren.size - precomposedCount - 1
if (startIndex <= lastReusableIndex) {
// construct the set of available slot ids
reusableSlotIdsSet.clear()
- if (!retainAllSlots) {
- for (i in startIndex..lastReusableIndex) {
- reusableSlotIdsSet.add(getSlotIdAtIndex(i))
- }
-
- slotReusePolicy.getSlotsToRetain(reusableSlotIdsSet)
+ for (i in startIndex..lastReusableIndex) {
+ reusableSlotIdsSet.add(getSlotIdAtIndex(i))
}
+
+ slotReusePolicy.getSlotsToRetain(reusableSlotIdsSet)
// iterating backwards so it is easier to remove items
var i = lastReusableIndex
while (i >= startIndex) {
val node = root.foldedChildren[i]
val nodeState = nodeToNodeState[node]!!
val slotId = nodeState.slotId
- if (retainAllSlots || reusableSlotIdsSet.contains(slotId)) {
+ if (reusableSlotIdsSet.contains(slotId)) {
node.measuredByParent = UsageByParent.NotUsed
reusableCount++
nodeState.active = false
- nodeState.forceResetContent = true
} else {
ignoreRemeasureRequests {
- slotIdToNode.remove(slotId)
nodeToNodeState.remove(node)
nodeState.composition?.dispose()
root.removeAt(i, 1)
@@ -718,19 +675,6 @@
}
}
- private fun scheduleNodeDispose(node: LayoutNode) {
- val nodeState = nodeToNodeState.remove(node) ?: return
- precomposeMap.remove(nodeState.slotId)?.let {
- precomposedCount--
- }
- slotIdToNode.remove(nodeState.slotId)
- if (!disposeOnDetach) {
- detachedNodes += nodeState
- } else {
- nodeState.composition?.dispose()
- }
- }
-
fun forceRecomposeChildren() {
nodeToNodeState.forEach { (_, nodeState) ->
nodeState.forceRecompose = true
@@ -755,56 +699,18 @@
private inline fun ignoreRemeasureRequests(block: () -> Unit) =
root.ignoreRemeasureRequests(block)
- fun keepDetachedNodes() {
- disposeOnDetach = false
- }
-
- fun disposeOrReuseChildren() {
- reuseCurrentNodes()
- disposeDetachedNodes()
-
- makeSureStateIsConsistent()
-
- disposeOnDetach = true
- }
-
- private fun reuseCurrentNodes() {
- precomposedCount = 0
- precomposeMap.clear()
- disposeOrReuseStartingFromIndex(0, retainAllSlots = true)
- }
-
- private fun disposeDetachedNodes() {
- detachedNodes.forEach {
- it.composition?.dispose()
- }
- detachedNodes.clear()
- }
-
- private fun disposeCurrentNodes() {
- if (!disposeOnDetach) {
- // Node is detached before DisposableEffect.onDispose, make sure we remove
- // composition there to save the state
- detachedNodes.addAll(nodeToNodeState.values)
- } else {
- // Effect already disposed this state, dispose all compositions
- root.ignoreRemeasureRequests {
- nodeToNodeState.values.forEach {
- it.composition?.dispose()
- }
+ fun disposeCurrentNodes() {
+ root.ignoreRemeasureRequests {
+ nodeToNodeState.values.forEach {
+ it.composition?.dispose()
}
+ root.removeAll()
}
-
nodeToNodeState.clear()
slotIdToNode.clear()
precomposedCount = 0
reusableCount = 0
precomposeMap.clear()
-
- root.ignoreRemeasureRequests {
- root.removeAll()
- }
-
makeSureStateIsConsistent()
}
@@ -814,8 +720,7 @@
var composition: Composition? = null
) {
var forceRecompose = false
- var forceResetContent = false
- var active: Boolean by mutableStateOf(true)
+ var active by mutableStateOf(true)
}
private inner class Scope : SubcomposeMeasureScope {
diff --git a/tv/tv-foundation/src/androidTest/java/androidx/tv/foundation/lazy/grid/LazyGridSlotsReuseTest.kt b/tv/tv-foundation/src/androidTest/java/androidx/tv/foundation/lazy/grid/LazyGridSlotsReuseTest.kt
index 0501ac7..cd16ae9 100644
--- a/tv/tv-foundation/src/androidTest/java/androidx/tv/foundation/lazy/grid/LazyGridSlotsReuseTest.kt
+++ b/tv/tv-foundation/src/androidTest/java/androidx/tv/foundation/lazy/grid/LazyGridSlotsReuseTest.kt
@@ -21,14 +21,13 @@
import androidx.compose.foundation.layout.height
import androidx.compose.foundation.layout.width
import androidx.compose.runtime.Composable
+import androidx.compose.runtime.remember
import androidx.compose.ui.Modifier
-import androidx.compose.ui.layout.layout
import androidx.compose.ui.platform.testTag
import androidx.compose.ui.test.assertIsDisplayed
import androidx.compose.ui.test.assertIsNotDisplayed
import androidx.compose.ui.test.junit4.createComposeRule
import androidx.compose.ui.test.onNodeWithTag
-import androidx.compose.ui.unit.IntOffset
import androidx.compose.ui.unit.dp
import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.filters.LargeTest
@@ -320,24 +319,9 @@
fun scrollingBackReusesTheSameSlot() {
lateinit var state: TvLazyGridState
var counter0 = 0
- var counter1 = 0
-
- val measureCountModifier0 = Modifier.layout { measurable, constraints ->
- counter0++
- val placeable = measurable.measure(constraints)
- layout(placeable.width, placeable.height) {
- placeable.place(IntOffset.Zero)
- }
- }
-
- val measureCountModifier1 = Modifier.layout { measurable, constraints ->
- counter1++
- val placeable = measurable.measure(constraints)
- layout(placeable.width, placeable.height) {
- placeable.place(IntOffset.Zero)
- }
- }
-
+ var counter1 = 10
+ var rememberedValue0 = -1
+ var rememberedValue1 = -1
rule.setContent {
state = rememberTvLazyGridState()
TvLazyVerticalGrid(
@@ -346,17 +330,13 @@
state
) {
items(100) {
- val modifier = when (it) {
- 0 -> measureCountModifier0
- 1 -> measureCountModifier1
- else -> Modifier
+ if (it == 0) {
+ rememberedValue0 = remember { counter0++ }
}
- Spacer(
- Modifier
- .height(itemsSizeDp)
- .testTag("$it")
- .then(modifier)
- )
+ if (it == 1) {
+ rememberedValue1 = remember { counter1++ }
+ }
+ Spacer(Modifier.height(itemsSizeDp).fillMaxWidth().testTag("$it"))
}
}
}
@@ -368,10 +348,10 @@
}
rule.runOnIdle {
- Truth.assertWithMessage("Item 0 measured $counter0 times, expected 1.")
- .that(counter0).isEqualTo(1)
- Truth.assertWithMessage("Item 1 measured $counter1 times, expected 1.")
- .that(counter1).isEqualTo(1)
+ Truth.assertWithMessage("Item 0 restored remembered value is $rememberedValue0")
+ .that(rememberedValue0).isEqualTo(0)
+ Truth.assertWithMessage("Item 1 restored remembered value is $rememberedValue1")
+ .that(rememberedValue1).isEqualTo(10)
}
rule.onNodeWithTag("0")
diff --git a/tv/tv-foundation/src/androidTest/java/androidx/tv/foundation/lazy/list/LazyListSlotsReuseTest.kt b/tv/tv-foundation/src/androidTest/java/androidx/tv/foundation/lazy/list/LazyListSlotsReuseTest.kt
index aea2b2f..3083be7 100644
--- a/tv/tv-foundation/src/androidTest/java/androidx/tv/foundation/lazy/list/LazyListSlotsReuseTest.kt
+++ b/tv/tv-foundation/src/androidTest/java/androidx/tv/foundation/lazy/list/LazyListSlotsReuseTest.kt
@@ -23,14 +23,13 @@
import androidx.compose.foundation.layout.height
import androidx.compose.foundation.layout.width
import androidx.compose.runtime.Composable
+import androidx.compose.runtime.remember
import androidx.compose.ui.Modifier
-import androidx.compose.ui.layout.layout
import androidx.compose.ui.platform.testTag
import androidx.compose.ui.test.assertIsDisplayed
import androidx.compose.ui.test.assertIsNotDisplayed
import androidx.compose.ui.test.junit4.createComposeRule
import androidx.compose.ui.test.onNodeWithTag
-import androidx.compose.ui.unit.IntOffset
import androidx.compose.ui.unit.dp
import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.filters.LargeTest
@@ -337,24 +336,9 @@
fun scrollingBackReusesTheSameSlot() {
lateinit var state: TvLazyListState
var counter0 = 0
- var counter1 = 0
-
- val measureCountModifier0 = Modifier.layout { measurable, constraints ->
- counter0++
- val placeable = measurable.measure(constraints)
- layout(placeable.width, placeable.height) {
- placeable.place(IntOffset.Zero)
- }
- }
-
- val measureCountModifier1 = Modifier.layout { measurable, constraints ->
- counter1++
- val placeable = measurable.measure(constraints)
- layout(placeable.width, placeable.height) {
- placeable.place(IntOffset.Zero)
- }
- }
-
+ var counter1 = 10
+ var rememberedValue0 = -1
+ var rememberedValue1 = -1
rule.setContent {
state = rememberTvLazyListState()
TvLazyColumn(
@@ -363,18 +347,15 @@
pivotOffsets = PivotOffsets(parentFraction = 0f)
) {
items(100) {
- val modifier = when (it) {
- 0 -> measureCountModifier0
- 1 -> measureCountModifier1
- else -> Modifier
+ if (it == 0) {
+ rememberedValue0 = remember { counter0++ }
}
- Spacer(
- Modifier
- .height(itemsSizeDp)
- .fillParentMaxWidth()
- .testTag("$it")
- .then(modifier)
- )
+ if (it == 1) {
+ rememberedValue1 = remember { counter1++ }
+ }
+ Box(
+ Modifier.height(itemsSizeDp).fillParentMaxWidth().testTag("$it")
+ .focusable())
}
}
}
@@ -386,10 +367,10 @@
}
rule.runOnIdle {
- Truth.assertWithMessage("Item 0 measured $counter0 times, expected 1.")
- .that(counter0).isEqualTo(1)
- Truth.assertWithMessage("Item 1 measured $counter1 times, expected 1.")
- .that(counter1).isEqualTo(1)
+ Truth.assertWithMessage("Item 0 restored remembered value is $rememberedValue0")
+ .that(rememberedValue0).isEqualTo(0)
+ Truth.assertWithMessage("Item 1 restored remembered value is $rememberedValue1")
+ .that(rememberedValue1).isEqualTo(10)
}
rule.onNodeWithTag("0")