On Tue, 22 Jul 2025 07:09:58 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:
> 
>   Add copyright sections

Looks good. I left a few minor comments / questions as well as noting a couple 
follow-up items inline.

There are several follow-on issues to be filed. You might link them to 
[JDK-8324941](https://bugs.openjdk.org/browse/JDK-8324941) when you file them.

One more follow-up: It might be useful to add two manual headless tests (or one 
test with two modes) -- one using `Scene::snapshot` and the other using 
`Robot::getScreenCapture` -- the tests would contain a scene and then save the 
resulting snapshot / screen capture to a PNG file, which could then be opened. 
Alternatively, the test(s) could be a headful Swing app that draws the captured 
image into a Swing JPanel for display. Since the automated system tests already 
exercise some of this, I think this manual test could be done separately.

modules/javafx.graphics/src/main/java/com/sun/glass/ui/headless/HeadlessPlatformFactory.java
 line 50:

> 48:     public MenuBarDelegate createMenuBarDelegate(MenuBar menubar) {
> 49:         throw new UnsupportedOperationException("Not supported yet.");
> 50:     }

Suggestion: file one or more follow-on enhancements to add missing features 
(here and elsewhere).

modules/javafx.graphics/src/main/java/com/sun/glass/ui/headless/HeadlessRobot.java
 line 58:

> 56: 
> 57:     void windowAdded(HeadlessWindow window) {
> 58:         if (this.activeWindow == null) activeWindow = window;

I think this means that if an app creates two windows, then closes the second, 
robot won't be able to capture anything from the first. Or will this method be 
called again on the first window when the second closes?

modules/javafx.graphics/src/main/java/com/sun/glass/ui/headless/HeadlessWindowManager.java
 line 38:

> 36:                 .filter(win -> win.isVisible()).toList();
> 37:         for (Window win : windows) {
> 38:             if (win.isVisible() && (!win.isMinimized())) {

Minor: since you are already using streams, you could use a `.forEach` as the 
terminal operator rather than collecting it into a list to iterate over again. 
Separately, you could also convert the `!win.isMinimized()` test into a filter 
operation (note that the test for `win.isVisible()` is redundant since you 
already filter out non-visible windows).

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

Marked as reviewed by kcr (Lead).

PR Review: https://git.openjdk.org/jfx/pull/1836#pullrequestreview-3055871049
PR Review Comment: https://git.openjdk.org/jfx/pull/1836#discussion_r2231391957
PR Review Comment: https://git.openjdk.org/jfx/pull/1836#discussion_r2231707118
PR Review Comment: https://git.openjdk.org/jfx/pull/1836#discussion_r2231500757

Reply via email to