From b9ff342bcc34f2266f5eb60e293ac14b9c95c3ff Mon Sep 17 00:00:00 2001 From: Tudor Brindus Date: Sun, 18 Oct 2020 20:31:46 -0400 Subject: [PATCH] transaction: stop matching X transactions by geometry JetBrains IDEs really keep providing the most pathological cases for Xwayland... The "Search everywhere" dialog can be resized, but it responds to a configure request with a configure notify including double the growth in width. So, if the user drags the left edge by 5px, we send Xwayland a configure request based off the current geometry, -5px on the x axis, and +5px width. JetBrains IDEs then respond with -5px on the x axis, and +10px width. Presumably they do this to keep the dialog symmetric. Still, it breaks Sway because we match commits to transactions by geometry, so any resizes of the "Search everywhere" dialog will cause Sway's transactions to time out. Instead, we can just assume that the first commit following a transaction is in response to that transaction. This is obviously race-y, but I don't think we can do anything particularly smarter in the face of crazy applications. --- include/sway/desktop/transaction.h | 9 ----- sway/desktop/transaction.c | 13 ------ sway/desktop/xwayland.c | 64 ++++++++++++++++-------------- 3 files changed, 34 insertions(+), 52 deletions(-) diff --git a/include/sway/desktop/transaction.h b/include/sway/desktop/transaction.h index 175489c57..e7bd8c1be 100644 --- a/include/sway/desktop/transaction.h +++ b/include/sway/desktop/transaction.h @@ -36,15 +36,6 @@ void transaction_commit_dirty(void); void transaction_notify_view_ready_by_serial(struct sway_view *view, uint32_t serial); -/** - * Notify the transaction system that a view is ready for the new layout, but - * identifying the instruction by geometry rather than by serial. - * - * This is used by xwayland views, as they don't have serials. - */ -void transaction_notify_view_ready_by_geometry(struct sway_view *view, - double x, double y, int width, int height); - /** * Unconditionally notify the transaction system that a view is ready for the * new layout. diff --git a/sway/desktop/transaction.c b/sway/desktop/transaction.c index eac389917..196f83274 100644 --- a/sway/desktop/transaction.c +++ b/sway/desktop/transaction.c @@ -519,19 +519,6 @@ void transaction_notify_view_ready_by_serial(struct sway_view *view, } } -void transaction_notify_view_ready_by_geometry(struct sway_view *view, - double x, double y, int width, int height) { - struct sway_transaction_instruction *instruction = - view->container->node.instruction; - if (instruction != NULL && - (int)instruction->container_state.content_x == (int)x && - (int)instruction->container_state.content_y == (int)y && - instruction->container_state.content_width == width && - instruction->container_state.content_height == height) { - set_instruction_ready(instruction); - } -} - void transaction_notify_view_ready_immediately(struct sway_view *view) { struct sway_transaction_instruction *instruction = view->container->node.instruction; diff --git a/sway/desktop/xwayland.c b/sway/desktop/xwayland.c index 186502b2d..2bbb0167d 100644 --- a/sway/desktop/xwayland.c +++ b/sway/desktop/xwayland.c @@ -56,15 +56,12 @@ static void unmanaged_handle_set_geometry(struct wl_listener *listener, void *da wl_container_of(listener, surface, set_geometry); struct wlr_xwayland_surface *xsurface = surface->wlr_xwayland_surface; - if (xsurface->x != surface->lx || xsurface->y != surface->ly) { - // Surface has moved - desktop_damage_surface(xsurface->surface, surface->lx, surface->ly, - true); - surface->lx = xsurface->x; - surface->ly = xsurface->y; - desktop_damage_surface(xsurface->surface, surface->lx, surface->ly, - true); - } + desktop_damage_surface(xsurface->surface, surface->lx, surface->ly, + true); + surface->lx = xsurface->x; + surface->ly = xsurface->y; + desktop_damage_surface(xsurface->surface, surface->lx, surface->ly, + true); } static void unmanaged_handle_map(struct wl_listener *listener, void *data) { @@ -396,33 +393,40 @@ static void handle_commit(struct wl_listener *listener, void *data) { struct sway_xwayland_view *xwayland_view = wl_container_of(listener, xwayland_view, commit); struct sway_view *view = &xwayland_view->view; - struct wlr_xwayland_surface *xsurface = view->wlr_xwayland_surface; - struct wlr_surface_state *state = &xsurface->surface->current; - if (view->container->node.instruction) { - get_geometry(view, &view->geometry); - transaction_notify_view_ready_by_geometry(view, - xsurface->x, xsurface->y, state->width, state->height); - } else { - struct wlr_box new_geo; - get_geometry(view, &new_geo); + struct wlr_box new_geo; + get_geometry(view, &new_geo); + if ((new_geo.width != view->geometry.width || + new_geo.height != view->geometry.height || + new_geo.x != view->geometry.x || + new_geo.y != view->geometry.y)) { + desktop_damage_view(view); + view_update_size(view, new_geo.width, new_geo.height); + memcpy(&view->geometry, &new_geo, sizeof(struct wlr_box)); + desktop_damage_view(view); - if ((new_geo.width != view->geometry.width || - new_geo.height != view->geometry.height || - new_geo.x != view->geometry.x || - new_geo.y != view->geometry.y)) { + if (!view->container->node.instruction) { // The view has unexpectedly sent a new size // eg. The Firefox "Save As" dialog when downloading a file - desktop_damage_view(view); - view_update_size(view, new_geo.width, new_geo.height); - memcpy(&view->geometry, &new_geo, sizeof(struct wlr_box)); - desktop_damage_view(view); transaction_commit_dirty(); - transaction_notify_view_ready_by_geometry(view, - xsurface->x, xsurface->y, new_geo.width, new_geo.height); - } else { - memcpy(&view->geometry, &new_geo, sizeof(struct wlr_box)); } + } else { + memcpy(&view->geometry, &new_geo, sizeof(struct wlr_box)); + } + + // We have no serial to match commits with transactions. We could naively + // match based on geometry, but this breaks for applications that respond + // to configure requests with different geometries than requested. For + // example, JetBrains IDEs do this when resizing the "Search everywhere" + // dialog, since they resize both edges at the same time to keep the dialog + // symmetric (even though Sway requested a single edge to be resized). + // + // Instead, just assume that the first commit following a transaction is in + // response to that transaction. This is obviously racey. (N.B. we can also + // enter this branch if we just started a transaction above, but that case + // is not interesting.) + if (view->container->node.instruction) { + transaction_notify_view_ready_immediately(view); } view_damage_from(view);