[Mesa-dev] [PATCH] vbo: introduce vbo_get_minmax_indices function

Roland Scheidegger sroland at vmware.com
Wed Jan 4 10:23:24 PST 2012


Am 04.01.2012 04:59, schrieb Yuanhan Liu:
> On Wed, Jan 04, 2012 at 11:20:07AM +0800, Yuanhan Liu wrote:
>> On Tue, Jan 03, 2012 at 08:25:31PM +0100, Roland Scheidegger wrote:
>>> Ah index scanning...
>>> I don't like that this will map/unmap the ib once for each prim,
>> Me either :)
>>
>>> though
>>> I don't really see a nice way to avoid that (I think if you have to
>>
>> Well, I thought a while, we may do some combine to reduce some
>> map/unmap.
> 
> Ok, here is the new patch, please help to review it.
> 
> And Brian, since it touches the mesa core, it would be nice if you'd
> review it.
Looks good to me.
Reviewed-by: Roland Scheidegger <sroland at vmware.com>

> --
> 
> From 7956b5c93bdfd0e94b6d3e25336c99cd7457f550 Mon Sep 17 00:00:00 2001
> From: Yuanhan Liu <yuanhan.liu at linux.intel.com>
> Date: Sat, 31 Dec 2011 14:22:46 +0800
> Subject: [PATCH] vbo: introduce vbo_get_minmax_indices function
> 
> Introduce vbo_get_minmax_indices() function to handle the min/max index
> computation for nr_prims(>= 1). The old code just compute the first
> prim's min/max index; this would results an error rendering if user
> called functions like glMultiDrawElements(). This patch servers as
> fixing this issue.
> 
> As when nr_prims = 1, we can pass 1 to paramter nr_prims, thus I made
> vbo_get_minmax_index() static.
> 
> v2: per Roland's suggestion, put the indices address compuation into
>     vbo_get_minmax_index() instead.
> 
>     Also do comination if possible to reduce map/unmap count
> 
> Signed-off-by: Yuanhan Liu <yuanhan.liu at linux.intel.com>
> ---
>  src/mesa/drivers/dri/i965/brw_draw.c         |    2 +-
>  src/mesa/drivers/dri/nouveau/nouveau_vbo_t.c |    3 +-
>  src/mesa/main/api_validate.c                 |    2 +-
>  src/mesa/state_tracker/st_draw.c             |    3 +-
>  src/mesa/state_tracker/st_draw_feedback.c    |    2 +-
>  src/mesa/tnl/t_draw.c                        |    2 +-
>  src/mesa/vbo/vbo.h                           |    6 ++--
>  src/mesa/vbo/vbo_exec_array.c                |   49 +++++++++++++++++++++----
>  8 files changed, 52 insertions(+), 17 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_draw.c b/src/mesa/drivers/dri/i965/brw_draw.c
> index 621195d..f50fffd 100644
> --- a/src/mesa/drivers/dri/i965/brw_draw.c
> +++ b/src/mesa/drivers/dri/i965/brw_draw.c
> @@ -586,7 +586,7 @@ void brw_draw_prims( struct gl_context *ctx,
>  
>     if (!vbo_all_varyings_in_vbos(arrays)) {
>        if (!index_bounds_valid)
> -	 vbo_get_minmax_index(ctx, prim, ib, &min_index, &max_index);
> +	 vbo_get_minmax_indices(ctx, prim, ib, &min_index, &max_index, nr_prims);
>  
>        /* Decide if we want to rebase.  If so we end up recursing once
>         * only into this function.
> diff --git a/src/mesa/drivers/dri/nouveau/nouveau_vbo_t.c b/src/mesa/drivers/dri/nouveau/nouveau_vbo_t.c
> index de04d18..59f1542 100644
> --- a/src/mesa/drivers/dri/nouveau/nouveau_vbo_t.c
> +++ b/src/mesa/drivers/dri/nouveau/nouveau_vbo_t.c
> @@ -437,7 +437,8 @@ TAG(vbo_render_prims)(struct gl_context *ctx,
>  	struct nouveau_render_state *render = to_render_state(ctx);
>  
>  	if (!index_bounds_valid)
> -		vbo_get_minmax_index(ctx, prims, ib, &min_index, &max_index);
> +		vbo_get_minmax_indices(ctx, prims, ib, &min_index, &max_index,
> +				       nr_prims);
>  
>  	vbo_choose_render_mode(ctx, arrays);
>  	vbo_choose_attrs(ctx, arrays);
> diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c
> index 945f127..b6871d0 100644
> --- a/src/mesa/main/api_validate.c
> +++ b/src/mesa/main/api_validate.c
> @@ -184,7 +184,7 @@ check_index_bounds(struct gl_context *ctx, GLsizei count, GLenum type,
>     ib.ptr = indices;
>     ib.obj = ctx->Array.ArrayObj->ElementArrayBufferObj;
>  
> -   vbo_get_minmax_index(ctx, &prim, &ib, &min, &max);
> +   vbo_get_minmax_indices(ctx, &prim, &ib, &min, &max, 1);
>  
>     if ((int)(min + basevertex) < 0 ||
>         max + basevertex > ctx->Array.ArrayObj->_MaxElement) {
> diff --git a/src/mesa/state_tracker/st_draw.c b/src/mesa/state_tracker/st_draw.c
> index 954f15a..6327a4c 100644
> --- a/src/mesa/state_tracker/st_draw.c
> +++ b/src/mesa/state_tracker/st_draw.c
> @@ -933,7 +933,8 @@ st_draw_vbo(struct gl_context *ctx,
>        /* Gallium probably doesn't want this in some cases. */
>        if (!index_bounds_valid)
>           if (!all_varyings_in_vbos(arrays))
> -            vbo_get_minmax_index(ctx, prims, ib, &min_index, &max_index);
> +            vbo_get_minmax_indices(ctx, prims, ib, &min_index, &max_index,
> +                                   nr_prims);
>  
>        for (i = 0; i < nr_prims; i++) {
>           num_instances = MAX2(num_instances, prims[i].num_instances);
> diff --git a/src/mesa/state_tracker/st_draw_feedback.c b/src/mesa/state_tracker/st_draw_feedback.c
> index a99eb2b..f38f44c 100644
> --- a/src/mesa/state_tracker/st_draw_feedback.c
> +++ b/src/mesa/state_tracker/st_draw_feedback.c
> @@ -119,7 +119,7 @@ st_feedback_draw_vbo(struct gl_context *ctx,
>     st_validate_state(st);
>  
>     if (!index_bounds_valid)
> -      vbo_get_minmax_index(ctx, prims, ib, &min_index, &max_index);
> +      vbo_get_minmax_indices(ctx, prims, ib, &min_index, &max_index, nr_prims);
>  
>     /* must get these after state validation! */
>     vp = st->vp;
> diff --git a/src/mesa/tnl/t_draw.c b/src/mesa/tnl/t_draw.c
> index f949c34..17042cf 100644
> --- a/src/mesa/tnl/t_draw.c
> +++ b/src/mesa/tnl/t_draw.c
> @@ -418,7 +418,7 @@ void _tnl_vbo_draw_prims(struct gl_context *ctx,
>  			 struct gl_transform_feedback_object *tfb_vertcount)
>  {
>     if (!index_bounds_valid)
> -      vbo_get_minmax_index(ctx, prim, ib, &min_index, &max_index);
> +      vbo_get_minmax_indices(ctx, prim, ib, &min_index, &max_index, nr_prims);
>  
>     _tnl_draw_prims(ctx, arrays, prim, nr_prims, ib, min_index, max_index);
>  }
> diff --git a/src/mesa/vbo/vbo.h b/src/mesa/vbo/vbo.h
> index ed8fc17..bf925ab 100644
> --- a/src/mesa/vbo/vbo.h
> +++ b/src/mesa/vbo/vbo.h
> @@ -127,9 +127,9 @@ int
>  vbo_sizeof_ib_type(GLenum type);
>  
>  void
> -vbo_get_minmax_index(struct gl_context *ctx, const struct _mesa_prim *prim,
> -		     const struct _mesa_index_buffer *ib,
> -		     GLuint *min_index, GLuint *max_index);
> +vbo_get_minmax_indices(struct gl_context *ctx, const struct _mesa_prim *prim,
> +                       const struct _mesa_index_buffer *ib,
> +                       GLuint *min_index, GLuint *max_index, GLuint nr_prims);
>  
>  void vbo_use_buffer_objects(struct gl_context *ctx);
>  
> diff --git a/src/mesa/vbo/vbo_exec_array.c b/src/mesa/vbo/vbo_exec_array.c
> index fec49d3..9c7e202 100644
> --- a/src/mesa/vbo/vbo_exec_array.c
> +++ b/src/mesa/vbo/vbo_exec_array.c
> @@ -99,24 +99,23 @@ vbo_sizeof_ib_type(GLenum type)
>   * If primitive restart is enabled, we need to ignore restart
>   * indexes when computing min/max.
>   */
> -void
> +static void
>  vbo_get_minmax_index(struct gl_context *ctx,
>  		     const struct _mesa_prim *prim,
>  		     const struct _mesa_index_buffer *ib,
> -		     GLuint *min_index, GLuint *max_index)
> +		     GLuint *min_index, GLuint *max_index,
> +		     const GLuint count)
>  {
>     const GLboolean restart = ctx->Array.PrimitiveRestart;
>     const GLuint restartIndex = ctx->Array.RestartIndex;
> -   const GLuint count = prim->count;
>     const void *indices;
>     GLuint i;
>  
> +   indices = (void *)ib->ptr + prim->start * vbo_sizeof_ib_type(ib->type);
>     if (_mesa_is_bufferobj(ib->obj)) {
> -      indices = ctx->Driver.MapBufferRange(ctx, (GLsizeiptr) ib->ptr,
> -                                           count * vbo_sizeof_ib_type(ib->type),
> -					   GL_MAP_READ_BIT, ib->obj);
> -   } else {
> -      indices = ib->ptr;
> +      GLsizeiptr size = MIN2(count * vbo_sizeof_ib_type(ib->type), ib->obj->Size);
> +      indices = ctx->Driver.MapBufferRange(ctx, (GLsizeiptr) indices, size,
> +                                           GL_MAP_READ_BIT, ib->obj);
>     }
>  
>     switch (ib->type) {
> @@ -196,6 +195,40 @@ vbo_get_minmax_index(struct gl_context *ctx,
>     }
>  }
>  
> +/**
> + * Compute min and max elements for nr_prims
> + */
> +void
> +vbo_get_minmax_indices(struct gl_context *ctx,
> +                       const struct _mesa_prim *prims,
> +                       const struct _mesa_index_buffer *ib,
> +                       GLuint *min_index,
> +                       GLuint *max_index,
> +                       GLuint nr_prims)
> +{
> +   struct _mesa_prim start_prim;
> +   GLuint tmp_min, tmp_max;
> +   GLuint i;
> +   GLuint count;
> +
> +   *min_index = ~0;
> +   *max_index = 0;
> +
> +   for (i = 0; i < nr_prims; i++) {
> +      start_prim = prims[i];
> +      count = start_prim.count;
> +      /* Do combination if possible to reduce map/unmap count */
> +      while ((i + 1 < nr_prims) &&
> +             (prims[i].start + prims[i].count == prims[i+1].start)) {
> +         count += prims[i+1].count;
> +         i++;
> +      }
> +      vbo_get_minmax_index(ctx, &start_prim, ib, &tmp_min, &tmp_max, count);
> +      *min_index = MIN2(*min_index, tmp_min);
> +      *max_index = MAX2(*max_index, tmp_max);
> +   }
> +}
> +
>  
>  /**
>   * Check that element 'j' of the array has reasonable data.



More information about the mesa-dev mailing list