Please sign in to comment.
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 #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 #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 #17952 Differential Revision: D6980333 Pulled By: hramos fbshipit-source-id: 0a501358cfbf7f139fca46056d0d972b1daf6ae3
- Loading branch information...
Showing with 26 additions and 7 deletions.