diff options
| author | Android Build Coastguard Worker <android-build-coastguard-worker@google.com> | 2023-06-06 03:23:15 +0000 |
|---|---|---|
| committer | Gerrit Code Review <noreply-gerritcodereview@google.com> | 2023-06-06 03:23:15 +0000 |
| commit | b76b96455de6f7a21c22983d33ca78df07aa7a37 (patch) | |
| tree | ee01d4bf3d0d6ab701ebdabe1a9168d682d533eb | |
| parent | 7ddc921d2b356e3b3906937478fbebb274a12b4e (diff) | |
| parent | 84cce69f7a676211f65729f766344a78fd55a707 (diff) | |
| download | support-b76b96455de6f7a21c22983d33ca78df07aa7a37.tar.gz | |
Merge "Do not rerun placement block of layout modifiers when parent moves children" into snap-temp-L75800000961093494
21 files changed, 981 insertions, 329 deletions
diff --git a/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/grid/LazyGridBeyondBoundsTest.kt b/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/grid/LazyGridBeyondBoundsTest.kt index f0315addc13..107ee850b21 100644 --- a/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/grid/LazyGridBeyondBoundsTest.kt +++ b/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/grid/LazyGridBeyondBoundsTest.kt @@ -18,6 +18,7 @@ package androidx.compose.foundation.lazy.grid import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.size +import androidx.compose.foundation.lazy.list.TrackPlacedElement import androidx.compose.runtime.CompositionLocalProvider import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf @@ -32,7 +33,6 @@ import androidx.compose.ui.layout.BeyondBoundsLayout.LayoutDirection.Companion.B import androidx.compose.ui.layout.BeyondBoundsLayout.LayoutDirection.Companion.Left import androidx.compose.ui.layout.BeyondBoundsLayout.LayoutDirection.Companion.Right import androidx.compose.ui.layout.ModifierLocalBeyondBoundsLayout -import androidx.compose.ui.layout.onPlaced import androidx.compose.ui.modifier.modifierLocalConsumer import androidx.compose.ui.platform.LocalLayoutDirection import androidx.compose.ui.test.junit4.ComposeContentTestRule @@ -97,7 +97,7 @@ class LazyGridBeyondBoundsTest(param: Param) { Box( Modifier .size(10.toDp()) - .onPlaced { placedItems += index } + .trackPlaced(index) ) } } @@ -117,7 +117,7 @@ class LazyGridBeyondBoundsTest(param: Param) { Box( Modifier .size(10.toDp()) - .onPlaced { placedItems += index } + .trackPlaced(index) ) } } @@ -137,7 +137,7 @@ class LazyGridBeyondBoundsTest(param: Param) { Box( Modifier .size(10.toDp()) - .onPlaced { placedItems += index } + .trackPlaced(index) ) } } @@ -191,13 +191,13 @@ class LazyGridBeyondBoundsTest(param: Param) { Box( Modifier .size(10.toDp()) - .onPlaced { placedItems += index } + .trackPlaced(index) ) } item { Box(Modifier .size(10.toDp()) - .onPlaced { placedItems += 5 } + .trackPlaced(5) .modifierLocalConsumer { beyondBoundsLayout = ModifierLocalBeyondBoundsLayout.current } @@ -207,11 +207,10 @@ class LazyGridBeyondBoundsTest(param: Param) { Box( Modifier .size(10.toDp()) - .onPlaced { placedItems += index + 6 } + .trackPlaced(index + 6) ) } } - rule.runOnIdle { placedItems.clear() } // Act. rule.runOnUiThread { @@ -224,7 +223,6 @@ class LazyGridBeyondBoundsTest(param: Param) { assertThat(placedItems).containsExactly(5, 6, 7, 8) assertThat(visibleItems).containsExactly(5, 6, 7) } - placedItems.clear() // Just return true so that we stop as soon as we run this once. // This should result in one extra item being added. true @@ -251,13 +249,13 @@ class LazyGridBeyondBoundsTest(param: Param) { Box( Modifier .size(itemSizeDp) - .onPlaced { placedItems += index } + .trackPlaced(index) ) } item { Box(Modifier .size(itemSizeDp) - .onPlaced { placedItems += 11 } + .trackPlaced(11) .modifierLocalConsumer { beyondBoundsLayout = ModifierLocalBeyondBoundsLayout.current } @@ -267,11 +265,10 @@ class LazyGridBeyondBoundsTest(param: Param) { Box( Modifier .size(itemSizeDp) - .onPlaced { placedItems += index + 12 } + .trackPlaced(index + 12) ) } } - rule.runOnIdle { placedItems.clear() } // Act. rule.runOnUiThread { @@ -284,7 +281,6 @@ class LazyGridBeyondBoundsTest(param: Param) { assertThat(placedItems).containsExactly(10, 11, 12, 13, 14, 15, 16) assertThat(visibleItems).containsExactly(10, 11, 12, 13, 14, 15) } - placedItems.clear() // Just return true so that we stop as soon as we run this once. // This should result in one extra item being added. true @@ -307,14 +303,14 @@ class LazyGridBeyondBoundsTest(param: Param) { Box( Modifier .size(10.toDp()) - .onPlaced { placedItems += index } + .trackPlaced(index) ) } item { Box( Modifier .size(10.toDp()) - .onPlaced { placedItems += 5 } + .trackPlaced(5) .modifierLocalConsumer { beyondBoundsLayout = ModifierLocalBeyondBoundsLayout.current } @@ -324,17 +320,15 @@ class LazyGridBeyondBoundsTest(param: Param) { Box( Modifier .size(10.toDp()) - .onPlaced { placedItems += index + 6 } + .trackPlaced(index + 6) ) } } - rule.runOnIdle { placedItems.clear() } // Act. rule.runOnUiThread { beyondBoundsLayout!!.layout(beyondBoundsLayoutDirection) { if (--extraItemCount > 0) { - placedItems.clear() // Return null to continue the search. null } else { @@ -346,7 +340,6 @@ class LazyGridBeyondBoundsTest(param: Param) { assertThat(placedItems).containsExactly(5, 6, 7, 8, 9) assertThat(visibleItems).containsExactly(5, 6, 7) } - placedItems.clear() // Return true to stop the search. true } @@ -368,7 +361,7 @@ class LazyGridBeyondBoundsTest(param: Param) { Box( Modifier .size(10.toDp()) - .onPlaced { placedItems += index } + .trackPlaced(index) ) } item { @@ -378,26 +371,22 @@ class LazyGridBeyondBoundsTest(param: Param) { .modifierLocalConsumer { beyondBoundsLayout = ModifierLocalBeyondBoundsLayout.current } - .onPlaced { placedItems += 5 } + .trackPlaced(5) ) } items(5) { index -> Box( Modifier .size(10.toDp()) - .onPlaced { - placedItems += index + 6 - } + .trackPlaced(index + 6) ) } } - rule.runOnIdle { placedItems.clear() } // Act. rule.runOnUiThread { beyondBoundsLayout!!.layout(beyondBoundsLayoutDirection) { if (hasMoreContent) { - placedItems.clear() // Just return null so that we keep adding more items till we reach the end. null } else { @@ -409,7 +398,6 @@ class LazyGridBeyondBoundsTest(param: Param) { assertThat(placedItems).containsExactly(5, 6, 7, 8, 9, 10) assertThat(visibleItems).containsExactly(5, 6, 7) } - placedItems.clear() // Return true to end the search. true } @@ -431,14 +419,14 @@ class LazyGridBeyondBoundsTest(param: Param) { Box( Modifier .size(10.toDp()) - .onPlaced { placedItems += index } + .trackPlaced(index) ) } item { Box( Modifier .size(10.toDp()) - .onPlaced { placedItems += 5 } + .trackPlaced(5) .modifierLocalConsumer { beyondBoundsLayout = ModifierLocalBeyondBoundsLayout.current } @@ -448,14 +436,13 @@ class LazyGridBeyondBoundsTest(param: Param) { Box( Modifier .size(10.toDp()) - .onPlaced { placedItems += index + 6 } + .trackPlaced(index + 6) ) } } rule.runOnIdle { assertThat(placedItems).containsExactly(5, 6, 7) assertThat(visibleItems).containsExactly(5, 6, 7) - placedItems.clear() } // Act. @@ -477,7 +464,6 @@ class LazyGridBeyondBoundsTest(param: Param) { } } } - placedItems.clear() // Just return true so that we stop as soon as we run this once. // This should result in one extra item being added. true @@ -509,9 +495,7 @@ class LazyGridBeyondBoundsTest(param: Param) { Box( Modifier .size(10.toDp()) - .onPlaced { - placedItems += index - } + .trackPlaced(index) ) } item { @@ -521,20 +505,17 @@ class LazyGridBeyondBoundsTest(param: Param) { .modifierLocalConsumer { beyondBoundsLayout = ModifierLocalBeyondBoundsLayout.current } - .onPlaced { placedItems += 5 } + .trackPlaced(5) ) } items(5) { index -> Box( Modifier .size(10.toDp()) - .onPlaced { - placedItems += index + 6 - } + .trackPlaced(index + 6) ) } } - rule.runOnIdle { placedItems.clear() } // Act. var count = 0 @@ -542,7 +523,6 @@ class LazyGridBeyondBoundsTest(param: Param) { beyondBoundsLayout!!.layout(beyondBoundsLayoutDirection) { // Assert that we don't keep iterating when there is no ending condition. assertThat(count++).isLessThan(lazyGridState.layoutInfo.totalItemsCount) - placedItems.clear() // Always return null to continue the search. null } @@ -636,4 +616,7 @@ class LazyGridBeyondBoundsTest(param: Param) { private fun unsupportedDirection(): Nothing = error( "Lazy list does not support beyond bounds layout for the specified direction" ) + + private fun Modifier.trackPlaced(index: Int): Modifier = + this then TrackPlacedElement(placedItems, index) } diff --git a/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/list/LazyListBeyondBoundsAndExtraItemsTest.kt b/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/list/LazyListBeyondBoundsAndExtraItemsTest.kt index 559f3bb4f3e..3be7e1910ac 100644 --- a/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/list/LazyListBeyondBoundsAndExtraItemsTest.kt +++ b/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/list/LazyListBeyondBoundsAndExtraItemsTest.kt @@ -31,7 +31,6 @@ import androidx.compose.ui.layout.BeyondBoundsLayout.LayoutDirection.Companion.B import androidx.compose.ui.layout.BeyondBoundsLayout.LayoutDirection.Companion.Left import androidx.compose.ui.layout.BeyondBoundsLayout.LayoutDirection.Companion.Right import androidx.compose.ui.layout.ModifierLocalBeyondBoundsLayout -import androidx.compose.ui.layout.onPlaced import androidx.compose.ui.modifier.modifierLocalConsumer import androidx.compose.ui.platform.LocalLayoutDirection import androidx.compose.ui.unit.LayoutDirection @@ -51,12 +50,12 @@ class LazyListBeyondBoundsAndExtraItemsTest(val config: Config) : private val beyondBoundsLayoutDirection = config.beyondBoundsLayoutDirection private val reverseLayout = config.reverseLayout private val layoutDirection = config.layoutDirection + private val placedItems = mutableSetOf<Int>() @OptIn(ExperimentalComposeUiApi::class) @Test fun verifyItemsArePlacedBeforeBeyondBoundsItems_oneBeyondBoundItem() { // Arrange - val placedItems = mutableSetOf<Int>() var beyondBoundsLayout: BeyondBoundsLayout? = null val lazyListState = LazyListState() rule.setContent { @@ -71,14 +70,14 @@ class LazyListBeyondBoundsAndExtraItemsTest(val config: Config) : Box( Modifier .size(10.dp) - .onPlaced { placedItems += index } + .trackPlaced(index) ) } item { Box( Modifier .size(10.dp) - .onPlaced { placedItems += 5 } + .trackPlaced(5) .modifierLocalConsumer { beyondBoundsLayout = ModifierLocalBeyondBoundsLayout.current } @@ -88,14 +87,13 @@ class LazyListBeyondBoundsAndExtraItemsTest(val config: Config) : Box( Modifier .size(10.dp) - .onPlaced { placedItems += index + 6 } + .trackPlaced(index + 6) ) } } } } rule.runOnIdle { runBlocking { lazyListState.scrollToItem(5) } } - rule.runOnIdle { placedItems.clear() } // Act rule.runOnUiThread { @@ -107,7 +105,6 @@ class LazyListBeyondBoundsAndExtraItemsTest(val config: Config) : assertThat(placedItems).containsAtLeast(4, 5, 6, 7, 8, 9) } assertThat(lazyListState.visibleItems).containsAtLeast(5, 6, 7) - placedItems.clear() true } } @@ -123,7 +120,6 @@ class LazyListBeyondBoundsAndExtraItemsTest(val config: Config) : @Test fun verifyItemsArePlacedBeforeBeyondBoundsItems_twoBeyondBoundItem() { // Arrange - val placedItems = mutableSetOf<Int>() var beyondBoundsLayout: BeyondBoundsLayout? = null val lazyListState = LazyListState() var extraItemCount = 2 @@ -139,14 +135,14 @@ class LazyListBeyondBoundsAndExtraItemsTest(val config: Config) : Box( Modifier .size(10.dp) - .onPlaced { placedItems += index } + .trackPlaced(index) ) } item { Box( Modifier .size(10.dp) - .onPlaced { placedItems += 5 } + .trackPlaced(5) .modifierLocalConsumer { beyondBoundsLayout = ModifierLocalBeyondBoundsLayout.current } @@ -156,20 +152,18 @@ class LazyListBeyondBoundsAndExtraItemsTest(val config: Config) : Box( Modifier .size(10.dp) - .onPlaced { placedItems += index + 6 } + .trackPlaced(index + 6) ) } } } } rule.runOnIdle { runBlocking { lazyListState.scrollToItem(5) } } - rule.runOnIdle { placedItems.clear() } // Act rule.runOnUiThread { beyondBoundsLayout!!.layout(beyondBoundsLayoutDirection) { if (--extraItemCount > 0) { - placedItems.clear() // Return null to continue the search. null } else { @@ -180,7 +174,6 @@ class LazyListBeyondBoundsAndExtraItemsTest(val config: Config) : assertThat(placedItems).containsAtLeast(4, 5, 6, 7, 8, 9, 10) } assertThat(lazyListState.visibleItems).containsAtLeast(5, 6, 7) - placedItems.clear() true } } @@ -249,4 +242,7 @@ class LazyListBeyondBoundsAndExtraItemsTest(val config: Config) : Before -> true else -> error("Unsupported BeyondBoundsDirection") } + + private fun Modifier.trackPlaced(index: Int): Modifier = + this then TrackPlacedElement(placedItems, index) }
\ No newline at end of file diff --git a/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/list/LazyListBeyondBoundsTest.kt b/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/list/LazyListBeyondBoundsTest.kt index d431ea40844..4e0a1edc243 100644 --- a/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/list/LazyListBeyondBoundsTest.kt +++ b/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/list/LazyListBeyondBoundsTest.kt @@ -36,9 +36,12 @@ import androidx.compose.ui.layout.BeyondBoundsLayout.LayoutDirection.Companion.B import androidx.compose.ui.layout.BeyondBoundsLayout.LayoutDirection.Companion.Below import androidx.compose.ui.layout.BeyondBoundsLayout.LayoutDirection.Companion.Left import androidx.compose.ui.layout.BeyondBoundsLayout.LayoutDirection.Companion.Right +import androidx.compose.ui.layout.LayoutCoordinates import androidx.compose.ui.layout.ModifierLocalBeyondBoundsLayout -import androidx.compose.ui.layout.onPlaced import androidx.compose.ui.modifier.modifierLocalConsumer +import androidx.compose.ui.node.LayoutAwareModifierNode +import androidx.compose.ui.node.ModifierNodeElement +import androidx.compose.ui.platform.InspectorInfo import androidx.compose.ui.platform.LocalLayoutDirection import androidx.compose.ui.test.junit4.ComposeContentTestRule import androidx.compose.ui.test.junit4.createComposeRule @@ -102,7 +105,7 @@ class LazyListBeyondBoundsTest(param: Param) { Box( Modifier .size(10.toDp()) - .onPlaced { placedItems += index } + .trackPlaced(index) ) } } @@ -122,7 +125,7 @@ class LazyListBeyondBoundsTest(param: Param) { Box( Modifier .size(10.toDp()) - .onPlaced { placedItems += index } + .trackPlaced(index) ) } } @@ -142,7 +145,7 @@ class LazyListBeyondBoundsTest(param: Param) { Box( Modifier .size(10.toDp()) - .onPlaced { placedItems += index } + .trackPlaced(index) ) } } @@ -196,13 +199,13 @@ class LazyListBeyondBoundsTest(param: Param) { Box( Modifier .size(10.toDp()) - .onPlaced { placedItems += index } + .trackPlaced(index) ) } item { Box(Modifier .size(10.toDp()) - .onPlaced { placedItems += 5 } + .trackPlaced(5) .modifierLocalConsumer { beyondBoundsLayout = ModifierLocalBeyondBoundsLayout.current } @@ -212,11 +215,10 @@ class LazyListBeyondBoundsTest(param: Param) { Box( Modifier .size(10.toDp()) - .onPlaced { placedItems += index + 6 } + .trackPlaced(index + 6) ) } } - rule.runOnIdle { placedItems.clear() } // Act. rule.runOnUiThread { @@ -229,7 +231,6 @@ class LazyListBeyondBoundsTest(param: Param) { assertThat(placedItems).containsExactly(5, 6, 7, 8) assertThat(visibleItems).containsExactly(5, 6, 7) } - placedItems.clear() // Just return true so that we stop as soon as we run this once. // This should result in one extra item being added. true @@ -252,14 +253,14 @@ class LazyListBeyondBoundsTest(param: Param) { Box( Modifier .size(10.toDp()) - .onPlaced { placedItems += index } + .trackPlaced(index) ) } item { Box( Modifier .size(10.toDp()) - .onPlaced { placedItems += 5 } + .trackPlaced(5) .modifierLocalConsumer { beyondBoundsLayout = ModifierLocalBeyondBoundsLayout.current } @@ -269,17 +270,15 @@ class LazyListBeyondBoundsTest(param: Param) { Box( Modifier .size(10.toDp()) - .onPlaced { placedItems += index + 6 } + .trackPlaced(index + 6) ) } } - rule.runOnIdle { placedItems.clear() } // Act. rule.runOnUiThread { beyondBoundsLayout!!.layout(beyondBoundsLayoutDirection) { if (--extraItemCount > 0) { - placedItems.clear() // Return null to continue the search. null } else { @@ -291,7 +290,6 @@ class LazyListBeyondBoundsTest(param: Param) { assertThat(placedItems).containsExactly(5, 6, 7, 8, 9) assertThat(visibleItems).containsExactly(5, 6, 7) } - placedItems.clear() // Return true to stop the search. true } @@ -313,7 +311,7 @@ class LazyListBeyondBoundsTest(param: Param) { Box( Modifier .size(10.toDp()) - .onPlaced { placedItems += index } + .trackPlaced(index) ) } item { @@ -323,26 +321,22 @@ class LazyListBeyondBoundsTest(param: Param) { .modifierLocalConsumer { beyondBoundsLayout = ModifierLocalBeyondBoundsLayout.current } - .onPlaced { placedItems += 5 } + .trackPlaced(5) ) } items(5) { index -> Box( Modifier .size(10.toDp()) - .onPlaced { - placedItems += index + 6 - } + .trackPlaced(index + 6) ) } } - rule.runOnIdle { placedItems.clear() } // Act. rule.runOnUiThread { beyondBoundsLayout!!.layout(beyondBoundsLayoutDirection) { if (hasMoreContent) { - placedItems.clear() // Just return null so that we keep adding more items till we reach the end. null } else { @@ -354,7 +348,6 @@ class LazyListBeyondBoundsTest(param: Param) { assertThat(placedItems).containsExactly(5, 6, 7, 8, 9, 10) assertThat(visibleItems).containsExactly(5, 6, 7) } - placedItems.clear() // Return true to end the search. true } @@ -376,14 +369,14 @@ class LazyListBeyondBoundsTest(param: Param) { Box( Modifier .size(10.toDp()) - .onPlaced { placedItems += index } + .trackPlaced(index) ) } item { Box( Modifier .size(10.toDp()) - .onPlaced { placedItems += 5 } + .trackPlaced(5) .modifierLocalConsumer { beyondBoundsLayout = ModifierLocalBeyondBoundsLayout.current } @@ -393,14 +386,13 @@ class LazyListBeyondBoundsTest(param: Param) { Box( Modifier .size(10.toDp()) - .onPlaced { placedItems += index + 6 } + .trackPlaced(index + 6) ) } } rule.runOnIdle { assertThat(placedItems).containsExactly(5, 6, 7) assertThat(visibleItems).containsExactly(5, 6, 7) - placedItems.clear() } // Act. @@ -422,7 +414,6 @@ class LazyListBeyondBoundsTest(param: Param) { } } } - placedItems.clear() // Just return true so that we stop as soon as we run this once. // This should result in one extra item being added. true @@ -454,9 +445,7 @@ class LazyListBeyondBoundsTest(param: Param) { Box( Modifier .size(10.toDp()) - .onPlaced { - placedItems += index - } + .trackPlaced(index) ) } item { @@ -466,20 +455,17 @@ class LazyListBeyondBoundsTest(param: Param) { .modifierLocalConsumer { beyondBoundsLayout = ModifierLocalBeyondBoundsLayout.current } - .onPlaced { placedItems += 5 } + .trackPlaced(5) ) } items(5) { index -> Box( Modifier .size(10.toDp()) - .onPlaced { - placedItems += index + 6 - } + .trackPlaced(index + 6) ) } } - rule.runOnIdle { placedItems.clear() } // Act. var count = 0 @@ -487,7 +473,6 @@ class LazyListBeyondBoundsTest(param: Param) { beyondBoundsLayout!!.layout(beyondBoundsLayoutDirection) { // Assert that we don't keep iterating when there is no ending condition. assertThat(count++).isLessThan(lazyListState.layoutInfo.totalItemsCount) - placedItems.clear() // Always return null to continue the search. null } @@ -576,4 +561,37 @@ class LazyListBeyondBoundsTest(param: Param) { private fun unsupportedDirection(): Nothing = error( "Lazy list does not support beyond bounds layout for the specified direction" ) + + private fun Modifier.trackPlaced(index: Int): Modifier = + this then TrackPlacedElement(placedItems, index) +} + +internal data class TrackPlacedElement( + var placedItems: MutableSet<Int>, + var index: Int +) : ModifierNodeElement<TrackPlacedNode>() { + override fun create() = TrackPlacedNode(placedItems, index) + + override fun update(node: TrackPlacedNode) { + node.placedItems = placedItems + node.index = index + } + + override fun InspectorInfo.inspectableProperties() { + name = "trackPlaced" + properties["index"] = index + } +} + +internal class TrackPlacedNode( + var placedItems: MutableSet<Int>, + var index: Int +) : LayoutAwareModifierNode, Modifier.Node() { + override fun onPlaced(coordinates: LayoutCoordinates) { + placedItems += index + } + + override fun onDetach() { + placedItems -= index + } } diff --git a/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/staggeredgrid/LazyStaggeredGridBeyondBoundsTest.kt b/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/staggeredgrid/LazyStaggeredGridBeyondBoundsTest.kt index 1b82ba82f6a..116e9ae7a50 100644 --- a/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/staggeredgrid/LazyStaggeredGridBeyondBoundsTest.kt +++ b/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/staggeredgrid/LazyStaggeredGridBeyondBoundsTest.kt @@ -18,6 +18,7 @@ package androidx.compose.foundation.lazy.staggeredgrid import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.size +import androidx.compose.foundation.lazy.list.TrackPlacedElement import androidx.compose.runtime.CompositionLocalProvider import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf @@ -32,7 +33,6 @@ import androidx.compose.ui.layout.BeyondBoundsLayout.LayoutDirection.Companion.B import androidx.compose.ui.layout.BeyondBoundsLayout.LayoutDirection.Companion.Left import androidx.compose.ui.layout.BeyondBoundsLayout.LayoutDirection.Companion.Right import androidx.compose.ui.layout.ModifierLocalBeyondBoundsLayout -import androidx.compose.ui.layout.onPlaced import androidx.compose.ui.modifier.modifierLocalConsumer import androidx.compose.ui.platform.LocalLayoutDirection import androidx.compose.ui.test.junit4.ComposeContentTestRule @@ -97,7 +97,7 @@ class LazyStaggeredGridBeyondBoundsTest(param: Param) { Box( Modifier .size(10.toDp()) - .onPlaced { placedItems += index } + .trackPlaced(index) ) } } @@ -117,7 +117,7 @@ class LazyStaggeredGridBeyondBoundsTest(param: Param) { Box( Modifier .size(10.toDp()) - .onPlaced { placedItems += index } + .trackPlaced(index) ) } } @@ -137,7 +137,7 @@ class LazyStaggeredGridBeyondBoundsTest(param: Param) { Box( Modifier .size(10.toDp()) - .onPlaced { placedItems += index } + .trackPlaced(index) ) } } @@ -191,13 +191,13 @@ class LazyStaggeredGridBeyondBoundsTest(param: Param) { Box( Modifier .size(10.toDp()) - .onPlaced { placedItems += index } + .trackPlaced(index) ) } item { Box(Modifier .size(10.toDp()) - .onPlaced { placedItems += 5 } + .trackPlaced(5) .modifierLocalConsumer { beyondBoundsLayout = ModifierLocalBeyondBoundsLayout.current } @@ -207,11 +207,10 @@ class LazyStaggeredGridBeyondBoundsTest(param: Param) { Box( Modifier .size(10.toDp()) - .onPlaced { placedItems += index + 6 } + .trackPlaced(index + 6) ) } } - rule.runOnIdle { placedItems.clear() } // Act. rule.runOnUiThread { @@ -224,7 +223,6 @@ class LazyStaggeredGridBeyondBoundsTest(param: Param) { assertThat(placedItems).containsExactly(5, 6, 7, 8) assertThat(visibleItems).containsExactly(5, 6, 7) } - placedItems.clear() // Just return true so that we stop as soon as we run this once. // This should result in one extra item being added. true @@ -251,13 +249,13 @@ class LazyStaggeredGridBeyondBoundsTest(param: Param) { Box( Modifier .size(itemSizeDp) - .onPlaced { placedItems += index } + .trackPlaced(index) ) } item { Box(Modifier .size(itemSizeDp) - .onPlaced { placedItems += 11 } + .trackPlaced(11) .modifierLocalConsumer { beyondBoundsLayout = ModifierLocalBeyondBoundsLayout.current } @@ -267,11 +265,10 @@ class LazyStaggeredGridBeyondBoundsTest(param: Param) { Box( Modifier .size(itemSizeDp) - .onPlaced { placedItems += index + 12 } + .trackPlaced(index + 12) ) } } - rule.runOnIdle { placedItems.clear() } // Act. rule.runOnUiThread { @@ -284,7 +281,6 @@ class LazyStaggeredGridBeyondBoundsTest(param: Param) { assertThat(placedItems).containsExactly(10, 11, 12, 13, 14, 15, 16) assertThat(visibleItems).containsExactly(10, 11, 12, 13, 14, 15) } - placedItems.clear() // Just return true so that we stop as soon as we run this once. // This should result in one extra item being added. true @@ -319,13 +315,13 @@ class LazyStaggeredGridBeyondBoundsTest(param: Param) { Box( Modifier .size(itemSizeDp * if (index % 2 == 0) 2f else 1f) - .onPlaced { placedItems += index } + .trackPlaced(index) ) } item(span = StaggeredGridItemSpan.FullLine) { Box(Modifier .size(itemSizeDp) - .onPlaced { placedItems += 4 } + .trackPlaced(4) .modifierLocalConsumer { beyondBoundsLayout = ModifierLocalBeyondBoundsLayout.current } @@ -335,11 +331,10 @@ class LazyStaggeredGridBeyondBoundsTest(param: Param) { Box( Modifier .size(itemSizeDp * if (index % 2 == 0) 2f else 1f) - .onPlaced { placedItems += index + 5 } + .trackPlaced(index + 5) ) } } - rule.runOnIdle { placedItems.clear() } // Act. rule.runOnUiThread { @@ -352,7 +347,6 @@ class LazyStaggeredGridBeyondBoundsTest(param: Param) { assertThat(placedItems).containsExactly(4, 5, 6, 7, 8) assertThat(visibleItems).containsExactly(4, 5, 6, 7) } - placedItems.clear() // Just return true so that we stop as soon as we run this once. // This should result in one extra item being added. true @@ -375,14 +369,14 @@ class LazyStaggeredGridBeyondBoundsTest(param: Param) { Box( Modifier .size(10.toDp()) - .onPlaced { placedItems += index } + .trackPlaced(index) ) } item { Box( Modifier .size(10.toDp()) - .onPlaced { placedItems += 5 } + .trackPlaced(5) .modifierLocalConsumer { beyondBoundsLayout = ModifierLocalBeyondBoundsLayout.current } @@ -392,17 +386,15 @@ class LazyStaggeredGridBeyondBoundsTest(param: Param) { Box( Modifier .size(10.toDp()) - .onPlaced { placedItems += index + 6 } + .trackPlaced(index + 6) ) } } - rule.runOnIdle { placedItems.clear() } // Act. rule.runOnUiThread { beyondBoundsLayout!!.layout(beyondBoundsLayoutDirection) { if (--extraItemCount > 0) { - placedItems.clear() // Return null to continue the search. null } else { @@ -414,7 +406,6 @@ class LazyStaggeredGridBeyondBoundsTest(param: Param) { assertThat(placedItems).containsExactly(5, 6, 7, 8, 9) assertThat(visibleItems).containsExactly(5, 6, 7) } - placedItems.clear() // Return true to stop the search. true } @@ -436,7 +427,7 @@ class LazyStaggeredGridBeyondBoundsTest(param: Param) { Box( Modifier .size(10.toDp()) - .onPlaced { placedItems += index } + .trackPlaced(index) ) } item { @@ -446,26 +437,22 @@ class LazyStaggeredGridBeyondBoundsTest(param: Param) { .modifierLocalConsumer { beyondBoundsLayout = ModifierLocalBeyondBoundsLayout.current } - .onPlaced { placedItems += 5 } + .trackPlaced(5) ) } items(5) { index -> Box( Modifier .size(10.toDp()) - .onPlaced { - placedItems += index + 6 - } + .trackPlaced(index + 6) ) } } - rule.runOnIdle { placedItems.clear() } // Act. rule.runOnUiThread { beyondBoundsLayout!!.layout(beyondBoundsLayoutDirection) { if (hasMoreContent) { - placedItems.clear() // Just return null so that we keep adding more items till we reach the end. null } else { @@ -477,7 +464,6 @@ class LazyStaggeredGridBeyondBoundsTest(param: Param) { assertThat(placedItems).containsExactly(5, 6, 7, 8, 9, 10) assertThat(visibleItems).containsExactly(5, 6, 7) } - placedItems.clear() // Return true to end the search. true } @@ -499,14 +485,14 @@ class LazyStaggeredGridBeyondBoundsTest(param: Param) { Box( Modifier .size(10.toDp()) - .onPlaced { placedItems += index } + .trackPlaced(index) ) } item { Box( Modifier .size(10.toDp()) - .onPlaced { placedItems += 5 } + .trackPlaced(5) .modifierLocalConsumer { beyondBoundsLayout = ModifierLocalBeyondBoundsLayout.current } @@ -516,14 +502,13 @@ class LazyStaggeredGridBeyondBoundsTest(param: Param) { Box( Modifier .size(10.toDp()) - .onPlaced { placedItems += index + 6 } + .trackPlaced(index + 6) ) } } rule.runOnIdle { assertThat(placedItems).containsExactly(5, 6, 7) assertThat(visibleItems).containsExactly(5, 6, 7) - placedItems.clear() } // Act. @@ -545,7 +530,6 @@ class LazyStaggeredGridBeyondBoundsTest(param: Param) { } } } - placedItems.clear() // Just return true so that we stop as soon as we run this once. // This should result in one extra item being added. true @@ -577,9 +561,7 @@ class LazyStaggeredGridBeyondBoundsTest(param: Param) { Box( Modifier .size(10.toDp()) - .onPlaced { - placedItems += index - } + .trackPlaced(index) ) } item { @@ -589,20 +571,17 @@ class LazyStaggeredGridBeyondBoundsTest(param: Param) { .modifierLocalConsumer { beyondBoundsLayout = ModifierLocalBeyondBoundsLayout.current } - .onPlaced { placedItems += 5 } + .trackPlaced(5) ) } items(5) { index -> Box( Modifier .size(10.toDp()) - .onPlaced { - placedItems += index + 6 - } + .trackPlaced(index + 6) ) } } - rule.runOnIdle { placedItems.clear() } // Act. var count = 0 @@ -610,7 +589,6 @@ class LazyStaggeredGridBeyondBoundsTest(param: Param) { beyondBoundsLayout!!.layout(beyondBoundsLayoutDirection) { // Assert that we don't keep iterating when there is no ending condition. assertThat(count++).isLessThan(lazyStaggeredGridState.layoutInfo.totalItemsCount) - placedItems.clear() // Always return null to continue the search. null } @@ -704,4 +682,7 @@ class LazyStaggeredGridBeyondBoundsTest(param: Param) { private fun unsupportedDirection(): Nothing = error( "Lazy list does not support beyond bounds layout for the specified direction" ) + + private fun Modifier.trackPlaced(index: Int): Modifier = + this then TrackPlacedElement(placedItems, index) } diff --git a/compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/draw/DrawReorderingTest.kt b/compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/draw/DrawReorderingTest.kt index 57531e57a87..ebea07382b2 100644 --- a/compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/draw/DrawReorderingTest.kt +++ b/compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/draw/DrawReorderingTest.kt @@ -43,6 +43,7 @@ import androidx.compose.ui.zIndex import androidx.test.ext.junit.runners.AndroidJUnit4 import androidx.test.filters.MediumTest import androidx.test.filters.SdkSuppress +import com.google.common.truth.Truth.assertThat import java.util.concurrent.CountDownLatch import java.util.concurrent.TimeUnit import org.junit.Assert.assertNotNull @@ -1040,20 +1041,28 @@ class DrawReorderingTest { @Test @SdkSuppress(minSdkVersion = Build.VERSION_CODES.O) - fun placingInDifferentOrderTriggersRedraw() { + fun changingPlaceOrderInLayout() { var reverseOrder by mutableStateOf(false) + var childRelayoutCount = 0 + val childRelayoutModifier = Modifier.layout { measurable, constraints -> + val placeable = measurable.measure(constraints) + layout(placeable.width, placeable.height) { + childRelayoutCount++ + placeable.place(0, 0) + } + } rule.runOnUiThread { activity.setContent { Layout( content = { - FixedSize(30) { + FixedSize(30, childRelayoutModifier) { FixedSize( 10, Modifier.padding(10) .background(Color.White) ) } - FixedSize(30) { + FixedSize(30, childRelayoutModifier) { FixedSize( 30, Modifier.background(Color.Red) @@ -1084,6 +1093,7 @@ class DrawReorderingTest { rule.runOnUiThread { drawLatch = CountDownLatch(1) reverseOrder = true + childRelayoutCount = 0 } rule.validateSquareColors( @@ -1092,6 +1102,74 @@ class DrawReorderingTest { size = 10, drawLatch = drawLatch ) + rule.runOnUiThread { + // changing drawing order doesn't require child's layer block rerun + assertThat(childRelayoutCount).isEqualTo(0) + } + } + + @Test + @SdkSuppress(minSdkVersion = Build.VERSION_CODES.O) + fun changingZIndexInLayout() { + var zIndex by mutableStateOf(1f) + var childRelayoutCount = 0 + val childRelayoutModifier = Modifier.layout { measurable, constraints -> + val placeable = measurable.measure(constraints) + layout(placeable.width, placeable.height) { + childRelayoutCount++ + placeable.place(0, 0) + } + } + rule.runOnUiThread { + activity.setContent { + Layout( + content = { + FixedSize(30, childRelayoutModifier) { + FixedSize( + 10, + Modifier.padding(10) + .background(Color.White) + ) + } + FixedSize(30, childRelayoutModifier) { + FixedSize( + 30, + Modifier.background(Color.Red) + ) + } + }, + modifier = Modifier.drawLatchModifier() + ) { measurables, _ -> + val newConstraints = Constraints.fixed(30, 30) + val placeables = measurables.map { m -> + m.measure(newConstraints) + } + layout(newConstraints.maxWidth, newConstraints.maxWidth) { + placeables[0].place(0, 0) + placeables[1].place(0, 0, zIndex) + } + } + } + } + + assertTrue(drawLatch.await(1, TimeUnit.SECONDS)) + + rule.runOnUiThread { + drawLatch = CountDownLatch(1) + zIndex = -1f + childRelayoutCount = 0 + } + + rule.validateSquareColors( + outerColor = Color.Red, + innerColor = Color.White, + size = 10, + drawLatch = drawLatch + ) + rule.runOnUiThread { + // changing zIndex doesn't require child's layer block rerun + assertThat(childRelayoutCount).isEqualTo(0) + } } fun Modifier.drawLatchModifier() = drawBehind { drawLatch.countDown() } diff --git a/compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/draw/GraphicsLayerTest.kt b/compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/draw/GraphicsLayerTest.kt index ba80c9152d8..cff91584285 100644 --- a/compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/draw/GraphicsLayerTest.kt +++ b/compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/draw/GraphicsLayerTest.kt @@ -121,9 +121,12 @@ class GraphicsLayerTest { rule.setContent { FixedSize( 30, - Modifier.padding(10).graphicsLayer().onGloballyPositioned { - coords = it - } + Modifier + .padding(10) + .graphicsLayer() + .onGloballyPositioned { + coords = it + } ) { /* no-op */ } } @@ -148,9 +151,11 @@ class GraphicsLayerTest { Padding(10) { FixedSize( 10, - Modifier.graphicsLayer(scaleX = 2f, scaleY = 3f).onGloballyPositioned { - coords = it - } + Modifier + .graphicsLayer(scaleX = 2f, scaleY = 3f) + .onGloballyPositioned { + coords = it + } ) { } } @@ -171,9 +176,11 @@ class GraphicsLayerTest { Padding(10) { FixedSize( 10, - Modifier.scale(scaleX = 2f, scaleY = 3f).onGloballyPositioned { - coords = it - } + Modifier + .scale(scaleX = 2f, scaleY = 3f) + .onGloballyPositioned { + coords = it + } ) { } } @@ -194,9 +201,11 @@ class GraphicsLayerTest { Padding(10) { FixedSize( 10, - Modifier.scale(scale = 2f).onGloballyPositioned { - coords = it - } + Modifier + .scale(scale = 2f) + .onGloballyPositioned { + coords = it + } ) { } } @@ -217,9 +226,11 @@ class GraphicsLayerTest { Padding(10) { FixedSize( 10, - Modifier.graphicsLayer(scaleY = 3f, rotationZ = 90f).onGloballyPositioned { - coords = it - } + Modifier + .graphicsLayer(scaleY = 3f, rotationZ = 90f) + .onGloballyPositioned { + coords = it + } ) { } } @@ -240,9 +251,11 @@ class GraphicsLayerTest { Padding(10) { FixedSize( 10, - Modifier.rotate(90f).onGloballyPositioned { - coords = it - } + Modifier + .rotate(90f) + .onGloballyPositioned { + coords = it + } ) { } } @@ -263,12 +276,14 @@ class GraphicsLayerTest { Padding(10) { FixedSize( 10, - Modifier.graphicsLayer( - rotationZ = 90f, - transformOrigin = TransformOrigin(1.0f, 1.0f) - ).onGloballyPositioned { - coords = it - } + Modifier + .graphicsLayer( + rotationZ = 90f, + transformOrigin = TransformOrigin(1.0f, 1.0f) + ) + .onGloballyPositioned { + coords = it + } ) } } @@ -288,12 +303,14 @@ class GraphicsLayerTest { Padding(10) { FixedSize( 10, - Modifier.graphicsLayer( - translationX = 5.0f, - translationY = 8.0f - ).onGloballyPositioned { - coords = it - } + Modifier + .graphicsLayer( + translationX = 5.0f, + translationY = 8.0f + ) + .onGloballyPositioned { + coords = it + } ) } } @@ -314,9 +331,11 @@ class GraphicsLayerTest { FixedSize(10, Modifier.graphicsLayer(clip = true)) { FixedSize( 10, - Modifier.graphicsLayer(scaleX = 2f).onGloballyPositioned { - coords = it - } + Modifier + .graphicsLayer(scaleX = 2f) + .onGloballyPositioned { + coords = it + } ) { } } @@ -339,18 +358,20 @@ class GraphicsLayerTest { rule.setContent { with(LocalDensity.current) { Box( - Modifier.requiredSize(25.toDp()) + Modifier + .requiredSize(25.toDp()) .graphicsLayer( rotationZ = 30f, clip = true ) ) { Box( - Modifier.graphicsLayer( - rotationZ = 90f, - transformOrigin = TransformOrigin(0f, 1f), - clip = true - ) + Modifier + .graphicsLayer( + rotationZ = 90f, + transformOrigin = TransformOrigin(0f, 1f), + clip = true + ) .requiredSize(20.toDp(), 10.toDp()) .align(AbsoluteAlignment.TopLeft) .onGloballyPositioned { @@ -395,7 +416,9 @@ class GraphicsLayerTest { if (Build.VERSION.SDK_INT == 28) return // b/260095151 val testTag = "parent" rule.setContent { - Box(modifier = Modifier.testTag(testTag).wrapContentSize()) { + Box(modifier = Modifier + .testTag(testTag) + .wrapContentSize()) { Box( modifier = Modifier .requiredSize(100.dp) @@ -431,7 +454,10 @@ class GraphicsLayerTest { } val tag = "testTag" rule.setContent { - Box(modifier = Modifier.testTag(tag).requiredSize(100.dp).background(Color.Blue)) { + Box(modifier = Modifier + .testTag(tag) + .requiredSize(100.dp) + .background(Color.Blue)) { Box( modifier = Modifier .matchParentSize() @@ -454,9 +480,11 @@ class GraphicsLayerTest { FixedSize(10, Modifier.graphicsLayer(clip = true)) { FixedSize( 10, - Modifier.padding(20).onGloballyPositioned { - coords = it - } + Modifier + .padding(20) + .onGloballyPositioned { + coords = it + } ) { } } @@ -493,7 +521,8 @@ class GraphicsLayerTest { drawBlock: DrawScope.() -> Unit ) { Box( - Modifier.testTag(tag) + Modifier + .testTag(tag) .size(size) .background(Color.Black) .graphicsLayer { @@ -606,7 +635,11 @@ class GraphicsLayerTest { val size = (sizePx / density) val squareSize = (squarePx / density) offset = (20f / density).roundToInt() - Box(Modifier.size(size.dp).background(Color.LightGray).testTag(testTag)) { + Box( + Modifier + .size(size.dp) + .background(Color.LightGray) + .testTag(testTag)) { Box( Modifier .layout { measurable, constraints -> @@ -664,7 +697,11 @@ class GraphicsLayerTest { val size = (sizePx / density) val squareSize = (squarePx / density) offset = (20f / density).roundToInt() - Box(Modifier.size(size.dp).background(Color.LightGray).testTag(testTag)) { + Box( + Modifier + .size(size.dp) + .background(Color.LightGray) + .testTag(testTag)) { Box( Modifier .layout { measurable, constraints -> @@ -689,7 +726,8 @@ class GraphicsLayerTest { drawRect( color = Color.Red, topLeft = Offset(-width, -height), - size = Size(width * 2, height * 2)) + size = Size(width * 2, height * 2) + ) } ) } @@ -738,7 +776,11 @@ class GraphicsLayerTest { val size = (sizePx / density) val squareSize = (squarePx / density) offset = (20f / density).roundToInt() - Box(Modifier.size(size.dp).background(Color.LightGray).testTag(testTag)) { + Box( + Modifier + .size(size.dp) + .background(Color.LightGray) + .testTag(testTag)) { Box( Modifier .layout { measurable, constraints -> @@ -764,7 +806,8 @@ class GraphicsLayerTest { drawRect( color = Color.Red, topLeft = Offset(-width, -height), - size = Size(width * 2, height * 2)) + size = Size(width * 2, height * 2) + ) } ) } @@ -815,7 +858,11 @@ class GraphicsLayerTest { val size = (sizePx / density) val squareSize = (squarePx / density) offset = (20f / density).roundToInt() - Box(Modifier.size(size.dp).background(Color.LightGray).testTag(testTag)) { + Box( + Modifier + .size(size.dp) + .background(Color.LightGray) + .testTag(testTag)) { Box( Modifier .layout { measurable, constraints -> @@ -845,7 +892,8 @@ class GraphicsLayerTest { drawRect( color = Color.Red, topLeft = Offset(-width, -height), - size = Size(width * 2, height * 2)) + size = Size(width * 2, height * 2) + ) } ) } @@ -896,7 +944,11 @@ class GraphicsLayerTest { val size = (sizePx / density) val squareSize = (squarePx / density) offset = (20f / density).roundToInt() - Box(Modifier.size(size.dp).background(Color.LightGray).testTag(testTag)) { + Box( + Modifier + .size(size.dp) + .background(Color.LightGray) + .testTag(testTag)) { Box( Modifier .layout { measurable, constraints -> @@ -971,7 +1023,11 @@ class GraphicsLayerTest { val size = (sizePx / density) val squareSize = (squarePx / density) offset = (20f / density).roundToInt() - Box(Modifier.size(size.dp).background(Color.LightGray).testTag(testTag)) { + Box( + Modifier + .size(size.dp) + .background(Color.LightGray) + .testTag(testTag)) { Box( Modifier .layout { measurable, constraints -> @@ -1044,7 +1100,10 @@ class GraphicsLayerTest { rule.setContent { FixedSize( 5, - Modifier.graphicsLayer().testTag("tag").background(color) + Modifier + .graphicsLayer() + .testTag("tag") + .background(color) ) } @@ -1092,14 +1151,18 @@ class GraphicsLayerTest { Layout( content = { Box( - Modifier.fillMaxSize().clickable { - firstClicked = true - } + Modifier + .fillMaxSize() + .clickable { + firstClicked = true + } ) Box( - Modifier.fillMaxSize().clickable { - secondClicked = true - } + Modifier + .fillMaxSize() + .clickable { + secondClicked = true + } ) }, modifier = Modifier.testTag("layout") @@ -1167,7 +1230,8 @@ class GraphicsLayerTest { rule.setContent { Canvas( modifier = - Modifier.testTag(tag) + Modifier + .testTag(tag) .size((dimen / LocalDensity.current.density).dp) .background(Color.Black) .graphicsLayer( @@ -1209,7 +1273,8 @@ class GraphicsLayerTest { rule.setContent { Canvas( modifier = - Modifier.testTag(tag) + Modifier + .testTag(tag) .size((dimen / LocalDensity.current.density).dp) .background(Color.LightGray) .graphicsLayer( @@ -1244,7 +1309,8 @@ class GraphicsLayerTest { rule.setContent { Canvas( modifier = - Modifier.testTag(tag) + Modifier + .testTag(tag) .size((dimen / LocalDensity.current.density).dp) .background(Color.Black) .graphicsLayer( @@ -1360,7 +1426,10 @@ class GraphicsLayerTest { val size = 100 rule.setContent { val sizeDp = with(LocalDensity.current) { size.toDp() } - LazyColumn(Modifier.testTag("lazy").background(Color.Blue)) { + LazyColumn( + Modifier + .testTag("lazy") + .background(Color.Blue)) { items(4) { Box( Modifier @@ -1512,7 +1581,10 @@ class GraphicsLayerTest { } } rule.setContent { - Box(Modifier.graphicsLayer(translationX = translationX).then(layoutModifier)) { + Box( + Modifier + .graphicsLayer(translationX = translationX) + .then(layoutModifier)) { Layout(Modifier.onGloballyPositioned { coordinates = it }) { _, _ -> layout(10, 10) {} } @@ -1550,7 +1622,10 @@ class GraphicsLayerTest { } } rule.setContent { - Box(Modifier.graphicsLayer(lambda).then(layoutModifier)) { + Box( + Modifier + .graphicsLayer(lambda) + .then(layoutModifier)) { Layout(Modifier.onGloballyPositioned { coordinates = it }) { _, _ -> layout(10, 10) {} } @@ -1572,4 +1647,92 @@ class GraphicsLayerTest { assertEquals(0, relayoutCount) } } + + @Test + fun addingLayerForChildDoesntTriggerChildRelayout() { + var relayoutCount = 0 + var modifierRelayoutCount = 0 + var needLayer by mutableStateOf(false) + var layerBlockCalled = false + rule.setContent { + Layout(content = { + Layout( + modifier = Modifier.layout { measurable, constraints -> + val placeable = measurable.measure(constraints) + layout(placeable.width, placeable.height) { + modifierRelayoutCount++ + placeable.place(0, 0) + } + } + ) { _, _ -> + layout(10, 10) { + relayoutCount++ + } + } + }) { measurables, constraints -> + val placeable = measurables[0].measure(constraints) + layout(placeable.width, placeable.height) { + if (needLayer) { + placeable.placeWithLayer(0, 0) { + layerBlockCalled = true + } + } else { + placeable.place(0, 0) + } + } + } + } + + rule.runOnIdle { + relayoutCount = 0 + modifierRelayoutCount = 0 + needLayer = true + } + + rule.runOnIdle { + assertEquals(0, relayoutCount) + assertTrue(layerBlockCalled) + assertEquals(0, modifierRelayoutCount) + } + } + + @Test + fun movingChildsLayerDoesntTriggerChildRelayout() { + var relayoutCount = 0 + var modifierRelayoutCount = 0 + var position by mutableStateOf(0) + rule.setContent { + Layout(content = { + Layout( + modifier = Modifier.layout { measurable, constraints -> + val placeable = measurable.measure(constraints) + layout(placeable.width, placeable.height) { + modifierRelayoutCount++ + placeable.place(0, 0) + } + } + ) { _, _ -> + layout(10, 10) { + relayoutCount++ + } + } + }) { measurables, constraints -> + val placeable = measurables[0].measure(constraints) + layout(placeable.width, placeable.height) { + placeable.placeWithLayer(position, 0) + } + } + } + + rule.runOnIdle { + relayoutCount = 0 + modifierRelayoutCount = 0 + position = 10 + } + + rule.runOnIdle { + assertEquals(0, relayoutCount) + assertEquals(0, modifierRelayoutCount) + } + } } diff --git a/compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/layout/LayoutCooperationTest.kt b/compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/layout/LayoutCooperationTest.kt index 154f1fd51b0..e91f316f69b 100644 --- a/compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/layout/LayoutCooperationTest.kt +++ b/compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/layout/LayoutCooperationTest.kt @@ -29,9 +29,12 @@ import androidx.compose.ui.Modifier import androidx.compose.ui.background import androidx.compose.ui.graphics.Color import androidx.compose.ui.platform.testTag +import androidx.compose.ui.test.assertLeftPositionInRootIsEqualTo +import androidx.compose.ui.test.assertTopPositionInRootIsEqualTo import androidx.compose.ui.test.captureToImage import androidx.compose.ui.test.junit4.createAndroidComposeRule import androidx.compose.ui.test.onNodeWithTag +import androidx.compose.ui.unit.Constraints import androidx.compose.ui.unit.IntSize import androidx.test.ext.junit.runners.AndroidJUnit4 import androidx.test.filters.MediumTest @@ -52,8 +55,14 @@ class LayoutCooperationTest { val size = 48 var initialOuterSize by mutableStateOf((size / 2).toDp()) rule.setContent { - Box(Modifier.size(initialOuterSize).testTag("outer")) { - Box(Modifier.requiredSize(size.toDp()).background(Color.Yellow)) + Box( + Modifier + .size(initialOuterSize) + .testTag("outer")) { + Box( + Modifier + .requiredSize(size.toDp()) + .background(Color.Yellow)) } } @@ -65,4 +74,45 @@ class LayoutCooperationTest { Color.Yellow } } + + @Test + fun relayoutSkippingModifiersDoesntBreakCooperation() { + with(rule.density) { + val containerSize = 100 + val width = 50 + val widthDp = width.toDp() + val height = 40 + val heightDp = height.toDp() + var offset by mutableStateOf(0) + rule.setContent { + Layout(content = { + Box(Modifier.requiredSize(widthDp, heightDp)) { + Box(Modifier.testTag("child")) + } + }) { measurables, _ -> + val placeable = + measurables.first().measure(Constraints.fixed(containerSize, containerSize)) + layout(containerSize, containerSize) { + placeable.place(offset, offset) + } + } + } + + var expectedTop = ((containerSize - height) / 2).toDp() + var expectedLeft = ((containerSize - width) / 2).toDp() + rule.onNodeWithTag("child") + .assertTopPositionInRootIsEqualTo(expectedTop) + .assertLeftPositionInRootIsEqualTo(expectedLeft) + + rule.runOnIdle { + offset = 10 + } + + expectedTop += offset.toDp() + expectedLeft += offset.toDp() + rule.onNodeWithTag("child") + .assertTopPositionInRootIsEqualTo(expectedTop) + .assertLeftPositionInRootIsEqualTo(expectedLeft) + } + } }
\ No newline at end of file diff --git a/compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/layout/LookaheadScopeTest.kt b/compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/layout/LookaheadScopeTest.kt index 378d5b729d7..66c3ac92bdb 100644 --- a/compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/layout/LookaheadScopeTest.kt +++ b/compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/layout/LookaheadScopeTest.kt @@ -1729,6 +1729,7 @@ class LookaheadScopeTest { repeat(3) { id -> subcompose(id) { Box(Modifier.trackMainPassPlacement { + iteration.toString() // state read to make callback called actualPlacementOrder.add(id) }) }.fastMap { it.measure(constraints) }.let { placeables.addAll(it) } @@ -1739,6 +1740,7 @@ class LookaheadScopeTest { val id = index + 3 subcompose(id) { Box(Modifier.trackMainPassPlacement { + iteration.toString() // state read to make callback called actualPlacementOrder.add(id) }) }.fastMap { it.measure(constraints) }.let { allPlaceables.addAll(it) } diff --git a/compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/layout/PlacementLayoutCoordinatesTest.kt b/compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/layout/PlacementLayoutCoordinatesTest.kt index f6553819a4f..ca5805c34e7 100644 --- a/compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/layout/PlacementLayoutCoordinatesTest.kt +++ b/compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/layout/PlacementLayoutCoordinatesTest.kt @@ -257,7 +257,7 @@ class PlacementLayoutCoordinatesTest { Layout(content, Modifier.alignByBaseline()) { measurables, constraints -> val p = measurables[0].measure(constraints) layout(p.width, p.height) { - locations += coordinates + locations += coordinates.use() p.place(0, 0) } } @@ -287,7 +287,7 @@ class PlacementLayoutCoordinatesTest { Layout(content, Modifier.alignByBaseline()) { measurables, constraints -> val p = measurables[0].measure(constraints) layout(p.width, p.height) { - locations += coordinates + locations += coordinates.use() p.place(0, 0) } } @@ -329,7 +329,7 @@ class PlacementLayoutCoordinatesTest { }) { measurables, constraints -> val p = measurables[0].measure(constraints) layout(p.width, p.height) { - locations += coordinates + locations += coordinates.use() p.place(0, 0) } } @@ -343,7 +343,7 @@ class PlacementLayoutCoordinatesTest { } @Test - fun parentCoordateChangeCausesRelayout() { + fun parentCoordinateChangeCausesRelayout() { val locations = mutableStateListOf<LayoutCoordinates?>() var offset by mutableStateOf(DpOffset(0.dp, 0.dp)) rule.setContent { @@ -354,7 +354,7 @@ class PlacementLayoutCoordinatesTest { .layout { measurable, constraints -> val p = measurable.measure(constraints) layout(p.width, p.height) { - locations += coordinates + locations += coordinates.use() p.place(0, 0) } } @@ -386,7 +386,7 @@ class PlacementLayoutCoordinatesTest { .layout { measurable, constraints -> val p = measurable.measure(constraints) layout(p.width, p.height) { - locations += coordinates + locations += coordinates.use() p.place(0, 0) } } @@ -422,7 +422,7 @@ class PlacementLayoutCoordinatesTest { .layout { measurable, constraints -> val p = measurable.measure(constraints) layout(p.width, p.height) { - locations += coordinates + locations += coordinates.use() p.place(0, 0) } } @@ -459,7 +459,8 @@ class PlacementLayoutCoordinatesTest { .layout { measurable, constraints -> val p = measurable.measure(constraints) layout(p.width, p.height) { - layoutCalls += if (readCoordinates) coordinates else null + layoutCalls += + if (readCoordinates) coordinates.use() else null p.place(0, 0) } } @@ -507,7 +508,7 @@ class PlacementLayoutCoordinatesTest { .layout { measurable, constraints -> val p = measurable.measure(constraints) layout(p.width, p.height) { - locations += coordinates + locations += coordinates.use() p.place(0, 0) } } @@ -568,7 +569,7 @@ class PlacementLayoutCoordinatesTest { .layout { measurable, constraints -> val p = measurable.measure(constraints) layout(p.width, p.height) { - locations += coordinates + locations += coordinates.use() p.place(0, 0) } } @@ -604,7 +605,7 @@ class PlacementLayoutCoordinatesTest { .layout { measurable, constraints -> val p = measurable.measure(constraints) layout(p.width, p.height) { - locations += coordinates + locations += coordinates.use() p.place(0, 0) } } @@ -634,7 +635,7 @@ class PlacementLayoutCoordinatesTest { .layout { measurable, constraints -> val p = measurable.measure(constraints) layout(p.width, p.height) { - locations += coordinates + locations += coordinates.use() p.place(0, 0) } } @@ -668,4 +669,235 @@ class PlacementLayoutCoordinatesTest { rule.waitForIdle() assertEquals(1, locations.size) } + + @Test + fun readingFromMainLayoutPolicyAfterMultipleMoves() { + var offset by mutableStateOf(0) + var layoutBlockCalls = 0 + rule.setContent { + Layout(content = { + Layout { _, _ -> + layout(10, 10) { + coordinates?.positionInParent() + layoutBlockCalls++ + } + } + }) { measurables, constraints -> + val placeable = measurables.first().measure(constraints) + layout(placeable.width, placeable.height) { + placeable.place(offset, 0) + } + } + } + + rule.runOnIdle { + layoutBlockCalls = 0 + offset = 1 + } + + rule.runOnIdle { + assertEquals(1, layoutBlockCalls) + layoutBlockCalls = 0 + offset = 2 + } + + rule.runOnIdle { + assertEquals(1, layoutBlockCalls) + } + } + + @Test + fun onlyRealPositionReadsTriggerRelayout() { + var offset by mutableStateOf(0) + var coordinatesAction: (LayoutCoordinates) -> Unit by mutableStateOf({}) + var layoutBlockCalls = 0 + rule.setContent { + Layout(content = { + Layout { _, _ -> + layout(10, 10) { + coordinates?.let(coordinatesAction) + layoutBlockCalls++ + } + } + }) { measurables, constraints -> + val placeable = measurables.first().measure(constraints) + layout(placeable.width, placeable.height) { + placeable.place(offset, 0) + } + } + } + + fun assert( + relayoutExpected: Boolean, + description: String, + action: (LayoutCoordinates) -> Unit + ) { + coordinatesAction = action + rule.runOnIdle { + layoutBlockCalls = 0 + offset = if (offset == 0) 10 else 0 + } + rule.runOnIdle { + assertEquals( + "Relayout because of `$description` read was " + + "${if (!relayoutExpected) " not" else ""} expected, but " + + "$layoutBlockCalls calls happened", + if (relayoutExpected) 1 else 0, + layoutBlockCalls + ) + } + } + + assert(relayoutExpected = true, "positionInParent()") { it.positionInParent() } + assert(relayoutExpected = true, "positionInRoot()") { it.positionInRoot() } + assert(relayoutExpected = true, "positionInWindow()") { it.positionInWindow() } + assert(relayoutExpected = true, "boundsInParent()") { it.boundsInParent() } + assert(relayoutExpected = true, "boundsInRoot()") { it.boundsInRoot() } + assert(relayoutExpected = true, "boundsInWindow()") { it.boundsInWindow() } + + assert(relayoutExpected = false, "empty") { } + assert(relayoutExpected = false, "size") { it.size } + assert(relayoutExpected = false, "isAttached") { it.isAttached } + assert(relayoutExpected = false, "providedAlignmentLines") { it.providedAlignmentLines } + } + + @Test + fun onlyRealPositionReadsTriggerRelayout_inModifier() { + var offset by mutableStateOf(0) + var coordinatesAction: (LayoutCoordinates) -> Unit by mutableStateOf({}) + var layoutBlockCalls = 0 + rule.setContent { + Layout(content = { + Box( + Modifier + .layout { measurable, constraints -> + val p = measurable.measure(constraints) + layout(p.width, p.height) { + coordinates?.let(coordinatesAction) + layoutBlockCalls++ + p.place(0, 0) + } + } + ) + }) { measurables, constraints -> + val placeable = measurables.first().measure(constraints) + layout(placeable.width, placeable.height) { + placeable.place(offset, 0) + } + } + } + + fun assert( + relayoutExpected: Boolean, + description: String, + action: (LayoutCoordinates) -> Unit + ) { + coordinatesAction = action + rule.runOnIdle { + layoutBlockCalls = 0 + offset = if (offset == 0) 10 else 0 + } + rule.runOnIdle { + assertEquals( + "Relayout because of `$description` read was " + + "${if (!relayoutExpected) " not" else ""} expected, but " + + "$layoutBlockCalls calls happened", + if (relayoutExpected) 1 else 0, + layoutBlockCalls + ) + } + } + + assert(relayoutExpected = true, "positionInParent()") { it.positionInParent() } + assert(relayoutExpected = true, "positionInRoot()") { it.positionInRoot() } + assert(relayoutExpected = true, "positionInWindow()") { it.positionInWindow() } + assert(relayoutExpected = true, "boundsInParent()") { it.boundsInParent() } + assert(relayoutExpected = true, "boundsInRoot()") { it.boundsInRoot() } + assert(relayoutExpected = true, "boundsInWindow()") { it.boundsInWindow() } + + assert(relayoutExpected = false, "empty") { } + assert(relayoutExpected = false, "size") { it.size } + assert(relayoutExpected = false, "isAttached") { it.isAttached } + assert(relayoutExpected = false, "providedAlignmentLines") { it.providedAlignmentLines } + } + + @OptIn(ExperimentalComposeUiApi::class) + @Test + fun onlyRealPositionReadsTriggerRelayout_inLookahead() { + var offset by mutableStateOf(0) + var coordinatesAction: (LayoutCoordinates) -> Unit by mutableStateOf({}) + var intermediateLayoutBlockCalls = 0 + rule.setContent { + LookaheadScope { + Layout(content = { + Box( + Modifier + .intermediateLayout { measurable, constraints -> + val p = measurable.measure(constraints) + layout(p.width, p.height) { + coordinates?.let(coordinatesAction) + intermediateLayoutBlockCalls++ + p.place(0, 0) + } + } + .layout { measurable, constraints -> + val p = measurable.measure(constraints) + layout(10, 10) { + // if we don't read the coordinates here as well + // the read of coordinates in intermediate layout could be + // skipped as both passes share the same + // coordinatesAccessedDuringPlacement property. + // filed b/284153462 to track this issue + coordinates?.let(coordinatesAction) + p.place(0, 0) + } + } + ) + }) { measurables, constraints -> + val placeable = measurables.first().measure(constraints) + layout(placeable.width, placeable.height) { + placeable.place(offset, 0) + } + } + } + } + + fun assert( + relayoutExpected: Boolean, + description: String, + action: (LayoutCoordinates) -> Unit + ) { + coordinatesAction = action + rule.runOnIdle { + intermediateLayoutBlockCalls = 0 + offset = if (offset == 0) 10 else 0 + } + rule.runOnIdle { + assertEquals( + "Relayout because of `$description` read was " + + "${if (!relayoutExpected) " not" else ""} expected, but " + + "$intermediateLayoutBlockCalls calls happened", + if (relayoutExpected) 1 else 0, + intermediateLayoutBlockCalls + ) + } + } + + assert(relayoutExpected = true, "positionInParent()") { it.positionInParent() } + assert(relayoutExpected = true, "positionInRoot()") { it.positionInRoot() } + assert(relayoutExpected = true, "positionInWindow()") { it.positionInWindow() } + assert(relayoutExpected = true, "boundsInParent()") { it.boundsInParent() } + assert(relayoutExpected = true, "boundsInRoot()") { it.boundsInRoot() } + assert(relayoutExpected = true, "boundsInWindow()") { it.boundsInWindow() } + + assert(relayoutExpected = false, "empty") { } + assert(relayoutExpected = false, "size") { it.size } + assert(relayoutExpected = false, "isAttached") { it.isAttached } + assert(relayoutExpected = false, "providedAlignmentLines") { it.providedAlignmentLines } + } +} + +private fun LayoutCoordinates?.use(): LayoutCoordinates? { + this?.parentCoordinates + return this } diff --git a/compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/platform/AndroidComposeView.android.kt b/compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/platform/AndroidComposeView.android.kt index fe78b5942a3..a0e6a00351d 100644 --- a/compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/platform/AndroidComposeView.android.kt +++ b/compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/platform/AndroidComposeView.android.kt @@ -870,20 +870,30 @@ internal class AndroidComposeView(context: Context, coroutineContext: CoroutineC } override fun measureAndLayout(sendPointerUpdate: Boolean) { - trace("AndroidOwner:measureAndLayout") { - val resend = if (sendPointerUpdate) resendMotionEventOnLayout else null - val rootNodeResized = measureAndLayoutDelegate.measureAndLayout(resend) - if (rootNodeResized) { - requestLayout() + // only run the logic when we have something pending + if (measureAndLayoutDelegate.hasPendingMeasureOrLayout || + measureAndLayoutDelegate.hasPendingOnPositionedCallbacks + ) { + trace("AndroidOwner:measureAndLayout") { + val resend = if (sendPointerUpdate) resendMotionEventOnLayout else null + val rootNodeResized = measureAndLayoutDelegate.measureAndLayout(resend) + if (rootNodeResized) { + requestLayout() + } + measureAndLayoutDelegate.dispatchOnPositionedCallbacks() } - measureAndLayoutDelegate.dispatchOnPositionedCallbacks() } } override fun measureAndLayout(layoutNode: LayoutNode, constraints: Constraints) { trace("AndroidOwner:measureAndLayout") { measureAndLayoutDelegate.measureAndLayout(layoutNode, constraints) - measureAndLayoutDelegate.dispatchOnPositionedCallbacks() + // only dispatch the callbacks if we don't have other nodes to process as otherwise + // we will have one more measureAndLayout() pass anyway in the same frame. + // it allows us to not traverse the hierarchy twice. + if (!measureAndLayoutDelegate.hasPendingMeasureOrLayout) { + measureAndLayoutDelegate.dispatchOnPositionedCallbacks() + } } } diff --git a/compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/platform/RenderNodeLayer.android.kt b/compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/platform/RenderNodeLayer.android.kt index f718f7b6639..a2c822069a1 100644 --- a/compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/platform/RenderNodeLayer.android.kt +++ b/compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/platform/RenderNodeLayer.android.kt @@ -208,8 +208,12 @@ internal class RenderNodeLayer( val newLeft = position.x val newTop = position.y if (oldLeft != newLeft || oldTop != newTop) { - renderNode.offsetLeftAndRight(newLeft - oldLeft) - renderNode.offsetTopAndBottom(newTop - oldTop) + if (oldLeft != newLeft) { + renderNode.offsetLeftAndRight(newLeft - oldLeft) + } + if (oldTop != newTop) { + renderNode.offsetTopAndBottom(newTop - oldTop) + } triggerRepaint() matrixCache.invalidate() } diff --git a/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/layout/LayoutCoordinates.kt b/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/layout/LayoutCoordinates.kt index af49d34430a..e674eed2c7a 100644 --- a/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/layout/LayoutCoordinates.kt +++ b/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/layout/LayoutCoordinates.kt @@ -24,7 +24,7 @@ import androidx.compose.ui.node.NodeCoordinator import androidx.compose.ui.unit.IntSize /** - * A holder of the measured bounds for the layout (MeasureBox). + * A holder of the measured bounds for the [Layout]. */ @JvmDefaultWithCompatibility interface LayoutCoordinates { diff --git a/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/layout/LookaheadLayoutCoordinates.kt b/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/layout/LookaheadLayoutCoordinates.kt index 354fe564a8e..a6747907fc1 100644 --- a/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/layout/LookaheadLayoutCoordinates.kt +++ b/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/layout/LookaheadLayoutCoordinates.kt @@ -89,6 +89,7 @@ internal class LookaheadLayoutCoordinatesImpl(val lookaheadDelegate: LookaheadDe ): Offset { if (sourceCoordinates is LookaheadLayoutCoordinatesImpl) { val source = sourceCoordinates.lookaheadDelegate + source.coordinator.onCoordinatesUsed() val commonAncestor = coordinator.findCommonAncestor(source.coordinator) return commonAncestor.lookaheadDelegate?.let { ancestor -> diff --git a/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/layout/Placeable.kt b/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/layout/Placeable.kt index 94893a6a42b..b20aa16c347 100644 --- a/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/layout/Placeable.kt +++ b/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/layout/Placeable.kt @@ -71,11 +71,11 @@ abstract class Placeable : Measured { set(value) { if (field != value) { field = value - recalculateWidthAndHeight() + onMeasuredSizeChanged() } } - private fun recalculateWidthAndHeight() { + private fun onMeasuredSizeChanged() { width = measuredSize.width.coerceIn( measurementConstraints.minWidth, measurementConstraints.maxWidth @@ -84,6 +84,8 @@ abstract class Placeable : Measured { measurementConstraints.minHeight, measurementConstraints.maxHeight ) + apparentToRealOffset = + IntOffset((width - measuredSize.width) / 2, (height - measuredSize.height) / 2) } /** @@ -110,7 +112,7 @@ abstract class Placeable : Measured { set(value) { if (field != value) { field = value - recalculateWidthAndHeight() + onMeasuredSizeChanged() } } @@ -119,8 +121,8 @@ abstract class Placeable : Measured { * The real layout will be centered on the space assigned by the parent, which computed the * child's position only seeing its apparent size. */ - protected val apparentToRealOffset: IntOffset - get() = IntOffset((width - measuredSize.width) / 2, (height - measuredSize.height) / 2) + protected var apparentToRealOffset: IntOffset = IntOffset.Zero + private set /** * Receiver scope that permits explicit placement of a [Placeable]. @@ -158,6 +160,10 @@ abstract class Placeable : Measured { * When [coordinates] is `null`, there will always be a follow-up placement call in which * [coordinates] is not-`null`. * + * If you read a position from the coordinates during the placement block the block + * will be automatically re-executed when the parent layout changes a position. If you + * don't read it the placement block execution can be skipped as an optimization. + * * @sample androidx.compose.ui.samples.PlacementScopeCoordinatesSample */ open val coordinates: LayoutCoordinates? @@ -340,7 +346,11 @@ abstract class Placeable : Measured { override val coordinates: LayoutCoordinates? get() { - layoutDelegate?.coordinatesAccessedDuringPlacement = true + // if coordinates are not null we will only set this flag when the inner + // coordinate values are read. see NodeCoordinator.onCoordinatesUsed() + if (_coordinates == null) { + layoutDelegate?.onCoordinatesUsed() + } return _coordinates } diff --git a/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/node/LayoutNodeAlignmentLines.kt b/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/node/LayoutNodeAlignmentLines.kt index fc84004a882..b682397f370 100644 --- a/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/node/LayoutNodeAlignmentLines.kt +++ b/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/node/LayoutNodeAlignmentLines.kt @@ -203,7 +203,7 @@ internal sealed class AlignmentLines(val alignmentLinesOwner: AlignmentLinesOwne alignmentLinesOwner.requestMeasure() } if (usedByModifierLayout) { - parent.requestLayout() + alignmentLinesOwner.requestLayout() } parent.alignmentLines.onAlignmentsChanged() } diff --git a/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/node/LayoutNodeLayoutDelegate.kt b/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/node/LayoutNodeLayoutDelegate.kt index e66dc1cb70a..9e079fe7577 100644 --- a/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/node/LayoutNodeLayoutDelegate.kt +++ b/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/node/LayoutNodeLayoutDelegate.kt @@ -26,7 +26,6 @@ import androidx.compose.ui.node.LayoutNode.LayoutState import androidx.compose.ui.unit.Constraints import androidx.compose.ui.unit.IntOffset import androidx.compose.ui.unit.IntSize -import androidx.compose.ui.util.fastForEach /** * This class works as a layout delegate for [LayoutNode]. It delegates all the measure/layout @@ -173,9 +172,30 @@ internal class LayoutNodeLayoutDelegate( val oldValue = field if (oldValue != value) { field = value - if (value) { + if (value && !coordinatesAccessedDuringModifierPlacement) { + // if first out of both flags changes to true increment childrenAccessingCoordinatesDuringPlacement++ - } else { + } else if (!value && !coordinatesAccessedDuringModifierPlacement) { + // if both flags changes to false decrement + childrenAccessingCoordinatesDuringPlacement-- + } + } + } + + /** + * Similar to [coordinatesAccessedDuringPlacement], but tracks the coordinates read happening + * during the modifier layout blocks run. + */ + var coordinatesAccessedDuringModifierPlacement = false + set(value) { + val oldValue = field + if (oldValue != value) { + field = value + if (value && !coordinatesAccessedDuringPlacement) { + // if first out of both flags changes to true increment + childrenAccessingCoordinatesDuringPlacement++ + } else if (!value && !coordinatesAccessedDuringPlacement) { + // if both flags changes to false decrement childrenAccessingCoordinatesDuringPlacement-- } } @@ -216,6 +236,25 @@ internal class LayoutNodeLayoutDelegate( internal var lookaheadPassDelegate: LookaheadPassDelegate? = null private set + fun onCoordinatesUsed() { + val state = layoutNode.layoutState + if (state == LayoutState.LayingOut || state == LayoutState.LookaheadLayingOut) { + if (measurePassDelegate.layingOutChildren) { + coordinatesAccessedDuringPlacement = true + } else { + coordinatesAccessedDuringModifierPlacement = true + } + } + if (state == LayoutState.LookaheadLayingOut) { + // TODO lookahead should have its own flags b/284153462 + if (lookaheadPassDelegate?.layingOutChildren == true) { + coordinatesAccessedDuringPlacement = true + } else { + coordinatesAccessedDuringModifierPlacement = true + } + } + } + /** * [MeasurePassDelegate] manages the measure/layout and alignmentLine related queries for the * actual measure/layout pass. @@ -293,7 +332,11 @@ internal class LayoutNodeLayoutDelegate( return _childDelegates.asMutableList() } + var layingOutChildren = false + private set + override fun layoutChildren() { + layingOutChildren = true alignmentLines.recalculateQueryOwner() if (layoutPending) { @@ -308,6 +351,7 @@ internal class LayoutNodeLayoutDelegate( layoutPending = false val oldLayoutState = layoutState layoutState = LayoutState.LayingOut + coordinatesAccessedDuringPlacement = false with(layoutNode) { val owner = requireOwner() owner.snapshotObserver.observeLayoutSnapshotReads( @@ -341,6 +385,8 @@ internal class LayoutNodeLayoutDelegate( alignmentLines.previousUsedDuringParentLayout = true } if (alignmentLines.dirty && alignmentLines.required) alignmentLines.recalculate() + + layingOutChildren = false } private fun checkChildrenPlaceOrderForUpdates() { @@ -463,7 +509,7 @@ internal class LayoutNodeLayoutDelegate( } private inline fun forEachChildDelegate(block: (MeasurePassDelegate) -> Unit) { - layoutNode.children.fastForEach { + layoutNode.forEachChild { block(it.measurePassDelegate) } } @@ -581,6 +627,10 @@ internal class LayoutNodeLayoutDelegate( layerBlock: (GraphicsLayerScope.() -> Unit)? ) { if (position != lastPosition) { + if (coordinatesAccessedDuringModifierPlacement || + coordinatesAccessedDuringPlacement) { + layoutPending = true + } notifyChildrenUsingCoordinatesWhilePlacing() } // This can actually be called as soon as LookaheadMeasure is done, but devs may expect @@ -603,9 +653,7 @@ internal class LayoutNodeLayoutDelegate( } // Post-lookahead (if any) placement - layoutState = LayoutState.LayingOut placeOuterCoordinator(position, zIndex, layerBlock) - layoutState = LayoutState.Idle } private fun placeOuterCoordinator( @@ -613,26 +661,34 @@ internal class LayoutNodeLayoutDelegate( zIndex: Float, layerBlock: (GraphicsLayerScope.() -> Unit)? ) { + layoutState = LayoutState.LayingOut + lastPosition = position lastZIndex = zIndex lastLayerBlock = layerBlock - placedOnce = true - alignmentLines.usedByModifierLayout = false - coordinatesAccessedDuringPlacement = false + val owner = layoutNode.requireOwner() - owner.snapshotObserver.observeLayoutModifierSnapshotReads( - layoutNode, - affectsLookahead = false - ) { - with(PlacementScope) { - if (layerBlock == null) { - outerCoordinator.place(position, zIndex) - } else { - outerCoordinator.placeWithLayer(position, zIndex, layerBlock) + if (!layoutPending && isPlaced) { + outerCoordinator.placeSelfApparentToRealOffset(position, zIndex, layerBlock) + onNodePlaced() + } else { + alignmentLines.usedByModifierLayout = false + coordinatesAccessedDuringModifierPlacement = false + owner.snapshotObserver.observeLayoutModifierSnapshotReads( + layoutNode, affectsLookahead = false + ) { + with(PlacementScope) { + if (layerBlock == null) { + outerCoordinator.place(position, zIndex) + } else { + outerCoordinator.placeWithLayer(position, zIndex, layerBlock) + } } } } + + layoutState = LayoutState.Idle } /** @@ -730,7 +786,7 @@ internal class LayoutNodeLayoutDelegate( get() = layoutNode.parent?.layoutDelegate?.alignmentLinesOwner override fun forEachChildAlignmentLinesOwner(block: (AlignmentLinesOwner) -> Unit) { - layoutNode.children.fastForEach { + layoutNode.forEachChild { block(it.layoutDelegate.alignmentLinesOwner) } } @@ -756,11 +812,11 @@ internal class LayoutNodeLayoutDelegate( */ fun notifyChildrenUsingCoordinatesWhilePlacing() { if (childrenAccessingCoordinatesDuringPlacement > 0) { - layoutNode.children.fastForEach { child -> + layoutNode.forEachChild { child -> val childLayoutDelegate = child.layoutDelegate - if (childLayoutDelegate.coordinatesAccessedDuringPlacement && - !childLayoutDelegate.layoutPending - ) { + val accessed = childLayoutDelegate.coordinatesAccessedDuringPlacement || + childLayoutDelegate.coordinatesAccessedDuringModifierPlacement + if (accessed && !childLayoutDelegate.layoutPending) { child.requestRelayout() } childLayoutDelegate.measurePassDelegate @@ -939,12 +995,16 @@ internal class LayoutNodeLayoutDelegate( return _childDelegates.asMutableList() } + var layingOutChildren = false + private set + private inline fun forEachChildDelegate(block: (LookaheadPassDelegate) -> Unit) = layoutNode.forEachChild { block(it.layoutDelegate.lookaheadPassDelegate!!) } override fun layoutChildren() { + layingOutChildren = true alignmentLines.recalculateQueryOwner() if (lookaheadLayoutPending) { @@ -961,6 +1021,7 @@ internal class LayoutNodeLayoutDelegate( val oldLayoutState = layoutState layoutState = LayoutState.LookaheadLayingOut val owner = layoutNode.requireOwner() + coordinatesAccessedDuringPlacement = false owner.snapshotObserver.observeLayoutSnapshotReads(layoutNode) { clearPlaceOrder() forEachChildAlignmentLinesOwner { child -> @@ -985,6 +1046,8 @@ internal class LayoutNodeLayoutDelegate( alignmentLines.previousUsedDuringParentLayout = true } if (alignmentLines.dirty && alignmentLines.required) alignmentLines.recalculate() + + layingOutChildren = false } private fun checkChildrenPlaceOrderForUpdates() { @@ -1030,7 +1093,7 @@ internal class LayoutNodeLayoutDelegate( get() = layoutNode.parent?.layoutDelegate?.lookaheadAlignmentLinesOwner override fun forEachChildAlignmentLinesOwner(block: (AlignmentLinesOwner) -> Unit) { - layoutNode.children.fastForEach { + layoutNode.forEachChild { block(it.layoutDelegate.lookaheadAlignmentLinesOwner!!) } } @@ -1056,11 +1119,11 @@ internal class LayoutNodeLayoutDelegate( */ fun notifyChildrenUsingCoordinatesWhilePlacing() { if (childrenAccessingCoordinatesDuringPlacement > 0) { - layoutNode.children.fastForEach { child -> + layoutNode.forEachChild { child -> val childLayoutDelegate = child.layoutDelegate - if (childLayoutDelegate.coordinatesAccessedDuringPlacement && - !childLayoutDelegate.layoutPending - ) { + val accessed = childLayoutDelegate.coordinatesAccessedDuringPlacement || + childLayoutDelegate.coordinatesAccessedDuringModifierPlacement + if (accessed && !childLayoutDelegate.layoutPending) { child.requestLookaheadRelayout() } childLayoutDelegate.lookaheadPassDelegate @@ -1160,14 +1223,23 @@ internal class LayoutNodeLayoutDelegate( layoutState = LayoutState.LookaheadLayingOut placedOnce = true if (position != lastPosition) { + if (coordinatesAccessedDuringModifierPlacement || + coordinatesAccessedDuringPlacement) { + lookaheadLayoutPending = true + } notifyChildrenUsingCoordinatesWhilePlacing() } - alignmentLines.usedByModifierLayout = false val owner = layoutNode.requireOwner() - coordinatesAccessedDuringPlacement = false - owner.snapshotObserver.observeLayoutModifierSnapshotReads(layoutNode) { - with(PlacementScope) { - outerCoordinator.lookaheadDelegate!!.place(position) + + if (!lookaheadLayoutPending && isPlaced) { + onNodePlaced() + } else { + coordinatesAccessedDuringModifierPlacement = false + alignmentLines.usedByModifierLayout = false + owner.snapshotObserver.observeLayoutModifierSnapshotReads(layoutNode) { + with(PlacementScope) { + outerCoordinator.lookaheadDelegate!!.place(position) + } } } lastPosition = position @@ -1294,8 +1366,8 @@ internal class LayoutNodeLayoutDelegate( } if (parent != null) { if (!relayoutWithoutParentInProgress && - parent.layoutState == LayoutState.LayingOut || - parent.layoutState == LayoutState.LookaheadLayingOut + (parent.layoutState == LayoutState.LayingOut || + parent.layoutState == LayoutState.LookaheadLayingOut) ) { // the parent is currently placing its children check(placeOrder == NotPlacedPlaceOrder) { diff --git a/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/node/MeasureAndLayoutDelegate.kt b/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/node/MeasureAndLayoutDelegate.kt index d297aeff46b..e743af77f9a 100644 --- a/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/node/MeasureAndLayoutDelegate.kt +++ b/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/node/MeasureAndLayoutDelegate.kt @@ -48,6 +48,11 @@ internal class MeasureAndLayoutDelegate(private val root: LayoutNode) { val hasPendingMeasureOrLayout get() = relayoutNodes.isNotEmpty() /** + * Whether any on positioned callbacks need to be dispatched + */ + val hasPendingOnPositionedCallbacks get() = onPositionedDispatcher.isNotEmpty() + + /** * Flag to indicate that we're currently measuring. */ private var duringMeasureLayout = false @@ -365,7 +370,7 @@ internal class MeasureAndLayoutDelegate(private val root: LayoutNode) { private fun recurseRemeasure(layoutNode: LayoutNode) { remeasureOnly(layoutNode) - layoutNode._children.forEach { child -> + layoutNode.forEachChild { child -> if (child.measureAffectsParent) { recurseRemeasure(child) } diff --git a/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/node/NodeCoordinator.kt b/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/node/NodeCoordinator.kt index 2a0b6e9f93b..f80a25234af 100644 --- a/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/node/NodeCoordinator.kt +++ b/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/node/NodeCoordinator.kt @@ -247,15 +247,21 @@ internal abstract class NodeCoordinator( return null } + internal fun onCoordinatesUsed() { + layoutNode.layoutDelegate.onCoordinatesUsed() + } + final override val parentLayoutCoordinates: LayoutCoordinates? get() { check(isAttached) { ExpectAttachedLayoutCoordinates } + onCoordinatesUsed() return layoutNode.outerCoordinator.wrappedBy } final override val parentCoordinates: LayoutCoordinates? get() { check(isAttached) { ExpectAttachedLayoutCoordinates } + onCoordinatesUsed() return wrappedBy } @@ -301,6 +307,14 @@ internal abstract class NodeCoordinator( zIndex: Float, layerBlock: (GraphicsLayerScope.() -> Unit)? ) { + placeSelf(position, zIndex, layerBlock) + } + + private fun placeSelf( + position: IntOffset, + zIndex: Float, + layerBlock: (GraphicsLayerScope.() -> Unit)? + ) { updateLayerBlock(layerBlock) if (this.position != position) { this.position = position @@ -318,6 +332,14 @@ internal abstract class NodeCoordinator( this.zIndex = zIndex } + fun placeSelfApparentToRealOffset( + position: IntOffset, + zIndex: Float, + layerBlock: (GraphicsLayerScope.() -> Unit)? + ) { + placeSelf(position + apparentToRealOffset, zIndex, layerBlock) + } + /** * Draws the content of the LayoutNode */ @@ -374,9 +396,9 @@ internal abstract class NodeCoordinator( layerBlock: (GraphicsLayerScope.() -> Unit)?, forceUpdateLayerParameters: Boolean = false ) { - val updateParameters = this.layerBlock !== layerBlock || layerDensity != layoutNode - .density || layerLayoutDirection != layoutNode.layoutDirection || - forceUpdateLayerParameters + val layoutNode = layoutNode + val updateParameters = forceUpdateLayerParameters || this.layerBlock !== layerBlock || + layerDensity != layoutNode.density || layerLayoutDirection != layoutNode.layoutDirection this.layerBlock = layerBlock this.layerDensity = layoutNode.density this.layerLayoutDirection = layoutNode.layoutDirection @@ -737,6 +759,7 @@ internal abstract class NodeCoordinator( } val nodeCoordinator = sourceCoordinates.toCoordinator() + nodeCoordinator.onCoordinatesUsed() val commonAncestor = findCommonAncestor(nodeCoordinator) var position = relativeToSource @@ -751,6 +774,7 @@ internal abstract class NodeCoordinator( override fun transformFrom(sourceCoordinates: LayoutCoordinates, matrix: Matrix) { val coordinator = sourceCoordinates.toCoordinator() + coordinator.onCoordinatesUsed() val commonAncestor = findCommonAncestor(coordinator) matrix.reset() @@ -795,6 +819,7 @@ internal abstract class NodeCoordinator( "LayoutCoordinates $sourceCoordinates is not attached!" } val srcCoordinator = sourceCoordinates.toCoordinator() + srcCoordinator.onCoordinatesUsed() val commonAncestor = findCommonAncestor(srcCoordinator) val bounds = rectCache @@ -842,6 +867,7 @@ internal abstract class NodeCoordinator( override fun localToRoot(relativeToLocal: Offset): Offset { check(isAttached) { ExpectAttachedLayoutCoordinates } + onCoordinatesUsed() var coordinator: NodeCoordinator? = this var position = relativeToLocal while (coordinator != null) { @@ -1178,7 +1204,8 @@ internal abstract class NodeCoordinator( val layoutNode = coordinator.layoutNode val layoutDelegate = layoutNode.layoutDelegate if (layoutDelegate.childrenAccessingCoordinatesDuringPlacement > 0) { - if (layoutDelegate.coordinatesAccessedDuringPlacement) { + if (layoutDelegate.coordinatesAccessedDuringModifierPlacement || + layoutDelegate.coordinatesAccessedDuringPlacement) { layoutNode.requestRelayout() } layoutDelegate.measurePassDelegate diff --git a/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/node/OnPositionedDispatcher.kt b/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/node/OnPositionedDispatcher.kt index 07b7f50c268..3e9e6199ded 100644 --- a/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/node/OnPositionedDispatcher.kt +++ b/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/node/OnPositionedDispatcher.kt @@ -25,6 +25,8 @@ import androidx.compose.runtime.collection.mutableVectorOf internal class OnPositionedDispatcher { private val layoutNodes = mutableVectorOf<LayoutNode>() + fun isNotEmpty() = layoutNodes.isNotEmpty() + fun onNodePositioned(node: LayoutNode) { layoutNodes += node node.needsOnPositionedDispatch = true diff --git a/compose/ui/ui/src/test/kotlin/androidx/compose/ui/node/LayoutNodeTest.kt b/compose/ui/ui/src/test/kotlin/androidx/compose/ui/node/LayoutNodeTest.kt index df004df15be..74a2ab71a60 100644 --- a/compose/ui/ui/src/test/kotlin/androidx/compose/ui/node/LayoutNodeTest.kt +++ b/compose/ui/ui/src/test/kotlin/androidx/compose/ui/node/LayoutNodeTest.kt @@ -521,10 +521,10 @@ class LayoutNodeTest { @Test fun testLocalPositionOfWithSiblings() { - val node0 = LayoutNode() + val node0 = ZeroSizedLayoutNode() node0.attach(MockOwner()) - val node1 = LayoutNode() - val node2 = LayoutNode() + val node1 = ZeroSizedLayoutNode() + val node2 = ZeroSizedLayoutNode() node0.insertAt(0, node1) node0.insertAt(1, node2) node1.place(10, 20) diff --git a/tv/tv-foundation/src/androidTest/java/androidx/tv/foundation/lazy/list/LazyListBeyondBoundsTest.kt b/tv/tv-foundation/src/androidTest/java/androidx/tv/foundation/lazy/list/LazyListBeyondBoundsTest.kt index 149cf479769..45447046951 100644 --- a/tv/tv-foundation/src/androidTest/java/androidx/tv/foundation/lazy/list/LazyListBeyondBoundsTest.kt +++ b/tv/tv-foundation/src/androidTest/java/androidx/tv/foundation/lazy/list/LazyListBeyondBoundsTest.kt @@ -39,9 +39,12 @@ import androidx.compose.ui.layout.BeyondBoundsLayout.LayoutDirection.Companion.B import androidx.compose.ui.layout.BeyondBoundsLayout.LayoutDirection.Companion.Below import androidx.compose.ui.layout.BeyondBoundsLayout.LayoutDirection.Companion.Left import androidx.compose.ui.layout.BeyondBoundsLayout.LayoutDirection.Companion.Right +import androidx.compose.ui.layout.LayoutCoordinates import androidx.compose.ui.layout.ModifierLocalBeyondBoundsLayout -import androidx.compose.ui.layout.onPlaced import androidx.compose.ui.modifier.modifierLocalConsumer +import androidx.compose.ui.node.LayoutAwareModifierNode +import androidx.compose.ui.node.ModifierNodeElement +import androidx.compose.ui.platform.InspectorInfo import androidx.compose.ui.platform.LocalLayoutDirection import androidx.compose.ui.test.junit4.ComposeContentTestRule import androidx.compose.ui.test.junit4.createComposeRule @@ -107,7 +110,7 @@ class LazyListBeyondBoundsTest(param: Param) { Box( Modifier .size(10.toDp()) - .onPlaced { placedItems += index } + .trackPlaced(index) ) } } @@ -127,7 +130,7 @@ class LazyListBeyondBoundsTest(param: Param) { Box( Modifier .size(10.toDp()) - .onPlaced { placedItems += index } + .trackPlaced(index) ) } } @@ -147,7 +150,7 @@ class LazyListBeyondBoundsTest(param: Param) { Box( Modifier .size(10.toDp()) - .onPlaced { placedItems += index } + .trackPlaced(index) ) } } @@ -181,8 +184,7 @@ class LazyListBeyondBoundsTest(param: Param) { } // Act. - rule.waitForIdle() - val hasMoreContent = rule.runOnUiThread { + val hasMoreContent = rule.runOnIdle { beyondBoundsLayoutRef.layout(beyondBoundsLayoutDirection) { hasMoreContent } @@ -202,28 +204,27 @@ class LazyListBeyondBoundsTest(param: Param) { Box( Modifier .size(10.toDp()) - .onPlaced { placedItems += index } + .trackPlaced(index) ) } item { Box( Modifier - .size(10.toDp()) - .onPlaced { placedItems += 5 } - .modifierLocalConsumer { - beyondBoundsLayout = ModifierLocalBeyondBoundsLayout.current - } + .size(10.toDp()) + .trackPlaced(5) + .modifierLocalConsumer { + beyondBoundsLayout = ModifierLocalBeyondBoundsLayout.current + } ) } items(5) { index -> Box( Modifier .size(10.toDp()) - .onPlaced { placedItems += index + 6 } + .trackPlaced(index + 6) ) } } - rule.runOnIdle { placedItems.clear() } // Act. rule.runOnUiThread { @@ -236,7 +237,6 @@ class LazyListBeyondBoundsTest(param: Param) { assertThat(placedItems).containsExactly(5, 6, 7, 8) assertThat(visibleItems).containsExactly(5, 6, 7) } - placedItems.clear() // Just return true so that we stop as soon as we run this once. // This should result in one extra item being added. true @@ -259,14 +259,14 @@ class LazyListBeyondBoundsTest(param: Param) { Box( Modifier .size(10.toDp()) - .onPlaced { placedItems += index } + .trackPlaced(index) ) } item { Box( Modifier .size(10.toDp()) - .onPlaced { placedItems += 5 } + .trackPlaced(5) .modifierLocalConsumer { beyondBoundsLayout = ModifierLocalBeyondBoundsLayout.current } @@ -276,17 +276,15 @@ class LazyListBeyondBoundsTest(param: Param) { Box( Modifier .size(10.toDp()) - .onPlaced { placedItems += index + 6 } + .trackPlaced(index + 6) ) } } - rule.runOnIdle { placedItems.clear() } // Act. rule.runOnUiThread { beyondBoundsLayout!!.layout(beyondBoundsLayoutDirection) { if (--extraItemCount > 0) { - placedItems.clear() // Return null to continue the search. null } else { @@ -298,7 +296,6 @@ class LazyListBeyondBoundsTest(param: Param) { assertThat(placedItems).containsExactly(5, 6, 7, 8, 9) assertThat(visibleItems).containsExactly(5, 6, 7) } - placedItems.clear() // Return true to stop the search. true } @@ -320,7 +317,7 @@ class LazyListBeyondBoundsTest(param: Param) { Box( Modifier .size(10.toDp()) - .onPlaced { placedItems += index } + .trackPlaced(index) ) } item { @@ -330,26 +327,22 @@ class LazyListBeyondBoundsTest(param: Param) { .modifierLocalConsumer { beyondBoundsLayout = ModifierLocalBeyondBoundsLayout.current } - .onPlaced { placedItems += 5 } + .trackPlaced(5) ) } items(5) { index -> Box( Modifier .size(10.toDp()) - .onPlaced { - placedItems += index + 6 - } + .trackPlaced(index + 6) ) } } - rule.runOnIdle { placedItems.clear() } // Act. rule.runOnUiThread { beyondBoundsLayout!!.layout(beyondBoundsLayoutDirection) { if (hasMoreContent) { - placedItems.clear() // Just return null so that we keep adding more items till we reach the end. null } else { @@ -361,7 +354,6 @@ class LazyListBeyondBoundsTest(param: Param) { assertThat(placedItems).containsExactly(5, 6, 7, 8, 9, 10) assertThat(visibleItems).containsExactly(5, 6, 7) } - placedItems.clear() // Return true to end the search. true } @@ -383,14 +375,14 @@ class LazyListBeyondBoundsTest(param: Param) { Box( Modifier .size(10.toDp()) - .onPlaced { placedItems += index } + .trackPlaced(index) ) } item { Box( Modifier .size(10.toDp()) - .onPlaced { placedItems += 5 } + .trackPlaced(5) .modifierLocalConsumer { beyondBoundsLayout = ModifierLocalBeyondBoundsLayout.current } @@ -400,14 +392,13 @@ class LazyListBeyondBoundsTest(param: Param) { Box( Modifier .size(10.toDp()) - .onPlaced { placedItems += index + 6 } + .trackPlaced(index + 6) ) } } rule.runOnIdle { assertThat(placedItems).containsExactly(5, 6, 7) assertThat(visibleItems).containsExactly(5, 6, 7) - placedItems.clear() } // Act. @@ -416,7 +407,6 @@ class LazyListBeyondBoundsTest(param: Param) { beyondBoundsLayoutCount++ when (beyondBoundsLayoutDirection) { Left, Right, Above, Below -> { - assertThat(placedItems).containsExactlyElementsIn(visibleItems) assertThat(placedItems).containsExactly(5, 6, 7) assertThat(visibleItems).containsExactly(5, 6, 7) } @@ -430,7 +420,6 @@ class LazyListBeyondBoundsTest(param: Param) { } } } - placedItems.clear() // Just return true so that we stop as soon as we run this once. // This should result in one extra item being added. true @@ -462,9 +451,7 @@ class LazyListBeyondBoundsTest(param: Param) { Box( Modifier .size(10.toDp()) - .onPlaced { - placedItems += index - } + .trackPlaced(index) ) } item { @@ -474,20 +461,17 @@ class LazyListBeyondBoundsTest(param: Param) { .modifierLocalConsumer { beyondBoundsLayout = ModifierLocalBeyondBoundsLayout.current } - .onPlaced { placedItems += 5 } + .trackPlaced(5) ) } items(5) { index -> Box( Modifier .size(10.toDp()) - .onPlaced { - placedItems += index + 6 - } + .trackPlaced(index + 6) ) } } - rule.runOnIdle { placedItems.clear() } // Act. var count = 0 @@ -495,7 +479,6 @@ class LazyListBeyondBoundsTest(param: Param) { beyondBoundsLayout!!.layout(beyondBoundsLayoutDirection) { // Assert that we don't keep iterating when there is no ending condition. assertThat(count++).isLessThan(lazyListState.layoutInfo.totalItemsCount) - placedItems.clear() // Always return null to continue the search. null } @@ -515,7 +498,9 @@ class LazyListBeyondBoundsTest(param: Param) { Column { BasicText( text = "Outer button", - Modifier.focusRequester(buttonFocusRequester).focusable()) + Modifier + .focusRequester(buttonFocusRequester) + .focusable()) TvLazyColumn { items(3) { @@ -617,4 +602,37 @@ class LazyListBeyondBoundsTest(param: Param) { private fun unsupportedDirection(): Nothing = error( "Lazy list does not support beyond bounds layout for the specified direction" ) + + private fun Modifier.trackPlaced(index: Int): Modifier = + this then TrackPlacedElement(placedItems, index) +} + +internal data class TrackPlacedElement( + var placedItems: MutableSet<Int>, + var index: Int +) : ModifierNodeElement<TrackPlacedNode>() { + override fun create() = TrackPlacedNode(placedItems, index) + + override fun update(node: TrackPlacedNode) { + node.placedItems = placedItems + node.index = index + } + + override fun InspectorInfo.inspectableProperties() { + name = "trackPlaced" + properties["index"] = index + } +} + +internal class TrackPlacedNode( + var placedItems: MutableSet<Int>, + var index: Int +) : LayoutAwareModifierNode, Modifier.Node() { + override fun onPlaced(coordinates: LayoutCoordinates) { + placedItems += index + } + + override fun onDetach() { + placedItems -= index + } } |