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

Commit ff114b1b authored by omarmt's avatar omarmt
Browse files

MultiPointerDraggable velocityTracker should keep track of all events

Inspired by the recent draggable modifier changes in Compose
aosp/3248036, the MultiPointerDraggable velocityTracker should keep
track of all events, even those that occur during touch slop.

We now use our pointerTracker() method to track the velocity of all
events during the initial pass. This change is intended to improve the
responsiveness and accuracy of the velocity tracking.
## How was it tested?
The gesture was performed using the command `adb shell uinput - <
my-recording.evemu`, as described in b/359962905#comment1.

## Test results
### Compose
A **log line** was added in the `onDragEnd` method, right after
`velocityTracker.calculateVelocity(maxVelocity)`.

#### Compose - Previous implementation
```
Fling velocity: (-387.83572, -5217.695) px/sec
Fling velocity: (-121.62777, -2772.1882) px/sec
Fling velocity: (-121.16518, -2712.5664) px/sec
Fling velocity: (-117.05918, -2687.84) px/sec
Fling velocity: (-166.02173, -3007.2937) px/sec
Fling velocity: (-115.61061, -3315.9968) px/sec
Fling velocity: (-160.37592, -2785.0784) px/sec
Fling velocity: (-265.48474, -4178.468) px/sec
```

#### Compose - New implementation
```
Fling velocity: (-725.88043, -7380.7256) px/sec
Fling velocity: (-702.4923, -7150.632) px/sec
Fling velocity: (-651.1258, -7338.5747) px/sec
Fling velocity: (-637.64764, -7095.27) px/sec
Fling velocity: (-689.847, -6968.339) px/sec
Fling velocity: (-625.7267, -7121.101) px/sec
Fling velocity: (-706.514, -7223.7437) px/sec
Fling velocity: (-641.24524, -7241.089) px/sec
```

### Android View

Tested on a `ListView`, with a **breakpoint** set at line 4293 of
`AbsListView.java` using the debugger.

The velocity was then read after fling with methods `velocityTracker
.getXVelocity(mActivePointerId)` and `velocityTracker.getXVelocity
(mActivePointerId)` using the debugger.

```
Fling velocity: (-618.7875, -6606.1953) px/sec
Fling velocity: (-715.31885, -7390.3955) px/sec
Fling velocity: (-715.31885, -7390.3955) px/sec
Fling velocity: (-715.31885, -7390.3955) px/sec
Fling velocity: (-715.31885, -7390.3955) px/sec
Fling velocity: (-715.31885, -7390.3955) px/sec
Fling velocity: (-715.31885, -7390.3955) px/sec
Fling velocity: (-715.31885, -7390.3955) px/sec
```

