From 3f5ebb98b74cca8488ee7093aab87125ee5341ed Mon Sep 17 00:00:00 2001 From: Ian Romanick Date: Mon, 20 Oct 2014 16:21:54 -0700 Subject: [PATCH] mesa: Rework location == -1 error checking MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Only one caller wanted to generate an error when location == -1, so move the error generation to that caller. There will be more callers in the future that do not want to generate errors. Move the location == -1 check later in validate_uniform_parameters. As currently implemented, glUniform1iv(-1, -1, data) would not generate an error, but it should due to count being < 0. The location that I have moved it to will make more sense with the next commit. Valgrind callgrind results for a trace of Tesseract: _mesa_Uniform4fv _mesa_Uniform4f _mesa_Uniform1i Before (64-bit): 51,241,217 17,740,162 689,181 After (64-bit): 50,499,557 17,487,316 686,227 _mesa_Uniform4fv _mesa_Uniform4f _mesa_Uniform1i Before (32-bit): 63,940,605 21,987,918 831,065 After (32-bit): 62,968,039 21,732,380 828,147 Signed-off-by: Ian Romanick Reviewed-by: Brian Paul Reviewed-by: Tapani Pälli --- src/mesa/main/uniform_query.cpp | 76 ++++++++++++++++----------------- 1 file changed, 38 insertions(+), 38 deletions(-) diff --git a/src/mesa/main/uniform_query.cpp b/src/mesa/main/uniform_query.cpp index a6992c7767b..a1ca3678cf5 100644 --- a/src/mesa/main/uniform_query.cpp +++ b/src/mesa/main/uniform_query.cpp @@ -176,46 +176,13 @@ validate_uniform_parameters(struct gl_context *ctx, struct gl_shader_program *shProg, GLint location, GLsizei count, unsigned *array_index, - const char *caller, - bool negative_one_is_not_valid) + const char *caller) { if (!shProg || !shProg->LinkStatus) { _mesa_error(ctx, GL_INVALID_OPERATION, "%s(program not linked)", caller); return NULL; } - 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 NULL; - } - /* 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 @@ -233,6 +200,9 @@ validate_uniform_parameters(struct gl_context *ctx, return NULL; } + if (location == -1) + return NULL; + /* Page 82 (page 96 of the PDF) of the OpenGL 2.1 spec says: * * "If any of the following conditions occur, an INVALID_OPERATION @@ -308,9 +278,39 @@ _mesa_get_uniform(struct gl_context *ctx, GLuint program, GLint location, struct gl_uniform_storage *const uni = validate_uniform_parameters(ctx, shProg, location, 1, - &offset, "glGetUniform", true); - if (uni == NULL) + &offset, "glGetUniform"); + if (uni == NULL) { + /* 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. + * + * If the location was -1, validate_unfirom_parameters will return NULL + * without raising an error. Raise the error here. + */ + if (location == -1) { + _mesa_error(ctx, GL_INVALID_OPERATION, "glGetUniform(location=%d)", + location); + } + return; + } { unsigned elements = (uni->type->is_sampler()) @@ -590,7 +590,7 @@ _mesa_uniform(struct gl_context *ctx, struct gl_shader_program *shProg, struct gl_uniform_storage *const uni = validate_uniform_parameters(ctx, shProg, location, count, - &offset, "glUniform", false); + &offset, "glUniform"); if (uni == NULL) return; @@ -796,7 +796,7 @@ _mesa_uniform_matrix(struct gl_context *ctx, struct gl_shader_program *shProg, struct gl_uniform_storage *const uni = validate_uniform_parameters(ctx, shProg, location, count, - &offset, "glUniformMatrix", false); + &offset, "glUniformMatrix"); if (uni == NULL) return; -- 2.30.2