From 12971a574689f2e29af4ae6adf5bc974a18436e7 Mon Sep 17 00:00:00 2001 From: Jesse Wilson Date: Wed, 6 Dec 2023 09:56:30 -0500 Subject: [PATCH 1/2] Add a test to confirm there's no unnecessary recompositions Also expose the size of the preload window in LazyListState. It should be possible for the application layer to change this as necessary. --- .../redwood/lazylayout/compose/LazyList.kt | 10 ++-- .../lazylayout/compose/LazyListState.kt | 8 +++ .../lazylayout/compose/LazyListTest.kt | 51 +++++++++++++++++++ 3 files changed, 63 insertions(+), 6 deletions(-) diff --git a/redwood-lazylayout-compose/src/commonMain/kotlin/app/cash/redwood/lazylayout/compose/LazyList.kt b/redwood-lazylayout-compose/src/commonMain/kotlin/app/cash/redwood/lazylayout/compose/LazyList.kt index 6cedec7f70..ec36e94eda 100644 --- a/redwood-lazylayout-compose/src/commonMain/kotlin/app/cash/redwood/lazylayout/compose/LazyList.kt +++ b/redwood-lazylayout-compose/src/commonMain/kotlin/app/cash/redwood/lazylayout/compose/LazyList.kt @@ -29,8 +29,6 @@ import app.cash.redwood.layout.api.CrossAxisAlignment import app.cash.redwood.ui.Margin import kotlin.jvm.JvmName -private const val OFFSCREEN_ITEMS_BUFFER_COUNT = 30 - @Composable internal fun LazyList( isVertical: Boolean, @@ -45,8 +43,8 @@ internal fun LazyList( ) { val itemProvider = rememberLazyListItemProvider(content) val itemCount = itemProvider.itemCount - val itemsBefore = (state.firstIndex - OFFSCREEN_ITEMS_BUFFER_COUNT / 2).coerceAtLeast(0) - val itemsAfter = (itemCount - (state.lastIndex + OFFSCREEN_ITEMS_BUFFER_COUNT / 2).coerceAtMost(itemCount)).coerceAtLeast(0) + val itemsBefore = (state.firstIndex - state.preloadBeforeItemCount).coerceAtLeast(0) + val itemsAfter = (itemCount - (state.lastIndex + state.preloadAfterItemCount).coerceAtMost(itemCount)).coerceAtLeast(0) // TODO(jwilson): drop this down to 20 once this is fixed: // https://github.com/cashapp/redwood/issues/1551 var placeholderPoolSize by remember { mutableStateOf(30) } @@ -98,8 +96,8 @@ internal fun RefreshableLazyList( ) { val itemProvider = rememberLazyListItemProvider(content) val itemCount = itemProvider.itemCount - val itemsBefore = (state.firstIndex - OFFSCREEN_ITEMS_BUFFER_COUNT / 2).coerceAtLeast(0) - val itemsAfter = (itemCount - (state.lastIndex + OFFSCREEN_ITEMS_BUFFER_COUNT / 2).coerceAtMost(itemCount)).coerceAtLeast(0) + val itemsBefore = (state.firstIndex - state.preloadBeforeItemCount).coerceAtLeast(0) + val itemsAfter = (itemCount - (state.lastIndex + state.preloadAfterItemCount).coerceAtMost(itemCount)).coerceAtLeast(0) var placeholderPoolSize by remember { mutableStateOf(20) } RefreshableLazyList( isVertical, diff --git a/redwood-lazylayout-compose/src/commonMain/kotlin/app/cash/redwood/lazylayout/compose/LazyListState.kt b/redwood-lazylayout-compose/src/commonMain/kotlin/app/cash/redwood/lazylayout/compose/LazyListState.kt index 4732546b7b..dcf78cfdf4 100644 --- a/redwood-lazylayout-compose/src/commonMain/kotlin/app/cash/redwood/lazylayout/compose/LazyListState.kt +++ b/redwood-lazylayout-compose/src/commonMain/kotlin/app/cash/redwood/lazylayout/compose/LazyListState.kt @@ -23,6 +23,8 @@ import androidx.compose.runtime.saveable.rememberSaveable import androidx.compose.runtime.setValue import app.cash.redwood.lazylayout.api.ScrollItemIndex +private const val DEFAULT_PRELOAD_ITEM_COUNT = 15 + @Composable public fun rememberLazyListState(): LazyListState { return rememberSaveable(saver = saver) { @@ -59,6 +61,12 @@ public open class LazyListState { public var lastIndex: Int by mutableStateOf(0) private set + /** How many items to load in anticipation of scrolling up. */ + public var preloadBeforeItemCount: Int by mutableStateOf(DEFAULT_PRELOAD_ITEM_COUNT) + + /** How many items to load in anticipation of scrolling down. */ + public var preloadAfterItemCount: Int by mutableStateOf(DEFAULT_PRELOAD_ITEM_COUNT) + /** Perform a programmatic scroll. */ public fun programmaticScroll( firstIndex: Int, diff --git a/redwood-lazylayout-compose/src/commonTest/kotlin/app/cash/redwood/lazylayout/compose/LazyListTest.kt b/redwood-lazylayout-compose/src/commonTest/kotlin/app/cash/redwood/lazylayout/compose/LazyListTest.kt index b57d2fcc88..d3340974d2 100644 --- a/redwood-lazylayout-compose/src/commonTest/kotlin/app/cash/redwood/lazylayout/compose/LazyListTest.kt +++ b/redwood-lazylayout-compose/src/commonTest/kotlin/app/cash/redwood/lazylayout/compose/LazyListTest.kt @@ -110,4 +110,55 @@ class LazyListTest { } } } + + @Test + fun scrollDoesNotTriggerRecompose() = runTest { + TestSchemaTester { + var index5ComposeCount = 0 + setContent { + val lazyListState = rememberLazyListState().apply { + preloadBeforeItemCount = 0 + preloadAfterItemCount = 0 + } + LazyColumn( + state = lazyListState, + placeholder = { Text("Placeholder") } + ) { + items(100) { + if (it == 5) index5ComposeCount++ + Text(it.toString()) + } + } + } + + // Initially, the item at index 5 is never composed. + val lazyList = awaitSnapshot().filterIsInstance().single() + assertThat(index5ComposeCount).isEqualTo(0) + + // Growing the scroll window to include the preceding item doesn't change that. + lazyList.onViewportChanged(0, 4) + awaitSnapshot() + assertThat(index5ComposeCount).isEqualTo(0) + + // But scrolling to include it causes the first composition. + lazyList.onViewportChanged(2, 6) + awaitSnapshot() + assertThat(index5ComposeCount).isEqualTo(1) + + // Further scrolling doesn't cause it to be recomposed. + lazyList.onViewportChanged(4, 8) + awaitSnapshot() + assertThat(index5ComposeCount).isEqualTo(1) + + // Even when it's scrolled off-screen. + lazyList.onViewportChanged(6, 10) + awaitSnapshot() + assertThat(index5ComposeCount).isEqualTo(1) + + // But it's recomposed again when scrolled back on screen. + lazyList.onViewportChanged(4, 8) + awaitSnapshot() + assertThat(index5ComposeCount).isEqualTo(2) + } + } } From b1121c8a2a3900171112ce74f630de8a3e0f1718 Mon Sep 17 00:00:00 2001 From: Jesse Wilson Date: Wed, 6 Dec 2023 11:18:39 -0500 Subject: [PATCH 2/2] Spotless --- .../kotlin/app/cash/redwood/lazylayout/compose/LazyListTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/redwood-lazylayout-compose/src/commonTest/kotlin/app/cash/redwood/lazylayout/compose/LazyListTest.kt b/redwood-lazylayout-compose/src/commonTest/kotlin/app/cash/redwood/lazylayout/compose/LazyListTest.kt index d3340974d2..97f35aaa29 100644 --- a/redwood-lazylayout-compose/src/commonTest/kotlin/app/cash/redwood/lazylayout/compose/LazyListTest.kt +++ b/redwood-lazylayout-compose/src/commonTest/kotlin/app/cash/redwood/lazylayout/compose/LazyListTest.kt @@ -122,7 +122,7 @@ class LazyListTest { } LazyColumn( state = lazyListState, - placeholder = { Text("Placeholder") } + placeholder = { Text("Placeholder") }, ) { items(100) { if (it == 5) index5ComposeCount++