Skip to content

Commit 34d0045

Browse files
Abbondanzometa-codesync[bot]
authored andcommitted
Skip empty text decoration draws
Summary: `drawSpannedDecoration` now skips zero-line layouts and empty or fully hidden spans before measuring or drawing. Add native regression coverage for the clamped ranges. Changelog: [Android][Fixed] - Skip empty and fully hidden text decoration ranges Differential Revision: D125038370
1 parent 8ee7e96 commit 34d0045

2 files changed

Lines changed: 102 additions & 24 deletions

File tree

‎packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/TextDecorationStyle.kt‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,12 @@ internal fun drawSpannedDecoration(
148148
style: TextDecorationStyle,
149149
yOffsetForLine: (paint: Paint, baseline: Float, thickness: Float) -> Float,
150150
) {
151+
if (layout.lineCount == 0) return
152+
val visibleEnd = visibleTextEnd(layout)
153+
val clampedStart = min(start, visibleEnd)
154+
val clampedEnd = min(end, visibleEnd)
155+
if (clampedStart >= clampedEnd) return
156+
151157
val textPaint = layout.paint
152158
val effectiveColor =
153159
if (color != Color.TRANSPARENT) {
@@ -194,10 +200,6 @@ internal fun drawSpannedDecoration(
194200
}
195201
}
196202

197-
val visibleEnd = visibleTextEnd(layout)
198-
val clampedStart = min(start, visibleEnd)
199-
val clampedEnd = min(end, visibleEnd)
200-
201203
val startLine = layout.getLineForOffset(clampedStart)
202204
val endLine = layout.getLineForOffset(clampedEnd)
203205
for (line in startLine..endLine) {

‎packages/react-native/ReactAndroid/src/test/java/com/facebook/react/views/text/TextDecorationStyleTest.kt‎

Lines changed: 96 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -7,24 +7,34 @@
77

88
package com.facebook.react.views.text
99

10+
import android.graphics.Bitmap
1011
import android.graphics.Canvas
1112
import android.graphics.Color
1213
import android.os.Build
14+
import android.text.Layout
1315
import android.text.StaticLayout
1416
import android.text.TextPaint
1517
import android.text.TextUtils
1618
import androidx.annotation.RequiresApi
19+
import androidx.core.graphics.createBitmap
1720
import org.assertj.core.api.Assertions.assertThat
1821
import org.junit.Test
1922
import org.junit.runner.RunWith
2023
import org.mockito.kotlin.any
2124
import org.mockito.kotlin.eq
2225
import org.mockito.kotlin.mock
2326
import org.mockito.kotlin.verify
27+
import org.mockito.kotlin.verifyNoInteractions
2428
import org.robolectric.RobolectricTestRunner
29+
import org.robolectric.annotation.Config
30+
import org.robolectric.annotation.GraphicsMode
31+
import org.robolectric.annotation.TextLayoutMode
2532

26-
@RequiresApi(Build.VERSION_CODES.M)
33+
@RequiresApi(Build.VERSION_CODES.Q)
2734
@RunWith(RobolectricTestRunner::class)
35+
@Config(sdk = [Build.VERSION_CODES.UPSIDE_DOWN_CAKE, Build.VERSION_CODES.VANILLA_ICE_CREAM])
36+
@GraphicsMode(GraphicsMode.Mode.NATIVE)
37+
@TextLayoutMode(TextLayoutMode.Mode.REALISTIC)
2838
class TextDecorationStyleTest {
2939
@Test
3040
fun fromStringSolid() {
@@ -73,11 +83,9 @@ class TextDecorationStyleTest {
7383
val baseline = layout.getLineBaseline(0).toFloat()
7484
val x1 = layout.getPrimaryHorizontal(0)
7585
val x2 = layout.getPrimaryHorizontal(visibleEnd)
86+
val y = baseline + layout.paint.underlineThickness + 1f
7687
val canvas = mock<Canvas>()
7788

78-
// The span covers the whole string, well past what survived the ellipsis; pre-fix this
79-
// called layout.getPrimaryHorizontal(TAIL_TEXT.length) and crashed with
80-
// IndexOutOfBoundsException, since the ellipsized layout only resolves up to visibleEnd.
8189
drawSpannedDecoration(
8290
0,
8391
TAIL_TEXT.length,
@@ -89,31 +97,99 @@ class TextDecorationStyleTest {
8997
lineBaseline + thickness + 1f
9098
}
9199

92-
verify(canvas).drawLine(eq(x1), eq(baseline + 1f), eq(x2), eq(baseline + 1f), any())
100+
verify(canvas).drawLine(eq(x1), eq(y), eq(x2), eq(y), any())
93101
}
94102

95103
@Test
96104
fun drawSpannedDecorationSkipsSpanEntirelyPastTailEllipsis() {
97105
val layout = buildTailEllipsizedLayout()
98106
val visibleEnd = layout.getLineStart(0) + layout.getEllipsisStart(0)
99-
val baseline = layout.getLineBaseline(0).toFloat()
100-
val x = layout.getPrimaryHorizontal(visibleEnd)
101-
val canvas = mock<Canvas>()
102107

103-
// The whole span (e.g. a nested Text) starts after the ellipsis, fully hidden: it must
104-
// collapse to a zero-length line at the visible boundary, not draw anything past it.
105-
drawSpannedDecoration(
106-
visibleEnd,
107-
TAIL_TEXT.length,
108-
canvas,
109-
layout,
110-
Color.BLACK,
111-
TextDecorationStyle.SOLID,
112-
) { _, lineBaseline, thickness ->
113-
lineBaseline + thickness + 1f
108+
for (style in TextDecorationStyle.entries) {
109+
val canvas = mock<Canvas>()
110+
drawSpannedDecoration(visibleEnd, TAIL_TEXT.length, canvas, layout, Color.BLACK, style) {
111+
_,
112+
lineBaseline,
113+
thickness ->
114+
lineBaseline + thickness + 1f
115+
}
116+
verifyNoInteractions(canvas)
114117
}
118+
}
119+
120+
@Test
121+
fun skipsDecorationsOutsideTruncatedLayout() {
122+
val text =
123+
"Visible first line\n\nThird paragraph\n\nHidden decorated text: decorated text outside the one-line layout"
124+
val layout = buildLayout(text, text.indexOf('\n'))
125+
val start = text.indexOf("decorated text outside")
126+
assertThat(layout.lineCount).isEqualTo(1)
127+
assertThat(layout.text.length).isEqualTo(text.length)
128+
assertThat(layout.getLineEnd(0)).isLessThan(start)
129+
130+
for (style in TextDecorationStyle.entries) {
131+
assertThat(drawDecoration(start, text.length, layout, style)).isZero()
132+
}
133+
}
115134

116-
verify(canvas).drawLine(eq(x), eq(baseline + 1f), eq(x), eq(baseline + 1f), any())
135+
@Test
136+
fun clipsDecorationsCrossingTruncatedLayout() {
137+
val text = "Visible first line\nHidden decorated text"
138+
val layout = buildLayout(text, text.indexOf('\n'))
139+
assertThat(layout.getLineEnd(0)).isLessThan(text.length)
140+
141+
for (style in TextDecorationStyle.entries) {
142+
assertThat(drawDecoration(8, text.length, layout, style)).isEqualTo(1)
143+
}
144+
}
145+
146+
@Test
147+
fun keepsVisibleDecorations() {
148+
val layout = buildLayout("Visible first line")
149+
150+
for (style in TextDecorationStyle.entries) {
151+
assertThat(drawDecoration(0, 7, layout, style)).isEqualTo(1)
152+
}
153+
}
154+
155+
@Test
156+
fun skipsEmptyDecorations() {
157+
val layout = buildLayout("Visible first line")
158+
assertThat(drawDecoration(0, 0, layout, TextDecorationStyle.SOLID)).isZero()
159+
}
160+
161+
@Test
162+
fun skipsZeroLineLayout() {
163+
val layout = mock<Layout>()
164+
assertThat(drawDecoration(0, 1, layout, TextDecorationStyle.SOLID)).isZero()
165+
}
166+
167+
@Test
168+
fun keepsUntruncatedDecorations() {
169+
val text = "Visible first line\nHidden decorated text"
170+
val layout = buildLayout(text)
171+
172+
for (style in TextDecorationStyle.entries) {
173+
assertThat(drawDecoration(8, text.length, layout, style)).isEqualTo(2)
174+
}
175+
}
176+
177+
private fun buildLayout(text: String, end: Int = text.length): StaticLayout =
178+
StaticLayout.Builder.obtain(text, 0, end, TextPaint().apply { textSize = 20f }, 400).build()
179+
180+
private fun drawDecoration(
181+
start: Int,
182+
end: Int,
183+
layout: Layout,
184+
style: TextDecorationStyle,
185+
): Int {
186+
val canvas = Canvas(createBitmap(400, 200, Bitmap.Config.ARGB_8888))
187+
var drawnLines = 0
188+
drawSpannedDecoration(start, end, canvas, layout, Color.BLACK, style) { _, baseline, _ ->
189+
drawnLines++
190+
baseline
191+
}
192+
return drawnLines
117193
}
118194

119195
/**

0 commit comments

Comments
 (0)