[Mesa-dev] [PATCH] vbo: count min/max_index before vbo->draw_prims

[Yuanhan Liu]

On Tue, Dec 27, 2011 at 11:15:42AM -0800, Eric Anholt wrote:
> On Sun, 25 Dec 2011 12:26:25 +0800, Liu Aleaxander <aleaxander at gmail.com> wrote:
> > On Sun, Dec 25, 2011 at 8:03 AM, Eric Anholt <eric at anholt.net> wrote:
> > > On Thu, 22 Dec 2011 18:55:50 +0800, Yuanhan Liu <yuanhan.liu at linux.intel.com> wrote:
> > >> For the case that index data is stored in element array buffer object,
> > >> and user called glMultiDrawElements, count the min/max_index before
> > >> calling vbo->draw_prims. vbo_get_minmax_index() isn't friendly to this
> > >> case. So do it while building the prim info.
> > >>
> > >> Signed-off-by: Yuanhan Liu <yuanhan.liu at linux.intel.com>
> > >
> > > I'm really failing to understand what this commit is for.  Does it fix
> > > some testcase?
> > 
> > Sorry, my bad. I should note something more about this issue. And
> > yes, it does fixes something. Say for following case:
> > 
> > GLubyte indices_data[] = {0, 1, 2, 3, 4, 5, 6,    1, 7, 8, 9, 10, 11};
> > GLsizei count[] = {7, 6}
> > GLvoid * indices_off[2] = {0, 7};
> > 
> > /* here copy the indices data into Element Array Buffer Object */
> > .....glBind..glBufferData....
> > 
> > /* Here call glMulitDrawElements */
> > glMultiDrawElements(GL_LINE_STRIP, count,
> >                                 GL_UNSIGNED_BYTE,
> >                                 indices_off, 2);
> > 
> > The current code in brw_draw.c would just count the min_index
> > and max_index of the first prim, thus got a min_index of 0, and
> > max_index of 6. This is not right, thus the second prim(and any
> >  later, if you have more than 2 prim to render) rendered incorrectly.
> > 
> > I found it's a little ugly to add those code similar in this patch at
> > brw_draw_prims(), then I figured out a way to add it at the caller,
> > vbo_validated_multidrawelements(). I guess it's reasonable, since
> > the prim info is built at there.
> 
> What about when the next caller that does multiple primitives comes
> along. 

We don't have the next caller besides glMultiDrawElements* _for_now_ , right?

> Would you add workaround code to it too?  Please fix the actual
> broken code in brw_draw.c instead.

Yes, anyway, you are right. I sent out a patch to fix this issue in
brw_draw_prims(). And I found it isn't that ugly to fix it at there as I
tought. ;)

Thanks,
Yuanhan Liu

> 
> This would be a good time to check for the same breakage in other
> callers of vbo_get_minmax_index(), and see if we want a function that
> does this multi-primitive vbo_get_minmax_index() logic.