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

[Ian Romanick]

On 09/29/2011 04:42 PM, Kenneth Graunke wrote:
> 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.

Attribute arrays were added in GLSL 1.50, so we can't encounter them yet:

     "Vertex shader inputs can only be float, floating-point vectors,
     matrices, signed and unsigned integers and integer vectors. Vertex
     shader inputs can also form arrays of these types, but not
     structures."

> 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)");
>>   }