[Mesa-dev] [PATCH v5 25/70] glsl: Add std430 related member functions to glsl_type class

[Samuel Iglesias Gonsálvez]

On 16/09/15 10:52, Samuel Iglesias Gonsálvez wrote:
[...]
>>>> As way of validation, have you tried running your modified script
>>>> against any other drivers? They may well have bugs in them as well,
>>>> but it should be possible to determine if the bug is in the script or
>>>> the other impl, should they not match up.
>>>>
>>>>   -ilia
>>>>
>>>
>>> I tested it on NVIDIA proprietary driver version 352.21. It has an issue
>>> when we query shader storage block members when they are arrays of
>>> structs and the index is different than zero -> it doesn't find them as
>>> active. For example:
>>>
>>> struct B {
>>>         vec4 a[2];
>>> }
>>>
>>> layout(std430) buffer Block {
>>>         B[2] s;
>>>         vec3 v;
>>> };
>>>
>>> NVIDIA marked v, s[0].a[0] and s[0].a[1] as active but s[1].a[0] and
>>> s[0].a[1] as inactive even when they are referenced in main().
>>>
> 
> ... but s[1].a[0] and s[*1*].a[1] as inactive ...
> 
>>> I have not checked yet what ARB_program_interface_query spec says
>>> regarding to that to see if it is a bug in our driver or in NVIDIA's.
>>>
>>> However, I modified by hand the failed autogenerated tests removing the
>>> queries to those "inactive" variables. As we are interested in offsets,
>>> arrays strides and so on, polling index zero of all the top level array
>>> of struct members is enough. With that modifications, those tests pass.
>>>
>>> I plan to try to adapt my script to NVIDIA behavior so I can validate it
>>> properly. However, these preliminary results indicate that I don't have
>>> made any big mistake.
>>
>> From the spec:
>>
>>       * For an active variable declared as an array of basic types, a single
>>         entry will be generated, with its name string formed by concatenating
>>         the name of the array and the string "[0]".
>>
>>       * For an active shader storage block member declared as an array, an
>>         entry will be generated only for the first array element, regardless
>>         of its type.  For arrays of aggregate types, the enumeration rules are
>>         applied recursively for the single enumerated array element.
>>
>> I find this stuff very difficult to interpret properly, TBH, but I
>> think the above may be relevant.
>>
>>   -ilia
>>
> 
> OK, I will take a look at it as it mentions explicitly active shader
> storage block members declared as an array. It is possible that we have
> a bug in our code.
> 

I checked it and there is a bug in our code, we need to apply the
top-level array flattening for shader storage block members.

We even have an example on ARB_program_interface_query, "Issues" section:

     "We also apply the top-level array flattening for shader storage
      block members.  For example:

       buffer Block {
         vec4 a[5][4][3];
       };

     would enumerate only four entries -- "a[0][0][0]" through
     "a[0][3][0]", where each enumerated entry corresponds to an array
     of size 3, has a top-level size of 5, and has a top-level stride
     appropriate for its layout in memory."

I am going to write a follow-up patch fixing this.

Thanks,

Sam