[PATCH v3 0/9] MR4533: winex11: Detach / attach vulkan window surfaces as needed.

[Rémi Bernon]

This is some preparatory work for vulkan child window and other features which currently live in Proton. I think this could be upstream already, and it will make rebasing patches on top of it simpler. To get vulkan child window upstream we'd probably still need to rework it a bit and maybe try to move the wait and blit to `vkQueuePresent` somehow.

The idea here is also that although Vulkan spec does not allow multiple active swapchains for the same window, Windows tends to allow it especially with D3D, and we also need to support the case where multiple APIs are use to draw onto the same window surface (as we implement D3D on top of GL/Vk for instance).

--
  v3: winex11: Rename X11DRV_FLUSH_GL_DRAWABLE to X11DRV_PRESENT_DRAWABLE.
      winex11: Put detached client windows surfaces offscreen.
      winex11: Re-attach vulkan surfaces when images are acquired.
      winevulkan: Add vkAcquireNextImage(2)KHR driver entries.
      winex11: Resize detached vulkan surfaces when window is resized.
      winex11: Detach the vulkan surfaces when their HWND is destroyed.
      winex11: Detach vulkan client windows before destroying them.
      winex11: Introduce a new detach_client_window helper.
      winex11: Add traces to vulkan surface grab / release.
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4533

[PATCH v3 1/9] winex11: Add traces to vulkan surface grab / release.

[Rémi Bernon]

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

---
 dlls/winex11.drv/vulkan.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/dlls/winex11.drv/vulkan.c b/dlls/winex11.drv/vulkan.c
index 101504a7887..f8987e2c49d 100644
--- a/dlls/winex11.drv/vulkan.c
+++ b/dlls/winex11.drv/vulkan.c
@@ -196,16 +196,18 @@ static VkResult wine_vk_instance_convert_create_info(const VkInstanceCreateInfo
     return VK_SUCCESS;
 }
 
-static struct wine_vk_surface *wine_vk_surface_grab(struct wine_vk_surface *surface)
+static struct wine_vk_surface *wine_vk_surface_grab( struct wine_vk_surface *surface )
 {
-    InterlockedIncrement(&surface->ref);
+    int refcount = InterlockedIncrement( &surface->ref );
+    TRACE( "surface %p, refcount %d.\n", surface, refcount );
     return surface;
 }
 
-static void wine_vk_surface_release(struct wine_vk_surface *surface)
+static void wine_vk_surface_release( struct wine_vk_surface *surface )
 {
-    if (InterlockedDecrement(&surface->ref))
-        return;
+    int refcount = InterlockedDecrement( &surface->ref );
+    TRACE( "surface %p, refcount %d.\n", surface, refcount );
+    if (refcount) return;
 
     if (surface->entry.next)
     {
-- 
GitLab


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

[PATCH v3 2/9] winex11: Introduce a new detach_client_window helper.

[Rémi Bernon]

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

---
 dlls/winex11.drv/window.c | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/dlls/winex11.drv/window.c b/dlls/winex11.drv/window.c
index 53982bb8c3b..4237e92990e 100644
--- a/dlls/winex11.drv/window.c
+++ b/dlls/winex11.drv/window.c
@@ -1549,6 +1549,24 @@ Window get_dummy_parent(void)
 }
 
 
+/**********************************************************************
+ *		detach_client_window
+ */
+static void detach_client_window( struct x11drv_win_data *data, Window window )
+{
+    if (!data->client_window || data->client_window != window) return;
+
+    XSelectInput( data->display, data->client_window, 0 );
+    XFlush( data->display ); /* make sure XSelectInput is disabled for client_window after this point */
+    XDeleteContext( data->display, data->client_window, winContext );
+
+    XReparentWindow( gdi_display, data->client_window, get_dummy_parent(), 0, 0 );
+    TRACE( "%p/%lx detached client window %lx\n", data->hwnd, data->whole_window, data->client_window );
+
+    data->client_window = 0;
+}
+
+
 /**********************************************************************
  *		create_dummy_client_window
  */
@@ -1588,12 +1606,7 @@ Window create_client_window( HWND hwnd, const XVisualInfo *visual )
         data->window_rect = data->whole_rect = data->client_rect;
     }
 
