RFR: 8185261: Font fallback sometimes doesn't work in Swing text components [v2]

[Dmitry Batrak]

On Wed, 2 Feb 2022 21:29:28 GMT, Sergey Bylokhov <serb at openjdk.org> wrote:

>> src/java.desktop/share/classes/java/awt/Font.java line 1861:
>> 
>>> 1859:                 pointSize == font.pointSize &&
>>> 1860:                 withFallback == font.withFallback &&
>>> 1861:                 name.equals(font.name)) {
>> 
>> I don't know how much of the fix falls apart if this is changed, but I am not sure about adding this invisible attribute to equals(). I suppose font2DHandle would have the same issue .. but I didn't think that could be used in equals because it is assigned lazily so it would present even more problem.
>> I do see that it would be tricky to have some private extra equality testing in the code that finds the right fall back but I'd still like to see if that approach might work. Can you add in to the bug a more detailed explanation of where exactly you see the problem occur ? 
>> 
>> I need some convincing and really would like to see if there is an alternative solution that I might prefer but that is something I can't switch on to right now since nothing is coming to me which means it is likely to take some time.
>> 
>> I didn't get the JCK comment createdFont is transient .. and nothing serializes the font data.
>> Serialization of Font objects is just serializing the fields liek name and style and size but there's no guarantee that it will behave the same .. I mean if I serialize Segoe UI on WIndows and try to deserialize it on Linux nothing in the world can fix the issue that Linux doesn't have that font.
>
>> Serialization of Font objects is just serializing the fields liek name and style and size but there's no guarantee that it will behave the same .. I mean if I serialize Segoe UI on WIndows and try to deserialize it on Linux nothing in the world can fix the issue that Linux doesn't have that font.
> 
> Since the Font object will lose some fields during serialization its behavior will be changed even on the same platform.

I've added some more details to the ticket about why exactly given test case isn't working as expected. Please let me know if I wasn't clear enough.

> I don't know how much of the fix falls apart if this is changed, but I am not sure about adding this invisible attribute to equals().

That added line in `equals()` method is actually the main change in the fix. As you say, one could possibly also try to compare `font2DHandle`-s instead, but that would require forcing the population of that field in `equals` method, which seems to have larger potential for side effects.

As a font with fallback components can produce different rendering that the font without them, I believe, they shouldn't be considered 'equal'. Of course, there's an alternative method to fix this - make font metrics cache distinguish such fonts explicitly, without modifying `Font.equals()`. But font metrics cache isn't the only place comparing fonts using `equals()` method, and such a solution, I believe, would just mean waiting for other trouble in future.

The fact that `equals` would be using some private information isn't perfect of course. Ideally, this information (whether the font supports font fallback or not) should be public, and it should be possible to specify this property when creating a font object, but that needs introducing new API methods, and I don't want to go that far at the moment. Also, we're already living in a non-perfect world, where two `Font` objects with identical public properties might behave differently, and now we're just changing `equals` method to reflect that.

As for the comment about JCK, that relates to a yet another implementation option - do not introduce a new field, and just add comparing of `createdFont` fields in `equals` method. That's the way it was originally implemented in JBR 4 years ago, but after we found out it breaks JCK, the fix was changed to use a separate field. Corresponding JCK test, creates a font object from a file on filesystem, using `Font.createFont` method, serializes/deserializes it, and checks that the result is 'equal' to the initially created font. As `createdFont` field is transient, it's reset to `false` after serialization/deserialization, and the test obviously fails, if we try to include `createdFont` check into `equals`. Making `createdFont` field not transient also doesn't seem to be an option, as that is also considered an API change.

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

PR: https://git.openjdk.java.net/jdk/pull/7313