Remove picture pre-allocation from SkPictureRecorder

Remove picture pre-allocation from SkPictureRecorder


This CL improves the separation of the SkPicture and SkPictureRecord classes. It delays creation of the SkPicture (in SkPictureRecorder) until recording is actually completed. To accomplish this the SkRecord-derived classes now get SkPathHeap and SkPictureContentInfo members that are absorbed by the SkPicture when it is constructed.

As an ancillary change, this CL also moves the SkPictureContentInfo object from SkPicture to SkPicturePlayback. This is intended to centralize all the data in the SkPicturePlayback object.

Committed: https://skia.googlesource.com/skia/+/0bdbea75ff1a6f3c313c18cab0139728967cb93e

[reed1, 2014-06-11 15:14:53]

rubberstamp lgtm -- deferring to mtklein

[mtklein, 2014-06-11 15:30:22]

https://codereview.chromium.org/324293004/diff/20001/include/core/SkPicture.h
File include/core/SkPicture.h (right):

https://codereview.chromium.org/324293004/diff/20001/include/core/SkPicture.h...
include/core/SkPicture.h:289: SkPicture(int width, int height, SkPictureRecord&
record, bool deepCopyOps);
is const SkPictureRecord& possible here?  possible if we move path heap to
picture playback too?

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

https://codereview.chromium.org/324293004/diff/20001/src/core/SkPicture.cpp#n...
src/core/SkPicture.cpp:141: fPathHeap.reset(SkSafeRef(record.pathHeap()));
Is it just me or is it weird to have the path heap here with everything else
part of playback?  Can that move there as a const SkPathHeap* ?

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

https://codereview.chromium.org/324293004/diff/20001/src/core/SkPictureRecord...
src/core/SkPictureRecord.h:78: this->validate(fWriter.bytesWritten(), 0);
Isn't this a tautology?  It expands to SkASSERT(fWritter.bytesWritten() ==
fWriter.bytesWritten() + 0);

[mtklein, 2014-06-11 15:31:49]

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

https://codereview.chromium.org/324293004/diff/20001/src/core/SkPicture.cpp#n...
src/core/SkPicture.cpp:136: : fAccelData(NULL)
GCC whined at me that fWidth and fHeight should come first, then fAccelData.

[robertphillips, 2014-06-11 17:06:50]

https://codereview.chromium.org/324293004/diff/20001/include/core/SkPicture.h
File include/core/SkPicture.h (right):

https://codereview.chromium.org/324293004/diff/20001/include/core/SkPicture.h...
include/core/SkPicture.h:289: SkPicture(int width, int height, SkPictureRecord&
record, bool deepCopyOps);
I want to move the path heap to SkPicturePlayback in a separate CL. After that
is complete then we could make this const.

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

https://codereview.chromium.org/324293004/diff/20001/src/core/SkPicture.cpp#n...
src/core/SkPicture.cpp:136: : fAccelData(NULL)
On 2014/06/11 15:31:49, mtklein wrote:
> GCC whined at me that fWidth and fHeight should come first, then fAccelData.

Done.

https://codereview.chromium.org/324293004/diff/20001/src/core/SkPicture.cpp#n...
src/core/SkPicture.cpp:141: fPathHeap.reset(SkSafeRef(record.pathHeap()));
Yep - that is probably my next move (i.e., moving pathheap to
SkPicturePlayback).

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

https://codereview.chromium.org/324293004/diff/20001/src/core/SkPictureRecord...
src/core/SkPictureRecord.h:78: this->validate(fWriter.bytesWritten(), 0);
It expands to more if one defines SK_DEBUG_VALIDATE

[mtklein, 2014-06-11 18:21:18]

sgtm, lgtm

[robertphillips, 2014-06-11 18:23:10]

The CQ bit was checked by robertphillips@google.com

[commit-bot: I haz the power, 2014-06-11 18:23:57]

CQ is trying da patch. Follow status at

https://skia-tree-status.appspot.com/cq/robertphillips@google.com/324293004/4...

[commit-bot: I haz the power, 2014-06-11 18:38:01]

Change committed as 0bdbea75ff1a6f3c313c18cab0139728967cb93e