-    if (data->client_window)
-    {
-        XDeleteContext( data->display, data->client_window, winContext );
-        XReparentWindow( gdi_display, data->client_window, dummy_parent, 0, 0 );
-        TRACE( "%p reparent xwin %lx/%lx\n", data->hwnd, data->whole_window, data->client_window );
-    }
+    detach_client_window( data, data->client_window );
 
     if (data->client_colormap) XFreeColormap( gdi_display, data->client_colormap );
     data->client_colormap = XCreateColormap( gdi_display, dummy_parent, visual->visual,
@@ -1731,12 +1744,7 @@ static void destroy_whole_window( struct x11drv_win_data *data, BOOL already_des
     }
     else
     {
-        if (data->client_window && !already_destroyed)
-        {
-            XSelectInput( data->display, data->client_window, 0 );
-            XFlush( data->display ); /* make sure XSelectInput doesn't use client_window after this point */
-            XReparentWindow( gdi_display, data->client_window, get_dummy_parent(), 0, 0 );
-        }
+        if (!already_destroyed) detach_client_window( data, data->client_window );
         XDeleteContext( data->display, data->whole_window, winContext );
         if (!already_destroyed)
         {
-- 
GitLab


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

[PATCH v3 3/9] winex11: Detach vulkan client windows before destroying them.

[Rémi Bernon]

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

---
 dlls/winex11.drv/vulkan.c | 12 +++++++++++-
 dlls/winex11.drv/window.c |  4 ++--
 dlls/winex11.drv/x11drv.h |  1 +
 3 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/dlls/winex11.drv/vulkan.c b/dlls/winex11.drv/vulkan.c
index f8987e2c49d..6597771c4c6 100644
--- a/dlls/winex11.drv/vulkan.c
+++ b/dlls/winex11.drv/vulkan.c
@@ -217,7 +217,17 @@ static void wine_vk_surface_release( struct wine_vk_surface *surface )
     }
 
     if (surface->window)
-        XDestroyWindow(gdi_display, surface->window);
+    {
+        struct x11drv_win_data *data;
+
+        if ((data = get_win_data( surface->hwnd )))
+        {
+            detach_client_window( data, surface->window );
+            release_win_data( data );
+        }
+
+        XDestroyWindow( gdi_display, surface->window );
+    }
 
     free(surface);
 }
diff --git a/dlls/winex11.drv/window.c b/dlls/winex11.drv/window.c
index 4237e92990e..d38035e2042 100644
--- a/dlls/winex11.drv/window.c
+++ b/dlls/winex11.drv/window.c
@@ -1552,9 +1552,9 @@ Window get_dummy_parent(void)
 /**********************************************************************
  *		detach_client_window
  */
-static void detach_client_window( struct x11drv_win_data *data, Window window )
+void detach_client_window( struct x11drv_win_data *data, Window window )
 {
-    if (!data->client_window || data->client_window != window) return;
+    if (data->client_window != window || !data->client_window) return;
 
     XSelectInput( data->display, data->client_window, 0 );
     XFlush( data->display ); /* make sure XSelectInput is disabled for client_window after this point */
diff --git a/dlls/winex11.drv/x11drv.h b/dlls/winex11.drv/x11drv.h
index 2c15a95dfab..dc14d5ff372 100644
--- a/dlls/winex11.drv/x11drv.h
+++ b/dlls/winex11.drv/x11drv.h
@@ -657,6 +657,7 @@ extern void update_net_wm_states( struct x11drv_win_data *data ) DECLSPEC_HIDDEN
 extern void make_window_embedded( struct x11drv_win_data *data ) DECLSPEC_HIDDEN;
 extern Window create_dummy_client_window(void) DECLSPEC_HIDDEN;
 extern Window create_client_window( HWND hwnd, const XVisualInfo *visual ) DECLSPEC_HIDDEN;
+extern void detach_client_window( struct x11drv_win_data *data, Window window );
 extern void set_window_visual( struct x11drv_win_data *data, const XVisualInfo *vis, BOOL use_alpha ) DECLSPEC_HIDDEN;
 extern void change_systray_owner( Display *display, Window systray_window ) DECLSPEC_HIDDEN;
 extern HWND create_foreign_window( Display *display, Window window ) DECLSPEC_HIDDEN;
-- 
GitLab


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

[PATCH v3 4/9] winex11: Detach the vulkan surfaces when their HWND is destroyed.

[Rémi Bernon]

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

---
 dlls/winex11.drv/vulkan.c | 59 ++++++++++++++++++---------------------
 dlls/winex11.drv/window.c |  2 +-
 dlls/winex11.drv/x11drv.h |  2 +-
 3 files changed, 29 insertions(+), 34 deletions(-)

diff --git a/dlls/winex11.drv/vulkan.c b/dlls/winex11.drv/vulkan.c
index 6597771c4c6..005a429f3df 100644
--- a/dlls/winex11.drv/vulkan.c
+++ b/dlls/winex11.drv/vulkan.c
@@ -49,8 +49,6 @@ WINE_DECLARE_DEBUG_CHANNEL(fps);
 
 static pthread_mutex_t vulkan_mutex;
 
-static XContext vulkan_hwnd_context;
-
 #define VK_STRUCTURE_TYPE_XLIB_SURFACE_CREATE_INFO_KHR 1000004000
 
 static struct list surface_list = LIST_INIT( surface_list );
@@ -140,7 +138,6 @@ static void wine_vk_init(void)
 #undef LOAD_FUNCPTR
 #undef LOAD_OPTIONAL_FUNCPTR
 
-    vulkan_hwnd_context = XUniqueContext();
     return;
 
 fail:
@@ -196,13 +193,6 @@ static VkResult wine_vk_instance_convert_create_info(const VkInstanceCreateInfo
     return VK_SUCCESS;
 }
 
-static struct wine_vk_surface *wine_vk_surface_grab( struct wine_vk_surface *surface )
-{
-    int refcount = InterlockedIncrement( &surface->ref );
-    TRACE( "surface %p, refcount %d.\n", surface, refcount );
-    return surface;
-}
-
 static void wine_vk_surface_release( struct wine_vk_surface *surface )
 {
     int refcount = InterlockedDecrement( &surface->ref );
@@ -232,18 +222,33 @@ static void wine_vk_surface_release( struct wine_vk_surface *surface )
     free(surface);
 }
 
-void wine_vk_surface_destroy(HWND hwnd)
+static void wine_vk_surface_detach( struct wine_vk_surface *surface )
 {
-    struct wine_vk_surface *surface;
-    pthread_mutex_lock(&vulkan_mutex);
-    if (!XFindContext(gdi_display, (XID)hwnd, vulkan_hwnd_context, (char **)&surface))
+    struct x11drv_win_data *data;
+
+    TRACE( "Detaching surface %p, hwnd %p.\n", surface, surface->hwnd );
+
+    if (surface->window && (data = get_win_data( surface->hwnd )))
     {
-        surface->hwnd_thread_id = 0;
-        surface->hwnd = NULL;
-        wine_vk_surface_release(surface);
+        detach_client_window( data, surface->window );
+        release_win_data( data );
     }
-    XDeleteContext(gdi_display, (XID)hwnd, vulkan_hwnd_context);
-    pthread_mutex_unlock(&vulkan_mutex);
+
+    surface->hwnd_thread_id = 0;
+    surface->hwnd = 0;
+}
+
+void destroy_vk_surface( HWND hwnd )
+{
+    struct wine_vk_surface *surface, *next;
+
+    pthread_mutex_lock( &vulkan_mutex );
+    LIST_FOR_EACH_ENTRY_SAFE( surface, next, &surface_list, struct wine_vk_surface, entry )
+    {
+        if (surface->hwnd != hwnd) continue;
+        wine_vk_surface_detach( surface );
+    }
+    pthread_mutex_unlock( &vulkan_mutex );
 }
 
 void vulkan_thread_detach(void)
@@ -254,13 +259,8 @@ void vulkan_thread_detach(void)
     pthread_mutex_lock(&vulkan_mutex);
     LIST_FOR_EACH_ENTRY_SAFE(surface, next, &surface_list, struct wine_vk_surface, entry)
     {
-        if (surface->hwnd_thread_id != thread_id)
-            continue;
-
-        TRACE("Detaching surface %p, hwnd %p.\n", surface, surface->hwnd);
-        XReparentWindow(gdi_display, surface->window, get_dummy_parent(), 0, 0);
-        XSync(gdi_display, False);
-        wine_vk_surface_destroy(surface->hwnd);
+        if (surface->hwnd_thread_id != thread_id) continue;
+        wine_vk_surface_detach( surface );
     }
     pthread_mutex_unlock(&vulkan_mutex);
 }
@@ -371,11 +371,6 @@ static VkResult X11DRV_vkCreateWin32SurfaceKHR(VkInstance instance,
     }
 
     pthread_mutex_lock(&vulkan_mutex);
-    if (x11_surface->hwnd)
-    {
-        wine_vk_surface_destroy( x11_surface->hwnd );
-        XSaveContext(gdi_display, (XID)create_info->hwnd, vulkan_hwnd_context, (char *)wine_vk_surface_grab(x11_surface));
-    }
     list_add_tail(&surface_list, &x11_surface->entry);
     pthread_mutex_unlock(&vulkan_mutex);
 
@@ -754,7 +749,7 @@ const struct vulkan_funcs *get_vulkan_driver(UINT version)
     return NULL;
 }
 
-void wine_vk_surface_destroy(HWND hwnd)
+void destroy_vk_surface( HWND hwnd )
 {
 }
 
diff --git a/dlls/winex11.drv/window.c b/dlls/winex11.drv/window.c
index d38035e2042..96fb90e0cec 100644
--- a/dlls/winex11.drv/window.c
+++ b/dlls/winex11.drv/window.c
@@ -1867,7 +1867,7 @@ void X11DRV_DestroyWindow( HWND hwnd )
     release_win_data( data );
     free( data );
     destroy_gl_drawable( hwnd );
-    wine_vk_surface_destroy( hwnd );
+    destroy_vk_surface( hwnd );
 }
 
 
diff --git a/dlls/winex11.drv/x11drv.h b/dlls/winex11.drv/x11drv.h
index dc14d5ff372..caebb115e74 100644
--- a/dlls/winex11.drv/x11drv.h
+++ b/dlls/winex11.drv/x11drv.h
@@ -646,7 +646,7 @@ extern Window get_dummy_parent(void) DECLSPEC_HIDDEN;
 extern void sync_gl_drawable( HWND hwnd, BOOL known_child ) DECLSPEC_HIDDEN;
 extern void set_gl_drawable_parent( HWND hwnd, HWND parent ) DECLSPEC_HIDDEN;
 extern void destroy_gl_drawable( HWND hwnd ) DECLSPEC_HIDDEN;
-extern void wine_vk_surface_destroy( HWND hwnd ) DECLSPEC_HIDDEN;
+extern void destroy_vk_surface( HWND hwnd ) DECLSPEC_HIDDEN;
 extern void vulkan_thread_detach(void) DECLSPEC_HIDDEN;
 
 extern void wait_for_withdrawn_state( HWND hwnd, BOOL set ) DECLSPEC_HIDDEN;
-- 
GitLab


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

[PATCH v3 5/9] winex11: Resize detached vulkan surfaces when window is resized.

[Rémi Bernon]

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

---
 dlls/winex11.drv/vulkan.c | 18 ++++++++++++++++++
 dlls/winex11.drv/window.c |  1 +
 dlls/winex11.drv/x11drv.h |  1 +
 3 files changed, 20 insertions(+)

diff --git a/dlls/winex11.drv/vulkan.c b/dlls/winex11.drv/vulkan.c
index 005a429f3df..635a90d9689 100644
--- a/dlls/winex11.drv/vulkan.c
+++ b/dlls/winex11.drv/vulkan.c
@@ -251,6 +251,20 @@ void destroy_vk_surface( HWND hwnd )
     pthread_mutex_unlock( &vulkan_mutex );
 }
 
