On Tue, 30 Jul 2024 23:34:13 GMT, Martin Fox <m...@openjdk.org> wrote:

>> When drawing to the screen JavaFX is producing sRGB colors but on macOS 
>> that’s not necessarily what the user is seeing. Since the pixels are not 
>> tagged as sRGB the OS is copying them unmodified to the frame buffer to be 
>> displayed in the screen’s color space. In general Mac’s don’t default to 
>> sRGB so the colors will be wrong. The fix for this is a one-liner; we just 
>> need to declare that our CALayer uses the sRGB color space so the OS will 
>> convert it to the screen’s space (presumably with a slight performance 
>> penalty).
>> 
>> In the reverse direction the Robot should be returning sRGB colors. The 
>> getPixelColor calls were making no conversion. The getScreenCapture calls 
>> were converting to genericRGB, not sRGB, and so the results didn’t match the 
>> getPixelColor calls. This PR fixes these bugs; getPixelColor and 
>> getScreenCapture both return sRGB.
>> 
>> Now that everything is working in the same space when JavaFX writes out a 
>> pixel and then reads it back in the colors should match within a limited 
>> tolerance (due to rounding issues when converting from float to int and 
>> back). But that just means the various glass code paths are using the same 
>> space to perform conversions, not that it’s sRGB. AWT is color space aware 
>> and so the automated test employs an AWT Robot to double-check the results.
>> 
>> I swept through the rest of the Mac glass code and found a few places where 
>> colors were being converted to deviceRGB instead of sRGB e.g. when reading 
>> colors for PlatformPreferences or creating images for drag and drop. I could 
>> not think of a good way of writing automated tests for these cases.
>> 
>> I started investigating this since Robot tests were failing unless the 
>> monitor’s profile was set to sRGB. Unfortunately this PR doesn’t entirely 
>> fix that. My monitor uses Display P3 and I’m still seeing failures on the 
>> SwingNodeJDialogTest. The test writes out pure BLUE colors and gets back 
>> colors with a surprising amount of red. I’ve verified that this has nothing 
>> to do with JavaFX, it happens when I use CoreGraphics to make the sRGB => 
>> Display P3 color conversions directly. I guess this is a cautionary tale 
>> about what happens when you work near the edges of a color space’s gamut.
>
> Martin Fox has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Updated comments concerning the AWT Robot

The reproducer in the ticket fails without the fix and works as described on 
2021 macOS 14.5 M1, using built-in retina and an external Samsung LS27A600U 
display (which seems to have a different color profile).

I would like to request a change for SRGBTest.COMPONENT_TOLERANCE value (or 
comment), please see inline comments.

modules/javafx.graphics/src/main/native-glass/mac/GlassRobot.m line 423:

> 421:                         components[1] = (CGFloat)((color & 0x0000FF00) 
> >> 8) / 255.0;
> 422:                         components[2] = (CGFloat)((color & 0x000000FF)) 
> / 255.0;
> 423:                         components[3] = (CGFloat)((color & 0xFF000000) 
> >> 24) / 255.0;

is this line guaranteed not to produce negative values?  e.g. when `color = 
0xffffffff`

tests/system/src/test/java/test/robot/javafx/scene/SRGBTest.java line 100:

> 98:     // An AWT Robot is color space aware and will correctly convert from 
> the
> 99:     // screeen's color space to sRGB. We use one to verify that the JavaFX
> 100:     // Robot is performing the same conversions.

this comment is much clearer, thank you!

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

PR Review: https://git.openjdk.org/jfx/pull/1473#pullrequestreview-2213465658
PR Review Comment: https://git.openjdk.org/jfx/pull/1473#discussion_r1700569990
PR Review Comment: https://git.openjdk.org/jfx/pull/1473#discussion_r1700605558

Reply via email to