Test: atest MultiPointerDraggableTest
Bug: 363950493
Flag: com.android.systemui.scene_container
Change-Id: I6b22086dc345becb199a35fae6deb3bb50c95dfc
parent cad79bfe
Loading
Loading
Loading
Loading
+53 −15
Original line number Diff line number Diff line
@@ -221,21 +221,61 @@ internal class MultiPointerDraggableNode(
    private suspend fun PointerInputScope.pointerTracker() {
        val currentContext = currentCoroutineContext()
        awaitPointerEventScope {
            var velocityPointerId: PointerId? = null
            // Intercepts pointer inputs and exposes [PointersInfo], via
            // [requireAncestorPointersInfoOwner], to our descendants.
            while (currentContext.isActive) {
                // During the Initial pass, we receive the event after our ancestors.
                val pointers = awaitPointerEvent(PointerEventPass.Initial).changes
                pointersDown = pointers.countDown()
                if (pointersDown == 0) {
                    // There are no more pointers down
                val changes = awaitPointerEvent(PointerEventPass.Initial).changes
                pointersDown = changes.countDown()

                when {
                    // There are no more pointers down.
                    pointersDown == 0 -> {
                        startedPosition = null
                } else if (startedPosition == null) {
                    startedPosition = pointers.first().position

                        // This is the last pointer up
                        velocityTracker.addPointerInputChange(changes.single())
                    }

                    // The first pointer down, startedPosition was not set.
                    startedPosition == null -> {
                        val firstPointerDown = changes.single()
                        velocityPointerId = firstPointerDown.id
                        velocityTracker.resetTracking()
                        velocityTracker.addPointerInputChange(firstPointerDown)
                        startedPosition = firstPointerDown.position
                        if (enabled()) {
                            onFirstPointerDown()
                        }
                    }

                    // Changes with at least one pointer
                    else -> {
                        val pointerChange = changes.first()

                        // Assuming that the list of changes doesn't have two changes with the same
                        // id (PointerId), we can check:
                        // - If the first change has `id` equals to `velocityPointerId` (this should
                        //   always be true unless the pointer has been removed).
                        // - If it does, we've found our change event (assuming there aren't any
                        //   others changes with the same id in this PointerEvent - not checked).
                        // - If it doesn't, we can check that the change with that id isn't in first
                        //   place (which should never happen - this will crash).
                        check(
                            pointerChange.id == velocityPointerId ||
                                !changes.fastAny { it.id == velocityPointerId }
                        ) {
                            "$velocityPointerId is present, but not the first: $changes"
                        }

                        // If the previous pointer has been removed, we use the first available
                        // change to keep tracking the velocity.
                        velocityPointerId = pointerChange.id

                        velocityTracker.addPointerInputChange(pointerChange)
                    }
                }
            }
        }
    }
@@ -253,11 +293,9 @@ internal class MultiPointerDraggableNode(
                        orientation = orientation,
                        startDragImmediately = startDragImmediately,
                        onDragStart = { startedPosition, overSlop, pointersDown ->
                            velocityTracker.resetTracking()
                            onDragStarted(startedPosition, overSlop, pointersDown)
                        },
                        onDrag = { controller, change, amount ->
                            velocityTracker.addPointerInputChange(change)
                        onDrag = { controller, amount ->
                            dispatchScrollEvents(
                                availableOnPreScroll = amount,
                                onScroll = { controller.onDrag(it) },
@@ -403,7 +441,7 @@ internal class MultiPointerDraggableNode(
        startDragImmediately: (startedPosition: Offset) -> Boolean,
        onDragStart:
            (startedPosition: Offset, overSlop: Float, pointersDown: Int) -> DragController,
        onDrag: (controller: DragController, change: PointerInputChange, dragAmount: Float) -> Unit,
        onDrag: (controller: DragController, dragAmount: Float) -> Unit,
        onDragEnd: (controller: DragController) -> Unit,
        onDragCancel: (controller: DragController) -> Unit,
        swipeDetector: SwipeDetector,
@@ -482,14 +520,14 @@ internal class MultiPointerDraggableNode(

            val successful: Boolean
            try {
                onDrag(controller, drag, overSlop)
                onDrag(controller, overSlop)

                successful =
                    drag(
                        initialPointerId = drag.id,
                        hasDragged = { it.positionChangeIgnoreConsumed().toFloat() != 0f },
                        onDrag = {
                            onDrag(controller, it, it.positionChange().toFloat())
                            onDrag(controller, it.positionChange().toFloat())
                            it.consume()
                        },
                        onIgnoredEvent = {
+87 −0
Original line number Diff line number Diff line
@@ -38,12 +38,15 @@ import androidx.compose.ui.input.pointer.PointerInputChange
import androidx.compose.ui.input.pointer.pointerInput
import androidx.compose.ui.platform.LocalDensity
import androidx.compose.ui.platform.LocalViewConfiguration
import androidx.compose.ui.test.TouchInjectionScope
import androidx.compose.ui.test.junit4.createComposeRule
import androidx.compose.ui.test.onRoot
import androidx.compose.ui.test.performTouchInput
import androidx.compose.ui.unit.Density
import androidx.compose.ui.unit.Velocity
import androidx.test.ext.junit.runners.AndroidJUnit4
import com.google.common.truth.Truth.assertThat
import kotlin.properties.Delegates
import kotlinx.coroutines.coroutineScope
import kotlinx.coroutines.isActive
import org.junit.Rule
@@ -719,4 +722,88 @@ class MultiPointerDraggableTest {
        assertThat(availableOnPreFling).isEqualTo(consumedOnDragStop)
        assertThat(availableOnPostFling).isEqualTo(0f)
    }

    @Test
    fun multiPointerOnStopVelocity() {
        val size = 200f
        val middle = Offset(size / 2f, size / 2f)

        var stopped = false
        var lastVelocity = -1f
        var touchSlop = 0f
        var density: Density by Delegates.notNull()
        rule.setContent {
            touchSlop = LocalViewConfiguration.current.touchSlop
            density = LocalDensity.current
            Box(
                Modifier.size(with(density) { Size(size, size).toDpSize() })
                    .nestedScrollDispatcher()
                    .multiPointerDraggable(
                        orientation = Orientation.Vertical,
                        enabled = { true },
                        startDragImmediately = { false },
                        onDragStarted = { _, _, _ ->
                            SimpleDragController(
                                onDrag = { /* do nothing */ },
                                onStop = {
                                    stopped = true
                                    lastVelocity = it
                                },
                            )
                        },
                        dispatcher = defaultDispatcher,
                    )
            )
        }

        var eventMillis: Long by Delegates.notNull()
        rule.onRoot().performTouchInput { eventMillis = eventPeriodMillis }

        fun swipeGesture(block: TouchInjectionScope.() -> Unit) {
            stopped = false
            rule.onRoot().performTouchInput {
                down(middle)
                block()
                up()
            }
            assertThat(stopped).isEqualTo(true)
        }

        val shortDistance = touchSlop / 2f
        swipeGesture {
            moveBy(delta = Offset(0f, shortDistance), delayMillis = eventMillis)
            moveBy(delta = Offset(0f, shortDistance), delayMillis = eventMillis)
        }
        assertThat(lastVelocity).isGreaterThan(0f)
        assertThat(lastVelocity).isWithin(1f).of((shortDistance / eventMillis) * 1000f)

        val longDistance = touchSlop * 4f
        swipeGesture {
            moveBy(delta = Offset(0f, longDistance), delayMillis = eventMillis)
            moveBy(delta = Offset(0f, longDistance), delayMillis = eventMillis)
        }
        assertThat(lastVelocity).isGreaterThan(0f)
        assertThat(lastVelocity).isWithin(1f).of((longDistance / eventMillis) * 1000f)

        rule.onRoot().performTouchInput {
            down(pointerId = 0, position = middle)
            down(pointerId = 1, position = middle)
            moveBy(pointerId = 0, delta = Offset(0f, longDistance), delayMillis = eventMillis)
            moveBy(pointerId = 0, delta = Offset(0f, longDistance), delayMillis = eventMillis)
            // The velocity should be:
            // (longDistance / eventMillis) pixels/ms

            // 1 pointer left, the second one
            up(pointerId = 0)

            // After a few events the velocity should be:
            // (shortDistance / eventMillis) pixels/ms
            repeat(10) {
                moveBy(pointerId = 1, delta = Offset(0f, shortDistance), delayMillis = eventMillis)
            }
            up(pointerId = 1)
        }
        assertThat(lastVelocity).isGreaterThan(0f)
        assertThat(lastVelocity).isWithin(1f).of((shortDistance / eventMillis) * 1000f)
    }
}