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

Commit 0fd55d7e authored by Jordan Demeulenaere's avatar Jordan Demeulenaere Committed by Android (Google) Code Review
Browse files

Merge "Don't always intercept swipe transitions" into main

parents 4e07be21 e5c48c90
Loading
Loading
Loading
Loading
+5 −5
Original line number Original line Diff line number Diff line
@@ -68,7 +68,7 @@ import kotlin.math.sign
internal fun Modifier.multiPointerDraggable(
internal fun Modifier.multiPointerDraggable(
    orientation: Orientation,
    orientation: Orientation,
    enabled: () -> Boolean,
    enabled: () -> Boolean,
    startDragImmediately: () -> Boolean,
    startDragImmediately: (startedPosition: Offset) -> Boolean,
    onDragStarted: (startedPosition: Offset, overSlop: Float, pointersDown: Int) -> Unit,
    onDragStarted: (startedPosition: Offset, overSlop: Float, pointersDown: Int) -> Unit,
    onDragDelta: (delta: Float) -> Unit,
    onDragDelta: (delta: Float) -> Unit,
    onDragStopped: (velocity: Float) -> Unit,
    onDragStopped: (velocity: Float) -> Unit,
@@ -87,7 +87,7 @@ internal fun Modifier.multiPointerDraggable(
private data class MultiPointerDraggableElement(
private data class MultiPointerDraggableElement(
    private val orientation: Orientation,
    private val orientation: Orientation,
    private val enabled: () -> Boolean,
    private val enabled: () -> Boolean,
    private val startDragImmediately: () -> Boolean,
    private val startDragImmediately: (startedPosition: Offset) -> Boolean,
    private val onDragStarted:
    private val onDragStarted:
        (startedPosition: Offset, overSlop: Float, pointersDown: Int) -> Unit,
        (startedPosition: Offset, overSlop: Float, pointersDown: Int) -> Unit,
    private val onDragDelta: (Float) -> Unit,
    private val onDragDelta: (Float) -> Unit,
@@ -116,7 +116,7 @@ private data class MultiPointerDraggableElement(
internal class MultiPointerDraggableNode(
internal class MultiPointerDraggableNode(
    orientation: Orientation,
    orientation: Orientation,
    enabled: () -> Boolean,
    enabled: () -> Boolean,
    var startDragImmediately: () -> Boolean,
    var startDragImmediately: (startedPosition: Offset) -> Boolean,
    var onDragStarted: (startedPosition: Offset, overSlop: Float, pointersDown: Int) -> Unit,
    var onDragStarted: (startedPosition: Offset, overSlop: Float, pointersDown: Int) -> Unit,
    var onDragDelta: (Float) -> Unit,
    var onDragDelta: (Float) -> Unit,
    var onDragStopped: (velocity: Float) -> Unit,
    var onDragStopped: (velocity: Float) -> Unit,
@@ -224,7 +224,7 @@ internal class MultiPointerDraggableNode(
 */
 */
private suspend fun PointerInputScope.detectDragGestures(
private suspend fun PointerInputScope.detectDragGestures(
    orientation: Orientation,
    orientation: Orientation,
    startDragImmediately: () -> Boolean,
    startDragImmediately: (startedPosition: Offset) -> Boolean,
    onDragStart: (startedPosition: Offset, overSlop: Float, pointersDown: Int) -> Unit,
    onDragStart: (startedPosition: Offset, overSlop: Float, pointersDown: Int) -> Unit,
    onDragEnd: () -> Unit,
    onDragEnd: () -> Unit,
    onDragCancel: () -> Unit,
    onDragCancel: () -> Unit,
@@ -234,7 +234,7 @@ private suspend fun PointerInputScope.detectDragGestures(
        val initialDown = awaitFirstDown(requireUnconsumed = false, pass = PointerEventPass.Initial)
        val initialDown = awaitFirstDown(requireUnconsumed = false, pass = PointerEventPass.Initial)
        var overSlop = 0f
        var overSlop = 0f
        val drag =
        val drag =
            if (startDragImmediately()) {
            if (startDragImmediately(initialDown.position)) {
                initialDown.consume()
                initialDown.consume()
                initialDown
                initialDown
            } else {
            } else {
+76 −26
Original line number Original line Diff line number Diff line
@@ -84,8 +84,35 @@ internal class SceneGestureHandler(
    private var upOrLeftResult: UserActionResult? = null
    private var upOrLeftResult: UserActionResult? = null
    private var downOrRightResult: 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) {
    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.
            // This [transition] was already driving the animation: simply take over it.
            // Stop animating and start from where the current offset.
            // Stop animating and start from where the current offset.
            swipeTransition.cancelOffsetAnimation()
            swipeTransition.cancelOffsetAnimation()
@@ -93,9 +120,6 @@ internal class SceneGestureHandler(
            return
            return
        }
        }


        check(overSlop != 0f) {
            "onDragStarted() called while isDrivingTransition=false overSlop=0f"
        }
        val transitionState = layoutState.transitionState
        val transitionState = layoutState.transitionState
        if (transitionState is TransitionState.Transition) {
        if (transitionState is TransitionState.Transition) {
            // TODO(b/290184746): Better handle interruptions here if state != idle.
            // TODO(b/290184746): Better handle interruptions here if state != idle.
@@ -211,7 +235,7 @@ internal class SceneGestureHandler(


    private fun updateSwipesResults(fromScene: Scene) {
    private fun updateSwipesResults(fromScene: Scene) {
        val (upOrLeftResult, downOrRightResult) =
        val (upOrLeftResult, downOrRightResult) =
            swipesResults(
            computeSwipesResults(
                fromScene,
                fromScene,
                this.swipes ?: error("updateSwipes() should be called before updateSwipesResults()")
                this.swipes ?: error("updateSwipes() should be called before updateSwipesResults()")
            )
            )
@@ -220,7 +244,7 @@ internal class SceneGestureHandler(
        this.downOrRightResult = downOrRightResult
        this.downOrRightResult = downOrRightResult
    }
    }


    private fun swipesResults(
    private fun computeSwipesResults(
        fromScene: Scene,
        fromScene: Scene,
        swipes: Swipes
        swipes: Swipes
    ): Pair<UserActionResult?, UserActionResult?> {
    ): Pair<UserActionResult?, UserActionResult?> {
@@ -346,6 +370,11 @@ internal class SceneGestureHandler(
            return
            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) {
        fun animateTo(targetScene: Scene, targetOffset: Float) {
            // If the effective current scene changed, it should be reflected right now in the
            // 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
            // current scene state, even before the settle animation is ongoing. That way all the
@@ -651,6 +680,7 @@ internal class SceneNestedScrollHandler(
        }
        }


        val source = this
        val source = this
        var isIntercepting = false


        return PriorityNestedScrollConnection(
        return PriorityNestedScrollConnection(
            orientation = orientation,
            orientation = orientation,
@@ -658,7 +688,9 @@ internal class SceneNestedScrollHandler(
                canChangeScene = offsetBeforeStart == 0f
                canChangeScene = offsetBeforeStart == 0f


                val canInterceptSwipeTransition =
                val canInterceptSwipeTransition =
                    canChangeScene && gestureHandler.isDrivingTransition && offsetAvailable != 0f
                    canChangeScene &&
                        offsetAvailable != 0f &&
                        gestureHandler.shouldImmediatelyIntercept(startedPosition = null)
                if (!canInterceptSwipeTransition) return@PriorityNestedScrollConnection false
                if (!canInterceptSwipeTransition) return@PriorityNestedScrollConnection false


                val swipeTransition = gestureHandler.swipeTransition
                val swipeTransition = gestureHandler.swipeTransition
@@ -676,7 +708,12 @@ internal class SceneNestedScrollHandler(
                }
                }


                // Start only if we cannot consume this event
                // Start only if we cannot consume this event
                !shouldSnapToIdle
                val canStart = !shouldSnapToIdle
                if (canStart) {
                    isIntercepting = true
                }

                canStart
            },
            },
            canStartPostScroll = { offsetAvailable, offsetBeforeStart ->
            canStartPostScroll = { offsetAvailable, offsetBeforeStart ->
                val behavior: NestedScrollBehavior =
                val behavior: NestedScrollBehavior =
@@ -688,6 +725,7 @@ internal class SceneNestedScrollHandler(


                val isZeroOffset = offsetBeforeStart == 0f
                val isZeroOffset = offsetBeforeStart == 0f


                val canStart =
                    when (behavior) {
                    when (behavior) {
                        NestedScrollBehavior.DuringTransitionBetweenScenes -> {
                        NestedScrollBehavior.DuringTransitionBetweenScenes -> {
                            canChangeScene = false // unused: added for consistency
                            canChangeScene = false // unused: added for consistency
@@ -706,6 +744,12 @@ internal class SceneNestedScrollHandler(
                            hasNextScene(offsetAvailable)
                            hasNextScene(offsetAvailable)
                        }
                        }
                    }
                    }

                if (canStart) {
                    isIntercepting = false
                }

                canStart
            },
            },
            canStartPostFling = { velocityAvailable ->
            canStartPostFling = { velocityAvailable ->
                val behavior: NestedScrollBehavior =
                val behavior: NestedScrollBehavior =
@@ -717,7 +761,13 @@ internal class SceneNestedScrollHandler(


                // We could start an overscroll animation
                // We could start an overscroll animation
                canChangeScene = false
                canChangeScene = false
                behavior.canStartOnPostFling && hasNextScene(velocityAvailable)

                val canStart = behavior.canStartOnPostFling && hasNextScene(velocityAvailable)
                if (canStart) {
                    isIntercepting = false
                }

                canStart
            },
            },
            canContinueScroll = { true },
            canContinueScroll = { true },
            canScrollOnFling = false,
            canScrollOnFling = false,
@@ -726,7 +776,7 @@ internal class SceneNestedScrollHandler(
                gestureHandler.onDragStarted(
                gestureHandler.onDragStarted(
                    pointersDown = 1,
                    pointersDown = 1,
                    startedPosition = null,
                    startedPosition = null,
                    overSlop = offsetAvailable,
                    overSlop = if (isIntercepting) 0f else offsetAvailable,
                )
                )
            },
            },
            onScroll = { offsetAvailable ->
            onScroll = { offsetAvailable ->
+5 −7
Original line number Original line Diff line number Diff line
@@ -19,6 +19,7 @@ package com.android.compose.animation.scene
import androidx.compose.foundation.gestures.Orientation
import androidx.compose.foundation.gestures.Orientation
import androidx.compose.runtime.Stable
import androidx.compose.runtime.Stable
import androidx.compose.ui.Modifier
import androidx.compose.ui.Modifier
import androidx.compose.ui.geometry.Offset
import androidx.compose.ui.input.pointer.PointerEvent
import androidx.compose.ui.input.pointer.PointerEvent
import androidx.compose.ui.input.pointer.PointerEventPass
import androidx.compose.ui.input.pointer.PointerEventPass
import androidx.compose.ui.node.DelegatingNode
import androidx.compose.ui.node.DelegatingNode
@@ -94,13 +95,10 @@ private class SwipeToSceneNode(
        return userActions.keys.any { it is Swipe && it.direction.orientation == orientation }
        return userActions.keys.any { it is Swipe && it.direction.orientation == orientation }
    }
    }


    private fun startDragImmediately(): Boolean {
    private fun startDragImmediately(startedPosition: Offset): Boolean {
        // Immediately start the drag if this our transition is currently animating to a scene
        // Immediately start the drag if the user can't swipe in the other direction and the gesture
        // (i.e. the user released their input pointer after swiping in this orientation) and the
        // handler can intercept it.
        // user can't swipe in the other direction.
        return !canOppositeSwipe() && gestureHandler.shouldImmediatelyIntercept(startedPosition)
        return gestureHandler.isDrivingTransition &&
            gestureHandler.swipeTransition.isAnimatingOffset &&
            !canOppositeSwipe()
    }
    }


    private fun canOppositeSwipe(): Boolean {
    private fun canOppositeSwipe(): Boolean {
+77 −19
Original line number Original line Diff line number Diff line
@@ -146,27 +146,39 @@ class SceneGestureHandlerTest {
            fromScene: SceneKey? = null,
            fromScene: SceneKey? = null,
            toScene: SceneKey? = null,
            toScene: SceneKey? = null,
            progress: Float? = null,
            progress: Float? = null,
            isUserInputOngoing: Boolean? = null
        ) {
        ) {
            val transition = transitionState
            assertWithMessage("transitionState must be Transition")
            assertWithMessage("transitionState must be Transition")
                .that(transitionState is Transition)
                .that(transition is Transition)
                .isTrue()
                .isTrue()
            transition as Transition

            if (currentScene != null)
            if (currentScene != null)
                assertWithMessage("currentScene does not match")
                assertWithMessage("currentScene does not match")
                    .that(transitionState.currentScene)
                    .that(transition.currentScene)
                    .isEqualTo(currentScene)
                    .isEqualTo(currentScene)

            if (fromScene != null)
            if (fromScene != null)
                assertWithMessage("fromScene does not match")
                assertWithMessage("fromScene does not match")
                    .that((transitionState as? Transition)?.fromScene)
                    .that(transition.fromScene)
                    .isEqualTo(fromScene)
                    .isEqualTo(fromScene)

            if (toScene != null)
            if (toScene != null)
                assertWithMessage("toScene does not match")
                assertWithMessage("toScene does not match")
                    .that((transitionState as? Transition)?.toScene)
                    .that(transition.toScene)
                    .isEqualTo(toScene)
                    .isEqualTo(toScene)

            if (progress != null)
            if (progress != null)
                assertWithMessage("progress does not match")
                assertWithMessage("progress does not match")
                    .that((transitionState as? Transition)?.progress)
                    .that(transition.progress)
                    .isWithin(0f) // returns true when comparing 0.0f with -0.0f
                    .isWithin(0f) // returns true when comparing 0.0f with -0.0f
                    .of(progress)
                    .of(progress)

            if (isUserInputOngoing != null)
                assertWithMessage("isUserInputOngoing does not match")
                    .that(transition.isUserInputOngoing)
                    .isEqualTo(isUserInputOngoing)
        }
        }
    }
    }


@@ -583,22 +595,22 @@ class SceneGestureHandlerTest {


    @Test
    @Test
    fun scrollAndFling_scrollMaxInterceptable_interceptPreScrollEvents() = runGestureTest {
    fun scrollAndFling_scrollMaxInterceptable_interceptPreScrollEvents() = runGestureTest {
        val firstScroll = (1f - transitionInterceptionThreshold - 0.0001f) * SCREEN_SIZE
        val firstScroll = -(1f - transitionInterceptionThreshold - 0.0001f) * SCREEN_SIZE
        val secondScroll = 1f
        val secondScroll = -1f


        preScrollAfterSceneTransition(firstScroll = firstScroll, secondScroll = secondScroll)
        preScrollAfterSceneTransition(firstScroll = firstScroll, secondScroll = secondScroll)


        assertTransition(progress = (firstScroll + secondScroll) / SCREEN_SIZE)
        assertTransition(progress = -(firstScroll + secondScroll) / SCREEN_SIZE)
    }
    }


    @Test
    @Test
    fun scrollAndFling_scrollMoreThanInterceptable_goToIdleOnNextScene() = runGestureTest {
    fun scrollAndFling_scrollMoreThanInterceptable_goToIdleOnNextScene() = runGestureTest {
        val firstScroll = (1f - transitionInterceptionThreshold + 0.0001f) * SCREEN_SIZE
        val firstScroll = -(1f - transitionInterceptionThreshold + 0.0001f) * SCREEN_SIZE
        val secondScroll = 0.01f
        val secondScroll = -0.01f


        preScrollAfterSceneTransition(firstScroll = firstScroll, secondScroll = secondScroll)
        preScrollAfterSceneTransition(firstScroll = firstScroll, secondScroll = secondScroll)


        assertIdle(SceneC)
        assertIdle(SceneB)
    }
    }


    @Test
    @Test
@@ -740,29 +752,75 @@ class SceneGestureHandlerTest {
    @Test
    @Test
    fun startNestedScrollWhileDragging() = runGestureTest {
    fun startNestedScrollWhileDragging() = runGestureTest {
        val nestedScroll = nestedScrollConnection(nestedScrollBehavior = EdgeAlways)
        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)
        assertTransition(currentScene = SceneA)
        assertThat(progress).isEqualTo(0.1f)
        assertThat(progress).isEqualTo(0.1f)


        // now we can intercept the scroll events
        // now we can intercept the scroll events
        nestedScroll.scroll(available = offsetY10)
        nestedScroll.scroll(available = -offsetY10)
        assertThat(progress).isEqualTo(0.2f)
        assertThat(progress).isEqualTo(0.2f)


        // this should be ignored, we are scrolling now!
        // this should be ignored, we are scrolling now!
        draggable.onDragStopped(velocityThreshold)
        draggable.onDragStopped(-velocityThreshold)
        assertTransition(currentScene = SceneA)
        assertTransition(currentScene = SceneA)


        nestedScroll.scroll(available = offsetY10)
        nestedScroll.scroll(available = -offsetY10)
        assertThat(progress).isEqualTo(0.3f)
        assertThat(progress).isEqualTo(0.3f)


        nestedScroll.scroll(available = offsetY10)
        nestedScroll.scroll(available = -offsetY10)
        assertThat(progress).isEqualTo(0.4f)
        assertThat(progress).isEqualTo(0.4f)


        nestedScroll.onPreFling(available = Velocity(0f, velocityThreshold))
        nestedScroll.onPreFling(available = Velocity(0f, -velocityThreshold))
        assertTransition(currentScene = SceneC)
        assertTransition(currentScene = SceneB)


        // wait for the stop animation
        // wait for the stop animation
        advanceUntilIdle()
        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)
    }
    }
}
}