[Mesa-dev] [PATCH] mesa: fix ARRAY_SIZE query for GetProgramResourceiv

Tapani Pälli tapani.palli at intel.com
Mon Sep 28 03:51:43 PDT 2015


Patch also refactors name length queries which were using array size
in computation, this has to be done in same time to avoid regression in
arb_program_interface_query-resource-query Piglit test.

Fixes rest of the failures with
   ES31-CTS.program_interface_query.no-locations

v2: make additional check only for GS inputs
v3: create helper function for resource name length
    so that it gets calculated only in one place

Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
Reviewed-by: Martin Peres <martin.peres at linux.intel.com> [v2]
---
 src/mesa/main/program_resource.c |  8 ++--
 src/mesa/main/shader_query.cpp   | 94 ++++++++++++++++++++++++----------------
 src/mesa/main/shaderapi.h        |  3 ++
 3 files changed, 62 insertions(+), 43 deletions(-)

diff --git a/src/mesa/main/program_resource.c b/src/mesa/main/program_resource.c
index c609abe..eb71fdd 100644
--- a/src/mesa/main/program_resource.c
+++ b/src/mesa/main/program_resource.c
@@ -111,11 +111,9 @@ _mesa_GetProgramInterfaceiv(GLuint program, GLenum programInterface,
       for (i = 0, *params = 0; i < shProg->NumProgramResourceList; i++) {
          if (shProg->ProgramResourceList[i].Type != programInterface)
             continue;
-         const char *name =
-            _mesa_program_resource_name(&shProg->ProgramResourceList[i]);
-         unsigned array_size =
-            _mesa_program_resource_array_size(&shProg->ProgramResourceList[i]);
-         *params = MAX2(*params, strlen(name) + (array_size ? 3 : 0) + 1);
+         unsigned len =
+            _mesa_program_resource_name_len(&shProg->ProgramResourceList[i]);
+         *params = MAX2(*params, len + 1);
       }
       break;
    case GL_MAX_NUM_ACTIVE_VARIABLES:
diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
index 99d9e10..a5572ff 100644
--- a/src/mesa/main/shader_query.cpp
+++ b/src/mesa/main/shader_query.cpp
@@ -478,7 +478,7 @@ _mesa_program_resource_array_size(struct gl_program_resource *res)
              RESOURCE_XFB(res)->Size : 0;
    case GL_PROGRAM_INPUT:
    case GL_PROGRAM_OUTPUT:
-      return RESOURCE_VAR(res)->data.max_array_access;
+      return RESOURCE_VAR(res)->type->length;
    case GL_UNIFORM:
    case GL_VERTEX_SUBROUTINE_UNIFORM:
    case GL_GEOMETRY_SUBROUTINE_UNIFORM:
@@ -670,6 +670,57 @@ _mesa_program_resource_find_index(struct gl_shader_program *shProg,
    return NULL;
 }
 
+/* Function returns if resource name is expected to have index
+ * appended into it.
+ *
+ *
+ * Page 61 (page 73 of the PDF) in section 2.11 of the OpenGL ES 3.0
+ * spec says:
+ *
+ *     "If the active uniform is an array, the uniform name returned in
+ *     name will always be the name of the uniform array appended with
+ *     "[0]"."
+ *
+ * The same text also appears in the OpenGL 4.2 spec.  It does not,
+ * however, appear in any previous spec.  Previous specifications are
+ * ambiguous in this regard.  However, either name can later be passed
+ * to glGetUniformLocation (and related APIs), so there shouldn't be any
+ * harm in always appending "[0]" to uniform array names.
+ *
+ * Geometry shader stage has different naming convention where the 'normal'
+ * condition is an array, therefore for variables referenced in geometry
+ * stage we do not add '[0]'.
+ *
+ * Note, that TCS outputs and TES inputs should not have index appended
+ * either.
+ */
+bool
+add_index_to_name(struct gl_program_resource *res)
+{
+   bool add_index = !(((res->Type == GL_PROGRAM_INPUT) &&
+                       res->StageReferences & (1 << MESA_SHADER_GEOMETRY)));
+
+   /* Transform feedback varyings have array index already appended
+    * in their names.
+    */
+   if (res->Type == GL_TRANSFORM_FEEDBACK_VARYING)
+      add_index = false;
+
+   return add_index;
+}
+
+/* Get name length of a program resource. This consists of
+ * base name + 3 for '[0]' if resource is an array.
+ */
+extern unsigned
+_mesa_program_resource_name_len(struct gl_program_resource *res)
+{
+   unsigned length = strlen(_mesa_program_resource_name(res));
+   if (_mesa_program_resource_array_size(res) && add_index_to_name(res))
+      length += 3;
+   return length;
+}
+
 /* Get full name of a program resource.
  */
 bool
@@ -705,36 +756,7 @@ _mesa_get_program_resource_name(struct gl_shader_program *shProg,
 
    _mesa_copy_string(name, bufSize, length, _mesa_program_resource_name(res));
 
-   /* Page 61 (page 73 of the PDF) in section 2.11 of the OpenGL ES 3.0
-    * spec says:
-    *
-    *     "If the active uniform is an array, the uniform name returned in
-    *     name will always be the name of the uniform array appended with
-    *     "[0]"."
-    *
-    * The same text also appears in the OpenGL 4.2 spec.  It does not,
-    * however, appear in any previous spec.  Previous specifications are
-    * ambiguous in this regard.  However, either name can later be passed
-    * to glGetUniformLocation (and related APIs), so there shouldn't be any
-    * harm in always appending "[0]" to uniform array names.
-    *
-    * Geometry shader stage has different naming convention where the 'normal'
-    * condition is an array, therefore for variables referenced in geometry
-    * stage we do not add '[0]'.
-    *
-    * Note, that TCS outputs and TES inputs should not have index appended
-    * either.
-    */
-   bool add_index = !(((programInterface == GL_PROGRAM_INPUT) &&
-                       res->StageReferences & (1 << MESA_SHADER_GEOMETRY)));
-
-   /* Transform feedback varyings have array index already appended
-    * in their names.
-    */
-   if (programInterface == GL_TRANSFORM_FEEDBACK_VARYING)
-      add_index = false;
-
-   if (add_index && _mesa_program_resource_array_size(res)) {
+   if (_mesa_program_resource_array_size(res) && add_index_to_name(res)) {
       int i;
 
       /* The comparison is strange because *length does *NOT* include the
@@ -1205,13 +1227,9 @@ _mesa_program_resource_prop(struct gl_shader_program *shProg,
       switch (res->Type) {
       case GL_ATOMIC_COUNTER_BUFFER:
          goto invalid_operation;
-      case GL_TRANSFORM_FEEDBACK_VARYING:
-         *val = strlen(_mesa_program_resource_name(res)) + 1;
-         break;
       default:
-         /* Base name +3 if array '[0]' + terminator. */
-         *val = strlen(_mesa_program_resource_name(res)) +
-            (_mesa_program_resource_array_size(res) > 0 ? 3 : 0) + 1;
+         /* Resource name length + terminator. */
+         *val = _mesa_program_resource_name_len(res) + 1;
       }
       return 1;
    case GL_TYPE:
@@ -1238,7 +1256,7 @@ _mesa_program_resource_prop(struct gl_shader_program *shProg,
             return 1;
       case GL_PROGRAM_INPUT:
       case GL_PROGRAM_OUTPUT:
-         *val = MAX2(RESOURCE_VAR(res)->type->length, 1);
+         *val = MAX2(_mesa_program_resource_array_size(res), 1);
          return 1;
       case GL_TRANSFORM_FEEDBACK_VARYING:
          *val = MAX2(RESOURCE_XFB(res)->Size, 1);
diff --git a/src/mesa/main/shaderapi.h b/src/mesa/main/shaderapi.h
index 0a10191..fba767b 100644
--- a/src/mesa/main/shaderapi.h
+++ b/src/mesa/main/shaderapi.h
@@ -245,6 +245,9 @@ _mesa_get_program_resource_name(struct gl_shader_program *shProg,
                                 GLsizei bufSize, GLsizei *length,
                                 GLchar *name, const char *caller);
 
+extern unsigned
+_mesa_program_resource_name_len(struct gl_program_resource *res);
+
 extern GLint
 _mesa_program_resource_location(struct gl_shader_program *shProg,
                                 GLenum programInterface, const char *name);
-- 
2.4.3



More information about the mesa-dev mailing list