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

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

Prevent outdated layers to update Element.SceneState.lastAlpha 

This CL ensures that the graphic layer used to fade elements does not
update SceneState.lastAlpha once that layer is invalid/outdated. See
b/343138966 for details.

Bug: 343138966
Test: ElementTest
Flag: com.android.systemui.scene_container
Change-Id: Iad78755d706f1059441c5f8f188c42c4fbcaaed3
parent 3f379a74

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

+11 −0
Original line number Diff line number Diff line
@@ -370,6 +370,17 @@ internal class ElementNode( 
                placeable.place(offset)

            } else {

                placeable.placeWithLayer(offset) {

                    // This layer might still run on its own (outside of the placement phase) even

                    // if this element is not placed anymore, so we need to double check again here

                    // before calling [elementAlpha] (which will update [SceneState.lastAlpha]). We

                    // also need to recompute the current transition to make sure that we are using

                    // the current transition and not a reference to an old one. See b/343138966 for

                    // details.

                    val transition = elementTransition(element, currentTransitions)

                    if (!shouldPlaceElement(layoutImpl, scene, element, transition)) {

                        return@placeWithLayer

                    }



                    alpha = elementAlpha(layoutImpl, scene, element, transition, sceneState)

                    compositingStrategy = CompositingStrategy.ModulateAlpha

                }

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

+63 −0
Original line number Diff line number Diff line
@@ -1418,4 +1418,67 @@ class ElementTest { 
        assertThat(bState.targetSize).isNotEqualTo(Element.SizeUnspecified)

        assertThat(bState.targetOffset).isNotEqualTo(Offset.Unspecified)

    }



    @Test

    fun lastAlphaIsNotSetByOutdatedLayer() = runTest {

        val state =

            rule.runOnUiThread {

                MutableSceneTransitionLayoutStateImpl(

                    SceneA,

                    transitions { from(SceneA, to = SceneB) { fade(TestElements.Foo) } }

                )

            }



        lateinit var layoutImpl: SceneTransitionLayoutImpl

        rule.setContent {

            SceneTransitionLayoutForTesting(state, onLayoutImpl = { layoutImpl = it }) {

                scene(SceneA) {}

                scene(SceneB) { Box(Modifier.element(TestElements.Foo)) }

                scene(SceneC) { Box(Modifier.element(TestElements.Foo)) }

            }

        }



        // Start A => B at 0.5f.

        var aToBProgress by mutableStateOf(0.5f)

        rule.runOnUiThread {

            state.startTransition(

                transition(

                    from = SceneA,

                    to = SceneB,

                    progress = { aToBProgress },

                    onFinish = neverFinish(),

                )

            )

        }

        rule.waitForIdle()



        val foo = checkNotNull(layoutImpl.elements[TestElements.Foo])

        assertThat(foo.sceneStates[SceneA]).isNull()



        val fooInB = foo.sceneStates[SceneB]

        assertThat(fooInB).isNotNull()

        assertThat(fooInB!!.lastAlpha).isEqualTo(0.5f)



        // Move the progress of A => B to 0.7f.

        aToBProgress = 0.7f

        rule.waitForIdle()

        assertThat(fooInB.lastAlpha).isEqualTo(0.7f)



        // Start B => C at 0.3f.

        rule.runOnUiThread {

            state.startTransition(transition(from = SceneB, to = SceneC, progress = { 0.3f }))

        }

        rule.waitForIdle()

        val fooInC = foo.sceneStates[SceneC]

        assertThat(fooInC).isNotNull()

        assertThat(fooInC!!.lastAlpha).isEqualTo(1f)

        assertThat(fooInB.lastAlpha).isEqualTo(Element.AlphaUnspecified)



        // Move the progress of A => B to 0.9f. This shouldn't change anything given that B => C is

        // now the transition applied to Foo.

        aToBProgress = 0.9f

        rule.waitForIdle()

        assertThat(fooInC.lastAlpha).isEqualTo(1f)

        assertThat(fooInB.lastAlpha).isEqualTo(Element.AlphaUnspecified)

    }

}

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

+1 −1
Original line number Diff line number Diff line
@@ -30,7 +30,7 @@ fun transition( 
    current: () -> SceneKey = { from },

    progress: () -> Float = { 0f },

    progressVelocity: () -> Float = { 0f },

    interruptionProgress: () -> Float = { 100f },

    interruptionProgress: () -> Float = { 0f },

    isInitiatedByUserInput: Boolean = false,

    isUserInputOngoing: Boolean = false,

    isUpOrLeft: Boolean = false,