GPU buffers refactor

This patch attempts to clean up and document the GPU buffers
code. There are a few bug fixes as well.

Summary:

* Bugfix: make GPU_buffer_copy_normal convert from shorts to floats
  correctly, also fixed the use of cached face normal CustomData.

* Bugfix: changed the `mat_nr' field of GPUBufferMaterial from char to
  short.

* Changed color buffer setup to not alloc a temporary copy of color
  data, just passes the MCol data in directly.

* Changed the GPU buffer pool code to make clearer what operates
  specifically on the global pool.

* Lots of refactoring for GPU_drawobject_new; should operate mostly
  the same (except got rid of one unecessary allocation), just split
  into more functions and without macros now.

* Converted some #defines into enumerations.

* Made some stuff private, pulled out of header file.

* Deleted unused function GPU_buffer_pool_free_unused().

* Removed GPU_interleaved_setup and related #defines. (I think this
  was used for editmode VBOs, but those were disabled.)

* Added lots of comments.

* Added a few comments in the code signed `--nicholas' to note places
  where I am unsure about design or usage, would be good to address
  these better.

* Code formatting changed to be more consistent with the rest of
  Blender.

* Renamed some fields and variables to be more consistent with
  Blender's naming conventions.

* Renamed some fields and variables to use more descriptive names,
  e.g. renamed `redir' to `mat_orig_to_new'.

* Removed print outs with DEBUG_VBO -- don't feel too strongly about
  this one, just not used elsewhere in Blender, could be easily added
  back if others disagree though.

* Moved the PBVH drawing code down to the bottom of the file, before
  was sitting in the middle of the other VBO code

[psy-fi, 2011-06-19 19:00:28]

Better indeed, though in my opinion a bigger refactor should be done on
cdderivedmesh. The code should ideally setup the vbo's once (on editmode exit)
and the draw functions should be as simple as glVertexPointer, gl*Pointer,
glDrawArrays. Not 100% sure if this is possible but it would be ideal if it is.

[psy-fi, 2011-06-19 19:01:00]

http://codereview.appspot.com/4631052/diff/1/source/blender/gpu/intern/gpu_bu...
File source/blender/gpu/intern/gpu_buffers.c (right):

http://codereview.appspot.com/4631052/diff/1/source/blender/gpu/intern/gpu_bu...
source/blender/gpu/intern/gpu_buffers.c:104: useVBOs =
(GL_ARB_vertex_buffer_object ? 1 : 0);
shouldn't GL_ARB_vertex_buffer_object be GLEW_ARB_vertex_buffer_object?

http://codereview.appspot.com/4631052/diff/1/source/blender/gpu/intern/gpu_bu...
source/blender/gpu/intern/gpu_buffers.c:399: int points_per_mat[MAX_MATERIALS];
one of the tragedies in the code is allocating MAX_MATERIALS(which is the
maximum short if memory serves right?) integers on the stack. It does work
apparently, though. I guess some other dynamic structure such as a Linked List
would serve better maybe. Likewise for every other function that uses this
scheme.

[nicholasbishop, 2011-06-19 19:37:04]

Thanks for the review psy-fi.

> Better indeed, though in my opinion a bigger refactor should be done on
> cdderivedmesh. The code should ideally setup the vbo's once (on editmode exit)
> and the draw functions should be as simple as glVertexPointer, gl*Pointer,
> glDrawArrays. Not 100% sure if this is possible but it would be ideal if it
is.

Regarding refactoring in cdderivedmesh -- I agree that this is needed, and in
fact a more cohesive refactoring of drawing across all the DerivedMesh code is
my eventual goal. (For example, CCGSubsurf doesn't even use VBOs, except in
combination with PBVH drawing.)

The problem is, there's a whole lot of drawing code already, and it handles a
lot of settings and combinations successfully. I don't want to break things
while refactoring, so I'm trying to clean up existing code first; then we can
get a better handle on larger redesign.

>
http://codereview.appspot.com/4631052/diff/1/source/blender/gpu/intern/gpu_bu...
> source/blender/gpu/intern/gpu_buffers.c:104: useVBOs =
> (GL_ARB_vertex_buffer_object ? 1 : 0);
> shouldn't GL_ARB_vertex_buffer_object be GLEW_ARB_vertex_buffer_object?

Not sure about this; the original code had GL_ARB_vertex_buffer_object, so I
kept it the same. Don't know enough about GLEW to say if it's wrong though.

