[Mesa-dev] [PATCH] vbo: Ignore invalid element ranges if fixing 'end' breaks 'start'.

[Kenneth Graunke]

Certain applications, such as Regnum Online, appear to pass invalid
start/end values to glDrawRangeElements.  In particular, the 'end' index
sometimes exceeds the maximum array element.

In this case, we issued a warning and attempted to "fix" the end value
by clamping it to the last array element.  Then we re-checked whether
end < start (which would normally result in an API error), and if so,
dropped the call on the floor.

This seems foolish: if 'end' is invalid, why should we trust 'start'?
The application has already proven that it can't track these things.

This patch takes a more conservative approach:
1. Force 'end' to be the maximum array element.
2. If end < start, assume both bounds were rubbish and discard them,
   treating the call as an ordinary glDrawElements().

This should allow more broken applications to work, while preserving the
useful "This should probably be fixed in the application" warning.

NOTE: This is a candidate for release branches.

Cc: Mateusz Kaduk <mateusz.kaduk at gmail.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=45214
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=44701
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=41152
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=40361
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=28138
Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
---
 src/mesa/vbo/vbo_exec_array.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

Originally, I thought we must be doing something wrong, as I saw this
message in some Unigine demos, Regnum Online, and even an oglconform
test case.

It looks like the oglconform test case is indeed specifying an 'end'
value way larger than the buffer size, which is wrong (though the GL
isn't required to raise an error.)

I've also gone through the _MaxElement calculations and convinced
myself that they're correct.  Regnum is also passing us a range
that's _completely_ outside of the buffer---both start and end are
past _MaxElement.  A small miscalculation wouldn't account for this.

I also did some googling and found a bunch of bug reports about this
where application authors were told about this warning and did appear
to find and fix bugs in their application.

So, at the end of the day, I think these apps are broken, but since
we're already working around them, we may as well make them work.

I'd appreciate review since this is my first foray into VBO land.
The ~0's match what vbo_exec_DrawElements uses, so I feel pretty safe.

Tested on Sandybridge.  No regressions in Piglit's quick-driver.tests,
the Khronos ES 2.0 test suite, or the vao tests in Intel's oglconform.

diff --git a/src/mesa/vbo/vbo_exec_array.c b/src/mesa/vbo/vbo_exec_array.c
index d6b4d61..8a3be1b 100644
--- a/src/mesa/vbo/vbo_exec_array.c
+++ b/src/mesa/vbo/vbo_exec_array.c
@@ -937,7 +937,12 @@ vbo_exec_DrawRangeElementsBaseVertex(GLenum mode,
       end = ctx->Array.ArrayObj->_MaxElement - 1;
 
       if (end < start) {
-         return;
+	 /* We found that 'end' was too large, and corrected it to be the
+	  * maximum value.  Now it's less than 'start', which probably means
+	  * that 'start' was invalid too.  Just ignore the range entirely and
+	  * treat this as a call to glDrawElements.
+	  */
+	 start = end = ~0;
       }
    }
 
-- 
1.7.7.6