On Wed, 25 Oct 2023 23:42:08 GMT, Phil Race <p...@openjdk.org> wrote:
>> 8318364: Add an FFM-based implementation of harfbuzz OpenType layout
>
> Phil Race has updated the pull request incrementally with one additional
> commit since the last revision:
>
> indentation
src/java.desktop/share/classes/sun/font/HBShaper.java line 628:
> 626: MemorySegment glyphInfoArr =
> glyphInfo.reinterpret(glyphInfoSize);
> 627:
> 628: for (int i=0; i<glyphCount; i++) {
Spacing between operators
src/java.desktop/share/classes/sun/font/HBShaper.java line 658:
> 656: startPt.x = advX;
> 657: startPt.y = advY;
> 658: startPt.x = advX;
duplicate assignment of startPt.x to advX...
src/java.desktop/share/classes/sun/font/SunLayoutEngine.java line 167:
> 165: }
> 166:
> 167: static boolean useFFM = true;
So, we want to enable FFM by default?
src/java.desktop/share/classes/sun/font/SunLayoutEngine.java line 184:
> 182: FontStrike strike = font.getStrike(desc);
> 183: if (useFFM) {
> 184: java.lang.foreign.MemorySegment face =
> HBShaper.getFace(font);
Guess import at start will be more nicer...
src/java.desktop/share/native/libfontmanager/HBShaper_Panama.c line 141:
> 139: hb_buffer_destroy (buffer);
> 140: hb_font_destroy(hbfont);
> 141: if (features != NULL) free(features);
Guess coding style warrants braces { and next statement in separate line...
src/java.desktop/share/native/libfontmanager/hb-jdk-font-p.cc line 238:
> 236: HBFloatToFixed(ptSize*devScale),
> 237: HBFloatToFixed(ptSize*devScale));
> 238: return font;
indentation is off..
test/jdk/java/awt/font/GlyphVector/LayoutCompatTest.java line 29:
> 27: /*
> 28: @test
> 29: @summary verify JNI and FFM harfbuzz OpenType layout implementations
> are equivalent.
bug id is missing
test/jdk/java/awt/font/GlyphVector/LayoutCompatTest.java line 45:
> 43: public class LayoutCompatTest {
> 44:
> 45: static String jni = "jni.txt";
Seems test is failing without fix with Exception in jtreg
java.io.FileNotFoundException: jni.txt (The system cannot find the file
specified)
Also in standalone mode. I was expecting it will fail with RuntimeException
"files differ byte offset"
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15476#discussion_r1395720270
PR Review Comment: https://git.openjdk.org/jdk/pull/15476#discussion_r1395721851
PR Review Comment: https://git.openjdk.org/jdk/pull/15476#discussion_r1395723605
PR Review Comment: https://git.openjdk.org/jdk/pull/15476#discussion_r1395726378
PR Review Comment: https://git.openjdk.org/jdk/pull/15476#discussion_r1395748600
PR Review Comment: https://git.openjdk.org/jdk/pull/15476#discussion_r1395750344
PR Review Comment: https://git.openjdk.org/jdk/pull/15476#discussion_r1395752417
PR Review Comment: https://git.openjdk.org/jdk/pull/15476#discussion_r1395788998
