[Mesa-dev] [PATCH 2/4] vbo: Ignore invalid element ranges where 'start' is past the end.

[Roland Scheidegger]

Am 08.02.2012 14:08, schrieb Kenneth Graunke:
> Some applications, such as Regnum Online, appear to pass invalid
> start/end values to glDrawRangeElements.  In particular, the 'start'
> index sometimes exceeds the maximum array element.  This is clearly
> invalid behavior, and although the spec isn't clear, seems to result
> in undefined, implementation-specific behavior.
> 
> This patch takes the conservative approach and simply ignores the range,
> while issuing a warning indicating that the application is broken and
> should be fixed.
> 
> NOTE: This is a candidate for release branches.
> 
> 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 |   25 +++++++++++++++++++++++++
>  1 files changed, 25 insertions(+), 0 deletions(-)
> 
> diff --git a/src/mesa/vbo/vbo_exec_array.c b/src/mesa/vbo/vbo_exec_array.c
> index ec4cb4f..c26a8cd 100644
> --- a/src/mesa/vbo/vbo_exec_array.c
> +++ b/src/mesa/vbo/vbo_exec_array.c
> @@ -858,6 +858,7 @@ vbo_exec_DrawRangeElementsBaseVertex(GLenum mode,
>  				     const GLvoid *indices,
>  				     GLint basevertex)
>  {
> +   static GLuint warnCount = 0;
>     GET_CURRENT_CONTEXT(ctx);
>  
>     if (MESA_VERBOSE & VERBOSE_DRAW)
> @@ -870,6 +871,30 @@ vbo_exec_DrawRangeElementsBaseVertex(GLenum mode,
>                                            type, indices, basevertex ))
>        return;
>  
> +   if ((int)(start + basevertex) < 0 ||
> +       start + basevertex >= ctx->Array.ArrayObj->_MaxElement) {
I don't quite agree with this; if you only want to warn if the range is
not plausible then it must be end + basevertex < 0 or start + basevertex
>= _MaxElement, i.e. all possible indices are outside of the array.
start + basevertex < 0 would be just fine by the reasoning that end +
basevertex >= _MaxElement is also ok.

> +      /* The application requested we draw using a range of indices that's
> +       * outside the bounds of the current VBO.  This is invalid and appears
> +       * to give undefined results.  The safest thing to do is to simply
> +       * ignore the range, in case the application botched their range tracking
> +       * but did provide valid indices.  Also issue a warning indicating that
> +       * the application is broken.
> +       */
> +      if (warnCount++ < 10) {
> +         _mesa_warning(ctx, "glDrawRangeElements(start %u, end %u, "
> +                       "basevertex %d, count %d, type 0x%x, indices=%p):\n"
> +                       "\tstart is out of bounds (max=%u); ignoring range.\n"
> +                       "\tThis should be fixed in the application.",
> +                       start, end, basevertex, count, type, indices,
> +                       ctx->Array.ArrayObj->_MaxElement - 1);
> +      }
The warning text would also need some adjustment to account for end
being possibly wrong.

> +
> +      /* Just do an ordinary glDrawElementsBaseVertex(). */
> +      vbo_validated_drawrangeelements(ctx, mode, GL_FALSE, ~0, ~0,
> +                                      count, type, indices, basevertex, 1);
> +      return;
> +   }
> +
>     /* NOTE: It's important that 'end' is a reasonable value.
>      * in _tnl_draw_prims(), we use end to determine how many vertices
>      * to transform.  If it's too large, we can unnecessarily split prims

Roland