8354943: [Linux] Simplify and update glass gtk backend: window sizing, positioning, and state management issues #1789

tsayao · 2025-04-22T12:01:56Z

This is a continuation to JDK-8236651 and it aims to stabilize the linux glass gtk backend.

Overall, it has been made more robust within its scope, particularly in terms of sizing, positioning, and state management.

List of changes:

It embraces the asynchronous nature of X11 by reporting geometry changes only upon receiving a configure event, rather than immediately as before. This is because it merely requests changes from the window manager, which may or may not honor them. However, it still reports changes immediately in certain special cases, such as when the window has not yet been realized (i.e., when the window has not actually been created yet). One scenario where this behavior is evident is when we request the window to move to position (0, 0), but the window manager instead places it in the top-right corner where panels converge.
FullScreen now keeps track of geometry changes and apply them on restore as documented on Stage.java. No geometry changes affects the FullScreen state;
States (fullscreen, maximized and iconify) are now reported back to Java when it actually happens rather than immediately (except when not realized);
When a window is maximized, it will ignore geometry changes and restore to the geometry it had prior to being maximized. After some testing, I believe this is the best behavior for platform compatibility;
Unifies the WindowContext class: previously, there were three separate classes—two of which (for applets and Java Web Start) were removed, leaving only one. However, the supporting infrastructure was still there partially. Unify WindowContext in glass-gtk
Tests were added and re-enabled to ensure everything works correctly. The stage tests now cover various StageStyle configurations, as I found that DECORATED stages often behave differently from UNDECORATED or UTILITY stages;
Added Logs for debugging. Enable it with -PCONF=DebugNative;
Old work-arounds dating back to Ubuntu 16.04 with Compiz were removed.

A simple manual test is also provided but I would prefer to move it's functionality to monkey tester:
java @build/run.args tests/manual/stage/TestStage.java

List of fixed issues:

The Xorg-related bugs were not issues within JavaFX itself, so they were addressed through workarounds - such as delaying the initial state and re-checking the geometry when the window is first mapped.

IMPORTANT: While this is open for review, please allow me some time to further polish and test it before final approval. Feedback is welcome—especially regarding any need to disable the new tests on specific platforms, as some of them might fail.

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed (2 reviews required, with at least 2 Reviewers)

Issue

JDK-8354943: [Linux] Simplify and update glass gtk backend: window sizing, positioning, and state management issues (Task - P4)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jfx.git pull/1789/head:pull/1789
$ git checkout pull/1789

Update a local copy of the PR:
$ git checkout pull/1789
$ git pull https://git.openjdk.org/jfx.git pull/1789/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 1789

View PR using the GUI difftool:
$ git pr show -t 1789

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/1789.diff

Using Webrev

Link to Webrev Comment

…ning, and state management issues

bridgekeeper · 2025-04-22T12:02:18Z

👋 Welcome back tsayao! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2025-04-22T12:02:23Z

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

mlbridge · 2025-04-22T12:06:03Z

Webrevs

mlbridge · 2025-04-22T12:06:04Z

Webrevs

00: Full (0502b66a)

kevinrushforth · 2025-04-22T17:21:29Z

/reviewers 2 reviewers

Reviewers: @lukostyra @kevinrushforth

@beldenfox If you have time, your review would be appreciated as well.

openjdk · 2025-04-22T17:21:43Z

@kevinrushforth
The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 2 (with at least 2 Reviewers).

- Fix fullscreen unresizable

beldenfox · 2025-04-23T19:25:42Z

Thanks for adding these tests. I'm currently tracking down two bugs on macOS that should have been caught a long time ago. Looks like these tests cover those cases (finally).

I will at least review and run the tests on macOS and report back. I might have time to run the tests on Windows 11. I don't have a Windows 10 box to test one.

lukostyra · 2025-04-24T07:50:12Z

I'll give this a thorough read and report back.

In the meantime, I recently was tackling JDK-8321624 and i have to wonder if it got fixed in the process of your changes? It is intermittent, I managed to get it to fail locally on my VM and it seemed like a race between window manager showing the window and us wanting to move it. Judging by the list of bugs you fixed this one could maybe also make he list.

beldenfox

I did a rough pass on macOS and Windows. There's some failures in the StageLocation and StageAttributes tests that I haven't had time to look into.

You might want to consider adding a few delay constants and using them instead of the 500's you've scattered through these tests. There's the delay needed for big state changes (like entering and exiting fullscreen) and that needs to be 500 or more. Then there's the delay for waiting for a layout pulse and I'm assuming that could be significantly shorter.

