Change command buffer client code to use structures....

Change command buffer client code to use structures.

I didn't update the big_test although I'm happy to
do that in another CL. 

I changed the CB renderer code to use this. The only
place I didn't is the state handling code. I'll
consider changing that in another CL.

I changed DoCommand so it gets passed the entire
command data including the command itself where
as it used to get passed the command buffer entry
after the command. I wanted to put the commands
into the structures as I think it makes them easier
to use since they can then correctly set their header.
I could have left DoCommand as is but there would
have been a lot of funky pointer math and I thought
this change made it cleaner.

Some questions I had while doing this.

There are a few places in the code that use 
unsigned int instead of uint32. It seems
we should use uint32 because unsigned int
is not guarnteed to be 32bits. So for example
ResourceID is unsigned int right now.

The CMD::Set/CMD::Init/CommandBufferHelper commands
are currently fairly untyped. For example 
DESTROY_TEXTURE takes a uint32 id. Should it take
a ResourceID instead? DRAW should maybe take an
enum for primitive_type? If we decide to do that
I'd like to do it in another CL.

There's no checking for overflow. We could add
a bunch of DCHECK like DCHECK_LE(width, 65536) 
or DCHECK_LE(semantic_index, 16). I'd like to
do those in another CL as well as I think we
need to discuss what commands we want to change.

All the code is in .h files because we want all
of this code to inline. Theoretically this should
be just as efficient as poking values into arrays
in the opt builds.

[petersont, 2009-09-18 22:02:34]

I think this is a great improvement, but I confess I haven't read every single
line yet.  For now, a handful of minor nits.

http://codereview.chromium.org/212018/diff/1021/1036
File command_buffer/common/cross/cmd_buffer_format.h (right):

http://codereview.chromium.org/212018/diff/1021/1036#newcode42
Line 42: // CMD::SET_VIEWPORT::Set(ptrToSomeSharedMemory,
CMD -> cmd ?

http://codereview.chromium.org/212018/diff/1021/1036#newcode47
Line 47: // void* dest = ptrToSomeSharedMemory;
Maybe it's me, but it took me a while to figure out that the way this works is
that it returns the pointer to the place the next instruction should go.  Could
you add a comment explaining that?

http://codereview.chromium.org/212018/diff/1021/1036#newcode413
Line 413: // Gets the address of memory just after a structure.
Could you add a comment explaining the difference between ImmediateDataAddress,
NextCmdAddress and NextImmediateCmdAddress?

http://codereview.chromium.org/212018/diff/1021/1031
File command_buffer/service/cross/gapi_decoder.cc (right):

http://codereview.chromium.org/212018/diff/1021/1031#newcode65
Line 65: sizeof(cmd::name) / sizeof(uint32) - 1, },   \
Perhaps add // NOLINT just to clear up the one error.

http://codereview.chromium.org/212018/diff/1021/1024
File core/cross/command_buffer/texture_cb.cc (right):

http://codereview.chromium.org/212018/diff/1021/1024#newcode139
Line 139: helper->SET_TEXTURE_DATA(
That's a lot nicer. :)

[gman, 2009-09-19 02:52:55]

I fixed those.  Once we decide what to do about the bitfields (looks like we'll
leave them) and what to do about the types of params (uint32 or ResourceID,
uint32 or TextureCUBE::CubeFace) etc. then we can document all the structs.

Vangelis requested I change the commands from FOO_BAR to FooBar and in keeping
with the style guide I changed the enums from FOO_BAR to kFooBar.  That ended up
changing quite a few files. 

I also added in the state stuff and big_test converted to command buffers.  I'll
do the bubble test on monday in another CL if I can.

http://codereview.chromium.org/212018/diff/1021/1036
File command_buffer/common/cross/cmd_buffer_format.h (right):

http://codereview.chromium.org/212018/diff/1021/1036#newcode42
Line 42: // CMD::SET_VIEWPORT::Set(ptrToSomeSharedMemory,
On 2009/09/18 22:02:34, petersont wrote:
> CMD -> cmd ?

Done.

http://codereview.chromium.org/212018/diff/1021/1036#newcode47
Line 47: // void* dest = ptrToSomeSharedMemory;
On 2009/09/18 22:02:34, petersont wrote:
> Maybe it's me, but it took me a while to figure out that the way this works is
> that it returns the pointer to the place the next instruction should go. 
Could
> you add a comment explaining that?

Done.

http://codereview.chromium.org/212018/diff/1021/1036#newcode413
Line 413: // Gets the address of memory just after a structure.
On 2009/09/18 22:02:34, petersont wrote:
> Could you add a comment explaining the difference between
ImmediateDataAddress,
> NextCmdAddress and NextImmediateCmdAddress?

Done.

http://codereview.chromium.org/212018/diff/1021/1024
File core/cross/command_buffer/texture_cb.cc (right):

http://codereview.chromium.org/212018/diff/1021/1024#newcode139
Line 139: helper->SET_TEXTURE_DATA(
On 2009/09/18 22:02:34, petersont wrote:
> That's a lot nicer. :)

Thanks. I think once we make a few samples using this stuff (your big_test and
the bubble sample for starters), command buffers won't look so foreign.

[petersont, 2009-09-21 23:38:32]

There are still some lint errors, otherwise, LGTM.

The changes you suggested for future cls all sound righteous to me as well. 
Changing uint32 to ResourceID where appropriate seems like a good idea, as does
expanding bitfields that we think will need to be bigger one day when hardware
improves etc.