[Mesa-dev] [PATCH 05/14] mesa: add KHR_no_error support to glUniform*() functions

Nicolai Hähnle nhaehnle at gmail.com
Thu Apr 13 09:50:27 UTC 2017


On 13.04.2017 01:42, Timothy Arceri wrote:
> ---
>  src/mesa/main/uniform_query.cpp | 76 ++++++++++++++++++++++++++++-------------
>  1 file changed, 53 insertions(+), 23 deletions(-)
>
> diff --git a/src/mesa/main/uniform_query.cpp b/src/mesa/main/uniform_query.cpp
> index e613898..7aa035a 100644
> --- a/src/mesa/main/uniform_query.cpp
> +++ b/src/mesa/main/uniform_query.cpp
> @@ -775,57 +775,50 @@ glsl_type_name(enum glsl_base_type type)
>     case GLSL_TYPE_VOID:
>        return "void";
>     case GLSL_TYPE_ERROR:
>        return "error";
>     default:
>        return "other";
>     }
>  }
>
>
> -/**
> - * Called via glUniform*() functions.
> - */
> -extern "C" void
> -_mesa_uniform(GLint location, GLsizei count, const GLvoid *values,
> -              struct gl_context *ctx, struct gl_shader_program *shProg,
> -              enum glsl_base_type basicType, unsigned src_components)
> +struct gl_uniform_storage *
> +validate_uniform(GLint location, GLsizei count, const GLvoid *values,
> +                 unsigned *offset, struct gl_context *ctx,
> +                 struct gl_shader_program *shProg,
> +                 enum glsl_base_type basicType, unsigned src_components)

This should probably be a static function.


