On Tue, 29 Aug 2023 22:04:40 GMT, Phil Race <p...@openjdk.org> wrote:
> 8318364: Add an FFM-based implementation of harfbuzz OpenType layout
src/java.desktop/share/classes/sun/font/HBShaper.java line 310:
> 308: SequenceLayout glyphInfosLayout =
> MemoryLayout.sequenceLayout(maxinfo, GlyphInfoLayout);
> 309: codePointHandle = getVarHandle(glyphInfosLayout, "codepoint");
> 310: clusterHandle = getVarHandle(glyphInfosLayout, "cluster");
There are better ways to do this in the latest API, by using the extra offset
parameter and `MemoryLayout::scaleHandle`.
I suggest changing this to:
Suggestion:
x_offsetHandle = getVarHandle(PositionLayout, "x_offset");
y_offsetHandle = getVarHandle(PositionLayout, "y_offset");
x_advanceHandle = getVarHandle(PositionLayout, "x_advance");
y_advanceHandle = getVarHandle(PositionLayout, "y_advance");
codePointHandle = getVarHandle(GlyphInfoLayout, "codepoint");
clusterHandle = getVarHandle(GlyphInfoLayout, "cluster");
And then in `getVarHandle`:
private static VarHandle getVarHandle(MemoryLayout enclosing, String name) {
VarHandle h = layout.varHandle(PathElement.groupElement(name));
// scale offset by the size of 'enclosing'
h = MethodHandles.collectCoordinates(h, 1, enclosing.scaleHandle());
/* insert 0 offset so don't need to pass arg every time */
return MethodHandles.insertCoordinates(h, 1,
0L).withInvokeExactBehavior();
}
(hope I eyeballed that correctly...)
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15476#discussion_r1361989861
