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

Commit 4a4d840f authored by Omar Miatello's avatar Omar Miatello
Browse files

MM: Improve state and lifecycle in reveal modifiers 

This change refactors VerticalFadeContentRevealModifier and
VerticalTactileSurfaceRevealModifier to improve clarity, performance,
and state management.

Key changes:

- **Deferred MotionValue Creation:** The `ManagedMotionValue` that
  drives the animation is now created lazily during the first lookahead
  layout pass. This ensures it's only instantiated after its
  dependencies (`lookAheadHeight` and `layoutOffsetY`) are calculated
  and ready, leading to more robust animation setup.

- **Reduced Compose State:** The `deltaY` parameter is now an immutable
  `val` within the node, removing a `MutableState` dependency. This
  simplifies the state logic, prevents unsupported updates, and reduces
  the number of state reads.

- **Improved Documentation:** Added detailed comments and KDocs to
  clarify the lifecycle of the node's internal properties. This makes
  it easier to understand when each property is initialized, when it can
  be used safely, and how the overall animation is orchestrated.

Test: Manually tested in the demo app
Bug: 392535471
Flag: com.android.systemui.scene_container
Change-Id: I18d1d38d933261e9a07601fa4bf10626d8fec106
parent e7c65076

mechanics/compose/src/com/android/mechanics/compose/modifier/VerticalFadeContentRevealModifier.kt

