From 9e58301df7f09660cb36337211cbf5700d99810c Mon Sep 17 00:00:00 2001 From: Simon Ser Date: Tue, 1 Jun 2021 12:18:53 +0200 Subject: [PATCH] surface: allow placing subsurfaces below parent Prior to this commit, subsurfaces could only be placed above their parent. Any place_{above,below} request involving the parent would fail with a protocol error. However the Wayland protocol allows using the parent surface in the place_{above,below} requests, and allows subsurfaces to be placed below their parent. Weston's implementation adds a dummy wl_list node in the subsurface list. However this is potentially dangerous: iterating the list requires making sure the dummy wl_list node is checked for, otherwise memory corruption will happen. Instead, split the list in two: one for subsurfaces above the parent, the other for subsurfaces below. Tested with wleird's subsurfaces demo client. Closes: https://github.com/swaywm/wlroots/issues/1865 --- include/wlr/types/wlr_surface.h | 9 ++- types/wlr_surface.c | 135 +++++++++++++++++++++++--------- 2 files changed, 104 insertions(+), 40 deletions(-) diff --git a/include/wlr/types/wlr_surface.h b/include/wlr/types/wlr_surface.h index d99c182a..00db6cd8 100644 --- a/include/wlr/types/wlr_surface.h +++ b/include/wlr/types/wlr_surface.h @@ -139,10 +139,13 @@ struct wlr_surface { struct wl_signal destroy; } events; - struct wl_list subsurfaces; // wlr_subsurface::parent_link + // wlr_subsurface.parent_link + struct wl_list subsurfaces_below; + struct wl_list subsurfaces_above; - // wlr_subsurface::parent_pending_link - struct wl_list subsurface_pending_list; + // wlr_subsurface.parent_pending_link + struct wl_list subsurfaces_pending_below; + struct wl_list subsurfaces_pending_above; struct wl_list current_outputs; // wlr_surface_output::link diff --git a/types/wlr_surface.c b/types/wlr_surface.c index 78879550..54a81489 100644 --- a/types/wlr_surface.c +++ b/types/wlr_surface.c @@ -341,7 +341,10 @@ static void surface_damage_subsurfaces(struct wlr_subsurface *subsurface) { subsurface->reordered = false; struct wlr_subsurface *child; - wl_list_for_each(child, &subsurface->surface->subsurfaces, parent_link) { + wl_list_for_each(child, &subsurface->surface->subsurfaces_below, parent_link) { + surface_damage_subsurfaces(child); + } + wl_list_for_each(child, &subsurface->surface->subsurfaces_above, parent_link) { surface_damage_subsurfaces(child); } } @@ -440,10 +443,20 @@ static void surface_commit_state(struct wlr_surface *surface, // commit subsurface order struct wlr_subsurface *subsurface; - wl_list_for_each_reverse(subsurface, &surface->subsurface_pending_list, + wl_list_for_each_reverse(subsurface, &surface->subsurfaces_pending_above, + parent_pending_link) { + wl_list_remove(&subsurface->parent_link); + wl_list_insert(&surface->subsurfaces_above, &subsurface->parent_link); + + if (subsurface->reordered) { + // TODO: damage all the subsurfaces + surface_damage_subsurfaces(subsurface); + } + } + wl_list_for_each_reverse(subsurface, &surface->subsurfaces_pending_below, parent_pending_link) { wl_list_remove(&subsurface->parent_link); - wl_list_insert(&surface->subsurfaces, &subsurface->parent_link); + wl_list_insert(&surface->subsurfaces_below, &subsurface->parent_link); if (subsurface->reordered) { // TODO: damage all the subsurfaces @@ -509,7 +522,10 @@ static void subsurface_parent_commit(struct wlr_subsurface *subsurface, } struct wlr_subsurface *subsurface; - wl_list_for_each(subsurface, &surface->subsurfaces, parent_link) { + wl_list_for_each(subsurface, &surface->subsurfaces_below, parent_link) { + subsurface_parent_commit(subsurface, true); + } + wl_list_for_each(subsurface, &surface->subsurfaces_above, parent_link) { subsurface_parent_commit(subsurface, true); } } @@ -541,7 +557,10 @@ static void surface_commit(struct wl_client *client, surface_commit_pending(surface); - wl_list_for_each(subsurface, &surface->subsurfaces, parent_link) { + wl_list_for_each(subsurface, &surface->subsurfaces_below, parent_link) { + subsurface_parent_commit(subsurface, false); + } + wl_list_for_each(subsurface, &surface->subsurfaces_above, parent_link) { subsurface_parent_commit(subsurface, false); } } @@ -732,8 +751,10 @@ struct wlr_surface *surface_create(struct wl_client *client, wl_signal_init(&surface->events.commit); wl_signal_init(&surface->events.destroy); wl_signal_init(&surface->events.new_subsurface); - wl_list_init(&surface->subsurfaces); - wl_list_init(&surface->subsurface_pending_list); + wl_list_init(&surface->subsurfaces_above); + wl_list_init(&surface->subsurfaces_below); + wl_list_init(&surface->subsurfaces_pending_above); + wl_list_init(&surface->subsurfaces_pending_below); wl_list_init(&surface->current_outputs); wl_list_init(&surface->cached); pixman_region32_init(&surface->buffer_damage); @@ -867,7 +888,12 @@ static struct wlr_subsurface *subsurface_find_sibling( struct wlr_surface *parent = subsurface->parent; struct wlr_subsurface *sibling; - wl_list_for_each(sibling, &parent->subsurfaces, parent_link) { + wl_list_for_each(sibling, &parent->subsurfaces_below, parent_link) { + if (sibling->surface == surface && sibling != subsurface) { + return sibling; + } + } + wl_list_for_each(sibling, &parent->subsurfaces_above, parent_link) { if (sibling->surface == surface && sibling != subsurface) { return sibling; } @@ -885,20 +911,25 @@ static void subsurface_handle_place_above(struct wl_client *client, struct wlr_surface *sibling_surface = wlr_surface_from_resource(sibling_resource); - struct wlr_subsurface *sibling = - subsurface_find_sibling(subsurface, sibling_surface); - - if (!sibling) { - wl_resource_post_error(subsurface->resource, - WL_SUBSURFACE_ERROR_BAD_SURFACE, - "%s: wl_surface@%" PRIu32 "is not a parent or sibling", - "place_above", wl_resource_get_id(sibling_surface->resource)); - return; + + struct wl_list *node; + if (sibling_surface == subsurface->parent) { + node = &subsurface->parent->subsurfaces_pending_above; + } else { + struct wlr_subsurface *sibling = + subsurface_find_sibling(subsurface, sibling_surface); + if (!sibling) { + wl_resource_post_error(subsurface->resource, + WL_SUBSURFACE_ERROR_BAD_SURFACE, + "%s: wl_surface@%" PRIu32 "is not a parent or sibling", + "place_above", wl_resource_get_id(sibling_resource)); + return; + } + node = &sibling->parent_pending_link; } wl_list_remove(&subsurface->parent_pending_link); - wl_list_insert(&sibling->parent_pending_link, - &subsurface->parent_pending_link); + wl_list_insert(node, &subsurface->parent_pending_link); subsurface->reordered = true; } @@ -912,20 +943,25 @@ static void subsurface_handle_place_below(struct wl_client *client, struct wlr_surface *sibling_surface = wlr_surface_from_resource(sibling_resource); - struct wlr_subsurface *sibling = - subsurface_find_sibling(subsurface, sibling_surface); - - if (!sibling) { - wl_resource_post_error(subsurface->resource, - WL_SUBSURFACE_ERROR_BAD_SURFACE, - "%s: wl_surface@%" PRIu32 " is not a parent or sibling", - "place_below", wl_resource_get_id(sibling_surface->resource)); - return; + + struct wl_list *node; + if (sibling_surface == subsurface->parent) { + node = subsurface->parent->subsurfaces_pending_below.prev; + } else { + struct wlr_subsurface *sibling = + subsurface_find_sibling(subsurface, sibling_surface); + if (!sibling) { + wl_resource_post_error(subsurface->resource, + WL_SUBSURFACE_ERROR_BAD_SURFACE, + "%s: wl_surface@%" PRIu32 " is not a parent or sibling", + "place_below", wl_resource_get_id(sibling_resource)); + return; + } + node = sibling->parent_pending_link.prev; } wl_list_remove(&subsurface->parent_pending_link); - wl_list_insert(sibling->parent_pending_link.prev, - &subsurface->parent_pending_link); + wl_list_insert(node, &subsurface->parent_pending_link); subsurface->reordered = true; } @@ -1002,7 +1038,10 @@ static void subsurface_consider_map(struct wlr_subsurface *subsurface, // Try mapping all children too struct wlr_subsurface *child; - wl_list_for_each(child, &subsurface->surface->subsurfaces, parent_link) { + wl_list_for_each(child, &subsurface->surface->subsurfaces_below, parent_link) { + subsurface_consider_map(child, false); + } + wl_list_for_each(child, &subsurface->surface->subsurfaces_above, parent_link) { subsurface_consider_map(child, false); } } @@ -1017,7 +1056,10 @@ static void subsurface_unmap(struct wlr_subsurface *subsurface) { // Unmap all children struct wlr_subsurface *child; - wl_list_for_each(child, &subsurface->surface->subsurfaces, parent_link) { + wl_list_for_each(child, &subsurface->surface->subsurfaces_below, parent_link) { + subsurface_unmap(child); + } + wl_list_for_each(child, &subsurface->surface->subsurfaces_above, parent_link) { subsurface_unmap(child); } } @@ -1131,8 +1173,8 @@ struct wlr_subsurface *wlr_subsurface_create(struct wlr_surface *surface, subsurface->parent = parent; wl_signal_add(&parent->events.destroy, &subsurface->parent_destroy); subsurface->parent_destroy.notify = subsurface_handle_parent_destroy; - wl_list_insert(parent->subsurfaces.prev, &subsurface->parent_link); - wl_list_insert(parent->subsurface_pending_list.prev, + wl_list_insert(parent->subsurfaces_above.prev, &subsurface->parent_link); + wl_list_insert(parent->subsurfaces_pending_above.prev, &subsurface->parent_pending_link); surface->role_data = subsurface; @@ -1175,7 +1217,7 @@ bool wlr_surface_point_accepts_input(struct wlr_surface *surface, struct wlr_surface *wlr_surface_surface_at(struct wlr_surface *surface, double sx, double sy, double *sub_x, double *sub_y) { struct wlr_subsurface *subsurface; - wl_list_for_each_reverse(subsurface, &surface->subsurfaces, parent_link) { + wl_list_for_each_reverse(subsurface, &surface->subsurfaces_above, parent_link) { double _sub_x = subsurface->current.x; double _sub_y = subsurface->current.y; struct wlr_surface *sub = wlr_surface_surface_at(subsurface->surface, @@ -1195,6 +1237,16 @@ struct wlr_surface *wlr_surface_surface_at(struct wlr_surface *surface, return surface; } + wl_list_for_each_reverse(subsurface, &surface->subsurfaces_below, parent_link) { + double _sub_x = subsurface->current.x; + double _sub_y = subsurface->current.y; + struct wlr_surface *sub = wlr_surface_surface_at(subsurface->surface, + sx - _sub_x, sy - _sub_y, sub_x, sub_y); + if (sub != NULL) { + return sub; + } + } + return NULL; } @@ -1290,10 +1342,19 @@ void wlr_surface_send_frame_done(struct wlr_surface *surface, static void surface_for_each_surface(struct wlr_surface *surface, int x, int y, wlr_surface_iterator_func_t iterator, void *user_data) { + struct wlr_subsurface *subsurface; + wl_list_for_each(subsurface, &surface->subsurfaces_below, parent_link) { + struct wlr_subsurface_state *state = &subsurface->current; + int sx = state->x; + int sy = state->y; + + surface_for_each_surface(subsurface->surface, x + sx, y + sy, + iterator, user_data); + } + iterator(surface, x, y, user_data); - struct wlr_subsurface *subsurface; - wl_list_for_each(subsurface, &surface->subsurfaces, parent_link) { + wl_list_for_each(subsurface, &surface->subsurfaces_above, parent_link) { struct wlr_subsurface_state *state = &subsurface->current; int sx = state->x; int sy = state->y;