Make the canvas draw looper internal layer not part of SkCanvas save count

Make the canvas draw looper internal layer not part of SkCanvas save count

Image filter in a paint would leave save count in wrong state
for normal draws. This could be observed through the canvas
references during the draw call. An example of this is
inspecting the canvas during a draw looper.

Instead of recording deferred saves for each MCRec, record 
the current save count when the MCRec was created. Internal
layers, i.e. layers that are created to implement a drawing
mode instead of client calling SkCanvas::saveLayer, do not 
increase the save count even though they create MCRecs.

BUG=skia:3388

[Kimmo Kinnunen, 2015-03-10 08:23:14]

kkinnunen@nvidia.com changed reviewers:
+ reed@google.com

[reed1, 2015-03-24 17:49:17]

The fix seems good, thanks.

I don't understand why the test has to also involve mutating green and
readpixels. Are those parts necessary to show the assert?

[Kimmo Kinnunen, 2015-03-25 06:18:26]

On 2015/03/24 17:49:17, reed1 wrote:
> The fix seems good, thanks.
> 
> I don't understand why the test has to also involve mutating green and
> readpixels. Are those parts necessary to show the assert?

The lack of assert can be result of either bug fixed or code not run? 
Mutating the color and the read pixels is proof that the code was run..

[Kimmo Kinnunen, 2015-03-25 06:21:09]

The CQ bit was checked by kkinnunen@nvidia.com

[commit-bot: I haz the power, 2015-03-25 06:21:17]

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

[commit-bot: I haz the power, 2015-03-25 06:21:17]

Note for Reviewers:
The CQ is waiting for an approval.
If you believe that the CL is not ready yet, or if you would like to L-G-T-M
with comments then please uncheck the CQ checkbox.

Waiting for LGTM from valid reviewer(s) till 2015-03-25 12:21 UTC

[commit-bot: I haz the power, 2015-03-25 12:21:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-03-25 12:21:22]

No LGTM from a valid reviewer yet. Please ask for an LGTM from a
full Skia committer from
https://skia.googlesource.com/skia/+/master/CQ_COMMITTERS

[reed1, 2015-03-26 20:14:05]

The CQ bit was checked by reed@google.com

[commit-bot: I haz the power, 2015-03-26 20:14:19]

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

[commit-bot: I haz the power, 2015-03-26 20:14:21]

Note for Reviewers:
The CQ is waiting for an approval.
If you believe that the CL is not ready yet, or if you would like to L-G-T-M
with comments then please uncheck the CQ checkbox.

Waiting for LGTM from valid reviewer(s) till 2015-03-27 02:14 UTC

[commit-bot: I haz the power, 2015-03-26 20:15:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-03-26 20:15:49]

Try jobs failed on following builders:
  Build-Ubuntu-GCC-x86_64-Release-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-x86...)

[Kimmo Kinnunen, 2015-03-31 13:28:43]

Thanks for the feedback.

How about this new version?



For reference:

This was landed in:
https://codereview.chromium.org/1034033004/

This was reverted in:
https://codereview.chromium.org/1037653004/

Reason for revert:
makes internalSave and internalSaveLayer inconsistent. Need to find a different
solution.


If I understand correctly, the inconsistency refers to following:
* internalSave would not increase the fSaveCount. It is the callers
responsibility of increasing the fSaveCount.

* internalSaveLayer would always increase the fSaveCount



For reference: A month ago, I also uploaded patch which had following
properties:
* internalSave would not increase the fSaveCount. It is the callers
responsibility of increasing the fSaveCount.

* internalSaveLayer would not increase the fSaveCount. It is the callers
responsibility of increasing the fSaveCount.

(so internalSave and internalSaveLayer were sort of consistent)

This version had this feedback:

On 2015/03/06 13:37:56, reed1 wrote:
> Is the bug/fix that we didn't call fSaveCount += 1 in autodrawlooper? If so,
> perhaps that should land as its own fix (it seems unrelated to kImage_Type).
> Also, I wonder if we can restructure canvas so that this looks more natural
> (e.g. some alternate to the public saveLayer() so that the bookkeeping of
> (private) fSaveCount is still encapsulated).

From this I interpret that the fact that fSaveCount was increased by the caller
was the violation of encapsulation. The caller was canvas-private implementation
of looper, SkAutoDrawLooper, which already violates all of SkCanvas
encapsulation.

[reed1, 2015-04-03 20:03:52]

reed@google.com changed reviewers:
+ robertphillips@google.com

[reed1, 2015-04-03 20:03:52]

adding a 2nd pair of eyes.