RFR: 8246104: Some complex text doesn't render correctly on macOS [v2]

[Kevin Rushforth]

On Tue, 4 Apr 2023 20:47:09 GMT, Phil Race <prr at openjdk.org> wrote:

>> This PR addresses some font problems on macOS.
>> (1) Garbled Arabic with the System font
>> (2) Non-ideal fallback fonts for all fonts
>> (3) No bold for System font.
>> 
>> In particular the standard System Font was garbling Arabic text - random glyphs from another font.
>> The root of this issue is that several releases ago macOS introduced UI fonts that are hidden from
>> enumeration. must be loaded by a special API and cannot be loaded by name, even if you extract
>> the name from the core text font loaded by that special API.
>> These fonts usually have a name that begins with "." such as the postscript name ".AppleUISystemFont"
>> and "name" for the main instance of this will be "System Font Regular". These live in files called "SFNS.ttf"
>> and "SFArabic.ttf" and similar.
>> 
>> JavaFX has its own "System" font family logical name and in order to map to the macOS system font,
>> several releases ago we started using the special API referenced above.
>> 
>> Normally we have our own list of fall back fonts for other scripts.
>> But when we encounter complex scripts such as Arabic, and ask CoreText to layout the glyphs,
>> it may use other system fonts for Arabic (eg ".SFArabic") and the FX code dynamically adds
>> this new font to its fallback list - by name.
>> But this new font can't be directly referenced by name either, but that's what used to work and the FX code tries.
>> 
>> So to fix this we need to grab the reference to the font that was returned by CoreText and use
>> that to create the PrismFont we add as a fall back.
>> The code that first recognises it needs to go this route is in CTGlyphLayout.java when
>> "getSlotForFont(String name") fails.
>> Instead it calls new code in CTFactory which in turn calls a new constructor and supporting code in CTFontFile.java.
>> There's also supporting changes in native code - coretext.c
>> 
>> Additionally to support that when we ask the platform to enumerate fallbacks, we might get such "." fonts,
>> we need to make some more changes
>> Now on to more "fallback" enhancements.
>> Prior to this fix, on macOS we had a short, hard-coded global list.
>> This fix calls the macOS API to get the cascading fallback list for a font.
>> This improves the fallback behaviour in respect to providing
>> (1) More appropriate styles, (2) More supported code pointd, (3) Code point support coming from a preferred font.
>> There was some desirable re-factoring as part of this, as the location of fallbacks was pushed from
>> "if <windows> else if <linux> else <mac> in shared code, down into the appropriate platform factory
>> 
>> This also made it easier to do some required changes to allow fallbacks to be pre-initialised with the
>> native font resource, not just a name+ file. The new class FontFallbackInfo encapsulates this and changes
>> were made to follow it.
>> In practice on macOS it seems that some "." cascading fallbacks for the "." system font come with a "NULL"
>> file name. Without being able to identify the file we aren't able to use the font to skip them.
>> More changes would be needed to provide a PrismFont subclass that can handle this case.
>> Fortunately that isn't happening for any of the fallbacks we get from complex text layout.
>> 
>> The final thing this fixes is that "System Bold" was the same as "System Regular".
>> The needed name getting lost along the way to constructing the native font.
>> 
>> The work done here also highlighted that we really need to add code that at least recognises OpenType variation fonts.
>> And some day after that we could expose API that allows apps to request non-named variations.
>
> Phil Race has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fix up Thai font name

modules/javafx.graphics/src/main/java/com/sun/javafx/font/coretext/CTFactory.java line 59:

> 57:             System.err.println("Fix up double use of Regular in name : " + name);
> 58:             if (debugFonts) {
> 59:             }

I guess you meant to put the print inside the `if (debugFonts)`?

-------------

PR Review Comment: https://git.openjdk.org/jfx/pull/1067#discussion_r1157745044