[Mesa-dev] [PATCH 08/23] mesa: implementation of glGetProgramResourceiv

Ilia Mirkin imirkin at alum.mit.edu
Mon Mar 16 11:30:42 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.
>
> The property query functionality can be tested with tests for
> following functions that are refactored by later patches:
>
>    GetActiveAtomicCounterBufferiv
>    GetActiveUniformBlockiv
>    GetActiveUniformsiv
>
> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
> ---
>  src/mesa/main/program_resource.c |  23 ++++
>  src/mesa/main/shader_query.cpp   | 258 +++++++++++++++++++++++++++++++++++++++
>  src/mesa/main/shaderapi.h        |  12 ++
>  3 files changed, 293 insertions(+)
>
> diff --git a/src/mesa/main/program_resource.c b/src/mesa/main/program_resource.c
> index ae987de..cdd5690 100644
> --- a/src/mesa/main/program_resource.c
> +++ b/src/mesa/main/program_resource.c
> @@ -241,6 +241,29 @@ _mesa_GetProgramResourceiv(GLuint program, GLenum programInterface,
>                             const GLenum *props, GLsizei bufSize,
>                             GLsizei *length, GLint *params)
>  {
> +   GET_CURRENT_CONTEXT(ctx);
> +
> +   struct gl_shader_program *shProg =
> +      _mesa_lookup_shader_program_err(ctx, program,
> +                                      "glGetProgramResourceiv");
> +   if (!shProg)
> +      return;
> +
> +   /* The error INVALID_VALUE is generated if <propCount> is zero.
> +    * Note that we check < 0 here because it makes sense.
> +    */
> +   if (propCount <= 0) {
> +      _mesa_error(ctx, GL_INVALID_VALUE,
> +                  "glGetProgramResourceiv(propCount <= 0)");
> +      return;
> +   }
> +
> +   /* No need to write any properties, user requested none. */
> +   if (bufSize <= 0)
> +      return;
> +
> +   _mesa_get_program_resourceiv(shProg, programInterface, index,
> +                                propCount, props, bufSize, length, params);
>  }
>
>  /**
> diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
> index d3264db..749cd32 100644
> --- a/src/mesa/main/shader_query.cpp
> +++ b/src/mesa/main/shader_query.cpp
> @@ -825,3 +825,261 @@ _mesa_program_resource_location_index(struct gl_shader_program *shProg,
>
>     return RESOURCE_VAR(res)->data.index;
>  }
> +
> +static uint8_t
> +stage_from_enum(GLenum ref)
> +{
> +   switch (ref) {
> +   case GL_REFERENCED_BY_VERTEX_SHADER:
> +      return MESA_SHADER_VERTEX;
> +   case GL_REFERENCED_BY_GEOMETRY_SHADER:
> +      return MESA_SHADER_GEOMETRY;
> +   case GL_REFERENCED_BY_FRAGMENT_SHADER:
> +      return MESA_SHADER_FRAGMENT;
> +   default:
> +      assert(!"shader stage not supported");
> +      return MESA_SHADER_STAGES;
> +   }
> +}
> +
> +/**
> + * Check if resource is referenced by given 'referenced by' stage enum.
> + * ATC and UBO resources hold stage references of their own.
> + */
> +static GLint
> +is_resource_referenced(struct gl_shader_program *shProg,
> +                       struct gl_program_resource *res,
> +                       GLuint index, uint8_t stage)
> +{
> +   if (res->Type == GL_ATOMIC_COUNTER_BUFFER)
> +      return RESOURCE_ATC(res)->StageReferences[stage];
> +
> +   if (res->Type == GL_UNIFORM_BLOCK)
> +      return shProg->UniformBlockStageIndex[stage][index] !=
> +         -1 ? 1 : 0;

This is _really_ hard to read. On the bright side you can just change this to

return foo != -1; and be done with it. This evaluates to 1/0.

> +
> +   return (res->StageReferences & (1 << stage));

return is not a function, no need for the parens around the returned value.

> +}
> +
> +static bool
> +get_buffer_property(struct gl_shader_program *shProg,
> +                    struct gl_program_resource *res, const GLenum prop,
> +                    GLint *val)
> +{
> +   if (res->Type != GL_UNIFORM_BLOCK &&
> +       res->Type != GL_ATOMIC_COUNTER_BUFFER)
> +      return false;
> +
> +   switch (prop) {
> +   case GL_BUFFER_BINDING:

Gah! This function is disgusting (not really your fault). Would it
look any less disgusting if you were to reverse the order, i.e.

if (res->type == uniform) {
  ubo_type *ubo = RESOURCE_UBO(res);
  switch () {
  case foo:
    return ubo->bar;
  }
} else {
  atc_type *atc = ...
  switch ...
}

There would be less annoying uniform/atomic repetition, and you'd lose
the ambiguous if/elseif/no else case which can't logically happen but
compiler might not notice situations.

Anyways, this is *completely* optional, merely wanted to suggest it in
case you liked it better.

> +      if (res->Type == GL_UNIFORM_BLOCK)
> +         *val = RESOURCE_UBO(res)->Binding;
> +      else if (res->Type == GL_ATOMIC_COUNTER_BUFFER)
> +         *val = RESOURCE_ATC(res)->Binding;
> +      return true;
> +   case GL_BUFFER_DATA_SIZE:
> +      if (res->Type == GL_UNIFORM_BLOCK)
> +         *val = RESOURCE_UBO(res)->UniformBufferSize;
> +      else if (res->Type == GL_ATOMIC_COUNTER_BUFFER)
> +         *val = RESOURCE_ATC(res)->MinimumSize;
> +      return true;
> +   case GL_NUM_ACTIVE_VARIABLES:
> +      if (res->Type == GL_UNIFORM_BLOCK)
> +         *val = RESOURCE_UBO(res)->NumUniforms;
> +      else if (res->Type == GL_ATOMIC_COUNTER_BUFFER)
> +         *val = RESOURCE_ATC(res)->NumUniforms;
> +      return true;
> +   case GL_ACTIVE_VARIABLES:
> +      if (res->Type == GL_ATOMIC_COUNTER_BUFFER) {
> +         int amount = RESOURCE_ATC(res)->NumUniforms;
> +         for (int i = 0; i < amount; i++)
> +            *val++ = RESOURCE_ATC(res)->Uniforms[i];
> +         return true;
> +      }
> +      if (res->Type == GL_UNIFORM_BLOCK) {
> +         int amount = RESOURCE_UBO(res)->NumUniforms;
> +         for (int i = 0; i < amount; i++) {
> +            const char *iname = RESOURCE_UBO(res)->Uniforms[i].IndexName;
> +            *val = program_resource_location(shProg, res, iname);
> +         }
> +         return true;
> +      }
> +   default:
> +      assert(!"support for property type not implemented");
> +   }
> +   return false;
> +}
> +
> +bool
> +_mesa_program_resource_prop(struct gl_shader_program *shProg,
> +                            struct gl_program_resource *res, GLuint index,
> +                            const GLenum prop, GLint *val, const char *caller)
> +{
> +   GET_CURRENT_CONTEXT(ctx);
> +
> +#define VALIDATE_TYPE(type)\
> +   if (res->Type != type)\
> +      goto invalid_operation;
> +
> +   switch(prop) {
> +   case GL_NAME_LENGTH:
> +      if (res->Type == GL_ATOMIC_COUNTER_BUFFER)
> +         goto invalid_operation;
> +      /* Base name +3 if array '[0]' + terminator. */
> +      *val = strlen(_mesa_program_resource_name(res)) +
> +         (_mesa_program_resource_array_size(res) > 0 ? 3 : 0) + 1;
> +      return true;
> +   case GL_TYPE:
> +      switch (res->Type) {
> +      case GL_UNIFORM:
> +         *val = RESOURCE_UNI(res)->type->gl_type;
> +         return true;
> +      case GL_PROGRAM_INPUT:
> +      case GL_PROGRAM_OUTPUT:
> +         *val = RESOURCE_VAR(res)->type->gl_type;
> +         return true;
> +      case GL_TRANSFORM_FEEDBACK_VARYING:
> +         *val = RESOURCE_XFB(res)->Type;
> +         return true;
> +      default:
> +         goto invalid_operation;
> +      }
> +   case GL_ARRAY_SIZE:
> +      switch (res->Type) {
> +      case GL_UNIFORM:
> +            *val = MAX2(RESOURCE_UNI(res)->array_elements, 1);
> +            return true;

indent is funny here.

> +      case GL_PROGRAM_INPUT:
> +      case GL_PROGRAM_OUTPUT:
> +         *val = MAX2(RESOURCE_VAR(res)->type->length, 1);
> +         return true;
> +      case GL_TRANSFORM_FEEDBACK_VARYING:
> +         *val = MAX2(RESOURCE_XFB(res)->Size, 1);
> +         return true;
> +      default:
> +         goto invalid_operation;
> +      }
> +   case GL_OFFSET:
> +      VALIDATE_TYPE(GL_UNIFORM);
> +      *val = RESOURCE_UNI(res)->offset;
> +      return true;
> +   case GL_BLOCK_INDEX:
> +      VALIDATE_TYPE(GL_UNIFORM);
> +      *val = RESOURCE_UNI(res)->block_index;
> +      return true;
> +   case GL_ARRAY_STRIDE:
> +      VALIDATE_TYPE(GL_UNIFORM);
> +      *val = RESOURCE_UNI(res)->array_stride;
> +      return true;
> +   case GL_MATRIX_STRIDE:
> +      VALIDATE_TYPE(GL_UNIFORM);
> +      *val = RESOURCE_UNI(res)->matrix_stride;
> +      return true;
> +   case GL_IS_ROW_MAJOR:
> +      VALIDATE_TYPE(GL_UNIFORM);
> +      *val = RESOURCE_UNI(res)->row_major;
> +      return true;
> +   case GL_ATOMIC_COUNTER_BUFFER_INDEX:
> +      VALIDATE_TYPE(GL_UNIFORM);
> +      *val = RESOURCE_UNI(res)->atomic_buffer_index;
> +      return true;
> +   case GL_BUFFER_BINDING:
> +   case GL_BUFFER_DATA_SIZE:
> +   case GL_NUM_ACTIVE_VARIABLES:
> +   case GL_ACTIVE_VARIABLES:
> +      if (!get_buffer_property(shProg, res, prop, val))
> +         goto invalid_operation;
> +      return true;
> +   case GL_REFERENCED_BY_VERTEX_SHADER:
> +   case GL_REFERENCED_BY_GEOMETRY_SHADER:
> +   case GL_REFERENCED_BY_FRAGMENT_SHADER:
> +      switch (res->Type) {
> +      case GL_UNIFORM:
> +      case GL_PROGRAM_INPUT:
> +      case GL_PROGRAM_OUTPUT:
> +      case GL_UNIFORM_BLOCK:
> +      case GL_ATOMIC_COUNTER_BUFFER:
> +         *val = is_resource_referenced(shProg, res, index,
> +                                       stage_from_enum(prop));
> +         return true;
> +      default:
> +         goto invalid_operation;
> +      }
> +   case GL_LOCATION:
> +      switch (res->Type) {
> +      case GL_UNIFORM:
> +      case GL_PROGRAM_INPUT:
> +      case GL_PROGRAM_OUTPUT:
> +         *val = program_resource_location(shProg, res,
> +                                          _mesa_program_resource_name(res));
> +         return true;
> +      default:
> +         goto invalid_operation;
> +      }
> +   case GL_LOCATION_INDEX:
> +      if (res->Type != GL_PROGRAM_OUTPUT)
> +         goto invalid_operation;
> +      *val = RESOURCE_VAR(res)->data.index;
> +      return true;
> +
> +   /* GL_ARB_tessellation_shader */
> +   case GL_IS_PER_PATCH:
> +   case GL_REFERENCED_BY_TESS_CONTROL_SHADER:
> +   case GL_REFERENCED_BY_TESS_EVALUATION_SHADER:
> +   /* GL_ARB_compute_shader */
> +   case GL_REFERENCED_BY_COMPUTE_SHADER:
> +   default:
> +      _mesa_error(ctx, GL_INVALID_ENUM,
> +                  "%s(%s prop %s)", caller,
> +                  _mesa_lookup_enum_by_nr(res->Type),
> +                  _mesa_lookup_enum_by_nr(prop));
> +      return false;
> +   }
> +
> +invalid_operation:
> +   _mesa_error(ctx, GL_INVALID_OPERATION,
> +               "%s(%s prop %s)", caller,
> +               _mesa_lookup_enum_by_nr(res->Type),
> +               _mesa_lookup_enum_by_nr(prop));
> +   return false;

The classy thing to do is to undef things like VALIDATE_TYPE since
they're super-specific to this function.

> +}
> +
> +extern void
> +_mesa_get_program_resourceiv(struct gl_shader_program *shProg,
> +                             GLenum interface, GLuint index, GLsizei propCount,
> +                             const GLenum *props, GLsizei bufSize,
> +                             GLsizei *length, GLint *params)
> +{
> +   GET_CURRENT_CONTEXT(ctx);
> +   GLint *val = (GLint *) params;
> +   const GLenum *prop = props;
> +   GLsizei amount = 0;
> +
> +   struct gl_program_resource *res =
> +      _mesa_program_resource_find_index(shProg, interface, index);
> +
> +   /* No such resource found. */
> +   if (!res) {
> +      _mesa_error(ctx, GL_INVALID_VALUE,
> +                  "glGetProgramResourceiv(%s index %d)",
> +                  _mesa_lookup_enum_by_nr(interface), index);
> +      return;
> +   }
> +
> +   /* Write values until error occurs or bufSize reached. */
> +   for (int i = 0; i < bufSize; i++, val++, prop++) {
> +      if (_mesa_program_resource_prop(shProg, res, index, *prop, val,
> +          "glGetProgramResourceiv")) {

this should be indented further in to make it clear that it's an
argument to the function. I think emacs would (by default) indent it
to the function's opening (, but other people will just indent it
another 3 or 6 chars... not sure if there's a fixed standard.

> +         amount++;
> +      } else {
> +         /* Error happened. */
> +         return;
> +      }
> +   }
> +
> +   /* If <length> is not NULL, the actual number of integer values
> +    * written to <params> will be written to <length>.
> +    */
> +   if (length)
> +      *length = amount;
> +}
> diff --git a/src/mesa/main/shaderapi.h b/src/mesa/main/shaderapi.h
> index 5046018..f1d038e 100644
> --- a/src/mesa/main/shaderapi.h
> +++ b/src/mesa/main/shaderapi.h
> @@ -252,6 +252,18 @@ extern GLint
>  _mesa_program_resource_location_index(struct gl_shader_program *shProg,
>                                        GLenum interface, const char *name);
>
> +extern bool
> +_mesa_program_resource_prop(struct gl_shader_program *shProg,
> +                            struct gl_program_resource *res, GLuint index,
> +                            const GLenum prop, GLint *val, const char *caller);
> +
> +extern void
> +_mesa_get_program_resourceiv(struct gl_shader_program *shProg,
> +                             GLenum interface, GLuint index,
> +                             GLsizei propCount, const GLenum *props,
> +                             GLsizei bufSize, GLsizei *length,
> +                             GLint *params);
> +
>  #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