Update SkPictureRecord to allow some ops to be written separately

Delaying & reordering the emission of save/saveLayer/restore, clip and matrix calls requires that we be able to write out the delayed operations separate from the usual serialization. This CL adds new *Impl entry points that actual write out the delayable operations.

This CL also bubbles the clip op's offset location up the call stack to allow alternate implementations of the op skipping system.

[reed1, 2014-02-04 15:47:16]

any measurable impact on our record benches?

https://codereview.chromium.org/153883002/diff/70001/src/core/SkPictureRecord.h
File src/core/SkPictureRecord.h (right):

https://codereview.chromium.org/153883002/diff/70001/src/core/SkPictureRecord...
src/core/SkPictureRecord.h:111: int
recordRestoreOffsetPlaceholder(SkRegion::Op);
not new, but soon we should start adding dox to these internal methods, since
this stuff is already hard to understand...

https://codereview.chromium.org/153883002/diff/70001/src/core/SkPictureRecord...
src/core/SkPictureRecord.h:240: void concatImpl(const SkMatrix& matrix);
block comment as to why we have these "helpers"?

[mtklein, 2014-02-04 16:33:57]

https://codereview.chromium.org/153883002/diff/180001/src/core/SkPictureRecor...
File src/core/SkPictureRecord.cpp (right):

https://codereview.chromium.org/153883002/diff/180001/src/core/SkPictureRecor...
src/core/SkPictureRecord.cpp:167: /*  Don't actually call saveLayer, because
that will try to allocate an
While we're at it, can't hurt to make it clear by saying

/* Don't actually call INHERITED::saveLayer ...

https://codereview.chromium.org/153883002/diff/180001/src/core/SkPictureRecord.h
File src/core/SkPictureRecord.h (right):

https://codereview.chromium.org/153883002/diff/180001/src/core/SkPictureRecor...
src/core/SkPictureRecord.h:167: int addPathToHeap(const SkPath& path);
// does not write to ops stream
?

https://codereview.chromium.org/153883002/diff/180001/src/core/SkPictureRecor...
src/core/SkPictureRecord.h:248: void saveImpl(SaveFlags flags);
given the way they're used,  maybe rename to recordSave, recordSaveLayer, etc. ?

[robertphillips, 2014-02-04 19:08:01]

This change has no measureable impact on the recording bench times.

https://codereview.chromium.org/153883002/diff/70001/src/core/SkPictureRecord.h
File src/core/SkPictureRecord.h (right):

https://codereview.chromium.org/153883002/diff/70001/src/core/SkPictureRecord...
src/core/SkPictureRecord.h:111: int
recordRestoreOffsetPlaceholder(SkRegion::Op);
I will keep this in mind. If the collapsing code sticks the entire offset
recording system will change.

https://codereview.chromium.org/153883002/diff/70001/src/core/SkPictureRecord...
src/core/SkPictureRecord.h:240: void concatImpl(const SkMatrix& matrix);
On 2014/02/04 15:47:17, reed1 wrote:
> block comment as to why we have these "helpers"?

Done.

https://codereview.chromium.org/153883002/diff/180001/src/core/SkPictureRecor...
File src/core/SkPictureRecord.cpp (right):

https://codereview.chromium.org/153883002/diff/180001/src/core/SkPictureRecor...
src/core/SkPictureRecord.cpp:167: /*  Don't actually call saveLayer, because
that will try to allocate an
On 2014/02/04 16:33:58, mtklein wrote:
> While we're at it, can't hurt to make it clear by saying
> 
> /* Don't actually call INHERITED::saveLayer ...

Done.

https://codereview.chromium.org/153883002/diff/180001/src/core/SkPictureRecord.h
File src/core/SkPictureRecord.h (right):

https://codereview.chromium.org/153883002/diff/180001/src/core/SkPictureRecor...
src/core/SkPictureRecord.h:167: int addPathToHeap(const SkPath& path);
Right now they still do.

https://codereview.chromium.org/153883002/diff/180001/src/core/SkPictureRecor...
src/core/SkPictureRecord.h:248: void saveImpl(SaveFlags flags);
On 2014/02/04 16:33:58, mtklein wrote:
> given the way they're used,  maybe rename to recordSave, recordSaveLayer, etc.
?

Done.

[mtklein, 2014-02-04 19:09:02]

https://codereview.chromium.org/153883002/diff/180001/src/core/SkPictureRecord.h
File src/core/SkPictureRecord.h (right):

https://codereview.chromium.org/153883002/diff/180001/src/core/SkPictureRecor...
src/core/SkPictureRecord.h:167: int addPathToHeap(const SkPath& path);
On 2014/02/04 19:08:01, robertphillips wrote:
> Right now they still do.

But not addPathToHeap, right?  That's the surprising thing, that it doesn't and
the rest do.

[robertphillips, 2014-02-04 19:20:32]

https://codereview.chromium.org/153883002/diff/180001/src/core/SkPictureRecord.h
File src/core/SkPictureRecord.h (right):

https://codereview.chromium.org/153883002/diff/180001/src/core/SkPictureRecor...
src/core/SkPictureRecord.h:167: int addPathToHeap(const SkPath& path);
On 2014/02/04 19:09:02, mtklein wrote:
> On 2014/02/04 19:08:01, robertphillips wrote:
> > Right now they still do.
> 
> But not addPathToHeap, right?  That's the surprising thing, that it doesn't
and
> the rest do.

Fair enough. Done & moved to protected block.

[mtklein, 2014-02-04 19:59:52]

lgtm, modulo restoreImpl.

https://codereview.chromium.org/153883002/diff/190003/src/core/SkPictureRecord.h
File src/core/SkPictureRecord.h (right):

https://codereview.chromium.org/153883002/diff/190003/src/core/SkPictureRecor...
src/core/SkPictureRecord.h:251: void restoreImpl();
Rename this guy too?

[robertphillips, 2014-02-04 20:02:52]

https://codereview.chromium.org/153883002/diff/190003/src/core/SkPictureRecord.h
File src/core/SkPictureRecord.h (right):

https://codereview.chromium.org/153883002/diff/190003/src/core/SkPictureRecor...
src/core/SkPictureRecord.h:251: void restoreImpl();
On 2014/02/04 19:59:53, mtklein wrote:
> Rename this guy too?

Done.

[robertphillips, 2014-02-04 20:08:09]

committed as r13311