Fix onFirstVisible modifier (#2233)

MatkovIvan · web-flow · commit 52e6edf26c88 · 2025-07-10T16:54:56.000+02:00
[CMP-8183](https://youtrack.jetbrains.com/issue/CMP-8183) Support calling of RectManager.updateOffsets [CMP-8544](https://youtrack.jetbrains.com/issue/CMP-8544) onFirstVisible Modifier doesn't work on non-Android platforms ## Release Notes ### Fixes - Multiple Platforms - _(prerelease fix)_ Fix trigger of `Modifier.onFirstVisible` modifier (added in Jetpack Compose `1.9.0-alpha03`)
diff --git a/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/node/RootNodeOwner.skiko.kt b/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/node/RootNodeOwner.skiko.kt
@@ -41,6 +41,7 @@ import androidx.compose.ui.geometry.Offset
 import androidx.compose.ui.geometry.Rect
 import androidx.compose.ui.geometry.isUnspecified
 import androidx.compose.ui.graphics.Canvas
+import androidx.compose.ui.graphics.Matrix
 import androidx.compose.ui.graphics.SkiaGraphicsContext
 import androidx.compose.ui.graphics.layer.GraphicsLayer
 import androidx.compose.ui.input.InputMode
@@ -89,8 +90,10 @@ import androidx.compose.ui.text.font.createFontFamilyResolver
 import androidx.compose.ui.text.input.TextInputService
 import androidx.compose.ui.unit.Constraints
 import androidx.compose.ui.unit.Density
+import androidx.compose.ui.unit.IntOffset
 import androidx.compose.ui.unit.IntSize
 import androidx.compose.ui.unit.LayoutDirection
+import androidx.compose.ui.unit.round
 import androidx.compose.ui.unit.toIntRect
 import androidx.compose.ui.unit.toRect
 import androidx.compose.ui.util.fastAll
@@ -159,19 +162,23 @@ internal class RootNodeOwner(
     private val pointerInputEventProcessor = PointerInputEventProcessor(owner.root)
     private val measureAndLayoutDelegate = MeasureAndLayoutDelegate(owner.root)
     private var isDisposed = false
-    private var currentCornersOnScreen: List<Offset>? = cornersOnScreen()
+
+    private var windowPosition: Offset? = null
+    private var globalPosition: Offset? = null
+
+    // TODO: Android assumes matrix for some APIs, so we need store something to avoid extra
+    //  allocations. Clean up this APIs and remove it.
+    private val identityMatrix = Matrix()
 
     init {
         snapshotObserver.startObserving()
         owner.root.attach(owner)
         platformContext.rootForTestListener?.onRootForTestCreated(rootForTest)
         onRootConstrainsChanged(size?.toConstraints())
-        onLightingInfoChanged()
+        updatePositionCacheAndDispatch()
         coroutineScope.launch {
             snapshotFlow { platformContext.windowInfo.containerSize }
-                .collect {
-                    onLightingInfoChanged()
-                }
+                .collect { updatePositionCacheAndDispatch() }
         }
     }
 
@@ -216,43 +223,65 @@ internal class RootNodeOwner(
 
     fun measureAndLayout() {
         owner.measureAndLayout(sendPointerUpdate = true)
+        updatePositionCacheAndDispatch()
     }
 
-    fun invalidatePositionInWindow() {
-        owner.root.layoutDelegate.measurePassDelegate.notifyChildrenUsingCoordinatesWhilePlacing()
-        measureAndLayoutDelegate.dispatchOnPositionedCallbacks(forceDispatch = true)
-        onLightingInfoChanged()
+    private fun updatePositionCacheAndDispatch() {
+        val globalPosition = platformContext.convertLocalToScreenPosition(Offset.Zero)
+        val hasGlobalPositionChanged = if (platformContext.hasNonTranslationComponents) {
+            this.globalPosition = null
+            true // Always invalidate in case of rotation, skew, etc.
+        } else if (globalPosition != this.globalPosition) {
+            this.globalPosition = globalPosition
+            true
+        } else false
+
+        val windowPosition = platformContext.convertLocalToWindowPosition(Offset.Zero)
+        val hasWindowPositionChanged = if (platformContext.hasNonTranslationComponents) {
+            this.windowPosition = null
+            true // Always invalidate in case of rotation, skew, etc.
+        } else if (windowPosition != this.windowPosition) {
+            this.windowPosition = windowPosition
+            true
+        } else false
+
+        if (hasGlobalPositionChanged || hasWindowPositionChanged) {
+            owner.root.layoutDelegate.measurePassDelegate.notifyChildrenUsingCoordinatesWhilePlacing()
+        }
+        val containerSize = platformContext.windowInfo.containerSize
+        owner.rectManager.updateOffsets(
+            screenOffset = globalPosition.round(),
+            windowOffset = windowPosition.round(),
+            viewToWindowMatrix = identityMatrix, // TODO: Replace viewToWindowMatrix to delegates
+            windowWidth = containerSize.width,
+            windowHeight = containerSize.height,
+        )
+        measureAndLayoutDelegate.dispatchOnPositionedCallbacks(
+            forceDispatch = hasGlobalPositionChanged || hasWindowPositionChanged
+        )
+        if (ComposeUiFlags.isRectTrackingEnabled) {
+            owner.rectManager.dispatchCallbacks()
+        }
+        if (hasWindowPositionChanged) {
+            graphicsContext.setLightingInfo(
+                canvasOffset = windowPosition,
+                density = density,
+                containerSize = containerSize
+            )
+        }
     }
 
-    private fun cornersOnScreen() = size?.let { size ->
-        val width = size.width.toFloat()
-        val height = size.height.toFloat()
-        val corners =
-            listOf(
-                Offset.Zero,
-                Offset(x = width, y = 0f),
-                Offset(x = 0f, y = height),
-                Offset(x = width, y = height)
-            ).fastMap {
-                platformContext.convertLocalToScreenPosition(it)
-            }
-        if (corners.fastAny { it.isUnspecified }) null else corners
+    fun invalidatePositionInWindow() {
+        updatePositionCacheAndDispatch()
     }
 
     fun invalidatePositionOnScreen() {
-        // Look at all corners, because platformContext.convertLocalToScreenPosition can also
-        // rotate, skew etc.
-        val cornersOnScreen = cornersOnScreen()
-        if (cornersOnScreen != currentCornersOnScreen) {
-            measureAndLayoutDelegate.dispatchOnPositionedCallbacks(forceDispatch = true)
-            currentCornersOnScreen = cornersOnScreen
-        }
+        updatePositionCacheAndDispatch()
     }
 
     fun draw(canvas: Canvas) = trace("RootNodeOwner:draw") {
         ownedLayerManager.draw(canvas)
         clearInvalidObservations()
-        @OptIn(ExperimentalComposeUiApi::class)
         if (ComposeUiFlags.isRectTrackingEnabled) {
             owner.rectManager.dispatchCallbacks()
         }
@@ -269,14 +298,6 @@ internal class RootNodeOwner(
         }
     }
 
-    private fun onLightingInfoChanged() {
-        graphicsContext.setLightingInfo(
-            canvasOffset = platformContext.convertLocalToWindowPosition(Offset.Zero),
-            density = density,
-            containerSize = platformContext.windowInfo.containerSize
-        )
-    }
-
     fun onCancelPointerInput() {
         pointerInputEventProcessor.processCancel()
     }
@@ -522,7 +543,6 @@ internal class RootNodeOwner(
                         snapshotInvalidationTracker.requestDraw()
                     }
                     measureAndLayoutDelegate.dispatchOnPositionedCallbacks()
-                    @OptIn(ExperimentalComposeUiApi::class)
                     if (ComposeUiFlags.isRectTrackingEnabled) {
                        rectManager.dispatchCallbacks()
                     }
@@ -540,7 +560,6 @@ internal class RootNodeOwner(
                 if (!measureAndLayoutDelegate.hasPendingMeasureOrLayout) {
                     measureAndLayoutDelegate.dispatchOnPositionedCallbacks()
                 }
-                @OptIn(ExperimentalComposeUiApi::class)
                 if (ComposeUiFlags.isRectTrackingEnabled) {
                     rectManager.dispatchCallbacks()
                 }
diff --git a/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/platform/PlatformContext.skiko.kt b/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/platform/PlatformContext.skiko.kt
@@ -68,6 +68,22 @@ interface PlatformContext {
      */
     val isWindowTransparent: Boolean get() = false
 
+    /**
+     * Indicates whether the transformation between local and window/screen coordinate spaces
+     * includes components other than simple translation (such as rotation, scaling, or skewing).
+     *
+     * When this property returns `true`:
+     * - Position calculations require full matrix transformations rather than simple offsets
+     * - Certain optimizations for translation-only transformations cannot be applied
+     *
+     * @return `true` if the transformation includes rotation, scaling, skewing, or other
+     *   non-translation components; `false` if only translation is used.
+     *
+     * @see convertLocalToWindowPosition
+     * @see convertWindowToLocalPosition
+     */
+    val hasNonTranslationComponents: Boolean get() = false
+
     /**
      * Converts [localPosition] relative to the [ComposeScene] into an [Offset] relative to
      * the containing window.
@@ -293,4 +309,3 @@ internal class DelegateRootForTestListener : PlatformContext.RootForTestListener
         }
     }
 }
-
diff --git a/compose/ui/ui/src/skikoTest/kotlin/androidx/compose/ui/layout/OnFirstVisibleTest.kt b/compose/ui/ui/src/skikoTest/kotlin/androidx/compose/ui/layout/OnFirstVisibleTest.kt
@@ -0,0 +1,70 @@
+/*
+ * Copyright 2025 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.layout
+
+import androidx.compose.foundation.layout.Box
+import androidx.compose.foundation.layout.fillMaxSize
+import androidx.compose.foundation.layout.size
+import androidx.compose.foundation.lazy.LazyColumn
+import androidx.compose.foundation.lazy.LazyListState
+import androidx.compose.foundation.lazy.items
+import androidx.compose.foundation.lazy.rememberLazyListState
+import androidx.compose.runtime.rememberCoroutineScope
+import androidx.compose.ui.Modifier
+import androidx.compose.ui.assertThat
+import androidx.compose.ui.geometry.Size
+import androidx.compose.ui.isEqualTo
+import androidx.compose.ui.test.ExperimentalTestApi
+import androidx.compose.ui.test.runSkikoComposeUiTest
+import androidx.compose.ui.unit.dp
+import kotlin.test.Test
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.launch
+
+@OptIn(ExperimentalTestApi::class)
+class OnFirstVisibleTest {
+
+    @Test
+    fun callOnFirstVisible() = runSkikoComposeUiTest(Size(100f, 200f)) {
+        class ItemState(
+            var value: Int = 0
+        )
+        val data = List(50) { ItemState(0) }
+        lateinit var lazyListState: LazyListState
+        lateinit var scope: CoroutineScope
+        setContent {
+            lazyListState = rememberLazyListState()
+            scope = rememberCoroutineScope()
+            LazyColumn(Modifier.fillMaxSize(), lazyListState) {
+                items(items = data, key = { it }) {
+                    Box(Modifier.size(100.dp).onFirstVisible {
+                        it.value++
+                    })
+                }
+            }
+        }
+
+        scope.launch {
+            repeat(50) {
+                lazyListState.animateScrollToItem(it)
+            }
+        }
+
+        waitForIdle()
+        assertThat(data.map { it.value }).isEqualTo(List(50) { 1 })
+    }
+}
