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

Commit acc993fd authored by Jordan Demeulenaere's avatar Jordan Demeulenaere
Browse files

Fix scroll and velocity dispatching in NestedDraggable

The nested scroll and fling logic introduced in ag/30548674 was quite
wrong. This CL fixes it and adds tests to make sure that the available
scrolls and velocities we get match what we expect.

Bug: 378470603
Test: atest NestedDraggableTest
Flag: EXEMPT NestedDraggable is not used yet
Change-Id: Ib8d59768e2529ea9863ed26afea7fa67c705dce9
parent c1e7c48d
Loading
Loading
Loading
Loading
+4 −5
Original line number Diff line number Diff line
@@ -444,7 +444,7 @@ private class NestedDraggableNode(
        val left = available - consumed
        val postConsumed =
            nestedScrollDispatcher.dispatchPostScroll(
                consumed = preConsumed + consumed,
                consumed = consumed,
                available = left,
                source = NestedScrollSource.UserInput,
            )
@@ -482,10 +482,9 @@ private class NestedDraggableNode(
        val available = velocity - preConsumed
        val consumed = performFling(available)
        val left = available - consumed
        return nestedScrollDispatcher.dispatchPostFling(
            consumed = consumed + preConsumed,
            available = left,
        )
        val postConsumed =
            nestedScrollDispatcher.dispatchPostFling(consumed = consumed, available = left)
        return preConsumed + consumed + postConsumed
    }

    /*
+154 −0
Original line number Diff line number Diff line
@@ -36,6 +36,7 @@ import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.geometry.Offset
import androidx.compose.ui.input.nestedscroll.NestedScrollConnection
import androidx.compose.ui.input.nestedscroll.NestedScrollSource
import androidx.compose.ui.input.nestedscroll.nestedScroll
import androidx.compose.ui.input.pointer.PointerInputChange
import androidx.compose.ui.input.pointer.PointerType
@@ -52,6 +53,7 @@ import androidx.compose.ui.test.performMouseInput
import androidx.compose.ui.test.performTouchInput
import androidx.compose.ui.test.swipeDown
import androidx.compose.ui.test.swipeLeft
import androidx.compose.ui.test.swipeWithVelocity
import androidx.compose.ui.unit.Velocity
import com.google.common.truth.Truth.assertThat
import kotlin.math.ceil
@@ -796,6 +798,158 @@ class NestedDraggableTest(override val orientation: Orientation) : OrientationAw
        assertThat(draggable.onDragStartedCalled).isFalse()
    }

    @Test
    fun availableAndConsumedScrollDeltas() {
        val totalScroll = 200f
        val consumedByEffectPreScroll = 10f // 200f => 190f
        val consumedByConnectionPreScroll = 20f // 190f => 170f
        val consumedByScroll = 30f // 170f => 140f
        val consumedByConnectionPostScroll = 40f // 140f => 100f

        // Available scroll values that we will check later.
        var availableToEffectPreScroll = 0f
        var availableToConnectionPreScroll = 0f
        var availableToScroll = 0f
        var availableToConnectionPostScroll = 0f
        var availableToEffectPostScroll = 0f

        val effect =
            TestOverscrollEffect(
                orientation,
                onPreScroll = {
                    availableToEffectPreScroll = it
                    consumedByEffectPreScroll
                },
                onPostScroll = {
                    availableToEffectPostScroll = it
                    it
                },
            )

        val connection =
            object : NestedScrollConnection {
                override fun onPreScroll(available: Offset, source: NestedScrollSource): Offset {
                    availableToConnectionPreScroll = available.toFloat()
                    return consumedByConnectionPreScroll.toOffset()
                }

                override fun onPostScroll(
                    consumed: Offset,
                    available: Offset,
                    source: NestedScrollSource,
                ): Offset {
                    assertThat(consumed.toFloat()).isEqualTo(consumedByScroll)
                    availableToConnectionPostScroll = available.toFloat()
                    return consumedByConnectionPostScroll.toOffset()
                }
            }

        val draggable =
            TestDraggable(
                onDrag = {
                    availableToScroll = it
                    consumedByScroll
                }
            )

        val touchSlop =
            rule.setContentWithTouchSlop {
                Box(
                    Modifier.fillMaxSize()
                        .nestedScroll(connection)
                        .nestedDraggable(draggable, orientation, effect)
                )
            }

        rule.onRoot().performTouchInput {
            down(center)
            moveBy((touchSlop + totalScroll).toOffset())
        }

        assertThat(availableToEffectPreScroll).isEqualTo(200f)
        assertThat(availableToConnectionPreScroll).isEqualTo(190f)
        assertThat(availableToScroll).isEqualTo(170f)
        assertThat(availableToConnectionPostScroll).isEqualTo(140f)
        assertThat(availableToEffectPostScroll).isEqualTo(100f)
    }

    @Test
    fun availableAndConsumedVelocities() {
        val totalVelocity = 200f
        val consumedByEffectPreFling = 10f // 200f => 190f
        val consumedByConnectionPreFling = 20f // 190f => 170f
        val consumedByFling = 30f // 170f => 140f
        val consumedByConnectionPostFling = 40f // 140f => 100f

        // Available velocities that we will check later.
        var availableToEffectPreFling = 0f
        var availableToConnectionPreFling = 0f
        var availableToFling = 0f
        var availableToConnectionPostFling = 0f
        var availableToEffectPostFling = 0f

        val effect =
            TestOverscrollEffect(
                orientation,
                onPreFling = {
                    availableToEffectPreFling = it
                    consumedByEffectPreFling
                },
                onPostFling = {
                    availableToEffectPostFling = it
                    it
                },
                onPostScroll = { 0f },
            )

        val connection =
            object : NestedScrollConnection {
                override suspend fun onPreFling(available: Velocity): Velocity {
                    availableToConnectionPreFling = available.toFloat()
                    return consumedByConnectionPreFling.toVelocity()
                }

                override suspend fun onPostFling(
                    consumed: Velocity,
                    available: Velocity,
                ): Velocity {
                    assertThat(consumed.toFloat()).isEqualTo(consumedByFling)
                    availableToConnectionPostFling = available.toFloat()
                    return consumedByConnectionPostFling.toVelocity()
                }
            }

        val draggable =
            TestDraggable(
                onDragStopped = { velocity, _ ->
                    availableToFling = velocity
                    consumedByFling
                },
                onDrag = { 0f },
            )

        rule.setContent {
            Box(
                Modifier.fillMaxSize()
                    .nestedScroll(connection)
                    .nestedDraggable(draggable, orientation, effect)
            )
        }

        rule.onRoot().performTouchInput {
            when (orientation) {
                Orientation.Horizontal -> swipeWithVelocity(topLeft, topRight, totalVelocity)
                Orientation.Vertical -> swipeWithVelocity(topLeft, bottomLeft, totalVelocity)
            }
        }

        assertThat(availableToEffectPreFling).isWithin(1f).of(200f)
        assertThat(availableToConnectionPreFling).isWithin(1f).of(190f)
        assertThat(availableToFling).isWithin(1f).of(170f)
        assertThat(availableToConnectionPostFling).isWithin(1f).of(140f)
        assertThat(availableToEffectPostFling).isWithin(1f).of(100f)
    }

    private fun ComposeContentTestRule.setContentWithTouchSlop(
        content: @Composable () -> Unit
    ): Float {
+13 −7
Original line number Diff line number Diff line
@@ -24,6 +24,8 @@ import androidx.compose.ui.unit.Velocity

class TestOverscrollEffect(
    override val orientation: Orientation,
    private val onPreScroll: (Float) -> Float = { 0f },
    private val onPreFling: suspend (Float) -> Float = { 0f },
    private val onPostFling: suspend (Float) -> Float = { it },
    private val onPostScroll: (Float) -> Float,
) : OverscrollEffect, OrientationAware {
@@ -36,19 +38,23 @@ class TestOverscrollEffect(
        source: NestedScrollSource,
        performScroll: (Offset) -> Offset,
    ): Offset {
        val consumedByScroll = performScroll(delta)
        val available = delta - consumedByScroll
        val consumedByEffect = onPostScroll(available.toFloat()).toOffset()
        return consumedByScroll + consumedByEffect
        val consumedByPreScroll = onPreScroll(delta.toFloat()).toOffset()
        val availableToScroll = delta - consumedByPreScroll
        val consumedByScroll = performScroll(availableToScroll)
        val availableToPostScroll = availableToScroll - consumedByScroll
        val consumedByPostScroll = onPostScroll(availableToPostScroll.toFloat()).toOffset()
        return consumedByPreScroll + consumedByScroll + consumedByPostScroll
    }

    override suspend fun applyToFling(
        velocity: Velocity,
        performFling: suspend (Velocity) -> Velocity,
    ) {
        val consumedByFling = performFling(velocity)
        val available = velocity - consumedByFling
        onPostFling(available.toFloat())
        val consumedByPreFling = onPreFling(velocity.toFloat()).toVelocity()
        val availableToFling = velocity - consumedByPreFling
        val consumedByFling = performFling(availableToFling)
        val availableToPostFling = availableToFling - consumedByFling
        onPostFling(availableToPostFling.toFloat())
        applyToFlingDone = true
    }
}