Improve bounds calculation for ViewHierarchy and Gesture Target location

Author: markushi

sentry-compose/api/android/sentry-compose.api

@@ -6,6 +6,10 @@ public final class io/sentry/compose/BuildConfig {
 	public fun <init> ()V
 }
 
+public final class io/sentry/compose/SentryComposeHelperKt {
+	public static final fun boundsInWindow (Landroidx/compose/ui/layout/LayoutCoordinates;Landroidx/compose/ui/layout/LayoutCoordinates;)Landroidx/compose/ui/geometry/Rect;
+}
+
 public final class io/sentry/compose/SentryComposeTracingKt {
 	public static final fun SentryTraced (Ljava/lang/String;Landroidx/compose/ui/Modifier;ZLkotlin/jvm/functions/Function3;Landroidx/compose/runtime/Composer;II)V
 }
@@ -34,11 +38,7 @@ public final class io/sentry/compose/gestures/ComposeGestureTargetLocator$Compan
 
 public final class io/sentry/compose/viewhierarchy/ComposeViewHierarchyExporter : io/sentry/internal/viewhierarchy/ViewHierarchyExporter {
 	public static final field $stable I
-	public static final field Companion Lio/sentry/compose/viewhierarchy/ComposeViewHierarchyExporter$Companion;
 	public fun <init> (Lio/sentry/ILogger;)V
 	public fun export (Lio/sentry/protocol/ViewHierarchyNode;Ljava/lang/Object;)Z
 }
 
-public final class io/sentry/compose/viewhierarchy/ComposeViewHierarchyExporter$Companion {
-}
-

sentry-compose/src/androidMain/kotlin/io/sentry/compose/SentryComposeHelper.kt

@@ -3,21 +3,20 @@
 package io.sentry.compose
 
 import androidx.compose.ui.Modifier
+import androidx.compose.ui.geometry.Offset
 import androidx.compose.ui.geometry.Rect
