[Mesa-dev] [PATCH 06/23] mesa: glGetProgramResourceLocation

Ilia Mirkin imirkin at alum.mit.edu
Mon Mar 16 10:08:33 PDT 2015


On Fri, Mar 13, 2015 at 4:37 AM, Tapani Pälli <tapani.palli at intel.com> wrote:
> Patch adds required helper functions to shaderapi.h and
> the actual implementation.
>
> corresponding Piglit test:
>    arb_program_interface_query-resource-location
>
> The added functionality can be tested by tests for following
> functions that are refactored by later patches:
>
>    GetAttribLocation
>    GetUniformLocation
>    GetFragDataLocation
>
> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
> ---
>  src/mesa/main/program_resource.c | 81 +++++++++++++++++++++++++++++++++++++++-
>  src/mesa/main/shader_query.cpp   | 64 +++++++++++++++++++++++++++++++
>  src/mesa/main/shaderapi.h        |  4 ++
>  3 files changed, 148 insertions(+), 1 deletion(-)
>
> diff --git a/src/mesa/main/program_resource.c b/src/mesa/main/program_resource.c
> index 4fa6ac6..87a0144 100644
> --- a/src/mesa/main/program_resource.c
> +++ b/src/mesa/main/program_resource.c
> @@ -243,11 +243,90 @@ _mesa_GetProgramResourceiv(GLuint program, GLenum programInterface,
>  {
>  }
>
> +/**
> + * 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 *array = strrchr(name, '[');
> +
> +   if (!array)
> +      return false;
> +
> +   /* No '+' or ' ' allowed anywhere. */
> +   if (strchr(name, '+') || strchr(name, ' '))

I guess it'd be mildly better to do a strchr('[') and use that for the
second strchr's? You could do it like

char *first = strchr(name, '[');
char *last = strrchr(first, '[');
if (strchr(first, '+') || ... )

That way you avoid iterating over the "name" portion of it
unnecessarily. Probably doesn't amount to too much.

> +      return true;
> +
> +   /* Check that last array index is 0. */
> +   if (array[1] == '0' && array[2] != ']')
> +      return true;
> +
> +   return false;
> +}
> +
> +static struct gl_shader_program *
> +lookup_linked_program(GLuint program, const char *caller)
> +{
> +   GET_CURRENT_CONTEXT(ctx);
> +   struct gl_shader_program *prog =
> +      _mesa_lookup_shader_program_err(ctx, program, caller);
> +
> +   if (!prog)
> +      return NULL;
> +
> +   if (prog->LinkStatus == GL_FALSE) {
> +      _mesa_error(ctx, GL_INVALID_OPERATION,
> +                  "%s(program not linked)", caller);
> +      return NULL;
> +   }
> +   return prog;
> +}
> +
>  GLint GLAPIENTRY
>  _mesa_GetProgramResourceLocation(GLuint program, GLenum programInterface,
>                                   const GLchar *name)
>  {
> -   return -1;
> +   GET_CURRENT_CONTEXT(ctx);
> +   struct gl_shader_program *shProg =
> +      lookup_linked_program(program, "glGetProgramResourceLocation");
> +
> +   if (!shProg || invalid_array_element_syntax(name))
> +      return -1;
> +
> +   /* Validate programInterface. */
> +   switch (programInterface) {
> +   case GL_UNIFORM:
> +   case GL_PROGRAM_INPUT:
> +   case GL_PROGRAM_OUTPUT:
> +      break;
> +
> +   /* For reference valid cases requiring addition extension support:
> +    * GL_ARB_shader_subroutine
> +    * GL_ARB_tessellation_shader
> +    * GL_ARB_compute_shader
> +    */
> +   case GL_VERTEX_SUBROUTINE_UNIFORM:
> +   case GL_TESS_CONTROL_SUBROUTINE_UNIFORM:
> +   case GL_TESS_EVALUATION_SUBROUTINE_UNIFORM:
> +   case GL_GEOMETRY_SUBROUTINE_UNIFORM:
> +   case GL_FRAGMENT_SUBROUTINE_UNIFORM:
> +   case GL_COMPUTE_SUBROUTINE_UNIFORM:
> +
> +   default:
> +      _mesa_error(ctx, GL_INVALID_ENUM,
> +                  "glGetProgramResourceLocation(%s %s)",
> +                  _mesa_lookup_enum_by_nr(programInterface), name);
> +   }
> +
> +   return _mesa_program_resource_location(shProg, programInterface, name);
>  }
>
>  GLint GLAPIENTRY
> diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
> index 77a4af0..4ae00a6 100644
> --- a/src/mesa/main/shader_query.cpp
> +++ b/src/mesa/main/shader_query.cpp
> @@ -743,3 +743,67 @@ _mesa_get_program_resource_name(struct gl_shader_program *shProg,
>     }
>     return true;
>  }
> +
> +static GLint
> +program_resource_location(struct gl_shader_program *shProg,
> +                          struct gl_program_resource *res, const char *name)
> +{
> +   unsigned index, offset;
> +   int array_index = -1;

And I suppose leaving off the initializer here makes gcc complain?

> +
> +   if (res->Type == GL_PROGRAM_INPUT ||
> +       res->Type == GL_PROGRAM_OUTPUT) {

put all on one line?

> +      array_index = array_index_of_resource(res, name);
> +      if (array_index < 0)
> +         return -1;
> +   }
> +
> +   switch (res->Type) {
> +   case GL_PROGRAM_INPUT:
> +      return RESOURCE_VAR(res)->data.location + array_index - VERT_ATTRIB_GENERIC0;
> +   case GL_PROGRAM_OUTPUT:
> +      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)
> +         return -1;
> +
> +      /* From the GL_ARB_uniform_buffer_object spec:
> +       *
> +       *     "The value -1 will be returned if <name> does not correspond to an
> +       *     active uniform variable name in <program>, if <name> is associated
> +       *     with a named uniform block, or if <name> starts with the reserved
> +       *     prefix "gl_"."
> +       */
> +      if (RESOURCE_UNI(res)->block_index != -1 ||
> +          RESOURCE_UNI(res)->atomic_buffer_index != -1)
> +         return -1;
> +
> +      /* location in remap table + array element offset */
> +      return RESOURCE_UNI(res)->remap_location + offset;
> +
> +   default:
> +      return -1;
> +   }
> +}
> +
> +/**
> + * Function implements following location queries:
> + *    glGetAttribLocation
> + *    glGetFragDataLocation
> + *    glGetUniformLocation
> + */
> +GLint
> +_mesa_program_resource_location(struct gl_shader_program *shProg,
> +                                GLenum interface, const char *name)
> +{
> +   struct gl_program_resource *res =
> +      _mesa_program_resource_find_name(shProg, interface, name);
> +
> +   /* Resource not found. */
> +   if (!res)
> +      return -1;
> +
> +   return program_resource_location(shProg, res, name);
> +}
> diff --git a/src/mesa/main/shaderapi.h b/src/mesa/main/shaderapi.h
> index 7a7e3e9..73ebf60 100644
> --- a/src/mesa/main/shaderapi.h
> +++ b/src/mesa/main/shaderapi.h
> @@ -244,6 +244,10 @@ _mesa_get_program_resource_name(struct gl_shader_program *shProg,
>                                  GLsizei bufSize, GLsizei *length,
>                                  GLchar *name, const char *caller);
>
> +extern GLint
> +_mesa_program_resource_location(struct gl_shader_program *shProg,
> +                                GLenum interface, const char *name);
> +
>  #ifdef __cplusplus
>  }
>  #endif
> --
> 2.1.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list