From: Anuj Phogat Date: Wed, 2 Apr 2014 20:01:43 +0000 (-0700) Subject: mesa: Fix querying location of nth element of an array variable X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=dc75479b7a7b9207d9d7c9487085ad14fa5d26cc;p=mesa.git mesa: Fix querying location of nth element of an array variable 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: Signed-off-by: Anuj Phogat Reviewed-by: Ian Romanick --- diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp index f66c117338c..36d1d9cc025 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;