On Fri, 24 Feb 2023 18:58:37 GMT, Kevin Rushforth <k...@openjdk.org> wrote:
>> This fix properly supports colour rendering of Emoji on macOS
>>
>>
>> On other platforms the Emoji will be rendered as ordinary greyscale glyphs -
>> if there is font
>> support for the requested code point.
>>
>> A simple manual test is provided which uses a Text node, Label control
>> and editable TextField control.
>>
>> Some highlights of the code
>> - To determine if it is a color emoji glyph probe the 'sbix' font table
>> which is what is used by Apple
>> - Text runs now break at an Emoji glyph
>> - The Emoji is retrieved as an BGRA image - ie 4 channel including alpha
>> - It was necessary to retrieve the Emoji glyph bounds via a different
>> CoreText API since
>> the bounds that were being retrieved were wrong for the Emoji image -
>> causing clipping
>> - It was necessary to retrieve the Emoji code point advance via a CoreText
>> API since
>> the HMTX metrics were very wrong - causing overlapping glyphs
>> - drawString checks if it is an Emoji run and redirects to a new
>> drawColorGlyph method
>> which draws the image as a texture
>> - All 3 rendering pipelines have this support and have been verified on all
>> 3 desktop platforms
>
> modules/javafx.graphics/src/main/java/com/sun/javafx/font/PrismFontFile.java
> line 1182:
>
>> 1180:
>> 1181:
>> 1182: public float getAdvance(int glyphCode, float ptSize) {
>
> Did you mean to remove the `@Override` for this method?
Nope. I'll put it back.
> modules/javafx.graphics/src/main/java/com/sun/javafx/font/PrismFontFile.java
> line 1184:
>
>> 1182: public float getAdvance(int glyphCode, float ptSize) {
>> 1183: if (glyphCode == CharToGlyphMapper.INVISIBLE_GLYPH_ID)
>> 1184: return 0f;
>
> Please surround with curly braces.
OK .. but I did not add that as such .. it was there already.
> modules/javafx.graphics/src/main/java/com/sun/javafx/font/PrismFontFile.java
> line 1430:
>
>> 1428:
>> 1429: private static int USHORT_MASK = 0xffff;
>> 1430: private static int UINT_MASK = 0xffffffff;
>
> Minor: can be final.
ok
> modules/javafx.graphics/src/main/java/com/sun/javafx/font/PrismFontFile.java
> line 1448:
>
>> 1446: return false;
>> 1447: }
>> 1448: return dataOffsets[gid] < dataOffsets[gid+1];
>
> If `gid` is equal to `dataOffsets.length - 1`, won't this throw AIOOBE?
No, this is correct.
See https://learn.microsoft.com/en-us/typography/opentype/spec/sbix
"The glyphDataOffset array includes offsets for every glyph ID, plus one extra."
> modules/javafx.graphics/src/main/java/com/sun/javafx/font/coretext/CTGlyph.java
> line 90:
>
>> 88:
>> 89: if (drawShapes) return;
>> 90:
>
> Just curious: do you know why this test was there in the first place? And why
> it needs to be removed?
My guess is that the info it gathered was not used in the glyph image path
and it was an attempt at optimisation.
As to why it needed to be removed, it is because I DO need that information
and there'll be a NPE (IIRC) if I don't remove it.
-------------
PR: https://git.openjdk.org/jfx/pull/1047
