[Mesa-dev] [PATCH 1/4] mesa/glsl: Separate parsing logic from _mesa_get_uniform_location.
Paul Berry
stereotype441 at gmail.com
Thu Jan 31 16:34:40 PST 2013
The parsing logic is moved to a new function in the GLSL module,
parse_program_resource_name(). This name was chosen because it should
eventually be useful for handling everything that OpenGL 4.3 calls
"program resources" (e.g. uniforms, vertex inputs, fragment outputs,
and transform feedback varyings).
Future patches will make use of this function for linking transform
feedback varyings.
NOTE: This is a candidate for the 9.1 branch.
---
src/glsl/linker.cpp | 59 ++++++++++++++++++++++++++++
src/glsl/program.h | 4 ++
src/mesa/main/uniform_query.cpp | 85 ++++++++++++-----------------------------
src/mesa/main/uniforms.h | 4 ++
4 files changed, 91 insertions(+), 61 deletions(-)
diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
index 63ce178..57e7a9a 100644
--- a/src/glsl/linker.cpp
+++ b/src/glsl/linker.cpp
@@ -200,6 +200,65 @@ linker_warning(gl_shader_program *prog, const char *fmt, ...)
}
+/**
+ * Given a string identifying a program resource, break it into a base name
+ * and an optional array index in square brackets.
+ *
+ * If an array index is present, \c out_base_name_end is set to point to the
+ * "[" that precedes the array index, and the array index itself is returned
+ * as a long.
+ *
+ * If no array index is present (or if the array index is negative or
+ * mal-formed), \c out_base_name_end, is set to point to the null terminator
+ * at the end of the input string, and -1 is returned.
+ *
+ * Only the final array index is parsed; if the string contains other array
+ * indices (or structure field accesses), they are left in the base name.
+ *
+ * No attempt is made to check that the base name is properly formed;
+ * typically the caller will look up the base name in a hash table, so
+ * ill-formed base names simply turn into hash table lookup failures.
+ */
+long
+parse_program_resource_name(const GLchar *name,
+ const GLchar **out_base_name_end)
+{
+ /* Section 7.3.1 ("Program Interfaces") of the OpenGL 4.3 spec says:
+ *
+ * "When an integer array element or block instance number is part of
+ * the name string, it will be specified in decimal form without a "+"
+ * or "-" sign or any extra leading zeroes. Additionally, the name
+ * string will not include white space anywhere in the string."
+ */
+
+ const size_t len = strlen(name);
+ *out_base_name_end = name + len;
+
+ if (len == 0 || name[len-1] != ']')
+ return -1;
+
+ /* Walk backwards over the string looking for a non-digit character. This
+ * had better be the opening bracket for an array index.
+ *
+ * Initially, i specifies the location of the ']'. Since the string may
+ * contain only the ']' charcater, walk backwards very carefully.
+ */
+ unsigned i;
+ for (i = len - 1; (i > 0) && isdigit(name[i-1]); --i)
+ /* empty */ ;
+
+ if ((i == 0) || name[i-1] != '[')
+ return -1;
+
+ long array_index = strtol(&name[i], NULL, 10);
+ if (array_index < 0)
+ return -1;
+
+ *out_base_name_end = name + (i - 1);
+ return array_index;
+}
+
+
void
link_invalidate_variable_locations(gl_shader *sh, int input_base,
int output_base)
diff --git a/src/glsl/program.h b/src/glsl/program.h
index 437ca14..46ce9dc 100644
--- a/src/glsl/program.h
+++ b/src/glsl/program.h
@@ -33,3 +33,7 @@ linker_error(gl_shader_program *prog, const char *fmt, ...)
extern void
linker_warning(gl_shader_program *prog, const char *fmt, ...)
PRINTFLIKE(2, 3);
+
+extern long
+parse_program_resource_name(const GLchar *name,
+ const GLchar **out_base_name_end);
diff --git a/src/mesa/main/uniform_query.cpp b/src/mesa/main/uniform_query.cpp
index dc550bc..b8335fe 100644
--- a/src/mesa/main/uniform_query.cpp
+++ b/src/mesa/main/uniform_query.cpp
@@ -929,6 +929,7 @@ _mesa_uniform_matrix(struct gl_context *ctx, struct gl_shader_program *shProg,
_mesa_propagate_uniforms_to_driver_storage(uni, offset, count);
}
+
/**
* Called via glGetUniformLocation().
*
@@ -944,73 +945,35 @@ _mesa_get_uniform_location(struct gl_context *ctx,
const GLchar *name,
unsigned *out_offset)
{
- const size_t len = strlen(name);
- long offset;
- bool array_lookup;
- char *name_copy;
-
- /* If the name ends with a ']', assume that it refers to some element of an
- * array. Malformed array references will fail the hash table look up
- * below, so it doesn't matter that they are not caught here. This code
- * only wants to catch the "leaf" array references so that arrays of
- * structures containing arrays will be handled correctly.
+ /* Page 80 (page 94 of the PDF) of the OpenGL 2.1 spec says:
+ *
+ * "The first element of a uniform array is identified using the
+ * name of the uniform array appended with "[0]". Except if the last
+ * part of the string name indicates a uniform array, then the
+ * location of the first element of that array can be retrieved by
+ * either using the name of the uniform array, or the name of the
+ * uniform array appended with "[0]"."
+ *
+ * Note: since uniform names are not allowed to use whitespace, and array
+ * indices within uniform names are not allowed to use "+", "-", or leading
+ * zeros, it follows that each uniform has a unique name up to the possible
+ * ambiguity with "[0]" noted above. Therefore we don't need to worry
+ * about mal-formed inputs--they will properly fail when we try to look up
+ * the uniform name in shProg->UniformHash.
*/
- if (name[len-1] == ']') {
- unsigned i;
-
- /* Walk backwards over the string looking for a non-digit character.
- * This had better be the opening bracket for an array index.
- *
- * Initially, i specifies the location of the ']'. Since the string may
- * contain only the ']' charcater, walk backwards very carefully.
- */
- for (i = len - 1; (i > 0) && isdigit(name[i-1]); --i)
- /* empty */ ;
-
- /* Page 80 (page 94 of the PDF) of the OpenGL 2.1 spec says:
- *
- * "The first element of a uniform array is identified using the
- * name of the uniform array appended with "[0]". Except if the last
- * part of the string name indicates a uniform array, then the
- * location of the first element of that array can be retrieved by
- * either using the name of the uniform array, or the name of the
- * uniform array appended with "[0]"."
- *
- * Page 79 (page 93 of the PDF) of the OpenGL 2.1 spec says:
- *
- * "name must be a null terminated string, without white space."
- *
- * Return an error if there is no opening '[' to match the closing ']'.
- * An error will also be returned if there is intervening white space
- * (or other non-digit characters) before the opening '['.
- */
- if ((i == 0) || name[i-1] != '[')
- return GL_INVALID_INDEX;
- /* Return an error if there are no digits between the opening '[' to
- * match the closing ']'.
- */
- if (i == (len - 1))
- return GL_INVALID_INDEX;
-
- /* Make a new string that is a copy of the old string up to (but not
- * including) the '[' character.
- */
- name_copy = (char *) malloc(i);
- memcpy(name_copy, name, i - 1);
- name_copy[i-1] = '\0';
-
- offset = strtol(&name[i], NULL, 10);
- if (offset < 0) {
- free(name_copy);
- return GL_INVALID_INDEX;
- }
+ const GLchar *base_name_end;
+ long offset = parse_program_resource_name(name, &base_name_end);
+ bool array_lookup = offset >= 0;
+ char *name_copy;
- array_lookup = true;
+ if (array_lookup) {
+ name_copy = (char *) malloc(base_name_end - name + 1);
+ memcpy(name_copy, name, base_name_end - name);
+ name_copy[base_name_end - name] = '\0';
} else {
name_copy = (char *) name;
offset = 0;
- array_lookup = false;
}
unsigned location = 0;
diff --git a/src/mesa/main/uniforms.h b/src/mesa/main/uniforms.h
index f175031..a12ad9b 100644
--- a/src/mesa/main/uniforms.h
+++ b/src/mesa/main/uniforms.h
@@ -167,6 +167,10 @@ _mesa_GetActiveUniformsiv(GLuint program,
void GLAPIENTRY
_mesa_GetUniformiv(GLhandleARB, GLint, GLint *);
+long
+_mesa_parse_program_resource_name(const GLchar *name,
+ const GLchar **out_base_name_end);
+
unsigned
_mesa_get_uniform_location(struct gl_context *ctx, struct gl_shader_program *shProg,
const GLchar *name, unsigned *offset);
--
1.8.1.2
More information about the mesa-dev
mailing list