Make ClipStack honor save flags

This CL addresses "SkCanvas::save flags are not respected by GPU backend" (https://code.google.com/p/skia/issues/detail?id=1503).

Committed: http://code.google.com/p/skia/source/detail?r=10762

[bsalomon, 2013-08-14 15:49:13]

https://codereview.chromium.org/22947003/diff/1/src/core/SkCanvas.cpp
File src/core/SkCanvas.cpp (right):

https://codereview.chromium.org/22947003/diff/1/src/core/SkCanvas.cpp#newcode912
src/core/SkCanvas.cpp:912: if (NULL == fMCRec || fMCRec->fRasterClip !=
oldRasterClip) {
eek.. don't we want to stop computing raster clip for the GPU?

Is the problem than the MCRec doesn't remember the flags? Can it?

[robertphillips, 2013-08-14 16:16:57]

How about this?

https://codereview.chromium.org/22947003/diff/1/src/core/SkCanvas.cpp
File src/core/SkCanvas.cpp (right):

https://codereview.chromium.org/22947003/diff/1/src/core/SkCanvas.cpp#newcode912
src/core/SkCanvas.cpp:912: if (NULL == fMCRec || fMCRec->fRasterClip !=
oldRasterClip) {
That is the problem.

[bsalomon, 2013-08-14 16:21:50]

On 2013/08/14 16:16:57, robertphillips wrote:
> How about this?
> 
> https://codereview.chromium.org/22947003/diff/1/src/core/SkCanvas.cpp
> File src/core/SkCanvas.cpp (right):
> 
>
https://codereview.chromium.org/22947003/diff/1/src/core/SkCanvas.cpp#newcode912
> src/core/SkCanvas.cpp:912: if (NULL == fMCRec || fMCRec->fRasterClip !=
> oldRasterClip) {
> That is the problem.

lgtm, but Mike should have a look as I'm on shaky ground here.

[reed1, 2013-08-14 16:51:31]

https://codereview.chromium.org/22947003/diff/1/src/core/SkCanvas.cpp
File src/core/SkCanvas.cpp (right):

https://codereview.chromium.org/22947003/diff/1/src/core/SkCanvas.cpp#newcode912
src/core/SkCanvas.cpp:912: if (NULL == fMCRec || fMCRec->fRasterClip !=
oldRasterClip) {
On 2013/08/14 16:16:58, robertphillips wrote:
> That is the problem.

If we pass the flags (or a subset of them) to clipstack.save(), would that make
the restore easier? (at least for the caller)

[robertphillips, 2013-08-14 17:00:16]

https://codereview.chromium.org/22947003/diff/1/src/core/SkCanvas.cpp
File src/core/SkCanvas.cpp (right):

https://codereview.chromium.org/22947003/diff/1/src/core/SkCanvas.cpp#newcode912
src/core/SkCanvas.cpp:912: if (NULL == fMCRec || fMCRec->fRasterClip !=
oldRasterClip) {
So a ClipStack frame would store the number of restores to ignore before really
restoring itself?

[bsalomon, 2013-08-14 18:23:28]

https://codereview.chromium.org/22947003/diff/1/src/core/SkCanvas.cpp
File src/core/SkCanvas.cpp (right):

https://codereview.chromium.org/22947003/diff/1/src/core/SkCanvas.cpp#newcode912
src/core/SkCanvas.cpp:912: if (NULL == fMCRec || fMCRec->fRasterClip !=
oldRasterClip) {
On 2013/08/14 17:00:16, robertphillips wrote:
> So a ClipStack frame would store the number of restores to ignore before
really
> restoring itself?

yeah I don't see why clip stack should see saves that don't affect it. It seems
legit to have the caller (SkCanvas) examine the flags to decide whether to
save/restore the clipstack.

[reed1, 2013-08-14 18:28:04]

patch #2 is a lgtm -- I was really scared by that rasterclip== check (even just
for perf)

[commit-bot: I haz the power, 2013-08-16 00:36:52]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/robertphillips@google.com/22947003/6001

[commit-bot: I haz the power, 2013-08-16 00:53:38]

Change committed as 10762