On Sat, 1 Jun 2024 04:53:48 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:
>> 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. Yes the issue is specific to Mac only. Windows uses `DWGlyphLayout` for positions as you pointed out. Since the position obtained from the underlying platform itself is not correct Mac, looked like fixing it in either `TextRun` or in the native side does not make huge difference so went with this approach. However I will explore how we can fix it on the native side. If the `positions` is not supposed to contain negative value, it is an issue from Apple not in JavaFX. In this case should we actually fix the issue in JavaFX, as it becomes a workaround rather than a fix regardless of where we fix it. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1468#discussion_r1625369735
