Move vertex layout from GeometrySrcState to GrDrawState

Move vertex layout from GeometrySrcState to GrDrawState.

The main chunk is pulling the vertex layout from GrDrawTarget::GeometrySrcState to GrDrawState. GeometrySrcState still stores the vertex size because it's needed for allocations. 

Because GeometrySrcState can be pushed and popped and we may need this behavior for the vertex layout, AutoGeometryPush now includes an AutoStateRestore and has been renamed to AutoGeometryAndStatePush.

Finally vertexLayout has been removed as an argument to a number of routines. Before allocating geometry using AutoReleaseGeometry and reserveVertexAndIndexSpace(), it is now necessary to explicitly set any new vertexLayout in the current GrDrawState.

[JimVV, 2013-02-04 20:59:36]

A couple of oddities: I'm not sure if line 206 (or so) in GrTextContext.cpp is
the correct solution -- it avoids a NULL pointer crash and setting the vertex
layout twice. I'm also not sure if there's a better way to handle the
playbackState in GrInOrderDrawBuffer::flushTo(), since we're already storing the
previous state in the AutoGeometryAndStatePush.

[bsalomon, 2013-02-04 22:37:12]

https://codereview.appspot.com/7286047/diff/1/src/gpu/GrClipMaskManager.cpp
File src/gpu/GrClipMaskManager.cpp (right):

https://codereview.appspot.com/7286047/diff/1/src/gpu/GrClipMaskManager.cpp#n...
src/gpu/GrClipMaskManager.cpp:580: GrDrawTarget::AutoGeometryAndStatePush
agasp(fGpu, GrDrawTarget::kReset_ASRInit);
agasp - love it. Do we need reset vs preserve or can we just always reset?

https://codereview.appspot.com/7286047/diff/1/src/gpu/GrDefaultPathRenderer.cpp
File src/gpu/GrDefaultPathRenderer.cpp (right):

https://codereview.appspot.com/7286047/diff/1/src/gpu/GrDefaultPathRenderer.c...
src/gpu/GrDefaultPathRenderer.cpp:479: GrDrawTarget::AutoGeometryAndStatePush
agasp(target, GrDrawTarget::kPreserve_ASRInit);
I see... this answers my earlier question.

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

