"Fix" bug in debug canvas

This addresses issue 1805 (Skia Debugger crashes when profiling SkPicture from chrome). The crux of the problem is that SkPicturePlayback::draw uses SkCanvas' quickRejectY method to cull clipped text. This can make the number of commands in the SkDebugCanvas substantially less then the number of commands in the captured skp - breaking a fundamental assumption of the debugger.

This CL patches the problem by setting the debug canvas' clip region to be very large (thus thwarting quickRejectY's culling). I'm not in love with this solution (since it is very side-effecty) but it does limit the changes to the DebugCanvas. The alternative seems to be to add a flag to SkPicturePlayback to optionally disable all optimizations.

[Tom Hudson, 2013-11-06 16:41:01]

Driveby looks-good, although this is obviously a hack rather than an ideal fix.

This stops crashes, *and* fixes the rendering in the debugger: without the
patch, you get approximately 0 text drawn on the example SKP, while with the
patch you get all the text modulo platform-specific font data. Since
SkDebugCanvas is compiled into Chromium, presumably this will have an impact on
the frame viewer.

There's still an apparent problem with profiling? On Linux, the debugger says
80% of the time on the page is spent on a single call to DrawRect, although the
rect does seem likely to be expensive. Does maximizing the clip on the
DebugCanvas break all clipping, and mean we're timing a 16Mpx rect blur instead
of a 1-3Mpx rect blur?

Parameters: 
SkRect: (0, 192, 360, 45780)

SkPaint:
TextSize:
12
TextScaleX:
1
TextSkewX:
0
DrawLooper:
SkLayerDrawLooper (2): 

0: flagsMask: (None) paintBits: (MaskFilter|ColorFilter) mode: kSrc offset: (0,
0) postTranslate: false 
SkPaint:
TextSize:
12
TextScaleX:
1
TextSkewX:
0
MaskFilter:
SkBlurMaskFilterImpl: (sigma: 2.2320499 style: normal flags: (HighQuality))
ColorFilter:
SkModeColorFilter: color: 0xFFDEDEDE mode: SrcIn
Color:
0xFF000000
Stroke Width:
0
Stroke Miter:
4
Flags:
(None)
FilterLevel:
None
TextAlign:
Left
CapType:
Butt
JoinType:
Miter
Style:
Fill
TextEncoding:
UTF8
Hinting:
Normal

1: flagsMask: (None) paintBits: (None) mode: kDst offset: (0, 0) postTranslate:
false 
SkPaint:
TextSize:
12
TextScaleX:
1
TextSkewX:
0
Color:
0xFF000000
Stroke Width:
0
Stroke Miter:
4
Flags:
(None)
FilterLevel:
None
TextAlign:
Left
CapType:
Butt
JoinType:
Miter
Style:
Fill
TextEncoding:
UTF8
Hinting:
Normal

Color:
0xFFFFFFFF
Stroke Width:
0
Stroke Miter:
4
Flags:
(AntiAlias)
FilterLevel:
None
TextAlign:
Left
CapType:
Butt
JoinType:
Miter
Style:
Fill
TextEncoding:
UTF8
Hinting:
Normal

[robertphillips, 2013-11-06 17:11:45]

To test this I disabled the proposed change and instead just disabled the
quickRejectY calls in SkPicturePlayback. The profiler still reports that large
blurred rect as taking >80% of the time.

[Justin Novosad, 2013-11-06 18:01:55]

Do we not encode the size of the picture in the skp?  Why not use that info?

https://codereview.chromium.org/59073008/diff/1/src/utils/debugger/SkDebugCan...
File src/utils/debugger/SkDebugCanvas.cpp (right):

https://codereview.chromium.org/59073008/diff/1/src/utils/debugger/SkDebugCan...
src/utils/debugger/SkDebugCanvas.cpp:45: largeIRect.inset(100, 100);
Why the inset?

[robertphillips, 2013-11-06 18:58:48]

The debugger assumes that no operations are removed during the rendering into
the DebugCanvas. To that end I just wanted to completely short-circuit the quick
reject tests. In this instance the DRAW_POS_TEXT_H_TOP_BOTTOM calls are coming
in with top/bottom values of ~19000/~19000 while the reported picture size is
just 390x9188.

[robertphillips, 2013-11-06 18:59:15]

