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

Commit 0a31e669 authored by Jordan Demeulenaere's avatar Jordan Demeulenaere
Browse files

Ensure that OverscrollEffect.applyToFling() is called 

This CL ensures that we always call OverscrollEffect.applyToFling() when
stopping a drag, and that this is called on the nested dispatcher scope
so that the call won't be cancelled even if the draggable node is
removed.

Bug: 378470603
Test: atest NestedDraggableTest
Flag: EXEMPT new API not used anywhere yet
Change-Id: I306cac168b7b97e70a6d762c133cb22d5bdf14b4
parent 9c60e917

packages/SystemUI/compose/core/src/com/android/compose/gesture/NestedDraggable.kt

+13 −19
Original line number Diff line number Diff line
@@ -317,13 +317,9 @@ private class NestedDraggableNode( 
                }



                check(pointersDownCount > 0) { "pointersDownCount is equal to $pointersDownCount" }

                val wrappedController =

                    WrappedController(

                        coroutineScope,

                        draggable.onDragStarted(down.position, sign, pointersDownCount),

                    )

                val controller = draggable.onDragStarted(down.position, sign, pointersDownCount)

                if (overSlop != 0f) {

                    onDrag(wrappedController, drag, overSlop, velocityTracker)

                    onDrag(controller, drag, overSlop, velocityTracker)

                }



                // If a drag was started, we cancel any other drag started by a nested scrollable.
