<div dir="ltr">Hi Phil,<br><br>I think I've figured out JDK-8269888 (Thai text rendered incorrectly using some AffineTransform-derived fonts).<br><br>I don't have a PR yet, since it builds on the test fix which is not yet integrated, but I'll submit the PR as soon as JDK-8283664 (make PrintTextTest runnable) is resolved.<br><br>The fix for JDK-8269888 involves removing devScale and ptSize from the GlyphLayout -> HB shaping sequence, and replacing them with xPtSize and yPtSize. The problem seems to have been that GlyphLayout._mat is based on SDCache.gtx, which combines device and font transforms. However, GlyphLayout._mat is used only to communicate font scale to HB, and should therefore not contain device scaling information. I've replaced SDCache.gtx (combined device + font transform) with SDCache.ftx (font transform), which I think removes any need for knowledge of devScale in the C code.<br><br>I think this also removes the need for ever using HB_NODEVTX, but I'm not sure if this environment variable was intended for testing, or for specific customers to workaround the device transform issue, or for someone else. Could you clarify its purpose?<br><br><div>Here are the changes as they currently stand, if you'd like to have a look: <a href="https://github.com/gredler/jdk/commit/d8dff2404c085e94b308450152c152d092b1fe2e">https://github.com/gredler/jdk/commit/d8dff2404c085e94b308450152c152d092b1fe2e</a></div><div><br></div><div>Take care,</div><div><br></div><div>Daniel<br></div><div><br></div><div><br></div></div><br><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Oct 25, 2024 at 7:48 PM Daniel Gredler <<a href="mailto:djgredler@gmail.com">djgredler@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr">Hi Phil,<br><br>It looks like there is already a bug open to fix this test: <a href="https://bugs.openjdk.org/browse/JDK-8283664" target="_blank">https://bugs.openjdk.org/browse/JDK-8283664</a><br><br>I've created a PR to get this test working again, as a first step -- can you have a look? <a href="https://github.com/openjdk/jdk/pull/21716" target="_blank">https://github.com/openjdk/jdk/pull/21716</a><br><br>Thanks!<br><br>Daniel<br><div><br></div><div>(+CC Lawrence, since he created JDK-8283664)<br></div><div><br></div><div><br></div></div><br><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Oct 10, 2024 at 1:06 PM Daniel Gredler <<a href="mailto:djgredler@gmail.com" target="_blank">djgredler@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Understood.<br>
<br>
I did run across a comment in the bug tracker for a different bug,<br>
JDK-8148334, [1] that seems to indicate that<br>
`test/jdk/java/awt/print/PrinterJob/PrintTextTest.java` might be able<br>
to replicate the issue. I'd like to try reverting the JDK-8145901 fix<br>
[2] locally and running that test to check, but I'm having issues<br>
running the test. It uses `@run main/manual=yesno PrintTextTest`,<br>
which based on JDK-8274456 [3] and corresponding commit [4] it looks<br>
like may no longer be supported? However, the jtreg FAQ [5] does not<br>
mention any deprecation of `yesno`. Will I need to convert this test<br>
to `@run main/manual PrintTextTest`, the pattern which most recent<br>
tests seem to use?<br>
<br>
Here's what I'm seeing right now:<br>
<br>
> ../jtreg-7.4/bin/jtreg -jdk:build/linux-x86_64-server-release/images/jdk/ -w build/jtreg/work -r build/jtreg/report -verbose:error -manual test/jdk/java/awt/print/PrinterJob/PrintTextTest.java<br>
> --------------------------------------------------<br>
> TEST: java/awt/print/PrinterJob/PrintTextTest.java<br>
> TEST JDK: /home/daniel/openjdk-src/jdk/build/linux-x86_64-server-release/images/jdk<br>
><br>
> TEST RESULT: Error. Parse Exception: Arguments to `manual' option not supported: yesno<br>
> --------------------------------------------------<br>
> Test results: error: 1<br>
> Report written to /home/daniel/openjdk-src/jdk/build/jtreg/report/html/report.html<br>
> Results written to /home/daniel/openjdk-src/jdk/build/jtreg/work<br>
> Error: Some tests failed or other problems occurred.<br>
<br>
---<br>
<br>
[1] <a href="https://bugs.openjdk.org/browse/JDK-8148334" rel="noreferrer" target="_blank">https://bugs.openjdk.org/browse/JDK-8148334</a><br>
[2] <a href="https://github.com/openjdk/jdk/commit/5935292ae022e970bd4075de4d704719f3b05575" rel="noreferrer" target="_blank">https://github.com/openjdk/jdk/commit/5935292ae022e970bd4075de4d704719f3b05575</a><br>
[3] <a href="https://bugs.openjdk.org/browse/JDK-8274456" rel="noreferrer" target="_blank">https://bugs.openjdk.org/browse/JDK-8274456</a><br>
[4] <a href="https://github.com/openjdk/jdk/commit/d57fb6f684eac5a7e68842dcf3284309e3867521" rel="noreferrer" target="_blank">https://github.com/openjdk/jdk/commit/d57fb6f684eac5a7e68842dcf3284309e3867521</a><br>
[5] <a href="https://openjdk.org/jtreg/faq.html" rel="noreferrer" target="_blank">https://openjdk.org/jtreg/faq.html</a><br>
<br>
---<br>
<br>
<br>
<br>
On Wed, Oct 9, 2024 at 5:48 PM Philip Race <<a href="mailto:philip.race@oracle.com" target="_blank">philip.race@oracle.com</a>> wrote:<br>
><br>
> That test was from an internal SQE test suite, and there's nothing I<br>
> have to open source.<br>
> But it was reproducible with just things like printing a JTable and<br>
> unrelated to Thai/complex text.<br>
> It is a painful issue where Swing needs to be told to layout text at<br>
> printer resolution when "printing the UI contents".<br>
> If you don't, then glyphs overlap, even for simple Latin "abc" text. Or<br>
> if they don't overlap the string is clipped.<br>
> So unrelated.<br>
><br>
> -phil<br>
><br>
> On 10/9/24 6:09 AM, Daniel Gredler wrote:<br>
> > Hi Phil,<br>
> ><br>
> > I've noticed that quite a few AWT / client lib tests have been open<br>
> > sourced over the past few weeks. That's great for those of us trying<br>
> > to contribute from outside of Oracle, so thanks for that!<br>
> ><br>
> > I wanted to follow up on this Thai rendering bug and check to see<br>
> > whether the tests for JDK-8145901 have been (or will be) open sourced<br>
> > as part of this effort. I'd still like to try to fix this bug at some<br>
> > point, and a fix would require less coordination if those tests were<br>
> > available publicly.<br>
> ><br>
> > Thanks!<br>
> ><br>
> > Daniel<br>
> ><br>
> ><br>
> ><br>
> ><br>
> > On Wed, Sep 8, 2021 at 8:39 PM Philip Race <<a href="mailto:philip.race@oracle.com" target="_blank">philip.race@oracle.com</a>> wrote:<br>
> >> JDK-8145901 isn't accessible because some confidential information is in the description and attachments ..<br>
> >> and in fact it was JCK failure so we need to be careful not to regress that<br>
> >> But I guess you found the changeset for it.<br>
> >><br>
> >> The problem we were trying to solve there was<br>
> >> ===<br>
> >> The font interface supplied to harfbuzz is setting the point size (scale)<br>
> >> based on the device size font. When printing, this is scaled from the<br>
> >> user size font by (typically) around 4X.<br>
> >> So when performing a kerning adjustment the affected glyphs are<br>
> >> moved accordingly.<br>
> >><br>
> >> But advances are being provided in user space and so the kerning<br>
> >> adjustment is too great relative to the advances.<br>
> >> ====<br>
> >><br>
> >> I'd need to spend some time refreshing my memory on this 5 yr old issue<br>
> >> so I can't say offhand that what you propose won't need fix up elsewhere.<br>
> >><br>
> >> But I can say that testing this will need to do on screen rendering as well as<br>
> >> printing of kerned and ligatured text under various tranforms .. inc. rotations, scales,<br>
> >><br>
> >> -phil.<br>
> >><br>
> >><br>
> >> On 9/7/21 4:43 PM, Daniel Gredler wrote:<br>
> >><br>
> >> Hi all,<br>
> >><br>
> >> I'm trying to figure out a fix for JDK-8269888 [1]. The font that I'm using<br>
> >> to replicate the issue, Google Noto Sans Thai Regular [2], uses the GPOS<br>
> >> table internally. It looks like the GPOS adjustment in HB is a two-step<br>
> >> process, where HB first sets the glyph position x_offset using anchor data<br>
> >> from the GPOS table [3], and then adjusts that x_offset using existing<br>
> >> x_advance data [4], all coordinated via the<br>
> >> HB_BUFFER_SCRATCH_FLAG_HAS_GPOS_ATTACHMENT scratch flag.<br>
> >><br>
> >> Internally, OpenJDK is using the font_funcs virtual method functionality<br>
> >> [5] to customize most of the font property callbacks [6]. Java provides<br>
> >> users two ways to customize the size of a font: you can set the pt size, or<br>
> >> you can set an affine transform (scale, rotate, shear, translate, etc).<br>
> >> When you use the pt size approach, OpenJDK calls hb_font_set_scale with the<br>
> >> appropriate pt size [7]. However, when a scaling affine transform is used,<br>
> >> the pt size is technically unchanged, and a value of 1pt is used. Usually<br>
> >> this works fine, because Java's font_funcs callbacks are providing scaled<br>
> >> origin and advance numbers back to HB, but this breaks the two-step GPOS<br>
> >> attachment logic: the first step is not aware of the scaling applied by the<br>
> >> OpenJDK font_funcs callbacks (especially the<br>
> >> hb_font_get_glyph_h_advance_func_t), but the second step uses a scaled<br>
> >> advance value provided by an OpenJDK callback. In practice, at e.g. x50<br>
> >> scaling, this means that the first step sets the x_offset to a relatively<br>
> >> small value (e.g. 32899), but the second step adjusts the x_offset by a<br>
> >> relatively large value (e.g. 1612184).<br>
> >><br>
> >> A possible fix would be for OpenJDK to take the affine transform scale into<br>
> >> account when setting the font size via hb_font_set_scale, and the HarfBuzz<br>
> >> team has confirmed that this seems like the sensible approach [8]. In fact,<br>
> >> it seems like prior to 2016 this was indeed the case [9], but the logic was<br>
> >> changed to fix JDK-8145901 (Printed content is overlapping). JDK-8145901<br>
> >> doesn't seem to be public in the bug tracker, and I'm not sure where to<br>
> >> find the HB_NODEVTX trigger used to request initialization of devScale in<br>
> >> HBShaper.c, so I'm not sure how to fix JDK-8269888 without risking a<br>
> >> regression on JDK-8145901. Is devScale still needed? Can we use xPtSize and<br>
> >> yPtSize (instead of ptSize*devScale) in _hb_jdk_font_create() and<br>
> >> _hb_jdk_ct_font_create()? I've confirmed that using xPtSize and yPtSize<br>
> >> fixes the Thai text rendering use case, at least.<br>
> >><br>
> >> Thanks!<br>
> >><br>
> >> Daniel<br>
> >><br>
> >> [1] <a href="https://bugs.openjdk.java.net/browse/JDK-8269888" rel="noreferrer" target="_blank">https://bugs.openjdk.java.net/browse/JDK-8269888</a><br>
> >> [2] <a href="https://www.google.com/get/noto/#sans-thai" rel="noreferrer" target="_blank">https://www.google.com/get/noto/#sans-thai</a><br>
> >> [3]<br>
> >> <a href="https://github.com/harfbuzz/harfbuzz/blob/bbeb3a62b0efbb598d8683f7c4b6cc7069a58aeb/src/hb-ot-layout-gpos-table.hh#L708" rel="noreferrer" target="_blank">https://github.com/harfbuzz/harfbuzz/blob/bbeb3a62b0efbb598d8683f7c4b6cc7069a58aeb/src/hb-ot-layout-gpos-table.hh#L708</a><br>
> >> [4]<br>
> >> <a href="https://github.com/harfbuzz/harfbuzz/blob/bbeb3a62b0efbb598d8683f7c4b6cc7069a58aeb/src/hb-ot-layout-gpos-table.hh#L2922" rel="noreferrer" target="_blank">https://github.com/harfbuzz/harfbuzz/blob/bbeb3a62b0efbb598d8683f7c4b6cc7069a58aeb/src/hb-ot-layout-gpos-table.hh#L2922</a><br>
> >> [5] <a href="https://harfbuzz.github.io/fonts-and-faces-custom-functions.html" rel="noreferrer" target="_blank">https://harfbuzz.github.io/fonts-and-faces-custom-functions.html</a><br>
> >> [6]<br>
> >> <a href="https://github.com/openjdk/jdk/blob/005d8a7fca8b4d9519d2bde0a7cdbbece1cd3981/src/java.desktop/share/native/libfontmanager/hb-jdk-font.cc#L271" rel="noreferrer" target="_blank">https://github.com/openjdk/jdk/blob/005d8a7fca8b4d9519d2bde0a7cdbbece1cd3981/src/java.desktop/share/native/libfontmanager/hb-jdk-font.cc#L271</a><br>
> >> [7]<br>
> >> <a href="https://github.com/openjdk/jdk/blob/005d8a7fca8b4d9519d2bde0a7cdbbece1cd3981/src/java.desktop/share/native/libfontmanager/hb-jdk-font.cc#L422" rel="noreferrer" target="_blank">https://github.com/openjdk/jdk/blob/005d8a7fca8b4d9519d2bde0a7cdbbece1cd3981/src/java.desktop/share/native/libfontmanager/hb-jdk-font.cc#L422</a><br>
> >> [8] <a href="https://github.com/harfbuzz/harfbuzz/discussions/3191" rel="noreferrer" target="_blank">https://github.com/harfbuzz/harfbuzz/discussions/3191</a><br>
> >> [9]<br>
> >> <a href="https://github.com/openjdk/jdk/commit/5935292ae022e970bd4075de4d704719f3b05575#diff-6a155985752d3cc5ca8a74d4bf51c899d80ea0337e30029a4fafe856893456f3L327-L328" rel="noreferrer" target="_blank">https://github.com/openjdk/jdk/commit/5935292ae022e970bd4075de4d704719f3b05575#diff-6a155985752d3cc5ca8a74d4bf51c899d80ea0337e30029a4fafe856893456f3L327-L328</a><br>
> >><br>
> >><br>
><br>
</blockquote></div>
</div>
</blockquote></div>