It isn't safe to access the view object from any thread other than the main thread. In fact, if you try to call vkCreateMacOSSurfaceMVK() from any other thread, MoltenVK prints out a big, scary warning telling you not to do this! Instead, get the layer from the view ourselves and pass that to MoltenVK. Recent versions of MoltenVK can accept either the view or the layer.
Signed-off-by: Chip Davis cdavis@codeweavers.com --- dlls/winemac.drv/cocoa_window.m | 14 ++++++++++++++ dlls/winemac.drv/macdrv_cocoa.h | 2 ++ dlls/winemac.drv/vulkan.c | 2 +- 3 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/dlls/winemac.drv/cocoa_window.m b/dlls/winemac.drv/cocoa_window.m index 877653ea007..6ac30843025 100644 --- a/dlls/winemac.drv/cocoa_window.m +++ b/dlls/winemac.drv/cocoa_window.m @@ -3796,6 +3796,20 @@ macdrv_metal_view macdrv_view_create_metal_view(macdrv_view v, macdrv_metal_devi return (macdrv_metal_view)metalView; }
+macdrv_metal_layer macdrv_view_get_metal_layer(macdrv_metal_view v) +{ + WineMetalView* view = (WineMetalView*)v; + __block CAMetalLayer* layer; + + if ([NSThread isMainThread]) + layer = (CAMetalLayer*)view.layer; + else OnMainThread(^{ + layer = (CAMetalLayer*)view.layer; + }); + + return (macdrv_metal_layer)layer; +} + void macdrv_view_release_metal_view(macdrv_metal_view v) { WineMetalView* view = (WineMetalView*)v; diff --git a/dlls/winemac.drv/macdrv_cocoa.h b/dlls/winemac.drv/macdrv_cocoa.h index 676adb435bb..b02ad79f025 100644 --- a/dlls/winemac.drv/macdrv_cocoa.h +++ b/dlls/winemac.drv/macdrv_cocoa.h @@ -140,6 +140,7 @@ #ifdef HAVE_METAL_METAL_H typedef struct macdrv_opaque_metal_device* macdrv_metal_device; typedef struct macdrv_opaque_metal_view* macdrv_metal_view; +typedef struct macdrv_opaque_metal_layer* macdrv_metal_layer; #endif typedef struct macdrv_opaque_status_item* macdrv_status_item; struct macdrv_event; @@ -587,6 +588,7 @@ extern void macdrv_set_window_color_key(macdrv_window w, CGFloat keyRed, CGFloat extern macdrv_metal_device macdrv_create_metal_device(void) DECLSPEC_HIDDEN; extern void macdrv_release_metal_device(macdrv_metal_device d) DECLSPEC_HIDDEN; extern macdrv_metal_view macdrv_view_create_metal_view(macdrv_view v, macdrv_metal_device d) DECLSPEC_HIDDEN; +extern macdrv_metal_layer macdrv_view_get_metal_layer(macdrv_metal_view v) DECLSPEC_HIDDEN; extern void macdrv_view_release_metal_view(macdrv_metal_view v) DECLSPEC_HIDDEN; #endif extern int macdrv_get_view_backing_size(macdrv_view v, int backing_size[2]) DECLSPEC_HIDDEN; diff --git a/dlls/winemac.drv/vulkan.c b/dlls/winemac.drv/vulkan.c index cc7f22f6788..21e93827c15 100644 --- a/dlls/winemac.drv/vulkan.c +++ b/dlls/winemac.drv/vulkan.c @@ -282,7 +282,7 @@ static VkResult macdrv_vkCreateWin32SurfaceKHR(VkInstance instance, create_info_host.sType = VK_STRUCTURE_TYPE_MACOS_SURFACE_CREATE_INFO_MVK; create_info_host.pNext = NULL; create_info_host.flags = 0; /* reserved */ - create_info_host.pView = mac_surface->view; + create_info_host.pView = macdrv_view_get_metal_layer(mac_surface->view);
res = pvkCreateMacOSSurfaceMVK(instance, &create_info_host, NULL /* allocator */, &mac_surface->surface); if (res != VK_SUCCESS)
Prefer it to VK_MVK_macos_surface when present.
MoltenVK has deprecated VK_MVK_macos_surface in favor of VK_EXT_metal_surface. It's likely that this extension will vanish at some point.
Signed-off-by: Chip Davis cdavis@codeweavers.com --- dlls/winemac.drv/vulkan.c | 66 +++++++++++++++++++++++++++++++++------ 1 file changed, 57 insertions(+), 9 deletions(-)
diff --git a/dlls/winemac.drv/vulkan.c b/dlls/winemac.drv/vulkan.c index 21e93827c15..60f3dcda6af 100644 --- a/dlls/winemac.drv/vulkan.c +++ b/dlls/winemac.drv/vulkan.c @@ -50,6 +50,9 @@ WINE_DECLARE_DEBUG_CHANNEL(fps); typedef VkFlags VkMacOSSurfaceCreateFlagsMVK; #define VK_STRUCTURE_TYPE_MACOS_SURFACE_CREATE_INFO_MVK 1000123000
+typedef VkFlags VkMetalSurfaceCreateFlagsEXT; +#define VK_STRUCTURE_TYPE_METAL_SURFACE_CREATE_INFO_EXT 1000217000 + struct wine_vk_surface { macdrv_metal_device device; @@ -65,9 +68,18 @@ typedef struct VkMacOSSurfaceCreateInfoMVK const void *pView; /* NSView */ } VkMacOSSurfaceCreateInfoMVK;
+typedef struct VkMetalSurfaceCreateInfoEXT +{ + VkStructureType sType; + const void *pNext; + VkMetalSurfaceCreateFlagsEXT flags; + const void *pLayer; /* CAMetalLayer */ +} VkMetalSurfaceCreateInfoEXT; + static VkResult (*pvkCreateInstance)(const VkInstanceCreateInfo *, const VkAllocationCallbacks *, VkInstance *); static VkResult (*pvkCreateSwapchainKHR)(VkDevice, const VkSwapchainCreateInfoKHR *, const VkAllocationCallbacks *, VkSwapchainKHR *); static VkResult (*pvkCreateMacOSSurfaceMVK)(VkInstance, const VkMacOSSurfaceCreateInfoMVK*, const VkAllocationCallbacks *, VkSurfaceKHR *); +static VkResult (*pvkCreateMetalSurfaceEXT)(VkInstance, const VkMetalSurfaceCreateInfoEXT*, const VkAllocationCallbacks *, VkSurfaceKHR *); static void (*pvkDestroyInstance)(VkInstance, const VkAllocationCallbacks *); static void (*pvkDestroySurfaceKHR)(VkInstance, VkSurfaceKHR, const VkAllocationCallbacks *); static void (*pvkDestroySwapchainKHR)(VkDevice, VkSwapchainKHR, const VkAllocationCallbacks *); @@ -103,6 +115,7 @@ static BOOL WINAPI wine_vk_init(INIT_ONCE *once, void *param, void **context) LOAD_FUNCPTR(vkCreateInstance) LOAD_FUNCPTR(vkCreateSwapchainKHR) LOAD_FUNCPTR(vkCreateMacOSSurfaceMVK) + LOAD_FUNCPTR(vkCreateMetalSurfaceEXT) LOAD_FUNCPTR(vkDestroyInstance) LOAD_FUNCPTR(vkDestroySurfaceKHR) LOAD_FUNCPTR(vkDestroySwapchainKHR) @@ -159,7 +172,7 @@ static VkResult wine_vk_instance_convert_create_info(const VkInstanceCreateInfo */ if (!strcmp(src->ppEnabledExtensionNames[i], "VK_KHR_win32_surface")) { - enabled_extensions[i] = "VK_MVK_macos_surface"; + enabled_extensions[i] = pvkCreateMetalSurfaceEXT ? "VK_EXT_metal_surface" : "VK_MVK_macos_surface"; } else { @@ -239,7 +252,6 @@ static VkResult macdrv_vkCreateWin32SurfaceKHR(VkInstance instance, const VkAllocationCallbacks *allocator, VkSurfaceKHR *surface) { VkResult res; - VkMacOSSurfaceCreateInfoMVK create_info_host; struct wine_vk_surface *mac_surface; struct macdrv_win_data *data;
@@ -279,12 +291,26 @@ static VkResult macdrv_vkCreateWin32SurfaceKHR(VkInstance instance, goto err; }
- create_info_host.sType = VK_STRUCTURE_TYPE_MACOS_SURFACE_CREATE_INFO_MVK; - create_info_host.pNext = NULL; - create_info_host.flags = 0; /* reserved */ - create_info_host.pView = macdrv_view_get_metal_layer(mac_surface->view); + if (pvkCreateMetalSurfaceEXT) + { + VkMetalSurfaceCreateInfoEXT create_info_host; + create_info_host.sType = VK_STRUCTURE_TYPE_METAL_SURFACE_CREATE_INFO_EXT; + create_info_host.pNext = NULL; + create_info_host.flags = 0; /* reserved */ + create_info_host.pLayer = macdrv_view_get_metal_layer(mac_surface->view);
- res = pvkCreateMacOSSurfaceMVK(instance, &create_info_host, NULL /* allocator */, &mac_surface->surface); + res = pvkCreateMetalSurfaceEXT(instance, &create_info_host, NULL /* allocator */, &mac_surface->surface); + } + else + { + VkMacOSSurfaceCreateInfoMVK create_info_host; + create_info_host.sType = VK_STRUCTURE_TYPE_MACOS_SURFACE_CREATE_INFO_MVK; + create_info_host.pNext = NULL; + create_info_host.flags = 0; /* reserved */ + create_info_host.pView = macdrv_view_get_metal_layer(mac_surface->view); + + res = pvkCreateMacOSSurfaceMVK(instance, &create_info_host, NULL /* allocator */, &mac_surface->surface); + } if (res != VK_SUCCESS) { ERR("Failed to create MoltenVK surface, res=%d\n", res); @@ -341,7 +367,7 @@ static void macdrv_vkDestroySwapchainKHR(VkDevice device, VkSwapchainKHR swapcha static VkResult macdrv_vkEnumerateInstanceExtensionProperties(const char *layer_name, uint32_t *count, VkExtensionProperties* properties) { - unsigned int i; + unsigned int i, mvk_surface = -1, ext_surface = -1; VkResult res;
TRACE("layer_name %s, count %p, properties %p\n", debugstr_a(layer_name), count, properties); @@ -364,14 +390,36 @@ static VkResult macdrv_vkEnumerateInstanceExtensionProperties(const char *layer_
for (i = 0; i < *count; i++) { - /* For now the only MoltenVK extension we need to fixup. Long-term we may need an array. */ + /* For now the only MoltenVK extensions we need to fixup. Long-term we may need an array. */ if (!strcmp(properties[i].extensionName, "VK_MVK_macos_surface")) { + if (ext_surface != -1) + { + /* If we've already seen EXT_metal_surface, just hide this one. */ + memcpy(properties + i, properties + i + 1, (--(*count) - i) * sizeof(VkExtensionProperties)); + continue; + } TRACE("Substituting VK_MVK_macos_surface for VK_KHR_win32_surface\n");
snprintf(properties[i].extensionName, sizeof(properties[i].extensionName), VK_KHR_WIN32_SURFACE_EXTENSION_NAME); properties[i].specVersion = VK_KHR_WIN32_SURFACE_SPEC_VERSION; + mvk_surface = i; + } + if (!strcmp(properties[i].extensionName, "VK_EXT_metal_surface")) + { + if (mvk_surface != -1) + { + /* If we've already seen MVK_macos_surface, just hide this one. */ + memcpy(properties + i, properties + i + 1, (--(*count) - i) * sizeof(VkExtensionProperties)); + continue; + } + TRACE("Substituting VK_EXT_metal_surface for VK_KHR_win32_surface\n"); + + snprintf(properties[i].extensionName, sizeof(properties[i].extensionName), + VK_KHR_WIN32_SURFACE_EXTENSION_NAME); + properties[i].specVersion = VK_KHR_WIN32_SURFACE_SPEC_VERSION; + ext_surface = i; } }
On Dec 13, 2019, at 10:57 AM, Chip Davis cdavis@codeweavers.com wrote:
Prefer it to VK_MVK_macos_surface when present.
MoltenVK has deprecated VK_MVK_macos_surface in favor of VK_EXT_metal_surface. It's likely that this extension will vanish at some point.
Signed-off-by: Chip Davis cdavis@codeweavers.com
dlls/winemac.drv/vulkan.c | 66 +++++++++++++++++++++++++++++++++------ 1 file changed, 57 insertions(+), 9 deletions(-)
diff --git a/dlls/winemac.drv/vulkan.c b/dlls/winemac.drv/vulkan.c index 21e93827c15..60f3dcda6af 100644 --- a/dlls/winemac.drv/vulkan.c +++ b/dlls/winemac.drv/vulkan.c […] @@ -341,7 +367,7 @@ static void macdrv_vkDestroySwapchainKHR(VkDevice device, VkSwapchainKHR swapcha static VkResult macdrv_vkEnumerateInstanceExtensionProperties(const char *layer_name, uint32_t *count, VkExtensionProperties* properties) {
- unsigned int i;
unsigned int i, mvk_surface = -1, ext_surface = -1; VkResult res;
TRACE("layer_name %s, count %p, properties %p\n", debugstr_a(layer_name), count, properties);
@@ -364,14 +390,36 @@ static VkResult macdrv_vkEnumerateInstanceExtensionProperties(const char *layer_
for (i = 0; i < *count; i++) {
/* For now the only MoltenVK extension we need to fixup. Long-term we may need an array. */
/* For now the only MoltenVK extensions we need to fixup. Long-term we may need an array. */ if (!strcmp(properties[i].extensionName, "VK_MVK_macos_surface")) {if (ext_surface != -1){/* If we've already seen EXT_metal_surface, just hide this one. */memcpy(properties + i, properties + i + 1, (--(*count) - i) * sizeof(VkExtensionProperties));
This and the memcpy() below should be memmove(), strictly speaking.
Use sizeof(*properties) instead of sizeof(VkExtensionProperties).
Also, please decrement *count in a separate statement. And you need to decrement i, too, or you skip the extension that was moved to properties[i].
continue;} TRACE("Substituting VK_MVK_macos_surface for VK_KHR_win32_surface\n"); snprintf(properties[i].extensionName, sizeof(properties[i].extensionName), VK_KHR_WIN32_SURFACE_EXTENSION_NAME); properties[i].specVersion = VK_KHR_WIN32_SURFACE_SPEC_VERSION;mvk_surface = i;}if (!strcmp(properties[i].extensionName, "VK_EXT_metal_surface")){if (mvk_surface != -1){/* If we've already seen MVK_macos_surface, just hide this one. */memcpy(properties + i, properties + i + 1, (--(*count) - i) * sizeof(VkExtensionProperties));continue;}TRACE("Substituting VK_EXT_metal_surface for VK_KHR_win32_surface\n");snprintf(properties[i].extensionName, sizeof(properties[i].extensionName),VK_KHR_WIN32_SURFACE_EXTENSION_NAME);properties[i].specVersion = VK_KHR_WIN32_SURFACE_SPEC_VERSION;ext_surface = i; }
These two "if" statements are so similar they can and should be combined. There can be a single flag for "seen either surface extension". (No need to track the index.)
Finally, there's a comment above the call to the host vkEnumerateInstanceExtensionProperties() that asserts this function will return the same count. That's no longer true.
-Ken
On Dec 13, 2019, at 10:57 AM, Chip Davis cdavis@codeweavers.com wrote:
It isn't safe to access the view object from any thread other than the main thread. In fact, if you try to call vkCreateMacOSSurfaceMVK() from any other thread, MoltenVK prints out a big, scary warning telling you not to do this! Instead, get the layer from the view ourselves and pass that to MoltenVK. Recent versions of MoltenVK can accept either the view or the layer.
Signed-off-by: Chip Davis cdavis@codeweavers.com
dlls/winemac.drv/cocoa_window.m | 14 ++++++++++++++ dlls/winemac.drv/macdrv_cocoa.h | 2 ++ dlls/winemac.drv/vulkan.c | 2 +- 3 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/dlls/winemac.drv/cocoa_window.m b/dlls/winemac.drv/cocoa_window.m index 877653ea007..6ac30843025 100644 --- a/dlls/winemac.drv/cocoa_window.m +++ b/dlls/winemac.drv/cocoa_window.m @@ -3796,6 +3796,20 @@ macdrv_metal_view macdrv_view_create_metal_view(macdrv_view v, macdrv_metal_devi return (macdrv_metal_view)metalView; }
+macdrv_metal_layer macdrv_view_get_metal_layer(macdrv_metal_view v) +{
- WineMetalView* view = (WineMetalView*)v;
- __block CAMetalLayer* layer;
- if ([NSThread isMainThread])
layer = (CAMetalLayer*)view.layer;
This will never be called from the main thread.
- else OnMainThread(^{
layer = (CAMetalLayer*)view.layer;- });
- return (macdrv_metal_layer)layer;
+}
void macdrv_view_release_metal_view(macdrv_metal_view v) { WineMetalView* view = (WineMetalView*)v;
-
Chip Davis -
Ken Thomases