On Mon, 29 Jul 2024 23:26:46 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>> Martin Fox has updated the pull request incrementally with one additional >> commit since the last revision: >> >> The last deprecated declaration has been removed from GlassView3D.m > > tests/system/src/test/java/test/robot/javafx/scene/SRGBTest.java line 52: > >> 50: >> 51: import org.junit.Test; >> 52: import static org.junit.Assume.assumeTrue; > > should we use junit5 for the new tests? This test is based on the VisualTestBase test harness class which is written using junit4. I use that test harness since it contains waitFirstFrame and waitNextFrame as well as existing utility routines for reporting on color mismatches. > tests/system/src/test/java/test/robot/javafx/scene/SRGBTest.java line 68: > >> 66: // The component tolerance allows one bit of rounding when writing a >> color >> 67: // out and another bit when reading it back in. >> 68: static final double COMPONENT_TOLERANCE = 2.0 / 255.0; > > suggestion: 2.0001 / 255.0 to account for floating point errors I'm not sure this test needs to be that precise. And I'm fine with the threshold falling below 2.0/255.0 since it should be as small as possible (I even experimented briefly with lower numbers). I haven't run into an issue on the three platforms I've run this test on. > tests/system/src/test/java/test/robot/javafx/scene/SRGBTest.java line 99: > >> 97: >> 98: // We use an AWT Robot since it is color space aware and will >> correctly convert >> 99: // from the screeen's color space to sRGB. > > do we plan to fix FX robot eventually? This PR fixes the JavaFX Robot. I give a more detailed explanation of why I'm using the AWT Robot in a comment just before the sRGBPixelTest. I've updated the comments. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1473#discussion_r1697713199 PR Review Comment: https://git.openjdk.org/jfx/pull/1473#discussion_r1697713019 PR Review Comment: https://git.openjdk.org/jfx/pull/1473#discussion_r1697712939
