RFR: 8318364: Add an FFM-based implementation of harfbuzz OpenType layout [v4]

[Prasanta Sadhukhan]

On Wed, 25 Oct 2023 23:42:08 GMT, Phil Race <prr at 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