From: Ian Romanick Date: Fri, 14 Oct 2011 03:18:31 +0000 (-0700) Subject: mesa: Refactor parameter validate for GetUniform, Uniform, and UniformMatrix X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=65add4327d786826d28c72ea34034df8137f65d9;p=mesa.git mesa: Refactor parameter validate for GetUniform, Uniform, and UniformMatrix v2: Update a comment block about the different treatment of location=-1 based on feedback from Ken. Signed-off-by: Ian Romanick Tested-by: Tom Stellard --- diff --git a/src/mesa/main/uniforms.c b/src/mesa/main/uniforms.c index 041e0db2203..b68d63d7853 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 #include "main/glheader.h" #include "main/context.h" #include "main/dispatch.h" @@ -318,6 +318,90 @@ 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) { + /* For glGetUniform, 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." + * + * For glUniform, 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." + * + * Allowing -1 for the location parameter of glUniform allows + * applications to avoid error paths in the case that, for example, some + * uniform variable is removed by the compiler / linker after + * optimization. In this case, the new value of the uniform is dropped + * on the floor. For the case of glGetUniform, there is nothing + * sensible to do for a location of -1. + * + * The negative_one_is_not_valid flag selects between the two behaviors. + */ + 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. */ @@ -328,14 +412,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 { @@ -744,41 +828,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); @@ -925,30 +988,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; @@ -956,7 +1003,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 */