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

[Emil Velikov]

On 8 March 2016 at 15:39, Marek Olšák <maraeo at gmail.com> wrote:
> On Thu, Mar 3, 2016 at 11:56 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>> Hi Marek,
>>
>> A small question, and a few trivial suggestions. Hopefully I'm not too
>> late for the party.
>>
>> On 3 March 2016 at 19:46, Marek Olšák <maraeo at gmail.com> wrote:
>>
>>> +typedef struct _mesa_glinterop_device_info {
>>> +   uint32_t size; /* size of this structure */
>>> +
>> I believe Michel suggested a similar thing: Wouldn't it be better to
>> use a version one just like we do for the DRI extensions ? Many other
>> interfaces also use version, some with a combination of size, but this
>> is the first one in my experience that does only size.
>>
>>
>>> +typedef struct _mesa_glinterop_export_in {
>>
>>> +   /* Size of memory pointed to by out_driver_data. */
>>> +   uint32_t out_driver_data_size;
>>> +
>>> +   /* If the caller wants to query driver-specific data about the OpenGL
>>> +    * object, this should point to the memory where that data will be stored.
>>> +    */
>>> +   void *out_driver_data;
>> I take it that the structure and format of this data will be
>> internal/implementation specific, correct ? As on each side there will
>
> Yes.
>
>> be some sanity checking, wouldn't to be better to have size (version
>> and/or other) within that structure format.
>
> Since amdgpu isn't going to use this feature, I don't care too much about it.
>
> Having the size outside of the driver-specific structure seems safer.
>
Trying future proof things does not work nicely, most of the time.
Imho it should be added as there is a user for it.

>>
>> IMHO it's worth mentioning any of that, plus some information about
>> the lifetime expectancy of the data. Thus it's perfectly clear to the
>> user how to manage/use it.
>
> The data pointer should only be used for querying stuff from Mesa. The
> same rules as for the "out" pointer apply.
>
I think there is some misunderstanding here. I wasn't asking "Who is
going to use this data ?", but "Can they use the pointer reliably, or
should they copy the data from it before using it. Copy, because the
opposite end will discard/free the block shortly after the call". I've
seen some people referring to this as lifetime expectancy, not sure if
it's the correct terminology to use here.

Would be nice to add a comment about this in the API.

>>
>>
>>> +GLAPI int GLAPIENTRY
>>> +MesaGLInteropGLXExportObject(Display *dpy, GLXContext context,
>>> +                             mesa_glinterop_export_in *in,
>>> +                             mesa_glinterop_export_out *out);
>> Annotating EGL/GLX display and context as const is very uncommon,
>> although we should do that for 'in'. Shouldn't we ?
>
> We can do that, yes.
>
Thanks.

Emil