[PATCH 0/9] MR6374: winewayland: Get rid of the wayland surface lock.
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 a2699d66e51..c96ed7bde18 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) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6374
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winewayland.drv/opengl.c | 22 +--------------------- dlls/winewayland.drv/vulkan.c | 18 +----------------- dlls/winewayland.drv/waylanddrv.h | 2 ++ dlls/winewayland.drv/window.c | 21 +++++++++++++++++++++ 4 files changed, 25 insertions(+), 38 deletions(-) diff --git a/dlls/winewayland.drv/opengl.c b/dlls/winewayland.drv/opengl.c index fd13bdbb406..b99d80cbcf2 100644 --- a/dlls/winewayland.drv/opengl.c +++ b/dlls/winewayland.drv/opengl.c @@ -308,26 +308,6 @@ 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; - - if (!(wayland_surface = wayland_surface_lock_hwnd(gl->hwnd))) return; - - 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); - } - - pthread_mutex_unlock(&wayland_surface->mutex); -} - static BOOL wgl_context_make_current(struct wgl_context *ctx, HDC draw_hdc, HDC read_hdc) { BOOL ret; @@ -753,7 +733,7 @@ static BOOL wayland_wglSwapBuffers(HDC hdc) if (!(gl = wayland_gl_drawable_get(NtUserWindowFromDC(hdc), hdc))) return FALSE; if (ctx) wgl_context_refresh(ctx); - wayland_gl_drawable_sync_surface_state(gl); + ensure_window_surface_contents(gl->hwnd); /* Although all the EGL surfaces we create are double-buffered, we want to * use some as single-buffered, so avoid swapping those. */ if (gl->double_buffered) p_eglSwapBuffers(egl_display, gl->surface); diff --git a/dlls/winewayland.drv/vulkan.c b/dlls/winewayland.drv/vulkan.c index 16084175013..a828d736da1 100644 --- a/dlls/winewayland.drv/vulkan.c +++ b/dlls/winewayland.drv/vulkan.c @@ -138,23 +138,7 @@ static void wayland_vulkan_surface_detach(HWND hwnd, void *private) static void wayland_vulkan_surface_presented(HWND hwnd, VkResult result) { - struct wayland_surface *wayland_surface; - - if ((wayland_surface = wayland_surface_lock_hwnd(hwnd))) - { - 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); - } - - pthread_mutex_unlock(&wayland_surface->mutex); - } + ensure_window_surface_contents(hwnd); } static VkBool32 wayland_vkGetPhysicalDeviceWin32PresentationSupportKHR(VkPhysicalDevice phys_dev, diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index 40f86526cd2..46c283865d8 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -297,6 +297,8 @@ struct wayland_win_data struct wayland_win_data *wayland_win_data_get(HWND hwnd); void wayland_win_data_release(struct wayland_win_data *data); +void ensure_window_surface_contents(HWND hwnd); + /********************************************************************** * Wayland Keyboard */ diff --git a/dlls/winewayland.drv/window.c b/dlls/winewayland.drv/window.c index 73a8de37f77..9665d33375e 100644 --- a/dlls/winewayland.drv/window.c +++ b/dlls/winewayland.drv/window.c @@ -710,3 +710,24 @@ struct wayland_surface *wayland_surface_lock_hwnd(HWND hwnd) return surface; } + +void ensure_window_surface_contents(HWND hwnd) +{ + struct wayland_surface *wayland_surface; + + if ((wayland_surface = wayland_surface_lock_hwnd(hwnd))) + { + 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); + } + + pthread_mutex_unlock(&wayland_surface->mutex); + } +} -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6374
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winewayland.drv/waylanddrv.h | 1 + dlls/winewayland.drv/window.c | 23 +++++++++++++++++++++++ dlls/winewayland.drv/window_surface.c | 14 +------------- 3 files changed, 25 insertions(+), 13 deletions(-) diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index 46c283865d8..496a6f1ea60 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -297,6 +297,7 @@ struct wayland_win_data struct wayland_win_data *wayland_win_data_get(HWND hwnd); void wayland_win_data_release(struct wayland_win_data *data); +BOOL set_window_surface_contents(HWND hwnd, struct wayland_shm_buffer *shm_buffer, HRGN damage_region); void ensure_window_surface_contents(HWND hwnd); /********************************************************************** diff --git a/dlls/winewayland.drv/window.c b/dlls/winewayland.drv/window.c index 9665d33375e..096dd2bb5fb 100644 --- a/dlls/winewayland.drv/window.c +++ b/dlls/winewayland.drv/window.c @@ -711,6 +711,29 @@ struct wayland_surface *wayland_surface_lock_hwnd(HWND hwnd) return surface; } +BOOL set_window_surface_contents(HWND hwnd, struct wayland_shm_buffer *shm_buffer, HRGN damage_region) +{ + struct wayland_surface *wayland_surface; + BOOL committed = FALSE; + + if ((wayland_surface = wayland_surface_lock_hwnd(hwnd))) + { + if (wayland_surface_reconfigure(wayland_surface)) + { + wayland_surface_attach_shm(wayland_surface, shm_buffer, damage_region); + wl_surface_commit(wayland_surface->wl_surface); + committed = TRUE; + } + else + { + TRACE("Wayland surface not configured yet, not flushing\n"); + } + pthread_mutex_unlock(&wayland_surface->mutex); + } + + return committed; +} + void ensure_window_surface_contents(HWND hwnd) { struct wayland_surface *wayland_surface; diff --git a/dlls/winewayland.drv/window_surface.c b/dlls/winewayland.drv/window_surface.c index 570c186861b..0d5eb0bd9ff 100644 --- a/dlls/winewayland.drv/window_surface.c +++ b/dlls/winewayland.drv/window_surface.c @@ -386,19 +386,7 @@ static BOOL wayland_window_surface_flush(struct window_surface *window_surface, wayland_shm_buffer_copy_data(shm_buffer, color_bits, &surface_rect, copy_from_window_region); - pthread_mutex_lock(&wws->wayland_surface->mutex); - if (wayland_surface_reconfigure(wws->wayland_surface)) - { - wayland_surface_attach_shm(wws->wayland_surface, shm_buffer, - surface_damage_region); - wl_surface_commit(wws->wayland_surface->wl_surface); - flushed = TRUE; - } - else - { - TRACE("Wayland surface not configured yet, not flushing\n"); - } - pthread_mutex_unlock(&wws->wayland_surface->mutex); + flushed = set_window_surface_contents(window_surface->hwnd, shm_buffer, surface_damage_region); wl_display_flush(process_wayland.wl_display); NtGdiSetRectRgn(shm_buffer->damage_region, 0, 0, 0, 0); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6374
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winewayland.drv/waylanddrv.h | 1 + dlls/winewayland.drv/window.c | 20 ++++++++++++++++++++ dlls/winewayland.drv/window_surface.c | 17 ++++++----------- 3 files changed, 27 insertions(+), 11 deletions(-) diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index 496a6f1ea60..8d554d6a3fa 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -298,6 +298,7 @@ struct wayland_win_data *wayland_win_data_get(HWND hwnd); void wayland_win_data_release(struct wayland_win_data *data); BOOL set_window_surface_contents(HWND hwnd, struct wayland_shm_buffer *shm_buffer, HRGN damage_region); +struct wayland_shm_buffer *get_window_surface_contents(HWND hwnd); void ensure_window_surface_contents(HWND hwnd); /********************************************************************** diff --git a/dlls/winewayland.drv/window.c b/dlls/winewayland.drv/window.c index 096dd2bb5fb..45ef876c4eb 100644 --- a/dlls/winewayland.drv/window.c +++ b/dlls/winewayland.drv/window.c @@ -728,12 +728,32 @@ BOOL set_window_surface_contents(HWND hwnd, struct wayland_shm_buffer *shm_buffe { TRACE("Wayland surface not configured yet, not flushing\n"); } + + /* 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. */ + if (wayland_surface->latest_window_buffer) + wayland_shm_buffer_unref(wayland_surface->latest_window_buffer); + wayland_shm_buffer_ref((wayland_surface->latest_window_buffer = shm_buffer)); + pthread_mutex_unlock(&wayland_surface->mutex); } return committed; } +struct wayland_shm_buffer *get_window_surface_contents(HWND hwnd) +{ + struct wayland_surface *wayland_surface; + struct wayland_shm_buffer *shm_buffer; + + if (!(wayland_surface = wayland_surface_lock_hwnd(hwnd))) return NULL; + if ((shm_buffer = wayland_surface->latest_window_buffer)) wayland_shm_buffer_ref(shm_buffer); + pthread_mutex_unlock(&wayland_surface->mutex); + + return shm_buffer; +} + void ensure_window_surface_contents(HWND hwnd) { struct wayland_surface *wayland_surface; diff --git a/dlls/winewayland.drv/window_surface.c b/dlls/winewayland.drv/window_surface.c index 0d5eb0bd9ff..0e4747fce2f 100644 --- a/dlls/winewayland.drv/window_surface.c +++ b/dlls/winewayland.drv/window_surface.c @@ -324,7 +324,7 @@ static BOOL wayland_window_surface_flush(struct window_surface *window_surface, { RECT surface_rect = {.right = color_info->bmiHeader.biWidth, .bottom = abs(color_info->bmiHeader.biHeight)}; struct wayland_window_surface *wws = wayland_window_surface_cast(window_surface); - struct wayland_shm_buffer *shm_buffer = NULL; + struct wayland_shm_buffer *shm_buffer = NULL, *latest_buffer; BOOL flushed = FALSE; HRGN surface_damage_region = NULL; HRGN copy_from_window_region; @@ -353,12 +353,12 @@ static BOOL wayland_window_surface_flush(struct window_surface *window_surface, goto done; } - if (wws->wayland_surface->latest_window_buffer) + if ((latest_buffer = get_window_surface_contents(window_surface->hwnd))) { - TRACE("latest_window_buffer=%p\n", wws->wayland_surface->latest_window_buffer); + TRACE("latest_window_buffer=%p\n", latest_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) + if (latest_buffer != shm_buffer) { HRGN copy_from_latest_region = NtGdiCreateRectRgn(0, 0, 0, 0); if (!copy_from_latest_region) @@ -368,13 +368,14 @@ static BOOL wayland_window_surface_flush(struct window_surface *window_surface, } NtGdiCombineRgn(copy_from_latest_region, shm_buffer->damage_region, surface_damage_region, RGN_DIFF); - wayland_shm_buffer_copy(wws->wayland_surface->latest_window_buffer, + wayland_shm_buffer_copy(latest_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; + wayland_shm_buffer_unref(latest_buffer); } else { @@ -390,12 +391,6 @@ static BOOL 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. */ - 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)); done: if (surface_damage_region) NtGdiDeleteObjectApp(surface_damage_region); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6374
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winewayland.drv/window_surface.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/dlls/winewayland.drv/window_surface.c b/dlls/winewayland.drv/window_surface.c index 0e4747fce2f..4e5ea3d366d 100644 --- a/dlls/winewayland.drv/window_surface.c +++ b/dlls/winewayland.drv/window_surface.c @@ -386,12 +386,11 @@ static BOOL wayland_window_surface_flush(struct window_surface *window_surface, } wayland_shm_buffer_copy_data(shm_buffer, color_bits, &surface_rect, copy_from_window_region); + NtGdiSetRectRgn(shm_buffer->damage_region, 0, 0, 0, 0); flushed = set_window_surface_contents(window_surface->hwnd, shm_buffer, surface_damage_region); wl_display_flush(process_wayland.wl_display); - NtGdiSetRectRgn(shm_buffer->damage_region, 0, 0, 0, 0); - done: if (surface_damage_region) NtGdiDeleteObjectApp(surface_damage_region); return flushed; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6374
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winewayland.drv/wayland_surface.c | 3 --- dlls/winewayland.drv/waylanddrv.h | 3 ++- dlls/winewayland.drv/window.c | 32 ++++++++++++++++---------- 3 files changed, 22 insertions(+), 16 deletions(-) diff --git a/dlls/winewayland.drv/wayland_surface.c b/dlls/winewayland.drv/wayland_surface.c index c96ed7bde18..8acd43e1729 100644 --- a/dlls/winewayland.drv/wayland_surface.c +++ b/dlls/winewayland.drv/wayland_surface.c @@ -230,9 +230,6 @@ void wayland_surface_destroy(struct wayland_surface *surface) pthread_mutex_unlock(&surface->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 8d554d6a3fa..fc2a0918b3d 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -198,7 +198,6 @@ struct wayland_surface struct wp_viewport *wp_viewport; pthread_mutex_t mutex; struct wayland_surface_config pending, requested, processing, current; - struct wayland_shm_buffer *latest_window_buffer; BOOL resizing; struct wayland_window_config window; struct wayland_client_surface *client; @@ -285,6 +284,8 @@ struct wayland_win_data struct rb_entry entry; /* hwnd that this private data belongs to */ HWND hwnd; + /* last buffer that was set as window contents */ + struct wayland_shm_buffer *window_contents; /* wayland surface (if any) for this window */ struct wayland_surface *wayland_surface; /* wine window_surface backing this window */ diff --git a/dlls/winewayland.drv/window.c b/dlls/winewayland.drv/window.c index 45ef876c4eb..816ab843b91 100644 --- a/dlls/winewayland.drv/window.c +++ b/dlls/winewayland.drv/window.c @@ -125,6 +125,7 @@ static void wayland_win_data_destroy(struct wayland_win_data *data) window_surface_release(data->window_surface); } if (data->wayland_surface) wayland_surface_destroy(data->wayland_surface); + if (data->window_contents) wayland_shm_buffer_unref(data->window_contents); free(data); } @@ -714,10 +715,15 @@ struct wayland_surface *wayland_surface_lock_hwnd(HWND hwnd) BOOL set_window_surface_contents(HWND hwnd, struct wayland_shm_buffer *shm_buffer, HRGN damage_region) { struct wayland_surface *wayland_surface; + struct wayland_win_data *data; BOOL committed = FALSE; - if ((wayland_surface = wayland_surface_lock_hwnd(hwnd))) + if (!(data = wayland_win_data_get(hwnd))) return FALSE; + + if ((wayland_surface = data->wayland_surface)) { + pthread_mutex_lock(&wayland_surface->mutex); + if (wayland_surface_reconfigure(wayland_surface)) { wayland_surface_attach_shm(wayland_surface, shm_buffer, damage_region); @@ -729,27 +735,29 @@ BOOL set_window_surface_contents(HWND hwnd, struct wayland_shm_buffer *shm_buffe TRACE("Wayland surface not configured yet, not flushing\n"); } - /* 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. */ - if (wayland_surface->latest_window_buffer) - wayland_shm_buffer_unref(wayland_surface->latest_window_buffer); - wayland_shm_buffer_ref((wayland_surface->latest_window_buffer = shm_buffer)); - pthread_mutex_unlock(&wayland_surface->mutex); } + /* 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. */ + if (data->window_contents) + wayland_shm_buffer_unref(data->window_contents); + wayland_shm_buffer_ref((data->window_contents = shm_buffer)); + + wayland_win_data_release(data); + return committed; } struct wayland_shm_buffer *get_window_surface_contents(HWND hwnd) { - struct wayland_surface *wayland_surface; struct wayland_shm_buffer *shm_buffer; + struct wayland_win_data *data; - if (!(wayland_surface = wayland_surface_lock_hwnd(hwnd))) return NULL; - if ((shm_buffer = wayland_surface->latest_window_buffer)) wayland_shm_buffer_ref(shm_buffer); - pthread_mutex_unlock(&wayland_surface->mutex); + if (!(data = wayland_win_data_get(hwnd))) return NULL; + if ((shm_buffer = data->window_contents)) wayland_shm_buffer_ref(shm_buffer); + wayland_win_data_release(data); return shm_buffer; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6374
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winewayland.drv/waylanddrv.h | 3 +-- dlls/winewayland.drv/window.c | 13 ++---------- dlls/winewayland.drv/window_surface.c | 30 ++++++++------------------- 3 files changed, 12 insertions(+), 34 deletions(-) diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index fc2a0918b3d..636424230be 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -271,8 +271,7 @@ void wayland_shm_buffer_unref(struct wayland_shm_buffer *shm_buffer); * Wayland window surface */ -void wayland_window_surface_update_wayland_surface(struct window_surface *surface, const RECT *visible_rect, - struct wayland_surface *wayland_surface); +void wayland_window_surface_set_visible_rect(struct window_surface *surface, const RECT *visible_rect); /********************************************************************** * Wayland Window diff --git a/dlls/winewayland.drv/window.c b/dlls/winewayland.drv/window.c index 816ab843b91..e0f3f235cac 100644 --- a/dlls/winewayland.drv/window.c +++ b/dlls/winewayland.drv/window.c @@ -119,11 +119,7 @@ static void wayland_win_data_destroy(struct wayland_win_data *data) pthread_mutex_unlock(&win_data_mutex); - if (data->window_surface) - { - wayland_window_surface_update_wayland_surface(data->window_surface, NULL, NULL); - window_surface_release(data->window_surface); - } + if (data->window_surface) window_surface_release(data->window_surface); if (data->wayland_surface) wayland_surface_destroy(data->wayland_surface); if (data->window_contents) wayland_shm_buffer_unref(data->window_contents); free(data); @@ -210,8 +206,6 @@ static void wayland_win_data_update_wayland_surface(struct wayland_win_data *dat /* We don't want wayland surfaces for child windows. */ if (parent != NtUserGetDesktopWindow() && parent != 0) { - 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; @@ -248,10 +242,6 @@ 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, - &data->rects.visible, surface); - /* Size/position changes affect the effective pointer constraint, so update * it as needed. */ if (data->hwnd == NtUserGetForegroundWindow()) reapply_cursor_clipping(); @@ -466,6 +456,7 @@ void WAYLAND_WindowPosChanged(HWND hwnd, HWND insert_after, UINT swp_flags, cons data->window_surface = surface; wayland_win_data_update_wayland_surface(data); + if (surface) wayland_window_surface_set_visible_rect(surface, &data->rects.visible); if (data->wayland_surface) wayland_win_data_update_wayland_state(data); wayland_win_data_release(data); diff --git a/dlls/winewayland.drv/window_surface.c b/dlls/winewayland.drv/window_surface.c index 4e5ea3d366d..5131d6043cf 100644 --- a/dlls/winewayland.drv/window_surface.c +++ b/dlls/winewayland.drv/window_surface.c @@ -43,7 +43,6 @@ struct wayland_buffer_queue struct wayland_window_surface { struct window_surface header; - struct wayland_surface *wayland_surface; struct wayland_buffer_queue *wayland_buffer_queue; }; @@ -329,10 +328,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_buffer_queue) { - ERR("missing wayland surface=%p or buffer_queue=%p, returning\n", - wws->wayland_surface, wws->wayland_buffer_queue); + ERR("missing buffer_queue=%p, returning\n", wws->wayland_buffer_queue); goto done; } @@ -444,9 +442,9 @@ static struct window_surface *wayland_window_surface_create(HWND hwnd, const REC /*********************************************************************** * wayland_window_surface_update_wayland_surface */ -void wayland_window_surface_update_wayland_surface(struct window_surface *window_surface, const RECT *visible_rect, - struct wayland_surface *wayland_surface) +void wayland_window_surface_set_visible_rect(struct window_surface *window_surface, const RECT *visible_rect) { + UINT width = visible_rect->right - visible_rect->left, height = visible_rect->bottom - visible_rect->top; struct wayland_window_surface *wws; /* ignore calls with the dummy surface */ @@ -455,23 +453,13 @@ void wayland_window_surface_update_wayland_surface(struct window_surface *window wws = wayland_window_surface_cast(window_surface); window_surface_lock(window_surface); - TRACE("surface=%p hwnd=%p visible_rect=%s wayland_surface=%p\n", wws, window_surface->hwnd, - wine_dbgstr_rect(visible_rect), wayland_surface); + TRACE("surface=%p hwnd=%p visible_rect=%s\n", wws, window_surface->hwnd, wine_dbgstr_rect(visible_rect)); - 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) + if (!wws->wayland_buffer_queue || wws->wayland_buffer_queue->width != width || + wws->wayland_buffer_queue->height != height) { - wws->wayland_buffer_queue = - wayland_buffer_queue_create(visible_rect->right - visible_rect->left, - visible_rect->bottom - visible_rect->top); + if (wws->wayland_buffer_queue) wayland_buffer_queue_destroy(wws->wayland_buffer_queue); + wws->wayland_buffer_queue = wayland_buffer_queue_create(width, height); } window_surface_unlock(window_surface); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6374
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winewayland.drv/opengl.c | 7 +-- dlls/winewayland.drv/wayland_surface.c | 82 +++++++++++++++----------- dlls/winewayland.drv/waylanddrv.h | 1 + 3 files changed, 49 insertions(+), 41 deletions(-) diff --git a/dlls/winewayland.drv/opengl.c b/dlls/winewayland.drv/opengl.c index b99d80cbcf2..fe7499f5256 100644 --- a/dlls/winewayland.drv/opengl.c +++ b/dlls/winewayland.drv/opengl.c @@ -218,13 +218,10 @@ static struct wayland_gl_drawable *wayland_gl_drawable_create(HWND hwnd, int for client_width = client_height = 1; pthread_mutex_unlock(&wayland_surface->mutex); } - else if ((wayland_surface = wayland_surface_create(0))) + else { - gl->client = wayland_surface_get_client(wayland_surface); + gl->client = wayland_client_surface_create(hwnd); client_width = client_height = 1; - /* It's fine to destroy the wayland surface, the client surface - * can safely outlive it. */ - wayland_surface_destroy(wayland_surface); } if (!gl->client) goto err; diff --git a/dlls/winewayland.drv/wayland_surface.c b/dlls/winewayland.drv/wayland_surface.c index 8acd43e1729..545378368c5 100644 --- a/dlls/winewayland.drv/wayland_surface.c +++ b/dlls/winewayland.drv/wayland_surface.c @@ -768,36 +768,26 @@ BOOL wayland_client_surface_release(struct wayland_client_surface *client) return TRUE; } -/********************************************************************** - * wayland_surface_get_client - */ -struct wayland_client_surface *wayland_surface_get_client(struct wayland_surface *surface) +struct wayland_client_surface *wayland_client_surface_create(HWND hwnd) { + struct wayland_client_surface *client; struct wl_region *empty_region; - if (surface->client) - { - InterlockedIncrement(&surface->client->ref); - return surface->client; - } - - surface->client = calloc(1, sizeof(*surface->client)); - if (!surface->client) + if (!(client = calloc(1, sizeof(*client)))) { ERR("Failed to allocate space for client surface\n"); - goto err; + return NULL; } + client->ref = 1; - surface->client->ref = 1; - - surface->client->wl_surface = + client->wl_surface = wl_compositor_create_surface(process_wayland.wl_compositor); - if (!surface->client->wl_surface) + if (!client->wl_surface) { ERR("Failed to create client wl_surface\n"); goto err; } - wl_surface_set_user_data(surface->client->wl_surface, surface->hwnd); + wl_surface_set_user_data(client->wl_surface, hwnd); /* Let parent handle all pointer events. */ empty_region = wl_compositor_create_region(process_wayland.wl_compositor); @@ -806,40 +796,60 @@ struct wayland_client_surface *wayland_surface_get_client(struct wayland_surface ERR("Failed to create wl_region\n"); goto err; } - wl_surface_set_input_region(surface->client->wl_surface, empty_region); + wl_surface_set_input_region(client->wl_surface, empty_region); wl_region_destroy(empty_region); - surface->client->wl_subsurface = + if (process_wayland.wp_viewporter) + { + client->wp_viewport = + wp_viewporter_get_viewport(process_wayland.wp_viewporter, + client->wl_surface); + } + + return client; + +err: + wayland_client_surface_release(client); + return NULL; +} + +/********************************************************************** + * wayland_surface_get_client + */ +struct wayland_client_surface *wayland_surface_get_client(struct wayland_surface *surface) +{ + struct wayland_client_surface *client; + + if ((client = surface->client)) + { + InterlockedIncrement(&client->ref); + return client; + } + + if (!(client = wayland_client_surface_create(surface->hwnd))) + return NULL; + + client->wl_subsurface = wl_subcompositor_get_subsurface(process_wayland.wl_subcompositor, - surface->client->wl_surface, + client->wl_surface, surface->wl_surface); - if (!surface->client->wl_subsurface) + if (!client->wl_subsurface) { ERR("Failed to create client wl_subsurface\n"); goto err; } /* 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); - } + wl_subsurface_set_desync(client->wl_subsurface); wayland_surface_reconfigure_client(surface); /* Commit to apply subsurface positioning. */ wl_surface_commit(surface->wl_surface); - return surface->client; + surface->client = client; + return client; err: - if (surface->client) - { - wayland_client_surface_release(surface->client); - surface->client = NULL; - } + wayland_client_surface_release(client); return NULL; } diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index 636424230be..6d3a4b78474 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -253,6 +253,7 @@ void wayland_surface_coords_from_window(struct wayland_surface *surface, void wayland_surface_coords_to_window(struct wayland_surface *surface, double surface_x, double surface_y, int *window_x, int *window_y); +struct wayland_client_surface *wayland_client_surface_create(HWND hwnd); 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_ensure_contents(struct wayland_surface *surface); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6374
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winewayland.drv/opengl.c | 53 +++++++++++-------- dlls/winewayland.drv/vulkan.c | 28 +++++----- dlls/winewayland.drv/wayland_keyboard.c | 10 ++-- dlls/winewayland.drv/wayland_pointer.c | 40 +++++++++++---- dlls/winewayland.drv/wayland_surface.c | 30 +++++------ dlls/winewayland.drv/waylanddrv.h | 2 - dlls/winewayland.drv/window.c | 68 ++++++++++--------------- 7 files changed, 120 insertions(+), 111 deletions(-) diff --git a/dlls/winewayland.drv/opengl.c b/dlls/winewayland.drv/opengl.c index fe7499f5256..94ff32c81f1 100644 --- a/dlls/winewayland.drv/opengl.c +++ b/dlls/winewayland.drv/opengl.c @@ -166,12 +166,12 @@ static void wayland_gl_drawable_release(struct wayland_gl_drawable *gl) if (gl->client) { HWND hwnd = wl_surface_get_user_data(gl->client->wl_surface); - struct wayland_surface *wayland_surface = wayland_surface_lock_hwnd(hwnd); + struct wayland_win_data *data = wayland_win_data_get(hwnd); - if (wayland_client_surface_release(gl->client) && wayland_surface) - wayland_surface->client = NULL; + if (wayland_client_surface_release(gl->client) && data && data->wayland_surface) + data->wayland_surface->client = NULL; - if (wayland_surface) pthread_mutex_unlock(&wayland_surface->mutex); + if (data) wayland_win_data_release(data); } free(gl); @@ -194,6 +194,7 @@ static struct wayland_gl_drawable *wayland_gl_drawable_create(HWND hwnd, int for struct wayland_gl_drawable *gl; struct wayland_surface *wayland_surface; int client_width = 0, client_height = 0; + struct wayland_win_data *data; TRACE("hwnd=%p format=%d\n", hwnd, format); @@ -207,16 +208,24 @@ 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 ((data = wayland_win_data_get(hwnd))) { - gl->client = wayland_surface_get_client(wayland_surface); - client_width = wayland_surface->window.client_rect.right - - wayland_surface->window.client_rect.left; - client_height = wayland_surface->window.client_rect.bottom - - wayland_surface->window.client_rect.top; - if (client_width == 0 || client_height == 0) + if (!(wayland_surface = data->wayland_surface)) + { + gl->client = wayland_client_surface_create(hwnd); client_width = client_height = 1; - pthread_mutex_unlock(&wayland_surface->mutex); + } + else + { + gl->client = wayland_surface_get_client(wayland_surface); + client_width = wayland_surface->window.client_rect.right - + wayland_surface->window.client_rect.left; + client_height = wayland_surface->window.client_rect.bottom - + wayland_surface->window.client_rect.top; + if (client_width == 0 || client_height == 0) + client_width = client_height = 1; + } + wayland_win_data_release(data); } else { @@ -285,23 +294,27 @@ static void wayland_update_gl_drawable(HWND hwnd, struct wayland_gl_drawable *ne static void wayland_gl_drawable_sync_size(struct wayland_gl_drawable *gl) { - int client_width, client_height; + int client_width = 0, client_height = 0; struct wayland_surface *wayland_surface; + struct wayland_win_data *data; if (InterlockedCompareExchange(&gl->resized, FALSE, TRUE)) { - if (!(wayland_surface = wayland_surface_lock_hwnd(gl->hwnd))) return; + if (!(data = wayland_win_data_get(gl->hwnd))) return; + + if ((wayland_surface = data->wayland_surface)) + { + client_width = wayland_surface->window.client_rect.right - + wayland_surface->window.client_rect.left; + client_height = wayland_surface->window.client_rect.bottom - + wayland_surface->window.client_rect.top; + } - client_width = wayland_surface->window.client_rect.right - - wayland_surface->window.client_rect.left; - client_height = wayland_surface->window.client_rect.bottom - - wayland_surface->window.client_rect.top; if (client_width == 0 || client_height == 0) client_width = client_height = 1; + wayland_win_data_release(data); wl_egl_window_resize(gl->wl_egl_window, client_width, client_height, 0, 0); - - pthread_mutex_unlock(&wayland_surface->mutex); } } diff --git a/dlls/winewayland.drv/vulkan.c b/dlls/winewayland.drv/vulkan.c index a828d736da1..7a075a925b7 100644 --- a/dlls/winewayland.drv/vulkan.c +++ b/dlls/winewayland.drv/vulkan.c @@ -58,22 +58,15 @@ static VkBool32 (*pvkGetPhysicalDeviceWaylandPresentationSupportKHR)(VkPhysicalD static const struct vulkan_driver_funcs wayland_vulkan_driver_funcs; -static HWND wine_vk_surface_get_hwnd(struct wayland_client_surface *client) -{ - return wl_surface_get_user_data(client->wl_surface); -} - static void wine_vk_surface_destroy(struct wayland_client_surface *client) { - HWND hwnd = wine_vk_surface_get_hwnd(client); - struct wayland_surface *wayland_surface = wayland_surface_lock_hwnd(hwnd); + HWND hwnd = wl_surface_get_user_data(client->wl_surface); + struct wayland_win_data *data = wayland_win_data_get(hwnd); - if (wayland_client_surface_release(client) && wayland_surface) - { - wayland_surface->client = NULL; - } + if (wayland_client_surface_release(client) && data && data->wayland_surface) + data->wayland_surface->client = NULL; - if (wayland_surface) pthread_mutex_unlock(&wayland_surface->mutex); + if (data) wayland_win_data_release(data); } static VkResult wayland_vulkan_surface_create(HWND hwnd, VkInstance instance, VkSurfaceKHR *surface, void **private) @@ -82,18 +75,21 @@ static VkResult wayland_vulkan_surface_create(HWND hwnd, VkInstance instance, Vk VkWaylandSurfaceCreateInfoKHR create_info_host; struct wayland_surface *wayland_surface; struct wayland_client_surface *client; + struct wayland_win_data *data; TRACE("%p %p %p %p\n", hwnd, instance, surface, private); - wayland_surface = wayland_surface_lock_hwnd(hwnd); - if (!wayland_surface) + if (!(data = wayland_win_data_get(hwnd))) { ERR("Failed to find wayland surface for hwnd=%p\n", hwnd); return VK_ERROR_OUT_OF_HOST_MEMORY; } - client = wayland_surface_get_client(wayland_surface); - pthread_mutex_unlock(&wayland_surface->mutex); + if ((wayland_surface = data->wayland_surface)) + client = wayland_surface_get_client(wayland_surface); + else + client = wayland_client_surface_create(hwnd); + wayland_win_data_release(data); if (!client) { diff --git a/dlls/winewayland.drv/wayland_keyboard.c b/dlls/winewayland.drv/wayland_keyboard.c index 5c2fe15273e..8f7b6dce30f 100644 --- a/dlls/winewayland.drv/wayland_keyboard.c +++ b/dlls/winewayland.drv/wayland_keyboard.c @@ -736,12 +736,13 @@ static void keyboard_handle_keymap(void *data, struct wl_keyboard *wl_keyboard, xkb_keymap_unref(xkb_keymap); } -static void keyboard_handle_enter(void *data, struct wl_keyboard *wl_keyboard, +static void keyboard_handle_enter(void *private, struct wl_keyboard *wl_keyboard, uint32_t serial, struct wl_surface *wl_surface, struct wl_array *keys) { struct wayland_keyboard *keyboard = &process_wayland.keyboard; struct wayland_surface *surface; + struct wayland_win_data *data; HWND hwnd; if (!wl_surface) return; @@ -758,7 +759,9 @@ static void keyboard_handle_enter(void *data, struct wl_keyboard *wl_keyboard, NtUserPostMessage(keyboard->focused_hwnd, WM_INPUTLANGCHANGEREQUEST, 0 /*FIXME*/, (LPARAM)keyboard_hkl); - if ((surface = wayland_surface_lock_hwnd(hwnd))) + if (!(data = wayland_win_data_get(hwnd))) return; + + if ((surface = data->wayland_surface)) { /* TODO: Drop the internal message and call NtUserSetForegroundWindow * directly once it's updated to not explicitly deactivate the old @@ -766,8 +769,9 @@ static void keyboard_handle_enter(void *data, struct wl_keyboard *wl_keyboard, * are in the same non-current thread. */ if (surface->window.managed) NtUserPostMessage(hwnd, WM_WAYLAND_SET_FOREGROUND, 0, 0); - pthread_mutex_unlock(&surface->mutex); } + + wayland_win_data_release(data); } static void keyboard_handle_leave(void *data, struct wl_keyboard *wl_keyboard, diff --git a/dlls/winewayland.drv/wayland_pointer.c b/dlls/winewayland.drv/wayland_pointer.c index 1d8acaeabd2..c1a364925f0 100644 --- a/dlls/winewayland.drv/wayland_pointer.c +++ b/dlls/winewayland.drv/wayland_pointer.c @@ -53,9 +53,15 @@ static void pointer_handle_motion_internal(wl_fixed_t sx, wl_fixed_t sy) HWND hwnd; POINT screen; struct wayland_surface *surface; + struct wayland_win_data *data; if (!(hwnd = wayland_pointer_get_focused_hwnd())) return; - if (!(surface = wayland_surface_lock_hwnd(hwnd))) return; + if (!(data = wayland_win_data_get(hwnd))) return; + if (!(surface = data->wayland_surface)) + { + wayland_win_data_release(data); + return; + } window_rect = &surface->window.rect; @@ -72,7 +78,7 @@ static void pointer_handle_motion_internal(wl_fixed_t sx, wl_fixed_t sy) if (screen.y >= window_rect->bottom) screen.y = window_rect->bottom - 1; else if (screen.y < window_rect->top) screen.y = window_rect->top; - pthread_mutex_unlock(&surface->mutex); + wayland_win_data_release(data); /* Hardware input events are in physical coordinates. */ if (!NtUserLogicalToPerMonitorDPIPhysicalPoint(hwnd, &screen)) return; @@ -248,7 +254,7 @@ static const struct wl_pointer_listener pointer_listener = pointer_handle_axis_discrete }; -static void relative_pointer_v1_relative_motion(void *data, +static void relative_pointer_v1_relative_motion(void *private, struct zwp_relative_pointer_v1 *zwp_relative_pointer_v1, uint32_t utime_hi, uint32_t utime_lo, wl_fixed_t dx, wl_fixed_t dy, @@ -258,10 +264,16 @@ static void relative_pointer_v1_relative_motion(void *data, HWND hwnd; POINT screen, origin; struct wayland_surface *surface; + struct wayland_win_data *data; RECT window_rect; if (!(hwnd = wayland_pointer_get_focused_hwnd())) return; - if (!(surface = wayland_surface_lock_hwnd(hwnd))) return; + if (!(data = wayland_win_data_get(hwnd))) return; + if (!(surface = data->wayland_surface)) + { + wayland_win_data_release(data); + return; + } window_rect = surface->window.rect; @@ -270,7 +282,7 @@ static void relative_pointer_v1_relative_motion(void *data, wl_fixed_to_double(dy), (int *)&screen.x, (int *)&screen.y); - pthread_mutex_unlock(&surface->mutex); + wayland_win_data_release(data); /* We clip the relative motion within the window rectangle so that * the NtUserLogicalToPerMonitorDPIPhysicalPoint calls later succeed. @@ -671,16 +683,22 @@ static void wayland_set_cursor(HWND hwnd, HCURSOR hcursor, BOOL use_hcursor) { struct wayland_pointer *pointer = &process_wayland.pointer; struct wayland_surface *surface; + struct wayland_win_data *data; double scale; BOOL reapply_clip = FALSE; - if ((surface = wayland_surface_lock_hwnd(hwnd))) + if ((data = wayland_win_data_get(hwnd))) { + if (!(surface = data->wayland_surface)) + { + wayland_win_data_release(data); + return; + } scale = surface->window.scale; if (use_hcursor) surface->hcursor = hcursor; else hcursor = surface->hcursor; use_hcursor = TRUE; - pthread_mutex_unlock(&surface->mutex); + wayland_win_data_release(data); } else { @@ -901,22 +919,24 @@ BOOL WAYLAND_ClipCursor(const RECT *clip, BOOL reset) struct wayland_pointer *pointer = &process_wayland.pointer; struct wl_surface *wl_surface = NULL; struct wayland_surface *surface = NULL; + struct wayland_win_data *data; BOOL covers_vscreen = FALSE; RECT confine_rect; TRACE("clip=%s reset=%d\n", wine_dbgstr_rect(clip), reset); - if ((surface = wayland_surface_lock_hwnd(NtUserGetForegroundWindow()))) + if (!(data = wayland_win_data_get(NtUserGetForegroundWindow()))) return FALSE; + if ((surface = data->wayland_surface)) { wl_surface = surface->wl_surface; if (clip) wayland_surface_calc_confine(surface, clip, &confine_rect); covers_vscreen = wayland_surface_client_covers_vscreen(surface); - pthread_mutex_unlock(&surface->mutex); } + wayland_win_data_release(data); /* Since we are running in the context of the foreground thread we know * that the wl_surface of the foreground HWND will not be invalidated, - * so we can access it without having the surface lock. */ + * so we can access it without having the win data lock. */ pthread_mutex_lock(&pointer->mutex); wayland_pointer_update_constraint(wl_surface, (clip && wl_surface) ? &confine_rect : NULL, diff --git a/dlls/winewayland.drv/wayland_surface.c b/dlls/winewayland.drv/wayland_surface.c index 545378368c5..6e767b5d680 100644 --- a/dlls/winewayland.drv/wayland_surface.c +++ b/dlls/winewayland.drv/wayland_surface.c @@ -34,20 +34,21 @@ WINE_DEFAULT_DEBUG_CHANNEL(waylanddrv); -static void xdg_surface_handle_configure(void *data, struct xdg_surface *xdg_surface, +static void xdg_surface_handle_configure(void *private, struct xdg_surface *xdg_surface, uint32_t serial) { struct wayland_surface *surface; BOOL should_post = FALSE, initial_configure = FALSE; - HWND hwnd = data; + struct wayland_win_data *data; + HWND hwnd = private; TRACE("serial=%u\n", serial); - if (!(surface = wayland_surface_lock_hwnd(hwnd))) return; + if (!(data = wayland_win_data_get(hwnd))) return; /* Handle this event only if wayland_surface is still associated with * the target xdg_surface. */ - if (surface->xdg_surface == xdg_surface) + if ((surface = data->wayland_surface) && surface->xdg_surface == xdg_surface) { /* If we have a previously requested config, we have already sent a * WM_WAYLAND_CONFIGURE which hasn't been handled yet. In that case, @@ -59,7 +60,7 @@ static void xdg_surface_handle_configure(void *data, struct xdg_surface *xdg_sur memset(&surface->pending, 0, sizeof(surface->pending)); } - pthread_mutex_unlock(&surface->mutex); + wayland_win_data_release(data); if (should_post) NtUserPostMessage(hwnd, WM_WAYLAND_CONFIGURE, 0, 0); @@ -76,15 +77,16 @@ static const struct xdg_surface_listener xdg_surface_listener = xdg_surface_handle_configure }; -static void xdg_toplevel_handle_configure(void *data, +static void xdg_toplevel_handle_configure(void *private, struct xdg_toplevel *xdg_toplevel, int32_t width, int32_t height, struct wl_array *states) { struct wayland_surface *surface; - HWND hwnd = data; + HWND hwnd = private; uint32_t *state; enum wayland_surface_config_state config_state = 0; + struct wayland_win_data *data; wl_array_for_each(state, states) { @@ -112,16 +114,16 @@ static void xdg_toplevel_handle_configure(void *data, TRACE("hwnd=%p %dx%d,%#x\n", hwnd, width, height, config_state); - if (!(surface = wayland_surface_lock_hwnd(hwnd))) return; + if (!(data = wayland_win_data_get(hwnd))) return; - if (surface->xdg_toplevel == xdg_toplevel) + if ((surface = data->wayland_surface) && surface->xdg_toplevel == xdg_toplevel) { surface->pending.width = width; surface->pending.height = height; surface->pending.state = config_state; } - pthread_mutex_unlock(&surface->mutex); + wayland_win_data_release(data); } static void xdg_toplevel_handle_close(void *data, struct xdg_toplevel *xdg_toplevel) @@ -153,8 +155,6 @@ struct wayland_surface *wayland_surface_create(HWND hwnd) TRACE("surface=%p\n", surface); - pthread_mutex_init(&surface->mutex, NULL); - surface->hwnd = hwnd; surface->wl_surface = wl_compositor_create_surface(process_wayland.wl_compositor); if (!surface->wl_surface) @@ -202,8 +202,6 @@ void wayland_surface_destroy(struct wayland_surface *surface) process_wayland.keyboard.focused_hwnd = NULL; pthread_mutex_unlock(&process_wayland.keyboard.mutex); - pthread_mutex_lock(&surface->mutex); - if (surface->wp_viewport) { wp_viewport_destroy(surface->wp_viewport); @@ -228,12 +226,8 @@ void wayland_surface_destroy(struct wayland_surface *surface) surface->wl_surface = NULL; } - pthread_mutex_unlock(&surface->mutex); - wl_display_flush(process_wayland.wl_display); - pthread_mutex_destroy(&surface->mutex); - free(surface); } diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index 6d3a4b78474..32c356da625 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -196,7 +196,6 @@ struct wayland_surface struct xdg_surface *xdg_surface; struct xdg_toplevel *xdg_toplevel; struct wp_viewport *wp_viewport; - pthread_mutex_t mutex; struct wayland_surface_config pending, requested, processing, current; BOOL resizing; struct wayland_window_config window; @@ -242,7 +241,6 @@ void wayland_surface_clear_role(struct wayland_surface *surface); 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); 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 e0f3f235cac..82523581563 100644 --- a/dlls/winewayland.drv/window.c +++ b/dlls/winewayland.drv/window.c @@ -217,8 +217,6 @@ static void wayland_win_data_update_wayland_surface(struct wayland_win_data *dat visible = (NtUserGetWindowLongW(data->hwnd, GWL_STYLE) & WS_VISIBLE) == WS_VISIBLE; xdg_visible = surface->xdg_toplevel != NULL; - pthread_mutex_lock(&surface->mutex); - if (visible != xdg_visible) { /* If we have a pre-existing surface ensure it has no role. */ @@ -240,8 +238,6 @@ static void wayland_win_data_update_wayland_surface(struct wayland_win_data *dat wayland_win_data_get_config(data, &surface->window); - pthread_mutex_unlock(&surface->mutex); - /* Size/position changes affect the effective pointer constraint, so update * it as needed. */ if (data->hwnd == NtUserGetForegroundWindow()) reapply_cursor_clipping(); @@ -256,8 +252,6 @@ static void wayland_win_data_update_wayland_state(struct wayland_win_data *data) struct wayland_surface *surface = data->wayland_surface; BOOL processing_config; - pthread_mutex_lock(&surface->mutex); - if (!surface->xdg_toplevel) goto out; processing_config = surface->processing.serial && @@ -302,7 +296,6 @@ static void wayland_win_data_update_wayland_state(struct wayland_win_data *data) } out: - pthread_mutex_unlock(&surface->mutex); wl_display_flush(process_wayland.wl_display); } @@ -472,20 +465,26 @@ static void wayland_configure_window(HWND hwnd) DWORD style; BOOL needs_enter_size_move = FALSE; BOOL needs_exit_size_move = FALSE; + struct wayland_win_data *data; - if (!(surface = wayland_surface_lock_hwnd(hwnd))) return; + if (!(data = wayland_win_data_get(hwnd))) return; + if (!(surface = data->wayland_surface)) + { + wayland_win_data_release(data); + return; + } if (!surface->xdg_toplevel) { TRACE("missing xdg_toplevel, returning\n"); - pthread_mutex_unlock(&surface->mutex); + wayland_win_data_release(data); return; } if (!surface->requested.serial) { TRACE("requested configure event already handled, returning\n"); - pthread_mutex_unlock(&surface->mutex); + wayland_win_data_release(data); return; } @@ -547,7 +546,7 @@ static void wayland_configure_window(HWND hwnd) wayland_surface_coords_to_window(surface, width, height, &window_width, &window_height); - pthread_mutex_unlock(&surface->mutex); + wayland_win_data_release(data); TRACE("processing=%dx%d,%#x\n", width, height, state); @@ -626,14 +625,16 @@ static enum xdg_toplevel_resize_edge hittest_to_resize_edge(WPARAM hittest) */ void WAYLAND_SetWindowText(HWND hwnd, LPCWSTR text) { - struct wayland_surface *surface = wayland_surface_lock_hwnd(hwnd); + struct wayland_surface *surface; + struct wayland_win_data *data; TRACE("hwnd=%p text=%s\n", hwnd, wine_dbgstr_w(text)); - if (surface) + if ((data = wayland_win_data_get(hwnd))) { - if (surface->xdg_toplevel) wayland_surface_set_title(surface, text); - pthread_mutex_unlock(&surface->mutex); + if ((surface = data->wayland_surface) && surface->xdg_toplevel) + wayland_surface_set_title(surface, text); + wayland_win_data_release(data); } } @@ -647,6 +648,7 @@ LRESULT WAYLAND_SysCommand(HWND hwnd, WPARAM wparam, LPARAM lparam) uint32_t button_serial; struct wl_seat *wl_seat; struct wayland_surface *surface; + struct wayland_win_data *data; TRACE("cmd=%lx hwnd=%p, %lx, %lx\n", (long)command, hwnd, (long)wparam, lparam); @@ -660,11 +662,11 @@ LRESULT WAYLAND_SysCommand(HWND hwnd, WPARAM wparam, LPARAM lparam) if (command == SC_MOVE || command == SC_SIZE) { - if ((surface = wayland_surface_lock_hwnd(hwnd))) + if ((data = wayland_win_data_get(hwnd))) { pthread_mutex_lock(&process_wayland.seat.mutex); wl_seat = process_wayland.seat.wl_seat; - if (wl_seat && surface->xdg_toplevel && button_serial) + if (wl_seat && (surface = data->wayland_surface) && surface->xdg_toplevel && button_serial) { if (command == SC_MOVE) { @@ -677,7 +679,7 @@ LRESULT WAYLAND_SysCommand(HWND hwnd, WPARAM wparam, LPARAM lparam) } } pthread_mutex_unlock(&process_wayland.seat.mutex); - pthread_mutex_unlock(&surface->mutex); + wayland_win_data_release(data); ret = 0; } } @@ -686,23 +688,6 @@ LRESULT WAYLAND_SysCommand(HWND hwnd, WPARAM wparam, LPARAM lparam) return ret; } -/********************************************************************** - * wayland_surface_lock_hwnd - */ -struct wayland_surface *wayland_surface_lock_hwnd(HWND hwnd) -{ - struct wayland_win_data *data = wayland_win_data_get(hwnd); - struct wayland_surface *surface; - - if (!data) return NULL; - - if ((surface = data->wayland_surface)) pthread_mutex_lock(&surface->mutex); - - wayland_win_data_release(data); - - return surface; -} - BOOL set_window_surface_contents(HWND hwnd, struct wayland_shm_buffer *shm_buffer, HRGN damage_region) { struct wayland_surface *wayland_surface; @@ -713,8 +698,6 @@ BOOL set_window_surface_contents(HWND hwnd, struct wayland_shm_buffer *shm_buffe if ((wayland_surface = data->wayland_surface)) { - pthread_mutex_lock(&wayland_surface->mutex); - if (wayland_surface_reconfigure(wayland_surface)) { wayland_surface_attach_shm(wayland_surface, shm_buffer, damage_region); @@ -725,8 +708,6 @@ BOOL set_window_surface_contents(HWND hwnd, struct wayland_shm_buffer *shm_buffe { TRACE("Wayland surface not configured yet, not flushing\n"); } - - pthread_mutex_unlock(&wayland_surface->mutex); } /* Update the latest window buffer for the wayland surface. Note that we @@ -756,8 +737,11 @@ struct wayland_shm_buffer *get_window_surface_contents(HWND hwnd) void ensure_window_surface_contents(HWND hwnd) { struct wayland_surface *wayland_surface; + struct wayland_win_data *data; + + if (!(data = wayland_win_data_get(hwnd))) return; - if ((wayland_surface = wayland_surface_lock_hwnd(hwnd))) + if ((wayland_surface = data->wayland_surface)) { wayland_surface_ensure_contents(wayland_surface); @@ -769,7 +753,7 @@ void ensure_window_surface_contents(HWND hwnd) { wl_surface_commit(wayland_surface->wl_surface); } - - pthread_mutex_unlock(&wayland_surface->mutex); } + + wayland_win_data_release(data); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6374
Thanks, this approach is serializing access across all Wayland surfaces, but I don't think that's a big loss and the simplicity it brings seems to be worth it. This broader locking scheme has introduced a new deadlock situation, though, between `window_surface->mutex` and `win_data_mutex`: Thread 1: **window_surface_lock** -> window_surface_flush -> {get,set}_window_surface_contents -> **wayland_win_data_get** Thread 2: WAYLAND_WindowPosChanged -> **wayland_win_data_get** -> wayland_window_surface_set_visible_rect -> **window_surface_lock** An example where I am seeing this consistently is while resizing firefox. Moving the call to `wayland_window_surface_set_visible_rect` after `wayland_win_data_release` in `WAYLAND_WindowPosChanged` seems to be a reasonable fix? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6374#note_80240
On Wed Aug 28 08:37:08 2024 +0000, Alexandros Frantzis wrote:
Thanks, this approach is serializing access across all Wayland surfaces, but I don't think that's a big loss and the simplicity it brings seems to be worth it. This broader locking scheme has introduced a new deadlock situation, though, between `window_surface->mutex` and `win_data_mutex`: Thread 1: **window_surface_lock** -> window_surface_flush -> {get,set}_window_surface_contents -> **wayland_win_data_get** Thread 2: WAYLAND_WindowPosChanged -> **wayland_win_data_get** -> wayland_window_surface_set_visible_rect -> **window_surface_lock** An example where I am seeing this consistently is while resizing firefox. Moving the call to `wayland_window_surface_set_visible_rect` after `wayland_win_data_release` in `WAYLAND_WindowPosChanged` seems to be a reasonable fix? Sounds reasonable, then I'm tempted to get rid of this window surface buffer queue recreation entirely, removing the need for `wayland_window_surface_set_visible_rect` altogether. The only reason we do it I believe, is because buffers need to match the surface dimensions at the moment, but what about using `wp_viewport_set_source` to crop the buffers instead?
This would let us use the surface dimensions for the buffers, and only reallocate the queue and buffers when surface needs to be re-created, which seems to be its intended usage (we round the surface rectangles to multiple of 128 for that purpose). It seems that the only thing preventing that is that wp_viewporter is currently optional, do you think it would be an issue to make it a requirement? I think it will let us make the code much simpler, and even allow further integration into win32u, allowing win32u to draw directly to the buffer for instance. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6374#note_80241
On Wed Aug 28 08:38:01 2024 +0000, Rémi Bernon wrote:
Sounds reasonable, then I'm tempted to get rid of this window surface buffer queue recreation entirely, removing the need for `wayland_window_surface_set_visible_rect` altogether. The only reason we do it I believe, is because buffers need to match the surface dimensions at the moment, but what about using `wp_viewport_set_source` to crop the buffers instead? This would let us use the surface dimensions for the buffers, and only reallocate the queue and buffers when surface needs to be re-created, which seems to be its intended usage (we round the surface rectangles to multiple of 128 for that purpose). It seems that the only thing preventing that is that wp_viewporter is currently optional, do you think it would be an issue to make it a requirement? I think it will let us make the code much simpler, and even allow further integration into win32u, allowing win32u to draw directly to the buffer for instance. Something like https://gitlab.winehq.org/wine/wine/-/merge_requests/6386.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6374#note_80244
participants (2)
-
Alexandros Frantzis (＠afrantzis) -
Rémi Bernon