Replace GrDrawState::AutoDeviceCoordDraw with GrDrawState::AutoViewMatrixRestore::setIdentity(). s

Replace GrDrawState::AutoDeviceCoordDraw with GrDrawState::AutoViewMatrixRestore::setIdentity(). s

R=robertphillips@google.com

Committed: https://code.google.com/p/skia/source/detail?r=9331

[bsalomon, 2013-05-22 21:36:53]

Makes removing the single fixed sized effect array slightly easier to
consolidate these very similar classes.

[robertphillips, 2013-05-22 23:47:01]

lgtm + a question & a suggestion

https://codereview.chromium.org/15780002/diff/2001/src/gpu/GrDrawState.cpp
File src/gpu/GrDrawState.cpp (right):

https://codereview.chromium.org/15780002/diff/2001/src/gpu/GrDrawState.cpp#ne...
src/gpu/GrDrawState.cpp:401: 
is it right to abort if preconcatMatrix is I? Couldn't they alter the viewMatrix
via another means and still want it restored?

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

https://codereview.chromium.org/15780002/diff/2001/src/gpu/GrDrawState.h#newc...
src/gpu/GrDrawState.h:711: 
delete this?

[jvanverth1, 2013-05-23 12:50:45]

Is there any effect on perf? It looks like you're replacing pointer references
with copies.

On 2013/05/22 23:47:01, robertphillips wrote:
> lgtm + a question & a suggestion
> 
> https://codereview.chromium.org/15780002/diff/2001/src/gpu/GrDrawState.cpp
> File src/gpu/GrDrawState.cpp (right):
> 
>
https://codereview.chromium.org/15780002/diff/2001/src/gpu/GrDrawState.cpp#ne...
> src/gpu/GrDrawState.cpp:401: 
> is it right to abort if preconcatMatrix is I? Couldn't they alter the
viewMatrix
> via another means and still want it restored?
> 
> https://codereview.chromium.org/15780002/diff/2001/src/gpu/GrDrawState.h
> File src/gpu/GrDrawState.h (right):
> 
>
https://codereview.chromium.org/15780002/diff/2001/src/gpu/GrDrawState.h#newc...
> src/gpu/GrDrawState.h:711: 
> delete this?

[bsalomon, 2013-05-23 12:52:02]

https://codereview.chromium.org/15780002/diff/2001/src/gpu/GrDrawState.cpp
File src/gpu/GrDrawState.cpp (right):

https://codereview.chromium.org/15780002/diff/2001/src/gpu/GrDrawState.cpp#ne...
src/gpu/GrDrawState.cpp:401: 
On 2013/05/22 23:47:01, robertphillips wrote:
> is it right to abort if preconcatMatrix is I? Couldn't they alter the
viewMatrix
> via another means and still want it restored?

Good point! I'll audit the current callers. If none rely on this then I'll
rename this to not imply that the VM is always restored. I don't think any users
should be whacking on the matrix directly since the effects won't compensate.

[bsalomon, 2013-05-29 13:41:10]

In the updated patch there is no direct setting of GrDrawState's matrix. All
updates go through functions/helper classes that update GrEffectStage coord
change matrices.

To Jim's question about perf: I did pre/post bench runs and only found noise.

[robertphillips, 2013-05-29 21:16:20]

lgtm + nits

https://codereview.chromium.org/15780002/diff/11001/src/gpu/GrDrawState.cpp
File src/gpu/GrDrawState.cpp (right):

https://codereview.chromium.org/15780002/diff/11001/src/gpu/GrDrawState.cpp#n...
src/gpu/GrDrawState.cpp:15: if (this->isStageEnabled(s)) {
if (!inverted) {
  if (!fCommon....) {
     return false;
  }
  inverted = true;
}

it just seems odd to keep setting inverted to true.

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

https://codereview.chromium.org/15780002/diff/11001/src/gpu/GrDrawState.h#new...
src/gpu/GrDrawState.h:954: } else {
fViewMatrix = -- add space

[bsalomon, 2013-05-29 21:28:53]

Committed patchset #4 manually as r9331 (presubmit successful).

[bsalomon, 2013-06-03 14:31:40]

https://codereview.chromium.org/15780002/diff/11001/src/gpu/GrDrawState.cpp
File src/gpu/GrDrawState.cpp (right):

https://codereview.chromium.org/15780002/diff/11001/src/gpu/GrDrawState.cpp#n...
src/gpu/GrDrawState.cpp:15: if (this->isStageEnabled(s)) {
On 2013/05/29 21:16:20, robertphillips wrote:
> if (!inverted) {
>   if (!fCommon....) {
>      return false;
>   }
>   inverted = true;
> }
> 
> it just seems odd to keep setting inverted to true.

Done.

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

https://codereview.chromium.org/15780002/diff/11001/src/gpu/GrDrawState.h#new...
src/gpu/GrDrawState.h:954: } else {
On 2013/05/29 21:16:20, robertphillips wrote:
> fViewMatrix = -- add space

Done.