On Wed, 27 Jan 2021 11:40:10 GMT, Matthias Perktold <github.com+7334069+mperkt...@openjdk.org> wrote:
> This PR supersedes #2223. > The original PR was created from master, whereas this PR is created from a > fresh branch. > > Below is a copy of the original description, but as @mrserb correctly > [pointed > out](https://github.com/openjdk/jdk/pull/2223#issuecomment-767048399), there > is actually no relevant change from CSS 2.1 to CSS 2.2, so I <strike>striked > out</strike> that part. > > ### Original Description > > @prsadhuk asked me to take over his pull request #1628, since I filed the bug > an suggested this fix. > > I thought the current behavior would be buggy, but actually the units are > quite precise. I checked the size of a text in font-size of 1 in, and it > really was approximately 1 inch. The problem is just that browsers behave > differently. > > <strike> > Swing follows the CSS 2.1 spec: > https://www.w3.org/TR/CSS21/syndata.html#length-units. > But in CSS 2.2, length units where redefined: > https://www.w3.org/TR/CSS22/syndata.html#length-units. > Now px is also an absolute unit, and there are constant conversion factors > between all absolute units. > > The CSS 2.2 spec includes the following two notes regarding this change: > > Note that if the anchor unit is the pixel unit, the physical units might > not match their physical measurements. Alternatively if the anchor unit is a > physical unit, the pixel unit might not map to a whole number of device > pixels. > > Note that this definition of the pixel unit and the physical units > differs from previous versions of CSS. In particular, in previous versions of > CSS the pixel unit and the physical units were not related by a fixed ratio: > the physical units were always tied to their physical measurements while the > pixel unit would vary to most closely match the reference pixel. (This change > was made because too much existing content relies on the assumption of 96dpi, > and breaking that assumption breaks the content.) > > So the spec changed the behavior to better support high-DPI devices with > existing content, and that is exactly my intention with this PR as well. > </strike> Changes requested by aivanov (Reviewer). src/java.desktop/share/classes/javax/swing/text/html/CSS.java line 2864: > 2862: lengthMapping.put("pc", 12f); > 2863: lengthMapping.put("in", 72f); > 2864: // mapping according to the CSS2.2 spec > Should we not put the latest CSS link here in addition/instead of in > JEditorPane? I agree the link to Length Units should also be added here: Suggestion: // Mapping according to the CSS2.2 spec // https://www.w3.org/TR/CSS22/syndata.html#length-units Or https://www.w3.org/TR/CSS22/syndata.html#x39 which leads directly to _Absolute length units_. src/java.desktop/share/classes/javax/swing/text/html/CSS.java line 2870: > 2868: w3cLengthMapping.put("cm", 37.7952f); //96px/2.54 > 2869: w3cLengthMapping.put("pc", 16f); //1/6 of 1in > 2870: w3cLengthMapping.put("in", 96f); //96px I suggest adding a space after `//`. Align the comments? As for *px*, CSS defines, “1px is equal to 0.75pt”, then 1pt is 4/3px — this is where 1.3f comes from (=96/72). test/jdk/javax/swing/text/html/HtmlFontSizeTest.java line 79: > 77: frame.setLocationRelativeTo(null); > 78: > 79: return frame.getBounds(); `frame` is redundant, `htmlPane.getPreferredSize()` does the job. test/jdk/javax/swing/text/html/HtmlFontSizeTest.java line 94: > 92: > 93: float ratio = (float)w3cFrameSize.width / > (float)stdFrameSize.width; > 94: System.out.println("w3cFrameSize.width/stdFrameSize.width " + > ratio); Shall the ratio be `(float)w3cFrameSize.height / (float)stdFrameSize.height`? You measure the font size so height seems a better candidate. However, the width does change as well. test/jdk/javax/swing/text/html/HtmlFontSizeTest.java line 98: > 96: String str = String.format("%.1g%n", ratio); > 97: if (str.compareTo(String.format("%.1g%n", 1.3f)) != 0) { > 98: throw new RuntimeException("HTML font size too large with > high-DPI scaling and W3C_LENGTH_UNITS"); The test passes for me with the fresh build of JDK, even without the fix applied. Indeed, I get the same frame dimensions in both cases: 150×42. However, the test fails when run with JDK 11.0.10 or JDK 15, the dimension of `htmlPane` is 409×76. The formatted string, `str`, contains only the number before the decimal point. I mean for `1.3f`, the formatted string is `"1"`. I'd expect the formatted string to have at least one digit after the decimal point but there's none. Does if ("1.3".equals(String.format("%1.1", ratio)) look clearer? I guess the test does not fail if the system scaling (of the main monitor) is set to 100% because `res` value read from `Toolkit.getDefaultToolkit().getScreenResolution()` is 96/72 which is 1.3. In my testing, `sun.java2d.uiScale` has no effect on the result. ------------- PR: https://git.openjdk.java.net/jdk/pull/2256
