GNOME Bugzilla – Bug 781909
Implement KDE's server-decoration protocol
Last modified: 2018-04-26 18:12:00 UTC
The protocol is here: https://cgit.kde.org/kwayland.git/tree/src/client/protocols/server-decoration.xml This allows the client to negotiate between server or client side decorations. Some GTK windows should use CSD (when they have customized the title bar, that is), but for others it may be more appropriate to use SSD. This protocol informs the client of the server's preference and allows the client to inform the server which strategy it intends to use. I'm partway done with a patch for this.
Created attachment 350696 [details] [review] Implements the KDE server-side decorations protocol
Review of attachment 350696 [details] [review]: Hi; thanks for the patch — and I apologise for taking this long to get a review. In general, this looks okay to me, and fits in with the GTK approach. ::: gdk/wayland/gdkdisplay-wayland.c @@ +336,3 @@ static void +server_decoration_manager_default_mode(void *data, + struct org_kde_kwin_server_decoration_manager *manager, Coding style: 1. missing space between function name and open parenthesis 2. the argument types should be aligned @@ +340,3 @@ +{ + GdkWaylandDisplay *display_wayland = data; + g_warning("Compositor prefers decoration mode %d", mode); This should be a g_debug(), and it would be nice to translate the mode from integer to string, according to the protocol. @@ +479,3 @@ + wl_registry_bind (display_wayland->wl_registry, id, + &org_kde_kwin_server_decoration_manager_interface, 1); + org_kde_kwin_server_decoration_manager_add_listener(display_wayland->server_decoration_manager, Coding style: missing space between function name and open parenthesis. ::: gdk/wayland/gdkwaylanddisplay.h @@ +59,3 @@ +gboolean +gdk_wayland_display_prefers_ssd (GdkDisplay *display); Coding style: the name of the function should be on the same line as the return type. ::: gdk/wayland/gdkwaylandwindow.h @@ +82,3 @@ char *parent_handle_str); +void gdk_wayland_window_enable_csd (GdkWindow *window); I think this is a bit of a misnomer; we're not really enabling CSD per se — we are telling the compositor that we are using client-side decorations. I don't have a better name to propose, though.
Created attachment 356550 [details] [review] Implements the KDE server-side decoration protocol Updated per feedback
Whoops, has some indentation issues. v3 incoming
Created attachment 356551 [details] [review] Implements the KDE server-side decoration protocol
Unfortunately, the patch doesn't apply cleanly, and I don't have time to fix things up manually right now. So it will have to wait until after the release I'm doing right now.
I can rebase the patch and shoot you a new one if you like.
Created attachment 357164 [details] [review] Implements the KDE server-side decoration protocol
Review of attachment 357164 [details] [review]: I'm not in a position to test this right now, but it looks ok to me ::: gdk/wayland/gdkdisplay-wayland.c @@ +52,3 @@ +#include "xdg-shell-unstable-v6-client-protocol.h" +#include "xdg-foreign-unstable-v1-client-protocol.h" +#include "server-decoration-client-protocol.h" This seems to be duplicating the includes for xdg-shell and xdg-foreign ?
Thanks for the review, Matthias. Here's a new patch, rebased and with a fix for your comment.
Created attachment 361386 [details] [review] patch
Created attachment 361387 [details] [review] patch Eeep, I missed a tab->space in the meson build. New patch.
> make[4]: *** No rule to make target 'server-decoration-client-protocol.h', needed by 'all'. Stop. > make[4]: *** Waiting for unfinished jobs.... It possible in some way check 'server-decoration-client-protocol.h' header file?
(In reply to vitalik_p from comment #13) > > make[4]: *** No rule to make target 'server-decoration-client-protocol.h', needed by 'all'. Stop. > > make[4]: *** Waiting for unfinished jobs.... > > It possible in some way check 'server-decoration-client-protocol.h' header > file? This was already fixed in bug 789630 and released with GTK+ 3.22.26. Please, the next time open a new bug if you want to report a build issue.