Moved paint color to vertex colors for batched rects

The Eq/NotEq change doesn't fill me with joy but overall this seems to be a win and not to awful.

Before this CL:

tablebench  
8888: cmsecs =  14.43   
GPU: cmsecs =  54.76 gmsecs =  51.49  
NULLGPU: cmsecs =  43.99 gmsecs =   0.09

After this CL:

tablebench  
8888: cmsecs =  13.81   
GPU: cmsecs =  17.78 gmsecs =  14.17  
NULLGPU: cmsecs =  16.38 gmsecs =   0.09

[bsalomon, 2012-10-04 20:30:09]

Your changes reveals some unfortunate coupling... that the GrDrawState's color
is ignored if the vertex layout specifies per-vert color. Ugh.

Idea/Proposal for the long term: Make the per-vert/const color a very simple
custom stage that is installed as the first stage. This would require some more
thinking about how custom stages interact with vertex data. Eventually we want
to eliminate the vertex layout bits entirely, anyway.

In the meantime could we keep the current ==/!= on GrDrawState and just make the
GrIODB set the color to some known value, e.g. solid white, (and have a good
comment) rather than adding params to equality? It seems like it keeps the ugly
more tightly confined to one function.

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

https://codereview.appspot.com/6620045/diff/1/src/gpu/GrDrawState.h#newcode105
src/gpu/GrDrawState.h:105: const GrColor& getColor() const { return fColor; }
hmm.. do we really want to pass a pointer to a word sized thing rather than just
the thing?

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

https://codereview.appspot.com/6620045/diff/1/src/gpu/GrDrawTarget.h#newcode1022
src/gpu/GrDrawTarget.h:1022: static void SetRectVertices(const GrRect& rect,
Document color? I probably should have documented this function before but the
color param is a bit trickier than the others since it must be set iff the
layout has kColor.

[robertphillips, 2012-10-05 13:26:33]

On the coupling - Yep - I was surprised when stuff just started working after I
filled in the vertex colors.

Where were you thinking I could set the paint's color to white? In
needsNewState? I don't think we want to alter the back of the fStates vector.

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

https://codereview.appspot.com/6620045/diff/1/src/gpu/GrDrawState.h#newcode105
src/gpu/GrDrawState.h:105: const GrColor& getColor() const { return fColor; }
On 2012/10/04 20:30:09, bsalomon wrote:
> hmm.. do we really want to pass a pointer to a word sized thing rather than
just
> the thing?

Done.

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

https://codereview.appspot.com/6620045/diff/1/src/gpu/GrDrawTarget.h#newcode1022
src/gpu/GrDrawTarget.h:1022: static void SetRectVertices(const GrRect& rect,
On 2012/10/04 20:30:09, bsalomon wrote:
> Document color? I probably should have documented this function before but the
> color param is a bit trickier than the others since it must be set iff the
> layout has kColor.

Done.

[bsalomon, 2012-10-05 13:34:22]

On 2012/10/05 13:26:33, robertphillips wrote:
> On the coupling - Yep - I was surprised when stuff just started working after
I
> filled in the vertex colors.
> 
> Where were you thinking I could set the paint's color to white? In
> needsNewState? I don't think we want to alter the back of the fStates vector.

We could replace the AutoDeviceCoordDraw with an AutoStateRestore and then after
it is installed do this->drawState()->setColor(0xffffffff);

It would mean that we'd also have to add the code to go to dev coords in the
function. Or we could expand the ASRInit enum to have a dev coord value.



> 
> https://codereview.appspot.com/6620045/diff/1/src/gpu/GrDrawState.h
> File src/gpu/GrDrawState.h (right):
> 
> https://codereview.appspot.com/6620045/diff/1/src/gpu/GrDrawState.h#newcode105
> src/gpu/GrDrawState.h:105: const GrColor& getColor() const { return fColor; }
> On 2012/10/04 20:30:09, bsalomon wrote:
> > hmm.. do we really want to pass a pointer to a word sized thing rather than
> just
> > the thing?
> 
> Done.
> 
> https://codereview.appspot.com/6620045/diff/1/src/gpu/GrDrawTarget.h
> File src/gpu/GrDrawTarget.h (right):
> 
>
https://codereview.appspot.com/6620045/diff/1/src/gpu/GrDrawTarget.h#newcode1022
> src/gpu/GrDrawTarget.h:1022: static void SetRectVertices(const GrRect& rect,
> On 2012/10/04 20:30:09, bsalomon wrote:
> > Document color? I probably should have documented this function before but
the
> > color param is a bit trickier than the others since it must be set iff the
> > layout has kColor.
> 
> Done.

[robertphillips, 2012-10-05 15:27:43]

The AutoColorRestore class was a good call. I think this may be ready for prime
time.

[bsalomon, 2012-10-05 15:32:42]

Yeah, the change is much smaller now. And it may come in handy in the future.
LGTM.

https://codereview.appspot.com/6620045/diff/4004/src/gpu/GrDrawTarget.cpp
File src/gpu/GrDrawTarget.cpp (right):

https://codereview.appspot.com/6620045/diff/4004/src/gpu/GrDrawTarget.cpp#new...
src/gpu/GrDrawTarget.cpp:1071: // is present in 'layout'
I like the change to always take a color and ignore it if kColor isn't
specified.

[robertphillips, 2012-10-05 15:37:25]

committed as r5830