[Mesa-dev] [PATCH] mesa: fixes for DrawRangeElements[BaseVertex] index validation

[Roland Scheidegger]

In vbo_exec_DrawRangeElementsBaseVertex, take into account the basevertex.
As far as I can tell it is completely ok (though maybe stupid) to have
start/end of 100/199, with _MaxElement being 100, if the basevertex
is -100 (since the start/end are prior to adding basevertex). The opposite
is also true (0/99 is not ok if _MaxElement is 100 and and basevertex is 100).
Previously we would have issued a warning in such cases and worse probably
"fixed" end to a bogus value.
Untested, found by code inspection when looking at bug 40361 (which
seems unrelated after all).

v2: As per mailing list discussion and the suggestion by Kenneth Graunke just drop
the questionable effort to fix up the range and revert to have no range instead.
Rearrange some (disabled) debug bits a bit for slightly less complex code.
---
 src/mesa/vbo/vbo_exec_array.c |   63 +++++++++++++++++++---------------------
 1 files changed, 30 insertions(+), 33 deletions(-)

diff --git a/src/mesa/vbo/vbo_exec_array.c b/src/mesa/vbo/vbo_exec_array.c
index d6b4d61..1f62e3a 100644
--- a/src/mesa/vbo/vbo_exec_array.c
+++ b/src/mesa/vbo/vbo_exec_array.c
@@ -885,17 +885,33 @@ vbo_exec_DrawRangeElementsBaseVertex(GLenum mode,
       end = MIN2(end, 0xffff);
    }
 
-   if (end >= ctx->Array.ArrayObj->_MaxElement) {
-      /* the max element is out of bounds of one or more enabled arrays */
+   if (0) {
+      printf("glDrawRangeElements{,BaseVertex}"
+	     "(start %u, end %u, type 0x%x, count %d) ElemBuf %u, "
+	     "base %d\n",
+	     start, end, type, count,
+	     ctx->Array.ArrayObj->ElementArrayBufferObj->Name,
+	     basevertex);
+   }
+
+#if 0
+   check_draw_elements_data(ctx, count, type, indices);
+#else
+   (void) check_draw_elements_data;
+#endif
+
+   if ((int)(start + basevertex) < 0 ||
+       end + basevertex >= ctx->Array.ArrayObj->_MaxElement) {
+      /* the max (or min) element is out of bounds of one or more enabled arrays */
       warnCount++;
 
       if (warnCount < 10) {
-         _mesa_warning(ctx, "glDraw[Range]Elements(start %u, end %u, count %d, "
-                       "type 0x%x, indices=%p)\n"
+         _mesa_warning(ctx, "glDrawRangeElements[BaseVertex](start %u, end %u, count %d, "
+                       "type 0x%x, indices=%p, base %d)\n"
                        "\tend is out of bounds (max=%u)  "
                        "Element Buffer %u (size %d)\n"
                        "\tThis should probably be fixed in the application.",
-                       start, end, count, type, indices,
+                       start, end, count, type, indices, basevertex,
                        ctx->Array.ArrayObj->_MaxElement - 1,
                        ctx->Array.ArrayObj->ElementArrayBufferObj->Name,
                        (int) ctx->Array.ArrayObj->ElementArrayBufferObj->Size);
@@ -913,14 +929,14 @@ vbo_exec_DrawRangeElementsBaseVertex(GLenum mode,
       if (0) {
          GLuint max = _mesa_max_buffer_index(ctx, count, type, indices,
                                              ctx->Array.ArrayObj->ElementArrayBufferObj);
-         if (max >= ctx->Array.ArrayObj->_MaxElement) {
+         if (max + basevertex >= ctx->Array.ArrayObj->_MaxElement) {
             if (warnCount < 10) {
-               _mesa_warning(ctx, "glDraw[Range]Elements(start %u, end %u, "
-                             "count %d, type 0x%x, indices=%p)\n"
+               _mesa_warning(ctx, "glDrawRangeElements[BaseVertex](start %u, end %u, "
+                             "count %d, type 0x%x, indices=%p, base %d)\n"
                              "\tindex=%u is out of bounds (max=%u)  "
                              "Element Buffer %u (size %d)\n"
                              "\tSkipping the glDrawRangeElements() call",
-                             start, end, count, type, indices, max,
+                             start, end, count, type, indices, basevertex, max,
                              ctx->Array.ArrayObj->_MaxElement - 1,
                              ctx->Array.ArrayObj->ElementArrayBufferObj->Name,
                              (int) ctx->Array.ArrayObj->ElementArrayBufferObj->Size);
@@ -928,34 +944,15 @@ vbo_exec_DrawRangeElementsBaseVertex(GLenum mode,
          }
          /* XXX we could also find the min index and compare to 'start'
           * to see if start is correct.  But it's more likely to get the
-          * upper bound wrong.
+          * upper bound wrong, at least for the non-BaseVertex variant...
           */
       }
-
-      /* Set 'end' to the max possible legal value */
-      assert(ctx->Array.ArrayObj->_MaxElement >= 1);
-      end = ctx->Array.ArrayObj->_MaxElement - 1;
-
-      if (end < start) {
-         return;
-      }
-   }
-
-   if (0) {
-      printf("glDraw[Range]Elements{,BaseVertex}"
-	     "(start %u, end %u, type 0x%x, count %d) ElemBuf %u, "
-	     "base %d\n",
-	     start, end, type, count,
-	     ctx->Array.ArrayObj->ElementArrayBufferObj->Name,
-	     basevertex);
+      /* range is bogus, instead of trying to fix it up just revert to no range */
+      vbo_validated_drawrangeelements(ctx, mode, GL_FALSE, ~0, ~0,
+                                      count, type, indices, basevertex, 1);
+      return;
    }
 
-#if 0
-   check_draw_elements_data(ctx, count, type, indices);
-#else
-   (void) check_draw_elements_data;
-#endif
-
    vbo_validated_drawrangeelements(ctx, mode, GL_TRUE, start, end,
 				   count, type, indices, basevertex, 1);
 }
-- 
1.7.1