Remove SkPicture pointer from SkPicturePlayback
This CL simplifies the relationship between SkPicture and SkPicturePlayback by moving the path heap into SkPicturePlayback and removing SkPicturePlayback's SkPicture pointer.
R=mtklein@google.com, reed@google.com
Author: robertphillips@google.com
Review URL: https://codereview.chromium.org/334493002
diff --git a/debugger/QT/SkDebuggerGUI.cpp b/debugger/QT/SkDebuggerGUI.cpp
index 0f816ee..1873afc 100644
--- a/debugger/QT/SkDebuggerGUI.cpp
+++ b/debugger/QT/SkDebuggerGUI.cpp
@@ -160,23 +160,21 @@
// offsets to individual commands.
class SkTimedPicturePlayback : public SkPicturePlayback {
public:
- static SkTimedPicturePlayback* CreateFromStream(SkPicture* picture,
- SkStream* stream, const SkPictInfo& info,
+ static SkTimedPicturePlayback* CreateFromStream(SkStream* stream, const SkPictInfo& info,
SkPicture::InstallPixelRefProc proc,
const SkTDArray<bool>& deletedCommands) {
// Mimics SkPicturePlayback::CreateFromStream
SkAutoTDelete<SkTimedPicturePlayback> playback(SkNEW_ARGS(SkTimedPicturePlayback,
- (picture, deletedCommands, info)));
- if (!playback->parseStream(picture, stream, proc)) {
+ (deletedCommands, info)));
+ if (!playback->parseStream(stream, proc)) {
return NULL; // we're invalid
}
return playback.detach();
}
- SkTimedPicturePlayback(SkPicture* picture,
- const SkTDArray<bool>& deletedCommands,
+ SkTimedPicturePlayback(const SkTDArray<bool>& deletedCommands,
const SkPictInfo& info)
- : INHERITED(picture, info)
+ : INHERITED(info)
, fSkipCommands(deletedCommands)
, fTot(0.0)
, fCurCommand(0) {
@@ -272,21 +270,20 @@
return NULL;
}
- SkTimedPicture* newPict = SkNEW_ARGS(SkTimedPicture, (NULL, info.fWidth, info.fHeight));
// Check to see if there is a playback to recreate.
if (stream->readBool()) {
SkTimedPicturePlayback* playback = SkTimedPicturePlayback::CreateFromStream(
- newPict, stream,
+ stream,
info, proc,
deletedCommands);
if (NULL == playback) {
- SkDELETE(newPict);
return NULL;
}
- newPict->fPlayback = playback;
+
+ return SkNEW_ARGS(SkTimedPicture, (playback, info.fWidth, info.fHeight));
}
- return newPict;
+ return NULL;
}
void resetTimes() { ((SkTimedPicturePlayback*) fPlayback)->resetTimes(); }
diff --git a/include/core/SkPicture.h b/include/core/SkPicture.h
index 0b6261a..fcedcef 100644
--- a/include/core/SkPicture.h
+++ b/include/core/SkPicture.h
@@ -23,7 +23,6 @@
class SkCanvas;
class SkDrawPictureCallback;
class SkData;
-class SkPathHeap;
class SkPicturePlayback;
class SkPictureRecord;
class SkStream;
@@ -286,23 +285,12 @@
// playback is unchanged.
SkPicture(SkPicturePlayback*, int width, int height);
- SkPicture(int width, int height, SkPictureRecord& record, bool deepCopyOps);
+ SkPicture(int width, int height, const SkPictureRecord& record, bool deepCopyOps);
private:
- SkAutoTUnref<SkPathHeap> fPathHeap; // reference counted
-
- const SkPath& getPath(int index) const;
- int addPathToHeap(const SkPath& path);
-
- void flattenToBuffer(SkWriteBuffer& buffer) const;
- bool parseBufferTag(SkReadBuffer& buffer, uint32_t tag, uint32_t size);
-
static void WriteTagSize(SkWriteBuffer& buffer, uint32_t tag, size_t size);
static void WriteTagSize(SkWStream* stream, uint32_t tag, size_t size);
- void initForPlayback() const;
- void dumpSize() const;
-
// An OperationList encapsulates a set of operation offsets into the picture byte
// stream along with the CTMs needed for those operation.
class OperationList : ::SkNoncopyable {
diff --git a/src/core/SkPicture.cpp b/src/core/SkPicture.cpp
index 9b71778..76c2b9d 100644
--- a/src/core/SkPicture.cpp
+++ b/src/core/SkPicture.cpp
@@ -131,18 +131,16 @@
}
SkPicture::SkPicture(int width, int height,
- SkPictureRecord& record,
+ const SkPictureRecord& record,
bool deepCopyOps)
: fWidth(width)
, fHeight(height)
, fAccelData(NULL) {
this->needsNewGenID();
- fPathHeap.reset(SkSafeRef(record.pathHeap()));
-
SkPictInfo info;
this->createHeader(&info);
- fPlayback = SkNEW_ARGS(SkPicturePlayback, (this, record, info, deepCopyOps));
+ fPlayback = SkNEW_ARGS(SkPicturePlayback, (record, info, deepCopyOps));
}
SkPicture::SkPicture(const SkPicture& src)
@@ -152,47 +150,12 @@
fWidth = src.fWidth;
fHeight = src.fHeight;
- /* We want to copy the src's playback. However, if that hasn't been built
- yet, we need to fake a call to endRecording() without actually calling
- it (since it is destructive, and we don't want to change src).
- */
if (src.fPlayback) {
- fPlayback = SkNEW_ARGS(SkPicturePlayback, (this, *src.fPlayback));
+ fPlayback = SkNEW_ARGS(SkPicturePlayback, (*src.fPlayback));
fUniqueID = src.uniqueID(); // need to call method to ensure != 0
} else {
fPlayback = NULL;
}
-
- fPathHeap.reset(SkSafeRef(src.fPathHeap.get()));
-}
-
-const SkPath& SkPicture::getPath(int index) const {
- return (*fPathHeap.get())[index];
-}
-
-int SkPicture::addPathToHeap(const SkPath& path) {
- if (NULL == fPathHeap) {
- fPathHeap.reset(SkNEW(SkPathHeap));
- }
-#ifdef SK_DEDUP_PICTURE_PATHS
- return fPathHeap->insert(path);
-#else
- return fPathHeap->append(path);
-#endif
-}
-
-void SkPicture::initForPlayback() const {
- // ensure that the paths bounds are pre-computed
- if (NULL != fPathHeap.get()) {
- for (int i = 0; i < fPathHeap->count(); i++) {
- (*fPathHeap.get())[i].updateBoundsCache();
- }
- }
-}
-
-void SkPicture::dumpSize() const {
- SkDebugf("--- picture size: paths=%d\n",
- SafeCount(fPathHeap.get()));
}
SkPicture::~SkPicture() {
@@ -206,7 +169,6 @@
SkTSwap(fAccelData, other.fAccelData);
SkTSwap(fWidth, other.fWidth);
SkTSwap(fHeight, other.fHeight);
- fPathHeap.swap(&other.fPathHeap);
}
SkPicture* SkPicture::clone() const {
@@ -273,13 +235,11 @@
copyInfo.initialized = true;
}
- clone->fPlayback = SkNEW_ARGS(SkPicturePlayback, (clone, *fPlayback, ©Info));
+ clone->fPlayback = SkNEW_ARGS(SkPicturePlayback, (*fPlayback, ©Info));
clone->fUniqueID = this->uniqueID(); // need to call method to ensure != 0
} else {
clone->fPlayback = NULL;
}
-
- clone->fPathHeap.reset(SkSafeRef(fPathHeap.get()));
}
}
@@ -389,20 +349,17 @@
return NULL;
}
- SkPicture* newPict = SkNEW_ARGS(SkPicture, (NULL, info.fWidth, info.fHeight));
-
// Check to see if there is a playback to recreate.
if (stream->readBool()) {
- SkPicturePlayback* playback = SkPicturePlayback::CreateFromStream(newPict, stream,
- info, proc);
+ SkPicturePlayback* playback = SkPicturePlayback::CreateFromStream(stream, info, proc);
if (NULL == playback) {
- SkDELETE(newPict);
return NULL;
}
- newPict->fPlayback = playback;
+
+ return SkNEW_ARGS(SkPicture, (playback, info.fWidth, info.fHeight));
}
- return newPict;
+ return NULL;
}
SkPicture* SkPicture::CreateFromBuffer(SkReadBuffer& buffer) {
@@ -412,19 +369,17 @@
return NULL;
}
- SkPicture* newPict = SkNEW_ARGS(SkPicture, (NULL, info.fWidth, info.fHeight));
-
// Check to see if there is a playback to recreate.
if (buffer.readBool()) {
- SkPicturePlayback* playback = SkPicturePlayback::CreateFromBuffer(newPict, buffer, info);
+ SkPicturePlayback* playback = SkPicturePlayback::CreateFromBuffer(buffer, info);
if (NULL == playback) {
- SkDELETE(newPict);
return NULL;
}
- newPict->fPlayback = playback;
+
+ return SkNEW_ARGS(SkPicture, (playback, info.fWidth, info.fHeight));
}
- return newPict;
+ return NULL;
}
void SkPicture::createHeader(SkPictInfo* info) const {
@@ -474,32 +429,6 @@
stream->write32(SkToU32(size));
}
-bool SkPicture::parseBufferTag(SkReadBuffer& buffer,
- uint32_t tag,
- uint32_t size) {
- switch (tag) {
- case SK_PICT_PATH_BUFFER_TAG:
- if (size > 0) {
- fPathHeap.reset(SkNEW_ARGS(SkPathHeap, (buffer)));
- }
- break;
- default:
- // The tag was invalid.
- return false;
- }
-
- return true; // success
-}
-
-void SkPicture::flattenToBuffer(SkWriteBuffer& buffer) const {
- int n;
-
- if ((n = SafeCount(fPathHeap.get())) > 0) {
- WriteTagSize(buffer, SK_PICT_PATH_BUFFER_TAG, n);
- fPathHeap->flatten(buffer);
- }
-}
-
void SkPicture::flatten(SkWriteBuffer& buffer) const {
SkPicturePlayback* playback = fPlayback;
diff --git a/src/core/SkPicturePlayback.cpp b/src/core/SkPicturePlayback.cpp
index aa2b021..75801fb 100644
--- a/src/core/SkPicturePlayback.cpp
+++ b/src/core/SkPicturePlayback.cpp
@@ -50,18 +50,24 @@
}
#endif
-SkPicturePlayback::SkPicturePlayback(const SkPicture* picture, const SkPictInfo& info)
- : fPicture(picture)
- , fInfo(info) {
+SkPicturePlayback::SkPicturePlayback(const SkPictInfo& info)
+ : fInfo(info) {
this->init();
}
-SkPicturePlayback::SkPicturePlayback(const SkPicture* picture,
- const SkPictureRecord& record,
+void SkPicturePlayback::initForPlayback() const {
+ // ensure that the paths bounds are pre-computed
+ if (NULL != fPathHeap.get()) {
+ for (int i = 0; i < fPathHeap->count(); i++) {
+ (*fPathHeap.get())[i].updateBoundsCache();
+ }
+ }
+}
+
+SkPicturePlayback::SkPicturePlayback(const SkPictureRecord& record,
const SkPictInfo& info,
bool deepCopyOps)
- : fPicture(picture)
- , fInfo(info) {
+ : fInfo(info) {
#ifdef SK_DEBUG_SIZE
size_t overallBytes, bitmapBytes, matricesBytes,
paintBytes, pathBytes, pictureBytes, regionBytes;
@@ -121,8 +127,9 @@
fPaints = record.fPaints.unflattenToArray();
fBitmapHeap.reset(SkSafeRef(record.fBitmapHeap));
+ fPathHeap.reset(SkSafeRef(record.pathHeap()));
- picture->initForPlayback();
+ this->initForPlayback();
const SkTDArray<const SkPicture* >& pictures = record.getPictureRefs();
fPictureCount = pictures.count();
@@ -156,14 +163,12 @@
#endif
}
-SkPicturePlayback::SkPicturePlayback(const SkPicture* picture,
- const SkPicturePlayback& src,
- SkPictCopyInfo* deepCopyInfo)
- : fPicture(picture)
- , fInfo(src.fInfo) {
+SkPicturePlayback::SkPicturePlayback(const SkPicturePlayback& src, SkPictCopyInfo* deepCopyInfo)
+ : fInfo(src.fInfo) {
this->init();
fBitmapHeap.reset(SkSafeRef(src.fBitmapHeap.get()));
+ fPathHeap.reset(SkSafeRef(src.fPathHeap.get()));
fOpData = SkSafeRef(src.fOpData);
@@ -254,7 +259,8 @@
fOpData->size(),
SafeCount(fBitmaps), SafeCount(fBitmaps) * sizeof(SkBitmap),
SafeCount(fPaints), SafeCount(fPaints) * sizeof(SkPaint));
- fPicture->dumpSize();
+ SkDebugf("--- picture size: paths=%d\n",
+ SafeCount(fPathHeap.get()));
}
bool SkPicturePlayback::containsBitmaps() const {
@@ -351,7 +357,10 @@
}
}
- fPicture->flattenToBuffer(buffer);
+ if ((n = SafeCount(fPathHeap.get())) > 0) {
+ SkPicture::WriteTagSize(buffer, SK_PICT_PATH_BUFFER_TAG, n);
+ fPathHeap->flatten(buffer);
+ }
}
void SkPicturePlayback::serialize(SkWStream* stream,
@@ -433,8 +442,7 @@
return rbMask;
}
-bool SkPicturePlayback::parseStreamTag(SkPicture* picture,
- SkStream* stream,
+bool SkPicturePlayback::parseStreamTag(SkStream* stream,
uint32_t tag,
uint32_t size,
SkPicture::InstallPixelRefProc proc) {
@@ -536,7 +544,7 @@
while (!buffer.eof()) {
tag = buffer.readUInt();
size = buffer.readUInt();
- if (!this->parseBufferTag(picture, buffer, tag, size)) {
+ if (!this->parseBufferTag(buffer, tag, size)) {
return false;
}
}
@@ -546,8 +554,7 @@
return true; // success
}
-bool SkPicturePlayback::parseBufferTag(SkPicture* picture,
- SkReadBuffer& buffer,
+bool SkPicturePlayback::parseBufferTag(SkReadBuffer& buffer,
uint32_t tag, uint32_t size) {
switch (tag) {
case SK_PICT_BITMAP_BUFFER_TAG: {
@@ -567,7 +574,9 @@
}
} break;
case SK_PICT_PATH_BUFFER_TAG:
- picture->parseBufferTag(buffer, tag, size);
+ if (size > 0) {
+ fPathHeap.reset(SkNEW_ARGS(SkPathHeap, (buffer)));
+ }
break;
case SK_PICT_READER_TAG: {
SkAutoMalloc storage(size);
@@ -611,32 +620,29 @@
return true; // success
}
-SkPicturePlayback* SkPicturePlayback::CreateFromStream(SkPicture* picture,
- SkStream* stream,
+SkPicturePlayback* SkPicturePlayback::CreateFromStream(SkStream* stream,
const SkPictInfo& info,
SkPicture::InstallPixelRefProc proc) {
- SkAutoTDelete<SkPicturePlayback> playback(SkNEW_ARGS(SkPicturePlayback, (picture, info)));
+ SkAutoTDelete<SkPicturePlayback> playback(SkNEW_ARGS(SkPicturePlayback, (info)));
- if (!playback->parseStream(picture, stream, proc)) {
+ if (!playback->parseStream(stream, proc)) {
return NULL;
}
return playback.detach();
}
-SkPicturePlayback* SkPicturePlayback::CreateFromBuffer(SkPicture* picture,
- SkReadBuffer& buffer,
+SkPicturePlayback* SkPicturePlayback::CreateFromBuffer(SkReadBuffer& buffer,
const SkPictInfo& info) {
- SkAutoTDelete<SkPicturePlayback> playback(SkNEW_ARGS(SkPicturePlayback, (picture, info)));
+ SkAutoTDelete<SkPicturePlayback> playback(SkNEW_ARGS(SkPicturePlayback, (info)));
buffer.setVersion(info.fVersion);
- if (!playback->parseBuffer(picture, buffer)) {
+ if (!playback->parseBuffer(buffer)) {
return NULL;
}
return playback.detach();
}
-bool SkPicturePlayback::parseStream(SkPicture* picture,
- SkStream* stream,
+bool SkPicturePlayback::parseStream(SkStream* stream,
SkPicture::InstallPixelRefProc proc) {
for (;;) {
uint32_t tag = stream->readU32();
@@ -645,14 +651,14 @@
}
uint32_t size = stream->readU32();
- if (!this->parseStreamTag(picture, stream, tag, size, proc)) {
+ if (!this->parseStreamTag(stream, tag, size, proc)) {
return false; // we're invalid
}
}
return true;
}
-bool SkPicturePlayback::parseBuffer(SkPicture* picture, SkReadBuffer& buffer) {
+bool SkPicturePlayback::parseBuffer(SkReadBuffer& buffer) {
for (;;) {
uint32_t tag = buffer.readUInt();
if (SK_PICT_EOF_TAG == tag) {
@@ -660,7 +666,7 @@
}
uint32_t size = buffer.readUInt();
- if (!this->parseBufferTag(picture, buffer, tag, size)) {
+ if (!this->parseBufferTag(buffer, tag, size)) {
return false; // we're invalid
}
}
diff --git a/src/core/SkPicturePlayback.h b/src/core/SkPicturePlayback.h
index 9751e3a..23c49cf 100644
--- a/src/core/SkPicturePlayback.h
+++ b/src/core/SkPicturePlayback.h
@@ -9,29 +9,27 @@
#ifndef SkPicturePlayback_DEFINED
#define SkPicturePlayback_DEFINED
-#include "SkPicture.h"
-#include "SkReader32.h"
-
#include "SkBitmap.h"
-#include "SkData.h"
-#include "SkMatrix.h"
-#include "SkReadBuffer.h"
-#include "SkPaint.h"
-#include "SkPath.h"
#include "SkPathHeap.h"
-#include "SkRegion.h"
-#include "SkRRect.h"
+#include "SkPicture.h"
#include "SkPictureFlat.h"
#ifdef SK_BUILD_FOR_ANDROID
#include "SkThread.h"
#endif
+class SkData;
class SkPictureRecord;
+class SkReader32;
class SkStream;
class SkWStream;
class SkBBoxHierarchy;
+class SkMatrix;
+class SkPaint;
+class SkPath;
class SkPictureStateTree;
+class SkReadBuffer;
+class SkRegion;
struct SkPictInfo {
enum Flags {
@@ -126,16 +124,13 @@
class SkPicturePlayback {
public:
- SkPicturePlayback(const SkPicture* picture, const SkPicturePlayback& src,
+ SkPicturePlayback(const SkPicturePlayback& src,
SkPictCopyInfo* deepCopyInfo = NULL);
- SkPicturePlayback(const SkPicture* picture, const SkPictureRecord& record,
- const SkPictInfo&, bool deepCopyOps);
- static SkPicturePlayback* CreateFromStream(SkPicture* picture,
- SkStream*,
+ SkPicturePlayback(const SkPictureRecord& record, const SkPictInfo&, bool deepCopyOps);
+ static SkPicturePlayback* CreateFromStream(SkStream*,
const SkPictInfo&,
SkPicture::InstallPixelRefProc);
- static SkPicturePlayback* CreateFromBuffer(SkPicture* picture,
- SkReadBuffer&,
+ static SkPicturePlayback* CreateFromBuffer(SkReadBuffer&,
const SkPictInfo&);
virtual ~SkPicturePlayback();
@@ -163,10 +158,10 @@
void resetOpID() { fCurOffset = 0; }
protected:
- explicit SkPicturePlayback(const SkPicture* picture, const SkPictInfo& info);
+ explicit SkPicturePlayback(const SkPictInfo& info);
- bool parseStream(SkPicture* picture, SkStream*, SkPicture::InstallPixelRefProc);
- bool parseBuffer(SkPicture* picture, SkReadBuffer& buffer);
+ bool parseStream(SkStream*, SkPicture::InstallPixelRefProc);
+ bool parseBuffer(SkReadBuffer& buffer);
#ifdef SK_DEVELOPER
virtual bool preDraw(int opIndex, int type);
virtual void postDraw(int opIndex);
@@ -197,7 +192,8 @@
}
const SkPath& getPath(SkReader32& reader) {
- return fPicture->getPath(reader.readInt() - 1);
+ int index = reader.readInt() - 1;
+ return (*fPathHeap.get())[index];
}
const SkPicture* getPicture(SkReader32& reader) {
@@ -277,18 +273,14 @@
#endif
private: // these help us with reading/writing
- bool parseStreamTag(SkPicture* picture, SkStream*, uint32_t tag, uint32_t size,
- SkPicture::InstallPixelRefProc);
- bool parseBufferTag(SkPicture* picture, SkReadBuffer&, uint32_t tag, uint32_t size);
+ bool parseStreamTag(SkStream*, uint32_t tag, uint32_t size, SkPicture::InstallPixelRefProc);
+ bool parseBufferTag(SkReadBuffer&, uint32_t tag, uint32_t size);
void flattenToBuffer(SkWriteBuffer&) const;
private:
friend class SkPicture;
friend class SkGpuDevice; // for access to setDrawLimits & setReplacements
- // The picture that owns this SkPicturePlayback object
- const SkPicture* fPicture;
-
// Only used by getBitmap() if the passed in index is SkBitmapHeap::INVALID_SLOT. This empty
// bitmap allows playback to draw nothing and move on.
SkBitmap fBadBitmap;
@@ -300,6 +292,8 @@
SkData* fOpData; // opcodes and parameters
+ SkAutoTUnref<const SkPathHeap> fPathHeap; // reference counted
+
const SkPicture** fPictureRefs;
int fPictureCount;
@@ -403,6 +397,8 @@
static void WriteFactories(SkWStream* stream, const SkFactorySet& rec);
static void WriteTypefaces(SkWStream* stream, const SkRefCntSet& rec);
+ void initForPlayback() const;
+
#ifdef SK_BUILD_FOR_ANDROID
SkMutex fDrawMutex;
bool fAbortCurrentPlayback;
diff --git a/src/core/SkPictureRecord.h b/src/core/SkPictureRecord.h
index b4920e3..7e5c5c6 100644
--- a/src/core/SkPictureRecord.h
+++ b/src/core/SkPictureRecord.h
@@ -88,8 +88,8 @@
return fWriter.snapshotAsData();
}
- SkPathHeap* pathHeap() {
- return fPathHeap;
+ const SkPathHeap* pathHeap() const {
+ return fPathHeap.get();
}
const SkPictureContentInfo& contentInfo() const {