Create Read-only Base class for GrDrawState that holds data members and getters

Create Read-only Base class for GrDrawState that holds data members and getters

Base class will but used also in follow up cl that will create an OptDrawState class which will
share much of the same data/functions as DrawState. Thus the base class
BUG=skia:

Committed: https://skia.googlesource.com/skia/+/21aed57023bad4b04076c59e37097c7563efc028

[egdaniel, 2014-08-26 18:01:22]

egdaniel@google.com changed reviewers:
+ bsalomon@google.com

[bsalomon, 2014-08-26 18:15:01]

https://codereview.chromium.org/506803003/diff/1/src/gpu/GrDrawState.h
File src/gpu/GrDrawState.h (right):

https://codereview.chromium.org/506803003/diff/1/src/gpu/GrDrawState.h#newcod...
src/gpu/GrDrawState.h:113: bool srcAlphaWillBeOne() const;
Shouldn't this be in RO?

https://codereview.chromium.org/506803003/diff/1/src/gpu/GrDrawState.h#newcod...
src/gpu/GrDrawState.h:315: void invalidateBlendOptFlags() {
Is this public? Need it be?

https://codereview.chromium.org/506803003/diff/1/src/gpu/GrDrawState.h#newcod...
src/gpu/GrDrawState.h:323: bool canIgnoreColorAttribute() const;
Should this be in RO? I guess it is going to go away soon?

https://codereview.chromium.org/506803003/diff/1/src/gpu/GrRODrawState.h
File src/gpu/GrRODrawState.h (right):

https://codereview.chromium.org/506803003/diff/1/src/gpu/GrRODrawState.h#newc...
src/gpu/GrRODrawState.h:21: * Read-only base class for GrDrawState. This class
contains all the necessariy data to represent a
typo i in necessary

[egdaniel, 2014-08-26 18:23:46]

https://codereview.chromium.org/506803003/diff/1/src/gpu/GrDrawState.h
File src/gpu/GrDrawState.h (right):

https://codereview.chromium.org/506803003/diff/1/src/gpu/GrDrawState.h#newcod...
src/gpu/GrDrawState.h:113: bool srcAlphaWillBeOne() const;
On 2014/08/26 18:15:01, bsalomon wrote:
> Shouldn't this be in RO?

This is only called in one place, calcBlendOpt. It actually could be a private
function in GrDrawState

https://codereview.chromium.org/506803003/diff/1/src/gpu/GrDrawState.h#newcod...
src/gpu/GrDrawState.h:315: void invalidateBlendOptFlags() {
On 2014/08/26 18:15:01, bsalomon wrote:
> Is this public? Need it be?

Should be private.

https://codereview.chromium.org/506803003/diff/1/src/gpu/GrDrawState.h#newcod...
src/gpu/GrDrawState.h:323: bool canIgnoreColorAttribute() const;
On 2014/08/26 18:15:01, bsalomon wrote:
> Should this be in RO? I guess it is going to go away soon?

The entire idea of ignoring color attribs is removed once I connect in the opt
code. But yes should probably makes more sense in RO for now.

[bsalomon, 2014-08-26 18:25:56]

https://codereview.chromium.org/506803003/diff/1/src/gpu/GrDrawState.h
File src/gpu/GrDrawState.h (right):

https://codereview.chromium.org/506803003/diff/1/src/gpu/GrDrawState.h#newcode17
src/gpu/GrDrawState.h:17: * Sub class that is derived from GrRODrawState. The
majority of the data the represents a draw
Subclass?

"data the"->"data that"?

Maybe say "Editable subclass of GrRODrawState ..." or "Modifiable"?

Maybe mention something like "This is used to configure the state used when
issuing draws via GrDrawTarget." rather than the vague language around "majority
of cases."?

[bsalomon, 2014-08-26 18:26:41]

https://codereview.chromium.org/506803003/diff/1/src/gpu/GrDrawState.h
File src/gpu/GrDrawState.h (right):

https://codereview.chromium.org/506803003/diff/1/src/gpu/GrDrawState.h#newcod...
src/gpu/GrDrawState.h:113: bool srcAlphaWillBeOne() const;
On 2014/08/26 18:23:46, egdaniel wrote:
> On 2014/08/26 18:15:01, bsalomon wrote:
> > Shouldn't this be in RO?
> 
> This is only called in one place, calcBlendOpt. It actually could be a private
> function in GrDrawState

That sgtm

[egdaniel, 2014-08-26 18:45:23]

https://codereview.chromium.org/506803003/diff/1/src/gpu/GrDrawState.h
File src/gpu/GrDrawState.h (right):

https://codereview.chromium.org/506803003/diff/1/src/gpu/GrDrawState.h#newcode17
src/gpu/GrDrawState.h:17: * Sub class that is derived from GrRODrawState. The
majority of the data the represents a draw
On 2014/08/26 18:25:55, bsalomon wrote:
> Subclass?
> 
> "data the"->"data that"?
> 
> Maybe say "Editable subclass of GrRODrawState ..." or "Modifiable"?
> 
> Maybe mention something like "This is used to configure the state used when
> issuing draws via GrDrawTarget." rather than the vague language around
"majority
> of cases."?
> 
> 

Done.

https://codereview.chromium.org/506803003/diff/1/src/gpu/GrDrawState.h#newcod...
src/gpu/GrDrawState.h:323: bool canIgnoreColorAttribute() const;
On 2014/08/26 18:23:46, egdaniel wrote:
> On 2014/08/26 18:15:01, bsalomon wrote:
> > Should this be in RO? I guess it is going to go away soon?
> 
> The entire idea of ignoring color attribs is removed once I connect in the opt
> code. But yes should probably makes more sense in RO for now.

So I realized the reason I had this here is because the function itself calls
getBlendOpts. I figured it was easier to leave it here in GrDrawState instead of
either moving getBlendOpts to RO or having it override an unused version in RO,
especially knowing it will disappear soon.

https://codereview.chromium.org/506803003/diff/1/src/gpu/GrRODrawState.h
File src/gpu/GrRODrawState.h (right):

https://codereview.chromium.org/506803003/diff/1/src/gpu/GrRODrawState.h#newc...
src/gpu/GrRODrawState.h:21: * Read-only base class for GrDrawState. This class
contains all the necessariy data to represent a
On 2014/08/26 18:15:01, bsalomon wrote:
> typo i in necessary

Done.

[bsalomon, 2014-08-26 18:51:53]

https://codereview.chromium.org/506803003/diff/1/src/gpu/GrDrawState.h
File src/gpu/GrDrawState.h (right):

https://codereview.chromium.org/506803003/diff/1/src/gpu/GrDrawState.h#newcod...
src/gpu/GrDrawState.h:323: bool canIgnoreColorAttribute() const;
On 2014/08/26 18:45:23, egdaniel wrote:
> On 2014/08/26 18:23:46, egdaniel wrote:
> > On 2014/08/26 18:15:01, bsalomon wrote:
> > > Should this be in RO? I guess it is going to go away soon?
> > 
> > The entire idea of ignoring color attribs is removed once I connect in the
opt
> > code. But yes should probably makes more sense in RO for now.
> 
> So I realized the reason I had this here is because the function itself calls
> getBlendOpts. I figured it was easier to leave it here in GrDrawState instead
of
> either moving getBlendOpts to RO or having it override an unused version in
RO,
> especially knowing it will disappear soon.

Ok if it disappears (or goes private) at the end of the three CL sequence then
fine by me.

https://codereview.chromium.org/506803003/diff/20001/src/gpu/GrDrawState.h
File src/gpu/GrDrawState.h (right):

https://codereview.chromium.org/506803003/diff/20001/src/gpu/GrDrawState.h#ne...
src/gpu/GrDrawState.h:555: 
some extra newlines here

[bsalomon, 2014-08-26 18:53:21]

lgtm

[egdaniel, 2014-08-26 19:07:38]

The CQ bit was checked by egdaniel@google.com

[commit-bot: I haz the power, 2014-08-26 19:08:44]

CQ is trying da patch. Follow status at
 https://skia-tree-status.appspot.com/cq/egdaniel@google.com/506803003/40001

[commit-bot: I haz the power, 2014-08-26 19:24:12]

Committed patchset #3 (40001) as 21aed57023bad4b04076c59e37097c7563efc028