[Mesa-dev] [PATCH 3/4] vbo: Rework checking of 'end' against _MaxElement.

[Brian Paul]

On 02/08/2012 06:08 AM, Kenneth Graunke wrote:
> This failed to take basevertex into account:
>
> If basevertex<  0:
>     (end + basevertex) might actually be in-bounds while 'end' is not.
>     We would have clamped in this case when we probably shouldn't.
>     This could break application drawing.
>
> If basevertex>  0:
>     'end' might be in-bounds while (end + basevertex) might not.
>     We would have failed to clamp in this place.  There's a comment
>     indicating the TNL module depends on max_index being in-bounds;
>     if so, it would likely break horribly.
>
> Rather than trying to clamp correctly in the face of basevertex, simply
> delete the clamping code and indicate that we don't have a valid range.
> This causes _tnl_vbo_draw_prims to use vbo_get_minmax_indices() to
> compute the actual bounds, which is much safer.
>
> NOTE: This is a candidate for release branches.
>
> Signed-off-by: Kenneth Graunke<kenneth at whitecape.org>
> ---
>   src/mesa/vbo/vbo_exec_array.c |   17 ++++++-----------
>   1 files changed, 6 insertions(+), 11 deletions(-)
>
> diff --git a/src/mesa/vbo/vbo_exec_array.c b/src/mesa/vbo/vbo_exec_array.c
> index c26a8cd..67073df 100644
> --- a/src/mesa/vbo/vbo_exec_array.c
> +++ b/src/mesa/vbo/vbo_exec_array.c
> @@ -859,6 +859,7 @@ vbo_exec_DrawRangeElementsBaseVertex(GLenum mode,
>   				     GLint basevertex)
>   {
>      static GLuint warnCount = 0;
> +   GLboolean index_bounds_valid = GL_TRUE;
>      GET_CURRENT_CONTEXT(ctx);
>
>      if (MESA_VERBOSE&  VERBOSE_DRAW)
> @@ -911,16 +912,6 @@ vbo_exec_DrawRangeElementsBaseVertex(GLenum mode,
>         end = MIN2(end, 0xffff);
>      }
>
> -   if (end>= ctx->Array.ArrayObj->_MaxElement) {
> -      /* 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, "
> @@ -930,13 +921,17 @@ vbo_exec_DrawRangeElementsBaseVertex(GLenum mode,
>   	     basevertex);
>      }
>
> +   if ((int)(end + basevertex)<  0 ||

Again, maybe do this as:

       if ((int) end + basevertex < 0 ||


> +       end + basevertex>= ctx->Array.ArrayObj->_MaxElement)
> +      index_bounds_valid = GL_FALSE;
> +
>   #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,
> +   vbo_validated_drawrangeelements(ctx, mode, index_bounds_valid, start, end,
>   				   count, type, indices, basevertex, 1);
>   }
>

Reviewed-by: Brian Paul <brianp at vmware.com>