[Mesa-dev] [PATCH 1/2] main: Remove interface block array index for doing the name comparison
Timothy Arceri
timothy.arceri at collabora.com
Thu Oct 22 04:06:27 PDT 2015
On Thu, 2015-10-22 at 11:01 +0200, Samuel Iglesias Gonsalvez wrote:
> From ARB_program_query_interface spec:
>
> "uint GetProgramResourceIndex(uint program, enum programInterface,
> const char *name);
> [...]
> If <name> exactly matches the name string of one of the active
> resources
> for <programInterface>, the index of the matched resource is
> returned.
> Additionally, if <name> would exactly match the name string of an
> active
> resource if "[0]" were appended to <name>, the index of the matched
> resource is returned. [...]"
>
> "A string provided to GetProgramResourceLocation or
> GetProgramResourceLocationIndex is considered to match an active
> variable
> if:
> [...]
> * 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;
> [...]
> "
>
> But this is only happening on those ARB_program_query_interface's
> queries.
> For the rest of specs we need to keep old behavior. For that reason,
> arb_program_interface_query boolean is added to the affected
> functions.
>
> Fixes the following two dEQP-GLES31 tests:
>
> dEQP
> -GLES31.functional.program_interface_query.shader_storage_block.resou
> rce_list.block_array
> dEQP
> -GLES31.functional.program_interface_query.shader_storage_block.resou
> rce_list.block_array_single_element
> Signed-off-by: Samuel Iglesias Gonsalvez <siglesias at igalia.com>
> Cc: Tapani Pälli <tapani.palli at intel.com>
> ---
> src/mesa/main/program_resource.c | 6 ++---
> src/mesa/main/shader_query.cpp | 58
> +++++++++++++++++++++++++++++++---------
> src/mesa/main/shaderapi.c | 4 +--
> src/mesa/main/shaderapi.h | 9 ++++---
> src/mesa/main/uniforms.c | 6 ++---
> 5 files changed, 60 insertions(+), 23 deletions(-)
>
> diff --git a/src/mesa/main/program_resource.c
> b/src/mesa/main/program_resource.c
> index eb71fdd..d029926 100644
> --- a/src/mesa/main/program_resource.c
> +++ b/src/mesa/main/program_resource.c
> @@ -251,7 +251,7 @@ _mesa_GetProgramResourceIndex(GLuint program,
> GLenum programInterface,
> case GL_UNIFORM_BLOCK:
> case GL_SHADER_STORAGE_BLOCK:
> res = _mesa_program_resource_find_name(shProg,
> programInterface, name,
> - &array_index);
> + &array_index, true);
> if (!res || array_index > 0)
> return GL_INVALID_INDEX;
>
> @@ -377,7 +377,7 @@ _mesa_GetProgramResourceLocation(GLuint program,
> GLenum programInterface,
> goto fail;
> }
>
> - return _mesa_program_resource_location(shProg, programInterface,
> name);
> + return _mesa_program_resource_location(shProg, programInterface,
> name, true);
> fail:
> _mesa_error(ctx, GL_INVALID_ENUM,
> "glGetProgramResourceLocation(%s %s)",
> _mesa_enum_to_string(programInterface), name);
> @@ -411,5 +411,5 @@ _mesa_GetProgramResourceLocationIndex(GLuint
> program, GLenum programInterface,
> }
>
> return _mesa_program_resource_location_index(shProg,
> GL_PROGRAM_OUTPUT,
> - name);
> + name, true);
> }
> diff --git a/src/mesa/main/shader_query.cpp
> b/src/mesa/main/shader_query.cpp
> index 8182d3d..49766ca 100644
> --- a/src/mesa/main/shader_query.cpp
> +++ b/src/mesa/main/shader_query.cpp
> @@ -218,7 +218,7 @@ _mesa_GetAttribLocation(GLhandleARB program,
> const GLcharARB * name)
> unsigned array_index = 0;
> struct gl_program_resource *res =
> _mesa_program_resource_find_name(shProg, GL_PROGRAM_INPUT,
> name,
> - &array_index);
> + &array_index, false);
>
> if (!res)
> return -1;
> @@ -367,7 +367,7 @@ _mesa_GetFragDataIndex(GLuint program, const
> GLchar *name)
> return -1;
>
> return _mesa_program_resource_location_index(shProg,
> GL_PROGRAM_OUTPUT,
> - name);
> + name, false);
> }
>
> GLint GLAPIENTRY
> @@ -404,7 +404,7 @@ _mesa_GetFragDataLocation(GLuint program, const
> GLchar *name)
> unsigned array_index = 0;
> struct gl_program_resource *res =
> _mesa_program_resource_find_name(shProg, GL_PROGRAM_OUTPUT,
> name,
> - &array_index);
> + &array_index, false);
>
> if (!res)
> return -1;
> @@ -533,9 +533,12 @@ valid_array_index(const GLchar *name, unsigned
> *array_index)
> struct gl_program_resource *
> _mesa_program_resource_find_name(struct gl_shader_program *shProg,
> GLenum programInterface, const char
> *name,
> - unsigned *array_index)
> + unsigned *array_index,
> + bool arb_program_interface_query)
> {
> struct gl_program_resource *res = shProg->ProgramResourceList;
> + const char *name_first_square_bracket = strchr(name, '[');
> +
> for (unsigned i = 0; i < shProg->NumProgramResourceList; i++,
> res++) {
> if (res->Type != programInterface)
> continue;
> @@ -543,6 +546,33 @@ _mesa_program_resource_find_name(struct
> gl_shader_program *shProg,
> /* Resource basename. */
> const char *rname = _mesa_program_resource_name(res);
> unsigned baselen = strlen(rname);
> + const char *rname_first_square_bracket = strchr(rname, '[');
> +
> + /* From ARB_program_interface_query spec:
> + *
> + * "uint GetProgramResourceIndex(uint program, enum
> programInterface,
> + * const char *name);
> + * [...]
> + * If <name> exactly matches the name string of one of the
> active
> + * resources for <programInterface>, the index of the matched
> resource is
> + * returned. Additionally, if <name> would exactly match the
> name string
> + * of an active resource if "[0]" were appended to <name>,
> the index of
> + * the matched resource is returned. [...]"
> + *
> + * "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; [...]"
> + */
> + /* Remove array's index from interface block name comparison
> */
> + if (arb_program_interface_query &&
> + !name_first_square_bracket && rname_first_square_bracket)
> + baselen -= strlen(rname_first_square_bracket);
This patch doesn't look right to me for starters I don't think it will
work for arrays of arrays as matching works like this
resource_name[2][3] or resource_name[2][3][0]
Also this looks like a problem only with shader storage blocks, for
other types normaly _mesa_program_resource_name() returns the resource
with the innermost array removed (it normally doesn't get appended in
the first place, at least thats how uniforms work see the name building
in link_uniforms.cpp).
I don't have time to dip deeper right now but just a couple of things
to consider.
>
> if (strncmp(rname, name, baselen) == 0) {
> switch (programInterface) {
> @@ -845,12 +875,14 @@ program_resource_location(struct
> gl_shader_program *shProg,
> */
> GLint
> _mesa_program_resource_location(struct gl_shader_program *shProg,
> - GLenum programInterface, const char
> *name)
> + GLenum programInterface, const char
> *name,
> + bool arb_program_interface_query)
> {
> unsigned array_index = 0;
> struct gl_program_resource *res =
> _mesa_program_resource_find_name(shProg, programInterface,
> name,
> - &array_index);
> + &array_index,
> + arb_program_interface_query);
>
> /* Resource not found. */
> if (!res)
> @@ -865,10 +897,12 @@ _mesa_program_resource_location(struct
> gl_shader_program *shProg,
> */
> GLint
> _mesa_program_resource_location_index(struct gl_shader_program
> *shProg,
> - GLenum programInterface, const
> char *name)
> + GLenum programInterface, const
> char *name,
> + bool
> arb_program_interface_query)
> {
> struct gl_program_resource *res =
> - _mesa_program_resource_find_name(shProg, programInterface,
> name, NULL);
> + _mesa_program_resource_find_name(shProg, programInterface,
> name, NULL,
> + arb_program_interface_query);
>
> /* Non-existent variable or resource is not referenced by
> fragment stage. */
> if (!res || !(res->StageReferences & (1 <<
> MESA_SHADER_FRAGMENT)))
> @@ -946,7 +980,7 @@ get_buffer_property(struct gl_shader_program
> *shProg,
> const char *iname = RESOURCE_UBO(res)
> ->Uniforms[i].IndexName;
> struct gl_program_resource *uni =
> _mesa_program_resource_find_name(shProg, GL_UNIFORM,
> iname,
> - NULL);
> + NULL, false);
> if (!uni)
> continue;
> (*val)++;
> @@ -958,7 +992,7 @@ get_buffer_property(struct gl_shader_program
> *shProg,
> const char *iname = RESOURCE_UBO(res)
> ->Uniforms[i].IndexName;
> struct gl_program_resource *uni =
> _mesa_program_resource_find_name(shProg, GL_UNIFORM,
> iname,
> - NULL);
> + NULL, false);
> if (!uni)
> continue;
> *val++ =
> @@ -982,7 +1016,7 @@ get_buffer_property(struct gl_shader_program
> *shProg,
> const char *iname = RESOURCE_UBO(res)
> ->Uniforms[i].IndexName;
> struct gl_program_resource *uni =
> _mesa_program_resource_find_name(shProg,
> GL_BUFFER_VARIABLE,
> - iname, NULL);
> + iname, NULL, false);
> if (!uni)
> continue;
> (*val)++;
> @@ -994,7 +1028,7 @@ get_buffer_property(struct gl_shader_program
> *shProg,
> const char *iname = RESOURCE_UBO(res)
> ->Uniforms[i].IndexName;
> struct gl_program_resource *uni =
> _mesa_program_resource_find_name(shProg,
> GL_BUFFER_VARIABLE,
> - iname, NULL);
> + iname, NULL, false);
> if (!uni)
> continue;
> *val++ =
> diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
> index 765602e..00cefd0 100644
> --- a/src/mesa/main/shaderapi.c
> +++ b/src/mesa/main/shaderapi.c
> @@ -2267,7 +2267,7 @@ _mesa_GetSubroutineUniformLocation(GLuint
> program, GLenum shadertype,
> }
>
> resource_type = _mesa_shader_stage_to_subroutine_uniform(stage);
> - return _mesa_program_resource_location(shProg, resource_type,
> name);
> + return _mesa_program_resource_location(shProg, resource_type,
> name, false);
> }
>
> GLuint GLAPIENTRY
> @@ -2302,7 +2302,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, NULL);
> + res = _mesa_program_resource_find_name(shProg, resource_type,
> name, NULL, false);
> 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 fba767b..2e6b9af 100644
> --- a/src/mesa/main/shaderapi.h
> +++ b/src/mesa/main/shaderapi.h
> @@ -233,7 +233,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,
> - unsigned *array_index);
> + unsigned *array_index,
> + bool arb_program_interface_query);
>
> extern struct gl_program_resource *
> _mesa_program_resource_find_index(struct gl_shader_program *shProg,
> @@ -250,11 +251,13 @@ _mesa_program_resource_name_len(struct
> gl_program_resource *res);
>
> extern GLint
> _mesa_program_resource_location(struct gl_shader_program *shProg,
> - GLenum programInterface, const char
> *name);
> + GLenum programInterface, const char
> *name,
> + bool arb_program_interface_query);
>
> extern GLint
> _mesa_program_resource_location_index(struct gl_shader_program
> *shProg,
> - GLenum programInterface, const
> char *name);
> + GLenum programInterface, const
> char *name,
> + bool
> arb_program_interface_query);
>
> extern unsigned
> _mesa_program_resource_prop(struct gl_shader_program *shProg,
> diff --git a/src/mesa/main/uniforms.c b/src/mesa/main/uniforms.c
> index bc23538..d240183 100644
> --- a/src/mesa/main/uniforms.c
> +++ b/src/mesa/main/uniforms.c
> @@ -921,7 +921,7 @@ _mesa_GetUniformLocation(GLuint programObj, const
> GLcharARB *name)
> return -1;
> }
>
> - return _mesa_program_resource_location(shProg, GL_UNIFORM, name);
> + return _mesa_program_resource_location(shProg, GL_UNIFORM, name,
> false);
> }
>
> GLuint GLAPIENTRY
> @@ -943,7 +943,7 @@ _mesa_GetUniformBlockIndex(GLuint program,
>
> struct gl_program_resource *res =
> _mesa_program_resource_find_name(shProg, GL_UNIFORM_BLOCK,
> - uniformBlockName, NULL);
> + uniformBlockName, NULL,
> false);
> if (!res)
> return GL_INVALID_INDEX;
>
> @@ -979,7 +979,7 @@ _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],
> - NULL);
> + NULL, false);
> uniformIndices[i] = _mesa_program_resource_index(shProg, res);
> }
> }
More information about the mesa-dev
mailing list