On 9/22/20 3:56 PM, Dad Schoorse wrote:
On 23. Sept. 2020, 00:03, Liam Middlebrook wrote:
On 9/22/20 7:31 AM, Georg Lehmann wrote:
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=49813
Signed-off-by: Georg Lehmann <dadschoorse at gmail.com
dlls/winevulkan/make_vulkan | 8 +- dlls/winevulkan/vulkan.c | 193 +++++++++++++++++++++++++++++++ dlls/winevulkan/vulkan_private.h | 31 +++++ 3 files changed, 231 insertions(+), 1 deletion(-)
diff --git a/dlls/winevulkan/make_vulkan b/dlls/winevulkan/make_vulkan index f4d223daad..77034cfe36 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 = [ @@ -944,6 +948,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 2747f440ad..11dd0f761e 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) +{
- VkBaseOutStructure *header;
I think VkBaseInStructure would be more appropriate here for preserving const-ness, and given the sole use-case of wine_vk_count_struct that you introduce below.
- 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; @@ -111,6 +126,76 @@ static uint64_t wine_vk_get_wrapper(struct
VkInstance_T *instance, uint64_t nati
return result; }
+static VkBool32 is_wrapped(VkObjectType type) +{
- return type == VK_OBJECT_TYPE_INSTANCE ||
- type == VK_OBJECT_TYPE_PHYSICAL_DEVICE ||
- type == VK_OBJECT_TYPE_DEVICE ||
- type == VK_OBJECT_TYPE_QUEUE ||
- type == VK_OBJECT_TYPE_COMMAND_BUFFER ||
- type == VK_OBJECT_TYPE_COMMAND_POOL ||
- type == VK_OBJECT_TYPE_DEBUG_UTILS_MESSENGER_EXT;
+}
Can this be made into an auto-generated function from make_vulkan? It'd be nicer to keep the amount of manual-touchups to add a new wrapped object type to a minimum.
I later wanted to auto-generate this and unwrap_object_handle in a separate patch series, but I can put it in this patch series if you want.
Sure I'd be fine with this in a follow-up in the series if you were planning something a bit more complex with it.
+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;
- int i;
- object = user_data;
- if (!object->instance->instance)
- {
- /* instance wasn't yet created, this is a message from the
native loader*/
nit: 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 (is_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;
Given that the function will exit early, and a callback will not be serviced back to WINE-side listeners, I think this should be an ERROR and not a WARN.
There are some common situations where this occurs. The loader/validation tools send messages for a new handle during its creation. In that case winevulkan doesn't know the handle yet. I didn't want to introduce too much log spam, that's why it's WARN.
That makes sense. I wasn't sure if this was just theoretical, but if we're hitting this in intended use-cases then I agree that ERROR seems a bit too spammy.
- }
- }
- else
- {
- object_name_infos[i].objectHandle =
callback_data->pObjects[i].objectHandle;
- }
- }
- wine_callback_data.pObjects = object_name_infos;
- /* applications should always return VK_FALSE */
- object->user_callback(severity, message_types,
&wine_callback_data, object->user_data);
- heap_free(object_name_infos);
- return VK_FALSE;
The VK specification states:
The callback returns a VkBool32, which is interpreted in a
layer-specified manner. The application should always return VK_FALSE. The VK_TRUE value is reserved for use in layer development.
So I think we should return the value that user_callback returned to remain faithful to whomever registered a callback and what they returned.
+}
static void wine_vk_physical_device_free(struct VkPhysicalDevice_T
*phys_dev)
{ if (!phys_dev) @@ -391,6 +476,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;
- VkBaseOutStructure *header;
unsigned int i; VkResult res;
@@ -402,6 +489,22 @@ 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 = (VkBaseOutStructure *) 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;
- debug_utils_messenger->pfnUserCallback = (void *)
&debug_utils_callback_conversion;
- debug_utils_messenger->pUserData =
&object->utils_messengers[i];
I don't think we should be abusing VkBaseOutStructure like this as a way to circumvent const-ness of a read-only pNext chain. When we detect that there are VkDebugUtilsMessengerCreateInfoEXT structures in the pNext chain I think it would be cleaner to create a local copy and then use that however we need to.
I'm not really happy with this either, but convert_VkInstanceCreateInfo_struct_chain already copies the pNext chain. So nothing changed here will affect the application running in wine.
Yeah, we're already doing the copy here, I just don't think it makes sense to use VkBaseOutStructure here when VkBaseInStructure is more appropriate (given the structure is part of an input pNext chain).
- }
/* ICDs don't support any layers, so nothing to copy. Modern
versions of the loader
* filter this data out as well. */ @@ -425,6 +528,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; @@ -522,6 +629,8 @@ static void wine_vk_instance_free(struct
VkInstance_T *instance)
if (instance->instance) vk_funcs->p_vkDestroyInstance(instance->instance, NULL /*
allocator */);
- heap_free(instance->utils_messengers);
heap_free(instance); }
@@ -1607,6 +1716,10 @@ static uint64_t
unwrap_object_handle(VkObjectType type, uint64_t handle)
{ switch (type) {
- case VK_OBJECT_TYPE_INSTANCE:
- return (uint64_t) (uintptr_t) ((VkInstance) (uintptr_t)
handle)->instance;
- case VK_OBJECT_TYPE_PHYSICAL_DEVICE:
- return (uint64_t) (uintptr_t) ((VkPhysicalDevice)
(uintptr_t) handle)->phys_dev;
Were these meant to go into this commit? I can see why they're needed for debug_utils, but it just feels out of place to see them added in this commit.
I thought it wasn't worth a separate commit. Ideally we should auto-generate this function anyway to make adding new wrapped handles easier.
Yeah, as above. If you're planning on getting this in a separate commit in the series I think that would be fine also.
Thanks,
Liam Middlebrook
Thanks,
Georg Lehmann
Thanks,
Liam Middlebrook
case VK_OBJECT_TYPE_DEVICE: return (uint64_t) (uintptr_t) ((VkDevice) (uintptr_t)
handle)->device;
case VK_OBJECT_TYPE_QUEUE: @@ -1615,6 +1728,8 @@ static uint64_t
unwrap_object_handle(VkObjectType type, uint64_t handle)
return (uint64_t) (uintptr_t) ((VkCommandBuffer)
(uintptr_t) handle)->command_buffer;
case VK_OBJECT_TYPE_COMMAND_POOL: return (uint64_t)
wine_cmd_pool_from_handle(handle)->command_pool;
- case VK_OBJECT_TYPE_DEBUG_UTILS_MESSENGER_EXT:
- return (uint64_t)
wine_debug_utils_messenger_from_handle(handle)->debug_messenger;
default: return handle; } @@ -1685,6 +1800,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_HANDLE_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 =
unwrap_object_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 146bbd8fa4..17aa3835cf 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;