[Mesa-dev] [PATCH 04/38] drivers/dri: set the implemented version of __DRItexBufferExtension

[Zhao, Juan J]

>-----Original Message-----
>From: Emil Velikov [mailto:emil.l.velikov at gmail.com]
>Sent: Tuesday, February 18, 2014 7:29 AM
>To: Ilia Mirkin; Zhao, Juan J
>Cc: emil.l.velikov at gmail.com; mesa-dev at lists.freedesktop.org
>Subject: Re: [Mesa-dev] [PATCH 04/38] drivers/dri: set the implemented
>version of __DRItexBufferExtension
>
>On 13/02/14 03:03, Ilia Mirkin wrote:
>> On Wed, Feb 12, 2014 at 9:56 PM, Emil Velikov
><emil.l.velikov at gmail.com> wrote:
>>> On 13/02/14 02:47, Emil Velikov wrote:
>>>> On 13/02/14 02:34, Ilia Mirkin wrote:
>>>>> On Wed, Feb 12, 2014 at 8:17 PM, Emil Velikov
><emil.l.velikov at gmail.com> wrote:
>>>>>> ... over the version number provided by the headers.
>>>>>> Explicitly set extension members to improve clarity.
>>>>>
>>>>> So... the current value is 2, but I see code that does
>>>>>
>>>>> src/egl/drivers/dri2/egl_dri2.c:#if __DRI_TEX_BUFFER_VERSION >= 3
>>>>> src/glx/dri2_glx.c:#if __DRI_TEX_BUFFER_VERSION >= 3
>>>>> src/glx/drisw_glx.c:#if __DRI_TEX_BUFFER_VERSION >= 3
>>>>>
>>>>> What's that all about?
>>>>>
>>>> Chaos wrt documentation and keeping up with it.
>>>>
>>>> Until git log churns this is the only reasonable thing that I can
>think of:
>>>>
>>>>  - releaseTexBuffer was added with version 3 in mind.
>>>>  - no driver implemented the function
>>>>  - the version was never bumped or reverted.
>>>>
>>> Looks like cvs (or whatever was used back in 2009) failure :\
>>>
>>> Eric introduced v2 with commit 66175aac760, bumping the version 1>2
>>> Ian made an update with commit 82634ee8df7, bumping the version 1>2
>>
>> Meh, that's a merge-fail from a long time ago.
>>
>>>
>>> I'll update the header but I'll keep version 2 everywhere (dropping
>>> the null member).
>>
>> releaseTexBuffer was actually introduced with e59fa4c46c8 ("dri2:
>> release texture image."), which also added 2 of the #if's. I'm
>> guessing the third got copy/pasted in.
>>
>Good catch, while the commit effectively adds a new revision of the
>extension if avoids bumping the version number.
>
>At the same time it adds a glx/egl dri2 implementations wrapped in "if
>__DRI_TEX_BUFFER_VERSION >=3". Bumping the version in the header, will
>result in broken build (2) due to missing _eglReleaseTexImage and
>possibly others. With the build issues resolved, the function have some
>runtime cost, despite that no driver implements
>__DRItexBufferExtensionRec::releaseTexBuffer.
>
>
>Juan Zhao
>
>Can you provide some clarity on the case ? Namely with e59fa4c46c8
>("dri2: release texture image.") you've updated the
>__DRItexBufferExtensionRec but the code you've added seems untested.
>
> - Did you deliberately omitted bumping the __DRI_TEX_BUFFER_VERSION,
>what is the idea behind it.
>
> - Is there any plans on implementing _eglReleaseTexImage that is
>required in order to get egl_dri2 building/working ?
>
>IMHO we can safely bump the version in the header, and comment out/drop
>the broken code if no-one is going to step up and clean it.

IIRC, it's to enable texture from pixmap for the TV platform.
It's a patch on 2011, sorry, I cannot remember the details quite well for this patch.
Please feel free to make your contribution to make it better.

Best Regards,
Juan

>
>Thanks
>-Emil
>
>
>>   -ilia
>>