On Mon, 23 Jun 2025 08:26:30 GMT, Johan Vos <j...@openjdk.org> wrote:
>> After spending a year in the sandbox repository, the Headless Platform is >> now ready to be reviewed in the main repository. >> >> ### the Headless Platform >> The Headless Platform is a top-level com.sun.glass.ui platform that replaces >> the second-level Monocle-Headless subplatform, that is part of the top-level >> Monocle platform. >> The platform can be used like any other platform, especially for running >> headless JavaFX applications, or for running tests (e.g. on CI systems) >> >> ### changes >> The code for the Headless Platform is in a new package >> com.sun.glass.ui.headless in the javafx.graphics module, and it does not >> require a code change in other packages. >> This PR adds a simple change in the `build.gradle` file, to make the >> Headless Platform the standard when running headless tests (instead of using >> Monocle/Headless) >> >> ### enable the Headless Platform >> Setting the system property `glass.platform` to `Headless` will select the >> Headless Platform instead of the default one (either gtk, mac or win). >> >> ### testing >> `gradlew --info -PHEADLESS_TEST=true -PFULL_TEST=true :systemTests:cleanTest >> :systemTests:test` >> runs all the system tests, apart from the robot tests. There are 2 failing >> tests, but there are valid reasons for those to fail. >> >> ### robot tests >> Most of the robot tests are working on headless as well. add `-PUSE_ROBOT` >> to test those. > > Johan Vos has updated the pull request incrementally with one additional > commit since the last revision: > > Process reviewer comments Added a little code to the monkey tester to take a snapshot of the scene. Here is the result launching it on the headless platform on macOS 15.5:  Good job! Question: While the screen size is limited in this version to 1000x1000, the size of the snapshot exceeds that. Is that expected? modules/javafx.graphics/src/main/java/com/sun/glass/ui/headless/HeadlessApplication.java line 22: > 20: private final NestedRunnableProcessor processor = new > NestedRunnableProcessor(); > 21: private final HeadlessWindowManager windowManager = new > HeadlessWindowManager(); > 22: private Screen[] screens = null; minor: no need to assign null, also L24 modules/javafx.graphics/src/main/java/com/sun/glass/ui/headless/HeadlessApplication.java line 41: > 39: @Override > 40: protected void _invokeAndWait(Runnable runnable) { > 41: throw new UnsupportedOperationException("Not supported yet."); is there a follow-up bug for this, or the list of things to do? modules/javafx.graphics/src/main/java/com/sun/glass/ui/headless/HeadlessApplication.java line 92: > 90: @Override > 91: protected void staticCursor_setVisible(boolean visible) { > 92: throw new UnsupportedOperationException("Not supported yet."); should it be a no-op instead? will cursor be supported at all? modules/javafx.graphics/src/main/java/com/sun/glass/ui/headless/HeadlessRobot.java line 109: > 107: } > 108: int modifiers = 0; > 109: view.notifyMouse(mouseEvent, buttonEvent, (int)mouseX-wx, > (int)mouseY-wy, (int)mouseX, (int)mouseY, modifiers, false, false); should the coordinates be rounded instead of floor'ed (cast to int)? (also in other places below) modules/javafx.graphics/src/main/java/com/sun/glass/ui/headless/HeadlessRobot.java line 161: > 159: int repeat = Math.abs(wheelAmt); > 160: for (int i = 0; i < repeat; i++) { > 161: view.notifyScroll((int) mouseX, (int) mouseY, wx, wy, 0, > dff, 0, 0, 0, 0, 0, multiplierX, multiplierY); are all parameters correct? `modifiers` and `lines`, specifically? modules/javafx.graphics/src/main/java/com/sun/glass/ui/headless/HeadlessRobot.java line 179: > 177: public void getScreenCapture(int x, int y, int width, int height, > int[] data, boolean scaleToFit) { > 178: checkWindowFocused(); > 179: ((HeadlessWindow)activeWindow).getScreenCapture(x, y, width, > height, data, scaleToFit); maybe the `activeWindow` can be declared as `HeadlessWindow` to avoid casting modules/javafx.graphics/src/main/java/com/sun/glass/ui/headless/HeadlessRobot.java line 231: > 229: if (windows.isEmpty()) return null; > 230: if (windows.size() == 1) return (HeadlessWindow)windows.get(0); > 231: return (HeadlessWindow)windows.get(windows.size() -1); call me old fashioned, but would a straightforward reverse `for` loop be better in this case? no unnecessary memory allocations required. here and in two methods below? modules/javafx.graphics/src/main/java/com/sun/glass/ui/headless/HeadlessWindow.java line 19: > 17: > 18: private static final AtomicInteger ptrCount = new AtomicInteger(0); > 19: private long ptr; since `ptr` is long, should `ptrCount` be `AtomicLong`? modules/javafx.graphics/src/main/java/com/sun/glass/ui/headless/HeadlessWindow.java line 316: > 314: int b = rgba & 0xFF; > 315: > 316: Color color = Color.color( could use `Color::rgb(int,int,int,double)`; modules/javafx.graphics/src/main/java/com/sun/glass/ui/headless/HeadlessWindow.java line 329: > 327: for (int j = 0; j < width; j++) { > 328: int idx = i * width + j; > 329: int fidx = (y + i) * 1000 + x + j; where does 1000 come from? modules/javafx.graphics/src/main/java/com/sun/glass/ui/headless/HeadlessWindow.java line 341: > 339: int offsetX = this.getX(); > 340: int offsetY = this.getY(); > 341: int stride = 1000; same magic 1000 here modules/javafx.graphics/src/main/java/com/sun/glass/ui/headless/HeadlessWindow.java line 356: > 354: > 355: void clearRect(int x0, int w0, int y0, int h0) { > 356: int stride = 1000; and here... modules/javafx.graphics/src/main/java/com/sun/glass/ui/headless/NestedRunnableProcessor.java line 40: > 38: latch.await(); > 39: } catch (InterruptedException e) { > 40: e.printStackTrace(); should we print here or send to `Application.reportException(e)` ? ------------- PR Comment: https://git.openjdk.org/jfx/pull/1836#issuecomment-3014300491 PR Review Comment: https://git.openjdk.org/jfx/pull/1836#discussion_r2172457055 PR Review Comment: https://git.openjdk.org/jfx/pull/1836#discussion_r2172458562 PR Review Comment: https://git.openjdk.org/jfx/pull/1836#discussion_r2172463000 PR Review Comment: https://git.openjdk.org/jfx/pull/1836#discussion_r2172698720 PR Review Comment: https://git.openjdk.org/jfx/pull/1836#discussion_r2172703243 PR Review Comment: https://git.openjdk.org/jfx/pull/1836#discussion_r2172705046 PR Review Comment: https://git.openjdk.org/jfx/pull/1836#discussion_r2172729945 PR Review Comment: https://git.openjdk.org/jfx/pull/1836#discussion_r2172790233 PR Review Comment: https://git.openjdk.org/jfx/pull/1836#discussion_r2172819470 PR Review Comment: https://git.openjdk.org/jfx/pull/1836#discussion_r2172820759 PR Review Comment: https://git.openjdk.org/jfx/pull/1836#discussion_r2172821109 PR Review Comment: https://git.openjdk.org/jfx/pull/1836#discussion_r2172821466 PR Review Comment: https://git.openjdk.org/jfx/pull/1836#discussion_r2172827253
