[Mesa-dev] [PATCH v2] main: fix TOP_LEVEL_ARRAY_SIZE and TOP_LEVEL_ARRAY_STRIDE

[Ilia Mirkin]

On Tue, Oct 6, 2015 at 4:08 AM, Samuel Iglesias Gonsalvez
<siglesias at igalia.com> wrote:
> When the active variable is an array which is already a top-level
> shader storage block member, don't return its array size and stride
> when querying TOP_LEVEL_ARRAY_SIZE and TOP_LEVEL_ARRAY_STRIDE
> respectively.
>
> Fixes the following 12 dEQP-GLES31 tests:
>
> dEQP-GLES31.functional.ssbo.layout.single_basic_array.shared.mat3x4
> dEQP-GLES31.functional.ssbo.layout.single_basic_array.shared.row_major_mat3x4
> dEQP-GLES31.functional.ssbo.layout.single_basic_array.shared.column_major_mat3x4
> dEQP-GLES31.functional.ssbo.layout.single_basic_array.packed.mat3x4
> dEQP-GLES31.functional.ssbo.layout.single_basic_array.packed.row_major_mat3x4
> dEQP-GLES31.functional.ssbo.layout.single_basic_array.packed.column_major_mat3x4
> dEQP-GLES31.functional.ssbo.layout.single_basic_array.std140.mat3x4
> dEQP-GLES31.functional.ssbo.layout.single_basic_array.std140.row_major_mat3x4
> dEQP-GLES31.functional.ssbo.layout.single_basic_array.std140.column_major_mat3x4
> dEQP-GLES31.functional.ssbo.layout.single_basic_array.std430.mat3x4
> dEQP-GLES31.functional.ssbo.layout.single_basic_array.std430.row_major_mat3x4
> dEQP-GLES31.functional.ssbo.layout.single_basic_array.std430.column_major_mat3x4
>
> Signed-off-by: Samuel Iglesias Gonsalvez <siglesias at igalia.com>
> Tested-by: Tapani Pälli <tapani.palli at intel.com>
>
> v2:
> - Fix check when the shader storage block is instanced
> - Write auxiliary function to do the check.
>
> Signed-off-by: Samuel Iglesias Gonsalvez <siglesias at igalia.com>
> ---
>  src/mesa/main/shader_query.cpp | 51 +++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 50 insertions(+), 1 deletion(-)
>
> diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
> index 9e33dd9..9fc43d4 100644
> --- a/src/mesa/main/shader_query.cpp
> +++ b/src/mesa/main/shader_query.cpp
> @@ -872,6 +872,44 @@ get_var_name(const char *name)
>     return strndup(first_dot+1, strlen(first_dot) - 1);
>  }
>
> +static bool
> +is_top_level_shader_storage_block_member(const char* name,
> +                                         const char* interface_name,
> +                                         const char* field_name)
> +{
> +   bool result = false;
> +
> +   /* If the given variable is already a top-level shader storage
> +    * block member, then return array_size = 1.
> +    * We could have two possibilities: if we have an instanced
> +    * shader storage block or not instanced.
> +    *
> +    * For the first, we check create a name as it was in top level and
> +    * compare it with the real name. If they are the same, then
> +    * the variable is already at top-level.
> +    *
> +    * Full instanced name is: interface name + '.' + var name +
> +    *    NULL character
> +    */
> +   int name_length = strlen(interface_name) + 1 + strlen(field_name) + 1;
> +   char *full_instanced_name = (char *) calloc(name_length, sizeof(char));
> +   snprintf(full_instanced_name, name_length, "%s.%s",
> +            interface_name, field_name);

Should this happen after the null check?

> +   if (!full_instanced_name) {
> +      fprintf(stderr, "%s: Cannot allocate space for name\n", __func__);
> +      return false;
> +   }
> +   /* Check if its top-level shader storage block member of an
> +    * instanced interface block, or of a unnamed interface block.
> +    */
> +   if ((strcmp(name, full_instanced_name) == 0) ||
> +       strcmp(name, field_name) == 0)

Why is one strcmp wrapped in () but the other isn't?

> +      result = true;
> +
> +   free(full_instanced_name);
> +   return result;
> +}
> +
>  static GLint
>  program_resource_top_level_array_size(struct gl_shader_program *shProg,
>                                        struct gl_program_resource *res,
> @@ -921,12 +959,17 @@ program_resource_top_level_array_size(struct gl_shader_program *shProg,
>               * the top-level block member is an array with no declared size,
>               * the value zero is written to <params>.
>               */
> -            if (field->type->is_unsized_array())
> +            if (is_top_level_shader_storage_block_member(name,
> +                                                         interface_name,
> +                                                         var_name))
> +               array_size = 1;
> +            else if (field->type->is_unsized_array())
>                 array_size = 0;
>              else if (field->type->is_array())
>                 array_size = field->type->length;
>              else
>                 array_size = 1;
> +
>              goto found_top_level_array_size;
>           }
>        }
> @@ -995,6 +1038,12 @@ program_resource_top_level_array_stride(struct gl_shader_program *shProg,
>                 bool row_major = matrix_layout == GLSL_MATRIX_LAYOUT_ROW_MAJOR;
>                 const glsl_type *array_type = field->type->fields.array;
>
> +               if (is_top_level_shader_storage_block_member(name,
> +                                                            interface_name,
> +                                                            var_name)) {
> +                  array_stride = 0;
> +                  goto found_top_level_array_stride;
> +               }
>                 if (interface->interface_packing != GLSL_INTERFACE_PACKING_STD430) {
>                    if (array_type->is_record()) {
>                       array_stride = array_type->std140_size(row_major);
> --
> 2.1.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev