[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