[go: nahoru, domu]

Improve snapping animations in Pager

Updated how snapping animations work to make them more consistent. We updated the SnapLayoutInfoProvider for Pager to make the animation specs used more deterministic. We also optimized the animations for 1 pager Pagers, the most common use case. In those cases we will always use the same animation spec (usually a spring) to run the settling animation. For multi page snapping, the specs used will vary but we also updated the fling behavior documentation to make it clear which specs should be used to control the settling animations in that case as well.

Relnote: N/A
Test: N/A
Fixes: 272110321
Change-Id: I553c8c67bd1c5323f758ce033b9cc4b622b59661
diff --git a/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/pager/PagerScrollingTest.kt b/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/pager/PagerScrollingTest.kt
index 8d4a2b4..d3dd669 100644
--- a/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/pager/PagerScrollingTest.kt
+++ b/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/pager/PagerScrollingTest.kt
@@ -19,12 +19,7 @@
 import androidx.compose.foundation.ExperimentalFoundationApi
 import androidx.compose.foundation.gestures.snapping.MinFlingVelocityDp
 import androidx.compose.foundation.layout.fillMaxSize
-import androidx.compose.runtime.getValue
-import androidx.compose.runtime.key
-import androidx.compose.runtime.mutableStateOf
-import androidx.compose.runtime.setValue
 import androidx.compose.ui.Modifier
-import androidx.compose.ui.platform.testTag
 import androidx.compose.ui.test.assertIsDisplayed
 import androidx.compose.ui.test.onNodeWithTag
 import androidx.compose.ui.test.performTouchInput
@@ -644,68 +639,6 @@
         assertThat(pagerState.currentPage - initialPage).isEqualTo(pageDisplacement)
     }
 
-    @Test
-    fun pagerStateChange_flingBehaviorShouldRecreate() {
-        var initialPage by mutableStateOf(0)
-        rule.setContent {
-            val state = key(initialPage) {
-                rememberPagerState(
-                    initialPage = initialPage,
-                    initialPageOffsetFraction = 0f
-                ) {
-                    10
-                }.also { pagerState = it }
-            }
-
-            HorizontalOrVerticalPager(
-                modifier = Modifier
-                    .fillMaxSize()
-                    .testTag(PagerTestTag),
-                state = state,
-                pageSize = PageSize.Fill
-            ) {
-                Page(index = it)
-            }
-        }
-        val delta = pageSize * 0.4f * scrollForwardSign
-        runAndWaitForPageSettling {
-            onPager().performTouchInput {
-                swipeWithVelocityAcrossMainAxis(
-                    with(rule.density) { 1.1f * MinFlingVelocityDp.toPx() },
-                    delta
-                )
-            }
-        }
-        rule.onNodeWithTag("1").assertIsDisplayed()
-        confirmPageIsInCorrectPosition(1)
-
-        runAndWaitForPageSettling {
-            onPager().performTouchInput {
-                swipeWithVelocityAcrossMainAxis(
-                    with(rule.density) { 1.1f * MinFlingVelocityDp.toPx() },
-                    delta
-                )
-            }
-        }
-
-        rule.onNodeWithTag("2").assertIsDisplayed()
-        confirmPageIsInCorrectPosition(2)
-
-        rule.runOnIdle { initialPage = 1 }
-
-        rule.waitForIdle()
-        runAndWaitForPageSettling {
-            onPager().performTouchInput {
-                swipeWithVelocityAcrossMainAxis(
-                    with(rule.density) { 1.1f * MinFlingVelocityDp.toPx() },
-                    delta
-                )
-            }
-        }
-
-        confirmPageIsInCorrectPosition(2)
-    }
-
     companion object {
         @JvmStatic
         @Parameterized.Parameters(name = "{0}")
diff --git a/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/pager/PagerTest.kt b/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/pager/PagerTest.kt
index b7a50b8..9cb6cfa 100644
--- a/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/pager/PagerTest.kt
+++ b/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/pager/PagerTest.kt
@@ -17,6 +17,7 @@
 package androidx.compose.foundation.pager
 
 import androidx.compose.foundation.ExperimentalFoundationApi
+import androidx.compose.foundation.gestures.snapping.SnapFlingBehavior
 import androidx.compose.foundation.layout.Spacer
 import androidx.compose.foundation.layout.fillMaxSize
 import androidx.compose.runtime.Composable
@@ -270,7 +271,8 @@
             ) {
                 Spacer(
                     Modifier
-                        .fillMaxSize())
+                        .fillMaxSize()
+                )
             }
         }
 
