[PATCH v2 0/4] MR3234: winewayland.drv: part 5: window_surface flush optimizations
This MR optimizes and makes correctness improvements to the window_surface flush implementation: 1. Implement a buffer queue to avoid constantly allocating new buffers. 2. Take into account the bounds of the window_surface flush and track per-buffer damage to reduce copying from the window_surface. 3. Copy from the window_surface only pixel data that lies within the flushed bounds. Maintain the latest contents for pixels outside these bounds, by copying them from the latest valid buffer. 4. Communicate damaged surface regions to the compositor to allow it to perform more efficient pixel data transfers (e.g., texture uploads). Thanks! -- v2: winewayland.drv: Send surface damage region to the compositor. winewayland.drv: Update only the flushed bounds from the window_surface. winewayland.drv: Track damaged buffer regions. winewayland.drv: Use a buffer queue for window_surface. https://gitlab.winehq.org/wine/wine/-/merge_requests/3234
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> Create a buffer queue to hold the SHM buffers for each window_surface, to be able to reuse released buffers instead of constantly creating new ones. --- dlls/winewayland.drv/wayland_surface.c | 23 ++- dlls/winewayland.drv/waylanddrv.h | 6 +- dlls/winewayland.drv/window_surface.c | 193 +++++++++++++++++++++++-- 3 files changed, 204 insertions(+), 18 deletions(-) diff --git a/dlls/winewayland.drv/wayland_surface.c b/dlls/winewayland.drv/wayland_surface.c index ad30745fbc5..e200be769f7 100644 --- a/dlls/winewayland.drv/wayland_surface.c +++ b/dlls/winewayland.drv/wayland_surface.c @@ -234,13 +234,25 @@ void wayland_surface_attach_shm(struct wayland_surface *surface, } /********************************************************************** - * wayland_shm_buffer_destroy + * wayland_shm_buffer_ref * - * Destroys a SHM buffer. + * Increases the reference count of a SHM buffer. */ -void wayland_shm_buffer_destroy(struct wayland_shm_buffer *shm_buffer) +void wayland_shm_buffer_ref(struct wayland_shm_buffer *shm_buffer) { - TRACE("%p map=%p\n", shm_buffer, shm_buffer->map_data); + InterlockedIncrement(&shm_buffer->ref); +} + +/********************************************************************** + * wayland_shm_buffer_unref + * + * Decreases the reference count of a SHM buffer (and may destroy it). + */ +void wayland_shm_buffer_unref(struct wayland_shm_buffer *shm_buffer) +{ + if (InterlockedDecrement(&shm_buffer->ref) > 0) return; + + TRACE("destroying %p map=%p\n", shm_buffer, shm_buffer->map_data); if (shm_buffer->wl_buffer) wl_buffer_destroy(shm_buffer->wl_buffer); @@ -284,6 +296,7 @@ struct wayland_shm_buffer *wayland_shm_buffer_create(int width, int height, TRACE("%p %dx%d format=%d size=%d\n", shm_buffer, width, height, format, size); + shm_buffer->ref = 1; shm_buffer->width = width; shm_buffer->height = height; shm_buffer->map_size = size; @@ -340,6 +353,6 @@ struct wayland_shm_buffer *wayland_shm_buffer_create(int width, int height, err: if (fd >= 0) close(fd); if (handle) NtClose(handle); - if (shm_buffer) wayland_shm_buffer_destroy(shm_buffer); + if (shm_buffer) wayland_shm_buffer_unref(shm_buffer); return NULL; } diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index 0afc03f4147..5abc5ae2d43 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -107,10 +107,13 @@ struct wayland_surface struct wayland_shm_buffer { + struct wl_list link; struct wl_buffer *wl_buffer; int width, height; void *map_data; size_t map_size; + BOOL busy; + LONG ref; }; /********************************************************************** @@ -145,7 +148,8 @@ void wayland_surface_attach_shm(struct wayland_surface *surface, struct wayland_shm_buffer *wayland_shm_buffer_create(int width, int height, enum wl_shm_format format) DECLSPEC_HIDDEN; -void wayland_shm_buffer_destroy(struct wayland_shm_buffer *shm_buffer) DECLSPEC_HIDDEN; +void wayland_shm_buffer_ref(struct wayland_shm_buffer *shm_buffer) DECLSPEC_HIDDEN; +void wayland_shm_buffer_unref(struct wayland_shm_buffer *shm_buffer) DECLSPEC_HIDDEN; /********************************************************************** * Wayland window surface diff --git a/dlls/winewayland.drv/window_surface.c b/dlls/winewayland.drv/window_surface.c index ca613c667c6..76084a7c8ad 100644 --- a/dlls/winewayland.drv/window_surface.c +++ b/dlls/winewayland.drv/window_surface.c @@ -32,11 +32,20 @@ WINE_DEFAULT_DEBUG_CHANNEL(waylanddrv); +struct wayland_buffer_queue +{ + struct wl_event_queue *wl_event_queue; + struct wl_list buffer_list; + int width; + int height; +}; + struct wayland_window_surface { struct window_surface header; HWND hwnd; struct wayland_surface *wayland_surface; + struct wayland_buffer_queue *wayland_buffer_queue; RECT bounds; void *bits; pthread_mutex_t mutex; @@ -59,11 +68,149 @@ static void buffer_release(void *data, struct wl_buffer *buffer) { struct wayland_shm_buffer *shm_buffer = data; TRACE("shm_buffer=%p\n", shm_buffer); - wayland_shm_buffer_destroy(shm_buffer); + shm_buffer->busy = FALSE; + wayland_shm_buffer_unref(shm_buffer); } static const struct wl_buffer_listener buffer_listener = { buffer_release }; +/********************************************************************** + * wayland_buffer_queue_destroy + * + * Destroys a buffer queue and any contained buffers. + */ +static void wayland_buffer_queue_destroy(struct wayland_buffer_queue *queue) +{ + struct wayland_shm_buffer *shm_buffer, *next; + + wl_list_for_each_safe(shm_buffer, next, &queue->buffer_list, link) + { + wl_list_remove(&shm_buffer->link); + wl_list_init(&shm_buffer->link); + /* Since this buffer may still be busy, attach it to the per-process + * wl_event_queue to handle any future buffer release events. */ + wl_proxy_set_queue((struct wl_proxy *)shm_buffer->wl_buffer, + process_wayland.wl_event_queue); + wayland_shm_buffer_unref(shm_buffer); + } + + if (queue->wl_event_queue) + { + /* Dispatch the event queue before destruction to process any + * pending buffer release events. This is required after changing + * the buffer proxy event queue in the previous step, to avoid + * missing any events. */ + wl_display_dispatch_queue_pending(process_wayland.wl_display, + queue->wl_event_queue); + wl_event_queue_destroy(queue->wl_event_queue); + } + + free(queue); +} + +/********************************************************************** + * wayland_buffer_queue_create + * + * Creates a buffer queue containing buffers with the specified width and height. + */ +static struct wayland_buffer_queue *wayland_buffer_queue_create(int width, int height) +{ + struct wayland_buffer_queue *queue; + + queue = calloc(1, sizeof(*queue)); + if (!queue) goto err; + + queue->wl_event_queue = wl_display_create_queue(process_wayland.wl_display); + if (!queue->wl_event_queue) goto err; + queue->width = width; + queue->height = height; + + wl_list_init(&queue->buffer_list); + + return queue; + +err: + if (queue) wayland_buffer_queue_destroy(queue); + return NULL; +} + +/********************************************************************** + * wayland_buffer_queue_acquire_buffer + * + * Acquires a free buffer from the buffer queue. If no free buffers + * are available this function blocks until it can provide one. + * + * The returned buffer is marked as unavailable until committed to + * a surface and subsequently released by the compositor. + */ +static struct wayland_shm_buffer *wayland_buffer_queue_acquire_buffer(struct wayland_buffer_queue *queue) +{ + struct wayland_shm_buffer *shm_buffer; + + TRACE("queue=%p\n", queue); + + while (TRUE) + { + int nbuffers = 0; + + /* Dispatch any pending buffer release events. */ + wl_display_dispatch_queue_pending(process_wayland.wl_display, + queue->wl_event_queue); + + /* Search through our buffers to find an available one. */ + wl_list_for_each(shm_buffer, &queue->buffer_list, link) + { + if (!shm_buffer->busy) goto out; + nbuffers++; + } + + /* Dynamically create up to 3 buffers. */ + if (nbuffers < 3) + { + shm_buffer = wayland_shm_buffer_create(queue->width, queue->height, + WL_SHM_FORMAT_XRGB8888); + if (shm_buffer) + { + /* Buffer events go to their own queue so that we can dispatch + * them independently. */ + wl_proxy_set_queue((struct wl_proxy *) shm_buffer->wl_buffer, + queue->wl_event_queue); + wl_buffer_add_listener(shm_buffer->wl_buffer, &buffer_listener, + shm_buffer); + wl_list_insert(&queue->buffer_list, &shm_buffer->link); + goto out; + } + else if (nbuffers < 2) + { + /* If we failed to allocate a new buffer, but we have at least two + * buffers busy, there is a good chance the compositor will + * eventually release one of them, so dispatch events and wait + * below. Otherwise, give up and return a NULL buffer. */ + ERR(" => failed to acquire buffer\n"); + return NULL; + } + } + + /* We don't have any buffers available, so block waiting for a buffer + * release event. */ + if (wl_display_dispatch_queue(process_wayland.wl_display, + queue->wl_event_queue) == -1) + { + return NULL; + } + } + +out: + TRACE(" => %p %dx%d map=[%p, %p)\n", + shm_buffer, shm_buffer->width, shm_buffer->height, shm_buffer->map_data, + (unsigned char*)shm_buffer->map_data + shm_buffer->map_size); + + shm_buffer->busy = TRUE; + wayland_shm_buffer_ref(shm_buffer); + + return shm_buffer; +} + /*********************************************************************** * wayland_window_surface_lock */ @@ -119,33 +266,30 @@ static void wayland_window_surface_set_region(struct window_surface *window_surf static void wayland_window_surface_flush(struct window_surface *window_surface) { struct wayland_window_surface *wws = wayland_window_surface_cast(window_surface); - struct wayland_shm_buffer *shm_buffer; + struct wayland_shm_buffer *shm_buffer = NULL; BOOL flushed = FALSE; wayland_window_surface_lock(window_surface); if (IsRectEmpty(&wws->bounds)) goto done; - if (!wws->wayland_surface) + if (!wws->wayland_surface || !wws->wayland_buffer_queue) { - ERR("missing wayland surface, returning\n"); + ERR("missing wayland surface=%p or buffer_queue=%p, returning\n", + wws->wayland_surface, wws->wayland_buffer_queue); goto done; } TRACE("surface=%p hwnd=%p surface_rect=%s bounds=%s\n", wws, wws->hwnd, wine_dbgstr_rect(&wws->header.rect), wine_dbgstr_rect(&wws->bounds)); - shm_buffer = wayland_shm_buffer_create(wws->info.bmiHeader.biWidth, - abs(wws->info.bmiHeader.biHeight), - WL_SHM_FORMAT_XRGB8888); + shm_buffer = wayland_buffer_queue_acquire_buffer(wws->wayland_buffer_queue); if (!shm_buffer) { - ERR("failed to create Wayland SHM buffer, returning\n"); + ERR("failed to acquire Wayland SHM buffer, returning\n"); goto done; } - wl_buffer_add_listener(shm_buffer->wl_buffer, &buffer_listener, shm_buffer); - memcpy(shm_buffer->map_data, wws->bits, shm_buffer->map_size); pthread_mutex_lock(&wws->wayland_surface->mutex); @@ -158,13 +302,23 @@ static void wayland_window_surface_flush(struct window_surface *window_surface) else { TRACE("Wayland surface not configured yet, not flushing\n"); - wayland_shm_buffer_destroy(shm_buffer); } pthread_mutex_unlock(&wws->wayland_surface->mutex); wl_display_flush(process_wayland.wl_display); done: - if (flushed) reset_bounds(&wws->bounds); + if (flushed) + { + reset_bounds(&wws->bounds); + } + else + { + if (shm_buffer) + { + shm_buffer->busy = FALSE; + wayland_shm_buffer_unref(shm_buffer); + } + } wayland_window_surface_unlock(window_surface); } @@ -178,6 +332,8 @@ static void wayland_window_surface_destroy(struct window_surface *window_surface TRACE("surface=%p\n", wws); pthread_mutex_destroy(&wws->mutex); + if (wws->wayland_buffer_queue) + wayland_buffer_queue_destroy(wws->wayland_buffer_queue); free(wws->bits); free(wws); } @@ -254,5 +410,18 @@ void wayland_window_surface_update_wayland_surface(struct window_surface *window wws->wayland_surface = wayland_surface; + /* We only need a buffer queue if we have a surface to commit to. */ + if (wws->wayland_surface && !wws->wayland_buffer_queue) + { + wws->wayland_buffer_queue = + wayland_buffer_queue_create(wws->info.bmiHeader.biWidth, + abs(wws->info.bmiHeader.biHeight)); + } + else if (!wws->wayland_surface && wws->wayland_buffer_queue) + { + wayland_buffer_queue_destroy(wws->wayland_buffer_queue); + wws->wayland_buffer_queue = NULL; + } + wayland_window_surface_unlock(window_surface); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3234
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> Track the buffer regions that require an update since the last buffer commit, and copy only those parts from the window_surface to the buffer. --- dlls/winewayland.drv/wayland_surface.c | 12 ++- dlls/winewayland.drv/waylanddrv.h | 18 ++++ dlls/winewayland.drv/window_surface.c | 122 ++++++++++++++++++++++++- 3 files changed, 147 insertions(+), 5 deletions(-) diff --git a/dlls/winewayland.drv/wayland_surface.c b/dlls/winewayland.drv/wayland_surface.c index e200be769f7..cf49140d8bc 100644 --- a/dlls/winewayland.drv/wayland_surface.c +++ b/dlls/winewayland.drv/wayland_surface.c @@ -33,9 +33,6 @@ WINE_DEFAULT_DEBUG_CHANNEL(waylanddrv); -/* We only use 4 byte formats. */ -#define WINEWAYLAND_BYTES_PER_PIXEL 4 - /* Protects access to the user data of xdg_surface */ static pthread_mutex_t xdg_data_mutex = PTHREAD_MUTEX_INITIALIZER; @@ -258,6 +255,8 @@ void wayland_shm_buffer_unref(struct wayland_shm_buffer *shm_buffer) wl_buffer_destroy(shm_buffer->wl_buffer); if (shm_buffer->map_data) NtUnmapViewOfSection(GetCurrentProcess(), shm_buffer->map_data); + if (shm_buffer->damage_region) + NtGdiDeleteObjectApp(shm_buffer->damage_region); free(shm_buffer); } @@ -301,6 +300,13 @@ struct wayland_shm_buffer *wayland_shm_buffer_create(int width, int height, shm_buffer->height = height; shm_buffer->map_size = size; + shm_buffer->damage_region = NtGdiCreateRectRgn(0, 0, width, height); + if (!shm_buffer->damage_region) + { + ERR("Failed to create buffer damage region\n"); + goto err; + } + section_size.QuadPart = size; status = NtCreateSection(&handle, GENERIC_READ | SECTION_MAP_READ | SECTION_MAP_WRITE, diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index 5abc5ae2d43..992f504e3d7 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -32,11 +32,15 @@ #include "windef.h" #include "winbase.h" +#include "ntgdi.h" #include "wine/gdi_driver.h" #include "wine/rbtree.h" #include "unixlib.h" +/* We only use 4 byte formats. */ +#define WINEWAYLAND_BYTES_PER_PIXEL 4 + /********************************************************************** * Globals */ @@ -114,6 +118,7 @@ struct wayland_shm_buffer size_t map_size; BOOL busy; LONG ref; + HRGN damage_region; }; /********************************************************************** @@ -160,6 +165,19 @@ void wayland_window_surface_update_wayland_surface(struct window_surface *surfac struct wayland_surface *wayland_surface) DECLSPEC_HIDDEN; void wayland_window_flush(HWND hwnd) DECLSPEC_HIDDEN; +/********************************************************************** + * Helpers + */ + +static inline BOOL intersect_rect(RECT *dst, const RECT *src1, const RECT *src2) +{ + dst->left = max(src1->left, src2->left); + dst->top = max(src1->top, src2->top); + dst->right = min(src1->right, src2->right); + dst->bottom = min(src1->bottom, src2->bottom); + return !IsRectEmpty(dst); +} + /********************************************************************** * USER driver functions */ diff --git a/dlls/winewayland.drv/window_surface.c b/dlls/winewayland.drv/window_surface.c index 76084a7c8ad..d3bded821a7 100644 --- a/dlls/winewayland.drv/window_surface.c +++ b/dlls/winewayland.drv/window_surface.c @@ -211,6 +211,20 @@ out: return shm_buffer; } +/********************************************************************** + * wayland_buffer_queue_add_damage + */ +static void wayland_buffer_queue_add_damage(struct wayland_buffer_queue *queue, HRGN damage) +{ + struct wayland_shm_buffer *shm_buffer; + + wl_list_for_each(shm_buffer, &queue->buffer_list, link) + { + NtGdiCombineRgn(shm_buffer->damage_region, shm_buffer->damage_region, + damage, RGN_OR); + } +} + /*********************************************************************** * wayland_window_surface_lock */ @@ -260,6 +274,95 @@ static void wayland_window_surface_set_region(struct window_surface *window_surf /* TODO */ } +/********************************************************************** + * get_region_data + */ +static RGNDATA *get_region_data(HRGN region) +{ + RGNDATA *data; + DWORD size; + + if (!region) return NULL; + if (!(size = NtGdiGetRegionData(region, 0, NULL))) return NULL; + if (!(data = malloc(size))) return NULL; + if (!NtGdiGetRegionData(region, size, data)) + { + free(data); + return NULL; + } + + return data; +} + +/********************************************************************** + * copy_pixel_region + */ +static void copy_pixel_region(char *src_pixels, RECT *src_rect, + char *dst_pixels, RECT *dst_rect, + HRGN region) +{ + static const int bpp = WINEWAYLAND_BYTES_PER_PIXEL; + RGNDATA *rgndata = get_region_data(region); + RECT *rgn_rect; + RECT *rgn_rect_end; + int src_stride, dst_stride; + + if (!rgndata) return; + + src_stride = (src_rect->right - src_rect->left) * bpp; + dst_stride = (dst_rect->right - dst_rect->left) * bpp; + + rgn_rect = (RECT *)rgndata->Buffer; + rgn_rect_end = rgn_rect + rgndata->rdh.nCount; + + for (;rgn_rect < rgn_rect_end; rgn_rect++) + { + char *src, *dst; + int y, width_bytes, height; + RECT rc; + + TRACE("rect %s\n", wine_dbgstr_rect(rgn_rect)); + + if (!intersect_rect(&rc, rgn_rect, src_rect)) continue; + if (!intersect_rect(&rc, &rc, dst_rect)) continue; + + src = src_pixels + rc.top * src_stride + rc.left * bpp; + dst = dst_pixels + rc.top * dst_stride + rc.left * bpp; + width_bytes = (rc.right - rc.left) * bpp; + height = rc.bottom - rc.top; + + /* Fast path for full width rectangles. */ + if (width_bytes == src_stride && width_bytes == dst_stride) + { + memcpy(dst, src, height * width_bytes); + continue; + } + + for (y = 0; y < height; y++) + { + memcpy(dst, src, width_bytes); + src += src_stride; + dst += dst_stride; + } + } + + free(rgndata); +} + +/********************************************************************** + * wayland_window_surface_copy_to_buffer + */ +static void wayland_window_surface_copy_to_buffer(struct wayland_window_surface *wws, + struct wayland_shm_buffer *buffer, + HRGN region) +{ + RECT wws_rect = {0, 0, wws->info.bmiHeader.biWidth, + abs(wws->info.bmiHeader.biHeight)}; + RECT buffer_rect = {0, 0, buffer->width, buffer->height}; + TRACE("wws=%p buffer=%p\n", wws, buffer); + copy_pixel_region(wws->bits, &wws_rect, buffer->map_data, &buffer_rect, region); +} + /*********************************************************************** * wayland_window_surface_flush */ @@ -268,10 +371,12 @@ static void wayland_window_surface_flush(struct window_surface *window_surface) struct wayland_window_surface *wws = wayland_window_surface_cast(window_surface); struct wayland_shm_buffer *shm_buffer = NULL; BOOL flushed = FALSE; + RECT damage_rect; + HRGN surface_damage_region; wayland_window_surface_lock(window_surface); - if (IsRectEmpty(&wws->bounds)) goto done; + if (!intersect_rect(&damage_rect, &wws->header.rect, &wws->bounds)) goto done; if (!wws->wayland_surface || !wws->wayland_buffer_queue) { @@ -283,6 +388,17 @@ static void wayland_window_surface_flush(struct window_surface *window_surface) TRACE("surface=%p hwnd=%p surface_rect=%s bounds=%s\n", wws, wws->hwnd, wine_dbgstr_rect(&wws->header.rect), wine_dbgstr_rect(&wws->bounds)); + surface_damage_region = NtGdiCreateRectRgn(damage_rect.left, damage_rect.top, + damage_rect.right, damage_rect.bottom); + if (!surface_damage_region) + { + ERR("failed to create surface damage region\n"); + goto done; + } + + wayland_buffer_queue_add_damage(wws->wayland_buffer_queue, surface_damage_region); + NtGdiDeleteObjectApp(surface_damage_region); + shm_buffer = wayland_buffer_queue_acquire_buffer(wws->wayland_buffer_queue); if (!shm_buffer) { @@ -290,7 +406,7 @@ static void wayland_window_surface_flush(struct window_surface *window_surface) goto done; } - memcpy(shm_buffer->map_data, wws->bits, shm_buffer->map_size); + wayland_window_surface_copy_to_buffer(wws, shm_buffer, shm_buffer->damage_region); pthread_mutex_lock(&wws->wayland_surface->mutex); if (wws->wayland_surface->current_serial) @@ -306,6 +422,8 @@ static void wayland_window_surface_flush(struct window_surface *window_surface) pthread_mutex_unlock(&wws->wayland_surface->mutex); wl_display_flush(process_wayland.wl_display); + NtGdiSetRectRgn(shm_buffer->damage_region, 0, 0, 0, 0); + done: if (flushed) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3234
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> When flushing a window_surface, copy from the window_surface only the pixel data contained in the flushed bounds. If any other pixel data are needed, get them from the latest window buffer for the wayland surface, to ensure the data are valid and unchanged. --- dlls/winewayland.drv/wayland_surface.c | 3 ++ dlls/winewayland.drv/waylanddrv.h | 1 + dlls/winewayland.drv/window_surface.c | 55 ++++++++++++++++++++++++-- 3 files changed, 56 insertions(+), 3 deletions(-) diff --git a/dlls/winewayland.drv/wayland_surface.c b/dlls/winewayland.drv/wayland_surface.c index cf49140d8bc..ed9ae4ee0e4 100644 --- a/dlls/winewayland.drv/wayland_surface.c +++ b/dlls/winewayland.drv/wayland_surface.c @@ -148,6 +148,9 @@ void wayland_surface_destroy(struct wayland_surface *surface) pthread_mutex_unlock(&surface->mutex); pthread_mutex_unlock(&xdg_data_mutex); + if (surface->latest_window_buffer) + wayland_shm_buffer_unref(surface->latest_window_buffer); + wl_display_flush(process_wayland.wl_display); pthread_mutex_destroy(&surface->mutex); diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index 992f504e3d7..909c8ef5b7b 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -107,6 +107,7 @@ struct wayland_surface struct xdg_toplevel *xdg_toplevel; pthread_mutex_t mutex; uint32_t current_serial; + struct wayland_shm_buffer *latest_window_buffer; }; struct wayland_shm_buffer diff --git a/dlls/winewayland.drv/window_surface.c b/dlls/winewayland.drv/window_surface.c index d3bded821a7..f88b1eb6010 100644 --- a/dlls/winewayland.drv/window_surface.c +++ b/dlls/winewayland.drv/window_surface.c @@ -363,6 +363,16 @@ static void wayland_window_surface_copy_to_buffer(struct wayland_window_surface copy_pixel_region(wws->bits, &wws_rect, buffer->map_data, &buffer_rect, region); } +static void wayland_shm_buffer_copy(struct wayland_shm_buffer *src, + struct wayland_shm_buffer *dst, + HRGN region) +{ + RECT src_rect = {0, 0, src->width, src->height}; + RECT dst_rect = {0, 0, dst->width, dst->height}; + TRACE("src=%p dst=%p\n", src, dst); + copy_pixel_region(src->map_data, &src_rect, dst->map_data, &dst_rect, region); +} + /*********************************************************************** * wayland_window_surface_flush */ @@ -372,7 +382,8 @@ static void wayland_window_surface_flush(struct window_surface *window_surface) struct wayland_shm_buffer *shm_buffer = NULL; BOOL flushed = FALSE; RECT damage_rect; - HRGN surface_damage_region; + HRGN surface_damage_region = NULL; + HRGN copy_from_window_region; wayland_window_surface_lock(window_surface); @@ -397,7 +408,6 @@ static void wayland_window_surface_flush(struct window_surface *window_surface) } wayland_buffer_queue_add_damage(wws->wayland_buffer_queue, surface_damage_region); - NtGdiDeleteObjectApp(surface_damage_region); shm_buffer = wayland_buffer_queue_acquire_buffer(wws->wayland_buffer_queue); if (!shm_buffer) @@ -406,7 +416,38 @@ static void wayland_window_surface_flush(struct window_surface *window_surface) goto done; } - wayland_window_surface_copy_to_buffer(wws, shm_buffer, shm_buffer->damage_region); + if (wws->wayland_surface->latest_window_buffer) + { + TRACE("latest_window_buffer=%p\n", wws->wayland_surface->latest_window_buffer); + /* If we have a latest buffer, use it as the source of all pixel + * data that are not contained in the bounds of the flush... */ + if (wws->wayland_surface->latest_window_buffer != shm_buffer) + { + HRGN copy_from_latest_region = NtGdiCreateRectRgn(0, 0, 0, 0); + if (!copy_from_latest_region) + { + ERR("failed to create copy_from_latest region\n"); + goto done; + } + NtGdiCombineRgn(copy_from_latest_region, shm_buffer->damage_region, + surface_damage_region, RGN_DIFF); + wayland_shm_buffer_copy(wws->wayland_surface->latest_window_buffer, + shm_buffer, copy_from_latest_region); + NtGdiDeleteObjectApp(copy_from_latest_region); + } + /* ... and use the window_surface as the source of pixel data contained + * in the flush bounds. */ + copy_from_window_region = surface_damage_region; + } + else + { + TRACE("latest_window_buffer=NULL\n"); + /* If we don't have a latest buffer, use the window_surface as + * the source of all pixel data. */ + copy_from_window_region = shm_buffer->damage_region; + } + + wayland_window_surface_copy_to_buffer(wws, shm_buffer, copy_from_window_region); pthread_mutex_lock(&wws->wayland_surface->mutex); if (wws->wayland_surface->current_serial) @@ -423,6 +464,13 @@ static void wayland_window_surface_flush(struct window_surface *window_surface) wl_display_flush(process_wayland.wl_display); NtGdiSetRectRgn(shm_buffer->damage_region, 0, 0, 0, 0); + /* Update the latest window buffer for the wayland surface. Note that we + * only care whether the buffer contains the latest window contents, + * it's irrelevant if it was actually committed or not. */ + wayland_shm_buffer_ref(shm_buffer); + if (wws->wayland_surface->latest_window_buffer) + wayland_shm_buffer_unref(wws->wayland_surface->latest_window_buffer); + wws->wayland_surface->latest_window_buffer = shm_buffer; done: if (flushed) @@ -437,6 +485,7 @@ done: wayland_shm_buffer_unref(shm_buffer); } } + if (surface_damage_region) NtGdiDeleteObjectApp(surface_damage_region); wayland_window_surface_unlock(window_surface); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3234
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> Send the surface damage region to the compositor, to enable it to optimize pixel data transfers. --- dlls/winewayland.drv/wayland_surface.c | 26 +++++++++++++++++++++++--- dlls/winewayland.drv/waylanddrv.h | 5 ++++- dlls/winewayland.drv/window_surface.c | 5 +++-- 3 files changed, 30 insertions(+), 6 deletions(-) diff --git a/dlls/winewayland.drv/wayland_surface.c b/dlls/winewayland.drv/wayland_surface.c index ed9ae4ee0e4..13764a2e645 100644 --- a/dlls/winewayland.drv/wayland_surface.c +++ b/dlls/winewayland.drv/wayland_surface.c @@ -223,14 +223,34 @@ void wayland_surface_clear_role(struct wayland_surface *surface) * Attaches a SHM buffer to a wayland surface. */ void wayland_surface_attach_shm(struct wayland_surface *surface, - struct wayland_shm_buffer *shm_buffer) + struct wayland_shm_buffer *shm_buffer, + HRGN surface_damage_region) { + RGNDATA *surface_damage; + TRACE("surface=%p shm_buffer=%p (%dx%d)\n", surface, shm_buffer, shm_buffer->width, shm_buffer->height); wl_surface_attach(surface->wl_surface, shm_buffer->wl_buffer, 0, 0); - wl_surface_damage_buffer(surface->wl_surface, 0, 0, - shm_buffer->width, shm_buffer->height); + + /* Add surface damage, i.e., which parts of the surface have changed since + * the last surface commit. Note that this is different from the buffer + * damage region. */ + surface_damage = get_region_data(surface_damage_region); + if (surface_damage) + { + RECT *rgn_rect = (RECT *)surface_damage->Buffer; + RECT *rgn_rect_end = rgn_rect + surface_damage->rdh.nCount; + + for (;rgn_rect < rgn_rect_end; rgn_rect++) + { + wl_surface_damage_buffer(surface->wl_surface, + rgn_rect->left, rgn_rect->top, + rgn_rect->right - rgn_rect->left, + rgn_rect->bottom - rgn_rect->top); + } + free(surface_damage); + } } /********************************************************************** diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index 909c8ef5b7b..0fd2a7a6c77 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -146,7 +146,8 @@ void wayland_surface_destroy(struct wayland_surface *surface) DECLSPEC_HIDDEN; void wayland_surface_make_toplevel(struct wayland_surface *surface) DECLSPEC_HIDDEN; void wayland_surface_clear_role(struct wayland_surface *surface) DECLSPEC_HIDDEN; void wayland_surface_attach_shm(struct wayland_surface *surface, - struct wayland_shm_buffer *shm_buffer) DECLSPEC_HIDDEN; + struct wayland_shm_buffer *shm_buffer, + HRGN surface_damage_region) DECLSPEC_HIDDEN; /********************************************************************** * Wayland SHM buffer @@ -179,6 +180,8 @@ static inline BOOL intersect_rect(RECT *dst, const RECT *src1, const RECT *src2) return !IsRectEmpty(dst); } +RGNDATA *get_region_data(HRGN region) DECLSPEC_HIDDEN; + /********************************************************************** * USER driver functions */ diff --git a/dlls/winewayland.drv/window_surface.c b/dlls/winewayland.drv/window_surface.c index f88b1eb6010..e1d8ab68ddf 100644 --- a/dlls/winewayland.drv/window_surface.c +++ b/dlls/winewayland.drv/window_surface.c @@ -277,7 +277,7 @@ static void wayland_window_surface_set_region(struct window_surface *window_surf /********************************************************************** * get_region_data */ -static RGNDATA *get_region_data(HRGN region) +RGNDATA *get_region_data(HRGN region) { RGNDATA *data; DWORD size; @@ -452,7 +452,8 @@ static void wayland_window_surface_flush(struct window_surface *window_surface) pthread_mutex_lock(&wws->wayland_surface->mutex); if (wws->wayland_surface->current_serial) { - wayland_surface_attach_shm(wws->wayland_surface, shm_buffer); + wayland_surface_attach_shm(wws->wayland_surface, shm_buffer, + surface_damage_region); wl_surface_commit(wws->wayland_surface->wl_surface); flushed = TRUE; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3234
Rémi Bernon (@rbernon) commented about dlls/winewayland.drv/window_surface.c:
else { TRACE("Wayland surface not configured yet, not flushing\n"); - wayland_shm_buffer_destroy(shm_buffer);
What about `shm_buffer->busy = FALSE;` here? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3234#note_37849
Rémi Bernon (@rbernon) commented about dlls/winewayland.drv/window_surface.c:
+ shm_buffer = wayland_buffer_queue_acquire_buffer(wws->wayland_buffer_queue); if (!shm_buffer) { - ERR("failed to create Wayland SHM buffer, returning\n"); + ERR("failed to acquire Wayland SHM buffer, returning\n"); goto done; }
- wl_buffer_add_listener(shm_buffer->wl_buffer, &buffer_listener, shm_buffer); - memcpy(shm_buffer->map_data, wws->bits, shm_buffer->map_size);
pthread_mutex_lock(&wws->wayland_surface->mutex); if (wws->wayland_surface->current_serial) { wayland_surface_attach_shm(wws->wayland_surface, shm_buffer); Then changing `wayland_surface_attach_shm` to increase the reference count inside, instead of sinking the reference, which is rather unusual in Wine,
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3234#note_37850
Rémi Bernon (@rbernon) commented about dlls/winewayland.drv/window_surface.c:
wl_display_flush(process_wayland.wl_display);
done: - if (flushed) reset_bounds(&wws->bounds); + if (flushed) + { + reset_bounds(&wws->bounds); + } + else + { + if (shm_buffer) + { + shm_buffer->busy = FALSE; + wayland_shm_buffer_unref(shm_buffer); + } + }
And here, keeping `if (flushed) reset_bounds(&wws->bounds);` and adding next `if (shm_buffer) wayland_shm_buffer_unref(shm_buffer);` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3234#note_37851
Rémi Bernon (@rbernon) commented about dlls/winewayland.drv/window_surface.c:
wl_display_flush(process_wayland.wl_display);
NtGdiSetRectRgn(shm_buffer->damage_region, 0, 0, 0, 0); + /* Update the latest window buffer for the wayland surface. Note that we + * only care whether the buffer contains the latest window contents, + * it's irrelevant if it was actually committed or not. */ + wayland_shm_buffer_ref(shm_buffer); + if (wws->wayland_surface->latest_window_buffer) + wayland_shm_buffer_unref(wws->wayland_surface->latest_window_buffer); + wws->wayland_surface->latest_window_buffer = shm_buffer;
This is also a bit unusual, I'd rather do: ```suggestion:-3+0 if (wws->wayland_surface->latest_window_buffer) wayland_shm_buffer_unref(wws->wayland_surface->latest_window_buffer); wayland_shm_buffer_ref((wws->wayland_surface->latest_window_buffer = shm_buffer)); ``` Or on separate lines if you prefer, but adding the ref count just next to the assignment. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3234#note_37852
I only had a quick look so far, but it seems okay already. I wondered if there was a way to use more of NtGdi function to avoid doing the pixel copy by hand, but I don't know very much the API. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3234#note_37853
On Tue Jul 4 19:02:44 2023 +0000, Rémi Bernon wrote:
This is also a bit unusual, I'd rather do: ```suggestion:-3+0 if (wws->wayland_surface->latest_window_buffer) wayland_shm_buffer_unref(wws->wayland_surface->latest_window_buffer); wayland_shm_buffer_ref((wws->wayland_surface->latest_window_buffer = shm_buffer)); ``` Or on separate lines if you prefer, but adding the ref count just next to the assignment. I guess this was maybe supposed to cover the case when `last == shm_buffer`, but as with the changes I suggested above you would still own a reference over then entire function it should be fine.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3234#note_37858
participants (3)
-
Alexandros Frantzis -
Alexandros Frantzis (＠afrantzis)
-
Rémi Bernon