[PATCH 0/6] MR5274: winevulkan: Avoid double indirection in device/queue wrappers.
This saves one pointer indirection every time a device function is called from a device client handle. I have similar changes for other wrappers. One thing that could also be done, and which would then remove the need for the object_sizes metadata, is to move the unix struct definition within `struct VkDevice_T`, hiding the unixlib members from the PE side using `#ifdef WINE_UNIX_LIB` but I'm not sure it's much cleaner though, so maybe keeping the object sizes is better. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5274
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winevulkan/vulkan.c | 134 +++++++++++++++---------------- dlls/winevulkan/vulkan_private.h | 34 ++++---- 2 files changed, 83 insertions(+), 85 deletions(-) diff --git a/dlls/winevulkan/vulkan.c b/dlls/winevulkan/vulkan.c index c8cb525929d..543a7c1afa5 100644 --- a/dlls/winevulkan/vulkan.c +++ b/dlls/winevulkan/vulkan.c @@ -61,48 +61,53 @@ static uint32_t wine_vk_count_struct_(void *s, VkStructureType t) static const struct vulkan_funcs *vk_funcs; -#define WINE_VK_ADD_DISPATCHABLE_MAPPING(instance, client_handle, host_handle, object) \ - wine_vk_add_handle_mapping((instance), (uintptr_t)(client_handle), (uintptr_t)(host_handle), &(object)->mapping) -#define WINE_VK_ADD_NON_DISPATCHABLE_MAPPING(instance, client_handle, host_handle, object) \ - wine_vk_add_handle_mapping((instance), (uintptr_t)(client_handle), (host_handle), &(object)->mapping) -static void wine_vk_add_handle_mapping(struct wine_instance *instance, uint64_t wrapped_handle, - uint64_t host_handle, struct wine_vk_mapping *mapping) +static int wrapper_entry_compare(const void *key, const struct rb_entry *entry) +{ + struct wrapper_entry *wrapper = RB_ENTRY_VALUE(entry, struct wrapper_entry, entry); + const uint64_t *host_handle = key; + return *host_handle - wrapper->host_handle; +} + +static void add_handle_mapping(struct wine_instance *instance, uint64_t client_handle, + uint64_t host_handle, struct wrapper_entry *entry) { if (instance->enable_wrapper_list) { - mapping->host_handle = host_handle; - mapping->wine_wrapped_handle = wrapped_handle; + entry->host_handle = host_handle; + entry->client_handle = client_handle; + pthread_rwlock_wrlock(&instance->wrapper_lock); - list_add_tail(&instance->wrappers, &mapping->link); + rb_put(&instance->wrappers, &host_handle, &entry->entry); pthread_rwlock_unlock(&instance->wrapper_lock); } } -#define WINE_VK_REMOVE_HANDLE_MAPPING(instance, object) \ - wine_vk_remove_handle_mapping((instance), &(object)->mapping) -static void wine_vk_remove_handle_mapping(struct wine_instance *instance, struct wine_vk_mapping *mapping) +static void add_handle_mapping_ptr(struct wine_instance *instance, void *client_handle, + void *host_handle, struct wrapper_entry *entry) +{ + add_handle_mapping(instance, (uintptr_t)client_handle, (uintptr_t)host_handle, entry); +} + +static void remove_handle_mapping(struct wine_instance *instance, struct wrapper_entry *entry) { if (instance->enable_wrapper_list) { pthread_rwlock_wrlock(&instance->wrapper_lock); - list_remove(&mapping->link); + rb_remove(&instance->wrappers, &entry->entry); pthread_rwlock_unlock(&instance->wrapper_lock); } } -static uint64_t wine_vk_get_wrapper(struct wine_instance *instance, uint64_t host_handle) +static uint64_t client_handle_from_host(struct wine_instance *instance, uint64_t host_handle) { - struct wine_vk_mapping *mapping; + struct rb_entry *entry; uint64_t result = 0; pthread_rwlock_rdlock(&instance->wrapper_lock); - LIST_FOR_EACH_ENTRY(mapping, &instance->wrappers, struct wine_vk_mapping, link) + if ((entry = rb_get(&instance->wrappers, &host_handle))) { - if (mapping->host_handle == host_handle) - { - result = mapping->wine_wrapped_handle; - break; - } + struct wrapper_entry *wrapper = RB_ENTRY_VALUE(entry, struct wrapper_entry, entry); + result = wrapper->client_handle; } pthread_rwlock_unlock(&instance->wrapper_lock); return result; @@ -148,7 +153,7 @@ static VkBool32 debug_utils_callback_conversion(VkDebugUtilsMessageSeverityFlagB 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); + object_name_infos[i].objectHandle = client_handle_from_host(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)); @@ -201,7 +206,7 @@ static VkBool32 debug_report_callback_conversion(VkDebugReportFlagsEXT flags, Vk params.layer_prefix = layer_prefix; params.message = message; - params.object_handle = wine_vk_get_wrapper(object->instance, object_handle); + params.object_handle = client_handle_from_host(object->instance, object_handle); if (!params.object_handle) params.object_type = VK_DEBUG_REPORT_OBJECT_TYPE_UNKNOWN_EXT; @@ -215,7 +220,7 @@ static void wine_vk_physical_device_free(struct wine_phys_dev *phys_dev) if (!phys_dev) return; - WINE_VK_REMOVE_HANDLE_MAPPING(phys_dev->instance, phys_dev); + remove_handle_mapping(phys_dev->instance, &phys_dev->wrapper_entry); free(phys_dev->extensions); free(phys_dev); } @@ -239,7 +244,7 @@ static struct wine_phys_dev *wine_vk_physical_device_alloc(struct wine_instance object->host_physical_device = phys_dev; handle->base.unix_handle = (uintptr_t)object; - WINE_VK_ADD_DISPATCHABLE_MAPPING(instance, handle, phys_dev, object); + add_handle_mapping_ptr(instance, handle, phys_dev, &object->wrapper_entry); instance->funcs.p_vkGetPhysicalDeviceMemoryProperties(phys_dev, &object->memory_properties); @@ -378,7 +383,7 @@ static void wine_vk_free_command_buffers(struct wine_device *device, device->funcs.p_vkFreeCommandBuffers(device->host_device, pool->host_command_pool, 1, &buffer->host_command_buffer); - WINE_VK_REMOVE_HANDLE_MAPPING(device->phys_dev->instance, buffer); + remove_handle_mapping(device->phys_dev->instance, &buffer->wrapper_entry); buffer->handle->base.unix_handle = 0; free(buffer); } @@ -421,7 +426,7 @@ static void wine_vk_device_get_queues(struct wine_device *device, } queue->handle->base.unix_handle = (uintptr_t)queue; - WINE_VK_ADD_DISPATCHABLE_MAPPING(device->phys_dev->instance, queue->handle, queue->host_queue, queue); + add_handle_mapping_ptr(device->phys_dev->instance, queue->handle, queue->host_queue, &queue->wrapper_entry); TRACE("Got device %p queue %p, host_queue %p.\n", device, queue, queue->host_queue); } @@ -516,7 +521,7 @@ static void wine_vk_device_free(struct wine_device *device) { queue = &device->queues[i]; if (queue && queue->host_queue) - WINE_VK_REMOVE_HANDLE_MAPPING(device->phys_dev->instance, queue); + remove_handle_mapping(device->phys_dev->instance, &queue->wrapper_entry); } free(device->queues); device->queues = NULL; @@ -524,7 +529,7 @@ static void wine_vk_device_free(struct wine_device *device) if (device->host_device && device->funcs.p_vkDestroyDevice) { - WINE_VK_REMOVE_HANDLE_MAPPING(device->phys_dev->instance, device); + remove_handle_mapping(device->phys_dev->instance, &device->wrapper_entry); device->funcs.p_vkDestroyDevice(device->host_device, NULL /* pAllocator */); } @@ -738,10 +743,11 @@ static void wine_vk_instance_free(struct wine_instance *instance) if (instance->host_instance) { vk_funcs->p_vkDestroyInstance(instance->host_instance, NULL /* allocator */); - WINE_VK_REMOVE_HANDLE_MAPPING(instance, instance); + remove_handle_mapping(instance, &instance->wrapper_entry); } pthread_rwlock_destroy(&instance->wrapper_lock); + rb_destroy(&instance->wrappers, NULL, NULL); free(instance->utils_messengers); free(instance); @@ -783,7 +789,7 @@ VkResult wine_vkAllocateCommandBuffers(VkDevice handle, const VkCommandBufferAll res = device->funcs.p_vkAllocateCommandBuffers(device->host_device, &allocate_info_host, &buffer->host_command_buffer); buffer->handle->base.unix_handle = (uintptr_t)buffer; - WINE_VK_ADD_DISPATCHABLE_MAPPING(device->phys_dev->instance, buffer->handle, buffer->host_command_buffer, buffer); + add_handle_mapping_ptr(device->phys_dev->instance, buffer->handle, buffer->host_command_buffer, &buffer->wrapper_entry); if (res != VK_SUCCESS) { ERR("Failed to allocate command buffer, res=%d.\n", res); @@ -838,7 +844,7 @@ VkResult wine_vkCreateDevice(VkPhysicalDevice phys_dev_handle, const VkDeviceCre res = instance->funcs.p_vkCreateDevice(phys_dev->host_physical_device, &create_info_host, NULL /* allocator */, &object->host_device); free_conversion_context(&ctx); - WINE_VK_ADD_DISPATCHABLE_MAPPING(instance, device_handle, object->host_device, object); + add_handle_mapping_ptr(instance, device_handle, object->host_device, &object->wrapper_entry); if (res != VK_SUCCESS) { WARN("Failed to create device, res=%d.\n", res); @@ -914,7 +920,7 @@ VkResult wine_vkCreateInstance(const VkInstanceCreateInfo *create_info, ERR("Failed to allocate memory for instance\n"); return VK_ERROR_OUT_OF_HOST_MEMORY; } - list_init(&object->wrappers); + rb_init(&object->wrappers, wrapper_entry_compare); pthread_rwlock_init(&object->wrapper_lock, NULL); init_conversion_context(&ctx); @@ -930,7 +936,7 @@ VkResult wine_vkCreateInstance(const VkInstanceCreateInfo *create_info, } object->handle = client_instance; - WINE_VK_ADD_DISPATCHABLE_MAPPING(object, object->handle, object->host_instance, object); + add_handle_mapping_ptr(object, object->handle, object->host_instance, &object->wrapper_entry); /* Load all instance functions we are aware of. Note the loader takes care * of any filtering for extensions which were not requested, but which the @@ -1195,21 +1201,18 @@ VkResult wine_vkCreateCommandPool(VkDevice device_handle, const VkCommandPoolCre return VK_ERROR_OUT_OF_HOST_MEMORY; res = device->funcs.p_vkCreateCommandPool(device->host_device, info, NULL, &object->host_command_pool); - - if (res == VK_SUCCESS) - { - object->handle = (uintptr_t)handle; - handle->unix_handle = (uintptr_t)object; - WINE_VK_ADD_NON_DISPATCHABLE_MAPPING(device->phys_dev->instance, object->handle, - object->host_command_pool, object); - *command_pool = object->handle; - } - else + if (res != VK_SUCCESS) { free(object); + return res; } - return res; + object->handle = (uintptr_t)handle; + handle->unix_handle = (uintptr_t)object; + + *command_pool = object->handle; + add_handle_mapping(device->phys_dev->instance, *command_pool, object->host_command_pool, &object->wrapper_entry); + return VK_SUCCESS; } void wine_vkDestroyCommandPool(VkDevice device_handle, VkCommandPool handle, @@ -1221,8 +1224,7 @@ void wine_vkDestroyCommandPool(VkDevice device_handle, VkCommandPool handle, if (allocator) FIXME("Support for allocation callbacks not implemented yet\n"); - WINE_VK_REMOVE_HANDLE_MAPPING(device->phys_dev->instance, pool); - + remove_handle_mapping(device->phys_dev->instance, &pool->wrapper_entry); device->funcs.p_vkDestroyCommandPool(device->host_device, pool->host_command_pool, NULL); free(pool); } @@ -1604,13 +1606,10 @@ VkResult wine_vkCreateWin32SurfaceKHR(VkInstance handle, const VkWin32SurfaceCre } object->host_surface = vk_funcs->p_wine_get_host_surface(object->driver_surface); - WINE_VK_ADD_NON_DISPATCHABLE_MAPPING(instance, object, object->host_surface, object); - - *surface = wine_surface_to_handle(object); if (dummy) NtUserDestroyWindow(dummy); *surface = wine_surface_to_handle(object); - + add_handle_mapping(instance, *surface, object->host_surface, &object->wrapper_entry); return VK_SUCCESS; } @@ -1624,7 +1623,7 @@ void wine_vkDestroySurfaceKHR(VkInstance handle, VkSurfaceKHR surface, return; instance->funcs.p_vkDestroySurfaceKHR(instance->host_instance, object->driver_surface, NULL); - WINE_VK_REMOVE_HANDLE_MAPPING(instance, object); + remove_handle_mapping(instance, &object->wrapper_entry); free(object); } @@ -1666,10 +1665,9 @@ VkResult wine_vkCreateSwapchainKHR(VkDevice device_handle, const VkSwapchainCrea return res; } - WINE_VK_ADD_NON_DISPATCHABLE_MAPPING(device->phys_dev->instance, object, object->host_swapchain, object); *swapchain_handle = wine_swapchain_to_handle(object); - - return res; + add_handle_mapping(instance, *swapchain_handle, object->host_swapchain, &object->wrapper_entry); + return VK_SUCCESS; } void wine_vkDestroySwapchainKHR(VkDevice device_handle, VkSwapchainKHR swapchain_handle, @@ -1682,7 +1680,7 @@ void wine_vkDestroySwapchainKHR(VkDevice device_handle, VkSwapchainKHR swapchain if (!swapchain) return; device->funcs.p_vkDestroySwapchainKHR(device->host_device, swapchain->host_swapchain, NULL); - WINE_VK_REMOVE_HANDLE_MAPPING(device->phys_dev->instance, swapchain); + remove_handle_mapping(device->phys_dev->instance, &swapchain->wrapper_entry); free(swapchain); } @@ -1776,10 +1774,11 @@ VkResult wine_vkAllocateMemory(VkDevice handle, const VkMemoryAllocateInfo *allo return result; } - WINE_VK_ADD_NON_DISPATCHABLE_MAPPING(device->phys_dev->instance, memory, memory->host_memory, memory); memory->size = info.allocationSize; memory->vm_map = mapping; + *ret = (VkDeviceMemory)(uintptr_t)memory; + add_handle_mapping(device->phys_dev->instance, *ret, memory->host_memory, &memory->wrapper_entry); return VK_SUCCESS; } @@ -1803,7 +1802,7 @@ void wine_vkFreeMemory(VkDevice handle, VkDeviceMemory memory_handle, const VkAl device->funcs.p_vkUnmapMemory2KHR(device->host_device, &info); } - WINE_VK_REMOVE_HANDLE_MAPPING(device->phys_dev->instance, memory); + remove_handle_mapping(device->phys_dev->instance, &memory->wrapper_entry); device->funcs.p_vkFreeMemory(device->host_device, memory->host_memory, NULL); if (memory->vm_map) @@ -2155,9 +2154,8 @@ VkResult wine_vkCreateDebugUtilsMessengerEXT(VkInstance handle, return res; } - WINE_VK_ADD_NON_DISPATCHABLE_MAPPING(instance, object, object->host_debug_messenger, object); *messenger = wine_debug_utils_messenger_to_handle(object); - + add_handle_mapping(instance, *messenger, object->host_debug_messenger, &object->wrapper_entry); return VK_SUCCESS; } @@ -2173,7 +2171,7 @@ void wine_vkDestroyDebugUtilsMessengerEXT(VkInstance handle, VkDebugUtilsMesseng return; instance->funcs.p_vkDestroyDebugUtilsMessengerEXT(instance->host_instance, object->host_debug_messenger, NULL); - WINE_VK_REMOVE_HANDLE_MAPPING(instance, object); + remove_handle_mapping(instance, &object->wrapper_entry); free(object); } @@ -2211,9 +2209,8 @@ VkResult wine_vkCreateDebugReportCallbackEXT(VkInstance handle, return res; } - WINE_VK_ADD_NON_DISPATCHABLE_MAPPING(instance, object, object->host_debug_callback, object); *callback = wine_debug_report_callback_to_handle(object); - + add_handle_mapping(instance, *callback, object->host_debug_callback, &object->wrapper_entry); return VK_SUCCESS; } @@ -2229,14 +2226,14 @@ void wine_vkDestroyDebugReportCallbackEXT(VkInstance handle, VkDebugReportCallba return; instance->funcs.p_vkDestroyDebugReportCallbackEXT(instance->host_instance, object->host_debug_callback, NULL); - WINE_VK_REMOVE_HANDLE_MAPPING(instance, object); + remove_handle_mapping(instance, &object->wrapper_entry); free(object); } VkResult wine_vkCreateDeferredOperationKHR(VkDevice handle, const VkAllocationCallbacks* allocator, - VkDeferredOperationKHR* deferredOperation) + VkDeferredOperationKHR* operation) { struct wine_device *device = wine_device_from_handle(handle); struct wine_deferred_operation *object; @@ -2258,9 +2255,8 @@ VkResult wine_vkCreateDeferredOperationKHR(VkDevice handle, init_conversion_context(&object->ctx); - WINE_VK_ADD_NON_DISPATCHABLE_MAPPING(device->phys_dev->instance, object, object->host_deferred_operation, object); - *deferredOperation = wine_deferred_operation_to_handle(object); - + *operation = wine_deferred_operation_to_handle(object); + add_handle_mapping(device->phys_dev->instance, *operation, object->host_deferred_operation, &object->wrapper_entry); return VK_SUCCESS; } @@ -2277,7 +2273,7 @@ void wine_vkDestroyDeferredOperationKHR(VkDevice handle, return; device->funcs.p_vkDestroyDeferredOperationKHR(device->host_device, object->host_deferred_operation, NULL); - WINE_VK_REMOVE_HANDLE_MAPPING(device->phys_dev->instance, object); + remove_handle_mapping(device->phys_dev->instance, &object->wrapper_entry); free_conversion_context(&object->ctx); free(object); diff --git a/dlls/winevulkan/vulkan_private.h b/dlls/winevulkan/vulkan_private.h index 94eecc5e5d5..7b1e24f3e56 100644 --- a/dlls/winevulkan/vulkan_private.h +++ b/dlls/winevulkan/vulkan_private.h @@ -28,14 +28,16 @@ #include "vulkan_loader.h" #include "vulkan_thunks.h" +#include "wine/rbtree.h" + /* Some extensions have callbacks for those we need to be able to * get the wine wrapper for a host handle */ -struct wine_vk_mapping +struct wrapper_entry { - struct list link; + struct rb_entry entry; uint64_t host_handle; - uint64_t wine_wrapped_handle; + uint64_t client_handle; }; struct wine_cmd_buffer @@ -45,7 +47,7 @@ struct wine_cmd_buffer VkCommandBuffer handle; /* client command buffer */ VkCommandBuffer host_command_buffer; - struct wine_vk_mapping mapping; + struct wrapper_entry wrapper_entry; }; static inline struct wine_cmd_buffer *wine_cmd_buffer_from_handle(VkCommandBuffer handle) @@ -64,7 +66,7 @@ struct wine_device struct wine_queue *queues; uint32_t queue_count; - struct wine_vk_mapping mapping; + struct wrapper_entry wrapper_entry; }; static inline struct wine_device *wine_device_from_handle(VkDevice handle) @@ -83,7 +85,7 @@ struct wine_debug_report_callback PFN_vkDebugReportCallbackEXT user_callback; void *user_data; - struct wine_vk_mapping mapping; + struct wrapper_entry wrapper_entry; }; struct wine_instance @@ -101,7 +103,7 @@ struct wine_instance VkBool32 enable_win32_surface; VkBool32 enable_wrapper_list; - struct list wrappers; + struct rb_tree wrappers; pthread_rwlock_t wrapper_lock; struct wine_debug_utils_messenger *utils_messengers; @@ -111,7 +113,7 @@ struct wine_instance unsigned int quirks; - struct wine_vk_mapping mapping; + struct wrapper_entry wrapper_entry; }; static inline struct wine_instance *wine_instance_from_handle(VkInstance handle) @@ -133,7 +135,7 @@ struct wine_phys_dev uint32_t external_memory_align; uint32_t map_placed_align; - struct wine_vk_mapping mapping; + struct wrapper_entry wrapper_entry; }; static inline struct wine_phys_dev *wine_phys_dev_from_handle(VkPhysicalDevice handle) @@ -152,7 +154,7 @@ struct wine_queue uint32_t queue_index; VkDeviceQueueCreateFlags flags; - struct wine_vk_mapping mapping; + struct wrapper_entry wrapper_entry; }; static inline struct wine_queue *wine_queue_from_handle(VkQueue handle) @@ -165,7 +167,7 @@ struct wine_cmd_pool VkCommandPool handle; VkCommandPool host_command_pool; - struct wine_vk_mapping mapping; + struct wrapper_entry wrapper_entry; }; static inline struct wine_cmd_pool *wine_cmd_pool_from_handle(VkCommandPool handle) @@ -180,7 +182,7 @@ struct wine_device_memory VkDeviceSize size; void *vm_map; - struct wine_vk_mapping mapping; + struct wrapper_entry wrapper_entry; }; static inline struct wine_device_memory *wine_device_memory_from_handle(VkDeviceMemory handle) @@ -197,7 +199,7 @@ struct wine_debug_utils_messenger PFN_vkDebugUtilsMessengerCallbackEXT user_callback; void *user_data; - struct wine_vk_mapping mapping; + struct wrapper_entry wrapper_entry; }; static inline struct wine_debug_utils_messenger *wine_debug_utils_messenger_from_handle( @@ -230,7 +232,7 @@ struct wine_surface VkSurfaceKHR driver_surface; HWND hwnd; - struct wine_vk_mapping mapping; + struct wrapper_entry wrapper_entry; }; static inline struct wine_surface *wine_surface_from_handle(VkSurfaceKHR handle) @@ -247,7 +249,7 @@ struct wine_swapchain { VkSwapchainKHR host_swapchain; - struct wine_vk_mapping mapping; + struct wrapper_entry wrapper_entry; }; static inline struct wine_swapchain *wine_swapchain_from_handle(VkSwapchainKHR handle) @@ -314,7 +316,7 @@ struct wine_deferred_operation { VkDeferredOperationKHR host_deferred_operation; struct conversion_context ctx; /* to keep params alive. */ - struct wine_vk_mapping mapping; + struct wrapper_entry wrapper_entry; }; static inline struct wine_deferred_operation *wine_deferred_operation_from_handle( -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5274
From: Rémi Bernon <rbernon(a)codeweavers.com> We don't yet have mappings when objects are constructed, so this also removes them consistently before destroying the objects. --- dlls/winevulkan/vulkan.c | 97 ++++++++++++++++++++++++---------------- 1 file changed, 58 insertions(+), 39 deletions(-) diff --git a/dlls/winevulkan/vulkan.c b/dlls/winevulkan/vulkan.c index 543a7c1afa5..a2112765008 100644 --- a/dlls/winevulkan/vulkan.c +++ b/dlls/winevulkan/vulkan.c @@ -220,7 +220,6 @@ static void wine_vk_physical_device_free(struct wine_phys_dev *phys_dev) if (!phys_dev) return; - remove_handle_mapping(phys_dev->instance, &phys_dev->wrapper_entry); free(phys_dev->extensions); free(phys_dev); } @@ -244,7 +243,6 @@ static struct wine_phys_dev *wine_vk_physical_device_alloc(struct wine_instance object->host_physical_device = phys_dev; handle->base.unix_handle = (uintptr_t)object; - add_handle_mapping_ptr(instance, handle, phys_dev, &object->wrapper_entry); instance->funcs.p_vkGetPhysicalDeviceMemoryProperties(phys_dev, &object->memory_properties); @@ -383,7 +381,6 @@ static void wine_vk_free_command_buffers(struct wine_device *device, device->funcs.p_vkFreeCommandBuffers(device->host_device, pool->host_command_pool, 1, &buffer->host_command_buffer); - remove_handle_mapping(device->phys_dev->instance, &buffer->wrapper_entry); buffer->handle->base.unix_handle = 0; free(buffer); } @@ -426,8 +423,6 @@ static void wine_vk_device_get_queues(struct wine_device *device, } queue->handle->base.unix_handle = (uintptr_t)queue; - add_handle_mapping_ptr(device->phys_dev->instance, queue->handle, queue->host_queue, &queue->wrapper_entry); - TRACE("Got device %p queue %p, host_queue %p.\n", device, queue, queue->host_queue); } } @@ -509,27 +504,14 @@ static VkResult wine_vk_device_convert_create_info(struct wine_phys_dev *phys_de */ static void wine_vk_device_free(struct wine_device *device) { - struct wine_queue *queue; - - if (!device) - return; - if (device->queues) { - unsigned int i; - for (i = 0; i < device->queue_count; i++) - { - queue = &device->queues[i]; - if (queue && queue->host_queue) - remove_handle_mapping(device->phys_dev->instance, &queue->wrapper_entry); - } free(device->queues); device->queues = NULL; } if (device->host_device && device->funcs.p_vkDestroyDevice) { - remove_handle_mapping(device->phys_dev->instance, &device->wrapper_entry); device->funcs.p_vkDestroyDevice(device->host_device, NULL /* pAllocator */); } @@ -734,17 +716,12 @@ static void wine_vk_instance_free(struct wine_instance *instance) unsigned int i; for (i = 0; i < instance->phys_dev_count; i++) - { wine_vk_physical_device_free(instance->phys_devs[i]); - } free(instance->phys_devs); } if (instance->host_instance) - { vk_funcs->p_vkDestroyInstance(instance->host_instance, NULL /* allocator */); - remove_handle_mapping(instance, &instance->wrapper_entry); - } pthread_rwlock_destroy(&instance->wrapper_lock); rb_destroy(&instance->wrappers, NULL, NULL); @@ -789,7 +766,6 @@ VkResult wine_vkAllocateCommandBuffers(VkDevice handle, const VkCommandBufferAll res = device->funcs.p_vkAllocateCommandBuffers(device->host_device, &allocate_info_host, &buffer->host_command_buffer); buffer->handle->base.unix_handle = (uintptr_t)buffer; - add_handle_mapping_ptr(device->phys_dev->instance, buffer->handle, buffer->host_command_buffer, &buffer->wrapper_entry); if (res != VK_SUCCESS) { ERR("Failed to allocate command buffer, res=%d.\n", res); @@ -799,9 +775,17 @@ VkResult wine_vkAllocateCommandBuffers(VkDevice handle, const VkCommandBufferAll } if (res != VK_SUCCESS) + { wine_vk_free_command_buffers(device, pool, i + 1, buffers); + return res; + } - return res; + for (i = 0; i < allocate_info->commandBufferCount; i++) + { + struct wine_cmd_buffer *buffer = wine_cmd_buffer_from_handle(buffers[i]); + add_handle_mapping_ptr(device->phys_dev->instance, buffer->handle, buffer->host_command_buffer, &buffer->wrapper_entry); + } + return VK_SUCCESS; } VkResult wine_vkCreateDevice(VkPhysicalDevice phys_dev_handle, const VkDeviceCreateInfo *create_info, @@ -844,7 +828,6 @@ VkResult wine_vkCreateDevice(VkPhysicalDevice phys_dev_handle, const VkDeviceCre res = instance->funcs.p_vkCreateDevice(phys_dev->host_physical_device, &create_info_host, NULL /* allocator */, &object->host_device); free_conversion_context(&ctx); - add_handle_mapping_ptr(instance, device_handle, object->host_device, &object->wrapper_entry); if (res != VK_SUCCESS) { WARN("Failed to create device, res=%d.\n", res); @@ -891,9 +874,16 @@ VkResult wine_vkCreateDevice(VkPhysicalDevice phys_dev_handle, const VkDeviceCre device_handle->quirks = instance->quirks; device_handle->base.unix_handle = (uintptr_t)object; - *ret_device = device_handle; TRACE("Created device %p, host_device %p.\n", object, object->host_device); + for (i = 0; i < object->queue_count; i++) + { + struct wine_queue *queue = object->queues + i; + add_handle_mapping_ptr(instance, queue->handle, queue->host_queue, &queue->wrapper_entry); + } + + *ret_device = device_handle; + add_handle_mapping_ptr(instance, *ret_device, object->host_device, &object->wrapper_entry); return VK_SUCCESS; fail: @@ -910,6 +900,7 @@ VkResult wine_vkCreateInstance(const VkInstanceCreateInfo *create_info, const VkApplicationInfo *app_info; struct conversion_context ctx; struct wine_instance *object; + unsigned int i; VkResult res; if (allocator) @@ -936,7 +927,6 @@ VkResult wine_vkCreateInstance(const VkInstanceCreateInfo *create_info, } object->handle = client_instance; - add_handle_mapping_ptr(object, object->handle, object->host_instance, &object->wrapper_entry); /* Load all instance functions we are aware of. Note the loader takes care * of any filtering for extensions which were not requested, but which the @@ -973,18 +963,35 @@ VkResult wine_vkCreateInstance(const VkInstanceCreateInfo *create_info, } client_instance->base.unix_handle = (uintptr_t)object; - *instance = client_instance; + TRACE("Created instance %p, host_instance %p.\n", object, object->host_instance); + for (i = 0; i < object->phys_dev_count; i++) + { + struct wine_phys_dev *phys_dev = object->phys_devs[i]; + add_handle_mapping_ptr(object, phys_dev->handle, phys_dev->host_physical_device, &phys_dev->wrapper_entry); + } + *instance = client_instance; + add_handle_mapping_ptr(object, *instance, object->host_instance, &object->wrapper_entry); return VK_SUCCESS; } void wine_vkDestroyDevice(VkDevice handle, const VkAllocationCallbacks *allocator) { struct wine_device *device = wine_device_from_handle(handle); + unsigned int i; if (allocator) FIXME("Support for allocation callbacks not implemented yet\n"); + if (!device) + return; + + for (i = 0; i < device->queue_count; i++) + { + struct wine_queue *queue = device->queues + i; + remove_handle_mapping(device->phys_dev->instance, &queue->wrapper_entry); + } + remove_handle_mapping(device->phys_dev->instance, &device->wrapper_entry); wine_vk_device_free(device); } @@ -992,9 +999,19 @@ void wine_vkDestroyDevice(VkDevice handle, const VkAllocationCallbacks *allocato void wine_vkDestroyInstance(VkInstance handle, const VkAllocationCallbacks *allocator) { struct wine_instance *instance = wine_instance_from_handle(handle); + unsigned int i; if (allocator) FIXME("Support allocation allocators\n"); + if (!instance) + return; + + for (i = 0; i < instance->phys_dev_count; i++) + { + struct wine_phys_dev *phys_dev = instance->phys_devs[i]; + remove_handle_mapping(phys_dev->instance, &phys_dev->wrapper_entry); + } + remove_handle_mapping(instance, &instance->wrapper_entry); wine_vk_instance_free(instance); } @@ -1138,6 +1155,13 @@ void wine_vkFreeCommandBuffers(VkDevice handle, VkCommandPool command_pool, uint { struct wine_device *device = wine_device_from_handle(handle); struct wine_cmd_pool *pool = wine_cmd_pool_from_handle(command_pool); + unsigned int i; + + for (i = 0; i < count; i++) + { + struct wine_cmd_buffer *buffer = wine_cmd_buffer_from_handle(buffers[i]); + if (buffer) remove_handle_mapping(device->phys_dev->instance, &buffer->wrapper_entry); + } wine_vk_free_command_buffers(device, pool, count, buffers); } @@ -1622,9 +1646,8 @@ void wine_vkDestroySurfaceKHR(VkInstance handle, VkSurfaceKHR surface, if (!object) return; - instance->funcs.p_vkDestroySurfaceKHR(instance->host_instance, object->driver_surface, NULL); remove_handle_mapping(instance, &object->wrapper_entry); - + instance->funcs.p_vkDestroySurfaceKHR(instance->host_instance, object->driver_surface, NULL); free(object); } @@ -1679,9 +1702,8 @@ void wine_vkDestroySwapchainKHR(VkDevice device_handle, VkSwapchainKHR swapchain if (allocator) FIXME("Support for allocation callbacks not implemented yet\n"); if (!swapchain) return; - device->funcs.p_vkDestroySwapchainKHR(device->host_device, swapchain->host_swapchain, NULL); remove_handle_mapping(device->phys_dev->instance, &swapchain->wrapper_entry); - + device->funcs.p_vkDestroySwapchainKHR(device->host_device, swapchain->host_swapchain, NULL); free(swapchain); } @@ -2170,9 +2192,8 @@ void wine_vkDestroyDebugUtilsMessengerEXT(VkInstance handle, VkDebugUtilsMesseng if (!object) return; - instance->funcs.p_vkDestroyDebugUtilsMessengerEXT(instance->host_instance, object->host_debug_messenger, NULL); remove_handle_mapping(instance, &object->wrapper_entry); - + instance->funcs.p_vkDestroyDebugUtilsMessengerEXT(instance->host_instance, object->host_debug_messenger, NULL); free(object); } @@ -2225,9 +2246,8 @@ void wine_vkDestroyDebugReportCallbackEXT(VkInstance handle, VkDebugReportCallba if (!object) return; - instance->funcs.p_vkDestroyDebugReportCallbackEXT(instance->host_instance, object->host_debug_callback, NULL); remove_handle_mapping(instance, &object->wrapper_entry); - + instance->funcs.p_vkDestroyDebugReportCallbackEXT(instance->host_instance, object->host_debug_callback, NULL); free(object); } @@ -2272,9 +2292,8 @@ void wine_vkDestroyDeferredOperationKHR(VkDevice handle, if (!object) return; - device->funcs.p_vkDestroyDeferredOperationKHR(device->host_device, object->host_deferred_operation, NULL); remove_handle_mapping(device->phys_dev->instance, &object->wrapper_entry); - + device->funcs.p_vkDestroyDeferredOperationKHR(device->host_device, object->host_deferred_operation, NULL); free_conversion_context(&object->ctx); free(object); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5274
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winevulkan/vulkan.c | 35 ++++++++--------------------------- 1 file changed, 8 insertions(+), 27 deletions(-) diff --git a/dlls/winevulkan/vulkan.c b/dlls/winevulkan/vulkan.c index a2112765008..9631b3a0eab 100644 --- a/dlls/winevulkan/vulkan.c +++ b/dlls/winevulkan/vulkan.c @@ -499,25 +499,6 @@ static VkResult wine_vk_device_convert_create_info(struct wine_phys_dev *phys_de return VK_SUCCESS; } -/* Helper function used for freeing a device structure. This function supports full - * and partial object cleanups and can thus be used for vkCreateDevice failures. - */ -static void wine_vk_device_free(struct wine_device *device) -{ - if (device->queues) - { - free(device->queues); - device->queues = NULL; - } - - if (device->host_device && device->funcs.p_vkDestroyDevice) - { - device->funcs.p_vkDestroyDevice(device->host_device, NULL /* pAllocator */); - } - - free(device); -} - NTSTATUS init_vulkan(void *args) { vk_funcs = __wine_get_vulkan_driver(WINE_VULKAN_DRIVER_VERSION); @@ -831,7 +812,8 @@ VkResult wine_vkCreateDevice(VkPhysicalDevice phys_dev_handle, const VkDeviceCre if (res != VK_SUCCESS) { WARN("Failed to create device, res=%d.\n", res); - goto fail; + free(object); + return res; } /* Just load all function pointers we are aware off. The loader takes care of filtering. @@ -854,8 +836,9 @@ VkResult wine_vkCreateDevice(VkPhysicalDevice phys_dev_handle, const VkDeviceCre if (!(object->queues = calloc(object->queue_count, sizeof(*object->queues)))) { - res = VK_ERROR_OUT_OF_HOST_MEMORY; - goto fail; + object->funcs.p_vkDestroyDevice(object->host_device, NULL /* pAllocator */); + free(object); + return VK_ERROR_OUT_OF_HOST_MEMORY; } next_queue = object->queues; @@ -885,10 +868,6 @@ VkResult wine_vkCreateDevice(VkPhysicalDevice phys_dev_handle, const VkDeviceCre *ret_device = device_handle; add_handle_mapping_ptr(instance, *ret_device, object->host_device, &object->wrapper_entry); return VK_SUCCESS; - -fail: - wine_vk_device_free(object); - return res; } VkResult wine_vkCreateInstance(const VkInstanceCreateInfo *create_info, @@ -993,7 +972,9 @@ void wine_vkDestroyDevice(VkDevice handle, const VkAllocationCallbacks *allocato } remove_handle_mapping(device->phys_dev->instance, &device->wrapper_entry); - wine_vk_device_free(device); + device->funcs.p_vkDestroyDevice(device->host_device, NULL /* pAllocator */); + free(device->queues); + free(device); } void wine_vkDestroyInstance(VkInstance handle, const VkAllocationCallbacks *allocator) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5274
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winevulkan/vulkan.c | 25 +++++------------ dlls/winevulkan/vulkan_private.h | 46 +++++++++++++++++--------------- 2 files changed, 31 insertions(+), 40 deletions(-) diff --git a/dlls/winevulkan/vulkan.c b/dlls/winevulkan/vulkan.c index 9631b3a0eab..66821ce18f4 100644 --- a/dlls/winevulkan/vulkan.c +++ b/dlls/winevulkan/vulkan.c @@ -781,7 +781,7 @@ VkResult wine_vkCreateDevice(VkPhysicalDevice phys_dev_handle, const VkDeviceCre struct wine_queue *next_queue; struct conversion_context ctx; struct wine_device *object; - unsigned int i; + unsigned int queue_count, i; VkResult res; if (allocator) @@ -798,7 +798,11 @@ VkResult wine_vkCreateDevice(VkPhysicalDevice phys_dev_handle, const VkDeviceCre TRACE("Driver version: %#x.\n", properties.driverVersion); } - if (!(object = calloc(1, sizeof(*object)))) + /* We need to cache all queues within the device as each requires wrapping since queues are dispatchable objects. */ + for (queue_count = 0, i = 0; i < create_info->queueCreateInfoCount; i++) + queue_count += create_info->pQueueCreateInfos[i].queueCount; + + if (!(object = calloc(1, offsetof(struct wine_device, queues[queue_count])))) return VK_ERROR_OUT_OF_HOST_MEMORY; object->phys_dev = phys_dev; @@ -826,21 +830,6 @@ VkResult wine_vkCreateDevice(VkPhysicalDevice phys_dev_handle, const VkDeviceCre ALL_VK_DEVICE_FUNCS() #undef USE_VK_FUNC - /* We need to cache all queues within the device as each requires wrapping since queues are - * dispatchable objects. - */ - for (i = 0; i < create_info_host.queueCreateInfoCount; i++) - { - object->queue_count += create_info_host.pQueueCreateInfos[i].queueCount; - } - - if (!(object->queues = calloc(object->queue_count, sizeof(*object->queues)))) - { - object->funcs.p_vkDestroyDevice(object->host_device, NULL /* pAllocator */); - free(object); - return VK_ERROR_OUT_OF_HOST_MEMORY; - } - next_queue = object->queues; queue_handles = device_handle->queues; for (i = 0; i < create_info_host.queueCreateInfoCount; i++) @@ -854,6 +843,7 @@ VkResult wine_vkCreateDevice(VkPhysicalDevice phys_dev_handle, const VkDeviceCre wine_vk_device_get_queues(object, family_index, queue_count, flags, next_queue, &queue_handles); next_queue += queue_count; } + object->queue_count = next_queue - object->queues; device_handle->quirks = instance->quirks; device_handle->base.unix_handle = (uintptr_t)object; @@ -973,7 +963,6 @@ void wine_vkDestroyDevice(VkDevice handle, const VkAllocationCallbacks *allocato remove_handle_mapping(device->phys_dev->instance, &device->wrapper_entry); device->funcs.p_vkDestroyDevice(device->host_device, NULL /* pAllocator */); - free(device->queues); free(device); } diff --git a/dlls/winevulkan/vulkan_private.h b/dlls/winevulkan/vulkan_private.h index 7b1e24f3e56..31e9e981168 100644 --- a/dlls/winevulkan/vulkan_private.h +++ b/dlls/winevulkan/vulkan_private.h @@ -55,6 +55,25 @@ static inline struct wine_cmd_buffer *wine_cmd_buffer_from_handle(VkCommandBuffe return (struct wine_cmd_buffer *)(uintptr_t)handle->base.unix_handle; } +struct wine_queue +{ + struct wine_device *device; /* parent */ + + VkQueue handle; /* client queue */ + VkQueue host_queue; + + uint32_t family_index; + uint32_t queue_index; + VkDeviceQueueCreateFlags flags; + + struct wrapper_entry wrapper_entry; +}; + +static inline struct wine_queue *wine_queue_from_handle(VkQueue handle) +{ + return (struct wine_queue *)(uintptr_t)handle->base.unix_handle; +} + struct wine_device { struct vulkan_device_funcs funcs; @@ -63,12 +82,14 @@ struct wine_device VkDevice handle; /* client device */ VkDevice host_device; - struct wine_queue *queues; - uint32_t queue_count; - struct wrapper_entry wrapper_entry; + + uint32_t queue_count; + struct wine_queue queues[]; }; +C_ASSERT(sizeof(struct wine_device) == offsetof(struct wine_device, queues[0])); + static inline struct wine_device *wine_device_from_handle(VkDevice handle) { return (struct wine_device *)(uintptr_t)handle->base.unix_handle; @@ -143,25 +164,6 @@ static inline struct wine_phys_dev *wine_phys_dev_from_handle(VkPhysicalDevice h return (struct wine_phys_dev *)(uintptr_t)handle->base.unix_handle; } -struct wine_queue -{ - struct wine_device *device; /* parent */ - - VkQueue handle; /* client queue */ - VkQueue host_queue; - - uint32_t family_index; - uint32_t queue_index; - VkDeviceQueueCreateFlags flags; - - struct wrapper_entry wrapper_entry; -}; - -static inline struct wine_queue *wine_queue_from_handle(VkQueue handle) -{ - return (struct wine_queue *)(uintptr_t)handle->base.unix_handle; -} - struct wine_cmd_pool { VkCommandPool handle; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5274
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winevulkan/vulkan.c | 41 ++++++++++++++++------------------------ 1 file changed, 16 insertions(+), 25 deletions(-) diff --git a/dlls/winevulkan/vulkan.c b/dlls/winevulkan/vulkan.c index 66821ce18f4..a326971cf5d 100644 --- a/dlls/winevulkan/vulkan.c +++ b/dlls/winevulkan/vulkan.c @@ -386,45 +386,48 @@ static void wine_vk_free_command_buffers(struct wine_device *device, } } -static void wine_vk_device_get_queues(struct wine_device *device, - uint32_t family_index, uint32_t queue_count, VkDeviceQueueCreateFlags flags, - struct wine_queue *queues, VkQueue *handles) +static void wine_vk_device_init_queues(struct wine_device *device, const VkDeviceQueueCreateInfo *info, + VkQueue *handles) { VkDeviceQueueInfo2 queue_info; - unsigned int i; + UINT i; + + TRACE("Queue family index %u, queue count %u.\n", info->queueFamilyIndex, info->queueCount); - for (i = 0; i < queue_count; i++) + for (i = 0; i < info->queueCount; i++) { - struct wine_queue *queue = &queues[i]; + struct wine_queue *queue = device->queues + device->queue_count + i; queue->device = device; queue->handle = (*handles)++; - queue->family_index = family_index; + queue->family_index = info->queueFamilyIndex; queue->queue_index = i; - queue->flags = flags; + queue->flags = info->flags; /* The Vulkan spec says: * * "vkGetDeviceQueue must only be used to get queues that were created * with the flags parameter of VkDeviceQueueCreateInfo set to zero." */ - if (flags && device->funcs.p_vkGetDeviceQueue2) + if (info->flags && device->funcs.p_vkGetDeviceQueue2) { queue_info.sType = VK_STRUCTURE_TYPE_DEVICE_QUEUE_INFO_2; queue_info.pNext = NULL; - queue_info.flags = flags; - queue_info.queueFamilyIndex = family_index; + queue_info.flags = info->flags; + queue_info.queueFamilyIndex = info->queueFamilyIndex; queue_info.queueIndex = i; device->funcs.p_vkGetDeviceQueue2(device->host_device, &queue_info, &queue->host_queue); } else { - device->funcs.p_vkGetDeviceQueue(device->host_device, family_index, i, &queue->host_queue); + device->funcs.p_vkGetDeviceQueue(device->host_device, info->queueFamilyIndex, i, &queue->host_queue); } queue->handle->base.unix_handle = (uintptr_t)queue; TRACE("Got device %p queue %p, host_queue %p.\n", device, queue, queue->host_queue); } + + device->queue_count += info->queueCount; } static const char *find_extension(const char *const *extensions, uint32_t count, const char *ext) @@ -778,7 +781,6 @@ VkResult wine_vkCreateDevice(VkPhysicalDevice phys_dev_handle, const VkDeviceCre VkDevice device_handle = client_ptr; VkDeviceCreateInfo create_info_host; struct VkQueue_T *queue_handles; - struct wine_queue *next_queue; struct conversion_context ctx; struct wine_device *object; unsigned int queue_count, i; @@ -830,20 +832,9 @@ VkResult wine_vkCreateDevice(VkPhysicalDevice phys_dev_handle, const VkDeviceCre ALL_VK_DEVICE_FUNCS() #undef USE_VK_FUNC - next_queue = object->queues; queue_handles = device_handle->queues; for (i = 0; i < create_info_host.queueCreateInfoCount; i++) - { - uint32_t flags = create_info_host.pQueueCreateInfos[i].flags; - uint32_t family_index = create_info_host.pQueueCreateInfos[i].queueFamilyIndex; - uint32_t queue_count = create_info_host.pQueueCreateInfos[i].queueCount; - - TRACE("Queue family index %u, queue count %u.\n", family_index, queue_count); - - wine_vk_device_get_queues(object, family_index, queue_count, flags, next_queue, &queue_handles); - next_queue += queue_count; - } - object->queue_count = next_queue - object->queues; + wine_vk_device_init_queues(object, create_info_host.pQueueCreateInfos + i, &queue_handles); device_handle->quirks = instance->quirks; device_handle->base.unix_handle = (uintptr_t)object; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5274
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winevulkan/loader.c | 20 ++++++++------ dlls/winevulkan/loader_thunks.h | 1 - dlls/winevulkan/make_vulkan | 1 - dlls/winevulkan/vulkan.c | 45 ++++++++++++-------------------- dlls/winevulkan/vulkan_loader.h | 13 ++++++--- dlls/winevulkan/vulkan_private.h | 8 +++--- dlls/winevulkan/vulkan_thunks.c | 5 ++-- dlls/winevulkan/vulkan_thunks.h | 2 +- 8 files changed, 44 insertions(+), 51 deletions(-) diff --git a/dlls/winevulkan/loader.c b/dlls/winevulkan/loader.c index bd49b08057d..17342428869 100644 --- a/dlls/winevulkan/loader.c +++ b/dlls/winevulkan/loader.c @@ -36,6 +36,7 @@ DEFINE_DEVPROPKEY(DEVPROPKEY_GPU_LUID, 0x60b193cb, 0x5276, 0x4d0f, 0x96, 0xfc, 0 DEFINE_DEVPROPKEY(WINE_DEVPROPKEY_GPU_VULKAN_UUID, 0x233a9ef3, 0xafc4, 0x4abd, 0xb5, 0x64, 0xc3, 0x2f, 0x21, 0xf1, 0x53, 0x5c, 2); static HINSTANCE hinstance; +static struct object_sizes object_sizes; static void *wine_vk_get_global_proc_addr(const char *name); @@ -232,7 +233,7 @@ VkResult WINAPI vk_icdNegotiateLoaderICDInterfaceVersion(uint32_t *supported_ver static BOOL WINAPI wine_vk_init(INIT_ONCE *once, void *param, void **context) { - return !__wine_init_unix_call() && !UNIX_CALL(init, NULL); + return !__wine_init_unix_call() && !UNIX_CALL(init, &object_sizes); } static BOOL wine_vk_init_once(void) @@ -470,21 +471,24 @@ VkResult WINAPI vkCreateDevice(VkPhysicalDevice phys_dev, const VkDeviceCreateIn for (i = 0; i < create_info->queueCreateInfoCount; i++) queue_count += create_info->pQueueCreateInfos[i].queueCount; - if (!(device = alloc_vk_object(FIELD_OFFSET(struct VkDevice_T, queues[queue_count])))) + if (!(device = alloc_vk_object(object_sizes.device_size + queue_count * object_sizes.queue_size))) return VK_ERROR_OUT_OF_HOST_MEMORY; - for (i = 0; i < queue_count; i++) - device->queues[i].base.loader_magic = VULKAN_ICD_MAGIC_VALUE; params.physicalDevice = phys_dev; params.pCreateInfo = create_info; params.pAllocator = allocator; - params.pDevice = ret; - params.client_ptr = device; + params.pDevice = &device; status = UNIX_CALL(vkCreateDevice, ¶ms); assert(!status); - if (!device->base.unix_handle) + + if (params.result != VK_SUCCESS) + { free(device); - return params.result; + return params.result; + } + + *ret = device; + return VK_SUCCESS; } void WINAPI vkDestroyDevice(VkDevice device, const VkAllocationCallbacks *allocator) diff --git a/dlls/winevulkan/loader_thunks.h b/dlls/winevulkan/loader_thunks.h index 8b06032c637..55e5fc8f8eb 100644 --- a/dlls/winevulkan/loader_thunks.h +++ b/dlls/winevulkan/loader_thunks.h @@ -2821,7 +2821,6 @@ struct vkCreateDevice_params const VkDeviceCreateInfo *pCreateInfo; const VkAllocationCallbacks *pAllocator; VkDevice *pDevice; - void *client_ptr; VkResult result; }; diff --git a/dlls/winevulkan/make_vulkan b/dlls/winevulkan/make_vulkan index 6b4debf20ff..34713901a13 100755 --- a/dlls/winevulkan/make_vulkan +++ b/dlls/winevulkan/make_vulkan @@ -173,7 +173,6 @@ FUNCTION_OVERRIDES = { "vkCreateInstance" : {"extra_param" : "client_ptr"}, # Instance functions - "vkCreateDevice" : {"extra_param" : "client_ptr"}, "vkDestroyInstance" : {"dispatch" : False}, "vkGetPhysicalDeviceExternalBufferProperties" : {"dispatch" : False}, "vkGetPhysicalDeviceExternalFenceProperties" : {"dispatch" : False}, diff --git a/dlls/winevulkan/vulkan.c b/dlls/winevulkan/vulkan.c index a326971cf5d..5e3b6f2e37c 100644 --- a/dlls/winevulkan/vulkan.c +++ b/dlls/winevulkan/vulkan.c @@ -386,8 +386,7 @@ static void wine_vk_free_command_buffers(struct wine_device *device, } } -static void wine_vk_device_init_queues(struct wine_device *device, const VkDeviceQueueCreateInfo *info, - VkQueue *handles) +static void wine_vk_device_init_queues(struct wine_device *device, const VkDeviceQueueCreateInfo *info) { VkDeviceQueueInfo2 queue_info; UINT i; @@ -398,8 +397,8 @@ static void wine_vk_device_init_queues(struct wine_device *device, const VkDevic { struct wine_queue *queue = device->queues + device->queue_count + i; + queue->client_queue.loader_magic = VULKAN_ICD_MAGIC_VALUE; queue->device = device; - queue->handle = (*handles)++; queue->family_index = info->queueFamilyIndex; queue->queue_index = i; queue->flags = info->flags; @@ -423,7 +422,6 @@ static void wine_vk_device_init_queues(struct wine_device *device, const VkDevic device->funcs.p_vkGetDeviceQueue(device->host_device, info->queueFamilyIndex, i, &queue->host_queue); } - queue->handle->base.unix_handle = (uintptr_t)queue; TRACE("Got device %p queue %p, host_queue %p.\n", device, queue, queue->host_queue); } @@ -504,6 +502,8 @@ static VkResult wine_vk_device_convert_create_info(struct wine_phys_dev *phys_de NTSTATUS init_vulkan(void *args) { + struct object_sizes *sizes = args; + vk_funcs = __wine_get_vulkan_driver(WINE_VULKAN_DRIVER_VERSION); if (!vk_funcs) { @@ -519,6 +519,9 @@ NTSTATUS init_vulkan(void *args) zero_bits = (ULONG_PTR)info.HighestUserAddress | 0x7fffffff; } + sizes->device_size = sizeof(struct wine_device); + sizes->queue_size = sizeof(struct wine_queue); + return STATUS_SUCCESS; } @@ -773,17 +776,14 @@ VkResult wine_vkAllocateCommandBuffers(VkDevice handle, const VkCommandBufferAll } VkResult wine_vkCreateDevice(VkPhysicalDevice phys_dev_handle, const VkDeviceCreateInfo *create_info, - const VkAllocationCallbacks *allocator, VkDevice *ret_device, - void *client_ptr) + const VkAllocationCallbacks *allocator, VkDevice *client_device) { struct wine_phys_dev *phys_dev = wine_phys_dev_from_handle(phys_dev_handle); + struct wine_device *object = *(struct wine_device **)client_device; struct wine_instance *instance = phys_dev->instance; - VkDevice device_handle = client_ptr; VkDeviceCreateInfo create_info_host; - struct VkQueue_T *queue_handles; struct conversion_context ctx; - struct wine_device *object; - unsigned int queue_count, i; + unsigned int i; VkResult res; if (allocator) @@ -800,13 +800,6 @@ VkResult wine_vkCreateDevice(VkPhysicalDevice phys_dev_handle, const VkDeviceCre TRACE("Driver version: %#x.\n", properties.driverVersion); } - /* We need to cache all queues within the device as each requires wrapping since queues are dispatchable objects. */ - for (queue_count = 0, i = 0; i < create_info->queueCreateInfoCount; i++) - queue_count += create_info->pQueueCreateInfos[i].queueCount; - - if (!(object = calloc(1, offsetof(struct wine_device, queues[queue_count])))) - return VK_ERROR_OUT_OF_HOST_MEMORY; - object->phys_dev = phys_dev; init_conversion_context(&ctx); @@ -818,7 +811,6 @@ VkResult wine_vkCreateDevice(VkPhysicalDevice phys_dev_handle, const VkDeviceCre if (res != VK_SUCCESS) { WARN("Failed to create device, res=%d.\n", res); - free(object); return res; } @@ -832,22 +824,19 @@ VkResult wine_vkCreateDevice(VkPhysicalDevice phys_dev_handle, const VkDeviceCre ALL_VK_DEVICE_FUNCS() #undef USE_VK_FUNC - queue_handles = device_handle->queues; for (i = 0; i < create_info_host.queueCreateInfoCount; i++) - wine_vk_device_init_queues(object, create_info_host.pQueueCreateInfos + i, &queue_handles); + wine_vk_device_init_queues(object, create_info_host.pQueueCreateInfos + i); - device_handle->quirks = instance->quirks; - device_handle->base.unix_handle = (uintptr_t)object; + object->client_device.quirks = instance->quirks; TRACE("Created device %p, host_device %p.\n", object, object->host_device); for (i = 0; i < object->queue_count; i++) { struct wine_queue *queue = object->queues + i; - add_handle_mapping_ptr(instance, queue->handle, queue->host_queue, &queue->wrapper_entry); + add_handle_mapping_ptr(instance, &queue->client_queue, queue->host_queue, &queue->wrapper_entry); } - *ret_device = device_handle; - add_handle_mapping_ptr(instance, *ret_device, object->host_device, &object->wrapper_entry); + add_handle_mapping_ptr(instance, &object->client_device, object->host_device, &object->wrapper_entry); return VK_SUCCESS; } @@ -954,7 +943,6 @@ void wine_vkDestroyDevice(VkDevice handle, const VkAllocationCallbacks *allocato remove_handle_mapping(device->phys_dev->instance, &device->wrapper_entry); device->funcs.p_vkDestroyDevice(device->host_device, NULL /* pAllocator */); - free(device); } void wine_vkDestroyInstance(VkInstance handle, const VkAllocationCallbacks *allocator) @@ -1130,17 +1118,16 @@ void wine_vkFreeCommandBuffers(VkDevice handle, VkCommandPool command_pool, uint static VkQueue wine_vk_device_find_queue(VkDevice handle, const VkDeviceQueueInfo2 *info) { struct wine_device *device = wine_device_from_handle(handle); - struct wine_queue *queue; uint32_t i; for (i = 0; i < device->queue_count; i++) { - queue = &device->queues[i]; + struct wine_queue *queue = device->queues + i; if (queue->family_index == info->queueFamilyIndex && queue->queue_index == info->queueIndex && queue->flags == info->flags) { - return queue->handle; + return &queue->client_queue; } } diff --git a/dlls/winevulkan/vulkan_loader.h b/dlls/winevulkan/vulkan_loader.h index 710c07772fc..586fb87eb71 100644 --- a/dlls/winevulkan/vulkan_loader.h +++ b/dlls/winevulkan/vulkan_loader.h @@ -40,6 +40,12 @@ #define WINEVULKAN_QUIRK_GET_DEVICE_PROC_ADDR 0x00000001 +struct object_sizes +{ + UINT32 device_size; + UINT32 queue_size; +}; + /* Base 'class' for our Vulkan dispatchable objects such as VkDevice and VkInstance. * This structure MUST be the first element of a dispatchable object as the ICD * loader depends on it. For now only contains loader_magic, but over time more common @@ -68,14 +74,13 @@ struct VkInstance_T struct VkQueue_T { - struct wine_vk_base base; + UINT64 loader_magic; /* MUST be first, ICD loader depends on it */ }; struct VkDevice_T { - struct wine_vk_base base; - unsigned int quirks; - struct VkQueue_T queues[1]; + UINT64 loader_magic; /* MUST be first, ICD loader depends on it */ + UINT32 quirks; }; struct vk_command_pool diff --git a/dlls/winevulkan/vulkan_private.h b/dlls/winevulkan/vulkan_private.h index 31e9e981168..6d2cef1e325 100644 --- a/dlls/winevulkan/vulkan_private.h +++ b/dlls/winevulkan/vulkan_private.h @@ -57,9 +57,9 @@ static inline struct wine_cmd_buffer *wine_cmd_buffer_from_handle(VkCommandBuffe struct wine_queue { + struct VkQueue_T client_queue; /* MUST be first */ struct wine_device *device; /* parent */ - VkQueue handle; /* client queue */ VkQueue host_queue; uint32_t family_index; @@ -71,15 +71,15 @@ struct wine_queue static inline struct wine_queue *wine_queue_from_handle(VkQueue handle) { - return (struct wine_queue *)(uintptr_t)handle->base.unix_handle; + return CONTAINING_RECORD(handle, struct wine_queue, client_queue); } struct wine_device { + struct VkDevice_T client_device; /* MUST be first */ struct vulkan_device_funcs funcs; struct wine_phys_dev *phys_dev; /* parent */ - VkDevice handle; /* client device */ VkDevice host_device; struct wrapper_entry wrapper_entry; @@ -92,7 +92,7 @@ C_ASSERT(sizeof(struct wine_device) == offsetof(struct wine_device, queues[0])); static inline struct wine_device *wine_device_from_handle(VkDevice handle) { - return (struct wine_device *)(uintptr_t)handle->base.unix_handle; + return CONTAINING_RECORD(handle, struct wine_device, client_device); } struct wine_debug_utils_messenger; diff --git a/dlls/winevulkan/vulkan_thunks.c b/dlls/winevulkan/vulkan_thunks.c index 93e52b46a9d..07e728cac27 100644 --- a/dlls/winevulkan/vulkan_thunks.c +++ b/dlls/winevulkan/vulkan_thunks.c @@ -37416,7 +37416,7 @@ static NTSTATUS thunk64_vkCreateDevice(void *args) init_conversion_context(ctx); convert_VkDeviceCreateInfo_win64_to_host(ctx, params->pCreateInfo, &pCreateInfo_host); - params->result = wine_vkCreateDevice(params->physicalDevice, &pCreateInfo_host, params->pAllocator, params->pDevice, params->client_ptr); + params->result = wine_vkCreateDevice(params->physicalDevice, &pCreateInfo_host, params->pAllocator, params->pDevice); free_conversion_context(ctx); return STATUS_SUCCESS; } @@ -37430,7 +37430,6 @@ static NTSTATUS thunk32_vkCreateDevice(void *args) PTR32 pCreateInfo; PTR32 pAllocator; PTR32 pDevice; - PTR32 client_ptr; VkResult result; } *params = args; VkDeviceCreateInfo pCreateInfo_host; @@ -37443,7 +37442,7 @@ static NTSTATUS thunk32_vkCreateDevice(void *args) init_conversion_context(ctx); convert_VkDeviceCreateInfo_win32_to_host(ctx, (const VkDeviceCreateInfo32 *)UlongToPtr(params->pCreateInfo), &pCreateInfo_host); pDevice_host = UlongToPtr(*(PTR32 *)UlongToPtr(params->pDevice)); - params->result = wine_vkCreateDevice((VkPhysicalDevice)UlongToPtr(params->physicalDevice), &pCreateInfo_host, (const VkAllocationCallbacks *)UlongToPtr(params->pAllocator), &pDevice_host, UlongToPtr(params->client_ptr)); + params->result = wine_vkCreateDevice((VkPhysicalDevice)UlongToPtr(params->physicalDevice), &pCreateInfo_host, (const VkAllocationCallbacks *)UlongToPtr(params->pAllocator), &pDevice_host); *(PTR32 *)UlongToPtr(params->pDevice) = PtrToUlong(pDevice_host); free_conversion_context(ctx); return STATUS_SUCCESS; diff --git a/dlls/winevulkan/vulkan_thunks.h b/dlls/winevulkan/vulkan_thunks.h index 2321bb91c09..37db9a2ae1e 100644 --- a/dlls/winevulkan/vulkan_thunks.h +++ b/dlls/winevulkan/vulkan_thunks.h @@ -22,7 +22,7 @@ VkResult wine_vkCreateCommandPool(VkDevice device, const VkCommandPoolCreateInfo VkResult wine_vkCreateDebugReportCallbackEXT(VkInstance instance, const VkDebugReportCallbackCreateInfoEXT *pCreateInfo, const VkAllocationCallbacks *pAllocator, VkDebugReportCallbackEXT *pCallback); VkResult wine_vkCreateDebugUtilsMessengerEXT(VkInstance instance, const VkDebugUtilsMessengerCreateInfoEXT *pCreateInfo, const VkAllocationCallbacks *pAllocator, VkDebugUtilsMessengerEXT *pMessenger); VkResult wine_vkCreateDeferredOperationKHR(VkDevice device, const VkAllocationCallbacks *pAllocator, VkDeferredOperationKHR *pDeferredOperation); -VkResult wine_vkCreateDevice(VkPhysicalDevice physicalDevice, const VkDeviceCreateInfo *pCreateInfo, const VkAllocationCallbacks *pAllocator, VkDevice *pDevice, void *client_ptr); +VkResult wine_vkCreateDevice(VkPhysicalDevice physicalDevice, const VkDeviceCreateInfo *pCreateInfo, const VkAllocationCallbacks *pAllocator, VkDevice *pDevice); VkResult wine_vkCreateImage(VkDevice device, const VkImageCreateInfo *pCreateInfo, const VkAllocationCallbacks *pAllocator, VkImage *pImage); VkResult wine_vkCreateInstance(const VkInstanceCreateInfo *pCreateInfo, const VkAllocationCallbacks *pAllocator, VkInstance *pInstance, void *client_ptr); VkResult wine_vkCreateSwapchainKHR(VkDevice device, const VkSwapchainCreateInfoKHR *pCreateInfo, const VkAllocationCallbacks *pAllocator, VkSwapchainKHR *pSwapchain); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5274
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=143939 Your paranoid android. === debian11b (64 bit WoW report) === secur32: schannel.c:538: Test failed: cert_cnt = 2
This saves one pointer indirection every time a device function is called from a device client handle. I have similar changes for other wrappers.
Is it somehow causing any actual problem? I considered something like that when implementing PE separation and decided against it. It's essentially an opaque to feed PE side with information about Unix lib layout that it does not need to deal with. What's wrong with having a clear separation? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5274#note_64259
Is it somehow causing any actual problem?
It adds an unnecessary indirection every time the CPU needs to fetch a device function pointer to call it, in a very performance critical path. It depends what you consider an actual problem then but we removed traces in these functions for similar reasons. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5274#note_64265
IIRC the reason to remove traces was to allow compiler to emit leaf functions, this doesn't seem nearly as impactful. Does it make a measurable difference? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5274#note_64365
It does make a small difference in the vkoverhead benchmark, measurable for instance in the descriptor_buffer_offset_combined_sampler test. Anyway I don't really mind about the last change, but I think the other ones make the code simpler, how about dropping the last commit? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5274#note_64382
Jacek Caban (@jacek) commented about dlls/winevulkan/vulkan.c:
if (!object) return;
- instance->funcs.p_vkDestroySurfaceKHR(instance->host_instance, object->driver_surface, NULL); remove_handle_mapping(instance, &object->wrapper_entry); - + instance->funcs.p_vkDestroySurfaceKHR(instance->host_instance, object->driver_surface, NULL);
Changes like that will make break unwrapping handles in debug callbacks from inside `vkDestroySurfaceKHR` (or similarly others), can we for some reason assume that they don't emit such callbacks? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5274#note_64416
On Tue Mar 12 13:57:01 2024 +0000, Jacek Caban wrote:
Changes like that will make break unwrapping handles in debug callbacks from inside `vkDestroySurfaceKHR` (or similarly others), can we for some reason assume that they don't emit such callbacks? The rationale was that we cannot map callbacks during creation, so this would be unbalanced with callbacks during destruction (if anything tracks leaks for instance), and we will need instead to generate our own callbacks to balance it.
Still I'm also fine keeping it unbalanced, though I think it wasn't completely consistent. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5274#note_64610
On Wed Mar 13 16:06:44 2024 +0000, Rémi Bernon wrote:
The rationale was that we cannot map callbacks during creation, so this would be unbalanced with callbacks during destruction (if anything tracks leaks for instance), and we will need instead to generate our own callbacks to balance it. Still I'm also fine keeping it unbalanced, though I think it wasn't completely consistent. I think that this being unbalanced matches how it looks from host Vulkan point of view: constructors can't expect the caller to understand the new handle in callbacks (so presumably they won't emit such callbacks), while destructors can.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5274#note_64614
On Wed Mar 13 16:39:19 2024 +0000, Jacek Caban wrote:
I think that this being unbalanced matches how it looks from host Vulkan point of view: constructors can't expect the caller to understand the new handle in callbacks (so presumably they won't emit such callbacks), while destructors can. Well you still have object types and handles during construction, so callbacks could still be understandable. Whereas in Wine we simply cannot map these to any client handle because we don't yet have the mapping, so these callbacks are simply dropped.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5274#note_64615
participants (3)
-
Jacek Caban (＠jacek) -
Marvin -
Rémi Bernon