facebook/react-native

Lineheight behaves differently on ios and android

MarcoPolo opened this issue ยท 22 comments

Description

On ios:
screen shot 2016-11-02 at 4 58 11 pm

On Android:
screen shot 2016-11-02 at 4 57 40 pm

note how the on android it seems like the line height is only applied on top of the text, while on ios it's applied to both top and bottom. This is also an issue when you have lineheights < fontSize. On ios it trims from the top, but on android it'll trim from top and bottom, so the text doesn't line up and get's truncated differently.

Reproduction

RNplay demo: https://rnplay.org/apps/OJFGEg

Additional Information

  • React Native version: 0.36
  • Platform: iOS & Android
  • Operating System: MacOS

Just ran into this issue, can confirm still exists

Ditto, can confirm that I'm still seeing this on RN v0.45.0 (latest).

It's not apparent to me if this is intended behavior based on the corresponding underlying native capabilities, or rather an unintentional discrepancy.

I'd just like to add that it certainly makes achieving a consistent layout between both platforms challenging, in achieving parity with pre-designed screens. I've resorted to disabling lineHeight on Android in the interim, as I'm unable to control how exactly the text appears.

Is this on anyone's radar by chance?

I did a little test on CustomLineHeightSpan.java. It works on new pure Android project but not on RNTesterApp. I am still investigating what the different between ReactNative TextView and PureAndroid TextView. I would like to pair with ReactNative core team to work on this investigation (if possible).

The idea is simple: use descent and baseline as base metrics, then recalculate bottom, ascent and top based on fontSize and lineHeight. ascent and descent will be centered.

// 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;
  private final int mSize;

  CustomLineHeightSpan(float size, float height) {
    this.mSize = (int) size;
    this.mHeight = Math.max((int) height, mSize);
  }

  @Override
  public void chooseHeight(
      CharSequence text,
      int start,
      int end,
      int spanstartv,
      int v,
      Paint.FontMetricsInt fm) {
    int spacing = (mHeight - mSize) / 2;
    fm.bottom = fm.descent + spacing;
    fm.ascent = fm.descent - mSize;
    fm.top = fm.descent - mSize - spacing;
  }
}

screen shot 2017-07-07 at 4 07 32 pm

Checkout my proposal for CustomLineHeightSpan.java fixed. henrytao-me#1.

screen shot 2017-07-07 at 5 45 20 pm

Same problems with fonts on RN: 0.44.0. There are few issues as far as I know about this, or very similar behaviour.

It's a big problem for me. I'm working on a project in which correct font rendering is critical. As a temporary workaround I have applied lineHeight bigger than fontSize.
I also added paddingBottom and marginTop to negative value. I had to manually adjust these values for each font. This was to prevent Text component from growing due to increased line height. But it's not a production solution, I don't recommend that, this was my last resort.

@henrytao-me it seems that you've made a pull-request against your own forked react-native repo whilst you should have made it against the Facebook repo. Making a pull-request against master is going to be the most effective way of getting some decent feedback.

Please check up on #14611

Thanks @peterp for suggestion. Some how I missed this conversation. I will create new PR soon.

Facing the same issues...any alternatives??

I've opened a PR fixing this issue. The reason I worked on a simpler solution than @henrytao-me's proposed solution is that the number of cases when working with font rendering is high and the only difference between Android and iOS rendering is the priority of ascent over descent. The change in #16448 touches only that and adds some unit test coverage for confidence that nothing else changes in the calculation. I'm hoping this will make it easier to review and land this fix soon.

Thanks @bartolkaruza. I was busy recently on other stuffs. ๐Ÿ™‡

@bartolkaruza that link does not seem to be correct, I guess it's #16448 ?

Thank you @guidobouman, I've edited my reference as well.

Any updates on getting this in a release? I saw it brought up in the 0.51 issue thread but it doesn't seem to have been released. Let me know if there's anything I can do to help speed things along.

0.52 has been fully released! Should be good to go...

Cool. I will check it soon. Back to react native now.

I updated to 0.52 and it's a lot better, but still not quite correct.

There is a weird issue with multiline text where depending on the relation between the font size and the line height the line-height is actually slightly shorter when one Text element spans multiple lines compared to the same number of Text elements with only one line (with no additional padding or margins)
It's not much but enough to make the text a look a more cramped than it should.

--

E.g. with fontSize: 16, lineHeight: 32 and no padding/margin, you'd expect nine lines to be 9 * 32pt = 288pt high. But in reality if you stack 9 Text elements with 1 line each the total height is shorter. And a single Text element with 9 lines is even shorter than that.

My device has 2px pr pt so when i measure the screenshot i find the total height of the box with an explicit height of 288pt to be 576px as expected (288*2=576). But the 9 lines is for some reason only 567px which gives a total height of 283.5pt which divided by 9 gives an actual lineHeight of 31.5pt, measuring the distance between the bottom of the letters also gives the same lineHeight.

While the single Text element with nine lines is even shorter at 526px which is 263pt, divided by 9 this gives an actual lineHeight of 29.22222... which is weird. But when i measure the distance between the lines it's only 58px = 29pt which means there is some magic padding of 1pt added top and bottom even though I explicitly set no padding.

