Don't always intercept swipe transitions

This CL improves the interception logic of SceneGestureHandler so that
the started position is taken into account when deciding whether a
current transition should be intercepted rather than interrupted. See
b/322291138 for details on use-case.

This CL also removes the condition that the current swipe transition
must be animating (i.e. the user must have moved their finger up) for it
to be interceptable.

Bug: 322291138
Test: atest SceneGestureHandlerTest
Flag: N/A
Change-Id: I317241e544f2a244af5cf6610f17ca18e6e321b9

4 files changed, 163 insertions, 57 deletions

diff --git a/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/MultiPointerDraggable.kt b/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/MultiPointerDraggable.kt
index 7c4fd5a32596..5f615fdbe239 100644
--- a/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/MultiPointerDraggable.kt
+++ b/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/MultiPointerDraggable.kt
@@ -68,7 +68,7 @@ import kotlin.math.sign
 internal fun Modifier.multiPointerDraggable(
     orientation: Orientation,
     enabled: () -> Boolean,
-    startDragImmediately: () -> Boolean,
+    startDragImmediately: (startedPosition: Offset) -> Boolean,
     onDragStarted: (startedPosition: Offset, overSlop: Float, pointersDown: Int) -> Unit,
     onDragDelta: (delta: Float) -> Unit,
     onDragStopped: (velocity: Float) -> Unit,
@@ -87,7 +87,7 @@ internal fun Modifier.multiPointerDraggable(
 private data class MultiPointerDraggableElement(
     private val orientation: Orientation,
     private val enabled: () -> Boolean,
-    private val startDragImmediately: () -> Boolean,
+    private val startDragImmediately: (startedPosition: Offset) -> Boolean,
     private val onDragStarted:
         (startedPosition: Offset, overSlop: Float, pointersDown: Int) -> Unit,
     private val onDragDelta: (Float) -> Unit,
@@ -116,7 +116,7 @@ private data class MultiPointerDraggableElement(
 internal class MultiPointerDraggableNode(
     orientation: Orientation,
     enabled: () -> Boolean,
-    var startDragImmediately: () -> Boolean,
+    var startDragImmediately: (startedPosition: Offset) -> Boolean,
     var onDragStarted: (startedPosition: Offset, overSlop: Float, pointersDown: Int) -> Unit,
     var onDragDelta: (Float) -> Unit,
     var onDragStopped: (velocity: Float) -> Unit,
@@ -224,7 +224,7 @@ internal class MultiPointerDraggableNode(
  */
 private suspend fun PointerInputScope.detectDragGestures(
     orientation: Orientation,
-    startDragImmediately: () -> Boolean,
+    startDragImmediately: (startedPosition: Offset) -> Boolean,
     onDragStart: (startedPosition: Offset, overSlop: Float, pointersDown: Int) -> Unit,
     onDragEnd: () -> Unit,
     onDragCancel: () -> Unit,
@@ -234,7 +234,7 @@ private suspend fun PointerInputScope.detectDragGestures(
         val initialDown = awaitFirstDown(requireUnconsumed = false, pass = PointerEventPass.Initial)
         var overSlop = 0f
         val drag =
-            if (startDragImmediately()) {
+            if (startDragImmediately(initialDown.position)) {
                 initialDown.consume()
                 initialDown
             } else {
diff --git a/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/SceneGestureHandler.kt b/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/SceneGestureHandler.kt
index aed04f688628..8305428aa9c8 100644
--- a/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/SceneGestureHandler.kt
+++ b/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/SceneGestureHandler.kt
@@ -84,8 +84,35 @@ internal class SceneGestureHandler(
     private var upOrLeftResult: UserActionResult? = null
     private var downOrRightResult: UserActionResult? = null
 
+    /**
+     * Whether we should immediately intercept a gesture.
+     *
+     * Note: if this returns true, then [onDragStarted] will be called with overSlop equal to 0f,
+     * indicating that the transition should be intercepted.
+     */
+    internal fun shouldImmediatelyIntercept(startedPosition: Offset?): Boolean {
+        // We don't intercept the touch if we are not currently driving the transition.
+        if (!isDrivingTransition) {
+            return false
+        }
+
+        // Only intercept the current transition if one of the 2 swipes results is also a transition
+        // between the same pair of scenes.
+        val fromScene = swipeTransition._currentScene
+        val swipes = computeSwipes(fromScene, startedPosition, pointersDown = 1)
+        val (upOrLeft, downOrRight) = computeSwipesResults(fromScene, swipes)
+        return (upOrLeft != null &&
+            swipeTransition.isTransitioningBetween(fromScene.key, upOrLeft.toScene)) ||
+            (downOrRight != null &&
+                swipeTransition.isTransitioningBetween(fromScene.key, downOrRight.toScene))
+    }
+
     internal fun onDragStarted(pointersDown: Int, startedPosition: Offset?, overSlop: Float) {
-        if (isDrivingTransition) {
+        if (overSlop == 0f) {
+            check(isDrivingTransition) {
+                "onDragStarted() called while isDrivingTransition=false overSlop=0f"
+            }
+
             // This [transition] was already driving the animation: simply take over it.
             // Stop animating and start from where the current offset.
             swipeTransition.cancelOffsetAnimation()
@@ -93,9 +120,6 @@ internal class SceneGestureHandler(
             return
         }
 
-        check(overSlop != 0f) {
-            "onDragStarted() called while isDrivingTransition=false overSlop=0f"
-        }
         val transitionState = layoutState.transitionState
         if (transitionState is TransitionState.Transition) {
             // TODO(b/290184746): Better handle interruptions here if state != idle.
@@ -211,7 +235,7 @@ internal class SceneGestureHandler(
 
     private fun updateSwipesResults(fromScene: Scene) {
         val (upOrLeftResult, downOrRightResult) =
-            swipesResults(
+            computeSwipesResults(
                 fromScene,
                 this.swipes ?: error("updateSwipes() should be called before updateSwipesResults()")
             )
@@ -220,7 +244,7 @@ internal class SceneGestureHandler(
         this.downOrRightResult = downOrRightResult
     }
 
-    private fun swipesResults(
+    private fun computeSwipesResults(
         fromScene: Scene,
         swipes: Swipes
     ): Pair<UserActionResult?, UserActionResult?> {
@@ -346,6 +370,11 @@ internal class SceneGestureHandler(
             return
         }
 
+        // Important: Make sure that all the code here references the current transition when
+        // [onDragStopped] is called, otherwise the callbacks (like onAnimationCompleted()) might
+        // incorrectly finish a new transition that replaced this one.
+        val swipeTransition = this.swipeTransition
+
         fun animateTo(targetScene: Scene, targetOffset: Float) {
             // If the effective current scene changed, it should be reflected right now in the
             // current scene state, even before the settle animation is ongoing. That way all the
@@ -651,6 +680,7 @@ internal class SceneNestedScrollHandler(
         }
 
         val source = this
+        var isIntercepting = false
 
         return PriorityNestedScrollConnection(
             orientation = orientation,
@@ -658,7 +688,9 @@ internal class SceneNestedScrollHandler(
                 canChangeScene = offsetBeforeStart == 0f
 
                 val canInterceptSwipeTransition =
-                    canChangeScene && gestureHandler.isDrivingTransition && offsetAvailable != 0f
+                    canChangeScene &&
+                        offsetAvailable != 0f &&
+                        gestureHandler.shouldImmediatelyIntercept(startedPosition = null)
                 if (!canInterceptSwipeTransition) return@PriorityNestedScrollConnection false
 
                 val swipeTransition = gestureHandler.swipeTransition
@@ -676,7 +708,12 @@ internal class SceneNestedScrollHandler(
                 }
 
                 // Start only if we cannot consume this event
-                !shouldSnapToIdle
+                val canStart = !shouldSnapToIdle
+                if (canStart) {
+                    isIntercepting = true
+                }
+
+                canStart
             },
             canStartPostScroll = { offsetAvailable, offsetBeforeStart ->
                 val behavior: NestedScrollBehavior =
@@ -688,24 +725,31 @@ internal class SceneNestedScrollHandler(
 
                 val isZeroOffset = offsetBeforeStart == 0f
 
-                when (behavior) {
-                    NestedScrollBehavior.DuringTransitionBetweenScenes -> {
-                        canChangeScene = false // unused: added for consistency
-                        false
-                    }
-                    NestedScrollBehavior.EdgeNoPreview -> {
-                        canChangeScene = isZeroOffset
-                        isZeroOffset && hasNextScene(offsetAvailable)
-                    }
-                    NestedScrollBehavior.EdgeWithPreview -> {
-                        canChangeScene = isZeroOffset
-                        hasNextScene(offsetAvailable)
-                    }
-                    NestedScrollBehavior.EdgeAlways -> {
-                        canChangeScene = true
-                        hasNextScene(offsetAvailable)
+                val canStart =
+                    when (behavior) {
+                        NestedScrollBehavior.DuringTransitionBetweenScenes -> {
+                            canChangeScene = false // unused: added for consistency
+                            false
+                        }
+                        NestedScrollBehavior.EdgeNoPreview -> {
+                            canChangeScene = isZeroOffset
+                            isZeroOffset && hasNextScene(offsetAvailable)
+                        }
+                        NestedScrollBehavior.EdgeWithPreview -> {
+                            canChangeScene = isZeroOffset
+                            hasNextScene(offsetAvailable)
+                        }
+                        NestedScrollBehavior.EdgeAlways -> {
+                            canChangeScene = true
+                            hasNextScene(offsetAvailable)
+                        }
                     }
+
+                if (canStart) {
+                    isIntercepting = false
                 }
+
+                canStart
             },
             canStartPostFling = { velocityAvailable ->
                 val behavior: NestedScrollBehavior =
@@ -717,7 +761,13 @@ internal class SceneNestedScrollHandler(
 
                 // We could start an overscroll animation
                 canChangeScene = false
-                behavior.canStartOnPostFling && hasNextScene(velocityAvailable)
+
+                val canStart = behavior.canStartOnPostFling && hasNextScene(velocityAvailable)
+                if (canStart) {
+                    isIntercepting = false
+                }
+
+                canStart
             },
             canContinueScroll = { true },
             canScrollOnFling = false,
@@ -726,7 +776,7 @@ internal class SceneNestedScrollHandler(
                 gestureHandler.onDragStarted(
                     pointersDown = 1,
                     startedPosition = null,
-                    overSlop = offsetAvailable,
+                    overSlop = if (isIntercepting) 0f else offsetAvailable,
                 )
             },
             onScroll = { offsetAvailable ->
diff --git a/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/SwipeToScene.kt b/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/SwipeToScene.kt
index b9c4ac0cd006..61f497818c89 100644
--- a/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/SwipeToScene.kt
+++ b/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/SwipeToScene.kt
@@ -19,6 +19,7 @@ package com.android.compose.animation.scene
 import androidx.compose.foundation.gestures.Orientation
 import androidx.compose.runtime.Stable
 import androidx.compose.ui.Modifier
+import androidx.compose.ui.geometry.Offset
 import androidx.compose.ui.input.pointer.PointerEvent
 import androidx.compose.ui.input.pointer.PointerEventPass
 import androidx.compose.ui.node.DelegatingNode
@@ -94,13 +95,10 @@ private class SwipeToSceneNode(
         return userActions.keys.any { it is Swipe && it.direction.orientation == orientation }
     }
 
-    private fun startDragImmediately(): Boolean {
-        // Immediately start the drag if this our transition is currently animating to a scene
-        // (i.e. the user released their input pointer after swiping in this orientation) and the
-        // user can't swipe in the other direction.
-        return gestureHandler.isDrivingTransition &&
-            gestureHandler.swipeTransition.isAnimatingOffset &&
-            !canOppositeSwipe()
+    private fun startDragImmediately(startedPosition: Offset): Boolean {
+        // Immediately start the drag if the user can't swipe in the other direction and the gesture
+        // handler can intercept it.
+        return !canOppositeSwipe() && gestureHandler.shouldImmediatelyIntercept(startedPosition)
     }
 
     private fun canOppositeSwipe(): Boolean {
diff --git a/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/SceneGestureHandlerTest.kt b/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/SceneGestureHandlerTest.kt
index a1f40077ecc2..e8cc0eca0d8b 100644
--- a/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/SceneGestureHandlerTest.kt
+++ b/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/SceneGestureHandlerTest.kt
@@ -146,27 +146,39 @@ class SceneGestureHandlerTest {
             fromScene: SceneKey? = null,
             toScene: SceneKey? = null,
             progress: Float? = null,
+            isUserInputOngoing: Boolean? = null
         ) {
+            val transition = transitionState
             assertWithMessage("transitionState must be Transition")
-                .that(transitionState is Transition)
+                .that(transition is Transition)
                 .isTrue()
+            transition as Transition
+
             if (currentScene != null)
                 assertWithMessage("currentScene does not match")
-                    .that(transitionState.currentScene)
+                    .that(transition.currentScene)
                     .isEqualTo(currentScene)
+
             if (fromScene != null)
                 assertWithMessage("fromScene does not match")
-                    .that((transitionState as? Transition)?.fromScene)
+                    .that(transition.fromScene)
                     .isEqualTo(fromScene)
+
             if (toScene != null)
                 assertWithMessage("toScene does not match")
-                    .that((transitionState as? Transition)?.toScene)
+                    .that(transition.toScene)
                     .isEqualTo(toScene)
+
             if (progress != null)
                 assertWithMessage("progress does not match")
-                    .that((transitionState as? Transition)?.progress)
+                    .that(transition.progress)
                     .isWithin(0f) // returns true when comparing 0.0f with -0.0f
                     .of(progress)
+
+            if (isUserInputOngoing != null)
+                assertWithMessage("isUserInputOngoing does not match")
+                    .that(transition.isUserInputOngoing)
+                    .isEqualTo(isUserInputOngoing)
         }
     }
 
@@ -583,22 +595,22 @@ class SceneGestureHandlerTest {
 
     @Test
     fun scrollAndFling_scrollMaxInterceptable_interceptPreScrollEvents() = runGestureTest {
-        val firstScroll = (1f - transitionInterceptionThreshold - 0.0001f) * SCREEN_SIZE
-        val secondScroll = 1f
+        val firstScroll = -(1f - transitionInterceptionThreshold - 0.0001f) * SCREEN_SIZE
+        val secondScroll = -1f
 
         preScrollAfterSceneTransition(firstScroll = firstScroll, secondScroll = secondScroll)
 
-        assertTransition(progress = (firstScroll + secondScroll) / SCREEN_SIZE)
+        assertTransition(progress = -(firstScroll + secondScroll) / SCREEN_SIZE)
     }
 
     @Test
     fun scrollAndFling_scrollMoreThanInterceptable_goToIdleOnNextScene() = runGestureTest {
-        val firstScroll = (1f - transitionInterceptionThreshold + 0.0001f) * SCREEN_SIZE
-        val secondScroll = 0.01f
+        val firstScroll = -(1f - transitionInterceptionThreshold + 0.0001f) * SCREEN_SIZE
+        val secondScroll = -0.01f
 
         preScrollAfterSceneTransition(firstScroll = firstScroll, secondScroll = secondScroll)
 
-        assertIdle(SceneC)
+        assertIdle(SceneB)
     }
 
     @Test
@@ -740,29 +752,75 @@ class SceneGestureHandlerTest {
     @Test
     fun startNestedScrollWhileDragging() = runGestureTest {
         val nestedScroll = nestedScrollConnection(nestedScrollBehavior = EdgeAlways)
-        draggable.onDragStarted(overSlop = down(0.1f))
+
+        // Start a drag and then stop it, given that
+        draggable.onDragStarted(overSlop = up(0.1f))
+
         assertTransition(currentScene = SceneA)
         assertThat(progress).isEqualTo(0.1f)
 
         // now we can intercept the scroll events
-        nestedScroll.scroll(available = offsetY10)
+        nestedScroll.scroll(available = -offsetY10)
         assertThat(progress).isEqualTo(0.2f)
 
         // this should be ignored, we are scrolling now!
-        draggable.onDragStopped(velocityThreshold)
+        draggable.onDragStopped(-velocityThreshold)
         assertTransition(currentScene = SceneA)
 
-        nestedScroll.scroll(available = offsetY10)
+        nestedScroll.scroll(available = -offsetY10)
         assertThat(progress).isEqualTo(0.3f)
 
-        nestedScroll.scroll(available = offsetY10)
+        nestedScroll.scroll(available = -offsetY10)
         assertThat(progress).isEqualTo(0.4f)
 
-        nestedScroll.onPreFling(available = Velocity(0f, velocityThreshold))
-        assertTransition(currentScene = SceneC)
+        nestedScroll.onPreFling(available = Velocity(0f, -velocityThreshold))
+        assertTransition(currentScene = SceneB)
 
         // wait for the stop animation
         advanceUntilIdle()
-        assertIdle(currentScene = SceneC)
+        assertIdle(currentScene = SceneB)
+    }
+
+    @Test
+    fun interceptTransition() = runGestureTest {
+        // Start at scene C.
+        navigateToSceneC()
+
+        // Swipe up from the middle to transition to scene B.
+        val middle = Offset(SCREEN_SIZE / 2f, SCREEN_SIZE / 2f)
+        draggable.onDragStarted(startedPosition = middle, overSlop = up(0.1f))
+        assertTransition(
+            currentScene = SceneC,
+            fromScene = SceneC,
+            toScene = SceneB,
+            isUserInputOngoing = true,
+        )
+
+        val firstTransition = transitionState
+
+        // During the current gesture, start a new gesture, still in the middle of the screen. We
+        // should intercept it. Because it is intercepted, the overSlop passed to onDragStarted()
+        // should be 0f.
+        assertThat(sceneGestureHandler.shouldImmediatelyIntercept(middle)).isTrue()
+        draggable.onDragStarted(startedPosition = middle, overSlop = 0f)
+
+        // We should have intercepted the transition, so the transition should be the same object.
+        assertTransition(currentScene = SceneC, fromScene = SceneC, toScene = SceneB)
+        assertThat(transitionState).isSameInstanceAs(firstTransition)
+
+        // Start a new gesture from the bottom of the screen. Because swiping up from the bottom of
+        // C leads to scene A (and not B), the previous transitions is *not* intercepted and we
+        // instead animate from C to A.
+        val bottom = Offset(SCREEN_SIZE / 2, SCREEN_SIZE)
+        assertThat(sceneGestureHandler.shouldImmediatelyIntercept(bottom)).isFalse()
+        draggable.onDragStarted(startedPosition = bottom, overSlop = up(0.1f))
+
+        assertTransition(
+            currentScene = SceneC,
+            fromScene = SceneC,
+            toScene = SceneA,
+            isUserInputOngoing = true,
+        )
+        assertThat(transitionState).isNotSameInstanceAs(firstTransition)
     }
 }