[PATCH v4 0/5] MR6386: winewayland: Reduce window surface buffer queue recreations.
-- v4: winewayland: Avoid recreating window surface buffer queues. winewayland: Create the window surface buffer queue unconditionally. winewayland: Set the window viewport source rectangle. winewayland: Rename surface buffer size to content size. winewayland: Require the wp_viewporter protocol. https://gitlab.winehq.org/wine/wine/-/merge_requests/6386
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winewayland.drv/wayland.c | 5 +++ dlls/winewayland.drv/wayland_pointer.c | 16 ++++----- dlls/winewayland.drv/wayland_surface.c | 49 +++++++++++--------------- 3 files changed, 34 insertions(+), 36 deletions(-) diff --git a/dlls/winewayland.drv/wayland.c b/dlls/winewayland.drv/wayland.c index c94b35e173c..35dbcb5f0ef 100644 --- a/dlls/winewayland.drv/wayland.c +++ b/dlls/winewayland.drv/wayland.c @@ -269,6 +269,11 @@ BOOL wayland_process_init(void) ERR("Wayland compositor doesn't support wl_subcompositor\n"); return FALSE; } + if (!process_wayland.wp_viewporter) + { + ERR("Wayland compositor doesn't support wp_viewporter\n"); + return FALSE; + } if (!process_wayland.zwp_pointer_constraints_v1) { ERR("Wayland compositor doesn't support zwp_pointer_constraints_v1\n"); diff --git a/dlls/winewayland.drv/wayland_pointer.c b/dlls/winewayland.drv/wayland_pointer.c index 1d8acaeabd2..456176e25c7 100644 --- a/dlls/winewayland.drv/wayland_pointer.c +++ b/dlls/winewayland.drv/wayland_pointer.c @@ -611,13 +611,16 @@ static void wayland_pointer_update_cursor_surface(double scale) } } - if (!cursor->wp_viewport && process_wayland.wp_viewporter) + if (!cursor->wp_viewport) { cursor->wp_viewport = wp_viewporter_get_viewport(process_wayland.wp_viewporter, cursor->wl_surface); if (!cursor->wp_viewport) - WARN("Failed to create wp_viewport for cursor\n"); + { + ERR("Failed to create wp_viewport for cursor\n"); + goto clear_cursor; + } } /* Commit the cursor buffer to the cursor surface. */ @@ -631,12 +634,9 @@ static void wayland_pointer_update_cursor_surface(double scale) * scale. Note that setting the viewport destination overrides * the buffer scale, so it's fine to set both. */ wl_surface_set_buffer_scale(cursor->wl_surface, round(scale)); - if (cursor->wp_viewport) - { - wp_viewport_set_destination(cursor->wp_viewport, - round(cursor->shm_buffer->width / scale), - round(cursor->shm_buffer->height / scale)); - } + wp_viewport_set_destination(cursor->wp_viewport, + round(cursor->shm_buffer->width / scale), + round(cursor->shm_buffer->height / scale)); wl_surface_commit(cursor->wl_surface); return; diff --git a/dlls/winewayland.drv/wayland_surface.c b/dlls/winewayland.drv/wayland_surface.c index a2699d66e51..906b33dd358 100644 --- a/dlls/winewayland.drv/wayland_surface.c +++ b/dlls/winewayland.drv/wayland_surface.c @@ -163,11 +163,13 @@ struct wayland_surface *wayland_surface_create(HWND hwnd) } wl_surface_set_user_data(surface->wl_surface, hwnd); - if (process_wayland.wp_viewporter) + surface->wp_viewport = + wp_viewporter_get_viewport(process_wayland.wp_viewporter, + surface->wl_surface); + if (!surface->wp_viewport) { - surface->wp_viewport = - wp_viewporter_get_viewport(process_wayland.wp_viewporter, - surface->wl_surface); + ERR("Failed to create wp_viewport Wayland surface\n"); + goto err; } surface->window.scale = 1.0; @@ -474,13 +476,10 @@ static void wayland_surface_reconfigure_size(struct wayland_surface *surface, { TRACE("hwnd=%p size=%dx%d\n", surface->hwnd, width, height); - if (surface->wp_viewport) - { - if (width != 0 && height != 0) - wp_viewport_set_destination(surface->wp_viewport, width, height); - else - wp_viewport_set_destination(surface->wp_viewport, -1, -1); - } + if (width != 0 && height != 0) + wp_viewport_set_destination(surface->wp_viewport, width, height); + else + wp_viewport_set_destination(surface->wp_viewport, -1, -1); } /********************************************************************** @@ -511,19 +510,11 @@ static void wayland_surface_reconfigure_client(struct wayland_surface *surface) wl_subsurface_set_position(surface->client->wl_subsurface, x, y); - if (surface->client->wp_viewport) - { - if (width != 0 && height != 0) - { - wp_viewport_set_destination(surface->client->wp_viewport, - width, height); - } - else - { - /* We can't have a 0x0 destination, use 1x1 instead. */ - wp_viewport_set_destination(surface->client->wp_viewport, 1, 1); - } - } + if (width != 0 && height != 0) + wp_viewport_set_destination(surface->client->wp_viewport, + width, height); + else /* We can't have a 0x0 destination, use 1x1 instead. */ + wp_viewport_set_destination(surface->client->wp_viewport, 1, 1); wl_surface_commit(surface->client->wl_surface); @@ -823,11 +814,13 @@ struct wayland_client_surface *wayland_surface_get_client(struct wayland_surface /* Present contents independently of the parent surface. */ wl_subsurface_set_desync(surface->client->wl_subsurface); - if (process_wayland.wp_viewporter) + surface->client->wp_viewport = + wp_viewporter_get_viewport(process_wayland.wp_viewporter, + surface->client->wl_surface); + if (!surface->client->wp_viewport) { - surface->client->wp_viewport = - wp_viewporter_get_viewport(process_wayland.wp_viewporter, - surface->client->wl_surface); + ERR("Failed to create client wp_viewport\n"); + goto err; } wayland_surface_reconfigure_client(surface); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6386
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> --- dlls/winewayland.drv/wayland_surface.c | 13 +++++++------ dlls/winewayland.drv/waylanddrv.h | 2 +- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/dlls/winewayland.drv/wayland_surface.c b/dlls/winewayland.drv/wayland_surface.c index 906b33dd358..cfeb44508e8 100644 --- a/dlls/winewayland.drv/wayland_surface.c +++ b/dlls/winewayland.drv/wayland_surface.c @@ -304,8 +304,8 @@ void wayland_surface_clear_role(struct wayland_surface *surface) wl_surface_attach(surface->wl_surface, NULL, 0, 0); wl_surface_commit(surface->wl_surface); - surface->buffer_width = 0; - surface->buffer_height = 0; + surface->content_width = 0; + surface->content_height = 0; wl_display_flush(process_wayland.wl_display); } @@ -323,6 +323,7 @@ void wayland_surface_attach_shm(struct wayland_surface *surface, HRGN surface_damage_region) { RGNDATA *surface_damage; + int win_width, win_height; TRACE("surface=%p shm_buffer=%p (%dx%d)\n", surface, shm_buffer, shm_buffer->width, shm_buffer->height); @@ -351,8 +352,8 @@ void wayland_surface_attach_shm(struct wayland_surface *surface, free(surface_damage); } - surface->buffer_width = shm_buffer->width; - surface->buffer_height = shm_buffer->height; + surface->content_width = shm_buffer->width; + surface->content_height = shm_buffer->height; } /********************************************************************** @@ -866,8 +867,8 @@ void wayland_surface_ensure_contents(struct wayland_surface *surface) width = surface->window.rect.right - surface->window.rect.left; height = surface->window.rect.bottom - surface->window.rect.top; needs_contents = surface->window.visible && - (surface->buffer_width != width || - surface->buffer_height != height); + (surface->content_width != width || + surface->content_height != height); TRACE("surface=%p hwnd=%p needs_contents=%d\n", surface, surface->hwnd, needs_contents); diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index 40f86526cd2..e5db629a5b1 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -202,7 +202,7 @@ struct wayland_surface BOOL resizing; struct wayland_window_config window; struct wayland_client_surface *client; - int buffer_width, buffer_height; + int content_width, content_height; HCURSOR hcursor; }; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6386
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winewayland.drv/wayland_surface.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/dlls/winewayland.drv/wayland_surface.c b/dlls/winewayland.drv/wayland_surface.c index cfeb44508e8..5ca8b010746 100644 --- a/dlls/winewayland.drv/wayland_surface.c +++ b/dlls/winewayland.drv/wayland_surface.c @@ -352,8 +352,22 @@ void wayland_surface_attach_shm(struct wayland_surface *surface, free(surface_damage); } - surface->content_width = shm_buffer->width; - surface->content_height = shm_buffer->height; + win_width = surface->window.rect.right - surface->window.rect.left; + win_height = surface->window.rect.bottom - surface->window.rect.top; + + /* It is an error to specify a wp_viewporter source rectangle that + * is partially or completely outside of the wl_buffe. + * 0 is also an invalid width / height value so use 1x1 instead. + */ + win_width = max(1, min(win_width, shm_buffer->width)); + win_height = max(1, min(win_height, shm_buffer->height)); + + wp_viewport_set_source(surface->wp_viewport, 0, 0, + wl_fixed_from_int(win_width), + wl_fixed_from_int(win_height)); + + surface->content_width = win_width; + surface->content_height = win_height; } /********************************************************************** -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6386
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winewayland.drv/window_surface.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/dlls/winewayland.drv/window_surface.c b/dlls/winewayland.drv/window_surface.c index 570c186861b..ddd4fb45976 100644 --- a/dlls/winewayland.drv/window_surface.c +++ b/dlls/winewayland.drv/window_surface.c @@ -329,10 +329,9 @@ static BOOL wayland_window_surface_flush(struct window_surface *window_surface, HRGN surface_damage_region = NULL; HRGN copy_from_window_region; - if (!wws->wayland_surface || !wws->wayland_buffer_queue) + if (!wws->wayland_surface) { - ERR("missing wayland surface=%p or buffer_queue=%p, returning\n", - wws->wayland_surface, wws->wayland_buffer_queue); + ERR("missing wayland surface=%p, returning\n", wws->wayland_surface); goto done; } @@ -423,8 +422,7 @@ static void wayland_window_surface_destroy(struct window_surface *window_surface TRACE("surface=%p\n", wws); - if (wws->wayland_buffer_queue) - wayland_buffer_queue_destroy(wws->wayland_buffer_queue); + wayland_buffer_queue_destroy(wws->wayland_buffer_queue); } static const struct window_surface_funcs wayland_window_surface_funcs = @@ -444,6 +442,7 @@ static struct window_surface *wayland_window_surface_create(HWND hwnd, const REC struct wayland_window_surface *wws; int width = rect->right - rect->left; int height = rect->bottom - rect->top; + struct window_surface *window_surface; TRACE("hwnd %p rect %s\n", hwnd, wine_dbgstr_rect(rect)); @@ -456,7 +455,13 @@ static struct window_surface *wayland_window_surface_create(HWND hwnd, const REC info->bmiHeader.biSizeImage = width * height * 4; info->bmiHeader.biCompression = BI_RGB; - return window_surface_create(sizeof(*wws), &wayland_window_surface_funcs, hwnd, rect, info, 0); + if ((window_surface = window_surface_create(sizeof(*wws), &wayland_window_surface_funcs, hwnd, rect, info, 0))) + { + struct wayland_window_surface *wws = wayland_window_surface_cast(window_surface); + wws->wayland_buffer_queue = wayland_buffer_queue_create(width, height); + } + + return window_surface; } /*********************************************************************** -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6386
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winewayland.drv/window_surface.c | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/dlls/winewayland.drv/window_surface.c b/dlls/winewayland.drv/window_surface.c index ddd4fb45976..7e4ea202e89 100644 --- a/dlls/winewayland.drv/window_surface.c +++ b/dlls/winewayland.drv/window_surface.c @@ -482,21 +482,6 @@ void wayland_window_surface_update_wayland_surface(struct window_surface *window wine_dbgstr_rect(visible_rect), wayland_surface); wws->wayland_surface = wayland_surface; - - if (wws->wayland_buffer_queue) - { - wayland_buffer_queue_destroy(wws->wayland_buffer_queue); - wws->wayland_buffer_queue = NULL; - } - - /* We only need a buffer queue if we have a surface to commit to. */ - if (wws->wayland_surface) - { - wws->wayland_buffer_queue = - wayland_buffer_queue_create(visible_rect->right - visible_rect->left, - visible_rect->bottom - visible_rect->top); - } - window_surface_unlock(window_surface); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6386
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=148075 Your paranoid android. === debian11 (build log) === error: patch failed: dlls/winewayland.drv/wayland.c:269 error: patch failed: dlls/winewayland.drv/wayland_pointer.c:611 error: patch failed: dlls/winewayland.drv/wayland_surface.c:163 error: patch failed: dlls/winewayland.drv/wayland_surface.c:352 error: patch failed: dlls/winewayland.drv/window_surface.c:329 error: patch failed: dlls/winewayland.drv/window_surface.c:482 Task: Patch failed to apply === debian11b (build log) === error: patch failed: dlls/winewayland.drv/wayland.c:269 error: patch failed: dlls/winewayland.drv/wayland_pointer.c:611 error: patch failed: dlls/winewayland.drv/wayland_surface.c:163 error: patch failed: dlls/winewayland.drv/wayland_surface.c:352 error: patch failed: dlls/winewayland.drv/window_surface.c:329 error: patch failed: dlls/winewayland.drv/window_surface.c:482 Task: Patch failed to apply
On Thu Aug 29 18:00:42 2024 +0000, Alexandros Frantzis wrote:
Thanks, I have some more proposed changes/fixes at https://gitlab.winehq.org/afrantzis/wine/-/commits/wip/wayland-buffer-queue . This MR effectively makes `wp_viewport` a requirement for the Wayland driver, even for non-scaled scenarios, so I would say let's check for it at startup like we do with other protocols and remove further runtime checks (can be done in a follow-up MR). Thanks, I've split and included them.
I made the protocol a requirement before actually using it for buffer cropping, because it would break otherwise somewhere within this MR. Made a couple of other tweaks, kept the 0x0 check but using min(1, ...) instead. I'm not sure it's really worth it, I would say that it's not supposed to be happening. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6386#note_80424
This merge request was approved by Rémi Bernon. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6386
participants (3)
-
Alexandros Frantzis -
Marvin -
Rémi Bernon