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

[Emil Velikov]

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
be some sanity checking, wouldn't to be better to have size (version
and/or other) within that structure format.

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.


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

Thanks
Emil