https://codereview.chromium.org/59073008/diff/1/src/utils/debugger/SkDebugCan...
File src/utils/debugger/SkDebugCanvas.cpp (right):

https://codereview.chromium.org/59073008/diff/1/src/utils/debugger/SkDebugCan...
src/utils/debugger/SkDebugCanvas.cpp:45: largeIRect.inset(100, 100);
The SkRect is converted to an SkIRect internally and then rounded out. Setting
it to the largest IRect results in the rect collapsing during the rounding out
process.

[Justin Novosad, 2013-11-06 19:07:52]

On 2013/11/06 18:59:15, robertphillips wrote:
>
https://codereview.chromium.org/59073008/diff/1/src/utils/debugger/SkDebugCan...
> File src/utils/debugger/SkDebugCanvas.cpp (right):
> 
>
https://codereview.chromium.org/59073008/diff/1/src/utils/debugger/SkDebugCan...
> src/utils/debugger/SkDebugCanvas.cpp:45: largeIRect.inset(100, 100);
> The SkRect is converted to an SkIRect internally and then rounded out. Setting
> it to the largest IRect results in the rect collapsing during the rounding out
> process.

But this inset puts the top-left corner of the clip at (100,100). Isn't that
bad?

[Justin Novosad, 2013-11-06 19:29:41]

On 2013/11/06 19:07:52, junov wrote:
> On 2013/11/06 18:59:15, robertphillips wrote:
> >
>
https://codereview.chromium.org/59073008/diff/1/src/utils/debugger/SkDebugCan...
> > File src/utils/debugger/SkDebugCanvas.cpp (right):
> > 
> >
>
https://codereview.chromium.org/59073008/diff/1/src/utils/debugger/SkDebugCan...
> > src/utils/debugger/SkDebugCanvas.cpp:45: largeIRect.inset(100, 100);
> > The SkRect is converted to an SkIRect internally and then rounded out.
Setting
> > it to the largest IRect results in the rect collapsing during the rounding
out
> > process.
> 
> But this inset puts the top-left corner of the clip at (100,100). Isn't that
> bad?

FYI, Robert point-out to me by chat that MakeLargest puts the top left corner at
the extreme negative value.  So I recall the comment above.

[robertphillips, 2013-11-06 19:46:30]

Improved the hack.

[Justin Novosad, 2013-11-06 19:57:34]

https://codereview.chromium.org/59073008/diff/120001/src/utils/debugger/SkDeb...
File src/utils/debugger/SkDebugCanvas.cpp (right):

https://codereview.chromium.org/59073008/diff/120001/src/utils/debugger/SkDeb...
src/utils/debugger/SkDebugCanvas.cpp:52: large.round(&largeIRect);
roundOut instead of round?

[robertphillips, 2013-11-06 20:07:50]

https://codereview.chromium.org/59073008/diff/120001/src/utils/debugger/SkDeb...
File src/utils/debugger/SkDebugCanvas.cpp (right):

https://codereview.chromium.org/59073008/diff/120001/src/utils/debugger/SkDeb...
src/utils/debugger/SkDebugCanvas.cpp:52: large.round(&largeIRect);
Since it is BW, SkRasterClip::op just does a round and I wanted to match that
step.

[robertphillips, 2013-11-06 20:40:21]

https://codereview.chromium.org/59073008/diff/120001/src/utils/debugger/SkDeb...
File src/utils/debugger/SkDebugCanvas.cpp (right):

https://codereview.chromium.org/59073008/diff/120001/src/utils/debugger/SkDeb...
src/utils/debugger/SkDebugCanvas.cpp:52: large.round(&largeIRect);
On 2013/11/06 19:57:35, junov wrote:
> roundOut instead of round?

Done.

[Justin Novosad, 2013-11-06 20:47:38]

On 2013/11/06 20:40:21, robertphillips wrote:
>
https://codereview.chromium.org/59073008/diff/120001/src/utils/debugger/SkDeb...
> File src/utils/debugger/SkDebugCanvas.cpp (right):
> 
>
https://codereview.chromium.org/59073008/diff/120001/src/utils/debugger/SkDeb...
> src/utils/debugger/SkDebugCanvas.cpp:52: large.round(&largeIRect);
> On 2013/11/06 19:57:35, junov wrote:
> > roundOut instead of round?
> 
> Done.

lgtm

[robertphillips, 2013-11-07 22:20:57]

committed as r12180