Skip to content

Commit

Permalink
Add a test to confirm there's no unnecessary recompositions
Browse files Browse the repository at this point in the history
Also expose the size of the preload window in
LazyListState. It should be possible for the
application layer to change this as necessary.
  • Loading branch information
squarejesse committed Dec 6, 2023
1 parent c5f2ed0 commit 12971a5
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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) }
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<LazyListValue>().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)
}
}
}

0 comments on commit 12971a5

Please sign in to comment.