@@ -293,6 +295,37 @@
         }
     }
 
+    @Test
+    fun pagerStateChange_flingBehaviorShouldRecreate() {
+        var previousFlingBehavior: SnapFlingBehavior? = null
+        var latestFlingBehavior: SnapFlingBehavior? = null
+        val stateHolder = mutableStateOf(PagerStateImpl(0, 0.0f) { 10 })
+        rule.setContent {
+            HorizontalOrVerticalPager(
+                modifier = Modifier
+                    .fillMaxSize()
+                    .testTag(PagerTestTag),
+                state = stateHolder.value,
+                pageSize = PageSize.Fill,
+                flingBehavior = PagerDefaults.flingBehavior(state = stateHolder.value).also {
+                    latestFlingBehavior = it
+                    if (previousFlingBehavior == null) {
+                        previousFlingBehavior = it
+                    }
+                }
+            ) {
+                Page(index = it)
+            }
+        }
+
+        rule.runOnIdle {
+            stateHolder.value = PagerStateImpl(0, 0.0f) { 20 }
+        }
+
+        rule.waitForIdle()
+        assertThat(previousFlingBehavior).isNotEqualTo(latestFlingBehavior)
+    }
+
     companion object {
         @JvmStatic
         @Parameterized.Parameters(name = "{0}")
diff --git a/compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/gestures/snapping/SnapFlingBehavior.kt b/compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/gestures/snapping/SnapFlingBehavior.kt
index 76e4f37..4df8844 100644
--- a/compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/gestures/snapping/SnapFlingBehavior.kt
+++ b/compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/gestures/snapping/SnapFlingBehavior.kt
@@ -237,6 +237,10 @@
     ): Boolean {
         val decayOffset = highVelocityAnimationSpec.calculateTargetValue(NoDistance, velocity)
         val snapStepSize = with(snapLayoutInfoProvider) { density.calculateSnapStepSize() }
+        debugLog {
+            "Evaluating decay possibility with " +
+                "decayOffset=$decayOffset and proposed approach=$offset"
+        }
         return decayOffset.absoluteValue >= (offset.absoluteValue + snapStepSize)
     }
 
@@ -420,6 +424,10 @@
         consumedUpToNow += consumed
     }
 
+    debugLog {
+        "Snap Animation: Proposed Offset=$targetOffset Achieved Offset=$consumedUpToNow"
+    }
+
     // Always course correct velocity so they don't become too large.
     val finalVelocity = animationState.velocity.coerceToTarget(initialVelocity)
     return AnimationResult(
diff --git a/compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/pager/Pager.kt b/compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/pager/Pager.kt
index 946e681..4956155 100644
--- a/compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/pager/Pager.kt
+++ b/compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/pager/Pager.kt
@@ -52,12 +52,9 @@
 import androidx.compose.ui.unit.Dp
 import androidx.compose.ui.unit.Velocity
 import androidx.compose.ui.unit.dp
-import androidx.compose.ui.util.fastFirstOrNull
 import androidx.compose.ui.util.fastForEach
 import kotlin.math.abs
 import kotlin.math.absoluteValue
-import kotlin.math.ceil
-import kotlin.math.floor
 import kotlin.math.sign
 import kotlinx.coroutines.CancellationException
 import kotlinx.coroutines.launch
@@ -487,39 +484,66 @@
      * @see androidx.compose.foundation.gestures.snapping.SnapFlingBehavior for more information
      * on what which parameter controls in the overall snapping animation.
      *
+     * The animation specs used by the fling behavior will depend on 3 factors:
+     * 1) The gesture velocity.
+     * 2) The target page proposed by [pagerSnapDistance].
+     * 3) The minimum velocity determined by [snapVelocityThreshold].
+     *
+     * If you're using single page snapping (the most common use case for [Pager]), there won't
+     * be enough space to actually run a decay animation to approach the target page, so the Pager
+     * will always use the snapping animation from [snapAnimationSpec].
+     * If the gesture velocity is smaller than the [snapVelocityThreshold], the Pager will also use
+     * [snapAnimationSpec].
+     * If you're using multi-page snapping (this means you're abs(targetPage - currentPage) > 1)
+     * the Pager may use [highVelocityAnimationSpec] or [lowVelocityAnimationSpec] to approach the
+     * targetPage, it will depend on the velocity generated by the triggering gesture.
+     * If the gesture has a high enough velocity to approach the target page, the Pager will use
+     * [highVelocityAnimationSpec] followed by [snapAnimationSpec] for the final step of the
+     * animation. If the gesture doesn't have enough velocity, the Pager will use
+     * [lowVelocityAnimationSpec] + [snapAnimationSpec] in a similar fashion.
+     *
      * @param state The [PagerState] that controls the which to which this FlingBehavior will
      * be applied to.
      * @param pagerSnapDistance A way to control the snapping destination for this [Pager].
      * The default behavior will result in any fling going to the next page in the direction of the
