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

Commit 32af9bf7 authored by Jordan Demeulenaere's avatar Jordan Demeulenaere
Browse files

Remove interception capabilities in STL drag handler

This CL removes the interception capabilities of SceneTransitionLayout:
we won't be able to simply put a finger down during a transition to stop
it, and we instead now have to drag the touch slop to start a new
transition. Moreover, nested scrollables now have priority even during a
transition and will consume scroll events first before sending
unconsumed events to the nested scroll connection/STL draggable.

This capability was a nice-to-have that we added when STL was introduced
from the start. It has a high cost in terms of cognitive overload when
thinking about the participation of STL in the nested scroll chain, and
the gains (if any) are not obvious. Moreover, any complexity added in
the event chaining/interceptions can introduce subtle gesture bugs that
are hard to debug. For instance, b/379847834 seems to be mostly fixed
with this change, *probably* because of the removed pre-scroll
interception. In the worst case, we can still revert back this CL should
we realize that we really need these interception capabilities.

Because we don't intercept transitions, we also don't need to support
the "accelerated swipes" anymore.

Bug: 379281707
Bug: 379847834
Test: atest PlatformSceneTransitionLayouTests
Test: Manual, played with flexiglass and Communal
Flag: com.android.systemui.scene_container
Change-Id: I3128bfd6ec0f960b8b57ffb1b55908b4cc2c2ce7
parent d5eaee40
Loading
Loading
Loading
Loading
+36 −191
Original line number Diff line number Diff line
@@ -14,8 +14,6 @@
 * limitations under the License.
 */

@file:Suppress("NOTHING_TO_INLINE")

package com.android.compose.animation.scene

