Use EWMH for DisplayServerX11::_window_minimize_check() implementation

[PorkrollPosadist]

Proposed solution for issue #77333

After a lot of testing (some of which is included in the issue discussion) I observed multiple failure mechanisms which all ended up leading back to DisplayServerX11::_window_minimize_check() returning an unexpected result. Since EWMH features are used liberally throughout the rest of the DisplayServerX11 implementation, I figured I would use use EWMH to figure out the minimized state instead of ICCCM (the original implementation).

In my limited testing so far, this solves the issue on Gnome/Wayland (Gentoo x86_64). I also tested it on an alternate machine (Fedora 38, Gnome/X11 x86_64). This patch is going to need some more testing to ensure it doesn't introduce any regressions (i.e. fix XWayland at the expense of breaking X11). At least a handful of X11 window managers and Wayland compositors should be tried. Also, it would be wise to test some Godot projects which make advanced use of it's multi-window GUI features to check some corner cases which are less likely to be exposed by point-and-click usage.

• Bugsquad edit, fixes: Pop-up windows instantly minimize on Fedora Linux 38 GNOME (DisplayServer regression) #77333
• Bugsquad edit, alternative to: Fix XWayland popup issues #80032

[PorkrollPosadist]

Seems to be broken on Fedora 38 XFCE (X11), but I don't if/how the behavior is different from the master branch. Whole new can of worms :)

XFCE iconifies multiple windows from the same application into the same menu on the pager. On this PR and master, If you minimize a dialog, switch workspaces, and switch back, the dialog becomes visible again. Some other similar, but strange behavior may or may not be exclusive to the PR.

This might not be a bad thing, since the dialog is modal and blocks interaction with the main editor window, but fiddling with the pager menu also causes the windows to hide and re-appear inexplicably. I don't know what exactly is going on here, but after a little more testing I have seen it in both master and the PR.

Update: It looks like XFCE indicates the minimized state by greying out the window icon in the pager menu. This reflects the state when you minimize a window using either the pager or the window decorations. When you switch workspaces, you can see this go out of sync, the icon is still grey, but the dialog becomes visible again. Based on this visual cue, there is a disagreement between Godot and the window manager about this state (observed in master).

[adamscott]

Thanks for your first PR! I just enabled the CI/CD build, as it is blocked for first-time contributors by default. After the build's done, I will test it locally to see if that fixes the issue. :)

[adamscott]

Congrats! I'm approving your PR, it was pretty stable on my system. Be sure to update what @AThousandShips asked you to edit, though.

[Riteo]

I'm not that versed in X11 stuff at the moment, but the changes sound fine!

I've always only heard about _NET_WM_STATE_HIDDEN so hopefully it's handled better across implementations, if that makes sense. Apologies if I said something very dumb, take that with a huge grain of salt.

Unfortunately I couldn't personally see whether it actually fixes the issue in practice (I don't have a problematic software setup at hand) but it definitely seems to work fine as it did on sway/XWayland so no regression got introduced from my point of view.

As @adamscott said, don't forget to apply @AThousandShips's requested changes!

Windowing is always a mess, so thanks a lot for taking the time to debug this issue!

[PorkrollPosadist]

@AThousandShips @adamscott It is done :)

[PorkrollPosadist]

@Riteo

I'm not that versed in X11 stuff at the moment

That makes two of us :). Up until now, I have only ever dealt with toolkits like GTK+, wxWidgets, Qt, etc.

I've always only heard about _NET_WM_STATE_HIDDEN so hopefully it's handled better across implementations, if that makes sense

So during my spelunking, it seems like ICCCM was the original standard, published in 1994. It proposed a WM_STATE property with 3 possible values - WithdrawnState, NormalState, and IconicState. NormalState roughly translates to "the window is on your screen and you can interact with it," and IconicState roughly translates to "the window has been shrunk to some taskbar or pager or something" AKA "minimized." WithdrawnState was apparently used for recycling windows which were no longer needed or something. I don't know. It sounds atrocious. :) WM_STATE could only be set to one of these three values.

A lot of innovation happened in window managers in the following years, and this framework proved insufficient. Applications and window managers all started doing their own thing. Roughly 10 years later, in 2005 EWHM was standardized. Notably, this unified the extensions which were being employed by Gnome and KDE. It included the _NET_WM_STATE property, which added states for focused, hidden, fullscreen, modal, maximized, sticky, shaded, and more. Unlike WM_STATE, _NET_WM_STATE could have several of these states enabled simultaneously. It is widely supported, but not universally supported. Window managers (including Wayland compositors using Xwayland) should still honor ICCCM properties if EWMH properties aren't present.

