On Fri, 31 May 2024 16:54:58 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>> The issue is specific to Mac. The glyph positions returned from native side >> for complex text is not handled in the text render logic. This issue is >> observed only when trailing spaces are present along with LTR text mixed >> with RTL text (Example: "Arabic: العربية"). >> >> Made changes in `getPosX` of `TextRun` class to handle negative values. >> >> Tested the changes manually with the sample code present in the bug and >> using the MonkeyTester. > > modules/javafx.graphics/src/main/java/com/sun/javafx/text/TextRun.java line > 332: > >> 330: return x; >> 331: } >> 332: int prevIdx = (glyphIndex - 1)<<1; > > would it be possible to add a comment explaining why this code is doing what > it's doing? specifically, the reason for the while() loop and Math.abs(). > and perhaps mention that this happens on mac only. I think if this is specific to mac, and doesn't occur on other platforms that the supplied `positions` may be incorrect by the underlying `*GlyphLayout` code. Under Windows it will likely use the `DWGlyphLayout` to supply `positions`. On Mac this will be `CTGlyphLayout`. The more prudent course of action than seems to be to fix what is supplied by `CTGlyphLayout` instead of working around it in `TextRun`. I haven't checked it that extensively yet, but I think `positions` is not supposed to contain negative `x` values at all when not in compact mode. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1468#discussion_r1623146989
