On 11/02/2012 01:12 PM, Frank Henigman wrote:
validate_uniform_parameters now checks that the array index is
valid.  This means if an index is out of bounds, glGetUniform* now
fails with GL_INVALID_OPERATION, as it should.
_mesa_uniform and _mesa_uniform_matrix also call
validate_uniform_parameters so the bounds checks there became
redundant and were removed.

The test in glGetUniformLocation is modified to check array bounds
so it now returns GL_INVALID_INDEX (-1) if you ask for the location
of a non-existent array element, as it should.

Do we have piglit tests for either of these cases?

Do all of the existing piglit tests that pass still pass with this change? It seems like they should...

---
  src/mesa/main/uniform_query.cpp |   26 +++++++++++++-------------
  1 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/src/mesa/main/uniform_query.cpp b/src/mesa/main/uniform_query.cpp
index bddb8f9..66a2399 100644
--- a/src/mesa/main/uniform_query.cpp
+++ b/src/mesa/main/uniform_query.cpp
@@ -237,11 +237,14 @@ validate_uniform_parameters(struct gl_context *ctx,
        return false;
     }

-   /* This case should be impossible.  The implication is that a call like
-    * glGetUniformLocation(prog, "foo[8]") was successful but "foo" is not an
-    * array.
+   /* 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.
      */
-   if (*array_index != 0 && shProg->UniformStorage[*loc].array_elements == 0) {
+   unsigned limit = shProg->UniformStorage[*loc].array_elements;
+   if (limit == 0)
+      limit = 1;
+   if (*array_index >= limit) {
        _mesa_error(ctx, GL_INVALID_OPERATION, "%s(location=%d)",
                  caller, location);
        return false;
@@ -728,9 +731,6 @@ _mesa_uniform(struct gl_context *ctx, struct 
gl_shader_program *shProg,
      * will have already generated an error.
      */
     if (uni->array_elements != 0) {
-      if (offset >= uni->array_elements)
-        return;
-
        count = MIN2(count, (int) (uni->array_elements - offset));
     }

@@ -885,9 +885,6 @@ _mesa_uniform_matrix(struct gl_context *ctx, struct 
gl_shader_program *shProg,
      * will have already generated an error.
      */
     if (uni->array_elements != 0) {
-      if (offset >= uni->array_elements)
-        return;
-
        count = MIN2(count, (int) (uni->array_elements - offset));
     }

@@ -1021,10 +1018,13 @@ _mesa_get_uniform_location(struct gl_context *ctx,
     if (!found)
        return GL_INVALID_INDEX;

-   /* Since array_elements is 0 for non-arrays, this causes look-ups of 'a[0]'
-    * to (correctly) fail if 'a' is not an array.
+   /* If the uniform is an array, fail if the index is out of bounds.
+    * (A negative index is caught above.)  This also fails if the uniform
+    * is not an array, but the user is trying to index it, because
+    * array_elements is zero and offset >= 0.
      */
-   if (array_lookup && shProg->UniformStorage[location].array_elements == 0) {
+   if (array_lookup
+        && offset >= shProg->UniformStorage[location].array_elements) {
        return GL_INVALID_INDEX;
     }



_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to