From 21ff87d72b44604d348cf71da3175b85ac5b2f75 Mon Sep 17 00:00:00 2001 From: Ryan Dwyer Date: Thu, 27 Sep 2018 22:44:57 +1000 Subject: [PATCH] Improve CSD logic This does the following: * Removes the xdg-decoration surface_commit listener. I was under the impression the client could ignore the server's preference and set whatever decoration they like using this protocol, but I don't think that's right. * Adds a listener for the xdg-decoration request_mode signal. The protocol states that the server should respond to this with its preference. We'll always respond with SSD here. * Makes it so tiled views which use CSD will still have sway decorations rendered. To do this, using_csd had to be added back to the view struct, and the border is changed when floating or unfloating a view. --- include/sway/tree/view.h | 11 +++++++++ include/sway/xdg_decoration.h | 2 +- sway/commands/border.c | 45 ++++++++++++++++++++++++++++++----- sway/tree/container.c | 6 +++++ sway/tree/view.c | 8 ++++++- sway/xdg_decoration.c | 30 +++++++---------------- 6 files changed, 72 insertions(+), 30 deletions(-) diff --git a/include/sway/tree/view.h b/include/sway/tree/view.h index e7aaffd7..2c7b4c2b 100644 --- a/include/sway/tree/view.h +++ b/include/sway/tree/view.h @@ -76,8 +76,19 @@ struct sway_view { int natural_width, natural_height; char *title_format; + + // Our border types are B_NONE, B_PIXEL, B_NORMAL and B_CSD. We normally + // just assign this to the border property and ignore the other two. + // However, when a view using CSD is tiled, we want to render our own + // borders as well. So in this case the border property becomes one of the + // first three, and using_csd is true. + // Lastly, views can change their decoration mode at any time. When an SSD + // view becomes CSD without our approval, we save the SSD border type so it + // can be restored if/when the view returns from CSD to SSD. enum sway_container_border border; enum sway_container_border saved_border; + bool using_csd; + int border_thickness; bool border_top; bool border_bottom; diff --git a/include/sway/xdg_decoration.h b/include/sway/xdg_decoration.h index 46fb8d34..8bef4c6d 100644 --- a/include/sway/xdg_decoration.h +++ b/include/sway/xdg_decoration.h @@ -10,7 +10,7 @@ struct sway_xdg_decoration { struct sway_view *view; struct wl_listener destroy; - struct wl_listener surface_commit; + struct wl_listener request_mode; }; struct sway_xdg_decoration *xdg_decoration_from_surface( diff --git a/sway/commands/border.c b/sway/commands/border.c index 5e101564..bfd3b9ed 100644 --- a/sway/commands/border.c +++ b/sway/commands/border.c @@ -7,15 +7,49 @@ #include "sway/tree/container.h" #include "sway/tree/view.h" +// A couple of things here: +// - view->border should never be B_CSD when the view is tiled, even when CSD is +// in use (we set using_csd instead and render a sway border). +// - view->saved_border should be the last applied border when switching to CSD. +// - view->using_csd should always reflect whether CSD is applied or not. static void set_border(struct sway_view *view, enum sway_container_border new_border) { - if (view->border == B_CSD && new_border != B_CSD) { + if (view->using_csd && new_border != B_CSD) { view_set_csd_from_server(view, false); - } else if (view->border != B_CSD && new_border == B_CSD) { + } else if (!view->using_csd && new_border == B_CSD) { view_set_csd_from_server(view, true); + view->saved_border = view->border; + } + if (new_border != B_CSD || container_is_floating(view->container)) { + view->border = new_border; + } + view->using_csd = new_border == B_CSD; +} + +static void border_toggle(struct sway_view *view) { + if (view->using_csd) { + set_border(view, B_NONE); + return; + } + switch (view->border) { + case B_NONE: + set_border(view, B_PIXEL); + break; + case B_PIXEL: + set_border(view, B_NORMAL); + break; + case B_NORMAL: + if (view->xdg_decoration) { + set_border(view, B_CSD); + } else { + set_border(view, B_NONE); + } + break; + case B_CSD: + // view->using_csd should be true so it would have returned above + sway_assert(false, "Unreachable"); + break; } - view->saved_border = view->border; - view->border = new_border; } struct cmd_results *cmd_border(int argc, char **argv) { @@ -44,8 +78,7 @@ struct cmd_results *cmd_border(int argc, char **argv) { } set_border(view, B_CSD); } else if (strcmp(argv[0], "toggle") == 0) { - int num_available = view->xdg_decoration ? 4 : 3; - set_border(view, (view->border + 1) % num_available); + border_toggle(view); } else { return cmd_results_new(CMD_INVALID, "border", "Expected 'border ' " diff --git a/sway/tree/container.c b/sway/tree/container.c index d75e34a5..9b671c1d 100644 --- a/sway/tree/container.c +++ b/sway/tree/container.c @@ -669,6 +669,9 @@ void container_set_floating(struct sway_container *container, bool enable) { container_init_floating(container); if (container->view) { view_set_tiled(container->view, false); + if (container->view->using_csd) { + container->view->border = B_CSD; + } } if (old_parent) { container_reap_empty(old_parent); @@ -695,6 +698,9 @@ void container_set_floating(struct sway_container *container, bool enable) { } if (container->view) { view_set_tiled(container->view, true); + if (container->view->using_csd) { + container->view->border = container->view->saved_border; + } } container->is_sticky = false; } diff --git a/sway/tree/view.c b/sway/tree/view.c index 1c94de4c..9ffcf206 100644 --- a/sway/tree/view.c +++ b/sway/tree/view.c @@ -309,6 +309,7 @@ void view_request_activate(struct sway_view *view) { } void view_set_csd_from_server(struct sway_view *view, bool enabled) { + wlr_log(WLR_DEBUG, "Telling view %p to set CSD to %i", view, enabled); if (view->xdg_decoration) { uint32_t mode = enabled ? WLR_XDG_TOPLEVEL_DECORATION_V1_MODE_CLIENT_SIDE : @@ -316,15 +317,20 @@ void view_set_csd_from_server(struct sway_view *view, bool enabled) { wlr_xdg_toplevel_decoration_v1_set_mode( view->xdg_decoration->wlr_xdg_decoration, mode); } + view->using_csd = enabled; } void view_update_csd_from_client(struct sway_view *view, bool enabled) { + wlr_log(WLR_DEBUG, "View %p updated CSD to %i", view, enabled); if (enabled && view->border != B_CSD) { view->saved_border = view->border; - view->border = B_CSD; + if (container_is_floating(view->container)) { + view->border = B_CSD; + } } else if (!enabled && view->border == B_CSD) { view->border = view->saved_border; } + view->using_csd = enabled; } void view_set_tiled(struct sway_view *view, bool tiled) { diff --git a/sway/xdg_decoration.c b/sway/xdg_decoration.c index 80b2f57e..39e0df56 100644 --- a/sway/xdg_decoration.c +++ b/sway/xdg_decoration.c @@ -12,31 +12,22 @@ static void xdg_decoration_handle_destroy(struct wl_listener *listener, wl_container_of(listener, deco, destroy); deco->view->xdg_decoration = NULL; wl_list_remove(&deco->destroy.link); - wl_list_remove(&deco->surface_commit.link); + wl_list_remove(&deco->request_mode.link); wl_list_remove(&deco->link); free(deco); } -static void xdg_decoration_handle_surface_commit(struct wl_listener *listener, +static void xdg_decoration_handle_request_mode(struct wl_listener *listener, void *data) { - struct sway_xdg_decoration *decoration = - wl_container_of(listener, decoration, surface_commit); - - bool csd = decoration->wlr_xdg_decoration->current_mode == - WLR_XDG_TOPLEVEL_DECORATION_V1_MODE_CLIENT_SIDE; - struct sway_view *view = decoration->view; - - view_update_csd_from_client(view, csd); - - arrange_container(view->container); - transaction_commit_dirty(); + struct sway_xdg_decoration *deco = + wl_container_of(listener, deco, request_mode); + wlr_xdg_toplevel_decoration_v1_set_mode(deco->wlr_xdg_decoration, + WLR_XDG_TOPLEVEL_DECORATION_V1_MODE_SERVER_SIDE); } void handle_xdg_decoration(struct wl_listener *listener, void *data) { struct wlr_xdg_toplevel_decoration_v1 *wlr_deco = data; struct sway_xdg_shell_view *xdg_shell_view = wlr_deco->surface->data; - struct wlr_xdg_surface *wlr_xdg_surface = - xdg_shell_view->view.wlr_xdg_surface; struct sway_xdg_decoration *deco = calloc(1, sizeof(*deco)); if (deco == NULL) { @@ -50,13 +41,8 @@ void handle_xdg_decoration(struct wl_listener *listener, void *data) { wl_signal_add(&wlr_deco->events.destroy, &deco->destroy); deco->destroy.notify = xdg_decoration_handle_destroy; - // Note: We don't listen to the request_mode signal here, effectively - // ignoring any modes the client asks to set. The client can still force a - // mode upon us, in which case we get upset but live with it. - - deco->surface_commit.notify = xdg_decoration_handle_surface_commit; - wl_signal_add(&wlr_xdg_surface->surface->events.commit, - &deco->surface_commit); + wl_signal_add(&wlr_deco->events.request_mode, &deco->request_mode); + deco->request_mode.notify = xdg_decoration_handle_request_mode; wl_list_insert(&server.xdg_decorations, &deco->link); }