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.
R=mtklein@google.com, reed@google.com
Author: robertphillips@google.com
Review URL: https://codereview.chromium.org/324293004
diff --git a/src/core/SkBBoxHierarchyRecord.cpp b/src/core/SkBBoxHierarchyRecord.cpp
index d6fbd21..96e6cdf 100644
--- a/src/core/SkBBoxHierarchyRecord.cpp
+++ b/src/core/SkBBoxHierarchyRecord.cpp
@@ -9,11 +9,10 @@
#include "SkBBoxHierarchyRecord.h"
#include "SkPictureStateTree.h"
-SkBBoxHierarchyRecord::SkBBoxHierarchyRecord(SkPicture* picture,
- const SkISize& size,
+SkBBoxHierarchyRecord::SkBBoxHierarchyRecord(const SkISize& size,
uint32_t recordFlags,
SkBBoxHierarchy* h)
- : INHERITED(picture, size, recordFlags) {
+ : INHERITED(size, recordFlags) {
fStateTree = SkNEW(SkPictureStateTree);
fBoundingHierarchy = h;
fBoundingHierarchy->ref();
diff --git a/src/core/SkBBoxHierarchyRecord.h b/src/core/SkBBoxHierarchyRecord.h
index f6fac18..51fce0d 100644
--- a/src/core/SkBBoxHierarchyRecord.h
+++ b/src/core/SkBBoxHierarchyRecord.h
@@ -19,8 +19,7 @@
class SkBBoxHierarchyRecord : public SkBBoxRecord, public SkBBoxHierarchyClient {
public:
/** This will take a ref of h */
- SkBBoxHierarchyRecord(SkPicture* picture, const SkISize& size,
- uint32_t recordFlags, SkBBoxHierarchy* h);
+ SkBBoxHierarchyRecord(const SkISize& size, uint32_t recordFlags, SkBBoxHierarchy* h);
virtual void handleBBox(const SkRect& bounds) SK_OVERRIDE;
diff --git a/src/core/SkBBoxRecord.h b/src/core/SkBBoxRecord.h
index 123a91f..f3d72b0 100644
--- a/src/core/SkBBoxRecord.h
+++ b/src/core/SkBBoxRecord.h
@@ -19,8 +19,8 @@
class SkBBoxRecord : public SkPictureRecord {
public:
- SkBBoxRecord(SkPicture* picture, const SkISize& size, uint32_t recordFlags)
- : INHERITED(picture, size, recordFlags) {
+ SkBBoxRecord(const SkISize& size, uint32_t recordFlags)
+ : INHERITED(size, recordFlags) {
}
virtual ~SkBBoxRecord() { }
diff --git a/src/core/SkPicture.cpp b/src/core/SkPicture.cpp
index 7120f61..9b71778 100644
--- a/src/core/SkPicture.cpp
+++ b/src/core/SkPicture.cpp
@@ -130,26 +130,24 @@
fWidth = fHeight = 0;
}
-// This method makes a SkPicturePlayback object from an in-progress recording.
-// Unfortunately, it does not include the restoreToCount of a real endRecording
-// call.
-SkPicturePlayback* SkPicture::FakeEndRecording(const SkPicture* resourceSrc,
- const SkPictureRecord& record) {
- SkPictInfo info;
- resourceSrc->createHeader(&info);
+SkPicture::SkPicture(int width, int height,
+ SkPictureRecord& record,
+ bool deepCopyOps)
+ : fWidth(width)
+ , fHeight(height)
+ , fAccelData(NULL) {
+ this->needsNewGenID();
- // FakeEndRecording is only called from partialReplay. For that use case
- // we cannot be certain that the next call to SkWriter32::overwriteTAt
- // will be preceded by an append (i.e., that the required copy on write
- // will occur). In this case just force a deep copy of the operations.
- const bool deepCopyOps = true;
- return SkNEW_ARGS(SkPicturePlayback, (resourceSrc, record, info, deepCopyOps));
+ fPathHeap.reset(SkSafeRef(record.pathHeap()));
+
+ SkPictInfo info;
+ this->createHeader(&info);
+ fPlayback = SkNEW_ARGS(SkPicturePlayback, (this, record, info, deepCopyOps));
}
SkPicture::SkPicture(const SkPicture& src)
: INHERITED()
- , fAccelData(NULL)
- , fContentInfo(src.fContentInfo) {
+ , fAccelData(NULL) {
this->needsNewGenID();
fWidth = src.fWidth;
fHeight = src.fHeight;
@@ -209,7 +207,6 @@
SkTSwap(fWidth, other.fWidth);
SkTSwap(fHeight, other.fHeight);
fPathHeap.swap(&other.fPathHeap);
- fContentInfo.swap(&other.fContentInfo);
}
SkPicture* SkPicture::clone() const {
@@ -228,7 +225,6 @@
clone->fWidth = fWidth;
clone->fHeight = fHeight;
SkDELETE(clone->fPlayback);
- clone->fContentInfo.set(fContentInfo);
/* 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
@@ -524,23 +520,14 @@
#if SK_SUPPORT_GPU
bool SkPicture::suitableForGpuRasterization(GrContext* context, const char **reason) const {
- // TODO: the heuristic used here needs to be refined
- static const int kNumPaintWithPathEffectUsesTol = 1;
- static const int kNumAAConcavePaths = 5;
-
- SkASSERT(this->numAAHairlineConcavePaths() <= this->numAAConcavePaths());
-
- bool ret = this->numPaintWithPathEffectUses() < kNumPaintWithPathEffectUsesTol &&
- (this->numAAConcavePaths()-this->numAAHairlineConcavePaths()) < kNumAAConcavePaths;
- if (!ret && reason) {
- if (this->numPaintWithPathEffectUses() >= kNumPaintWithPathEffectUsesTol)
- *reason = "Too many path effects.";
- else if ((this->numAAConcavePaths()-this->numAAHairlineConcavePaths()) >= kNumAAConcavePaths)
- *reason = "Too many anti-aliased concave paths.";
- else
- *reason = "Unknown reason for GPU unsuitability.";
+ if (NULL == fPlayback) {
+ if (NULL != reason) {
+ *reason = "Missing playback object.";
+ }
+ return false;
}
- return ret;
+
+ return fPlayback->suitableForGpuRasterization(context, reason);
}
#endif
diff --git a/src/core/SkPicturePlayback.cpp b/src/core/SkPicturePlayback.cpp
index 3f83b6a..aa2b021 100644
--- a/src/core/SkPicturePlayback.cpp
+++ b/src/core/SkPicturePlayback.cpp
@@ -99,26 +99,16 @@
record.dumpPaints();
#endif
- record.validate(record.writeStream().bytesWritten(), 0);
- const SkWriter32& writer = record.writeStream();
this->init();
- SkASSERT(!fOpData);
- if (writer.bytesWritten() == 0) {
- fOpData = SkData::NewEmpty();
- return;
- }
- if (deepCopyOps) {
- // Don't try to do anything clever w.r.t. copy on write
- fOpData = SkData::NewWithCopy(writer.contiguousArray(), writer.bytesWritten());
- } else {
- fOpData = writer.snapshotAsData();
- }
+
+ fOpData = record.opData(deepCopyOps);
fBoundingHierarchy = record.fBoundingHierarchy;
fStateTree = record.fStateTree;
SkSafeRef(fBoundingHierarchy);
SkSafeRef(fStateTree);
+ fContentInfo.set(record.fContentInfo);
if (NULL != fBoundingHierarchy) {
fBoundingHierarchy->flushDeferredInserts();
@@ -166,7 +156,8 @@
#endif
}
-SkPicturePlayback::SkPicturePlayback(const SkPicture* picture, const SkPicturePlayback& src,
+SkPicturePlayback::SkPicturePlayback(const SkPicture* picture,
+ const SkPicturePlayback& src,
SkPictCopyInfo* deepCopyInfo)
: fPicture(picture)
, fInfo(src.fInfo) {
@@ -178,6 +169,7 @@
fBoundingHierarchy = src.fBoundingHierarchy;
fStateTree = src.fStateTree;
+ fContentInfo.set(src.fContentInfo);
SkSafeRef(fBoundingHierarchy);
SkSafeRef(fStateTree);
@@ -1335,6 +1327,30 @@
// this->dumpSize();
}
+
+#if SK_SUPPORT_GPU
+bool SkPicturePlayback::suitableForGpuRasterization(GrContext* context, const char **reason) const {
+ // TODO: the heuristic used here needs to be refined
+ static const int kNumPaintWithPathEffectUsesTol = 1;
+ static const int kNumAAConcavePaths = 5;
+
+ SkASSERT(fContentInfo.numAAHairlineConcavePaths() <= fContentInfo.numAAConcavePaths());
+
+ bool ret = fContentInfo.numPaintWithPathEffectUses() < kNumPaintWithPathEffectUsesTol &&
+ (fContentInfo.numAAConcavePaths() - fContentInfo.numAAHairlineConcavePaths())
+ < kNumAAConcavePaths;
+ if (!ret && NULL != reason) {
+ if (fContentInfo.numPaintWithPathEffectUses() >= kNumPaintWithPathEffectUsesTol)
+ *reason = "Too many path effects.";
+ else if ((fContentInfo.numAAConcavePaths() - fContentInfo.numAAHairlineConcavePaths())
+ >= kNumAAConcavePaths)
+ *reason = "Too many anti-aliased concave paths.";
+ else
+ *reason = "Unknown reason for GPU unsuitability.";
+ }
+ return ret;
+}
+#endif
///////////////////////////////////////////////////////////////////////////////
#ifdef SK_DEBUG_SIZE
diff --git a/src/core/SkPicturePlayback.h b/src/core/SkPicturePlayback.h
index 9d4c996..9751e3a 100644
--- a/src/core/SkPicturePlayback.h
+++ b/src/core/SkPicturePlayback.h
@@ -62,6 +62,56 @@
// Always write this guy last (with no length field afterwards)
#define SK_PICT_EOF_TAG SkSetFourByteTag('e', 'o', 'f', ' ')
+// SkPictureContentInfo is not serialized! It is intended solely for use
+// with suitableForGpuRasterization.
+class SkPictureContentInfo {
+public:
+ SkPictureContentInfo() { this->reset(); }
+
+ SkPictureContentInfo(const SkPictureContentInfo& src) { this->set(src); }
+
+ void set(const SkPictureContentInfo& src) {
+ fNumPaintWithPathEffectUses = src.fNumPaintWithPathEffectUses;
+ fNumAAConcavePaths = src.fNumAAConcavePaths;
+ fNumAAHairlineConcavePaths = src.fNumAAHairlineConcavePaths;
+ }
+
+ void reset() {
+ fNumPaintWithPathEffectUses = 0;
+ fNumAAConcavePaths = 0;
+ fNumAAHairlineConcavePaths = 0;
+ }
+
+ void swap(SkPictureContentInfo* other) {
+ SkTSwap(fNumPaintWithPathEffectUses, other->fNumPaintWithPathEffectUses);
+ SkTSwap(fNumAAConcavePaths, other->fNumAAConcavePaths);
+ SkTSwap(fNumAAHairlineConcavePaths, other->fNumAAHairlineConcavePaths);
+ }
+
+ void incPaintWithPathEffectUses() { ++fNumPaintWithPathEffectUses; }
+ int numPaintWithPathEffectUses() const { return fNumPaintWithPathEffectUses; }
+
+ void incAAConcavePaths() { ++fNumAAConcavePaths; }
+ int numAAConcavePaths() const { return fNumAAConcavePaths; }
+
+ void incAAHairlineConcavePaths() {
+ ++fNumAAHairlineConcavePaths;
+ SkASSERT(fNumAAHairlineConcavePaths <= fNumAAConcavePaths);
+ }
+ int numAAHairlineConcavePaths() const { return fNumAAHairlineConcavePaths; }
+
+private:
+ // This field is incremented every time a paint with a path effect is
+ // used (i.e., it is not a de-duplicated count)
+ int fNumPaintWithPathEffectUses;
+ // This field is incremented every time an anti-aliased drawPath call is
+ // issued with a concave path
+ int fNumAAConcavePaths;
+ // This field is incremented every time a drawPath call is
+ // issued for a hairline stroked concave path.
+ int fNumAAHairlineConcavePaths;
+};
+
/**
* Container for data that is needed to deep copy a SkPicture. The container
* enables the data to be generated once and reused for subsequent copies.
@@ -78,8 +128,8 @@
public:
SkPicturePlayback(const SkPicture* picture, const SkPicturePlayback& src,
SkPictCopyInfo* deepCopyInfo = NULL);
- SkPicturePlayback(const SkPicture* picture, const SkPictureRecord& record, const SkPictInfo&,
- bool deepCopyOps);
+ SkPicturePlayback(const SkPicture* picture, const SkPictureRecord& record,
+ const SkPictInfo&, bool deepCopyOps);
static SkPicturePlayback* CreateFromStream(SkPicture* picture,
SkStream*,
const SkPictInfo&,
@@ -222,6 +272,10 @@
void dump() const;
#endif
+#if SK_SUPPORT_GPU
+ bool suitableForGpuRasterization(GrContext* context, const char **reason) const;
+#endif
+
private: // these help us with reading/writing
bool parseStreamTag(SkPicture* picture, SkStream*, uint32_t tag, uint32_t size,
SkPicture::InstallPixelRefProc);
@@ -252,6 +306,8 @@
SkBBoxHierarchy* fBoundingHierarchy;
SkPictureStateTree* fStateTree;
+ SkPictureContentInfo fContentInfo;
+
// Limit the opcode playback to be between the offsets 'start' and 'stop'.
// The opcode at 'start' should be a saveLayer while the opcode at
// 'stop' should be a restore. Neither of those commands will be issued.
diff --git a/src/core/SkPictureRecord.cpp b/src/core/SkPictureRecord.cpp
index 1a1aecc..5d2eca7 100644
--- a/src/core/SkPictureRecord.cpp
+++ b/src/core/SkPictureRecord.cpp
@@ -36,7 +36,7 @@
static const uint32_t kSaveLayerNoBoundsSize = 4 * kUInt32Size;
static const uint32_t kSaveLayerWithBoundsSize = 4 * kUInt32Size + sizeof(SkRect);
-SkPictureRecord::SkPictureRecord(SkPicture* picture, const SkISize& dimensions, uint32_t flags)
+SkPictureRecord::SkPictureRecord(const SkISize& dimensions, uint32_t flags)
: INHERITED(dimensions.width(), dimensions.height())
, fBoundingHierarchy(NULL)
, fStateTree(NULL)
@@ -49,7 +49,6 @@
fPointWrites = fRectWrites = fTextWrites = 0;
#endif
- fPicture = picture;
fBitmapHeap = SkNEW(SkBitmapHeap);
fFlattenableHeap.setBitmapStorage(fBitmapHeap);
@@ -1066,11 +1065,11 @@
void SkPictureRecord::drawPath(const SkPath& path, const SkPaint& paint) {
if (paint.isAntiAlias() && !path.isConvex()) {
- fPicture->incAAConcavePaths();
+ fContentInfo.incAAConcavePaths();
if (SkPaint::kStroke_Style == paint.getStyle() &&
0 == paint.getStrokeWidth()) {
- fPicture->incAAHairlineConcavePaths();
+ fContentInfo.incAAHairlineConcavePaths();
}
}
@@ -1589,7 +1588,7 @@
const SkFlatData* SkPictureRecord::addPaintPtr(const SkPaint* paint) {
if (NULL != paint && NULL != paint->getPathEffect()) {
- fPicture->incPaintWithPathEffectUses();
+ fContentInfo.incPaintWithPathEffectUses();
}
const SkFlatData* data = paint ? getFlatPaintData(*paint) : NULL;
@@ -1603,7 +1602,14 @@
}
int SkPictureRecord::addPathToHeap(const SkPath& path) {
- return fPicture->addPathToHeap(path);
+ if (NULL == fPathHeap) {
+ fPathHeap.reset(SkNEW(SkPathHeap));
+ }
+#ifdef SK_DEDUP_PICTURE_PATHS
+ return fPathHeap->insert(path);
+#else
+ return fPathHeap->append(path);
+#endif
}
void SkPictureRecord::addPath(const SkPath& path) {
diff --git a/src/core/SkPictureRecord.h b/src/core/SkPictureRecord.h
index d6cdf05..b4920e3 100644
--- a/src/core/SkPictureRecord.h
+++ b/src/core/SkPictureRecord.h
@@ -15,6 +15,7 @@
#endif
#include "SkPathHeap.h"
#include "SkPicture.h"
+#include "SkPicturePlayback.h"
#include "SkPictureFlat.h"
#include "SkTemplates.h"
#include "SkWriter32.h"
@@ -33,7 +34,7 @@
class SkPictureRecord : public SkCanvas {
public:
- SkPictureRecord(SkPicture* picture, const SkISize& dimensions, uint32_t recordFlags);
+ SkPictureRecord(const SkISize& dimensions, uint32_t recordFlags);
virtual ~SkPictureRecord();
virtual void clear(SkColor) SK_OVERRIDE;
@@ -73,6 +74,28 @@
return fPictureRefs;
}
+ SkData* opData(bool deepCopy) const {
+ this->validate(fWriter.bytesWritten(), 0);
+
+ if (fWriter.bytesWritten() == 0) {
+ return SkData::NewEmpty();
+ }
+
+ if (deepCopy) {
+ return SkData::NewWithCopy(fWriter.contiguousArray(), fWriter.bytesWritten());
+ }
+
+ return fWriter.snapshotAsData();
+ }
+
+ SkPathHeap* pathHeap() {
+ return fPathHeap;
+ }
+
+ const SkPictureContentInfo& contentInfo() const {
+ return fContentInfo;
+ }
+
void setFlags(uint32_t recordFlags) {
fRecordFlags = recordFlags;
}
@@ -283,12 +306,12 @@
SkBitmapHeap* fBitmapHeap;
private:
- // The owning SkPicture
- SkPicture* fPicture;
-
friend class MatrixClipState; // for access to *Impl methods
friend class SkMatrixClipStateMgr; // for access to *Impl methods
+ SkPictureContentInfo fContentInfo;
+ SkAutoTUnref<SkPathHeap> fPathHeap;
+
SkChunkFlatController fFlattenableHeap;
SkPaintDictionary fPaints;
diff --git a/src/core/SkPictureRecorder.cpp b/src/core/SkPictureRecorder.cpp
index 863dbcd..3abd08e 100644
--- a/src/core/SkPictureRecorder.cpp
+++ b/src/core/SkPictureRecorder.cpp
@@ -18,20 +18,19 @@
SkCanvas* SkPictureRecorder::beginRecording(int width, int height,
SkBBHFactory* bbhFactory /* = NULL */,
uint32_t recordFlags /* = 0 */) {
- SkSafeSetNull(fCanvas);
- fPicture.reset(SkNEW(SkPicture));
+ SkSafeSetNull(fCanvas); // terminate any prior recording(s)
- fPicture->fWidth = width;
- fPicture->fHeight = height;
+ fWidth = width;
+ fHeight = height;
const SkISize size = SkISize::Make(width, height);
if (NULL != bbhFactory) {
SkAutoTUnref<SkBBoxHierarchy> tree((*bbhFactory)(width, height));
SkASSERT(NULL != tree);
- fCanvas = SkNEW_ARGS(SkBBoxHierarchyRecord, (fPicture, size, recordFlags, tree.get()));
+ fCanvas = SkNEW_ARGS(SkBBoxHierarchyRecord, (size, recordFlags, tree.get()));
} else {
- fCanvas = SkNEW_ARGS(SkPictureRecord, (fPicture, size, recordFlags));
+ fCanvas = SkNEW_ARGS(SkPictureRecord, (size, recordFlags));
}
fCanvas->beginRecording();
@@ -44,23 +43,18 @@
}
SkPicture* SkPictureRecorder::endRecording() {
- if (NULL == fPicture.get()) {
+ if (NULL == fCanvas) {
return NULL;
}
- SkASSERT(NULL == fPicture->fPlayback);
- SkASSERT(NULL != fCanvas);
-
fCanvas->endRecording();
- SkPictInfo info;
- fPicture->createHeader(&info);
const bool deepCopyOps = false;
- fPicture->fPlayback = SkNEW_ARGS(SkPicturePlayback, (fPicture, *fCanvas, info, deepCopyOps));
-
+ SkAutoTUnref<SkPicture> picture(SkNEW_ARGS(SkPicture, (fWidth, fHeight,
+ *fCanvas, deepCopyOps)));
SkSafeSetNull(fCanvas);
- return fPicture.detach();
+ return picture.detach();
}
void SkPictureRecorder::internalOnly_EnableOpts(bool enableOpts) {
@@ -70,14 +64,13 @@
}
void SkPictureRecorder::partialReplay(SkCanvas* canvas) const {
- if (NULL == fPicture.get() || NULL == canvas) {
+ if (NULL == fCanvas || NULL == canvas) {
// Not recording or nothing to replay into
return;
}
- SkASSERT(NULL != fCanvas);
-
- SkAutoTDelete<SkPicturePlayback> playback(SkPicture::FakeEndRecording(fPicture.get(),
- *fCanvas));
- playback->draw(*canvas, NULL);
+ const bool deepCopyOps = true;
+ SkAutoTUnref<SkPicture> picture(SkNEW_ARGS(SkPicture, (fWidth, fHeight,
+ *fCanvas, deepCopyOps)));
+ picture->draw(canvas);
}