There seem to be places where you compare an attribute like Stage.getWidth() against a constant using strict equality and other places where you provide a tolerance delta. I suspect you want a delta in more locations but someone else will have to chime in on that. I think this mostly affects Windows machines using fractional scaling.

There was a discussion a while back about naming conventions and I think the consensus was that new tests should not have "test" in the name (so testMinSize should be minSize or minStageSize). But I might be wrong on that and in the end it's a matter of style.

beldenfox · 2025-04-23T19:48:52Z

tests/system/src/test/java/test/javafx/stage/FullScreenTest.java

+    @EnumSource(value = StageStyle.class,
+            mode = EnumSource.Mode.INCLUDE,
+            names = {"DECORATED", "UNDECORATED", "TRANSPARENT"})
+    void testUnFullScreenChangedSize(StageStyle stageStyle) {


According to the spec changes to the size or position of a window while it's in fullscreen mode will be ignored and applied after the window leaves fullscreen mode. That's not how it currently works on macOS or Windows 11. Actually implementing that part of the spec would be complicated and probably not worth the development cycles. I would rather remove that wording and drop the testUnFullScreenChangeSize and Position tests.

I imagine this was easy to implement back when fullscreen was implemented as a separate window. It's not clear this is useful behavior, the spec might just have captured an implementation detail and elevated it to a feature.

I was kind of on the fence with this one. Maybe it’s a case where the docs in Stage.java need fixing — they do also say that width and height should reflect the unfullscreened size. But there are several tests that check the fullscreen size against the screen size, so I opted for notifying the fullscreen sizes.

beldenfox · 2025-04-24T13:58:05Z

tests/system/src/test/java/test/javafx/stage/SizingTest.java

+    @EnumSource(value = StageStyle.class,
+            mode = EnumSource.Mode.INCLUDE,
+            names = {"DECORATED", "UNDECORATED", "TRANSPARENT"})
+    void testFullScreenMaxSize(StageStyle stageStyle) {


You're assuming that fullscreen mode should ignore the max size properties. That's not how Windows 11 and macOS work. On macOS the window expands to the max size and is then centered on a black screen. On Windows 11 it ends up in the upper left corner with the desktop showing beneath it. It looks weird but I don't think there's much point to changing the implementation on these two platforms.

I’m going to remove this test and undo the workaround I added to support it. I initially thought this behavior was consistent with how it works on Windows, but I think I got confused — Windows does allow a fullscreen, unresizable Stage.

Fixed it. But it does bug a little different than windows, it acts like it's fullscreen, but its not.

beldenfox · 2025-04-24T16:05:11Z

tests/system/src/test/java/test/javafx/stage/SizingTest.java

+    @EnumSource(value = StageStyle.class,
+            mode = EnumSource.Mode.INCLUDE,
+            names = {"DECORATED", "UNDECORATED", "TRANSPARENT"})
+    void testMaximizeMaxSize(StageStyle stageStyle) {


This test assumes that maximizing a window will ignore the max size settings. That's not how it works on macOS and it would be a bear to try to implement this. I think we should remove this test.

I actually worked around by removing the constraints, maximize and restore back the constraints when unmaximized. Window does allow this. I would prefer to not allow as well and invert the test (or remove it).

beldenfox · 2025-04-24T16:20:46Z

tests/system/src/test/java/test/javafx/stage/SizingTest.java

+    @EnumSource(value = StageStyle.class,
+            mode = EnumSource.Mode.INCLUDE,
+            names = {"DECORATED", "UNDECORATED", "TRANSPARENT", "UTILITY"})
+    void testMinSize(StageStyle stageStyle) {


Yikes! This test uncovers a long-standing problem and is failing on macOS and Windows. In general glass will only send a resize notification if the window's size actually changes. When you set the width and height here JavaFX records the new values, sends them on to glass, and glass doesn't respond because the window size doesn't change. The property values do not get corrected to reflect the actual window size and the test fails.

(I am not a fan of the JavaFX property-based API for setting window attributes. Basically JavaFX records an attribute change as if it has already happened and THEN sends it on to glass to be acted on. This creates endless complications like this.)

I had to work around this. It's a little worse, because if the Stage is fullscreen and we call setWitdth, for example, JavaFX will imediatelly set the width (but the Stage is fullscreen!).

beldenfox · 2025-04-24T16:28:49Z

tests/system/src/test/java/test/robot/javafx/stage/StageLocationTest.java

+    private static final int TO_X = 500;
+    private static final int TO_Y = 500;
+    private static final Color COLOR = Color.RED;
+    private static final double TOLERANCE = 0.00;


You probably want a non-zero tolerance for color matching.

…ng when unresizable) - Improve code on gtk_window.cpp - Add delta for sizing (as Martin suggested) - Add constants for wait times (as Martin suggested)

tsayao · 2025-04-26T14:54:25Z

I'll give this a thorough read and report back.

In the meantime, I recently was tackling JDK-8321624 and i have to wonder if it got fixed in the process of your changes? It is intermittent, I managed to get it to fail locally on my VM and it seemed like a race between window manager showing the window and us wanting to move it. Judging by the list of bugs you fixed this one could maybe also make he list.

The DualWindow test consistently fails without these changes on my Ubuntu 24.04 test VM, and consistently passes with the changes applied. However, on a physical machine running Ubuntu 24.04, the test passes even without the changes.

Since I changed the positioning code and it does passes the tests, it might be fixed.

tsayao · 2025-04-27T10:38:01Z

Please wait a little before reviewing the glass GTK part, as I may have found a way to improve it further

lukostyra · 2025-04-28T17:03:12Z

The DualWindow test consistently fails without these changes on my Ubuntu 24.04 test VM, and consistently passes with the changes applied. However, on a physical machine running Ubuntu 24.04, the test passes even without the changes.

Since I changed the positioning code and it does passes the tests, it might be fixed.

Good news! Problems were usually on slower environments ex. VMs. I would add this test to the list as well, and I'll verify for its stability during my review.

[Linux] Simplify and update glass gtk backend: window sizing, positio…

0502b66

…ning, and state management issues

openjdk bot added the rfr Ready for review label Apr 22, 2025

tsayao added 4 commits April 22, 2025 21:39

Increase sleep on SizingTest.testFullscreenUnresizable

14c89a2

- Adjust some comments

4593dd6

- Fix fullscreen unresizable

- Add check if the window is still alive

14cb832

Reenable RestoreSceneSizeTest (JDK-8353556)

02b6ab3

beldenfox reviewed Apr 24, 2025

View reviewed changes

tsayao added 2 commits April 26, 2025 09:57

- Do not allow maximize, fullscreen past max constraints (keep allowi…

036be1f

…ng when unresizable) - Improve code on gtk_window.cpp - Add delta for sizing (as Martin suggested) - Add constants for wait times (as Martin suggested)

- Re-enable DualWindowTest

8ac7978

tsayao added 2 commits April 26, 2025 15:20

- Remove unused var

3bfdb75

Remove old comment

61230d5

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8354943: [Linux] Simplify and update glass gtk backend: window sizing, positioning, and state management issues #1789

8354943: [Linux] Simplify and update glass gtk backend: window sizing, positioning, and state management issues #1789

tsayao commented Apr 22, 2025 •

edited by openjdk bot

Loading

bridgekeeper bot commented Apr 22, 2025

openjdk bot commented Apr 22, 2025

mlbridge bot commented Apr 22, 2025 •

edited

Loading

mlbridge bot commented Apr 22, 2025

kevinrushforth commented Apr 22, 2025

openjdk bot commented Apr 22, 2025

beldenfox commented Apr 23, 2025

lukostyra commented Apr 24, 2025

beldenfox left a comment

beldenfox Apr 23, 2025

tsayao Apr 24, 2025

beldenfox Apr 24, 2025

tsayao Apr 24, 2025

tsayao Apr 26, 2025

beldenfox Apr 24, 2025

tsayao Apr 24, 2025

tsayao Apr 26, 2025

beldenfox Apr 24, 2025

tsayao Apr 24, 2025

beldenfox Apr 24, 2025

tsayao commented Apr 26, 2025

tsayao commented Apr 27, 2025

lukostyra commented Apr 28, 2025

8354943: [Linux] Simplify and update glass gtk backend: window sizing, positioning, and state management issues #1789

Are you sure you want to change the base?

8354943: [Linux] Simplify and update glass gtk backend: window sizing, positioning, and state management issues #1789

Conversation

tsayao commented Apr 22, 2025 • edited by openjdk bot Loading

Progress

Issue

Reviewing

bridgekeeper bot commented Apr 22, 2025

openjdk bot commented Apr 22, 2025

mlbridge bot commented Apr 22, 2025 • edited Loading

Webrevs

mlbridge bot commented Apr 22, 2025

Webrevs

kevinrushforth commented Apr 22, 2025

openjdk bot commented Apr 22, 2025

beldenfox commented Apr 23, 2025

lukostyra commented Apr 24, 2025

beldenfox left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tsayao commented Apr 26, 2025

tsayao commented Apr 27, 2025

lukostyra commented Apr 28, 2025

tsayao commented Apr 22, 2025 •

edited by openjdk bot

Loading

mlbridge bot commented Apr 22, 2025 •

edited

Loading