On Tue, 2 Feb 2021 06:20:44 GMT, Prasanta Sadhukhan <[email protected]>
wrote:
>> 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.
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.
-------------
PR: https://git.openjdk.java.net/jdk/pull/2256