+void resize_vk_surfaces( HWND hwnd, Window active, int mask, XWindowChanges *changes )
+{
+    struct wine_vk_surface *surface;
+
+    pthread_mutex_lock( &vulkan_mutex );
+    LIST_FOR_EACH_ENTRY( surface, &surface_list, struct wine_vk_surface, entry )
+    {
+        if (surface->hwnd != hwnd) continue;
+        if (!surface->window || surface->window == active) continue;
+        XConfigureWindow( gdi_display, surface->window, mask, changes );
+    }
+    pthread_mutex_unlock( &vulkan_mutex );
+}
+
 void vulkan_thread_detach(void)
 {
     struct wine_vk_surface *surface, *next;
@@ -753,6 +767,10 @@ void destroy_vk_surface( HWND hwnd )
 {
 }
 
+void resize_vk_surfaces( HWND hwnd, Window active, int mask, XWindowChanges changes )
+{
+}
+
 void vulkan_thread_detach(void)
 {
 }
diff --git a/dlls/winex11.drv/window.c b/dlls/winex11.drv/window.c
index 96fb90e0cec..b709db79b68 100644
--- a/dlls/winex11.drv/window.c
+++ b/dlls/winex11.drv/window.c
@@ -1432,6 +1432,7 @@ static void sync_client_position( struct x11drv_win_data *data,
         TRACE( "setting client win %lx pos %d,%d,%dx%d changes=%x\n",
                data->client_window, changes.x, changes.y, changes.width, changes.height, mask );
         XConfigureWindow( gdi_display, data->client_window, mask, &changes );
+        resize_vk_surfaces( data->hwnd, data->client_window, mask, &changes );
     }
 }
 
diff --git a/dlls/winex11.drv/x11drv.h b/dlls/winex11.drv/x11drv.h
index caebb115e74..a6406099771 100644
--- a/dlls/winex11.drv/x11drv.h
+++ b/dlls/winex11.drv/x11drv.h
@@ -647,6 +647,7 @@ extern void sync_gl_drawable( HWND hwnd, BOOL known_child ) DECLSPEC_HIDDEN;
 extern void set_gl_drawable_parent( HWND hwnd, HWND parent ) DECLSPEC_HIDDEN;
 extern void destroy_gl_drawable( HWND hwnd ) DECLSPEC_HIDDEN;
 extern void destroy_vk_surface( HWND hwnd ) DECLSPEC_HIDDEN;
+extern void resize_vk_surfaces( HWND hwnd, Window active, int mask, XWindowChanges *changes );
 extern void vulkan_thread_detach(void) DECLSPEC_HIDDEN;
 
 extern void wait_for_withdrawn_state( HWND hwnd, BOOL set ) DECLSPEC_HIDDEN;
-- 
GitLab


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

[PATCH v3 6/9] winevulkan: Add vkAcquireNextImage(2)KHR driver entries.

[Rémi Bernon]

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

---
 dlls/winemac.drv/vulkan.c    | 2 ++
 dlls/winevulkan/make_vulkan  | 2 ++
 dlls/winex11.drv/vulkan.c    | 2 ++
 include/wine/vulkan_driver.h | 6 ++++++
 4 files changed, 12 insertions(+)

