Eric Anholt wrote:
> This saves mapping the index buffer to get a bounds on the indices that
> drivers just drop on the floor in the VBO case (cache win), saves a bonus
> walk of the indices in the CheckArrayBounds case, and other miscellaneous
> validation. On intel it's a particularly a large win (50-100% in my app)
> because even though we let the indices stay in both CPU and GPU caches, we
> still end up waiting for the GPU to be done with the buffer before reading
> from it.
>
> Drivers supporting draw_prims must now handle min_index == ~0 indicating that
> min/max_index are not computed, if they want to use these fields.
This looks pretty good.
I guess the only thing I'd question is using min_index==~0 as the
indicator that indices have not been validated. Another approach
would be for drivers to call a new
vbo_enable_index_validation(GLboolean) function to tell the VBO module
whether or not it should validate indices. Would that be any better?
One other comment below...
-Brian
> ---
> src/mesa/drivers/dri/i965/brw_draw.c | 52 +++-------
> src/mesa/drivers/dri/i965/brw_draw_upload.c | 1 +
> src/mesa/drivers/dri/r300/r300_draw.c | 6 +
> src/mesa/tnl/t_draw.c | 3 +
> src/mesa/vbo/vbo.h | 5 +-
> src/mesa/vbo/vbo_exec_array.c | 151
> ++++++++++++++-------------
> 6 files changed, 107 insertions(+), 111 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_draw.c
> b/src/mesa/drivers/dri/i965/brw_draw.c
> index 5152c3f..01e5f36 100644
> --- a/src/mesa/drivers/dri/i965/brw_draw.c
> +++ b/src/mesa/drivers/dri/i965/brw_draw.c
> @@ -422,34 +422,6 @@ static GLboolean brw_try_draw_prims( GLcontext *ctx,
> return retval;
> }
>
> -static GLboolean brw_need_rebase( GLcontext *ctx,
> - const struct gl_client_array *arrays[],
> - const struct _mesa_index_buffer *ib,
> - GLuint min_index )
> -{
> - if (min_index == 0)
> - return GL_FALSE;
> -
> - if (ib) {
> - if (!vbo_all_varyings_in_vbos(arrays))
> - return GL_TRUE;
> - else
> - return GL_FALSE;
> - }
> - else {
> - /* Hmm. This isn't quite what I wanted. BRW can actually
> - * handle the mixed case well enough that we shouldn't need to
> - * rebase. However, it's probably not very common, nor hugely
> - * expensive to do it this way:
> - */
> - if (!vbo_all_varyings_in_vbos(arrays))
> - return GL_TRUE;
> - else
> - return GL_FALSE;
> - }
> -}
> -
> -
> void brw_draw_prims( GLcontext *ctx,
> const struct gl_client_array *arrays[],
> const struct _mesa_prim *prim,
> @@ -460,16 +432,20 @@ void brw_draw_prims( GLcontext *ctx,
> {
> GLboolean retval;
>
> - /* Decide if we want to rebase. If so we end up recursing once
> - * only into this function.
> - */
> - if (brw_need_rebase( ctx, arrays, ib, min_index )) {
> - vbo_rebase_prims( ctx, arrays,
> - prim, nr_prims,
> - ib, min_index, max_index,
> - brw_draw_prims );
> -
> - return;
> + if (!vbo_all_varyings_in_vbos(arrays)) {
> + if (min_index == ~0)
> + vbo_get_minmax_index(ctx, prim, ib, &min_index, &max_index);
> +
> + /* Decide if we want to rebase. If so we end up recursing once
> + * only into this function.
> + */
> + if (min_index != 0) {
> + vbo_rebase_prims(ctx, arrays,
> + prim, nr_prims,
> + ib, min_index, max_index,
> + brw_draw_prims );
> + return;
> + }
> }
>
> /* Make a first attempt at drawing:
> diff --git a/src/mesa/drivers/dri/i965/brw_draw_upload.c
> b/src/mesa/drivers/dri/i965/brw_draw_upload.c
> index 4bdb373..a4457b8 100644
> --- a/src/mesa/drivers/dri/i965/brw_draw_upload.c
> +++ b/src/mesa/drivers/dri/i965/brw_draw_upload.c
> @@ -411,6 +411,7 @@ static void brw_prepare_vertices(struct brw_context *brw)
> */
> assert(input->offset < input->bo->size);
> } else {
> + assert(min_index != ~0);
> input->count = input->glarray->StrideB ? max_index + 1 - min_index :
> 1;
> if (input->bo != NULL) {
> /* Already-uploaded vertex data is present from a previous
> diff --git a/src/mesa/drivers/dri/r300/r300_draw.c
> b/src/mesa/drivers/dri/r300/r300_draw.c
> index fcfd309..bc5a234 100644
> --- a/src/mesa/drivers/dri/r300/r300_draw.c
> +++ b/src/mesa/drivers/dri/r300/r300_draw.c
> @@ -476,6 +476,12 @@ static void r300DrawPrims(GLcontext *ctx,
> limits.max_indices = 65535;
> limits.max_vb_size = 1024*1024;
>
> + /* This check should get folded into just the places that
> + * min/max index are really needed.
> + */
> + if (min_index == ~0)
> + vbo_get_minmax_index(ctx, prim, ib, &min_index, &max_index);
> +
> if (min_index) {
> vbo_rebase_prims( ctx, arrays, prim, nr_prims, ib, min_index,
> max_index, r300DrawPrims );
> return;
> diff --git a/src/mesa/tnl/t_draw.c b/src/mesa/tnl/t_draw.c
> index 2ec65d5..d093293 100644
> --- a/src/mesa/tnl/t_draw.c
> +++ b/src/mesa/tnl/t_draw.c
> @@ -388,6 +388,9 @@ void _tnl_draw_prims( GLcontext *ctx,
> prim[i].count);
> }
>
> + if (min_index == ~0)
> + vbo_get_minmax_index(ctx, prim, ib, &min_index, &max_index);
> +
> if (min_index) {
> /* We always translate away calls with min_index != 0.
> */
> diff --git a/src/mesa/vbo/vbo.h b/src/mesa/vbo/vbo.h
> index 5362226..9052ce3 100644
> --- a/src/mesa/vbo/vbo.h
> +++ b/src/mesa/vbo/vbo.h
> @@ -112,7 +112,10 @@ void vbo_rebase_prims( GLcontext *ctx,
> GLuint min_index,
> GLuint max_index,
> vbo_draw_func draw );
> -
> +void
> +vbo_get_minmax_index(GLcontext *ctx, const struct _mesa_prim *prim,
> + const struct _mesa_index_buffer *ib,
> + GLuint *min_index, GLuint *max_index);
>
> void vbo_use_buffer_objects(GLcontext *ctx);
>
> diff --git a/src/mesa/vbo/vbo_exec_array.c b/src/mesa/vbo/vbo_exec_array.c
> index f4b9b2f..9ea3e4a 100644
> --- a/src/mesa/vbo/vbo_exec_array.c
> +++ b/src/mesa/vbo/vbo_exec_array.c
> @@ -41,15 +41,29 @@
> /**
> * Compute min and max elements for glDraw[Range]Elements() calls.
> */
> -static void
> -get_minmax_index(GLuint count, GLuint type, const GLvoid *indices,
> - GLuint *min_index, GLuint *max_index)
> +void
> +vbo_get_minmax_index(GLcontext *ctx,
> + const struct _mesa_prim *prim,
> + const struct _mesa_index_buffer *ib,
> + GLuint *min_index, GLuint *max_index)
> {
> GLuint i;
> + GLsizei count = prim->count;
> + const void *indices;
> +
> + if (ib->obj->Name) {
> + const GLvoid *map = ctx->Driver.MapBuffer(ctx,
> + GL_ELEMENT_ARRAY_BUFFER_ARB,
> + GL_READ_ONLY,
> + ib->obj);
> + indices = ADD_POINTERS(map, ib->ptr);
> + } else {
> + indices = ib->ptr;
> + }
>
> - switch(type) {
> + switch (ib->type) {
> case GL_UNSIGNED_INT: {
> - const GLuint *ui_indices = (const GLuint *)indices;
> + const GLuint *ui_indices = (const GLuint *)ib->ptr;
Are you sure the above change is correct? Don't you want the indices
ptr? I presume the ushort and ubyte cases still use 'indices'.
> GLuint max_ui = ui_indices[count-1];
> GLuint min_ui = ui_indices[0];
> for (i = 0; i < count; i++) {
> @@ -88,6 +102,12 @@ get_minmax_index(GLuint count, GLuint type, const GLvoid
> *indices,
> assert(0);
> break;
> }
> +
> + if (ib->obj->Name != 0) {
> + ctx->Driver.UnmapBuffer(ctx,
> + GL_ELEMENT_ARRAY_BUFFER_ARB,
> + ib->obj);
> + }
> }
>
>
> @@ -566,53 +586,18 @@ dump_element_buffer(GLcontext *ctx, GLenum type)
> ctx->Array.ElementArrayBufferObj);
> }
>
> -
> -static void GLAPIENTRY
> -vbo_exec_DrawRangeElements(GLenum mode,
> - GLuint start, GLuint end,
> - GLsizei count, GLenum type, const GLvoid *indices)
> +/* Inner support for both _mesa_DrawElements and _mesa_DrawRangeElements */
> +static void
> +vbo_validated_drawrangeelements(GLcontext *ctx, GLenum mode,
> + GLuint start, GLuint end,
> + GLsizei count, GLenum type,
> + const GLvoid *indices)
> {
> - GET_CURRENT_CONTEXT(ctx);
> struct vbo_context *vbo = vbo_context(ctx);
> struct vbo_exec_context *exec = &vbo->exec;
> struct _mesa_index_buffer ib;
> struct _mesa_prim prim[1];
>
> - if (!_mesa_validate_DrawRangeElements( ctx, mode, start, end, count,
> - type, indices ))
> - return;
> -
> - if (end >= ctx->Array.ArrayObj->_MaxElement) {
> - /* the max element is out of bounds of one or more enabled arrays */
> - _mesa_warning(ctx, "glDraw[Range]Elements(start %u, end %u, count %d, "
> - "type 0x%x, indices=%p)\n"
> - "\tindex=%u is out of bounds (max=%u) "
> - "Element Buffer %u (size %d)",
> - start, end, count, type, indices, end,
> - ctx->Array.ArrayObj->_MaxElement - 1,
> - ctx->Array.ElementArrayBufferObj->Name,
> - ctx->Array.ElementArrayBufferObj->Size);
> -
> - if (0)
> - dump_element_buffer(ctx, type);
> -
> - if (0)
> - _mesa_print_arrays(ctx);
> - return;
> - }
> - else if (0) {
> - _mesa_printf("glDraw[Range]Elements"
> - "(start %u, end %u, type 0x%x, count %d) ElemBuf %u\n",
> - start, end, type, count,
> - ctx->Array.ElementArrayBufferObj->Name);
> - }
> -
> -#if 0
> - check_draw_elements_data(ctx, count, type, indices);
> -#else
> - (void) check_draw_elements_data;
> -#endif
> -
> FLUSH_CURRENT( ctx, 0 );
>
> if (ctx->NewState)
> @@ -623,13 +608,13 @@ vbo_exec_DrawRangeElements(GLenum mode,
> return;
> }
>
> - bind_arrays( ctx );
> -
> if (ctx->NewState)
> _mesa_update_state( ctx );
>
> + bind_arrays( ctx );
> +
> ib.count = count;
> - ib.type = type;
> + ib.type = type;
> ib.obj = ctx->Array.ElementArrayBufferObj;
> ib.ptr = indices;
>
> @@ -676,41 +661,63 @@ vbo_exec_DrawRangeElements(GLenum mode,
> vbo->draw_prims( ctx, exec->array.inputs, prim, 1, &ib, start, end );
> }
>
> -
> static void GLAPIENTRY
> -vbo_exec_DrawElements(GLenum mode, GLsizei count, GLenum type,
> - const GLvoid *indices)
> +vbo_exec_DrawRangeElements(GLenum mode,
> + GLuint start, GLuint end,
> + GLsizei count, GLenum type, const GLvoid *indices)
> {
> GET_CURRENT_CONTEXT(ctx);
> - GLuint min_index = 0;
> - GLuint max_index = 0;
>
> - if (!_mesa_validate_DrawElements( ctx, mode, count, type, indices ))
> + if (!_mesa_validate_DrawRangeElements( ctx, mode, start, end, count,
> + type, indices ))
> return;
>
> - if (!vbo_validate_shaders(ctx)) {
> - _mesa_error(ctx, GL_INVALID_OPERATION, "glDrawElements(bad shader)");
> + if (end >= ctx->Array.ArrayObj->_MaxElement) {
> + /* the max element is out of bounds of one or more enabled arrays */
> + _mesa_warning(ctx, "glDraw[Range]Elements(start %u, end %u, count %d, "
> + "type 0x%x, indices=%p)\n"
> + "\tindex=%u is out of bounds (max=%u) "
> + "Element Buffer %u (size %d)",
> + start, end, count, type, indices, end,
> + ctx->Array.ArrayObj->_MaxElement - 1,
> + ctx->Array.ElementArrayBufferObj->Name,
> + ctx->Array.ElementArrayBufferObj->Size);
> +
> + if (0)
> + dump_element_buffer(ctx, type);
> +
> + if (0)
> + _mesa_print_arrays(ctx);
> return;
> }
> + else if (0) {
> + _mesa_printf("glDraw[Range]Elements"
> + "(start %u, end %u, type 0x%x, count %d) ElemBuf %u\n",
> + start, end, type, count,
> + ctx->Array.ElementArrayBufferObj->Name);
> + }
>
> - if (ctx->Array.ElementArrayBufferObj->Name) {
> - const GLvoid *map = ctx->Driver.MapBuffer(ctx,
> - GL_ELEMENT_ARRAY_BUFFER_ARB,
> - GL_READ_ONLY,
> -
> ctx->Array.ElementArrayBufferObj);
> +#if 0
> + check_draw_elements_data(ctx, count, type, indices);
> +#else
> + (void) check_draw_elements_data;
> +#endif
> +
> + vbo_validated_drawrangeelements(ctx, mode, start, end,
> + count, type, indices);
> +}
>
> - get_minmax_index(count, type, ADD_POINTERS(map, indices),
> - &min_index, &max_index);
>
> - ctx->Driver.UnmapBuffer(ctx,
> - GL_ELEMENT_ARRAY_BUFFER_ARB,
> - ctx->Array.ElementArrayBufferObj);
> - }
> - else {
> - get_minmax_index(count, type, indices, &min_index, &max_index);
> - }
> +static void GLAPIENTRY
> +vbo_exec_DrawElements(GLenum mode, GLsizei count, GLenum type,
> + const GLvoid *indices)
> +{
> + GET_CURRENT_CONTEXT(ctx);
> +
> + if (!_mesa_validate_DrawElements( ctx, mode, count, type, indices ))
> + return;
>
> - vbo_exec_DrawRangeElements(mode, min_index, max_index, count, type,
> indices);
> + vbo_validated_drawrangeelements(ctx, mode, ~0, ~0, count, type, indices);
> }
>
>
> --
> 1.6.3.3
>
>
> ------------------------------------------------------------------------------
> Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day
> trial. Simplify your report design, integration and deployment - and focus on
> what you do best, core application coding. Discover what's new with
> Crystal Reports now. http://p.sf.net/sfu/bobj-july
> _______________________________________________
> Mesa3d-dev mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
> .
>
------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day
trial. Simplify your report design, integration and deployment - and focus on
what you do best, core application coding. Discover what's new with
Crystal Reports now. http://p.sf.net/sfu/bobj-july
_______________________________________________
Mesa3d-dev mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
