Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit e5c48c90 authored by Jordan Demeulenaere's avatar Jordan Demeulenaere
Browse files

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
parent ab726ea1

packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/MultiPointerDraggable.kt

+5 −5
Original line number Diff line number Diff line
@@ -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 {

packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/SceneGestureHandler.kt

+76 −26
Original line number Diff line number Diff line
@@ -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,6 +725,7 @@ internal class SceneNestedScrollHandler( 


                val isZeroOffset = offsetBeforeStart == 0f



                val canStart =

                    when (behavior) {

                        NestedScrollBehavior.DuringTransitionBetweenScenes -> {

                            canChangeScene = false // unused: added for consistency
@@ -706,6 +744,12 @@ internal class SceneNestedScrollHandler( 
                            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 ->

packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/SwipeToScene.kt

+5 −7
Original line number Diff line number Diff line
@@ -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 {

packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/SceneGestureHandlerTest.kt

+77 −19
Original line number Diff line number Diff line
@@ -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)

    }

}