[Mesa-dev] [PATCH v4 (part2) 53/59] glapi: add ARB_shader_storage_block_buffer_object

[Emil Velikov]

On 25 August 2015 at 06:42, Samuel Iglesias Gonsálvez
<siglesias at igalia.com> wrote:
> On 24/08/15 16:10, Emil Velikov wrote:
>> Hi Samuel, Iago,
>>
>> On 05/08/15 09:30, Iago Toral Quiroga wrote:
>>> From: Samuel Iglesias Gonsalvez <siglesias at igalia.com>
>>>
>>> v2:
>>> - Add ShaderStorageBlockBinding to static_data.py
>>>
>> Why (see comment below) ?
>>
>>
>> v4:
>>  - Ship ARB_shader_storage_buffer_object.xml in the tarball.
>>
>> Seems like other patches are also missing v4 revision history :|
>>
>
> We usually send a new version of the patch series when we have not
> received more reviews for several days/weeks
How about pinging individual patches (or a list in the series cover letter) ?

> and we have enough number
> of changes in different patches (because of reviews and/or major changes
> in master that invalidate our code).
>
Just do a follow up revision of the said patch(es) ?

> For that reason, some of our patches don't have new comments in the
> revision history.
>
Afaics not even one patch in v4 does has revision history. Please don't do that.

Another thing you can try is gradually sending patches rather than
accumulating X months of work. This way the reviewer(s) will do
(almost) all it in one go.

>>
>>> --- a/src/mapi/glapi/gen/static_data.py
>>> +++ b/src/mapi/glapi/gen/static_data.py
>>> @@ -1305,6 +1305,7 @@ functions = [
>>>     "ShaderBinary",
>>>     "ShaderSource",
>>>     "ShaderSourceARB",
>>> +   "ShaderStorageBlockBinding",
>> This list is for functions exported by libGL.so, which imho should not
>> change (ever). If there is there a program that depends on this static
>> symbol (and the binary drivers expose it), it should be noted in the
>> commit message.
>>
>
> This is a new function defined by ARB_shader_storage_buffer_object
> specification [0].
>
The spec does not say that the function should be statically available
(via libGL.so). The OpenGL ABI has some information as to which
functions should be available statically/dynamically [1]

[1] https://www.opengl.org/registry/ABI/

> I have a test program that uses that function. If I remove the
> static_data.py definition, I hit this linker error:
>
> <file>: undefined reference to `glShaderStorageBlockBinding'
>
Sounds like a bug in the app. As mentioned by Ian, use libepoxy or
alike which will fetch all the symbols using the appropriate method.

> So following what how other functions are defined, I added it to
> static_data.py.
git log shows that the list of functions hasn't changed since day one,
while at the same time other extensions were added/enabled in mesa.

Hope this answers things from a different angle rather then flocking
up a dead horse :)

Thanks
Emil