Split SkPicturePlayback out of SkPictureData

Split SkPicturePlayback out of SkPictureData


This splits the playback functionality out of SkPictureData. The old SkPictureData::draw method is pulled out along
with its supporting functions as verbatim as possible. Some follow on CLs will be required to:

   re-enable profiling in the debugger (and remove the vestiges of SkTimedPicture)
   re-enable display of command offsets in the picture (this should probably wait until we've switched to SkRecord though)
   Clean up CachedOperationList (maybe fuse with SkPicture::OperationList)
   Split SkPicturePlayback into a base class and two derived classes
   Implement parallel version of GatherGPUInfo for SkRecord

Landing this is blocked on removing Android's use of the abortPlayback entry point.

Committed: https://skia.googlesource.com/skia/+/ce4dd3de38cd7c29bf5b9d8a8efb55c08ec9be47

[mtklein, 2014-07-07 19:32:08]

Just nits

https://codereview.chromium.org/377623002/diff/20001/src/core/SkPictureData.cpp
File src/core/SkPictureData.cpp (right):

https://codereview.chromium.org/377623002/diff/20001/src/core/SkPictureData.c...
src/core/SkPictureData.cpp:708: CachedOperationList* activeOps =
SkNEW(CachedOperationList);
Given that these are no longer cached, might want to change the name?  It could
be confusing, but SkPictureData::OperationList should work fine.

https://codereview.chromium.org/377623002/diff/20001/src/core/SkPicturePlayba...
File src/core/SkPicturePlayback.h (right):

https://codereview.chromium.org/377623002/diff/20001/src/core/SkPicturePlayba...
src/core/SkPicturePlayback.h:19: class SkPicturePlayback {
: SkNoncopyable ?

https://codereview.chromium.org/377623002/diff/20001/src/gpu/GrPictureUtils.cpp
File src/gpu/GrPictureUtils.cpp (right):

https://codereview.chromium.org/377623002/diff/20001/src/gpu/GrPictureUtils.c...
src/gpu/GrPictureUtils.cpp:238: if (NULL != picture->fData.get()) {
This bit is confusing me now.  Can you comment it up with what we're doing on
both branches?

[reed1, 2014-07-07 19:43:01]

api lgtm

[robertphillips, 2014-07-07 19:55:55]

https://codereview.chromium.org/377623002/diff/20001/src/core/SkPictureData.cpp
File src/core/SkPictureData.cpp (right):

https://codereview.chromium.org/377623002/diff/20001/src/core/SkPictureData.c...
src/core/SkPictureData.cpp:708: CachedOperationList* activeOps =
SkNEW(CachedOperationList);
On 2014/07/07 19:32:08, mtklein wrote:
> Given that these are no longer cached, might want to change the name?  It
could
> be confusing, but SkPictureData::OperationList should work fine.

Done. I'm going to just move this into SkPicture::OperationList in a followup
CL.

https://codereview.chromium.org/377623002/diff/20001/src/core/SkPicturePlayba...
File src/core/SkPicturePlayback.h (right):

https://codereview.chromium.org/377623002/diff/20001/src/core/SkPicturePlayba...
src/core/SkPicturePlayback.h:19: class SkPicturePlayback {
On 2014/07/07 19:32:08, mtklein wrote:
> : SkNoncopyable ?

Done.

https://codereview.chromium.org/377623002/diff/20001/src/gpu/GrPictureUtils.cpp
File src/gpu/GrPictureUtils.cpp (right):

https://codereview.chromium.org/377623002/diff/20001/src/gpu/GrPictureUtils.c...
src/gpu/GrPictureUtils.cpp:238: if (NULL != picture->fData.get()) {
On 2014/07/07 19:32:08, mtklein wrote:
> This bit is confusing me now.  Can you comment it up with what we're doing on
> both branches?

Done. All this does is collect information about the saveLayer/restore pairs. I
will write a far simpler version for SkRecord in a followup CL and switch the
behavior in GatherGPUInfo rather than here.

[mtklein, 2014-07-07 19:58:30]

lgtm

https://codereview.chromium.org/377623002/diff/10018/src/gpu/GrPictureUtils.cpp
File src/gpu/GrPictureUtils.cpp (right):

https://codereview.chromium.org/377623002/diff/10018/src/gpu/GrPictureUtils.c...
src/gpu/GrPictureUtils.cpp:247: // SkPicture::draw.
OOOH.  Thanks, this helped a lot.

[robertphillips, 2014-07-07 20:22:44]

The CQ bit was checked by robertphillips@google.com

[commit-bot: I haz the power, 2014-07-07 20:23:37]

CQ is trying da patch. Follow status at

https://skia-tree-status.appspot.com/cq/robertphillips@google.com/377623002/1...

[robertphillips, 2014-07-07 20:34:09]

The CQ bit was checked by robertphillips@google.com

[commit-bot: I haz the power, 2014-07-07 20:34:13]

CQ is trying da patch. Follow status at

https://skia-tree-status.appspot.com/cq/robertphillips@google.com/377623002/7...

[commit-bot: I haz the power, 2014-07-07 20:46:44]

Change committed as ce4dd3de38cd7c29bf5b9d8a8efb55c08ec9be47