On Tue, 27 Aug 2024 15:50:54 GMT, Jose Pereda <jper...@openjdk.org> 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? So far, the documentation in https://wiki.openjdk.org/display/OpenJFX/Font+Setup is still somehow valid in this situation (no fontConfig and providing embedded fonts). So I can revert the change in `LogicalFont`, and modify the message in `FontConfigManager::initFontConfigLogFonts`: System.err.println("Error: JavaFX detected no fonts! " + "Please refer to release notes for proper font configuration"); ``` to something more meaningful, like? System.err.println("Error: JavaFX detected no fonts!\n " + "For proper font configuration, please check https://wiki.openjdk.org/display/OpenJFX/Font+Setup";); ``` The NPE will be still present though. Thoughts? ------------- PR Comment: https://git.openjdk.org/jfx/pull/1546#issuecomment-2320964768
