Fix: incorrect line-height calculation

Summary:
There seems to be a rounding error in the android code for line height, so that for some fonts and at some combinations of line height and font size the actual height of the elements seems to be slightly too short.

I've identified one issue that I mentioned here facebook#10712 (comment) that could at least explain some of the problem. That when the line-height minus the original sum of the absolute value of top  and bottom from the metrics, happens to be an odd number, the division by two causes a rounding error of 1, so that the actual line height is 1pt less than it should.

The fix uses floating point division instead of integer division, and rounds (arbitrarily) the negative values up and the positive values down so that the total is still the correct for odd numbers.

It turns out that only ascent and descent is used to give the actual line-height between lines in the same text-element. The top and bottom values are only used for padding the top and bottom of the text. So when the line-height is greater than the font size and the extra padding this PR sets the ascent and descent to the same value as the top and bottom respectively.

I've renamed the shouldIncreaseAllMetricsProportionally test to evenLineHeightShouldIncreaseAllMetricsProportionally and added an extra assertion to check that bottom-top still equals the line height.

Added another test oddLineHeightShouldAlsoWork that is similar but uses an odd number for the line height to test that it still works with odd numbers. This test only uses the sum of the values so that it's indifferent to what value the implementation chooses to round up or down.

Improvement on facebook#16448

Fix line-height calculation on Android.

| Before        | After           |
| ------------- |-------------|
| ![without fix](https://user-images.githubusercontent.com/2144849/36150230-4404a0cc-10c3-11e8-8880-4ab84339c741.png)      | ![actual fix](https://user-images.githubusercontent.com/2144849/36156620-eb496d0e-10d7-11e8-8bd1-1cb536a38fbf.png) |

(All three columns have font size 16 and lineHeight: 32. The first one is has fixed height 9*32, the second is 9 Text elements, the last is one text element with lots of text limited to 9 lines, so they should be the same height. )
Closes facebook#17952

Differential Revision: D6980333

Pulled By: hramos

fbshipit-source-id: 0a501358cfbf7f139fca46056d0d972b1daf6ae3

ReactAndroid/src/main/java/com/facebook/react/views/text/CustomLineHeightSpan.java

@@ -47,8 +47,13 @@ public void chooseHeight(
     } else {
       // Show proportionally additional ascent and top
       final int additional = mHeight - (-fm.top + fm.bottom);
-      fm.top -= additional;
-      fm.ascent -= additional;
+
+      // Round up for the negative values and down for the positive values  (arbritary choice)
+      // So that bottom - top equals additional even if it's an odd number.
+      fm.top -= Math.ceil(additional / 2.0f);
+      fm.bottom += Math.floor(additional / 2.0f);
+      fm.ascent = fm.top;
+      fm.descent = fm.bottom;
     }
   }
 }

ReactAndroid/src/test/java/com/facebook/react/views/text/CustomLineHeightSpanTest.java

