RFR: 8350203: [macos] Newlines and tabs are not ignored when drawing text to a Graphics2D object [v3]

[Harshitha Onkar]

On Tue, 6 May 2025 23:31:34 GMT, Daniel Gredler <dgredler at openjdk.org> wrote:

>> src/java.desktop/macosx/classes/sun/font/CCharToGlyphMapper.java line 138:
>> 
>>> 136:         return code == 0x0009 || code == 0x000a || code == 0x000d;
>>> 137:     }
>>> 138: 
>> 
>> @prrace @gredler 
>> Following is an existing issue NOT related to the recent harfbuzz upgrade PR nor this issue fix but it is similar in nature so a separate JBS issue can be created to handle it. I wanted to discuss it before integrating the HB upgrade tomorrow.
>> 
>> There seems to be similar issue on windows and linux where the hexcode - 0x2028 (line separator) and 0x2029 (paragraph separator) are shown with a non-zero advance - `/java/awt/font/TextLayout/TestControls.java`
>> 
>> **On Windows:**
>> 
>> <img width="287" alt="image" src="https://github.com/user-attachments/assets/9eec9009-e3a5-49be-b505-619d041bfa3b" />
>> 
>> **On Linux:**
>> 
>> <img width="272" alt="image" src="https://github.com/user-attachments/assets/73a56d6d-53ec-4d6f-ad49-6fc13bdceea0" />
>> 
>> 
>> If adding these codes (0x2028, 0x2029) to platform specific src code (I believe it is NativeGlyphMapper (linux) and WPathGraphics (windows)) fixes the above issue then should we consider moving isIgnorableWhitespace() to common class - `CharToGlyphMapper` instead of macOS specific src?
>
> Yeah, I think right now `\t` `\r` `\n` handling is done separately for each platform, and I think also separately for screen display vs. printing.
> 
> As an example, this fix addresses the screen display side of things for macOS, but printing on macOS needs to be fixed separately (as can be seen when you run `test/jdk/java/awt/print/PrinterJob/PrintTextTest.java`). I had a quick look at the corresponding macOS printing issue a few months ago and the fix wasn't obvious, especially given how that part of the code is organized (not really 1-to-1 with the other platforms).
> 
> So I think `\t` `\r` `\n` handling is repeated quite a bit, and if we wanted to handle `0x2028` and `0x2029` exactly like we do `\t` `\r` `\n` then we'd need to change multiple places right now.
> 
>> should we consider moving isIgnorableWhitespace() to common class - CharToGlyphMapper instead of macOS specific src
> 
> That sounds like it could be a good idea, but I don't have a sense of how likely it is to cause breakage elsewhere -- it'd be a matter of "do it and check for test regressions". Maybe you have a better gut feel for the likelihood of unintended side-effects?
> 
> Note these two characters (`0x2028` and `0x2029`) are not default-ignorable, though. It sounds like we just don't want to render their glyphs, if they have any (similar to `\t` `\r` `\n`, which are also not default-ignorable).
> 
> Over on PR #24412 Nikita has suggested an interesting change to `CharToGlyphMapper` and subclasses which might make this change easier, but I'd like some feedback from a reviewer or two before moving forward. The PR is complete from my POV, but we need some design direction (good as is, or needs tweaks, or need to use a different approach entirely, etc).

@gredler This issue _[JDK-8356803](https://bugs.openjdk.org/browse/JDK-8356803) - Test TextLayout/TestControls fails on windows & linux line and paragraph separator show a non-zero advance_, is re-assigned to you since it is a regression of [JDK-8208377](https://bugs.openjdk.org/browse/JDK-8208377)

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23665#discussion_r2085495237