diff --git a/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/platform/RenderNodeLayer.skiko.kt b/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/platform/RenderNodeLayer.skiko.kt index e676e3e04b3bb..428567b1f8a21 100644 --- a/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/platform/RenderNodeLayer.skiko.kt +++ b/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/platform/RenderNodeLayer.skiko.kt @@ -231,8 +231,8 @@ internal class RenderNodeLayer( if (!isDestroyed && picture != null) { picture?.close() picture = null - invalidateParentLayer() } + invalidateParentLayer() } override fun drawLayer(canvas: Canvas) { diff --git a/compose/ui/ui/src/skikoTest/kotlin/androidx/compose/ui/platform/GraphicLayerBugTest.kt b/compose/ui/ui/src/skikoTest/kotlin/androidx/compose/ui/platform/GraphicLayerBugTest.kt new file mode 100644 index 0000000000000..431ce42f5e298 --- /dev/null +++ b/compose/ui/ui/src/skikoTest/kotlin/androidx/compose/ui/platform/GraphicLayerBugTest.kt @@ -0,0 +1,80 @@ +/* + * Copyright 2024 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package androidx.compose.ui.platform + +import androidx.compose.foundation.Canvas +import androidx.compose.foundation.ExperimentalFoundationApi +import androidx.compose.foundation.layout.Spacer +import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.foundation.layout.size +import androidx.compose.foundation.pager.HorizontalPager +import androidx.compose.foundation.pager.rememberPagerState +import androidx.compose.material.Scaffold +import androidx.compose.material.TextField +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue +import androidx.compose.ui.Modifier +import androidx.compose.ui.assertThat +import androidx.compose.ui.draw.drawBehind +import androidx.compose.ui.isEqualTo +import androidx.compose.ui.test.ExperimentalTestApi +import androidx.compose.ui.test.runComposeUiTest +import androidx.compose.ui.unit.dp +import kotlin.test.Test + +// Tests for fixed bugs +class GraphicLayerBugTest { + // https://github.com/JetBrains/compose-multiplatform/issues/4681 + @OptIn(ExperimentalTestApi::class, ExperimentalFoundationApi::class) + @Test + fun draw_invalidates_inside_complex_layout() = + runComposeUiTest { + var valueForDraw by mutableStateOf(0f) + var lastDrawnValue = -1f + + setContent { + val pagerState = rememberPagerState { 1 } + Scaffold( + modifier = Modifier.fillMaxSize(), + topBar = { + Spacer(Modifier.drawBehind { pagerState.targetPage }) + }, + ) { + HorizontalPager(modifier = Modifier.fillMaxSize(), state = pagerState) { + var value by remember { mutableStateOf("") } + TextField( + value = value, + onValueChange = { value = it }, + ) + + Canvas(Modifier.size(40.dp)) { + lastDrawnValue = valueForDraw + } + } + } + } + + waitForIdle() + assertThat(lastDrawnValue).isEqualTo(0f) + + valueForDraw = 1f + waitForIdle() + assertThat(lastDrawnValue).isEqualTo(1f) + } +} diff --git a/compose/ui/ui/src/skikoTest/kotlin/androidx/compose/ui/platform/RenderNodeLayerTest.kt b/compose/ui/ui/src/skikoTest/kotlin/androidx/compose/ui/platform/RenderNodeLayerTest.kt index a106c17d0a02d..a4afaa0b90678 100644 --- a/compose/ui/ui/src/skikoTest/kotlin/androidx/compose/ui/platform/RenderNodeLayerTest.kt +++ b/compose/ui/ui/src/skikoTest/kotlin/androidx/compose/ui/platform/RenderNodeLayerTest.kt @@ -17,8 +17,10 @@ package androidx.compose.ui.platform import androidx.compose.foundation.shape.CircleShape +import androidx.compose.ui.assertThat import androidx.compose.ui.geometry.Offset import androidx.compose.ui.graphics.* +import androidx.compose.ui.isEqualTo import androidx.compose.ui.unit.* import kotlin.math.PI import kotlin.math.cos @@ -26,6 +28,7 @@ import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse import kotlin.test.assertTrue +import org.jetbrains.skia.Surface class RenderNodeLayerTest { @@ -464,11 +467,56 @@ class RenderNodeLayerTest { assertTrue(layer.isInLayer(Offset(50f, 100f))) } - private fun TestRenderNodeLayer() = RenderNodeLayer( + @Test + fun invalidate_parent_layer() { + var parentDrawCount = 0 + + var childLayer: RenderNodeLayer? = null + val parentLayer = TestRenderNodeLayer( + drawBlock = { + parentDrawCount++ + childLayer?.drawLayer(it) + }, + ) + + val surface = Surface.makeNull(10, 10) + val canvas = surface.canvas.asComposeCanvas() + + assertThat(parentDrawCount).isEqualTo(0) + + parentLayer.drawLayer(canvas) + assertThat(parentDrawCount).isEqualTo(1) + + // shouldn't be drawn again, as it isn't changed + parentLayer.drawLayer(canvas) + assertThat(parentDrawCount).isEqualTo(1) + + // https://github.com/JetBrains/compose-multiplatform/issues/4681 + // parent should be drawn again, as we add a child + childLayer = TestRenderNodeLayer( + invalidateParentLayer = parentLayer::invalidate, + drawBlock = {}, + ) + childLayer.invalidate() + parentLayer.drawLayer(canvas) + assertThat(parentDrawCount).isEqualTo(2) + + parentLayer.drawLayer(canvas) + assertThat(parentDrawCount).isEqualTo(2) + + childLayer.invalidate() + parentLayer.drawLayer(canvas) + assertThat(parentDrawCount).isEqualTo(3) + } + + private fun TestRenderNodeLayer( + invalidateParentLayer: () -> Unit = {}, + drawBlock: (Canvas) -> Unit = {}, + ) = RenderNodeLayer( Density(1f, 1f), measureDrawBounds = false, - invalidateParentLayer = {}, - drawBlock = {} + invalidateParentLayer = invalidateParentLayer, + drawBlock = drawBlock, ) private fun assertMapping(from: Offset, to: Offset) {