[Mesa-dev] [PATCH] glsl: Respect std430 layout in lower_buffer_access

[Timothy Arceri]

These changes seem reasonable. Are you able to create a piglit test that 
exercises the bug also?

See the basic ssbo test as an example [1]. After building piglit you can 
run these shader runner tests from the piglit dir like so:

./bin/shader_runner 
tests/spec/arb_shader_storage_buffer_object/execution/basic.shader_test 
-auto

You can remove -auto to see the output window.

[1] 
https://cgit.freedesktop.org/piglit/tree/tests/spec/arb_shader_storage_buffer_object/execution/basic.shader_test

On 06/01/18 01:33, Florian Will wrote:
> Respect the std430 rules for determining offset and size of struct
> members when using a std430 buffer. std140 rules lead to wrong buffer
> offsets in that case.
> 
> Fixes my test case attached in Bugzilla. No piglit changes.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104492
> ---
>   src/compiler/glsl/lower_buffer_access.cpp | 14 ++++++++++----
>   1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/src/compiler/glsl/lower_buffer_access.cpp b/src/compiler/glsl/lower_buffer_access.cpp
> index db6e8e367b..ff6f9c1fcf 100644
> --- a/src/compiler/glsl/lower_buffer_access.cpp
> +++ b/src/compiler/glsl/lower_buffer_access.cpp
> @@ -73,16 +73,22 @@ lower_buffer_access::emit_access(void *mem_ctx,
>               new(mem_ctx) ir_dereference_record(deref->clone(mem_ctx, NULL),
>                                                  field->name);
>   
> -         field_offset =
> -            glsl_align(field_offset,
> -                       field->type->std140_base_alignment(row_major));
> +         unsigned field_align;
> +         if (packing == GLSL_INTERFACE_PACKING_STD430)
> +            field_align = field->type->std430_base_alignment(row_major);
> +         else
> +            field_align = field->type->std140_base_alignment(row_major);
> +         field_offset = glsl_align(field_offset, field_align);
>   
>            emit_access(mem_ctx, is_write, field_deref, base_offset,
>                        deref_offset + field_offset,
>                        row_major, NULL, packing,
>                        writemask_for_size(field_deref->type->vector_elements));
>   
> -         field_offset += field->type->std140_size(row_major);
> +         if (packing == GLSL_INTERFACE_PACKING_STD430)
> +            field_offset += field->type->std430_size(row_major);
> +         else
> +            field_offset += field->type->std140_size(row_major);
>         }
>         return;
>      }
>