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? I don't understand why this change is needed. Perhaps I'm misunderstanding, but if usefontconfig is false, that means "I have supplied embedded fonts instead". In which case the API calls where the null checks are added should not be returning null. In other words, setting this to false, and not supplying fonts is a misconfiguration. Arguably we should disable this property and always look for fontconfig. But one would only know how to misconfigure it by reading the sources to discover this property. ------------- PR Comment: https://git.openjdk.org/jfx/pull/1546#issuecomment-2316356553
