Set the ComposeSceneLayer's content in DisposableEffect rather th…

…an in the composition.
JetBrains · Apr 16, 2024 · f297f73 · f297f73
1 parent 1c13834
commit f297f73
Show file tree

Hide file tree

Showing 4 changed files with 69 additions and 20 deletions.
diff --git a/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/scene/ComposeSceneLayer.skiko.kt b/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/scene/ComposeSceneLayer.skiko.kt
@@ -20,8 +20,10 @@ import androidx.compose.runtime.Composable
 import androidx.compose.runtime.CompositionLocalContext
 import androidx.compose.runtime.DisposableEffect
 import androidx.compose.runtime.currentCompositionLocalContext
+import androidx.compose.runtime.getValue
 import androidx.compose.runtime.remember
 import androidx.compose.runtime.rememberCompositionContext
+import androidx.compose.runtime.rememberUpdatedState
 import androidx.compose.ui.InternalComposeUiApi
 import androidx.compose.ui.graphics.Color
 import androidx.compose.ui.input.key.KeyEvent
@@ -190,3 +192,17 @@ internal fun rememberComposeSceneLayer(
     }
     return layer
 }
+
+/**
+ * Sets the content of the layer to [content].
+ */
+@Composable
+internal fun ComposeSceneLayer.applyContent(content: @Composable () -> Unit) {
+    val currentContent by rememberUpdatedState(content)
+    DisposableEffect(this) {
+        setContent {
+            currentContent()
+        }
+        onDispose {  }
+    }
+}
diff --git a/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/window/Dialog.skiko.kt b/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/window/Dialog.skiko.kt
@@ -18,7 +18,9 @@ package androidx.compose.ui.window
 
 import androidx.compose.runtime.Composable
 import androidx.compose.runtime.Immutable
+import androidx.compose.runtime.getValue
 import androidx.compose.runtime.remember
+import androidx.compose.runtime.rememberUpdatedState
 import androidx.compose.ui.ExperimentalComposeUiApi
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.graphics.BlendMode
@@ -35,13 +37,13 @@ import androidx.compose.ui.platform.PlatformInsets
 import androidx.compose.ui.platform.PlatformInsetsConfig
 import androidx.compose.ui.platform.union
 import androidx.compose.ui.scene.ComposeSceneLayer
+import androidx.compose.ui.scene.applyContent
 import androidx.compose.ui.scene.rememberComposeSceneLayer
 import androidx.compose.ui.semantics.dialog
 import androidx.compose.ui.semantics.semantics
 import androidx.compose.ui.unit.IntRect
 import androidx.compose.ui.unit.IntSize
 import androidx.compose.ui.unit.center
