[Mesa-dev] [PATCH 2/2] mesa: remove duplicate array index validation and extra uniform location lookup

Timothy Arceri t_arceri at yahoo.com.au
Fri Jul 3 05:35:13 PDT 2015


On Fri, 2015-07-03 at 15:08 +0300, Martin Peres wrote:
> 
> On 03/07/15 13:55, Timothy Arceri wrote:
> > 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.peres at linux.intel.com>
> > Cc: Tapani Pälli <tapani.palli at 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) {
> 
> RESOURCE_VAR instead RESOURCE_UNI.
> 
> > +         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_UNI(res)->type->length) {
> 
> RESOURCE_VAR instead RESOURCE_UNI.

Whoops. I guess there are no piglit test that push the boundary.

> 
> > +         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;
> >   
> > +      /* 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;
> > +      }
> >         /* From the GL_ARB_uniform_buffer_object spec:
> >          *
> >          *     "The value -1 will be returned if <name> does not 
> > correspond to an
> > @@ -778,7 +735,7 @@ program_resource_location(struct gl_shader_program 
> > *shProg,
> >            return -1;
> >   
> >         /* location in remap table + array element offset */
> > -      return RESOURCE_UNI(res)->remap_location + offset;
> > +      return RESOURCE_UNI(res)->remap_location + array_index;
> >   
> >      default:
> >         return -1;
> > @@ -787,22 +744,23 @@ program_resource_location(struct gl_shader_program 
> > *shProg,
> >   
> >   /**
> >    * Function implements following location queries:
> > - *    glGetAttribLocation
> > - *    glGetFragDataLocation
> Move the comment to the right function implementing these.

I dont think this is necessary as those functions are:
_mesa_GetFragDataLocation
_mesa_GetAttribLocation

I didn't change this I just noticed the comment was wrong. Maybe this should
have been its own patch.

> 
> I will read the patch in details later, but this is not my code so I 
> feel like we should wait for tapani to be back from vacation. If 
> necessary, I will review it but that will take me a lot of time and this 
> is not something I have right now :)

Sure I just Cc'd you as you guys seemed to be working closely on this stuff.
Just out of interest when is Tapani due back?

> 
> >    *    glGetUniformLocation
> >    */
> >   GLint
> >   _mesa_program_resource_location(struct gl_shader_program *shProg,
> > -                                GLenum programInterface, const char 
> > *name)
> > +                                GLenum programInterface, const char 
> > *name,
> > +                                bool is_resource_query)
> >   {
> > +   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, false);
> >   
> >      /* Resource not found. */
> >      if (!res)
> >         return -1;
> >   
> > -   return program_resource_location(shProg, res, name);
> > +   return program_resource_location(shProg, res, name, array_index);
> >   }
> >   
> >   /**
> > @@ -814,7 +772,8 @@ _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, false);
> >   
> >      /* Non-existent variable or resource is not referenced by fragment 
> > stage. */
> >      if (!res || !(res->StageReferences & (1 << MESA_SHADER_FRAGMENT)))
> > @@ -886,7 +845,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, false);
> >               if (!uni)
> >                  continue;
> >               (*val)++;
> > @@ -896,7 +856,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, false);
> >               if (!uni)
> >                  continue;
> >               *val++ =
> > @@ -1034,7 +995,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.h b/src/mesa/main/shaderapi.h
> > index aba6d5d..186b10d 100644
> > --- a/src/mesa/main/shaderapi.h
> > +++ b/src/mesa/main/shaderapi.h
> > @@ -232,7 +232,9 @@ _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,
> > +                                 bool is_resource_query);
> >   
> >   extern struct gl_program_resource *
> >   _mesa_program_resource_find_index(struct gl_shader_program *shProg,
> > @@ -246,7 +248,8 @@ _mesa_get_program_resource_name(struct 
> > gl_shader_program *shProg,
> >   
> >   extern GLint
> >   _mesa_program_resource_location(struct gl_shader_program *shProg,
> > -                                GLenum programInterface, const char 
> > *name);
> > +                                GLenum programInterface, const char 
> > *name,
> > +                                bool is_resource_query);
> >   
> >   extern GLint
> >   _mesa_program_resource_location_index(struct gl_shader_program *shProg,
> > diff --git a/src/mesa/main/uniform_query.cpp 
> > b/src/mesa/main/uniform_query.cpp
> > index cab5083..680ba16 100644
> > --- a/src/mesa/main/uniform_query.cpp
> > +++ b/src/mesa/main/uniform_query.cpp
> > @@ -978,81 +978,6 @@ _mesa_uniform_matrix(struct gl_context *ctx, struct 
> > gl_shader_program *shProg,
> >   }
> >   
> >   
> > -/**
> > - * Called via glGetUniformLocation().
> > - *
> > - * Returns the uniform index into UniformStorage (also the
> > - * glGetActiveUniformsiv uniform index), and stores the referenced
> > - * array offset in *offset, or GL_INVALID_INDEX (-1).
> > - */
> > -extern "C" unsigned
> > -_mesa_get_uniform_location(struct gl_shader_program *shProg,
> > -                           const GLchar *name,
> > -                           unsigned *out_offset)
> > -{
> > -   /* 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.
> > -    */
> > -
> > -   const GLchar *base_name_end;
> > -   long offset = parse_program_resource_name(name, &base_name_end);
> > -   bool array_lookup = offset >= 0;
> > -   char *name_copy;
> > -
> > -   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;
> > -   }
> > -
> > -   unsigned location = 0;
> > -   const bool found = shProg->UniformHash->get(location, name_copy);
> > -
> > -   assert(!found
> > -     || strcmp(name_copy, shProg->UniformStorage[location].name) == 
> > 0);
> > -
> > -   /* Free the temporary buffer *before* possibly returning an error.
> > -    */
> > -   if (name_copy != name)
> > -      free(name_copy);
> > -
> > -   if (!found)
> > -      return GL_INVALID_INDEX;
> > -
> > -   /* If the uniform is built-in, fail. */
> > -   if (shProg->UniformStorage[location].builtin)
> > -      return GL_INVALID_INDEX;
> > -
> > -   /* If the uniform is an array, fail if the index is out of bounds.
> > -    * (A negative index is caught above.)  This also fails if the uniform
> > -    * is not an array, but the user is trying to index it, because
> > -    * array_elements is zero and offset >= 0.
> > -    */
> > -   if (array_lookup
> > -       && offset >= (long) shProg
> > ->UniformStorage[location].array_elements) {
> > -      return GL_INVALID_INDEX;
> > -   }
> > -
> > -   *out_offset = offset;
> > -   return location;
> > -}
> > -
> >   extern "C" bool
> >   _mesa_sampler_uniforms_are_valid(const struct gl_shader_program *shProg,
> >                              char *errMsg, size_t errMsgLength)
> > diff --git a/src/mesa/main/uniforms.c b/src/mesa/main/uniforms.c
> > index 5548d1d..7c9ff7e 100644
> > --- a/src/mesa/main/uniforms.c
> > +++ b/src/mesa/main/uniforms.c
> > @@ -930,7 +930,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
> > @@ -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, false);
> >      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, false);
> >         uniformIndices[i] = _mesa_program_resource_index(shProg, res);
> >      }
> >   }
> > diff --git a/src/mesa/main/uniforms.h b/src/mesa/main/uniforms.h
> > index bd7b05e..e62eaa5 100644
> > --- a/src/mesa/main/uniforms.h
> > +++ b/src/mesa/main/uniforms.h
> > @@ -343,10 +343,6 @@ void GLAPIENTRY
> >   _mesa_ProgramUniformMatrix4x3dv(GLuint program, GLint location, GLsizei 
> > count,
> >                                   GLboolean transpose, const GLdouble 
> > *value);
> >   
> > -unsigned
> > -_mesa_get_uniform_location(struct gl_shader_program *shProg,
> > -                      const GLchar *name, unsigned *offset);
> > -
> >   void
> >   _mesa_uniform(struct gl_context *ctx, struct gl_shader_program 
> > *shader_program,
> >           GLint location, GLsizei count,
> 


More information about the mesa-dev mailing list