[PATCH v4 0/1] MR11178: winewayland: Implement xdg-popup for unmanaged windows.
This patch significantly improves compositor behavior across various compositors, as popups are guaranteed not to get clipped at the window boundary. In addition, it enables server-side decorations to be implemented on the winewayland driver. To achieve this, I attempted to reuse as much of the exsisting xdg-toplevel code as possible. -- v4: winewayland: Implement xdg-popup for unmanaged windows. https://gitlab.winehq.org/wine/wine/-/merge_requests/11178
From: Etaash Mathamsetty <etaash.mathamsetty@gmail.com> --- dlls/winewayland.drv/wayland.c | 8 +- dlls/winewayland.drv/wayland_surface.c | 235 ++++++++++++++++++++++++- dlls/winewayland.drv/waylanddrv.h | 25 ++- dlls/winewayland.drv/window.c | 43 ++++- 4 files changed, 290 insertions(+), 21 deletions(-) diff --git a/dlls/winewayland.drv/wayland.c b/dlls/winewayland.drv/wayland.c index 830fef51a01..a7b302e0872 100644 --- a/dlls/winewayland.drv/wayland.c +++ b/dlls/winewayland.drv/wayland.c @@ -120,11 +120,9 @@ static void registry_handle_global(void *data, struct wl_registry *registry, } else if (strcmp(interface, "xdg_wm_base") == 0) { - /* Bind version 2 so that compositors (e.g., sway) can properly send tiled - * states, instead of falling back to (ab)using the maximized state. */ - process_wayland.xdg_wm_base = - wl_registry_bind(registry, id, &xdg_wm_base_interface, - version < 2 ? version : 2); + /* version 3 is required for xdg_popup::reposition */ + if (version < 3) return; + process_wayland.xdg_wm_base = wl_registry_bind(registry, id, &xdg_wm_base_interface, 3); xdg_wm_base_add_listener(process_wayland.xdg_wm_base, &xdg_wm_base_listener, NULL); } else if (strcmp(interface, "wl_shm") == 0) diff --git a/dlls/winewayland.drv/wayland_surface.c b/dlls/winewayland.drv/wayland_surface.c index b28072331d6..eedc6791e8d 100644 --- a/dlls/winewayland.drv/wayland_surface.c +++ b/dlls/winewayland.drv/wayland_surface.c @@ -48,8 +48,13 @@ static void xdg_surface_handle_configure(void *private, struct xdg_surface *xdg_ /* Handle this event only if wayland_surface is still associated with * the target xdg_surface. */ - if ((surface = data->wayland_surface) && wayland_surface_is_toplevel(surface) && - surface->xdg_surface == xdg_surface) + if (!(surface = data->wayland_surface) || surface->xdg_surface != xdg_surface) + { + wayland_win_data_release(data); + return; + } + + if (wayland_surface_is_toplevel(surface)) { /* If we have a previously requested config, we have already sent a * WM_WAYLAND_CONFIGURE which hasn't been handled yet. In that case, @@ -60,6 +65,18 @@ static void xdg_surface_handle_configure(void *private, struct xdg_surface *xdg_ surface->requested = surface->pending; memset(&surface->pending, 0, sizeof(surface->pending)); } + else if (wayland_surface_is_popup(surface)) + { + /* expose the surface to ensure that the new config is ack-ed + * and the popup can move if needed */ + initial_configure = surface->current.serial == 0 || + wayland_surface_popup_config_changed(&surface->current, + &surface->pending); + surface->pending.serial = serial; + surface->processing = surface->pending; + surface->processing.processed = 1; + memset(&surface->pending, 0, sizeof(surface->pending)); + } wayland_win_data_release(data); @@ -138,11 +155,88 @@ static const struct xdg_toplevel_listener xdg_toplevel_listener = xdg_toplevel_handle_close }; +static void xdg_popup_handle_configure(void *private, struct xdg_popup *xdg_popup, + int32_t x, int32_t y, int32_t width, int32_t height) +{ + HWND hwnd = private; + struct wayland_win_data *data; + struct wayland_surface *surface; + + TRACE("hwnd=%p (%d,%d) %dx%d\n", hwnd, x, y, width, height); + + if (!(data = wayland_win_data_get(hwnd))) return; + + if ((surface = data->wayland_surface) && wayland_surface_is_popup(surface)) + { + surface->pending.x = x; + surface->pending.y = y; + surface->pending.width = width; + surface->pending.height = height; + surface->pending.state = 0; + } + + wayland_win_data_release(data); +} + +static void xdg_popup_handle_done(void *private, struct xdg_popup *xdg_popup) +{ + struct wayland_win_data *data, *owner_data; + struct wayland_surface *surface = NULL, *owner_surface = NULL; + HWND hwnd = private; + + if (!xdg_popup) return; + + /* Recreate the popup if the compositor dismissed it for some reason. + * The protocol does not explicitly prohibit this from occuring on ungrabbed popups. */ + WARN("Compositor dismissed popup hwnd=%p\n", private); + + if (!(data = wayland_win_data_get(hwnd)) || !(surface = data->wayland_surface)) + { + ERR("Failed to recreate popup!\n"); + xdg_popup_destroy(xdg_popup); + goto done; + } + + /* this is a stale event */ + if (!wayland_surface_is_popup(surface)) goto done; + if (surface->xdg_popup != xdg_popup) goto done; + + /* the protocol requires us to destroy the xdg_popup */ + wayland_surface_clear_role(surface); + + if (!(owner_data = wayland_win_data_get_nolock(surface->owner_hwnd)) || + !(owner_surface = owner_data->wayland_surface) || !owner_surface->xdg_surface) + { + ERR("Failed to recreate popup!\n"); + goto done; + } + + wayland_surface_make_popup(surface, owner_surface); + +done: + if (data) wayland_win_data_release(data); + if (surface && owner_surface && owner_surface->xdg_surface) + NtUserExposeWindowSurface(hwnd, 0, NULL, 0); +} + +static void xdg_popup_handle_reposition(void *private, struct xdg_popup *xdg_popup, uint32_t token) +{ + /* we also get a configure event in this case */ + TRACE("hwnd=%p\n", private); +} + +static const struct xdg_popup_listener xdg_popup_listener = +{ + xdg_popup_handle_configure, + xdg_popup_handle_done, + xdg_popup_handle_reposition, +}; + void wp_fractional_scale_handle_scale(void* user_data, struct wp_fractional_scale_v1 *fractional_scale_v1, uint32_t scale_fixed) { - struct wayland_win_data *data; + struct wayland_win_data *data, *owner_data; struct wayland_client_surface *client; struct wayland_surface *surface; double scale = scale_fixed / 120.0; @@ -170,6 +264,14 @@ void wp_fractional_scale_handle_scale(void* user_data, surface->processing.processed = TRUE; } + /* the popup x,y position has changed */ + if (wayland_surface_is_popup(surface) && + (owner_data = wayland_win_data_get_nolock(surface->owner_hwnd)) && + owner_data->wayland_surface) + { + wayland_surface_make_popup(surface, owner_data->wayland_surface); + } + wayland_win_data_release(data); NtUserExposeWindowSurface(hwnd, 0, NULL, 0); @@ -413,6 +515,110 @@ err: ERR("Failed to assign subsurface role to wayland surface\n"); } +/* helper to intialize the positioner using a given surface config */ +static struct xdg_positioner *wayland_surface_create_positioner(struct wayland_surface *surface, + struct wayland_surface *owner, + struct wayland_surface_config *config) +{ + struct xdg_positioner *xdg_positioner = + xdg_wm_base_create_positioner(process_wayland.xdg_wm_base); + + if (!xdg_positioner) return NULL; + + if (!config->width) config->width = 1; + if (!config->height) config->height = 1; + + /* this anchor rect is always valid */ + xdg_positioner_set_anchor_rect(xdg_positioner, 0, 0, 1, 1); + xdg_positioner_set_anchor(xdg_positioner, XDG_POSITIONER_ANCHOR_TOP_LEFT); + xdg_positioner_set_gravity(xdg_positioner, XDG_POSITIONER_GRAVITY_BOTTOM_RIGHT); + /* then we offset by the requested amount */ + xdg_positioner_set_offset(xdg_positioner, config->x, config->y); + xdg_positioner_set_size(xdg_positioner, config->width, config->height); + + return xdg_positioner; +} + +/********************************************************************** + * wayland_surface_make_popup + * + * Gives the popup role to a plain wayland surface. + */ +void wayland_surface_make_popup(struct wayland_surface *surface, + struct wayland_surface *owner) +{ + struct wayland_surface_config config; + struct xdg_positioner *xdg_positioner = NULL; + const RECT *rect = &surface->window.rect; + + config.x = rect->left - owner->window.rect.left; + config.y = rect->top - owner->window.rect.top; + config.width = rect->right - rect->left; + config.height = rect->bottom - rect->top; + + assert(owner->xdg_surface); + assert(!surface->role || surface->role == WAYLAND_SURFACE_ROLE_POPUP); + + if (surface->xdg_popup && surface->owner_hwnd == owner->hwnd) + { + if (!surface->current.serial) return; + + wayland_surface_coords_from_window(surface, config.x, config.y, + &config.x, &config.y); + wayland_surface_coords_from_window(surface, config.width, config.height, + &config.width, &config.height); + + /* reposition the popup if needed */ + if (!wayland_surface_popup_config_changed(&surface->current, &config)) + return; + + xdg_positioner = wayland_surface_create_positioner(surface, owner, &config); + if (!xdg_positioner) + { + ERR("Failed to create positioner!\n"); + return; + } + + xdg_popup_reposition(surface->xdg_popup, xdg_positioner, 0); + xdg_positioner_destroy(xdg_positioner); + wl_surface_commit(surface->wl_surface); + wl_display_flush(process_wayland.wl_display); + return; + } + + wayland_surface_clear_role(surface); + surface->role = WAYLAND_SURFACE_ROLE_POPUP; + + wayland_surface_init_fractional_scale(surface, owner->window.scale); + + surface->xdg_surface = xdg_wm_base_get_xdg_surface(process_wayland.xdg_wm_base, + surface->wl_surface); + if (!surface->xdg_surface) goto err; + xdg_surface_add_listener(surface->xdg_surface, &xdg_surface_listener, surface->hwnd); + + wayland_surface_coords_from_window(surface, config.x, config.y, &config.x, &config.y); + wayland_surface_coords_from_window(surface, config.width, config.height, + &config.width, &config.height); + + xdg_positioner = wayland_surface_create_positioner(surface, owner, &config); + if (!xdg_positioner) goto err; + + surface->xdg_popup = xdg_surface_get_popup(surface->xdg_surface, owner->xdg_surface, + xdg_positioner); + xdg_positioner_destroy(xdg_positioner); + if (!surface->xdg_popup) goto err; + xdg_popup_add_listener(surface->xdg_popup, &xdg_popup_listener, surface->hwnd); + + surface->owner_hwnd = owner->hwnd; + wl_surface_commit(surface->wl_surface); + wl_display_flush(process_wayland.wl_display); + + return; +err: + wayland_surface_clear_role(surface); + ERR("Failed to assign popup role to wayland surface\n"); +} + /********************************************************************** * wayland_surface_clear_role * @@ -437,6 +643,23 @@ void wayland_surface_clear_role(struct wayland_surface *surface) case WAYLAND_SURFACE_ROLE_NONE: break; + case WAYLAND_SURFACE_ROLE_POPUP: + + if (surface->xdg_popup) + { + xdg_popup_destroy(surface->xdg_popup); + surface->xdg_popup = NULL; + } + + if (surface->xdg_surface) + { + xdg_surface_destroy(surface->xdg_surface); + surface->xdg_surface = NULL; + } + + surface->owner_hwnd = NULL; + break; + case WAYLAND_SURFACE_ROLE_TOPLEVEL: if (surface->xdg_toplevel_icon) { @@ -475,7 +698,6 @@ void wayland_surface_clear_role(struct wayland_surface *surface) memset(&surface->requested, 0, sizeof(surface->requested)); memset(&surface->processing, 0, sizeof(surface->processing)); memset(&surface->current, 0, sizeof(surface->current)); - surface->toplevel_hwnd = 0; /* Ensure no buffer is attached, otherwise future role assignments may fail. */ wl_surface_attach(surface->wl_surface, NULL, 0, 0); @@ -657,8 +879,8 @@ static void wayland_surface_reconfigure_geometry(struct wayland_surface *surface { int width = rect.right - rect.left, height = rect.bottom - rect.top; xdg_surface_set_window_geometry(surface->xdg_surface, - rect.left, rect.top, - width, height); + 0, 0, width, height); + if (!wayland_surface_is_toplevel(surface)) return; if (surface->window.resizeable) { xdg_toplevel_set_min_size(surface->xdg_toplevel, 0, 0); @@ -834,6 +1056,7 @@ BOOL wayland_surface_reconfigure(struct wayland_surface *surface) case WAYLAND_SURFACE_ROLE_NONE: break; case WAYLAND_SURFACE_ROLE_TOPLEVEL: + case WAYLAND_SURFACE_ROLE_POPUP: if (!surface->xdg_surface) break; /* surface role has been cleared */ if (!wayland_surface_reconfigure_xdg(surface, width, height)) return FALSE; break; diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index 07ac10e9f9e..36ec83cf4d7 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -85,6 +85,7 @@ enum wayland_surface_role WAYLAND_SURFACE_ROLE_NONE, WAYLAND_SURFACE_ROLE_TOPLEVEL, WAYLAND_SURFACE_ROLE_SUBSURFACE, + WAYLAND_SURFACE_ROLE_POPUP, }; struct wayland_keyboard @@ -226,6 +227,7 @@ struct wayland_output struct wayland_surface_config { + int32_t x, y; int32_t width, height; enum wayland_surface_config_state state; uint32_t serial; @@ -280,15 +282,22 @@ struct wayland_surface struct wayland_shm_buffer *big_icon_buffer; enum wayland_surface_role role; + + struct xdg_surface *xdg_surface; + union { struct { - struct xdg_surface *xdg_surface; struct xdg_toplevel *xdg_toplevel; struct xdg_toplevel_icon_v1 *xdg_toplevel_icon; }; struct + { + struct xdg_popup *xdg_popup; + HWND owner_hwnd; + }; + struct { struct wl_subsurface *wl_subsurface; HWND toplevel_hwnd; @@ -326,6 +335,8 @@ void wayland_surface_destroy(struct wayland_surface *surface); void wayland_surface_make_toplevel(struct wayland_surface *surface); void wayland_surface_make_subsurface(struct wayland_surface *surface, struct wayland_surface *parent); +void wayland_surface_make_popup(struct wayland_surface *surface, + struct wayland_surface *owner); void wayland_surface_clear_role(struct wayland_surface *surface); void wayland_surface_attach_shm(struct wayland_surface *surface, struct wayland_shm_buffer *shm_buffer, @@ -352,6 +363,18 @@ static inline BOOL wayland_surface_is_toplevel(struct wayland_surface *surface) return surface->role == WAYLAND_SURFACE_ROLE_TOPLEVEL && surface->xdg_toplevel; } +static inline BOOL wayland_surface_is_popup(struct wayland_surface *surface) +{ + return surface->role == WAYLAND_SURFACE_ROLE_POPUP && surface->xdg_popup; +} + +static inline BOOL wayland_surface_popup_config_changed(struct wayland_surface_config *c1, + struct wayland_surface_config *c2) +{ + return c1->height != c2->height || c1->width != c2->width || + c1->x != c2->x || c1->y != c2->y; +} + /********************************************************************** * Wayland SHM buffer */ diff --git a/dlls/winewayland.drv/window.c b/dlls/winewayland.drv/window.c index 8e05292cb62..260829fe615 100644 --- a/dlls/winewayland.drv/window.c +++ b/dlls/winewayland.drv/window.c @@ -188,7 +188,9 @@ static void reapply_cursor_clipping(void) NtUserSetThreadDpiAwarenessContext(context); } -static BOOL wayland_win_data_create_wayland_surface(struct wayland_win_data *data, struct wayland_surface *toplevel_surface) +static BOOL wayland_win_data_create_wayland_surface(struct wayland_win_data *data, + struct wayland_surface *toplevel_surface, + struct wayland_surface *owner_surface) { struct wayland_client_surface *client = data->client_surface; struct wayland_surface *surface; @@ -203,6 +205,7 @@ static BOOL wayland_win_data_create_wayland_surface(struct wayland_win_data *dat (!(exstyle & WS_EX_LAYERED) || data->layered_attribs_set); if (!visible) role = WAYLAND_SURFACE_ROLE_NONE; + else if (owner_surface) role = WAYLAND_SURFACE_ROLE_POPUP; else if (toplevel_surface) role = WAYLAND_SURFACE_ROLE_SUBSURFACE; else role = WAYLAND_SURFACE_ROLE_TOPLEVEL; @@ -224,6 +227,8 @@ static BOOL wayland_win_data_create_wayland_surface(struct wayland_win_data *dat wl_surface_set_input_region(surface->wl_surface, input_region); if (input_region) wl_region_destroy(input_region); + wayland_win_data_get_config(data, &surface->window); + /* If the window is a visible toplevel make it a wayland * xdg_toplevel. Otherwise keep it role-less to avoid polluting the * compositor with empty xdg_toplevels. */ @@ -232,6 +237,9 @@ static BOOL wayland_win_data_create_wayland_surface(struct wayland_win_data *dat case WAYLAND_SURFACE_ROLE_NONE: wayland_surface_clear_role(surface); break; + case WAYLAND_SURFACE_ROLE_POPUP: + wayland_surface_make_popup(surface, owner_surface); + break; case WAYLAND_SURFACE_ROLE_TOPLEVEL: wayland_surface_make_toplevel(surface); break; @@ -241,7 +249,6 @@ static BOOL wayland_win_data_create_wayland_surface(struct wayland_win_data *dat } if (visible && client) wayland_client_surface_attach(client, data->hwnd); - wayland_win_data_get_config(data, &surface->window); /* Size/position changes affect the effective pointer constraint, so update * it as needed. */ @@ -309,6 +316,8 @@ static void wayland_win_data_update_wayland_state(struct wayland_win_data *data) switch (surface->role) { case WAYLAND_SURFACE_ROLE_NONE: + /* popups do not have any state to update */ + case WAYLAND_SURFACE_ROLE_POPUP: break; case WAYLAND_SURFACE_ROLE_TOPLEVEL: if (!surface->xdg_surface) break; /* surface role has been cleared */ @@ -446,22 +455,38 @@ BOOL WAYLAND_WindowPosChanging(HWND hwnd, UINT swp_flags, BOOL shaped, const str void WAYLAND_WindowPosChanged(HWND hwnd, HWND insert_after, HWND owner_hint, UINT swp_flags, const struct window_rects *new_rects, struct window_surface *surface) { - HWND toplevel = NtUserGetAncestor(hwnd, GA_ROOT); - struct wayland_surface *toplevel_surface; + HWND toplevel = NtUserGetAncestor(hwnd, GA_ROOT), owner = NULL; + struct wayland_surface *toplevel_surface = NULL, *owner_surface = NULL; struct wayland_client_surface *client; - struct wayland_win_data *data, *toplevel_data; + struct wayland_win_data *data, *toplevel_data, *owner_data; BOOL managed, fullscreen = swp_flags & WINE_SWP_FULLSCREEN; - TRACE("hwnd %p new_rects %s after %p flags %08x\n", hwnd, debugstr_window_rects(new_rects), insert_after, swp_flags); - /* Get the managed state with win_data unlocked, as is_window_managed * may need to query win_data information about other HWNDs and thus * acquire the lock itself internally. */ - if (!(managed = is_window_managed(hwnd, swp_flags, fullscreen)) && surface) toplevel = owner_hint; + if (!(managed = is_window_managed(hwnd, swp_flags, fullscreen)) && surface) + { + owner = owner_hint; + toplevel = NULL; + } + + TRACE("hwnd %p owner %p toplevel %p new_rects %s after %p flags %08x\n", hwnd, + owner, toplevel, debugstr_window_rects(new_rects), insert_after, swp_flags); if (!(data = wayland_win_data_get(hwnd))) return; toplevel_data = toplevel && toplevel != hwnd ? wayland_win_data_get_nolock(toplevel) : NULL; toplevel_surface = toplevel_data ? toplevel_data->wayland_surface : NULL; + owner_data = owner && owner != hwnd ? wayland_win_data_get_nolock(owner) : NULL; + owner_surface = owner_data ? owner_data->wayland_surface : NULL; + /* for it to be a popup, we need a valid xdg surface. + * Demote to a subsurface instead if this condition is not met. */ + if (owner_surface && !owner_surface->xdg_surface) + { + /* if the window is unmanaged and the owner + * is not visible, we cannot use a subsurface. */ + if (owner_surface->window.visible) toplevel_surface = owner_surface; + owner_surface = NULL; + } data->rects = *new_rects; data->is_fullscreen = fullscreen; @@ -484,7 +509,7 @@ void WAYLAND_WindowPosChanged(HWND hwnd, HWND insert_after, HWND owner_hint, UIN data->wayland_surface = NULL; } } - else if (wayland_win_data_create_wayland_surface(data, toplevel_surface)) + else if (wayland_win_data_create_wayland_surface(data, toplevel_surface, owner_surface)) { wayland_win_data_update_wayland_state(data); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11178
Etaash Mathamsetty (@etaash.mathamsetty) commented about dlls/winewayland.drv/wayland_surface.c:
+ + /* this is a stale event */ + if (!wayland_surface_is_popup(surface)) goto done; + if (surface->xdg_popup != xdg_popup) goto done; + + /* the protocol requires us to destroy the xdg_popup */ + wayland_surface_clear_role(surface); + + if (!(owner_data = wayland_win_data_get_nolock(surface->owner_hwnd)) || + !(owner_surface = owner_data->wayland_surface) || !owner_surface->xdg_surface) + { + ERR("Failed to recreate popup!\n"); + goto done; + } + + wayland_surface_make_popup(surface, owner_surface); Another question: Should we handle there being a potential feedback loop here? Where we keep getting handle_done because of making a popup in here. Should I try using a solution similar to what is used in xdg-toplevel where we use the win32u message loop to handle this?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/11178#note_143447
Fwiw this will take time to review, I'm unfamiliar with wayland in general and this surface management is complicated and has implications on other win32u architectural decisions. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11178#note_143577
participants (3)
-
Etaash Mathamsetty -
Etaash Mathamsetty (＠etaash.mathamsetty)
-
Rémi Bernon (＠rbernon)