On 24/01/17 00:50, Jason Ekstrand wrote:
On Mon, Jan 23, 2017 at 4:25 PM, Chad Versace <[email protected] <mailto:[email protected]>> wrote:

    On Mon 23 Jan 2017, Jason Ekstrand wrote:
    > On Mon, Jan 23, 2017 at 3:31 PM, Chad Versace
    <[email protected] <mailto:[email protected]>> wrote:
    >
    >     On Mon 23 Jan 2017, Jason Ekstrand wrote:
    >     > On Mon, Jan 23, 2017 at 2:28 PM, Chad Versace
    <[email protected] <mailto:[email protected]>>
    >     wrote:
    >     >
    >     >     Implement each vkFoo2KHR() by trivially passing it
    through to the
    >     >     original vkFoo().
    >     >
    >     >
    >     > As I mentioned to Lionel when he wrote basically this
    exact same patch, I
    >     think
    >     > that may be backwards.  I can see two ways of doing this
    long-term:
    >
    >     If we look into the future, my patch is indeed backwards.
    >     >
    >     > 1) Implement all of the queries (of a particular type) in
    a single
    >     function and
    >     > the legacy query calls the query2 variant and then copies
    the data over.
    >
    >     Option 1 is definitely better than my patch.
    >
    >     > 2) Implement each query as its own function and the
    queries2 function
    >     loops
    >     > over the data structures calling the appropriate function
    on each one.
    >
    >     I don't see exactly what you're proposing in option 2. Do
    you mean, for
    >     example,
    >     that vkGetPhysicalDeviceFormatProperties2KHR() would, for
    each structure
    >     chained off of the input and output structs, including the
    toplevel
    >     structs themselves, call some function specific to those
    structs?
    >
    >
    > I mean it would be
    >
    > for (struct_base *s = pPhysicalDeviceProperties; s; s = s->pNext) {
    >    switch (s->type) {
    >    case VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_PROPERTIES2: {
    >       VkPhysicalDeviceProperties2 *props = s;
    >       anv_GetPhysicalDeviceProperties(pdev, &props->props);
    >       break;
    >    case VK_STRUCTURE_TYPE_SOMETHING_ELSE:
    >       VkSomethingElse *thing = s;
    >       anv_get_something_else(pdev, thing);
    >       break;
    >    ...
    >    default:
    >       assert(!"Invalid structure type");
    >    }
    > }

    All vkGetFoo2KHR() funcs have output structs; only a subset have input
    structs. Therefore, if we choose to do option 2, for uniformity's sake
    we should implement it by iterating over the output structs, even when
    input structs are present.

    What do you think?


You bring up an interesting point. I'm wondering if we don't want to do the helper thing and also pass the query info struct to all of the helpers. If they want to pull information out of chained children, it's their job to crawl through and find them. Otherwise, we would have to come up with some sort of weird double-iterator and I can't imagine that ending well.

The more I think about this, the more convinced I become that we want a helper per chaining query so maybe your patch is actually ok modulo adding some for loops when it comes time to extend one of the queries. I think I'd be a fan of adding the for loops now though.

    Also, about that assertion in the default case... I believe
    drivers are
    required to ignore unrecongized extension structs. From the Vulkan
    1.0.38 spec:

       Any component of the implementation (the loader, any enabled
    layers,
       and drivers) must skip over, without processing (other than
    reading the
       sType and pNext members) any chained structures with sType
    values not
       defined by extensions supported by that component.


Right...


What do you think about https://patchwork.freedesktop.org/patch/134841/ ?
Is that close enough to what you have in mind?

Thanks,

-
Lionel
_______________________________________________
mesa-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to