From 1898c59269d4d8ce7a03d7131638e597e1cf5a1a Mon Sep 17 00:00:00 2001 From: omarmt Date: Thu, 6 Jun 2024 14:06:30 +0000 Subject: Refactor detectDragGestures in MultiPointerDraggable The horizontalDrag and verticalDrag methods have been replaced with their current implementation. However, these methods do not currently handle ignored events (those that are not hasDragged). Support for these events will be added in a later CL. Test: No new test, simple refactor Bug: 345434452 Flag: com.android.systemui.scene_container Change-Id: I6a0ac353bf563132fcfa33bf7100e3d9f216c53e --- .../animation/scene/MultiPointerDraggable.kt | 96 ++++++++++++++++++---- 1 file changed, 82 insertions(+), 14 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 3cc8431cd87e..780af77c20c7 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 @@ -19,8 +19,6 @@ package com.android.compose.animation.scene import androidx.compose.foundation.gestures.Orientation import androidx.compose.foundation.gestures.awaitHorizontalTouchSlopOrCancellation import androidx.compose.foundation.gestures.awaitVerticalTouchSlopOrCancellation -import androidx.compose.foundation.gestures.horizontalDrag -import androidx.compose.foundation.gestures.verticalDrag import androidx.compose.runtime.Stable import androidx.compose.ui.Modifier import androidx.compose.ui.geometry.Offset @@ -32,7 +30,9 @@ import androidx.compose.ui.input.pointer.PointerInputChange import androidx.compose.ui.input.pointer.PointerInputScope import androidx.compose.ui.input.pointer.SuspendingPointerInputModifierNode import androidx.compose.ui.input.pointer.changedToDownIgnoreConsumed +import androidx.compose.ui.input.pointer.changedToUpIgnoreConsumed import androidx.compose.ui.input.pointer.positionChange +import androidx.compose.ui.input.pointer.positionChangeIgnoreConsumed import androidx.compose.ui.input.pointer.util.VelocityTracker import androidx.compose.ui.input.pointer.util.addPointerInputChange import androidx.compose.ui.node.CompositionLocalConsumerModifierNode @@ -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.fastAll +import androidx.compose.ui.util.fastFirstOrNull import androidx.compose.ui.util.fastForEach import kotlin.coroutines.cancellation.CancellationException import kotlin.math.sign @@ -297,18 +298,14 @@ internal class MultiPointerDraggableNode( onDrag(controller, drag, overSlop) successful = - when (orientation) { - Orientation.Horizontal -> - horizontalDrag(drag.id) { - onDrag(controller, it, it.positionChange().toFloat()) - it.consume() - } - Orientation.Vertical -> - verticalDrag(drag.id) { - onDrag(controller, it, it.positionChange().toFloat()) - it.consume() - } - } + drag( + initialPointerId = drag.id, + hasDragged = { it.positionChangeIgnoreConsumed().toFloat() != 0f }, + onDrag = { + onDrag(controller, it, it.positionChange().toFloat()) + it.consume() + }, + ) } catch (t: Throwable) { onDragCancel(controller) throw t @@ -352,4 +349,75 @@ internal class MultiPointerDraggableNode( Orientation.Horizontal -> x } } + + /** + * Continues to read drag events until all pointers are up or the drag event is canceled. The + * initial pointer to use for driving the drag is [initialPointerId]. [hasDragged] passes the + * result whether a change was detected from the drag function or not. [onDrag] is called + * whenever the pointer moves and [hasDragged] returns non-zero. + * + * @return true when gesture ended with all pointers up and false when the gesture was canceled. + * + * Note: Inspired by DragGestureDetector.kt + */ + private suspend inline fun AwaitPointerEventScope.drag( + initialPointerId: PointerId, + hasDragged: (PointerInputChange) -> Boolean, + onDrag: (PointerInputChange) -> Unit, + ): Boolean { + val pointer = currentEvent.changes.fastFirstOrNull { it.id == initialPointerId } + val isPointerUp = pointer?.pressed != true + if (isPointerUp) { + return false // The pointer has already been lifted, so the gesture is canceled + } + var pointerId = initialPointerId + while (true) { + val change = awaitDragOrUp(pointerId, hasDragged) ?: return false + + if (change.isConsumed) { + return false + } + + if (change.changedToUpIgnoreConsumed()) { + return true + } + + onDrag(change) + pointerId = change.id + } + } + + /** + * Waits for a single drag in one axis, final pointer up, or all pointers are up. When + * [initialPointerId] has lifted, another pointer that is down is chosen to be the finger + * governing the drag. When the final pointer is lifted, that [PointerInputChange] is returned. + * When a drag is detected, that [PointerInputChange] is returned. A drag is only detected when + * [hasDragged] returns `true`. + * + * `null` is returned if there was an error in the pointer input stream and the pointer that was + * down was dropped before the 'up' was received. + * + * Note: Copied from DragGestureDetector.kt + */ + private suspend inline fun AwaitPointerEventScope.awaitDragOrUp( + initialPointerId: PointerId, + hasDragged: (PointerInputChange) -> Boolean, + ): PointerInputChange? { + var pointerId = initialPointerId + while (true) { + val event = awaitPointerEvent() + val dragEvent = event.changes.fastFirstOrNull { it.id == pointerId } ?: return null + if (dragEvent.changedToUpIgnoreConsumed()) { + val otherDown = event.changes.fastFirstOrNull { it.pressed } + if (otherDown == null) { + // This is the last "up" + return dragEvent + } else { + pointerId = otherDown.id + } + } else if (hasDragged(dragEvent)) { + return dragEvent + } + } + } } -- cgit v1.2.3-59-g8ed1b From a93c960d61256882d74ed7b03f671d8bc2de9af2 Mon Sep 17 00:00:00 2001 From: omarmt Date: Thu, 6 Jun 2024 16:28:21 +0000 Subject: Avoiding conflicts with multiple MultiPointerDraggables Two adjustments were necessary to accomplish this: - Initial event: During the Final phase, if the previous event has been consumed, the event that enables the drag gesture to begin is captured. - Dragging phase: Even the events that are not sent to onDrag are consumed because they are part of the same gesture. ## Initial event We are searching for an event that can be used as the starting point for the drag gesture. Our options are: - Initial: These events should never be consumed by the MultiPointerDraggable since our ancestors can consume the gesture, but we would eliminate this possibility for our descendants. - Main: These events are consumed during the drag gesture, and they are a good place to start if the previous event has not been consumed. - Final: If the previous event has been consumed, we can wait for the Main pass to finish. If none of our ancestors were interested in the event, we can wait for an unconsumed event in the Final pass. ## Dragging phase We are still dragging an object, but this event is not of interest to the caller. This event will not trigger the onDrag event, but we will consume the event to prevent another pointerInput from interrupting the current gesture just because the event was ignored. Test: atest MultiPointerDraggableTest Bug: 345434452 Flag: com.android.systemui.scene_container Change-Id: I907b231f1fd9af4a9eb744a158a298cd694586c9 --- .../animation/scene/MultiPointerDraggable.kt | 53 ++++++++-- .../animation/scene/MultiPointerDraggableTest.kt | 115 +++++++++++++++++++++ 2 files changed, 157 insertions(+), 11 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 780af77c20c7..6001f1fd6db0 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.fastAll +import androidx.compose.ui.util.fastAny import androidx.compose.ui.util.fastFirstOrNull import androidx.compose.ui.util.fastForEach import kotlin.coroutines.cancellation.CancellationException @@ -237,8 +238,23 @@ internal class MultiPointerDraggableNode( onDragCancel: (controller: DragController) -> Unit, swipeDetector: SwipeDetector, ) { - // Wait for a consumable event in [PointerEventPass.Main] pass - val consumablePointer = awaitConsumableEvent().changes.first() + val consumablePointer = + awaitConsumableEvent { + // We are searching for an event that can be used as the starting point for the + // drag gesture. Our options are: + // - Initial: These events should never be consumed by the MultiPointerDraggable + // since our ancestors can consume the gesture, but we would eliminate this + // possibility for our descendants. + // - Main: These events are consumed during the drag gesture, and they are a + // good place to start if the previous event has not been consumed. + // - Final: If the previous event has been consumed, we can wait for the Main + // pass to finish. If none of our ancestors were interested in the event, we + // can wait for an unconsumed event in the Final pass. + val previousConsumed = currentEvent.changes.fastAny { it.isConsumed } + if (previousConsumed) PointerEventPass.Final else PointerEventPass.Main + } + .changes + .first() var overSlop = 0f val drag = @@ -305,6 +321,14 @@ internal class MultiPointerDraggableNode( onDrag(controller, it, it.positionChange().toFloat()) it.consume() }, + onIgnoredEvent = { + // We are still dragging an object, but this event is not of interest to + // the caller. + // This event will not trigger the onDrag event, but we will consume the + // event to prevent another pointerInput from interrupting the current + // gesture just because the event was ignored. + it.consume() + }, ) } catch (t: Throwable) { onDragCancel(controller) @@ -319,7 +343,9 @@ internal class MultiPointerDraggableNode( } } - private suspend fun AwaitPointerEventScope.awaitConsumableEvent(): PointerEvent { + private suspend fun AwaitPointerEventScope.awaitConsumableEvent( + pass: () -> PointerEventPass, + ): PointerEvent { fun canBeConsumed(changes: List): Boolean { // All pointers must be: return changes.fastAll { @@ -334,9 +360,7 @@ internal class MultiPointerDraggableNode( var event: PointerEvent do { - // To allow the descendants with the opportunity to consume the event, we wait for it in - // the Main pass. - event = awaitPointerEvent() + event = awaitPointerEvent(pass = pass()) } while (!canBeConsumed(event.changes)) // We found a consumable event in the Main pass @@ -353,8 +377,10 @@ internal class MultiPointerDraggableNode( /** * Continues to read drag events until all pointers are up or the drag event is canceled. The * initial pointer to use for driving the drag is [initialPointerId]. [hasDragged] passes the - * result whether a change was detected from the drag function or not. [onDrag] is called - * whenever the pointer moves and [hasDragged] returns non-zero. + * result whether a change was detected from the drag function or not. + * + * Whenever the pointer moves, if [hasDragged] returns true, [onDrag] is called; otherwise, + * [onIgnoredEvent] is called. * * @return true when gesture ended with all pointers up and false when the gesture was canceled. * @@ -364,6 +390,7 @@ internal class MultiPointerDraggableNode( initialPointerId: PointerId, hasDragged: (PointerInputChange) -> Boolean, onDrag: (PointerInputChange) -> Unit, + onIgnoredEvent: (PointerInputChange) -> Unit, ): Boolean { val pointer = currentEvent.changes.fastFirstOrNull { it.id == initialPointerId } val isPointerUp = pointer?.pressed != true @@ -372,7 +399,7 @@ internal class MultiPointerDraggableNode( } var pointerId = initialPointerId while (true) { - val change = awaitDragOrUp(pointerId, hasDragged) ?: return false + val change = awaitDragOrUp(pointerId, hasDragged, onIgnoredEvent) ?: return false if (change.isConsumed) { return false @@ -392,16 +419,18 @@ internal class MultiPointerDraggableNode( * [initialPointerId] has lifted, another pointer that is down is chosen to be the finger * governing the drag. When the final pointer is lifted, that [PointerInputChange] is returned. * When a drag is detected, that [PointerInputChange] is returned. A drag is only detected when - * [hasDragged] returns `true`. + * [hasDragged] returns `true`. Events that should not be captured are passed to + * [onIgnoredEvent]. * * `null` is returned if there was an error in the pointer input stream and the pointer that was * down was dropped before the 'up' was received. * - * Note: Copied from DragGestureDetector.kt + * Note: Inspired by DragGestureDetector.kt */ private suspend inline fun AwaitPointerEventScope.awaitDragOrUp( initialPointerId: PointerId, hasDragged: (PointerInputChange) -> Boolean, + onIgnoredEvent: (PointerInputChange) -> Unit, ): PointerInputChange? { var pointerId = initialPointerId while (true) { @@ -417,6 +446,8 @@ internal class MultiPointerDraggableNode( } } else if (hasDragged(dragEvent)) { return dragEvent + } else { + onIgnoredEvent(dragEvent) } } } diff --git a/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/MultiPointerDraggableTest.kt b/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/MultiPointerDraggableTest.kt index 4bb643f8b89e..1a0740b54892 100644 --- a/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/MultiPointerDraggableTest.kt +++ b/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/MultiPointerDraggableTest.kt @@ -348,6 +348,121 @@ class MultiPointerDraggableTest { assertThat(stopped).isTrue() } + @Test + fun multiPointerDuringAnotherGestureWaitAConsumableEventAfterMainPass() { + val size = 200f + val middle = Offset(size / 2f, size / 2f) + + var verticalStarted = false + var verticalDragged = false + var verticalStopped = false + var horizontalStarted = false + var horizontalDragged = false + var horizontalStopped = false + + var touchSlop = 0f + rule.setContent { + touchSlop = LocalViewConfiguration.current.touchSlop + Box( + Modifier.size(with(LocalDensity.current) { Size(size, size).toDpSize() }) + .multiPointerDraggable( + orientation = Orientation.Vertical, + enabled = { true }, + startDragImmediately = { false }, + onDragStarted = { _, _, _ -> + verticalStarted = true + object : DragController { + override fun onDrag(delta: Float) { + verticalDragged = true + } + + override fun onStop(velocity: Float, canChangeScene: Boolean) { + verticalStopped = true + } + } + }, + ) + .multiPointerDraggable( + orientation = Orientation.Horizontal, + enabled = { true }, + startDragImmediately = { false }, + onDragStarted = { _, _, _ -> + horizontalStarted = true + object : DragController { + override fun onDrag(delta: Float) { + horizontalDragged = true + } + + override fun onStop(velocity: Float, canChangeScene: Boolean) { + horizontalStopped = true + } + } + }, + ) + ) + } + + fun startDraggingDown() { + rule.onRoot().performTouchInput { + down(middle) + moveBy(Offset(0f, touchSlop)) + } + } + + fun startDraggingRight() { + rule.onRoot().performTouchInput { + down(middle) + moveBy(Offset(touchSlop, 0f)) + } + } + + fun stopDragging() { + rule.onRoot().performTouchInput { up() } + } + + fun continueDown() { + rule.onRoot().performTouchInput { moveBy(Offset(0f, touchSlop)) } + } + + fun continueRight() { + rule.onRoot().performTouchInput { moveBy(Offset(touchSlop, 0f)) } + } + + startDraggingDown() + assertThat(verticalStarted).isTrue() + assertThat(verticalDragged).isTrue() + assertThat(verticalStopped).isFalse() + + // Ignore right swipe, do not interrupt the dragging gesture. + continueRight() + assertThat(horizontalStarted).isFalse() + assertThat(horizontalDragged).isFalse() + assertThat(horizontalStopped).isFalse() + assertThat(verticalStopped).isFalse() + + stopDragging() + assertThat(verticalStopped).isTrue() + + verticalStarted = false + verticalDragged = false + verticalStopped = false + + startDraggingRight() + assertThat(horizontalStarted).isTrue() + assertThat(horizontalDragged).isTrue() + assertThat(horizontalStopped).isFalse() + + // Ignore down swipe, do not interrupt the dragging gesture. + continueDown() + assertThat(verticalStarted).isFalse() + assertThat(verticalDragged).isFalse() + assertThat(verticalStopped).isFalse() + assertThat(horizontalStopped).isFalse() + + stopDragging() + assertThat(horizontalStopped).isTrue() + } + @Test fun multiPointerSwipeDetectorInteraction() { val size = 200f -- cgit v1.2.3-59-g8ed1b