@@ -0,0 +1,106 @@
+package com.facebook.react.views.text;
+
+import android.graphics.Paint;
+
+import static org.fest.assertions.api.Assertions.assertThat;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.powermock.core.classloader.annotations.PowerMockIgnore;
+import org.robolectric.RobolectricTestRunner;
+
+@RunWith(RobolectricTestRunner.class)
+@PowerMockIgnore({"org.mockito.*", "org.robolectric.*", "android.*"})
+public class CustomLineHeightSpanTest {
+
+  @Test
+  public void evenLineHeightShouldIncreaseAllMetricsProportionally() {
+    CustomLineHeightSpan customLineHeightSpan = new CustomLineHeightSpan(22);
+    Paint.FontMetricsInt fm = new Paint.FontMetricsInt();
+    fm.top = -10;
+    fm.ascent = -5;
+    fm.descent = 5;
+    fm.bottom = 10;
+    customLineHeightSpan.chooseHeight("Hi", 0, 2, 0, 0, fm);
+    // Since line height is even it should be equally added to top and bottom.
+    assertThat(fm.top).isEqualTo(-11);
+    assertThat(fm.ascent).isEqualTo(-11);
+    assertThat(fm.descent).isEqualTo(11);
+    assertThat(fm.bottom).isEqualTo(11);
+    assertThat(fm.bottom - fm.top).isEqualTo(22);
+  }
+
+  @Test
+  public void oddLineHeightShouldAlsoWork() {
+    CustomLineHeightSpan customLineHeightSpan = new CustomLineHeightSpan(23);
+    Paint.FontMetricsInt fm = new Paint.FontMetricsInt();
+    fm.top = -10;
+    fm.ascent = -5;
+    fm.descent = 5;
+    fm.bottom = 10;
+    customLineHeightSpan.chooseHeight("Hi", 0, 2, 0, 0, fm);
+    // Only test that the sum is correct so the implementation
+    // is free to add the odd value either on top or bottom.
+    assertThat(fm.descent - fm.ascent).isEqualTo(23);
+    assertThat(fm.bottom - fm.top).isEqualTo(23);
+  }
+
+  @Test
+  public void shouldReduceTopFirst() {
+    CustomLineHeightSpan customLineHeightSpan = new CustomLineHeightSpan(19);
+    Paint.FontMetricsInt fm = new Paint.FontMetricsInt();
+    fm.top = -10;
+    fm.ascent = -5;
+    fm.descent = 5;
+    fm.bottom = 10;
+    customLineHeightSpan.chooseHeight("Hi", 0, 2, 0, 0, fm);
+    assertThat(fm.top).isEqualTo(-9);
+    assertThat(fm.ascent).isEqualTo(-5);
+    assertThat(fm.descent).isEqualTo(5);
+    assertThat(fm.bottom).isEqualTo(10);
+  }
+
+  @Test
+  public void shouldReduceBottomSecond() {
+    CustomLineHeightSpan customLineHeightSpan = new CustomLineHeightSpan(14);
+    Paint.FontMetricsInt fm = new Paint.FontMetricsInt();
+    fm.top = -10;
+    fm.ascent = -5;
+    fm.descent = 5;
+    fm.bottom = 10;
+    customLineHeightSpan.chooseHeight("Hi", 0, 2, 0, 0, fm);
+    assertThat(fm.top).isEqualTo(-5);
+    assertThat(fm.ascent).isEqualTo(-5);
+    assertThat(fm.descent).isEqualTo(5);
+    assertThat(fm.bottom).isEqualTo(9);
+  }
+
+  @Test
+  public void shouldReduceAscentThird() {
+    CustomLineHeightSpan customLineHeightSpan = new CustomLineHeightSpan(9);
+    Paint.FontMetricsInt fm = new Paint.FontMetricsInt();
+    fm.top = -10;
+    fm.ascent = -5;
+    fm.descent = 5;
+    fm.bottom = 10;
+    customLineHeightSpan.chooseHeight("Hi", 0, 2, 0, 0, fm);
+    assertThat(fm.top).isEqualTo(-4);
+    assertThat(fm.ascent).isEqualTo(-4);
+    assertThat(fm.descent).isEqualTo(5);
+    assertThat(fm.bottom).isEqualTo(5);
+  }
+
+  @Test
+  public void shouldReduceDescentLast() {
+    CustomLineHeightSpan customLineHeightSpan = new CustomLineHeightSpan(4);
+    Paint.FontMetricsInt fm = new Paint.FontMetricsInt();
+    fm.top = -10;
+    fm.ascent = -5;
+    fm.descent = 5;
+    fm.bottom = 10;
+    customLineHeightSpan.chooseHeight("Hi", 0, 2, 0, 0, fm);
+    assertThat(fm.top).isEqualTo(0);
+    assertThat(fm.ascent).isEqualTo(0);
+    assertThat(fm.descent).isEqualTo(4);
+    assertThat(fm.bottom).isEqualTo(4);
+  }
+}