On Mon, 1 Feb 2021 23:03:30 GMT, Alexey Ivanov <aiva...@openjdk.org> wrote:
>> Matthias Perktold has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Link to CSS 2.2 spec from length mapping comments > > Changes requested by aivanov (Reviewer). > > > > It seems for screen with low resolution, this change might cause some > > failure as can be seen in the testcase attached in JBS Test.java. > > I'm afraid we can't make 72pt be exactly 96px because of the nature of > floating point calculations which are not precise. > The font size for 72pt is 94px instead of the expected 96px, the line height > is 120px instead of 123px. > > What we can do to improve the accuracy is not to hard-code the constant as > suggested at the moment but put 96/72 to map 'pt' unit to pixels. > > So 1.3 * 72 = 93.6 which is rounded to 94. Then 1.33 * 72 = 95.76 which is > rounded to 96; 1.333 * 72 = 95.976 and so on. If 96/72 is stored as float, > we'll have the most precise value. > > However, I'm pretty sure there are size / unit combinations which could make > your test fail. But still, it's a good way to estimate the accuracy. Shall we > add it as another test for this issue? > > If you disable, W3C_LENGTH_UNITS, you'll get a dramatic difference: 72pt = > 72px, line height 92px but 'font-size: 96px' results in font size of 125px > and line height of 159. > > Before @mperktold's fix is applied, the difference in size with > W3C_LENGTH_UNITS is also significant, the letter 'C' is twice as small as the > other letters; the two letters are rendered on the second line. In this case > 72pt = 192px and line height of 244px, but 'font-size: 96px' has the expected > size of 96px and line height of 123 px. Yes, it seems right to not hardcode and use 96/72f for "pt". I guess this needs to be fixed as this test is part of our regression test and that will fail after this fix, if not taken care and it will be considered as regression. I will also prefer that JDK-8260687 is also fixed as part of this PR as that also concerns JEditorPane.W3C_LENGTH_UNITS property. ------------- PR: https://git.openjdk.java.net/jdk/pull/2256
