Implement multi-threaded picture playback via cloning.
The CL adds SkPicture.clone() which produces a thread-safe copy by
creating a shallow copy of the thread-safe data within the picture and
a deep copy of the data that is not (e.g. SkPaint). This implementation
re-flattens the paints when cloning instead of retaining the flattened
paints from the recording process.
Changes were also needed to various classes to ensure thread safety
Review URL: https://codereview.appspot.com/6459105
git-svn-id: http://skia.googlecode.com/svn/trunk@5335 2bbb7eff-a529-9590-31e7-b0007b416f81
diff --git a/include/core/SkColorTable.h b/include/core/SkColorTable.h
index 01d6ac1..f8d1ccf 100644
--- a/include/core/SkColorTable.h
+++ b/include/core/SkColorTable.h
@@ -70,7 +70,7 @@
the colors were changed during the lock.
*/
SkPMColor* lockColors() {
- SkDEBUGCODE(fColorLockCount += 1;)
+ SkDEBUGCODE(sk_atomic_inc(&fColorLockCount);)
return fColors;
}
/** Balancing call to lockColors(). If the colors have been changed, pass true.
diff --git a/include/core/SkInstCnt.h b/include/core/SkInstCnt.h
index 7ed866b..09ce0ca 100644
--- a/include/core/SkInstCnt.h
+++ b/include/core/SkInstCnt.h
@@ -21,6 +21,7 @@
#ifdef SK_ENABLE_INST_COUNT
#include <stdlib.h>
#include "SkTArray.h"
+#include "SkThread_platform.h"
extern bool gPrintInstCount;
@@ -50,15 +51,15 @@
gChildren = new SkTArray<PFCheckInstCnt>; \
gInited = true; \
} \
- gInstanceCount++; \
+ sk_atomic_inc(&gInstanceCount); \
} \
\
SkInstanceCountHelper(const SkInstanceCountHelper& other) { \
- gInstanceCount++; \
+ sk_atomic_inc(&gInstanceCount); \
} \
\
~SkInstanceCountHelper() { \
- gInstanceCount--; \
+ sk_atomic_dec(&gInstanceCount); \
} \
\
static int32_t gInstanceCount; \
diff --git a/include/core/SkPicture.h b/include/core/SkPicture.h
index 8a633dc..83ff725 100644
--- a/include/core/SkPicture.h
+++ b/include/core/SkPicture.h
@@ -48,6 +48,18 @@
* Swap the contents of the two pictures. Guaranteed to succeed.
*/
void swap(SkPicture& other);
+
+ /**
+ * Creates a thread-safe clone of the picture that is ready for playback.
+ */
+ SkPicture* clone() const;
+
+ /**
+ * Creates multiple thread-safe clones of this picture that are ready for
+ * playback. The resulting clones are stored in the provided array of
+ * SkPictures.
+ */
+ void clone(SkPicture* pictures, int count) const;
enum RecordingFlags {
/* This flag specifies that when clipPath() is called, the path will
diff --git a/src/core/SkColorTable.cpp b/src/core/SkColorTable.cpp
index ce143f8..2a27906 100644
--- a/src/core/SkColorTable.cpp
+++ b/src/core/SkColorTable.cpp
@@ -81,7 +81,7 @@
void SkColorTable::unlockColors(bool changed)
{
SkASSERT(fColorLockCount != 0);
- SkDEBUGCODE(fColorLockCount -= 1;)
+ SkDEBUGCODE(sk_atomic_dec(&fColorLockCount);)
if (changed)
this->inval16BitCache();
}
diff --git a/src/core/SkPath.cpp b/src/core/SkPath.cpp
index 0b1444c..dd9249d 100644
--- a/src/core/SkPath.cpp
+++ b/src/core/SkPath.cpp
@@ -457,8 +457,8 @@
SkDEBUGCODE(this->validate();)
SkASSERT(fBoundsIsDirty);
- fBoundsIsDirty = false;
fIsFinite = compute_pt_bounds(&fBounds, fPts);
+ fBoundsIsDirty = false;
}
void SkPath::setConvexity(Convexity c) {
diff --git a/src/core/SkPicture.cpp b/src/core/SkPicture.cpp
index 70e329e..13c5f6c 100644
--- a/src/core/SkPicture.cpp
+++ b/src/core/SkPicture.cpp
@@ -138,6 +138,37 @@
SkTSwap(fHeight, other.fHeight);
}
+SkPicture* SkPicture::clone() const {
+ SkPicture* clonedPicture = SkNEW(SkPicture);
+ clone(clonedPicture, 1);
+ return clonedPicture;
+}
+
+void SkPicture::clone(SkPicture* pictures, int count) const {
+ SkPictCopyInfo copyInfo;
+
+ for (int i = 0; i < count; i++) {
+ SkPicture* clone = &pictures[i];
+
+ clone->fWidth = fWidth;
+ clone->fHeight = fHeight;
+ clone->fRecord = NULL;
+
+ /* 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 (fPlayback) {
+ clone->fPlayback = SkNEW_ARGS(SkPicturePlayback, (*fPlayback, ©Info));
+ } else if (fRecord) {
+ // here we do a fake src.endRecording()
+ clone->fPlayback = SkNEW_ARGS(SkPicturePlayback, (*fRecord, true));
+ } else {
+ clone->fPlayback = NULL;
+ }
+ }
+}
+
///////////////////////////////////////////////////////////////////////////////
SkCanvas* SkPicture::beginRecording(int width, int height,
diff --git a/src/core/SkPicturePlayback.cpp b/src/core/SkPicturePlayback.cpp
index 40297dc..457b860 100644
--- a/src/core/SkPicturePlayback.cpp
+++ b/src/core/SkPicturePlayback.cpp
@@ -25,7 +25,7 @@
this->init();
}
-SkPicturePlayback::SkPicturePlayback(const SkPictureRecord& record) {
+SkPicturePlayback::SkPicturePlayback(const SkPictureRecord& record, bool deepCopy) {
#ifdef SK_DEBUG_SIZE
size_t overallBytes, bitmapBytes, matricesBytes,
paintBytes, pathBytes, pictureBytes, regionBytes;
@@ -80,20 +80,32 @@
// copy over the refcnt dictionary to our reader
record.fFlattenableHeap.setupPlaybacks();
- fBitmaps = record.fBitmapHeap.extractBitmaps();
+ fBitmaps = record.fBitmapHeap->extractBitmaps();
fMatrices = record.fMatrices.unflattenToArray();
fPaints = record.fPaints.unflattenToArray();
fRegions = record.fRegions.unflattenToArray();
- SkRefCnt_SafeAssign(fPathHeap, record.fPathHeap);
+ fBitmapHeap.reset(SkSafeRef(record.fBitmapHeap));
+ fPathHeap.reset(SkSafeRef(record.fPathHeap));
+
+ // ensure that the paths bounds are pre-computed
+ if (fPathHeap.get()) {
+ for (int i = 0; i < fPathHeap->count(); i++) {
+ (*fPathHeap)[i].updateBoundsCache();
+ }
+ }
const SkTDArray<SkPicture* >& pictures = record.getPictureRefs();
fPictureCount = pictures.count();
if (fPictureCount > 0) {
fPictureRefs = SkNEW_ARRAY(SkPicture*, fPictureCount);
for (int i = 0; i < fPictureCount; i++) {
- fPictureRefs[i] = pictures[i];
- fPictureRefs[i]->ref();
+ if (deepCopy) {
+ fPictureRefs[i] = pictures[i]->clone();
+ } else {
+ fPictureRefs[i] = pictures[i];
+ fPictureRefs[i]->ref();
+ }
}
}
@@ -119,28 +131,66 @@
#endif
}
-template <typename T> T* SafeRefReturn(T* obj) {
- if (obj) {
- obj->ref();
- }
- return obj;
-}
-
-SkPicturePlayback::SkPicturePlayback(const SkPicturePlayback& src) {
+SkPicturePlayback::SkPicturePlayback(const SkPicturePlayback& src, SkPictCopyInfo* deepCopyInfo) {
this->init();
- fPathHeap = SafeRefReturn(src.fPathHeap);
- fBitmaps = SafeRefReturn(src.fBitmaps);
- fMatrices = SafeRefReturn(src.fMatrices);
- fPaints = SafeRefReturn(src.fPaints);
- fRegions = SafeRefReturn(src.fRegions);
- fOpData = SafeRefReturn(src.fOpData);
+ fBitmapHeap.reset(SkSafeRef(src.fBitmapHeap.get()));
+ fPathHeap.reset(SkSafeRef(src.fPathHeap.get()));
+
+ fMatrices = SkSafeRef(src.fMatrices);
+ fRegions = SkSafeRef(src.fRegions);
+ fOpData = SkSafeRef(src.fOpData);
+
+ if (deepCopyInfo) {
+
+ if (src.fBitmaps) {
+ fBitmaps = SkTRefArray<SkBitmap>::Create(src.fBitmaps->begin(), src.fBitmaps->count());
+ }
+
+ if (!deepCopyInfo->initialized) {
+ /* The alternative to doing this is to have a clone method on the paint and have it make
+ * the deep copy of its internal structures as needed. The holdup to doing that is at
+ * this point we would need to pass the SkBitmapHeap so that we don't unnecessarily
+ * flatten the pixels in a bitmap shader.
+ */
+ deepCopyInfo->paintData.setCount(src.fPaints->count());
+
+ SkDEBUGCODE(int heapSize = SafeCount(fBitmapHeap.get());)
+ for (int i = 0; i < src.fPaints->count(); i++) {
+ deepCopyInfo->paintData[i] = SkFlatData::Create(&deepCopyInfo->controller,
+ &src.fPaints->at(i), 0,
+ &SkFlattenObjectProc<SkPaint>);
+ }
+ SkASSERT(SafeCount(fBitmapHeap.get()) == heapSize);
+
+ // needed to create typeface playback
+ deepCopyInfo->controller.setupPlaybacks();
+ deepCopyInfo->initialized = true;
+ }
+
+ fPaints = SkTRefArray<SkPaint>::Create(src.fPaints->count());
+ SkASSERT(deepCopyInfo->paintData.count() == src.fPaints->count());
+ for (int i = 0; i < src.fPaints->count(); i++) {
+ deepCopyInfo->paintData[i]->unflatten(&fPaints->writableAt(i),
+ &SkUnflattenObjectProc<SkPaint>,
+ deepCopyInfo->controller.getBitmapHeap(),
+ deepCopyInfo->controller.getTypefacePlayback());
+ }
+
+ } else {
+ fBitmaps = SkSafeRef(src.fBitmaps);
+ fPaints = SkSafeRef(src.fPaints);
+ }
fPictureCount = src.fPictureCount;
fPictureRefs = SkNEW_ARRAY(SkPicture*, fPictureCount);
for (int i = 0; i < fPictureCount; i++) {
- fPictureRefs[i] = src.fPictureRefs[i];
- fPictureRefs[i]->ref();
+ if (deepCopyInfo) {
+ fPictureRefs[i] = src.fPictureRefs[i]->clone();
+ } else {
+ fPictureRefs[i] = src.fPictureRefs[i];
+ fPictureRefs[i]->ref();
+ }
}
}
@@ -148,7 +198,6 @@
fBitmaps = NULL;
fMatrices = NULL;
fPaints = NULL;
- fPathHeap = NULL;
fPictureRefs = NULL;
fRegions = NULL;
fPictureCount = 0;
@@ -163,7 +212,6 @@
SkSafeUnref(fMatrices);
SkSafeUnref(fPaints);
SkSafeUnref(fRegions);
- SkSafeUnref(fPathHeap);
for (int i = 0; i < fPictureCount; i++) {
fPictureRefs[i]->unref();
@@ -179,7 +227,7 @@
SafeCount(fBitmaps), SafeCount(fBitmaps) * sizeof(SkBitmap),
SafeCount(fMatrices), SafeCount(fMatrices) * sizeof(SkMatrix),
SafeCount(fPaints), SafeCount(fPaints) * sizeof(SkPaint),
- SafeCount(fPathHeap),
+ SafeCount(fPathHeap.get()),
SafeCount(fRegions));
}
@@ -277,8 +325,8 @@
buffer.writePaint((*fPaints)[i]);
}
}
-
- if ((n = SafeCount(fPathHeap)) > 0) {
+
+ if ((n = SafeCount(fPathHeap.get())) > 0) {
writeTagSize(buffer, PICT_PATH_BUFFER_TAG, n);
fPathHeap->flatten(buffer);
}
@@ -445,7 +493,7 @@
} break;
case PICT_PATH_BUFFER_TAG:
if (size > 0) {
- fPathHeap = SkNEW_ARGS(SkPathHeap, (buffer));
+ fPathHeap.reset(SkNEW_ARGS(SkPathHeap, (buffer)));
}
break;
case PICT_REGION_BUFFER_TAG: {
diff --git a/src/core/SkPicturePlayback.h b/src/core/SkPicturePlayback.h
index 077719f..350839b 100644
--- a/src/core/SkPicturePlayback.h
+++ b/src/core/SkPicturePlayback.h
@@ -42,11 +42,23 @@
uint32_t fFlags;
};
+/**
+ * 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.
+ */
+struct SkPictCopyInfo {
+ SkPictCopyInfo() : initialized(false), controller(1024) {}
+
+ bool initialized;
+ SkChunkFlatController controller;
+ SkTDArray<SkFlatData*> paintData;
+};
+
class SkPicturePlayback {
public:
SkPicturePlayback();
- SkPicturePlayback(const SkPicturePlayback& src);
- explicit SkPicturePlayback(const SkPictureRecord& record);
+ SkPicturePlayback(const SkPicturePlayback& src, SkPictCopyInfo* deepCopyInfo = NULL);
+ explicit SkPicturePlayback(const SkPictureRecord& record, bool deepCopy = false);
SkPicturePlayback(SkStream*, const SkPictInfo&, bool* isValid);
virtual ~SkPicturePlayback();
@@ -167,7 +179,9 @@
void flattenToBuffer(SkOrderedWriteBuffer&) const;
private:
- SkPathHeap* fPathHeap; // reference counted
+ SkAutoTUnref<SkBitmapHeap> fBitmapHeap;
+ SkAutoTUnref<SkPathHeap> fPathHeap;
+
SkTRefArray<SkBitmap>* fBitmaps;
SkTRefArray<SkMatrix>* fMatrices;
SkTRefArray<SkPaint>* fPaints;
diff --git a/src/core/SkPictureRecord.cpp b/src/core/SkPictureRecord.cpp
index 3d124c4..afa2deb 100644
--- a/src/core/SkPictureRecord.cpp
+++ b/src/core/SkPictureRecord.cpp
@@ -31,12 +31,14 @@
fRestoreOffsetStack.setReserve(32);
fInitialSaveCount = kNoInitialSave;
- fFlattenableHeap.setBitmapStorage(&fBitmapHeap);
+ fBitmapHeap = SkNEW(SkBitmapHeap);
+ fFlattenableHeap.setBitmapStorage(fBitmapHeap);
fPathHeap = NULL; // lazy allocate
fFirstSavedLayerIndex = kNoSavedLayerIndex;
}
SkPictureRecord::~SkPictureRecord() {
+ SkSafeUnref(fBitmapHeap);
SkSafeUnref(fPathHeap);
fFlattenableHeap.setBitmapStorage(NULL);
fPictureRefs.unrefAll();
@@ -521,7 +523,7 @@
///////////////////////////////////////////////////////////////////////////////
void SkPictureRecord::addBitmap(const SkBitmap& bitmap) {
- addInt(fBitmapHeap.insert(bitmap));
+ addInt(fBitmapHeap->insert(bitmap));
}
void SkPictureRecord::addMatrix(const SkMatrix& matrix) {
diff --git a/src/core/SkPictureRecord.h b/src/core/SkPictureRecord.h
index a19a084..7c388a0 100644
--- a/src/core/SkPictureRecord.h
+++ b/src/core/SkPictureRecord.h
@@ -163,7 +163,7 @@
#endif
private:
- SkBitmapHeap fBitmapHeap;
+ SkBitmapHeap* fBitmapHeap;
SkChunkFlatController fFlattenableHeap;
SkMatrixDictionary fMatrices;
diff --git a/tests/CanvasTest.cpp b/tests/CanvasTest.cpp
index 6063dd9..9449bfe 100644
--- a/tests/CanvasTest.cpp
+++ b/tests/CanvasTest.cpp
@@ -654,8 +654,8 @@
CanvasTestStep* testStep) {
REPORTER_ASSERT_MESSAGE(reporter,
- referenceRecord->fBitmapHeap.count() ==
- testRecord->fBitmapHeap.count(), testStep->assertMessage());
+ referenceRecord->fBitmapHeap->count() ==
+ testRecord->fBitmapHeap->count(), testStep->assertMessage());
REPORTER_ASSERT_MESSAGE(reporter,
referenceRecord->fMatrices.count() ==
testRecord->fMatrices.count(), testStep->assertMessage());