diff options
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) } } |