[Mesa-dev] [PATCH 1/5] include/GL: add mesa_glinterop.h for OpenGL-OpenCL interop (v3)

[Emil Velikov]

On 9 March 2016 at 18:36, Marek Olšák <maraeo at gmail.com> wrote:
> On Wed, Mar 9, 2016 at 6:51 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>> On 9 March 2016 at 17:11, Marek Olšák <maraeo at gmail.com> wrote:
>>> On Wed, Mar 9, 2016 at 4:31 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>>>> On 8 March 2016 at 22:29, Marek Olšák <maraeo at gmail.com> wrote:
>>>>
>>>>> Actually, I don't see how the version number would make it any better
>>>>> for the structures, but returning the version number by
>>>>> QueryDeviceInfo would be useful for the caller to know what to expect
>>>>> if Mesa version < caller version. The sizes are still useful if Mesa
>>>>> version > caller version.
>>>>>
>>>> If any of this is an issue, then the whole DRI model just won't work ;-)
>>>
>>> The DRI extension versions only determine the number of function
>>> callbacks, not function parameters and return values. This interop
>>> thing is a lot more complicated than that, since it allows the "in"
>>> and "out" structures to grow, and different rules apply to each.
>> The fact that the DRI extension version is used only to determine the
>> presence of certain function, is implementation detail imho.
>>
>> If you look at the struct as a whole, it doesn't matter what the
>> contents (part the version field) are. For all one care there could be
>> none ?
>>
>>> Also,
>>> the implementation of DRI extensions allocates the structures, while
>>> in the interop the user allocates the structures. It's a totally
>>> different model.
>>>
>> True, it's a slightly different model. The following should still work ?
>>
>> Library A: allocates memory for the struct, and set the currently
>> maximum supported version
>> Library B: retrieves the 'empty' struct. Checks which version is lower
>> (self and the one in the struct) and only populates for up-to that
>> version.
>>
>>>>
>>>> I'm thinking that the following should work. Please let me know if I'm
>>>> loosing the plot.
>>>>
>>>> Caller sets the structure and sets version of the interface it
>>>> provides. Then callee first checks if it can work with the provider
>>>> version. then proceeds as planned.
>>>
>>> The callee must always proceed even if the version is too low or too
>>> high.
>> What do you mean with "proceed" here ?
>>
>> Are you saying that even if the callee can work with up-to v2, it
>> should somehow populate the v3 fields ?
>> Shouldn't it use the lowest version of the two and only handle those fields ?
>
> A v1 callee can only read and write v1 fields.
> A v2 callee can only read and write v2 fields, but will only write v1
> fields if the size doesn't include v2 fields.
> I could go on.
>
> The callee doesn't care about the caller's version at all.
>
With the last sentence removed, that's roughly what I was saying.

The library that allocates the struct, sets the layout version ('size'
if you prefer). Then both libraries can read/write fields up-to the
smaller version (size) between the one already set in the struct and
the one they support. To make is more prominent one can explicitly
mark the version as const and add a small comment that only the
function which allocates the struct is responsible for setting the
variable. This last sentence also applies for 'size' fwiw.

Out of curiosity: did you draw some inspiration about using 'size'
based on some other interfaces ? I'm sorry but they serve no benefit,
plus the implementation requires some nasty code to work with them.

>>
>>> The caller knows the Mesa interop version from QueryDeviceInfo
>>> (defined in v4 and later) and knows what it should expect. Mesa only
>>> checks the sizes, but otherwise doesn't care about the caller's
>>> version.
>>>
>>>> Passing around multiple sizes is ugly and error prone to a point. One
>>>> gets the order wrong (swaps in_size and out_size) or just the same
>>>> sizeof(struct foo) in both places.
>>>
>>> The order of parameters in v5 is: in_size, *in, out_size, *out
>>
>>> If the implementation swaps in_size and out_size, it's no my problem.
>> No offence, but you realise that this sounds rather funny, right ?
>> In the context of designing interfaces, of course.
>
> It's not my problem for the same reason that swapping src and dst in
> memcpy is not glibc's problem. We have to draw the line somewhere. :)
>
Was talking about the sizes, not the in/out pointers. There is a
subtle difference with the possible side-falls and their
detection/prevention.

Regards,
Emil