Merge changes I8acb28c2,I0008a425 into androidx-main
* changes:
Do less recompositions when the sliding window updates
Ensure Lazy layouts recomposition with the same input doesn't cause remeasure
diff --git a/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/grid/LazyGridTest.kt b/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/grid/LazyGridTest.kt
index 5848b5c..ba84f74 100644
--- a/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/grid/LazyGridTest.kt
+++ b/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/grid/LazyGridTest.kt
@@ -44,6 +44,8 @@
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.setValue
import androidx.compose.ui.Modifier
+import androidx.compose.ui.composed
+import androidx.compose.ui.layout.layout
import androidx.compose.ui.platform.LocalLayoutDirection
import androidx.compose.ui.platform.testTag
import androidx.compose.ui.semantics.SemanticsActions
@@ -1031,5 +1033,75 @@
rule.onNodeWithTag("7").assertDoesNotExist()
}
+ @Test
+ fun recomposingWithNewComposedModifierObjectIsNotCausingRemeasure() {
+ var remeasureCount = 0
+ val layoutModifier = Modifier.layout { measurable, constraints ->
+ remeasureCount++
+ val placeable = measurable.measure(constraints)
+ layout(placeable.width, placeable.height) {
+ placeable.place(0, 0)
+ }
+ }
+ val counter = mutableStateOf(0)
+
+ rule.setContentWithTestViewConfiguration {
+ counter.value // just to trigger recomposition
+ LazyVerticalGrid(
+ GridCells.Fixed(1),
+ // this will return a new object everytime causing LazyVerticalGrid recomposition
+ // without causing remeasure
+ Modifier.composed { layoutModifier }
+ ) {
+ items(1) {
+ Spacer(Modifier.size(10.dp))
+ }
+ }
+ }
+
+ rule.runOnIdle {
+ Truth.assertThat(remeasureCount).isEqualTo(1)
+ counter.value++
+ }
+
+ rule.runOnIdle {
+ Truth.assertThat(remeasureCount).isEqualTo(1)
+ }
+ }
+
+ @Test
+ fun scrollingALotDoesntCauseLazyLayoutRecomposition() {
+ var recomposeCount = 0
+ lateinit var state: LazyGridState
+
+ rule.setContentWithTestViewConfiguration {
+ state = rememberLazyGridState()
+ LazyVerticalGrid(
+ GridCells.Fixed(1),
+ Modifier.composed {
+ recomposeCount++
+ Modifier
+ }.size(100.dp),
+ state
+ ) {
+ items(1000) {
+ Spacer(Modifier.size(100.dp))
+ }
+ }
+ }
+
+ rule.runOnIdle {
+ Truth.assertThat(recomposeCount).isEqualTo(1)
+
+ runBlocking {
+ state.scrollToItem(100)
+ }
+ }
+
+ rule.runOnIdle {
+ Truth.assertThat(recomposeCount).isEqualTo(1)
+ }
+ }
+
// TODO: add tests for the cache logic
}
diff --git a/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/list/LazyListTest.kt b/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/list/LazyListTest.kt
index c546d2f25..6192186 100644
--- a/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/list/LazyListTest.kt
+++ b/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/list/LazyListTest.kt
@@ -44,10 +44,12 @@
import androidx.compose.testutils.WithTouchSlop
import androidx.compose.testutils.assertShape
import androidx.compose.ui.Modifier
+import androidx.compose.ui.composed
import androidx.compose.ui.draw.drawBehind
import androidx.compose.ui.geometry.Offset
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.graphics.RectangleShape
+import androidx.compose.ui.layout.layout
import androidx.compose.ui.platform.testTag
import androidx.compose.ui.semantics.SemanticsActions
import androidx.compose.ui.semantics.SemanticsProperties
@@ -1646,6 +1648,74 @@
rule.onNodeWithTag("3").assertIsDisplayed()
}
+ @Test
+ fun recomposingWithNewComposedModifierObjectIsNotCausingRemeasure() {
+ var remeasureCount = 0
+ val layoutModifier = Modifier.layout { measurable, constraints ->
+ remeasureCount++
+ val placeable = measurable.measure(constraints)
+ layout(placeable.width, placeable.height) {
+ placeable.place(0, 0)
+ }
+ }
+ val counter = mutableStateOf(0)
+
+ rule.setContentWithTestViewConfiguration {
+ counter.value // just to trigger recomposition
+ LazyColumnOrRow(
+ // this will return a new object everytime causing Lazy list recomposition
+ // without causing remeasure
+ Modifier.composed { layoutModifier }
+ ) {
+ items(1) {
+ Spacer(Modifier.size(10.dp))
+ }
+ }
+ }
+
+ rule.runOnIdle {
+ assertThat(remeasureCount).isEqualTo(1)
+ counter.value++
+ }
+
+ rule.runOnIdle {
+ assertThat(remeasureCount).isEqualTo(1)
+ }
+ }
+
+ @Test
+ fun scrollingALotDoesntCauseLazyLayoutRecomposition() {
+ var recomposeCount = 0
+ lateinit var state: LazyListState
+
+ rule.setContentWithTestViewConfiguration {
+ state = rememberLazyListState()
+ LazyColumnOrRow(
+ Modifier.composed {
+ recomposeCount++
+ Modifier
+ },
+ state
+ ) {
+ items(1000) {
+ Spacer(Modifier.size(10.dp))
+ }
+ }
+ }
+
+ rule.runOnIdle {
+ assertThat(recomposeCount).isEqualTo(1)
+
+ runBlocking {
+ state.scrollToItem(100)
+ }
+ }
+
+ rule.runOnIdle {
+ assertThat(recomposeCount).isEqualTo(1)
+ }
+ }
+
// ********************* END OF TESTS *********************
// Helper functions, etc. live below here
diff --git a/compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/grid/LazyGrid.kt b/compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/grid/LazyGrid.kt
index 08ed113..e80d43d 100644
--- a/compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/grid/LazyGrid.kt
+++ b/compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/grid/LazyGrid.kt
@@ -109,10 +109,7 @@
state.prefetchPolicy = rememberLazyLayoutPrefetchPolicy()
val innerState = rememberLazyLayoutState().also { state.innerState = it }
- val itemsProvider = stateOfItemsProvider.value
- if (itemsProvider.itemsCount > 0) {
- state.updateScrollPositionIfTheFirstItemWasMoved(itemsProvider)
- }
+ ScrollPositionUpdater(stateOfItemsProvider, state)
LazyLayout(
modifier = modifier
@@ -152,6 +149,19 @@
)
}
+/** Extracted to minimize the recomposition scope */
+@OptIn(ExperimentalFoundationApi::class)
+@Composable
+private fun ScrollPositionUpdater(
+ stateOfItemsProvider: State<LazyGridItemsProvider>,
+ state: LazyGridState
+) {
+ val itemsProvider = stateOfItemsProvider.value
+ if (itemsProvider.itemsCount > 0) {
+ state.updateScrollPositionIfTheFirstItemWasMoved(itemsProvider)
+ }
+}
+
@OptIn(ExperimentalFoundationApi::class)
@Composable
private fun rememberLazyGridMeasurePolicy(
diff --git a/compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/grid/LazyGridItemsProviderImpl.kt b/compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/grid/LazyGridItemsProviderImpl.kt
index 7ea9ee4..8953fbd 100644
--- a/compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/grid/LazyGridItemsProviderImpl.kt
+++ b/compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/grid/LazyGridItemsProviderImpl.kt
@@ -45,7 +45,7 @@
val latestContent = rememberUpdatedState(content)
val nearestItemsRangeState = remember(state) {
mutableStateOf(
- calculateNearestItemsRange(state.firstVisibleItemIndex)
+ calculateNearestItemsRange(state.firstVisibleItemIndexNonObservable.value)
)
}
LaunchedEffect(nearestItemsRangeState) {
diff --git a/compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/grid/LazySemantics.kt b/compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/grid/LazySemantics.kt
index 47844fd..4622ceb 100644
--- a/compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/grid/LazySemantics.kt
+++ b/compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/grid/LazySemantics.kt
@@ -20,7 +20,9 @@
import androidx.compose.foundation.gestures.ScrollableState
import androidx.compose.foundation.gestures.animateScrollBy
import androidx.compose.foundation.lazy.LazyGridState
+import androidx.compose.runtime.Composable
import androidx.compose.runtime.State
+import androidx.compose.runtime.remember
import androidx.compose.ui.Modifier
import androidx.compose.ui.semantics.CollectionInfo
import androidx.compose.ui.semantics.ScrollAxisRange
@@ -35,6 +37,8 @@
import kotlinx.coroutines.launch
@OptIn(ExperimentalFoundationApi::class)
+@Suppress("ComposableModifierFactory", "ModifierInspectorInfo")
+@Composable
internal fun Modifier.lazyGridSemantics(
stateOfItemsProvider: State<LazyGridItemsProvider>,
state: LazyGridState,
@@ -42,16 +46,24 @@
isVertical: Boolean,
reverseScrolling: Boolean,
userScrollEnabled: Boolean
-): Modifier {
- return semantics {
- indexForKey { needle ->
+) = this.then(
+ remember(
+ stateOfItemsProvider,
+ state,
+ isVertical,
+ reverseScrolling,
+ userScrollEnabled
+ ) {
+ val indexForKeyMapping: (Any) -> Int = { needle ->
val key = stateOfItemsProvider.value::getKey
+ var result = -1
for (index in 0 until stateOfItemsProvider.value.itemsCount) {
if (key(index) == needle) {
- return@indexForKey index
+ result = index
+ break
}
}
- -1
+ result
}
val accessibilityScrollState = ScrollAxisRange(
@@ -74,23 +86,26 @@
},
reverseScrolling = reverseScrolling
)
- if (isVertical) {
- verticalScrollAxisRange = accessibilityScrollState
- } else {
- horizontalScrollAxisRange = accessibilityScrollState
- }
- if (userScrollEnabled) {
- scrollBy { x, y ->
- val delta = if (isVertical) { y } else { x }
+ val scrollByAction: ((x: Float, y: Float) -> Boolean)? = if (userScrollEnabled) {
+ { x, y ->
+ val delta = if (isVertical) {
+ y
+ } else {
+ x
+ }
coroutineScope.launch {
(state as ScrollableState).animateScrollBy(delta)
}
// TODO(aelias): is it important to return false if we know in advance we cannot scroll?
true
}
+ } else {
+ null
+ }
- scrollToIndex { index ->
+ val scrollToIndexAction: ((Int) -> Boolean)? = if (userScrollEnabled) {
+ { index ->
require(index >= 0 && index < state.layoutInfo.totalItemsCount) {
"Can't scroll to index $index, it is out of " +
"bounds [0, ${state.layoutInfo.totalItemsCount})"
@@ -100,9 +115,31 @@
}
true
}
+ } else {
+ null
}
// TODO(popam): check if this is correct - it would be nice to provide correct columns here
- collectionInfo = CollectionInfo(rowCount = -1, columnCount = -1)
+ val collectionInfo = CollectionInfo(rowCount = -1, columnCount = -1)
+
+ Modifier.semantics {
+ indexForKey(indexForKeyMapping)
+
+ if (isVertical) {
+ verticalScrollAxisRange = accessibilityScrollState
+ } else {
+ horizontalScrollAxisRange = accessibilityScrollState
+ }
+
+ if (scrollByAction != null) {
+ scrollBy(action = scrollByAction)
+ }
+
+ if (scrollToIndexAction != null) {
+ scrollToIndex(action = scrollToIndexAction)
+ }
+
+ this.collectionInfo = collectionInfo
+ }
}
-}
+)
diff --git a/compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/list/LazyList.kt b/compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/list/LazyList.kt
index 715dfaf..291d2b2 100644
--- a/compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/list/LazyList.kt
+++ b/compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/list/LazyList.kt
@@ -107,10 +107,7 @@
state.prefetchPolicy = rememberLazyLayoutPrefetchPolicy()
val innerState = rememberLazyLayoutState().also { state.innerState = it }
- val itemsProvider = stateOfItemsProvider.value
- if (itemsProvider.itemsCount > 0) {
- state.updateScrollPositionIfTheFirstItemWasMoved(itemsProvider)
- }
+ ScrollPositionUpdater(stateOfItemsProvider, state)
LazyLayout(
modifier = modifier
@@ -149,6 +146,18 @@
)
}
+/** Extracted to minimize the recomposition scope */
+@Composable
+private fun ScrollPositionUpdater(
+ stateOfItemsProvider: State<LazyListItemsProvider>,
+ state: LazyListState
+) {
+ val itemsProvider = stateOfItemsProvider.value
+ if (itemsProvider.itemsCount > 0) {
+ state.updateScrollPositionIfTheFirstItemWasMoved(itemsProvider)
+ }
+}
+
@Composable
private fun rememberLazyListMeasurePolicy(
/** State containing the items provider of the list. */
diff --git a/compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/list/LazySemantics.kt b/compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/list/LazySemantics.kt
index 2a3753c..797138d 100644
--- a/compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/list/LazySemantics.kt
+++ b/compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/list/LazySemantics.kt
@@ -19,7 +19,9 @@
import androidx.compose.foundation.gestures.ScrollableState
import androidx.compose.foundation.gestures.animateScrollBy
import androidx.compose.foundation.lazy.LazyListState
+import androidx.compose.runtime.Composable
import androidx.compose.runtime.State
+import androidx.compose.runtime.remember
import androidx.compose.ui.Modifier
import androidx.compose.ui.semantics.CollectionInfo
import androidx.compose.ui.semantics.ScrollAxisRange
@@ -33,6 +35,8 @@
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.launch
+@Suppress("ComposableModifierFactory", "ModifierInspectorInfo")
+@Composable
internal fun Modifier.lazyListSemantics(
stateOfItemsProvider: State<LazyListItemsProvider>,
state: LazyListState,
@@ -40,16 +44,24 @@
isVertical: Boolean,
reverseScrolling: Boolean,
userScrollEnabled: Boolean
-): Modifier {
- return semantics {
- indexForKey { needle ->
+) = this.then(
+ remember(
+ stateOfItemsProvider,
+ state,
+ isVertical,
+ reverseScrolling,
+ userScrollEnabled
+ ) {
+ val indexForKeyMapping: (Any) -> Int = { needle ->
val key = stateOfItemsProvider.value::getKey
+ var result = -1
for (index in 0 until stateOfItemsProvider.value.itemsCount) {
if (key(index) == needle) {
- return@indexForKey index
+ result = index
+ break
}
}
- -1
+ result
}
val accessibilityScrollState = ScrollAxisRange(
@@ -72,14 +84,8 @@
},
reverseScrolling = reverseScrolling
)
- if (isVertical) {
- verticalScrollAxisRange = accessibilityScrollState
- } else {
- horizontalScrollAxisRange = accessibilityScrollState
- }
-
- if (userScrollEnabled) {
- scrollBy { x, y ->
+ val scrollByAction: ((x: Float, y: Float) -> Boolean)? = if (userScrollEnabled) {
+ { x, y ->
val delta = if (isVertical) {
y
} else {
@@ -91,8 +97,12 @@
// TODO(aelias): is it important to return false if we know in advance we cannot scroll?
true
}
+ } else {
+ null
+ }
- scrollToIndex { index ->
+ val scrollToIndexAction: ((Int) -> Boolean)? = if (userScrollEnabled) {
+ { index ->
require(index >= 0 && index < state.layoutInfo.totalItemsCount) {
"Can't scroll to index $index, it is out of " +
"bounds [0, ${state.layoutInfo.totalItemsCount})"
@@ -102,11 +112,33 @@
}
true
}
+ } else {
+ null
}
- collectionInfo = CollectionInfo(
+ val collectionInfo = CollectionInfo(
rowCount = if (isVertical) -1 else 1,
columnCount = if (isVertical) 1 else -1
)
+
+ Modifier.semantics {
+ indexForKey(indexForKeyMapping)
+
+ if (isVertical) {
+ verticalScrollAxisRange = accessibilityScrollState
+ } else {
+ horizontalScrollAxisRange = accessibilityScrollState
+ }
+
+ if (scrollByAction != null) {
+ scrollBy(action = scrollByAction)
+ }
+
+ if (scrollToIndexAction != null) {
+ scrollToIndex(action = scrollToIndexAction)
+ }
+
+ this.collectionInfo = collectionInfo
+ }
}
-}
+)
