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

Commit 47f03223 authored by omarmt's avatar omarmt
Browse files

STL: Reset PriorityNestedScrollConnection on first pointer down 

Context: ag/28322124 moved the DraggableHandler from
NestedScrollToSceneNode to SwipeToSceneNode.

During this move, we overlooked the possibility of a scrollable
component unexpectedly disappearing from the composition, resulting in
the loss of some events.
The most important is "pointer up", which triggers the "onPreFling()"
call, serving as a signal to reset the tracking of the scroll consumed
by the SceneTransitionLayout children.

This could cause a flickering effect because specific
transitions might not be terminated at the appropriate time.

In this CL, we introduce a new approach to reset the tracking.
When the first pointer is detected on the screen, the tracker of the
scroll already consumed is reset.

Test: atest NestedScrollToSceneTest
Bug: 336710600
Flag: com.android.systemui.scene_container
Change-Id: If91d86531839cf16778b7818acfe8c9192ef0fa0
parent 44488a44

packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/MultiPointerDraggable.kt

+7 −2
Original line number Original line Diff line number Diff line
@@ -36,13 +36,11 @@ import androidx.compose.ui.input.pointer.positionChangeIgnoreConsumed 
import androidx.compose.ui.input.pointer.util.VelocityTracker

import androidx.compose.ui.input.pointer.util.VelocityTracker

import androidx.compose.ui.input.pointer.util.addPointerInputChange

import androidx.compose.ui.input.pointer.util.addPointerInputChange

import androidx.compose.ui.node.CompositionLocalConsumerModifierNode

import androidx.compose.ui.node.CompositionLocalConsumerModifierNode

import androidx.compose.ui.node.DelegatableNode

import androidx.compose.ui.node.DelegatingNode

import androidx.compose.ui.node.DelegatingNode

import androidx.compose.ui.node.ModifierNodeElement

import androidx.compose.ui.node.ModifierNodeElement

import androidx.compose.ui.node.ObserverModifierNode

import androidx.compose.ui.node.ObserverModifierNode

import androidx.compose.ui.node.PointerInputModifierNode

import androidx.compose.ui.node.PointerInputModifierNode

import androidx.compose.ui.node.currentValueOf

import androidx.compose.ui.node.currentValueOf

import androidx.compose.ui.node.findNearestAncestor

import androidx.compose.ui.node.observeReads

import androidx.compose.ui.node.observeReads

import androidx.compose.ui.platform.LocalViewConfiguration

import androidx.compose.ui.platform.LocalViewConfiguration

import androidx.compose.ui.unit.IntSize

import androidx.compose.ui.unit.IntSize
@@ -77,6 +75,7 @@ internal fun Modifier.multiPointerDraggable( 
    enabled: () -> Boolean,

    enabled: () -> Boolean,

    startDragImmediately: (startedPosition: Offset) -> Boolean,

    startDragImmediately: (startedPosition: Offset) -> Boolean,

    onDragStarted: (startedPosition: Offset, overSlop: Float, pointersDown: Int) -> DragController,

    onDragStarted: (startedPosition: Offset, overSlop: Float, pointersDown: Int) -> DragController,

    onFirstPointerDown: () -> Unit = {},

    swipeDetector: SwipeDetector = DefaultSwipeDetector,

    swipeDetector: SwipeDetector = DefaultSwipeDetector,

): Modifier =

): Modifier =

    this.then(

    this.then(
@@ -85,6 +84,7 @@ internal fun Modifier.multiPointerDraggable( 
            enabled,

            enabled,

            startDragImmediately,

            startDragImmediately,

            onDragStarted,

            onDragStarted,

            onFirstPointerDown,

            swipeDetector,

            swipeDetector,

        )

        )

    )

    )
