[Mesa-dev] [PATCH] vbo: introduce vbo_get_minmax_indices function
Brian Paul
brianp at vmware.com
Wed Jan 11 17:50:18 PST 2012
On 01/11/2012 01:17 AM, Yuanhan Liu wrote:
> On Tue, Jan 10, 2012 at 08:43:18PM -0700, Brian Paul wrote:
>> On Tue, Jan 3, 2012 at 8:59 PM, Yuanhan Liu<yuanhan.liu at linux.intel.com> wrote:
>>> 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
> [snip]..
>>> +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;
>>
>> I think you could use a pointer for start_prim:
>>
>> const struct _mesa_prim *start_prim;
>>
>> to avoid copying 20 bytes per loop iteration below. The declaration
>> could also be moved inside the loop.
>
> Aha, yes. I should do this. Thanks, here is the updated patch:
>
> From 66f309648a20736c932eb1d393ca7cad6679532a 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
>
> v3: per Brian's suggestion, use a pointer for start_prim to avoid
> structure copy per loop.
>
> Signed-off-by: Yuanhan Liu<yuanhan.liu at linux.intel.com>
> Reviewed-by: Roland Scheidegger<sroland at vmware.com>
> Reviewed-by: Brian Paul<brianp at vmware.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 | 50 +++++++++++++++++++++----
> 8 files changed, 53 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 6d6fc85..c0554cf 100644
> --- a/src/mesa/state_tracker/st_draw.c
> +++ b/src/mesa/state_tracker/st_draw.c
> @@ -990,7 +990,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 fbf0349..a559b73 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..263e429 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,41 @@ 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)
> +{
> + GLuint tmp_min, tmp_max;
> + GLuint i;
> + GLuint count;
> +
> + *min_index = ~0;
> + *max_index = 0;
> +
> + for (i = 0; i< nr_prims; i++) {
> + const struct _mesa_prim *start_prim;
> +
> + 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.
That looks better.
Reviewed-by: Brian Paul <brianp at vmware.com>
More information about the mesa-dev
mailing list