Change clear() to respect the clip

Change clear() to respect the clip

patch from issue 769703002 at patchset 1 (http://crrev.com/769703002#ps1)

BUG=skia:

Committed: https://skia.googlesource.com/skia/+/3729469d6a12266037b697c2192768545e097ab0

[reed1, 2014-12-01 19:06:27]

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

[reed1, 2014-12-01 19:06:28]

prev patch abandoned

[reed1, 2014-12-02 13:40:09]

The CQ bit was checked by reed@google.com

[commit-bot: I haz the power, 2014-12-02 13:41:05]

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

[commit-bot: I haz the power, 2014-12-02 13:41:06]

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 2014-12-02 19:41 UTC

[reed1, 2014-12-02 15:19:35]

ptal -- the try-canaries are busted, so I'd like to land this this morning, and
watch the real bots

[mtklein, 2014-12-02 15:27:08]

The CQ bit was unchecked by mtklein@google.com

[mtklein, 2014-12-02 15:27:37]

lgtm.  Just want to leave a few notes to remind us how and when to clean up.

https://codereview.chromium.org/772533004/diff/40001/include/core/SkCanvas.h
File include/core/SkCanvas.h (right):

https://codereview.chromium.org/772533004/diff/40001/include/core/SkCanvas.h#...
include/core/SkCanvas.h:625: virtual void clear(SkColor color) {
How about we leave ourselves the note here:
 // FYI: M41 was the last Chrome to call clear() before it was translated to
drawColor().

https://codereview.chromium.org/772533004/diff/40001/tests/DeferredCanvasTest...
File tests/DeferredCanvasTest.cpp (right):

https://codereview.chromium.org/772533004/diff/40001/tests/DeferredCanvasTest...
tests/DeferredCanvasTest.cpp:287: #if 0
Can you swap this to #ifdef SK_CLEAR_IGNORES_CLIP so it's easier to find when we
follow up cleaning all this up?

https://codereview.chromium.org/772533004/diff/40001/tests/RecordDrawTest.cpp
File tests/RecordDrawTest.cpp (right):

https://codereview.chromium.org/772533004/diff/40001/tests/RecordDrawTest.cpp...
tests/RecordDrawTest.cpp:205: #if 0
Same?

[reed1, 2014-12-02 16:21:15]

https://codereview.chromium.org/772533004/diff/40001/include/core/SkCanvas.h
File include/core/SkCanvas.h (right):

https://codereview.chromium.org/772533004/diff/40001/include/core/SkCanvas.h#...
include/core/SkCanvas.h:625: virtual void clear(SkColor color) {
On 2014/12/02 15:27:37, mtklein wrote:
> How about we leave ourselves the note here:
>  // FYI: M41 was the last Chrome to call clear() before it was translated to
> drawColor().

Hmmm, that seems like a note better suited for where we version pictures... yes?

https://codereview.chromium.org/772533004/diff/40001/tests/DeferredCanvasTest...
File tests/DeferredCanvasTest.cpp (right):

https://codereview.chromium.org/772533004/diff/40001/tests/DeferredCanvasTest...
tests/DeferredCanvasTest.cpp:287: #if 0
On 2014/12/02 15:27:37, mtklein wrote:
> Can you swap this to #ifdef SK_CLEAR_IGNORES_CLIP so it's easier to find when
we
> follow up cleaning all this up?

Shouldn't I just remove this?

https://codereview.chromium.org/772533004/diff/40001/tests/RecordDrawTest.cpp
File tests/RecordDrawTest.cpp (right):

https://codereview.chromium.org/772533004/diff/40001/tests/RecordDrawTest.cpp...
tests/RecordDrawTest.cpp:205: #if 0
On 2014/12/02 15:27:37, mtklein wrote:
> Same?

Seems like I should just remove this, since clear() is now just an alias for
drawPaint

[mtklein, 2014-12-02 16:29:28]

https://codereview.chromium.org/772533004/diff/40001/include/core/SkCanvas.h
File include/core/SkCanvas.h (right):

https://codereview.chromium.org/772533004/diff/40001/include/core/SkCanvas.h#...
include/core/SkCanvas.h:625: virtual void clear(SkColor color) {
On 2014/12/02 16:21:15, reed1 wrote:
> On 2014/12/02 15:27:37, mtklein wrote:
> > How about we leave ourselves the note here:
> >  // FYI: M41 was the last Chrome to call clear() before it was translated to
> > drawColor().
> 
> Hmmm, that seems like a note better suited for where we version pictures...
yes?

Sure.  As long as we leave a note, anywhere sensible is fine by me.

https://codereview.chromium.org/772533004/diff/60001/tests/RecordDrawTest.cpp
File tests/RecordDrawTest.cpp (right):

https://codereview.chromium.org/772533004/diff/60001/tests/RecordDrawTest.cpp...
tests/RecordDrawTest.cpp:205: #if 0
Sure, we can remove this now, but if we do there's more we want to remove at the
same time, namely the whole SkRecords::PartialDraw class in SkRecordDraw.cpp. 
It was only different fro SkRecords::Draw in clear().

[reed1, 2014-12-02 16:39:10]

On 2014/12/02 16:29:28, mtklein wrote:
> https://codereview.chromium.org/772533004/diff/40001/include/core/SkCanvas.h
> File include/core/SkCanvas.h (right):
> 
>
https://codereview.chromium.org/772533004/diff/40001/include/core/SkCanvas.h#...
> include/core/SkCanvas.h:625: virtual void clear(SkColor color) {
> On 2014/12/02 16:21:15, reed1 wrote:
> > On 2014/12/02 15:27:37, mtklein wrote:
> > > How about we leave ourselves the note here:
> > >  // FYI: M41 was the last Chrome to call clear() before it was translated
to
> > > drawColor().
> > 
> > Hmmm, that seems like a note better suited for where we version pictures...
> yes?
> 
> Sure.  As long as we leave a note, anywhere sensible is fine by me.
> 
> https://codereview.chromium.org/772533004/diff/60001/tests/RecordDrawTest.cpp
> File tests/RecordDrawTest.cpp (right):
> 
>
https://codereview.chromium.org/772533004/diff/60001/tests/RecordDrawTest.cpp...
> tests/RecordDrawTest.cpp:205: #if 0
> Sure, we can remove this now, but if we do there's more we want to remove at
the
> same time, namely the whole SkRecords::PartialDraw class in SkRecordDraw.cpp. 
> It was only different fro SkRecords::Draw in clear().

Ah, didn't know that. Will look at PartialDraw now
... I'm also working on cleaning up SkDevice's (now obsolete) clear() method

[reed1, 2014-12-02 17:37:16]

ptal

[mtklein, 2014-12-02 17:43:11]

This LGTM, but I'd want Robert's nod too.

[reed1, 2014-12-02 17:44:07]

reed@google.com changed required reviewers:
+ mtklein@google.com, robertphillips@google.com

[robertphillips, 2014-12-02 17:52:10]

lgtm + dox change

https://codereview.chromium.org/772533004/diff/120001/src/core/SkRecordDraw.h
File src/core/SkRecordDraw.h (right):

https://codereview.chromium.org/772533004/diff/120001/src/core/SkRecordDraw.h...
src/core/SkRecordDraw.h:31: 
Remove the "while replacing clears with drawRects" part ?

[reed1, 2014-12-02 17:56:22]

https://codereview.chromium.org/772533004/diff/120001/src/core/SkRecordDraw.h
File src/core/SkRecordDraw.h (right):

https://codereview.chromium.org/772533004/diff/120001/src/core/SkRecordDraw.h...
src/core/SkRecordDraw.h:31: 
On 2014/12/02 17:52:09, robertphillips wrote:
> Remove the "while replacing clears with drawRects" part ?

Done.

[reed1, 2014-12-02 17:56:29]

The CQ bit was checked by reed@google.com

[commit-bot: I haz the power, 2014-12-02 17:57:29]

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

[commit-bot: I haz the power, 2014-12-02 18:08:15]

Committed patchset #8 (id:140001) as
https://skia.googlesource.com/skia/+/3729469d6a12266037b697c2192768545e097ab0

[reed2, 2014-12-03 03:12:29]

A revert of this CL (patchset #8 id:140001) has been created in
https://codereview.chromium.org/747313004/ by reed@chromium.org.

The reason for reverting is: failing win unittests.