8354943: [Linux] Simplify and update glass gtk backend: window sizing, positioning, and state management issues#2139
8354943: [Linux] Simplify and update glass gtk backend: window sizing, positioning, and state management issues#2139tsayao wants to merge 24 commits intoopenjdk:masterfrom
Conversation
…, positioning, and state management issues
|
👋 Welcome back tsayao! A progress list of the required criteria for merging this PR into |
|
❗ This change is not yet ready to be integrated. |
Webrevs
|
|
We'll need to evaluate this approach vs that of #1789 and choose one or the other. It is accurate to say that the main difference between the two approaches is that PR #1789 replaces GtkWindow with GdkWindow whereas this PR sticks with GtkWindow? What other differences will help guide the reviewers? Does this PR address the same set of issues that PR #1789 does? That one lists 10 additional bugs as also being resolved by that PR. /reviewers 2 reviewers |
|
@kevinrushforth |
With the introduction of #2025, a By reverting to Experience shows that such changes can introduce regressions. Since this PR preserves the use of
At this time, the answer is likely yes, as all tests are passing on Ubuntu 24.04 with XWayland. I will follow up with additional validation and explicitly list the confirmed fixes. The decision between #1789 and this PR will ultimately depend on the outcome of these tests.
|
|
/issue add 8354943,8316425,8316891,8337400,8353612,8355073,8353556,8321624,8367893,8366009 |
|
@tsayao This issue is referenced in the PR title - it will now be updated. Adding additional issue to issue list: Adding additional issue to issue list: Adding additional issue to issue list: Adding additional issue to issue list: Adding additional issue to issue list: Adding additional issue to issue list: Adding additional issue to issue list: Adding additional issue to issue list: Adding additional issue to issue list: |
- Improve tests to be more consistent - GdkWindow -> GtkWindow related fixes
- Prevent repeated 'a' being typed - Small fixes
# Conflicts: # modules/javafx.graphics/src/main/native-glass/gtk/glass_window.cpp
|
|
|
@tsayao should you add JDK-8365201 to the list too? Edit: oh sorry, I just read that this is an alternative to the other proposal. So the question is, does this fixes the above-mentioned issue too? |
I still need to set up a multi-monitor environment to properly test it, but it will probably fix this issue as well. |
|
/template append |
|
@tsayao The pull request template has been appended to the pull request body |
- Details on going back to GtkWindow - unref cursor
|
Reviewers: @lukostyra and some combination of: @mstr2 @beldenfox @kevinrushforth |
lukostyra
left a comment
lukostyra
left a comment
There was a problem hiding this comment.
I have reviewed a part of this change but I noticed there's still some commits being added.
Let us know when the change is ready to review. In the meantime, I'm leaving off the comment I found.
|
|
||
| #else | ||
|
|
||
| #define LOG(...) ((void)0) |
There was a problem hiding this comment.
Minor: while it probably won't matter much right now, a preferred syntax here is:
#define LOG(...) do {} while(0)
It compiles to the same thing (a noop) but it is the most language-conforming statement that can be used basically anywhere without triggering a sudden compiler error. Here there could be a problem when for example we would try to do:
if (cond)
LOG("success");
else
LOG("error");
As a side-note, this also should apply to other LOG macros in this file, but we already have JDK-8356327 for this, so you can leave them and it will get adjusted there.
Since it started using GtkWindow again, the behavior is slightly different in some situations, so I'm rechecking the possible scenarios—that’s why these commits. I also added comments in some places because I no longer remembered why they were there (they're usually edge cases). |
4 participants
This is a continuation to 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
@ParameterizedTestto 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
GtkWindowto be used when setting the parent of the file chooser dialog.To show debug messages, build with
-PCONF=DebugNativeand run with-Dglass.gtk.verbose=CATEGORYCATEGORYcan be one or more of: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.javaI have added constants for delay times in systemTests (
Util.java) for geometry, state, and focus. These can be configured at runtime, for example:-Dtest.geometry.delay=DELAY. Please let me know if you would prefer these to be removed.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.
Additional testing on other OS versions and manual validation is in progress.
Progress
Issues
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jfx.git pull/2139/head:pull/2139$ git checkout pull/2139Update a local copy of the PR:
$ git checkout pull/2139$ git pull https://git.openjdk.org/jfx.git pull/2139/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 2139View PR using the GUI difftool:
$ git pr show -t 2139Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/2139.diff
Using Webrev
Link to Webrev Comment