[PATCH] winevulkan: Add support for unwrapping handles in thunks.

Derek Lesho dlesho at codeweavers.com
Wed May 12 11:00:43 CDT 2021


On 5/12/21 7:58 AM, Georg Lehmann wrote:

> On 11.05.21 22:13, Derek Lesho wrote:
>> ---
>> Right now this patch allows us to remove/shrink some our manual 
>> thunks, but as more wrappable handles are added, this should become 
>> more useful.
>> ---
>>   dlls/winevulkan/make_vulkan | 541 +++++++++++++++++++++++++++++-------
>>   dlls/winevulkan/vulkan.c    |  32 +--
>>   2 files changed, 440 insertions(+), 133 deletions(-)
>>
>> diff --git a/dlls/winevulkan/make_vulkan b/dlls/winevulkan/make_vulkan
>> index d5cc62d2097..fe62cb0fb35 100755
>> --- a/dlls/winevulkan/make_vulkan
>> +++ b/dlls/winevulkan/make_vulkan
>> @@ -204,14 +204,14 @@ FUNCTION_OVERRIDES = {
>>         # VK_KHR_get_surface_capabilities2
>>       "vkGetPhysicalDeviceSurfaceCapabilities2KHR" : {"dispatch" : 
>> True, "driver" : True, "thunk" : ThunkType.PRIVATE},
>> -    "vkGetPhysicalDeviceSurfaceFormats2KHR" : {"dispatch" : True, 
>> "driver" : True, "thunk" : ThunkType.PRIVATE},
>> +    "vkGetPhysicalDeviceSurfaceFormats2KHR" : {"dispatch" : True, 
>> "driver" : True, "thunk" : ThunkType.PUBLIC},
>>         # VK_KHR_win32_surface
>>       "vkCreateWin32SurfaceKHR" : {"dispatch" : True, "driver" : 
>> True, "thunk" : ThunkType.NONE},
>>       "vkGetPhysicalDeviceWin32PresentationSupportKHR" : {"dispatch" 
>> : True, "driver" : True, "thunk" : ThunkType.PUBLIC},
>>         # VK_KHR_swapchain
>> -    "vkCreateSwapchainKHR" : {"dispatch" : True, "driver" : True, 
>> "thunk" : ThunkType.PRIVATE},
>> +    "vkCreateSwapchainKHR" : {"dispatch" : True, "driver" : True, 
>> "thunk" : ThunkType.PUBLIC},
>>       "vkDestroySwapchainKHR" : {"dispatch" : True, "driver" : True, 
>> "thunk" : ThunkType.PUBLIC},
>>       "vkGetSwapchainImagesKHR": {"dispatch" : True, "driver" : True, 
>> "thunk" : ThunkType.PUBLIC},
>>       "vkQueuePresentKHR": {"dispatch" : True, "driver" : True, 
>> "thunk" : ThunkType.PUBLIC},
>> @@ -645,6 +645,18 @@ class VkFunction(object):
>>             return False
>>   +    def needs_unwrapping(self):
>> +        """ Check if the function needs any input/output type 
>> unwrapping.
>> +        Functions need input/output unwrapping if struct parameters 
>> have
>> +        wrapped dispatchable handles.
>> +        """
>> +
>> +        for p in self.params:
>> +            if p.needs_unwrapping():
>> +                return True
>> +
>> +        return False
>> +
>>       def needs_dispatch(self):
>>           return self.dispatch
>>   @@ -748,13 +760,16 @@ class VkFunction(object):
>>             # Declare any tmp parameters for conversion.
>>           for p in self.params:
>> -            if not p.needs_conversion():
>> -                continue
>> -
>> -            if p.is_dynamic_array():
>> -                body += "    {0}_host *{1}_host;\n".format(p.type, 
>> p.name)
>> -            else:
>> -                body += "    {0}_host {1}_host;\n".format(p.type, 
>> p.name)
>> +            if p.needs_conversion():
>> +                if p.is_dynamic_array():
>> +                    body += "    {0}_host 
>> *{1}_host;\n".format(p.type, p.name)
>> +                else:
>> +                    body += "    {0}_host 
>> {1}_host;\n".format(p.type, p.name)
>> +            elif p.needs_unwrapping():
>> +                if p.is_dynamic_array():
>> +                    body += "    {0} *{1}_host;\n".format(p.type, 
>> p.name)
>> +                else:
>> +                    body += "    {0} {1}_host;\n".format(p.type, 
>> p.name)
>>             if not self.needs_private_thunk():
>>               body += "    {0}\n".format(self.trace())
>> @@ -798,6 +813,51 @@ class VkFunction(object):
>>             return body
>>   +    def body_unwrapping(self):
>
> This is mostly a copy of body_conversion, can those be unified?
Yeah, that's a good idea, did this in v2.
>
>> +        body = ""
>> +
>> +        # Declare a variable to hold the result for non-void functions.
>> +        if self.type != "void":
>> +            body += "    {0} result;\n".format(self.type)
>> +
>> +        for p in self.params:
>> +            if p.needs_unwrapping():
>> +                if p.is_dynamic_array():
>> +                    body += "    {0} *{1}_host;\n".format(p.type, 
>> p.name)
>> +                else:
>> +                    body += "    {0} {1}_host;\n".format(p.type, 
>> p.name)
>> +
>> +        if not self.needs_private_thunk():
>> +            body += "    {0}".format(self.trace())
>> +
>> +        # Call unwrapping win_to_host conversion calls.
>> +        for p in self.params:
>> +            if p.needs_unwrapping() and p.needs_input_conversion():
>> +                body += p.copy(Direction.INPUT)
>> +
>> +        params = ", ".join([p.variable(conv=False) for p in 
>> self.params])
>> +
>> +        # Call the native Vulkan function.
>> +        if self.type == "void":
>> +            body += " 
>> {0}.p_{1}({2});\n".format(self.params[0].dispatch_table(), self.name, 
>> params)
>> +        else:
>> +            body += "    result = 
>> {0}.p_{1}({2});\n".format(self.params[0].dispatch_table(), self.name, 
>> params)
>> +
>> +        body += "\n"
>> +
>> +        # Perform any required cleanups. Most of these are for array 
>> functions.
>> +        for p in self.params:
>> +            if not p.needs_free() or not p.needs_unwrapping():
>> +                continue
>> +
>> +            body += p.free()
>> +
>> +        # Finally return the result.
>> +        if self.type != "void":
>> +            body += "    return result;\n"
>> +
>> +        return body
>> +
>>       def spec(self, prefix=None, symbol=None):
>>           """ Generate spec file entry for this function.
>>   @@ -842,8 +902,13 @@ class VkFunction(object):
>>               thunk += "#if defined(USE_STRUCT_CONVERSION)\n"
>>               thunk += self.body_conversion()
>>               thunk += "#else\n"
>> -            thunk += self.body()
>> +            if self.needs_unwrapping():
>> +                thunk += self.body_unwrapping()
>> +            else:
>> +                thunk += self.body()
>>               thunk += "#endif\n"
>> +        elif self.needs_unwrapping():
>> +            thunk += self.body_unwrapping()
>>           else:
>>               thunk += self.body()
>>   @@ -1063,6 +1128,12 @@ class VkHandle(object):
>>       def is_wrapped(self):
>>           return self.native_handle("test") is not None
>>   +    def needs_conversion(self):
>> +        return False
>> +
>> +    def needs_unwrapping(self):
>> +        return self.is_wrapped()
>> +
>>   class VkMember(object):
>>       def __init__(self, const=False, 
>> struct_fwd_decl=False,_type=None, pointer=None, name=None, 
>> array_len=None,
>>               dyn_array_len=None, optional=False, values=None):
>> @@ -1148,10 +1219,11 @@ class VkMember(object):
>>           return VkMember(const=const, 
>> struct_fwd_decl=struct_fwd_decl, _type=member_type, pointer=pointer, 
>> name=name_elem.text,
>>                   array_len=array_len, dyn_array_len=dyn_array_len, 
>> optional=optional, values=values)
>>   -    def copy(self, input, output, direction):
>> -        """ Helper method for use by conversion logic to generate a 
>> C-code statement to copy this member. """
>> +    def copy(self, input, output, direction, conv):
>> +        """ Helper method for use by conversion logic to generate a 
>> C-code statement to copy this member.
>> +            - `conv` indicates whether the statement is in a struct 
>> alignment conversion path. """
>>   -        if self.needs_conversion():
>> +        if (conv and self.needs_conversion()) or 
>> self.needs_unwrapping():
>>               if self.is_dynamic_array():
>>                   if direction == Direction.OUTPUT:
>>                       LOGGER.warn("TODO: implement copying of 
>> returnedonly dynamic array for {0}.{1}".format(self.type, self.name))
>> @@ -1167,6 +1239,12 @@ class VkMember(object):
>>                   else:
>>                       # Nothing needed this yet.
>>                       LOGGER.warn("TODO: implement copying of static 
>> array for {0}.{1}".format(self.type, self.name))
>> +            elif self.is_handle() and self.needs_unwrapping():
>> +                if direction == Direction.OUTPUT:
>> +                    LOGGER.err("Only INPUT parameters can be 
>> unwrapped")
>
> Log the handle name here.
👌
>
>> +                else:
>> +                    handle = self.type_info["data"]
>> +                    return "{0}{1} = {2};\n".format(output, 
>> self.name, handle.driver_handle("{0}{1}".format(input, self.name)))
>>               else:
>>                   if direction == Direction.OUTPUT:
>>                       return "convert_{0}_host_to_win(&{2}{1}, 
>> &{3}{1});\n".format(self.type, self.name, input, output)
>> @@ -1178,6 +1256,27 @@ class VkMember(object):
>>           else:
>>               return "{0}{1} = {2}{1};\n".format(output, self.name, 
>> input)
>>   +    def free(self, location, conv):
>> +        """ Helper method for use by conversion logic to generate a 
>> C-code statement to free this member. """
>> +
>> +        if not self.needs_unwrapping() and not conv:
>> +            return ""
>> +
>> +        cast = ("(" + self.type + ("_host" if conv and 
>> self.needs_conversion() else "") + " *)") if self.is_const() else ""
>> +        if self.is_dynamic_array():
>> +            count = self.dyn_array_len if 
>> isinstance(self.dyn_array_len, int) else "{0}{1}".format(location, 
>> self.dyn_array_len)
>> +            if self.is_struct() and 
>> self.type_info["data"].returnedonly:
>> +                # For returnedonly, counts is stored in a pointer.
>> +                return "free_{0}_array({1}{2}{3}, 
>> *{4});\n".format(self.type, cast, location, self.name, count)
>> +            else:
>> +                return "free_{0}_array({1}{2}{3}, 
>> {4});\n".format(self.type, cast, location, self.name, count)
>> +        else:
>> +            # We are operating on a single structure. Some structs 
>> (very rare) contain dynamic members,
>> +            # which would need freeing.
>> +            if self.needs_free():
>> +                return "free_{0}({1}&{2}{3});\n".format(self.type, 
>> cast, location, self.name)
>> +        return ""
>> +
>>       def definition(self, align=False, conv=False):
>>           """ Generate prototype for given function.
>>   @@ -1216,31 +1315,35 @@ class VkMember(object):
>>             # Check if we need conversion either for this member 
>> itself or for any child members
>>           # in case member represents a struct.
>> -        if not self.needs_conversion():
>> +        if not self.needs_conversion() and not self.needs_unwrapping():
>>               return None
>>             conversions = []
>>             # Collect any conversion for any member structs.
>> -        struct = self.type_info["data"]
>> -        for m in struct:
>> -            m.needs_struct_extensions_conversion()
>> -            if m.needs_conversion():
>> -                conversions.extend(m.get_conversions())
>> +        direction = None
>
> This declaration is redundant.
👌
>
>> +        if self.is_struct():
>> +            struct = self.type_info["data"]
>> +            for m in struct:
>> +                m.needs_struct_extensions_conversion()
>> +                if m.needs_conversion() or m.needs_unwrapping():
>> +                    conversions.extend(m.get_conversions())
>>   -        struct.needs_struct_extensions_conversion()
>> +            struct.needs_struct_extensions_conversion()
>> +            direction = Direction.OUTPUT if struct.returnedonly else 
>> Direction.INPUT
>> +        elif self.is_handle():
>> +            direction = Direction.INPUT
>>   -        struct = self.type_info["data"]
>> -        direction = Direction.OUTPUT if struct.returnedonly else 
>> Direction.INPUT
>> +        operand = self.type_info["data"]
>>           if self.is_dynamic_array():
>> -            conversions.append(ConversionFunction(False, True, 
>> direction, struct))
>> +            conversions.append(ConversionFunction(False, True, 
>> direction, operand))
>>           elif self.is_static_array():
>> -            conversions.append(ConversionFunction(True, False, 
>> direction, struct))
>> +            conversions.append(ConversionFunction(True, False, 
>> direction, operand))
>>           else:
>> -            conversions.append(ConversionFunction(False, False, 
>> direction, struct))
>> +            conversions.append(ConversionFunction(False, False, 
>> direction, operand))
>>             if self.needs_free():
>> - conversions.append(FreeFunction(self.is_dynamic_array(), struct))
>> + conversions.append(FreeFunction(self.is_dynamic_array(), operand))
>>             return conversions
>>   @@ -1307,8 +1410,21 @@ class VkMember(object):
>>           struct = self.type_info["data"]
>>           return struct.needs_conversion()
>>   +    def needs_unwrapping(self):
>> +        """ Structures with wrapped dispatchable handles, need 
>> unwrapping. """
>> +
>> +        if self.is_struct():
>> +            struct = self.type_info["data"]
>> +            return struct.needs_unwrapping()
>> +
>> +        if self.is_handle():
>> +            handle = self.type_info["data"]
>> +            return handle.is_wrapped()
>> +
>> +        return False
>> +
>>       def needs_free(self):
>> -        if not self.needs_conversion():
>> +        if not self.needs_conversion() and not self.needs_unwrapping():
>>               return False
>>             if self.is_dynamic_array():
>> @@ -1392,7 +1508,7 @@ class VkParam(object):
>>           self.free_func = None
>>           self.input_conv = None
>>           self.output_conv = None
>> -        if not self.needs_conversion():
>> +        if not self.needs_conversion() and not self.needs_unwrapping():
>>               return
>>             # Input functions require win to host conversion.
>> @@ -1570,7 +1686,7 @@ class VkParam(object):
>>           for m in self.struct:
>>               m.needs_struct_extensions_conversion()
>>   -        if not self.needs_conversion():
>> +        if not self.needs_conversion() and not self.needs_unwrapping():
>>               return None
>>             conversions = []
>> @@ -1582,7 +1698,7 @@ class VkParam(object):
>>               if not m.is_struct():
>>                   continue
>>   -            if not m.needs_conversion():
>> +            if not m.needs_conversion() and not m.needs_unwrapping():
>>                   continue
>>                 conversions.extend(m.get_conversions())
>> @@ -1641,6 +1757,15 @@ class VkParam(object):
>>             return False
>>   +    def needs_unwrapping(self):
>> +        """ Returns if parameter needs unwrapping of dispatchable 
>> handle. """
>> +
>> +        # Wrapped handle parameters are handled seperately, only 
>> look for wrapped handles in structs
>
> What about dynamic arrays of wrapped handles? e.g. vkCmdExecuteCommands
> I mean, this commit is already big, so if it's non-trivial to support 
> a error would be enough imo.
I added support for this in v2, didn't require too much new code.
>
>> +        if self.is_struct():
>> +            return self.struct.needs_unwrapping()
>> +
>> +        return False
>> +
>>       def needs_free(self):
>>           return self.free_func is not None
>>   @@ -1880,6 +2005,14 @@ class VkStruct(Sequence):
>>                   return True
>>           return False
>>   +    def needs_unwrapping(self):
>> +        """ Returns if struct members need unwrapping of a 
>> dispatchable handle. """
>> +
>> +        for m in self.members:
>> +            if m.needs_unwrapping():
>> +                return True
>> +        return False
>> +
>>       def needs_free(self):
>>           """ Check if any struct member needs some memory freeing."""
>>   @@ -1897,7 +2030,10 @@ class VkStruct(Sequence):
>>             for e in self.struct_extensions:
>>               if e.required and e.needs_conversion():
>> -                LOGGER.error("Unhandled pNext chain conversion for 
>> {0}".format(e.name))
>> +                LOGGER.error("Unhandled pNext chain alignment 
>> conversion for {0}".format(e.name))
>> +                ret = True
>> +            if e.required and e.needs_unwrapping():
>> +                LOGGER.error("Unhandled pNext chain unwrapping 
>> conversion for {0}".format(e.name))
>>                   ret = True
>>             return ret
>> @@ -1913,12 +2049,12 @@ class VkStruct(Sequence):
>>       class ConversionFunction(object):
>> -    def __init__(self, array, dyn_array, direction, struct):
>> +    def __init__(self, array, dyn_array, direction, operand):
>>           self.array = array
>>           self.direction = direction
>>           self.dyn_array = dyn_array
>> -        self.struct = struct
>> -        self.type = struct.name
>> +        self.operand = operand
>> +        self.type = operand.name
>>             self._set_name()
>>   @@ -1926,21 +2062,44 @@ class ConversionFunction(object):
>>           return self.name == other.name
>>         def _generate_array_conversion_func(self):
>> -        """ Helper function for generating a conversion function for 
>> array structs. """
>> +        """ Helper function for generating a conversion function for 
>> array operands. """
>> +
>> +        body = ""
>> +
>> +        if self.operand.needs_conversion():
>> +            if self.direction == Direction.OUTPUT:
>> +                params = ["const {0}_host *in".format(self.type), 
>> "uint32_t count"]
>> +                return_type = self.type
>> +            else:
>> +                params = ["const {0} *in".format(self.type), 
>> "uint32_t count"]
>> +                return_type = "{0}_host".format(self.type)
>> +
>> +            # Generate function prototype.
>> +            body += "static inline {0} *{1}(".format(return_type, 
>> self.name)
>> +            body += ", ".join(p for p in params)
>> +            body += ")\n{\n"
>> +
>> +            body += "    {0} *out;\n".format(return_type)
>> +
>> +        if self.operand.needs_unwrapping():
>> +            if self.operand.needs_conversion():
>> +                body += "#else\n"
>> +            else:
>> +                body += "#endif /* USE_STRUCT_CONVERSION */\n\n"
>>   -        if self.direction == Direction.OUTPUT:
>> -            params = ["const {0}_host *in".format(self.type), 
>> "uint32_t count"]
>> -            return_type = self.type
>> -        else:
>>               params = ["const {0} *in".format(self.type), "uint32_t 
>> count"]
>> -            return_type = "{0}_host".format(self.type)
>> +            return_type = "{0}".format(self.type)
>>   -        # Generate function prototype.
>> -        body = "static inline {0} *{1}(".format(return_type, self.name)
>> -        body += ", ".join(p for p in params)
>> -        body += ")\n{\n"
>> +            # Generate function prototype.
>> +            body += "static inline {0} *{1}(".format(return_type, 
>> self.name)
>> +            body += ", ".join(p for p in params)
>> +            body += ")\n{\n"
>> +
>> +            body += "    {0} *out;\n".format(return_type)
>> +
>> +            if self.operand.needs_conversion():
>> +                body += "#endif /* USE_STRUCT_CONVERSION */\n"
>>   -        body += "    {0} *out;\n".format(return_type)
>>           body += "    unsigned int i;\n\n"
>>           body += "    if (!in) return NULL;\n\n"
>>   @@ -1949,10 +2108,25 @@ class ConversionFunction(object):
>>           body += "    for (i = 0; i < count; i++)\n"
>>           body += "    {\n"
>>   -        for m in self.struct:
>> -            # TODO: support copying of pNext extension structures!
>> -            # Luckily though no extension struct at this point needs 
>> conversion.
>> -            body += "        " + m.copy("in[i].", "out[i].", 
>> self.direction)
>> +        if isinstance(self.operand, VkStruct):
>> +            for m in self.operand:
>> +                # TODO: support copying of pNext extension structures!
>> +                # Luckily though no extension struct at this point 
>> needs conversion.
>> +                convert = m.copy("in[i].", "out[i].", 
>> self.direction, True)
>> +                if self.operand.needs_conversion() and not 
>> self.operand.needs_unwrapping():
>> +                    body += "        " + convert
>> +                else:
>> +                    unwrap = m.copy("in[i].", "out[i].", 
>> self.direction, False)
>> +                    if unwrap == convert:
>> +                        body += "        " + unwrap
>> +                    else:
>> +                        body += "#if 
>> defined(USE_STRUCT_CONVERSION)\n        {0}#else\n {1}#endif /* 
>> USE_STRUCT_CONVERSION */\n".format(convert, unwrap)
>> +
>> +        elif isinstance(self.operand, VkHandle) and self.direction 
>> == Direction.INPUT:
>> +            body += "        out[i] = " + 
>> self.operand.driver_handle("in[i]") + ";\n"
>> +        else:
>> +            LOGGER.warn("Unhandled conversion operand type")
>> +            body += "        out[i] = in[i];\n"
>>             body += "    }\n\n"
>>           body += "    return out;\n"
>> @@ -1960,22 +2134,47 @@ class ConversionFunction(object):
>>           return body
>>         def _generate_conversion_func(self):
>> -        """ Helper function for generating a conversion function for 
>> non-array structs. """
>> +        """ Helper function for generating a conversion function for 
>> non-array operands. """
>>   -        if self.direction == Direction.OUTPUT:
>> -            params = ["const {0}_host *in".format(self.type), "{0} 
>> *out".format(self.type)]
>> -        else:
>> -            params = ["const {0} *in".format(self.type), "{0}_host 
>> *out".format(self.type)]
>> +        # It doesn't make sense to generate conversion functions for 
>> non-struct variables
>> +        # which aren't in arrays, as this should be handled by the 
>> copy() function
>> +        if not isinstance(self.operand, VkStruct):
>> +            return ""
>> +
>> +        body = ""
>
> This is redundant.
👌
>
>>   -        body = "static inline void {0}(".format(self.name)
>> +        if self.operand.needs_conversion():
>> +            body += "static inline void {0}(".format(self.name)
>>   -        # Generate parameter list
>> -        body += ", ".join(p for p in params)
>> -        body += ")\n{\n"
>> +            if self.direction == Direction.OUTPUT:
>> +                params = ["const {0}_host *in".format(self.type), 
>> "{0} *out".format(self.type)]
>> +            else:
>> +                params = ["const {0} *in".format(self.type), 
>> "{0}_host *out".format(self.type)]
>>   -        body += "    if (!in) return;\n\n"
>> +            # Generate parameter list
>> +            body += ", ".join(p for p in params)
>> +            body += ")\n"
>>   -        if self.direction == Direction.INPUT and "pNext" in 
>> self.struct and self.struct.returnedonly:
>> +        if self.operand.needs_unwrapping():
>> +            if self.operand.needs_conversion():
>> +                body += "#else\n"
>> +            else:
>> +                body += "#endif /* USE_STRUCT_CONVERSION */\n\n"
>> +
>> +            body += "static inline void {0}(".format(self.name)
>> +
>> +            params = ["const {0} *in".format(self.type), "{0} 
>> *out".format(self.type)]
>> +
>> +            # Generate parameter list
>> +            body += ", ".join(p for p in params)
>> +            body += ")\n"
>> +
>> +            if self.operand.needs_conversion():
>> +                body += "#endif /* USE_STRUCT_CONVERSION */\n"
>> +
>> +        body += "{\n    if (!in) return;\n\n"
>> +
>> +        if self.direction == Direction.INPUT and "pNext" in 
>> self.operand and self.operand.returnedonly:
>>               # We are dealing with an input_output parameter. For 
>> these we only need to copy
>>               # pNext and sType as the other fields are filled in by 
>> the host. We do potentially
>>               # have to iterate over pNext and perform conversions 
>> based on switch(sType)!
>> @@ -1984,36 +2183,84 @@ class ConversionFunction(object):
>>               body += "    out->pNext = in->pNext;\n"
>>               body += "    out->sType = in->sType;\n"
>>           else:
>> -            for m in self.struct:
>> +            for m in self.operand:
>>                   # TODO: support copying of pNext extension structures!
>> -                body += "    " + m.copy("in->", "out->", 
>> self.direction)
>> +                convert = m.copy("in->", "out->", self.direction, True)
>> +                if self.operand.needs_conversion() and not 
>> self.operand.needs_unwrapping():
>> +                    body += "    " + convert
>> +                else:
>> +                    unwrap = m.copy("in->", "out->", self.direction, 
>> False)
>> +                    if unwrap == convert:
>> +                        body += "    " + unwrap
>> +                    else:
>> +                        body += "#if 
>> defined(USE_STRUCT_CONVERSION)\n   {0}#else\n    {1}#endif /* 
>> USE_STRUCT_CONVERSION */\n".format(convert, unwrap)
>>             body += "}\n\n"
>> +
>> +        if self.operand.needs_unwrapping() and not 
>> self.operand.needs_free():
>> +            body += "#if defined(USE_STRUCT_CONVERSION)\n"
>> +
>>           return body
>>         def _generate_static_array_conversion_func(self):
>> -        """ Helper function for generating a conversion function for 
>> array structs. """
>> +        """ Helper function for generating a conversion function for 
>> array operands. """
>>   -        if self.direction == Direction.OUTPUT:
>> -            params = ["const {0}_host *in".format(self.type), "{0} 
>> *out".format(self.type), "uint32_t count"]
>> -        else:
>> -            params = ["const {0} *in".format(self.type), "{0} 
>> *out_host".format(self.type), "uint32_t count"]
>> +        body = ""
>>   -        # Generate function prototype.
>> -        body = "static inline void {0}(".format(self.name)
>> -        body += ", ".join(p for p in params)
>> -        body += ")\n{\n"
>> +        if self.operand.needs_conversion():
>> +            if self.direction == Direction.OUTPUT:
>> +                params = ["const {0}_host *in".format(self.type), 
>> "{0} *out".format(self.type), "uint32_t count"]
>> +            else:
>> +                params = ["const {0} *in".format(self.type), "{0} 
>> *out_host".format(self.type), "uint32_t count"]
>> +
>> +            # Generate function prototype.
>> +            body += "static inline void {0}(".format(self.name)
>> +            body += ", ".join(p for p in params)
>> +            body += ")\n"
>> +
>> +        if self.operand.needs_unwrapping():
>> +            if self.operand.needs_conversion():
>> +                body += "#else\n"
>> +            else:
>> +                body += "#endif /* USE_STRUCT_CONVERSION */\n\n"
>> +
>> +            params = ["const {0} *in".format(self.type), "{0} 
>> *out".format(self.type), "uint32_t count"]
>> +
>> +            # Generate function prototype.
>> +            body += "static inline void {0}(".format(self.name)
>> +            body += ", ".join(p for p in params)
>> +            body += ")\n"
>> +
>> +        body += "{\n"
>>           body += "    unsigned int i;\n\n"
>>           body += "    if (!in) return;\n\n"
>>           body += "    for (i = 0; i < count; i++)\n"
>>           body += "    {\n"
>>   -        for m in self.struct:
>> -            # TODO: support copying of pNext extension structures!
>> -            body += "        " + m.copy("in[i].", "out[i].", 
>> self.direction)
>> +        if isinstance(self.operand, VkStruct):
>> +            for m in self.operand:
>> +                # TODO: support copying of pNext extension structures!
>> +                convert = m.copy("in[i].", "out[i].", 
>> self.direction, True)
>> +                if self.operand.needs_conversion() and not 
>> self.operand.needs_unwrapping():
>> +                    body += "        " + convert
>> +                else:
>> +                    unwrap = m.copy("in[i].", "out[i].", 
>> self.direction, False)
>> +                    if unwrap == convert:
>> +                        body += "        " + unwrap
>> +                    else:
>> +                        body += "#if 
>> defined(USE_STRUCT_CONVERSION)\n   {0}#else\n    {1}#endif /* 
>> USE_STRUCT_CONVERSION */\n".format(convert, unwrap)
>> +        elif isinstance(self.operand, VkHandle) and self.direction 
>> == Direction.INPUT:
>> +            body += "        out[i] = " + 
>> self.operand.driver_handle("in[i]") + ";\n"
>> +        else:
>> +            LOGGER.warn("Unhandled conversion operand type")
>> +            body += "        out[i] = in[i];\n"
>>             body += "    }\n"
>>           body += "}\n\n"
>> +
>> +        if self.operand.needs_unwrapping():
>> +            body += "#if defined(USE_STRUCT_CONVERSION)\n\n"
>> +
>>           return body
>>         def _set_name(self):
>> @@ -2044,10 +2291,10 @@ class ConversionFunction(object):
>>       class FreeFunction(object):
>> -    def __init__(self, dyn_array, struct):
>> +    def __init__(self, dyn_array, operand):
>>           self.dyn_array = dyn_array
>> -        self.struct = struct
>> -        self.type = struct.name
>> +        self.operand = operand
>> +        self.type = operand.name
>>             if dyn_array:
>>               self.name = "free_{0}_array".format(self.type)
>> @@ -2060,25 +2307,47 @@ class FreeFunction(object):
>>       def _generate_array_free_func(self):
>>           """ Helper function for cleaning up temporary buffers 
>> required for array conversions. """
>>   -        # Generate function prototype.
>> -        body = "static inline void {0}({1}_host *in, uint32_t 
>> count)\n{{\n".format(self.name, self.type)
>> +        body = ""
>> +
>> +        if self.operand.needs_conversion():
>> +            if self.operand.needs_unwrapping():
>> +                body += "#if defined(USE_STRUCT_CONVERSION)\n"
>> +
>> +            # Generate function prototype.
>> +            body += "static inline void {0}({1}_host *in, uint32_t 
>> count)\n".format(self.name, self.type)
>> +
>> +        if self.operand.needs_unwrapping():
>> +            if self.operand.needs_conversion():
>> +                body += "#else\n"
>> +
>> +            # Generate function prototype.
>> +            body += "static inline void {0}({1} *in, uint32_t 
>> count)\n".format(self.name, self.type)
>> +
>> +            if self.operand.needs_conversion():
>> +                body += "#endif /* USE_STRUCT_CONVERSION */\n"
>> +
>> +        body += "{\n"
>>             # E.g. VkGraphicsPipelineCreateInfo_host needs freeing 
>> for pStages.
>> -        if self.struct.needs_free():
>> +        if isinstance(self.operand, VkStruct) and 
>> self.operand.needs_free():
>>               body += "    unsigned int i;\n\n"
>>               body += "    if (!in) return;\n\n"
>>               body += "    for (i = 0; i < count; i++)\n"
>>               body += "    {\n"
>>   -            for m in self.struct:
>> -                if m.needs_conversion() and m.is_dynamic_array():
>> -                    if m.is_const():
>> -                        # Add a cast to ignore const on conversion 
>> structs we allocated ourselves.
>> -                        body += " free_{0}_array(({0}_host 
>> *)in[i].{1}, in[i].{2});\n".format(m.type, m.name, m.dyn_array_len)
>> +            for m in self.operand:
>> +                if m.needs_free():
>> +                    convert = m.free("in[i].", True)
>> +                    if self.operand.needs_conversion() and not 
>> self.operand.needs_unwrapping():
>> +                        body += "        " + convert
>>                       else:
>> -                        body += " free_{0}_array(in[i].{1}, 
>> in[i].{2});\n".format(m.type, m.name, m.dyn_array_len)
>> -                elif m.needs_conversion():
>> -                    LOGGER.error("Unhandled conversion for 
>> {0}".format(m.name))
>> +                        unwrap = m.free("in[i].", False)
>> +                        if convert == unwrap:
>> +                            body += "        " + unwrap
>> +                        elif unwrap == "":
>> +                            body += "#if 
>> defined(USE_STRUCT_CONVERSION)\n        {0}#endif /* 
>> USE_STRUCT_CONVERSION */\n".format(convert)
>> +                        else:
>> +                            body += "#if 
>> defined(USE_STRUCT_CONVERSION)\n        {0}#else\n {1}#endif /* 
>> USE_STRUCT_CONVERSION */\n".format(convert, unwrap)
>>               body += "    }\n"
>>           else:
>>               body += "    if (!in) return;\n\n"
>> @@ -2086,26 +2355,44 @@ class FreeFunction(object):
>>           body += "    free(in);\n"
>>             body += "}\n\n"
>> +
>> +        if self.operand.needs_unwrapping():
>> +            body += "#if defined(USE_STRUCT_CONVERSION)\n"
>> +
>>           return body
>>         def _generate_free_func(self):
>>           # E.g. VkCommandBufferBeginInfo.pInheritanceInfo needs 
>> freeing.
>> -        if not self.struct.needs_free():
>> +        if not self.operand.needs_free():
>>               return ""
>>   +        if not isinstance(self.operand, VkStruct):
>> +            return ""
>> +
>> +        body = ""
>> +
>>           # Generate function prototype.
>> -        body = "static inline void {0}({1}_host 
>> *in)\n{{\n".format(self.name, self.type)
>> +        body += "static inline void {0}({1}_host 
>> *in)\n{{\n".format(self.name, self.type)
>>   -        for m in self.struct:
>> -            if m.needs_conversion() and m.is_dynamic_array():
>> -                count = m.dyn_array_len if 
>> isinstance(m.dyn_array_len, int) else "in->{0}".format(m.dyn_array_len)
>> -                if m.is_const():
>> -                    # Add a cast to ignore const on conversion 
>> structs we allocated ourselves.
>> -                    body += "    free_{0}_array(({0}_host *)in->{1}, 
>> {2});\n".format(m.type, m.name, count)
>> +        for m in self.operand:
>> +            if m.needs_free():
>> +                convert = m.free("in->", True)
>> +                if self.operand.needs_conversion() and not 
>> self.operand.needs_unwrapping():
>> +                    body += "    " + convert
>>                   else:
>> -                    body += "    free_{0}_array(in->{1}, 
>> {2});\n".format(m.type, m.name, count)
>> +                    unwrap = m.free("in->", False)
>> +                    if convert == unwrap:
>> +                        body += "    " + unwrap
>> +                    elif unwrap == "":
>> +                        body += "#if 
>> defined(USE_STRUCT_CONVERSION)\n    {0}#endif /* 
>> USE_STRUCT_CONVERSION */\n".format(convert)
>> +                    else:
>> +                        body += "#if 
>> defined(USE_STRUCT_CONVERSION)\n    {0}#else\n    {1}#endif /* 
>> USE_STRUCT_CONVERSION */\n".format(convert, unwrap)
>>             body += "}\n\n"
>> +
>> +        if self.operand.needs_unwrapping():
>> +            body += "#if defined(USE_STRUCT_CONVERSION)\n"
>> +
>>           return body
>>         def definition(self):
>> @@ -2168,7 +2455,12 @@ class StructChainConversionFunction(object):
>>                   if m.name == "pNext":
>>                       body += "            out->pNext = NULL;\n"
>>                   else:
>> -                    body += "            " + m.copy("in->", "out->", 
>> self.direction)
>> +                    convert = m.copy("in->", "out->", 
>> self.direction, True)
>> +                    unwrap = m.copy("in->", "out->", self.direction, 
>> False)
>> +                    if unwrap == convert:
>> +                        body += "            " + unwrap
>> +                    else:
>> +                        body += "#if 
>> defined(USE_STRUCT_CONVERSION)\n {0}#else\n            {1}#endif /* 
>> USE_STRUCT_CONVERSION */\n".format(convert, unwrap)
>>                 body += "\n            out_header->pNext = 
>> (VkBaseOutStructure *)out;\n"
>>               body += "            out_header = out_header->pNext;\n"
>> @@ -2212,7 +2504,41 @@ class FreeStructChainFunction(object):
>>             body += "    while (header)\n"
>>           body += "    {\n"
>> -        body += "        void *prev = header;\n"
>> +        body += "        void *prev = header;\n\n"
>> +        body += "        switch (header->sType)\n"
>> +        body += "        {\n"
>> +
>> +        for e in self.struct.struct_extensions:
>> +            if not e.required:
>> +                continue
>> +
>> +            if not any(m.needs_free() for m in e):
>> +                continue
>> +
>> +            stype = next(x for x in e.members if x.name == "sType")
>> +
>> +            body += "            case {0}:\n".format(stype.values)
>> +            body += "            {\n"
>> +            body += "                {0} *structure = ({0} *) 
>> header;\n".format(e.name)
>> +
>> +            for m in e:
>> +                if m.needs_free():
>> +                    convert = m.free("structure->", True)
>> +                    unwrap = m.free("structure->", False)
>> +                    if convert == unwrap:
>> +                        body += "                " + unwrap
>> +                    elif unwrap == "":
>> +                        body += "#if 
>> defined(USE_STRUCT_CONVERSION)\n                {0}#endif /* 
>> USE_STRUCT_CONVERSION */\n".format(convert)
>> +                    else:
>> +                        body += "#if 
>> defined(USE_STRUCT_CONVERSION)\n {0}#else\n                {1}#endif 
>> /* USE_STRUCT_CONVERSION */\n".format(convert, unwrap)
>> +
>> +            body += "                break;\n"
>> +            body += "            }\n"
>> +
>> +        body += "            default:\n"
>> +        body += "                break;\n"
>> +        body += "        }\n"
>> +
>>           body += "        header = header->pNext;\n"
>>           body += "        free(prev);\n"
>>           body += "    }\n\n"
>
>
> I think the way preprocessor #ifs are handled in free and conversion 
> functions is confusing. Every function handle it on their own.
>
> e.g. if it does both conversion and unwrapping:
>
> #if defined(USE_STRUCT_CONVERSION)
> returnValue_host convert_to_host(params)
> #else
> returnValue convert_to_host(params)
> #endif
> {
>   //body
> }
>
> or if it does only alignment conversion
>
> #if defined(USE_STRUCT_CONVERSION)
> returnValue_host convert_to_host(params)
> {
>   //body
> }
> #endif
>
> or if it only does unwrapping
>
> returnValue convert_to_host(params)
> {
>   //body
> }
>
> Currently, there's a #if defined(USE_STRUCT_CONVERSION) and #endif 
> written in generate_thunks_c, which should then be removed.
👌
>
>> @@ -2235,7 +2561,7 @@ class VkGenerator(object):
>>               if not func.is_required():
>>                   continue
>>   -            if not func.needs_conversion():
>> +            if not func.needs_conversion() and not 
>> func.needs_unwrapping():
>>                   continue
>>                 conversions = func.get_conversions()
>> @@ -2246,15 +2572,26 @@ class VkGenerator(object):
>>                       if not any(c == conv for c in self.conversions):
>>                           self.conversions.append(conv)
>>   +                if not isinstance(conv.operand, VkStruct):
>> +                    continue
>> +
>>                   # Structs can be used in different ways by 
>> different conversions
>>                   # e.g. array vs non-array. Just make sure we pull 
>> in each struct once.
>> -                if not any(s.name == conv.struct.name for s in 
>> self.host_structs):
>> -                    self.host_structs.append(conv.struct)
>> +                if not any(s.name == conv.operand.name for s in 
>> self.host_structs):
>> +                    self.host_structs.append(conv.operand)
>>             for struct in self.registry.structs:
>>               if struct.name in STRUCT_CHAIN_CONVERSIONS:
>> self.struct_chain_conversions.append(StructChainConversionFunction(Direction.INPUT, 
>> struct))
>> self.struct_chain_conversions.append(FreeStructChainFunction(struct))
>> +                # Once we decide to support pNext chains conversion 
>> everywhere, move this under get_conversions
>> +                for e in struct.struct_extensions:
>> +                    for m in e:
>> +                        if m.needs_conversion() or 
>> m.needs_unwrapping():
>> +                            conversions = m.get_conversions()
>> +                            for conv in conversions:
>> +                                if not any(c == conv for c in 
>> self.conversions):
>> + self.conversions.append(conv)
>>         def _generate_copyright(self, f, spec_file=False):
>>           f.write("# " if spec_file else "/* ")
>> diff --git a/dlls/winevulkan/vulkan.c b/dlls/winevulkan/vulkan.c
>> index 45eda78e997..58a67c6d285 100644
>> --- a/dlls/winevulkan/vulkan.c
>> +++ b/dlls/winevulkan/vulkan.c
>> @@ -1540,19 +1540,6 @@ 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_vkCreateSwapchainKHR(VkDevice device, const 
>> VkSwapchainCreateInfoKHR *create_info,
>> -        const VkAllocationCallbacks *allocator, VkSwapchainKHR 
>> *swapchain)
>> -{
>> -    VkSwapchainCreateInfoKHR native_info;
>> -
>> -    TRACE("%p, %p, %p, %p\n", device, create_info, allocator, 
>> swapchain);
>> -
>> -    native_info = *create_info;
>> -    native_info.surface = 
>> wine_surface_from_handle(create_info->surface)->driver_surface;
>> -
>> -    return thunk_vkCreateSwapchainKHR(device, &native_info, 
>> allocator, swapchain);
>> -}
>> -
>>   VkResult WINAPI wine_vkCreateWin32SurfaceKHR(VkInstance instance,
>>           const VkWin32SurfaceCreateInfoKHR *createInfo, const 
>> VkAllocationCallbacks *allocator, VkSurfaceKHR *surface)
>>   {
>> @@ -1601,19 +1588,6 @@ void WINAPI 
>> wine_vkDestroySurfaceKHR(VkInstance instance, VkSurfaceKHR surface,
>>       free(object);
>>   }
>>   -VkResult WINAPI 
>> wine_vkGetPhysicalDeviceSurfaceFormats2KHR(VkPhysicalDevice phys_dev,
>> -        const VkPhysicalDeviceSurfaceInfo2KHR *surface_info, 
>> uint32_t *formats_count, VkSurfaceFormat2KHR *formats)
>> -{
>> -    VkPhysicalDeviceSurfaceInfo2KHR native_info;
>> -
>> -    TRACE("%p, %p, %p, %p\n", phys_dev, surface_info, formats_count, 
>> formats);
>> -
>> -    native_info = *surface_info;
>> -    native_info.surface = 
>> wine_surface_from_handle(surface_info->surface)->driver_surface;
>> -
>> -    return thunk_vkGetPhysicalDeviceSurfaceFormats2KHR(phys_dev, 
>> &native_info, formats_count, formats);
>> -}
>> -
>>   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
>> @@ -1647,15 +1621,11 @@ VkResult WINAPI 
>> wine_vkGetPhysicalDeviceSurfaceCapabilitiesKHR(VkPhysicalDevice
>>   VkResult WINAPI 
>> wine_vkGetPhysicalDeviceSurfaceCapabilities2KHR(VkPhysicalDevice 
>> phys_dev,
>>           const VkPhysicalDeviceSurfaceInfo2KHR *surface_info, 
>> VkSurfaceCapabilities2KHR *capabilities)
>>   {
>> -    VkPhysicalDeviceSurfaceInfo2KHR native_info;
>>       VkResult res;
>>         TRACE("%p, %p, %p\n", phys_dev, surface_info, capabilities);
>>   -    native_info = *surface_info;
>> -    native_info.surface = 
>> wine_surface_from_handle(surface_info->surface)->driver_surface;
>> -
>> -    res = thunk_vkGetPhysicalDeviceSurfaceCapabilities2KHR(phys_dev, 
>> &native_info, capabilities);
>> +    res = thunk_vkGetPhysicalDeviceSurfaceCapabilities2KHR(phys_dev, 
>> surface_info, capabilities);
>>         if (res == VK_SUCCESS)
>>           adjust_max_image_count(phys_dev, 
>> &capabilities->surfaceCapabilities);
>>
>
> You also have to delete the manual unwrapping of VkPhysicalDevice in 
> wine_vk_device_convert_create_info and wine_vk_device_free_create_info.
👌
>
> If vkQueueSubmit is changed to use a public thunk (which should work 
> now), convert_VkCommandBuffer_array_win_to_host and 
> free_VkCommandBuffer_array are used but not defined.
Fixed in v2.
>
> Thanks,
>
> Georg



More information about the wine-devel mailing list