[PATCH 0/1] MR5318: winevulkan: Allow only one vulkan surface at a time for an HWND.

[Rémi Bernon]

Fixes bb872831deb48382e4733979e00243e5122385d0 which lost this from the
winex11 driver. This might be something we want to relax at some point
to allow multiple APIs to draw to the same HWND, but it was done like
that before and it is technically a regression.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56380

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5318

[PATCH 1/1] winevulkan: Allow only one vulkan surface at a time for an HWND.

[Rémi Bernon]

From: Rémi Bernon <rbernon(a)codeweavers.com>

Fixes bb872831deb48382e4733979e00243e5122385d0 which lost this from the
winex11 driver. This might be something we want to relax at some point
to allow multiple APIs to draw to the same HWND, but it was done like
that before and it is technically a regression.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56380
---
 dlls/winevulkan/vulkan.c         | 41 ++++++++++++++++++++++++++++++++
 dlls/winevulkan/vulkan_private.h |  2 ++
 2 files changed, 43 insertions(+)

diff --git a/dlls/winevulkan/vulkan.c b/dlls/winevulkan/vulkan.c
index 9981bd0141e..0846f2dfc73 100644
--- a/dlls/winevulkan/vulkan.c
+++ b/dlls/winevulkan/vulkan.c
@@ -26,11 +26,50 @@
 
 #include "vulkan_private.h"
 #include "wine/vulkan_driver.h"
+#include "wine/rbtree.h"
 #include "ntgdi.h"
 #include "ntuser.h"
 
 WINE_DEFAULT_DEBUG_CHANNEL(vulkan);
 
+static int window_surface_compare(const void *key, const struct rb_entry *entry)
+{
+    const struct wine_surface *surface = RB_ENTRY_VALUE(entry, struct wine_surface, window_entry);
+    HWND key_hwnd = (HWND)key;
+
+    if (key_hwnd < surface->hwnd) return -1;
+    if (key_hwnd > surface->hwnd) return 1;
+    return 0;
+}
+
+static pthread_mutex_t window_surfaces_lock = PTHREAD_MUTEX_INITIALIZER;
+static struct rb_tree window_surfaces = {.compare = window_surface_compare};
+
+static void window_surfaces_insert(struct wine_surface *surface)
+{
+    struct wine_surface *previous;
+    struct rb_entry *ptr;
+
+    pthread_mutex_lock(&window_surfaces_lock);
+
+    if (!(ptr = rb_get(&window_surfaces, surface->hwnd)))
+        rb_put(&window_surfaces, surface->hwnd, &surface->window_entry);
+    else
+    {
+        previous = RB_ENTRY_VALUE(ptr, struct wine_surface, window_entry);
+        rb_replace(&window_surfaces, &previous->window_entry, &surface->window_entry);
+        previous->hwnd = 0; /* make sure previous surface becomes invalid */
+    }
+
+    pthread_mutex_unlock(&window_surfaces_lock);
+}
+
+static void window_surfaces_remove(struct wine_surface *surface)
+{
+    pthread_mutex_lock(&window_surfaces_lock);
+    if (surface->hwnd) rb_remove(&window_surfaces, &surface->window_entry);
+    pthread_mutex_unlock(&window_surfaces_lock);
+}
 
 static BOOL is_wow64(void)
 {
@@ -1542,6 +1581,7 @@ VkResult wine_vkCreateWin32SurfaceKHR(VkInstance handle, const VkWin32SurfaceCre
 
     object->host_surface = vk_funcs->p_wine_get_host_surface(object->driver_surface);
     if (dummy) NtUserDestroyWindow(dummy);
+    window_surfaces_insert(object);
 
     *surface = wine_surface_to_handle(object);
     add_handle_mapping(instance, *surface, object->host_surface, &object->wrapper_entry);
@@ -1559,6 +1599,7 @@ void wine_vkDestroySurfaceKHR(VkInstance handle, VkSurfaceKHR surface,
 
     instance->funcs.p_vkDestroySurfaceKHR(instance->host_instance, object->driver_surface, NULL);
     remove_handle_mapping(instance, &object->wrapper_entry);
+    window_surfaces_remove(object);
 
     free(object);
 }
diff --git a/dlls/winevulkan/vulkan_private.h b/dlls/winevulkan/vulkan_private.h
index f65b285e02a..b44285722a7 100644
--- a/dlls/winevulkan/vulkan_private.h
+++ b/dlls/winevulkan/vulkan_private.h
@@ -27,6 +27,7 @@
 
 #include "vulkan_loader.h"
 #include "vulkan_thunks.h"
+#include "wine/rbtree.h"
 
 #include "wine/rbtree.h"
 
@@ -236,6 +237,7 @@ struct wine_surface
     VkSurfaceKHR driver_surface;
     HWND hwnd;
 
+    struct rb_entry window_entry;
     struct wrapper_entry wrapper_entry;
 };
 
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/5318

[Jacek Caban (＠jacek)]

Jacek Caban (@jacek) commented about dlls/winevulkan/vulkan.c:
> #include "vulkan_private.h"
>  #include "wine/vulkan_driver.h"
> +#include "wine/rbtree.h"
>  #include "ntgdi.h"
>  #include "ntuser.h"
>
> WINE_DEFAULT_DEBUG_CHANNEL(vulkan);
>
> +static int window_surface_compare(const void *key, const struct rb_entry *entry)
> +{
> +    const struct wine_surface *surface = RB_ENTRY_VALUE(entry, struct wine_surface, window_entry);
> +    HWND key_hwnd = (HWND)key;

HWND is a void pointer, there is no need for the cast (nor the variable).

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5318#note_65365

[Jacek Caban (＠jacek)]

FWIW, this is an example of things that would be nicer in win32u, using the window struct to store the surface. I guess we could introduce a function that would set the surface and return previous one in win32u potentially. Anyway, I'm fine with it for now.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5318#note_65366

[Rémi Bernon]

On Tue Mar 19 16:11:34 2024 +0000, Jacek Caban wrote:
> HWND is a void pointer, there is no need for the cast (nor the variable).
> It's actually needed because key is a pointer to const.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5318#note_65372

[Rémi Bernon]

On Tue Mar 19 16:48:03 2024 +0000, Jacek Caban wrote:
> FWIW, this is an example of things that would be nicer in win32u, using
> the window struct to store the surface. I guess we could introduce a
> function that would set the surface and return previous one in win32u
> potentially. Anyway, I'm fine with it for now.
> Yes, and I have some local patches to start putting some vulkan logic in win32u directly, mainly to implement child and other process window rendering, with a fallback to GDI copy when needed, but I'm waiting on https://gitlab.winehq.org/wine/wine/-/merge_requests/5306 and still overall not entirely sure about the implementation yet.

This was there in the meantime to fix a regression, and I'm also still hoping we could perhaps drop it somehow in the future. Like I said in the description above, although not Vulkan compliant, having multiple Vulkan surfaces for the same HWND would be neeeded in Wine to have multiple APIs (D3D + Vulkan + OpenGL if done over Vulkan) drawing to the same window.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5318#note_65377

[PATCH 0/1] MR5318: winevulkan: Allow only one vulkan surface at a time for an HWND. - approved

[Jacek Caban (＠jacek)]

This merge request was approved by Jacek Caban.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5318