Paint Profiler: unroll SkPictures

Paint Profiler: unroll SkPictures

Perform SkPicure::playback() instead of SkCanvas::drawPicture() in
all paint profiler intercepting canvas, so that we actually expose
a step-by-step picture painting instead of one drawPicture() call.
Otherwise, paint profiler is unusable in slimming paint mode.

BUG=

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=197340

[caseq, 2015-06-17 18:03:51]

caseq@chromium.org changed reviewers:
+ fmalita@chromium.org, pfeldman@chromium.org

[f(malita), 2015-06-17 18:20:56]

LGTM

https://codereview.chromium.org/1183863005/diff/1/Source/platform/graphics/In...
File Source/platform/graphics/InterceptingCanvas.cpp (right):

https://codereview.chromium.org/1183863005/diff/1/Source/platform/graphics/In...
Source/platform/graphics/InterceptingCanvas.cpp:14: SkRect newBounds =
picture->cullRect();
Nit: the 'bounds' saveLayer arg is optional, so I think it would be OK to skip
this computation and just pass NULL instead.

https://codereview.chromium.org/1183863005/diff/1/Source/platform/graphics/Lo...
File Source/platform/graphics/LoggingCanvas.cpp (left):

https://codereview.chromium.org/1183863005/diff/1/Source/platform/graphics/Lo...
Source/platform/graphics/LoggingCanvas.cpp:789:
logger.logItemWithParams("drawPicture")->setObject("picture",
objectForSkPicture(*picture));
It would be nice to provide some indication of picture grouping.  For the Skia
debugger, we solved this by injecting pairs of synthetic beginPicture/endPicture
ops in the UI (plus indentation).

Not sure whether that approach is applicable here, and I don't think it needs to
be tied to this CL - just a thought.

[caseq, 2015-06-17 18:29:48]

https://codereview.chromium.org/1183863005/diff/1/Source/platform/graphics/In...
File Source/platform/graphics/InterceptingCanvas.cpp (right):

https://codereview.chromium.org/1183863005/diff/1/Source/platform/graphics/In...
Source/platform/graphics/InterceptingCanvas.cpp:14: SkRect newBounds =
picture->cullRect();
On 2015/06/17 18:20:56, f(malita) wrote:
> Nit: the 'bounds' saveLayer arg is optional, so I think it would be OK to skip
> this computation and just pass NULL instead.

Would it be the same performance-wise? I was thinking that perhaps for drawing a
small picture on a large bitmap-backed canvas this might matter, and since this
also runs in profiler, I thought we better follow what Skia would do.

https://codereview.chromium.org/1183863005/diff/1/Source/platform/graphics/Lo...
File Source/platform/graphics/LoggingCanvas.cpp (left):

https://codereview.chromium.org/1183863005/diff/1/Source/platform/graphics/Lo...
Source/platform/graphics/LoggingCanvas.cpp:789:
logger.logItemWithParams("drawPicture")->setObject("picture",
objectForSkPicture(*picture));
On 2015/06/17 18:20:56, f(malita) wrote:
> It would be nice to provide some indication of picture grouping.  For the Skia
> debugger, we solved this by injecting pairs of synthetic
beginPicture/endPicture
> ops in the UI (plus indentation).
> 
> Not sure whether that approach is applicable here, and I don't think it needs
to
> be tied to this CL - just a thought.

Yup, I agree. We need to figure out how to present it in the front-end though,
so let's do this in a separate patch.

[f(malita), 2015-06-17 18:41:57]

https://codereview.chromium.org/1183863005/diff/1/Source/platform/graphics/In...
File Source/platform/graphics/InterceptingCanvas.cpp (right):

https://codereview.chromium.org/1183863005/diff/1/Source/platform/graphics/In...
Source/platform/graphics/InterceptingCanvas.cpp:14: SkRect newBounds =
picture->cullRect();
On 2015/06/17 at 18:29:47, caseq wrote:
> On 2015/06/17 18:20:56, f(malita) wrote:
> > Nit: the 'bounds' saveLayer arg is optional, so I think it would be OK to
skip
> > this computation and just pass NULL instead.
> 
> Would it be the same performance-wise? I was thinking that perhaps for drawing
a small picture on a large bitmap-backed canvas this might matter, and since
this also runs in profiler, I thought we better follow what Skia would do.

It defaults to using the current clip bounds, but if the clip is not tight then
this can potentially be (a lot) more expensive.

I hadn't realized this is contributing to the profiler times, so yeah, it makes
sense to use the picture cull rect for perf reasons - ignore my nit :)

[pfeldman, 2015-06-18 06:49:46]

owner lgtm

[pfeldman, 2015-06-18 06:49:47]

owner lgtm

[caseq, 2015-06-18 10:03:27]

The CQ bit was checked by caseq@chromium.org

[commit-bot: I haz the power, 2015-06-18 10:03:51]

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

[commit-bot: I haz the power, 2015-06-18 11:11:56]

Committed patchset #1 (id:1) as
https://src.chromium.org/viewvc/blink?view=rev&revision=197340