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

2012-01-11 Thread Yuanhan Liu
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@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@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@linux.intel.com
Reviewed-by: Roland Scheidegger srol...@vmware.com
Reviewed-by: Brian Paul bri...@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,
+   

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

2012-01-11 Thread Brian Paul

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 Liuyuanhan@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 Liuyuanhan@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 Liuyuanhan@linux.intel.com
Reviewed-by: Roland Scheideggersrol...@vmware.com
Reviewed-by: Brian Paulbri...@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,
+  

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

2012-01-10 Thread Yuanhan Liu
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 srol...@vmware.com
 

___
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

2012-01-10 Thread Brian Paul
On Tue, Jan 3, 2012 at 8:59 PM, Yuanhan Liu yuanhan@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
  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 yuanhan@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@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, 

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

2012-01-04 Thread Roland Scheidegger
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

2012-01-04 Thread Roland Scheidegger
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 srol...@vmware.com

 --
 
 From 7956b5c93bdfd0e94b6d3e25336c99cd7457f550 Mon Sep 17 00:00:00 2001
 From: Yuanhan Liu yuanhan@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@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)
 -  

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

2012-01-03 Thread Roland Scheidegger
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 yuanhan@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|   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, 

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

2012-01-03 Thread 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.


 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

2012-01-03 Thread 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.

Thanks,
Yuanhan Liu


--

From 7956b5c93bdfd0e94b6d3e25336c99cd7457f550 Mon Sep 17 00:00:00 2001
From: Yuanhan Liu yuanhan@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@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 

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

2011-12-30 Thread 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 yuanhan@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|   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,
-GLuint