-import androidx.compose.ui.layout.boundsInWindow
-import androidx.compose.ui.node.LayoutNode
-import androidx.compose.ui.node.LayoutNodeLayoutDelegate
+import androidx.compose.ui.layout.LayoutCoordinates
+import androidx.compose.ui.layout.findRootCoordinates
 import androidx.compose.ui.semantics.SemanticsModifier
 import io.sentry.ILogger
 import io.sentry.SentryLevel
 import java.lang.reflect.Field
 
 internal class SentryComposeHelper(private val logger: ILogger) {
 
-    private val layoutDelegateField: Field? =
-        loadField(logger, "androidx.compose.ui.node.LayoutNode", "layoutDelegate")
     private val testTagElementField: Field? =
         loadField(logger, "androidx.compose.ui.platform.TestTagElement", "tag")
+
     private val sentryTagElementField: Field? =
         loadField(logger, "io.sentry.compose.SentryModifier.SentryTagModifierNodeElement", "tag")
 
@@ -58,19 +57,6 @@ internal class SentryComposeHelper(private val logger: ILogger) {
         return null
     }
 
-    fun getLayoutNodeBoundsInWindow(node: LayoutNode): Rect? {
-        if (layoutDelegateField != null) {
-            try {
-                val delegate =
-                    layoutDelegateField[node] as LayoutNodeLayoutDelegate
-                return delegate.outerCoordinator.coordinates.boundsInWindow()
-            } catch (e: Exception) {
-                logger.log(SentryLevel.WARNING, "Could not fetch position for LayoutNode", e)
-            }
-        }
-        return null
-    }
-
     companion object {
         private fun loadField(
             logger: ILogger,
@@ -89,3 +75,90 @@ internal class SentryComposeHelper(private val logger: ILogger) {
         }
     }
 }
+
+
+/**
+ * Copied from sentry-android-replay/src/main/java/io/sentry/android/replay/util/Nodes.kt
+ *
+ * A faster copy of https://github.com/androidx/androidx/blob/fc7df0dd68466ac3bb16b1c79b7a73dd0bfdd4c1/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/layout/LayoutCoordinates.kt#L187
+ *
+ * Since we traverse the tree from the root, we don't need to find it again from the leaf node and
+ * just pass it as an argument.
+ *
+ * @return boundaries of this layout relative to the window's origin.
+ */
+public fun LayoutCoordinates.boundsInWindow(rootCoordinates: LayoutCoordinates?): Rect {
+    val root = rootCoordinates ?: findRootCoordinates()
+
+    val rootWidth = root.size.width.toFloat()
+    val rootHeight = root.size.height.toFloat()
+
+    val bounds = root.localBoundingBoxOf(this)
+    val boundsLeft = bounds.left.fastCoerceIn(0f, rootWidth)
+    val boundsTop = bounds.top.fastCoerceIn(0f, rootHeight)
+    val boundsRight = bounds.right.fastCoerceIn(0f, rootWidth)
+    val boundsBottom = bounds.bottom.fastCoerceIn(0f, rootHeight)
+
+    if (boundsLeft == boundsRight || boundsTop == boundsBottom) {
+        return Rect.Zero
+    }
+
+    val topLeft = root.localToWindow(Offset(boundsLeft, boundsTop))
+    val topRight = root.localToWindow(Offset(boundsRight, boundsTop))
+    val bottomRight = root.localToWindow(Offset(boundsRight, boundsBottom))
+    val bottomLeft = root.localToWindow(Offset(boundsLeft, boundsBottom))
+
+    val topLeftX = topLeft.x
+    val topRightX = topRight.x
+    val bottomLeftX = bottomLeft.x
+    val bottomRightX = bottomRight.x
+
+    val left = fastMinOf(topLeftX, topRightX, bottomLeftX, bottomRightX)
+    val right = fastMaxOf(topLeftX, topRightX, bottomLeftX, bottomRightX)
+
+    val topLeftY = topLeft.y
+    val topRightY = topRight.y
+    val bottomLeftY = bottomLeft.y
+    val bottomRightY = bottomRight.y
+
+    val top = fastMinOf(topLeftY, topRightY, bottomLeftY, bottomRightY)
+    val bottom = fastMaxOf(topLeftY, topRightY, bottomLeftY, bottomRightY)
+
+    return Rect(left, top, right, bottom)
+}
+
+/**
+ * Returns the smaller of the given values. If any value is NaN, returns NaN. Preferred over
+ * `kotlin.comparisons.minOf()` for 4 arguments as it avoids allocating an array because of the
+ * varargs.
+ */
+private inline fun fastMinOf(a: Float, b: Float, c: Float, d: Float): Float {
+    return minOf(a, minOf(b, minOf(c, d)))
+}
+
+/**
+ * Returns the largest of the given values. If any value is NaN, returns NaN. Preferred over
+ * `kotlin.comparisons.maxOf()` for 4 arguments as it avoids allocating an array because of the
+ * varargs.
+ */
+private inline fun fastMaxOf(a: Float, b: Float, c: Float, d: Float): Float {
+    return maxOf(a, maxOf(b, maxOf(c, d)))
+}
+
+/**
+ * Returns this float value clamped in the inclusive range defined by [minimumValue] and
+ * [maximumValue]. Unlike [Float.coerceIn], the range is not validated: the caller must ensure that
+ * [minimumValue] is less than [maximumValue].
+ */
+private inline fun Float.fastCoerceIn(minimumValue: Float, maximumValue: Float) =
+    this.fastCoerceAtLeast(minimumValue).fastCoerceAtMost(maximumValue)
+
+/** Ensures that this value is not less than the specified [minimumValue]. */
+private inline fun Float.fastCoerceAtLeast(minimumValue: Float): Float {
+    return if (this < minimumValue) minimumValue else this
+}
+
+/** Ensures that this value is not greater than the specified [maximumValue]. */
+private inline fun Float.fastCoerceAtMost(maximumValue: Float): Float {
+    return if (this > maximumValue) maximumValue else this
+}

sentry-compose/src/androidMain/kotlin/io/sentry/compose/gestures/ComposeGestureTargetLocator.kt

@@ -2,13 +2,15 @@
 
 package io.sentry.compose.gestures
 
+import androidx.compose.ui.geometry.Offset
 import androidx.compose.ui.node.LayoutNode
 import androidx.compose.ui.node.Owner
 import androidx.compose.ui.semantics.SemanticsModifier
 import io.sentry.ILogger
 import io.sentry.SentryIntegrationPackageStorage
 import io.sentry.compose.BuildConfig
 import io.sentry.compose.SentryComposeHelper
+import io.sentry.compose.boundsInWindow
 import io.sentry.internal.gestures.GestureTargetLocator
 import io.sentry.internal.gestures.UiElement
 import io.sentry.util.AutoClosableReentrantLock
@@ -33,22 +35,23 @@ public class ComposeGestureTargetLocator public constructor(private val logger:
         y: Float,
         targetType: UiElement.Type
     ): UiElement? {
-        // lazy init composeHelper as it's using some reflection under the hood
+        if (root !is Owner) {
+            return null
+        }
 
+        // lazy init composeHelper as it's using some reflection under the hood
         if (composeHelper == null) {
-            lock.acquire().use { ignored ->
+            lock.acquire().use {
                 if (composeHelper == null) {
                     composeHelper = SentryComposeHelper(logger)
                 }
             }
         }
 
-        if (root !is Owner) {
-            return null
-        }
+        val rootLayoutNode = root.root
 
         val queue: Queue<LayoutNode> = LinkedList()
-        queue.add(root.root)
+        queue.add(rootLayoutNode)
 
         // the final tag to return
         var targetTag: String? = null
@@ -57,9 +60,8 @@ public class ComposeGestureTargetLocator public constructor(private val logger:
         var lastKnownTag: String? = null
         while (!queue.isEmpty()) {
             val node = queue.poll() ?: continue
-
             if (node.isPlaced && layoutNodeBoundsContain(
-                    composeHelper!!,
+                    rootLayoutNode,
                     node,
                     x,
                     y
@@ -128,21 +130,17 @@ public class ComposeGestureTargetLocator public constructor(private val logger:
         }
     }
 
+    private fun layoutNodeBoundsContain(
+        root: LayoutNode,
+        node: LayoutNode,
+        x: Float,
+        y: Float
+    ): Boolean {
+        val bounds = node.coordinates.boundsInWindow(root.coordinates)
+        return bounds.contains(Offset(x, y))
+    }
+
     public companion object {
         private const val ORIGIN = "jetpack_compose"
-
-        private fun layoutNodeBoundsContain(
-            composeHelper: SentryComposeHelper,
-            node: LayoutNode,
-            x: Float,
-            y: Float
-        ): Boolean {
-            val bounds = composeHelper.getLayoutNodeBoundsInWindow(node)
-            return if (bounds == null) {
-                false
-            } else {
-                x >= bounds.left && x <= bounds.right && y >= bounds.top && y <= bounds.bottom
-            }
-        }
     }
 }

sentry-compose/src/androidMain/kotlin/io/sentry/compose/viewhierarchy/ComposeViewHierarchyExporter.kt

@@ -2,6 +2,7 @@
 
 package io.sentry.compose.viewhierarchy
 
+import androidx.compose.ui.layout.boundsInParent
 import androidx.compose.ui.node.LayoutNode
 import androidx.compose.ui.node.Owner
 import io.sentry.ILogger
@@ -31,84 +32,61 @@ public class ComposeViewHierarchyExporter public constructor(private val logger:
         }
 
         val rootNode = element.root
-        addChild(composeHelper!!, parent, null, rootNode)
+        addChild(composeHelper!!, parent, rootNode, rootNode)
         return true
     }
 
-    public companion object {
-        private fun addChild(
-            composeHelper: SentryComposeHelper,
-            parent: ViewHierarchyNode,
-            parentNode: LayoutNode?,
-            node: LayoutNode
-        ) {
-            if (node.isPlaced) {
-                val vhNode = ViewHierarchyNode()
-                setTag(composeHelper, node, vhNode)
-                setBounds(composeHelper, node, parentNode, vhNode)
+    private fun addChild(
+        composeHelper: SentryComposeHelper,
+        parent: ViewHierarchyNode,
+        rootNode: LayoutNode,
+        node: LayoutNode
+    ) {
+        if (node.isPlaced) {
+            val vhNode = ViewHierarchyNode()
+            setTag(composeHelper, node, vhNode)
+            setBounds(node, vhNode)
+            vhNode.type = vhNode.tag ?: "@Composable"
 
-                if (vhNode.tag != null) {
-                    vhNode.type = vhNode.tag
-                } else {
-                    vhNode.type = "@Composable"
-                }
-
-                if (parent.children == null) {
-                    parent.children = ArrayList()
-                }
-                parent.children!!.add(vhNode)
+            if (parent.children == null) {
+                parent.children = ArrayList()
+            }
+            parent.children!!.add(vhNode)
 
-                val children = node.zSortedChildren
-                val childrenCount = children.size
-                for (i in 0 until childrenCount) {
-                    val child = children[i]
-                    addChild(composeHelper, vhNode, node, child)
-                }
+            val children = node.zSortedChildren
+            val childrenCount = children.size
+            for (i in 0 until childrenCount) {
+                val child = children[i]
+                addChild(composeHelper, vhNode, rootNode, child)
             }
         }
+    }
 
-        private fun setTag(
-            helper: SentryComposeHelper,
-            node: LayoutNode,
-            vhNode: ViewHierarchyNode
-        ) {
-            // needs to be in-sync with ComposeGestureTargetLocator
-            val modifiers = node.getModifierInfo()
-            for (modifierInfo in modifiers) {
-                val tag = helper.extractTag(modifierInfo.modifier)
-                if (tag != null) {
-                    vhNode.tag = tag
-                }
+    private fun setTag(
+        helper: SentryComposeHelper,
+        node: LayoutNode,
+        vhNode: ViewHierarchyNode
+    ) {
+        // needs to be in-sync with ComposeGestureTargetLocator
+        val modifiers = node.getModifierInfo()
+        for (modifierInfo in modifiers) {
+            val tag = helper.extractTag(modifierInfo.modifier)
+            if (tag != null) {
+                vhNode.tag = tag
             }
         }
+    }
 
-        private fun setBounds(
-            composeHelper: SentryComposeHelper,
-            node: LayoutNode,
-            parentNode: LayoutNode?,
-            vhNode: ViewHierarchyNode
-        ) {
-            val nodeHeight = node.height
-            val nodeWidth = node.width
-
-            vhNode.height = nodeHeight.toDouble()
-            vhNode.width = nodeWidth.toDouble()
+    private fun setBounds(
+        node: LayoutNode,
+        vhNode: ViewHierarchyNode
+    ) {
+        // layout coordinates for view hierarchy are relative to the parent node
+        val bounds = node.coordinates.boundsInParent()
 
-            val bounds = composeHelper.getLayoutNodeBoundsInWindow(node)
-            if (bounds != null) {
-                var x = bounds.left.toDouble()
-                var y = bounds.top.toDouble()
-                // layout coordinates for view hierarchy are relative to the parent node
-                parentNode?.let {
-                    val parentBounds = composeHelper.getLayoutNodeBoundsInWindow(it)
-                    if (parentBounds != null) {
-                        x -= parentBounds.left.toDouble()
-                        y -= parentBounds.top.toDouble()
-                    }
-                }
-                vhNode.x = x
-                vhNode.y = y
-            }
-        }
+        vhNode.x = bounds.left.toDouble()
+        vhNode.y = bounds.top.toDouble()
+        vhNode.height = bounds.height.toDouble()
+        vhNode.width = bounds.width.toDouble()
     }
 }