On Sun, 31 May 2026 13:47:54 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. >> >> It refactors the Glass GTK implementation with a primary focus on window >> sizing, positioning, and state management, addressing a number of >> long-standing issues. >> >> Previously, three separate context classes existed, two of which were used >> for Java Web Start and Applets. These have been unified, as they are no >> longer required. >> >> Additional tests have been introduced to improve coverage. Some tests >> produced different results depending on the StageStyle, so they have been >> converted to use `@ParameterizedTest` to exercise multiple styles. >> >> Although the primary focus is XWayland, the changes have also been verified >> to work correctly on Xorg. >> >> This replaces #1789. It removes the use of GdkWindow in favor of GtkWindow, >> reducing risk and simplifying the review process while preserving the same >> set of bug fixes. Additionally, #2025 requires a `GtkWindow` to be used when >> setting the parent of the file chooser dialog. >> >> To show debug messages, build with `-PCONF=DebugNative` and run with >> `-Djdk.gtk.verbose=true`. Log categories can be passed with >> `-Dglass.gtk.logCategories=CATEGORY` >> >> `CATEGORY` can be one or more of: >> - all >> - size >> - position >> - focus >> - state >> - lifecycle >> - input >> - dialog >> >> Multiple categories can be specified by separating them with commas (e.g. >> size,focus,input). >> >> A manual test is provided: >> `java @build/run.args tests/manual/stage/TestStage.java` >> >> When a window property is set, it is reported immediately. However, once it >> reaches the native Glass layer, it may be adjusted or rejected, causing the >> property to be updated again. Introducing a delay helps ensure the final >> state has been applied before it is verified. >> >> --------- >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > Thiago Milczarek Sayao has updated the pull request with a new target base > due to a merge or a rebase. The pull request now contains 39 commits: > > - Remove configurable delays > - Fix test > - Xorg fixes > - Revert "Rewrite WrongStageFocusWithApplicationModalityTest because it > fails intermittently" > > This reverts commit c6b9dd745e5d762adb89a3b596e53b0d8a790d7f. > - Rewrite WrongStageFocusWithApplicationModalityTest because it fails > intermittently > - Use existing verbose flag for GTK > - Fix window size update > - StageOwnershipTest: Update Y_DELTA > - Copyright year > - Merge branch 'master' into 8354943_v2 > - ... and 29 more: https://git.openjdk.org/jfx/compare/b690b3c4...68d684f9 modules/javafx.graphics/src/main/native-glass/gtk/glass_general.h line 49: > 47: #define ALPHA_CHANNEL_ERROR_MSG \ > 48: "Can't create transparent stage, because your screen doesn't support > alpha channel. " \ > 49: "You need to enable XComposite extension.\n" If you are touching this code, maybe you should consider improving the language. In particular, readers shouldn't be colloquially addressed with "you". Maybe something like: "Cannot create a transparent stage because the screen does not support an alpha channel without enabling the XComposite extension." modules/javafx.graphics/src/main/native-glass/gtk/glass_window.cpp line 803: > 801: > 802: if (gdk_cursor) { > 803: g_object_unref(gdk_cursor); The `gdk_cursor` field is unref'd here and in the destructor of `WindowContext`, but `_setCustomCursor()` passes in a cursor owned by `com.sun.glass.ui.Cursor` on the Java side. This may potentially destroy a cursor that is still used on the Java side. If `WindowContext` needs to participate in refcounting, it should probably take its own `g_object_ref()` for incoming cursors. modules/javafx.graphics/src/main/native-glass/gtk/glass_window.cpp line 1193: > 1191: } > 1192: > 1193: glong to_screen = getScreenPtrForLocation(event->x, event->y); `getScreenPtrForLocation()`, which calls `gdk_screen_get_monitor_at_point()`, expects coordinates "in the virtual screen". You pass in coordinates specified "relative to its parent", can you verify that this is intended? modules/javafx.graphics/src/main/native-glass/gtk/glass_window.cpp line 1680: > 1678: } > 1679: > 1680: if (mapped) { This looks a bit suspicious: `move_resize()` uses `mapped` to decide whether to call `gtk_window_resize()` or `gtk_window_set_default_size()`. The old code used `gtk_widget_get_realized()` and resized any realized window. Since the `mapped` field is always false for a `POPUP` window, the new code might not resize an already-realized popup. modules/javafx.graphics/src/main/native-glass/gtk/glass_window.h line 73: > 71: if (!onChange) return; > 72: if (notifying) { > 73: fprintf(stderr,"WARNING: Re-entrant call to > Observable::invalidate() (possible cyclic dependency)\n"); Should this use the `LOG` macro, or alternatively be hidden behind the `VERBOSE` flag? ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/2139#discussion_r3409732103 PR Review Comment: https://git.openjdk.org/jfx/pull/2139#discussion_r3409713840 PR Review Comment: https://git.openjdk.org/jfx/pull/2139#discussion_r3409720723 PR Review Comment: https://git.openjdk.org/jfx/pull/2139#discussion_r3409761222 PR Review Comment: https://git.openjdk.org/jfx/pull/2139#discussion_r3409735256