-     * fling (if the fling has enough velocity, otherwise we'll bounce back). Use
+     * fling (if the fling has enough velocity, otherwise  the Pager will bounce back). Use
      * [PagerSnapDistance.atMost] to define a maximum number of pages this [Pager] is allowed to
      * fling after scrolling is finished and fling has started.
-     * @param lowVelocityAnimationSpec The animation spec used to approach the target offset. When
+     * @param lowVelocityAnimationSpec An animation spec used to approach the target offset. When
      * the fling velocity is not large enough. Large enough means large enough to naturally decay.
+     * When snapping through many pages, the Pager may not be able to run a decay animation, so it
+     * will use this spec to run an animation to approach the target page requested by
+     * [pagerSnapDistance].
      * @param highVelocityAnimationSpec The animation spec used to approach the target offset. When
-     * the fling velocity is large enough. Large enough means large enough to naturally decay.
-     * @param snapAnimationSpec The animation spec used to finally snap to the position.
+     * the fling velocity is large enough. Large enough means large enough to naturally decay. For
+     * single page snapping this usually never happens since there won't be enough space to run a
+     * decay animation.
+     * @param snapAnimationSpec The animation spec used to finally snap to the position. This
+     * animation will be often used in 2 cases: 1) There was enough space to an approach animation,
+     * the Pager will use [snapAnimationSpec] in the last step of the animation to settle the page
+     * into position. 2) There was not enough space to run the approach animation. 3) In snapping
+     * when the gesture velocity is below the [snapVelocityThreshold].
      * @param snapVelocityThreshold The minimum velocity required for a fling to be considered
      * high enough to make pages animate through [lowVelocityAnimationSpec] and
      * [highVelocityAnimationSpec].
      * @param snapPositionalThreshold If the fling has a low velocity (e.g. slow scroll),
      * this fling behavior will use this snap threshold in order to determine if the pager should
      * snap back or move forward. Use a number between 0 and 1 as a fraction of the page size that
-     * needs to be scrolled before we consider it should move to the next page. For instance, if
-     * snapPositionalThreshold = 0.35, it means if this pager is scrolled with a slow velocity and
-     * we scroll more than 35% of the page size, then will jump to the next page, if not we scroll
-     * back. The default value is 50% meaning if we scroll the page more than 50% and let go it will
-     * snap to the next page.
+     * needs to be scrolled before the Pager considers it should move to the next page.
+     * For instance, if snapPositionalThreshold = 0.35, it means if this pager is scrolled with a
+     * slow velocity and the Pager scrolls more than 35% of the page size, then will jump to the
+     * next page, if not it scrolls back. The default value is 50% meaning if the Pager scrolls the
+     * page more than 50% and let go it will snap to the next page.
      * Note that any fling that has high enough velocity will *always* move to the next page
      * in the direction of the fling.
      *
      * @return An instance of [FlingBehavior] that will perform Snapping to the next page by
-     * default. The animation will be governed by the post scroll velocity and we'll use either
+     * default. The animation will be governed by the post scroll velocity and the Pager will use
+     * either
      * [lowVelocityAnimationSpec] or [highVelocityAnimationSpec] to approach the snapped position
-     * and the last step of the animation will be performed by [snapAnimationSpec]. If a velocity
-     * is not high enough (lower than [snapVelocityThreshold]) the pager will use
-     * [snapAnimationSpec] to reach the snapped position. If the velocity is high enough, we'll
-     * use the logic described in [highVelocityAnimationSpec] and [lowVelocityAnimationSpec].
+     * If a velocity is not high enough (lower than [snapVelocityThreshold]) the pager will use
+     * [snapAnimationSpec] to reach the snapped position. If the velocity is high enough, the Pager
+     * will use the logic described in [highVelocityAnimationSpec] and [lowVelocityAnimationSpec].
      */
     @Composable
     fun flingBehavior(
@@ -638,6 +662,13 @@
         pageSize: Int,
         pageSpacing: Int,
     ): Int {
+        debugLog {
+            "PagerSnapDistanceMaxPages: startPage=$startPage " +
+                "suggestedTargetPage=$suggestedTargetPage " +
+                "velocity=$velocity " +
+                "pageSize=$pageSize " +
+                "pageSpacing$pageSpacing"
+        }
         return suggestedTargetPage.coerceIn(startPage - pagesLimit, startPage + pagesLimit)
     }
 
