On Thu, 25 Jul 2024 23:45:27 GMT, Jorn Vernee <jver...@openjdk.org> wrote:
>> Phil Race has updated the pull request incrementally with one additional >> commit since the last revision: >> >> 8337237 > > src/java.desktop/share/classes/sun/java2d/pipe/RenderBuffer.java line 176: > >> 174: int lengthInBytes = length * SIZEOF_SHORT; >> 175: MemorySegment.copy(x, offsetInBytes, segment, JAVA_SHORT, >> curOffset, length); >> 176: position(position() + lengthInBytes); > > If you want, you could use `MemoryLayout::scale` here, i.e. > Suggestion: > > position(JAVA_SHORT.scale(position(), length)); > > (up to preference though) I guess that would allow me to get rid of lengthInBytes. But this scale API does one of the things in FFM that I find too easy to cause mistakes - the first param is offset count in bytes and the second param is the the one that is scaled by the layout size. One might assume both are scaled since it is called scale(), not scaleIndex(), You have to pay close attention. And that's what caught me out switching from one copy() overload to another. So I tried it, and I didn't like it as much as the existing code. I did however get rid of the lengthInBytes variable. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/20339#discussion_r1693447102
