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

Commit 206793ff authored by Mike Schneider's avatar Mike Schneider
Browse files

Keep animation loop running while there are input/state changes 

Previously, withFrameNanos was only guaranteed to run frames
back-to-back while there was an animation running (derived from
`!isStable`). Otherwise, the loop might have been suspended in
`wakeupChannel.receive()` while in practice it was not.

This change has multiple implications

# No side-effects until the beginning of the next frame
Deferring the update of the "last*" state to the beginning of the new
frame ensures that reads of output do not trigger a re-computation.
This fixes b/391259026, since MotionValues depending on the output of
other motion values will not trigger a recomputation when they capture
the `currentInput` after a frame.

# Velocity computation
Knowing that the animation loop is active on input/state change allows
to compute a change velocity, and enables fixing b/379612686 in a
follow-up CL

# Simpler suspension
This change removes the complexity of the snapshotFlow/channel/coroutine
scheduling, keeping a tight withFrameAnimation loop while there are any
changes, and suspending while there are none. Whenever compose sees
changes in the SnapshotState, an Choreographer frame is requested
anyways, so the MotionValue does not request additional animation
frames, unless it is actually animating. With this change, there is one
extra, trailing withFrameNanos being scheduled, to determine that there
are no more changes.

Fixed: 391259026
Fixed: 383979536
Test: MotionValueTest
Flag: com.android.systemui.scene_container
Change-Id: Iaf72a5f60b51c303366fc193d98a56a4a12f6177
parent 032991b8

mechanics/src/com/android/mechanics/MotionValue.kt

+99 −95
Original line number Diff line number Diff line
@@ -28,10 +28,8 @@ import androidx.compose.runtime.snapshotFlow 
import androidx.compose.runtime.withFrameNanos

import androidx.compose.ui.util.lerp

import androidx.compose.ui.util.packFloats

import androidx.compose.ui.util.packInts

import androidx.compose.ui.util.unpackFloat1

import androidx.compose.ui.util.unpackFloat2

import androidx.compose.ui.util.unpackInt1

import com.android.mechanics.debug.DebugInspector

import com.android.mechanics.debug.FrameData

import com.android.mechanics.spec.Breakpoint
@@ -46,8 +44,7 @@ import com.android.mechanics.spring.calculateUpdatedState 
import java.util.concurrent.atomic.AtomicInteger

import kotlin.math.max

import kotlinx.coroutines.CoroutineName

import kotlinx.coroutines.channels.Channel

import kotlinx.coroutines.launch

import kotlinx.coroutines.flow.first

import kotlinx.coroutines.withContext



