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