On Thu, 1 Aug 2024 19:51:09 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: > > Review feedback: floating point precision in test, ensure unsigned int > handling in Robot The code changes all look good. Everything looks good on macOS. I finished all my testing, including testing with the metal pipeline. The only thing needed is a timeout value on one of the tests to avoid hanging on Linux when running with a Wayland display server. I'll approve once you make that change. tests/system/src/test/java/test/robot/javafx/scene/SRGBTest.java line 218: > 216: // colors but they might both be working in the wrong space. We use > an > 217: // AWT Robot to verify that they are working in sRGB. > 218: @Test This test needs a timeout value to avoid hanging the test job on Linux with Wayland. See [JDK-8335468](https://bugs.openjdk.org/browse/JDK-8335468). I recommend: @Test(timeout = 15000) ------------- PR Review: https://git.openjdk.org/jfx/pull/1473#pullrequestreview-2217065570 PR Review Comment: https://git.openjdk.org/jfx/pull/1473#discussion_r1702793337
