[Mesa-dev] [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.
Signed-off-by: Yuanhan Liu
---
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| 29 +-
8 files changed, 39 insertions(+), 10 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,
-
Re: [Mesa-dev] [PATCH] vbo: introduce vbo_get_minmax_indices function
Ah index scanning...
I don't like that this will map/unmap the ib once for each prim, though
I don't really see a nice way to avoid that (I think if you have to
actually map the ib, you lose anyway). Hopefully won't hit that
performance hog often...
A comment inline.
Am 31.12.2011 07:32, schrieb Yuanhan Liu:
> 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.
>
> Signed-off-by: Yuanhan Liu
> ---
> 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| 29
> +-
> 8 files changed, 39 insertions(+), 10 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);
> +
Re: [Mesa-dev] [PATCH] vbo: introduce vbo_get_minmax_indices function
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.
> actually map the ib, you lose anyway). Hopefully won't hit that
> performance hog often...
> A comment inline.
>
>
> Am 31.12.2011 07:32, schrieb Yuanhan Liu:
[snip]...
> > + for (i = 0; i < nr_prims; i++) {
> > + tmp_ib.ptr = ib->ptr + prims[i].start * vbo_sizeof_ib_type(ib->type);
> I think you should not use a temporary ib. Figuring out the correct
> start offset clearly looks like it should be handled by
> vbo_get_minmax_index() itself (it should have done this previously
> probably, as there might never have been a guarantee that it is always 0
> even if there's only a single primitive).
Nice suggestion, thanks! Will fix it in the next patch.
--
Yuanhan Liu
>
> > + vbo_get_minmax_index(ctx, &prims[i], &tmp_ib, &tmp_min, &tmp_max);
> > + *min_index = MIN2(*min_index, tmp_min);
> > + *max_index = MAX2(*max_index, tmp_max);
> > + }
> > +}
> > +
> >
> > /**
> > * Check that element 'j' of the array has reasonable data.
>
> Otherwise looks ok to me.
>
> Roland
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] vbo: introduce vbo_get_minmax_indices function
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.
Thanks,
Yuanhan Liu
--
>From 7956b5c93bdfd0e94b6d3e25336c99cd7457f550 Mon Sep 17 00:00:00 2001
From: Yuanhan Liu
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
---
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 valida
Re: [Mesa-dev] [PATCH] vbo: introduce vbo_get_minmax_indices function
Am 04.01.2012 04:20, schrieb Yuanhan Liu:
> 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.
Yes, as long as the ranges are sequential it's not that bad.
You could get really fancy and merge all ranges (which would also avoid
having to scan some indices multiple times when the ranges overlap) but
that's not what I would qualify as "nice".
So I'm just not sure it's worth bothering for this (hopefully) corner case.
Roland
>
>
>> actually map the ib, you lose anyway). Hopefully won't hit that
>> performance hog often...
>> A comment inline.
>>
>>
>> Am 31.12.2011 07:32, schrieb Yuanhan Liu:
> [snip]...
>
>>> + for (i = 0; i < nr_prims; i++) {
>>> + tmp_ib.ptr = ib->ptr + prims[i].start * vbo_sizeof_ib_type(ib->type);
>> I think you should not use a temporary ib. Figuring out the correct
>> start offset clearly looks like it should be handled by
>> vbo_get_minmax_index() itself (it should have done this previously
>> probably, as there might never have been a guarantee that it is always 0
>> even if there's only a single primitive).
>
> Nice suggestion, thanks! Will fix it in the next patch.
>
> --
> Yuanhan Liu
>>
>>> + vbo_get_minmax_index(ctx, &prims[i], &tmp_ib, &tmp_min, &tmp_max);
>>> + *min_index = MIN2(*min_index, tmp_min);
>>> + *max_index = MAX2(*max_index, tmp_max);
>>> + }
>>> +}
>>> +
>>>
>>> /**
>>> * Check that element 'j' of the array has reasonable data.
>>
>> Otherwise looks ok to me.
>>
>> Roland
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] vbo: introduce vbo_get_minmax_indices function
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
> --
>
> From 7956b5c93bdfd0e94b6d3e25336c99cd7457f550 Mon Sep 17 00:00:00 2001
> From: Yuanhan Liu
> 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
> ---
> 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
Re: [Mesa-dev] [PATCH] vbo: introduce vbo_get_minmax_indices function
On Wed, Jan 04, 2012 at 07:23:24PM +0100, Roland Scheidegger wrote: > 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. Hi Brian, any comments? Thanks, Yuanhan Liu > Looks good to me. > Reviewed-by: Roland Scheidegger > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] vbo: introduce vbo_get_minmax_indices function
On Tue, Jan 3, 2012 at 8:59 PM, Yuanhan Liu 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
>> > 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.
>
> Thanks,
> Yuanhan Liu
>
>
> --
>
> From 7956b5c93bdfd0e94b6d3e25336c99cd7457f550 Mon Sep 17 00:00:00 2001
> From: Yuanhan Liu
> 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
> ---
> 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_
Re: [Mesa-dev] [PATCH] vbo: introduce vbo_get_minmax_indices function
On Tue, Jan 10, 2012 at 08:43:18PM -0700, Brian Paul wrote:
> On Tue, Jan 3, 2012 at 8:59 PM, Yuanhan Liu
> 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
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
Reviewed-by: Roland Scheidegger
Reviewed-by: Brian Paul
---
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;
Re: [Mesa-dev] [PATCH] vbo: introduce vbo_get_minmax_indices function
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 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
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
Reviewed-by: Roland Scheidegger
Reviewed-by: Brian Paul
---
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
