[Mesa-dev] [PATCH V2] mesa: Fix querying location of nth element of an array variable

[Anuj Phogat]

This patch makes changes to the behavior of glGetAttribLocation(),
glGetFragDataLocation() and glGetFragDataIndex() functions.

Code changes handle a case described in following example:

shader program:
layout(location = 1)in vec4[4] a;
void main()
{
}

Currently, glGetAttribLocation("a") returns 1.
glGetAttribLocation("a[i]"), where i = {0, 1, 2, 3}, returns -1.
But the expected locations for array elements are: 1, 2, 3 and 4
respectively.

This clarification came up with the addition of
ARB_program_interface_query to OpenGL 4.3.

>From Page 326 (page 347 of the PDF) of OpenGL 4.3 spec:
   "Otherwise, the command is equivalent to

    GetProgramResourceLocation(program, PROGRAM_INPUT, name);"

And, From Page 101 (page 122 of the PDF) of OpenGL 4.3 spec:

   "A string provided to GetProgramResourceLocation or
    GetProgramResourceLocationIndex is considered to match an active
    variable if

    • the string exactly matches the name of the active variable;
    • if the string identifies the base name of an active array, where
      the string would exactly match the name of the variable if the
      suffix "[0]" were appended to the string; or
    • if the string identifies an active element of the array, where
      the string ends with the concatenation of the "[" character, an
      integer (with no "+" sign, extra leading zeroes, or whitespace)
      identifying an array element, and the "]" character, the integer
      is less than the number of active elements of the array variable,
      and where the string would exactly match the enumerated name of
      the array if the decimal integer were replaced with zero."

V2: Simplify get_matching_index() function.
    Add relevant text from OpenGL spec in commit message.

Fixes failures in Khronos OpenGL CTS tests:
explicit_attrib_location_room
draw_instanced_max_vertex_attribs

Proprietary linux drivers of NVIDIA (331.49) matches the behavior
expected by OpenGL 4.3 spec.

Cc: <mesa-stable at lists.freedesktop.org>
Signed-off-by: Anuj Phogat <anuj.phogat at gmail.com>
---
 src/mesa/main/shader_query.cpp | 71 +++++++++++++++++++++++++++++++++++++++---
 1 file changed, 66 insertions(+), 5 deletions(-)

diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
index f66c117..36d1d9c 100644
--- a/src/mesa/main/shader_query.cpp
+++ b/src/mesa/main/shader_query.cpp
@@ -153,6 +153,63 @@ _mesa_GetActiveAttrib(GLhandleARB program, GLuint desired_index,
    _mesa_error(ctx, GL_INVALID_VALUE, "glGetActiveAttrib(index)");
 }
 
+/* Locations associated with shader variables (array or non-array) can be
+ * queried using its base name or using the base name appended with the
+ * valid array index. For example, in case of below vertex shader, valid
+ * queries can be made to know the location of "xyz", "array", "array[0]",
+ * "array[1]", "array[2]" and "array[3]". In this example index reurned
+ * will be 0, 0, 0, 1, 2, 3 respectively.
+ *
+ * [Vertex Shader]
+ * layout(location=0) in vec4 xyz;
+ * layout(location=1) in vec4[4] array;
+ * void main()
+ * { }
+ *
+ * This requirement came up with the addition of ARB_program_interface_query
+ * to OpenGL 4.3 specification. See page 101 (page 122 of the PDF) for details.
+ *
+ * This utility function is used by:
+ * _mesa_GetAttribLocation
+ * _mesa_GetFragDataLocation
+ * _mesa_GetFragDataIndex
+ *
+ * Returns 0:
+ *    if the 'name' string matches var->name.
+ * Returns 'matched index':
+ *    if the 'name' string matches var->name appended with valid array index.
+ */
+int static inline
+get_matching_index(const ir_variable *const var, const char *name) {
+   unsigned idx = 0;
+   const char *const paren = strchr(name, '[');
+   const unsigned len = (paren != NULL) ? paren - name : strlen(name);
+
+   if (paren != NULL) {
+      if (!var->type->is_array())
+         return -1;
+
+      char *endptr;
+      idx = (unsigned) strtol(paren + 1, &endptr, 10);
+      const unsigned idx_len = endptr != (paren + 1) ? endptr - paren - 1 : 0;
+
+      /* Validate the sub string representing index in 'name' string */
+      if ((idx > 0 && paren[1] == '0') /* leading zeroes */
+          || (idx == 0 && idx_len > 1) /* all zeroes */
+          || paren[1] == ' ' /* whitespace */
+          || endptr[0] != ']' /* closing brace */
+          || endptr[1] != '\0' /* null char */
+          || idx_len == 0 /* missing index */
+          || idx >= var->type->length) /* exceeding array bound */
+         return -1;
+   }
+
+   if (strncmp(var->name, name, len) == 0 && var->name[len] == '\0')
+      return idx;
+
+   return -1;
+}
+
 GLint GLAPIENTRY
 _mesa_GetAttribLocation(GLhandleARB program, const GLcharARB * name)
 {
@@ -196,8 +253,10 @@ _mesa_GetAttribLocation(GLhandleARB program, const GLcharARB * name)
 	  || var->data.location < VERT_ATTRIB_GENERIC0)
 	 continue;
 
-      if (strcmp(var->name, name) == 0)
-	 return var->data.location - VERT_ATTRIB_GENERIC0;
+      int index = get_matching_index(var, (const char *) name);
+
+      if (index >= 0)
+         return var->data.location + index - VERT_ATTRIB_GENERIC0;
    }
 
    return -1;
@@ -358,7 +417,7 @@ _mesa_GetFragDataIndex(GLuint program, const GLchar *name)
           || var->data.location < FRAG_RESULT_DATA0)
          continue;
 
-      if (strcmp(var->name, name) == 0)
+      if (get_matching_index(var, (const char *) name) >= 0)
          return var->data.index;
    }
 
@@ -414,8 +473,10 @@ _mesa_GetFragDataLocation(GLuint program, const GLchar *name)
 	  || var->data.location < FRAG_RESULT_DATA0)
 	 continue;
 
-      if (strcmp(var->name, name) == 0)
-	 return var->data.location - FRAG_RESULT_DATA0;
+      int index = get_matching_index(var, (const char *) name);
+
+      if (index >= 0)
+         return var->data.location + index - FRAG_RESULT_DATA0;
    }
 
    return -1;
-- 
1.8.3.1