@@ -697,12 +728,16 @@
 
             val isForward = pagerState.isScrollingForward()
 
+            // how many pages have I scrolled using a drag gesture.
             val offsetFromSnappedPosition =
                 pagerState.dragGestureDelta() / layoutInfo.pageSize.toFloat()
 
+            // we're only interested in the decimal part of the offset.
             val offsetFromSnappedPositionOverflow =
                 offsetFromSnappedPosition - offsetFromSnappedPosition.toInt().toFloat()
 
+            // If the velocity is not high, use the positional threshold to decide where to go.
+            // This is applicable mainly when the user scrolls and lets go without flinging.
             val finalDistance = when (sign(currentVelocity)) {
                 0f -> {
                     if (offsetFromSnappedPositionOverflow.absoluteValue > snapPositionalThreshold) {
@@ -727,36 +762,37 @@
         override fun Density.calculateSnapStepSize(): Float = layoutInfo.pageSize.toFloat()
 
         override fun Density.calculateApproachOffset(initialVelocity: Float): Float {
+            debugLog { "Approach Velocity=$initialVelocity" }
             val effectivePageSizePx = pagerState.pageSize + pagerState.pageSpacing
+
+            // given this velocity, where can I go with a decay animation.
             val animationOffsetPx =
                 decayAnimationSpec.calculateTargetValue(0f, initialVelocity)
+
             val startPage = if (initialVelocity < 0) {
                 pagerState.firstVisiblePage + 1
             } else {
                 pagerState.firstVisiblePage
             }
 
-            val scrollOffset =
-                layoutInfo.visiblePagesInfo.fastFirstOrNull { it.index == startPage }?.offset ?: 0
-
             debugLog {
-                "Initial Offset=$scrollOffset " +
-                    "\nAnimation Offset=$animationOffsetPx " +
+                "\nAnimation Offset=$animationOffsetPx " +
                     "\nFling Start Page=$startPage " +
                     "\nEffective Page Size=$effectivePageSizePx"
             }
 
-            val targetOffsetPx = startPage * effectivePageSizePx + animationOffsetPx
+            // How many pages fit in the animation offset.
+            val pagesInAnimationOffset = animationOffsetPx / effectivePageSizePx
 
-            val targetPageValue = targetOffsetPx / effectivePageSizePx
-            val targetPage = if (initialVelocity > 0) {
-                ceil(targetPageValue)
-            } else {
-                floor(targetPageValue)
-            }.toInt().coerceIn(0, pagerState.pageCount)
+            debugLog { "Pages in Animation Offset=$pagesInAnimationOffset" }
+
+            // Decide where this will get us in terms of target page.
+            val targetPage =
+                (startPage + pagesInAnimationOffset.toInt()).coerceIn(0, pagerState.pageCount)
 
             debugLog { "Fling Target Page=$targetPage" }
 
+            // Apply the snap distance suggestion.
             val correctedTargetPage = pagerSnapDistance.calculateTargetPage(
                 startPage,
                 targetPage,
@@ -767,13 +803,18 @@
 
             debugLog { "Fling Corrected Target Page=$correctedTargetPage" }
 
+            // Calculate the offset with the new target page. The offset is the amount of
+            // pixels between the start page and the new target page.
             val proposedFlingOffset = (correctedTargetPage - startPage) * effectivePageSizePx
 
             debugLog { "Proposed Fling Approach Offset=$proposedFlingOffset" }
 
-            val flingApproachOffsetPx =
-                (proposedFlingOffset.absoluteValue - scrollOffset.absoluteValue).coerceAtLeast(0)
+            // We'd like the approach animation to finish right before the last page so we can
+            // use a snapping animation for the rest.
+            val flingApproachOffsetPx = (abs(proposedFlingOffset) - effectivePageSizePx)
+                .coerceAtLeast(0)
 
+            // Apply the correct sign.
             return if (flingApproachOffsetPx == 0) {
                 flingApproachOffsetPx.toFloat()
             } else {