[Mesa-dev] [PATCH 11/15] mesa: Make _mesa_GetActiveAttribARB use the attributes in the shader IR

[Kenneth Graunke]

On 09/29/2011 10:52 AM, Ian Romanick wrote:
> From: Ian Romanick <ian.d.romanick at intel.com>
> 
> Instead of relying on the mirror in the Mesa IR assembly shader, just
> use the variables actually stored in the GLSL IR.  This will be a bit
> slower, but nobody cares about the performance of glGetActiveAttrib.
> 
> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
> ---
>  src/glsl/program.h             |    3 ++
>  src/mesa/main/shader_query.cpp |   49 ++++++++++++++++++++++++++++++---------
>  2 files changed, 40 insertions(+), 12 deletions(-)
> 
> diff --git a/src/glsl/program.h b/src/glsl/program.h
> index 437ca14..5a68d66 100644
> --- a/src/glsl/program.h
> +++ b/src/glsl/program.h
> @@ -26,6 +26,9 @@
>  extern void
>  link_shaders(struct gl_context *ctx, struct gl_shader_program *prog);
>  
> +extern unsigned
> +count_attribute_slots(const glsl_type *t);

Not seeing this used anywhere...?

>  extern void
>  linker_error(gl_shader_program *prog, const char *fmt, ...)
>     PRINTFLIKE(2, 3);
> diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
> index 7959201..84b25cd 100644
> --- a/src/mesa/main/shader_query.cpp
> +++ b/src/mesa/main/shader_query.cpp
> @@ -33,6 +33,7 @@
>  #include "ir.h"
>  #include "shaderobj.h"
>  #include "program/hash_table.h"
> +#include "../glsl/program.h"
>  
>  extern "C" {
>  #include "shaderapi.h"
> @@ -94,30 +95,54 @@ _mesa_GetActiveAttribARB(GLhandleARB program, GLuint index,
>                           GLenum * type, GLcharARB * name)
>  {
>     GET_CURRENT_CONTEXT(ctx);
> -   const struct gl_program_parameter_list *attribs = NULL;
>     struct gl_shader_program *shProg;
>  
>     shProg = _mesa_lookup_shader_program_err(ctx, program, "glGetActiveAttrib");
>     if (!shProg)
>        return;
>  
> -   if (shProg->VertexProgram)
> -      attribs = shProg->VertexProgram->Base.Attributes;
> +   if (!shProg->LinkStatus) {
> +      _mesa_error(ctx, GL_INVALID_VALUE,
> +                  "glGetActiveAttrib(program not linked)");
> +      return;
> +   }
>  
> -   if (!attribs || index >= attribs->NumParameters) {
> -      _mesa_error(ctx, GL_INVALID_VALUE, "glGetActiveAttrib(index)");
> +   if (shProg->_LinkedShaders[MESA_SHADER_VERTEX] == NULL) {
> +      _mesa_error(ctx, GL_INVALID_VALUE, "glGetActiveAttrib(no vertex shader)");
>        return;
>     }
>  
> -   _mesa_copy_string(name, maxLength, length,
> -                     attribs->Parameters[index].Name);
> +   exec_list *const ir = shProg->_LinkedShaders[MESA_SHADER_VERTEX]->ir;
> +   unsigned i = 0;
> +
> +   foreach_list(node, ir) {
> +      const ir_variable *const var = ((ir_instruction *) node)->as_variable();
> +
> +      if (var == NULL
> +	  || var->mode != ir_var_in
> +	  || var->location == -1
> +	  || var->location < VERT_ATTRIB_GENERIC0)
> +	 continue;
> +
> +      if (i == index) {
> +	 _mesa_copy_string(name, maxLength, length, var->name);
>  
> -   if (size)
> -      *size = attribs->Parameters[index].Size
> -         / _mesa_sizeof_glsl_type(attribs->Parameters[index].DataType);
> +	 if (size)
> +	    *size = (var->type->is_array()) ? var->type->length : 1;

I was concerned that structs might be possible here, but then found this
text in the GLSL 1.20 spec: "Attribute variables cannot be declared as
arrays or structures."

I'm having trouble finding how arrays happen, but clearly they must, or
else size would be irrelevant.

Looks good to me.
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>

> -   if (type)
> -      *type = attribs->Parameters[index].DataType;
> +	 if (type)
> +	    *type = var->type->gl_type;
> +
> +	 return;
> +      }
> +
> +      i++;
> +   }
> +
> +   /* If the loop did not return early, the caller must have asked for
> +    * an index that did not exit.  Set an error.
> +    */
> +   _mesa_error(ctx, GL_INVALID_VALUE, "glGetActiveAttrib(index)");
>  }