fdo-mirrors/mesa
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/mesa/mesa.git synced 2026-05-05 09:38:07 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

mesa: Rework location == -1 error checking

Browse source 
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 <ian.d.romanick@intel.com>
Reviewed-by: Brian Paul <brianp@vmware.com>
Reviewed-by: Tapani Pälli <tapani.palli@intel.com>
This commit is contained in:
Ian Romanick 2014-10-20 16:21:54 -07:00
parent 23dcbf623f
commit 3f5ebb98b7
1 changed files with 38 additions and 38 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

76
src/mesa/main/uniform_query.cpp
View file

@ -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;