Skip to content

Commit bb3c121

Browse files
TorinAsakurameta-codesync[bot]
authored andcommitted
Fix Android text descender clipping with tight lineHeight (#57115)
Summary: Fixes #49886. Close torin-asakura/workspace#115. Android TextView clips text drawing to the view bounds in its default onDraw path. For explicit lineHeight, CustomLineHeightSpan should keep paragraph layout bounds tied to the requested lineHeight; using font top/bottom to avoid clipping fixes the single-line repro but expands multiline text. This keeps the line-height layout contract intact and draws ordinary non-selectable ReactTextView text directly when overflow is visible, avoiding TextView's internal clipRect. Selectable/linkify text and non-visible overflow continue to use the regular TextView drawing path. ## Changelog: [ANDROID] [FIXED] - Prevent descenders from being clipped when Android text lineHeight matches fontSize Pull Request resolved: #57115 Test Plan: Command: ```sh ./gradlew -Preact.internal.useHermesStable=true :packages:react-native:ReactAndroid:testDebugUnitTest --tests com.facebook.react.views.text.internal.span.CustomLineHeightSpanTest --tests com.facebook.react.views.text.ReactTextViewTest ``` Result: ```text BUILD SUCCESSFUL ``` Command: ```sh ./gradlew -Preact.internal.useHermesStable=true :packages:react-native:ReactAndroid:ktfmtCheck ``` Result: ```text BUILD SUCCESSFUL ``` Command: ```sh git diff --check ``` Result: no output. Command: ```sh ./node_modules/.bin/prettier --check packages/rn-tester/js/examples/Text/TextExample.android.js ``` Result: ```text All matched files use Prettier code style! ``` Command: ```sh ./node_modules/.bin/eslint --max-warnings 0 packages/rn-tester/js/examples/Text/TextExample.android.js ``` Result: no output. RNTester visual repro with fontSize: 24, lineHeight: 24, and gjpqy. Visual evidence: <details> <summary>Before this PR</summary> ![rn57115-before.png](https://github.com/user-attachments/assets/76c0a1a4-effe-4718-a92e-fb9a20c79a33) </details> <details> <summary>After this PR</summary> ![rn57115-after.png](https://github.com/user-attachments/assets/881b3ab3-a273-439d-9155-b4c9f393496d) </details> Reviewed By: cortinico Differential Revision: D108406457 Pulled By: javache fbshipit-source-id: be5e33ebca61dc24a3aa18376c49cf6690891d07
1 parent 91b2537 commit bb3c121

3 files changed

Lines changed: 330 additions & 21 deletions

File tree

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextView.java

Lines changed: 77 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -218,28 +218,16 @@ protected void onDraw(Canvas canvas) {
218218
if (layout != null) {
219219
CanvasEffectSpan[] drawSpans =
220220
spanned.getSpans(0, spanned.length(), CanvasEffectSpan.class);
221-
if (drawSpans.length > 0) {
222-
canvas.save();
223-
canvas.translate(getCompoundPaddingLeft(), getExtendedPaddingTop());
224-
for (CanvasEffectSpan span : drawSpans) {
225-
int start = spanned.getSpanStart(span);
226-
int end = spanned.getSpanEnd(span);
227-
span.onPreDraw(start, end, canvas, layout);
228-
}
229-
canvas.restore();
230-
231-
super.onDraw(canvas);
232-
233-
canvas.save();
234-
canvas.translate(getCompoundPaddingLeft(), getExtendedPaddingTop());
235-
for (CanvasEffectSpan span : drawSpans) {
236-
int start = spanned.getSpanStart(span);
237-
int end = spanned.getSpanEnd(span);
238-
span.onDraw(start, end, canvas, layout);
239-
}
240-
canvas.restore();
221+
if (shouldDrawLayoutWithoutTextViewClip()) {
222+
drawLayoutWithoutTextViewClip(canvas, spanned, layout, drawSpans);
241223
} else {
242-
super.onDraw(canvas);
224+
if (drawSpans.length > 0) {
225+
drawTextEffects(canvas, spanned, layout, drawSpans, true, false);
226+
super.onDraw(canvas);
227+
drawTextEffects(canvas, spanned, layout, drawSpans, false, false);
228+
} else {
229+
super.onDraw(canvas);
230+
}
243231
}
244232
} else {
245233
super.onDraw(canvas);
@@ -250,6 +238,74 @@ protected void onDraw(Canvas canvas) {
250238
}
251239
}
252240

241+
private boolean shouldDrawLayoutWithoutTextViewClip() {
242+
return mOverflow == Overflow.VISIBLE && !mTextIsSelectable && getMovementMethod() == null;
243+
}
244+
245+
private void drawLayoutWithoutTextViewClip(
246+
Canvas canvas, Spannable spanned, Layout layout, CanvasEffectSpan[] drawSpans) {
247+
getPaint().setColor(getCurrentTextColor());
248+
getPaint().drawableState = getDrawableState();
249+
250+
drawTextEffects(canvas, spanned, layout, drawSpans, true, true);
251+
252+
canvas.save();
253+
canvas.translate(
254+
getCompoundPaddingLeft(), getExtendedPaddingTop() + getVerticalGravityOffset(layout));
255+
layout.draw(canvas);
256+
canvas.restore();
257+
258+
drawTextEffects(canvas, spanned, layout, drawSpans, false, true);
259+
}
260+
261+
private void drawTextEffects(
262+
Canvas canvas,
263+
Spannable spanned,
264+
Layout layout,
265+
CanvasEffectSpan[] drawSpans,
266+
boolean beforeText,
267+
boolean includeVerticalGravityOffset) {
268+
if (drawSpans.length == 0) {
269+
return;
270+
}
271+
272+
canvas.save();
273+
canvas.translate(
274+
getCompoundPaddingLeft(),
275+
getExtendedPaddingTop()
276+
+ (includeVerticalGravityOffset ? getVerticalGravityOffset(layout) : 0));
277+
for (CanvasEffectSpan span : drawSpans) {
278+
int start = spanned.getSpanStart(span);
279+
int end = spanned.getSpanEnd(span);
280+
if (beforeText) {
281+
span.onPreDraw(start, end, canvas, layout);
282+
} else {
283+
span.onDraw(start, end, canvas, layout);
284+
}
285+
}
286+
canvas.restore();
287+
}
288+
289+
private int getVerticalGravityOffset(Layout layout) {
290+
int availableVerticalSpace = getAvailableVerticalSpace();
291+
if (layout.getHeight() >= availableVerticalSpace) {
292+
return 0;
293+
}
294+
295+
int verticalGravity = getGravity() & Gravity.VERTICAL_GRAVITY_MASK;
296+
if (verticalGravity == Gravity.BOTTOM) {
297+
return availableVerticalSpace - layout.getHeight();
298+
} else if (verticalGravity == Gravity.CENTER_VERTICAL) {
299+
return (availableVerticalSpace - layout.getHeight()) / 2;
300+
}
301+
302+
return 0;
303+
}
304+
305+
private int getAvailableVerticalSpace() {
306+
return getHeight() - getExtendedPaddingTop() - getExtendedPaddingBottom();
307+
}
308+
253309
@Override
254310
protected void onMeasure(int widthMeasureSpec, int heightMeasureSpec) {
255311
try (SystraceSection s = new SystraceSection("ReactTextView.onMeasure")) {
Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
/*
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*/
7+
8+
package com.facebook.react.views.text
9+
10+
import android.content.Context
11+
import android.graphics.Bitmap
12+
import android.graphics.Canvas
13+
import android.graphics.Color
14+
import android.graphics.Paint
15+
import android.text.SpannableString
16+
import android.text.Spanned
17+
import android.text.style.ReplacementSpan
18+
import android.util.TypedValue
19+
import android.view.Gravity
20+
import android.view.View
21+
import androidx.core.graphics.createBitmap
22+
import androidx.core.graphics.get
23+
import org.assertj.core.api.Assertions.assertThat
24+
import org.junit.Test
25+
import org.junit.runner.RunWith
26+
import org.robolectric.RobolectricTestRunner
27+
import org.robolectric.RuntimeEnvironment
28+
29+
@RunWith(RobolectricTestRunner::class)
30+
class ReactTextViewTest {
31+
32+
@Test
33+
fun drawsGlyphInkOutsideLineHeightWhenOverflowIsVisible() {
34+
val bitmap = drawReactTextViewWithOverflow(null)
35+
36+
assertThat(hasVisiblePixelBelowViewBounds(bitmap)).isTrue()
37+
}
38+
39+
@Test
40+
fun bottomGravityDoesNotShiftLayoutUpWhenTextIsTallerThanView() {
41+
val lineHeight = 48
42+
val viewHeight = 24
43+
val bitmap = drawReactTextViewWithOverflow(null, lineHeight, viewHeight, Gravity.BOTTOM)
44+
45+
assertThat(firstVisiblePixelY(bitmap)).isGreaterThanOrEqualTo(lineHeight)
46+
}
47+
48+
private fun drawReactTextViewWithOverflow(overflow: String?): Bitmap {
49+
return drawReactTextViewWithOverflow(overflow, lineHeight = 24, viewHeight = 24, gravity = null)
50+
}
51+
52+
private fun drawReactTextViewWithOverflow(
53+
overflow: String?,
54+
lineHeight: Int,
55+
viewHeight: Int,
56+
gravity: Int?,
57+
): Bitmap {
58+
val width = 200
59+
val bitmapHeight = 80
60+
val text = SpannableString("x")
61+
text.setSpan(
62+
OverflowingInkSpan(lineHeight),
63+
0,
64+
text.length,
65+
Spanned.SPAN_EXCLUSIVE_EXCLUSIVE,
66+
)
67+
68+
val view = TestReactTextView(RuntimeEnvironment.getApplication())
69+
view.setTextColor(Color.BLACK)
70+
view.setTextSize(TypedValue.COMPLEX_UNIT_PX, 24f)
71+
view.includeFontPadding = true
72+
view.setSpanned(text)
73+
view.text = text
74+
view.setOverflow(overflow)
75+
if (gravity != null) {
76+
view.setGravityVertical(gravity)
77+
}
78+
view.measure(
79+
View.MeasureSpec.makeMeasureSpec(width, View.MeasureSpec.EXACTLY),
80+
View.MeasureSpec.makeMeasureSpec(viewHeight, View.MeasureSpec.EXACTLY),
81+
)
82+
view.layout(0, 0, width, viewHeight)
83+
84+
return createBitmap(width, bitmapHeight).also {
85+
view.drawTextForTest(Canvas(it))
86+
}
87+
}
88+
89+
private fun hasVisiblePixelBelowViewBounds(bitmap: Bitmap): Boolean {
90+
for (y in 24 until bitmap.height) {
91+
for (x in 0 until bitmap.width) {
92+
if (Color.alpha(bitmap[x, y]) != 0) {
93+
return true
94+
}
95+
}
96+
}
97+
98+
return false
99+
}
100+
101+
private fun firstVisiblePixelY(bitmap: Bitmap): Int {
102+
for (y in 0 until bitmap.height) {
103+
for (x in 0 until bitmap.width) {
104+
if (Color.alpha(bitmap[x, y]) != 0) {
105+
return y
106+
}
107+
}
108+
}
109+
110+
return bitmap.height
111+
}
112+
113+
private class TestReactTextView(context: Context) : ReactTextView(context) {
114+
fun drawTextForTest(canvas: Canvas) {
115+
super.draw(canvas)
116+
}
117+
}
118+
119+
private class OverflowingInkSpan(private val lineHeight: Int) : ReplacementSpan() {
120+
override fun getSize(
121+
paint: Paint,
122+
text: CharSequence,
123+
start: Int,
124+
end: Int,
125+
fm: Paint.FontMetricsInt?,
126+
): Int {
127+
fm?.ascent = -lineHeight
128+
fm?.descent = 0
129+
fm?.top = -lineHeight
130+
fm?.bottom = 0
131+
return lineHeight
132+
}
133+
134+
override fun draw(
135+
canvas: Canvas,
136+
text: CharSequence,
137+
start: Int,
138+
end: Int,
139+
x: Float,
140+
top: Int,
141+
y: Int,
142+
bottom: Int,
143+
paint: Paint,
144+
) {
145+
canvas.drawRect(
146+
x,
147+
y + (lineHeight / 4f),
148+
x + lineHeight,
149+
y + (lineHeight / 2f),
150+
paint,
151+
)
152+
}
153+
}
154+
}
Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
/*
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*/
7+
8+
package com.facebook.react.views.text.internal.span
9+
10+
import android.graphics.Paint
11+
import android.text.Layout
12+
import android.text.SpannableString
13+
import android.text.Spanned
14+
import android.text.StaticLayout
15+
import android.text.TextPaint
16+
import org.assertj.core.api.Assertions.assertThat
17+
import org.junit.Test
18+
import org.junit.runner.RunWith
19+
import org.robolectric.RobolectricTestRunner
20+
21+
@RunWith(RobolectricTestRunner::class)
22+
class CustomLineHeightSpanTest {
23+
24+
@Test
25+
fun tightLineHeightDoesNotClipFirstOrLastLineFontBounds() {
26+
val span = CustomLineHeightSpan(16f)
27+
val fm =
28+
Paint.FontMetricsInt().apply {
29+
top = -18
30+
ascent = -14
31+
descent = 6
32+
bottom = 8
33+
}
34+
35+
span.chooseHeight("gjpqy", 0, 5, 0, 0, fm)
36+
37+
assertThat(fm.ascent).isEqualTo(-12)
38+
assertThat(fm.descent).isEqualTo(4)
39+
assertThat(fm.top).isEqualTo(-12)
40+
assertThat(fm.bottom).isEqualTo(4)
41+
}
42+
43+
@Test
44+
fun looseLineHeightStillExpandsFirstAndLastLineBounds() {
45+
val span = CustomLineHeightSpan(24f)
46+
val fm =
47+
Paint.FontMetricsInt().apply {
48+
top = -18
49+
ascent = -14
50+
descent = 6
51+
bottom = 8
52+
}
53+
54+
span.chooseHeight("gjpqy", 0, 5, 0, 0, fm)
55+
56+
assertThat(fm.ascent).isEqualTo(-16)
57+
assertThat(fm.descent).isEqualTo(8)
58+
assertThat(fm.top).isEqualTo(-16)
59+
assertThat(fm.bottom).isEqualTo(8)
60+
}
61+
62+
@Test
63+
fun tightLineHeightDoesNotExpandStaticLayoutHeightWithFontPadding() {
64+
val layout = buildStaticLayout("gjpqy\ngjpqy\ngjpqy", lineHeight = 24)
65+
66+
assertThat(layout.lineCount).isEqualTo(3)
67+
assertThat(layout.height).isEqualTo(72)
68+
}
69+
70+
@Test
71+
fun tightLineHeightDoesNotExpandSingleLineStaticLayoutHeightWithFontPadding() {
72+
val layout = buildStaticLayout("gjpqy", lineHeight = 24)
73+
74+
assertThat(layout.lineCount).isEqualTo(1)
75+
assertThat(layout.height).isEqualTo(24)
76+
}
77+
78+
private fun buildStaticLayout(text: String, lineHeight: Int): StaticLayout {
79+
val spannable = SpannableString(text)
80+
spannable.setSpan(
81+
CustomLineHeightSpan(lineHeight.toFloat()),
82+
0,
83+
text.length,
84+
Spanned.SPAN_EXCLUSIVE_EXCLUSIVE,
85+
)
86+
87+
return StaticLayout.Builder.obtain(
88+
spannable,
89+
0,
90+
spannable.length,
91+
TextPaint().apply { textSize = 24f },
92+
400,
93+
)
94+
.setAlignment(Layout.Alignment.ALIGN_NORMAL)
95+
.setIncludePad(true)
96+
.setLineSpacing(0f, 1f)
97+
.build()
98+
}
99+
}

0 commit comments

Comments
 (0)