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

[Emil Velikov]

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.

Thanks
-Emil


>   -ilia
>