Drawstate on stack

Drawstate on stack

BUG=skia:

Committed: https://skia.googlesource.com/skia/+/9853ccef19c200be93a6211f32589fa82a53067c

[joshualitt, 2014-11-14 21:08:48]

joshualitt@google.com changed reviewers:
+ bsalomon@google.com, joshualitt@google.com

[joshualitt, 2014-11-14 21:08:49]

Okay, Brian, this is ready for review.  I'm still cleaning up bits and pieces. 
ClipMaskManager is where I think I need the most advice.  For the most part this
is just drawState on stack.

[bsalomon, 2014-11-14 21:51:03]

Does the clip stuff not work? I think you have the right idea.

Big Q: Can we make GrDrawState no longer ref-cnt'ed?

https://codereview.chromium.org/732693002/diff/60001/src/gpu/GrAAHairLinePath...
File src/gpu/GrAAHairLinePathRenderer.cpp (right):

https://codereview.chromium.org/732693002/diff/60001/src/gpu/GrAAHairLinePath...
src/gpu/GrAAHairLinePathRenderer.cpp:694: bool
does this need to be on its own line?

https://codereview.chromium.org/732693002/diff/60001/src/gpu/GrClipMaskManage...
File src/gpu/GrClipMaskManager.cpp (right):

https://codereview.chromium.org/732693002/diff/60001/src/gpu/GrClipMaskManage...
src/gpu/GrClipMaskManager.cpp:630: // We have to backup the drawstate because
the draw will consume ti
still true?

https://codereview.chromium.org/732693002/diff/60001/src/gpu/GrDrawTarget.cpp
File src/gpu/GrDrawTarget.cpp (right):

https://codereview.chromium.org/732693002/diff/60001/src/gpu/GrDrawTarget.cpp...
src/gpu/GrDrawTarget.cpp:732: SkASSERT(ds);
move to top?

https://codereview.chromium.org/732693002/diff/60001/src/gpu/GrDrawTarget.h
File src/gpu/GrDrawTarget.h (right):

https://codereview.chromium.org/732693002/diff/60001/src/gpu/GrDrawTarget.h#n...
src/gpu/GrDrawTarget.h:170: virtual bool geometryHints(size_t vertexStride,
one line?

[joshualitt, 2014-11-14 23:25:40]

https://codereview.chromium.org/732693002/diff/60001/src/gpu/GrClipMaskManage...
File src/gpu/GrClipMaskManager.cpp (right):

https://codereview.chromium.org/732693002/diff/60001/src/gpu/GrClipMaskManage...
src/gpu/GrClipMaskManager.cpp:630: // We have to backup the drawstate because
the draw will consume ti
On 2014/11/14 21:51:02, bsalomon wrote:
> still true?

Unfortunately it is.  DrawElements may call into things which modify the
drawState.

[joshualitt, 2014-11-14 23:28:04]

On 2014/11/14 23:25:40, joshualitt wrote:
>
https://codereview.chromium.org/732693002/diff/60001/src/gpu/GrClipMaskManage...
> File src/gpu/GrClipMaskManager.cpp (right):
> 
>
https://codereview.chromium.org/732693002/diff/60001/src/gpu/GrClipMaskManage...
> src/gpu/GrClipMaskManager.cpp:630: // We have to backup the drawstate because
> the draw will consume ti
> On 2014/11/14 21:51:02, bsalomon wrote:
> > still true?
> 
> Unfortunately it is.  DrawElements may call into things which modify the
> drawState.

Also other feedback incorporated.  I know there are places where I can cleanup
some of the restore stuff, but lets leave that for a follow on patch.  Ideally
after the view matrix / GrPaint stuff is worked out.

[bsalomon, 2014-11-17 14:41:17]

https://codereview.chromium.org/732693002/diff/60001/src/gpu/GrClipMaskManage...
File src/gpu/GrClipMaskManager.cpp (right):

https://codereview.chromium.org/732693002/diff/60001/src/gpu/GrClipMaskManage...
src/gpu/GrClipMaskManager.cpp:630: // We have to backup the drawstate because
the draw will consume ti
On 2014/11/14 23:25:40, joshualitt wrote:
> On 2014/11/14 21:51:02, bsalomon wrote:
> > still true?
> 
> Unfortunately it is.  DrawElements may call into things which modify the
> drawState.

Oh that makes sense. lgtm

[joshualitt, 2014-11-17 22:12:31]

The CQ bit was checked by joshualitt@google.com

[commit-bot: I haz the power, 2014-11-17 22:13:16]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/732693002/300001

[commit-bot: I haz the power, 2014-11-17 22:22:53]

Committed patchset #17 (id:300001) as
https://skia.googlesource.com/skia/+/9853ccef19c200be93a6211f32589fa82a53067c