On Wed, 2015-10-21 at 12:18 +0200, Samuel Iglesias Gonsalvez wrote: > Commit f24e5e did not take into account arrays of named shader > storage blocks. > > Fixes 20 dEQP-GLES31.functional.ssbo.* tests: > > dEQP > -GLES31.functional.ssbo.layout.single_struct_array.per_block_buffer.s > hared_instance_array > dEQP > -GLES31.functional.ssbo.layout.single_struct_array.per_block_buffer.p > acked_instance_array > dEQP > -GLES31.functional.ssbo.layout.single_struct_array.per_block_buffer.s > td140_instance_array > dEQP > -GLES31.functional.ssbo.layout.single_struct_array.per_block_buffer.s > td430_instance_array > dEQP > -GLES31.functional.ssbo.layout.single_struct_array.single_buffer.shar > ed_instance_array > dEQP > -GLES31.functional.ssbo.layout.single_struct_array.single_buffer.pack > ed_instance_array > dEQP > -GLES31.functional.ssbo.layout.single_struct_array.single_buffer.std1 > 40_instance_array > dEQP > -GLES31.functional.ssbo.layout.single_struct_array.single_buffer.std4 > 30_instance_array > dEQP > -GLES31.functional.ssbo.layout.single_nested_struct_array.per_block_b > uffer.shared_instance_array > dEQP > -GLES31.functional.ssbo.layout.single_nested_struct_array.per_block_b > uffer.packed_instance_array > dEQP > -GLES31.functional.ssbo.layout.single_nested_struct_array.per_block_b > uffer.std140_instance_array > dEQP > -GLES31.functional.ssbo.layout.single_nested_struct_array.per_block_b > uffer.std430_instance_array > dEQP > -GLES31.functional.ssbo.layout.single_nested_struct_array.single_buff > er.shared_instance_array > dEQP > -GLES31.functional.ssbo.layout.single_nested_struct_array.single_buff > er.packed_instance_array > dEQP > -GLES31.functional.ssbo.layout.single_nested_struct_array.single_buff > er.std140_instance_array > dEQP > -GLES31.functional.ssbo.layout.single_nested_struct_array.single_buff > er.std430_instance_array > dEQP-GLES31.functional.ssbo.layout.random.all_per_block_buffers.2 > dEQP-GLES31.functional.ssbo.layout.random.all_per_block_buffers.29 > dEQP-GLES31.functional.ssbo.layout.random.all_per_block_buffers.33 > dEQP-GLES31.functional.ssbo.layout.random.all_shared_buffer.3 > > Signed-off-by: Samuel Iglesias Gonsalvez <sigles...@igalia.com> > --- > src/glsl/linker.cpp | 34 +++++++++++++++++++++++++++++----- > 1 file changed, 29 insertions(+), 5 deletions(-) > > diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp > index 07ea0e0..6593e58 100644 > --- a/src/glsl/linker.cpp > +++ b/src/glsl/linker.cpp > @@ -3138,6 +3138,9 @@ should_add_buffer_variable(struct > gl_shader_program *shProg, > { > bool found_interface = false; > const char *block_name = NULL; > + unsigned block_name_len = 0; > + const char *first_dot = strchr(name, '.');
Maybe call this block_name_dot rather than reusing first_dot > + const char *first_square_bracket = strchr(name, '['); > > /* These rules only apply to buffer variables. So we return > * true for the rest of types. > @@ -3147,7 +3150,27 @@ should_add_buffer_variable(struct > gl_shader_program *shProg, > > for (unsigned i = 0; i < shProg->NumBufferInterfaceBlocks; i++) { > block_name = shProg->BufferInterfaceBlocks[i].Name; > - if (strncmp(block_name, name, strlen(block_name)) == 0) { > + block_name_len = strlen(block_name); > + > + const char *first_block_square_bracket = strchr(block_name, > '['); > + if (first_block_square_bracket) { > + /* The block is part of an array of named interfaces, > + * for the name comparison we ignore the "[x]" part. > + */ > + block_name_len -= strlen(first_block_square_bracket); > + } > + > + if (first_dot) { > + /* Check if the variable name starts with the interface > + * name. The interface name (if present) should have the > + * length than the interface block name we are comparing > to. > + */ > + unsigned len = strlen(name) - strlen(first_dot); > + if (len != block_name_len) > + continue; > + } > + > + if (strncmp(block_name, name, block_name_len) == 0) { > found_interface = true; > break; > } > @@ -3156,8 +3179,11 @@ should_add_buffer_variable(struct > gl_shader_program *shProg, > /* We remove the interface name from the buffer variable name, > * including the dot that follows it. > */ > - if (found_interface) > - name = name + strlen(block_name) + 1; > + if (found_interface) { > + name = name + block_name_len + 1; > + first_dot = strchr(name, '.'); > + first_square_bracket = strchr(name, '['); > + } > > /* From: ARB_program_interface_query extension: > * > @@ -3166,8 +3192,6 @@ should_add_buffer_variable(struct > gl_shader_program *shProg, > * of its type. For arrays of aggregate types, the enumeration > rules are > * applied recursively for the single enumerated array element. > */ > - const char *first_dot = strchr(name, '.'); Maybe leave this here and name it struct_dot > - const char *first_square_bracket = strchr(name, '['); I don't think you need to move the above variable right? With those couple of small nits fixed Reviewed-by: Timothy Arceri <timothy.arc...@collabora.com> > > /* The buffer variable is on top level and it is not an array */ > if (!first_square_bracket) { _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev