This change removes multiple functions designed to validate an array
subscript and replaces them with a call to a single function.
The change also means that validation is now only done once and the index
is retrived at the same time, as a result the getUniformLocation code can
be merged saving an extra hash table lookup (and yet another validation
call).
Cc: Martin Peres <martin.pe...@linux.intel.com>
Cc: Tapani Pälli <tapani.pa...@intel.com>
---
This clean-up was done to allow AoA program interface query support to
be
implemented more easily. When applied to my latest AoA work all but one
AoA program interface query subtest for the CTS now passes.
No Piglit regressions.
I also ran the following tests in the CTS without regression:
- ES31-CTS.program_interface_query*
- ES3
-CTS.gtf.GL3Tests.explicit_attrib_location.explicit_attrib_location_room
- ES3-CTS.gtf.GL3Tests.draw_instanced.draw_instanced_max_vertex_attribs
src/mesa/main/program_resource.c | 66 ++-----------
src/mesa/main/shader_query.cpp | 206 ++++++++++++++++-----------------
------
src/mesa/main/shaderapi.h | 7 +-
src/mesa/main/uniform_query.cpp | 75 --------------
src/mesa/main/uniforms.c | 7 +-
src/mesa/main/uniforms.h | 4 -
6 files changed, 100 insertions(+), 265 deletions(-)
diff --git a/src/mesa/main/program_resource.c
b/src/mesa/main/program_resource.c
index d857b84..0a306b8 100644
--- a/src/mesa/main/program_resource.c
+++ b/src/mesa/main/program_resource.c
@@ -173,32 +173,12 @@ is_xfb_marker(const char *str)
return false;
}
-/**
- * Checks if given name index is legal for GetProgramResourceIndex,
- * check is written to be compatible with GL_ARB_array_of_arrays.
- */
-static bool
-valid_program_resource_index_name(const GLchar *name)
-{
- const char *array = strstr(name, "[");
- const char *close = strrchr(name, ']');
-
- /* Not array, no need for the check. */
- if (!array)
- return true;
-
- /* Last array index has to be zero. */
- if (!close || *--close != '0')
- return false;
-
- return true;
-}
-
GLuint GLAPIENTRY
_mesa_GetProgramResourceIndex(GLuint program, GLenum programInterface,
const GLchar *name)
{
GET_CURRENT_CONTEXT(ctx);
+ unsigned array_index;
struct gl_program_resource *res;
struct gl_shader_program *shProg =
_mesa_lookup_shader_program_err(ctx, program,
@@ -221,12 +201,9 @@ _mesa_GetProgramResourceIndex(GLuint program, GLenum
programInterface,
case GL_PROGRAM_OUTPUT:
case GL_UNIFORM:
case GL_TRANSFORM_FEEDBACK_VARYING:
- /* Validate name syntax for array variables */
- if (!valid_program_resource_index_name(name))
- return GL_INVALID_INDEX;
- /* fall-through */
case GL_UNIFORM_BLOCK:
- res = _mesa_program_resource_find_name(shProg, programInterface,
name);
+ res = _mesa_program_resource_find_name(shProg, programInterface,
name,
+ &array_index, true);
if (!res)
return GL_INVALID_INDEX;
@@ -300,36 +277,6 @@ _mesa_GetProgramResourceiv(GLuint program, GLenum
programInterface,
propCount, props, bufSize, length,
params);
}
-/**
- * Function verifies syntax of given name for GetProgramResourceLocation
- * and GetProgramResourceLocationIndex for the following cases:
- *
- * "array element portion of a string passed to
GetProgramResourceLocation
- * or GetProgramResourceLocationIndex must not have, a "+" sign, extra
- * leading zeroes, or whitespace".
- *
- * Check is written to be compatible with GL_ARB_array_of_arrays.
- */
-static bool
-invalid_array_element_syntax(const GLchar *name)
-{
- char *first = strchr(name, '[');
- char *last = strrchr(name, '[');
-
- if (!first)
- return false;
-
- /* No '+' or ' ' allowed anywhere. */
- if (strchr(first, '+') || strchr(first, ' '))
- return true;
-
- /* Check that last array index is 0. */
- if (last[1] == '0' && last[2] != ']')
- return true;
-
- return false;
-}
-
static struct gl_shader_program *
lookup_linked_program(GLuint program, const char *caller)
{
@@ -356,7 +303,7 @@ _mesa_GetProgramResourceLocation(GLuint program,
GLenum programInterface,
struct gl_shader_program *shProg =
lookup_linked_program(program, "glGetProgramResourceLocation");
- if (!shProg || !name || invalid_array_element_syntax(name))
+ if (!shProg || !name)
return -1;
/* Validate programInterface. */
@@ -383,7 +330,8 @@ _mesa_GetProgramResourceLocation(GLuint program,
GLenum programInterface,
_mesa_lookup_enum_by_nr(programInterface), name);
}
- return _mesa_program_resource_location(shProg, programInterface,
name);
+ return _mesa_program_resource_location(shProg, programInterface, name,
+ true);
}
/**
@@ -397,7 +345,7 @@ _mesa_GetProgramResourceLocationIndex(GLuint program,
GLenum programInterface,
struct gl_shader_program *shProg =
lookup_linked_program(program,
"glGetProgramResourceLocationIndex");
- if (!shProg || !name || invalid_array_element_syntax(name))
+ if (!shProg || !name)
return -1;
/* From the GL_ARB_program_interface_query spec:
diff --git a/src/mesa/main/shader_query.cpp
b/src/mesa/main/shader_query.cpp
index a6246a3..46901cc 100644
--- a/src/mesa/main/shader_query.cpp
+++ b/src/mesa/main/shader_query.cpp
@@ -44,7 +44,8 @@ extern "C" {
static GLint
program_resource_location(struct gl_shader_program *shProg,
- struct gl_program_resource *res, const char
*name);
+ struct gl_program_resource *res, const char
*name,
+ unsigned array_index);
/**
* Declare convenience functions to return resource data in a given
type.
@@ -189,63 +190,6 @@ _mesa_GetActiveAttrib(GLhandleARB program, GLuint
desired_index,
(GLint *) type, "glGetActiveAttrib");
}
-/* 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)
{
@@ -271,13 +215,15 @@ _mesa_GetAttribLocation(GLhandleARB program, const
GLcharARB * name)
if (shProg->_LinkedShaders[MESA_SHADER_VERTEX] == NULL)
return -1;
+ unsigned array_index = 0;
struct gl_program_resource *res =
- _mesa_program_resource_find_name(shProg, GL_PROGRAM_INPUT, name);
+ _mesa_program_resource_find_name(shProg, GL_PROGRAM_INPUT, name,
+ &array_index, false);
if (!res)
return -1;
- GLint loc = program_resource_location(shProg, res, name);
+ GLint loc = program_resource_location(shProg, res, name, array_index);
/* The extra check against against 0 is made because of builtin
-attribute
* locations that have offset applied. Function
program_resource_location
@@ -455,13 +401,15 @@ _mesa_GetFragDataLocation(GLuint program, const
GLchar *name)
if (shProg->_LinkedShaders[MESA_SHADER_FRAGMENT] == NULL)
return -1;
+ unsigned array_index = 0;
struct gl_program_resource *res =
- _mesa_program_resource_find_name(shProg, GL_PROGRAM_OUTPUT, name);
+ _mesa_program_resource_find_name(shProg, GL_PROGRAM_OUTPUT, name,
+ &array_index, false);
if (!res)
return -1;
- GLint loc = program_resource_location(shProg, res, name);
+ GLint loc = program_resource_location(shProg, res, name, array_index);
/* The extra check against against 0 is made because of builtin
-attribute
* locations that have offset applied. Function
program_resource_location
@@ -525,39 +473,32 @@ _mesa_program_resource_array_size(struct
gl_program_resource *res)
return 0;
}
-static int
-array_index_of_resource(struct gl_program_resource *res,
- const char *name)
+/**
+ * Checks if array subscript is valid and if so sets array_index.
+ */
+static bool
+valid_array_index(const GLchar *name, unsigned *array_index)
{
- assert(res->Data);
+ unsigned idx = 0;
+ const GLchar *out_base_name_end;
- switch (res->Type) {
- case GL_PROGRAM_INPUT:
- case GL_PROGRAM_OUTPUT:
- return get_matching_index(RESOURCE_VAR(res), name);
- default:
- assert(!"support for resource type not implemented");
- return -1;
- }
+ idx = parse_program_resource_name(name, &out_base_name_end);
+ if (idx < 0)
+ return false;
+
+ if (array_index)
+ *array_index = idx;
+
+ return true;
}
/* Find a program resource with specific name in given interface.
*/
struct gl_program_resource *
_mesa_program_resource_find_name(struct gl_shader_program *shProg,
- GLenum programInterface, const char
*name)
+ GLenum programInterface, const char
*name,
+ unsigned *array_index, bool
is_program_query)
{
- GET_CURRENT_CONTEXT(ctx);
- const char *full_name = name;
-
- /* When context has 'VertexID_is_zero_based' set, gl_VertexID has been
- * lowered to gl_VertexIDMESA.
- */
- if (name && ctx->Const.VertexID_is_zero_based) {
- if (strcmp(name, "gl_VertexID") == 0)
- full_name = "gl_VertexIDMESA";
- }
-
struct gl_program_resource *res = shProg->ProgramResourceList;
for (unsigned i = 0; i < shProg->NumProgramResourceList; i++, res++)
{
if (res->Type != programInterface)
@@ -567,26 +508,36 @@ _mesa_program_resource_find_name(struct
gl_shader_program *shProg,
const char *rname = _mesa_program_resource_name(res);
unsigned baselen = strlen(rname);
- switch (programInterface) {
- case GL_TRANSFORM_FEEDBACK_VARYING:
- case GL_UNIFORM_BLOCK:
- case GL_UNIFORM:
- if (strncmp(rname, name, baselen) == 0) {
+ if (strncmp(rname, name, baselen) == 0) {
+ switch (programInterface) {
+ case GL_UNIFORM_BLOCK:
/* Basename match, check if array or struct. */
if (name[baselen] == '\0' ||
name[baselen] == '[' ||
name[baselen] == '.') {
return res;
}
+ break;
+ case GL_TRANSFORM_FEEDBACK_VARYING:
+ case GL_UNIFORM:
+ if (name[baselen] == '.') {
+ return res;
+ }
+ /* fall-through */
+ case GL_PROGRAM_INPUT:
+ case GL_PROGRAM_OUTPUT:
+ if (name[baselen] == '\0') {
+ return res;
+ } else if (name[baselen] == '[' &&
+ valid_array_index(name, array_index)) {
+ if (is_program_query && *array_index > 0)
+ return NULL;
+ return res;
+ }
+ break;
+ default:
+ assert(!"not implemented for given interface");
}
- break;
- case GL_PROGRAM_INPUT:
- case GL_PROGRAM_OUTPUT:
- if (array_index_of_resource(res, full_name) >= 0)
- return res;
- break;
- default:
- assert(!"not implemented for given interface");
}
}
return NULL;
@@ -736,17 +687,9 @@ _mesa_get_program_resource_name(struct
gl_shader_program *shProg,
static GLint
program_resource_location(struct gl_shader_program *shProg,
- struct gl_program_resource *res, const char
*name)
+ struct gl_program_resource *res, const char
*name,
+ unsigned array_index)
{
- unsigned index, offset;
- int array_index = -1;
-
- if (res->Type == GL_PROGRAM_INPUT || res->Type == GL_PROGRAM_OUTPUT) {
- array_index = array_index_of_resource(res, name);
- if (array_index < 0)
- return -1;
- }
-
/* Built-in locations should report GL_INVALID_INDEX. */
if (is_gl_identifier(name))
return GL_INVALID_INDEX;
@@ -757,15 +700,29 @@ program_resource_location(struct gl_shader_program
*shProg,
*/
switch (res->Type) {
case GL_PROGRAM_INPUT:
+ /* If the input is an array, fail if the index is out of bounds. */
+ if (array_index > 0
+ && array_index >= RESOURCE_UNI(res)->type->length) {