diff --git a/dlls/winemac.drv/vulkan.c b/dlls/winemac.drv/vulkan.c
index 00f5e8465ab..0c1a11af63d 100644
--- a/dlls/winemac.drv/vulkan.c
+++ b/dlls/winemac.drv/vulkan.c
@@ -591,6 +591,8 @@ static VkSurfaceKHR macdrv_wine_get_native_surface(VkSurfaceKHR surface)
 
 static const struct vulkan_funcs vulkan_funcs =
 {
+    NULL,
+    NULL,
     macdrv_vkCreateInstance,
     macdrv_vkCreateSwapchainKHR,
     macdrv_vkCreateWin32SurfaceKHR,
diff --git a/dlls/winevulkan/make_vulkan b/dlls/winevulkan/make_vulkan
index 9163f543c15..17a410f81bf 100755
--- a/dlls/winevulkan/make_vulkan
+++ b/dlls/winevulkan/make_vulkan
@@ -236,6 +236,8 @@ FUNCTION_OVERRIDES = {
     "vkGetPhysicalDeviceWin32PresentationSupportKHR" : {"dispatch" : True, "driver" : True, "thunk" : ThunkType.PUBLIC},
 
     # VK_KHR_swapchain
+    "vkAcquireNextImageKHR" : {"dispatch" : True, "driver" : True, "thunk" : ThunkType.PUBLIC},
+    "vkAcquireNextImage2KHR": {"dispatch" : True, "driver" : True, "thunk" : ThunkType.PUBLIC},
     "vkCreateSwapchainKHR" : {"dispatch" : True, "driver" : True, "thunk" : ThunkType.PUBLIC},
     "vkDestroySwapchainKHR" : {"dispatch" : True, "driver" : True, "thunk" : ThunkType.PUBLIC},
     "vkGetSwapchainImagesKHR": {"dispatch" : True, "driver" : True, "thunk" : ThunkType.PUBLIC},
diff --git a/dlls/winex11.drv/vulkan.c b/dlls/winex11.drv/vulkan.c
index 635a90d9689..fd4c5168901 100644
--- a/dlls/winex11.drv/vulkan.c
+++ b/dlls/winex11.drv/vulkan.c
@@ -704,6 +704,8 @@ static VkSurfaceKHR X11DRV_wine_get_native_surface(VkSurfaceKHR surface)
 
 static const struct vulkan_funcs vulkan_funcs =
 {
+    NULL,
+    NULL,
     X11DRV_vkCreateInstance,
     X11DRV_vkCreateSwapchainKHR,
     X11DRV_vkCreateWin32SurfaceKHR,
diff --git a/include/wine/vulkan_driver.h b/include/wine/vulkan_driver.h
index c9ac98b252b..61cdf7c69fe 100644
--- a/include/wine/vulkan_driver.h
+++ b/include/wine/vulkan_driver.h
@@ -21,6 +21,8 @@ struct vulkan_funcs
      * needs to provide. Other function calls will be provided indirectly by dispatch
      * tables part of dispatchable Vulkan objects such as VkInstance or vkDevice.
      */
+    VkResult (*p_vkAcquireNextImage2KHR)(VkDevice, const VkAcquireNextImageInfoKHR *, uint32_t *);
+    VkResult (*p_vkAcquireNextImageKHR)(VkDevice, VkSwapchainKHR, uint64_t, VkSemaphore, VkFence, uint32_t *);
     VkResult (*p_vkCreateInstance)(const VkInstanceCreateInfo *, const VkAllocationCallbacks *, VkInstance *);
     VkResult (*p_vkCreateSwapchainKHR)(VkDevice, const VkSwapchainCreateInfoKHR *, const VkAllocationCallbacks *, VkSwapchainKHR *);
     VkResult (*p_vkCreateWin32SurfaceKHR)(VkInstance, const VkWin32SurfaceCreateInfoKHR *, const VkAllocationCallbacks *, VkSurfaceKHR *);
@@ -53,6 +55,10 @@ static inline void *get_vulkan_driver_device_proc_addr(
 
     name += 2;
 
+    if (!strcmp(name, "AcquireNextImage2KHR"))
+        return vulkan_funcs->p_vkAcquireNextImage2KHR;
+    if (!strcmp(name, "AcquireNextImageKHR"))
+        return vulkan_funcs->p_vkAcquireNextImageKHR;
     if (!strcmp(name, "CreateSwapchainKHR"))
         return vulkan_funcs->p_vkCreateSwapchainKHR;
     if (!strcmp(name, "DestroySwapchainKHR"))
-- 
GitLab


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

[PATCH v3 7/9] winex11: Re-attach vulkan surfaces when images are acquired.

[Rémi Bernon]

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

---
 dlls/winex11.drv/vulkan.c | 59 +++++++++++++++++++++++++++++++++++++--
 dlls/winex11.drv/window.c | 20 +++++++++++++
 dlls/winex11.drv/x11drv.h |  1 +
 3 files changed, 77 insertions(+), 3 deletions(-)

diff --git a/dlls/winex11.drv/vulkan.c b/dlls/winex11.drv/vulkan.c
index fd4c5168901..d8ffc201afe 100644
--- a/dlls/winex11.drv/vulkan.c
+++ b/dlls/winex11.drv/vulkan.c
@@ -49,6 +49,8 @@ WINE_DECLARE_DEBUG_CHANNEL(fps);
 
 static pthread_mutex_t vulkan_mutex;
 
+static XContext vulkan_swapchain_context;
+
 #define VK_STRUCTURE_TYPE_XLIB_SURFACE_CREATE_INFO_KHR 1000004000
 
 static struct list surface_list = LIST_INIT( surface_list );
@@ -72,6 +74,7 @@ typedef struct VkXlibSurfaceCreateInfoKHR
     Window window;
 } VkXlibSurfaceCreateInfoKHR;
 
+static VkResult (*pvkAcquireNextImageKHR)(VkDevice, VkSwapchainKHR, uint64_t, VkSemaphore, VkFence, uint32_t *);
 static VkResult (*pvkCreateInstance)(const VkInstanceCreateInfo *, const VkAllocationCallbacks *, VkInstance *);
 static VkResult (*pvkCreateSwapchainKHR)(VkDevice, const VkSwapchainCreateInfoKHR *, const VkAllocationCallbacks *, VkSwapchainKHR *);
 static VkResult (*pvkCreateXlibSurfaceKHR)(VkInstance, const VkXlibSurfaceCreateInfoKHR *, const VkAllocationCallbacks *, VkSurfaceKHR *);
@@ -115,6 +118,7 @@ static void wine_vk_init(void)
 
 #define LOAD_FUNCPTR(f) if (!(p##f = dlsym(vulkan_handle, #f))) goto fail
 #define LOAD_OPTIONAL_FUNCPTR(f) p##f = dlsym(vulkan_handle, #f)
+    LOAD_FUNCPTR(vkAcquireNextImageKHR);
     LOAD_FUNCPTR(vkCreateInstance);
     LOAD_FUNCPTR(vkCreateSwapchainKHR);
     LOAD_FUNCPTR(vkCreateXlibSurfaceKHR);
@@ -138,6 +142,7 @@ static void wine_vk_init(void)
 #undef LOAD_FUNCPTR
 #undef LOAD_OPTIONAL_FUNCPTR
 
+    vulkan_swapchain_context = XUniqueContext();
     return;
 
 fail:
@@ -193,6 +198,13 @@ static VkResult wine_vk_instance_convert_create_info(const VkInstanceCreateInfo
     return VK_SUCCESS;
 }
 
+static struct wine_vk_surface *wine_vk_surface_grab( struct wine_vk_surface *surface )
+{
+    int refcount = InterlockedIncrement( &surface->ref );
+    TRACE( "surface %p, refcount %d.\n", surface, refcount );
+    return surface;
+}
+
 static void wine_vk_surface_release( struct wine_vk_surface *surface )
 {
     int refcount = InterlockedDecrement( &surface->ref );
@@ -312,6 +324,8 @@ static VkResult X11DRV_vkCreateSwapchainKHR(VkDevice device,
 {
     struct wine_vk_surface *x11_surface = surface_from_handle(create_info->surface);
     VkSwapchainCreateInfoKHR create_info_host;
+    VkResult result;
+
     TRACE("%p %p %p %p\n", device, create_info, allocator, swapchain);
 
     if (allocator)
@@ -323,7 +337,14 @@ static VkResult X11DRV_vkCreateSwapchainKHR(VkDevice device,
     create_info_host = *create_info;
     create_info_host.surface = x11_surface->surface;
 
-    return pvkCreateSwapchainKHR(device, &create_info_host, NULL /* allocator */, swapchain);
+    if ((result = pvkCreateSwapchainKHR( device, &create_info_host, NULL /* allocator */,
+                                         swapchain )) == VK_SUCCESS)
+    {
+        XSaveContext( gdi_display, (XID)(*swapchain), vulkan_swapchain_context,
+                      (char *)wine_vk_surface_grab( x11_surface ) );
+    }
+
+    return result;
 }
 
 static VkResult X11DRV_vkCreateWin32SurfaceKHR(VkInstance instance,
@@ -430,12 +451,17 @@ static void X11DRV_vkDestroySurfaceKHR(VkInstance instance, VkSurfaceKHR surface
 static void X11DRV_vkDestroySwapchainKHR(VkDevice device, VkSwapchainKHR swapchain,
          const VkAllocationCallbacks *allocator)
 {
+    struct wine_vk_surface *surface;
+
     TRACE("%p, 0x%s %p\n", device, wine_dbgstr_longlong(swapchain), allocator);
 
     if (allocator)
         FIXME("Support for allocation callbacks not implemented yet\n");
 
     pvkDestroySwapchainKHR(device, swapchain, NULL /* allocator */);
+
+    if (!XFindContext( gdi_display, (XID)swapchain, vulkan_swapchain_context, (char **)&surface ))
+        wine_vk_surface_release( surface );
 }
 
 static VkResult X11DRV_vkEnumerateInstanceExtensionProperties(const char *layer_name,
@@ -661,6 +687,33 @@ static VkResult X11DRV_vkGetSwapchainImagesKHR(VkDevice device,
     return pvkGetSwapchainImagesKHR(device, swapchain, count, images);
 }
 
+static VkResult X11DRV_vkAcquireNextImageKHR( VkDevice device, VkSwapchainKHR swapchain, uint64_t timeout,
+                                              VkSemaphore semaphore, VkFence fence, uint32_t *image_index )
+{
+    struct wine_vk_surface *surface;
+    struct x11drv_win_data *data;
+
+    if (XFindContext( gdi_display, (XID)swapchain, vulkan_swapchain_context, (char **)&surface ))
+        return VK_ERROR_SURFACE_LOST_KHR;
+
+    if ((data = get_win_data( surface->hwnd )))
+    {
+        attach_client_window( data, surface->window );
+        release_win_data( data );
+    }
+
+    return pvkAcquireNextImageKHR( device, swapchain, timeout, semaphore, fence, image_index );
+}
+
+static VkResult X11DRV_vkAcquireNextImage2KHR( VkDevice device, const VkAcquireNextImageInfoKHR *acquire_info,
+                                               uint32_t *image_index )
+{
+    static int once;
+    if (!once++) FIXME( "Emulating vkAcquireNextImage2KHR, ignoring pNext.\n" );
+    return X11DRV_vkAcquireNextImageKHR( device, acquire_info->swapchain, acquire_info->timeout,
+                                         acquire_info->semaphore, acquire_info->fence, image_index );
+}
+
 static VkResult X11DRV_vkQueuePresentKHR(VkQueue queue, const VkPresentInfoKHR *present_info)
 {
     VkResult res;
@@ -704,8 +757,8 @@ static VkSurfaceKHR X11DRV_wine_get_native_surface(VkSurfaceKHR surface)
 
 static const struct vulkan_funcs vulkan_funcs =
 {
-    NULL,
-    NULL,
+    X11DRV_vkAcquireNextImage2KHR,
+    X11DRV_vkAcquireNextImageKHR,
     X11DRV_vkCreateInstance,
     X11DRV_vkCreateSwapchainKHR,
     X11DRV_vkCreateWin32SurfaceKHR,
diff --git a/dlls/winex11.drv/window.c b/dlls/winex11.drv/window.c
index b709db79b68..c968b9dff24 100644
--- a/dlls/winex11.drv/window.c
+++ b/dlls/winex11.drv/window.c
@@ -1549,6 +1549,26 @@ Window get_dummy_parent(void)
     return dummy_parent;
 }
 
+/**********************************************************************
+ *		attach_client_window
+ */
+void attach_client_window( struct x11drv_win_data *data, Window window )
+{
+    if (data->client_window == window || !data->whole_window) return;
+    detach_client_window( data, data->client_window );
+
+    data->client_window = window;
+
+    XSaveContext( data->display, data->client_window, winContext, (char *)data->hwnd );
+    XSelectInput( data->display, data->client_window, ExposureMask );
+    XFlush( data->display ); /* make sure XSelectInput is enabled for client_window after this point */
+
+    XReparentWindow( gdi_display, data->client_window, data->whole_window,
+                     data->client_rect.left - data->whole_rect.left,
+                     data->client_rect.top - data->whole_rect.top );
+    TRACE( "%p/%lx attached client window %lx\n", data->hwnd, data->whole_window, data->client_window );
+}
+
 
 /**********************************************************************
  *		detach_client_window
diff --git a/dlls/winex11.drv/x11drv.h b/dlls/winex11.drv/x11drv.h
index a6406099771..82a1fb062ea 100644
--- a/dlls/winex11.drv/x11drv.h
+++ b/dlls/winex11.drv/x11drv.h
@@ -659,6 +659,7 @@ extern void make_window_embedded( struct x11drv_win_data *data ) DECLSPEC_HIDDEN
 extern Window create_dummy_client_window(void) DECLSPEC_HIDDEN;
 extern Window create_client_window( HWND hwnd, const XVisualInfo *visual ) DECLSPEC_HIDDEN;
 extern void detach_client_window( struct x11drv_win_data *data, Window window );
+extern void attach_client_window( struct x11drv_win_data *data, Window window );
 extern void set_window_visual( struct x11drv_win_data *data, const XVisualInfo *vis, BOOL use_alpha ) DECLSPEC_HIDDEN;
 extern void change_systray_owner( Display *display, Window systray_window ) DECLSPEC_HIDDEN;
 extern HWND create_foreign_window( Display *display, Window window ) DECLSPEC_HIDDEN;
-- 
GitLab


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

[PATCH v3 8/9] winex11: Put detached client windows surfaces offscreen.

[Rémi Bernon]

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

---
 dlls/winex11.drv/window.c      |  7 +++++++
 dlls/winex11.drv/x11drv_main.c | 18 +++++++++++++-----
 2 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/dlls/winex11.drv/window.c b/dlls/winex11.drv/window.c
index c968b9dff24..010dacd35bb 100644
--- a/dlls/winex11.drv/window.c
+++ b/dlls/winex11.drv/window.c
@@ -51,6 +51,7 @@
 #include "wingdi.h"
 #include "winuser.h"
 
+#include "xcomposite.h"
 #include "wine/debug.h"
 #include "wine/server.h"
 #include "mwm.h"
@@ -1566,6 +1567,9 @@ void attach_client_window( struct x11drv_win_data *data, Window window )
     XReparentWindow( gdi_display, data->client_window, data->whole_window,
                      data->client_rect.left - data->whole_rect.left,
                      data->client_rect.top - data->whole_rect.top );
+#ifdef SONAME_LIBXCOMPOSITE
+    if (usexcomposite) pXCompositeUnredirectWindow( gdi_display, data->client_window, CompositeRedirectManual );
+#endif
     TRACE( "%p/%lx attached client window %lx\n", data->hwnd, data->whole_window, data->client_window );
 }
 
@@ -1582,6 +1586,9 @@ void detach_client_window( struct x11drv_win_data *data, Window window )
     XDeleteContext( data->display, data->client_window, winContext );
 
     XReparentWindow( gdi_display, data->client_window, get_dummy_parent(), 0, 0 );
+#ifdef SONAME_LIBXCOMPOSITE
+    if (usexcomposite) pXCompositeRedirectWindow( gdi_display, data->client_window, CompositeRedirectManual );
+#endif
     TRACE( "%p/%lx detached client window %lx\n", data->hwnd, data->whole_window, data->client_window );
 
     data->client_window = 0;
diff --git a/dlls/winex11.drv/x11drv_main.c b/dlls/winex11.drv/x11drv_main.c
index 32a20e0e4f2..6daa2f459b8 100644
--- a/dlls/winex11.drv/x11drv_main.c
+++ b/dlls/winex11.drv/x11drv_main.c
@@ -96,6 +96,7 @@ static unsigned long err_serial;             /* serial number of first request *
 static int (*old_error_handler)( Display *, XErrorEvent * );
 static BOOL use_xim = TRUE;
 static WCHAR input_style[20];
+static int xcomp_opcode;
 
 static pthread_mutex_t d3dkmt_mutex = PTHREAD_MUTEX_INITIALIZER;
 static pthread_mutex_t error_mutex = PTHREAD_MUTEX_INITIALIZER;
@@ -247,6 +248,12 @@ static inline BOOL ignore_error( Display *display, XErrorEvent *event )
         {
             if (event->error_code == xrender_error_base + BadPicture) return TRUE;
         }
+#endif
+#ifdef SONAME_LIBXCOMPOSITE
+        if (xcomp_opcode && event->request_code == xcomp_opcode &&
+            (event->minor_code == X_CompositeRedirectWindow ||
+             event->minor_code == X_CompositeUnredirectWindow))
+            return TRUE;
 #endif
     }
     return FALSE;
@@ -573,15 +580,16 @@ static void X11DRV_XComposite_Init(void)
     LOAD_FUNCPTR(XCompositeNameWindowPixmap);
 #undef LOAD_FUNCPTR
 
-    if(!pXCompositeQueryExtension(gdi_display, &xcomp_event_base,
-                                  &xcomp_error_base)) {
-        TRACE("XComposite extension could not be queried; disabled\n");
-        dlclose(xcomposite_handle);
+    if (!XQueryExtension( gdi_display, "Composite", &xcomp_opcode, &xcomp_event_base, &xcomp_error_base ))
+    {
+        WARN( "XComposite extension could not be queried; disabled\n" );
+        dlclose( xcomposite_handle );
         xcomposite_handle = NULL;
         usexcomposite = FALSE;
         return;
     }
-    TRACE("XComposite is up and running error_base = %d\n", xcomp_error_base);
+    TRACE( "XComposite is up and running opcode = %d, error_base = %d, event_base %d\n",
+           xcomp_opcode, xcomp_error_base, xcomp_event_base );
     return;
 
 sym_not_found:
-- 
GitLab


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

[PATCH v3 9/9] winex11: Rename X11DRV_FLUSH_GL_DRAWABLE to X11DRV_PRESENT_DRAWABLE.

[Rémi Bernon]

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

---
 dlls/winex11.drv/init.c   |  8 ++++----
 dlls/winex11.drv/opengl.c | 40 +++++++++++++++++++--------------------
 dlls/winex11.drv/x11drv.h |  8 ++++----
 3 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/dlls/winex11.drv/init.c b/dlls/winex11.drv/init.c
index c3d54da1d4d..35364884e50 100644
--- a/dlls/winex11.drv/init.c
+++ b/dlls/winex11.drv/init.c
@@ -243,16 +243,16 @@ static INT X11DRV_ExtEscape( PHYSDEV dev, INT escape, INT in_count, LPCVOID in_d
                     return TRUE;
                 }
                 break;
-            case X11DRV_FLUSH_GL_DRAWABLE:
-                if (in_count >= sizeof(struct x11drv_escape_flush_gl_drawable))
+            case X11DRV_PRESENT_DRAWABLE:
+                if (in_count >= sizeof(struct x11drv_escape_present_drawable))
                 {
-                    const struct x11drv_escape_flush_gl_drawable *data = in_data;
+                    const struct x11drv_escape_present_drawable *data = in_data;
                     RECT rect = physDev->dc_rect;
 
                     OffsetRect( &rect, -physDev->dc_rect.left, -physDev->dc_rect.top );
                     if (data->flush) XFlush( gdi_display );
                     XSetFunction( gdi_display, physDev->gc, GXcopy );
-                    XCopyArea( gdi_display, data->gl_drawable, physDev->drawable, physDev->gc,
+                    XCopyArea( gdi_display, data->drawable, physDev->drawable, physDev->gc,
                                0, 0, rect.right, rect.bottom,
                                physDev->dc_rect.left, physDev->dc_rect.top );
                     add_device_bounds( physDev, &rect );
diff --git a/dlls/winex11.drv/opengl.c b/dlls/winex11.drv/opengl.c
index bb8f13f78b9..dd0753a624a 100644
--- a/dlls/winex11.drv/opengl.c
+++ b/dlls/winex11.drv/opengl.c
@@ -1980,20 +1980,20 @@ static BOOL glxdrv_wglShareLists(struct wgl_context *org, struct wgl_context *de
 
 static void wglFinish(void)
 {
-    struct x11drv_escape_flush_gl_drawable escape;
+    struct x11drv_escape_present_drawable escape;
     struct gl_drawable *gl;
     struct wgl_context *ctx = NtCurrentTeb()->glContext;
 
-    escape.code = X11DRV_FLUSH_GL_DRAWABLE;
-    escape.gl_drawable = 0;
+    escape.code = X11DRV_PRESENT_DRAWABLE;
+    escape.drawable = 0;
     escape.flush = FALSE;
 
     if ((gl = get_gl_drawable( NtUserWindowFromDC( ctx->hdc ), 0 )))
     {
         switch (gl->type)
         {
-        case DC_GL_PIXMAP_WIN: escape.gl_drawable = gl->pixmap; break;
-        case DC_GL_CHILD_WIN:  escape.gl_drawable = gl->window; break;
+        case DC_GL_PIXMAP_WIN: escape.drawable = gl->pixmap; break;
+        case DC_GL_CHILD_WIN:  escape.drawable = gl->window; break;
         default: break;
         }
         sync_context(ctx);
@@ -2001,26 +2001,26 @@ static void wglFinish(void)
     }
 
     pglFinish();
-    if (escape.gl_drawable)
+    if (escape.drawable)
         NtGdiExtEscape( ctx->hdc, NULL, 0, X11DRV_ESCAPE, sizeof(escape), (LPSTR)&escape, 0, NULL );
 }
 
 static void wglFlush(void)
 {
-    struct x11drv_escape_flush_gl_drawable escape;
+    struct x11drv_escape_present_drawable escape;
     struct gl_drawable *gl;
     struct wgl_context *ctx = NtCurrentTeb()->glContext;
 
-    escape.code = X11DRV_FLUSH_GL_DRAWABLE;
-    escape.gl_drawable = 0;
+    escape.code = X11DRV_PRESENT_DRAWABLE;
+    escape.drawable = 0;
     escape.flush = FALSE;
 
     if ((gl = get_gl_drawable( NtUserWindowFromDC( ctx->hdc ), 0 )))
     {
         switch (gl->type)
         {
-        case DC_GL_PIXMAP_WIN: escape.gl_drawable = gl->pixmap; break;
-        case DC_GL_CHILD_WIN:  escape.gl_drawable = gl->window; break;
+        case DC_GL_PIXMAP_WIN: escape.drawable = gl->pixmap; break;
+        case DC_GL_CHILD_WIN:  escape.drawable = gl->window; break;
         default: break;
         }
         sync_context(ctx);
@@ -2028,7 +2028,7 @@ static void wglFlush(void)
     }
 
     pglFlush();
-    if (escape.gl_drawable)
+    if (escape.drawable)
         NtGdiExtEscape( ctx->hdc, NULL, 0, X11DRV_ESCAPE, sizeof(escape), (LPSTR)&escape, 0, NULL );
 }
 
@@ -3348,15 +3348,15 @@ static void X11DRV_WineGL_LoadExtensions(void)
  */
 static BOOL glxdrv_wglSwapBuffers( HDC hdc )
 {
-    struct x11drv_escape_flush_gl_drawable escape;
+    struct x11drv_escape_present_drawable escape;
     struct gl_drawable *gl;
     struct wgl_context *ctx = NtCurrentTeb()->glContext;
     INT64 ust, msc, sbc, target_sbc = 0;
 
     TRACE("(%p)\n", hdc);
 
-    escape.code = X11DRV_FLUSH_GL_DRAWABLE;
-    escape.gl_drawable = 0;
+    escape.code = X11DRV_PRESENT_DRAWABLE;
+    escape.drawable = 0;
     escape.flush = !pglXWaitForSbcOML;
 
     if (!(gl = get_gl_drawable( NtUserWindowFromDC( hdc ), hdc )))
@@ -3377,7 +3377,7 @@ static BOOL glxdrv_wglSwapBuffers( HDC hdc )
     {
     case DC_GL_PIXMAP_WIN:
         if (ctx) sync_context( ctx );
-        escape.gl_drawable = gl->pixmap;
+        escape.drawable = gl->pixmap;
         if (pglXCopySubBufferMESA) {
             /* (glX)SwapBuffers has an implicit glFlush effect, however
              * GLX_MESA_copy_sub_buffer doesn't. Make sure GL is flushed before
@@ -3398,10 +3398,10 @@ static BOOL glxdrv_wglSwapBuffers( HDC hdc )
     case DC_GL_WINDOW:
     case DC_GL_CHILD_WIN:
         if (ctx) sync_context( ctx );
-        if (gl->type == DC_GL_CHILD_WIN) escape.gl_drawable = gl->window;
+        if (gl->type == DC_GL_CHILD_WIN) escape.drawable = gl->window;
         /* fall through */
     default:
-        if (escape.gl_drawable && pglXSwapBuffersMscOML)
+        if (escape.drawable && pglXSwapBuffersMscOML)
         {
             pglFlush();
             target_sbc = pglXSwapBuffersMscOML( gdi_display, gl->drawable, 0, 0, 0 );
@@ -3411,12 +3411,12 @@ static BOOL glxdrv_wglSwapBuffers( HDC hdc )
         break;
     }
 
-    if (escape.gl_drawable && pglXWaitForSbcOML)
+    if (escape.drawable && pglXWaitForSbcOML)
         pglXWaitForSbcOML( gdi_display, gl->drawable, target_sbc, &ust, &msc, &sbc );
 
     release_gl_drawable( gl );
 
-    if (escape.gl_drawable)
+    if (escape.drawable)
         NtGdiExtEscape( ctx->hdc, NULL, 0, X11DRV_ESCAPE, sizeof(escape), (LPSTR)&escape, 0, NULL );
     return TRUE;
 }
diff --git a/dlls/winex11.drv/x11drv.h b/dlls/winex11.drv/x11drv.h
index 82a1fb062ea..b7d09dad779 100644
--- a/dlls/winex11.drv/x11drv.h
+++ b/dlls/winex11.drv/x11drv.h
@@ -346,7 +346,7 @@ enum x11drv_escape_codes
     X11DRV_GET_DRAWABLE,     /* get current drawable for a DC */
     X11DRV_START_EXPOSURES,  /* start graphics exposures */
     X11DRV_END_EXPOSURES,    /* end graphics exposures */
-    X11DRV_FLUSH_GL_DRAWABLE /* flush changes made to the gl drawable */
+    X11DRV_PRESENT_DRAWABLE, /* present the drawable on screen */
 };
 
 struct x11drv_escape_set_drawable
@@ -365,10 +365,10 @@ struct x11drv_escape_get_drawable
     int                      pixel_format; /* internal GL pixel format */
 };
 
-struct x11drv_escape_flush_gl_drawable
+struct x11drv_escape_present_drawable
 {
-    enum x11drv_escape_codes code;         /* escape code (X11DRV_FLUSH_GL_DRAWABLE) */
-    Drawable                 gl_drawable;  /* GL drawable */
+    enum x11drv_escape_codes code;         /* escape code (X11DRV_PRESENT_DRAWABLE) */
+    Drawable                 drawable;     /* GL / VK drawable */
     BOOL                     flush;        /* flush X11 before copying */
 };
 
-- 
GitLab

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

[Rémi Bernon]

v3: Remove unnecessary if.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4533#note_54105

[Paul Gofman (＠gofman)]

Paul Gofman (@gofman) commented about dlls/winex11.drv/window.c:
> + *		detach_client_window
> + */
> +static void detach_client_window( struct x11drv_win_data *data, Window window )
> +{
> +    if (!data->client_window || data->client_window != window) return;
> +
> +    XSelectInput( data->display, data->client_window, 0 );
> +    XFlush( data->display ); /* make sure XSelectInput is disabled for client_window after this point */
> +    XDeleteContext( data->display, data->client_window, winContext );
> +
> +    XReparentWindow( gdi_display, data->client_window, get_dummy_parent(), 0, 0 );
> +    TRACE( "%p/%lx detached client window %lx\n", data->hwnd, data->whole_window, data->client_window );
> +
> +    data->client_window = 0;
> +}
> +
> I think after 'XReparentWindow( gdi_display, ...)' we always need XSync(gdi_display, False). I know that as far as this patch is concerned all the uses of the helper do that XSync after anyway. But it looks like the following patches which use detach_client_window (directly or through attach_client_window added later) miss that. Maybe worth considering putting XSync() here and rearranging the calling code so it doesn't call it the second time (doing the other ops on gdi_context before this call)?

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4533#note_54141

[Paul Gofman (＠gofman)]

Paul Gofman (@gofman) commented about dlls/winex11.drv/vulkan.c:
> }
>
> if (surface->window)
> -        XDestroyWindow(gdi_display, surface->window);
> +    {
> +        struct x11drv_win_data *data;
> +
> +        if ((data = get_win_data( surface->hwnd )))
> +        {
> +            detach_client_window( data, surface->window );

We are destoying the window right after and only want to update client window data here. Reparenting the window before looks excessive. It is not exactly cheap and triggers more WM events opening up a way to more races, so I guess it is important to avoid that.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4533#note_54142

[Paul Gofman (＠gofman)]

Paul Gofman (@gofman) commented about dlls/winex11.drv/vulkan.c:
> -void wine_vk_surface_destroy(HWND hwnd)
> +static void wine_vk_surface_detach( struct wine_vk_surface *surface )
>  {
> -    struct wine_vk_surface *surface;
> -    pthread_mutex_lock(&vulkan_mutex);
> -    if (!XFindContext(gdi_display, (XID)hwnd, vulkan_hwnd_context, (char **)&surface))
> +    struct x11drv_win_data *data;
> +
> +    TRACE( "Detaching surface %p, hwnd %p.\n", surface, surface->hwnd );
> +
> +    if (surface->window && (data = get_win_data( surface->hwnd )))
>      {
> -        surface->hwnd_thread_id = 0;
> -        surface->hwnd = NULL;
> -        wine_vk_surface_release(surface);
> +        detach_client_window( data, surface->window );
> Unless we are adding XSync to detach_client_window() as I suggested earlier, XSync(gdi_display) is probably needed here.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4533#note_54144

[Paul Gofman (＠gofman)]

Paul Gofman (@gofman) commented about dlls/winex11.drv/vulkan.c:
> -void wine_vk_surface_destroy(HWND hwnd)
> +static void wine_vk_surface_detach( struct wine_vk_surface *surface )
>  {
> -    struct wine_vk_surface *surface;
> -    pthread_mutex_lock(&vulkan_mutex);
> -    if (!XFindContext(gdi_display, (XID)hwnd, vulkan_hwnd_context, (char **)&surface))
> +    struct x11drv_win_data *data;
> +
> +    TRACE( "Detaching surface %p, hwnd %p.\n", surface, surface->hwnd );
> +
> +    if (surface->window && (data = get_win_data( surface->hwnd )))
>      {
> -        surface->hwnd_thread_id = 0;
> -        surface->hwnd = NULL;
> -        wine_vk_surface_release(surface);

I think this is important refcounting change but a bit separate from what the rest of the patch is doing. Before the patchset the code assumes that window holds the reference to the surface (while tracking the release is a bit convoluted). The idea of the change (which I believe is correct) is to get rid of this reference and make only app creating surface (and swapchain later in this patchset) hold the reference. Surface may be deleted without deleting window (currently doing so will effectively leak surfaces if the app creates and destroy surfaces). Do you think it maybe worth splitting the refcounting change from this patch to a separate one?

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4533#note_54143

[Paul Gofman (＠gofman)]

Paul Gofman (@gofman) commented about dlls/winex11.drv/vulkan.c:
> return pvkGetSwapchainImagesKHR(device, swapchain, count, images);
>  }
>
> +static VkResult X11DRV_vkAcquireNextImageKHR( VkDevice device, VkSwapchainKHR swapchain, uint64_t timeout,

I think this really belongs to vkQueuePresent rather than vkAquireNextImage. I know why the functionality related to blitting offscreen surfaces was done in Aquire in Proton initially: it was an attempt to workaround tearing and make sure that the image already presented to the (offscreen) surface when performing blit. But the workaround doesn't really work anymore (at least on AMD) after some Mesa WSI changes. There is VK_KHR_present_wait extenstion now which I believe we might be using for such purposes. It is not universally there yet (e. g, not on Xwayland where it is probably need that as most), but hopefully it will come to fully working state. We also know that bundling presentation in Acquire leads to bugs in apps which don't present continuously and expect the image to appear on screen after calling vkQueuePresent.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4533#note_54145

[Paul Gofman (＠gofman)]

Paul Gofman (@gofman) commented about dlls/winex11.drv/vulkan.c:
> return pvkGetSwapchainImagesKHR(device, swapchain, count, images);
>  }
>
> +static VkResult X11DRV_vkAcquireNextImageKHR( VkDevice device, VkSwapchainKHR swapchain, uint64_t timeout,
> +                                              VkSemaphore semaphore, VkFence fence, uint32_t *image_index )
> +{
> +    struct wine_vk_surface *surface;
> +    struct x11drv_win_data *data;
> +
> +    if (XFindContext( gdi_display, (XID)swapchain, vulkan_swapchain_context, (char **)&surface ))
> +        return VK_ERROR_SURFACE_LOST_KHR;
> +
> +    if ((data = get_win_data( surface->hwnd )))
> +    {
> +        attach_client_window( data, surface->window );

Here we also probably need XSync(gdi_display...) unless doing that inside the function.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4533#note_54146

[Paul Gofman (＠gofman)]

Paul Gofman (@gofman) commented about dlls/winex11.drv/window.c:
> XDeleteContext( data->display, data->client_window, winContext );
>
> XReparentWindow( gdi_display, data->client_window, get_dummy_parent(), 0, 0 );
> +#ifdef SONAME_LIBXCOMPOSITE
> +    if (usexcomposite) pXCompositeRedirectWindow( gdi_display, data->client_window, CompositeRedirectManual );

This is actually a workaround for Xwayland bug. When a Vulkan window has fullscreen size and position, Xwayland tends to fully flip it onscreen even regardless of it being a child of 1x1 size parent. It does so not only over some same app other windows but over the whole desktop breaking it completely. This workaround helps in some cases but only partially, there were still cases when it will flip not minding composite redirect as well. So I am not entirely certain if bringing in this part now is justified.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4533#note_54147

[Paul Gofman (＠gofman)]

I added my observations inline. Probably worth noting that this patchset, in particular, enables suport for multiple surfaces per single window. While looking at the present patches I tested that on Windows (AMD) with a standalone interactive sample and confirmed that creation and presenting on multiple window surfaces created for single hwnd works (contents is displayed on screen), both on Windows (at least AMD) and with Wine after this patchset.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4533#note_54148

[Rémi Bernon]

On Tue Nov 28 21:12:43 2023 +0000, Paul Gofman wrote:
> I think after 'XReparentWindow( gdi_display, ...)' we always need
> XSync(gdi_display, False). I know that as far as this patch is concerned
> all the uses of the helper do that XSync after anyway. But it looks like
> the following patches which use detach_client_window (directly or
> through attach_client_window added later) miss that. Maybe worth
> considering putting XSync() here and rearranging the calling code so it
> doesn't call it the second time (doing the other ops on gdi_context
> before this call)?
> I don't really see why it would need an additional `XSync` if the current synchronization is correct. The only synchronization points are the same as before or after this MR:

1) when client_window is created using the gdi_display, we need to make sure gdi_display knows about its whole_window parent beforehand. After the creation we also need a XFlush so to make sure the creation requests will be seen by other displays.

2) before XSelectInput is called with thread display, we need to make sure thread display know about the client window.

3) when whole_window is destroyed, through the thread display, we need to make sure any client window has been fully reparented to a different window. The reparenting request is made on the gdi_display, so we could do `XFlush( gdi_display ); XSync( thread->display )` but as this is only causing errors after the whole window request is sent and processed, a `XSync( gdi_display )` should be enough.

Re-parenting alone should not need any synchronization as whole_window is still valid at that point, and you can reparent windows as many times as you like, as long as both windows stay valid.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4533#note_54252

[Rémi Bernon]

On Tue Nov 28 21:12:46 2023 +0000, Paul Gofman wrote:
> Unless we are adding XSync to detach_client_window() as I suggested
> earlier, XSync(gdi_display) is probably needed here.
> As said above, I don't see why we'd need one.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4533#note_54254

[Rémi Bernon]

On Tue Nov 28 21:12:45 2023 +0000, Paul Gofman wrote:
> We are destoying the window right after and only want to update client
> window data here. Reparenting the window before looks excessive. It is
> not exactly cheap and triggers more WM events opening up a way to more
> races, so I guess it is important to avoid that.
> Sure.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4533#note_54253

[Rémi Bernon]

On Tue Nov 28 21:12:49 2023 +0000, Paul Gofman wrote:
> Here we also probably need XSync(gdi_display...) unless doing that
> inside the function.
> Ditto.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4533#note_54255

[Rémi Bernon]

On Tue Nov 28 21:12:48 2023 +0000, Paul Gofman wrote:
> I think this really belongs to vkQueuePresent rather than
> vkAquireNextImage. I know why the functionality related to blitting
> offscreen surfaces was done in Aquire in Proton initially: it was an
> attempt to workaround tearing and make sure that the image already
> presented to the (offscreen) surface when performing blit. But the
> workaround doesn't really work anymore (at least on AMD) after some Mesa
> WSI changes. There is VK_KHR_present_wait extenstion now which I believe
> we might be using for such purposes. It is not universally there yet (e.
> g, not on Xwayland where it is probably need that as most), but
> hopefully it will come to fully working state. We also know that
> bundling presentation in Acquire leads to bugs in apps which don't
> present continuously and expect the image to appear on screen after
> calling vkQueuePresent.
> Yeah, I didn't want to change that too much from Proton, but I agree that it probably belongs in Present instead. I'll put it there.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4533#note_54256

[Rémi Bernon]

On Tue Nov 28 21:12:50 2023 +0000, Paul Gofman wrote:
> This is actually a workaround for Xwayland bug. When a Vulkan window has
> fullscreen size and position, Xwayland tends to fully flip it onscreen
> even regardless of it being a child of 1x1 size parent. It does so not
> only over some same app other windows but over the whole desktop
> breaking it completely. This workaround helps in some cases but only
> partially, there were still cases when it will flip not minding
> composite redirect as well. So I am not entirely certain if bringing in
> this part now is justified.
> Sure, I'll leave it aside.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4533#note_54257

[Paul Gofman (＠gofman)]

On Wed Nov 29 15:20:34 2023 +0000, Rémi Bernon wrote:
> I don't really see why it would need an additional `XSync` if the
> current synchronization is correct. The only synchronization points are
> the same as before or after this MR:
> 1) when client_window is created using the gdi_display, we need to make
> sure gdi_display knows about its whole_window parent beforehand. After
> the creation we also need a XFlush so to make sure the creation requests
> will be seen by other displays.
> 2) before XSelectInput is called with thread display, we need to make
> sure thread display know about the client window.
> 3) when whole_window is destroyed, through the thread display, we need
> to make sure any client window has been fully reparented to a different
> window. The reparenting request is made on the gdi_display, so we could
> do `XFlush( gdi_display ); XSync( thread->display )` but as this is only
> causing errors after the whole window request is sent and processed, a
> `XSync( gdi_display )` should be enough.
> Re-parenting alone should not need any synchronization as whole_window
> is still valid at that point, and you can reparent windows as many times
> as you like, as long as both windows stay valid.
> Are you sure that deleting parent whole_window is the only case for which synchronization is needed? E. g., X11DRV_Expose() handles events for client window, don't we want to make sure the current X window tree is synced as soon as client window is detached? Also, if we have, e. g., some resize or iconify of the whole window, while X still thinks that client window is its child, what happen to it?

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4533#note_54279

[Rémi Bernon]

On Wed Nov 29 16:41:34 2023 +0000, Paul Gofman wrote:
> Are you sure that deleting parent whole_window is the only case for
> which synchronization is needed? E. g., X11DRV_Expose() handles events
> for client window, don't we want to make sure the current X window tree
> is synced as soon as client window is detached? Also, if we have, e. g.,
> some resize or iconify of the whole window, while X still thinks that
> client window is its child, what happen to it?
> I'm not sure because pretty much nothing is certain in this area, but if there's any need for additional XSync, it's not because of this MR and it is the case already, and should be added separately. As I don't have evidence that they are required I'm not going to add them now.

Regarding the events, this is why there also are XFlush before / after we call XSelectInput, making sure that we only register for Expose events on attached client windows. Of course, there may be pending events that we might receive later but is that even the case after XSelectInput deselected a window? There again I haven't yet seen any evidence it's happening.

Regarding resize / iconify of the top-level window, afaik it doesn't have any effect on the child. Resize of the client window is now done using the gdi_display as it should, since a6bc5f34b87393e04ff46659f518f2e7094cc7e4, and shouldn't need additional synchronization.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4533#note_54280

[Paul Gofman (＠gofman)]

> I'm not sure because pretty much nothing is certain in this area, but if there's any need for additional XSync, it's not because of this MR and it is the case already, and should be added separately.

I think now whenever reparent happens (in create_client_window, destroy_whole_window and set_window_visual) there is XSync(gdi_display ) after. After this MR (not exactly the patch introducing detach_client_window, later in the MR when new uses of the function are added) there are paths with XReparentWindow without XSync, so unless I am missing something MR does change things in this regard?
> As I don't have evidence that they are required I'm not going to add them now

I'd rather suggest maybe then to keep XSync always after the XReparentWindow (effectively, probably not exactly in detach_client_window or attach_client_window but in the added paths), and then remove those in a separate commit if there is a certainty that it is not needed?

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4533#note_54282

[Rémi Bernon]

On Wed Nov 29 17:01:15 2023 +0000, Paul Gofman wrote:
> > I'm not sure because pretty much nothing is certain in this area, but
> > if there's any need for additional XSync, it's not because of this MR
> > and it is the case already, and should be added separately.
> > I think now whenever reparent happens (in create_client_window,
> > destroy_whole_window and set_window_visual) there is XSync(gdi_display )
> > after. After this MR (not exactly the patch introducing
> > detach_client_window, later in the MR when new uses of the function are
> > added) there are paths with XReparentWindow without XSync, so unless I
> > am missing something MR does change things in this regard?
> > As I don't have evidence that they are required I'm not going to add
> > them now
> > I'd rather suggest maybe then to keep XSync always after the
> > XReparentWindow (effectively, probably not exactly in
> > detach_client_window or attach_client_window but in the added paths),
> > and then remove those in a separate commit if there is a certainty that
> > it is not needed?
> > But the current XSync is only there to synchronize between XReparentWindow and XDestroyWindow, which definitely caused crashes (and only because of XDestroyWindow, which still believed client window was a child and destroyed it as well, later causing some GL code using gdi_display to access an invalid client window), not between XReparentWindow and other calls.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4533#note_54284

[Paul Gofman (＠gofman)]

On Wed Nov 29 17:07:41 2023 +0000, Rémi Bernon wrote:
> But the current XSync is only there to synchronize between
> XReparentWindow and XDestroyWindow, which definitely caused crashes (and
> only because of XDestroyWindow, which still believed client window was a
> child and destroyed it as well, later causing some GL code using
> gdi_display to access an invalid client window), not between
> XReparentWindow and other calls.
> Maybe, but regardless of the motivation, it is de-facto there after each XReparentWindow now. We may have other desync problems besides fatal crashes with XBadWindow. If you ask me now for exact bugs without XSync on the added paths it would be hard to figure out, there were some in Proton version but that was without the referenced earlier upstream changes fixing the sync elsewhere, it might be that those are resolved this way. Usually if we have reasonable doubts if synchronization is needed somewhere we tend to do it and then think on whether / how we can relax it, or if extra sync is especially harmful for performance we want to be sure if it is needed, do we not? Any reason this should be different approach here: in case of doubts assume that it is not needed and then add once the evidence proves otherwise? We are not talking here about any X server operation, only X window tree changes, which are expensive anyway and also potentially sensitive to desync.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4533#note_54287

[Rémi Bernon]

On Wed Nov 29 17:18:12 2023 +0000, Paul Gofman wrote:
> Maybe, but regardless of the motivation, it is de-facto there after each
> XReparentWindow now. We may have other desync problems besides fatal
> crashes with XBadWindow. If you ask me now for exact bugs without XSync
> on the added paths it would be hard to figure out, there were some in
> Proton version but that was without the referenced earlier upstream
> changes fixing the sync elsewhere, it might be that those are resolved
> this way. Usually if we have reasonable doubts if synchronization is
> needed somewhere we tend to do it and then think on whether / how we can
> relax it, or if extra sync is especially harmful for performance we want
> to be sure if it is needed, do we not? Any reason this should be
> different approach here: in case of doubts assume that it is not needed
> and then add once the evidence proves otherwise? We are not talking here
> about any X server operation, only X window tree changes, which are
> expensive anyway and also potentially sensitive to desync.
> I don't think this has ever been like that on winex11 and it is actually the opposite: XSync and XFlush have always been frowned upon and avoided unless explicitly required.

The XSync addition here was meant only to synchronize *before* calling XDestroyWindow, and I'd keep it like that until we have an evidence that it's needed *after* XReparentWindow (which should be confirmed soon enough and be a trivial change).

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4533#note_54288

[Paul Gofman (＠gofman)]

On Wed Nov 29 17:23:08 2023 +0000, Rémi Bernon wrote:
> I don't think this has ever been like that on winex11 and it is actually
> the opposite: XSync and XFlush have always been frowned upon and avoided
> unless explicitly required.
> The XSync addition here was meant only to synchronize *before* calling
> XDestroyWindow, and I'd keep it like that until we have an evidence that
> it's needed *after* XReparentWindow (which should be confirmed soon
> enough and be a trivial change).
> Not sure about soon enough, given it obviously won't affect every app, given how fun is to debug X sync issues, especially when you can't reproduce that because your setup is just a bit different and issues are random, when it doesn't crash with XBadWindow but rather misbehaves with some wrong size when going fullscreen or out due to some intermediate resize, and as best it will be blamed to a commit which does other thing not directly concerning such synchronization. But if there are reasons to think XSync on reparenting is expensive enough, well, ok.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4533#note_54289

[Rémi Bernon]

I've split the client window helpers to a separate MR. I think they are useful on their own to make reasoning easier. I actually found an instance of a missing XSync btw (in `set_window_visual`, when `whole_window` is re-created and the new one used in a `XReparentWindow` call).

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4533#note_54414

[PATCH v3 0/9] MR4533: winex11: Detach / attach vulkan window surfaces as needed. - closed

[Rémi Bernon]

This merge request was closed by Rémi Bernon.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4533

[Rémi Bernon]

Will probably try to move more things in win32u instead of doing all this surface management in winex11.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4533#note_63035