[PATCH 0/3] MR11248: winewayland: Use xdg-popup instead of subsurfaces for owned or equivalent windows.
Same as https://gitlab.winehq.org/wine/wine/-/merge_requests/11178 with changes to allow resizing and sharing the logic between toplevel and popups. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11248
From: Etaash Mathamsetty <etaash.mathamsetty@gmail.com> --- dlls/winewayland.drv/wayland_surface.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/dlls/winewayland.drv/wayland_surface.c b/dlls/winewayland.drv/wayland_surface.c index 0dae0922179..015777ee9ea 100644 --- a/dlls/winewayland.drv/wayland_surface.c +++ b/dlls/winewayland.drv/wayland_surface.c @@ -640,19 +640,15 @@ static void wayland_surface_reconfigure_geometry(struct wayland_surface *surface } TRACE("Window is too large for Wayland state, using subregion\n"); } - else - { - OffsetRect(&rect, -rect.left, -rect.top); - } TRACE("hwnd=%p geometry=%s\n", surface->hwnd, wine_dbgstr_rect(&rect)); if (!IsRectEmpty(&rect)) { 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); + + xdg_surface_set_window_geometry(surface->xdg_surface, 0, 0, width, height); + if (surface->window.resizeable) { xdg_toplevel_set_min_size(surface->xdg_toplevel, 0, 0); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11248
From: Rémi Bernon <rbernon@codeweavers.com> --- dlls/winewayland.drv/wayland_surface.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/dlls/winewayland.drv/wayland_surface.c b/dlls/winewayland.drv/wayland_surface.c index 015777ee9ea..f9d1ae88f8d 100644 --- a/dlls/winewayland.drv/wayland_surface.c +++ b/dlls/winewayland.drv/wayland_surface.c @@ -643,11 +643,15 @@ static void wayland_surface_reconfigure_geometry(struct wayland_surface *surface TRACE("hwnd=%p geometry=%s\n", surface->hwnd, wine_dbgstr_rect(&rect)); - if (!IsRectEmpty(&rect)) + if (!IsRectEmpty(&rect) && !EqualRect(&rect, &surface->current.rect)) { int width = rect.right - rect.left, height = rect.bottom - rect.top; - xdg_surface_set_window_geometry(surface->xdg_surface, 0, 0, width, height); + } + + if (wayland_surface_is_toplevel(surface)) + { + int width = rect.right - rect.left, height = rect.bottom - rect.top; if (surface->window.resizeable) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11248
From: Etaash Mathamsetty <etaash.mathamsetty@gmail.com> --- dlls/winewayland.drv/wayland.c | 8 +- dlls/winewayland.drv/wayland_surface.c | 247 ++++++++++++++++--------- dlls/winewayland.drv/waylanddrv.h | 14 +- dlls/winewayland.drv/window.c | 53 ++---- 4 files changed, 188 insertions(+), 134 deletions(-) diff --git a/dlls/winewayland.drv/wayland.c b/dlls/winewayland.drv/wayland.c index 5b2b0c5b25b..7f56e1cfd20 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 f9d1ae88f8d..6b0a69eeef2 100644 --- a/dlls/winewayland.drv/wayland_surface.c +++ b/dlls/winewayland.drv/wayland_surface.c @@ -34,6 +34,16 @@ WINE_DEFAULT_DEBUG_CHANNEL(waylanddrv); +/* rountrip through WindowPosChanged to refresh the host window state */ +static void update_window_state(HWND hwnd) +{ + static const UINT swp_flags = SWP_NOSIZE | SWP_NOMOVE | SWP_NOCLIENTSIZE | SWP_NOCLIENTMOVE | + SWP_NOZORDER | SWP_NOACTIVATE | SWP_NOREDRAW; + static const RECT rect; + + NtUserSetRawWindowPos(hwnd, rect, swp_flags, FALSE); +} + static void xdg_surface_handle_configure(void *private, struct xdg_surface *xdg_surface, uint32_t serial) { @@ -48,19 +58,21 @@ 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) { - /* If we have a previously requested config, we have already sent a - * WM_WAYLAND_CONFIGURE which hasn't been handled yet. In that case, - * avoid sending another message to reduce message queue traffic. */ - should_post = surface->requested.serial == 0; - initial_configure = surface->current.serial == 0; - surface->pending.serial = serial; - surface->requested = surface->pending; - memset(&surface->pending, 0, sizeof(surface->pending)); + wayland_win_data_release(data); + return; } + /* If we have a previously requested config, we have already sent a + * WM_WAYLAND_CONFIGURE which hasn't been handled yet. In that case, + * avoid sending another message to reduce message queue traffic. */ + should_post = surface->requested.serial == 0; + initial_configure = surface->current.serial == 0; + surface->pending.serial = serial; + surface->requested = surface->pending; + memset(&surface->pending, 0, sizeof(surface->pending)); + wayland_win_data_release(data); if (should_post) NtUserPostMessage(hwnd, WM_WAYLAND_CONFIGURE, 0, 0); @@ -137,6 +149,65 @@ 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)) + { + SetRect(&surface->pending.rect, x, y, x + width, y + 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_surface *surface; + struct wayland_win_data *data; + 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", hwnd); + + /* the protocol requires us to destroy the xdg_popup */ + xdg_popup_destroy(xdg_popup); + + if (!(data = wayland_win_data_get(hwnd))) return; + if ((surface = data->wayland_surface) && surface->xdg_popup == xdg_popup) + { + surface->xdg_popup = NULL; + wayland_surface_clear_role(surface); + } + wayland_win_data_release(data); + + update_window_state(hwnd); +} + +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) @@ -162,16 +233,10 @@ void wp_fractional_scale_handle_scale(void* user_data, if ((client = data->client_surface)) wayland_client_surface_attach(client, client->toplevel); - /* the subsurface rect has changed */ - if (surface->role == WAYLAND_SURFACE_ROLE_SUBSURFACE) - { - surface->processing.serial = 1; - surface->processing.processed = TRUE; - } - wayland_win_data_release(data); NtUserExposeWindowSurface(hwnd, 0, NULL, 0); + update_window_state(hwnd); } static const struct wp_fractional_scale_v1_listener wp_fractional_scale_listener = @@ -322,6 +387,28 @@ static void wayland_surface_init_fractional_scale(struct wayland_surface *surfac surface->hwnd); } +/* helper to intialize the positioner using a given surface config */ +static struct xdg_positioner *xdg_positioner_create(RECT rect) +{ + struct xdg_positioner *xdg_positioner; + + if (!(xdg_positioner = xdg_wm_base_create_positioner(process_wayland.xdg_wm_base))) return NULL; + + if (rect.right == rect.left) rect.right = rect.left + 1; + if (rect.bottom == rect.top) rect.bottom = rect.top + 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, rect.left, rect.top); + xdg_positioner_set_size(xdg_positioner, rect.right - rect.left, rect.bottom - rect.top); + + return xdg_positioner; +} + /********************************************************************** * wayland_surface_make_toplevel * @@ -370,46 +457,53 @@ err: } /********************************************************************** - * wayland_surface_make_subsurface + * wayland_surface_make_popup * - * Gives the subsurface role to a plain wayland surface. + * Gives the popup role to a plain wayland surface. */ -void wayland_surface_make_subsurface(struct wayland_surface *surface, - struct wayland_surface *owner) +void wayland_surface_make_popup(struct wayland_surface *surface, + struct wayland_surface *owner) { - assert(!surface->role || surface->role == WAYLAND_SURFACE_ROLE_SUBSURFACE); - if (surface->wl_subsurface && surface->owner_hwnd == owner->hwnd) return; + struct xdg_positioner *xdg_positioner; + RECT rect; + + assert(!surface->role || surface->role == WAYLAND_SURFACE_ROLE_POPUP); + if (surface->xdg_surface && surface->xdg_toplevel && surface->owner_hwnd == owner->hwnd) return; wayland_surface_clear_role(surface); - surface->role = WAYLAND_SURFACE_ROLE_SUBSURFACE; + surface->role = WAYLAND_SURFACE_ROLE_POPUP; TRACE("surface=%p owner=%p\n", surface, owner); - surface->wl_subsurface = - wl_subcompositor_get_subsurface(process_wayland.wl_subcompositor, - surface->wl_surface, - owner->wl_surface); - if (!surface->wl_subsurface) - { - ERR("Failed to create client wl_subsurface\n"); - goto err; - } + 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); + + rect = surface->window.rect; + OffsetRect(&rect, -owner->window.rect.left, -owner->window.rect.top); + rect = map_rect_to_surface(surface, rect); + + if (!(xdg_positioner = xdg_positioner_create(rect))) 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); wayland_surface_init_fractional_scale(surface, owner->window.scale); - surface->role = WAYLAND_SURFACE_ROLE_SUBSURFACE; surface->owner_hwnd = owner->hwnd; - /* Present contents independently of the owner surface. */ - wl_subsurface_set_desync(surface->wl_subsurface); - + wl_surface_commit(surface->wl_surface); wl_display_flush(process_wayland.wl_display); return; err: wayland_surface_clear_role(surface); - ERR("Failed to assign subsurface role to wayland surface\n"); + ERR("Failed to assign popup role to wayland surface\n"); } /********************************************************************** @@ -451,25 +545,25 @@ void wayland_surface_clear_role(struct wayland_surface *surface) xdg_toplevel_destroy(surface->xdg_toplevel); surface->xdg_toplevel = NULL; } - - if (surface->xdg_surface) - { - xdg_surface_destroy(surface->xdg_surface); - surface->xdg_surface = NULL; - } break; - case WAYLAND_SURFACE_ROLE_SUBSURFACE: - if (surface->wl_subsurface) + case WAYLAND_SURFACE_ROLE_POPUP: + if (surface->xdg_popup) { - wl_subsurface_destroy(surface->wl_subsurface); - surface->wl_subsurface = NULL; + xdg_popup_destroy(surface->xdg_popup); + surface->xdg_popup = NULL; } surface->owner_hwnd = NULL; break; } + if (surface->xdg_surface) + { + xdg_surface_destroy(surface->xdg_surface); + surface->xdg_surface = NULL; + } + memset(&surface->pending, 0, sizeof(surface->pending)); memset(&surface->requested, 0, sizeof(surface->requested)); memset(&surface->processing, 0, sizeof(surface->processing)); @@ -647,6 +741,15 @@ 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, 0, 0, width, height); + + if (wayland_surface_is_popup(surface)) + { + struct xdg_positioner *xdg_positioner; + + if (!(xdg_positioner = xdg_positioner_create(rect))) return; + xdg_popup_reposition(surface->xdg_popup, xdg_positioner, 0); + xdg_positioner_destroy(xdg_positioner); + } } if (wayland_surface_is_toplevel(surface)) @@ -755,39 +858,6 @@ static BOOL wayland_surface_reconfigure_xdg(struct wayland_surface *surface, REC return TRUE; } -/********************************************************************** - * wayland_surface_reconfigure_subsurface - * - * Reconfigures the subsurface as needed to match the latest requested - * state. - */ -static void wayland_surface_reconfigure_subsurface(struct wayland_surface *surface) -{ - struct wayland_win_data *owner_data; - struct wayland_surface *owner_surface; - - if (surface->processing.serial && surface->processing.processed && - (owner_data = wayland_win_data_get_nolock(surface->owner_hwnd)) && - (owner_surface = owner_data->wayland_surface)) - { - RECT rect = surface->window.rect; - - OffsetRect(&rect, -owner_surface->window.rect.left, -owner_surface->window.rect.top); - rect = map_rect_to_surface(surface, rect); - - TRACE("hwnd=%p rect=%s\n", surface->hwnd, wine_dbgstr_rect(&rect)); - - wl_subsurface_set_position(surface->wl_subsurface, rect.left, rect.top); - if (owner_data->client_surface && owner_data->client_surface->wl_subsurface) - wl_subsurface_place_above(surface->wl_subsurface, owner_data->client_surface->wl_surface); - else - wl_subsurface_place_above(surface->wl_subsurface, owner_surface->wl_surface); - wl_surface_commit(owner_surface->wl_surface); - - memset(&surface->processing, 0, sizeof(surface->processing)); - } -} - /********************************************************************** * wayland_surface_reconfigure * @@ -797,10 +867,12 @@ static void wayland_surface_reconfigure_subsurface(struct wayland_surface *surfa BOOL wayland_surface_reconfigure(struct wayland_surface *surface) { struct wayland_window_config *window = &surface->window; - RECT rect = map_rect_to_surface(surface, surface->window.rect); + struct wayland_win_data *owner_data; + struct wayland_surface *owner; + RECT rect = window->rect; TRACE("hwnd=%p window=%s,%#x processing=%s,%#x current=%s,%#x\n", - surface->hwnd, wine_dbgstr_rect(&rect), window->state, + surface->hwnd, wine_dbgstr_rect(&window->rect), window->state, wine_dbgstr_rect(&surface->processing.rect), surface->processing.state, wine_dbgstr_rect(&surface->current.rect), surface->current.state); @@ -808,14 +880,15 @@ BOOL wayland_surface_reconfigure(struct wayland_surface *surface) { case WAYLAND_SURFACE_ROLE_NONE: break; + case WAYLAND_SURFACE_ROLE_POPUP: + if (!(owner_data = wayland_win_data_get_nolock(surface->owner_hwnd))) return FALSE; + if ((owner = owner_data->wayland_surface)) OffsetRect(&rect, -owner->window.rect.left, -owner->window.rect.top); + /* fallthrough */ case WAYLAND_SURFACE_ROLE_TOPLEVEL: + rect = map_rect_to_surface(surface, rect); if (!surface->xdg_surface) break; /* surface role has been cleared */ if (!wayland_surface_reconfigure_xdg(surface, rect)) return FALSE; break; - case WAYLAND_SURFACE_ROLE_SUBSURFACE: - if (!surface->wl_subsurface) break; /* surface role has been cleared */ - wayland_surface_reconfigure_subsurface(surface); - break; } wayland_surface_reconfigure_size(surface, rect.right - rect.left, rect.bottom - rect.top); diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index e0df8e56bb5..fe8494fa03b 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -84,7 +84,7 @@ enum wayland_surface_role { WAYLAND_SURFACE_ROLE_NONE, WAYLAND_SURFACE_ROLE_TOPLEVEL, - WAYLAND_SURFACE_ROLE_SUBSURFACE, + WAYLAND_SURFACE_ROLE_POPUP, }; struct wayland_keyboard @@ -280,17 +280,18 @@ 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 wl_subsurface *wl_subsurface; + struct xdg_popup *xdg_popup; HWND owner_hwnd; }; }; @@ -326,6 +327,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, @@ -349,6 +352,11 @@ 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; +} + /********************************************************************** * Wayland SHM buffer */ diff --git a/dlls/winewayland.drv/window.c b/dlls/winewayland.drv/window.c index afef743755b..d3a722390ea 100644 --- a/dlls/winewayland.drv/window.c +++ b/dlls/winewayland.drv/window.c @@ -203,7 +203,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_SUBSURFACE; + else if (owner_surface) role = WAYLAND_SURFACE_ROLE_POPUP; else role = WAYLAND_SURFACE_ROLE_TOPLEVEL; /* we can temporarily clear the role of a surface but cannot assign a different one after it's set */ @@ -224,6 +224,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. */ @@ -235,13 +237,12 @@ static BOOL wayland_win_data_create_wayland_surface(struct wayland_win_data *dat case WAYLAND_SURFACE_ROLE_TOPLEVEL: wayland_surface_make_toplevel(surface); break; - case WAYLAND_SURFACE_ROLE_SUBSURFACE: - wayland_surface_make_subsurface(surface, owner_surface); + case WAYLAND_SURFACE_ROLE_POPUP: + wayland_surface_make_popup(surface, owner_surface); break; } 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. */ @@ -252,8 +253,9 @@ static BOOL wayland_win_data_create_wayland_surface(struct wayland_win_data *dat return TRUE; } -static void wayland_surface_update_state_toplevel(struct wayland_surface *surface) +static void wayland_win_data_update_wayland_state(struct wayland_win_data *data) { + struct wayland_surface *surface = data->wayland_surface; BOOL processing_config = surface->processing.serial && !surface->processing.processed; @@ -264,7 +266,11 @@ static void wayland_surface_update_state_toplevel(struct wayland_surface *surfac /* If we are not processing a compositor requested config, use the * window state to determine and update the Wayland state. */ - if (!processing_config) + if (processing_config) + { + surface->processing.processed = TRUE; + } + else if (wayland_surface_is_toplevel(surface)) { /* First do all state unsettings, before setting new state. Some * Wayland compositors misbehave if the order is reversed. */ @@ -296,32 +302,6 @@ static void wayland_surface_update_state_toplevel(struct wayland_surface *surfac xdg_toplevel_set_minimized(surface->xdg_toplevel); } } - else - { - surface->processing.processed = TRUE; - } -} - -static void wayland_win_data_update_wayland_state(struct wayland_win_data *data) -{ - struct wayland_surface *surface = data->wayland_surface; - - switch (surface->role) - { - case WAYLAND_SURFACE_ROLE_NONE: - break; - case WAYLAND_SURFACE_ROLE_TOPLEVEL: - if (!surface->xdg_surface) break; /* surface role has been cleared */ - wayland_surface_update_state_toplevel(surface); - break; - case WAYLAND_SURFACE_ROLE_SUBSURFACE: - TRACE("hwnd=%p subsurface owner=%p\n", surface->hwnd, surface->owner_hwnd); - /* Although subsurfaces don't have a dedicated surface config mechanism, - * we use the config fields to mark them as updated. */ - surface->processing.serial = 1; - surface->processing.processed = TRUE; - break; - } wl_display_flush(process_wayland.wl_display); } @@ -462,6 +442,8 @@ void WAYLAND_WindowPosChanged(HWND hwnd, HWND insert_after, HWND owner_hint, UIN if (!(data = wayland_win_data_get(hwnd))) return; 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. */ + if (owner_surface && !owner_surface->xdg_surface) owner_surface = NULL; data->rects = *new_rects; data->is_fullscreen = fullscreen; @@ -512,13 +494,6 @@ static void wayland_configure_window(HWND hwnd) return; } - if (!wayland_surface_is_toplevel(surface)) - { - TRACE("missing xdg_toplevel, returning\n"); - wayland_win_data_release(data); - return; - } - if (!surface->requested.serial) { TRACE("requested configure event already handled, returning\n"); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11248
Rémi Bernon (@rbernon) commented about dlls/winewayland.drv/wayland_surface.c:
+ if ((surface = data->wayland_surface) && wayland_surface_is_popup(surface)) + { + SetRect(&surface->pending.rect, x, y, x + width, y + 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_surface *surface; + struct wayland_win_data *data; + HWND hwnd = private; + + if (!xdg_popup) return; I wasn't sure if this was even needed, I kept it just in case but IMO we should not add ifs unless really necessary.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/11248#note_144079
Etaash Mathamsetty (@etaash.mathamsetty) commented about dlls/winewayland.drv/wayland_surface.c:
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);
I think should use wine_dbgstr_rect here just to be consistent -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11248#note_144155
Etaash Mathamsetty (@etaash.mathamsetty) commented about dlls/winewayland.drv/window.c:
(!(exstyle & WS_EX_LAYERED) || data->layered_attribs_set);
if (!visible) role = WAYLAND_SURFACE_ROLE_NONE; - else if (owner_surface) role = WAYLAND_SURFACE_ROLE_SUBSURFACE; + else if (owner_surface) role = WAYLAND_SURFACE_ROLE_POPUP;
Do we need to create popup role for child windows? From my testing it works to just leave them roleless and avoid any configure events that for surfaces that will never be mapped since they dont have a window_surface. (Are there any situations where they could have a window surface?) Client surfaces are attached to their topmost window so we dont need it for client surfaces either. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11248#note_144157
On Thu Jun 25 03:58:55 2026 +0000, Rémi Bernon wrote:
I wasn't sure if this was even needed, I kept it just in case but IMO we should not add ifs unless really necessary. In that case you can remove it
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/11248#note_144158
Etaash Mathamsetty (@etaash.mathamsetty) commented about dlls/winewayland.drv/wayland_surface.c:
} TRACE("Window is too large for Wayland state, using subregion\n"); } - else - { - OffsetRect(&rect, -rect.left, -rect.top); - }
TRACE("hwnd=%p geometry=%s\n", surface->hwnd, wine_dbgstr_rect(&rect));
- if (!IsRectEmpty(&rect)) + if (!IsRectEmpty(&rect) && !EqualRect(&rect, &surface->current.rect))
for toplevels the current.rect contains a size hint, not the current size of the window. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11248#note_144159
Etaash Mathamsetty (@etaash.mathamsetty) commented about dlls/winewayland.drv/wayland_surface.c:
- if (!IsRectEmpty(&rect)) + if (!IsRectEmpty(&rect) && !EqualRect(&rect, &surface->current.rect)) { 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); + xdg_surface_set_window_geometry(surface->xdg_surface, 0, 0, width, height); + + if (wayland_surface_is_popup(surface)) + { + struct xdg_positioner *xdg_positioner; + + if (!(xdg_positioner = xdg_positioner_create(rect))) return; + xdg_popup_reposition(surface->xdg_popup, xdg_positioner, 0);
wayland_surface_reconfigure_geometry is called right after we ack a configure. If for example, the compositor decides to place our popup in a place different from requested we will constantly spam the compositor with requests to reposition the popup for each frame. In addition the way this is currently set up would prevent it from repositioning the popup unless a new contents is commited as well. This was another one of the reasons I had made the popup code not use wayland_configure_window step. If we wanted to position the popup without having to commit new contents it would need to be done during the pWindowPosChanged user driver call. Having the configure event call NtUserSetRawWindowPos would lead to a feedback loop if it emits pWindowPosChanged with SWP_NOSIZE. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11248#note_144160
Etaash Mathamsetty (@etaash.mathamsetty) commented about dlls/winewayland.drv/wayland_surface.c:
surface->hwnd); }
+/* helper to intialize the positioner using a given surface config */
its rect instead of a surface config now (perhaps the comment is not really needed) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11248#note_144156
On Thu Jun 25 03:58:55 2026 +0000, Etaash Mathamsetty wrote:
wayland_surface_reconfigure_geometry is called right after we ack a configure. If for example, the compositor decides to place our popup in a place different from requested we will constantly spam the compositor with requests to reposition the popup for each frame. In addition the way this is currently set up would prevent it from repositioning the popup unless a new contents is commited as well. This was another one of the reasons I had made the popup code not use wayland_configure_window step. If we wanted to position the popup without having to commit new contents it would need to be done during the pWindowPosChanged user driver call. Having the configure event call NtUserSetRawWindowPos would lead to a feedback loop if it emits pWindowPosChanged with SWP_NOSIZE. I read apply_window_pos and move_window_bits commits new contents if the window moves if I understood right, so this should be fine in that regard. I just don't like that we are causing a new configure to occur each frame if the compositor is being stubborn
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/11248#note_144164
On Thu Jun 25 03:58:55 2026 +0000, Etaash Mathamsetty wrote:
Do we need to create popup role for child windows? From my testing it works to just leave them roleless and avoid any configure events that for surfaces that will never be mapped since they dont have a window_surface. (Are there any situations where they could have a window surface?) Client surfaces are attached to their topmost window so we dont need it for client surfaces either. We don't and shouldn't create any surface for child windows. Their `struct window_surface *surface` WindowPosChanged parameter will be NULL and this should trigger the wayland surface destruction if the window was a toplevel and became a children.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/11248#note_144177
On Thu Jun 25 04:15:19 2026 +0000, Etaash Mathamsetty wrote:
I read apply_window_pos and move_window_bits commits new contents if the window moves if I understood right, so this should be fine in that regard. I just don't like that we are causing a new configure to occur each frame if the compositor is being stubborn Is there anything that needs to match between `xdg_surface_set_window_geometry` and `xdg_positioner_set_size` / `xdg_popup_reposition`? What is the positioner size used for exactly?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/11248#note_144178
participants (4)
-
Etaash Mathamsetty -
Etaash Mathamsetty (＠etaash.mathamsetty)
-
Rémi Bernon
-
Rémi Bernon (＠rbernon)