On Wed, 15 Nov 2023 15:06:55 GMT, Jayathirth D V <j...@openjdk.org> wrote:
>> 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 66:
>
>> 64: * };
>> 65: */
>> 66: private static final UnionLayout VarIntLayout =
>> MemoryLayout.unionLayout(
>
> Indentation issue.
fixed
> src/java.desktop/share/classes/sun/font/HBShaper.java line 84:
>
>> 82: * };
>> 83: */
>> 84: private static final StructLayout PositionLayout =
>> MemoryLayout.structLayout(
>
> Indentation issue.
fixed
> src/java.desktop/share/classes/sun/font/HBShaper.java line 144:
>
>> 142: private static final MemorySegment store_layout_results_stub;
>> 143:
>> 144: private static FunctionDescriptor
>
> Indentation issue.
fixed
> src/java.desktop/share/classes/sun/font/HBShaper.java line 153:
>
>> 151: }
>> 152:
>> 153: private static MethodHandle getMethodHandle
>
> Indentation issue.
fixed
> src/java.desktop/share/classes/sun/font/HBShaper.java line 165:
>
>> 163: }
>> 164:
>> 165: static {
>
> Indentation issue.
fixed
> src/java.desktop/share/classes/sun/font/HBShaper.java line 212:
>
>> 210: jdk_hb_shape_handle = tmp4;
>> 211:
>> 212: Arena garena = Arena.global(); // creating stubs that exist
>> until VM exit.
>
> Variable name `gArena`(for global arena) is better than using `garena`.
i'm using lowercase in the other names, so I think it OK
> src/java.desktop/share/classes/sun/font/HBShaper.java line 347:
>
>> 345: glyphIDPtr.setAtIndex(JAVA_INT, 0, glyphID);
>> 346: return (glyphID != 0) ? 1 : 0;
>> 347: }
>
> Indentation issue.
fixed
> src/java.desktop/share/classes/sun/font/HBShaper.java line 659:
>
>> 657: startPt.y = advY;
>> 658: startPt.x = advX;
>> 659: }
>
> Indentation issue.
fixed
> src/java.desktop/share/classes/sun/font/SunLayoutEngine.java line 193:
>
>> 191: } else {
>> 192: long pFace = getFacePtr(font);
>> 193: shape(font, strike, ptSize, mat, pFace,
>
> Is it okay to not have `(pFace != 0)` check here?
i added back the check
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15476#discussion_r1396267684
PR Review Comment: https://git.openjdk.org/jdk/pull/15476#discussion_r1396267831
PR Review Comment: https://git.openjdk.org/jdk/pull/15476#discussion_r1396268039
PR Review Comment: https://git.openjdk.org/jdk/pull/15476#discussion_r1396268116
PR Review Comment: https://git.openjdk.org/jdk/pull/15476#discussion_r1396268358
PR Review Comment: https://git.openjdk.org/jdk/pull/15476#discussion_r1396268796
PR Review Comment: https://git.openjdk.org/jdk/pull/15476#discussion_r1396269031
PR Review Comment: https://git.openjdk.org/jdk/pull/15476#discussion_r1396269300
PR Review Comment: https://git.openjdk.org/jdk/pull/15476#discussion_r1396267142
