[Mesa-dev] [PATCH 25/24] glsl: add std140 layout support for AoA

[Samuel Iglesias Gonsálvez]

On 24/09/15 02:16, Timothy Arceri wrote:
> On Wed, 2015-09-23 at 13:21 +0200, Samuel Iglesias Gonsálvez wrote:
>> On 22/09/15 15:30, Samuel Iglesias Gonsálvez wrote:
>>> Reviewed-by: Samuel Iglesias Gonsálvez <siglesias at igalia.com>
>>>
>>
>> Forgot to say that we need to implement a similar patch for
>> std430_size() function, right?
> 
> Yeah I think so.
> 
>>
>> I will write a follow-up patch adding std430 support for AoA.
> 
> That would be great thanks.
> 
> I assume there will also be other places in the SSBO series that might
> need updates for AoA?
> 

As I am not very familiar with AoA, I will need to check how AoA affects
to SSBO-related code.

I plan to look at it soon and send follow-up patches (if needed). I
might ask you to review my patches as you have implemented AoA.

Thanks,

Sam

> 
>>
>> Sam
>>
>>> On 20/09/15 14:07, Timothy Arceri wrote:
>>>> ---
>>>>  I noticed this problem after adding AoA support [1] to Ian's
>>>> random UBO test
>>>>  script [2].
>>>>
>>>>  [1] http://patchwork.freedesktop.org/patch/59956/
>>>>  [2] http://cgit.freedesktop.org/~idr/piglit/log/?h=ubo-lolz
>>>>
>>>>  src/glsl/glsl_types.cpp | 13 +++++++------
>>>>  1 file changed, 7 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp
>>>> index 86f0ea5..952bd0a 100644
>>>> --- a/src/glsl/glsl_types.cpp
>>>> +++ b/src/glsl/glsl_types.cpp
>>>> @@ -1310,8 +1310,8 @@ glsl_type::std140_size(bool row_major)
>>>> const
>>>>        unsigned int array_len;
>>>>  
>>>>        if (this->is_array()) {
>>>> -	 element_type = this->fields.array;
>>>> -	 array_len = this->length;
>>>> +	 element_type = this->without_array();
>>>> +	 array_len = this->arrays_of_arrays_size();
>>>>        } else {
>>>>  	 element_type = this;
>>>>  	 array_len = 1;
>>>> @@ -1344,12 +1344,13 @@ glsl_type::std140_size(bool row_major)
>>>> const
>>>>      *      the array are laid out in order, according to rule
>>>> (9).
>>>>      */
>>>>     if (this->is_array()) {
>>>> -      if (this->fields.array->is_record()) {
>>>> -	 return this->length * this->fields.array
>>>> ->std140_size(row_major);
>>>> +      if (this->without_array()->is_record()) {
>>>> +	 return this->arrays_of_arrays_size() *
>>>> +            this->without_array()->std140_size(row_major);
>>>>        } else {
>>>>  	 unsigned element_base_align =
>>>> -	    this->fields.array
>>>> ->std140_base_alignment(row_major);
>>>> -	 return this->length * MAX2(element_base_align, 16);
>>>> +	    this->without_array()
>>>> ->std140_base_alignment(row_major);
>>>> +	 return this->arrays_of_arrays_size() *
>>>> MAX2(element_base_align, 16);
>>>>        }
>>>>     }
>>>>  
>>>>
>