mesa: Rework array error checks in validate_uniform_parameters
authorIan Romanick <ian.d.romanick@intel.com>
Mon, 20 Oct 2014 23:40:50 +0000 (16:40 -0700)
committerIan Romanick <ian.d.romanick@intel.com>
Mon, 10 Nov 2014 12:25:40 +0000 (04:25 -0800)
Before ARB_explicit_uniform_location, Mesa's location encoding allowed
locations for non-array types that had non-zero array indices.
Basically, part of the location was the uniform and part was the array
index.  This meant that some checks had to occur for arrays and
non-arrays.  This is no longer possible, we the checks can be split up.

Valgrind callgrind results for a trace of Tesseract:

                 _mesa_Uniform4fv  _mesa_Uniform4f  _mesa_Uniform1i
Before (64-bit):       50,499,557      17,487,316           686,227
After  (64-bit):       50,023,791      17,274,432           684,293

                 _mesa_Uniform4fv  _mesa_Uniform4f  _mesa_Uniform1i
Before (32-bit):       62,968,039       21,732,380          828,147
After  (32-bit):       62,373,967       21,490,756          826,223

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>
src/mesa/main/uniform_query.cpp

index 16e08d44cd67076fe1cd50657d62f798f54df9fa..b87dbdf50d5cc42a26491896ae763f05fd9507cc 100644 (file)
@@ -252,27 +252,30 @@ validate_uniform_parameters(struct gl_context *ctx,
 
    struct gl_uniform_storage *const uni = shProg->UniformRemapTable[location];
 
-   if (uni->array_elements == 0 && count > 1) {
-      _mesa_error(ctx, GL_INVALID_OPERATION,
-                 "%s(count > 1 for non-array, location=%d)",
-                 caller, location);
-      return NULL;
-   }
+   if (uni->array_elements == 0) {
+      if (count > 1) {
+         _mesa_error(ctx, GL_INVALID_OPERATION,
+                     "%s(count > 1 for non-array, location=%d)",
+                     caller, location);
+         return NULL;
+      }
 
-   /* The array index specified by the uniform location is just the uniform
-    * location minus the base location of of the uniform.
-    */
-   *array_index = location - uni->remap_location;
+      assert((location - uni->remap_location) == 0);
+      *array_index = 0;
+   } else {
+      /* The array index specified by the uniform location is just the uniform
+       * location minus the base location of of the uniform.
+       */
+      *array_index = location - uni->remap_location;
 
-   /* If the uniform is an array, check that array_index is in bounds.
-    * If not an array, check that array_index is zero.
-    * array_index is unsigned so no need to check for less than zero.
-    */
-   const unsigned limit = MAX2(uni->array_elements, 1);
-   if (*array_index >= limit) {
-      _mesa_error(ctx, GL_INVALID_OPERATION, "%s(location=%d)",
-                 caller, location);
-      return NULL;
+      /* If the uniform is an array, check that array_index is in bounds.
+       * array_index is unsigned so no need to check for less than zero.
+       */
+      if (*array_index >= uni->array_elements) {
+         _mesa_error(ctx, GL_INVALID_OPERATION, "%s(location=%d)",
+                     caller, location);
+         return NULL;
+      }
    }
    return uni;
 }