[PATCH v5 3/3] winevulkan: Implement VK_EXT_debug_utils.
Paul Gofman
pgofman at codeweavers.com
Wed Nov 18 16:42:49 CST 2020
I was testing Serious Sam 4 crash on start on Nvidia and found that it
also happens due to debug function implementation in winevulkan. More
precisely, in case of Serious Sam, the crash happens in host
vkSetDebugUtilsObjectNameEXT() when called with objectType
VK_OBJECT_TYPE_SURFACE_KHR. wine_vk_unwrap_handle() returns an unwrapped
handler for this object type, while the native handle is actually
wrapped in struct wine_vk_surfaceby wine driver (winex11.drv). I tested
that correctly unwrapping it fixes the crash.
I am a bit unsure how to properly fix it. The straightforward way is to
pull all the debug functions to wine[x11|mac].drv, but that means
pulling rather lot. Besides, debugging functions have to be queried from
device which winex11 currently doesn't do so we would probably need to
also pull vkCreateDevice() to initialize native function pointers.
The other much shorter way would be to export the unwrapping function
from the wine driver which will be unwrapping handles wrapped in there.
It would probably be unfortunate to introduce a special export for that,
maybe we could put it in the generated 'struct vulkan_funcs' somehow?
Any suggestions?
On 10/6/20 06:35, Thomas Crider wrote:
> Just a heads up, I was testing these patches and found they cause
> Horizon Zero Dawn to crash on startup on nvidia. works fine on AMD/radv
>
> On Tue, Sep 29, 2020 at 5:21 PM Liam Middlebrook
> <lmiddlebrook at nvidia.com <mailto:lmiddlebrook at nvidia.com>> wrote:
>
> This looks good to me, but given the requested changes to dependent
> patches 1/3 and 2/3, I'll wait to sign-off on this one.
>
>
> Thanks,
>
> Liam Middlebrook
>
> On 9/25/20 8:16 AM, Georg Lehmann wrote:
> > Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=49813
> <https://bugs.winehq.org/show_bug.cgi?id=49813>
> >
> > Signed-off-by: Georg Lehmann <dadschoorse at gmail.com
> <mailto:dadschoorse at gmail.com>>
> > ---
> > dlls/winevulkan/make_vulkan | 8 +-
> > dlls/winevulkan/vulkan.c | 183
> +++++++++++++++++++++++++++++++
> > dlls/winevulkan/vulkan_private.h | 31 ++++++
> > 3 files changed, 221 insertions(+), 1 deletion(-)
> >
> > diff --git a/dlls/winevulkan/make_vulkan
> b/dlls/winevulkan/make_vulkan
> > index afea087a92..623d1b9cde 100755
> > --- a/dlls/winevulkan/make_vulkan
> > +++ b/dlls/winevulkan/make_vulkan
> > @@ -92,7 +92,6 @@ UNSUPPORTED_EXTENSIONS = [
> > # plumbing down to the native layer, we will get each
> message twice as we
> > # use 2 loaders (win32+native), but we may get output from
> the driver.
> > # In any case callback conversion is required.
> > - "VK_EXT_debug_utils",
> > "VK_EXT_validation_features",
> > "VK_EXT_validation_flags",
> > "VK_KHR_display", # Needs WSI work.
> > @@ -225,6 +224,11 @@ FUNCTION_OVERRIDES = {
> > # VK_EXT_calibrated_timestamps
> > "vkGetPhysicalDeviceCalibrateableTimeDomainsEXT" :
> {"dispatch" : True, "driver" : False, "thunk" : False},
> > "vkGetCalibratedTimestampsEXT" : {"dispatch" : True,
> "driver" : False, "thunk" : False},
> > +
> > + # VK_EXT_debug_utils
> > + "vkCreateDebugUtilsMessengerEXT" : {"dispatch": True,
> "driver" : False, "thunk" : False},
> > + "vkDestroyDebugUtilsMessengerEXT" : {"dispatch": True,
> "driver" : False, "thunk" : False},
> > + "vkSubmitDebugUtilsMessageEXT" : {"dispatch": True,
> "driver" : False, "thunk" : True, "private_thunk" : True},
> > }
> >
> > STRUCT_CHAIN_CONVERSIONS = [
> > @@ -941,6 +945,8 @@ class VkHandle(object):
> >
> > if self.name <http://self.name> == "VkCommandPool":
> > return
> "wine_cmd_pool_from_handle({0})->command_pool".format(name)
> > + if self.name <http://self.name> ==
> "VkDebugUtilsMessengerEXT":
> > + return
> "wine_debug_utils_messenger_from_handle({0})->debug_messenger".format(name)
> >
> > native_handle_name = None
> >
> > diff --git a/dlls/winevulkan/vulkan.c b/dlls/winevulkan/vulkan.c
> > index 54b910c5e5..f831d486d0 100644
> > --- a/dlls/winevulkan/vulkan.c
> > +++ b/dlls/winevulkan/vulkan.c
> > @@ -57,6 +57,21 @@ static void *wine_vk_find_struct_(void *s,
> VkStructureType t)
> > return NULL;
> > }
> >
> > +#define wine_vk_count_struct(s, t) wine_vk_count_struct_((void
> *)s, VK_STRUCTURE_TYPE_##t)
> > +static uint32_t wine_vk_count_struct_(void *s, VkStructureType t)
> > +{
> > + const VkBaseInStructure *header;
> > + uint32_t result = 0;
> > +
> > + for (header = s; header; header = header->pNext)
> > + {
> > + if (header->sType == t)
> > + result++;
> > + }
> > +
> > + return result;
> > +}
> > +
> > static void *wine_vk_get_global_proc_addr(const char *name);
> >
> > static HINSTANCE hinstance;
> > @@ -113,6 +128,68 @@ static uint64_t wine_vk_get_wrapper(struct
> VkInstance_T *instance, uint64_t nati
> > return result;
> > }
> >
> > +static VkBool32
> debug_utils_callback_conversion(VkDebugUtilsMessageSeverityFlagBitsEXT
> severity,
> > + VkDebugUtilsMessageTypeFlagsEXT message_types,
> > +#if defined(USE_STRUCT_CONVERSION)
> > + const VkDebugUtilsMessengerCallbackDataEXT_host *callback_data,
> > +#else
> > + const VkDebugUtilsMessengerCallbackDataEXT *callback_data,
> > +#endif
> > + void *user_data)
> > +{
> > + struct VkDebugUtilsMessengerCallbackDataEXT wine_callback_data;
> > + VkDebugUtilsObjectNameInfoEXT *object_name_infos;
> > + struct wine_debug_utils_messenger *object;
> > + VkBool32 result;
> > + int i;
> > +
> > + TRACE("%i, %u, %p, %p\n", severity, message_types,
> callback_data, user_data);
> > +
> > + object = user_data;
> > +
> > + if (!object->instance->instance)
> > + {
> > + /* instance wasn't yet created, this is a message from
> the native loader */
> > + return VK_FALSE;
> > + }
> > +
> > + wine_callback_data =
> *((VkDebugUtilsMessengerCallbackDataEXT *) callback_data);
> > +
> > + object_name_infos =
> heap_calloc(wine_callback_data.objectCount,
> sizeof(*object_name_infos));
> > +
> > + for (i = 0; i < wine_callback_data.objectCount; i++)
> > + {
> > + object_name_infos[i].sType =
> callback_data->pObjects[i].sType;
> > + object_name_infos[i].pNext =
> callback_data->pObjects[i].pNext;
> > + object_name_infos[i].objectType =
> callback_data->pObjects[i].objectType;
> > + object_name_infos[i].pObjectName =
> callback_data->pObjects[i].pObjectName;
> > +
> > + if
> (wine_vk_is_type_wrapped(callback_data->pObjects[i].objectType))
> > + {
> > + object_name_infos[i].objectHandle =
> wine_vk_get_wrapper(object->instance,
> callback_data->pObjects[i].objectHandle);
> > + if (!object_name_infos[i].objectHandle)
> > + {
> > + WARN("handle conversion failed 0x%s\n",
> wine_dbgstr_longlong(callback_data->pObjects[i].objectHandle));
> > + heap_free(object_name_infos);
> > + return VK_FALSE;
> > + }
> > + }
> > + else
> > + {
> > + object_name_infos[i].objectHandle =
> callback_data->pObjects[i].objectHandle;
> > + }
> > + }
> > +
> > + wine_callback_data.pObjects = object_name_infos;
> > +
> > + /* applications should always return VK_FALSE */
> > + result = object->user_callback(severity, message_types,
> &wine_callback_data, object->user_data);
> > +
> > + heap_free(object_name_infos);
> > +
> > + return result;
> > +}
> > +
> > static void wine_vk_physical_device_free(struct
> VkPhysicalDevice_T *phys_dev)
> > {
> > if (!phys_dev)
> > @@ -393,6 +470,8 @@ static void wine_vk_init_once(void)
> > static VkResult wine_vk_instance_convert_create_info(const
> VkInstanceCreateInfo *src,
> > VkInstanceCreateInfo *dst, struct VkInstance_T *object)
> > {
> > + VkDebugUtilsMessengerCreateInfoEXT *debug_utils_messenger;
> > + VkBaseInStructure *header;
> > unsigned int i;
> > VkResult res;
> >
> > @@ -404,6 +483,26 @@ static VkResult
> wine_vk_instance_convert_create_info(const VkInstanceCreateInfo
> > return res;
> > }
> >
> > + object->utils_messenger_count = wine_vk_count_struct(dst,
> DEBUG_UTILS_MESSENGER_CREATE_INFO_EXT);
> > + object->utils_messengers =
> heap_calloc(object->utils_messenger_count,
> sizeof(*object->utils_messengers));
> > + header = (VkBaseInStructure *) dst;
> > + for (i = 0; i < object->utils_messenger_count; i++)
> > + {
> > + header = wine_vk_find_struct(header->pNext,
> DEBUG_UTILS_MESSENGER_CREATE_INFO_EXT);
> > + debug_utils_messenger =
> (VkDebugUtilsMessengerCreateInfoEXT *) header;
> > +
> > + object->utils_messengers[i].instance = object;
> > + object->utils_messengers[i].debug_messenger =
> VK_NULL_HANDLE;
> > + object->utils_messengers[i].user_callback =
> debug_utils_messenger->pfnUserCallback;
> > + object->utils_messengers[i].user_data =
> debug_utils_messenger->pUserData;
> > +
> > + /* convert_VkInstanceCreateInfo_struct_chain already
> copied the chain,
> > + * so we can modify it in-place.
> > + */
> > + debug_utils_messenger->pfnUserCallback = (void *)
> &debug_utils_callback_conversion;
> > + debug_utils_messenger->pUserData =
> &object->utils_messengers[i];
> > + }
> > +
> > /* ICDs don't support any layers, so nothing to copy.
> Modern versions of the loader
> > * filter this data out as well.
> > */
> > @@ -427,6 +526,10 @@ static VkResult
> wine_vk_instance_convert_create_info(const VkInstanceCreateInfo
> > free_VkInstanceCreateInfo_struct_chain(dst);
> > return VK_ERROR_EXTENSION_NOT_PRESENT;
> > }
> > + if (!strcmp(extension_name, "VK_EXT_debug_utils"))
> > + {
> > + object->enable_wrapper_list = VK_TRUE;
> > + }
> > }
> >
> > return VK_SUCCESS;
> > @@ -527,6 +630,8 @@ static void wine_vk_instance_free(struct
> VkInstance_T *instance)
> > WINE_VK_REMOVE_HANDLE_MAPPING(instance, instance);
> > }
> >
> > + heap_free(instance->utils_messengers);
> > +
> > heap_free(instance);
> > }
> >
> > @@ -1673,6 +1778,84 @@ VkResult WINAPI
> wine_vkGetPhysicalDeviceSurfaceCapabilities2KHR(VkPhysicalDevice
> > return res;
> > }
> >
> > +VkResult WINAPI wine_vkCreateDebugUtilsMessengerEXT(VkInstance
> instance, const VkDebugUtilsMessengerCreateInfoEXT *create_info,
> > + const VkAllocationCallbacks *allocator,
> VkDebugUtilsMessengerEXT *messenger)
> > +{
> > + VkDebugUtilsMessengerCreateInfoEXT wine_create_info;
> > + struct wine_debug_utils_messenger *object;
> > + VkResult res;
> > +
> > + TRACE("%p, %p, %p, %p\n", instance, create_info, allocator,
> messenger);
> > +
> > + if (allocator)
> > + FIXME("Support for allocation callbacks not implemented
> yet\n");
> > +
> > + if (!(object = heap_alloc_zero(sizeof(*object))))
> > + return VK_ERROR_OUT_OF_HOST_MEMORY;
> > +
> > + object->instance = instance;
> > + object->user_callback = create_info->pfnUserCallback;
> > + object->user_data = create_info->pUserData;
> > +
> > + wine_create_info = *create_info;
> > +
> > + wine_create_info.pfnUserCallback = (void *)
> &debug_utils_callback_conversion;
> > + wine_create_info.pUserData = object;
> > +
> > + res =
> instance->funcs.p_vkCreateDebugUtilsMessengerEXT(instance->instance,
> &wine_create_info, NULL, &object->debug_messenger);
> > +
> > + if (res != VK_SUCCESS)
> > + {
> > + heap_free(object);
> > + return res;
> > + }
> > +
> > + WINE_VK_ADD_NON_DISPATCHABLE_MAPPING(instance, object,
> object->debug_messenger);
> > + *messenger = wine_debug_utils_messenger_to_handle(object);
> > +
> > + return VK_SUCCESS;
> > +}
> > +
> > +void WINAPI wine_vkDestroyDebugUtilsMessengerEXT(
> > + VkInstance instance, VkDebugUtilsMessengerEXT
> messenger, const VkAllocationCallbacks *allocator)
> > +{
> > + struct wine_debug_utils_messenger *object;
> > +
> > + TRACE("%p, 0x%s, %p\n", instance,
> wine_dbgstr_longlong(messenger), allocator);
> > +
> > + object = wine_debug_utils_messenger_from_handle(messenger);
> > +
> > +
> instance->funcs.p_vkDestroyDebugUtilsMessengerEXT(instance->instance,
> object->debug_messenger, NULL);
> > + WINE_VK_REMOVE_HANDLE_MAPPING(instance, object);
> > +
> > + heap_free(object);
> > +}
> > +
> > +void WINAPI wine_vkSubmitDebugUtilsMessageEXT(VkInstance
> instance, VkDebugUtilsMessageSeverityFlagBitsEXT severity,
> > + VkDebugUtilsMessageTypeFlagsEXT types, const
> VkDebugUtilsMessengerCallbackDataEXT *callback_data)
> > +{
> > + VkDebugUtilsMessengerCallbackDataEXT native_callback_data;
> > + VkDebugUtilsObjectNameInfoEXT *object_names;
> > + int i;
> > +
> > + TRACE("%p, %#x, %#x, %p\n", instance, severity, types,
> callback_data);
> > +
> > + native_callback_data = *callback_data;
> > + object_names = heap_calloc(callback_data->objectCount,
> sizeof(*object_names));
> > + memcpy(object_names, callback_data->pObjects,
> callback_data->objectCount * sizeof(*object_names));
> > + native_callback_data.pObjects = object_names;
> > +
> > + for (i = 0; i < callback_data->objectCount; i++)
> > + {
> > + object_names[i].objectHandle =
> > +
> wine_vk_unwrap_handle(callback_data->pObjects[i].objectType,
> callback_data->pObjects[i].objectHandle);
> > + }
> > +
> > + thunk_vkSubmitDebugUtilsMessageEXT(instance->instance,
> severity, types, &native_callback_data);
> > +
> > + heap_free(object_names);
> > +}
> > +
> > BOOL WINAPI DllMain(HINSTANCE hinst, DWORD reason, void *reserved)
> > {
> > TRACE("%p, %u, %p\n", hinst, reason, reserved);
> > diff --git a/dlls/winevulkan/vulkan_private.h
> b/dlls/winevulkan/vulkan_private.h
> > index 7000da7133..8379fafecf 100644
> > --- a/dlls/winevulkan/vulkan_private.h
> > +++ b/dlls/winevulkan/vulkan_private.h
> > @@ -95,6 +95,8 @@ struct VkDevice_T
> > struct wine_vk_mapping mapping;
> > };
> >
> > +struct wine_debug_utils_messenger;
> > +
> > struct VkInstance_T
> > {
> > struct wine_vk_base base;
> > @@ -111,6 +113,9 @@ struct VkInstance_T
> > struct list wrappers;
> > SRWLOCK wrapper_lock;
> >
> > + struct wine_debug_utils_messenger *utils_messengers;
> > + uint32_t utils_messenger_count;
> > +
> > unsigned int quirks;
> >
> > struct wine_vk_mapping mapping;
> > @@ -158,6 +163,32 @@ static inline VkCommandPool
> wine_cmd_pool_to_handle(struct wine_cmd_pool *cmd_po
> > return (VkCommandPool)(uintptr_t)cmd_pool;
> > }
> >
> > +struct wine_debug_utils_messenger
> > +{
> > + struct VkInstance_T *instance; /* parent */
> > + VkDebugUtilsMessengerEXT debug_messenger; /* native
> messenger */
> > +
> > + /* application callback + data */
> > + PFN_vkDebugUtilsMessengerCallbackEXT user_callback;
> > + void *user_data;
> > +
> > + struct wine_vk_mapping mapping;
> > +};
> > +
> > +static inline struct wine_debug_utils_messenger
> *wine_debug_utils_messenger_from_handle(
> > + VkDebugUtilsMessengerEXT handle)
> > +{
> > + return (struct wine_debug_utils_messenger *)(uintptr_t)handle;
> > +}
> > +
> > +static inline VkDebugUtilsMessengerEXT
> wine_debug_utils_messenger_to_handle(
> > + struct wine_debug_utils_messenger *debug_messenger)
> > +{
> > + return (VkDebugUtilsMessengerEXT)(uintptr_t)debug_messenger;
> > +}
> > +
> > +
> > +
> > void *wine_vk_get_device_proc_addr(const char *name)
> DECLSPEC_HIDDEN;
> > void *wine_vk_get_instance_proc_addr(const char *name)
> DECLSPEC_HIDDEN;
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20201119/c513eb9e/attachment-0001.htm>
More information about the wine-devel
mailing list