import androidx.compose.foundation.gestures.Orientation
@@ -97,68 +95,11 @@ internal class DraggableHandlerImpl(
    internal val positionalThreshold
        get() = with(layoutImpl.density) { 56.dp.toPx() }

    /**
     * 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(pointersDown: PointersInfo.PointersDown?): Boolean {
        // We don't intercept the touch if we are not currently driving the transition.
        val dragController = dragController
        if (dragController?.isDrivingTransition != true) {
            return false
        }

        val swipeAnimation = dragController.swipeAnimation

        // Only intercept the current transition if one of the 2 swipes results is also a transition
        // between the same pair of contents.
        val swipes = computeSwipes(pointersDown)
        val fromContent = layoutImpl.content(swipeAnimation.currentContent)
        val (upOrLeft, downOrRight) = swipes.computeSwipesResults(fromContent)
        val currentScene = layoutImpl.state.currentScene
        val contentTransition = swipeAnimation.contentTransition
        return (upOrLeft != null &&
            contentTransition.isTransitioningBetween(
                fromContent.key,
                upOrLeft.toContent(currentScene),
            )) ||
            (downOrRight != null &&
                contentTransition.isTransitioningBetween(
                    fromContent.key,
                    downOrRight.toContent(currentScene),
                ))
    }

    override fun onDragStarted(
        pointersDown: PointersInfo.PointersDown?,
        overSlop: Float,
    ): DragController {
        if (overSlop == 0f) {
            val oldDragController = dragController
            check(oldDragController != null && oldDragController.isDrivingTransition) {
                val isActive = oldDragController?.isDrivingTransition
                "onDragStarted(overSlop=0f) requires an active dragController, but was $isActive"
            }

            // This [transition] was already driving the animation: simply take over it.
            // Stop animating and start from the current offset.
            val oldSwipeAnimation = oldDragController.swipeAnimation

            // We need to recompute the swipe results since this is a new gesture, and the
            // fromScene.userActions may have changed.
            val swipes = oldDragController.swipes
            swipes.updateSwipesResults(
                fromContent = layoutImpl.content(oldSwipeAnimation.fromContent)
            )

            // A new gesture should always create a new SwipeAnimation. This way there cannot be
            // different gestures controlling the same transition.
            val swipeAnimation = createSwipeAnimation(oldSwipeAnimation)
            return updateDragController(swipes, swipeAnimation)
        }

        check(overSlop != 0f)
        val swipes = computeSwipes(pointersDown)
        val fromContent = layoutImpl.contentForUserActions()

@@ -196,7 +137,7 @@ internal class DraggableHandlerImpl(
        return createSwipeAnimation(layoutImpl, result, isUpOrLeft, orientation)
    }

    internal fun resolveSwipeSource(startedPosition: Offset): SwipeSource.Resolved? {
    private fun resolveSwipeSource(startedPosition: Offset): SwipeSource.Resolved? {
        return layoutImpl.swipeSourceDetector.source(
            layoutSize = layoutImpl.lastSize,
            position = startedPosition.round(),
@@ -291,71 +232,25 @@ private class DragControllerImpl(
            return 0f
        }

        val toContent = swipeAnimation.toContent
        val distance = swipeAnimation.distance()
        val previousOffset = swipeAnimation.dragOffset
        val desiredOffset = previousOffset + delta

        fun hasReachedToSceneUpOrLeft() =
            distance < 0 &&
                desiredOffset <= distance &&
                swipes.upOrLeftResult?.toContent(layoutState.currentScene) == toContent

        fun hasReachedToSceneDownOrRight() =
            distance > 0 &&
                desiredOffset >= distance &&
                swipes.downOrRightResult?.toContent(layoutState.currentScene) == toContent

        // Considering accelerated swipe: Change fromContent in the case where the user quickly
        // swiped multiple times in the same direction to accelerate the transition from A => B then
        // B => C.
        //
        // TODO(b/290184746): the second drag needs to pass B to work. Add support for flinging
        //  twice before B has been reached
        val hasReachedToContent =
            swipeAnimation.currentContent == toContent &&
                (hasReachedToSceneUpOrLeft() || hasReachedToSceneDownOrRight())

        val fromContent: ContentKey
        val currentTransitionOffset: Float
        val newOffset: Float
        val consumedDelta: Float
        if (hasReachedToContent) {
            // The new transition will start from the current toContent.
            fromContent = toContent

            // The current transition is completed (we have reached the full swipe distance).
            currentTransitionOffset = distance

            // The next transition will start with the remaining offset.
            newOffset = desiredOffset - distance
            consumedDelta = delta
        } else {
            fromContent = swipeAnimation.fromContent
        val desiredProgress = swipeAnimation.computeProgress(desiredOffset)

        // Note: the distance could be negative if fromContent is above or to the left of
        // toContent.
            currentTransitionOffset =
        val newOffset =
            when {
                distance == DistanceUnspecified ||
                    swipeAnimation.contentTransition.isWithinProgressRange(desiredProgress) ->
                    desiredOffset

                distance > 0f -> desiredOffset.fastCoerceIn(0f, distance)
                else -> desiredOffset.fastCoerceIn(distance, 0f)
            }

            // If there is a new transition, we will use the same offset
            newOffset = currentTransitionOffset
            consumedDelta = newOffset - previousOffset
        }

        swipeAnimation.dragOffset = currentTransitionOffset
        val consumedDelta = newOffset - previousOffset

        if (hasReachedToContent) {
            swipes.updateSwipesResults(draggableHandler.layoutImpl.content(fromContent))
        }
        swipeAnimation.dragOffset = newOffset
        val result = swipes.findUserActionResult(directionOffset = newOffset)

        if (result == null) {
@@ -372,13 +267,12 @@ private class DragControllerImpl(

        val needNewTransition =
            !currentTransitionIrreversible &&
                (hasReachedToContent ||
                    result.toContent(layoutState.currentScene) != swipeAnimation.toContent ||
                (result.toContent(layoutState.currentScene) != swipeAnimation.toContent ||
                    result.transitionKey != swipeAnimation.contentTransition.key)

        if (needNewTransition) {
            // Make sure the current transition will finish to the right current scene.
            swipeAnimation.currentContent = fromContent
            swipeAnimation.currentContent = swipeAnimation.fromContent

            val newSwipeAnimation = draggableHandler.createSwipeAnimation(swipes, result)
            newSwipeAnimation.dragOffset = newOffset
@@ -499,7 +393,9 @@ internal class Swipes(val upOrLeft: Swipe.Resolved, val downOrRight: Swipe.Resol
    var upOrLeftResult: UserActionResult? = null
    var downOrRightResult: UserActionResult? = null

    fun computeSwipesResults(fromContent: Content): Pair<UserActionResult?, UserActionResult?> {
    private fun computeSwipesResults(
        fromContent: Content
    ): Pair<UserActionResult?, UserActionResult?> {
        val upOrLeftResult = fromContent.findActionResultBestMatch(swipe = upOrLeft)
        val downOrRightResult = fromContent.findActionResultBestMatch(swipe = downOrRight)
        return upOrLeftResult to downOrRightResult
@@ -564,48 +460,9 @@ internal class NestedScrollHandlerImpl(
                .shouldEnableSwipes(draggableHandler.orientation)
        }

        var isIntercepting = false

        return PriorityNestedScrollConnection(
            orientation = draggableHandler.orientation,
            canStartPreScroll = { offsetAvailable, offsetBeforeStart, _ ->
                val pointersDown: PointersInfo.PointersDown? =
                    when (val info = pointersInfoOwner.pointersInfo()) {
                        PointersInfo.MouseWheel -> {
                            // Do not support mouse wheel interactions
                            return@PriorityNestedScrollConnection false
                        }

                        is PointersInfo.PointersDown -> info
                        null -> null
                    }

                canChangeScene =
                    if (isExternalOverscrollGesture()) false else offsetBeforeStart == 0f

                val canInterceptSwipeTransition =
                    canChangeScene &&
                        offsetAvailable != 0f &&
                        draggableHandler.shouldImmediatelyIntercept(pointersDown)
                if (!canInterceptSwipeTransition) return@PriorityNestedScrollConnection false

                val layoutImpl = draggableHandler.layoutImpl
                val threshold = layoutImpl.transitionInterceptionThreshold
                val hasSnappedToIdle = layoutImpl.state.snapToIdleIfClose(threshold)
                if (hasSnappedToIdle) {
                    // If the current swipe transition is closed to 0f or 1f, then we want to
                    // interrupt the transition (snapping it to Idle) and scroll the list.
                    return@PriorityNestedScrollConnection false
                }

                lastPointersDown = pointersDown

                // If the current swipe transition is *not* closed to 0f or 1f, then we want the
                // scroll events to intercept the current transition to continue the scene
                // transition.
                isIntercepting = true
                true
            },
            canStartPreScroll = { _, _, _ -> false },
            canStartPostScroll = { offsetAvailable, offsetBeforeStart, _ ->
                val behavior: NestedScrollBehavior =
                    when {
@@ -629,7 +486,6 @@ internal class NestedScrollHandlerImpl(
                    }
                lastPointersDown = pointersDown

                val canStart =
                when (behavior) {
                    NestedScrollBehavior.EdgeNoPreview -> {
                        canChangeScene = isZeroOffset
@@ -646,12 +502,6 @@ internal class NestedScrollHandlerImpl(
                        shouldEnableSwipes()
                    }
                }

                if (canStart) {
                    isIntercepting = false
                }

                canStart
            },
            canStartPostFling = { velocityAvailable ->
                val behavior: NestedScrollBehavior =
@@ -676,19 +526,14 @@ internal class NestedScrollHandlerImpl(
                    }
                lastPointersDown = pointersDown

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

                canStart
                behavior.canStartOnPostFling && shouldEnableSwipes()
            },
            onStart = { firstScroll ->
                scrollController(
                    dragController =
                        draggableHandler.onDragStarted(
                            pointersDown = lastPointersDown,
                            overSlop = if (isIntercepting) 0f else firstScroll,
                            overSlop = firstScroll,
                        ),
                    canChangeScene = canChangeScene,
                    pointersInfoOwner = pointersInfoOwner,
+37 −68
Original line number Diff line number Diff line
@@ -80,7 +80,6 @@ import kotlinx.coroutines.launch
@Stable
internal fun Modifier.multiPointerDraggable(
    orientation: Orientation,
    startDragImmediately: (pointersDown: PointersInfo.PointersDown) -> Boolean,
    onDragStarted: (pointersDown: PointersInfo.PointersDown, overSlop: Float) -> DragController,
    onFirstPointerDown: () -> Unit = {},
    swipeDetector: SwipeDetector = DefaultSwipeDetector,
@@ -89,7 +88,6 @@ internal fun Modifier.multiPointerDraggable(
    this.then(
        MultiPointerDraggableElement(
            orientation,
            startDragImmediately,
            onDragStarted,
            onFirstPointerDown,
            swipeDetector,
@@ -99,7 +97,6 @@ internal fun Modifier.multiPointerDraggable(

private data class MultiPointerDraggableElement(
    private val orientation: Orientation,
    private val startDragImmediately: (pointersDown: PointersInfo.PointersDown) -> Boolean,
    private val onDragStarted:
        (pointersDown: PointersInfo.PointersDown, overSlop: Float) -> DragController,
    private val onFirstPointerDown: () -> Unit,
@@ -109,7 +106,6 @@ private data class MultiPointerDraggableElement(
    override fun create(): MultiPointerDraggableNode =
        MultiPointerDraggableNode(
            orientation = orientation,
            startDragImmediately = startDragImmediately,
            onDragStarted = onDragStarted,
            onFirstPointerDown = onFirstPointerDown,
            swipeDetector = swipeDetector,
@@ -118,7 +114,6 @@ private data class MultiPointerDraggableElement(

    override fun update(node: MultiPointerDraggableNode) {
        node.orientation = orientation
        node.startDragImmediately = startDragImmediately
        node.onDragStarted = onDragStarted
        node.onFirstPointerDown = onFirstPointerDown
        node.swipeDetector = swipeDetector
@@ -127,7 +122,6 @@ private data class MultiPointerDraggableElement(

internal class MultiPointerDraggableNode(
    orientation: Orientation,
    var startDragImmediately: (pointersDown: PointersInfo.PointersDown) -> Boolean,
    var onDragStarted: (pointersDown: PointersInfo.PointersDown, overSlop: Float) -> DragController,
    var onFirstPointerDown: () -> Unit,
    swipeDetector: SwipeDetector = DefaultSwipeDetector,
@@ -297,7 +291,6 @@ internal class MultiPointerDraggableNode(
                try {
                    detectDragGestures(
                        orientation = orientation,
                        startDragImmediately = startDragImmediately,
                        onDragStart = { pointersDown, overSlop ->
                            onDragStarted(pointersDown, overSlop)
                        },
@@ -438,13 +431,11 @@ internal class MultiPointerDraggableNode(
     * Detect drag gestures in the given [orientation].
     *
     * This function is a mix of [androidx.compose.foundation.gestures.awaitDownAndSlop] and
     * [androidx.compose.foundation.gestures.detectVerticalDragGestures] to add support for:
     * 1) starting the gesture immediately without requiring a drag >= touch slope;
     * 2) passing the number of pointers down to [onDragStart].
     * [androidx.compose.foundation.gestures.detectVerticalDragGestures] to add support for passing
     * the number of pointers down to [onDragStart].
     */
    private suspend fun AwaitPointerEventScope.detectDragGestures(
        orientation: Orientation,
        startDragImmediately: (pointersDown: PointersInfo.PointersDown) -> Boolean,
        onDragStart: (pointersDown: PointersInfo.PointersDown, overSlop: Float) -> DragController,
        onDrag: (controller: DragController, dragAmount: Float) -> Unit,
        onDragEnd: (controller: DragController) -> Unit,
@@ -470,17 +461,6 @@ internal class MultiPointerDraggableNode(
                .first()

        var overSlop = 0f
        var lastPointersDown: PointersInfo.PointersDown =
            checkNotNull(pointersInfo()) {
                "We should have pointers down, last event: $currentEvent"
            }
                as PointersInfo.PointersDown

        val drag =
            if (startDragImmediately(lastPointersDown)) {
                consumablePointer.consume()
                consumablePointer
            } else {
        val onSlopReached = { change: PointerInputChange, over: Float ->
            if (swipeDetector.detectSwipe(change)) {
                change.consume()
@@ -493,27 +473,19 @@ internal class MultiPointerDraggableNode(
        val drag =
            when (orientation) {
                Orientation.Horizontal ->
                            awaitHorizontalTouchSlopOrCancellation(
                                consumablePointer.id,
                                onSlopReached,
                            )
                    awaitHorizontalTouchSlopOrCancellation(consumablePointer.id, onSlopReached)
                Orientation.Vertical ->
                            awaitVerticalTouchSlopOrCancellation(
                                consumablePointer.id,
                                onSlopReached,
                            )
                    awaitVerticalTouchSlopOrCancellation(consumablePointer.id, onSlopReached)
            } ?: return

                lastPointersDown =
        val lastPointersDown =
            checkNotNull(pointersInfo()) {
                "We should have pointers down, last event: $currentEvent"
            }
                as PointersInfo.PointersDown
        // Make sure that overSlop is not 0f. This can happen when the user drags by exactly
        // the touch slop. However, the overSlop we pass to onDragStarted() is used to
                // compute the direction we are dragging in, so overSlop should never be 0f unless
                // we intercept an ongoing swipe transition (i.e. startDragImmediately() returned
                // true).
        // compute the direction we are dragging in, so overSlop should never be 0f.
        if (overSlop == 0f) {
            // If the user drags in the opposite direction, the delta becomes zero because
            // we return to the original point. Therefore, we should use the previous event
@@ -530,11 +502,8 @@ internal class MultiPointerDraggableNode(
            }
            overSlop = delta.sign
        }
                drag
            }

        val controller = onDragStart(lastPointersDown, overSlop)

        val successful: Boolean
        try {
            onDrag(controller, overSlop)
+0 −16
Original line number Diff line number Diff line
@@ -149,7 +149,6 @@ private class SwipeToSceneNode(
        delegate(
            MultiPointerDraggableNode(
                orientation = draggableHandler.orientation,
                startDragImmediately = ::startDragImmediately,
                onDragStarted = draggableHandler::onDragStarted,
                onFirstPointerDown = ::onFirstPointerDown,
                swipeDetector = swipeDetector,
@@ -198,21 +197,6 @@ private class SwipeToSceneNode(
    ) = multiPointerDraggableNode.onPointerEvent(pointerEvent, pass, bounds)

    override fun onCancelPointerInput() = multiPointerDraggableNode.onCancelPointerInput()

    private fun startDragImmediately(pointersDown: PointersInfo.PointersDown): Boolean {
        // Immediately start the drag if the user can't swipe in the other direction and the gesture
        // handler can intercept it.
        return !canOppositeSwipe() && draggableHandler.shouldImmediatelyIntercept(pointersDown)
    }

    private fun canOppositeSwipe(): Boolean {
        val oppositeOrientation =
            when (draggableHandler.orientation) {
                Orientation.Vertical -> Orientation.Horizontal
                Orientation.Horizontal -> Orientation.Vertical
            }
        return draggableHandler.contentForSwipes().shouldEnableSwipes(oppositeOrientation)
    }
}

/** Find the [ScrollBehaviorOwner] for the current orientation. */
+6 −242

File changed.

Preview size limit exceeded, changes collapsed.

+0 −11
Original line number Diff line number Diff line
@@ -101,7 +101,6 @@ class MultiPointerDraggableTest {
                    .thenIf(enabled) {
                        Modifier.multiPointerDraggable(
                            orientation = Orientation.Vertical,
                            startDragImmediately = { false },
                            onDragStarted = { _, _ ->
                                started = true
                                SimpleDragController(
@@ -169,8 +168,6 @@ class MultiPointerDraggableTest {
                    .nestedScrollDispatcher()
                    .multiPointerDraggable(
                        orientation = Orientation.Vertical,
                        // We want to start a drag gesture immediately
                        startDragImmediately = { true },
                        onDragStarted = { _, _ ->
                            started = true
                            SimpleDragController(
@@ -242,7 +239,6 @@ class MultiPointerDraggableTest {
                    .nestedScrollDispatcher()
                    .multiPointerDraggable(
                        orientation = Orientation.Vertical,
                        startDragImmediately = { false },
                        onDragStarted = { _, _ ->
                            started = true
                            SimpleDragController(
@@ -361,7 +357,6 @@ class MultiPointerDraggableTest {
                    .nestedScrollDispatcher()
                    .multiPointerDraggable(
                        orientation = Orientation.Vertical,
                        startDragImmediately = { false },
                        onDragStarted = { _, _ ->
                            started = true
                            SimpleDragController(
@@ -466,7 +461,6 @@ class MultiPointerDraggableTest {
                    .nestedScrollDispatcher()
                    .multiPointerDraggable(
                        orientation = Orientation.Vertical,
                        startDragImmediately = { false },
                        onDragStarted = { _, _ ->
                            verticalStarted = true
                            SimpleDragController(
@@ -478,7 +472,6 @@ class MultiPointerDraggableTest {
                    )
                    .multiPointerDraggable(
                        orientation = Orientation.Horizontal,
                        startDragImmediately = { false },
                        onDragStarted = { _, _ ->
                            horizontalStarted = true
                            SimpleDragController(
@@ -570,7 +563,6 @@ class MultiPointerDraggableTest {
                    .nestedScrollDispatcher()
                    .multiPointerDraggable(
                        orientation = Orientation.Vertical,
                        startDragImmediately = { false },
                        swipeDetector =
                            object : SwipeDetector {
                                override fun detectSwipe(change: PointerInputChange): Boolean {
@@ -636,7 +628,6 @@ class MultiPointerDraggableTest {
                    .nestedScrollDispatcher()
                    .multiPointerDraggable(
                        orientation = Orientation.Vertical,
                        startDragImmediately = { false },
                        swipeDetector =
                            object : SwipeDetector {
                                override fun detectSwipe(change: PointerInputChange): Boolean {
@@ -738,7 +729,6 @@ class MultiPointerDraggableTest {
                    .nestedScrollDispatcher()
                    .multiPointerDraggable(
                        orientation = Orientation.Vertical,
                        startDragImmediately = { false },
                        onDragStarted = { _, _ ->
                            SimpleDragController(
                                onDrag = { consumedOnDrag = it },
@@ -809,7 +799,6 @@ class MultiPointerDraggableTest {
                    .nestedScrollDispatcher()
                    .multiPointerDraggable(
                        orientation = Orientation.Vertical,
                        startDragImmediately = { false },
                        onDragStarted = { _, _ ->
                            SimpleDragController(
                                onDrag = { /* do nothing */ },