>  {
> -   unsigned offset;
> -   int size_mul = glsl_base_type_is_64bit(basicType) ? 2 : 1;
> -
>     struct gl_uniform_storage *const uni =
> -      validate_uniform_parameters(location, count, &offset,
> +      validate_uniform_parameters(location, count, offset,
>                                    ctx, shProg, "glUniform");
>     if (uni == NULL)
> -      return;
> +      return NULL;
>
>     if (uni->type->is_matrix()) {
>        /* Can't set matrix uniforms (like mat4) with glUniform */
>        _mesa_error(ctx, GL_INVALID_OPERATION,
>                    "glUniform%u(uniform \"%s\"@%d is matrix)",
>                    src_components, uni->name, location);
> -      return;
> +      return NULL;
>     }
>
> -   /* Verify that the types are compatible.
> -    */

Why are you removing this comment?


>     const unsigned components = uni->type->is_sampler()
>        ? 1 : uni->type->vector_elements;
>
>     if (components != src_components) {
>        /* glUniformN() must match float/vecN type */
>        _mesa_error(ctx, GL_INVALID_OPERATION,
>                    "glUniform%u(\"%s\"@%u has %u components, not %u)",
>                    src_components, uni->name, location,
>                    components, src_components);
> -      return;
> +      return NULL;
>     }
>
>     bool match;
>     switch (uni->type->base_type) {
>     case GLSL_TYPE_BOOL:
>        match = (basicType != GLSL_TYPE_DOUBLE);
>        break;
>     case GLSL_TYPE_SAMPLER:
>        match = (basicType == GLSL_TYPE_INT);
>        break;
> @@ -836,26 +829,26 @@ _mesa_uniform(GLint location, GLsizei count, const GLvoid *values,
>        match = (basicType == uni->type->base_type);
>        break;
>     }
>
>     if (!match) {
>        _mesa_error(ctx, GL_INVALID_OPERATION,
>                    "glUniform%u(\"%s\"@%d is %s, not %s)",
>                    src_components, uni->name, location,
>                    glsl_type_name(uni->type->base_type),
>                    glsl_type_name(basicType));
> -      return;
> +      return NULL;
>     }
>
>     if (unlikely(ctx->_Shader->Flags & GLSL_UNIFORMS)) {
>        log_uniform(values, basicType, components, 1, count,
> -		  false, shProg, location, uni);
> +                  false, shProg, location, uni);
>     }
>
>     /* Page 100 (page 116 of the PDF) of the OpenGL 3.0 spec says:
>      *
>      *     "Setting a sampler's value to i selects texture image unit number
>      *     i. The values of i range from zero to the implementation- dependent
>      *     maximum supported number of texture image units."
>      *
>      * In addition, table 2.3, "Summary of GL errors," on page 17 (page 33 of
>      * the PDF) says:
> @@ -863,51 +856,88 @@ _mesa_uniform(GLint location, GLsizei count, const GLvoid *values,
>      *     "Error         Description                    Offending command
>      *                                                   ignored?
>      *     ...
>      *     INVALID_VALUE  Numeric argument out of range  Yes"
>      *
>      * Based on that, when an invalid sampler is specified, we generate a
>      * GL_INVALID_VALUE error and ignore the command.
>      */
>     if (uni->type->is_sampler()) {
>        for (int i = 0; i < count; i++) {
> -	 const unsigned texUnit = ((unsigned *) values)[i];
> +         const unsigned texUnit = ((unsigned *) values)[i];
>
>           /* check that the sampler (tex unit index) is legal */
>           if (texUnit >= ctx->Const.MaxCombinedTextureImageUnits) {
>              _mesa_error(ctx, GL_INVALID_VALUE,
>                          "glUniform1i(invalid sampler/tex unit index for "
> -			"uniform %d)",
> -                        location);
> -            return;
> +                        "uniform %d)", location);
> +            return NULL;
>           }
>        }
>        /* We need to reset the validate flag on changes to samplers in case
>         * two different sampler types are set to the same texture unit.
>         */
>        ctx->_Shader->Validated = GL_FALSE;
>     }
>
>     if (uni->type->is_image()) {
>        for (int i = 0; i < count; i++) {
>           const int unit = ((GLint *) values)[i];
>
>           /* check that the image unit is legal */
>           if (unit < 0 || unit >= (int)ctx->Const.MaxImageUnits) {
>              _mesa_error(ctx, GL_INVALID_VALUE,
>                          "glUniform1i(invalid image unit index for uniform %d)",
>                          location);
> -            return;
> +            return NULL;
>           }
>        }
>     }
>
> +   return uni;
> +}
> +
> +
> +/**
> + * Called via glUniform*() functions.
> + */
> +extern "C" void
> +_mesa_uniform(GLint location, GLsizei count, const GLvoid *values,
> +              struct gl_context *ctx, struct gl_shader_program *shProg,
> +              enum glsl_base_type basicType, unsigned src_components)
> +{
> +   unsigned offset;
> +   int size_mul = glsl_base_type_is_64bit(basicType) ? 2 : 1;
> +
> +   struct gl_uniform_storage *uni;
> +   if (_mesa_is_no_error_enabled(ctx)) {
> +      uni = shProg->UniformRemapTable[location];
> +
> +      if (uni->array_elements == 0) {
> +         assert((location - uni->remap_location) == 0);
> +         offset = 0;
> +      } else {
> +         /* The array index specified by the uniform location is just the
> +          * uniform location minus the base location of of the uniform.
> +          */
> +         offset = location - uni->remap_location;
> +      }

I think you can just simplify this to:

   assert(uni->array_element > 0 || location == uni->remap_location);
   offset = location - uni->remap_location;

Saves a branch...

Cheers,
Nicolai


> +   } else {
> +      uni = validate_uniform(location, count, values, &offset, ctx, shProg,
> +                             basicType, src_components);
> +      if (!uni)
> +         return;
> +   }
> +
> +   const unsigned components = uni->type->is_sampler()
> +      ? 1 : uni->type->vector_elements;
> +
>     /* Page 82 (page 96 of the PDF) of the OpenGL 2.1 spec says:
>      *
>      *     "When loading N elements starting at an arbitrary position k in a
>      *     uniform declared as an array, elements k through k + N - 1 in the
>      *     array will be replaced with the new values. Values for any array
>      *     element that exceeds the highest array element index used, as
>      *     reported by GetActiveUniform, will be ignored by the GL."
>      *
>      * Clamp 'count' to a valid value.  Note that for non-arrays a count > 1
>      * will have already generated an error.
>


-- 
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.


More information about the mesa-dev mailing list