+32 −22
Original line number Diff line number Diff line
@@ -74,7 +74,8 @@ private data class FadeContentRevealElement( 
        )



    override fun update(node: FadeContentRevealNode) {

        node.update(motionBuilderContext = motionBuilderContext, deltaY = deltaY)

        check(node.deltaY == deltaY) { "Cannot update deltaY from ${node.deltaY} to $deltaY" }

        node.update(motionBuilderContext = motionBuilderContext)

    }



    override fun InspectorInfo.inspectableProperties() {
@@ -86,26 +87,33 @@ private data class FadeContentRevealElement( 


private class FadeContentRevealNode(

    private var motionBuilderContext: MotionBuilderContext,

    deltaY: Float,

    val deltaY: Float,

    private val label: String?,

) : DelegatingNode(), ApproachLayoutModifierNode {

    // These properties are calculated during the lookahead pass (`lookAheadMeasure`) to

    // orchestrate the reveal animation. They are guaranteed to be updated before `approachMeasure`

    // is called.

    private var lookAheadHeight by mutableFloatStateOf(Float.NaN)

    private var layoutOffsetY by mutableFloatStateOf(0f)

    private var deltaY: Float by mutableFloatStateOf(deltaY)



    private lateinit var motionDriver: MotionDriver

    // Created after the first lookahead measure, guaranteed to be created before first measure

    private var layoutOffsetY by mutableFloatStateOf(Float.NaN)

    // Created lazily upon first lookahead and disposed in `onDetach`.

    private var revealAlpha: ManagedMotionValue? = null



    fun update(motionBuilderContext: MotionBuilderContext, deltaY: Float) {

        this.motionBuilderContext = motionBuilderContext

        this.deltaY = deltaY

    }

    /**

     * The [MotionDriver] that controls the parent's motion, used to determine the reveal

     * animation's progress.

     *

     * It is initialized in `onAttach` and is safe to use in all subsequent measure passes.

     */

    private lateinit var motionDriver: MotionDriver



    override fun onAttach() {

        motionDriver = findMotionDriver()

    }



    fun update(motionBuilderContext: MotionBuilderContext) {

        this.motionBuilderContext = motionBuilderContext

    }



    override fun onDetach() {

        revealAlpha?.dispose()

    }
@@ -117,7 +125,7 @@ private class FadeContentRevealNode( 
            }

            MotionDriver.State.Transition -> {

                motionBuilderContext.effectsMotionSpec(Mapping.Zero) {

                    after(layoutOffsetY + lookAheadHeight + deltaY, FixedValue.One)

                    after(layoutOffsetY + lookAheadHeight, FixedValue.One)

                }

            }

            MotionDriver.State.MaxValue -> {
@@ -144,6 +152,9 @@ private class FadeContentRevealNode( 
        val placeable = measurable.measure(constraints)

        val targetHeight = placeable.height.toFloat()

        lookAheadHeight = targetHeight

        return layout(placeable.width, placeable.height) {

            layoutOffsetY = with(motionDriver) { driverOffset() }.y + deltaY



            if (revealAlpha == null) {

                val maxHeightDriven =

                    motionDriver.maxHeightDriven(
@@ -153,8 +164,7 @@ private class FadeContentRevealNode( 
                revealAlpha = maxHeightDriven

                delegate(DebugMotionValueNode(maxHeightDriven))

            }

        return layout(placeable.width, placeable.height) {

            layoutOffsetY = with(motionDriver) { driverOffset() }.y



            placeable.place(IntOffset.Zero)

        }

    }

mechanics/compose/src/com/android/mechanics/compose/modifier/VerticalTactileSurfaceRevealModifier.kt

+35 −28
Original line number Diff line number Diff line
@@ -91,9 +91,9 @@ private data class VerticalTactileSurfaceRevealElement( 
        )



    override fun update(node: VerticalTactileSurfaceRevealNode) {

        check(node.deltaY == deltaY) { "Cannot update deltaY from ${node.deltaY} to $deltaY" }

        node.update(

            motionBuilderContext = motionBuilderContext,

            deltaY = deltaY,

            revealOnThreshold = revealOnThreshold,

        )

    }
@@ -108,32 +108,35 @@ private data class VerticalTactileSurfaceRevealElement( 


private class VerticalTactileSurfaceRevealNode(

    private var motionBuilderContext: MotionBuilderContext,

    deltaY: Float,

    val deltaY: Float,

    private var revealOnThreshold: RevealOnThreshold,

    private val label: String?,

) : DelegatingNode(), ApproachLayoutModifierNode {

    // These properties are calculated during the lookahead pass (`lookAheadMeasure`) to

    // orchestrate the reveal animation. They are guaranteed to be updated before `approachMeasure`

    // is called.

    private var lookAheadHeight by mutableFloatStateOf(Float.NaN)

    private var layoutOffsetY by mutableFloatStateOf(0f)

    private var deltaY: Float by mutableFloatStateOf(deltaY)



    private lateinit var motionDriver: MotionDriver

    // Created after the first lookahead measure, guaranteed to be created before first measure

    private var layoutOffsetY by mutableFloatStateOf(Float.NaN)

    // Created lazily upon first lookahead and disposed in `onDetach`.

    private var revealHeight: ManagedMotionValue? = null



    fun update(

        motionBuilderContext: MotionBuilderContext,

        deltaY: Float,

        revealOnThreshold: RevealOnThreshold,

    ) {

        this.motionBuilderContext = motionBuilderContext

        this.deltaY = deltaY

        this.revealOnThreshold = revealOnThreshold

    }

    /**

     * The [MotionDriver] that controls the parent's motion, used to determine the reveal

     * animation's progress.

     *

     * It is initialized in `onAttach` and is safe to use in all subsequent measure passes.

     */

    private lateinit var motionDriver: MotionDriver



    override fun onAttach() {

        motionDriver = findMotionDriver()

    }



    fun update(motionBuilderContext: MotionBuilderContext, revealOnThreshold: RevealOnThreshold) {

        this.motionBuilderContext = motionBuilderContext

        this.revealOnThreshold = revealOnThreshold

    }



    override fun onDetach() {

        revealHeight?.dispose()

    }
@@ -144,10 +147,12 @@ private class VerticalTactileSurfaceRevealNode( 
                motionBuilderContext.fixedSpatialValueSpec(0f)

            }

            MotionDriver.State.Transition -> {

                // Cache the state read to avoid the performance cost of accessing it twice.

                val start = layoutOffsetY

                motionBuilderContext.spatialMotionSpec(Mapping.Zero) {

                    between(

                        start = layoutOffsetY + deltaY,

                        end = layoutOffsetY + deltaY + lookAheadHeight,

                        start = start,

                        end = start + lookAheadHeight,

                        effect = revealOnThreshold,

                    )

                }
@@ -176,6 +181,9 @@ private class VerticalTactileSurfaceRevealNode( 
        val placeable = measurable.measure(constraints)

        val targetHeight = placeable.height.toFloat()

        lookAheadHeight = targetHeight

        return layout(placeable.width, placeable.height) {

            layoutOffsetY = with(motionDriver) { driverOffset() }.y + deltaY



            if (revealHeight == null) {

                val maxHeightDriven =

                    motionDriver.maxHeightDriven(
@@ -185,8 +193,7 @@ private class VerticalTactileSurfaceRevealNode( 
                revealHeight = maxHeightDriven

                delegate(DebugMotionValueNode(maxHeightDriven))

            }

        return layout(placeable.width, placeable.height) {

            layoutOffsetY = with(motionDriver) { driverOffset() }.y



            placeable.place(IntOffset.Zero)

        }

    }