RFR: 8240756: [macos] SwingSet2:TableDemo:Printed Japanese characters were garbled [v3]

[Phil Race]

On Thu, 21 Oct 2021 00:57:47 GMT, Toshio Nakamura <tnakamura at openjdk.org> wrote:

>> Hi,
>> 
>> Could you review the fix?
>> When non-English characters were printed from JTable on MacOS, CTextPipe.doDrawGlyphs was called by OSXSurfaceData.drawGlyphs. However, CTextPipe seems not support glyph with slot number of composite fonts.
>> 
>> The slot data mask of GlyphVector is 0xff000000. In my environment, Japanese font was loaded at slot 4, and glyph data is like [0x40003e5]. Then, unexpected glyph was drawn.
>
> Toshio Nakamura has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains seven additional commits since the last revision:
> 
>  - 8240756: [macos] SwingSet2:TableDemo:Printed Japanese characters were garbled
>  - revert previous proposal
>  - Merge branch 'master' into 8240756
>  - 2nd proposal
>  - Revert previous change
>  - Merge branch 'master' into 8240756
>    merge master
>  - 8240756: [macos] SwingSet2:TableDemo:Printed Japanese characters were garbled

Did this bug appear in some release, or do you think it never worked ?

Although I don't expect 255 slots your code like
if (gv.getGlyphCode(i) >= 0x1000000)

and
int slot = gV.getGlyphCode(i) >> 24;

will break if they are ..

The logic in drawGlyphVector where both initial slot != 0 and the final slot are handled
specially make this code more complex than desirable. Can you make the logic cleaner ?

The test relies heavily on internals. I'm not sure how long it will last.
"Pixel" (not Pixcel) count is an indicator that the rendering is the same, but why not
direct pixel to pixel comparison ?
Also it isn't actually testing that Japanese is rendered. It is testing that the images are the same
so if they are both "wrong" the test will still pass.

Did you ever look into why drawString did not need fixing too ?

Have you run other printing tests besides the new one to verify this all works ?

More importantly what on-screen testing have you done ?
Or have you proved CTextPipe is only used in printing ?

-------------

PR: https://git.openjdk.java.net/jdk/pull/3619