> 
>
http://codereview.appspot.com/4631052/diff/1/source/blender/gpu/intern/gpu_bu...
> source/blender/gpu/intern/gpu_buffers.c:399: int
points_per_mat[MAX_MATERIALS];
> one of the tragedies in the code is allocating MAX_MATERIALS(which is the
> maximum short if memory serves right?) integers on the stack. It does work
> apparently, though. I guess some other dynamic structure such as a Linked List
> would serve better maybe. Likewise for every other function that uses this
> scheme.

We could definitely replace these with e.g. a hash table from blenlib. I'm not
sure how much it matters in practice though; each of these arrays is 64kb
(MAX_MATERIALS=16384) and they're really only "allocated" once. So we could
definitely save a bunch of space (at least in the common case where the number
of active materials is way lower than MAX_MATERIALS), but I doubt if there would
be much impact (unless MAX_MATERIALS becomes 2^32 in the future or something.)

Thanks,
-Nicholas

[Jason.A.Wilkins, 2011-06-22 12:46:51]

On 2011/06/19 19:00:28, psy-fi wrote:
> Better indeed, though in my opinion a bigger refactor should be done on
> cdderivedmesh. The code should ideally setup the vbo's once (on editmode exit)
> and the draw functions should be as simple as glVertexPointer, gl*Pointer,
> glDrawArrays. Not 100% sure if this is possible but it would be ideal if it
is.

I would like to add that the more done to derived mesh and gpu buffers the more
likely there are to be merge conflicts with Onion.  Fortunately, for me at
least, I have not done anything in these areas except merge what Nick has
already done in another branch, so if there are conflicts at least Nick is
acquainted with it enough to resolve the problems, I hope.

I'm still not completely familiar enough with these files.  The only thing I've
done in Onion that is different from Nick's work is to write a mask/paint color
blending function that does not let the mask disappear even if a base paint
color component is 0 or 1, i.e., the mask always has a visible effect.

[brechtvl, 2011-06-22 14:13:03]

I agree we should do incremental improvement of drawing code, this kind of
refactoring is a good step in my opinion. As far as I'm concerned this can go in
after the 2.58 release is out and svn is open for changes again.

On 2011/06/19 19:37:04, nicholasbishop wrote:
> > source/blender/gpu/intern/gpu_buffers.c:104: useVBOs =
> > (GL_ARB_vertex_buffer_object ? 1 : 0);
> > shouldn't GL_ARB_vertex_buffer_object be GLEW_ARB_vertex_buffer_object?
> 
> Not sure about this; the original code had GL_ARB_vertex_buffer_object, so I
> kept it the same. Don't know enough about GLEW to say if it's wrong though.

It looks like this indeed a bug in the current code, well spotted :) GL_ is the
compile time check, GLEW_ is runtime to see if the driver supports it.

[nicholasbishop, 2011-06-28 00:50:18]

> I would like to add that the more done to derived mesh and gpu buffers the
more
> likely there are to be merge conflicts with Onion.  Fortunately, for me at
> least, I have not done anything in these areas except merge what Nick has
> already done in another branch, so if there are conflicts at least Nick is
> acquainted with it enough to resolve the problems, I hope.
> 
> I'm still not completely familiar enough with these files.  The only thing
I've
> done in Onion that is different from Nick's work is to write a mask/paint
color
> blending function that does not let the mask disappear even if a base paint
> color component is 0 or 1, i.e., the mask always has a visible effect.

Hi Jason, hopefully there should not be too many conflicts with this; the sculpt
code uses its own set of drawing functions that only slightly overlap with the
regular mesh drawing. (That in itself is something that should probably be fixed
over time.)


On 2011/06/22 14:13:03, brechtvl wrote:
> I agree we should do incremental improvement of drawing code, this kind of
> refactoring is a good step in my opinion. As far as I'm concerned this can go
in
> after the 2.58 release is out and svn is open for changes again.

Thanks for reviewing Brecht, I'll commit soon.

> It looks like this indeed a bug in the current code, well spotted :) GL_ is
the
> compile time check, GLEW_ is runtime to see if the driver supports it.

Committed this correction to trunk, thanks for finding it psy-fi :)

[jason.a.wilkins_gmail.com, 2011-06-28 03:15:20]

On Mon, Jun 27, 2011 at 7:50 PM,  <NicholasBishop@gmail.com> wrote:
> Hi Jason, hopefully there should not be too many conflicts with this;
> the sculpt code uses its own set of drawing functions that only slightly
> overlap with the regular mesh drawing. (That in itself is something that
> should probably be fixed over time.)

I agree.  The changes I've made are minor and it should not be too
difficult to fix it up.

[psy-fi, 2011-06-28 14:39:19]

No prob, thanks for commiting :)