@@ -336,7 +332,7 @@ private class NestedDraggableNode( 
                    try {

                        val onDrag = { change: PointerInputChange ->

                            onDrag(

                                wrappedController,

                                controller,

                                change,

                                change.positionChange().toFloat(),

                                velocityTracker,
@@ -349,7 +345,7 @@ private class NestedDraggableNode( 
                            Orientation.Vertical -> verticalDrag(drag.id, onDrag)

                        }

                    } catch (t: Throwable) {

                        wrappedController.ensureOnDragStoppedIsCalled()

                        onDragStopped(controller, velocity = 0f)

                        throw t

                    }
@@ -359,9 +355,9 @@ private class NestedDraggableNode( 
                        velocityTracker

                            .calculateVelocity(Velocity(maxVelocity, maxVelocity))

                            .toFloat()

                    onDragStopped(wrappedController, velocity)

                    onDragStopped(controller, velocity)

                } else {

                    onDragStopped(wrappedController, velocity = 0f)

                    onDragStopped(controller, velocity = 0f)

                }

            }

        }
@@ -382,17 +378,15 @@ private class NestedDraggableNode( 
        }

    }



    private fun onDragStopped(controller: WrappedController, velocity: Float) {

        coroutineScope.launch(start = CoroutineStart.UNDISPATCHED) {

            try {

    private fun onDragStopped(controller: NestedDraggable.Controller, velocity: Float) {

        // We launch in the scope of the dispatcher so that the fling is not cancelled if this node

        // is removed right after onDragStopped() is called.

        nestedScrollDispatcher.coroutineScope.launch {

            flingWithOverscroll(velocity) { velocityFromOverscroll ->

                flingWithNestedScroll(velocityFromOverscroll) { velocityFromNestedScroll ->

                    controller.onDragStopped(velocityFromNestedScroll)

                }

            }

            } finally {

                controller.ensureOnDragStoppedIsCalled()

            }

        }

    }

packages/SystemUI/compose/core/tests/src/com/android/compose/gesture/NestedDraggableTest.kt

+39 −7
Original line number Diff line number Diff line
@@ -43,7 +43,8 @@ import androidx.compose.ui.test.swipeLeft 
import androidx.compose.ui.unit.Velocity

import com.google.common.truth.Truth.assertThat

import kotlin.math.ceil

import kotlinx.coroutines.awaitCancellation

import kotlinx.coroutines.CompletableDeferred

import kotlinx.coroutines.delay

import org.junit.Ignore

import org.junit.Rule

import org.junit.Test
@@ -63,9 +64,10 @@ class NestedDraggableTest(override val orientation: Orientation) : OrientationAw 
    @Test

    fun simpleDrag() {

        val draggable = TestDraggable()

        val effect = TestOverscrollEffect(orientation) { 0f }

        val touchSlop =

            rule.setContentWithTouchSlop {

                Box(Modifier.fillMaxSize().nestedDraggable(draggable, orientation))

                Box(Modifier.fillMaxSize().nestedDraggable(draggable, orientation, effect))

            }



        assertThat(draggable.onDragStartedCalled).isFalse()
@@ -90,6 +92,7 @@ class NestedDraggableTest(override val orientation: Orientation) : OrientationAw 


        assertThat(draggable.onDragDelta).isEqualTo(30f)

        assertThat(draggable.onDragStoppedCalled).isFalse()

        assertThat(effect.applyToFlingDone).isFalse()



        rule.onRoot().performTouchInput {

            moveBy((-15f).toOffset())
@@ -98,6 +101,7 @@ class NestedDraggableTest(override val orientation: Orientation) : OrientationAw 


        assertThat(draggable.onDragDelta).isEqualTo(15f)

        assertThat(draggable.onDragStoppedCalled).isTrue()

        assertThat(effect.applyToFlingDone).isTrue()

    }



    @Test
@@ -151,10 +155,11 @@ class NestedDraggableTest(override val orientation: Orientation) : OrientationAw 
    @Test

    fun onDragStoppedIsCalledWhenDraggableIsUpdatedAndReset() {

        val draggable = TestDraggable()

        val effect = TestOverscrollEffect(orientation) { 0f }

        var orientation by mutableStateOf(orientation)

        val touchSlop =

            rule.setContentWithTouchSlop {

                Box(Modifier.fillMaxSize().nestedDraggable(draggable, orientation))

                Box(Modifier.fillMaxSize().nestedDraggable(draggable, orientation, effect))

            }



        assertThat(draggable.onDragStartedCalled).isFalse()
@@ -166,6 +171,7 @@ class NestedDraggableTest(override val orientation: Orientation) : OrientationAw 


        assertThat(draggable.onDragStartedCalled).isTrue()

        assertThat(draggable.onDragStoppedCalled).isFalse()

        assertThat(effect.applyToFlingDone).isFalse()



        orientation =

            when (orientation) {
@@ -174,6 +180,7 @@ class NestedDraggableTest(override val orientation: Orientation) : OrientationAw 
            }

        rule.waitForIdle()

        assertThat(draggable.onDragStoppedCalled).isTrue()

        assertThat(effect.applyToFlingDone).isTrue()

    }



    @Test
@@ -211,11 +218,28 @@ class NestedDraggableTest(override val orientation: Orientation) : OrientationAw 
    @Test

    fun onDragStoppedIsCalledWhenDraggableIsRemovedDuringDrag() {

        val draggable = TestDraggable()

        val postFlingDelay = 10 * 16L

        val effect =

            TestOverscrollEffect(

                orientation,

                onPostFling = {

                    // We delay the fling so that we can check that the draggable node methods are

                    // still called until completion even when the node is removed.

                    delay(postFlingDelay)

                    it

                },

            ) {

                0f

            }

        var composeContent by mutableStateOf(true)

        val touchSlop =

            rule.setContentWithTouchSlop {

                // We add an empty nested scroll connection here from which the scope will be used

                // when dispatching the flings.

                Box(Modifier.nestedScroll(remember { object : NestedScrollConnection {} })) {

                    if (composeContent) {

                    Box(Modifier.fillMaxSize().nestedDraggable(draggable, orientation))

                        Box(Modifier.fillMaxSize().nestedDraggable(draggable, orientation, effect))

                    }

                }

            }
@@ -228,10 +252,13 @@ class NestedDraggableTest(override val orientation: Orientation) : OrientationAw 


        assertThat(draggable.onDragStartedCalled).isTrue()

        assertThat(draggable.onDragStoppedCalled).isFalse()

        assertThat(effect.applyToFlingDone).isFalse()



        composeContent = false

        rule.waitForIdle()

        rule.mainClock.advanceTimeBy(postFlingDelay)

        assertThat(draggable.onDragStoppedCalled).isTrue()

        assertThat(effect.applyToFlingDone).isTrue()

    }



    @Test
@@ -267,8 +294,10 @@ class NestedDraggableTest(override val orientation: Orientation) : OrientationAw 
    @Test

    fun onDragStoppedIsCalledWhenDraggableIsRemovedDuringFling() {

        val draggable = TestDraggable()

        val effect = TestOverscrollEffect(orientation) { 0f }

        var composeContent by mutableStateOf(true)

        var preFlingCalled = false

        val unblockPrefling = CompletableDeferred<Velocity>()

        rule.setContent {

            if (composeContent) {

                Box(
@@ -282,12 +311,12 @@ class NestedDraggableTest(override val orientation: Orientation) : OrientationAw 
                                object : NestedScrollConnection {

                                    override suspend fun onPreFling(available: Velocity): Velocity {

                                        preFlingCalled = true

                                        awaitCancellation()

                                        return unblockPrefling.await()

                                    }

                                }

                            }

                        )

                        .nestedDraggable(draggable, orientation)

                        .nestedDraggable(draggable, orientation, effect)

                )

            }

        }
@@ -304,11 +333,14 @@ class NestedDraggableTest(override val orientation: Orientation) : OrientationAw 


        assertThat(draggable.onDragStartedCalled).isTrue()

        assertThat(draggable.onDragStoppedCalled).isFalse()

        assertThat(effect.applyToFlingDone).isFalse()

        assertThat(preFlingCalled).isTrue()



        composeContent = false

        unblockPrefling.complete(Velocity.Zero)

        rule.waitForIdle()

        assertThat(draggable.onDragStoppedCalled).isTrue()

        assertThat(effect.applyToFlingDone).isTrue()

    }



    @Test

packages/SystemUI/compose/core/tests/src/com/android/compose/gesture/TestOverscrollEffect.kt

0 → 100644
+54 −0
Original line number Diff line number Diff line 
/*

 * Copyright (C) 2024 The Android Open Source Project

 *

 * Licensed under the Apache License, Version 2.0 (the "License");

 * you may not use this file except in compliance with the License.

 * You may obtain a copy of the License at

 *

 *      http://www.apache.org/licenses/LICENSE-2.0

 *

 * Unless required by applicable law or agreed to in writing, software

 * distributed under the License is distributed on an "AS IS" BASIS,

 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

 * See the License for the specific language governing permissions and

 * limitations under the License.

 */



package com.android.compose.gesture



import androidx.compose.foundation.OverscrollEffect

import androidx.compose.foundation.gestures.Orientation

import androidx.compose.ui.geometry.Offset

import androidx.compose.ui.input.nestedscroll.NestedScrollSource

import androidx.compose.ui.unit.Velocity



class TestOverscrollEffect(

    override val orientation: Orientation,

    private val onPostFling: suspend (Float) -> Float = { it },

    private val onPostScroll: (Float) -> Float,

) : OverscrollEffect, OrientationAware {

    override val isInProgress: Boolean = false

    var applyToFlingDone = false

        private set



    override fun applyToScroll(

        delta: Offset,

        source: NestedScrollSource,

        performScroll: (Offset) -> Offset,

    ): Offset {

        val consumedByScroll = performScroll(delta)

        val available = delta - consumedByScroll

        val consumedByEffect = onPostScroll(available.toFloat()).toOffset()

        return consumedByScroll + consumedByEffect

    }



    override suspend fun applyToFling(

        velocity: Velocity,

        performFling: suspend (Velocity) -> Velocity,

    ) {

        val consumedByFling = performFling(velocity)

        val available = velocity - consumedByFling

        onPostFling(available.toFloat())

        applyToFlingDone = true

    }

}