From 8cd2f88845acd45ebcbaae2e68a8a47b3c17e6d5 Mon Sep 17 00:00:00 2001 From: Timothy Arceri Date: Sat, 25 Jul 2015 12:33:53 +1000 Subject: [PATCH] mesa: fix and simplify resource query for arrays MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit This removes the need for 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 simplified saving an extra hash table lookup (and yet another validation call). This chage also fixes some tests in: ES31-CTS.program_interface_query.uniform V3: rebase on subroutines, and move the resource index array == 0 check into _mesa_GetProgramResourceIndex() to simplify things further V2: Fix bounds checks for program input/output, split unrelated comment fix and _mesa_get_uniform_location() removal into their own patch. Cc: Tapani Pälli Reviewed-by: Tapani Pälli --- src/mesa/main/program_resource.c | 14 ++-- src/mesa/main/shader_query.cpp | 174 +++++++++++++++++++++------------------ src/mesa/main/shaderapi.c | 2 +- src/mesa/main/shaderapi.h | 3 +- src/mesa/main/uniforms.c | 5 +- 5 files changed, 106 insertions(+), 92 deletions(-) diff --git a/src/mesa/main/program_resource.c b/src/mesa/main/program_resource.c index 641ef22..fdbd5b3 100644 --- a/src/mesa/main/program_resource.c +++ b/src/mesa/main/program_resource.c @@ -229,6 +229,7 @@ _mesa_GetProgramResourceIndex(GLuint program, GLenum programInterface, const GLchar *name) { GET_CURRENT_CONTEXT(ctx); + unsigned array_index = 0; struct gl_program_resource *res; struct gl_shader_program *shProg = _mesa_lookup_shader_program_err(ctx, program, @@ -268,13 +269,10 @@ _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); - if (!res) + res = _mesa_program_resource_find_name(shProg, programInterface, name, + &array_index); + if (!res || array_index > 0) return GL_INVALID_INDEX; return _mesa_program_resource_index(shProg, res); @@ -403,7 +401,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. */ @@ -453,7 +451,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 3e52a09..dd11b81 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. @@ -272,13 +273,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); 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 @@ -456,13 +459,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); 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 @@ -552,39 +557,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) { - 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) @@ -594,38 +592,46 @@ _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: - case GL_VERTEX_SUBROUTINE_UNIFORM: - case GL_GEOMETRY_SUBROUTINE_UNIFORM: - case GL_FRAGMENT_SUBROUTINE_UNIFORM: - case GL_COMPUTE_SUBROUTINE_UNIFORM: - case GL_TESS_CONTROL_SUBROUTINE_UNIFORM: - case GL_TESS_EVALUATION_SUBROUTINE_UNIFORM: - case GL_VERTEX_SUBROUTINE: - case GL_GEOMETRY_SUBROUTINE: - case GL_FRAGMENT_SUBROUTINE: - case GL_COMPUTE_SUBROUTINE: - case GL_TESS_CONTROL_SUBROUTINE: - case GL_TESS_EVALUATION_SUBROUTINE: - 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: + case GL_VERTEX_SUBROUTINE_UNIFORM: + case GL_GEOMETRY_SUBROUTINE_UNIFORM: + case GL_FRAGMENT_SUBROUTINE_UNIFORM: + case GL_COMPUTE_SUBROUTINE_UNIFORM: + case GL_TESS_CONTROL_SUBROUTINE_UNIFORM: + case GL_TESS_EVALUATION_SUBROUTINE_UNIFORM: + case GL_VERTEX_SUBROUTINE: + case GL_GEOMETRY_SUBROUTINE: + case GL_FRAGMENT_SUBROUTINE: + case GL_COMPUTE_SUBROUTINE: + case GL_TESS_CONTROL_SUBROUTINE: + case GL_TESS_EVALUATION_SUBROUTINE: + 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)) { + 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; @@ -787,19 +793,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; - long offset_ret; - const GLchar *base_name_end; - - 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; @@ -810,13 +806,22 @@ 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_VAR(res)->type->length) { + return -1; + } return RESOURCE_VAR(res)->data.location + array_index - VERT_ATTRIB_GENERIC0; case GL_PROGRAM_OUTPUT: + /* If the output is an array, fail if the index is out of bounds. */ + if (array_index > 0 + && array_index >= RESOURCE_VAR(res)->type->length) { + return -1; + } return RESOURCE_VAR(res)->data.location + array_index - FRAG_RESULT_DATA0; case GL_UNIFORM: - index = _mesa_get_uniform_location(shProg, name, &offset); - - if (index == GL_INVALID_INDEX) + /* If the uniform is built-in, fail. */ + if (RESOURCE_UNI(res)->builtin) return -1; /* From the GL_ARB_uniform_buffer_object spec: @@ -830,17 +835,21 @@ program_resource_location(struct gl_shader_program *shProg, RESOURCE_UNI(res)->atomic_buffer_index != -1) return -1; - /* location in remap table + array element offset */ - return RESOURCE_UNI(res)->remap_location + offset; - + /* fallthrough */ case GL_VERTEX_SUBROUTINE_UNIFORM: case GL_GEOMETRY_SUBROUTINE_UNIFORM: case GL_FRAGMENT_SUBROUTINE_UNIFORM: case GL_COMPUTE_SUBROUTINE_UNIFORM: case GL_TESS_CONTROL_SUBROUTINE_UNIFORM: case GL_TESS_EVALUATION_SUBROUTINE_UNIFORM: - offset_ret = parse_program_resource_name(name, &base_name_end); - return RESOURCE_UNI(res)->remap_location + ((offset_ret != -1) ? offset_ret : 0); + /* If the uniform is an array, fail if the index is out of bounds. */ + if (array_index > 0 + && array_index >= RESOURCE_UNI(res)->array_elements) { + return -1; + } + + /* location in remap table + array element offset */ + return RESOURCE_UNI(res)->remap_location + array_index; default: return -1; } @@ -854,14 +863,16 @@ GLint _mesa_program_resource_location(struct gl_shader_program *shProg, GLenum programInterface, const char *name) { + unsigned array_index = 0; struct gl_program_resource *res = - _mesa_program_resource_find_name(shProg, programInterface, name); + _mesa_program_resource_find_name(shProg, programInterface, name, + &array_index); /* Resource not found. */ if (!res) return -1; - return program_resource_location(shProg, res, name); + return program_resource_location(shProg, res, name, array_index); } /** @@ -873,7 +884,7 @@ _mesa_program_resource_location_index(struct gl_shader_program *shProg, GLenum programInterface, const char *name) { struct gl_program_resource *res = - _mesa_program_resource_find_name(shProg, programInterface, name); + _mesa_program_resource_find_name(shProg, programInterface, name, NULL); /* Non-existent variable or resource is not referenced by fragment stage. */ if (!res || !(res->StageReferences & (1 << MESA_SHADER_FRAGMENT))) @@ -949,7 +960,8 @@ get_buffer_property(struct gl_shader_program *shProg, for (unsigned i = 0; i < RESOURCE_UBO(res)->NumUniforms; i++) { const char *iname = RESOURCE_UBO(res)->Uniforms[i].IndexName; struct gl_program_resource *uni = - _mesa_program_resource_find_name(shProg, GL_UNIFORM, iname); + _mesa_program_resource_find_name(shProg, GL_UNIFORM, iname, + NULL); if (!uni) continue; (*val)++; @@ -959,7 +971,8 @@ get_buffer_property(struct gl_shader_program *shProg, for (unsigned i = 0; i < RESOURCE_UBO(res)->NumUniforms; i++) { const char *iname = RESOURCE_UBO(res)->Uniforms[i].IndexName; struct gl_program_resource *uni = - _mesa_program_resource_find_name(shProg, GL_UNIFORM, iname); + _mesa_program_resource_find_name(shProg, GL_UNIFORM, iname, + NULL); if (!uni) continue; *val++ = @@ -1099,7 +1112,8 @@ _mesa_program_resource_prop(struct gl_shader_program *shProg, case GL_PROGRAM_INPUT: case GL_PROGRAM_OUTPUT: *val = program_resource_location(shProg, res, - _mesa_program_resource_name(res)); + _mesa_program_resource_name(res), + 0); return 1; default: goto invalid_operation; diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c index 0887c68..b702dcd 100644 --- a/src/mesa/main/shaderapi.c +++ b/src/mesa/main/shaderapi.c @@ -2229,7 +2229,7 @@ _mesa_GetSubroutineIndex(GLuint program, GLenum shadertype, } resource_type = _mesa_shader_stage_to_subroutine(stage); - res = _mesa_program_resource_find_name(shProg, resource_type, name); + res = _mesa_program_resource_find_name(shProg, resource_type, name, NULL); if (!res) { _mesa_error(ctx, GL_INVALID_OPERATION, "%s", api_name); return -1; diff --git a/src/mesa/main/shaderapi.h b/src/mesa/main/shaderapi.h index c081f74..0a10191 100644 --- a/src/mesa/main/shaderapi.h +++ b/src/mesa/main/shaderapi.h @@ -232,7 +232,8 @@ _mesa_program_resource_index(struct gl_shader_program *shProg, extern 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); extern struct gl_program_resource * _mesa_program_resource_find_index(struct gl_shader_program *shProg, diff --git a/src/mesa/main/uniforms.c b/src/mesa/main/uniforms.c index 6ba746e..ff1df72 100644 --- a/src/mesa/main/uniforms.c +++ b/src/mesa/main/uniforms.c @@ -952,7 +952,7 @@ _mesa_GetUniformBlockIndex(GLuint program, struct gl_program_resource *res = _mesa_program_resource_find_name(shProg, GL_UNIFORM_BLOCK, - uniformBlockName); + uniformBlockName, NULL); if (!res) return GL_INVALID_INDEX; @@ -987,7 +987,8 @@ _mesa_GetUniformIndices(GLuint program, for (i = 0; i < uniformCount; i++) { struct gl_program_resource *res = - _mesa_program_resource_find_name(shProg, GL_UNIFORM, uniformNames[i]); + _mesa_program_resource_find_name(shProg, GL_UNIFORM, uniformNames[i], + NULL); uniformIndices[i] = _mesa_program_resource_index(shProg, res); } } -- 2.7.4