[Mesa-dev] [PATCH 07/20] mesa: Refactor parameter validate for GetUniform, Uniform, and UniformMatrix
Kenneth Graunke
kenneth at whitecape.org
Sun Oct 30 22:33:17 PDT 2011
On 10/28/2011 10:42 AM, Ian Romanick wrote:
> From: Ian Romanick <ian.d.romanick at intel.com>
>
> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
> ---
> src/mesa/main/uniforms.c | 142 +++++++++++++++++++++++++++++-----------------
> 1 files changed, 90 insertions(+), 52 deletions(-)
>
> diff --git a/src/mesa/main/uniforms.c b/src/mesa/main/uniforms.c
> index f968d9b..2a5318d 100644
> --- a/src/mesa/main/uniforms.c
> +++ b/src/mesa/main/uniforms.c
> @@ -35,7 +35,7 @@
> * 2. Insert FLUSH_VERTICES calls in various places
> */
>
> -
> +#include <stdbool.h>
> #include "main/glheader.h"
> #include "main/context.h"
> #include "main/dispatch.h"
> @@ -317,6 +317,81 @@ get_uniform_rows_cols(const struct gl_program_parameter *p,
> }
> }
>
> +static bool
> +validate_uniform_parameters(struct gl_context *ctx,
> + struct gl_shader_program *shProg,
> + GLint location, GLsizei count,
> + unsigned *loc,
> + unsigned *array_index,
> + const char *caller,
> + bool negative_one_is_not_valid)
> +{
> + if (!shProg || !shProg->LinkStatus) {
> + _mesa_error(ctx, GL_INVALID_OPERATION, "%s(program not linked)", caller);
> + return false;
> + }
> +
> + if (location == -1) {
> + /* Page 264 (page 278 of the PDF) of the OpenGL 2.1 spec says:
> + *
> + * "The error INVALID_OPERATION is generated if program has not been
> + * linked successfully, or if location is not a valid location for
> + * program."
> + *
> + * However, page 82 (page 96 of the PDF) of the OpenGL 2.1 spec says:
> + *
> + * "If the value of location is -1, the Uniform* commands will
> + * silently ignore the data passed in, and the current uniform
> + * values will not be changed."
> + *
> + * The negative_one_is_not_valid flag selects between the two behaviors.
Your wording here makes it sound like the GL spec contradicts itself,
when it's actually quite clear. The first behavior is for GetUniform,
while the second is for setting Uniforms. Perhaps mention that for clarity?
Otherwise, looks like a nice cleanup.
> + */
> + if (negative_one_is_not_valid) {
> + _mesa_error(ctx, GL_INVALID_OPERATION, "%s(location=%d)",
> + caller, location);
> + }
> +
> + return false;
> + }
> +
> + /* From page 12 (page 26 of the PDF) of the OpenGL 2.1 spec:
> + *
> + * "If a negative number is provided where an argument of type sizei or
> + * sizeiptr is specified, the error INVALID_VALUE is generated."
> + */
> + if (count < 0) {
> + _mesa_error(ctx, GL_INVALID_VALUE, "%s(count < 0)", caller);
> + return false;
> + }
> +
> + /* Page 82 (page 96 of the PDF) of the OpenGL 2.1 spec says:
> + *
> + * "If any of the following conditions occur, an INVALID_OPERATION
> + * error is generated by the Uniform* commands, and no uniform values
> + * are changed:
> + *
> + * ...
> + *
> + * - if no variable with a location of location exists in the
> + * program object currently in use and location is not -1,
> + */
> + if (location < -1) {
> + _mesa_error(ctx, GL_INVALID_OPERATION, "%s(location=%d)",
> + caller, location);
> + return false;
> + }
> +
> + _mesa_uniform_split_location_offset(location, loc, array_index);
> +
> + if (*loc >= shProg->Uniforms->NumUniforms) {
> + _mesa_error(ctx, GL_INVALID_OPERATION, "%s(location=%d)",
> + caller, location);
> + return false;
> + }
> +
> + return true;
> +}
> +
> /**
> * Called via glGetUniform[fiui]v() to get the current value of a uniform.
> */
> @@ -327,14 +402,14 @@ _mesa_get_uniform(struct gl_context *ctx, GLuint program, GLint location,
> struct gl_shader_program *shProg =
> _mesa_lookup_shader_program_err(ctx, program, "glGetUniformfv");
> struct gl_program *prog;
> - GLint paramPos, offset;
> + GLint paramPos;
> + unsigned loc, offset;
>
> - if (!shProg)
> + if (!validate_uniform_parameters(ctx, shProg, location, 1,
> + &loc, &offset, "glGetUniform", true))
> return;
>
> - _mesa_uniform_split_location_offset(location, &location, &offset);
> -
> - if (!find_uniform_parameter_pos(shProg, location, &prog, ¶mPos)) {
> + if (!find_uniform_parameter_pos(shProg, loc, &prog, ¶mPos)) {
> _mesa_error(ctx, GL_INVALID_OPERATION, "glGetUniformfv(location)");
> }
> else {
> @@ -743,41 +818,20 @@ _mesa_uniform(struct gl_context *ctx, struct gl_shader_program *shProg,
> const GLvoid *values, GLenum type)
> {
> struct gl_uniform *uniform;
> - GLint elems, offset;
> + GLint elems;
> + unsigned loc, offset;
>
> ASSERT_OUTSIDE_BEGIN_END(ctx);
>
> - if (!shProg || !shProg->LinkStatus) {
> - _mesa_error(ctx, GL_INVALID_OPERATION, "glUniform(program not linked)");
> - return;
> - }
> -
> - if (location == -1)
> - return; /* The standard specifies this as a no-op */
> -
> - if (location < -1) {
> - _mesa_error(ctx, GL_INVALID_OPERATION, "glUniform(location=%d)",
> - location);
> - return;
> - }
> -
> - _mesa_uniform_split_location_offset(location, &location, &offset);
> -
> - if (location < 0 || location >= (GLint) shProg->Uniforms->NumUniforms) {
> - _mesa_error(ctx, GL_INVALID_VALUE, "glUniform(location=%d)", location);
> + if (!validate_uniform_parameters(ctx, shProg, location, count,
> + &loc, &offset, "glUniform", false))
> return;
> - }
> -
> - if (count < 0) {
> - _mesa_error(ctx, GL_INVALID_VALUE, "glUniform(count < 0)");
> - return;
> - }
>
> elems = _mesa_sizeof_glsl_type(type);
>
> FLUSH_VERTICES(ctx, _NEW_PROGRAM_CONSTANTS);
>
> - uniform = &shProg->Uniforms->Uniforms[location];
> + uniform = &shProg->Uniforms->Uniforms[loc];
>
> if (ctx->Shader.Flags & GLSL_UNIFORMS) {
> const GLenum basicType = base_uniform_type(type);
> @@ -924,30 +978,14 @@ _mesa_uniform_matrix(struct gl_context *ctx, struct gl_shader_program *shProg,
> GLboolean transpose, const GLfloat *values)
> {
> struct gl_uniform *uniform;
> - GLint offset;
> + unsigned loc, offset;
>
> ASSERT_OUTSIDE_BEGIN_END(ctx);
>
> - if (!shProg || !shProg->LinkStatus) {
> - _mesa_error(ctx, GL_INVALID_OPERATION,
> - "glUniformMatrix(program not linked)");
> + if (!validate_uniform_parameters(ctx, shProg, location, count,
> + &loc, &offset, "glUniformMatrix", false))
> return;
> - }
> -
> - if (location == -1)
> - return; /* The standard specifies this as a no-op */
>
> - if (location < -1) {
> - _mesa_error(ctx, GL_INVALID_OPERATION, "glUniformMatrix(location)");
> - return;
> - }
> -
> - _mesa_uniform_split_location_offset(location, &location, &offset);
> -
> - if (location < 0 || location >= (GLint) shProg->Uniforms->NumUniforms) {
> - _mesa_error(ctx, GL_INVALID_VALUE, "glUniformMatrix(location)");
> - return;
> - }
> if (values == NULL) {
> _mesa_error(ctx, GL_INVALID_VALUE, "glUniformMatrix");
> return;
> @@ -955,7 +993,7 @@ _mesa_uniform_matrix(struct gl_context *ctx, struct gl_shader_program *shProg,
>
> FLUSH_VERTICES(ctx, _NEW_PROGRAM_CONSTANTS);
>
> - uniform = &shProg->Uniforms->Uniforms[location];
> + uniform = &shProg->Uniforms->Uniforms[loc];
>
> if (shProg->_LinkedShaders[MESA_SHADER_VERTEX]) {
> /* convert uniform location to program parameter index */
More information about the mesa-dev
mailing list