This is just one example fontsize/lineheight combo. Different combinations gives different results. Which makes me suspect some of it is caused by some weird rounding error. A larger font size makes the line-height even shorter. So perhaps the font size is somehow rounded up and then subtracted from the lineheight or something?

Edit:
Looking at 3f1b021 I see that the code (which I assume is the one used) has a lot of integer division (i.e. division using x / 2 rather than x / 2.0 which truncates down the result to an integer when the x is an integer. E.g.: 3 / 2 == 1) at the end which probably explains some of the rounding issues.

I don't fully understand the code but all the explicit and implicit rounding/truncation going on, makes me think that this could be part of the issue. There probably should be added a test checking that the computed total height of a multi line element is equal to the line height specified times the number of lines (or at least within +/- 1 px).

Is there a good reason why the calculations doesn't use floats all the way? Or at least for all the calculations?

screenshot_lineheight_example

Snippet to reproduce:

// style.js
export default StyleSheet.create({
  container: {
    flexDirection: 'column',
    height: '100%',
  },
  bio: {
    marginVertical: 0,
    paddingVertical: 0,
    margin: 0,
    padding: 0,
    fontSize: 16,
    lineHeight: 32,
  },
  debug1: { backgroundColor: 'gray' },
  debug2: { backgroundColor: 'pink' },
})

// Render 
// ...
return (
  <View style={styles.container}>
    <ScrollView style={styles.descWrapper}>
      <Text style={[styles.bio, { marginVertical: 50 }]}>Example:</Text>

      <View style={{ flexDirection: 'row' }}>
        <View style={{ flex: 1 }}>
          <Text style={[styles.bio, styles.debug1, { height: 32 * 9 }]}>
            E
          </Text>
        </View>
        <View style={{ flex: 1 }}>
          <Text style={[styles.bio, styles.debug2]}>A</Text>
          <Text style={[styles.bio, styles.debug1]}>A</Text>
          <Text style={[styles.bio, styles.debug2]}>A</Text>
          <Text style={[styles.bio, styles.debug1]}>A</Text>
          <Text style={[styles.bio, styles.debug2]}>A</Text>
          <Text style={[styles.bio, styles.debug1]}>A</Text>
          <Text style={[styles.bio, styles.debug2]}>A</Text>
          <Text style={[styles.bio, styles.debug1]}>A</Text>
          <Text style={[styles.bio, styles.debug2]}>A</Text>
        </View>

        <View style={{ flex: 1 }}>
          <Text
            style={[styles.bio, styles.debug2]}
            numberOfLines={9}
            ellipsizeMode="tail"
          >
            Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do
            eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut
            enim ad minim veniam, quis nostrud exercitation ullamco laboris
            nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in
            reprehenderit in voluptate velit esse cillum dolore eu fugiat
            nulla pariatur. Excepteur sint occaecat cupidatat non proident,
            sunt in culpa qui officia deserunt mollit anim id est laborum.
          </Text>
        </View>
      </View>
    </ScrollView>
  </View>
);

I looked a bit closer at the final else block:

    } else {
      // Show proportionally additional ascent / top & descent / bottom
      final int additional = mHeight - (-fm.top + fm.bottom);

      fm.top -= additional / 2;
      fm.ascent -= additional / 2;
      fm.descent += additional / 2;
      fm.bottom += additional / 2;
    }

And I think the integer division is definitely part of the problem:

Let's say that (-fm.top + fm.bottom) happens to be 10.1 and mHeight is 40.
That would make additional == 29.9, except it's an int so it's truncated to just 29

Then this is divided by two which should have been 29.9/2 == 14.95, but again since
it's integers it becomes 29/2 == 14

So the total extra height added becomes only 28 even though the calculation really should have added 29.9. This 1.9 difference is probably later rounded up to exactly 2 when when the font is rendered as native pixels (since 1.9, 2.8 (2x) and 5.7 (3x) all rounds up).

So this explains the random 1 or 2 pt missing in the multiple elements case that appears or disappears depending on the fontSize. But it doesn't completely explain the multiline issue. That's probably because the actual distance between the baselines isn't adjusted correctly either.

Where is the code that adjusts the leading, height, lineSpaceMultiplier or whatever is used to control the distance between the baselines?

This should be resolved now that 74e54cb landed.

I faced this problem again in my RN 0.59.5 on Android 9.0 users. Users on Android 8.0 and 7.0 are working fine. iOS users are working fine too.

61738916_342011646487918_8564668360254554112_n

Currently, I fixed the issue by providing lineheight in the text style.

text: {
    fontSize: 16,
    lineHeight: 26,
    paddingVertical: 5
},

Hope this is helpful for anyone who is facing this issue.

a8t commented

Not sure if other folks are experiencing this, and not sure if it's only with Expo for whatever reason, but lineHeight: 0 does nothing on iOS while on Android it basically hides the text. Thought my fonts weren't loading on Android because of that ๐Ÿคฆ๐Ÿฝโ€โ™€๏ธ