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

[Marek Olšák]

On Wed, Mar 9, 2016 at 6:58 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
> On 9 March 2016 at 17:28, Marek Olšák <maraeo at gmail.com> wrote:
>> On Wed, Mar 9, 2016 at 4:47 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>>> 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.
>>
>> I agree, but:
>> 1) Intel want it, so there is a future user.
>> 2) One of our OpenCL guys and I have agreed to keep it in case we need
>> in the future too.
>>
> Don't mean to sound snarky - two sentences, each consisting the word
> "future" :-)
> There was a song somewhere called "Tomorrow never comes".

OK. If Intel say they don't want it, I'll remove it.

Marek