Permalink
Browse files

Fix Text incorrect line height

Summary:
Setting the line height with the help of Android-provided StaticLayout is incorrect. A
simple example app will display the following when `setLineSpacing(50.f, 0.f)`
is set: {F62987699}. You'll notice that the height of the first line is a few
pixels shorter than the other lines.
So we use a custom LineHeightSpan instead, which needs to be applied to the text
itself, and no height-related attributes need to be set on the TextView itself.

Reviewed By: lexs

Differential Revision: D3841658

fbshipit-source-id: 7257df4f1b2ce037554c7a7a5ca8f547a2056939
  • Loading branch information...
1 parent 16bdbee commit c79f6177426a417299b5eb13d483613c97b303b3 @andreicoman11 andreicoman11 committed with Facebook Github Bot 1 Sep 12, 2016
@@ -0,0 +1,54 @@
+// Copyright 2004-present Facebook. All Rights Reserved.
+
+package com.facebook.react.views.text;
+
+import android.graphics.Paint;
+import android.text.style.LineHeightSpan;
+
+/**
+ * We use a custom {@link LineHeightSpan}, because `lineSpacingExtra` is broken. Details here:
+ * https://github.com/facebook/react-native/issues/7546
+ */
+public class CustomLineHeightSpan implements LineHeightSpan {
+ private final int mHeight;
+
+ CustomLineHeightSpan(float height) {
+ this.mHeight = (int) Math.ceil(height);
+ }
+
+ @Override
+ public void chooseHeight(
+ CharSequence text,
+ int start,
+ int end,
+ int spanstartv,
+ int v,
+ Paint.FontMetricsInt fm) {
+ // This is more complicated that I wanted it to be. You can find a good explanation of what the
+ // FontMetrics mean here: http://stackoverflow.com/questions/27631736.
+ // The general solution is that if there's not enough height to show the full line height, we
+ // will prioritize in this order: ascent, descent, bottom, top
+
+ if (-fm.ascent > mHeight) {
+ // Show as much ascent as possible
+ fm.top = fm.ascent = -mHeight;
+ fm.bottom = fm.descent = 0;
+ } else if (-fm.ascent + fm.descent > mHeight) {
+ // Show all ascent, and as much descent as possible
+ fm.top = fm.ascent;
+ fm.bottom = fm.descent = mHeight + fm.ascent;
+ } else if (-fm.ascent + fm.bottom > mHeight) {
+ // Show all ascent, descent, as much bottom as possible
+ fm.top = fm.ascent;
+ fm.bottom = fm.ascent + mHeight;
+ } else if (-fm.top + fm.bottom > mHeight) {
+ // Show all ascent, descent, bottom, as much top as possible
+ fm.top = fm.bottom - mHeight;
+ } else {
+ // Show proportionally additional ascent and top
+ final int additional = mHeight - (-fm.top + fm.bottom);
+ fm.top -= additional;
+ fm.ascent -= additional;
+ }
+ }
+}
@@ -172,6 +172,12 @@ private static void buildSpannedFromTextCSSNode(
textCSSNode.mTextShadowRadius,
textCSSNode.mTextShadowColor)));
}
+ if (!Float.isNaN(textCSSNode.getEffectiveLineHeight())) {
+ ops.add(new SetSpanOperation(
+ start,
+ end,
+ new CustomLineHeightSpan(textCSSNode.getEffectiveLineHeight())));
+ }
ops.add(new SetSpanOperation(start, end, new ReactTagSpan(textCSSNode.getReactTag())));
}
}
@@ -235,14 +241,6 @@ public void measure(
// technically, width should never be negative, but there is currently a bug in
boolean unconstrainedWidth = widthMode == CSSMeasureMode.UNDEFINED || width < 0;
- float effectiveLineHeight = reactCSSNode.getEffectiveLineHeight();
- float lineSpacingExtra = 0;
- float lineSpacingMultiplier = 1;
- if (!Float.isNaN(effectiveLineHeight)) {
- lineSpacingExtra = effectiveLineHeight;
- lineSpacingMultiplier = 0;
- }
-
if (boring == null &&
(unconstrainedWidth ||
(!CSSConstants.isUndefined(desiredWidth) && desiredWidth <= width))) {
@@ -253,8 +251,8 @@ public void measure(
textPaint,
(int) Math.ceil(desiredWidth),
Layout.Alignment.ALIGN_NORMAL,
- lineSpacingMultiplier,
- lineSpacingExtra,
+ 1.f,
+ 0.f,
true);
} else if (boring != null && (unconstrainedWidth || boring.width <= width)) {
// Is used for single-line, boring text when the width is either unknown or bigger
@@ -264,8 +262,8 @@ public void measure(
textPaint,
boring.width,
Layout.Alignment.ALIGN_NORMAL,
- lineSpacingMultiplier,
- lineSpacingExtra,
+ 1.f,
+ 0.f,
boring,
true);
} else {
@@ -275,8 +273,8 @@ public void measure(
textPaint,
(int) width,
Layout.Alignment.ALIGN_NORMAL,
- lineSpacingMultiplier,
- lineSpacingExtra,
+ 1.f,
+ 0.f,
true);
}
@@ -588,7 +586,6 @@ public void onCollectExtraUpdates(UIViewOperationQueue uiViewOperationQueue) {
UNSET,
mContainsImages,
getPadding(),
- getEffectiveLineHeight(),
getTextAlign()
);
uiViewOperationQueue.enqueueUpdateExtraData(getReactTag(), reactTextUpdate);
@@ -10,7 +10,6 @@
package com.facebook.react.views.text;
import android.text.Spannable;
-import android.view.Gravity;
import com.facebook.csslayout.Spacing;
@@ -28,15 +27,13 @@
private final float mPaddingTop;
private final float mPaddingRight;
private final float mPaddingBottom;
- private final float mLineHeight;
private final int mTextAlign;
public ReactTextUpdate(
Spannable text,
int jsEventCounter,
boolean containsImages,
Spacing padding,
- float lineHeight,
int textAlign) {
mText = text;
mJsEventCounter = jsEventCounter;
@@ -45,7 +42,6 @@ public ReactTextUpdate(
mPaddingTop = padding.get(Spacing.TOP);
mPaddingRight = padding.get(Spacing.END);
mPaddingBottom = padding.get(Spacing.BOTTOM);
- mLineHeight = lineHeight;
mTextAlign = textAlign;
}
@@ -77,10 +73,6 @@ public float getPaddingBottom() {
return mPaddingBottom;
}
- public float getLineHeight() {
- return mLineHeight;
- }
-
public int getTextAlign() {
return mTextAlign;
}
@@ -20,7 +20,6 @@
import android.view.ViewGroup;
import android.widget.TextView;
-import com.facebook.csslayout.FloatUtil;
import com.facebook.react.uimanager.ReactCompoundView;
import com.facebook.react.uimanager.ViewDefaults;
import com.facebook.react.views.view.ReactViewBackgroundDrawable;
@@ -65,16 +64,6 @@ public void setText(ReactTextUpdate update) {
(int) Math.ceil(update.getPaddingRight()),
(int) Math.ceil(update.getPaddingBottom()));
- float nextLineHeight = update.getLineHeight();
- if (!FloatUtil.floatsEqual(mLineHeight, nextLineHeight)) {
- mLineHeight = nextLineHeight;
- if (Float.isNaN(mLineHeight)) { // NaN will be used if property gets reset
- setLineSpacing(0, 1);
- } else {
- setLineSpacing(mLineHeight, 0);
- }
- }
-
int nextTextAlign = update.getTextAlign();
if (mTextAlign != nextTextAlign) {
mTextAlign = nextTextAlign;
@@ -129,7 +129,6 @@ public void onCollectExtraUpdates(UIViewOperationQueue uiViewOperationQueue) {
mJsEventCount,
mContainsImages,
getPadding(),
- getEffectiveLineHeight(),
mTextAlign
);
uiViewOperationQueue.enqueueUpdateExtraData(getReactTag(), reactTextUpdate);

0 comments on commit c79f617

Please sign in to comment.