Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

### Fixes

- Session Replay: Fix Compose text masking mismatch with weighted text ([#5218](https://github.com/getsentry/sentry-java/pull/5218))

### Features

- Android: Add `beforeErrorSampling` callback to Session Replay ([#5214](https://github.com/getsentry/sentry-java/pull/5214))
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -202,7 +201,7 @@ internal object ComposeViewHierarchyNode {
TextViewHierarchyNode(
layout =
if (textLayoutResult != null && !isEditable && isLaidOut) {
ComposeTextLayout(textLayoutResult, hasFillModifier)
ComposeTextLayout(textLayoutResult)
} else {
null
},
Expand Down
Loading