Make GrDrawState object used by GrDrawTarget be settable; set in GrInOrderDrawBuffer playback

[bsalomon, 2012-03-26 20:59:50]

This avoids copies of GrDrawState during the draw buffer playback. Instead we
just point the target at states that we saved during the record.

http://codereview.appspot.com/5908062/diff/1/src/gpu/GrInOrderDrawBuffer.cpp
File src/gpu/GrInOrderDrawBuffer.cpp (right):

http://codereview.appspot.com/5908062/diff/1/src/gpu/GrInOrderDrawBuffer.cpp#...
src/gpu/GrInOrderDrawBuffer.cpp:531: GrDrawState* ds =
&GrDrawTarget::accessSavedDrawState(fStates[currState]);
This will be a little cleaner in a subsequent change that removes the
SavedDrawState class entirely.

[TomH, 2012-03-26 21:11:01]

LGTM

http://codereview.appspot.com/5908062/diff/1/src/gpu/GrDrawTarget.cpp
File src/gpu/GrDrawTarget.cpp (right):

http://codereview.appspot.com/5908062/diff/1/src/gpu/GrDrawTarget.cpp#newcode477
src/gpu/GrDrawTarget.cpp:477: fDrawState = &fDefaultDrawState;
It's dangerous to have a reference-counted object that isn't independently
heap-allocated, isn't it? Yeah, I know we're being careful and it's not going to
get accidentally unrefed, I just want to be explicit here...

http://codereview.appspot.com/5908062/diff/1/src/gpu/GrDrawTarget.cpp#newcode519
src/gpu/GrDrawTarget.cpp:519: fDrawState->unref();
Need to check for idempotency before we unref.

http://codereview.appspot.com/5908062/diff/1/src/gpu/GrDrawTarget.cpp#newcode529
src/gpu/GrDrawTarget.cpp:529: state->fState.set(this->getDrawState());
I know we want our clients to use the API, but is there a particular reason
you're having us use the API instead of referring to the member?

http://codereview.appspot.com/5908062/diff/1/src/gpu/GrInOrderDrawBuffer.cpp
File src/gpu/GrInOrderDrawBuffer.cpp (right):

http://codereview.appspot.com/5908062/diff/1/src/gpu/GrInOrderDrawBuffer.cpp#...
src/gpu/GrInOrderDrawBuffer.cpp:531: GrDrawState* ds =
&GrDrawTarget::accessSavedDrawState(fStates[currState]);
Good!

[bsalomon, 2012-03-27 17:23:07]

Closed with r3506.

http://codereview.appspot.com/5908062/diff/1/src/gpu/GrDrawTarget.cpp
File src/gpu/GrDrawTarget.cpp (right):

http://codereview.appspot.com/5908062/diff/1/src/gpu/GrDrawTarget.cpp#newcode477
src/gpu/GrDrawTarget.cpp:477: fDrawState = &fDefaultDrawState;
On 2012/03/26 21:11:01, TomH wrote:
> It's dangerous to have a reference-counted object that isn't independently
> heap-allocated, isn't it? Yeah, I know we're being careful and it's not going
to
> get accidentally unrefed, I just want to be explicit here...

I thought we had stack allocated ref count objects frequently in Skia (though I
could be wrong about that). I'll add a comment.

http://codereview.appspot.com/5908062/diff/1/src/gpu/GrDrawTarget.cpp#newcode519
src/gpu/GrDrawTarget.cpp:519: fDrawState->unref();
On 2012/03/26 21:11:01, TomH wrote:
> Need to check for idempotency before we unref.

nice catch

http://codereview.appspot.com/5908062/diff/1/src/gpu/GrDrawTarget.cpp#newcode529
src/gpu/GrDrawTarget.cpp:529: state->fState.set(this->getDrawState());
On 2012/03/26 21:11:01, TomH wrote:
> I know we want our clients to use the API, but is there a particular reason
> you're having us use the API instead of referring to the member?

I was thinking that in the future we might change to having a stack of
prealloc'ed GrDrawState objects or something along those lines. If everybody
calls the (currently inlined) function then it would easier to experiment /
change.