@@ -95,6 +95,7 @@ private data class MultiPointerDraggableElement( 
    private val startDragImmediately: (startedPosition: Offset) -> Boolean,

    private val startDragImmediately: (startedPosition: Offset) -> Boolean,

    private val onDragStarted:

    private val onDragStarted:

        (startedPosition: Offset, overSlop: Float, pointersDown: Int) -> DragController,

        (startedPosition: Offset, overSlop: Float, pointersDown: Int) -> DragController,

    private val onFirstPointerDown: () -> Unit,

    private val swipeDetector: SwipeDetector,

    private val swipeDetector: SwipeDetector,

) : ModifierNodeElement<MultiPointerDraggableNode>() {

) : ModifierNodeElement<MultiPointerDraggableNode>() {

    override fun create(): MultiPointerDraggableNode =

    override fun create(): MultiPointerDraggableNode =
@@ -103,6 +104,7 @@ private data class MultiPointerDraggableElement( 
            enabled = enabled,

            enabled = enabled,

            startDragImmediately = startDragImmediately,

            startDragImmediately = startDragImmediately,

            onDragStarted = onDragStarted,

            onDragStarted = onDragStarted,

            onFirstPointerDown = onFirstPointerDown,

            swipeDetector = swipeDetector,

            swipeDetector = swipeDetector,

        )

        )
@@ -111,6 +113,7 @@ private data class MultiPointerDraggableElement( 
        node.enabled = enabled

        node.enabled = enabled

        node.startDragImmediately = startDragImmediately

        node.startDragImmediately = startDragImmediately

        node.onDragStarted = onDragStarted

        node.onDragStarted = onDragStarted

        node.onFirstPointerDown = onFirstPointerDown

        node.swipeDetector = swipeDetector

        node.swipeDetector = swipeDetector

    }

    }

}

}
@@ -121,6 +124,7 @@ internal class MultiPointerDraggableNode( 
    var startDragImmediately: (startedPosition: Offset) -> Boolean,

    var startDragImmediately: (startedPosition: Offset) -> Boolean,

    var onDragStarted:

    var onDragStarted:

        (startedPosition: Offset, overSlop: Float, pointersDown: Int) -> DragController,

        (startedPosition: Offset, overSlop: Float, pointersDown: Int) -> DragController,

    var onFirstPointerDown: () -> Unit,

    var swipeDetector: SwipeDetector = DefaultSwipeDetector,

    var swipeDetector: SwipeDetector = DefaultSwipeDetector,

) :

) :

    DelegatingNode(),

    DelegatingNode(),
@@ -219,6 +223,7 @@ internal class MultiPointerDraggableNode( 
                            startedPosition = null

                            startedPosition = null

                        } else if (startedPosition == null) {

                        } else if (startedPosition == null) {

                            startedPosition = pointers.first().position

                            startedPosition = pointers.first().position

                            onFirstPointerDown()

                        }

                        }

                    }

                    }

                }

                }

packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/SwipeToScene.kt

+10 −0
Original line number Original line Diff line number Diff line
@@ -64,6 +64,7 @@ private class SwipeToSceneNode( 
                enabled = ::enabled,

                enabled = ::enabled,

                startDragImmediately = ::startDragImmediately,

                startDragImmediately = ::startDragImmediately,

                onDragStarted = draggableHandler::onDragStarted,

                onDragStarted = draggableHandler::onDragStarted,

                onFirstPointerDown = ::onFirstPointerDown,

                swipeDetector = swipeDetector,

                swipeDetector = swipeDetector,

            )

            )

        )

        )