You know what? Maybe that's the issue. Godot was using a bunch of EWMH properties and this might be what was causing Xwayland to act strange. There isn't a 1:1 mapping between ICCCM states and EWHM states. It might be as if Godot is trying to speak two different languages to the window manager. I don't know this for sure though. It is just a hunch I developed while typing this. What is Xwayland supposed to do if there are both ICCCM properties and EWMH properties set and they are in conflict? Seems like "undefined behavior" territory.

[Riteo]

Godot was using a bunch of EWMH properties and this might be what was causing Xwayland to act strange. There isn't a 1:1 mapping between ICCCM states and EWHM states. It might be as if Godot is trying to speak two different languages to the window manager. I don't know this for sure though. It is just a hunch I developed while typing this.

Might be, might be not. It's surely compositor dependent as, even with native X11, WMs have to set the right atoms themselves, so there's ought to be inconsistencies.

What is Xwayland supposed to do if there are both ICCCM properties and EWMH properties set and they are in conflict? Seems like "undefined behavior" territory.

Yeah that feels like it. BTW I just found out that wlroots doesn't implement the whole slew of EWMH 1.3 goodies: https://gitlab.freedesktop.org/wlroots/wlroots/-/issues/1496

That might be important to know if we want to use some of them.

[PorkrollPosadist]

Good find. Wlroots is used by a significant number of Wayland compositors.

[adamscott]

@YuriSizov Flagged to merge! :)

[akien-mga]

For the record, your commit seems not to be linked to your GitHub account. See: Why are my commits linked to the wrong user? for more info.

[dannroda]

Hi, I'm using ArchLinux with Gnome 44 under wayland and I don't really know if this is related to my issue BUT this PR makes Godot 4.1.1-stable usable without launching it with --single-window.
Without this PR it just won't stop shaking, now at least stops when the window is dragged.
I can't really tell if is another unrelated Xwayland issue.

Grabacion.de.pantalla.desde.2023-08-04.22-03-33.webm

[PorkrollPosadist]

@akien-mga Should be fixed. I added the email address to the account and verified it. Also, I had no idea you could use markdown in titles lmao.

@dannroda does it, like, stop shaking for good after dragging the window? Or only while dragging the window?

When the window is being dragged, Godot receives a continuous stream of ConfigureNotify events. The method I modified in this PR apparently gets called every time one of these is processed. I have not observed this shaking behavior at all personally though.

[dannroda]

Yes, if dragged or maximized it just stop shaking until a new window is created/called

[dannroda]

Hi!
Just wanted to confirm that my issue is Gnome-Related Xwayland issue, and the Gnome Team fixed it in the gnome-alpha 45.
Had to migrate to Plasma Desktop and it works perfectly!

[PorkrollPosadist]

Update on the testing front. I haven't really systematized a great way to check for regressions, but I have tried a few additional desktop environments / window managers.

• Herbstluftwm 0.9.5 (an X11 tiling window manager):
  • Not exactly sure what I was trying to accomplish here, but it was already on my system. Nothing seems broken.
• Budgie 10.7.2 (X11):
  • Nothing seems broken.
• MATE 1.26.1 (X11)
  • Nothing seems broken.
• Plasma 5.27.6 (X11):
  • When you have a sub-window of a sub-window open (i.e. open project settings, go to Localization, hit Add, and a file browser opens) and minimize the middle window (project settings), it does not appear again until you close the top level window (open files). A little odd, but hardly a show-stopper.
• Plasma 5.27.6 (Wayland):
  • The missing window as described under Plasma (X11) appears as expected.

XFCE remains the only DE I have observed windows un-minimize themselves when switching virtual desktops.

[akien-mga]

Thanks! And congrats for your first merged Godot contribution 🎉

[adamscott]

@PorkrollPosadist Thanks a ton! This issue was making me mad.

[uint]

Thanks @PorkrollPosadist!

[hunterloftis]

@PorkrollPosadist 🙇‍♂️ thank you for figuring this out, testing it, and getting it across the finish line. This issue is driving me bonkers in 4.2.dev3 and I'm very much looking forward to the release that includes your fix.

[YuriSizov]

Cherry-picked for 4.1.3.