This is the first of (tentatively) 3 MRs that add Vulkan support to the Wayland driver. Here is the proposed breakdown:
* part 10.1 (this one): Basic setup and VkSurfaceKHR integration * part 10.2: VkSwapchainKHR integration * part 10.3: Everything else (mostly passthrough implementations of other Vulkan functions)
I chose to go with Vulkan first, instead of OpenGL, since the integration is more straightforward, and allows us to implement required core driver changes (which will also be used for OpenGL later) with fewer distractions.
Please note that all 3 parts need to land before the Vulkan support is actually usable in applications/games. I have uploaded the full (tentative) series for people that want to take a look or try out the final state: https://gitlab.winehq.org/afrantzis/wine/-/tree/wayland-part-10
Finally, part 10 is not based on part 9, so they can land in either order. However, there are interactions and conflicts between the two, so whichever part lands last will need to be (slightly) adapted. Note, though, that without part 9, some things won't work well on scaled outputs (esp. fullscreen).
Thanks!
From: Alexandros Frantzis alexandros.frantzis@collabora.com
--- dlls/winewayland.drv/Makefile.in | 1 + dlls/winewayland.drv/vulkan.c | 81 ++++++++++++++++++++++++++ dlls/winewayland.drv/waylanddrv.h | 1 + dlls/winewayland.drv/waylanddrv_main.c | 3 +- 4 files changed, 85 insertions(+), 1 deletion(-) create mode 100644 dlls/winewayland.drv/vulkan.c
diff --git a/dlls/winewayland.drv/Makefile.in b/dlls/winewayland.drv/Makefile.in index e1019ad8348..bbce790899e 100644 --- a/dlls/winewayland.drv/Makefile.in +++ b/dlls/winewayland.drv/Makefile.in @@ -7,6 +7,7 @@ SOURCES = \ display.c \ dllmain.c \ version.rc \ + vulkan.c \ wayland.c \ wayland_output.c \ wayland_pointer.c \ diff --git a/dlls/winewayland.drv/vulkan.c b/dlls/winewayland.drv/vulkan.c new file mode 100644 index 00000000000..d4dc608c7e7 --- /dev/null +++ b/dlls/winewayland.drv/vulkan.c @@ -0,0 +1,81 @@ +/* WAYLANDDRV Vulkan implementation + * + * Copyright 2017 Roderick Colenbrander + * Copyright 2021 Alexandros Frantzis + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#if 0 +#pragma makedep unix +#endif + +#include "config.h" + +#include "waylanddrv.h" + +#include "wine/debug.h" + +#define VK_NO_PROTOTYPES +#define WINE_VK_HOST + +#include "wine/vulkan.h" +#include "wine/vulkan_driver.h" + +#include <dlfcn.h> + +WINE_DEFAULT_DEBUG_CHANNEL(vulkan); + +#ifdef SONAME_LIBVULKAN + +static void *vulkan_handle; + +static void wine_vk_init(void) +{ + if (!(vulkan_handle = dlopen(SONAME_LIBVULKAN, RTLD_NOW))) + ERR("Failed to load %s.\n", SONAME_LIBVULKAN); +} + +static const struct vulkan_funcs vulkan_funcs; + +/********************************************************************** + * WAYLAND_wine_get_vulkan_driver + */ +const struct vulkan_funcs *WAYLAND_wine_get_vulkan_driver(UINT version) +{ + static pthread_once_t init_once = PTHREAD_ONCE_INIT; + + if (version != WINE_VULKAN_DRIVER_VERSION) + { + ERR("version mismatch, vulkan wants %u but driver has %u\n", version, WINE_VULKAN_DRIVER_VERSION); + return NULL; + } + + pthread_once(&init_once, wine_vk_init); + if (vulkan_handle) + return &vulkan_funcs; + + return NULL; +} + +#else /* No vulkan */ + +const struct vulkan_funcs *WAYLAND_wine_get_vulkan_driver(UINT version) +{ + ERR("Wine was built without Vulkan support.\n"); + return NULL; +} + +#endif /* SONAME_LIBVULKAN */ diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index 4bcd9e6706e..6ef86ae5a37 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -268,5 +268,6 @@ void WAYLAND_WindowPosChanged(HWND hwnd, HWND insert_after, UINT swp_flags, BOOL WAYLAND_WindowPosChanging(HWND hwnd, HWND insert_after, UINT swp_flags, const RECT *window_rect, const RECT *client_rect, RECT *visible_rect, struct window_surface **surface) DECLSPEC_HIDDEN; +const struct vulkan_funcs *WAYLAND_wine_get_vulkan_driver(UINT version) DECLSPEC_HIDDEN;
#endif /* __WINE_WAYLANDDRV_H */ diff --git a/dlls/winewayland.drv/waylanddrv_main.c b/dlls/winewayland.drv/waylanddrv_main.c index 7151d7b931a..f94e1053e12 100644 --- a/dlls/winewayland.drv/waylanddrv_main.c +++ b/dlls/winewayland.drv/waylanddrv_main.c @@ -38,7 +38,8 @@ static const struct user_driver_funcs waylanddrv_funcs = .pUpdateDisplayDevices = WAYLAND_UpdateDisplayDevices, .pWindowMessage = WAYLAND_WindowMessage, .pWindowPosChanged = WAYLAND_WindowPosChanged, - .pWindowPosChanging = WAYLAND_WindowPosChanging + .pWindowPosChanging = WAYLAND_WindowPosChanging, + .pwine_get_vulkan_driver = WAYLAND_wine_get_vulkan_driver };
static NTSTATUS waylanddrv_unix_init(void *arg)
From: Alexandros Frantzis alexandros.frantzis@collabora.com
Create a Vulkan instance, ensuring we use the proper (Wayland) SurfaceKHR extension when forwarding the request to the native Vulkan platform. --- dlls/winewayland.drv/vulkan.c | 92 ++++++++++++++++++++++++++++++++++- 1 file changed, 91 insertions(+), 1 deletion(-)
diff --git a/dlls/winewayland.drv/vulkan.c b/dlls/winewayland.drv/vulkan.c index d4dc608c7e7..1ee668a4c77 100644 --- a/dlls/winewayland.drv/vulkan.c +++ b/dlls/winewayland.drv/vulkan.c @@ -35,20 +35,110 @@ #include "wine/vulkan_driver.h"
#include <dlfcn.h> +#include <stdlib.h>
WINE_DEFAULT_DEBUG_CHANNEL(vulkan);
#ifdef SONAME_LIBVULKAN
+static VkResult (*pvkCreateInstance)(const VkInstanceCreateInfo *, const VkAllocationCallbacks *, VkInstance *); + static void *vulkan_handle;
+/* Helper function for converting between win32 and Wayland compatible VkInstanceCreateInfo. + * Caller is responsible for allocation and cleanup of 'dst'. */ +static VkResult wine_vk_instance_convert_create_info(const VkInstanceCreateInfo *src, + VkInstanceCreateInfo *dst) +{ + unsigned int i; + const char **enabled_extensions = NULL; + + dst->sType = src->sType; + dst->flags = src->flags; + dst->pApplicationInfo = src->pApplicationInfo; + dst->pNext = src->pNext; + dst->enabledLayerCount = 0; + dst->ppEnabledLayerNames = NULL; + dst->enabledExtensionCount = 0; + dst->ppEnabledExtensionNames = NULL; + + if (src->enabledExtensionCount > 0) + { + enabled_extensions = calloc(src->enabledExtensionCount, + sizeof(*src->ppEnabledExtensionNames)); + if (!enabled_extensions) + { + ERR("Failed to allocate memory for enabled extensions\n"); + return VK_ERROR_OUT_OF_HOST_MEMORY; + } + + for (i = 0; i < src->enabledExtensionCount; i++) + { + /* Substitute extension with Wayland ones else copy. Long-term, when we + * support more extensions, we should store these in a list. */ + if (!strcmp(src->ppEnabledExtensionNames[i], "VK_KHR_win32_surface")) + enabled_extensions[i] = "VK_KHR_wayland_surface"; + else + enabled_extensions[i] = src->ppEnabledExtensionNames[i]; + } + dst->ppEnabledExtensionNames = enabled_extensions; + dst->enabledExtensionCount = src->enabledExtensionCount; + } + + return VK_SUCCESS; +} + +static VkResult wayland_vkCreateInstance(const VkInstanceCreateInfo *create_info, + const VkAllocationCallbacks *allocator, + VkInstance *instance) +{ + VkInstanceCreateInfo create_info_host; + VkResult res; + + TRACE("create_info %p, allocator %p, instance %p\n", create_info, allocator, instance); + + if (allocator) + FIXME("Support for allocation callbacks not implemented yet\n"); + + /* Perform a second pass on converting VkInstanceCreateInfo. Winevulkan + * performed a first pass in which it handles everything except for WSI + * functionality such as VK_KHR_win32_surface. Handle this now. */ + res = wine_vk_instance_convert_create_info(create_info, &create_info_host); + if (res != VK_SUCCESS) + { + ERR("Failed to convert instance create info, res=%d\n", res); + return res; + } + + res = pvkCreateInstance(&create_info_host, NULL /* allocator */, instance); + + free((void *)create_info_host.ppEnabledExtensionNames); + return res; +} + static void wine_vk_init(void) { if (!(vulkan_handle = dlopen(SONAME_LIBVULKAN, RTLD_NOW))) + { ERR("Failed to load %s.\n", SONAME_LIBVULKAN); + return; + } + +#define LOAD_FUNCPTR(f) if (!(p##f = dlsym(vulkan_handle, #f))) goto fail + LOAD_FUNCPTR(vkCreateInstance); +#undef LOAD_FUNCPTR + + return; + +fail: + dlclose(vulkan_handle); + vulkan_handle = NULL; }
-static const struct vulkan_funcs vulkan_funcs; +static const struct vulkan_funcs vulkan_funcs = +{ + .p_vkCreateInstance = wayland_vkCreateInstance +};
/********************************************************************** * WAYLAND_wine_get_vulkan_driver
From: Alexandros Frantzis alexandros.frantzis@collabora.com
--- dlls/winewayland.drv/vulkan.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/dlls/winewayland.drv/vulkan.c b/dlls/winewayland.drv/vulkan.c index 1ee668a4c77..b89e6cdcc45 100644 --- a/dlls/winewayland.drv/vulkan.c +++ b/dlls/winewayland.drv/vulkan.c @@ -42,6 +42,7 @@ WINE_DEFAULT_DEBUG_CHANNEL(vulkan); #ifdef SONAME_LIBVULKAN
static VkResult (*pvkCreateInstance)(const VkInstanceCreateInfo *, const VkAllocationCallbacks *, VkInstance *); +static void (*pvkDestroyInstance)(VkInstance, const VkAllocationCallbacks *);
static void *vulkan_handle;
@@ -116,6 +117,17 @@ static VkResult wayland_vkCreateInstance(const VkInstanceCreateInfo *create_info return res; }
+static void wayland_vkDestroyInstance(VkInstance instance, + const VkAllocationCallbacks *allocator) +{ + TRACE("%p %p\n", instance, allocator); + + if (allocator) + FIXME("Support for allocation callbacks not implemented yet\n"); + + pvkDestroyInstance(instance, NULL /* allocator */); +} + static void wine_vk_init(void) { if (!(vulkan_handle = dlopen(SONAME_LIBVULKAN, RTLD_NOW))) @@ -126,6 +138,7 @@ static void wine_vk_init(void)
#define LOAD_FUNCPTR(f) if (!(p##f = dlsym(vulkan_handle, #f))) goto fail LOAD_FUNCPTR(vkCreateInstance); + LOAD_FUNCPTR(vkDestroyInstance); #undef LOAD_FUNCPTR
return; @@ -137,7 +150,8 @@ fail:
static const struct vulkan_funcs vulkan_funcs = { - .p_vkCreateInstance = wayland_vkCreateInstance + .p_vkCreateInstance = wayland_vkCreateInstance, + .p_vkDestroyInstance = wayland_vkDestroyInstance };
/**********************************************************************
From: Alexandros Frantzis alexandros.frantzis@collabora.com
Create Win32 VkSurfaceKHR objects which are backed by native Wayland VkSurfaceKHR objects. For now we associate a dummy Wayland surface with the VkSurfaceKHR. --- dlls/winewayland.drv/vulkan.c | 95 ++++++++++++++++++++++++++++++++++- 1 file changed, 94 insertions(+), 1 deletion(-)
diff --git a/dlls/winewayland.drv/vulkan.c b/dlls/winewayland.drv/vulkan.c index b89e6cdcc45..6e2d430881a 100644 --- a/dlls/winewayland.drv/vulkan.c +++ b/dlls/winewayland.drv/vulkan.c @@ -41,11 +41,40 @@ WINE_DEFAULT_DEBUG_CHANNEL(vulkan);
#ifdef SONAME_LIBVULKAN
+#define VK_STRUCTURE_TYPE_WAYLAND_SURFACE_CREATE_INFO_KHR 1000006000 + +typedef struct VkWaylandSurfaceCreateInfoKHR +{ + VkStructureType sType; + const void *pNext; + VkWaylandSurfaceCreateFlagsKHR flags; + struct wl_display *display; + struct wl_surface *surface; +} VkWaylandSurfaceCreateInfoKHR; + static VkResult (*pvkCreateInstance)(const VkInstanceCreateInfo *, const VkAllocationCallbacks *, VkInstance *); +static VkResult (*pvkCreateWaylandSurfaceKHR)(VkInstance, const VkWaylandSurfaceCreateInfoKHR *, const VkAllocationCallbacks *, VkSurfaceKHR *); static void (*pvkDestroyInstance)(VkInstance, const VkAllocationCallbacks *);
static void *vulkan_handle;
+struct wine_vk_surface +{ + struct wl_surface *client; + VkSurfaceKHR native; +}; + +static struct wine_vk_surface *wine_vk_surface_from_handle(VkSurfaceKHR handle) +{ + return (struct wine_vk_surface *)(uintptr_t)handle; +} + +static void wine_vk_surface_destroy(struct wine_vk_surface *wine_vk_surface) +{ + if (wine_vk_surface->client) wl_surface_destroy(wine_vk_surface->client); + free(wine_vk_surface); +} + /* Helper function for converting between win32 and Wayland compatible VkInstanceCreateInfo. * Caller is responsible for allocation and cleanup of 'dst'. */ static VkResult wine_vk_instance_convert_create_info(const VkInstanceCreateInfo *src, @@ -117,6 +146,62 @@ static VkResult wayland_vkCreateInstance(const VkInstanceCreateInfo *create_info return res; }
+static VkResult wayland_vkCreateWin32SurfaceKHR(VkInstance instance, + const VkWin32SurfaceCreateInfoKHR *create_info, + const VkAllocationCallbacks *allocator, + VkSurfaceKHR *vk_surface) +{ + VkResult res; + VkWaylandSurfaceCreateInfoKHR create_info_host; + struct wine_vk_surface *wine_vk_surface; + + TRACE("%p %p %p %p\n", instance, create_info, allocator, vk_surface); + + if (allocator) + FIXME("Support for allocation callbacks not implemented yet\n"); + + wine_vk_surface = calloc(1, sizeof(*wine_vk_surface)); + if (!wine_vk_surface) + { + ERR("Failed to allocate memory for wayland vulkan surface\n"); + res = VK_ERROR_OUT_OF_HOST_MEMORY; + goto err; + } + + wine_vk_surface->client = wl_compositor_create_surface(process_wayland.wl_compositor); + if (!wine_vk_surface->client) + { + ERR("Failed to create client surface for hwnd=%p\n", create_info->hwnd); + /* VK_KHR_win32_surface only allows out of host and device memory as errors. */ + res = VK_ERROR_OUT_OF_HOST_MEMORY; + goto err; + } + + create_info_host.sType = VK_STRUCTURE_TYPE_WAYLAND_SURFACE_CREATE_INFO_KHR; + create_info_host.pNext = NULL; + create_info_host.flags = 0; /* reserved */ + create_info_host.display = process_wayland.wl_display; + create_info_host.surface = wine_vk_surface->client; + + res = pvkCreateWaylandSurfaceKHR(instance, &create_info_host, + NULL /* allocator */, + &wine_vk_surface->native); + if (res != VK_SUCCESS) + { + ERR("Failed to create vulkan wayland surface, res=%d\n", res); + goto err; + } + + *vk_surface = (uintptr_t)wine_vk_surface; + + TRACE("Created surface=0x%s\n", wine_dbgstr_longlong(*vk_surface)); + return VK_SUCCESS; + +err: + if (wine_vk_surface) wine_vk_surface_destroy(wine_vk_surface); + return res; +} + static void wayland_vkDestroyInstance(VkInstance instance, const VkAllocationCallbacks *allocator) { @@ -128,6 +213,11 @@ static void wayland_vkDestroyInstance(VkInstance instance, pvkDestroyInstance(instance, NULL /* allocator */); }
+static VkSurfaceKHR wayland_wine_get_native_surface(VkSurfaceKHR surface) +{ + return wine_vk_surface_from_handle(surface)->native; +} + static void wine_vk_init(void) { if (!(vulkan_handle = dlopen(SONAME_LIBVULKAN, RTLD_NOW))) @@ -138,6 +228,7 @@ static void wine_vk_init(void)
#define LOAD_FUNCPTR(f) if (!(p##f = dlsym(vulkan_handle, #f))) goto fail LOAD_FUNCPTR(vkCreateInstance); + LOAD_FUNCPTR(vkCreateWaylandSurfaceKHR); LOAD_FUNCPTR(vkDestroyInstance); #undef LOAD_FUNCPTR
@@ -151,7 +242,9 @@ fail: static const struct vulkan_funcs vulkan_funcs = { .p_vkCreateInstance = wayland_vkCreateInstance, - .p_vkDestroyInstance = wayland_vkDestroyInstance + .p_vkCreateWin32SurfaceKHR = wayland_vkCreateWin32SurfaceKHR, + .p_vkDestroyInstance = wayland_vkDestroyInstance, + .p_wine_get_native_surface = wayland_wine_get_native_surface };
/**********************************************************************
From: Alexandros Frantzis alexandros.frantzis@collabora.com
--- dlls/winewayland.drv/vulkan.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
diff --git a/dlls/winewayland.drv/vulkan.c b/dlls/winewayland.drv/vulkan.c index 6e2d430881a..13ae73645a1 100644 --- a/dlls/winewayland.drv/vulkan.c +++ b/dlls/winewayland.drv/vulkan.c @@ -55,6 +55,7 @@ typedef struct VkWaylandSurfaceCreateInfoKHR static VkResult (*pvkCreateInstance)(const VkInstanceCreateInfo *, const VkAllocationCallbacks *, VkInstance *); static VkResult (*pvkCreateWaylandSurfaceKHR)(VkInstance, const VkWaylandSurfaceCreateInfoKHR *, const VkAllocationCallbacks *, VkSurfaceKHR *); static void (*pvkDestroyInstance)(VkInstance, const VkAllocationCallbacks *); +static void (*pvkDestroySurfaceKHR)(VkInstance, VkSurfaceKHR, const VkAllocationCallbacks *);
static void *vulkan_handle;
@@ -213,6 +214,23 @@ static void wayland_vkDestroyInstance(VkInstance instance, pvkDestroyInstance(instance, NULL /* allocator */); }
+static void wayland_vkDestroySurfaceKHR(VkInstance instance, VkSurfaceKHR surface, + const VkAllocationCallbacks *allocator) +{ + struct wine_vk_surface *wine_vk_surface = wine_vk_surface_from_handle(surface); + + TRACE("%p 0x%s %p\n", instance, wine_dbgstr_longlong(surface), allocator); + + if (allocator) + FIXME("Support for allocation callbacks not implemented yet\n"); + + /* vkDestroySurfaceKHR must handle VK_NULL_HANDLE (0) for surface. */ + if (!wine_vk_surface) return; + + pvkDestroySurfaceKHR(instance, wine_vk_surface->native, NULL /* allocator */); + wine_vk_surface_destroy(wine_vk_surface); +} + static VkSurfaceKHR wayland_wine_get_native_surface(VkSurfaceKHR surface) { return wine_vk_surface_from_handle(surface)->native; @@ -230,6 +248,7 @@ static void wine_vk_init(void) LOAD_FUNCPTR(vkCreateInstance); LOAD_FUNCPTR(vkCreateWaylandSurfaceKHR); LOAD_FUNCPTR(vkDestroyInstance); + LOAD_FUNCPTR(vkDestroySurfaceKHR); #undef LOAD_FUNCPTR
return; @@ -244,6 +263,7 @@ static const struct vulkan_funcs vulkan_funcs = .p_vkCreateInstance = wayland_vkCreateInstance, .p_vkCreateWin32SurfaceKHR = wayland_vkCreateWin32SurfaceKHR, .p_vkDestroyInstance = wayland_vkDestroyInstance, + .p_vkDestroySurfaceKHR = wayland_vkDestroySurfaceKHR, .p_wine_get_native_surface = wayland_wine_get_native_surface };
From: Alexandros Frantzis alexandros.frantzis@collabora.com
Since we can't render to parts of surfaces, use a dedicated client area subsurface as the target of Vulkan rendering. --- dlls/winewayland.drv/vulkan.c | 33 +++++++++++-- dlls/winewayland.drv/wayland.c | 5 ++ dlls/winewayland.drv/wayland_surface.c | 67 ++++++++++++++++++++++++++ dlls/winewayland.drv/waylanddrv.h | 11 +++++ 4 files changed, 112 insertions(+), 4 deletions(-)
diff --git a/dlls/winewayland.drv/vulkan.c b/dlls/winewayland.drv/vulkan.c index 13ae73645a1..4a3a5dcb852 100644 --- a/dlls/winewayland.drv/vulkan.c +++ b/dlls/winewayland.drv/vulkan.c @@ -61,7 +61,7 @@ static void *vulkan_handle;
struct wine_vk_surface { - struct wl_surface *client; + struct wayland_client_surface *client; VkSurfaceKHR native; };
@@ -72,7 +72,20 @@ static struct wine_vk_surface *wine_vk_surface_from_handle(VkSurfaceKHR handle)
static void wine_vk_surface_destroy(struct wine_vk_surface *wine_vk_surface) { - if (wine_vk_surface->client) wl_surface_destroy(wine_vk_surface->client); + if (wine_vk_surface->client) + { + HWND hwnd = wl_surface_get_user_data(wine_vk_surface->client->wl_surface); + struct wayland_surface *wayland_surface = wayland_surface_lock_hwnd(hwnd); + + if (wayland_client_surface_release(wine_vk_surface->client) && + wayland_surface) + { + wayland_surface->client = NULL; + } + + if (wayland_surface) pthread_mutex_unlock(&wayland_surface->mutex); + } + free(wine_vk_surface); }
@@ -155,6 +168,7 @@ static VkResult wayland_vkCreateWin32SurfaceKHR(VkInstance instance, VkResult res; VkWaylandSurfaceCreateInfoKHR create_info_host; struct wine_vk_surface *wine_vk_surface; + struct wayland_surface *wayland_surface;
TRACE("%p %p %p %p\n", instance, create_info, allocator, vk_surface);
@@ -169,7 +183,18 @@ static VkResult wayland_vkCreateWin32SurfaceKHR(VkInstance instance, goto err; }
- wine_vk_surface->client = wl_compositor_create_surface(process_wayland.wl_compositor); + wayland_surface = wayland_surface_lock_hwnd(create_info->hwnd); + if (!wayland_surface) + { + ERR("Failed to find wayland surface for hwnd=%p\n", create_info->hwnd); + /* VK_KHR_win32_surface only allows out of host and device memory as errors. */ + res = VK_ERROR_OUT_OF_HOST_MEMORY; + goto err; + } + + wine_vk_surface->client = wayland_surface_get_client(wayland_surface); + pthread_mutex_unlock(&wayland_surface->mutex); + if (!wine_vk_surface->client) { ERR("Failed to create client surface for hwnd=%p\n", create_info->hwnd); @@ -182,7 +207,7 @@ static VkResult wayland_vkCreateWin32SurfaceKHR(VkInstance instance, create_info_host.pNext = NULL; create_info_host.flags = 0; /* reserved */ create_info_host.display = process_wayland.wl_display; - create_info_host.surface = wine_vk_surface->client; + create_info_host.surface = wine_vk_surface->client->wl_surface;
res = pvkCreateWaylandSurfaceKHR(instance, &create_info_host, NULL /* allocator */, diff --git a/dlls/winewayland.drv/wayland.c b/dlls/winewayland.drv/wayland.c index b8c69a105cb..c932735101c 100644 --- a/dlls/winewayland.drv/wayland.c +++ b/dlls/winewayland.drv/wayland.c @@ -138,6 +138,11 @@ static void registry_handle_global(void *data, struct wl_registry *registry, wl_seat_add_listener(seat->wl_seat, &seat_listener, NULL); pthread_mutex_unlock(&seat->mutex); } + else if (strcmp(interface, "wl_subcompositor") == 0) + { + process_wayland.wl_subcompositor = + wl_registry_bind(registry, id, &wl_subcompositor_interface, 1); + } }
static void registry_handle_global_remove(void *data, struct wl_registry *registry, diff --git a/dlls/winewayland.drv/wayland_surface.c b/dlls/winewayland.drv/wayland_surface.c index ae4812ebb08..61bbfdc2588 100644 --- a/dlls/winewayland.drv/wayland_surface.c +++ b/dlls/winewayland.drv/wayland_surface.c @@ -616,3 +616,70 @@ err: if (shm_buffer) wayland_shm_buffer_unref(shm_buffer); return NULL; } + +/********************************************************************** + * wayland_client_surface_release + */ +BOOL wayland_client_surface_release(struct wayland_client_surface *client) +{ + if (InterlockedDecrement(&client->ref)) return FALSE; + + if (client->wl_subsurface) + wl_subsurface_destroy(client->wl_subsurface); + if (client->wl_surface) + wl_surface_destroy(client->wl_surface); + + free(client); + + return TRUE; +} + +/********************************************************************** + * wayland_surface_get_client + */ +struct wayland_client_surface *wayland_surface_get_client(struct wayland_surface *surface) +{ + if (surface->client) + { + InterlockedIncrement(&surface->client->ref); + return surface->client; + } + + surface->client = calloc(1, sizeof(*surface->client)); + if (!surface->client) + { + ERR("Failed to allocate space for client surface\n"); + goto err; + } + + surface->client->ref = 1; + + surface->client->wl_surface = + wl_compositor_create_surface(process_wayland.wl_compositor); + if (!surface->client->wl_surface) + { + ERR("Failed to create client wl_surface\n"); + goto err; + } + wl_surface_set_user_data(surface->client->wl_surface, surface->hwnd); + + surface->client->wl_subsurface = + wl_subcompositor_get_subsurface(process_wayland.wl_subcompositor, + surface->client->wl_surface, + surface->wl_surface); + if (!surface->client->wl_subsurface) + { + ERR("Failed to create client wl_subsurface\n"); + goto err; + } + + return surface->client; + +err: + if (surface->client) + { + wayland_client_surface_release(surface->client); + surface->client = NULL; + } + return NULL; +} diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index 6ef86ae5a37..37c6cef570a 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -99,6 +99,7 @@ struct wayland struct wl_compositor *wl_compositor; struct xdg_wm_base *xdg_wm_base; struct wl_shm *wl_shm; + struct wl_subcompositor *wl_subcompositor; struct wayland_seat seat; struct wayland_pointer pointer; struct wl_list output_list; @@ -148,6 +149,13 @@ struct wayland_window_config enum wayland_surface_config_state state; };
+struct wayland_client_surface +{ + LONG ref; + struct wl_surface *wl_surface; + struct wl_subsurface *wl_subsurface; +}; + struct wayland_surface { HWND hwnd; @@ -159,6 +167,7 @@ struct wayland_surface struct wayland_shm_buffer *latest_window_buffer; BOOL resizing; struct wayland_window_config window; + struct wayland_client_surface *client; };
struct wayland_shm_buffer @@ -204,6 +213,8 @@ BOOL wayland_surface_reconfigure(struct wayland_surface *surface) DECLSPEC_HIDDE BOOL wayland_surface_config_is_compatible(struct wayland_surface_config *conf, int width, int height, enum wayland_surface_config_state state) DECLSPEC_HIDDEN; +struct wayland_client_surface *wayland_surface_get_client(struct wayland_surface *surface) DECLSPEC_HIDDEN; +BOOL wayland_client_surface_release(struct wayland_client_surface *client) DECLSPEC_HIDDEN;
/********************************************************************** * Wayland SHM buffer
From: Alexandros Frantzis alexandros.frantzis@collabora.com
Set the position of the client area subsurface relative to its parent surface. --- dlls/winewayland.drv/wayland_surface.c | 23 +++++++++++++++++++++++ dlls/winewayland.drv/waylanddrv.h | 1 + dlls/winewayland.drv/window.c | 11 +++++++++-- 3 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/dlls/winewayland.drv/wayland_surface.c b/dlls/winewayland.drv/wayland_surface.c index 61bbfdc2588..91c8d396975 100644 --- a/dlls/winewayland.drv/wayland_surface.c +++ b/dlls/winewayland.drv/wayland_surface.c @@ -427,6 +427,26 @@ static void wayland_surface_reconfigure_geometry(struct wayland_surface *surface } }
+/********************************************************************** + * wayland_surface_reconfigure_client + * + * Reconfigures the subsurface covering the client area. + */ +static void wayland_surface_reconfigure_client(struct wayland_surface *surface) +{ + struct wayland_window_config *window = &surface->window; + int x, y; + + if (!surface->client) return; + + x = window->client_rect.left - window->rect.left; + y = window->client_rect.top - window->rect.top; + + TRACE("hwnd=%p subsurface=%d,%d\n", surface->hwnd, x, y); + + wl_subsurface_set_position(surface->client->wl_subsurface, x, y); +} + /********************************************************************** * wayland_surface_reconfigure * @@ -480,6 +500,7 @@ BOOL wayland_surface_reconfigure(struct wayland_surface *surface) }
wayland_surface_reconfigure_geometry(surface); + wayland_surface_reconfigure_client(surface);
return TRUE; } @@ -673,6 +694,8 @@ struct wayland_client_surface *wayland_surface_get_client(struct wayland_surface goto err; }
+ wayland_surface_reconfigure_client(surface); + return surface->client;
err: diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index 37c6cef570a..b5d5462d18e 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -146,6 +146,7 @@ struct wayland_surface_config struct wayland_window_config { RECT rect; + RECT client_rect; enum wayland_surface_config_state state; };
diff --git a/dlls/winewayland.drv/window.c b/dlls/winewayland.drv/window.c index 57d2f15a06b..984dfbe5d26 100644 --- a/dlls/winewayland.drv/window.c +++ b/dlls/winewayland.drv/window.c @@ -45,6 +45,8 @@ struct wayland_win_data struct window_surface *window_surface; /* USER window rectangle relative to win32 parent window client area */ RECT window_rect; + /* USER client rectangle relative to win32 parent window client area */ + RECT client_rect; };
static int wayland_win_data_cmp_rb(const void *key, @@ -68,7 +70,8 @@ static struct rb_tree win_data_rb = { wayland_win_data_cmp_rb }; * Create a data window structure for an existing window. */ static struct wayland_win_data *wayland_win_data_create(HWND hwnd, - const RECT *window_rect) + const RECT *window_rect, + const RECT *client_rect) { struct wayland_win_data *data; struct rb_entry *rb_entry; @@ -83,6 +86,7 @@ static struct wayland_win_data *wayland_win_data_create(HWND hwnd,
data->hwnd = hwnd; data->window_rect = *window_rect; + data->client_rect = *client_rect;
pthread_mutex_lock(&win_data_mutex);
@@ -157,6 +161,7 @@ static void wayland_win_data_get_config(struct wayland_win_data *data, DWORD style;
conf->rect = data->window_rect; + conf->client_rect = data->client_rect; style = NtUserGetWindowLongW(data->hwnd, GWL_STYLE);
TRACE("window=%s style=%#lx\n", wine_dbgstr_rect(&conf->rect), (long)style); @@ -309,7 +314,8 @@ BOOL WAYLAND_WindowPosChanging(HWND hwnd, HWND insert_after, UINT swp_flags, hwnd, wine_dbgstr_rect(window_rect), wine_dbgstr_rect(client_rect), wine_dbgstr_rect(visible_rect), insert_after, swp_flags);
- if (!data && !(data = wayland_win_data_create(hwnd, window_rect))) return TRUE; + if (!data && !(data = wayland_win_data_create(hwnd, window_rect, client_rect))) + return TRUE;
/* Release the dummy surface wine provides for toplevels. */ if (*surface) window_surface_release(*surface); @@ -360,6 +366,7 @@ void WAYLAND_WindowPosChanged(HWND hwnd, HWND insert_after, UINT swp_flags, if (!data) return;
data->window_rect = *window_rect; + data->client_rect = *client_rect;
if (surface) window_surface_add_ref(surface); if (data->window_surface) window_surface_release(data->window_surface);
From: Alexandros Frantzis alexandros.frantzis@collabora.com
Use the viewporter Wayland protocol to set the size of the client area subsurface, so that it always covers the client area bounds exactly. This may transiently lead to scaled contents. --- dlls/winewayland.drv/Makefile.in | 1 + dlls/winewayland.drv/viewporter.xml | 180 +++++++++++++++++++++++++ dlls/winewayland.drv/wayland.c | 5 + dlls/winewayland.drv/wayland_surface.c | 25 +++- dlls/winewayland.drv/waylanddrv.h | 3 + 5 files changed, 213 insertions(+), 1 deletion(-) create mode 100644 dlls/winewayland.drv/viewporter.xml
diff --git a/dlls/winewayland.drv/Makefile.in b/dlls/winewayland.drv/Makefile.in index bbce790899e..15dd51e4d7c 100644 --- a/dlls/winewayland.drv/Makefile.in +++ b/dlls/winewayland.drv/Makefile.in @@ -7,6 +7,7 @@ SOURCES = \ display.c \ dllmain.c \ version.rc \ + viewporter.xml \ vulkan.c \ wayland.c \ wayland_output.c \ diff --git a/dlls/winewayland.drv/viewporter.xml b/dlls/winewayland.drv/viewporter.xml new file mode 100644 index 00000000000..d1048d1f332 --- /dev/null +++ b/dlls/winewayland.drv/viewporter.xml @@ -0,0 +1,180 @@ +<?xml version="1.0" encoding="UTF-8"?> +<protocol name="viewporter"> + + <copyright> + Copyright © 2013-2016 Collabora, Ltd. + + Permission is hereby granted, free of charge, to any person obtaining a + copy of this software and associated documentation files (the "Software"), + to deal in the Software without restriction, including without limitation + the rights to use, copy, modify, merge, publish, distribute, sublicense, + and/or sell copies of the Software, and to permit persons to whom the + Software is furnished to do so, subject to the following conditions: + + The above copyright notice and this permission notice (including the next + paragraph) shall be included in all copies or substantial portions of the + Software. + + THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + DEALINGS IN THE SOFTWARE. + </copyright> + + <interface name="wp_viewporter" version="1"> + <description summary="surface cropping and scaling"> + The global interface exposing surface cropping and scaling + capabilities is used to instantiate an interface extension for a + wl_surface object. This extended interface will then allow + cropping and scaling the surface contents, effectively + disconnecting the direct relationship between the buffer and the + surface size. + </description> + + <request name="destroy" type="destructor"> + <description summary="unbind from the cropping and scaling interface"> + Informs the server that the client will not be using this + protocol object anymore. This does not affect any other objects, + wp_viewport objects included. + </description> + </request> + + <enum name="error"> + <entry name="viewport_exists" value="0" + summary="the surface already has a viewport object associated"/> + </enum> + + <request name="get_viewport"> + <description summary="extend surface interface for crop and scale"> + Instantiate an interface extension for the given wl_surface to + crop and scale its content. If the given wl_surface already has + a wp_viewport object associated, the viewport_exists + protocol error is raised. + </description> + <arg name="id" type="new_id" interface="wp_viewport" + summary="the new viewport interface id"/> + <arg name="surface" type="object" interface="wl_surface" + summary="the surface"/> + </request> + </interface> + + <interface name="wp_viewport" version="1"> + <description summary="crop and scale interface to a wl_surface"> + An additional interface to a wl_surface object, which allows the + client to specify the cropping and scaling of the surface + contents. + + This interface works with two concepts: the source rectangle (src_x, + src_y, src_width, src_height), and the destination size (dst_width, + dst_height). The contents of the source rectangle are scaled to the + destination size, and content outside the source rectangle is ignored. + This state is double-buffered, and is applied on the next + wl_surface.commit. + + The two parts of crop and scale state are independent: the source + rectangle, and the destination size. Initially both are unset, that + is, no scaling is applied. The whole of the current wl_buffer is + used as the source, and the surface size is as defined in + wl_surface.attach. + + If the destination size is set, it causes the surface size to become + dst_width, dst_height. The source (rectangle) is scaled to exactly + this size. This overrides whatever the attached wl_buffer size is, + unless the wl_buffer is NULL. If the wl_buffer is NULL, the surface + has no content and therefore no size. Otherwise, the size is always + at least 1x1 in surface local coordinates. + + If the source rectangle is set, it defines what area of the wl_buffer is + taken as the source. If the source rectangle is set and the destination + size is not set, then src_width and src_height must be integers, and the + surface size becomes the source rectangle size. This results in cropping + without scaling. If src_width or src_height are not integers and + destination size is not set, the bad_size protocol error is raised when + the surface state is applied. + + The coordinate transformations from buffer pixel coordinates up to + the surface-local coordinates happen in the following order: + 1. buffer_transform (wl_surface.set_buffer_transform) + 2. buffer_scale (wl_surface.set_buffer_scale) + 3. crop and scale (wp_viewport.set*) + This means, that the source rectangle coordinates of crop and scale + are given in the coordinates after the buffer transform and scale, + i.e. in the coordinates that would be the surface-local coordinates + if the crop and scale was not applied. + + If src_x or src_y are negative, the bad_value protocol error is raised. + Otherwise, if the source rectangle is partially or completely outside of + the non-NULL wl_buffer, then the out_of_buffer protocol error is raised + when the surface state is applied. A NULL wl_buffer does not raise the + out_of_buffer error. + + If the wl_surface associated with the wp_viewport is destroyed, + all wp_viewport requests except 'destroy' raise the protocol error + no_surface. + + If the wp_viewport object is destroyed, the crop and scale + state is removed from the wl_surface. The change will be applied + on the next wl_surface.commit. + </description> + + <request name="destroy" type="destructor"> + <description summary="remove scaling and cropping from the surface"> + The associated wl_surface's crop and scale state is removed. + The change is applied on the next wl_surface.commit. + </description> + </request> + + <enum name="error"> + <entry name="bad_value" value="0" + summary="negative or zero values in width or height"/> + <entry name="bad_size" value="1" + summary="destination size is not integer"/> + <entry name="out_of_buffer" value="2" + summary="source rectangle extends outside of the content area"/> + <entry name="no_surface" value="3" + summary="the wl_surface was destroyed"/> + </enum> + + <request name="set_source"> + <description summary="set the source rectangle for cropping"> + Set the source rectangle of the associated wl_surface. See + wp_viewport for the description, and relation to the wl_buffer + size. + + If all of x, y, width and height are -1.0, the source rectangle is + unset instead. Any other set of values where width or height are zero + or negative, or x or y are negative, raise the bad_value protocol + error. + + The crop and scale state is double-buffered state, and will be + applied on the next wl_surface.commit. + </description> + <arg name="x" type="fixed" summary="source rectangle x"/> + <arg name="y" type="fixed" summary="source rectangle y"/> + <arg name="width" type="fixed" summary="source rectangle width"/> + <arg name="height" type="fixed" summary="source rectangle height"/> + </request> + + <request name="set_destination"> + <description summary="set the surface size for scaling"> + Set the destination size of the associated wl_surface. See + wp_viewport for the description, and relation to the wl_buffer + size. + + If width is -1 and height is -1, the destination size is unset + instead. Any other pair of values for width and height that + contains zero or negative values raises the bad_value protocol + error. + + The crop and scale state is double-buffered state, and will be + applied on the next wl_surface.commit. + </description> + <arg name="width" type="int" summary="surface width"/> + <arg name="height" type="int" summary="surface height"/> + </request> + </interface> + +</protocol> diff --git a/dlls/winewayland.drv/wayland.c b/dlls/winewayland.drv/wayland.c index c932735101c..525925ab00e 100644 --- a/dlls/winewayland.drv/wayland.c +++ b/dlls/winewayland.drv/wayland.c @@ -143,6 +143,11 @@ static void registry_handle_global(void *data, struct wl_registry *registry, process_wayland.wl_subcompositor = wl_registry_bind(registry, id, &wl_subcompositor_interface, 1); } + else if (strcmp(interface, "wp_viewporter") == 0) + { + process_wayland.wp_viewporter = + wl_registry_bind(registry, id, &wp_viewporter_interface, 1); + } }
static void registry_handle_global_remove(void *data, struct wl_registry *registry, diff --git a/dlls/winewayland.drv/wayland_surface.c b/dlls/winewayland.drv/wayland_surface.c index 91c8d396975..b1f027d4dad 100644 --- a/dlls/winewayland.drv/wayland_surface.c +++ b/dlls/winewayland.drv/wayland_surface.c @@ -436,15 +436,29 @@ static void wayland_surface_reconfigure_client(struct wayland_surface *surface) { struct wayland_window_config *window = &surface->window; int x, y; + int width, height;
if (!surface->client) return;
x = window->client_rect.left - window->rect.left; y = window->client_rect.top - window->rect.top; + width = window->client_rect.right - window->client_rect.left; + height = window->client_rect.bottom - window->client_rect.top;
- TRACE("hwnd=%p subsurface=%d,%d\n", surface->hwnd, x, y); + TRACE("hwnd=%p subsurface=%d,%d+%dx%d\n", surface->hwnd, x, y, width, height);
wl_subsurface_set_position(surface->client->wl_subsurface, x, y); + + if (surface->client->wp_viewport) + { + if (width != 0 && height != 0) + wp_viewport_set_destination(surface->client->wp_viewport, + width, height); + else + wp_viewport_set_destination(surface->client->wp_viewport, -1, -1); + } + + wl_surface_commit(surface->client->wl_surface); }
/********************************************************************** @@ -645,6 +659,8 @@ BOOL wayland_client_surface_release(struct wayland_client_surface *client) { if (InterlockedDecrement(&client->ref)) return FALSE;
+ if (client->wp_viewport) + wp_viewport_destroy(client->wp_viewport); if (client->wl_subsurface) wl_subsurface_destroy(client->wl_subsurface); if (client->wl_surface) @@ -694,6 +710,13 @@ struct wayland_client_surface *wayland_surface_get_client(struct wayland_surface goto err; }
+ if (process_wayland.wp_viewporter) + { + surface->client->wp_viewport = + wp_viewporter_get_viewport(process_wayland.wp_viewporter, + surface->client->wl_surface); + } + wayland_surface_reconfigure_client(surface);
return surface->client; diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index b5d5462d18e..06828bc4ae4 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -27,6 +27,7 @@
#include <pthread.h> #include <wayland-client.h> +#include "viewporter-client-protocol.h" #include "xdg-output-unstable-v1-client-protocol.h" #include "xdg-shell-client-protocol.h"
@@ -100,6 +101,7 @@ struct wayland struct xdg_wm_base *xdg_wm_base; struct wl_shm *wl_shm; struct wl_subcompositor *wl_subcompositor; + struct wp_viewporter *wp_viewporter; struct wayland_seat seat; struct wayland_pointer pointer; struct wl_list output_list; @@ -155,6 +157,7 @@ struct wayland_client_surface LONG ref; struct wl_surface *wl_surface; struct wl_subsurface *wl_subsurface; + struct wp_viewport *wp_viewport; };
struct wayland_surface
From: Alexandros Frantzis alexandros.frantzis@collabora.com
Set an empty input region for the client area surface, so that the compositor forwards input events to the main/parent surface instead. This simplifies input handling, since otherwise we would need to implement special handling of events on the client area surface and transform them accordingly. --- dlls/winewayland.drv/wayland_surface.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/dlls/winewayland.drv/wayland_surface.c b/dlls/winewayland.drv/wayland_surface.c index b1f027d4dad..06cea99764a 100644 --- a/dlls/winewayland.drv/wayland_surface.c +++ b/dlls/winewayland.drv/wayland_surface.c @@ -676,6 +676,8 @@ BOOL wayland_client_surface_release(struct wayland_client_surface *client) */ struct wayland_client_surface *wayland_surface_get_client(struct wayland_surface *surface) { + struct wl_region *empty_region; + if (surface->client) { InterlockedIncrement(&surface->client->ref); @@ -700,6 +702,16 @@ struct wayland_client_surface *wayland_surface_get_client(struct wayland_surface } wl_surface_set_user_data(surface->client->wl_surface, surface->hwnd);
+ /* Let parent handle all pointer events. */ + empty_region = wl_compositor_create_region(process_wayland.wl_compositor); + if (!empty_region) + { + ERR("Failed to create wl_region\n"); + goto err; + } + wl_surface_set_input_region(surface->client->wl_surface, empty_region); + wl_region_destroy(empty_region); + surface->client->wl_subsurface = wl_subcompositor_get_subsurface(process_wayland.wl_subcompositor, surface->client->wl_surface,
Rémi Bernon (@rbernon) commented about dlls/winewayland.drv/vulkan.c:
static const struct vulkan_funcs vulkan_funcs = {
- .p_vkCreateInstance = wayland_vkCreateInstance
- .p_vkCreateInstance = wayland_vkCreateInstance,
- .p_vkDestroyInstance = wayland_vkDestroyInstance
Just a nit but I think we should take the habit of always adding a trailing comma. This avoid unnecessary changes to the previous line.
Rémi Bernon (@rbernon) commented about dlls/winewayland.drv/wayland_surface.c:
- free(client);
- return TRUE;
+}
+/**********************************************************************
wayland_surface_get_client- */
+struct wayland_client_surface *wayland_surface_get_client(struct wayland_surface *surface) +{
- if (surface->client)
- {
InterlockedIncrement(&surface->client->ref);return surface->client;- }
Fwiw, something to keep in mind, if not necessarily something to take into account now, as winex11 has the same issue: Proton work showed us that multiple vulkan surfaces may be created for the same window, possibly outliving it.
Only one is active at a time, but we had to decouple the windowing data from the vulkan client surfaces. Instead we keep the vulkan surfaces and their X11 window live in a vulkan-specific list, reparenting them with their windows as they become active or inactive.
At the very least, I don't think we should return the client surface twice, when creating different win32 surfaces.
On Wed Nov 8 23:51:26 2023 +0000, Rémi Bernon wrote:
Just a nit but I think we should take the habit of always adding a trailing comma. This avoid unnecessary changes to the previous line.
Ack, I will update the MR (and subsequent MRs) accordingly.
On Wed Nov 8 23:51:27 2023 +0000, Rémi Bernon wrote:
Fwiw, something to keep in mind, if not necessarily something to take into account now, as winex11 has the same issue: Proton work showed us that multiple vulkan surfaces may be created for the same window, possibly outliving it. Only one is active at a time, but we had to decouple the windowing data from the vulkan client surfaces. Instead we keep the vulkan surfaces and their X11 window live in a vulkan-specific list, reparenting them with their windows as they become active or inactive. At the very least, I don't think we should return the client surface twice, when creating different win32 surfaces.
The proposed design is based on the following assumptions (which are what the Vulkan spec mandates, AFAIU):
1. We can have multiple VkSurfaceKHRs for the same HWND 2. We can have only one (non-retired) VkSwapchainKHR for a HWND (i.e., otherwise we get VK_ERROR_NATIVE_WINDOW_IN_USE_KHR) 3. The HWND may be destroyed while associated VkSurfaceKHRs are still around (which eventually leads to VK_ERROR_SURFACE_LOST_KHR).
Each `wayland_client_surface` may be associated with (but not owned by) a `wayland_surface`, and is implemented as a `wl_subsurface` of that associated parent `wl_surface`. The `wayland_client_surface` is owned collectively (e.g., through refcount) by the VkSurfaceKHRs only. When a HWND (and thus `wayland_surface`) is destroyed, any `wayland_client_surface` continues to live on as long as there are VkSurfaceKHRs targeting it, but it's not visible anymore since the parent `wl_surface` of the client `wl_subsurface` is now gone. In part 10.2 I will also emit VK_ERROR_SURFACE_LOST_KHR for such cases. The swapchain attaches and commits buffers to the subsurface from `wayland_client_surface`.
If I haven't misunderstood your concern, I think this design should cover the case you mentioned. Has some other aspect of the described behavior proved to be problematic in Proton?
On Thu Nov 9 09:14:13 2023 +0000, Alexandros Frantzis wrote:
The proposed design is based on the following assumptions (which are what the Vulkan spec mandates, AFAIU):
- We can have multiple VkSurfaceKHRs for the same HWND
- We can have only one (non-retired) VkSwapchainKHR for a HWND (i.e.,
otherwise we get VK_ERROR_NATIVE_WINDOW_IN_USE_KHR) 3. The HWND may be destroyed while associated VkSurfaceKHRs are still around (which eventually leads to VK_ERROR_SURFACE_LOST_KHR).
Each `wayland_client_surface` may be associated with (but not owned by) a `wayland_surface`, and is implemented as a `wl_subsurface` of that associated parent `wl_surface`. The `wayland_client_surface` is owned collectively (e.g., through refcount) by the VkSurfaceKHRs only. When a HWND (and thus `wayland_surface`) is destroyed, any `wayland_client_surface` continues to live on as long as there are VkSurfaceKHRs targeting it, but it's not visible anymore since the parent `wl_surface` of the client `wl_subsurface` is now gone. In part 10.2 I will also emit VK_ERROR_SURFACE_LOST_KHR for such cases. The swapchain attaches and commits buffers to the subsurface from `wayland_client_surface`.
If I haven't misunderstood your concern, I think this design should cover the case you mentioned. Has some other aspect of the described behavior proved to be problematic in Proton?
Well, I don't know for sure anymore. I believe we had issues when using a single client window. This isn't necessarily a blocker because right now upstream winex11 has the same kind of issues.
Rémi Bernon (@rbernon) commented about dlls/winewayland.drv/wayland_surface.c:
goto err; }
- if (process_wayland.wp_viewporter)
- {
Not asking to do that now, as it's the same for every of the other interfaces, but could we make these interfaces mandatory and get rid of the ifs everywhere? It makes no sense to start a process without wl_subcompositor or wl_viewporter as they are pretty much required for the code to behave correctly.
You should add the `vkEnumerateInstanceExtensionProperties` and `vkGetInstanceProcAddr` implementation as early as possible in this first MR. I don't think anything will try to create surfaces unless they have checked the VK_KHR_surface / VK_KHR_win32_surface extension presence, so all the functions introduced here are basically dead code.
Regarding the later upcoming changes I think you should leave the swapchain wrapping for later and instead use `minImageExtent` from `vkGetPhysicalDeviceSurfaceCapabilities2KHR` (which you should also implement before `vkCreateSwapchain`) to indicate that 0 is unsupported. If that's not enough we may later consider wrapping swapchains but I would be more comfortable if we didn't introduce it only for this purpose if there's a simpler way.
Last nit, "winewayland.drv: Implement vkDestroySurfaceKHR" misses a final dot.
Other than that it looks okay.
-
Alexandros Frantzis -
Alexandros Frantzis (＠afrantzis)
-
Rémi Bernon (＠rbernon)