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
This commit is contained in:
Andrei Coman 2016-09-12 05:03:27 -07:00 committed by Facebook Github Bot 1
parent 16bdbee165
commit c79f617742
5 changed files with 66 additions and 35 deletions

View File

@ -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;
}
}
}

View File

@ -172,6 +172,12 @@ public class ReactTextShadowNode extends LayoutShadowNode {
textCSSNode.mTextShadowRadius, textCSSNode.mTextShadowRadius,
textCSSNode.mTextShadowColor))); 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()))); ops.add(new SetSpanOperation(start, end, new ReactTagSpan(textCSSNode.getReactTag())));
} }
} }
@ -235,14 +241,6 @@ public class ReactTextShadowNode extends LayoutShadowNode {
// technically, width should never be negative, but there is currently a bug in // technically, width should never be negative, but there is currently a bug in
boolean unconstrainedWidth = widthMode == CSSMeasureMode.UNDEFINED || width < 0; 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 && if (boring == null &&
(unconstrainedWidth || (unconstrainedWidth ||
(!CSSConstants.isUndefined(desiredWidth) && desiredWidth <= width))) { (!CSSConstants.isUndefined(desiredWidth) && desiredWidth <= width))) {
@ -253,8 +251,8 @@ public class ReactTextShadowNode extends LayoutShadowNode {
textPaint, textPaint,
(int) Math.ceil(desiredWidth), (int) Math.ceil(desiredWidth),
Layout.Alignment.ALIGN_NORMAL, Layout.Alignment.ALIGN_NORMAL,
lineSpacingMultiplier, 1.f,
lineSpacingExtra, 0.f,
true); true);
} else if (boring != null && (unconstrainedWidth || boring.width <= width)) { } else if (boring != null && (unconstrainedWidth || boring.width <= width)) {
// Is used for single-line, boring text when the width is either unknown or bigger // Is used for single-line, boring text when the width is either unknown or bigger
@ -264,8 +262,8 @@ public class ReactTextShadowNode extends LayoutShadowNode {
textPaint, textPaint,
boring.width, boring.width,
Layout.Alignment.ALIGN_NORMAL, Layout.Alignment.ALIGN_NORMAL,
lineSpacingMultiplier, 1.f,
lineSpacingExtra, 0.f,
boring, boring,
true); true);
} else { } else {
@ -275,8 +273,8 @@ public class ReactTextShadowNode extends LayoutShadowNode {
textPaint, textPaint,
(int) width, (int) width,
Layout.Alignment.ALIGN_NORMAL, Layout.Alignment.ALIGN_NORMAL,
lineSpacingMultiplier, 1.f,
lineSpacingExtra, 0.f,
true); true);
} }
@ -588,7 +586,6 @@ public class ReactTextShadowNode extends LayoutShadowNode {
UNSET, UNSET,
mContainsImages, mContainsImages,
getPadding(), getPadding(),
getEffectiveLineHeight(),
getTextAlign() getTextAlign()
); );
uiViewOperationQueue.enqueueUpdateExtraData(getReactTag(), reactTextUpdate); uiViewOperationQueue.enqueueUpdateExtraData(getReactTag(), reactTextUpdate);

View File

@ -10,7 +10,6 @@
package com.facebook.react.views.text; package com.facebook.react.views.text;
import android.text.Spannable; import android.text.Spannable;
import android.view.Gravity;
import com.facebook.csslayout.Spacing; import com.facebook.csslayout.Spacing;
@ -28,7 +27,6 @@ public class ReactTextUpdate {
private final float mPaddingTop; private final float mPaddingTop;
private final float mPaddingRight; private final float mPaddingRight;
private final float mPaddingBottom; private final float mPaddingBottom;
private final float mLineHeight;
private final int mTextAlign; private final int mTextAlign;
public ReactTextUpdate( public ReactTextUpdate(
@ -36,7 +34,6 @@ public class ReactTextUpdate {
int jsEventCounter, int jsEventCounter,
boolean containsImages, boolean containsImages,
Spacing padding, Spacing padding,
float lineHeight,
int textAlign) { int textAlign) {
mText = text; mText = text;
mJsEventCounter = jsEventCounter; mJsEventCounter = jsEventCounter;
@ -45,7 +42,6 @@ public class ReactTextUpdate {
mPaddingTop = padding.get(Spacing.TOP); mPaddingTop = padding.get(Spacing.TOP);
mPaddingRight = padding.get(Spacing.END); mPaddingRight = padding.get(Spacing.END);
mPaddingBottom = padding.get(Spacing.BOTTOM); mPaddingBottom = padding.get(Spacing.BOTTOM);
mLineHeight = lineHeight;
mTextAlign = textAlign; mTextAlign = textAlign;
} }
@ -77,10 +73,6 @@ public class ReactTextUpdate {
return mPaddingBottom; return mPaddingBottom;
} }
public float getLineHeight() {
return mLineHeight;
}
public int getTextAlign() { public int getTextAlign() {
return mTextAlign; return mTextAlign;
} }

View File

@ -20,7 +20,6 @@ import android.view.Gravity;
import android.view.ViewGroup; import android.view.ViewGroup;
import android.widget.TextView; import android.widget.TextView;
import com.facebook.csslayout.FloatUtil;
import com.facebook.react.uimanager.ReactCompoundView; import com.facebook.react.uimanager.ReactCompoundView;
import com.facebook.react.uimanager.ViewDefaults; import com.facebook.react.uimanager.ViewDefaults;
import com.facebook.react.views.view.ReactViewBackgroundDrawable; import com.facebook.react.views.view.ReactViewBackgroundDrawable;
@ -65,16 +64,6 @@ public class ReactTextView extends TextView implements ReactCompoundView {
(int) Math.ceil(update.getPaddingRight()), (int) Math.ceil(update.getPaddingRight()),
(int) Math.ceil(update.getPaddingBottom())); (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(); int nextTextAlign = update.getTextAlign();
if (mTextAlign != nextTextAlign) { if (mTextAlign != nextTextAlign) {
mTextAlign = nextTextAlign; mTextAlign = nextTextAlign;

View File

@ -129,7 +129,6 @@ public class ReactTextInputShadowNode extends ReactTextShadowNode implements
mJsEventCount, mJsEventCount,
mContainsImages, mContainsImages,
getPadding(), getPadding(),
getEffectiveLineHeight(),
mTextAlign mTextAlign
); );
uiViewOperationQueue.enqueueUpdateExtraData(getReactTag(), reactTextUpdate); uiViewOperationQueue.enqueueUpdateExtraData(getReactTag(), reactTextUpdate);