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

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

Merge "Do not place elements if they are not going to be drawn anyways" into main

parents 237a75b1 b76f93b8
Loading
Loading
Loading
Loading
+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
+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
+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(
+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)
+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)
Loading