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

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

Do not place elements if they are not going to be drawn anyways 

This CL moves the location where we check whether an element should be
drawn from drawing time to placement time: there is no need to place
(and therefore compute the offset) of an element if we are not going to
draw it anyways.

Note that we still need to compute the size of an element in each scene
because the size of elements does impact the layout in both scenes the
element is in.

Bug: 308961608
Test: PlatformComposeSceneTransitionLayoutTests
Flag: N/A
Change-Id: Icaec9770dc06f0ee054541faea58052e193cc608
parent 8c4d65f5

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

+16 −13
Original line number Diff line number Diff line
@@ -235,7 +235,6 @@ internal class ElementNode( 
    }



    override fun ContentDrawScope.draw() {

        if (shouldDrawElement(layoutImpl, scene, element)) {

        val drawScale = getDrawScale(layoutImpl, element, scene, sceneValues)

        if (drawScale == Scale.Default) {

            drawContent()
@@ -249,7 +248,6 @@ internal class ElementNode( 
            }

        }

    }

    }



    companion object {

        private fun maybePruneMaps(
@@ -516,13 +514,18 @@ private fun IntermediateMeasureScope.place( 
    with(placementScope) {

        // Update the offset (relative to the SceneTransitionLayout) this element has in this scene

        // when idle.

        val coords = coordinates!!

        val coords = coordinates ?: error("Element ${element.key} does not have any coordinates")

        val targetOffsetInScene = lookaheadScopeCoordinates.localLookaheadPositionOf(coords)

        if (targetOffsetInScene != sceneValues.targetOffset) {

            // TODO(b/290930950): Better handle when this changes to avoid instant offset jumps.

            sceneValues.targetOffset = targetOffsetInScene

        }



        // No need to place the element in this scene if we don't want to draw it anyways.

        if (!shouldDrawElement(layoutImpl, scene, element)) {

            return

        }



        val currentOffset = lookaheadScopeCoordinates.localPositionOf(coords, Offset.Zero)

        val lastSharedValues = element.lastSharedValues

        val lastValues = sceneValues.lastValues

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

+40 −19
Original line number Diff line number Diff line
@@ -116,7 +116,13 @@ class ElementTest { 
            toSceneContent = {

                Box(Modifier.size(layoutSize)) {

                    // Shared element.

                    Element(TestElements.Foo, elementSize, elementOffset)

                    Element(

                        TestElements.Foo,

                        elementSize,

                        elementOffset,

                        onLayout = { fooLayouts++ },

                        onPlacement = { fooPlacements++ },

                    )

                }

            },

            transition = {
@@ -127,21 +133,25 @@ class ElementTest { 
                scaleSize(TestElements.Bar, width = 1f, height = 1f)

            },

        ) {

            var numberOfLayoutsAfterOneAnimationFrame = 0

            var numberOfPlacementsAfterOneAnimationFrame = 0

            var fooLayoutsAfterOneAnimationFrame = 0

            var fooPlacementsAfterOneAnimationFrame = 0

            var barLayoutsAfterOneAnimationFrame = 0

            var barPlacementsAfterOneAnimationFrame = 0



            fun assertNumberOfLayoutsAndPlacements() {

                assertThat(fooLayouts).isEqualTo(numberOfLayoutsAfterOneAnimationFrame)

                assertThat(fooPlacements).isEqualTo(numberOfPlacementsAfterOneAnimationFrame)

                assertThat(barLayouts).isEqualTo(numberOfLayoutsAfterOneAnimationFrame)

                assertThat(barPlacements).isEqualTo(numberOfPlacementsAfterOneAnimationFrame)

                assertThat(fooLayouts).isEqualTo(fooLayoutsAfterOneAnimationFrame)

                assertThat(fooPlacements).isEqualTo(fooPlacementsAfterOneAnimationFrame)

                assertThat(barLayouts).isEqualTo(barLayoutsAfterOneAnimationFrame)

                assertThat(barPlacements).isEqualTo(barPlacementsAfterOneAnimationFrame)

            }



            at(16) {

                // Capture the number of layouts and placements that happened after 1 animation

                // frame.

                numberOfLayoutsAfterOneAnimationFrame = fooLayouts

                numberOfPlacementsAfterOneAnimationFrame = fooPlacements

                fooLayoutsAfterOneAnimationFrame = fooLayouts

                fooPlacementsAfterOneAnimationFrame = fooPlacements

                barLayoutsAfterOneAnimationFrame = barLayouts

                barPlacementsAfterOneAnimationFrame = barPlacements

            }

            repeat(nFrames - 2) { i ->

                // Ensure that all animation frames (except the final one) don't relayout or replace
@@ -187,7 +197,13 @@ class ElementTest { 
            toSceneContent = {

                Box(Modifier.size(layoutSize)) {

                    // Shared element.

                    Element(TestElements.Foo, elementSize, offset = 20.dp)

                    Element(

                        TestElements.Foo,

                        elementSize,

                        offset = 20.dp,

                        onLayout = { fooLayouts++ },

                        onPlacement = { fooPlacements++ },

                    )

                }

            },

            transition = {
@@ -198,25 +214,30 @@ class ElementTest { 
                scaleSize(TestElements.Bar, width = 1f, height = 1f)

            },

        ) {

            var numberOfLayoutsAfterOneAnimationFrame = 0

            var lastNumberOfPlacements = 0

            var fooLayoutsAfterOneAnimationFrame = 0

            var barLayoutsAfterOneAnimationFrame = 0

            var lastFooPlacements = 0

            var lastBarPlacements = 0



            fun assertNumberOfLayoutsAndPlacements() {

                // The number of layouts have not changed.

                assertThat(fooLayouts).isEqualTo(numberOfLayoutsAfterOneAnimationFrame)

                assertThat(barLayouts).isEqualTo(numberOfLayoutsAfterOneAnimationFrame)

                assertThat(fooLayouts).isEqualTo(fooLayoutsAfterOneAnimationFrame)

                assertThat(barLayouts).isEqualTo(barLayoutsAfterOneAnimationFrame)



                // The number of placements have increased.

                assertThat(fooPlacements).isGreaterThan(lastNumberOfPlacements)

                assertThat(barPlacements).isGreaterThan(lastNumberOfPlacements)

                lastNumberOfPlacements = fooPlacements

                assertThat(fooPlacements).isGreaterThan(lastFooPlacements)

                assertThat(barPlacements).isGreaterThan(lastBarPlacements)

                lastFooPlacements = fooPlacements

                lastBarPlacements = barPlacements

            }



            at(16) {

                // Capture the number of layouts and placements that happened after 1 animation

                // frame.

                numberOfLayoutsAfterOneAnimationFrame = fooLayouts

                lastNumberOfPlacements = fooPlacements

                fooLayoutsAfterOneAnimationFrame = fooLayouts

                barLayoutsAfterOneAnimationFrame = barLayouts

                lastFooPlacements = fooPlacements

                lastBarPlacements = barPlacements

            }

            repeat(nFrames - 2) { i ->

                // Ensure that all animation frames (except the final one) only replaced the

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

+5 −4
Original line number Diff line number Diff line
@@ -30,6 +30,7 @@ import androidx.compose.runtime.remember 
import androidx.compose.runtime.setValue

import androidx.compose.ui.Modifier

import androidx.compose.ui.test.assertIsDisplayed

import androidx.compose.ui.test.assertIsNotDisplayed

import androidx.compose.ui.test.hasParent

import androidx.compose.ui.test.hasText

import androidx.compose.ui.test.junit4.createComposeRule
@@ -92,15 +93,15 @@ class MovableElementTest { 
            }



            at(32) {

                // In the middle of the transition, there are 2 copies of the counter: the previous

                // one from scene A (equal to 3) and the new one from scene B (equal to 0).

                // In the middle of the transition, 2 copies of the counter are composed but only

                // the one in scene B is placed/drawn.

                rule

                    .onNode(

                        hasText("count: 3") and

                            hasParent(isElement(TestElements.Foo, scene = TestScenes.SceneA))

                    )

                    .assertIsDisplayed()

                    .assertSizeIsEqualTo(75.dp, 75.dp)

                    .assertExists()

                    .assertIsNotDisplayed()



                rule

                    .onNode(

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

+4 −6
Original line number Diff line number Diff line
@@ -40,9 +40,7 @@ import androidx.compose.ui.test.assertIsDisplayed 
import androidx.compose.ui.test.assertPositionInRootIsEqualTo

import androidx.compose.ui.test.assertWidthIsEqualTo

import androidx.compose.ui.test.junit4.createAndroidComposeRule

import androidx.compose.ui.test.onAllNodesWithTag

import androidx.compose.ui.test.onChild

import androidx.compose.ui.test.onFirst

import androidx.compose.ui.test.onNodeWithTag

import androidx.compose.ui.test.onNodeWithText

import androidx.compose.ui.unit.Dp
@@ -251,9 +249,9 @@ class SceneTransitionLayoutTest { 
        // Advance to the middle of the animation.

        rule.mainClock.advanceTimeBy(TestTransitionDuration / 2)



        // We need to use onAllNodesWithTag().onFirst() here given that shared elements are

        // composed and laid out in both scenes (but drawn only in one).

        sharedFoo = rule.onAllNodesWithTag(TestElements.Foo.testTag).onFirst()

        // Foo is shared between Scene A and Scene B, and is therefore placed/drawn in Scene B given

        // that B has a higher zIndex than A.

        sharedFoo = rule.onNode(isElement(TestElements.Foo, TestScenes.SceneB))



        // In scene B, foo is at the top start (x = 0, y = 0) of the layout and has a size of

        // 100.dp. We pause at the middle of the transition, so it should now be 75.dp given that we
@@ -287,7 +285,7 @@ class SceneTransitionLayoutTest { 
        val expectedLeft = 0.dp

        val expectedSize = 100.dp + (150.dp - 100.dp) * interpolatedProgress



        sharedFoo = rule.onAllNodesWithTag(TestElements.Foo.testTag).onFirst()

        sharedFoo = rule.onNode(isElement(TestElements.Foo, TestScenes.SceneC))

        assertThat((layoutState.transitionState as TransitionState.Transition).progress)

            .isEqualTo(interpolatedProgress)

        sharedFoo.assertWidthIsEqualTo(expectedSize)

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

+23 −17
Original line number Diff line number Diff line
@@ -23,6 +23,7 @@ import androidx.compose.foundation.layout.fillMaxSize 
import androidx.compose.foundation.layout.offset

import androidx.compose.foundation.layout.size

import androidx.compose.ui.Modifier

import androidx.compose.ui.test.assertIsNotDisplayed

import androidx.compose.ui.test.assertPositionInRootIsEqualTo

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

import androidx.compose.ui.unit.dp
@@ -33,7 +34,6 @@ import com.android.compose.animation.scene.TestScenes 
import com.android.compose.animation.scene.inScene

import com.android.compose.animation.scene.testTransition

import com.android.compose.test.assertSizeIsEqualTo

import com.android.compose.test.onEach

import org.junit.Rule

import org.junit.Test

import org.junit.runner.RunWith
@@ -63,28 +63,34 @@ class SharedElementTest { 
                onElement(TestElements.Foo).assertSizeIsEqualTo(20.dp, 80.dp)

            }

            at(0) {

                onSharedElement(TestElements.Foo).onEach {

                    assertPositionInRootIsEqualTo(10.dp, 50.dp)

                    assertSizeIsEqualTo(20.dp, 80.dp)

                }

                // Shared elements are by default placed and drawn only in the scene with highest

                // zIndex.

                onElement(TestElements.Foo, TestScenes.SceneA).assertIsNotDisplayed()



                onElement(TestElements.Foo, TestScenes.SceneB)

                    .assertPositionInRootIsEqualTo(10.dp, 50.dp)

                    .assertSizeIsEqualTo(20.dp, 80.dp)

            }

            at(16) {

                onSharedElement(TestElements.Foo).onEach {

                    assertPositionInRootIsEqualTo(20.dp, 55.dp)

                    assertSizeIsEqualTo(17.5.dp, 70.dp)

                }

                onElement(TestElements.Foo, TestScenes.SceneA).assertIsNotDisplayed()



                onElement(TestElements.Foo, TestScenes.SceneB)

                    .assertPositionInRootIsEqualTo(20.dp, 55.dp)

                    .assertSizeIsEqualTo(17.5.dp, 70.dp)

            }

            at(32) {

                onSharedElement(TestElements.Foo).onEach {

                    assertPositionInRootIsEqualTo(30.dp, 60.dp)

                    assertSizeIsEqualTo(15.dp, 60.dp)

                }

                onElement(TestElements.Foo, TestScenes.SceneA).assertIsNotDisplayed()



                onElement(TestElements.Foo, TestScenes.SceneB)

                    .assertPositionInRootIsEqualTo(30.dp, 60.dp)

                    .assertSizeIsEqualTo(15.dp, 60.dp)

            }

            at(48) {

                onSharedElement(TestElements.Foo).onEach {

                    assertPositionInRootIsEqualTo(40.dp, 65.dp)

                    assertSizeIsEqualTo(12.5.dp, 50.dp)

                }

                onElement(TestElements.Foo, TestScenes.SceneA).assertIsNotDisplayed()



                onElement(TestElements.Foo, TestScenes.SceneB)

                    .assertPositionInRootIsEqualTo(40.dp, 65.dp)

                    .assertSizeIsEqualTo(12.5.dp, 50.dp)

            }

            after {

                onElement(TestElements.Foo).assertPositionInRootIsEqualTo(50.dp, 70.dp)