Skip to content

Commit

Permalink
Remove invalidation trigger in GraphicsLayer.record (#1555)
Browse files Browse the repository at this point in the history
Fixes [CMP-6660](https://youtrack.jetbrains.com/issue/CMP-6660)

## Testing
`GraphicsLayerTest`

## Release Notes
### Fixes - Multiple Platforms
- _(prerelease fix)_ Fix possible infinity invalidation loop triggered
by `GraphicsLayer.record`
  • Loading branch information
MatkovIvan authored Sep 12, 2024
1 parent 837a3b0 commit 183f549
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -156,18 +156,20 @@ actual class GraphicsLayer internal constructor() {

private var density: Density = Density(1f)

private fun invalidateMatrix() {
private fun invalidateMatrix(requestDraw: Boolean = true) {
matrixDirty = true
requestDraw()
if (requestDraw) {
requestDraw()
}
}

private fun requestDraw() {
drawState.value = Unit
}

private fun updateLayerConfiguration() {
private fun updateLayerConfiguration(requestDraw: Boolean = true) {
this.outlineDirty = true
invalidateMatrix()
invalidateMatrix(requestDraw)
}

actual fun record(
Expand All @@ -182,7 +184,12 @@ actual class GraphicsLayer internal constructor() {

this.density = density
this.size = size
updateLayerConfiguration()
updateLayerConfiguration(
// [record] doesn't change the state and should not explicitly request drawing
// (happens only on the next frame) to avoid infinity invalidation loop.
// It's designed to be handled externally.
requestDraw = false
)
val bounds = size.toSize().toRect().toSkiaRect()
val canvas = pictureRecorder.beginRecording(bounds)
val skiaCanvas = canvas.asComposeCanvas() as SkiaBackedCanvas
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
/*
* 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.graphics

import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.size
import androidx.compose.runtime.DisposableEffect
import androidx.compose.runtime.remember
import androidx.compose.ui.Modifier
import androidx.compose.ui.draw.drawBehind
import androidx.compose.ui.draw.drawWithContent
import androidx.compose.ui.graphics.layer.drawLayer
import androidx.compose.ui.platform.LocalGraphicsContext
import androidx.compose.ui.test.ExperimentalTestApi
import androidx.compose.ui.test.runSkikoComposeUiTest
import androidx.compose.ui.unit.dp
import kotlin.test.Test
import kotlin.test.assertEquals

@ExperimentalTestApi
class GraphicsLayerTest {

@Test // Bug: https://youtrack.jetbrains.com/issue/CMP-6660
fun recordInvalidation() = runSkikoComposeUiTest {
var backgroundCompositions = 0
var foregroundCompositions = 0
setContent {
val graphicsContext = LocalGraphicsContext.current
val graphicsLayer = remember { graphicsContext.createGraphicsLayer() }
Box(Modifier.size(100.dp).drawWithContent {
backgroundCompositions++
graphicsLayer.record {
this@drawWithContent.drawContent()
}
})
Box(Modifier.size(50.dp).drawBehind {
foregroundCompositions++
drawLayer(graphicsLayer)
})
DisposableEffect(graphicsLayer) {
onDispose { graphicsContext.releaseGraphicsLayer(graphicsLayer) }
}
}
waitForIdle()

// UnconfinedTestDispatcher makes some difference here: instead of possible infinite
// invalidation cycle, it resolves invalidations during the same render.
// But even in this case, with incorrect behaviour composition counters won't be equal 1

assertEquals(1, backgroundCompositions)
assertEquals(1, foregroundCompositions)
}
}

0 comments on commit 183f549

Please sign in to comment.