[PATCH 1/2] winevulkan: Wrap VkSurfaceKHR in winevulkan.

Liam Middlebrook lmiddlebrook at nvidia.com
Thu Jan 21 16:20:02 CST 2021



On 1/21/21 11:54 AM, Georg Lehmann wrote:
> On 21.01.21 19:36, Liam Middlebrook wrote:
>>
>>
>> On 1/21/21 10:12 AM, Georg Lehmann wrote:
>>> Signed-off-by: Georg Lehmann <dadschoorse at gmail.com>
>>> ---
>>>   dlls/winemac.drv/vulkan.c        |  5 ++--
>>>   dlls/winevulkan/make_vulkan      | 36 ++++++++++++++++++++++--
>>>   dlls/winevulkan/vulkan.c         | 47 ++++++++++++++++++++++++++++++++
>>>   dlls/winevulkan/vulkan_private.h | 17 ++++++++++++
>>>   dlls/winex11.drv/vulkan.c        |  5 ++--
>>>   5 files changed, 103 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/dlls/winemac.drv/vulkan.c b/dlls/winemac.drv/vulkan.c
>>> index dd92231d3ea..fa2abcd47ac 100644
>>> --- a/dlls/winemac.drv/vulkan.c
>>> +++ b/dlls/winemac.drv/vulkan.c
>>> @@ -99,7 +99,7 @@ static void 
>>> *macdrv_get_vk_instance_proc_addr(VkInstance instance, const char *n
>>>   static inline struct wine_vk_surface 
>>> *surface_from_handle(VkSurfaceKHR handle)
>>>   {
>>> -    return (struct wine_vk_surface *)(uintptr_t)handle;
>>> +    return vulkan_driver_get_surface_data(handle);
>>>   }
>>>   static void *vulkan_handle;
>>> @@ -320,7 +320,8 @@ static VkResult 
>>> macdrv_vkCreateWin32SurfaceKHR(VkInstance instance,
>>>           goto err;
>>>       }
>>> -    *surface = (uintptr_t)mac_surface;
>>> +    vulkan_driver_set_surface_data(*surface, mac_surface);
>>> +    vulkan_driver_set_native_surface(*surface, mac_surface->surface);
>>>       release_win_data(data);
>>> diff --git a/dlls/winevulkan/make_vulkan b/dlls/winevulkan/make_vulkan
>>> index 1ae6fdbff64..9fa0b2c9507 100755
>>> --- a/dlls/winevulkan/make_vulkan
>>> +++ b/dlls/winevulkan/make_vulkan
>>> @@ -131,7 +131,7 @@ CORE_EXTENSIONS = [
>>>   # Functions part of our winevulkan graphics driver interface.
>>>   # DRIVER_VERSION should be bumped on any change to driver interface
>>>   # in FUNCTION_OVERRIDES
>>> -DRIVER_VERSION = 8
>>> +DRIVER_VERSION = 9
>>>   # Table of functions for which we have a special implementation.
>>>   # These are regular device / instance functions for which we need
>>> @@ -174,7 +174,7 @@ FUNCTION_OVERRIDES = {
>>>       "vkQueueSubmit" : {"dispatch": True, "driver" : False, "thunk" 
>>> : False},
>>>       # VK_KHR_surface
>>> -    "vkDestroySurfaceKHR" : {"dispatch" : True, "driver" : True, 
>>> "thunk" : True},
>>> +    "vkDestroySurfaceKHR" : {"dispatch" : True, "driver" : True, 
>>> "thunk" : False},
>>>       "vkGetPhysicalDeviceSurfaceSupportKHR" : {"dispatch" : True, 
>>> "driver" : True, "thunk" : True},
>>>       "vkGetPhysicalDeviceSurfaceCapabilitiesKHR" : {"dispatch" : 
>>> True, "driver" : True, "thunk" : False, "private_thunk" : True},
>>>       "vkGetPhysicalDeviceSurfaceFormatsKHR" : {"dispatch" : True, 
>>> "driver" : True, "thunk" : True},
>>> @@ -185,7 +185,7 @@ FUNCTION_OVERRIDES = {
>>>       "vkGetPhysicalDeviceSurfaceFormats2KHR" : {"dispatch" : True, 
>>> "driver" : True, "thunk" : True},
>>>       # VK_KHR_win32_surface
>>> -    "vkCreateWin32SurfaceKHR" : {"dispatch" : True, "driver" : True, 
>>> "thunk" : True},
>>> +    "vkCreateWin32SurfaceKHR" : {"dispatch" : True, "driver" : True, 
>>> "thunk" : False},
>>>       "vkGetPhysicalDeviceWin32PresentationSupportKHR" : {"dispatch" 
>>> : True, "driver" : True, "thunk" : True},
>>>       # VK_KHR_swapchain
>>> @@ -2584,6 +2584,36 @@ class VkGenerator(object):
>>>           f.write("    name -= 2;\n\n")
>>>           f.write("    return 
>>> get_vulkan_driver_device_proc_addr(vulkan_funcs, name);\n}\n\n")
>>> +        f.write("struct wine_surface_base\n")
>>> +        f.write("{\n")
>>> +        f.write("    VkSurfaceKHR surface; /* native surface */\n")
>>
>> Just to be extra clear could you s/surface/native_surface/ in this 
>> structure?
> 
> All structs in vulkan_private.h use this style, so I wanted to be 
> consistent.

Okay, that's fine as-is then.

> 
>>
>>> +        f.write("    void *driver_data;\n")
>>
>> Maybe it's just because I have something else in mind when I think 
>> "driver" but this was a bit confusing for me to figure out. Could you 
>> add a comment that notes this is either a winex11/winemac structure. I 
>> guess a comment specifically denoting those could go stale if/when 
>> another GDI backend is created, so wording might be weird. Maybe even 
>> calling it gdi_driver_data would convey that? I'm not sure, I may be 
>> alone here in my brief confusion.
> 
> Yeah, I understand your confusion, but winemac/winex11 are referred to 
> as drivers all the time in winevulkan. I think I will add a comment.
> 
>>
>>> +        f.write("};\n\n")
>>> +
>>> +        f.write("static inline void 
>>> vulkan_driver_set_native_surface(VkSurfaceKHR surface, VkSurfaceKHR 
>>> native_surface)\n")
>>> +        f.write("{\n")
>>> +        f.write("    struct wine_surface_base *object = (void 
>>> *)(uintptr_t)surface;\n")
>>> +        f.write("    object->surface = native_surface;\n")
>>> +        f.write("};\n\n")
>>
>> I don't see vulkan_driver_set_surface_data() being called ever without 
>> this function immediately following it. Would it make sense to drop 
>> this function and just set the native surface in 
>> vulkan_driver_set_surface_data(). Maybe it would need a rename though.
> 
> Maybe vulkan_driver_init_surface since it's only supposed to be called 
> during surface creation?

Seems reasonable to me.


> 
>>
>>> +
>>> +        f.write("static inline VkSurfaceKHR 
>>> vulkan_driver_get_native_surface(VkSurfaceKHR surface)\n")
>>> +        f.write("{\n")
>>> +        f.write("    struct wine_surface_base *object = (void 
>>> *)(uintptr_t)surface;\n")
>>> +        f.write("    return object->surface;\n")
>>> +        f.write("};\n\n")
>>> +
>>> +        f.write("static inline void 
>>> vulkan_driver_set_surface_data(VkSurfaceKHR surface, void *data)\n")
>>> +        f.write("{\n")
>>> +        f.write("    struct wine_surface_base *object = (void 
>>> *)(uintptr_t)surface;\n")
>>> +        f.write("    object->driver_data = data;\n")
>>> +        f.write("};\n\n")
>>> +
>>> +        f.write("static inline void 
>>> *vulkan_driver_get_surface_data(VkSurfaceKHR surface)\n")
>>> +        f.write("{\n")
>>> +        f.write("    struct wine_surface_base *object = (void 
>>> *)(uintptr_t)surface;\n")
>>> +        f.write("    return object->driver_data;\n")
>>> +        f.write("};\n\n")
>>
>> Why are these in make_vulkan (and generated to the thunks file) rather 
>> than in vulkan.c (and the header defined in vulkan_private.h)?
> 
> These functions are part of the wine driver interface, so they should be 
> in vulkan_driver.h which is generated by make_vulkan.

Okay, that makes sense.

Thanks,

Liam Middlebrook

> 
> Thanks,
> 
> Georg Lehmann
> 
>>
>>
>> Thanks,
>>
>> Liam Middlebrook
>>
>>> +
>>>           f.write("#endif /* __WINE_VULKAN_DRIVER_H */\n")
>>>       def generate_vulkan_spec(self, f):
>>> diff --git a/dlls/winevulkan/vulkan.c b/dlls/winevulkan/vulkan.c
>>> index b0a8559fe2c..9b3064e2a44 100644
>>> --- a/dlls/winevulkan/vulkan.c
>>> +++ b/dlls/winevulkan/vulkan.c
>>> @@ -1770,6 +1770,53 @@ void WINAPI wine_vkGetPrivateDataEXT(VkDevice 
>>> device, VkObjectType object_type,
>>>       device->funcs.p_vkGetPrivateDataEXT(device->device, 
>>> object_type, object_handle, private_data_slot, data);
>>>   }
>>> +VkResult WINAPI wine_vkCreateWin32SurfaceKHR(VkInstance instance,
>>> +        const VkWin32SurfaceCreateInfoKHR *createInfo, const 
>>> VkAllocationCallbacks *allocator, VkSurfaceKHR *surface)
>>> +{
>>> +    struct wine_surface *object;
>>> +    VkResult res;
>>> +
>>> +    TRACE("%p, %p, %p, %p\n", instance, createInfo, allocator, 
>>> surface);
>>> +
>>> +    if (allocator)
>>> +        FIXME("Support for allocation callbacks not implemented 
>>> yet\n");
>>> +
>>> +    object = heap_alloc_zero(sizeof(*object));
>>> +
>>> +    if (!object)
>>> +        return VK_ERROR_OUT_OF_HOST_MEMORY;
>>> +
>>> +    *surface = wine_surface_to_handle(object);
>>> +
>>> +    res = 
>>> instance->funcs.p_vkCreateWin32SurfaceKHR(instance->instance, 
>>> createInfo, NULL, surface);
>>> +
>>> +    if (res != VK_SUCCESS)
>>> +    {
>>> +        heap_free(object);
>>> +        *surface = VK_NULL_HANDLE;
>>> +        return res;
>>> +    }
>>> +
>>> +    WINE_VK_ADD_NON_DISPATCHABLE_MAPPING(instance, object, 
>>> object->base.surface);
>>> +
>>> +    return VK_SUCCESS;
>>> +}
>>> +
>>> +void WINAPI wine_vkDestroySurfaceKHR(VkInstance instance, 
>>> VkSurfaceKHR surface, const VkAllocationCallbacks *allocator)
>>> +{
>>> +    struct wine_surface *object = wine_surface_from_handle(surface);
>>> +
>>> +    TRACE("%p, 0x%s, %p\n", instance, wine_dbgstr_longlong(surface), 
>>> allocator);
>>> +
>>> +    if (!object)
>>> +        return;
>>> +
>>> +    instance->funcs.p_vkDestroySurfaceKHR(instance->instance, 
>>> surface, NULL);
>>> +
>>> +    WINE_VK_REMOVE_HANDLE_MAPPING(instance, object);
>>> +    heap_free(object);
>>> +}
>>> +
>>>   static inline void adjust_max_image_count(VkPhysicalDevice 
>>> phys_dev, VkSurfaceCapabilitiesKHR* capabilities)
>>>   {
>>>       /* Many Windows games, for example Strange Brigade, No Man's 
>>> Sky, Path of Exile
>>> diff --git a/dlls/winevulkan/vulkan_private.h 
>>> b/dlls/winevulkan/vulkan_private.h
>>> index 92c04543587..bf49371e40e 100644
>>> --- a/dlls/winevulkan/vulkan_private.h
>>> +++ b/dlls/winevulkan/vulkan_private.h
>>> @@ -213,6 +213,23 @@ static inline VkDebugReportCallbackEXT 
>>> wine_debug_report_callback_to_handle(
>>>       return (VkDebugReportCallbackEXT)(uintptr_t)debug_messenger;
>>>   }
>>> +struct wine_surface
>>> +{
>>> +    struct wine_surface_base base;
>>> +
>>> +    struct wine_vk_mapping mapping;
>>> +};
>>> +
>>> +static inline struct wine_surface 
>>> *wine_surface_from_handle(VkSurfaceKHR handle)
>>> +{
>>> +    return (struct wine_surface *)(uintptr_t)handle;
>>> +}
>>> +
>>> +static inline VkSurfaceKHR wine_surface_to_handle(struct 
>>> wine_surface *surface)
>>> +{
>>> +    return (VkSurfaceKHR)(uintptr_t)surface;
>>> +}
>>> +
>>>   void *wine_vk_get_device_proc_addr(const char *name) DECLSPEC_HIDDEN;
>>>   void *wine_vk_get_instance_proc_addr(const char *name) 
>>> DECLSPEC_HIDDEN;
>>> diff --git a/dlls/winex11.drv/vulkan.c b/dlls/winex11.drv/vulkan.c
>>> index 4de82586906..9f123a91bb9 100644
>>> --- a/dlls/winex11.drv/vulkan.c
>>> +++ b/dlls/winex11.drv/vulkan.c
>>> @@ -99,7 +99,7 @@ static void 
>>> *X11DRV_get_vk_instance_proc_addr(VkInstance instance, const char *n
>>>   static inline struct wine_vk_surface 
>>> *surface_from_handle(VkSurfaceKHR handle)
>>>   {
>>> -    return (struct wine_vk_surface *)(uintptr_t)handle;
>>> +    return vulkan_driver_get_surface_data(handle);
>>>   }
>>>   static void *vulkan_handle;
>>> @@ -324,7 +324,8 @@ static VkResult 
>>> X11DRV_vkCreateWin32SurfaceKHR(VkInstance instance,
>>>       XSaveContext(gdi_display, (XID)create_info->hwnd, 
>>> vulkan_hwnd_context, (char *)wine_vk_surface_grab(x11_surface));
>>>       LeaveCriticalSection(&context_section);
>>> -    *surface = (uintptr_t)x11_surface;
>>> +    vulkan_driver_set_surface_data(*surface, x11_surface);
>>> +    vulkan_driver_set_native_surface(*surface, x11_surface->surface);
>>>       TRACE("Created surface=0x%s\n", wine_dbgstr_longlong(*surface));
>>>       return VK_SUCCESS;
>>>
>>
> 



More information about the wine-devel mailing list