[PATCH 0/5] MR11342: winewayland: Reuse a single client surface for vulkan.
This may seem unnecessary, but it is required for many Windows Vulkan apps to function correctly on Wayland when using dmabuf in certain situations (like direct scanout). Here is the mesa vulkan wayland WSI source code for reference: https://gitlab.freedesktop.org/mesa/mesa/-/blob/main/src/vulkan/wsi/wsi_comm... I'll use the examples of Doom Eternal and Wolfenstein Youngblood to show where problems arise: Doom Eternal: 1. VkCreateWin32SurfaceKHR -> create new client surface -> create new wl_surface -> VkCreateWaylandSurfaceKHR -> WSI queries and stores the set of default DMABUF modifiers -> create the per surface dmabuf feedback (but the compositor has no idea where the wl_surface will be placed, so it can't send us any modifiers yet) 3. VkCreateSwapchainKHR 4. [Do some presents] 5. Swapchain becomes suboptimal (because the compositor sent the modifiers for the wl_surface, which end up being different than the default one) 6. VkDestroySwapchainKHR 7. VkCreateSurfaceKHR: The same thing happens as in step 1. This leads us to have default dmabuf modifiers again. (Note: it leaks VkSurface. That is the primary difference compared to Wolfenstein.) 9. VkCreateSwapchainKHR (old swapchain is not passed in) 10. ... and we eventually get another suboptimal since the per wl_surface dmabuf modifiers arrived, which causes the entire process to repeat infinitely. Wolfenstein Youngblood: 1. VkCreateWin32SurfaceKHR -> create new client surface -> create new wl_surface -> VkCreateWaylandSurfaceKHR -> WSI queries and stores the set of default DMABUF modifiers 3. VkCreateSwapchainKHR 4. [Do some presents] 5. Swapchain becomes suboptimal (because the compositor sent the modifiers for the wl_surface, which end up being different from the default one) 6. VkDestroySwapchainKHR 7. VkDestroySurfaceKHR -> destroys the client surface -> destroys the wl_surface from step 1 8. VkCreateSurfaceKHR: The same thing happens as in step 1. This leads us to have default dmabuf modifiers again. 9. VkCreateSwapchainKHR (old swapchain is not passed in) 10. ... and we eventually get another suboptimal since the per wl_surface dmabuf modifiers arrived, which causes the entire process to repeat infinitely. Another non-obvious important piece of information: if we use the same wl_surface for all the VkSurfaces for the same hwnd, then the compositor can remember and send the correct surface feedback much earlier, avoiding the problem. (I believe it is this roundtrip that helps: https://gitlab.freedesktop.org/mesa/mesa/-/blob/2046b79f2f043502a44bd86b64f2...) Therefore, to fix the above two cases, we need to: 1. Use the same wl_surface even after client_surface_release 2. Use the same wl_surface for each successive call to pCreateClientSurface. However, this would conflict with the OpenGL behavior according to dlls/win32u/opengl.c, so we would need to differentiate between an OpenGL and Vulkan client surface. Should I move this to win32u like the unused_drawable member of the WND structure? Note: Mesa 26.1 and lower have a WSI bug that causes this solution to not work in the case of VkSurface leaks, which I have fixed here: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/34918 -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11342
From: Etaash Mathamsetty <etaash.mathamsetty@gmail.com> --- dlls/winewayland.drv/wayland_surface.c | 3 --- dlls/winewayland.drv/window.c | 2 +- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/dlls/winewayland.drv/wayland_surface.c b/dlls/winewayland.drv/wayland_surface.c index c2b4c4be891..c0c69c9fcab 100644 --- a/dlls/winewayland.drv/wayland_surface.c +++ b/dlls/winewayland.drv/wayland_surface.c @@ -689,13 +689,10 @@ static void wayland_surface_reconfigure_client(struct wayland_surface *surface, struct wayland_client_surface *client, const RECT *client_rect) { - struct wayland_window_config *window = &surface->window; RECT rect = client->rect; /* The offset of the client area origin relatively to the window origin. */ if (client_rect) rect = *client_rect; - OffsetRect(&rect, window->client_rect.left - window->rect.left, - window->client_rect.top - window->rect.top); rect = map_rect_to_surface(surface, rect); TRACE("hwnd=%p rect=%s\n", surface->hwnd, wine_dbgstr_rect(&rect)); diff --git a/dlls/winewayland.drv/window.c b/dlls/winewayland.drv/window.c index 55385eb3772..54bb6000d94 100644 --- a/dlls/winewayland.drv/window.c +++ b/dlls/winewayland.drv/window.c @@ -169,7 +169,7 @@ static void reapply_cursor_clipping(void) { RECT rect; UINT context = NtUserSetThreadDpiAwarenessContext(NTUSER_DPI_PER_MONITOR_AWARE); - if (NtUserGetClipCursor(&rect )) NtUserClipCursor(&rect); + if (NtUserGetClipCursor(&rect)) NtUserClipCursor(&rect); NtUserSetThreadDpiAwarenessContext(context); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11342
From: Etaash Mathamsetty <etaash.mathamsetty@gmail.com> --- dlls/win32u/window.c | 1 + 1 file changed, 1 insertion(+) diff --git a/dlls/win32u/window.c b/dlls/win32u/window.c index 88caec3a3e9..043d236aa42 100644 --- a/dlls/win32u/window.c +++ b/dlls/win32u/window.c @@ -442,6 +442,7 @@ void add_window_client_surface( HWND hwnd, struct client_surface *surface ) pthread_mutex_lock( &surfaces_lock ); surface->hwnd = hwnd; + list_init( &surface->entry ); list_add_tail( &client_surfaces, &surface->entry ); client_surface_update_locked( surface ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11342
From: Etaash Mathamsetty <etaash.mathamsetty@gmail.com> --- dlls/win32u/vulkan.c | 10 +++++++++- include/wine/gdi_driver.h | 11 +++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/dlls/win32u/vulkan.c b/dlls/win32u/vulkan.c index 1bb4735587f..707cd82772a 100644 --- a/dlls/win32u/vulkan.c +++ b/dlls/win32u/vulkan.c @@ -1835,7 +1835,12 @@ static VkResult win32u_vkCreateSwapchainKHR( VkDevice client_device, const VkSwa return VK_ERROR_INITIALIZATION_FAILED; } - if (surface) create_info_host.surface = surface->obj.host.surface; + if (surface) + { + create_info_host.surface = surface->obj.host.surface; + if (surface->client && client_surface_grab( surface->client )) + return VK_ERROR_NATIVE_WINDOW_IN_USE_KHR; + } if (old_swapchain) create_info_host.oldSwapchain = old_swapchain->obj.host.swapchain; /* Windows allows client rect to be empty, but host Vulkan often doesn't, adjust extents back to the host capabilities */ @@ -1882,6 +1887,7 @@ void win32u_vkDestroySwapchainKHR( VkDevice client_device, VkSwapchainKHR client struct vulkan_device *device = vulkan_device_from_handle( client_device ); struct vulkan_instance *instance = device->physical_device->instance; struct swapchain *swapchain = swapchain_from_handle( client_swapchain ); + struct surface *surface = swapchain->surface; if (allocator) FIXME( "Support for allocation callbacks not implemented yet\n" ); if (!swapchain) return; @@ -1889,6 +1895,8 @@ void win32u_vkDestroySwapchainKHR( VkDevice client_device, VkSwapchainKHR client device->p_vkDestroySwapchainKHR( device->host.device, swapchain->obj.host.swapchain, NULL ); instance->p_remove_object( instance, &swapchain->obj.obj ); + if (surface && surface->client) client_surface_drop( surface->client ); + free( swapchain ); } diff --git a/include/wine/gdi_driver.h b/include/wine/gdi_driver.h index 384fdcac18e..38c61d0a80a 100644 --- a/include/wine/gdi_driver.h +++ b/include/wine/gdi_driver.h @@ -263,6 +263,7 @@ struct client_surface const struct client_surface_funcs *funcs; struct list entry; /* entry in win32u managed list */ LONG ref; /* reference count */ + LONG grab_ref; /* underlying WSI is using the surface */ HWND hwnd; /* window the surface was created for */ LONG updated; /* has been moved / resized / reparented */ HWND toplevel; /* toplevel window of the surface */ @@ -279,6 +280,16 @@ W32KAPI void client_surface_update( struct client_surface *surface ); W32KAPI void update_client_surfaces( HWND hwnd ); W32KAPI void detach_client_surfaces( HWND hwnd ); +static inline LONG client_surface_grab( struct client_surface *surface ) +{ + return InterlockedCompareExchange( &surface->grab_ref, 1, 0 ); +} + +static inline LONG client_surface_drop( struct client_surface *surface ) +{ + return InterlockedCompareExchange( &surface->grab_ref, 0, 1 ); +} + static inline const char *debugstr_client_surface( struct client_surface *surface ) { if (!surface) return "(null)"; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11342
From: Etaash Mathamsetty <etaash.mathamsetty@gmail.com> --- dlls/win32u/driver.c | 2 +- dlls/win32u/opengl.c | 2 +- dlls/win32u/vulkan.c | 2 +- dlls/wineandroid.drv/android.h | 2 +- dlls/wineandroid.drv/opengl.c | 2 +- dlls/winemac.drv/macdrv.h | 2 +- dlls/winemac.drv/window.c | 2 +- dlls/winewayland.drv/wayland_surface.c | 2 +- dlls/winewayland.drv/waylanddrv.h | 2 +- dlls/winex11.drv/init.c | 2 +- dlls/winex11.drv/x11drv.h | 2 +- include/wine/gdi_driver.h | 2 +- 12 files changed, 12 insertions(+), 12 deletions(-) diff --git a/dlls/win32u/driver.c b/dlls/win32u/driver.c index 6270d9bbe10..3a3f876b6f1 100644 --- a/dlls/win32u/driver.c +++ b/dlls/win32u/driver.c @@ -904,7 +904,7 @@ static const struct client_surface_funcs nulldrv_surface_funcs = .present = nulldrv_surface_present, }; -static struct client_surface *nulldrv_CreateClientSurface( HWND hwnd, int pixel_format ) +static struct client_surface *nulldrv_CreateClientSurface( HWND hwnd, int pixel_format, BOOL gl ) { return client_surface_create( sizeof(struct client_surface), &nulldrv_surface_funcs, hwnd ); } diff --git a/dlls/win32u/opengl.c b/dlls/win32u/opengl.c index 02f85c4fe34..a62b68c9642 100644 --- a/dlls/win32u/opengl.c +++ b/dlls/win32u/opengl.c @@ -1463,7 +1463,7 @@ static struct opengl_drawable *get_window_unused_drawable( HWND hwnd, int format { struct client_surface *client; - if (!(client = user_driver->pCreateClientSurface( hwnd, format ))) + if (!(client = user_driver->pCreateClientSurface( hwnd, format, TRUE ))) WARN( "Failed to create a surface for window %p, format %d\n", hwnd, format ); else { diff --git a/dlls/win32u/vulkan.c b/dlls/win32u/vulkan.c index 707cd82772a..f551b0d755d 100644 --- a/dlls/win32u/vulkan.c +++ b/dlls/win32u/vulkan.c @@ -1533,7 +1533,7 @@ static VkResult win32u_vkCreateWin32SurfaceKHR( VkInstance client_instance, cons surface->hwnd = dummy; } - if (!(surface->client = user_driver->pCreateClientSurface( surface->hwnd, 0 ))) res = VK_ERROR_OUT_OF_HOST_MEMORY; + if (!(surface->client = user_driver->pCreateClientSurface( surface->hwnd, 0, FALSE ))) res = VK_ERROR_OUT_OF_HOST_MEMORY; else res = driver_funcs->p_vulkan_surface_create( surface->client, instance, &host_surface ); if (res) { diff --git a/dlls/wineandroid.drv/android.h b/dlls/wineandroid.drv/android.h index 3e69e52c736..0c916069f93 100644 --- a/dlls/wineandroid.drv/android.h +++ b/dlls/wineandroid.drv/android.h @@ -115,7 +115,7 @@ extern void ANDROID_SetCapture( HWND hwnd, UINT flags, HWND previous ); extern UINT ANDROID_ShowWindow( HWND hwnd, INT cmd, RECT *rect, UINT swp ); extern LRESULT ANDROID_WindowMessage( HWND hwnd, UINT msg, WPARAM wp, LPARAM lp ); extern BOOL ANDROID_WindowPosChanging( HWND hwnd, UINT swp_flags, BOOL shaped, const struct window_rects *rects ); -extern struct client_surface *ANDROID_CreateClientSurface( HWND hwnd, int pixel_format ); +extern struct client_surface *ANDROID_CreateClientSurface( HWND hwnd, int pixel_format, BOOL gl ); extern BOOL ANDROID_CreateWindowSurface( HWND hwnd, BOOL layered, const RECT *surface_rect, struct window_surface **surface ); extern void ANDROID_WindowPosChanged( HWND hwnd, HWND insert_after, HWND owner_hint, UINT swp_flags, const struct window_rects *new_rects, struct window_surface *surface ); diff --git a/dlls/wineandroid.drv/opengl.c b/dlls/wineandroid.drv/opengl.c index 12bae7fca29..77eea1ad5a4 100644 --- a/dlls/wineandroid.drv/opengl.c +++ b/dlls/wineandroid.drv/opengl.c @@ -189,7 +189,7 @@ static const struct client_surface_funcs android_client_surface_funcs = .present = android_client_surface_present, }; -struct client_surface *ANDROID_CreateClientSurface( HWND hwnd, int pixel_format ) +struct client_surface *ANDROID_CreateClientSurface( HWND hwnd, int pixel_format, BOOL gl ) { return client_surface_create( sizeof(struct client_surface), &android_client_surface_funcs, hwnd ); } diff --git a/dlls/winemac.drv/macdrv.h b/dlls/winemac.drv/macdrv.h index aeeadae122e..5c9a2676f4d 100644 --- a/dlls/winemac.drv/macdrv.h +++ b/dlls/winemac.drv/macdrv.h @@ -150,7 +150,7 @@ extern void macdrv_SetLayeredWindowAttributes(HWND hwnd, COLORREF key, BYTE alph extern LRESULT macdrv_WindowMessage(HWND hwnd, UINT msg, WPARAM wp, LPARAM lp); extern BOOL macdrv_WindowPosChanging(HWND hwnd, UINT swp_flags, BOOL shaped, const struct window_rects *rects); extern BOOL macdrv_GetWindowStyleMasks(HWND hwnd, UINT style, UINT ex_style, UINT *style_mask, UINT *ex_style_mask); -extern struct client_surface *macdrv_CreateClientSurface(HWND hwnd, int pixel_format); +extern struct client_surface *macdrv_CreateClientSurface(HWND hwnd, int pixel_format, BOOL gl); extern BOOL macdrv_CreateWindowSurface(HWND hwnd, BOOL layered, const RECT *surface_rect, struct window_surface **surface); extern void macdrv_WindowPosChanged(HWND hwnd, HWND insert_after, HWND owner_hint, UINT swp_flags, const struct window_rects *new_rects, struct window_surface *surface); diff --git a/dlls/winemac.drv/window.c b/dlls/winemac.drv/window.c index c0384f3f0d5..fa4ac8858bb 100644 --- a/dlls/winemac.drv/window.c +++ b/dlls/winemac.drv/window.c @@ -1145,7 +1145,7 @@ struct macdrv_client_surface *impl_from_client_surface(struct client_surface *cl return CONTAINING_RECORD(client, struct macdrv_client_surface, client); } -struct client_surface *macdrv_CreateClientSurface(HWND hwnd, int pixel_format) +struct client_surface *macdrv_CreateClientSurface(HWND hwnd, int pixel_format, BOOL gl) { struct macdrv_client_surface *surface; diff --git a/dlls/winewayland.drv/wayland_surface.c b/dlls/winewayland.drv/wayland_surface.c index c0c69c9fcab..644ad3b8559 100644 --- a/dlls/winewayland.drv/wayland_surface.c +++ b/dlls/winewayland.drv/wayland_surface.c @@ -1238,7 +1238,7 @@ struct wayland_client_surface *impl_from_client_surface(struct client_surface *c return CONTAINING_RECORD(client, struct wayland_client_surface, client); } -struct client_surface *WAYLAND_CreateClientSurface(HWND hwnd, int pixel_format) +struct client_surface *WAYLAND_CreateClientSurface(HWND hwnd, int pixel_format, BOOL gl) { struct wayland_client_surface *client; struct wl_region *empty_region; diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index 0ffc15581c1..6e2f6ab5608 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -468,7 +468,7 @@ LRESULT WAYLAND_WindowMessage(HWND hwnd, UINT msg, WPARAM wp, LPARAM lp); void WAYLAND_WindowPosChanged(HWND hwnd, HWND insert_after, HWND owner_hint, UINT swp_flags, const struct window_rects *new_rects, struct window_surface *surface); BOOL WAYLAND_WindowPosChanging(HWND hwnd, UINT swp_flags, BOOL shaped, const struct window_rects *rects); -struct client_surface *WAYLAND_CreateClientSurface(HWND hwnd, int pixel_format); +struct client_surface *WAYLAND_CreateClientSurface(HWND hwnd, int pixel_format, BOOL gl); BOOL WAYLAND_CreateWindowSurface(HWND hwnd, BOOL layered, const RECT *surface_rect, struct window_surface **surface); UINT WAYLAND_VulkanInit(UINT version, void *vulkan_handle, const struct vulkan_driver_funcs **driver_funcs); UINT WAYLAND_OpenGLInit(UINT version, const struct opengl_funcs *opengl_funcs, const struct opengl_driver_funcs **driver_funcs); diff --git a/dlls/winex11.drv/init.c b/dlls/winex11.drv/init.c index 754349f6892..4d2ef1379fa 100644 --- a/dlls/winex11.drv/init.c +++ b/dlls/winex11.drv/init.c @@ -433,7 +433,7 @@ struct x11drv_client_surface *impl_from_client_surface( struct client_surface *c return CONTAINING_RECORD( client, struct x11drv_client_surface, client ); } -struct client_surface *X11DRV_CreateClientSurface( HWND hwnd, int format ) +struct client_surface *X11DRV_CreateClientSurface( HWND hwnd, int format, BOOL gl ) { struct x11drv_client_surface *surface; XVisualInfo visual = default_visual; diff --git a/dlls/winex11.drv/x11drv.h b/dlls/winex11.drv/x11drv.h index fc59ac9f3b9..4414ac25608 100644 --- a/dlls/winex11.drv/x11drv.h +++ b/dlls/winex11.drv/x11drv.h @@ -251,7 +251,7 @@ extern LRESULT X11DRV_WindowMessage( HWND hwnd, UINT msg, WPARAM wp, LPARAM lp ) extern BOOL X11DRV_WindowPosChanging( HWND hwnd, UINT swp_flags, BOOL shaped, const struct window_rects *rects ); extern BOOL X11DRV_GetWindowStyleMasks( HWND hwnd, UINT style, UINT ex_style, UINT *style_mask, UINT *ex_style_mask ); extern BOOL X11DRV_GetWindowStateUpdates( HWND hwnd, UINT *state_cmd, UINT *swp_flags, RECT *rect, HWND *foreground ); -extern struct client_surface *X11DRV_CreateClientSurface( HWND hwnd, int format ); +extern struct client_surface *X11DRV_CreateClientSurface( HWND hwnd, int format, BOOL gl ); extern BOOL X11DRV_CreateWindowSurface( HWND hwnd, BOOL layered, const RECT *surface_rect, struct window_surface **surface ); extern void X11DRV_MoveWindowBits( HWND hwnd, const struct window_rects *old_rects, const struct window_rects *new_rects, const RECT *valid_rects ); diff --git a/include/wine/gdi_driver.h b/include/wine/gdi_driver.h index 38c61d0a80a..c17482c08a2 100644 --- a/include/wine/gdi_driver.h +++ b/include/wine/gdi_driver.h @@ -435,7 +435,7 @@ struct user_driver_funcs BOOL (*pWindowPosChanging)(HWND,UINT,BOOL,const struct window_rects *); BOOL (*pGetWindowStyleMasks)(HWND,UINT,UINT,UINT*,UINT*); BOOL (*pGetWindowStateUpdates)(HWND,UINT*,UINT*,RECT*,HWND*); - struct client_surface *(*pCreateClientSurface)(HWND,int); + struct client_surface *(*pCreateClientSurface)(HWND,int,BOOL); BOOL (*pCreateWindowSurface)(HWND,BOOL,const RECT *,struct window_surface**); void (*pMoveWindowBits)(HWND,const struct window_rects *,const struct window_rects *,const RECT *); void (*pWindowPosChanged)(HWND,HWND,HWND,UINT,const struct window_rects*,struct window_surface*); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11342
From: Etaash Mathamsetty <etaash.mathamsetty@gmail.com> --- dlls/winewayland.drv/wayland_surface.c | 28 +++++++++++++++++++++++--- dlls/winewayland.drv/waylanddrv.h | 2 ++ dlls/winewayland.drv/window.c | 1 + 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/dlls/winewayland.drv/wayland_surface.c b/dlls/winewayland.drv/wayland_surface.c index 644ad3b8559..78c387015f5 100644 --- a/dlls/winewayland.drv/wayland_surface.c +++ b/dlls/winewayland.drv/wayland_surface.c @@ -1238,15 +1238,35 @@ struct wayland_client_surface *impl_from_client_surface(struct client_surface *c return CONTAINING_RECORD(client, struct wayland_client_surface, client); } +struct wayland_client_surface *update_vulkan_client_surface(HWND hwnd, struct client_surface *client) +{ + struct wayland_win_data *data; + + if ((data = wayland_win_data_get(hwnd))) + { + if (client && !data->vulkan_client) data->vulkan_client = client; + else if (!client) client = data->vulkan_client; + wayland_win_data_release(data); + } + + if (client) + { + client_surface_add_ref(client); + return impl_from_client_surface(client); + } + + return NULL; +} + struct client_surface *WAYLAND_CreateClientSurface(HWND hwnd, int pixel_format, BOOL gl) { - struct wayland_client_surface *client; + struct wayland_client_surface *client = NULL; struct wl_region *empty_region; + if (!gl && (client = update_vulkan_client_surface(hwnd, NULL))) return &client->client; if (!(client = client_surface_create(sizeof(*client), &wayland_client_surface_funcs, hwnd))) return NULL; - client->wl_surface = - wl_compositor_create_surface(process_wayland.wl_compositor); + client->wl_surface = wl_compositor_create_surface(process_wayland.wl_compositor); if (!client->wl_surface) { ERR("Failed to create client wl_surface\n"); @@ -1273,6 +1293,8 @@ struct client_surface *WAYLAND_CreateClientSurface(HWND hwnd, int pixel_format, goto err; } + if (!gl) update_vulkan_client_surface(hwnd, &client->client); + return &client->client; err: diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index 6e2f6ab5608..b027dd05cae 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -378,6 +378,8 @@ struct wayland_win_data struct wayland_surface *wayland_surface; /* wayland client surface (if any) for this window */ struct wayland_client_surface *client_surface; + /* dedicated client surface for vulkan apps on this window */ + struct client_surface *vulkan_client; /* window rects, relative to parent client area */ struct window_rects rects; BOOL is_fullscreen; diff --git a/dlls/winewayland.drv/window.c b/dlls/winewayland.drv/window.c index 54bb6000d94..98ca6f1fd13 100644 --- a/dlls/winewayland.drv/window.c +++ b/dlls/winewayland.drv/window.c @@ -99,6 +99,7 @@ static void wayland_win_data_destroy(struct wayland_win_data *data) pthread_mutex_unlock(&win_data_mutex); + if (data->vulkan_client) client_surface_release(data->vulkan_client); if (data->wayland_surface) wayland_surface_destroy(data->wayland_surface); if (data->window_contents) wayland_shm_buffer_unref(data->window_contents); free(data); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11342
Etaash Mathamsetty (@etaash.mathamsetty) commented about dlls/winewayland.drv/waylanddrv.h:
struct wayland_surface *wayland_surface; /* wayland client surface (if any) for this window */ struct wayland_client_surface *client_surface; + /* dedicated client surface for vulkan apps on this window */ + struct client_surface *vulkan_client;
Should I move this to win32u like the unused_drawable member of the WND structure? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11342#note_145138
I'm really not sure about this. It's possible to have multiple D3D swapchains at the same time for the same window, or D3D *and* Vulkan, and the last one to present being the one visible. All this will end up with multiple VkSurfaceKHR created for the same window and I believe we need to use multiple wl_surface? Are we really only returning SUBOPTIMAL and are the games really recreating VkSurfaceKHR when seeing that result? Wouldn't that only require to recreate the swapchain? Fwiw, if Wayland really returns SUBOPTIMAL right after a swapchain creation when it is first used, and if applications are known to recreate surfaces entirely, it seems to me that this whole issue is a severe problem with Wayland Vulkan WSI implementation, and that it needs to be fixed there, not in Wine. And swapchains being SUBOPTIMAL right on creation seems like a bad choice in the first place. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11342#note_145140
Are we really only returning SUBOPTIMAL and are the games really recreating VkSurfaceKHR when seeing that result? Wouldn't that only require to recreate the swapchain?
Yes the games are broken in this way. Unfortunately, almost every single vulkan app written for windows does this for some reason and I just don't get it. (DXVK, vkd3d-proton and gravity mark don't just to name a few)
I'm really not sure about this. It's possible to have multiple D3D swapchains at the same time for the same window, or D3D _and_ Vulkan, and the last one to present being the one visible. All this will end up with multiple VkSurfaceKHR created for the same window and I believe we need to use multiple wl_surface?
According to the Vulkan spec, it is not legal to create multiple swapchains on the same underlying native surface and this would result in validation error.
Fwiw, if Wayland really returns SUBOPTIMAL right after a swapchain creation when it is first used, and if applications are known to recreate surfaces entirely, it seems to me that this whole issue is a severe problem with Wayland Vulkan WSI implementation, and that it needs to be fixed there, not in Wine. And swapchains being SUBOPTIMAL right on creation seems like a bad choice in the first place.
It cannot be fixed there for the foreseeable future (It would require a redesign of the dmabuf protocol, which has become stable fwiw. And even if it was redesigned, I'm not sure if it will fix this problem anyways, even xwayland is suboptimal to begin with but it just doesn't have this problem for whatever reason). I'll just rework the approach in this MR to allow multiple swapchains per HWND -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11342#note_145142
According to the Vulkan spec, it is not legal to create multiple swapchains on the same underlying native surface and this would result in validation error.
Yes, so that's what I am saying and we need to use one wl_surface for each VkSurfaceKHR the application may allocate. We could keep destroyed ones around maybe, and reuse them, but again it seems to me that this is all a bad design of the protocol, and that it *needs* to be redesigned. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11342#note_145143
Yes, so that's what I am saying and we need to use one wl_surface for each VkSurfaceKHR the application may allocate.
Yeah, but in the case of windows the native surface would be an HWND, so what wine does would not be legal according to the vulkan spec. However, clearly Windows behaves differently, so that overrides anything the vulkan spec can say.
it seems to me that this is all a bad design of the protocol, and that it _needs_ to be redesigned.
It might be, it might not be. Either way, I doubt one client can convince them to change a protocol that has been there for ~10 years. Especially considering XWayland also is suboptimal to start with (and nowhere in the vulkan spec does it say that suboptimal to begin with is illegal, because you aren't supposed to be creating a new VkSurface for every suboptimal in the first place) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11342#note_145146
On Wed Jul 8 07:00:21 2026 +0000, Etaash Mathamsetty wrote:
Yes, so that's what I am saying and we need to use one wl_surface for each VkSurfaceKHR the application may allocate. Yeah, but in the case of windows the native surface would be an HWND, so what wine does would not be legal according to the vulkan spec. However, clearly Windows behaves differently, so that overrides anything the vulkan spec can say. it seems to me that this is all a bad design of the protocol, and that it _needs_ to be redesigned. It might be, it might not be. Either way, I doubt one client can convince them to change a protocol that has been there for ~10 years. Especially considering XWayland also is suboptimal to start with (and nowhere in the vulkan spec does it say that suboptimal to begin with is illegal, because you aren't supposed to be creating a new VkSurface for every suboptimal in the first place) It's not about one client, it's about applications that respond to suboptimal by recreating their surfaces (which needs to be confirmed first, it could be coming from else where, dxvk or wined3d, idk).
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/11342#note_145147
It's not about one client, it's about applications that respond to suboptimal by recreating their surfaces (which needs to be confirmed first, it could be coming from else where, dxvk or wined3d, idk).
I haven't worked on any compositors yet, but I do know there are several factors that control the modifiers (since they are just an optimization for copying the buffer efficiently), and many of those cannot be determined at the swapchain creation time. (such as the position of the window, the window having focus, subsurfaces on top of the surface, etc) It is perfectly reasonable, IMO, to be suboptimal to begin with due to the many unknowns.
which needs to be confirmed first, it could be coming from else where, dxvk or wined3d, idk
It comes from the game (iirc), it can be seen from WINEDEBUG=+seh the game prints a message whenever it creates a new surface and swapchain (they both are native vulkan games, and these issues only occur on native vulkan games and apps). I'll double check this -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11342#note_145148
I haven't worked on any compositors yet, but I do know there are several factors that control the modifiers (since they are just an optimization for copying the buffer efficiently), and many of those cannot be determined at the swapchain creation time. (such as the position of the window, the window having focus, subsurfaces on top of the surface, etc) It is perfectly reasonable, IMO, to be suboptimal to begin with due to the many unknowns.
I'm not saying it's not reasonable, I'm saying that if applications don't care (especially closed source ones which don't care about being good citizen) and they recreate their surfaces when they don't need to, it's not really up to Wine to mitigate that. IMO the WSI should be aware of such issues and take them into consideration in their design, regardless of what is reasonable or possible by spec. Fwiw you could (and possibly do) very well have Linux native games doing the same thing, and Wine wouldn't be able to help in any way here. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11342#note_145150
I'm not saying it's not reasonable, I'm saying that if applications don't care (especially closed source ones which don't care about being good citizen) and they recreate their surfaces when they don't need to, it's not really up to Wine to mitigate that. IMO the WSI should be aware of such issues and take them into consideration in their design, regardless of what is reasonable or possible by spec.
That is true. However, if an app doesn't work on the platform it is meant to work on would cause the app developer to fix the bug on their side. The Wayland WSI works in a way that is legal by the Vulkan spec; Windows also works in a legal way. But there are a few handful of windows apps that don't work on wayland, and IMO that is Wine's job to correct since it translates the windows interpretation of these specs to the host system. Regardless, this bug is very difficult to trigger "accidentally" on a native app; you need to create a new wl_surface, VkSurface, and swapchain for every suboptimal. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11342#note_145152
if Wayland really returns SUBOPTIMAL right after a swapchain creation when it is first used, and if applications are known to recreate surfaces entirely, it seems to me that this whole issue is a severe problem with Wayland Vulkan WSI implementation, and that it needs to be fixed there
It's not quite that simple. Compositors send dmabuf-feedback for each surface, aka a list of GPU+formats+modifers+scanout hint, sorted by preference. Initially, that list is optimized for rendering, because that's the default assumption, and the swapchain is *not* suboptimal. Once the surface is shown, the compositor may try to offload the surface to a hardware plane. This *could* be immediately on the first frame, or five minutes later. If the current buffer isn't suitable for the hardware plane in question, it will send new dmabuf-feedback matching that surface as the most preferred configuration. If it does that, the Vulkan driver marks the swapchain as suboptimal. That feedback matching the hardware plane can't be sent immediately because until the surface is shown, the compositor has no idea if it would even attempt to put it on the plane. There's lots of reasons why it might not, like - there's blur on top of the surface - the surface doesn't update quickly enough for the offloading to be more efficient than copying - the required color transformations can't be represented with the plane - hardware specific stuff like the size not being compatible, or scaling, or the position not being an even number, or whatever Lots of hardware additionally has different formats+modifiers for each plane, making the whole thing even more complicated and necessarily dynamic. So, I don't see any way this could be fixed. I also don't know how Windows avoids the same issue, since it has very similar constraints.
Fwiw you could (and possibly do) very well have Linux native games doing the same thing, and Wine wouldn't be able to help in any way here.
Native games don't tend to use subsurfaces, and re-creating the toplevel surface means hiding and showing the window again. It would be immediately obvious that it's broken. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11342#note_145242
On Wed Jul 8 23:11:45 2026 +0000, Zamundaaa wrote: > > if Wayland really returns SUBOPTIMAL right after a swapchain creation > when it is first used, and if applications are known to recreate > surfaces entirely, it seems to me that this whole issue is a severe > problem with Wayland Vulkan WSI implementation, and that it needs to be > fixed there > It's not quite that simple. Compositors send dmabuf-feedback for each > surface, aka a list of GPU+formats+modifers+scanout hint, sorted by > preference. Initially, that list is optimized for rendering, because > that's the default assumption, and the swapchain is *not* suboptimal. > Once the surface is shown, the compositor may try to offload the surface > to a hardware plane. This *could* be immediately on the first frame, or > five minutes later. If the current buffer isn't suitable for the > hardware plane in question, it will send new dmabuf-feedback matching > that surface as the most preferred configuration. If it does that, the > Vulkan driver marks the swapchain as suboptimal. > That feedback matching the hardware plane can't be sent immediately > because until the surface is shown, the compositor has no idea if it > would even attempt to put it on the plane. There's lots of reasons why > it might not, like > - there's blur on top of the surface > - the surface doesn't update quickly enough for the offloading to be > more efficient than copying > - the required color transformations can't be represented with the plane > - hardware specific stuff like the size not being compatible, or > scaling, or the position not being an even number, or whatever > Lots of hardware additionally has different formats+modifiers for each > plane, making the whole thing even more complicated and necessarily dynamic. > So, I don't see any way this could be fixed. I also don't know how > Windows avoids the same issue, since it has very similar constraints. > > Fwiw you could (and possibly do) very well have Linux native games > doing the same thing, and Wine wouldn't be able to help in any way here. > Native games don't tend to use subsurfaces, and re-creating the toplevel > surface means hiding and showing the window again. It would be > immediately obvious that it's broken. So I understand that keeping the wl_surface alive is enough? I also understand from this MR that a wl_surface may be reused for a different Vulkan surface & swapchain it was using initially, so it's not an issue if we reuse the wrong one, but it's important we keep them alive. So, would something like https://gitlab.winehq.org/wine/wine/-/merge_requests/11351 be enough? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11342#note_145249
On Thu Jul 9 07:15:02 2026 +0000, Rémi Bernon wrote:
So I understand that keeping the wl_surface alive is enough? I also understand from this MR that a wl_surface may be reused for a different Vulkan surface & swapchain it was using initially, so it's not an issue if we reuse the wrong one, but it's important we keep them alive. So, would something like https://gitlab.winehq.org/wine/wine/-/merge_requests/11351 be enough? (Fwiw this version discards any unused surface whenever SetWindowPos is called though we could keep them around longer, eventually even resizing them but I am afraid of it causing surfaces staying unused but alive unnecessarily) Yes, keeping the wl_surface alive is enough, and assigning the wl_surface to a different VkSurface (of a different HWND) is fine too. I like the approach in !11351, but maybe it would be better to clear out all the unused client surfaces inside destroy_window like how it is done with win->unused_drawable, since the app can call SetWindowPos whenever it wants.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/11342#note_145252
On Thu Jul 9 07:39:04 2026 +0000, Etaash Mathamsetty wrote:
Yes, keeping the wl_surface alive is enough, and assigning the wl_surface to a different VkSurface (of a different or same HWND) is fine too. I like the approach in !11351, but maybe it would be better to clear out all the unused client surfaces inside destroy_window like how it is done with win->unused_drawable, since the app can call SetWindowPos whenever it wants. However, if the app destroys VkSurface and then keeps the window alive, then we are just holding on to them for no reason. Maybe limiting it to some fixed number could fix that (even holding onto just 1 client surface is enough to fix all the apps that I know have issues) That is also done of course, in detach_client_surfaces.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/11342#note_145253
On Thu Jul 9 07:41:18 2026 +0000, Rémi Bernon wrote:
maybe it would be better to clear out all the unused client surfaces inside destroy_window like how it is done with win-\>unused_drawable That is also done of course, in detach_client_surfaces. Cool, then I'm only worried about SetWindowPos; I'll see if Doom Eternal calls that during its swapchain recreation
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/11342#note_145254
So I understand that keeping the wl_surface alive is enough?
As long as you also don't unmap it, since that can reset the dmabuf feedback to the default again. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11342#note_145282
This merge request was closed by Rémi Bernon. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11342
I think this has been fixed with https://gitlab.winehq.org/wine/wine/-/merge_requests/11351? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11342#note_149376
participants (4)
-
Etaash Mathamsetty -
Etaash Mathamsetty (＠etaash.mathamsetty)
-
Rémi Bernon (＠rbernon)
-
Zamundaaa (＠Zamundaaa)