Skip to content

Commit cbc41cf

Browse files
Levi-MoreiraGerrit Code Review
authored andcommitted
Revert "CacheWindow refill update #2"
This reverts commit fdc614d. Reason for revert: broke g3 Change-Id: I932d0f0d2ff42cb8ae7342a3d3d65ada1891681d
1 parent fdc614d commit cbc41cf

5 files changed

Lines changed: 28 additions & 182 deletions

File tree

compose/foundation/foundation/integration-tests/lazy-tests/src/androidTest/kotlin/androidx/compose/foundation/lazy/list/LazyListCacheWindowTest.kt

Lines changed: 1 addition & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ class LazyListCacheWindowTest(orientation: Orientation) :
185185
}
186186

187187
@Test
188-
fun datasetChanged_shouldMakeSureNestedItemsChanged_afterScroll() {
188+
fun datasetChanged_shouldMakeSureNestedItemsChanged() {
189189
val items = mutableStateOf(listOf("a", "b", "c", "d", "e"))
190190

191191
rule.setContent {
@@ -264,80 +264,6 @@ class LazyListCacheWindowTest(orientation: Orientation) :
264264
rule.onNodeWithTag("second-nested-e").assertExists() // nested prefetched
265265
}
266266

267-
@Test
268-
fun datasetChanged_shouldMakeSureNestedItemsChanged_noScroll() {
269-
val items = mutableStateOf(listOf("a", "b", "c", "d"))
270-
271-
rule.setContent {
272-
@OptIn(ExperimentalFoundationApi::class)
273-
state = rememberLazyListState(cacheWindow = viewportWindow)
274-
LazyColumnOrRow(
275-
Modifier.mainAxisSize(itemsSizeDp * 2f)
276-
.then(
277-
object : RemeasurementModifier {
278-
override fun onRemeasurementAvailable(remeasurement: Remeasurement) {
279-
remeasure = remeasurement
280-
}
281-
}
282-
),
283-
state,
284-
) {
285-
items(items.value, key = { it }) {
286-
if (it == "e" || it == "f") {
287-
val state = rememberLazyListState(cacheWindow = viewportWindow)
288-
LazyRow(
289-
Modifier.mainAxisSize(itemsSizeDp).fillMaxCrossAxis().testTag(it),
290-
state = state,
291-
) {
292-
item {
293-
Spacer(
294-
Modifier.mainAxisSize(itemsSizeDp)
295-
.fillMaxCrossAxis()
296-
.testTag("first-nested-$it")
297-
)
298-
}
299-
300-
item {
301-
Spacer(
302-
Modifier.mainAxisSize(itemsSizeDp)
303-
.fillMaxCrossAxis()
304-
.testTag("second-nested-$it")
305-
)
306-
}
307-
}
308-
} else {
309-
Spacer(
310-
Modifier.mainAxisSize(itemsSizeDp)
311-
.fillMaxCrossAxis()
312-
.testTag(it)
313-
.layout { measurable, constraints ->
314-
val placeable = measurable.measure(constraints)
315-
layout(placeable.width, placeable.height) {
316-
placeable.place(0, 0)
317-
}
318-
}
319-
)
320-
}
321-
}
322-
}
323-
}
324-
325-
rule.onNodeWithTag("a").assertIsDisplayed() // fully visible
326-
rule.onNodeWithTag("b").assertIsDisplayed() // fully visible
327-
rule.onNodeWithTag("c").assertExists() // part of the window
328-
rule.onNodeWithTag("d").assertExists() // part of the window
329-
println("Changing Dataset")
330-
rule.runOnIdle { items.value = listOf("a", "b", "e", "f", "g", "h") }
331-
rule.waitForIdle()
332-
333-
rule.onNodeWithTag("e").assertExists() // item e will take place of item c
334-
rule.onNodeWithTag("first-nested-e").assertExists() // nested prefetched
335-
rule.onNodeWithTag("second-nested-e").assertExists() // nested prefetched
336-
rule.onNodeWithTag("f").assertExists() // item f will take place of item d
337-
rule.onNodeWithTag("first-nested-f").assertExists() // nested prefetched
338-
rule.onNodeWithTag("second-nested-f").assertExists() // nested prefetched
339-
}
340-
341267
@Test
342268
fun datasetChanged_noScrollHappened_shouldKeepAroundWithinBounds_notCrash() {
343269
val numItems = mutableStateOf(100)

compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/LazyListCacheWindowStrategy.kt

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -135,10 +135,6 @@ internal class LazyListCacheWindowScope() : CacheWindowScope {
135135

136136
override fun getLastIndexInLine(lineIndex: Int): Int = lineIndex
137137

138-
override fun getVisibleLineKey(indexInVisibleLines: Int): Any {
139-
return layoutInfo.visibleItemsInfo[indexInVisibleLines].key
140-
}
141-
142138
override fun getLastLineIndex(): Int {
143139
if (totalItemsCount == 0) return InvalidIndex
144140
return totalItemsCount - 1

compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/grid/LazyGridCacheWindowStrategy.kt

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import androidx.compose.foundation.gestures.snapping.offsetOnMainAxis
2121
import androidx.compose.foundation.gestures.snapping.sizeOnMainAxis
2222
import androidx.compose.foundation.lazy.layout.CacheWindowLogic
2323
import androidx.compose.foundation.lazy.layout.CacheWindowScope
24-
import androidx.compose.foundation.lazy.layout.CachedItem
2524
import androidx.compose.foundation.lazy.layout.InvalidIndex
2625
import androidx.compose.foundation.lazy.layout.LazyLayoutCacheWindow
2726
import androidx.compose.foundation.lazy.layout.LazyLayoutPrefetchState.PrefetchHandle
@@ -138,15 +137,6 @@ private class LazyGridCacheWindowScope() : CacheWindowScope {
138137
return tallestItemSize
139138
}
140139

141-
override fun getVisibleLineKey(indexInVisibleLines: Int): Any {
142-
// using the first item key to represent this line.
143-
val laneIndex = indexInVisibleLines + firstVisibleLineIndex
144-
return layoutInfo.visibleItemsInfo
145-
.fastFilter { it.lineIndex == laneIndex }
146-
.firstOrNull()
147-
?.key ?: CachedItem.NoKey
148-
}
149-
150140
override fun getVisibleItemLine(indexInVisibleLines: Int): Int =
151141
firstVisibleLineIndex + indexInVisibleLines
152142

compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/layout/CacheWindowLogic.kt

Lines changed: 27 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
package androidx.compose.foundation.lazy.layout
1818

19+
import androidx.collection.mutableIntIntMapOf
1920
import androidx.collection.mutableIntObjectMapOf
2021
import androidx.collection.mutableIntSetOf
2122
import androidx.compose.foundation.ExperimentalFoundationApi
@@ -42,7 +43,7 @@ internal abstract class CacheWindowLogic(
4243
* Cache for items sizes in the current window. Holds sizes for both visible and non-visible
4344
* items
4445
*/
45-
private val windowCache = mutableIntObjectMapOf<CachedItem>()
46+
private val windowCache = mutableIntIntMapOf()
4647
private var previousPassDelta = 0f
4748
private var previousPassItemCount = UnsetItemCount
4849
private var hasUpdatedVisibleItemsOnce = false
@@ -119,7 +120,15 @@ internal abstract class CacheWindowLogic(
119120
* changed.
120121
*/
121122
if (previousPassItemCount != UnsetItemCount && previousPassItemCount != totalItemsCount) {
122-
onDatasetChangedSize()
123+
debugLog { "Total Items Changed" }
124+
shouldRefillWindow = true
125+
prefetchWindowStartLine = prefetchWindowStartLine.coerceAtLeast(0)
126+
val lastLineIndex = getLastLineIndex()
127+
if (lastLineIndex != InvalidIndex) {
128+
prefetchWindowEndLine = prefetchWindowEndLine.coerceAtMost(lastLineIndex)
129+
}
130+
/** Free up the space so the fill will happen and not re-use old data. */
131+
removeOutOfBoundsItems(prefetchWindowEndLine, itemsCount - 1)
123132
}
124133

125134
itemsCount = totalItemsCount
@@ -128,8 +137,8 @@ internal abstract class CacheWindowLogic(
128137
// by [cancelOutOfBounds]. If any items changed sizes we re-trigger the window filling
129138
// update.
130139
if (hasVisibleItems) {
131-
forEachVisibleItem { index, key, mainAxisSize ->
132-
if (index != InvalidIndex) cacheVisibleItemsInfo(index, key, mainAxisSize)
140+
forEachVisibleItem { index, mainAxisSize ->
141+
if (index != InvalidIndex) cacheVisibleItemsInfo(index, mainAxisSize)
133142
}
134143
if (shouldRefillWindow) {
135144
// refill window in accordance with last pass delta
@@ -225,25 +234,6 @@ internal abstract class CacheWindowLogic(
225234
}
226235
}
227236

228-
private fun CacheWindowScope.onDatasetChangedSize() {
229-
debugLog { "Total Items Changed" }
230-
shouldRefillWindow = true
231-
prefetchWindowStartLine = prefetchWindowStartLine.coerceAtLeast(0)
232-
val lastLineIndex = getLastLineIndex()
233-
if (lastLineIndex != InvalidIndex) {
234-
prefetchWindowEndLine = prefetchWindowEndLine.coerceAtMost(lastLineIndex)
235-
}
236-
237-
/**
238-
* Resets the window state. We will refill the window on the direction of the last scroll.
239-
*/
240-
if (previousPassDelta <= 0f) {
241-
removeOutOfBoundsItems(lastVisibleLineIndex, itemsCount - 1)
242-
} else {
243-
removeOutOfBoundsItems(0, firstVisibleLineIndex)
244-
}
245-
}
246-
247237
fun resetStrategy() {
248238
prefetchWindowStartLine = Int.MAX_VALUE
249239
prefetchWindowEndLine = Int.MIN_VALUE
@@ -293,7 +283,7 @@ internal abstract class CacheWindowLogic(
293283
// If we get the same delta in the next frame, would we cover the extra space needed
294284
// to actually need this item? If so, mark it as urgent
295285
val isUrgent: Boolean =
296-
if (prefetchWindowEndLine + 1 == visibleWindowEnd + 1 && scrollDelta != 0.0f) {
286+
if (prefetchWindowEndLine + 1 == visibleWindowEnd + 1) {
297287
scrollDelta.absoluteValue >= mainAxisExtraSpaceEnd
298288
} else {
299289
false
@@ -325,9 +315,7 @@ internal abstract class CacheWindowLogic(
325315
// If we get the same delta in the next frame, would we cover the extra space needed
326316
// to actually need this item? If so, mark it as urgent
327317
val isUrgent: Boolean =
328-
if (
329-
prefetchWindowStartLine - 1 == visibleWindowStart - 1 && scrollDelta != 0.0f
330-
) {
318+
if (prefetchWindowStartLine - 1 == visibleWindowStart - 1) {
331319
scrollDelta.absoluteValue >= mainAxisExtraSpaceStart
332320
} else {
333321
false
@@ -370,7 +358,7 @@ internal abstract class CacheWindowLogic(
370358
while (prefetchWindowStartExtraSpace > 0 && prefetchWindowStartLine > 0) {
371359
val item =
372360
if (windowCache.containsKey(prefetchWindowStartLine - 1)) {
373-
windowCache[prefetchWindowStartLine - 1]!!.mainAxisSize
361+
windowCache[prefetchWindowStartLine - 1]
374362
} else {
375363
break
376364
}
@@ -385,7 +373,7 @@ internal abstract class CacheWindowLogic(
385373
while (prefetchWindowEndExtraSpace > 0 && prefetchWindowEndLine < itemsCount - 1) {
386374
val item =
387375
if (windowCache.containsKey(prefetchWindowEndLine + 1)) {
388-
windowCache[prefetchWindowEndLine + 1]!!.mainAxisSize
376+
windowCache[prefetchWindowEndLine + 1]
389377
} else {
390378
break
391379
}
@@ -398,16 +386,13 @@ internal abstract class CacheWindowLogic(
398386

399387
private fun CacheWindowScope.getItemSizeOrPrefetch(index: Int, isUrgent: Boolean): Int {
400388
return if (windowCache.containsKey(index)) {
401-
debugLog { "Item $index is Cached!" }
402-
windowCache[index]!!.mainAxisSize
389+
windowCache[index]
403390
} else if (prefetchWindowHandles.containsKey(index)) {
404391
// item is scheduled but didn't finish yet
405-
debugLog { "Item=$index is already scheduled. isUrgent=$isUrgent" }
406392
if (isUrgent) prefetchWindowHandles[index]?.fastForEach { it.markAsUrgent() }
407393
InvalidItemSize
408394
} else {
409395
// item is not scheduled
410-
debugLog { "Scheduling Prefetching for Item=$index. isUrgent=$isUrgent" }
411396
prefetchWindowHandles[index] =
412397
schedulePrefetch(index) { prefetchedIndex, size ->
413398
onItemPrefetched(prefetchedIndex, size)
@@ -419,7 +404,7 @@ internal abstract class CacheWindowLogic(
419404

420405
/** Grows the window with measured items and prefetched items. */
421406
private fun cachePrefetchedItem(index: Int, size: Int) {
422-
windowCache[index] = updateOrCreateCachedItem(index, size, CachedItem.NoKey)
407+
windowCache[index] = size
423408
if (index > prefetchWindowEndLine) {
424409
prefetchWindowEndLine = index
425410
prefetchWindowEndExtraSpace -= size
@@ -429,34 +414,18 @@ internal abstract class CacheWindowLogic(
429414
}
430415
}
431416

432-
private fun updateOrCreateCachedItem(index: Int, size: Int, key: Any): CachedItem {
433-
val cachedItem = windowCache[index]
434-
return if (cachedItem != null) {
435-
cachedItem.mainAxisSize = size
436-
cachedItem.key = key
437-
cachedItem
438-
} else {
439-
CachedItem(CachedItem.NoKey, size)
440-
}
441-
}
442-
443417
/**
444418
* When caching visible items we need to check if the existing item changed sizes. If so, we
445419
* will set [shouldRefillWindow] which will trigger a complete window filling and cancel any out
446-
* of bounds requests. The same is valid if items are replaced (have the same size by key
447-
* changed).
420+
* of bounds requests.
448421
*/
449-
private fun cacheVisibleItemsInfo(index: Int, key: Any, size: Int) {
450-
debugLog { "cacheVisibleItemsInfo item=$index size=$size key=$key" }
451-
if (windowCache.containsKey(index)) {
452-
val cachedSize = windowCache[index]!!.mainAxisSize
453-
val cachedKey = windowCache[index]!!.key
454-
if (cachedSize != size || cachedKey != key) {
455-
shouldRefillWindow = true
456-
}
422+
private fun cacheVisibleItemsInfo(index: Int, size: Int) {
423+
debugLog { "cacheVisibleItemsInfo item=$index size=$size" }
424+
if (windowCache.containsKey(index) && windowCache[index] != size) {
425+
shouldRefillWindow = true
457426
}
458427

459-
windowCache[index] = updateOrCreateCachedItem(index, size, key)
428+
windowCache[index] = size
460429
// We're caching a visible item, remove its handle since we won't need it anymore.
461430
prefetchWindowStartLine = minOf(prefetchWindowStartLine, index)
462431
prefetchWindowEndLine = maxOf(prefetchWindowEndLine, index)
@@ -470,8 +439,6 @@ internal abstract class CacheWindowLogic(
470439

471440
windowCache.forEachKey { if (it in startLine..endLine) indicesToRemove.add(it) }
472441

473-
debugLog { "Indices to remove=$indicesToRemove" }
474-
475442
indicesToRemove.forEach {
476443
prefetchWindowHandles.remove(it)?.fastForEach { it.cancel() }
477444
windowCache.remove(it)
@@ -534,19 +501,15 @@ internal interface CacheWindowScope {
534501

535502
fun getVisibleItemLine(indexInVisibleLines: Int): Int
536503

537-
fun getVisibleLineKey(indexInVisibleLines: Int): Any
538-
539504
fun getLastIndexInLine(lineIndex: Int): Int
540505

541506
fun getLastLineIndex(): Int
542507
}
543508

544509
internal inline fun CacheWindowScope.forEachVisibleItem(
545-
action: (itemIndex: Int, itemKey: Any, mainAxisSize: Int) -> Unit
510+
action: (itemIndex: Int, mainAxisSize: Int) -> Unit
546511
) {
547-
repeat(visibleLineCount) {
548-
action(getVisibleItemLine(it), getVisibleLineKey(it), getVisibleItemSize(it))
549-
}
512+
repeat(visibleLineCount) { action(getVisibleItemLine(it), getVisibleItemSize(it)) }
550513
}
551514

552515
private const val InvalidItemSize = -1
@@ -560,7 +523,3 @@ private inline fun debugLog(generateMsg: () -> String) {
560523
println("CacheWindowLogic: ${generateMsg()}")
561524
}
562525
}
563-
564-
internal class CachedItem(var key: Any, var mainAxisSize: Int) {
565-
companion object NoKey
566-
}

compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/pager/PagerCacheWindowLogic.kt

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ package androidx.compose.foundation.pager
1919
import androidx.compose.foundation.ExperimentalFoundationApi
2020
import androidx.compose.foundation.lazy.layout.CacheWindowLogic
2121
import androidx.compose.foundation.lazy.layout.CacheWindowScope
22-
import androidx.compose.foundation.lazy.layout.CachedItem
2322
import androidx.compose.foundation.lazy.layout.InvalidIndex
2423
import androidx.compose.foundation.lazy.layout.LazyLayoutCacheWindow
2524
import androidx.compose.foundation.lazy.layout.LazyLayoutPrefetchState
@@ -157,30 +156,6 @@ private class PagerCacheWindowScope(val itemCount: () -> Int) : CacheWindowScope
157156
return InvalidIndex
158157
}
159158

160-
override fun getVisibleLineKey(indexInVisibleLines: Int): Any {
161-
val extraPagesBeforeCount = layoutInfo.extraPagesBefore.size
162-
163-
val visiblePagesCount = layoutInfo.visiblePagesInfo.size
164-
165-
if (indexInVisibleLines < extraPagesBeforeCount) {
166-
return layoutInfo.extraPagesBefore[indexInVisibleLines].key
167-
}
168-
169-
if (
170-
indexInVisibleLines >= extraPagesBeforeCount &&
171-
indexInVisibleLines < extraPagesBeforeCount + visiblePagesCount
172-
) {
173-
return layoutInfo.visiblePagesInfo[indexInVisibleLines - extraPagesBeforeCount].key
174-
}
175-
176-
if (indexInVisibleLines >= extraPagesBeforeCount + visiblePagesCount) {
177-
return layoutInfo.extraPagesAfter[
178-
indexInVisibleLines - extraPagesBeforeCount - visiblePagesCount]
179-
.key
180-
}
181-
return CachedItem.NoKey
182-
}
183-
184159
override fun getLastIndexInLine(lineIndex: Int): Int = lineIndex
185160

186161
override fun getLastLineIndex(): Int {

0 commit comments

Comments
 (0)