Wayland: Implement native sub-windows

[deralmas]

psss... the text of this PR got completely rewritten now that it's ready. If you want to see the original version, use GH's history feature

Everything below will be extremely boring. So, here's a pretty picture of the editor running on sway:

I mean, it's still probably somewhat boring but that's the nature of what I do :P

---

The backend is now mature enough to not explode with multiple windows but the DisplayServer API still cannot meet some guarantees required by the various Wayland protocols we use. To meet those guarantees this patch adds three new elements to the DisplayServer API, with relative handling logic for Window and Popup nodes:

• WINDOW_EVENT_FORCE_CLOSE, which tells a window to forcefully close itself and ensure a proper cleanup of its references, as Wayland enforces this behavior;
• WINDOW_FLAG_POPUP_WM_HINT, which explicitly declares a window as a "popup", as Wayland enforces this distinction and heuristics are not reliable enough;
• FEATURE_SELF_FITTING_WINDOWS, which signals that the compositor can fit windows to the screen automatically and that nodes should not do that themselves.

Given the size of this feature, this patch also includes various WaylandThread reworks and fixes including:

• Improvements to frame wait logic, with fixes to various stalls and a configurable (through a #define) timeout amount; Edit: stalls fixed in Wayland: Fix engine stalls while waiting frames #102674.
• A proper implementation of window_can_draw;
• Complete overhaul of pointer and tablet handling. Now everything is always accumulated and handled only on each respective frame event. This makes their logic simpler and more robust.
• Better handling of pointer leaving and pointer enter/exit event sending;
• Keyboard focus tracking;
• More solid window references using IDs instead of raw pointers as windows can be deleted at any time;
• More aggressive messaging to window nodes to enforce rects imposed by the compositor.

---

The new APIs that I added will be useful for other DisplayServer implementations too, perhaps with the exception of FEATURE_SELF_FITTING_WINDOWS.

As you can see, this PR includes also a lot of core improvements and fixes to stuff that became more evident while testing multiple windows. I'm definitely going to backport/separate some of those.

It does not even include the FIFO additions (#101454) or fixes for some leftover issues that @arlo-phoenix found in the original PR (mainly I think only missing keyboard composition) so once everything is done the Wayland experience should become even more solid.

Known issues

• There's some error spam while starting the editor. It's an engine bug triggered by the fact that now we have a proper implementation of window_can_draw: At startup ERROR: editor/progress_dialog.cpp:169 - Do not use progress dialog (task) while flushing the message queue or using call_deferred()! #100900

• I forgot to account for wl_pointer::frame missing on very old compositors.

[scgm0]

When a subwindow appears, it will cause the main window to exit maximized mode.

2025-01-19.23-10-21.mp4

[deralmas]

Hi @scgm0, thank you for testing! I think I know why. I'll push a fix soon.

[bruvzg]

WINDOW_EVENT_FORCE_CLOSE, which is a surefire way of cleaning up a window. It basically makes the window call hide on itself.

This will be useful for Windows/macOS/X11 display server popup auto close logic, currently these are using WINDOW_EVENT_CLOSE_REQUEST, but expect window to be closed unconditionally, if it's not, it can stay opened in a partially broken state.

[deralmas]

This will be useful for Windows/macOS/X11 display server popup auto close logic, currently these are using WINDOW_EVENT_CLOSE_REQUEST, but expect window to be closed unconditionally, if it's not, it can stay opened in a partially broken state.

Hey that's exactly what I was trying to avoid! I'm really glad this will be useful for other backends too! :D

[deralmas]

@scgm0 I pushed quite a few changes, including a fix for the issue you mentioned.

[scgm0]

@scgm0 I pushed quite a few changes, including a fix for the issue you mentioned.

New issue, the borderless setting of the main window is not working

[deralmas]

New issue, the borderless setting of the main window is not working

Does this happen only on the main window? Because unfortunately that's a known issue with the decoration library we're using, see #93472. I had it disabled until this last push to make debugging simpler.

I think I might need to find a workaround since the upstream bug report went nowhere unfortunately.

[scgm0]

New issue, the borderless setting of the main window is not working

Does this happen only on the main window? Because unfortunately that's a known issue with the decoration library we're using, see #93472. I had it disabled until this last push to make debugging simpler.

I think I might need to find a workaround since the upstream bug report went nowhere unfortunately.

But before you fix the maximized window, the main window can be borderless, which is a bit strange.

[deralmas]

But before you fix the maximized window, the main window can be borderless, which is a bit strange.

It stopped working because I also pushed other things along the fix, including "Re-add libdecor support". I usually work in bursts and push everything at once.

[scgm0]

But before you fix the maximized window, the main window can be borderless, which is a bit strange.

It stopped working because I also pushed other things along the fix, including "Re-add libdecor support". I usually work in bursts and push everything at once.

I see, so would libdecor support be of any benefit?

[deralmas]

I see, so would libdecor support be of any benefit?

Ye it is, as some compositors do not have their own decorations, like GNOME. We could implement our own decorations instead of using a library but it's not exactly simple. I'd like to look into that eventually though as it allows fancier features.

[deralmas]

Heads up to whoever might be testing this PR: expect instability and crashes with popups in the latest batch of commits, I'm doing some experiments

[deralmas]

update: things should have come back to a decent state, but popups are a bit iffy when they get pushed back by the compositor, as godot tries to do some fitting itself. I'll need to find a good way to stop popups from trying to be smart.

[deralmas]

Folks, I think I have a plan!

It's quite simple, we add a feature flag for "self-fitting windows" (not a great name I know) and if that's available we avoid "adjusting" popup windows. Now things should work great!

Edit (I don't want to spam too much) here's what it allows:

Not bad for not having screen-space window handling :P

[TuxTheAstronaut]

@Riteo tested out again and looks like something you did or the 4.5 branch did fixed it because now the controller has been working normally again so far

[deralmas]

FEATURE_SELF_FITTING_WINDOWS, which signals that the compositor can fit windows to the screen automatically and that nodes should not do that themselves.

To the reviewers: perhaps, on second thought, this flag could be more useful to other platforms as FEATURE_ABSOLUTE_WINDOW_POSITIONING or something like that? "Self fitting windows" are a consequence of the absence of absolute window positioning after all, I think.

Edit: That would mean that every other platform must expose this feature (except android, web and iOS perhaps), dunno if that'd be a problem.

[Arignir]

As a fellow Wayland/Sway user, can't wait for this to get merged! Thank you for your work!

[bruvzg]

I have tested as is and rebased on top of master, apart from aforementioned popup issue everything seems to be working as expected. Code looks good.

Not sure if it's necessarily should be done in this PR, but having two FLAG_POPUP and FLAG_POPUP_WM_HINT flags which have extremely similar function and might be a bit confusing.

The only difference in the usage seems to be FLAG_POPUP is unset for tooltips (since it's controlling input capture), so it might be better to use FLAG_POPUP instead of FLAG_POPUP_WM_HINT for Wayland, and add a new FLAG_TOOLTIP which will override FLAG_POPUP on other platforms.

Something like this (a quick and completely untested draft): https://gist.github.com/bruvzg/a3f8e468af7ef416f5e8c10d0e4ea47b

Or we should rename FLAG_POPUP to something else, like FLAG_CAPTURE_ALL_INPUT_WITHOUT_FOCUS_AND_AUTOCLOSE_WHEN_CLICKED_OUTSIDE (no idea what the good name for it would be).

[deralmas]

Thank you bruvzg for the very thorough review! I'll address each point ASAP.

[deralmas]

Not sure if it's necessarily should be done in this PR, but having two FLAG_POPUP and FLAG_POPUP_WM_HINT flags which have extremely similar function and might be a bit confusing.

The only difference in the usage seems to be FLAG_POPUP is unset for tooltips (since it's controlling input capture), so it might be better to use FLAG_POPUP instead of FLAG_POPUP_WM_HINT for Wayland, and add a new FLAG_TOOLTIP which will override FLAG_POPUP on other platforms.

Or we should rename FLAG_POPUP to something else, like FLAG_CAPTURE_ALL_INPUT_WITHOUT_FOCUS_AND_AUTOCLOSE_WHEN_CLICKED_OUTSIDE (no idea what the good name for it would be).

Yeah it's definitely quite confusing. IMO FLAG_POPUP should be renamed to something more descriptive (as in my head popups are a more generic concept) but, in any case, can we even rename stuff without breaking compat?

[bruvzg]

but, in any case, can we even rename stuff without breaking compat?

Renaming enum flag won't break binary compatibility for GDExtension, and a deprecated extra flag with an old name can remain for script compatibility:

#ifndef DISABLE_DEPRECATED
   FLAG_POPUP = FLAG_NEW_NAME
#endif

But I do not think this is critical for this PR, it can be renamed later.

[deralmas]

All right, all the points should have been covered!

BTW, at least on sway, I'm noticing some !windows.has(p_window_id) errors when closing/opening certain windows. They seem mostly related to _send_window_event with closed windows so I guess I should just add an early return and perhaps get rid of some ERR_FAIL_* macros. Thing is I don't really remember them being there before, I wonder what changed...

They don't break anything AFAICT but they're definitely a bit annoying. Should I fix these errors in this PR or should I do that in a follow-up PR?

[bruvzg]

BTW, at least on sway, I'm noticing some !windows.has(p_window_id) errors when closing/opening certain windows. They seem mostly related to send_window_event with closed windows so I guess I should just add an early return and perhaps get rid of some ERR_FAIL* macros.

I can see this as well, seems like late focus event for closed popup, and can be fixed with this (not critical, and can be follow-up PR, maybe there are other similar conditions):

diff --git a/platform/linuxbsd/wayland/display_server_wayland.cpp b/platform/linuxbsd/wayland/display_server_wayland.cpp
index c41c1c594f..613775a3f8 100644
--- a/platform/linuxbsd/wayland/display_server_wayland.cpp
+++ b/platform/linuxbsd/wayland/display_server_wayland.cpp
@@ -1557,7 +1557,7 @@ void DisplayServerWayland::process_events() {
 		}
 
 		Ref<WaylandThread::WindowEventMessage> winev_msg = msg;
-		if (winev_msg.is_valid()) {
+		if (winev_msg.is_valid() && windows.has(winev_msg->id)) {
 			_send_window_event(winev_msg->event, winev_msg->id);
 
 			if (winev_msg->event == WINDOW_EVENT_FOCUS_IN) {

[Repiteo]

Thanks!

[Giganzo]

Does this make it possible to support the embedded game window on Wayland? It's currently disabled for prefer Wayland setting.

[deralmas]

Does this make it possible to support the embedded game window on Wayland? It's currently disabled for prefer Wayland setting.

It's a first step but no, not directly. We can't teleport arbitrary windows like other platforms do.

I'm working on it and I've found a solution that is coming along quite nicely, so just please be patient :D

[staakman]

Hey! I know this is most likely the wrong place but just wanted to say thank you all (and especially Riteo ofcourse)! I just switched from Windows to NixOS and can't wait to try this out! Couldn't be better timed.