Work around erroneous paragraph return values

Author: markushi

sentry-android-core/src/test/resources/snapshots/ScreenshotEventProcessorTest/screenshot_mask_all.png

@@ -13,38 +13,39 @@ import androidx.compose.ui.node.LayoutNode
 import androidx.compose.ui.text.TextLayoutResult
 import kotlin.math.roundToInt
 
-internal class ComposeTextLayout(
-  internal val layout: TextLayoutResult,
-  private val hasFillModifier: Boolean,
-) : TextLayout {
+internal class ComposeTextLayout(internal val layout: TextLayoutResult) : TextLayout {
   override val lineCount: Int
     get() = layout.lineCount
 
   override val dominantTextColor: Int?
     get() = null
 
-  override fun getPrimaryHorizontal(line: Int, offset: Int): Float {
-    val horizontalPos = layout.getHorizontalPosition(offset, usePrimaryDirection = true)
-    // when there's no `fill` modifier on a Text composable, compose still thinks that there's
-    // one and wrongly calculates horizontal position relative to node's start, not text's start
-    // for some reason. This is only the case for single-line text (multiline works fien).
-    // So we subtract line's left to get the correct position
-    return if (!hasFillModifier && lineCount == 1) {
-      horizontalPos - layout.getLineLeft(line)
-    } else {
-      horizontalPos
+  /**
+   * The paragraph may be laid out with a wider width (constraint maxWidth) than the actual node
+   * (layout result size). When that happens, getLineLeft/getLineRight return positions in the
+   * paragraph coordinate system, which don't match the node's bounds. In that case, text alignment
+   * has no visible effect, so we fall back to using line width starting from x=0.
+   */
+  private val paragraphWidthExceedsNode: Boolean
+    get() = layout.multiParagraph.width > layout.size.width
+
+  override fun getLineLeft(line: Int): Float {
+    if (paragraphWidthExceedsNode) {
+      return 0f
     }
+    return layout.getLineLeft(line)
   }
 
-  override fun getEllipsisCount(line: Int): Int = if (layout.isLineEllipsized(line)) 1 else 0
-
-  override fun getLineVisibleEnd(line: Int): Int = layout.getLineEnd(line, visibleEnd = true)
+  override fun getLineRight(line: Int): Float {
+    if (paragraphWidthExceedsNode) {
+      return layout.multiParagraph.getLineWidth(line)
+    }
+    return layout.getLineRight(line)
+  }
 
   override fun getLineTop(line: Int): Int = layout.getLineTop(line).roundToInt()
 
   override fun getLineBottom(line: Int): Int = layout.getLineBottom(line).roundToInt()
-
-  override fun getLineStart(line: Int): Int = layout.getLineStart(line)
 }
 
 // TODO: probably most of the below we can do via bytecode instrumentation and speed up at runtime
@@ -92,46 +93,31 @@ internal fun Painter.isMaskable(): Boolean {
     !className.contains("Brush")
 }
 
-internal data class TextAttributes(val color: Color?, val hasFillModifier: Boolean)
-
 /**
  * This method is necessary to mask text in Compose.
  *
  * We heuristically look up for classes that have a [Text] modifier, usually they all have a `Text`
  * string in their name, e.g. TextStringSimpleElement or TextAnnotatedStringElement. We then get the
  * color from the modifier, to be able to mask it with the correct color.
  *
- * We also look up for classes that have a [Fill] modifier, usually they all have a `Fill` string in
- * their name, e.g. FillElement. This is necessary to workaround a Compose bug where single-line
- * text composable without a `fill` modifier still thinks that there's one and wrongly calculates
- * horizontal position.
- *
  * We also add special proguard rules to keep the `Text` class names and their `color` member.
  */
-internal fun LayoutNode.findTextAttributes(): TextAttributes {
+internal fun LayoutNode.findTextColor(): Color? {
   val modifierInfos = getModifierInfo()
-  var color: Color? = null
-  var hasFillModifier = false
   for (index in modifierInfos.indices) {
     val modifier = modifierInfos[index].modifier
     val modifierClassName = modifier::class.java.name
     if (modifierClassName.contains("Text")) {
-      color =
-        try {
-          (modifier::class
-              .java
-              .getDeclaredField("color")
-              .apply { isAccessible = true }
-              .get(modifier) as? ColorProducer)
-            ?.invoke()
-        } catch (e: Throwable) {
-          null
-        }
-    } else if (modifierClassName.contains("Fill")) {
-      hasFillModifier = true
+      return try {
+        (modifier::class.java.getDeclaredField("color").apply { isAccessible = true }.get(modifier)
+            as? ColorProducer)
+          ?.invoke()
+      } catch (e: Throwable) {
+        null
+      }
     }
   }
-  return TextAttributes(color, hasFillModifier)
+  return null
 }
 
 /**

sentry-android-core/src/test/resources/snapshots/ScreenshotEventProcessorTest/screenshot_mask_text.png

@@ -13,15 +13,11 @@ internal interface TextLayout {
    */
   val dominantTextColor: Int?
 
-  fun getPrimaryHorizontal(line: Int, offset: Int): Float
+  fun getLineLeft(line: Int): Float
 
-  fun getEllipsisCount(line: Int): Int
-
-  fun getLineVisibleEnd(line: Int): Int
+  fun getLineRight(line: Int): Float
 
   fun getLineTop(line: Int): Int
 
   fun getLineBottom(line: Int): Int
-
-  fun getLineStart(line: Int): Int
 }

sentry-android-replay/src/main/java/io/sentry/android/replay/util/Nodes.kt

@@ -128,21 +128,14 @@ internal fun TextLayout?.getVisibleRects(
 
   val rects = mutableListOf<Rect>()
   for (i in 0 until lineCount) {
-    val lineStart = getPrimaryHorizontal(i, getLineStart(i)).toInt()
-    val ellipsisCount = getEllipsisCount(i)
-    val lineVisibleEnd = getLineVisibleEnd(i)
-    var lineEnd =
-      getPrimaryHorizontal(i, lineVisibleEnd - ellipsisCount + if (ellipsisCount > 0) 1 else 0)
-        .toInt()
-    if (lineEnd == 0 && lineVisibleEnd > 0) {
-      // looks like the case for when emojis are present in text
-      lineEnd = getPrimaryHorizontal(i, lineVisibleEnd - 1).toInt() + 1
-    }
+    val lineLeft = getLineLeft(i).toInt()
+    val lineRight = getLineRight(i).toInt()
     val lineTop = getLineTop(i)
     val lineBottom = getLineBottom(i)
     val rect = Rect()
-    rect.left = globalRect.left + paddingLeft + lineStart
-    rect.right = rect.left + (lineEnd - lineStart)
+
+    rect.left = globalRect.left + paddingLeft + lineLeft
+    rect.right = globalRect.left + paddingLeft + lineRight
     rect.top = globalRect.top + paddingTop + lineTop
     rect.bottom = rect.top + (lineBottom - lineTop)
 
@@ -197,18 +190,13 @@ internal class AndroidTextLayout(private val layout: Layout) : TextLayout {
       return dominantColor?.toOpaque()
     }
 
-  override fun getPrimaryHorizontal(line: Int, offset: Int): Float =
-    layout.getPrimaryHorizontal(offset)
+  override fun getLineLeft(line: Int): Float = layout.getLineLeft(line)
 
-  override fun getEllipsisCount(line: Int): Int = layout.getEllipsisCount(line)
-
-  override fun getLineVisibleEnd(line: Int): Int = layout.getLineVisibleEnd(line)
+  override fun getLineRight(line: Int): Float = layout.getLineRight(line)
 
   override fun getLineTop(line: Int): Int = layout.getLineTop(line)
 
   override fun getLineBottom(line: Int): Int = layout.getLineBottom(line)
-
-  override fun getLineStart(line: Int): Int = layout.getLineStart(line)
 }
 
 internal fun View?.addOnDrawListenerSafe(listener: ViewTreeObserver.OnDrawListener) {

sentry-android-replay/src/main/java/io/sentry/android/replay/util/TextLayout.kt

@@ -24,7 +24,7 @@ import io.sentry.android.replay.SentryReplayModifiers
 import io.sentry.android.replay.util.ComposeTextLayout
 import io.sentry.android.replay.util.boundsInWindow
 import io.sentry.android.replay.util.findPainter
-import io.sentry.android.replay.util.findTextAttributes
+import io.sentry.android.replay.util.findTextColor
 import io.sentry.android.replay.util.isMaskable
 import io.sentry.android.replay.util.toOpaque
 import io.sentry.android.replay.viewhierarchy.ViewHierarchyNode.GenericViewHierarchyNode
@@ -189,11 +189,10 @@ internal object ComposeViewHierarchyNode {
           ?.action
           ?.invoke(textLayoutResults)
 
-        val (color, hasFillModifier) = node.findTextAttributes()
         val textLayoutResult = textLayoutResults.firstOrNull()
         var textColor = textLayoutResult?.layoutInput?.style?.color
         if (textColor?.isUnspecified == true) {
-          textColor = color
+          textColor = node.findTextColor()
         }
         val isLaidOut = textLayoutResult?.layoutInput?.style?.fontSize != TextUnit.Unspecified
         // TODO: support editable text (currently there's a way to get @Composable's padding only
@@ -202,7 +201,7 @@ internal object ComposeViewHierarchyNode {
         TextViewHierarchyNode(
           layout =
             if (textLayoutResult != null && !isEditable && isLaidOut) {
-              ComposeTextLayout(textLayoutResult, hasFillModifier)
+              ComposeTextLayout(textLayoutResult)
             } else {
               null
             },