From 5c98d1a04a1439bf40c6e516086cfaff2d67f135 Mon Sep 17 00:00:00 2001 From: Kirill Primak Date: Wed, 14 Aug 2024 18:25:03 +0300 Subject: [PATCH] xdg-surface: fix window geometry handling It was completely wrong: according to the protocol, the effective geometry is only updated on commit time if there pending state has new state from xdg_surface.set_window_geometry or xdg_surface.set_window_geometry has never been sent at all. This commit adds wlr_xdg_surface.geometry which correctly matches the effective window geometry and removes now-useless wlr_xdg_surface_get_geometry(). --- include/wlr/types/wlr_xdg_shell.h | 22 ++++++------ tinywl/tinywl.c | 18 +++++----- types/scene/xdg_shell.c | 4 +-- types/wlr_layer_shell_v1.c | 8 ++--- types/xdg_shell/wlr_xdg_popup.c | 4 +-- types/xdg_shell/wlr_xdg_surface.c | 58 ++++++++++++++++++++----------- 6 files changed, 62 insertions(+), 52 deletions(-) diff --git a/include/wlr/types/wlr_xdg_shell.h b/include/wlr/types/wlr_xdg_shell.h index d54c5366b..940f35ec6 100644 --- a/include/wlr/types/wlr_xdg_shell.h +++ b/include/wlr/types/wlr_xdg_shell.h @@ -228,9 +228,16 @@ struct wlr_xdg_surface_configure { }; }; +enum wlr_xdg_surface_state_field { + WLR_XDG_SURFACE_STATE_WINDOW_GEOMETRY = 1 << 0, +}; + struct wlr_xdg_surface_state { - uint32_t configure_serial; + uint32_t committed; // enum wlr_xdg_surface_state_field + struct wlr_box geometry; + + uint32_t configure_serial; }; /** @@ -274,6 +281,8 @@ struct wlr_xdg_surface { // Whether the latest commit is an initial commit bool initial_commit; + struct wlr_box geometry; + struct { struct wl_signal destroy; struct wl_signal ping_timeout; @@ -523,17 +532,6 @@ struct wlr_xdg_toplevel *wlr_xdg_toplevel_try_from_wlr_surface(struct wlr_surfac */ struct wlr_xdg_popup *wlr_xdg_popup_try_from_wlr_surface(struct wlr_surface *surface); -/** - * Get the surface geometry. - * - * This is either the geometry as set by the client, or defaulted to the bounds - * of the surface + the subsurfaces (as specified by the protocol). - * - * The x and y value can be < 0. - */ -void wlr_xdg_surface_get_geometry(struct wlr_xdg_surface *surface, - struct wlr_box *box); - /** * Call `iterator` on each mapped surface and popup in the xdg-surface tree * (whether or not this xdg-surface is mapped), with the surface's position diff --git a/tinywl/tinywl.c b/tinywl/tinywl.c index 6c043ed51..edc6269b3 100644 --- a/tinywl/tinywl.c +++ b/tinywl/tinywl.c @@ -427,10 +427,9 @@ static void process_cursor_resize(struct tinywl_server *server, uint32_t time) { } } - struct wlr_box geo_box; - wlr_xdg_surface_get_geometry(toplevel->xdg_toplevel->base, &geo_box); + struct wlr_box *geo_box = &toplevel->xdg_toplevel->base->geometry; wlr_scene_node_set_position(&toplevel->scene_tree->node, - new_left - geo_box.x, new_top - geo_box.y); + new_left - geo_box->x, new_top - geo_box->y); int new_width = new_right - new_left; int new_height = new_bottom - new_top; @@ -727,17 +726,16 @@ static void begin_interactive(struct tinywl_toplevel *toplevel, server->grab_x = server->cursor->x - toplevel->scene_tree->node.x; server->grab_y = server->cursor->y - toplevel->scene_tree->node.y; } else { - struct wlr_box geo_box; - wlr_xdg_surface_get_geometry(toplevel->xdg_toplevel->base, &geo_box); + struct wlr_box *geo_box = &toplevel->xdg_toplevel->base->geometry; - double border_x = (toplevel->scene_tree->node.x + geo_box.x) + - ((edges & WLR_EDGE_RIGHT) ? geo_box.width : 0); - double border_y = (toplevel->scene_tree->node.y + geo_box.y) + - ((edges & WLR_EDGE_BOTTOM) ? geo_box.height : 0); + double border_x = (toplevel->scene_tree->node.x + geo_box->x) + + ((edges & WLR_EDGE_RIGHT) ? geo_box->width : 0); + double border_y = (toplevel->scene_tree->node.y + geo_box->y) + + ((edges & WLR_EDGE_BOTTOM) ? geo_box->height : 0); server->grab_x = server->cursor->x - border_x; server->grab_y = server->cursor->y - border_y; - server->grab_geobox = geo_box; + server->grab_geobox = *geo_box; server->grab_geobox.x += toplevel->scene_tree->node.x; server->grab_geobox.y += toplevel->scene_tree->node.y; diff --git a/types/scene/xdg_shell.c b/types/scene/xdg_shell.c index ed792633d..73d0acb79 100644 --- a/types/scene/xdg_shell.c +++ b/types/scene/xdg_shell.c @@ -34,10 +34,8 @@ static void scene_xdg_surface_update_position( struct wlr_scene_xdg_surface *scene_xdg_surface) { struct wlr_xdg_surface *xdg_surface = scene_xdg_surface->xdg_surface; - struct wlr_box geo = {0}; - wlr_xdg_surface_get_geometry(xdg_surface, &geo); wlr_scene_node_set_position(&scene_xdg_surface->surface_tree->node, - -geo.x, -geo.y); + -xdg_surface->geometry.x, -xdg_surface->geometry.y); if (xdg_surface->role == WLR_XDG_SURFACE_ROLE_POPUP) { struct wlr_xdg_popup *popup = xdg_surface->popup; diff --git a/types/wlr_layer_shell_v1.c b/types/wlr_layer_shell_v1.c index a59f1104a..2275ed065 100644 --- a/types/wlr_layer_shell_v1.c +++ b/types/wlr_layer_shell_v1.c @@ -574,8 +574,8 @@ void wlr_layer_surface_v1_for_each_popup_surface(struct wlr_layer_surface_v1 *su } double popup_sx, popup_sy; - popup_sx = popup->current.geometry.x - popup->base->current.geometry.x; - popup_sy = popup->current.geometry.y - popup->base->current.geometry.y; + popup_sx = popup->current.geometry.x - popup->base->geometry.x; + popup_sy = popup->current.geometry.y - popup->base->geometry.y; struct layer_surface_iterator_data data = { .user_iterator = iterator, @@ -609,8 +609,8 @@ struct wlr_surface *wlr_layer_surface_v1_popup_surface_at( } double popup_sx, popup_sy; - popup_sx = popup->current.geometry.x - popup->base->current.geometry.x; - popup_sy = popup->current.geometry.y - popup->base->current.geometry.y; + popup_sx = popup->current.geometry.x - popup->base->geometry.x; + popup_sy = popup->current.geometry.y - popup->base->geometry.y; struct wlr_surface *sub = wlr_xdg_surface_surface_at( popup->base, sx - popup_sx, sy - popup_sy, sub_x, sub_y); diff --git a/types/xdg_shell/wlr_xdg_popup.c b/types/xdg_shell/wlr_xdg_popup.c index a718b4528..df1e75de3 100644 --- a/types/xdg_shell/wlr_xdg_popup.c +++ b/types/xdg_shell/wlr_xdg_popup.c @@ -498,8 +498,8 @@ void wlr_xdg_popup_get_toplevel_coords(struct wlr_xdg_popup *popup, popup_sy += xdg_surface->popup->current.geometry.y; parent = xdg_surface->popup->parent; } else { - popup_sx += xdg_surface->current.geometry.x; - popup_sy += xdg_surface->current.geometry.y; + popup_sx += xdg_surface->geometry.x; + popup_sy += xdg_surface->geometry.y; break; } } diff --git a/types/xdg_shell/wlr_xdg_surface.c b/types/xdg_shell/wlr_xdg_surface.c index 9842a563c..97daa0d30 100644 --- a/types/xdg_shell/wlr_xdg_surface.c +++ b/types/xdg_shell/wlr_xdg_surface.c @@ -219,12 +219,12 @@ static void xdg_surface_handle_set_window_geometry(struct wl_client *client, } if (width <= 0 || height <= 0) { - wl_resource_post_error(resource, - XDG_SURFACE_ERROR_INVALID_SIZE, + wl_resource_post_error(resource, XDG_SURFACE_ERROR_INVALID_SIZE, "Tried to set invalid xdg-surface geometry"); return; } + surface->pending.committed |= WLR_XDG_SURFACE_STATE_WINDOW_GEOMETRY; surface->pending.geometry.x = x; surface->pending.geometry.y = y; surface->pending.geometry.width = width; @@ -256,6 +256,21 @@ static const struct xdg_surface_interface xdg_surface_implementation = { .set_window_geometry = xdg_surface_handle_set_window_geometry, }; +// The window geometry is updated on commit, unless the commit is going to map +// the surface, in which case it's updated on map, so that subsurfaces are +// mapped and surface extents are computed correctly. +static void update_geometry(struct wlr_xdg_surface *surface) { + if (!wlr_box_empty(&surface->current.geometry)) { + if ((surface->current.committed & WLR_XDG_SURFACE_STATE_WINDOW_GEOMETRY) != 0) { + wlr_surface_get_extents(surface->surface, &surface->geometry); + wlr_box_intersection(&surface->geometry, + &surface->current.geometry, &surface->geometry); + } + } else { + wlr_surface_get_extents(surface->surface, &surface->geometry); + } +} + static void xdg_surface_role_client_commit(struct wlr_surface *wlr_surface) { struct wlr_xdg_surface *surface = wlr_xdg_surface_try_from_wlr_surface(wlr_surface); assert(surface != NULL); @@ -320,11 +335,20 @@ static void xdg_surface_role_commit(struct wlr_surface *wlr_surface) { break; } - if (wlr_surface_has_buffer(wlr_surface)) { + if (wlr_surface->mapped) { + update_geometry(surface); + } else if (wlr_surface_has_buffer(wlr_surface)) { wlr_surface_map(wlr_surface); } } +static void xdg_surface_role_map(struct wlr_surface *wlr_surface) { + struct wlr_xdg_surface *surface = wlr_xdg_surface_try_from_wlr_surface(wlr_surface); + assert(surface != NULL); + + update_geometry(surface); +} + static void xdg_surface_role_destroy(struct wlr_surface *wlr_surface) { struct wlr_xdg_surface *surface = wlr_xdg_surface_try_from_wlr_surface(wlr_surface); if (surface == NULL) { @@ -339,11 +363,19 @@ static const struct wlr_surface_role xdg_surface_role = { .name = "xdg_surface", .client_commit = xdg_surface_role_client_commit, .commit = xdg_surface_role_commit, + .map = xdg_surface_role_map, .destroy = xdg_surface_role_destroy, }; +static void surface_synced_move_state(void *_dst, void *_src) { + struct wlr_xdg_surface_state *dst = _dst, *src = _src; + *dst = *src; + src->committed = 0; +} + static const struct wlr_surface_synced_impl surface_synced_impl = { .state_size = sizeof(struct wlr_xdg_surface_state), + .move_state = surface_synced_move_state, }; struct wlr_xdg_surface *wlr_xdg_surface_try_from_wlr_surface( @@ -522,12 +554,8 @@ void wlr_xdg_popup_get_position(struct wlr_xdg_popup *popup, double *popup_sx, double *popup_sy) { struct wlr_xdg_surface *parent = wlr_xdg_surface_try_from_wlr_surface(popup->parent); assert(parent != NULL); - struct wlr_box parent_geo; - wlr_xdg_surface_get_geometry(parent, &parent_geo); - *popup_sx = parent_geo.x + popup->current.geometry.x - - popup->base->current.geometry.x; - *popup_sy = parent_geo.y + popup->current.geometry.y - - popup->base->current.geometry.y; + *popup_sx = parent->geometry.x + popup->current.geometry.x - popup->base->geometry.x; + *popup_sy = parent->geometry.y + popup->current.geometry.y - popup->base->geometry.y; } struct wlr_surface *wlr_xdg_surface_surface_at( @@ -611,15 +639,3 @@ void wlr_xdg_surface_for_each_popup_surface(struct wlr_xdg_surface *surface, wlr_surface_iterator_func_t iterator, void *user_data) { xdg_surface_for_each_popup_surface(surface, 0, 0, iterator, user_data); } - -void wlr_xdg_surface_get_geometry(struct wlr_xdg_surface *surface, - struct wlr_box *box) { - wlr_surface_get_extents(surface->surface, box); - - /* The client never set the geometry */ - if (wlr_box_empty(&surface->current.geometry)) { - return; - } - - wlr_box_intersection(box, &surface->current.geometry, box); -}