[WIP] Implement a Wayland DisplayServer #57025
Conversation
|
@Riteo , |
|
@smohantty if you want to contribute for now I guess that you could wait for the new, asynchronous prototype (I'm working on it right now, I'm free pretty much only on late fridays and the weekend, sorry about that) and maybe review it then? Or I guess you could also share any findings you had or say what you think of the proposed new architecture in the PR. |
|
OK, I moved everything into another thread, but it looks like |
|
Update on that: the issue is not only caused by window rect changes but also by inputs in general, as the input dispatching function uses |
|
Following the recent issue with |
Currently, I am looking at kWayland(https://github.com/KDE/kwayland) and qtWayland(https://github.com/qt/qtwayland) for reference as they have implemented all the features ( multi-window , server and client-side decoration async event etc) will be helpful in designing wayland integration in godot. |
|
@smohantty are you sure we can reference them? They look to be mostly GPL code, a license incompatible with Godot's MIT license. Also, I'm not sure if QT's architecture might be comparable with Godot's. As you can see, there are quite a few shortcomings and limitation to take in considerations that are quite unique. |
The reference is just to understand the full feature set we have to provide not to copy the implementation. one more wayland integration for reference (https://github.com/Enlightenment/efl/tree/master/src/lib/ecore_wl2) So am just going through all different implementations to come up with all the building blocks required to provide all the features we need and at the same time easy to maintain as well as extend in the future. My rough idea is to develop the Wayland support as a separate module/library which can be consumed by the DisplayServer. |
|
@smohantty mhh, what you're proposing is actually kind of done in other parts of the engine, look in the At most, in my opinion, we could do just a For now, I'd make everything in the Maybe it could be me that I'm always too afraid of over-engineering things, but I think that at this point this is the best route to take IMO. |
|
Ok, new update: I've implemented a very basic message queue and it looks like it works. The commit message should contain some info on roughly what I did, still in the usual brain dump format used by this update too. As usual, after some other thinking, I changed the way things will go, understanding that if I were to wrap all Wayland events I would've reimplemented That's why I'll be just implementing the basic messages that the editor needs to know about in the main thread, which is pretty much stuff with callbacks (window changes, input events, etc), so that on the Wayland layer we can do everything that has to be immediate as soon as possible (especially to avoid filling up the oh-so-precious and limited Wayland event buffer), leaving the main thread all the time it needs to do whatever it pleases, and to let all of the scene tree and whatever know that these things happened, while the Right now the only message implemented and working is the window rect change message, which makes the I've still not tested this queue extensively, I've pretty much just rescaled the window a bunch of times and called it a day, as input dispatching is currently temporarily Synchronization is still pretty sketchy, but we've still got time to polish it and make absolutely sure that there won't be race conditions or stalls, also looking where to use the right mutexes at the right place, instead of one monolithic Closing up, I feel like I've finally gone out of the loop of thinking a lot about how to do something "the right way", implementing it and seeing it fail gloriously, which was pretty frustrating and took a lot of time. Instead, I found that concentrating first on making it work and eventually polishing seems to work pretty nicely for now, especially for my mental health. So there's the possibility that I might make certain choices in the moment without thinking about it too much just to make something work, and only then then try to improve/clean/refactor it. |
|
I've implemented scroll and modifier support. There's just this weird issue where non physical shortcuts get registered as lowercase. It may be something very stupid that I did, but for now I'll go to sleep. Although the running game window is kinda broken (on sway its buffer is offset for some reason), we're slowly getting closer to actually getting something that can be tried by more people! Edit: Fixed both of those issues. |
|
I've just added double click and keyboard support, so technically the editor should be just enough usable to do something. There's still a lot missing, but we're slowly getting there. |
|
I'm adding window event support too, but if on sway the window gets tabbed out, its focus out event isn't handled until it is shown again. Probably |
|
For some reason I avoided implementing the above mentioned fix for now, as I'm uncertain whether it's actually the right solution. |
Riteo
changed the title
|
After attempting to implement other things, I think that the prototype (read: "messy code") stage is currently over. Everything else that has to be implemented depends either on some external library or is complex enough to deserve some more well structured code. So for now, while trying to add more complex features, I'll try to refactor/cleanup/fix some things and slowly refine the whole implementation. I might also take more into consideration the older codebases, since I've built an idea of how Wayland works and stuff like clipboard support and advanced pointer features should be the same. The only big problem left is, IMO, multiple window support. The difference between popups and toplevels in XDG shell imposes some weird restrictions that will be interesting to fix. |
Riteo
force-pushed
the
wayland
branch
2 times, most recently
from
c19fd0b to
c67cf1b
Compare
|
@smohantty I noticed that you've been making some changes by yourself on your fork of my branch (namely, cursor support). I have no idea if you want to keep these for yourself, but in any case if you're interested in adding support for something in this PR let me know, in order to avoid stepping on our shoes. Also, sorry for the rebase! I did not expect you making other changes as you went silent. |
|
Welp, I tried making the CI work with this PR for fun by adding the relevant dependencies, but it somehow messes up. I guess that the code generated on my machine uses functions not available on the older |
|
This is the same error I get if I try to compile on Fedora 35 where the 1.19.0 version of libwayland-client is installed. |
|
@roberti42 I guess that this should be fixed once dynamic code generation is added. It isn't high in my todo list, but if that does improve testing, I can try to add it ASAP. I think that @ddevault made a scons module for that. |
|
@Riteo what is dynamic code generation? |
Sorry, didn't mean to stress ;-) Second issue does not occur anymore, awesome 💪 |
(cherry picked from commit fcb8d19)
The input mask was wrongly ignored in earlier versions. Now it is actually used so the input mask variable needs to be a valid number (cherry picked from commit 777d6ae)
(cherry picked from commit 51dcb38)
Cherry-picks for the 4.2 branch (future 4.2.1) - 2nd batch
- Set `-sSTACK_SIZE` to what it was before emscripten 3.1.27. It was renamed in 3.1.25 so also set `-sTOTAL_SIZE` for older versions for consistency. - Set `-sDEFAULT_PTHREAD_STACK_SIZE` to what it was before 3.1.30. Co-authored-by: Rémi Verschelde <[email protected]> (cherry picked from commit 8e5fbd4)
|
@arlo-phoenix I got reason to believe that the weird fullscreen size issue you had might be related to the one that @Domeee got with scaling. Could you see if you can still replicate it with the latest commit? |
@Riteo Doesn't happen anymore, thanks!
Not doing anything production related rn so I'm fine with just my hardcoded constants :), thanks for the info though. I think if the points of fractional scaling above are addressed it is not necessary for 99.9% of people to get fractional scaling. Couldn't find much, but what does windows do? There's fractional scale there as well and if there's no method there, we don't need it either (or am I missing something). The only thing I found about this topic with a quick search is godotengine/godot-proposals#2661, where someone suggests that you can kinda get it through the DPI changed callback. Overall I did exaggerate my issues in my last comments, I still haven't found out how in the world I broke my control positions/sizes which is why I didn't comment, but clicking enough always breaks something (I messed with project settings so maybe there?) I'm pretty certain it is not an issue related to this or wayland so forget that. I also wanna say I don't want this to become an issue list for this PR, imo this still should be merged very early alpha so more people test this. The only thing that actually matters imo is that crash and if there are any others hidden, all others have workarounds or are just nice to haves (e.g. window between screens). Just using this for the editor works great and the only time I crashed was the gltf import. It's already looking quite amazing in stability from my experience! |
Great, you're welcome! 🎉
Uh if that's expected behaviour we could roundtrip (read: sync) after asking for fullscreen. How do the other platforms react to a mode change like that?
Uh I'm aware of that issue. Quite annoying stuff; I looked at that a few days ago. IIRC it's a Godot limitation. In short, we can't change the editor UI scale at runtime yet, so we (read: all HiDPI-capable backends) are resorting to choosing the highest scale along all screens and just forcing that on. It's pretty wasteful, but it is what it is. It's recently getting worked on (#86022) though luckily :) The reason that we can't do that with frac scale is two fold: the data is per window (so we can't know a "max value" on start) and it would be a fractional mess and thus going against the point of frac scale in the first place. So I think that this is what's happening, we're changing the buffer-size/frac-scale without compensating in the UI, just like in the buffer case (which is definitely that). See ae0e07b for more info.
The stuff you're getting is already translated into "Godot Space" or, in other words, it's already scaled. The PR you linked is working on adding another field also representing the "raw" or "unscaled" value. So yeah, you should be able to use what you get without worrying about scaling or whatever. It should also work with fractional scale just fine as there are helper methods to get always a proper
That crash is a bit of a mistery to be completely honest. BTW FTR I also asked the Wayland folks and it looks like the client-side wayland event queue is unbounded, so this thing is not even having a chance to read the compositor-side (bounded) ring buffer. I tried replicating it very dumbly with a GDScript You know what. While writing this I had a theory. If you say that it happens while importing it could be related to I'll investigate after some good night sleep (I'm a bit sleep deprived as of writing). Thanks for giving a repro case of this issue!
I'm pretty sure Windows does nothing. Both Relevant code snippet (taken from this PR, thus including also an extra check for Wayland): float EditorSettings::get_auto_display_scale() const {
#ifdef LINUXBSD_ENABLED
if (DisplayServer::get_singleton()->get_name() == "Wayland") {
return DisplayServer::get_singleton()->screen_get_max_scale();
}
#endif
#if defined(MACOS_ENABLED) || defined(ANDROID_ENABLED)
return DisplayServer::get_singleton()->screen_get_max_scale();
#else
const int screen = DisplayServer::get_singleton()->window_get_current_screen();
if (DisplayServer::get_singleton()->screen_get_size(screen) == Vector2i()) {
// Invalid screen size, skip.
return 1.0;
}
// Use the smallest dimension to use a correct display scale on portrait displays.
const int smallest_dimension = MIN(DisplayServer::get_singleton()->screen_get_size(screen).x, DisplayServer::get_singleton()->screen_get_size(screen).y);
if (DisplayServer::get_singleton()->screen_get_dpi(screen) >= 192 && smallest_dimension >= 1400) {
// hiDPI display.
return 2.0;
} else if (smallest_dimension >= 1700) {
// Likely a hiDPI display, but we aren't certain due to the returned DPI.
// Use an intermediate scale to handle this situation.
return 1.5;
} else if (smallest_dimension <= 800) {
// Small loDPI display. Use a smaller display scale so that editor elements fit more easily.
// Icons won't look great, but this is better than having editor elements overflow from its window.
return 0.75;
}
return 1.0;
#endif
}
No worries, this is not a trivial task, so weird bugs are definitely to be expected. There are still lots of workarounds to "fit" Wayland into Godot, including workarounds to current "upstream" issues since native gaming is not as polished as other areas of the Wayland ecosystem, so I don't blame you for thinking that it was caused by this backend :) And who knows, maybe it was actually a rare edge case :P
Definitely. We're at the perfect time for merging and I'm already arranging everything for squashing/merging/communicating this ASAP. Expect something very soon! :D This will also allow for easier creation of new APIs like
Awesome! I'm really happy that it's working great for you! I don't expect this to have production-grade stability yet though, as people have a lot of different setups and supported protocol combinations and you can't stop the testing power of a lot of nightly users :P There's also a lot of compatibility code which has still to be written for newer and older globals than the versions selected by default, in order to harness the full feature set of new compositors. I'm also working on properly communicating what people should expect and how they can help before putting everything in place to avoid false expectations. That's exactly why we're pushing for merging this ASAP right at the start of the dev cycle after all, to test it very thoroughly :) |
… wayland Pretty self-explanatory.
|
@arlo-phoenix I can't replicate by loading a big glb either :( Let's apply the buffered input theory. Can you replicate the issue with this diff? diff --git a/platform/linuxbsd/wayland/display_server_wayland.cpp b/platform/linuxbsd/wayland/display_server_wayland.cpp
index 008c0d879a..c097047917 100644
--- a/platform/linuxbsd/wayland/display_server_wayland.cpp
+++ b/platform/linuxbsd/wayland/display_server_wayland.cpp
@@ -1027,7 +1027,7 @@ Key DisplayServerWayland::keyboard_get_keycode_from_physical(Key p_keycode) cons
}
void DisplayServerWayland::process_events() {
- MutexLock mutex_lock(wayland_thread.mutex);
+ wayland_thread.mutex.lock();
while (wayland_thread.has_message()) {
Ref<WaylandThread::Message> msg = wayland_thread.pop_message();
@@ -1069,9 +1069,11 @@ void DisplayServerWayland::process_events() {
wayland_thread.keyboard_echo_keys();
- Input::get_singleton()->flush_buffered_events();
-
frame = wayland_thread.get_reset_frame();
+
+ wayland_thread.mutex.unlock();
+
+ Input::get_singleton()->flush_buffered_events();
}
void DisplayServerWayland::release_rendering_thread() { |
| @@ -41,7 +41,7 @@ void register_linuxbsd_exporter_types() { | |||
| void register_linuxbsd_exporter() { | |||
| Ref<EditorExportPlatformLinuxBSD> platform; | |||
| platform.instantiate(); | |||
| platform->set_name("Linux/X11"); | |||
| platform->set_name("Linux/X11/Wayland"); | |||
There was a problem hiding this comment.
Wouldn't it now be better to just call the platform "Linux"?
There was a problem hiding this comment.
@adamscott not sure, as the display server is a pretty important information, although now it's probably not as important as when we had only X11.
If we had to rename it I'd go for "Linux/BSD", as it's actually also supposed to support BSD (and it probably unofficially supports other *nixes)
@Riteo Can't crash it anymore. Thanks! For reference if we were talking about the same issue, these were the error messages before crash before the patch (shouldn't matter anymore with this fixed. I'm pretty sure as well since I couldn't replicate it in several reimports and I could crash it every time before): Also about how I tested: With large glb file I meant around 100MB which you can easily create by just using a couple 4k textures and packing them into the glb. Then I just moved my mouse a bit and clicked a lot and it crashed after some time. |
wow, that's fantastic to know! I still wonder why I couldn't replicate it myself, mh. Well, it's simply best practice to not keep a mutex locked when not needed, so I also learned my lesson.
I think I got the same message that one time it happened, so it probably was it. For reference, error 32 is Anyways, thanks for testing! I'm applying this patch once I'm done finishing something 👀 |
This avoids locking the thread too long in situations like importing, where (probably) other methods are waiting for some mutex is stalled and triggering a wayland event queue overflow. I'm not sure of what exactly since I couldn't replicate it reliably.
Not really sure what was my idea, probably I wanted to share its code with X11 and forgot to do so, committing it in the process. I wouldn't bet on that though. The only thing I know is that this was 100% dead code. Oh well.
Hopefully I catched them all.
|
All right. It's time to get this merged in and polish it in-tree! To be clear, any issue you're experiencing right now will be fixed post-merge, to aid with reviewing, but I have not forgotten about them. I've made a new PR dedicated to reviewing where I squash the metric ton of history in one single commit, along with a descriptive comment, a review guide and credits for whoever helped with the code (I hope I didn't forget anyone!), although I thank every single one of you testing and commenting under here, your input has been extremely valuable! So, I can't believe I'm finally saying this: Superceded by #86180. |
Riteo
requested review from
RandomShaper
and removed request for
RandomShaper
Superceded by #86180.
Resolves godotengine/godot-proposals#990.
Multi windowing won't be implemented in this PR due to some incompatibilities with Wayland's and Godot windowing API which have to be sorted out. Trust me, I tried. Doing otherwise would just stall everything. I'm sorry.
The aim of this PR is to eventually implement a fully working
DisplayServercompatible with the Wayland protocol on Godot 4.x, which had architectural changes that led to the closing of #27463.Status
It is currently beta quality. It's definitely usable and most features are implemented but there's still lots of testing to do.
As there's a growing need of this backend due to poor XWayland compatibility, the main focus of this PR is polishing and documenting the current feature set as best as we can so that we can get this merged and stable by 4.3. Everything that's not implemented yet will be implemented post-merge.
For ease of testing this PR will track, whenever possible, the latest (pre)release closer to
master.Current version tracked: 4.2-stable
For a list of what's missing and of known issues see below.
Testing
Thanks to GitHub Actions, every commit is built and the result can be downloaded up to 90 days since the last change. See Testing pull requests for info on how to get a build.
Note: for users without an account, the steps outlined in the documentation are broken. Instead of putting a link to the workflow file in nightly.link, go to checks, click on a random job on the left bar and copy/paste the URL up to
runs/NUMBERS. You'll be able then to download the latest build.This means that usually there's no compilation required as it's already done for you.
At startup the engine will try initializing a Wayland display. If it fails, it prints an error and tries with X11. There's no need to have both libraries installed to run the executable as they're dynamically loaded at runtime if available.
Note that it's always possible to force the X11 backend to start first (triggering XWayland if available) by appending the
--display-driver x11argument.This PR is also completely HiDPI aware and includes fractional scaling (except for the cursor). Note that there are still some known issues with the HiDPI implementation in the cases of fractional scaling.
Building
If for whatever reason there's the need of building this branch from scratch, a plain
sconsinvocation should work fine, building an hybrid dynamically loaded X11/Wayland executable.Every new header and dynamic library wrapper is embedded in the source, so there are no extra build dependencies to download, with the current exception of
wayland-scanner, required for generating protocol wrappers at build time.Stuff not done yet
Important methods missing
window_set_mouse_passthrough(Wayland doesn't support polygonal regions, only rect regions). We could implement a bounding box approximation, but an entirely new rect-based API like Add mouse_passthrough_rectangles godot-proposals#8568 would be betterBig features missing
NOTE: Due to the urgency of this PR (and general eligibility), this list is purely informative and should not be treated as an hard roadmap for merging.
What about xdg-desktop-portal?
xdg-desktop-portal is essential for interacting with modern Wayland desktop environments but it should not be implemented in this PR directly. Luckily there is already a growing support for it upstream.
Known issues
This PR's
We don't have a lot of support code for older/newer globals yet.
Under certain unkown situations, when heavy work is done on the main thread, it's possible to block the Wayland thread and crash the engine due to a Wayland event queue overflow.
The low processor usage time is always set to 500 µs (0.0005 ms) due to latency concerns with manual frame throttling.
The cursor doesn't handle fractional scaling yet.
Autoscale returns a rounded-up size when using fractional scale as it comes from the perspecitve of the screen. A better window-centric API/workaround is needed.
Tablet support has been commented out as I didn't have a chance to refactor it during the
WaylandThreadsplit.The particular combination of
x11=no wayland=yes opengl=yes openxr=yesfails to build.Wayland specific (stuff that we can't fix directly or requires workarounds)
It looks like libdecor has some issues toggling server-side decorations.
I'm not really sure if we have a reliable way of setting size non-interactively. As such
DisplayServer::window_set_sizeis currently a no-op. Further investigation is required.At least on sway, it looks like DnD doesn't update the cursor properly. I can replicate this outside Godot so this might be a compositor bug.
Godot specific (stuff that looks related to this PR but is actually not)
Scrolling on a touchpad is a little bit too fast. See Oversensitivity in trackpad scrolling and gestures #49873.
The shaders'
TIMEvariable is wrong when the window is "hidden" (which is most of the time for us due to Wayland reasons). A proposed fix and more info is available over at Pass the actual draw delta to the RenderingServer #82222.The splash doesn't resize. This means that tiling WMs will have a wrongly sized window until the end of loading. There's no clean way of improving this and, while hacks are definitely possible, I'm not sure that it's currently worth it.
Thanks to
I'm always open to external contributions (especially as I've never worked on anything like this before) be it patches, testing, reviews, general technical help or even just moral support, they're all very appreciated!