From e26e65e8f831f7a5626c92d11bbb8c2cec1f70de Mon Sep 17 00:00:00 2001 From: robertphillips Date: Thu, 12 Jun 2014 05:51:22 -0700 Subject: [PATCH] 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 --- debugger/QT/SkDebuggerGUI.cpp | 21 ++++---- include/core/SkPicture.h | 14 +---- src/core/SkPicture.cpp | 95 +++++----------------------------- src/core/SkPicturePlayback.cpp | 74 ++++++++++++++------------ src/core/SkPicturePlayback.h | 50 ++++++++---------- src/core/SkPictureRecord.h | 4 +- 6 files changed, 87 insertions(+), 171 deletions(-) diff --git a/debugger/QT/SkDebuggerGUI.cpp b/debugger/QT/SkDebuggerGUI.cpp index 0f816eec7..1873afca6 100644 --- a/debugger/QT/SkDebuggerGUI.cpp +++ b/debugger/QT/SkDebuggerGUI.cpp @@ -160,23 +160,21 @@ void SkDebuggerGUI::showDeletes() { // 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& deletedCommands) { // Mimics SkPicturePlayback::CreateFromStream SkAutoTDelete 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& deletedCommands, + SkTimedPicturePlayback(const SkTDArray& deletedCommands, const SkPictInfo& info) - : INHERITED(picture, info) + : INHERITED(info) , fSkipCommands(deletedCommands) , fTot(0.0) , fCurCommand(0) { @@ -272,21 +270,20 @@ public: 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 0b6261af4..fcedcefb1 100644 --- a/include/core/SkPicture.h +++ b/include/core/SkPicture.h @@ -23,7 +23,6 @@ class SkBBoxHierarchy; class SkCanvas; class SkDrawPictureCallback; class SkData; -class SkPathHeap; class SkPicturePlayback; class SkPictureRecord; class SkStream; @@ -286,23 +285,12 @@ protected: // 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 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 9b7177870..76c2b9def 100644 --- a/src/core/SkPicture.cpp +++ b/src/core/SkPicture.cpp @@ -131,18 +131,16 @@ SkPicture::SkPicture() } 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 @@ SkPicture::SkPicture(const SkPicture& src) 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 @@ void SkPicture::swap(SkPicture& other) { SkTSwap(fAccelData, other.fAccelData); SkTSwap(fWidth, other.fWidth); SkTSwap(fHeight, other.fHeight); - fPathHeap.swap(&other.fPathHeap); } SkPicture* SkPicture::clone() const { @@ -273,13 +235,11 @@ void SkPicture::clone(SkPicture* pictures, int count) const { 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 @@ SkPicture* SkPicture::CreateFromStream(SkStream* stream, InstallPixelRefProc pro 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 @@ SkPicture* SkPicture::CreateFromBuffer(SkReadBuffer& buffer) { 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 @@ void SkPicture::WriteTagSize(SkWStream* stream, uint32_t tag, size_t size) { 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 aa2b02192..75801fb96 100644 --- a/src/core/SkPicturePlayback.cpp +++ b/src/core/SkPicturePlayback.cpp @@ -50,18 +50,24 @@ void SkPicturePlayback::PlaybackReplacements::validate() const { } #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 @@ SkPicturePlayback::SkPicturePlayback(const SkPicture* picture, fPaints = record.fPaints.unflattenToArray(); fBitmapHeap.reset(SkSafeRef(record.fBitmapHeap)); + fPathHeap.reset(SkSafeRef(record.pathHeap())); - picture->initForPlayback(); + this->initForPlayback(); const SkTDArray& pictures = record.getPictureRefs(); fPictureCount = pictures.count(); @@ -156,14 +163,12 @@ SkPicturePlayback::SkPicturePlayback(const SkPicture* picture, #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 @@ void SkPicturePlayback::dumpSize() const { 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 @@ void SkPicturePlayback::flattenToBuffer(SkWriteBuffer& buffer) const { } } - 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 @@ static uint32_t pictInfoFlagsToReadBufferFlags(uint32_t pictInfoFlags) { 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 @@ bool SkPicturePlayback::parseStreamTag(SkPicture* picture, 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 @@ bool SkPicturePlayback::parseStreamTag(SkPicture* picture, 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 @@ bool SkPicturePlayback::parseBufferTag(SkPicture* picture, } } 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 @@ bool SkPicturePlayback::parseBufferTag(SkPicture* picture, return true; // success } -SkPicturePlayback* SkPicturePlayback::CreateFromStream(SkPicture* picture, - SkStream* stream, +SkPicturePlayback* SkPicturePlayback::CreateFromStream(SkStream* stream, const SkPictInfo& info, SkPicture::InstallPixelRefProc proc) { - SkAutoTDelete playback(SkNEW_ARGS(SkPicturePlayback, (picture, info))); + SkAutoTDelete 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 playback(SkNEW_ARGS(SkPicturePlayback, (picture, info))); + SkAutoTDelete 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 @@ bool SkPicturePlayback::parseStream(SkPicture* picture, } 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 @@ bool SkPicturePlayback::parseBuffer(SkPicture* picture, SkReadBuffer& buffer) { } 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 9751e3a5e..23c49cfe7 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 @@ struct SkPictCopyInfo { 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 @@ public: 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 @@ private: } 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 @@ public: #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 @@ private: SkData* fOpData; // opcodes and parameters + SkAutoTUnref fPathHeap; // reference counted + const SkPicture** fPictureRefs; int fPictureCount; @@ -403,6 +397,8 @@ private: 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 b4920e37b..7e5c5c641 100644 --- a/src/core/SkPictureRecord.h +++ b/src/core/SkPictureRecord.h @@ -88,8 +88,8 @@ public: return fWriter.snapshotAsData(); } - SkPathHeap* pathHeap() { - return fPathHeap; + const SkPathHeap* pathHeap() const { + return fPathHeap.get(); } const SkPictureContentInfo& contentInfo() const {