Re: VBO patch

man, 23.08.2004 kl. 11.13 skrev Antoine Chavasse:
> I think it's because nvidia uses VAR, while ATI uses plain vertex 
> arrays, as VBO support was removed from CVS.

Internally, VBO support was removed from CVS only temporarily; it was
re-added again after a few bugfixes. But it still has performance
problems.

WarCraft III in Direct3D mode, and many other games, is slower with VBO
than plain vertex arrays. Direct3D has a NOOVERWRITE flag, which means
still-unused buffer space can be filled "on the fly" between render
calls. There is no equivalent flag in VBO's mmap-ed access, which means
there's a huge performance hit when a game use this kind of access
pattern with the current VBO code. Therefore, until we have a resolution
to this problem, VBO remains disabled for now.

But of course, if AO doesn't use NOOVERWRITE, then it stands to reason
that VBO would be faster for that game.

> I tried to re-enable it (by getting the previous versions of 
> x11drv/makefile.in and executebuffer.c from cvs, where th vbo support 
> was enabled), and to fix the massive rendering problems (it was 
> rendering almost only random polygons) it was causing. It turned out 
> that in d3dgl/fixedfunc.c, in GL_load_arrays(), the glXXXPointer 
> functions were only called if the adress/stride to set changed since the 
> last call.
> 
> It's bad with VBO. I think the driver internally computes the actual 
> start adress of the buffer at this time, using the current bound vertex 
> buffer and the offset given to the function, so not caling this function 
> after binding a new buffer leave the internal buffer address actually 
> used for rendering unchanged.

This is fixed internally. The array caching now checks for
vbuffer/address/stride, not just address/stride. Don't listen to Gav,
this *is* fixed for all pointers. However, there's still an issue with
multistream rendering through vertex declarations to be fixed, since
load_arrays can't yet load some arrays, switch to another stream source,
and then load others, which is also a problem because of this same VBO
issue, and which can't be fixed just by disabling the caching. I suppose
AO doesn't need this, though.

> And it also seems that VBO is compiled in the official binaries (as grep 
> finds stuff like the "ARB_VBO" string in the binaries, that are nowhere 
> to be found in the cvs source), so I don't know how outdated the VBO 
> code in CVS is (if it is), and whether this fix could be applicable to 
> the official version.

There are many changes in d3dgl. I don't think this patch applies, or is
even necessary there. Perhaps it could be applied to external CVS,
though, if the internal VBO fixes won't be merged out (something which
is up to the policy makers here).

> I also did another optimisation in x11drv/executebuffer.c: I added a 
> counter to know how many vertices remain to be converted, so the entire 
> buffer convertion function can be omitted if all the vertices of the 
> buffer are known to have been converted, instead of checking the 
> uptodate flags for all of them. oprofile showed that a lot of time was 
> wasted there.

That sounds like something that could be applied internally. At least if
you can base your patch on executebuffer.c revision 1.61, not 1.62,
since the reversion in 1.62 has been reversed internally.

> So, what the patch does is revert x11drv/makefile.in and 
> x11drv/executebuffer.c to the about 3 month ago versions where vbo 
> support was enabled (hence the size), changes d3dgl/fixedfunc.c to make 
> it so the glXXXpointer functions are called every time GL_load_arrays is 
> called, and adds the count optimisation thingy in x11drv/executebuffer.c.
> On nvidia cards, though, VAR is still used over VBO if available.

Good job, in any case.