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

[Marek Olšák]

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.

>
> 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.

>
>
>> +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.

Marek