On Sun, 6 Sep 2026 17:01:49 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 incrementally with one > additional commit since the last revision: > > Address review feedback: > - Fix programmatically resizing unresizable windows > - Fix removing minimize on disabling the window (only works on older window > managers) some comments, will do some more testing in the coming days. But as I already commented, it fixes some problems where a dialog is not correctly sized (and not just a bit, completely unusable initially), so definitely a big improvement. modules/javafx.graphics/src/main/java/com/sun/glass/ui/gtk/GtkWindow.java line 133: > 131: protected boolean _minimize(long ptr, boolean minimize) { > 132: minimizeImpl(ptr, minimize); > 133: notifyStateChanged(WindowEvent.MINIMIZE); `notifyStateChanged` is not called anymore, is it? Also saved as `jGtkWindowNotifyStateChanged` but not called anymore as far as I can see. modules/javafx.graphics/src/main/native-glass/gtk/GlassApplication.cpp line 138: > 136: const char* categories = env->GetStringUTFChars(logCategories, > nullptr); > 137: glass_gtk_log_init(categories); > 138: env->ReleaseStringUTFChars(logCategories, categories); as `categories` could be null if I understand correctly, should the `ReleaseStringUTFChars` be called with `null`? modules/javafx.graphics/src/main/native-glass/gtk/GlassView.cpp line 272: > 270: } > 271: > 272: empty newline modules/javafx.graphics/src/main/native-glass/gtk/glass_window.cpp line 132: > 130: > 131: if (frame_type != TITLED) { > 132: initial_wmf = GDK_FUNC_ALL; Should `current_wmf` be initialized here as well? modules/javafx.graphics/src/main/native-glass/gtk/glass_window.cpp line 1601: > 1599: newY = ySet ? y : loc.y.value(); > 1600: loc_set = true; > 1601: } What about the case when only `x` or only `y` changed? tests/manual/stage/TestStage.java line 2: > 1: /* > 2: * Copyright (c) 2025 Oracle and/or its affiliates. All rights reserved. Here and the other new files: Copyright should be 2026. and isn't the line: `Copyright (c) 2026, Oracle ...`, so with a comma? tests/system/src/test/java/test/robot/javafx/stage/StageLocationTest.java line 53: > 51: import static test.util.Util.TIMEOUT; > 52: > 53: @Timeout(value = TIMEOUT, unit= TimeUnit.MILLISECONDS) minor: missing space right after unit Suggestion: @Timeout(value = TIMEOUT, unit = TimeUnit.MILLISECONDS) tests/system/src/test/java/test/util/Util.java line 61: > 59: */ > 60: public class Util { > 61: public static final String PARAMETERIZED_TEST_DISPLAY = > "{displayName} [{index}] {arguments}"; Is this needed? tests/system/src/test/java/test/util/Util.java line 72: > 70: * to be processed. On Linux, these operations are asynchronous. > 71: * The native side may adjust values after Glass has applied them on > the Java side. > 72: * <p> Here and below, the empty `<p>` tag is not needed. ------------- PR Review: https://git.openjdk.org/jfx/pull/2139#pullrequestreview-5132853868 PR Review Comment: https://git.openjdk.org/jfx/pull/2139#discussion_r3950725640 PR Review Comment: https://git.openjdk.org/jfx/pull/2139#discussion_r3950703733 PR Review Comment: https://git.openjdk.org/jfx/pull/2139#discussion_r3950607146 PR Review Comment: https://git.openjdk.org/jfx/pull/2139#discussion_r3950735406 PR Review Comment: https://git.openjdk.org/jfx/pull/2139#discussion_r3950744079 PR Review Comment: https://git.openjdk.org/jfx/pull/2139#discussion_r3950457110 PR Review Comment: https://git.openjdk.org/jfx/pull/2139#discussion_r3950508566 PR Review Comment: https://git.openjdk.org/jfx/pull/2139#discussion_r3950502219 PR Review Comment: https://git.openjdk.org/jfx/pull/2139#discussion_r3950482697
