On 28/10/15 10:32, Iago Toral wrote:
> On Thu, 2015-10-22 at 11:01 +0200, Samuel Iglesias Gonsalvez wrote:
>> From ARB_program_interface_query:
>>
>> "For the property of BUFFER_DATA_SIZE, then the implementation-dependent
>> minimum total buffer object size, in basic machine units, required to hold
>> all active variables associated with an active uniform block, shader
>> storage block, or atomic counter buffer is written to <params>. If the
>> final member of an active shader storage block is array with no declared
>> size, the minimum buffer size is computed assuming the array was declared
>> as an array with one element."
>>
>> Fixes the following dEQP-GLES31 tests:
>>
>> dEQP-GLES31.functional.program_interface_query.shader_storage_block.buffer_data_size.named_block
>> dEQP-GLES31.functional.program_interface_query.shader_storage_block.buffer_data_size.unnamed_block
>> dEQP-GLES31.functional.program_interface_query.shader_storage_block.buffer_data_size.block_array
>>
>> Signed-off-by: Samuel Iglesias Gonsalvez <sigles...@igalia.com>
>> ---
>> src/glsl/link_uniform_blocks.cpp | 15 ++++++++++++---
>> 1 file changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/glsl/link_uniform_blocks.cpp
>> b/src/glsl/link_uniform_blocks.cpp
>> index 5285d8d..a10b44b 100644
>> --- a/src/glsl/link_uniform_blocks.cpp
>> +++ b/src/glsl/link_uniform_blocks.cpp
>> @@ -130,13 +130,22 @@ private:
>>
>> unsigned alignment = 0;
>> unsigned size = 0;
>> -
>> + /* From ARB_program_interface_query:
> Add a blank line here
>> + * "If the final member of an active shader storage block is array
>> with
>> + * no declared size, the minimum buffer size is computed assuming the
>> + * array was declared as an array with one element."
>
> Align the two lines above to the If in the first line and indent the
> block in quotes like it is done for other similar comments in the same
> function just below this.
>
OK
>> + *
>> + * For that reason, we use the base type of the unsized array to
>> calculate
>> + * its size.
>> + */
>
> I was wondering if we should also check that this is the last member
> explicitly (there is an unused bool parameter in this function that
> informs us about that). My understanding is that only SSBOs can have
> that, and the parser should ensure that they are last in the SSBO
> definition, so maybe it is redundant... if we don't want to add that
> check, then maybe it is worth amending the comment to explain why though
> (and even in that case maybe we want to add an assert).
>
It is redundant because that check is done in the parser as you said. I
will amend the comment to explain it and add the assert too.
> With these changes:
> Reviewed-by: Iago Toral Quiroga <ito...@igalia.com>
>
Thanks,
Sam
>> + const glsl_type *type_for_size =
>> + type->is_unsized_array() ? type->without_array() : type;
>> if (packing == GLSL_INTERFACE_PACKING_STD430) {
>> alignment = type->std430_base_alignment(v->RowMajor);
>> - size = type->std430_size(v->RowMajor);
>> + size = type_for_size->std430_size(v->RowMajor);
>> } else {
>> alignment = type->std140_base_alignment(v->RowMajor);
>> - size = type->std140_size(v->RowMajor);
>> + size = type_for_size->std140_size(v->RowMajor);
>> }
>>
>> this->offset = glsl_align(this->offset, alignment);
>
>
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
