<Swing Dev> RFR: 8231286: HTML font size too large with high-DPI scaling and W3C_LENGTH_UNITS [v4]
Matthias Perktold
github.com+7334069+mperktold at openjdk.java.net
Tue Feb 2 15:19:43 UTC 2021
On Tue, 2 Feb 2021 12:29:00 GMT, Prasanta Sadhukhan <psadhukhan at openjdk.org> wrote:
>>>
>>>
>>> > 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.
>
> It seems Toolkit.getDefaultToolkit().getScreenResolution() can return 93/94/95 instead of 96 in mac/linux in internal mach5 testing systems causing failure in this test Test.java. Probably we need to make the testcase hardcoded to 96
>
> res = 93
> Font Size for InlineView #0 = 96; height = 96; element = {
> <content
> span=font-size=72pt
> font-size=72pt
> name=content
> >
> [1,2][A]
> }
> Font Size for InlineView #1 = 96; height = 96; element = {
> <content
> span=font-size=6pc
> font-size=6pc
> name=content
> >
> [2,3][B]
> }
> Font Size for InlineView #2 = 93; height = 94; element = {
> <content
> span=font-size=93px
> font-size=93px
> name=content
> >
> [3,4][C]
> }
> Font Size for InlineView #3 = 96; height = 96; element = {
> <content
> span=font-size=25.4mm
> font-size=25.4mm
> name=content
> >
> [4,5][D]
> }
> Font Size for InlineView #4 = 96; height = 96; element = {
> <content
> span=font-size=2.54cm
> font-size=2.54cm
> name=content
> >
> [5,6][E]
> }
> Font Size for InlineView #5 = 96; height = 96; element = {
> <content
> span=font-size=1in
> font-size=1in
> name=content
> >
> [6,7][F]
> }
> ----------System.err:(5/314)----------
> java.lang.RuntimeException: Test failed.
>
> I also suggest fixing JDK-8260687 to not use font inherit for W3C_LENGTH_UNIT case
>
> --- a/src/java.desktop/share/classes/javax/swing/text/html/StyleSheet.java
> +++ b/src/java.desktop/share/classes/javax/swing/text/html/StyleSheet.java
> @@ -2823,7 +2823,7 @@ public class StyleSheet extends StyleContext {
> }
>
> Object doGetAttribute(Object key) {
> - if (key == CSS.Attribute.FONT_SIZE && !isDefined(key)) {
> + if (key == CSS.Attribute.FONT_SIZE && !isDefined(key) && !isW3CLengthUnits()) {
> // CSS.FontSize represents a specified value and we need
> // to inherit a computed value so don't resolve percentage
> // value from parent.
> It seems Toolkit.getDefaultToolkit().getScreenResolution() can return 93/94/95 instead of 96 in mac/linux in internal mach5 testing systems causing failure in this test Test.java. Probably we need to make the testcase hardcoded to 96
>
> ```
>
> res = 93
> Font Size for InlineView #0 = 96; height = 96; element = {
> <content
> span=font-size=72pt
> font-size=72pt
> name=content
> >
> [1,2][A]
> }
> [...]
> ----------System.err:(5/314)----------
> java.lang.RuntimeException: Test failed.
> ```
Sorry, but I don't understand. What do these examples represent? Are they the results of some tests you ran? Or should I encorporate them into the test somehow?
Also, regarding a resolution of 93/94/95, I think rather than hardcoding 96, we should ensure that the behavior is correct for various screen sizes. I don't know how to change the resolution just for the test, but if that is possible, I think we should test several values, like 96, something below that and something above.
> I also suggest fixing JDK-8260687 to not use font inherit for W3C_LENGTH_UNIT case
>
> ```
> --- a/src/java.desktop/share/classes/javax/swing/text/html/StyleSheet.java
> +++ b/src/java.desktop/share/classes/javax/swing/text/html/StyleSheet.java
> @@ -2823,7 +2823,7 @@ public class StyleSheet extends StyleContext {
> }
>
> Object doGetAttribute(Object key) {
> - if (key == CSS.Attribute.FONT_SIZE && !isDefined(key)) {
> + if (key == CSS.Attribute.FONT_SIZE && !isDefined(key) && !isW3CLengthUnits()) {
> // CSS.FontSize represents a specified value and we need
> // to inherit a computed value so don't resolve percentage
> // value from parent.
> ```
Just the be sure before I do that, are there going to be problems when I pull changes from master into this branch?
Or was [this bot comment](https://github.com/openjdk/jdk/pull/2223#issuecomment-766934951) just about the name of the branch, and everything should be fine?
-------------
PR: https://git.openjdk.java.net/jdk/pull/2256
More information about the swing-dev
mailing list