mesa: Rework location == -1 error checking

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>

diff --git a/src/mesa/main/uniform_query.cpp b/src/mesa/main/uniform_query.cpp
index a6992c7767b9f0d8ab5b58f6a54abbe9d708fe65..a1ca3678cf5ce8fcfb78eb2a92eaced41ca43c4e 100644 (file)
--- 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;