diff options
| author | 2024-01-26 10:36:13 +0100 | |
|---|---|---|
| committer | 2024-01-26 11:07:51 +0100 | |
| commit | ac9016aaf7f06e98d139fb1bfcd6d022cc41f2b4 (patch) | |
| tree | 2c0ac456d4251b3d98a5503dac9d048c1adb0bb7 | |
| parent | fb2738a273b3d4c6c392356939c510df4f5fbf98 (diff) | |
Fix regression in SceneGestureHandler
This CL fixes a crash introduced in ag/25994973: if the user drags its
finger by the exact same amount as the touch slop, then the overSlop
will be 0f and we won't be able to compute in which direction the user
is dragging. This CL makes sure that the overSlop is not 0f, unless we
are intercepting a current swipe transition.
Bug: 291053278
Test: SwipeToSceneTest
Flag: N/A
Change-Id: Ia99a388a6d177fa706cc066d4a9223b8efa919dd
2 files changed, 65 insertions, 6 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 8552aaf0ebff..436e5bf6debd 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 @@ -46,6 +46,7 @@ import androidx.compose.ui.platform.LocalViewConfiguration import androidx.compose.ui.unit.IntSize import androidx.compose.ui.unit.Velocity import androidx.compose.ui.util.fastForEach +import kotlin.math.sign /** * Make an element draggable in the given [orientation]. @@ -223,12 +224,31 @@ private suspend fun PointerInputScope.detectDragGestures( // TODO(b/291055080): Replace by await[Orientation]PointerSlopOrCancellation once // it is public. - when (orientation) { - Orientation.Horizontal -> - awaitHorizontalTouchSlopOrCancellation(down.id, onSlopReached) - Orientation.Vertical -> - awaitVerticalTouchSlopOrCancellation(down.id, onSlopReached) + val drag = + when (orientation) { + Orientation.Horizontal -> + awaitHorizontalTouchSlopOrCancellation(down.id, onSlopReached) + Orientation.Vertical -> + awaitVerticalTouchSlopOrCancellation(down.id, onSlopReached) + } + + // 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). + if (drag != null && overSlop == 0f) { + val deltaOffset = drag.position - initialDown.position + val delta = + when (orientation) { + Orientation.Horizontal -> deltaOffset.y + Orientation.Vertical -> deltaOffset.y + } + check(delta != 0f) + overSlop = delta.sign } + + drag } if (drag != null) { diff --git a/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/SwipeToSceneTest.kt b/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/SwipeToSceneTest.kt index 940335853221..560f3b7c7d53 100644 --- a/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/SwipeToSceneTest.kt +++ b/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/SwipeToSceneTest.kt @@ -74,6 +74,7 @@ class SwipeToSceneTest { mapOf( Swipe.Left to TestScenes.SceneB, Swipe.Down to TestScenes.SceneC, + Swipe.Up to TestScenes.SceneB, ), ) { Box(Modifier.fillMaxSize()) @@ -357,7 +358,7 @@ class SwipeToSceneTest { // detected as a drag event. var touchSlop = 0f - val layoutState = MutableSceneTransitionLayoutState(TestScenes.SceneA) + val layoutState = layoutState() val verticalSwipeDistance = 50.dp assertThat(verticalSwipeDistance).isNotEqualTo(LayoutHeight) @@ -392,4 +393,42 @@ class SwipeToSceneTest { assertThat(transition).isNotNull() assertThat(transition!!.progress).isEqualTo(0.5f) } + + @Test + fun swipeByTouchSlop() { + val layoutState = layoutState() + var touchSlop = 0f + rule.setContent { + touchSlop = LocalViewConfiguration.current.touchSlop + TestContent(layoutState) + } + + // Swipe down by exactly touchSlop, so that the drag overSlop is 0f. + rule.onRoot().performTouchInput { + down(middle) + moveBy(Offset(0f, touchSlop), delayMillis = 1_000) + } + + // We should still correctly compute that we are swiping down to scene C. + var transition = layoutState.currentTransition + assertThat(transition).isNotNull() + assertThat(transition?.toScene).isEqualTo(TestScenes.SceneC) + + // Release the finger, animating back to scene A. + rule.onRoot().performTouchInput { up() } + rule.waitForIdle() + assertThat(layoutState.currentTransition).isNull() + assertThat(layoutState.transitionState.currentScene).isEqualTo(TestScenes.SceneA) + + // Swipe up by exactly touchSlop, so that the drag overSlop is 0f. + rule.onRoot().performTouchInput { + down(middle) + moveBy(Offset(0f, -touchSlop), delayMillis = 1_000) + } + + // We should still correctly compute that we are swiping up to scene B. + transition = layoutState.currentTransition + assertThat(transition).isNotNull() + assertThat(transition?.toScene).isEqualTo(TestScenes.SceneB) + } } |