https://codereview.appspot.com/7286047/diff/1/src/gpu/GrInOrderDrawBuffer.cpp...
src/gpu/GrInOrderDrawBuffer.cpp:243: draw->fVertexLayout !=
drawState.getVertexLayout()) {
I think we don't this as a member of DrawRecord anymore. We should only get here
if the GrDrawState of the current draw matches that of the previous draw. Now
that you've made VL a member of GrDS, we've already checked that the VLs match
by checking the GrDSs match.

https://codereview.appspot.com/7286047/diff/1/src/gpu/GrTextContext.cpp
File src/gpu/GrTextContext.cpp (right):

https://codereview.appspot.com/7286047/diff/1/src/gpu/GrTextContext.cpp#newco...
src/gpu/GrTextContext.cpp:210: flush = flush &&
fDrawTarget->geometryHints(&fMaxVertices,
Can we simplify the logic for setting var flush?

bool flush = false;
if (NULL != fDT) {
 ...
 flush = fDT->gGH();
}

[robertphillips, 2013-02-04 23:08:12]

https://codereview.appspot.com/7286047/diff/1/src/gpu/GrAAHairLinePathRendere...
File src/gpu/GrAAHairLinePathRenderer.cpp (right):

https://codereview.appspot.com/7286047/diff/1/src/gpu/GrAAHairLinePathRendere...
src/gpu/GrAAHairLinePathRenderer.cpp:517:
target->drawState()->setVertexLayout(layout);
fold the getVertexSize & assert lines together or use debug code macro on
getVertexSize line?

https://codereview.appspot.com/7286047/diff/1/src/gpu/GrContext.cpp
File src/gpu/GrContext.cpp (right):

https://codereview.appspot.com/7286047/diff/1/src/gpu/GrContext.cpp#newcode741
src/gpu/GrContext.cpp:741: static const int worstCaseVertCount = 10;
Add some sort of #define for 0 in this case?

https://codereview.appspot.com/7286047/diff/1/src/gpu/GrContext.cpp#newcode1012
src/gpu/GrContext.cpp:1012: drawState->setVertexLayout(layout);
fold getVertexSize & assert lines together?

https://codereview.appspot.com/7286047/diff/1/src/gpu/GrDrawState.h
File src/gpu/GrDrawState.h (right):

https://codereview.appspot.com/7286047/diff/1/src/gpu/GrDrawState.h#newcode86
src/gpu/GrDrawState.h:86: fCommon.fColor = 0xffffffff;
#define here too - empty? invalid?

https://codereview.appspot.com/7286047/diff/1/src/gpu/GrDrawTarget.h
File src/gpu/GrDrawTarget.h (right):

https://codereview.appspot.com/7286047/diff/1/src/gpu/GrDrawTarget.h#newcode279
src/gpu/GrDrawTarget.h:279: * @param vertexArray   cpu array containing vertex
data.
delete vertexSize?

https://codereview.appspot.com/7286047/diff/1/src/gpu/GrTextContext.cpp
File src/gpu/GrTextContext.cpp (right):

https://codereview.appspot.com/7286047/diff/1/src/gpu/GrTextContext.cpp#newco...
src/gpu/GrTextContext.cpp:205: fMaxVertices = kMinRequestedVerts;
It makes me nervous that we use fDrawTarget to set part of flush and the assign
to it and use the new value to compute the rest of the flush flag.

[JimVV, 2013-02-05 14:47:59]

https://codereview.appspot.com/7286047/diff/1/src/gpu/GrAAHairLinePathRendere...
File src/gpu/GrAAHairLinePathRenderer.cpp (right):

https://codereview.appspot.com/7286047/diff/1/src/gpu/GrAAHairLinePathRendere...
src/gpu/GrAAHairLinePathRenderer.cpp:517:
target->drawState()->setVertexLayout(layout);
On 2013/02/04 23:08:12, robertphillips wrote:
> fold the getVertexSize & assert lines together or use debug code macro on
> getVertexSize line?

Done.

https://codereview.appspot.com/7286047/diff/1/src/gpu/GrContext.cpp
File src/gpu/GrContext.cpp (right):

https://codereview.appspot.com/7286047/diff/1/src/gpu/GrContext.cpp#newcode741
src/gpu/GrContext.cpp:741: static const int worstCaseVertCount = 10;
On 2013/02/04 23:08:12, robertphillips wrote:
> Add some sort of #define for 0 in this case?

Done.

https://codereview.appspot.com/7286047/diff/1/src/gpu/GrContext.cpp#newcode1012
src/gpu/GrContext.cpp:1012: drawState->setVertexLayout(layout);
On 2013/02/04 23:08:12, robertphillips wrote:
> fold getVertexSize & assert lines together?

Done.

https://codereview.appspot.com/7286047/diff/1/src/gpu/GrDrawState.h
File src/gpu/GrDrawState.h (right):

https://codereview.appspot.com/7286047/diff/1/src/gpu/GrDrawState.h#newcode86
src/gpu/GrDrawState.h:86: fCommon.fColor = 0xffffffff;
On 2013/02/04 23:08:12, robertphillips wrote:
> #define here too - empty? invalid?

Done. Called it kDefault_VertexLayout.

https://codereview.appspot.com/7286047/diff/1/src/gpu/GrDrawTarget.h
File src/gpu/GrDrawTarget.h (right):

https://codereview.appspot.com/7286047/diff/1/src/gpu/GrDrawTarget.h#newcode279
src/gpu/GrDrawTarget.h:279: * @param vertexArray   cpu array containing vertex
data.
On 2013/02/04 23:08:12, robertphillips wrote:
> delete vertexSize?

Done.

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

https://codereview.appspot.com/7286047/diff/1/src/gpu/GrInOrderDrawBuffer.cpp...
src/gpu/GrInOrderDrawBuffer.cpp:243: draw->fVertexLayout !=
drawState.getVertexLayout()) {
On 2013/02/04 22:37:12, bsalomon wrote:
> I think we don't this as a member of DrawRecord anymore. We should only get
here
> if the GrDrawState of the current draw matches that of the previous draw. Now
> that you've made VL a member of GrDS, we've already checked that the VLs match
> by checking the GrDSs match.

This sentence no verb. Does this mean I can just delete fVertexLayout from
DrawRecord?

https://codereview.appspot.com/7286047/diff/1/src/gpu/GrTextContext.cpp
File src/gpu/GrTextContext.cpp (right):

https://codereview.appspot.com/7286047/diff/1/src/gpu/GrTextContext.cpp#newco...
src/gpu/GrTextContext.cpp:205: fMaxVertices = kMinRequestedVerts;
On 2013/02/04 23:08:12, robertphillips wrote:
> It makes me nervous that we use fDrawTarget to set part of flush and the
assign
> to it and use the new value to compute the rest of the flush flag.

Changed to Brian's suggestion, below.

https://codereview.appspot.com/7286047/diff/1/src/gpu/GrTextContext.cpp#newco...
src/gpu/GrTextContext.cpp:210: flush = flush &&
fDrawTarget->geometryHints(&fMaxVertices,
On 2013/02/04 22:37:12, bsalomon wrote:
> Can we simplify the logic for setting var flush?
> 
> bool flush = false;
> if (NULL != fDT) {
>  ...
>  flush = fDT->gGH();
> }

Done.

[robertphillips, 2013-02-05 15:40:18]

lgtm

[bsalomon, 2013-02-05 15:43:18]

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

https://codereview.appspot.com/7286047/diff/1/src/gpu/GrInOrderDrawBuffer.cpp...
src/gpu/GrInOrderDrawBuffer.cpp:243: draw->fVertexLayout !=
drawState.getVertexLayout()) {
On 2013/02/05 14:47:59, JimVV wrote:
> On 2013/02/04 22:37:12, bsalomon wrote:
> > I think we don't this as a member of DrawRecord anymore. We should only get
> here
> > if the GrDrawState of the current draw matches that of the previous draw.
Now
> > that you've made VL a member of GrDS, we've already checked that the VLs
match
> > by checking the GrDSs match.
> 
> This sentence no verb. Does this mean I can just delete fVertexLayout from
> DrawRecord?

Yes.

[JimVV, 2013-02-05 16:18:16]

On 2013/02/05 15:43:18, bsalomon wrote:
> https://codereview.appspot.com/7286047/diff/1/src/gpu/GrInOrderDrawBuffer.cpp
> File src/gpu/GrInOrderDrawBuffer.cpp (right):
> 
>
https://codereview.appspot.com/7286047/diff/1/src/gpu/GrInOrderDrawBuffer.cpp...
> src/gpu/GrInOrderDrawBuffer.cpp:243: draw->fVertexLayout !=
> drawState.getVertexLayout()) {
> On 2013/02/05 14:47:59, JimVV wrote:
> > On 2013/02/04 22:37:12, bsalomon wrote:
> > > I think we don't this as a member of DrawRecord anymore. We should only
get
> > here
> > > if the GrDrawState of the current draw matches that of the previous draw.
> Now
> > > that you've made VL a member of GrDS, we've already checked that the VLs
> match
> > > by checking the GrDSs match.
> > 
> > This sentence no verb. Does this mean I can just delete fVertexLayout from
> > DrawRecord?
> 
> Yes.

Awesome. That was next on my list to get rid of.

[JimVV, 2013-02-05 16:45:38]

On 2013/02/05 16:18:16, JimVV wrote:
> On 2013/02/05 15:43:18, bsalomon wrote:
> >
https://codereview.appspot.com/7286047/diff/1/src/gpu/GrInOrderDrawBuffer.cpp
> > File src/gpu/GrInOrderDrawBuffer.cpp (right):
> > 
> >
>
https://codereview.appspot.com/7286047/diff/1/src/gpu/GrInOrderDrawBuffer.cpp...
> > src/gpu/GrInOrderDrawBuffer.cpp:243: draw->fVertexLayout !=
> > drawState.getVertexLayout()) {
> > On 2013/02/05 14:47:59, JimVV wrote:
> > > On 2013/02/04 22:37:12, bsalomon wrote:
> > > > I think we don't this as a member of DrawRecord anymore. We should only
> get
> > > here
> > > > if the GrDrawState of the current draw matches that of the previous
draw.
> > Now
> > > > that you've made VL a member of GrDS, we've already checked that the VLs
> > match
> > > > by checking the GrDSs match.
> > > 
> > > This sentence no verb. Does this mean I can just delete fVertexLayout from
> > > DrawRecord?
> > 
> > Yes.
> 
> Awesome. That was next on my list to get rid of.

Done, replaced with DrawRecord::fVertexSize (it's needed to set the amount of
reserved vertex data).

[bsalomon, 2013-02-05 16:50:10]

https://codereview.appspot.com/7286047/diff/16003/src/gpu/GrInOrderDrawBuffer...
File src/gpu/GrInOrderDrawBuffer.cpp (right):

https://codereview.appspot.com/7286047/diff/16003/src/gpu/GrInOrderDrawBuffer...
src/gpu/GrInOrderDrawBuffer.cpp:243: draw->fVertexSize != geomSrc.fVertexSize) {
I don't think we need this check here because the drawstate between the
previously recorded draw and this draw are known to be the same. So the vertex
sizes must match.

It seems like the remaining reads of DrawRecord::fVertexSize could be replaced
either with drawState->getVertexSize() or geomSrc.fVertexSize.

I'd still like to find a way to get rid of geomSrc.fVertexSize... but it doesn't
have to be in this CL.

[JimVV, 2013-02-05 18:24:59]

On 2013/02/05 16:50:10, bsalomon wrote:
>
https://codereview.appspot.com/7286047/diff/16003/src/gpu/GrInOrderDrawBuffer...
> File src/gpu/GrInOrderDrawBuffer.cpp (right):
> 
>
https://codereview.appspot.com/7286047/diff/16003/src/gpu/GrInOrderDrawBuffer...
> src/gpu/GrInOrderDrawBuffer.cpp:243: draw->fVertexSize != geomSrc.fVertexSize)
{
> I don't think we need this check here because the drawstate between the
> previously recorded draw and this draw are known to be the same. So the vertex
> sizes must match.
> 
> It seems like the remaining reads of DrawRecord::fVertexSize could be replaced
> either with drawState->getVertexSize() or geomSrc.fVertexSize.
> 
> I'd still like to find a way to get rid of geomSrc.fVertexSize... but it
doesn't
> have to be in this CL.

Okay, done.

[bsalomon, 2013-02-05 18:26:24]

lgtm

https://codereview.appspot.com/7286047/diff/23003/src/gpu/GrInOrderDrawBuffer...
File src/gpu/GrInOrderDrawBuffer.cpp (right):

https://codereview.appspot.com/7286047/diff/23003/src/gpu/GrInOrderDrawBuffer...
src/gpu/GrInOrderDrawBuffer.cpp:261: drawState.getVertexSize();
nit: this->

https://codereview.appspot.com/7286047/diff/23003/src/gpu/GrInOrderDrawBuffer...
src/gpu/GrInOrderDrawBuffer.cpp:325: drawState.getVertexSize();
nit: this->

https://codereview.appspot.com/7286047/diff/23003/src/gpu/GrInOrderDrawBuffer...
src/gpu/GrInOrderDrawBuffer.cpp:561: size_t vertexSize =
getDrawState().getVertexSize();
nit: this->