JDK-8269888: Thai text rendered incorrectly using some AffineTransform-derived fonts

Mon Oct 28 18:42:44 UTC 2024

Hi Phil,

I think I've figured out JDK-8269888 (Thai text rendered incorrectly using
some AffineTransform-derived fonts).

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.

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.

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?

Here are the changes as they currently stand, if you'd like to have a look:
https://github.com/gredler/jdk/commit/d8dff2404c085e94b308450152c152d092b1fe2e

Take care,

Daniel

On Fri, Oct 25, 2024 at 7:48 PM Daniel Gredler <djgredler at gmail.com> wrote:

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