[Mesa-dev] [PATCH] mesa: refactor active attrib queries for glGetProgramiv

[Martin Peres]

On 23/04/15 11:13, Tapani Pälli wrote:
> Main motivation here is to get rid of iterating IR and
> encapsulate queries within program resources.
> No functional changes.
>
> Piglit tests calling the modified functionality:
>
>     - gl-get-active-attrib-returns-all-inputs
>     - glsl-1.50-get-active-attrib-array
>     - getactiveattrib
>
> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
> ---
>   src/mesa/main/shader_query.cpp | 39 ++++++++++++++-------------------------
>   1 file changed, 14 insertions(+), 25 deletions(-)
>
> diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
> index bc6fec5..e047b09 100644
> --- a/src/mesa/main/shader_query.cpp
> +++ b/src/mesa/main/shader_query.cpp
> @@ -291,7 +291,6 @@ _mesa_GetAttribLocation(GLhandleARB program, const GLcharARB * name)
>      return (loc >= 0) ? loc : -1;
>   }
>   
> -
>   unsigned
>   _mesa_count_active_attribs(struct gl_shader_program *shProg)
>   {
> @@ -300,19 +299,13 @@ _mesa_count_active_attribs(struct gl_shader_program *shProg)
>         return 0;
>      }
>   
> -   exec_list *const ir = shProg->_LinkedShaders[MESA_SHADER_VERTEX]->ir;
> -   unsigned i = 0;
> -
> -   foreach_in_list(ir_instruction, node, ir) {
> -      const ir_variable *const var = node->as_variable();
> -
> -      if (!is_active_attrib(var))
> -         continue;
> -
> -      i++;
> +   struct gl_program_resource *res = shProg->ProgramResourceList;
> +   unsigned count = 0;
> +   for (unsigned j = 0; j < shProg->NumProgramResourceList; j++, res++) {
> +         if (is_active_attrib(RESOURCE_VAR(res)))
> +            count++;
>      }
> -
> -   return i;
> +   return count;
>   }
>   
>   
> @@ -324,20 +317,16 @@ _mesa_longest_attribute_name_length(struct gl_shader_program *shProg)
>         return 0;
>      }
>   
> -   exec_list *const ir = shProg->_LinkedShaders[MESA_SHADER_VERTEX]->ir;
> +   struct gl_program_resource *res = shProg->ProgramResourceList;
>      size_t longest = 0;
> +   for (unsigned j = 0; j < shProg->NumProgramResourceList; j++, res++) {
> +      if (res->Type == GL_PROGRAM_INPUT &&
> +          res->StageReferences & (1 << MESA_SHADER_VERTEX)) {
>   
> -   foreach_in_list(ir_instruction, node, ir) {
> -      const ir_variable *const var = node->as_variable();
> -
> -      if (var == NULL
> -	  || var->data.mode != ir_var_shader_in
> -	  || var->data.location == -1)
> -	 continue;
> -
> -      const size_t len = strlen(var->name);
> -      if (len >= longest)
> -	 longest = len + 1;
> +          const size_t length = strlen(RESOURCE_VAR(res)->name);
> +          if (length > longest)
The condition above is wrong, it should be length >= longest like the 
original hunk.

Other than that, it looks like a nice refactoring.

Reviewed-by: Martin Peres <martin.peres at linux.intel.com>
> +             longest = length + 1;
> +      }
>      }
>   
>      return longest;