@@ -97,6 +98,15 @@ private class SwipeToSceneNode( 
        delegate(ScrollBehaviorOwnerNode(draggableHandler.nestedScrollKey, nestedScrollHandlerImpl))

        delegate(ScrollBehaviorOwnerNode(draggableHandler.nestedScrollKey, nestedScrollHandlerImpl))

    }

    }





    private fun onFirstPointerDown() {

        // When we drag our finger across the screen, the NestedScrollConnection keeps track of all

        // the scroll events until we lift our finger. However, in some cases, the connection might

        // not receive the "up" event. This can lead to an incorrect initial state for the gesture.

        // To prevent this issue, we can call the reset() method when the first finger touches the

        // screen. This ensures that the NestedScrollConnection starts from a correct state.

        nestedScrollHandlerImpl.connection.reset()

    }



    override fun onDetach() {

    override fun onDetach() {

        // Make sure we reset the scroll connection when this modifier is removed from composition

        // Make sure we reset the scroll connection when this modifier is removed from composition

        nestedScrollHandlerImpl.connection.reset()

        nestedScrollHandlerImpl.connection.reset()

packages/SystemUI/compose/scene/src/com/android/compose/nestedscroll/PriorityNestedScrollConnection.kt

+5 −1
Original line number Original line Diff line number Diff line
@@ -129,7 +129,11 @@ class PriorityNestedScrollConnection( 
        return onPriorityStop(available)

        return onPriorityStop(available)

    }

    }





    /** Method to call before destroying the object or to reset the initial state. */

    /**

     * Method to call before destroying the object or to reset the initial state.

     *

     * TODO(b/303224944) This method should be removed.

     */

    fun reset() {

    fun reset() {

        // Step 3c: To ensure that an onStop is always called for every onStart.

        // Step 3c: To ensure that an onStop is always called for every onStart.

        onPriorityStop(velocity = Velocity.Zero)

        onPriorityStop(velocity = Velocity.Zero)

packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/NestedScrollToSceneTest.kt

+37 −0
Original line number Original line Diff line number Diff line
@@ -23,6 +23,9 @@ import androidx.compose.foundation.layout.Spacer 
import androidx.compose.foundation.layout.fillMaxSize

import androidx.compose.foundation.layout.fillMaxSize

import androidx.compose.foundation.layout.size

import androidx.compose.foundation.layout.size

import androidx.compose.runtime.Composable

import androidx.compose.runtime.Composable

import androidx.compose.runtime.getValue

import androidx.compose.runtime.mutableStateOf

import androidx.compose.runtime.setValue

import androidx.compose.ui.Modifier

import androidx.compose.ui.Modifier

import androidx.compose.ui.geometry.Offset

import androidx.compose.ui.geometry.Offset

import androidx.compose.ui.platform.LocalViewConfiguration

import androidx.compose.ui.platform.LocalViewConfiguration
@@ -266,4 +269,38 @@ class NestedScrollToSceneTest { 
        val transition = assertThat(state.transitionState).isTransition()

        val transition = assertThat(state.transitionState).isTransition()

        assertThat(transition).hasProgress(0.5f)

        assertThat(transition).hasProgress(0.5f)

    }

    }



    @Test

    fun resetScrollTracking_afterMissingPointerUpEvent() {

        var canScroll = true

        var hasScrollable by mutableStateOf(true)

        val state = setup2ScenesAndScrollTouchSlop {

            if (hasScrollable) {

                Modifier.scrollable(rememberScrollableState { if (canScroll) it else 0f }, Vertical)

            } else {

                Modifier

            }

        }



        // The gesture is consumed by the component in the scene.

        scrollUp(percent = 0.2f)



        // STL keeps track of the scroll consumed. The scene remains in Idle.

        assertThat(state.transitionState).isIdle()



        // The scrollable component disappears, and does not send the signal (pointer up) to reset

        // the consumed amount.

        hasScrollable = false

        pointerUp()



        // A new scrollable component appears and allows the scene to consume the scroll.

        hasScrollable = true

        canScroll = false

        pointerDownAndScrollTouchSlop()

        scrollUp(percent = 0.2f)



        // STL can only start the transition if it has reset the amount of scroll consumed.

        val transition = assertThat(state.transitionState).isTransition()

        assertThat(transition).hasProgress(0.2f)

    }

}
 
}