Check in r5063 again, along with fix for tests.
Thank you to Android build, for catching the problem, which would
show up elsewhere. Now we access entry->fStorageSlot before
deleting entry.
(Original message:)
Use the SkBitmapHeap to handle SkBitmaps in SkGPipe cross process.
Required moving the LRU handles from SkBitmapHeapEntry to LookupEntry.
Allows simplification of drawBitmap* calls in SkGPipeCanvas.
Review URL: https://codereview.appspot.com/6453113
git-svn-id: http://skia.googlecode.com/svn/trunk@5081 2bbb7eff-a529-9590-31e7-b0007b416f81
diff --git a/src/pipe/SkGPipeWrite.cpp b/src/pipe/SkGPipeWrite.cpp
index 128a458..fbb67be 100644
--- a/src/pipe/SkGPipeWrite.cpp
+++ b/src/pipe/SkGPipeWrite.cpp
@@ -163,6 +163,15 @@
if (this->needOpBytes()) {
this->writeOp(kDone_DrawOp);
this->doNotify();
+ if (shouldFlattenBitmaps(fFlags)) {
+ // In this case, a BitmapShuttle is reffed by the SharedHeap
+ // and refs this canvas. Unref the SharedHeap to end the
+ // circular reference. When shouldFlattenBitmaps is false,
+ // there is no circular reference, so the SharedHeap can be
+ // safely unreffed in the destructor.
+ fSharedHeap->unref();
+ fSharedHeap = NULL;
+ }
}
fDone = true;
}
@@ -172,11 +181,6 @@
size_t freeMemoryIfPossible(size_t bytesToFree);
size_t storageAllocatedForRecording() {
- // FIXME: This can be removed once fSharedHeap is used by cross process
- // case.
- if (NULL == fSharedHeap) {
- return 0;
- }
return fSharedHeap->bytesAllocated();
}
@@ -231,6 +235,11 @@
const SkPaint&) SK_OVERRIDE;
virtual void drawData(const void*, size_t) SK_OVERRIDE;
+ /**
+ * Flatten an SkBitmap to send to the reader, where it will be referenced
+ * according to slot.
+ */
+ bool shuttleBitmap(const SkBitmap&, int32_t slot);
private:
enum {
kNoSaveLayer = -1,
@@ -243,7 +252,7 @@
size_t fBlockSize; // amount allocated for writer
size_t fBytesNotified;
bool fDone;
- uint32_t fFlags;
+ const uint32_t fFlags;
SkRefCntSet fTypefaceSet;
@@ -273,27 +282,15 @@
// if a new SkFlatData was added when in cross process mode
void flattenFactoryNames();
- // These are only used when in cross process, but with no shared address
- // space, so bitmaps are flattened.
- FlattenableHeap fBitmapHeap;
- SkBitmapDictionary fBitmapDictionary;
- int flattenToIndex(const SkBitmap&);
-
FlattenableHeap fFlattenableHeap;
FlatDictionary fFlatDictionary;
int fCurrFlatIndex[kCount_PaintFlats];
int flattenToIndex(SkFlattenable* obj, PaintFlats);
- // Common code used by drawBitmap* when flattening.
- bool commonDrawBitmapFlatten(const SkBitmap& bm, DrawOps op, unsigned flags,
- size_t opBytesNeeded, const SkPaint* paint);
- // Common code used by drawBitmap* when storing in the heap.
- bool commonDrawBitmapHeap(const SkBitmap& bm, DrawOps op, unsigned flags,
- size_t opBytesNeeded, const SkPaint* paint);
- // Convenience type for function pointer
- typedef bool (SkGPipeCanvas::*BitmapCommonFunction)(const SkBitmap&,
- DrawOps, unsigned,
- size_t, const SkPaint*);
+ // Common code used by drawBitmap*. Behaves differently depending on the
+ // type of SkBitmapHeap being used, which is determined by the flags used.
+ bool commonDrawBitmap(const SkBitmap& bm, DrawOps op, unsigned flags,
+ size_t opBytesNeeded, const SkPaint* paint);
SkPaint fPaint;
void writePaint(const SkPaint&);
@@ -321,23 +318,20 @@
}
}
-int SkGPipeCanvas::flattenToIndex(const SkBitmap & bitmap) {
+bool SkGPipeCanvas::shuttleBitmap(const SkBitmap& bm, int32_t slot) {
SkASSERT(shouldFlattenBitmaps(fFlags));
- uint32_t flags = SkFlattenableWriteBuffer::kCrossProcess_Flag;
- bool added, replaced;
- const SkFlatData* flat = fBitmapDictionary.findAndReplace(
- bitmap, flags, fBitmapHeap.flatToReplace(), &added, &replaced);
-
- int index = flat->index();
- if (added) {
- this->flattenFactoryNames();
- size_t flatSize = flat->flatSize();
- if (this->needOpBytes(flatSize)) {
- this->writeOp(kDef_Bitmap_DrawOp, 0, index);
- fWriter.write(flat->data(), flatSize);
- }
+ SkOrderedWriteBuffer buffer(1024);
+ buffer.setNamedFactoryRecorder(fFactorySet);
+ bm.flatten(buffer);
+ this->flattenFactoryNames();
+ uint32_t size = buffer.size();
+ if (this->needOpBytes(size)) {
+ this->writeOp(kDef_Bitmap_DrawOp, 0, slot);
+ void* dst = static_cast<void*>(fWriter.reserve(size));
+ buffer.writeToMemory(dst);
+ return true;
}
- return index;
+ return false;
}
// return 0 for NULL (or unflattenable obj), or index-base-1
@@ -377,6 +371,24 @@
///////////////////////////////////////////////////////////////////////////////
+/**
+ * If SkBitmaps are to be flattened to send to the reader, this class is
+ * provided to the SkBitmapHeap to tell the SkGPipeCanvas to do so.
+ */
+class BitmapShuttle : public SkBitmapHeap::ExternalStorage {
+public:
+ BitmapShuttle(SkGPipeCanvas*);
+
+ ~BitmapShuttle();
+
+ virtual bool insert(const SkBitmap& bitmap, int32_t slot) SK_OVERRIDE;
+
+private:
+ SkGPipeCanvas* fCanvas;
+};
+
+///////////////////////////////////////////////////////////////////////////////
+
#define MIN_BLOCK_SIZE (16 * 1024)
#define BITMAPS_TO_KEEP 5
#define FLATTENABLES_TO_KEEP 10
@@ -386,8 +398,6 @@
: fFactorySet(isCrossProcess(flags) ? SkNEW(SkNamedFactorySet) : NULL)
, fWriter(*writer)
, fFlags(flags)
-, fBitmapHeap(BITMAPS_TO_KEEP, fFactorySet)
-, fBitmapDictionary(&fBitmapHeap)
, fFlattenableHeap(FLATTENABLES_TO_KEEP, fFactorySet)
, fFlatDictionary(&fFlattenableHeap) {
fController = controller;
@@ -411,10 +421,12 @@
}
if (shouldFlattenBitmaps(flags)) {
- // TODO: Use the shared heap for cross process case as well.
- fSharedHeap = NULL;
+ BitmapShuttle* shuttle = SkNEW_ARGS(BitmapShuttle, (this));
+ fSharedHeap = SkNEW_ARGS(SkBitmapHeap, (shuttle, BITMAPS_TO_KEEP));
+ shuttle->unref();
} else {
- fSharedHeap = SkNEW_ARGS(SkBitmapHeap, (5, controller->numberOfReaders()));
+ fSharedHeap = SkNEW_ARGS(SkBitmapHeap,
+ (BITMAPS_TO_KEEP, controller->numberOfReaders()));
if (this->needOpBytes(sizeof(void*))) {
this->writeOp(kShareHeap_DrawOp);
fWriter.writePtr(static_cast<void*>(fSharedHeap));
@@ -426,8 +438,6 @@
SkGPipeCanvas::~SkGPipeCanvas() {
this->finish();
SkSafeUnref(fFactorySet);
- // FIXME: This can be changed to unref() once fSharedHeap is used by cross
- // process case.
SkSafeUnref(fSharedHeap);
}
@@ -682,26 +692,10 @@
}
}
-bool SkGPipeCanvas::commonDrawBitmapFlatten(const SkBitmap& bm, DrawOps op,
- unsigned flags,
- size_t opBytesNeeded,
- const SkPaint* paint) {
- if (paint != NULL) {
- flags |= kDrawBitmap_HasPaint_DrawOpsFlag;
- this->writePaint(*paint);
- }
- int bitmapIndex = this->flattenToIndex(bm);
- if (this->needOpBytes(opBytesNeeded)) {
- this->writeOp(op, flags, bitmapIndex);
- return true;
- }
- return false;
-}
-
-bool SkGPipeCanvas::commonDrawBitmapHeap(const SkBitmap& bm, DrawOps op,
- unsigned flags,
- size_t opBytesNeeded,
- const SkPaint* paint) {
+bool SkGPipeCanvas::commonDrawBitmap(const SkBitmap& bm, DrawOps op,
+ unsigned flags,
+ size_t opBytesNeeded,
+ const SkPaint* paint) {
int32_t bitmapIndex = fSharedHeap->insert(bm);
if (SkBitmapHeap::INVALID_SLOT == bitmapIndex) {
return false;
@@ -722,11 +716,7 @@
NOTIFY_SETUP(this);
size_t opBytesNeeded = sizeof(SkScalar) * 2;
- BitmapCommonFunction bitmapCommon = shouldFlattenBitmaps(fFlags) ?
- &SkGPipeCanvas::commonDrawBitmapFlatten :
- &SkGPipeCanvas::commonDrawBitmapHeap;
-
- if ((*this.*bitmapCommon)(bm, kDrawBitmap_DrawOp, 0, opBytesNeeded, paint)) {
+ if (this->commonDrawBitmap(bm, kDrawBitmap_DrawOp, 0, opBytesNeeded, paint)) {
fWriter.writeScalar(left);
fWriter.writeScalar(top);
}
@@ -744,12 +734,8 @@
} else {
flags = 0;
}
-
- BitmapCommonFunction bitmapCommon = shouldFlattenBitmaps(fFlags) ?
- &SkGPipeCanvas::commonDrawBitmapFlatten :
- &SkGPipeCanvas::commonDrawBitmapHeap;
- if ((*this.*bitmapCommon)(bm, kDrawBitmapRect_DrawOp, flags, opBytesNeeded, paint)) {
+ if (this->commonDrawBitmap(bm, kDrawBitmapRect_DrawOp, flags, opBytesNeeded, paint)) {
if (hasSrc) {
fWriter.write32(src->fLeft);
fWriter.write32(src->fTop);
@@ -765,11 +751,7 @@
NOTIFY_SETUP(this);
size_t opBytesNeeded = matrix.writeToMemory(NULL);
- BitmapCommonFunction bitmapCommon = shouldFlattenBitmaps(fFlags) ?
- &SkGPipeCanvas::commonDrawBitmapFlatten :
- &SkGPipeCanvas::commonDrawBitmapHeap;
-
- if ((*this.*bitmapCommon)(bm, kDrawBitmapMatrix_DrawOp, 0, opBytesNeeded, paint)) {
+ if (this->commonDrawBitmap(bm, kDrawBitmapMatrix_DrawOp, 0, opBytesNeeded, paint)) {
fWriter.writeMatrix(matrix);
}
}
@@ -779,11 +761,7 @@
NOTIFY_SETUP(this);
size_t opBytesNeeded = sizeof(int32_t) * 4 + sizeof(SkRect);
- BitmapCommonFunction bitmapCommon = shouldFlattenBitmaps(fFlags) ?
- &SkGPipeCanvas::commonDrawBitmapFlatten :
- &SkGPipeCanvas::commonDrawBitmapHeap;
-
- if ((*this.*bitmapCommon)(bm, kDrawBitmapNine_DrawOp, 0, opBytesNeeded, paint)) {
+ if (this->commonDrawBitmap(bm, kDrawBitmapNine_DrawOp, 0, opBytesNeeded, paint)) {
fWriter.write32(center.fLeft);
fWriter.write32(center.fTop);
fWriter.write32(center.fRight);
@@ -797,11 +775,7 @@
NOTIFY_SETUP(this);
size_t opBytesNeeded = sizeof(int32_t) * 2;
- BitmapCommonFunction bitmapCommon = shouldFlattenBitmaps(fFlags) ?
- &SkGPipeCanvas::commonDrawBitmapFlatten :
- &SkGPipeCanvas::commonDrawBitmapHeap;
-
- if ((*this.*bitmapCommon)(bm, kDrawSprite_DrawOp, 0, opBytesNeeded, paint)) {
+ if (this->commonDrawBitmap(bm, kDrawSprite_DrawOp, 0, opBytesNeeded, paint)) {
fWriter.write32(left);
fWriter.write32(top);
}
@@ -960,11 +934,6 @@
}
size_t SkGPipeCanvas::freeMemoryIfPossible(size_t bytesToFree) {
- // FIXME: This can be removed once fSharedHeap is used by cross process
- // case.
- if (NULL == fSharedHeap) {
- return 0;
- }
return fSharedHeap->freeMemoryIfPossible(bytesToFree);
}
@@ -1145,3 +1114,18 @@
return NULL == fCanvas ? 0 : fCanvas->storageAllocatedForRecording();
}
+///////////////////////////////////////////////////////////////////////////////
+
+BitmapShuttle::BitmapShuttle(SkGPipeCanvas* canvas) {
+ SkASSERT(canvas != NULL);
+ fCanvas = canvas;
+ fCanvas->ref();
+}
+
+BitmapShuttle::~BitmapShuttle() {
+ fCanvas->unref();
+}
+
+bool BitmapShuttle::insert(const SkBitmap& bitmap, int32_t slot) {
+ return fCanvas->shuttleBitmap(bitmap, slot);
+}