On Sat, 28 Mar 2026 13:36:32 GMT, Thiago Milczarek Sayao <[email protected]> wrote:
>> This is a continuation to >> [JDK-8236651](https://bugs.openjdk.org/browse/JDK-8236651) and it aims to >> stabilize the linux glass gtk backend. >> >> This is a refactor of the Glass GTK implementation, primarily focused on >> window size, positioning, and state management to resolve numerous issues. >> >> The main change is that GtkWindow (which is a GtkWidget) has been replaced >> with a direct use of GdkWindow for windows. This makes sense because GTK >> includes its own rendering pipeline, whereas JavaFX renders directly to the >> underlying system window (such as the X11 window) via Prism ES2 using GL and >> GLX. Most GTK window-related calls have equivalent GDK calls. Since GDK >> serves as an abstraction over the system window and input events, it aligns >> better with the purposes of Glass. Additionally, using GTK required various >> workarounds to integrate with Glass, which are no longer necessary when >> using GDK directly. >> >> It uses a simple C++ Observable to notify the Java side about changes. This >> approach is more straightforward, as notifications occur at many points and >> the previous implementation was becoming cluttered. >> >> Previously, there were three separate context classes, two of which were >> used for Java Web Start and Applets. These have now been unified, as they >> are no longer required. >> >> Many tests were added to ensure everything is working correctly. I noticed >> that some tests produced different results depending on the StageStyle, so >> they now use @ParameterizedTest to vary the StageStyle. >> >> A manual test is also provided. I did not use MonkeyTester for this, as it >> was quicker to simply run and test manually:`java @build/run.args >> tests/manual/stage/TestStage.java ` >> >> While this is focused on XWayland, everything works on pure Xorg as well. > > Thiago Milczarek Sayao has updated the pull request incrementally with one > additional commit since the last revision: > > Location can be negative ## Pull request overview Refactors the JavaFX Glass GTK (Linux) backend to improve window sizing/positioning/state handling by moving from `GtkWindow`-centric code toward direct `GdkWindow` usage, and adds/updates system & robot tests to validate the corrected behavior across `StageStyle`s. **Changes:** - Replace/reshape native GTK window management (window state, sizing constraints, fullscreen/maximize/minimize behaviors, event processing) around `GdkWindow` and new state/notification plumbing. - Add a portal-backed file/folder chooser implementation with GTK-dialog fallback, plus a system property to disable the portal path. - Add/adjust a broad set of system/robot tests (including parameterized `StageStyle` variants) and a manual stage test app. ### Reviewed changes Copilot reviewed 31 out of 31 changed files in this pull request and generated 6 comments. <details> <summary>Show a summary per file</summary> | File | Description | | ---- | ----------- | | tests/system/src/test/java/test/util/Util.java | Adds helpers for parameterized test display strings, waiting on boolean properties, and screen selection for a `Stage`. | | tests/system/src/test/java/test/robot/javafx/stage/StageOwnershipTest.java | New robot tests for ownership/modality/focus/z-order behaviors across styles. | | tests/system/src/test/java/test/robot/javafx/stage/StageMixedSizeTest.java | Refactors to parameterized tests and simplifies stage-show synchronization. | | tests/system/src/test/java/test/robot/javafx/stage/StageLocationTest.java | New robot tests for stage movement (X/Y) across styles. | | tests/system/src/test/java/test/robot/javafx/stage/StageFocusTest.java | Removes Linux assumptions/guards around focus tests. | | tests/system/src/test/java/test/robot/javafx/stage/StageAttributesTest.java | Parameterizes by style and re-enables previously skipped coverage; adds Wayland-specific guard for undecorated fullscreen. | | tests/system/src/test/java/test/robot/javafx/stage/IconifyTest.java | Re-enables decorated iconify test on Linux. | | tests/system/src/test/java/test/robot/javafx/stage/DualWindowTest.java | Removes Linux instability gating. | | tests/system/src/test/java/test/javafx/stage/StageTestBase.java | New base class for stage geometry/state tests with common setup/teardown. | | tests/system/src/test/java/test/javafx/stage/SizingTest.java | New sizing/min/max constraint tests across `StageStyle`s. | | tests/system/src/test/java/test/javafx/stage/SizeToSceneTest.java | Re-enables SizeToScene tests previously skipped on Linux. | | tests/system/src/test/java/test/javafx/stage/MaximizeUndecorated.java | Removes old maximize test in favor of newer coverage. | | tests/system/src/test/java/test/javafx/stage/MaximizeTest.java | New maximize/restore geometry tests across styles, including pre-show maximize. | | tests/system/src/test/java/test/javafx/stage/FullScreenTest.java | New fullscreen/restore geometry tests across styles, including pre-show fullscreen. | | tests/system/src/test/java/test/javafx/scene/RestoreSceneSizeTest.java | Re-enables Linux execution (removes Linux skip). | | tests/manual/stage/TestStage.java | Adds a comprehensive manual stage test tool for exercising stage operations and properties. | | modules/javafx.graphics/src/main/native-glass/gtk/glass_window_ime.cpp | Updates IME handling method ownership to `WindowContext` and modernizes null usage. | | modules/javafx.graphics/src/main/native-glass/gtk/glass_window.h | Large refactor: new observable/state structs, new `WindowContext`/`WindowContextExtended` design, and cursor helpers. | | modules/javafx.graphics/src/main/native-glass/gtk/glass_key.cpp | Adds X11/GDK X11 includes for key handling integration. | | modules/javafx.graphics/src/main/native-glass/gtk/glass_general.h | Cleans up includes, adds alpha-channel error message constant, and adds a unified `LOG` macro. | | modules/javafx.graphics/src/main/native-glass/gtk/glass_general.cpp | Refactors/cleans legacy GTK2/ifdef paths; adjusts transparency + pixbuf helpers. | | modules/javafx.graphics/src/main/native-glass/gtk/filechooser_portal.h | Introduces portal file chooser abstraction and result/filter types. | | modules/javafx.graphics/src/main/native-glass/gtk/filechooser_portal.cpp | Implements XDG portal file/folder chooser via DBus and returns selected URIs/filters. | | modules/javafx.graphics/src/main/native-glass/gtk/GlassWindow.cpp | Updates window creation to use new context classes and adjusts native handle retrieval and max-size behavior. | | modules/javafx.graphics/src/main/native-glass/gtk/GlassView.cpp | Updates view position queries and adjusts fullscreen enter/exit path. | | modules/javafx.graphics/src/main/native-glass/gtk/GlassRobot.cpp | Switches screen capture to call `gdk_pixbuf_get_from_window` directly. | | modules/javafx.graphics/src/main/native-glass/gtk/GlassCommonDialogs.cpp | Adds portal-backed chooser path with GTK-dialog fallback and transient parenting via `GdkWindow`. | | modules/javafx.graphics/src/main/native-glass/gtk/GlassApplication.cpp | Adjusts ordering of GTK event processing vs Glass window processing for certain event types. | | modules/javafx.graphics/src/main/java/com/sun/glass/ui/gtk/GtkWindow.java | Removes eager state notifications for minimize/maximize and returns actual state. | | modules/javafx.graphics/src/main/java/com/sun/glass/ui/gtk/GtkCommonDialogs.java | Adds system property to disable portal chooser and passes the flag to native calls. | </details> --- š” <a href="/openjdk/jfx/new/master?filename=.github/instructions/*.instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Add Copilot custom instructions</a> for smarter, more guided reviews. <a href="https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot"; class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Learn how to get started</a>. modules/javafx.graphics/src/main/java/com/sun/glass/ui/gtk/GtkCommonDialogs.java line 52: > 50: String folder, > 51: String title, > 52: boolean usePortal); The native method parameter is named `usePortal`, but the value passed is `DISABLE_PORTAL_FILE_CHOOSER` and the C++ side treats the flag as `disablePortal`. Please rename the parameter (and/or constant) so the boolean semantics are consistent (e.g., `disablePortal`) to avoid accidentally inverting the logic in future changes. modules/javafx.graphics/src/main/native-glass/gtk/GlassCommonDialogs.cpp line 247: > 245: LOG0("Portal folder chooser disabled via > glass.gtk.disablePortalFileChooser=true\n") > 246: return PORTAL_FOLDER_UNAVAILABLE; > 247: } Using a non-null invalid `jstring` sentinel value (`(jstring)(intptr_t)-1`) is risky and can easily lead to undefined behavior if it ever escapes or is misused. Prefer returning `nullptr` plus an explicit out parameter / status flag (or wrap the result in a small struct) to represent āportal unavailableā vs āuser cancelledā. modules/javafx.graphics/src/main/native-glass/gtk/glass_general.cpp line 696: > 694: > 695: gdk_pixbuf_get_from_window (window, srcx, srcy, width, height); > 696: return ret; `glass_pixbuf_from_window` declares `ret` but never assigns the result of `gdk_pixbuf_get_from_window`, so it always returns NULL. Even if current call sites were switched to use `gdk_pixbuf_get_from_window` directly, this wrapper should either be fixed to return the pixbuf or removed to avoid future regressions / accidental use. tests/system/src/test/java/test/javafx/stage/StageTestBase.java line 59: > 57: > 58: /** > 59: * Creates a Scene for the test stage acoording to the {@link > StageStyle} Typo in Javadoc: "acoording" should be "according". Suggestion: * Creates a Scene for the test stage according to the {@link StageStyle} tests/system/src/test/java/test/robot/javafx/stage/StageAttributesTest.java line 195: > 193: assertTrue(topStage.isFullScreen()); > 194: > 195: // screen stage should take over the bottom stage Comment typo: "screen stage" looks like it should be "fullscreen stage" (to match the test intent and surrounding assertions). Suggestion: // fullscreen stage should take over the bottom stage tests/system/src/test/java/test/util/Util.java line 361: > 359: } > 360: }); > 361: } `waitForBoolean` adds a listener but never removes it after the expected value is observed. In longer test runs this can accumulate listeners on long-lived properties (e.g., Stage properties). Consider removing the listener once it fires (or using a weak listener) to avoid leaks and unexpected extra notifications. ------------- PR Review: https://git.openjdk.org/jfx/pull/1789#pullrequestreview-4025590758 PR Review Comment: https://git.openjdk.org/jfx/pull/1789#discussion_r3004842007 PR Review Comment: https://git.openjdk.org/jfx/pull/1789#discussion_r3004842012 PR Review Comment: https://git.openjdk.org/jfx/pull/1789#discussion_r3004842001 PR Review Comment: https://git.openjdk.org/jfx/pull/1789#discussion_r3004842023 PR Review Comment: https://git.openjdk.org/jfx/pull/1789#discussion_r3004842026 PR Review Comment: https://git.openjdk.org/jfx/pull/1789#discussion_r3004842017
