On Tue, 27 Aug 2024 15:50:54 GMT, Jose Pereda <[email protected]> wrote:
> This PR adds a couple of null checks to `LogicalFont` and `FTFactory`, that
> make use of `FontConfigManager::getFontConfigFont`, which under certain
> cases, can return null.
>
> I've tested this PR on both Linux (Ubuntu 22.04.4) and Android, just using
> `-Dprism.useFontConfig=false`.
>
> On Ubuntu, I've manually added some font files to the JDK path
> `$JAVA_HOME/lib/fonts` (which didn't exist), and the test passes now, though
> this message is still printed out:
>
> Error: JavaFX detected no fonts! Please refer to release notes for proper
> font configuration
>
> which is confusing to me, because fonts where found in the JDK path after
> all, and even in the case that there were no fonts found, "the release notes"
> is an ambiguous reference for the user.
>
> Also, instead of adding fonts to the JDK, I tested adding a
> `logicalfonts.properties` file with `-Dprism.fontdir` and without
> `fontConfig`, but this case was already working before the patch for this PR,
> and still works after it.
>
> Note that if there are no fonts in the JDK path, and `prism.fontdir` is not
> provided, without `fontConfig`, after this PR, there will still be an
> exception:
>
>
> Caused by: java.lang.NullPointerException: Cannot invoke
> "com.sun.javafx.font.FontResource.getDefaultAAMode()" because the return
> value of "com.sun.javafx.font.LogicalFont.getSlot0Resource()" is null
> at
> javafx.graphics@24-internal/com.sun.javafx.font.LogicalFont.getDefaultAAMode(LogicalFont.java:456)
> at
> javafx.graphics@24-internal/com.sun.javafx.font.LogicalFont.getStrike(LogicalFont.java:461)
> at
> javafx.graphics@24-internal/com.sun.javafx.font.PrismFont.getStrike(PrismFont.java:79)
> at
> javafx.graphics@24-internal/com.sun.javafx.text.PrismTextLayout.setContent(PrismTextLayout.java:139)
> at
> javafx.graphics@24-internal/javafx.scene.text.Text.getTextLayout(Text.java:303)
> at
> javafx.graphics@24-internal/javafx.scene.text.Text.needsFullTextLayout(Text.java:258)
> at
> javafx.graphics@24-internal/javafx.scene.text.Text$6.invalidated(Text.java:576)
> ...
>
>
> because `LogicalFont::getSlot0Resource` will return null. Adding null checks
> after this point will require many changes (`LogicalFont::getSlotResource(0)`
> is used in many places). Shouldn't we fail gracefully after
> `LogicalFont::getSlot0Resource` if `slot0FontResource` is finally null?
modules/javafx.graphics/src/main/java/com/sun/javafx/font/freetype/FTFactory.java
line 142:
> 140: for (int i=0; i<linkedFontNames.size(); i++) {
> 141: info.add(linkedFontNames.get(i), linkedFontFiles.get(i),
> null);
> 142: }
Downstream code seems to be able to deal with empty `FontFallbackInfo`, so I
guess this is also okay.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1546#discussion_r1733334899
