Skip to content

Commit b60bc09

Browse files
authored
Fix invalidation during removing the layer (#3146)
[CMP-10360](https://youtrack.jetbrains.com/issue/CMP-10360) Dialog hide animation hangs on the last frame Regression after #3096, not released yet ## Release Notes N/A
1 parent c5784eb commit b60bc09

7 files changed

Lines changed: 149 additions & 18 deletions

File tree

compose/ui/ui/src/desktopTest/kotlin/androidx/compose/ui/ComposeSceneTest.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -468,7 +468,7 @@ class ComposeSceneTest {
468468
screenshotRule.snap(surface, "frame4_change_height")
469469

470470
// see https://youtrack.jetbrains.com/issue/CMP-2171, we have extra rendered frames here
471-
skipRenders()
471+
skipRendersUntilIdle()
472472

473473
assertFalse(hasRenders())
474474
}

compose/ui/ui/src/desktopTest/kotlin/androidx/compose/ui/platform/RenderingTestScope.kt

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,10 @@ import androidx.compose.ui.scene.SingleComposeSceneRenderingScope
2626
import androidx.compose.ui.unit.Density
2727
import androidx.compose.ui.unit.IntSize
2828
import kotlin.coroutines.CoroutineContext
29+
import kotlin.time.Duration.Companion.milliseconds
2930
import kotlinx.coroutines.CompletableDeferred
3031
import kotlinx.coroutines.runBlocking
32+
import kotlinx.coroutines.withTimeout
3133
import kotlinx.coroutines.yield
3234
import org.jetbrains.skia.Surface
3335
import org.jetbrains.skiko.FrameDispatcher
@@ -37,13 +39,16 @@ internal fun renderingTest(
3739
width: Int,
3840
height: Int,
3941
context: CoroutineContext = MainUIDispatcher,
42+
timeoutMillis: Long = 10000,
4043
block: suspend RenderingTestScope.() -> Unit
4144
) = runBlocking(MainUIDispatcher) {
42-
val scope = RenderingTestScope(width, height, context)
43-
try {
44-
scope.block()
45-
} finally {
46-
scope.dispose()
45+
withTimeout(timeoutMillis.milliseconds) {
46+
val scope = RenderingTestScope(width, height, context)
47+
try {
48+
scope.block()
49+
} finally {
50+
scope.dispose()
51+
}
4752
}
4853
}
4954

@@ -101,9 +106,17 @@ internal class RenderingTestScope(
101106
onRender.await()
102107
}
103108

104-
suspend fun skipRenders() {
105-
repeat(1000) {
106-
yield()
109+
suspend fun skipRendersUntilIdle(maxFrames: Int = 1000) {
110+
var frames = 0
111+
while (frames < maxFrames) {
112+
currentTimeMillis += 16
113+
if (!hasRenders()) {
114+
yield()
115+
if (!hasRenders()) {
116+
return
117+
}
118+
}
119+
frames++
107120
}
108121
}
109122

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
/*
2+
* Copyright 2026 The Android Open Source Project
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package androidx.compose.ui.window
18+
19+
import androidx.compose.foundation.layout.Box
20+
import androidx.compose.foundation.layout.size
21+
import androidx.compose.runtime.getValue
22+
import androidx.compose.runtime.mutableStateOf
23+
import androidx.compose.runtime.setValue
24+
import androidx.compose.ui.Modifier
25+
import androidx.compose.ui.graphics.Color
26+
import androidx.compose.ui.graphics.toComposeImageBitmap
27+
import androidx.compose.ui.graphics.toPixelMap
28+
import androidx.compose.ui.platform.RenderingTestScope
29+
import androidx.compose.ui.platform.renderingTest
30+
import androidx.compose.ui.unit.dp
31+
import kotlin.test.assertEquals
32+
import org.junit.Test
33+
34+
class DesktopDialogTest {
35+
36+
@Test
37+
fun scrimDisappearsAfterDialogHideAnimation() = renderingTest(width = 200, height = 200) {
38+
var showDialog by mutableStateOf(true)
39+
40+
setContent {
41+
if (showDialog) {
42+
Dialog(onDismissRequest = {}) {
43+
Box(Modifier.size(50.dp))
44+
}
45+
}
46+
}
47+
48+
// Settle the shown state (the appearance animation also runs through the frame loop).
49+
awaitNextRender()
50+
skipRendersUntilIdle()
51+
assertEquals(Color.Black.copy(alpha = 0.6f), colorOfCornerPixel())
52+
53+
// Dismiss the dialog and let the on-demand loop run the hide animation to completion.
54+
showDialog = false
55+
skipRendersUntilIdle()
56+
57+
assertEquals(Color.Transparent, colorOfCornerPixel())
58+
}
59+
60+
private fun RenderingTestScope.colorOfCornerPixel(): Color =
61+
surface.makeImageSnapshot().toComposeImageBitmap().toPixelMap()[0, 0]
62+
}

compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/scene/BaseComposeScene.skiko.kt

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import androidx.compose.ui.input.rotary.RotaryScrollEvent
3838
import androidx.compose.ui.platform.FrameRecomposer
3939
import androidx.compose.ui.platform.ProvidePlatformCompositionLocals
4040
import androidx.compose.ui.util.trace
41+
import kotlin.concurrent.Volatile
4142

4243
/**
4344
* BaseComposeScene is an internal abstract class that implements the ComposeScene interface.
@@ -82,12 +83,25 @@ internal abstract class BaseComposeScene(
8283
}
8384
}
8485

85-
protected fun invokeInvalidationCallbacks() {
86+
@Volatile
87+
protected var hasForcedLayout: Boolean = false
88+
private set
89+
90+
@Volatile
91+
protected var hasForcedDraw: Boolean = false
92+
private set
93+
94+
protected fun invokeInvalidationCallbacks(
95+
forceLayout: Boolean = false,
96+
forceDraw: Boolean = false,
97+
) {
98+
hasForcedLayout = hasForcedLayout || forceLayout
99+
hasForcedDraw = hasForcedDraw || forceDraw
86100
if (isInvalidationDisabled || isClosed || composition == null) return
87-
if (hasPendingMeasureOrLayout) {
101+
if (hasForcedLayout || hasPendingMeasureOrLayout) {
88102
invalidateLayout()
89103
}
90-
if (hasPendingDraw) {
104+
if (hasForcedDraw || hasPendingDraw) {
91105
invalidateDraw()
92106
}
93107
}
@@ -139,6 +153,7 @@ internal abstract class BaseComposeScene(
139153

140154
override fun measureAndLayout() {
141155
if (isClosed) return
156+
hasForcedLayout = false
142157

143158
postponeInvalidation("BaseComposeScene:measureAndLayout") {
144159
doMeasureAndLayout()
@@ -154,6 +169,7 @@ internal abstract class BaseComposeScene(
154169

155170
override fun draw(canvas: Canvas) {
156171
if (isClosed) return
172+
hasForcedDraw = false
157173

158174
postponeInvalidation("BaseComposeScene:draw") {
159175
// FIXME: Remove applying the global snapshot here.

compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/scene/CanvasLayersComposeScene.skiko.kt

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -227,11 +227,11 @@ private class CanvasLayersComposeSceneImpl(
227227
}
228228

229229
override val hasPendingMeasureOrLayout: Boolean
230-
get() = mainOwner.hasPendingMeasureOrLayout
230+
get() = hasForcedLayout || mainOwner.hasPendingMeasureOrLayout
231231
|| layers.fastAny { it.owner.hasPendingMeasureOrLayout }
232232

233233
override val hasPendingDraw: Boolean
234-
get() = mainOwner.hasPendingDraw
234+
get() = hasForcedDraw || mainOwner.hasPendingDraw
235235
|| layers.fastAny { it.owner.hasPendingDraw }
236236

237237
override fun createComposition(
@@ -517,7 +517,7 @@ private class CanvasLayersComposeSceneImpl(
517517
onOwnerAppended(layer.owner)
518518

519519
inputHandler.onPointerUpdate()
520-
invokeInvalidationCallbacks()
520+
invokeInvalidationCallbacks(forceLayout = true, forceDraw = true)
521521
}
522522

523523
private fun detachLayer(layer: AttachedComposeSceneLayer) {
@@ -528,7 +528,9 @@ private class CanvasLayersComposeSceneImpl(
528528
onOwnerRemoved(layer.owner)
529529

530530
inputHandler.onPointerUpdate()
531-
invokeInvalidationCallbacks()
531+
// A detached layer was composited onto this scene's canvas, so its removal changes
532+
// the scene's output even though no remaining owner is dirty.
533+
invokeInvalidationCallbacks(forceLayout = true, forceDraw = true)
532534
}
533535

534536
private fun requestFocus(layer: AttachedComposeSceneLayer) {

compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/scene/PlatformLayersComposeScene.skiko.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -162,10 +162,10 @@ private class PlatformLayersComposeSceneImpl(
162162
}
163163

164164
override val hasPendingMeasureOrLayout: Boolean
165-
get() = mainOwner.hasPendingMeasureOrLayout
165+
get() = hasForcedLayout || mainOwner.hasPendingMeasureOrLayout
166166

167167
override val hasPendingDraw: Boolean
168-
get() = mainOwner.hasPendingDraw
168+
get() = hasForcedDraw || mainOwner.hasPendingDraw
169169

170170
override fun createComposition(
171171
parentCompositionContext: CompositionContext,

compose/ui/ui/src/skikoTest/kotlin/androidx/compose/ui/scene/CanvasLayersComposeSceneTest.kt

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import androidx.compose.foundation.layout.Box
2020
import androidx.compose.foundation.layout.fillMaxSize
2121
import androidx.compose.ui.Modifier
2222
import androidx.compose.ui.geometry.Offset
23+
import androidx.compose.ui.graphics.asComposeCanvas
2324
import androidx.compose.ui.input.pointer.PointerEventType
2425
import androidx.compose.ui.platform.FrameRecomposer
2526
import androidx.compose.ui.unit.IntSize
@@ -32,6 +33,7 @@ import kotlin.test.assertFalse
3233
import kotlin.test.assertTrue
3334
import kotlinx.coroutines.test.StandardTestDispatcher
3435
import kotlinx.coroutines.test.runTest
36+
import org.jetbrains.skia.Surface
3537

3638
class CanvasLayersComposeSceneTest {
3739

@@ -79,4 +81,40 @@ class CanvasLayersComposeSceneTest {
7981
}
8082
frameRecomposer.close()
8183
}
84+
85+
@Test
86+
fun detachingLayerRequestsDrawPass() = runTest(StandardTestDispatcher()) {
87+
var drawInvalidations = 0
88+
var layer: ComposeSceneLayer? = null
89+
val frameRecomposer = FrameRecomposer(coroutineContext)
90+
val surface = Surface.makeRasterN32Premul(100, 100)
91+
CanvasLayersComposeScene(
92+
frameRecomposer = frameRecomposer,
93+
size = IntSize(100, 100),
94+
invalidateDraw = { drawInvalidations++ },
95+
).use { scene ->
96+
scene.setContent {
97+
Box(Modifier.fillMaxSize())
98+
layer = rememberComposeSceneLayer(focusable = true)
99+
}
100+
101+
// Settle measure/layout/draw so every owner's pending-draw flag is cleared; otherwise
102+
// the close below would invalidate simply because an owner was still dirty.
103+
scene.measureAndLayout()
104+
scene.draw(surface.canvas.asComposeCanvas())
105+
assertFalse(scene.hasPendingMeasureOrLayout)
106+
assertFalse(scene.hasPendingDraw)
107+
108+
val drawInvalidationsBeforeClose = drawInvalidations
109+
layer!!.close()
110+
111+
assertTrue(scene.hasPendingMeasureOrLayout)
112+
assertTrue(scene.hasPendingDraw)
113+
assertTrue(
114+
drawInvalidations > drawInvalidationsBeforeClose,
115+
"Detaching a layer must request a draw pass to repaint the scene without it",
116+
)
117+
}
118+
frameRecomposer.close()
119+
}
82120
}

0 commit comments

Comments
 (0)