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

2011-09-30 Thread Ian Romanick

On 09/29/2011 04:42 PM, Kenneth Graunke wrote:

On 09/29/2011 10:52 AM, Ian Romanick wrote:

From: Ian Romanickian.d.roman...@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 Romanickian.d.roman...@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 Graunkekenn...@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));
  }

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2011-09-29 Thread Ian Romanick
From: Ian Romanick ian.d.roman...@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.roman...@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);
+
 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;
 
-   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));
 }
 
 
-- 
1.7.6

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2011-09-29 Thread Kenneth Graunke
On 09/29/2011 10:52 AM, Ian Romanick wrote:
 From: Ian Romanick ian.d.roman...@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.roman...@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 kenn...@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));
  }
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev