[Mesa-dev] [PATCH 1/2] main: Remove interface block array index for doing the name comparison

Tapani Pälli tapani.palli at intel.com
Thu Oct 22 04:08:22 PDT 2015


On 10/22/2015 12:01 PM, 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.

This seems strange as the extension spec (and GL core spec) has set of 
different examples that state the new queries to match behavior of old 
functions (using words "are equivalent to calling"), this is why our 
tests also cross-check that new and old functions return same values. 
I'll need to have a look at tests below.

As example extension spec says GetFragDataIndex to be equivalent to 
calling  GetProgramResourceLocationIndex(program, PROGRAM_OUTPUT, name).

> Fixes the following two dEQP-GLES31 tests:
>
> dEQP-GLES31.functional.program_interface_query.shader_storage_block.resource_list.block_array
> dEQP-GLES31.functional.program_interface_query.shader_storage_block.resource_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);
>   
>         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