wayland client-side decorations #4068
Conversation
cmake/sdlchecks.cmake
Outdated
| if(WAYLAND_LIBDECORATION) | ||
| include(ExternalProject) | ||
| ExternalProject_Add(libdecoration | ||
| PREFIX "${CMAKE_CURRENT_BINARY_DIR}/extern/libdecoration" | ||
| GIT_REPOSITORY https://gitlab.gnome.org/jadahl/libdecoration.git | ||
| GIT_TAG master | ||
| CONFIGURE_COMMAND meson --prefix "${CMAKE_CURRENT_BINARY_DIR}/install" --libdir "lib" ../libdecoration | ||
| BUILD_COMMAND ninja | ||
| INSTALL_COMMAND ninja install | ||
| ) | ||
|
|
||
| set(SDL_VIDEO_DRIVER_WAYLAND_DYNAMIC_LIBDECORATION "\"libdecoration.so\"") |
There was a problem hiding this comment.
I would really like to not have this pull in external code at configure time, especially if we have to count on meson being installed to make this work.
Generally we check if the header/lib is available on the system and either enable the feature, or not, from there.
Is libdecoration not available in distros yet?
There was a problem hiding this comment.
At the moment libdecoration is still in development, the ABI hasn't frozen yet. This probably won't be in distributions until 1.0 is tagged.
There was a problem hiding this comment.
I agree that the final merged support should not pull in the dependency like this. How is SDL handling third-party dependencies that are not distributed by distributions yet? In the age of flatpak and snaps, this is not an issue since these third-party dependencies can be pulled in manually from source.
libdecoration has not had a release yet. Some pending PRs are to be merged and a 0.1 release is imminent (cc @jadahl). But even then, it will take a while until this is picked up by distributions.
@icculus What do you propose?
- Do you want to wait until a first libdecoration 0.1 release is distributed by distributions, before supporting Wayland? If so, which distributions would that be?
- Should there be a way for SDL to depend on third-party libraries?
There was a problem hiding this comment.
The SDL source tree already contains it's own set of wayland-protocols instead of depending on the definitions supplied by the distributions. Should all third-party dependencies be integrated this way directly in the source tree?
There was a problem hiding this comment.
To be clear, I'm ignorant of obvious details here, so:
- If libdecoration is just a C callable API, we will require the headers be available on the system at build time and will dynamically load the shared library at runtime. If it's not in distributions yet, that's not our problem; if we see it when configuring, it's enabled, otherwise it's not. This lets the bleeding edge install it from source before the world catches up, but the binary that is built will work with or without the library on the end-user's system.
- If it's just a wayland protocol, that's a no-brainer, we just put the protocol files in SDL's tree and call it a day. At runtime we either have the protocol or we don't and we have to handle both cases anyhow. Assuming they're doing things in the usual Wayland way, we can commit before ABI is stable because the protocol name will change and we'll ignore the changed version (and later, we can decide to upgrade or support both codepaths as appropriate...for this it sounds like we'd just dump support for the pre-0.1 release).
But it would be helpful to for @jadahl to say how far from a stable ABI we are at the moment.
There was a problem hiding this comment.
Libdecoration is C API/ABI, that aims to lock into an ABI in the near future. I intend to package it for Fedora (F35) after that happens, and I hope it'll show up elsewhere as well. I'm sure you will see comments about it here when that happens.
|
I only have 3 concerns left at this point, other than the ABI freeze:
|
The SDL Debian package is still using the autoconf build script. I can look into integrating the libdecoration dependency into the script, once there is a released version.
Is C89 compatibility still a requirement? I would say that every distribution that includes Wayland also includes a C99 capable compiler. What would be the benefit of restricting the Wayland support to standards that are older than the Linux kernel itself?
If you want client-side decorations via libdecoration, it must come first. Prioritising |
We need it, but we can merge this and add that later, as long as we do it before 2.0.16 ships.
I'd have to look again, but if the headers use stdbool, that is tolerable, but we should not otherwise introduce it in our code.
Prefer over xdg-shell, but maybe SSD is preferred over this? What happens if libdecoration is available but you're running KDE? Do you get GTK decorations? |
libdecoration supports xdg-toplevel-decoration if available, and use SSD if that is preferred by the compositor. |
Ah cool, that was the only reason I asked - so yeah, where the libdecor path is at now works for me. So once the ABI is frozen this should be good to go I think, that should buy enough time for someone to write in the autoconf stuff. |
| *left = 0; | ||
| *bottom = 0; | ||
| *right = 0; | ||
| return SDL_SetError("FIXME libdecoration: Support GetWindowBordersSize"); |
There was a problem hiding this comment.
Forgot to mention here - if it helps with the API design on libdecor's part, we're okay with this having a return value indicating lack of support in the backend. From what I understand this is something CSD can do pretty easily but SSD/xdg-decoration don't and won't support this, so having an error path for those situations is fine.
There was a problem hiding this comment.
I think the best thing to do is to always return 0 no matter what. These values have no use case under Wayland, and the fact that there might be subtle differences depending on the configuration, any accidental use would be fragile, so things would be more predictable if it'd just always be assumed to be 0.
There was a problem hiding this comment.
I'm not so sure about that - @icculus, I know this is in UE4 but do you recall what made this necessary? As far as I can tell it's used for mouse input but I don't have a good setup to check how Wayland behaves without this.
There was a problem hiding this comment.
Pointer/mouse input in Wayland is always relative to a wl_surface.
One place it might be relevant is for placing xdg_popup surfaces; it would need to take the non-drop-shadow part of the window border size into account, e.g. the title bar. How exactly is popup positioning (menus etc) handled with SDL?
|
Yup, just be be clear, this should not be pulled until libdecoration has hit ABI freeze. |
|
Can someone tell me why the CI is trying to link against |
|
This got clobbered slightly by 57a927e. Nothing major, it just collides at SetWindowMin/MaxSize, the fix will look roughly the same as the other places where xdg/zxdg paths exist. |
I rebased and in the process squashed the commits to make the rebase easier. It works but I may have still forgotten something. |
|
What is left to do before this pull request will be merged? |
|
Thanks for answer. Probably we can expect distributions to pickup libdecor when libraries like SDL will merge support for it. I hope it will happen soon, SSD vs CSD issue needs to be fixed and I think libdecor is good way to do it. |
|
Can I just remark something visual: I think in the example screenshot the title bar looks very cramped, very 1995 (and not in a good way, IMHO). Edit: I made an upstream ticket for this now GNOME and also Windows 10 use larger title bars these days, which is also relevant for touch devices: GNOME: Windows 10: While KDE less so, it still looks larger than the libdecoration one: Compare this to the measly looking one from the demonstration screenshot: I think it makes sense to aim more for a Windows 10/GNOME size for the libdecoration default. E.g. this also impacts devices like the PineTab or PinePhone where touch is pretty central, so going really minimal on the height not only looks dated but can be an actual accessibility problem. |
|
The You can implement additional, more complex plugins. The idea is then that the library would select an appropriate plugin for the desktop. I am currently working on a GTK plugin. Every help in realising this is welcome :-) My personal opinion about decorations:
|
Can you elaborate why not? I think especially on a tablet or phone, actually it does. We're talking about 54% larger target area here. How is that not a significant difference?
Have you tried windowed GTK+ apps on a PinePhone or Librem 5 yourself? At least from my personal experience it's not that obtrusive visually, since on a phone all screen elements tend to be pretty big and spacey anyway with relatively minimal UI rather than cramping a lot onto the screen at once. And there is a real problem with accurately pressing any buttons, on a phone in particular, on a title bar that somehow tries to be minimal for whatever reason. (Which most do not try to be, thankfully.) So I personally really prefer the spacey title bars, and I don't think I am alone.
All Linux phones support seamless convergence, also there is still fullscreen for when you want the full multimedia space. So all of them use a stacked window manager, and it works fine. I can only suggest again to try one of the Linux phones for yourself to see how this feels like in practice. I think the default should be good accessibility, not some IMHO needless attempt to minimize space. Defaults are very important because many apps will not change them. Therefore, I don't think the current defaults are ideal. Edit: additional note on this one:
I don't think depending on something as giant as GTK+ just for a better default height of the title bar with some more focus on accessibility is a good solution, in particular since the cairo titlebar looks perfectly fine otherwise. Sure it doesn't blend in perfectly, but who cares. |
|
Is your concern about to the "90's" UI style of the decoration or its usability on touch screen devices? I am not against larger UI elements for touch devices, I am just not convinced anybody would use stacked window managers on small touch screens to move and resize application windows.
Until we see mass usage of stacked window managers on 5~10 inch touch screens, I really don't see the point of bothering with these details. If you want to use decorations on a particular system, you should go with the UI guidelines there and probably use their toolkit if you really have to (i.e. GTK in the Librem 5 case).
Nope, I don't own such devices. But Android and iOS are pretty successful, even without using stacked window managers. If you attach a large screen to experience "seamless convergence" then you probably also don't need thick title bars without widgets. If you really must use titlebars on phones or tablets, I would suggest drawing the decoration with the same toolkit that is used for applications to match the UI scale. The |
This bug is now fixed in upstream KWin, and the maximize button now is correctly hidden with libdecor if I run it against the latest KWin git. The fix should be in KDE Plasma 5.23. |
If CMake has become the preferred build system, we can move to CMake (not for Debian 11, which is in hard freeze, but certainly for Debian 12). For packages that have more than one build system, we usually keep using the one that seemed like it was recommended at the time we first packaged it, unless the upstream maintainer tells us that it has been deprecated and recommends that redistributors move to a replacement. For example, when GLib deprecated Autotools and started recommending Meson, they announced that in the release notes and we switched; but GTK 3 has Autotools and Meson in parallel with no particular recommendation of which one is considered better, so we have continued to use Autotools. |
|
@icculus @flibitijibibo @smcv An initial "0.1" release of |
|
Just tagged 0.1.0: https://gitlab.gnome.org/jadahl/libdecor/-/releases/0.1.0 will start packaging this for Fedora. |
|
@jadahl Ping me on Matrix/IRC with the review request and I'll take it on (if I haven't seen it yet). |
Will do! |
|
I'm unable to spend any more time on this, but the versioning scheme seems fine to me (really all we care about is that Once those are addressed and the build files are updated for the official naming system, this is good for final review. |
|
I added the fixed version @smcv Could you help me with getting |
|
@flibitijibibo @icculus @christianrauch @jadahl: |
|
In configure.ac, diff --git a/configure.ac b/configure.ac
index 615048e..c4839be 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1595,6 +1595,8 @@ CheckWayland()
SUMMARY_video="${SUMMARY_video} wayland"
fi
have_video=yes
+
+ CheckLibDecor
fi
fi
}
@@ -3609,7 +3611,6 @@ case "$host" in
CheckInotify
CheckIBus
CheckFcitx
- CheckLibDecor
case $ARCH in
linux)
CheckInputKD |
|
I put CheckLibDecor in there only because it was with the other Linux-y checks, this change makes sense to me! |
|
OK, pushed 9bcb5e7 for it. |
The attached set of patches implement client-side window decorations on Wayland using 'libdecoration' from https://gitlab.gnome.org/jadahl/libdecoration. The libdecoration dependency is downloaded directly from upstream at build-time.
This fixes https://bugzilla.libsdl.org/show_bug.cgi?id=2710
This fixes #3739.
Cheers and thanks for making Linux gaming happen :-)