-import androidx.compose.ui.unit.dp
 
 /**
  * The default scrim opacity.
@@ -181,14 +183,15 @@ private fun DialogLayout(
     onOutsidePointerEvent: ((eventType: PointerEventType) -> Unit)? = null,
     content: @Composable () -> Unit
 ) {
+    val currentContent by rememberUpdatedState(content)
     val platformInsets = properties.platformInsets
     val layer = rememberComposeSceneLayer(
         focusable = true
     )
     layer.scrimColor = properties.scrimColor
     layer.setKeyEventListener(onPreviewKeyEvent, onKeyEvent)
     layer.setOutsidePointerEventListener(onOutsidePointerEvent)
-    rememberLayerContent(layer) {
+    layer.applyContent {
         val containerSize = LocalWindowInfo.current.containerSize
         val measurePolicy = rememberDialogMeasurePolicy(
             layer = layer,
@@ -201,7 +204,7 @@ private fun DialogLayout(
             ime = properties.useSoftwareKeyboardInset,
         ) {
             Layout(
-                content = content,
+                content = currentContent,
                 modifier = modifier,
                 measurePolicy = measurePolicy
             )
@@ -224,13 +227,6 @@ private val DialogProperties.platformInsets: PlatformInsets
         return safeInsets.union(ime)
     }
 
-@Composable
-private fun rememberLayerContent(layer: ComposeSceneLayer, content: @Composable () -> Unit) {
-    remember(layer, content) {
-        layer.setContent(content)
-    }
-}
-
 @Composable
 private fun rememberDialogMeasurePolicy(
     layer: ComposeSceneLayer,

diff --git a/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/window/Popup.skiko.kt b/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/window/Popup.skiko.kt
@@ -41,6 +41,7 @@ import androidx.compose.ui.platform.LocalWindowInfo
 import androidx.compose.ui.platform.PlatformInsets
 import androidx.compose.ui.platform.PlatformInsetsConfig
 import androidx.compose.ui.scene.ComposeSceneLayer
+import androidx.compose.ui.scene.applyContent
 import androidx.compose.ui.scene.rememberComposeSceneLayer
 import androidx.compose.ui.semantics.popup
 import androidx.compose.ui.semantics.semantics
@@ -438,6 +439,7 @@ private fun PopupLayout(
     onOutsidePointerEvent: ((eventType: PointerEventType) -> Unit)? = null,
     content: @Composable () -> Unit
 ) {
+    val currentContent by rememberUpdatedState(content)
     val platformInsets = properties.platformInsets
     var layoutParentBoundsInWindow: IntRect? by remember { mutableStateOf(null) }
     EmptyLayout(Modifier.parentBoundsInWindow { layoutParentBoundsInWindow = it })
@@ -446,8 +448,8 @@ private fun PopupLayout(
     )
     layer.setKeyEventListener(onPreviewKeyEvent, onKeyEvent)
     layer.setOutsidePointerEventListener(onOutsidePointerEvent)
-    rememberLayerContent(layer) {
-        val parentBoundsInWindow = layoutParentBoundsInWindow ?: return@rememberLayerContent
+    layer.applyContent {
+        val parentBoundsInWindow = layoutParentBoundsInWindow ?: return@applyContent
         val containerSize = LocalWindowInfo.current.containerSize
         val layoutDirection = LocalLayoutDirection.current
         val measurePolicy = rememberPopupMeasurePolicy(
@@ -464,7 +466,7 @@ private fun PopupLayout(
             ime = false,
         ) {
             Layout(
-                content = content,
+                content = currentContent,
                 modifier = modifier,
                 measurePolicy = measurePolicy
             )
@@ -479,13 +481,6 @@ private val PopupProperties.platformInsets: PlatformInsets
         PlatformInsets.Zero
     }
 
-@Composable
-private fun rememberLayerContent(layer: ComposeSceneLayer, content: @Composable () -> Unit) {
-    remember(layer, content) {
-        layer.setContent(content)
-    }
-}
-
 private fun Modifier.parentBoundsInWindow(
     onBoundsChanged: (IntRect) -> Unit
 ) = this.onGloballyPositioned { childCoordinates ->

diff --git a/compose/ui/ui/src/skikoTest/kotlin/androidx/compose/ui/window/DialogTest.kt b/compose/ui/ui/src/skikoTest/kotlin/androidx/compose/ui/window/DialogTest.kt
@@ -18,7 +18,11 @@ package androidx.compose.ui.window
 
 import androidx.compose.foundation.layout.Box
 import androidx.compose.foundation.layout.size
+import androidx.compose.runtime.LaunchedEffect
+import androidx.compose.runtime.getValue
 import androidx.compose.runtime.mutableStateOf
+import androidx.compose.runtime.remember
+import androidx.compose.runtime.setValue
 import androidx.compose.ui.DialogState
 import androidx.compose.ui.FillBox
 import androidx.compose.ui.Modifier
@@ -30,15 +34,20 @@ import androidx.compose.ui.input.pointer.PointerButton
 import androidx.compose.ui.input.pointer.PointerButtons
 import androidx.compose.ui.input.pointer.PointerEventType
 import androidx.compose.ui.test.ExperimentalTestApi
+import androidx.compose.ui.test.InternalTestApi
 import androidx.compose.ui.test.assertIsDisplayed
 import androidx.compose.ui.test.assertPositionInRootIsEqualTo
 import androidx.compose.ui.test.onNodeWithTag
+import androidx.compose.ui.test.runInternalSkikoComposeUiTest
 import androidx.compose.ui.test.runSkikoComposeUiTest
 import androidx.compose.ui.touch
 import androidx.compose.ui.unit.IntSize
 import androidx.compose.ui.unit.dp
 import kotlin.test.Test
+import kotlin.test.assertEquals
 import kotlin.test.fail
+import kotlinx.coroutines.delay
+import kotlinx.coroutines.test.StandardTestDispatcher
 
 @OptIn(ExperimentalTestApi::class)
 class DialogTest {
@@ -208,4 +217,37 @@ class DialogTest {
         scene.sendPointerEvent(PointerEventType.Press, Offset(10f, 10f), buttons = buttons, button = PointerButton.Secondary)
         scene.sendPointerEvent(PointerEventType.Release, Offset(10f, 10f), button = PointerButton.Secondary)
     }
+
+    @OptIn(InternalTestApi::class)
+    @Test
+    fun dialogCompositionSubscribesToStateChangesImmediately() = runInternalSkikoComposeUiTest(
+        coroutineDispatcher = StandardTestDispatcher()
+    ) {
+        // https://github.com/JetBrains/compose-multiplatform/issues/4609
+        var showDialog by mutableStateOf(false)
+        var lastValueInComposition: Int? = null
+        setContent {
+            if (showDialog) {
+                Dialog(onDismissRequest = { showDialog = false }) {
+                    // https://issuetracker.google.com/issues/334996925
+                    var value by remember {
+                        mutableStateOf(0)
+                            .also {
+                                it.value = 1
+                            }
+                    }
+                    lastValueInComposition = value
+                    LaunchedEffect(Unit) {
+                        delay(1000)
+                        value = 2
+                    }
+                }
+            }
+        }
+
+        assertEquals(null, lastValueInComposition)
+        showDialog = true
+        waitForIdle()
+        assertEquals(2, lastValueInComposition)
+    }
 }