Apply compositionLocalContext, density and layoutDirection to layer i…

…mmediately in rememberComposeSceneLayer.
JetBrains · Apr 3, 2024 · 6315660 · 6315660
1 parent 07a6203
commit 6315660
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 12 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
@@ -19,12 +19,9 @@ package androidx.compose.ui.scene
 import androidx.compose.runtime.Composable
 import androidx.compose.runtime.CompositionLocalContext
 import androidx.compose.runtime.DisposableEffect
-import androidx.compose.runtime.SideEffect
 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
@@ -172,27 +169,24 @@ internal fun rememberComposeSceneLayer(
     val density = LocalDensity.current
     val layoutDirection = LocalLayoutDirection.current
     val parentComposition = rememberCompositionContext()
-    val compositionLocalContext by rememberUpdatedState(currentCompositionLocalContext)
+    val compositionLocalContext = currentCompositionLocalContext
     val layer = remember {
         scene.createLayer(
             density = density,
             layoutDirection = layoutDirection,
             focusable = focusable,
             compositionContext = parentComposition,
-        ).also {
-            it.compositionLocalContext = compositionLocalContext
-        }
+        )
     }
     layer.focusable = focusable
+    layer.compositionLocalContext = compositionLocalContext
+    layer.density = density
+    layer.layoutDirection = layoutDirection
+
     DisposableEffect(Unit) {
         onDispose {
             layer.close()
         }
     }
-    SideEffect {
-        layer.density = density
-        layer.layoutDirection = layoutDirection
-        layer.compositionLocalContext = compositionLocalContext
-    }
     return layer
 }
diff --git a/compose/ui/ui/src/skikoTest/kotlin/androidx/compose/ui/window/PopupTest.kt b/compose/ui/ui/src/skikoTest/kotlin/androidx/compose/ui/window/PopupTest.kt
@@ -20,6 +20,7 @@ import androidx.compose.foundation.layout.Box
 import androidx.compose.foundation.layout.fillMaxSize
 import androidx.compose.foundation.layout.offset
 import androidx.compose.foundation.layout.size
+import androidx.compose.material.Text
 import androidx.compose.runtime.Composable
 import androidx.compose.runtime.CompositionLocalProvider
 import androidx.compose.runtime.DisposableEffect
@@ -58,6 +59,7 @@ import androidx.compose.ui.test.assertIsDisplayed
 import androidx.compose.ui.test.assertPositionInRootIsEqualTo
 import androidx.compose.ui.test.assertWidthIsEqualTo
 import androidx.compose.ui.test.onNodeWithTag
+import androidx.compose.ui.test.runComposeUiTest
 import androidx.compose.ui.test.runSkikoComposeUiTest
 import androidx.compose.ui.touch
 import androidx.compose.ui.unit.Density
@@ -125,6 +127,41 @@ class PopupTest {
         assertThat(actualLocalValue).isEqualTo(3)
     }
 
+    // https://github.com/JetBrains/compose-multiplatform/issues/4558
+    @Test
+    fun changeInStaticCompositionLocalVisibleImmediatelyInPopup() = runComposeUiTest {
+        // Test that when the provided value of a staticCompositionLocalOf changes, the change is
+        // seen correctly in the content of a popup.
+        // The `text` variable is needed in order to cause a recomposition of the popup content in
+        // the same frame as CompositionLocalProvider is recomposed. Without this the bug doesn't
+        // reproduce because first CompositionLocalProvider is recomposed and that triggers
+        // recomposition of the popup content, which happens afterward.
+
+        val compositionLocal = staticCompositionLocalOf<Int> {
+            error("not set")
+        }
+        var providedValue by mutableStateOf(1)
+        var text by mutableStateOf("1")
+        var actualLocalValue = 0
+
+        setContent {
+            CompositionLocalProvider(compositionLocal provides providedValue) {
+                Popup {
+                    actualLocalValue = compositionLocal.current.also {
+                        println(it)
+                    }
+                    Text(text)
+                }
+            }
+        }
+
+        assertThat(actualLocalValue).isEqualTo(providedValue)
+        text = "2"
+        providedValue = 2
+        waitForIdle()
+        assertThat(actualLocalValue).isEqualTo(providedValue)
+    }
+
     // https://github.com/JetBrains/compose-multiplatform/issues/3142
     @Test
     fun passLayoutDirectionToPopup() = runSkikoComposeUiTest {