/**
@@ -179,112 +176,119 @@ class MotionValue( 
    suspend fun keepRunningWhile(continueRunning: MotionValue.() -> Boolean) =

        withContext(CoroutineName("MotionValue($label)")) {

            check(!isActive) { "MotionValue($label) is already running" }

            // The purpose of this implementation is to run an animation frame (via withFrameNanos)

            // whenever the input changes, or the spring is still settling, but otherwise just

            // suspend.



            // Used to suspend when no animations are running, and to wait for a wakeup signal.

            val wakeupChannel = Channel<Unit>(capacity = Channel.CONFLATED)

            isActive = true



            // `true` while the spring is settling.

            var runAnimationFrames = !isStable

            var cancellationRequested = !continueRunning.invoke(this@MotionValue)

            // These `previous*` values will be applied to the `last*` values, at the beginning

            // of the each new frame.



            val observationJob = launch {

                // TODO(b/383979536) use a SnapshotStateObserver instead

                val runAnimationFramesFlag = 1 shl 0

                val cancellationRequestFlag = 1 shl 1

            // TODO(b/397837971): Encapsulate the state in a StateRecord.

            var capturedSegment = currentSegment

            var capturedGuaranteeState = currentGuaranteeState

            var capturedAnimation = currentAnimation

            var capturedSpringState = currentSpringState

            var capturedFrameTimeNanos = currentAnimationTimeNanos

            var capturedInput = currentInput()

            var capturedGestureDragOffset = currentGestureDragOffset

            var capturedDirection = currentDirection



                snapshotFlow {

                        // observe all input values.

            try {

                debugIsAnimating = true

                while (continueRunning.invoke(this@MotionValue)) {



                        // This part of the code does not actually care about the value itself - it

                        // merely wants to know whether they changed. The hashCode is used as an

                        // proxy for this.

                        var inputHash = spec.hashCode()

                        inputHash = inputHash * 31 + currentInput().hashCode()

                        inputHash = inputHash * 31 + currentDirection.hashCode()

                        inputHash = inputHash * 31 + currentGestureDragOffset.hashCode()

                    withFrameNanos { frameTimeNanos ->

                        currentAnimationTimeNanos = frameTimeNanos



                        // The relevant things to observe here is whether `isStable` changed, or

                        // the `continueRunning()` is updated.

                        val doContinueRunning = continueRunning.invoke(this@MotionValue)

                        // With the new frame started, copy



                        lastSegment = capturedSegment

                        lastGuaranteeState = capturedGuaranteeState

                        lastAnimation = capturedAnimation

                        lastSpringState = capturedSpringState

                        lastFrameTimeNanos = capturedFrameTimeNanos

                        lastInput = capturedInput

                        lastGestureDragOffset = capturedGestureDragOffset

                    }



                    // At this point, the complete frame is done (including layout, drawing and

                    // everything else), and this MotionValue has been updated.



                        val stateFlags =

                            (if (!isStable) runAnimationFramesFlag else 0) or

                                (if (!doContinueRunning) cancellationRequestFlag else 0)

                    // Capture the `current*` MotionValue state, so that it can be applied as the

                    // `last*` state when the next frame starts. Its imperative to capture at this

                    // point already (since the input could change before the next frame starts),

                    // while at the same time not already applying the `last*` state (as this would

                    // cause a re-computation if the current state is being read before the next

                    // frame).



                        // Send both of it as packed value to avoid per-frame allocations.

                        packInts(stateFlags, inputHash)

                    var scheduleNextFrame = !isStable

                    if (capturedSegment != currentSegment) {

                        capturedSegment = currentSegment

                        scheduleNextFrame = true

                    }

                    .collect { packedState ->

                        val stateFlags = unpackInt1(packedState)

                        runAnimationFrames = (stateFlags and runAnimationFramesFlag) != 0

                        cancellationRequested = (stateFlags and cancellationRequestFlag) != 0



                        // nudge the animation runner in case its sleeping.

                        wakeupChannel.send(Unit)

                    if (capturedGuaranteeState != currentGuaranteeState) {

                        capturedGuaranteeState = currentGuaranteeState

                        scheduleNextFrame = true

                    }



                    if (capturedAnimation != currentAnimation) {

                        capturedAnimation = currentAnimation

                        scheduleNextFrame = true

                    }



            isActive = true

            try {

                while (!cancellationRequested) {

                    if (capturedSpringState != currentSpringState) {

                        capturedSpringState = currentSpringState

                        scheduleNextFrame = true

                    }



                    if (!runAnimationFrames) {

                        // While the spring does not need animation frames (its stable), wait until

                        // woken up - this can be for a single frame after an input change.

                        debugIsAnimating = false

                        wakeupChannel.receive()

                    if (capturedInput != currentInput()) {

                        capturedInput = currentInput()

                        scheduleNextFrame = true

                    }



                    debugIsAnimating = true

                    withFrameNanos { frameTimeNanos -> currentAnimationTimeNanos = frameTimeNanos }

                    if (capturedGestureDragOffset != currentGestureDragOffset) {

                        capturedGestureDragOffset = currentGestureDragOffset

                        scheduleNextFrame = true

                    }



                    if (capturedDirection != currentDirection) {

                        capturedDirection = currentDirection

                        scheduleNextFrame = true

                    }



                    capturedFrameTimeNanos = currentAnimationTimeNanos



                    // At this point, the complete frame is done (including layout, drawing and

                    // everything else). What follows next is similar what one would do in a

                    // `SideEffect`, were this composable code:

                    // If during the last frame, a new animation was started, or a new segment

                    // entered,  this state is copied over. If nothing changed, the computed

                    // `current*` state will be the same, it won't have a side effect.



                    // Capturing the state here is required since crossing a breakpoint is an

                    // event - the code has to record that this happened.



                    // Important - capture all values first, and only afterwards update the state.

                    // Interleaving read and update might trigger immediate re-computations.

                    val newSegment = currentSegment

                    val newGuaranteeState = currentGuaranteeState

                    val newAnimation = currentAnimation

                    val newSpringState = currentSpringState



                    // Capture the last frames input.

                    lastFrameTimeNanos = currentAnimationTimeNanos

                    lastInput = currentInput()

                    lastGestureDragOffset = currentGestureDragOffset

                    // Not capturing currentDirection and spec explicitly, they are included in

                    // lastSegment



                    // Update the state to the computed `current*` values

                    lastSegment = newSegment

                    lastGuaranteeState = newGuaranteeState

                    lastAnimation = newAnimation

                    lastSpringState = newSpringState

                    debugInspector?.run {

                        frame =

                            FrameData(

                                lastInput,

                                currentDirection,

                                lastGestureDragOffset,

                                lastFrameTimeNanos,

                                lastSpringState,

                                lastSegment,

                                lastAnimation,

                                capturedInput,

                                capturedDirection,

                                capturedGestureDragOffset,

                                capturedFrameTimeNanos,

                                capturedSpringState,

                                capturedSegment,

                                capturedAnimation,

                            )

                    }



                    if (scheduleNextFrame) {

                        continue

                    }



                    debugIsAnimating = false

                    snapshotFlow {

                            val wakeup =

                                !continueRunning.invoke(this@MotionValue) ||

                                    spec != capturedSegment.spec ||

                                    currentInput() != capturedInput ||

                                    currentDirection != capturedDirection ||

                                    currentGestureDragOffset != capturedGestureDragOffset

                            wakeup

                        }

                        .first { it }

                    debugIsAnimating = true

                }

            } finally {

                observationJob.cancel()

                isActive = false

                debugIsAnimating = false

            }

mechanics/tests/goldens/derivedValue_hasAnimationLifecycleOnItsOwn.json

0 → 100644
+553 −0
Original line number Diff line number Diff line 
{

  "frame_ids": [

    0,

    16,

    32,

    48,

    64,

    80,

    96,

    112,

    128,

    144,

    160,

    176,

    192,

    208,

    224,

    240,

    256,

    272,

    288,

    304,

    320,

    336,

    352,

    368,

    384

  ],

  "features": [

    {

      "name": "input",

      "type": "float",

      "data_points": [

        0,

        0.1,

        0.2,

        0.3,

        0.4,

        0.5,

        0.6,

        0.70000005,

        0.8000001,

        0.9000001,

        1,

        1,

        1,

        1,

        1,

        1,

        1,

        1,

        1,

        1,

        1,

        1,

        1,

        1,

        1

      ]

    },

    {

      "name": "gestureDirection",

      "type": "string",

      "data_points": [

        "Max",

        "Max",

        "Max",

        "Max",

        "Max",

        "Max",

        "Max",

        "Max",

        "Max",

        "Max",

        "Max",

        "Max",

        "Max",

        "Max",

        "Max",

        "Max",

        "Max",

        "Max",

        "Max",

        "Max",

        "Max",

        "Max",

        "Max",

        "Max",

        "Max"

      ]

    },

    {

      "name": "output",

      "type": "float",

      "data_points": [

        0,

        0,

        0,

        0,

        0,

        0,

        0.0696004,

        0.21705192,

        0.38261998,

        0.536185,

        0.6651724,

        0.7667498,

        0.8429822,

        0.89796525,

        0.93623626,

        0.9619781,

        0.9786911,

        0.98912483,

        0.9953385,

        1,

        1,

        1,

        1,

        1,

        1

      ]

    },

    {

      "name": "outputTarget",

      "type": "float",

      "data_points": [

        0,

        0,

        0,

        0,

        0,

        1,

        1,

        1,

        1,

        1,

        1,

        1,

        1,

        1,

        1,

        1,

        1,

        1,

        1,

        1,

        1,

        1,

        1,

        1,

        1

      ]

    },

    {

      "name": "outputSpring",

      "type": "springParameters",

      "data_points": [

        {

          "stiffness": 100000,

          "dampingRatio": 1

        },

        {

          "stiffness": 100000,

          "dampingRatio": 1

        },

        {

          "stiffness": 100000,

          "dampingRatio": 1

        },

        {

          "stiffness": 100000,

          "dampingRatio": 1

        },

        {

          "stiffness": 100000,

          "dampingRatio": 1

        },

        {

          "stiffness": 700,

          "dampingRatio": 0.9

        },

        {

          "stiffness": 700,

          "dampingRatio": 0.9

        },

        {

          "stiffness": 700,

          "dampingRatio": 0.9

        },

        {

          "stiffness": 700,

          "dampingRatio": 0.9

        },

        {

          "stiffness": 700,

          "dampingRatio": 0.9

        },

        {

          "stiffness": 700,

          "dampingRatio": 0.9

        },

        {

          "stiffness": 700,

          "dampingRatio": 0.9

        },

        {

          "stiffness": 700,

          "dampingRatio": 0.9

        },

        {

          "stiffness": 700,

          "dampingRatio": 0.9

        },

        {

          "stiffness": 700,

          "dampingRatio": 0.9

        },

        {

          "stiffness": 700,

          "dampingRatio": 0.9

        },

        {

          "stiffness": 700,

          "dampingRatio": 0.9

        },

        {

          "stiffness": 700,

          "dampingRatio": 0.9

        },

        {

          "stiffness": 700,

          "dampingRatio": 0.9

        },

        {

          "stiffness": 700,

          "dampingRatio": 0.9

        },

        {

          "stiffness": 700,

          "dampingRatio": 0.9

        },

        {

          "stiffness": 700,

          "dampingRatio": 0.9

        },

        {

          "stiffness": 700,

          "dampingRatio": 0.9

        },

        {

          "stiffness": 700,

          "dampingRatio": 0.9

        },

        {

          "stiffness": 700,

          "dampingRatio": 0.9

        }

      ]

    },

    {

      "name": "isStable",

      "type": "boolean",

      "data_points": [

        true,

        true,

        true,

        true,

        true,

        false,

        false,

        false,

        false,

        false,

        false,

        false,

        false,

        false,

        false,

        false,

        false,

        false,

        false,

        true,

        true,

        true,

        true,

        true,

        true

      ]

    },

    {

      "name": "derived-input",

      "type": "float",

      "data_points": [

        0,

        0,

        0,

        0,

        0,

        0,

        0.0696004,

        0.21705192,

        0.38261998,

        0.536185,

        0.6651724,

        0.7667498,

        0.8429822,

        0.89796525,

        0.93623626,

        0.9619781,

        0.9786911,

        0.98912483,

        0.9953385,

        1,

        1,

        1,

        1,

        1,

        1

      ]

    },

    {

      "name": "derived-gestureDirection",

      "type": "string",

      "data_points": [

        "Max",

        "Max",

        "Max",

        "Max",

        "Max",

        "Max",

        "Max",

        "Max",

        "Max",

        "Max",

        "Max",

        "Max",

        "Max",

        "Max",

        "Max",

        "Max",

        "Max",

        "Max",

        "Max",

        "Max",

        "Max",

        "Max",

        "Max",

        "Max",

        "Max"

      ]

    },

    {

      "name": "derived-output",

      "type": "float",

      "data_points": [

        1,

        1,

        1,

        1,

        1,

        1,

        1,

        1,

        1,

        0.9953138,

        0.89982635,

        0.74407,

        0.5794298,

        0.43098712,

        0.308447,

        0.21313858,

        0.1423173,

        0.091676354,

        0.056711912,

        0.0333848,

        0.01837182,

        0.009094477,

        0.003640592,

        0,

        0

      ]

    },

    {

      "name": "derived-outputTarget",

      "type": "float",

      "data_points": [

        1,

        1,

        1,

        1,

        1,

        1,

        1,

        1,

        1,

        0,

        0,

        0,

        0,

        0,

        0,

        0,

        0,

        0,

        0,

        0,

        0,

        0,

        0,

        0,

        0

      ]

    },

    {

      "name": "derived-outputSpring",

      "type": "springParameters",

      "data_points": [

        {

          "stiffness": 100000,

          "dampingRatio": 1

        },

        {

          "stiffness": 100000,

          "dampingRatio": 1

        },

        {

          "stiffness": 100000,

          "dampingRatio": 1

        },

        {

          "stiffness": 100000,

          "dampingRatio": 1

        },

        {

          "stiffness": 100000,

          "dampingRatio": 1

        },

        {

          "stiffness": 100000,

          "dampingRatio": 1

        },

        {

          "stiffness": 100000,

          "dampingRatio": 1

        },

        {

          "stiffness": 100000,

          "dampingRatio": 1

        },

        {

          "stiffness": 100000,

          "dampingRatio": 1

        },

        {

          "stiffness": 700,

          "dampingRatio": 0.9

        },

        {

          "stiffness": 700,

          "dampingRatio": 0.9

        },

        {

          "stiffness": 700,

          "dampingRatio": 0.9

        },

        {

          "stiffness": 700,

          "dampingRatio": 0.9

        },

        {

          "stiffness": 700,

          "dampingRatio": 0.9

        },

        {

          "stiffness": 700,

          "dampingRatio": 0.9

        },

        {

          "stiffness": 700,

          "dampingRatio": 0.9

        },

        {

          "stiffness": 700,

          "dampingRatio": 0.9

        },

        {

          "stiffness": 700,

          "dampingRatio": 0.9

        },

        {

          "stiffness": 700,

          "dampingRatio": 0.9

        },

        {

          "stiffness": 700,

          "dampingRatio": 0.9

        },

        {

          "stiffness": 700,

          "dampingRatio": 0.9

        },

        {

          "stiffness": 700,

          "dampingRatio": 0.9

        },

        {

          "stiffness": 700,

          "dampingRatio": 0.9

        },

        {

          "stiffness": 700,

          "dampingRatio": 0.9

        },

        {

          "stiffness": 700,

          "dampingRatio": 0.9

        }

      ]

    },

    {

      "name": "derived-isStable",

      "type": "boolean",

      "data_points": [

        true,

        true,

        true,

        true,

        true,

        true,

        true,

        true,

        true,

        false,

        false,

        false,

        false,

        false,

        false,

        false,

        false,

        false,

        false,

        false,

        false,

        false,

        false,

        true,

        true

      ]

    }

  ]

}
 
 No newline at end of file

mechanics/tests/goldens/derivedValue_reflectsInputChangeInSameFrame.json

0 → 100644
+477 −0

File added.

Preview size limit exceeded, changes collapsed.

mechanics/tests/src/com/android/mechanics/MotionValueTest.kt

+54 −4
Original line number Diff line number Diff line
@@ -26,6 +26,7 @@ import androidx.compose.ui.test.ExperimentalTestApi 
import androidx.compose.ui.test.TestMonotonicFrameClock

import androidx.compose.ui.test.junit4.createComposeRule

import androidx.test.ext.junit.runners.AndroidJUnit4

import com.android.internal.R.id.primary

import com.android.mechanics.spec.BreakpointKey

import com.android.mechanics.spec.DirectionalMotionSpec

import com.android.mechanics.spec.Guarantee
@@ -37,6 +38,7 @@ import com.android.mechanics.spec.reverseBuilder 
import com.android.mechanics.testing.DefaultSprings.matStandardDefault

import com.android.mechanics.testing.DefaultSprings.matStandardFast

import com.android.mechanics.testing.MotionValueToolkit

import com.android.mechanics.testing.MotionValueToolkit.Companion.dataPoints

import com.android.mechanics.testing.MotionValueToolkit.Companion.input

import com.android.mechanics.testing.MotionValueToolkit.Companion.isStable

import com.android.mechanics.testing.MotionValueToolkit.Companion.output
@@ -337,6 +339,46 @@ class MotionValueTest { 
        }

    }



    @Test

    fun derivedValue_reflectsInputChangeInSameFrame() {

        motion.goldenTest(

            spec = specBuilder(Mapping.Zero).toBreakpoint(0.5f).completeWith(Mapping.One),

            createDerived = { primary ->

                listOf(MotionValue.createDerived(primary, MotionSpec.Empty, label = "derived"))

            },

            verifyTimeSeries = {

                // the output of the derived value must match the primary value

                assertThat(output)

                    .containsExactlyElementsIn(dataPoints<Float>("derived-output"))

                    .inOrder()

                // and its never animated.

                assertThat(dataPoints<Float>("derived-isStable")).doesNotContain(false)

            },

        ) {

            animateValueTo(1f, changePerFrame = 0.1f)

            awaitStable()

        }

    }



    @Test

    fun derivedValue_hasAnimationLifecycleOnItsOwn() {

        motion.goldenTest(

            spec = specBuilder(Mapping.Zero).toBreakpoint(0.5f).completeWith(Mapping.One),

            createDerived = { primary ->

                listOf(

                    MotionValue.createDerived(

                        primary,

                        specBuilder(Mapping.One).toBreakpoint(0.5f).completeWith(Mapping.Zero),

                        label = "derived",

                    )

                )

            },

        ) {

            animateValueTo(1f, changePerFrame = 0.1f)

            awaitStable()

        }

    }



    @Test

    fun keepRunning_concurrentInvocationThrows() = runTestWithFrameClock { testScheduler, _ ->

        val underTest = MotionValue({ 1f }, FakeGestureContext, label = "Foo")
@@ -388,12 +430,20 @@ class MotionValueTest { 
        // Produces the frame..

        assertThat(framesCount).isEqualTo(1)

        // ... and is suspended again.

        assertThat(inspector.isAnimating).isTrue()



        rule.mainClock.advanceTimeByFrame()

        rule.awaitIdle()



        // Produces the frame..

        assertThat(framesCount).isEqualTo(2)

        // ... and is suspended again.

        assertThat(inspector.isAnimating).isFalse()



        rule.mainClock.autoAdvance = true

        rule.awaitIdle()

        // Ensure that no more frames are produced

        assertThat(framesCount).isEqualTo(1)

        assertThat(framesCount).isEqualTo(2)

    }



    @Test
@@ -449,10 +499,10 @@ class MotionValueTest { 


        rule.awaitIdle()



        // Stabilizing the spring during awaitIdle() took 176ms (obtained from looking at reference

        // Stabilizing the spring during awaitIdle() took 160ms (obtained from looking at reference

        // test runs). That time is expected to be 100% reproducible, given the starting

        // state/configuration of the spring before awaitIdle().

        assertThat(rule.mainClock.currentTime).isEqualTo(timeBeforeAutoAdvance + 176)

        assertThat(rule.mainClock.currentTime).isEqualTo(timeBeforeAutoAdvance + 160)

    }



    @Test
@@ -480,7 +530,7 @@ class MotionValueTest { 


        assertWithMessage("isActive").that(inspector.isActive).isFalse()

        assertWithMessage("isAnimating").that(inspector.isAnimating).isFalse()

        assertThat(rule.mainClock.currentTime).isEqualTo(timeBeforeStopRunning + 16)

        assertThat(rule.mainClock.currentTime).isEqualTo(timeBeforeStopRunning)

    }



    @Test

mechanics/tests/src/com/android/mechanics/testing/MotionValueToolkit.kt

+57 −20

File changed.

Preview size limit exceeded, changes collapsed.