[PATCH v4 0/9] MR6107: winewayland: Support GL/VK rendering in child windows
At the moment GL/VK content can only be presented in top-level windows, since child windows are not backed by Wayland surfaces. This MR adds support for such scenarios, in a few gradual steps: 1. Create Wayland (sub)surfaces for all child windows, anchoring them to their parent surface, which may also be a child window surface (i.e., we support GL/VK in nested child windows). This approach works, but it pollutes the compositor with mostly unused, and possibly nested (sub)surfaces. We will deal with this later in the MR. 2. Ensure that the child window (sub)surfaces are properly updated and reconfigured, and support WS_POPUP <-> WS_CHILD style changes (reparenting etc). 3. In the second to last commit, improve efficiency by creating (sub)surfaces only for the child windows needed by GL/VK, and anchor them directly to the parent toplevel. This removes (sub)surface bloat and unnecessary nesting, the trade-off being some extra complexity when dealing with updates. 4. Finally improve the display of GDI content along with accelerated content. Note that this MR doesn't clip GL/VK child window contents at the moment. The subsurface mechanism introduced in this MR could also handle other kinds of windows in the future, for example display and properly position transient windows, menus etc. -- v4: winewayland: Create Wayland surfaces for child windows on demand. winewayland: Ensure parent surface contents for accelerated windows. winewayland: Support Wayland surface role changes. win32u: Update children window state when the parent state changes. winewayland: Post WM_WAYLAND_CONFIGURE outside of the surface lock. winewayland: Use weak references for parent wayland_surfaces. winewayland: Handle subsurface reconfiguration. winewayland: Create subsurfaces for child windows. winewayland: Store all window rects in wayland_win_data. https://gitlab.winehq.org/wine/wine/-/merge_requests/6107
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> This allows wayland_win_data_update_wayland_surface() to be called outside of WAYLAND_WindowPosChange, which will be needed in upcoming commits. --- dlls/winewayland.drv/waylanddrv.h | 5 +---- dlls/winewayland.drv/window.c | 26 ++++++++++++-------------- 2 files changed, 13 insertions(+), 18 deletions(-) diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index 5bde94a0ac9..3ac3865efd2 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -290,10 +290,7 @@ struct wayland_win_data struct wayland_surface *wayland_surface; /* wine window_surface backing this window */ struct window_surface *window_surface; - /* USER window rectangle relative to win32 parent window client area */ - RECT window_rect; - /* USER client rectangle relative to win32 parent window client area */ - RECT client_rect; + struct window_rects rects; BOOL managed; }; diff --git a/dlls/winewayland.drv/window.c b/dlls/winewayland.drv/window.c index 233ca0e5a62..9e228b779f9 100644 --- a/dlls/winewayland.drv/window.c +++ b/dlls/winewayland.drv/window.c @@ -76,9 +76,7 @@ static struct rb_tree win_data_rb = { wayland_win_data_cmp_rb }; * * Create a data window structure for an existing window. */ -static struct wayland_win_data *wayland_win_data_create(HWND hwnd, - const RECT *window_rect, - const RECT *client_rect) +static struct wayland_win_data *wayland_win_data_create(HWND hwnd, struct window_rects *rects) { struct wayland_win_data *data; struct rb_entry *rb_entry; @@ -92,8 +90,7 @@ static struct wayland_win_data *wayland_win_data_create(HWND hwnd, if (!(data = calloc(1, sizeof(*data)))) return NULL; data->hwnd = hwnd; - data->window_rect = *window_rect; - data->client_rect = *client_rect; + data->rects = *rects; pthread_mutex_lock(&win_data_mutex); @@ -167,8 +164,8 @@ static void wayland_win_data_get_config(struct wayland_win_data *data, enum wayland_surface_config_state window_state = 0; DWORD style; - conf->rect = data->window_rect; - conf->client_rect = data->client_rect; + conf->rect = data->rects.window; + conf->client_rect = data->rects.client; style = NtUserGetWindowLongW(data->hwnd, GWL_STYLE); TRACE("window=%s style=%#lx\n", wine_dbgstr_rect(&conf->rect), (long)style); @@ -200,14 +197,14 @@ static void reapply_cursor_clipping(void) NtUserSetThreadDpiAwarenessContext(context); } -static void wayland_win_data_update_wayland_surface(struct wayland_win_data *data, const RECT *visible_rect) +static void wayland_win_data_update_wayland_surface(struct wayland_win_data *data) { struct wayland_surface *surface = data->wayland_surface; HWND parent = NtUserGetAncestor(data->hwnd, GA_PARENT); BOOL visible, xdg_visible; WCHAR text[1024]; - TRACE("hwnd=%p, rect=%s\n", data->hwnd, wine_dbgstr_rect(visible_rect)); + TRACE("hwnd=%p\n", data->hwnd); /* We don't want wayland surfaces for child windows. */ if (parent != NtUserGetDesktopWindow() && parent != 0) @@ -251,7 +248,8 @@ static void wayland_win_data_update_wayland_surface(struct wayland_win_data *dat pthread_mutex_unlock(&surface->mutex); if (data->window_surface) - wayland_window_surface_update_wayland_surface(data->window_surface, visible_rect, surface); + wayland_window_surface_update_wayland_surface(data->window_surface, + &data->rects.visible, surface); /* Size/position changes affect the effective pointer constraint, so update * it as needed. */ @@ -433,7 +431,8 @@ BOOL WAYLAND_WindowPosChanging(HWND hwnd, UINT swp_flags, BOOL shaped, struct wi TRACE("hwnd %p, swp_flags %04x, shaped %u, rects %s\n", hwnd, swp_flags, shaped, debugstr_window_rects(rects)); - if (!data && !(data = wayland_win_data_create(hwnd, &rects->window, &rects->client))) return FALSE; /* use default surface */ + if (!data && !(data = wayland_win_data_create(hwnd, rects))) return FALSE; + wayland_win_data_release(data); return TRUE; @@ -458,15 +457,14 @@ void WAYLAND_WindowPosChanged(HWND hwnd, HWND insert_after, UINT swp_flags, cons if (!(data = wayland_win_data_get(hwnd))) return; - data->window_rect = new_rects->window; - data->client_rect = new_rects->client; + data->rects = *new_rects; data->managed = managed; if (surface) window_surface_add_ref(surface); if (data->window_surface) window_surface_release(data->window_surface); data->window_surface = surface; - wayland_win_data_update_wayland_surface(data, &new_rects->visible); + wayland_win_data_update_wayland_surface(data); if (data->wayland_surface) wayland_win_data_update_wayland_state(data); wayland_win_data_release(data); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6107
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> Create (possibly nested) Wayland subsurfaces for all child windows, to allow GL/VK contents to be presented onto them. --- dlls/winewayland.drv/wayland_surface.c | 61 +++++++++++++++++++++++ dlls/winewayland.drv/waylanddrv.h | 12 +++++ dlls/winewayland.drv/window.c | 67 +++++++++++++++++--------- 3 files changed, 117 insertions(+), 23 deletions(-) diff --git a/dlls/winewayland.drv/wayland_surface.c b/dlls/winewayland.drv/wayland_surface.c index 39c3976cdfd..de3307ec51e 100644 --- a/dlls/winewayland.drv/wayland_surface.c +++ b/dlls/winewayland.drv/wayland_surface.c @@ -218,6 +218,12 @@ void wayland_surface_destroy(struct wayland_surface *surface) surface->xdg_surface = NULL; } + if (surface->wl_subsurface) + { + wl_subsurface_destroy(surface->wl_subsurface); + surface->wl_subsurface = NULL; + } + if (surface->wl_surface) { wl_surface_destroy(surface->wl_surface); @@ -253,6 +259,7 @@ void wayland_surface_make_toplevel(struct wayland_surface *surface) surface->xdg_toplevel = xdg_surface_get_toplevel(surface->xdg_surface); if (!surface->xdg_toplevel) goto err; xdg_toplevel_add_listener(surface->xdg_toplevel, &xdg_toplevel_listener, surface->hwnd); + surface->role = WAYLAND_SURFACE_ROLE_TOPLEVEL; if (process_name) xdg_toplevel_set_app_id(surface->xdg_toplevel, process_name); @@ -267,6 +274,53 @@ err: ERR("Failed to assign toplevel role to wayland surface\n"); } +/********************************************************************** + * wayland_surface_make_subsurface + * + * Gives the subsurface role to a plain wayland surface. + */ +void wayland_surface_make_subsurface(struct wayland_surface *surface, + struct wayland_surface *parent) +{ + struct wl_region *empty_region; + + TRACE("surface=%p parent=%p\n", surface, parent); + + surface->wl_subsurface = + wl_subcompositor_get_subsurface(process_wayland.wl_subcompositor, + surface->wl_surface, + parent->wl_surface); + if (!surface->wl_subsurface) + { + ERR("Failed to create client wl_subsurface\n"); + goto err; + } + + surface->role = WAYLAND_SURFACE_ROLE_SUBSURFACE; + surface->parent_hwnd = parent->hwnd; + + /* Let parent handle all pointer events. */ + empty_region = wl_compositor_create_region(process_wayland.wl_compositor); + if (!empty_region) + { + ERR("Failed to create wl_region\n"); + goto err; + } + wl_surface_set_input_region(surface->wl_surface, empty_region); + wl_region_destroy(empty_region); + + /* Present contents independently of the parent surface. */ + wl_subsurface_set_desync(surface->wl_subsurface); + + wl_display_flush(process_wayland.wl_display); + + return; + +err: + wayland_surface_clear_role(surface); + ERR("Failed to assign subsurface role to wayland surface\n"); +} + /********************************************************************** * wayland_surface_clear_role * @@ -290,10 +344,17 @@ void wayland_surface_clear_role(struct wayland_surface *surface) surface->xdg_surface = NULL; } + if (surface->wl_subsurface) + { + wl_subsurface_destroy(surface->wl_subsurface); + surface->wl_subsurface = NULL; + } + memset(&surface->pending, 0, sizeof(surface->pending)); memset(&surface->requested, 0, sizeof(surface->requested)); memset(&surface->processing, 0, sizeof(surface->processing)); memset(&surface->current, 0, sizeof(surface->current)); + surface->parent_hwnd = 0; /* Ensure no buffer is attached, otherwise future role assignments may fail. */ wl_surface_attach(surface->wl_surface, NULL, 0, 0); diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index 3ac3865efd2..22e8b27df82 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -73,6 +73,13 @@ enum wayland_surface_config_state WAYLAND_SURFACE_CONFIG_STATE_FULLSCREEN = (1 << 3) }; +enum wayland_surface_role +{ + WAYLAND_SURFACE_ROLE_NONE, + WAYLAND_SURFACE_ROLE_TOPLEVEL, + WAYLAND_SURFACE_ROLE_SUBSURFACE, +}; + struct wayland_keyboard { struct wl_keyboard *wl_keyboard; @@ -195,6 +202,7 @@ struct wayland_surface struct wl_surface *wl_surface; struct xdg_surface *xdg_surface; struct xdg_toplevel *xdg_toplevel; + struct wl_subsurface *wl_subsurface; struct wp_viewport *wp_viewport; pthread_mutex_t mutex; struct wayland_surface_config pending, requested, processing, current; @@ -204,6 +212,8 @@ struct wayland_surface struct wayland_client_surface *client; int buffer_width, buffer_height; HCURSOR hcursor; + enum wayland_surface_role role; + HWND parent_hwnd; }; struct wayland_shm_buffer @@ -239,6 +249,8 @@ void wayland_output_use_xdg_extension(struct wayland_output *output); struct wayland_surface *wayland_surface_create(HWND hwnd); 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_clear_role(struct wayland_surface *surface); void wayland_surface_attach_shm(struct wayland_surface *surface, struct wayland_shm_buffer *shm_buffer, diff --git a/dlls/winewayland.drv/window.c b/dlls/winewayland.drv/window.c index 9e228b779f9..9d0e649f9d6 100644 --- a/dlls/winewayland.drv/window.c +++ b/dlls/winewayland.drv/window.c @@ -129,19 +129,32 @@ static void wayland_win_data_destroy(struct wayland_win_data *data) } /*********************************************************************** - * wayland_win_data_get + * wayland_win_data_get_nolock * - * Lock and return the data structure associated with a window. + * Return the data structure associated with a window. This function does + * not lock the win_data_mutex, so it must be externally synchronized. */ -struct wayland_win_data *wayland_win_data_get(HWND hwnd) +static struct wayland_win_data *wayland_win_data_get_nolock(HWND hwnd) { struct rb_entry *rb_entry; - pthread_mutex_lock(&win_data_mutex); - if ((rb_entry = rb_get(&win_data_rb, hwnd))) return RB_ENTRY_VALUE(rb_entry, struct wayland_win_data, entry); + return NULL; +} + +/*********************************************************************** + * wayland_win_data_get + * + * Lock and return the data structure associated with a window. + */ +struct wayland_win_data *wayland_win_data_get(HWND hwnd) +{ + struct wayland_win_data *data; + + pthread_mutex_lock(&win_data_mutex); + if ((data = wayland_win_data_get_nolock(hwnd))) return data; pthread_mutex_unlock(&win_data_mutex); return NULL; @@ -200,38 +213,40 @@ static void reapply_cursor_clipping(void) static void wayland_win_data_update_wayland_surface(struct wayland_win_data *data) { struct wayland_surface *surface = data->wayland_surface; - HWND parent = NtUserGetAncestor(data->hwnd, GA_PARENT); - BOOL visible, xdg_visible; + struct wayland_win_data *parent_data; + enum wayland_surface_role role = WAYLAND_SURFACE_ROLE_NONE; WCHAR text[1024]; TRACE("hwnd=%p\n", data->hwnd); - /* We don't want wayland surfaces for child windows. */ - if (parent != NtUserGetDesktopWindow() && parent != 0) + if (NtUserGetWindowLongW(data->hwnd, GWL_STYLE) & WS_VISIBLE) { - if (data->window_surface) - wayland_window_surface_update_wayland_surface(data->window_surface, NULL, NULL); - if (surface) wayland_surface_destroy(surface); - surface = NULL; - goto out; + parent_data = wayland_win_data_get_nolock(NtUserGetAncestor(data->hwnd, GA_PARENT)); + if (!parent_data) + role = WAYLAND_SURFACE_ROLE_TOPLEVEL; + else if (parent_data->wayland_surface) + role = WAYLAND_SURFACE_ROLE_SUBSURFACE; } - /* Otherwise ensure that we have a wayland surface. */ - if (!surface && !(surface = wayland_surface_create(data->hwnd))) return; + /* We can temporarily remove a role from a wayland surface and add it back, + * but we can't change a surface's role. + * TODO: Recreate the surface to allow role change. */ + if (surface && role && surface->role && role != surface->role) goto out; - visible = (NtUserGetWindowLongW(data->hwnd, GWL_STYLE) & WS_VISIBLE) == WS_VISIBLE; - xdg_visible = surface->xdg_toplevel != NULL; + /* Ensure that we have a wayland surface. */ + if (!surface && !(surface = wayland_surface_create(data->hwnd))) goto out; pthread_mutex_lock(&surface->mutex); - if (visible != xdg_visible) + if ((role == WAYLAND_SURFACE_ROLE_TOPLEVEL) != !!(surface->xdg_toplevel) || + (role == WAYLAND_SURFACE_ROLE_SUBSURFACE) != !!(surface->wl_subsurface) || + (role == WAYLAND_SURFACE_ROLE_SUBSURFACE && surface->parent_hwnd != parent_data->hwnd)) { /* If we have a pre-existing surface ensure it has no role. */ if (data->wayland_surface) wayland_surface_clear_role(surface); - /* 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. */ - if (visible) + /* If the window is visible give it a role, otherwise keep it role-less + * to avoid polluting the compositor with unused role objects. */ + if (role == WAYLAND_SURFACE_ROLE_TOPLEVEL) { wayland_surface_make_toplevel(surface); if (surface->xdg_toplevel) @@ -241,6 +256,12 @@ static void wayland_win_data_update_wayland_surface(struct wayland_win_data *dat wayland_surface_set_title(surface, text); } } + else if (role == WAYLAND_SURFACE_ROLE_SUBSURFACE) + { + pthread_mutex_lock(&parent_data->wayland_surface->mutex); + wayland_surface_make_subsurface(surface, parent_data->wayland_surface); + pthread_mutex_unlock(&parent_data->wayland_surface->mutex); + } } wayland_win_data_get_config(data, &surface->window); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6107
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> --- dlls/winewayland.drv/wayland_surface.c | 99 +++++++++++++++++++++----- dlls/winewayland.drv/window.c | 11 ++- 2 files changed, 91 insertions(+), 19 deletions(-) diff --git a/dlls/winewayland.drv/wayland_surface.c b/dlls/winewayland.drv/wayland_surface.c index de3307ec51e..425c270099e 100644 --- a/dlls/winewayland.drv/wayland_surface.c +++ b/dlls/winewayland.drv/wayland_surface.c @@ -568,6 +568,7 @@ static void wayland_surface_reconfigure_client(struct wayland_surface *surface) TRACE("hwnd=%p subsurface=%d,%d+%dx%d\n", surface->hwnd, x, y, width, height); wl_subsurface_set_position(surface->client->wl_subsurface, x, y); + wl_subsurface_place_above(surface->client->wl_subsurface, surface->wl_surface); if (surface->client->wp_viewport) { @@ -589,29 +590,15 @@ static void wayland_surface_reconfigure_client(struct wayland_surface *surface) } /********************************************************************** - * wayland_surface_reconfigure + * wayland_surface_reconfigure_xdg * - * Reconfigures the wayland surface as needed to match the latest requested + * Reconfigures the xdg surface as needed to match the latest requested * state. */ -BOOL wayland_surface_reconfigure(struct wayland_surface *surface) +static BOOL wayland_surface_reconfigure_xdg(struct wayland_surface *surface, + int width, int height) { struct wayland_window_config *window = &surface->window; - int win_width, win_height, width, height; - - if (!surface->xdg_toplevel) return TRUE; - - win_width = surface->window.rect.right - surface->window.rect.left; - win_height = surface->window.rect.bottom - surface->window.rect.top; - - wayland_surface_coords_from_window(surface, win_width, win_height, - &width, &height); - - TRACE("hwnd=%p window=%dx%d,%#x processing=%dx%d,%#x current=%dx%d,%#x\n", - surface->hwnd, win_width, win_height, window->state, - surface->processing.width, surface->processing.height, - surface->processing.state, surface->current.width, - surface->current.height, surface->current.state); /* Acknowledge any compatible processed config. */ if (surface->processing.serial && surface->processing.processed && @@ -644,6 +631,82 @@ BOOL wayland_surface_reconfigure(struct wayland_surface *surface) } wayland_surface_reconfigure_geometry(surface, width, height); + + 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_surface *parent; + int local_x, local_y, x, y; + + /* TODO: Locking the parent surface using the HWND may lead to a deadlock, + * since we will try to acquire the win_data_mutex while holding a surface + * mutex (from the argument of this function), whereas all other paths + * acquire the win_data_mutex before any surface mutex. */ + if (surface->processing.serial && surface->processing.processed && + (parent = wayland_surface_lock_hwnd(surface->parent_hwnd))) + { + /* For now we use a subsurface only for child windows, whose window + * coordinates are relative to the client area of their parent. */ + local_x = surface->window.rect.left + + (parent->window.client_rect.left - parent->window.rect.left); + local_y = surface->window.rect.top + + (parent->window.client_rect.top - parent->window.rect.top); + wayland_surface_coords_from_window(surface, local_x, local_y, &x, &y); + + TRACE("hwnd=%p pos=%d,%d\n", surface->hwnd, x, y); + + wl_subsurface_set_position(surface->wl_subsurface, x, y); + if (parent->client) + wl_subsurface_place_above(surface->wl_subsurface, parent->client->wl_surface); + else + wl_subsurface_place_above(surface->wl_subsurface, parent->wl_surface); + wl_surface_commit(parent->wl_surface); + + pthread_mutex_unlock(&parent->mutex); + + memset(&surface->processing, 0, sizeof(surface->processing)); + } +} + +/********************************************************************** + * wayland_surface_reconfigure + * + * Reconfigures the wayland surface as needed to match the latest requested + * state. + */ +BOOL wayland_surface_reconfigure(struct wayland_surface *surface) +{ + int win_width, win_height, width, height; + + win_width = surface->window.rect.right - surface->window.rect.left; + win_height = surface->window.rect.bottom - surface->window.rect.top; + + wayland_surface_coords_from_window(surface, win_width, win_height, + &width, &height); + + TRACE("hwnd=%p window=%dx%d,%#x processing=%dx%d,%#x current=%dx%d,%#x\n", + surface->hwnd, win_width, win_height, surface->window.state, + surface->processing.width, surface->processing.height, + surface->processing.state, surface->current.width, + surface->current.height, surface->current.state); + + if (surface->xdg_toplevel) + { + if (!wayland_surface_reconfigure_xdg(surface, width, height)) return FALSE; + } + else if (surface->wl_subsurface) + { + wayland_surface_reconfigure_subsurface(surface); + } + wayland_surface_reconfigure_size(surface, width, height); wayland_surface_reconfigure_client(surface); diff --git a/dlls/winewayland.drv/window.c b/dlls/winewayland.drv/window.c index 9d0e649f9d6..de395389264 100644 --- a/dlls/winewayland.drv/window.c +++ b/dlls/winewayland.drv/window.c @@ -288,7 +288,16 @@ static void wayland_win_data_update_wayland_state(struct wayland_win_data *data) pthread_mutex_lock(&surface->mutex); - if (!surface->xdg_toplevel) goto out; + if (surface->wl_subsurface) + { + TRACE("hwnd=%p subsurface parent=%p\n", surface->hwnd, surface->parent_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; + goto out; + } + else if (!surface->xdg_toplevel) goto out; processing_config = surface->processing.serial && !surface->processing.processed; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6107
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> Don't access the parent wayland_surface through their HWND while holding a lock to the child surface, since this requires acquiring the win_data_mutex and may lead to a deadlock. Instead introduce a weak reference mechanism and use that to directly access the parent wayland_surface. --- dlls/winewayland.drv/wayland_surface.c | 58 +++++++++++++++++++++----- dlls/winewayland.drv/waylanddrv.h | 7 +++- dlls/winewayland.drv/window.c | 6 ++- 3 files changed, 58 insertions(+), 13 deletions(-) diff --git a/dlls/winewayland.drv/wayland_surface.c b/dlls/winewayland.drv/wayland_surface.c index 425c270099e..57fd217e0d3 100644 --- a/dlls/winewayland.drv/wayland_surface.c +++ b/dlls/winewayland.drv/wayland_surface.c @@ -168,6 +168,7 @@ struct wayland_surface *wayland_surface_create(HWND hwnd) } surface->window.scale = 1.0; + surface->weak_ref = 1; return surface; @@ -176,6 +177,43 @@ err: return NULL; } +/********************************************************************** + * wayland_surface_get_weak_ref + * + * Gets a weak reference to a wayland_surface. + */ +struct wayland_surface *wayland_surface_get_weak_ref(struct wayland_surface *surface) +{ + InterlockedIncrement(&surface->weak_ref); + return surface; +} + +/********************************************************************** + * wayland_surface_get_weak_ref + * + * Releases a weak reference to a wayland_surface. + */ +void wayland_surface_release_weak_ref(struct wayland_surface *surface) +{ + if (InterlockedDecrement(&surface->weak_ref) > 0) return; + pthread_mutex_destroy(&surface->mutex); + free(surface); +} + +/********************************************************************** + * wayland_surface_lock_weak_ref + * + * Returns a locked wayland_surface from a weak reference, or NULL if the + * surface has been destroyed. + */ +struct wayland_surface *wayland_surface_lock_weak_ref(struct wayland_surface *surface) +{ + pthread_mutex_lock(&surface->mutex); + if (!surface->destroyed) return surface; + pthread_mutex_unlock(&surface->mutex); + return NULL; +} + /********************************************************************** * wayland_surface_destroy * @@ -237,9 +275,7 @@ void wayland_surface_destroy(struct wayland_surface *surface) wl_display_flush(process_wayland.wl_display); - pthread_mutex_destroy(&surface->mutex); - - free(surface); + wayland_surface_release_weak_ref(surface); } /********************************************************************** @@ -297,7 +333,7 @@ void wayland_surface_make_subsurface(struct wayland_surface *surface, } surface->role = WAYLAND_SURFACE_ROLE_SUBSURFACE; - surface->parent_hwnd = parent->hwnd; + surface->parent_weak_ref = wayland_surface_get_weak_ref(parent); /* Let parent handle all pointer events. */ empty_region = wl_compositor_create_region(process_wayland.wl_compositor); @@ -350,11 +386,16 @@ void wayland_surface_clear_role(struct wayland_surface *surface) surface->wl_subsurface = NULL; } + if (surface->parent_weak_ref) + { + wayland_surface_release_weak_ref(surface->parent_weak_ref); + surface->parent_weak_ref = NULL; + } + memset(&surface->pending, 0, sizeof(surface->pending)); memset(&surface->requested, 0, sizeof(surface->requested)); memset(&surface->processing, 0, sizeof(surface->processing)); memset(&surface->current, 0, sizeof(surface->current)); - surface->parent_hwnd = 0; /* Ensure no buffer is attached, otherwise future role assignments may fail. */ wl_surface_attach(surface->wl_surface, NULL, 0, 0); @@ -646,12 +687,9 @@ static void wayland_surface_reconfigure_subsurface(struct wayland_surface *surfa struct wayland_surface *parent; int local_x, local_y, x, y; - /* TODO: Locking the parent surface using the HWND may lead to a deadlock, - * since we will try to acquire the win_data_mutex while holding a surface - * mutex (from the argument of this function), whereas all other paths - * acquire the win_data_mutex before any surface mutex. */ if (surface->processing.serial && surface->processing.processed && - (parent = wayland_surface_lock_hwnd(surface->parent_hwnd))) + surface->parent_weak_ref && + (parent = wayland_surface_lock_weak_ref(surface->parent_weak_ref))) { /* For now we use a subsurface only for child windows, whose window * coordinates are relative to the client area of their parent. */ diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index 22e8b27df82..d553f06a420 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -213,7 +213,9 @@ struct wayland_surface int buffer_width, buffer_height; HCURSOR hcursor; enum wayland_surface_role role; - HWND parent_hwnd; + LONG weak_ref; + BOOL destroyed; + struct wayland_surface *parent_weak_ref; }; struct wayland_shm_buffer @@ -247,6 +249,9 @@ void wayland_output_use_xdg_extension(struct wayland_output *output); */ struct wayland_surface *wayland_surface_create(HWND hwnd); +struct wayland_surface *wayland_surface_get_weak_ref(struct wayland_surface *surface); +void wayland_surface_release_weak_ref(struct wayland_surface *surface); +struct wayland_surface *wayland_surface_lock_weak_ref(struct wayland_surface *surface); 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, diff --git a/dlls/winewayland.drv/window.c b/dlls/winewayland.drv/window.c index de395389264..8453d73488c 100644 --- a/dlls/winewayland.drv/window.c +++ b/dlls/winewayland.drv/window.c @@ -240,7 +240,8 @@ static void wayland_win_data_update_wayland_surface(struct wayland_win_data *dat if ((role == WAYLAND_SURFACE_ROLE_TOPLEVEL) != !!(surface->xdg_toplevel) || (role == WAYLAND_SURFACE_ROLE_SUBSURFACE) != !!(surface->wl_subsurface) || - (role == WAYLAND_SURFACE_ROLE_SUBSURFACE && surface->parent_hwnd != parent_data->hwnd)) + (role == WAYLAND_SURFACE_ROLE_SUBSURFACE && + surface->parent_weak_ref && surface->parent_weak_ref->hwnd != parent_data->hwnd)) { /* If we have a pre-existing surface ensure it has no role. */ if (data->wayland_surface) wayland_surface_clear_role(surface); @@ -290,7 +291,8 @@ static void wayland_win_data_update_wayland_state(struct wayland_win_data *data) if (surface->wl_subsurface) { - TRACE("hwnd=%p subsurface parent=%p\n", surface->hwnd, surface->parent_hwnd); + TRACE("hwnd=%p subsurface parent=%p\n", surface->hwnd, + surface->parent_weak_ref ? surface->parent_weak_ref->hwnd : 0); /* Although subsurfaces don't have a dedicated surface config mechanism, * we use the config fields to mark them as updated. */ surface->processing.serial = 1; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6107
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winewayland.drv/wayland_surface.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/dlls/winewayland.drv/wayland_surface.c b/dlls/winewayland.drv/wayland_surface.c index 57fd217e0d3..2e5059d0980 100644 --- a/dlls/winewayland.drv/wayland_surface.c +++ b/dlls/winewayland.drv/wayland_surface.c @@ -38,7 +38,7 @@ static void xdg_surface_handle_configure(void *data, struct xdg_surface *xdg_sur uint32_t serial) { struct wayland_surface *surface; - BOOL initial_configure = FALSE; + BOOL should_post = FALSE, initial_configure = FALSE; HWND hwnd = data; TRACE("serial=%u\n", serial); @@ -52,16 +52,17 @@ static void xdg_surface_handle_configure(void *data, struct xdg_surface *xdg_sur /* 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. */ - BOOL should_post = surface->requested.serial == 0; + 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)); - if (should_post) NtUserPostMessage(hwnd, WM_WAYLAND_CONFIGURE, 0, 0); } pthread_mutex_unlock(&surface->mutex); + if (should_post) NtUserPostMessage(hwnd, WM_WAYLAND_CONFIGURE, 0, 0); + /* Flush the window surface in case there is content that we weren't * able to flush before due to the lack of the initial configure. */ if (initial_configure) wayland_window_flush(hwnd); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6107
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> Since changes in a parent window state may affect the children state in the driver, ensure the driver gets a chance to update its internal state. --- dlls/win32u/window.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/dlls/win32u/window.c b/dlls/win32u/window.c index 73478438928..e34a1a296fd 100644 --- a/dlls/win32u/window.c +++ b/dlls/win32u/window.c @@ -3517,6 +3517,8 @@ done: return after; } +static void update_children_window_state( HWND hwnd ); + /* NtUserSetWindowPos implementation */ BOOL set_window_pos( WINDOWPOS *winpos, int parent_x, int parent_y ) { @@ -3653,6 +3655,8 @@ BOOL set_window_pos( WINDOWPOS *winpos, int parent_x, int parent_y ) if ((winpos->flags & (SWP_NOSIZE|SWP_NOMOVE|SWP_FRAMECHANGED)) != (SWP_NOSIZE|SWP_NOMOVE)) NtUserNotifyWinEvent( EVENT_OBJECT_LOCATIONCHANGE, winpos->hwnd, OBJID_WINDOW, 0 ); + update_children_window_state( winpos->hwnd ); + ret = TRUE; done: set_thread_dpi_awareness_context( context ); @@ -4386,6 +4390,23 @@ void update_window_state( HWND hwnd ) if (surface) window_surface_release( surface ); set_thread_dpi_awareness_context( context ); + + update_children_window_state( hwnd ); +} + +static void update_children_window_state( HWND hwnd ) +{ + HWND *children; + int i; + + if (!(children = list_window_children( 0, hwnd, NULL, 0 ))) return; + + for (i = 0; children[i]; i++) + { + if (is_window( children[i] )) update_window_state( children[i] ); + } + + free( children ); } /*********************************************************************** -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6107
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> Windows supports changing between the WS_CHILD and WS_POPUP window styles which requires a Wayland surface role change between wl_subsurface and xdg_toplevel. Since Wayland doesn't support changing a surface's role, to support this scenario we recreate the Wayland surface and depend on win32u updating children surfaces so that they can use the new parent surface. --- dlls/winewayland.drv/wayland_surface.c | 31 ++++++++++++++++++++++++++ dlls/winewayland.drv/waylanddrv.h | 2 ++ dlls/winewayland.drv/window.c | 29 +++++++++++++++++++----- 3 files changed, 56 insertions(+), 6 deletions(-) diff --git a/dlls/winewayland.drv/wayland_surface.c b/dlls/winewayland.drv/wayland_surface.c index 2e5059d0980..6b582ada765 100644 --- a/dlls/winewayland.drv/wayland_surface.c +++ b/dlls/winewayland.drv/wayland_surface.c @@ -1005,6 +1005,37 @@ err: return NULL; } +/********************************************************************** + * wayland_surface_attach_client + */ +void wayland_surface_attach_client(struct wayland_surface *surface, + struct wayland_client_surface *client) +{ + assert(!surface->client && client); + + if (client->wl_subsurface) wl_subsurface_destroy(client->wl_subsurface); + + /* Create a new subsurface that it is attached to the proper parent. */ + client->wl_subsurface = + wl_subcompositor_get_subsurface(process_wayland.wl_subcompositor, + client->wl_surface, + surface->wl_surface); + if (!client->wl_subsurface) + { + ERR("Failed to create client wl_subsurface\n"); + return; + } + /* Present contents independently of the parent surface. */ + wl_subsurface_set_desync(client->wl_subsurface); + + InterlockedIncrement(&client->ref); + surface->client = client; + + wayland_surface_reconfigure_client(surface); + /* Commit to apply subsurface positioning. */ + wl_surface_commit(surface->wl_surface); +} + static void dummy_buffer_release(void *data, struct wl_buffer *buffer) { struct wayland_shm_buffer *shm_buffer = data; diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index d553f06a420..36e8c550b23 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -273,6 +273,8 @@ void wayland_surface_coords_to_window(struct wayland_surface *surface, int *window_x, int *window_y); struct wayland_client_surface *wayland_surface_get_client(struct wayland_surface *surface); BOOL wayland_client_surface_release(struct wayland_client_surface *client); +void wayland_surface_attach_client(struct wayland_surface *surface, + struct wayland_client_surface *client); void wayland_surface_ensure_contents(struct wayland_surface *surface); void wayland_surface_set_title(struct wayland_surface *surface, LPCWSTR title); diff --git a/dlls/winewayland.drv/window.c b/dlls/winewayland.drv/window.c index 8453d73488c..ddac117ee7d 100644 --- a/dlls/winewayland.drv/window.c +++ b/dlls/winewayland.drv/window.c @@ -210,11 +210,14 @@ static void reapply_cursor_clipping(void) NtUserSetThreadDpiAwarenessContext(context); } +static void wayland_win_data_update_wayland_state(struct wayland_win_data *data); + static void wayland_win_data_update_wayland_surface(struct wayland_win_data *data) { struct wayland_surface *surface = data->wayland_surface; struct wayland_win_data *parent_data; enum wayland_surface_role role = WAYLAND_SURFACE_ROLE_NONE; + struct wayland_client_surface *client = NULL; WCHAR text[1024]; TRACE("hwnd=%p\n", data->hwnd); @@ -229,19 +232,31 @@ static void wayland_win_data_update_wayland_surface(struct wayland_win_data *dat } /* We can temporarily remove a role from a wayland surface and add it back, - * but we can't change a surface's role. - * TODO: Recreate the surface to allow role change. */ - if (surface && role && surface->role && role != surface->role) goto out; + * but we can't change a surface's role. */ + if (surface && role && surface->role && role != surface->role) + { + if (data->window_surface) + wayland_window_surface_update_wayland_surface(data->window_surface, NULL, NULL); + pthread_mutex_lock(&surface->mutex); + if (surface->client) client = wayland_surface_get_client(surface); + pthread_mutex_unlock(&surface->mutex); + wayland_surface_destroy(surface); + surface = NULL; + } /* Ensure that we have a wayland surface. */ - if (!surface && !(surface = wayland_surface_create(data->hwnd))) goto out; + if (!surface) + { + surface = wayland_surface_create(data->hwnd); + if (!surface) goto out; + } pthread_mutex_lock(&surface->mutex); if ((role == WAYLAND_SURFACE_ROLE_TOPLEVEL) != !!(surface->xdg_toplevel) || (role == WAYLAND_SURFACE_ROLE_SUBSURFACE) != !!(surface->wl_subsurface) || - (role == WAYLAND_SURFACE_ROLE_SUBSURFACE && - surface->parent_weak_ref && surface->parent_weak_ref->hwnd != parent_data->hwnd)) + (role == WAYLAND_SURFACE_ROLE_SUBSURFACE && surface->parent_weak_ref && + surface->parent_weak_ref != parent_data->wayland_surface)) { /* If we have a pre-existing surface ensure it has no role. */ if (data->wayland_surface) wayland_surface_clear_role(surface); @@ -266,6 +281,7 @@ static void wayland_win_data_update_wayland_surface(struct wayland_win_data *dat } wayland_win_data_get_config(data, &surface->window); + if (client) wayland_surface_attach_client(surface, client); pthread_mutex_unlock(&surface->mutex); @@ -280,6 +296,7 @@ static void wayland_win_data_update_wayland_surface(struct wayland_win_data *dat out: TRACE("hwnd=%p surface=%p=>%p\n", data->hwnd, data->wayland_surface, surface); data->wayland_surface = surface; + if (client) wayland_client_surface_release(client); } static void wayland_win_data_update_wayland_state(struct wayland_win_data *data) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6107
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> --- dlls/winewayland.drv/opengl.c | 26 +++++++++++++++----------- dlls/winewayland.drv/vulkan.c | 4 +++- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/dlls/winewayland.drv/opengl.c b/dlls/winewayland.drv/opengl.c index 994154968b3..eaf07cabc6a 100644 --- a/dlls/winewayland.drv/opengl.c +++ b/dlls/winewayland.drv/opengl.c @@ -267,21 +267,25 @@ static void wayland_gl_drawable_sync_size(struct wayland_gl_drawable *gl) static void wayland_gl_drawable_sync_surface_state(struct wayland_gl_drawable *gl) { struct wayland_surface *wayland_surface; + HWND hwnd = gl->hwnd; - if (!(wayland_surface = wayland_surface_lock_hwnd(gl->hwnd))) return; + while (hwnd && (wayland_surface = wayland_surface_lock_hwnd(hwnd))) + { + wayland_surface_ensure_contents(wayland_surface); - wayland_surface_ensure_contents(wayland_surface); + /* Handle any processed configure request, to ensure the related + * surface state is applied by the compositor. */ + if (wayland_surface->processing.serial && + wayland_surface->processing.processed && + wayland_surface_reconfigure(wayland_surface)) + { + wl_surface_commit(wayland_surface->wl_surface); + } - /* Handle any processed configure request, to ensure the related - * surface state is applied by the compositor. */ - if (wayland_surface->processing.serial && - wayland_surface->processing.processed && - wayland_surface_reconfigure(wayland_surface)) - { - wl_surface_commit(wayland_surface->wl_surface); + hwnd = wayland_surface->parent_weak_ref ? + wayland_surface->parent_weak_ref->hwnd : 0; + pthread_mutex_unlock(&wayland_surface->mutex); } - - pthread_mutex_unlock(&wayland_surface->mutex); } static BOOL wgl_context_make_current(struct wgl_context *ctx, HWND draw_hwnd, diff --git a/dlls/winewayland.drv/vulkan.c b/dlls/winewayland.drv/vulkan.c index 16084175013..337bfb823d0 100644 --- a/dlls/winewayland.drv/vulkan.c +++ b/dlls/winewayland.drv/vulkan.c @@ -140,7 +140,7 @@ static void wayland_vulkan_surface_presented(HWND hwnd, VkResult result) { struct wayland_surface *wayland_surface; - if ((wayland_surface = wayland_surface_lock_hwnd(hwnd))) + while (hwnd && (wayland_surface = wayland_surface_lock_hwnd(hwnd))) { wayland_surface_ensure_contents(wayland_surface); @@ -153,6 +153,8 @@ static void wayland_vulkan_surface_presented(HWND hwnd, VkResult result) wl_surface_commit(wayland_surface->wl_surface); } + hwnd = wayland_surface->parent_weak_ref ? + wayland_surface->parent_weak_ref->hwnd : 0; pthread_mutex_unlock(&wayland_surface->mutex); } } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6107
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> Wayland surfaces for child windows are currently only needed to render GL/VK contents, so instead of creating them for all child windows, create them only when GL/VK needs them. Furthermore, these child window Wayland surfaces will now be anchored directly to their top-level parent to avoid unnecessary nested subsurface chains. --- dlls/winewayland.drv/opengl.c | 2 +- dlls/winewayland.drv/vulkan.c | 2 +- dlls/winewayland.drv/waylanddrv.h | 1 + dlls/winewayland.drv/window.c | 83 +++++++++++++++++++++++++++++-- 4 files changed, 81 insertions(+), 7 deletions(-) diff --git a/dlls/winewayland.drv/opengl.c b/dlls/winewayland.drv/opengl.c index eaf07cabc6a..473ab4e23c3 100644 --- a/dlls/winewayland.drv/opengl.c +++ b/dlls/winewayland.drv/opengl.c @@ -165,7 +165,7 @@ static struct wayland_gl_drawable *wayland_gl_drawable_create(HWND hwnd, int for /* Get the client surface for the HWND. If don't have a wayland surface * (e.g., HWND_MESSAGE windows) just create a dummy surface to act as the * target render surface. */ - if ((wayland_surface = wayland_surface_lock_hwnd(hwnd))) + if ((wayland_surface = wayland_surface_lock_accel_hwnd(hwnd))) { gl->client = wayland_surface_get_client(wayland_surface); client_width = wayland_surface->window.client_rect.right - diff --git a/dlls/winewayland.drv/vulkan.c b/dlls/winewayland.drv/vulkan.c index 337bfb823d0..9d73d80b1c3 100644 --- a/dlls/winewayland.drv/vulkan.c +++ b/dlls/winewayland.drv/vulkan.c @@ -85,7 +85,7 @@ static VkResult wayland_vulkan_surface_create(HWND hwnd, VkInstance instance, Vk TRACE("%p %p %p %p\n", hwnd, instance, surface, private); - wayland_surface = wayland_surface_lock_hwnd(hwnd); + wayland_surface = wayland_surface_lock_accel_hwnd(hwnd); if (!wayland_surface) { ERR("Failed to find wayland surface for hwnd=%p\n", hwnd); diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index 36e8c550b23..9dc05a06e16 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -261,6 +261,7 @@ void wayland_surface_attach_shm(struct wayland_surface *surface, struct wayland_shm_buffer *shm_buffer, HRGN surface_damage_region); struct wayland_surface *wayland_surface_lock_hwnd(HWND hwnd); +struct wayland_surface *wayland_surface_lock_accel_hwnd(HWND hwnd); BOOL wayland_surface_reconfigure(struct wayland_surface *surface); BOOL wayland_surface_config_is_compatible(struct wayland_surface_config *conf, int width, int height, diff --git a/dlls/winewayland.drv/window.c b/dlls/winewayland.drv/window.c index ddac117ee7d..89c1ea7b8db 100644 --- a/dlls/winewayland.drv/window.c +++ b/dlls/winewayland.drv/window.c @@ -36,6 +36,7 @@ WINE_DEFAULT_DEBUG_CHANNEL(waylanddrv); +#define UWS_FORCE_CREATE 0x01 /********************************************************************** * get_win_monitor_dpi @@ -210,9 +211,36 @@ static void reapply_cursor_clipping(void) NtUserSetThreadDpiAwarenessContext(context); } +static struct wayland_win_data *wayland_win_data_get_top_parent(struct wayland_win_data *data) +{ + HWND top = NtUserGetAncestor(data->hwnd, GA_ROOT); + /* Don't return ourselves */ + return top == data->hwnd ? NULL : wayland_win_data_get_nolock(top); +} + +static BOOL wayland_win_data_needs_wayland_surface(struct wayland_win_data *data) +{ + HWND parent = NtUserGetAncestor(data->hwnd, GA_PARENT); + + /* We want a Wayland surface for toplevel windows. */ + if (!parent || parent == NtUserGetDesktopWindow()) return TRUE; + + /* We want to keep the Wayland surface if we have a client area subsurface. */ + if (data->wayland_surface) + { + BOOL has_client; + pthread_mutex_lock(&data->wayland_surface->mutex); + has_client = !!data->wayland_surface->client; + pthread_mutex_unlock(&data->wayland_surface->mutex); + if (has_client) return TRUE; + } + + return FALSE; +} + static void wayland_win_data_update_wayland_state(struct wayland_win_data *data); -static void wayland_win_data_update_wayland_surface(struct wayland_win_data *data) +static void wayland_win_data_update_wayland_surface(struct wayland_win_data *data, UINT flags) { struct wayland_surface *surface = data->wayland_surface; struct wayland_win_data *parent_data; @@ -220,11 +248,25 @@ static void wayland_win_data_update_wayland_surface(struct wayland_win_data *dat struct wayland_client_surface *client = NULL; WCHAR text[1024]; - TRACE("hwnd=%p\n", data->hwnd); + TRACE("hwnd=%p flags=0x%x\n", data->hwnd, flags); + + /* Destroy unused surfaces of child windows. */ + if (!wayland_win_data_needs_wayland_surface(data) && !(flags & UWS_FORCE_CREATE)) + { + if (surface) + { + if (data->window_surface) + wayland_window_surface_update_wayland_surface(data->window_surface, NULL, NULL); + wayland_surface_destroy(surface); + surface = NULL; + } + goto out; + } - if (NtUserGetWindowLongW(data->hwnd, GWL_STYLE) & WS_VISIBLE) + if (NtUserIsWindowVisible(data->hwnd)) { - parent_data = wayland_win_data_get_nolock(NtUserGetAncestor(data->hwnd, GA_PARENT)); + /* We anchor child windows to their toplevel parent window. */ + parent_data = wayland_win_data_get_top_parent(data); if (!parent_data) role = WAYLAND_SURFACE_ROLE_TOPLEVEL; else if (parent_data->wayland_surface) @@ -513,7 +555,7 @@ void WAYLAND_WindowPosChanged(HWND hwnd, HWND insert_after, UINT swp_flags, cons if (data->window_surface) window_surface_release(data->window_surface); data->window_surface = surface; - wayland_win_data_update_wayland_surface(data); + wayland_win_data_update_wayland_surface(data, 0); if (data->wayland_surface) wayland_win_data_update_wayland_state(data); wayland_win_data_release(data); @@ -762,6 +804,8 @@ void wayland_window_flush(HWND hwnd) /********************************************************************** * wayland_surface_lock_hwnd + * + * Get the locked surface for a window. */ struct wayland_surface *wayland_surface_lock_hwnd(HWND hwnd) { @@ -776,3 +820,32 @@ struct wayland_surface *wayland_surface_lock_hwnd(HWND hwnd) return surface; } + +/********************************************************************** + * wayland_surface_lock_accel_hwnd + * + * Get the locked surface for a window, creating the surface for a child + * on demand if needed, so accelerated content can be presented into it. + */ +struct wayland_surface *wayland_surface_lock_accel_hwnd(HWND hwnd) +{ + struct wayland_win_data *data = wayland_win_data_get(hwnd); + struct wayland_surface *surface; + + if (!data) return NULL; + + /* If the hwnd is a child window we can anchor to some toplevel, + * create a wayland surface for it to be the target of accelerated + * rendering. */ + if (!data->wayland_surface && wayland_win_data_get_top_parent(data)) + { + wayland_win_data_update_wayland_surface(data, UWS_FORCE_CREATE); + if (data->wayland_surface) wayland_win_data_update_wayland_state(data); + } + + if ((surface = data->wayland_surface)) pthread_mutex_lock(&surface->mutex); + + wayland_win_data_release(data); + + return surface; +} -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6107
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=147836 Your paranoid android. === debian11 (build log) === error: patch failed: dlls/winewayland.drv/wayland_surface.c:38 Task: Patch failed to apply === debian11b (build log) === error: patch failed: dlls/winewayland.drv/wayland_surface.c:38 Task: Patch failed to apply
On Sun Aug 18 11:58:27 2024 +0000, Rémi Bernon wrote:
I don't really like that weak ref, I think you should be able to make it work without it. Just use `NtUserGetAncestor( hwnd, GA_ROOT )` to get the toplevel window and lock its wayland_surface. Unfortunately this would bring us again to deadlock territory, since it reintroduces the `[surface.mutex, win_data_mutex]` locking sequence in `wayland_surface_reconfigure_subsurface()` (see https://gitlab.winehq.org/wine/wine/-/merge_requests/6107#note_76873).
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6107#note_79135
On Sun Aug 18 11:58:27 2024 +0000, Rémi Bernon wrote:
You can use GA_ROOT I think? Yes (with the "don't return ourselves" tweak), thanks.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6107#note_79136
On Sun Aug 18 11:59:19 2024 +0000, Rémi Bernon wrote:
Do we actually want to keep the wayland_surface, does the client need anything from it? Could we just keep the client surface with the window data instead? Eventually re-create it with a different top-level surface if needed when re-parenting? This would effectively lead to the scenario I mentioned in the second part of my reply in https://gitlab.winehq.org/wine/wine/-/merge_requests/6107#note_78535, so the same concerns from that comment still apply.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6107#note_79137
On Sun Aug 18 11:58:27 2024 +0000, Rémi Bernon wrote:
This is doing too many different things and is getting quite complicated. It would be better to have different helpers to create different kind of surfaces, and move the destruction to the callers (which would save the need for UWS_FORCE_CREATE). I haven't made changes explicitly in this direction in v4 (although the code became a bit simpler because of the other changes). However, I have experimented a bit and pushed a WIP commit (the last one) at https://gitlab.winehq.org/afrantzis/wine/-/commits/wip/child-accel-rendering.... If you like the direction in that commit or you have some other ideas I can work on incorporating these in the next MR revision.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6107#note_79138
On Mon Aug 19 15:13:15 2024 +0000, Alexandros Frantzis wrote:
changed this line in [version 4 of the diff](/wine/wine/-/merge_requests/6107/diffs?diff_id=127235&start_sha=1f6c5d5744fcd689749b080535a000bf0e27aeab#be6b1a6381414c8dae6f91e46942652a04a69f08_368_342) I have gone with an experimental `update_window_state` approach in v4. I am somewhat unsure because I ended up having to call that from `set_window_pos` and perhaps that has other implications. Plus I am seeing some GDI related artifacts with this (e.g., systray icon turns black in OIV.exe, in winewayland only), so this needs a bit more investigation.
Also note that in v4 I have pulled in your deadlock fix, since with the `update_window_state` changes seem to have made the deadlock more frequent/annoying for me. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6107#note_79139
v4: * Use the parent weak_ref to detect a required subsurface recreation due to parent surface recreation, removing the need for `UWS_FORCE_ROLE_UPDATE` (also enables moving child window update loop out of the driver, see next point). * Experiment with win32u `update_window_state` calls to trigger update driver children states when a parent changes (also removes `UWS_NO_UPDATE_CHILDREN`). * Pull in deadlock fix from https://gitlab.winehq.org/wine/wine/-/merge_requests/6025. (Some additional WIP simplications at https://gitlab.winehq.org/afrantzis/wine/-/commits/wip/child-accel-rendering...) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6107#note_79140
Plus I am seeing some GDI related artifacts with this (e.g., systray icon turns black in OIV.exe, in winewayland only), so this needs a bit more investigation.
Never mind, all the artifacts are gone when rebasing this branch on master, so some update there fixed the problem (likely one of the win32u/window_surface changes/fixes). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6107#note_79231
I think the need for a weak ref is a sign that the locking pattern has gone out of hand. Adding more complexity to it isn't going to do any good IMO. I've created https://gitlab.winehq.org/wine/wine/-/merge_requests/6323 as an alternative solution, that I think would be better. I've moved the client surfaces out of the wayland_surface and to the wayland_win_data, decoupling the surfaces used for rendering from the wayland_surface which purpose is windowing logic (it is still used as a rendering target for GDI drawing, but I think it's an implementation detail that we can ignore, and we could use some kind of client surface for it too, except that it needs to draw to non-client areas). Then I also got rid of the wayland_surface locks entirely, doing anything that needed it within the win_data lock. I don't think there was anything time consuming there, so it's probably fine to hold the global lock for longer. This allows us to access any toplevel win data as well while holding a win data for a given window. Currently I'm only using that to position any rendering surfaces over the windowing surface, including any child window that has an accelerated client surface. I think you can then do the same kind of thing to access any owner or parent toplevel win data, to implement wl_subsurface and attach a given wayland_surface (and its client surfaces) to another wayland_surface, for windowing purposes. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6107#note_79287
This merge request was closed by Alexandros Frantzis. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6107
Closing in favor of !6323. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6107#note_83307
participants (4)
-
Alexandros Frantzis -
Alexandros Frantzis (＠afrantzis)
-
Marvin -
Rémi Bernon