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

Commit dd2e259c authored by Jordan Demeulenaere's avatar Jordan Demeulenaere Committed by Android (Google) Code Review
Browse files

Merge changes I7e5f70fd,Iad78755d,I5b882188 into main

* changes:
  Force new fading elements to fade from 0 alpha
  Prevent outdated layers to update Element.SceneState.lastAlpha
  Move the Element placement logic into ElementNode
parents 13e2a44d c74e9ff4
Loading
Loading
Loading
Loading
+97 −87
Original line number Diff line number Diff line
@@ -66,6 +66,9 @@ internal class Element(val key: ElementKey) {
     */
    var lastTransition: TransitionState.Transition? = null

    /** Whether this element was ever drawn in a scene. */
    var wasDrawnInAnyScene = false

    override fun toString(): String {
        return "Element(key=$key)"
    }
@@ -299,19 +302,98 @@ internal class ElementNode(
        val placeable =
            measure(layoutImpl, scene, element, transition, sceneState, measurable, constraints)
        sceneState.lastSize = placeable.size()
        return layout(placeable.width, placeable.height) {
            place(
        return layout(placeable.width, placeable.height) { place(transition, placeable) }
    }

    @OptIn(ExperimentalComposeUiApi::class)
    private fun Placeable.PlacementScope.place(
        transition: TransitionState.Transition?,
        placeable: Placeable,
    ) {
        with(layoutImpl.lookaheadScope) {
            // Update the offset (relative to the SceneTransitionLayout) this element has in this
            // scene when idle.
            val coords =
                coordinates ?: error("Element ${element.key} does not have any coordinates")
            val targetOffsetInScene = lookaheadScopeCoordinates.localLookaheadPositionOf(coords)

            // No need to place the element in this scene if we don't want to draw it anyways.
            if (!shouldPlaceElement(layoutImpl, scene, element, transition)) {
                sceneState.lastOffset = Offset.Unspecified
                sceneState.lastScale = Scale.Unspecified
                sceneState.lastAlpha = Element.AlphaUnspecified

                sceneState.clearValuesBeforeInterruption()
                sceneState.clearInterruptionDeltas()
                return
            }

            val currentOffset = lookaheadScopeCoordinates.localPositionOf(coords, Offset.Zero)
            val targetOffset =
                computeValue(
                    layoutImpl,
                    scene,
                    element,
                    transition,
                sceneState,
                placeable,
                    sceneValue = { it.targetOffset },
                    transformation = { it.offset },
                    idleValue = targetOffsetInScene,
                    currentValue = { currentOffset },
                    isSpecified = { it != Offset.Unspecified },
                    ::lerp,
                )

            val interruptedOffset =
                computeInterruptedValue(
                    layoutImpl,
                    transition,
                    value = targetOffset,
                    unspecifiedValue = Offset.Unspecified,
                    zeroValue = Offset.Zero,
                    getValueBeforeInterruption = { sceneState.offsetBeforeInterruption },
                    setValueBeforeInterruption = { sceneState.offsetBeforeInterruption = it },
                    getInterruptionDelta = { sceneState.offsetInterruptionDelta },
                    setInterruptionDelta = { sceneState.offsetInterruptionDelta = it },
                    diff = { a, b -> a - b },
                    add = { a, b, bProgress -> a + b * bProgress },
                )

            sceneState.lastOffset = interruptedOffset

            val offset = (interruptedOffset - currentOffset).round()
            if (
                isElementOpaque(scene, element, transition) &&
                    interruptedAlpha(layoutImpl, transition, sceneState, alpha = 1f) == 1f
            ) {
                sceneState.lastAlpha = 1f

                // TODO(b/291071158): Call placeWithLayer() if offset != IntOffset.Zero and size is
                // not animated once b/305195729 is fixed. Test that drawing is not invalidated in
                // that case.
                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
                }
            }
        }
    }

    override fun ContentDrawScope.draw() {
        element.wasDrawnInAnyScene = true

        val transition = elementTransition(element, currentTransitions)
        val drawScale = getDrawScale(layoutImpl, scene, element, transition, sceneState)
        if (drawScale == Scale.Default) {
@@ -649,6 +731,12 @@ private fun elementAlpha(
            )
            .fastCoerceIn(0f, 1f)

    // If the element is fading during this transition and that it is drawn for the first time, make
    // sure that it doesn't instantly appear on screen.
    if (!element.wasDrawnInAnyScene && alpha > 0f) {
        element.sceneStates.forEach { it.value.alphaBeforeInterruption = 0f }
    }

    val interruptedAlpha = interruptedAlpha(layoutImpl, transition, sceneState, alpha)
    sceneState.lastAlpha = interruptedAlpha
    return interruptedAlpha
@@ -807,84 +895,6 @@ private fun ContentDrawScope.getDrawScale(
    return interruptedScale
}

@OptIn(ExperimentalComposeUiApi::class)
private fun Placeable.PlacementScope.place(
    layoutImpl: SceneTransitionLayoutImpl,
    scene: Scene,
    element: Element,
    transition: TransitionState.Transition?,
    sceneState: Element.SceneState,
    placeable: Placeable,
) {
    with(layoutImpl.lookaheadScope) {
        // Update the offset (relative to the SceneTransitionLayout) this element has in this scene
        // when idle.
        val coords = coordinates ?: error("Element ${element.key} does not have any coordinates")
        val targetOffsetInScene = lookaheadScopeCoordinates.localLookaheadPositionOf(coords)

        // No need to place the element in this scene if we don't want to draw it anyways.
        if (!shouldPlaceElement(layoutImpl, scene, element, transition)) {
            sceneState.lastOffset = Offset.Unspecified
            sceneState.lastScale = Scale.Unspecified
            sceneState.lastAlpha = Element.AlphaUnspecified

            sceneState.clearValuesBeforeInterruption()
            sceneState.clearInterruptionDeltas()
            return
        }

        val currentOffset = lookaheadScopeCoordinates.localPositionOf(coords, Offset.Zero)
        val targetOffset =
            computeValue(
                layoutImpl,
                scene,
                element,
                transition,
                sceneValue = { it.targetOffset },
                transformation = { it.offset },
                idleValue = targetOffsetInScene,
                currentValue = { currentOffset },
                isSpecified = { it != Offset.Unspecified },
                ::lerp,
            )

        val interruptedOffset =
            computeInterruptedValue(
                layoutImpl,
                transition,
                value = targetOffset,
                unspecifiedValue = Offset.Unspecified,
                zeroValue = Offset.Zero,
                getValueBeforeInterruption = { sceneState.offsetBeforeInterruption },
                setValueBeforeInterruption = { sceneState.offsetBeforeInterruption = it },
                getInterruptionDelta = { sceneState.offsetInterruptionDelta },
                setInterruptionDelta = { sceneState.offsetInterruptionDelta = it },
                diff = { a, b -> a - b },
                add = { a, b, bProgress -> a + b * bProgress },
            )

        sceneState.lastOffset = interruptedOffset

        val offset = (interruptedOffset - currentOffset).round()
        if (
            isElementOpaque(scene, element, transition) &&
                interruptedAlpha(layoutImpl, transition, sceneState, alpha = 1f) == 1f
        ) {
            sceneState.lastAlpha = 1f

            // TODO(b/291071158): Call placeWithLayer() if offset != IntOffset.Zero and size is not
            // animated once b/305195729 is fixed. Test that drawing is not invalidated in that
            // case.
            placeable.place(offset)
        } else {
            placeable.placeWithLayer(offset) {
                alpha = elementAlpha(layoutImpl, scene, element, transition, sceneState)
                compositingStrategy = CompositingStrategy.ModulateAlpha
            }
        }
    }
}

/**
 * Return the value that should be used depending on the current layout state and transition.
 *
+111 −0
Original line number Diff line number Diff line
@@ -1418,4 +1418,115 @@ 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)
    }

    @Test
    fun fadingElementsDontAppearInstantly() {
        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)) }
            }
        }

        // Start A => B at 60%.
        var interruptionProgress by mutableStateOf(1f)
        rule.runOnUiThread {
            state.startTransition(
                transition(
                    from = SceneA,
                    to = SceneB,
                    progress = { 0.6f },
                    interruptionProgress = { interruptionProgress },
                ),
                transitionKey = null
            )
        }
        rule.waitForIdle()

        // Alpha of Foo should be 0f at interruption progress 100%.
        val fooInB = layoutImpl.elements.getValue(TestElements.Foo).sceneStates.getValue(SceneB)
        assertThat(fooInB.lastAlpha).isEqualTo(0f)

        // Alpha of Foo should be 0.6f at interruption progress 0%.
        interruptionProgress = 0f
        rule.waitForIdle()
        assertThat(fooInB.lastAlpha).isEqualTo(0.6f)

        // Alpha of Foo should be 0.3f at interruption progress 50%.
        interruptionProgress = 0.5f
        rule.waitForIdle()
        assertThat(fooInB.lastAlpha).isEqualTo(0.3f)
    }
}
+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,