Manually revert "4x allocation in PipeController is probably overkill."

This reverts commit 2d91efffdb57646a495de5bf859ff281ef86dd12.

Conflicts:
	src/pipe/SkGPipeWrite.cpp

BUG=372671
R=mtklein@google.com

Author: mtklein@chromium.org

Review URL: https://codereview.chromium.org/277653004

git-svn-id: http://skia.googlecode.com/svn/trunk@14725 2bbb7eff-a529-9590-31e7-b0007b416f81

diff --git a/src/pipe/SkGPipeWrite.cpp b/src/pipe/SkGPipeWrite.cpp
index 5b1257a..82848f8 100644 (file)
--- a/src/pipe/SkGPipeWrite.cpp
+++ b/src/pipe/SkGPipeWrite.cpp
@@ -29,14 +29,6 @@
 #include "SkTypeface.h"
 #include "SkWriter32.h"
 
-#ifdef SK_DEBUG
-// When debugging, allocate snuggly.
-static const size_t kMinBlockSize = 0;
-#else
-// For peformance, make large allocations.
-static const size_t kMinBlockSize = 16 * 1024;
-#endif
-
 enum {
     kSizeOfFlatRRect = sizeof(SkRect) + 4 * sizeof(SkVector)
 };
@@ -358,6 +350,15 @@ private:
     SkPaint fPaint;
     void writePaint(const SkPaint&);
 
+    class AutoPipeNotify {
+    public:
+        AutoPipeNotify(SkGPipeCanvas* canvas) : fCanvas(canvas) {}
+        ~AutoPipeNotify() { fCanvas->doNotify(); }
+    private:
+        SkGPipeCanvas* fCanvas;
+    };
+    friend class AutoPipeNotify;
+
     typedef SkCanvas INHERITED;
 };
 
@@ -365,7 +366,7 @@ void SkGPipeCanvas::flattenFactoryNames() {
     const char* name;
     while ((name = fFactorySet->getNextAddedFactoryName()) != NULL) {
         size_t len = strlen(name);
-        if (this->needOpBytes(SkWriter32::WriteStringSize(name, len))) {
+        if (this->needOpBytes(len)) {
             this->writeOp(kDef_Factory_DrawOp);
             fWriter.writeString(name, len);
         }
@@ -420,6 +421,7 @@ int SkGPipeCanvas::flattenToIndex(SkFlattenable* obj, PaintFlats paintflat) {
 
 ///////////////////////////////////////////////////////////////////////////////
 
+#define MIN_BLOCK_SIZE  (16 * 1024)
 #define BITMAPS_TO_KEEP 5
 #define FLATTENABLES_TO_KEEP 10
 
@@ -457,6 +459,7 @@ SkGPipeCanvas::SkGPipeCanvas(SkGPipeController* controller,
         }
     }
     fFlattenableHeap.setBitmapStorage(fBitmapHeap);
+    this->doNotify();
 }
 
 SkGPipeCanvas::~SkGPipeCanvas() {
@@ -471,13 +474,12 @@ bool SkGPipeCanvas::needOpBytes(size_t needed) {
     }
 
     needed += 4;  // size of DrawOp atom
-    needed = SkTMax(kMinBlockSize, needed);
-    needed = SkAlign4(needed);
     if (fWriter.bytesWritten() + needed > fBlockSize) {
-        // We're making a new block.  First push out the current block.
+        // Before we wipe out any data that has already been written, read it
+        // out.
         this->doNotify();
-
-        void* block = fController->requestBlock(needed, &fBlockSize);
+        size_t blockSize = SkTMax<size_t>(MIN_BLOCK_SIZE, needed);
+        void* block = fController->requestBlock(blockSize, &fBlockSize);
         if (NULL == block) {
             // Do not notify the readers, which would call this function again.
             this->finish(false);
@@ -508,7 +510,11 @@ uint32_t SkGPipeCanvas::getTypefaceID(SkTypeface* face) {
 
 ///////////////////////////////////////////////////////////////////////////////
 
+#define NOTIFY_SETUP(canvas)    \
+    AutoPipeNotify apn(canvas)
+
 void SkGPipeCanvas::willSave(SaveFlags flags) {
+    NOTIFY_SETUP(this);
     if (this->needOpBytes()) {
         this->writeOp(kSave_DrawOp, 0, flags);
     }
@@ -518,6 +524,7 @@ void SkGPipeCanvas::willSave(SaveFlags flags) {
 
 SkCanvas::SaveLayerStrategy SkGPipeCanvas::willSaveLayer(const SkRect* bounds, const SkPaint* paint,
                                                          SaveFlags saveFlags) {
+    NOTIFY_SETUP(this);
     size_t size = 0;
     unsigned opFlags = 0;
 
@@ -547,6 +554,7 @@ SkCanvas::SaveLayerStrategy SkGPipeCanvas::willSaveLayer(const SkRect* bounds, c
 }
 
 void SkGPipeCanvas::willRestore() {
+    NOTIFY_SETUP(this);
     if (this->needOpBytes()) {
         this->writeOp(kRestore_DrawOp);
     }
@@ -587,6 +595,7 @@ void SkGPipeCanvas::recordConcat(const SkMatrix& m) {
 
 void SkGPipeCanvas::didConcat(const SkMatrix& matrix) {
     if (!matrix.isIdentity()) {
+        NOTIFY_SETUP(this);
         switch (matrix.getType()) {
             case SkMatrix::kTranslate_Mask:
                 this->recordTranslate(matrix);
@@ -604,6 +613,7 @@ void SkGPipeCanvas::didConcat(const SkMatrix& matrix) {
 }
 
 void SkGPipeCanvas::didSetMatrix(const SkMatrix& matrix) {
+    NOTIFY_SETUP(this);
     if (this->needOpBytes(matrix.writeToMemory(NULL))) {
         this->writeOp(kSetMatrix_DrawOp);
         fWriter.writeMatrix(matrix);
@@ -613,6 +623,7 @@ void SkGPipeCanvas::didSetMatrix(const SkMatrix& matrix) {
 
 void SkGPipeCanvas::onClipRect(const SkRect& rect, SkRegion::Op rgnOp,
                                ClipEdgeStyle edgeStyle) {
+    NOTIFY_SETUP(this);
     if (this->needOpBytes(sizeof(SkRect))) {
         unsigned flags = 0;
         if (kSoft_ClipEdgeStyle == edgeStyle) {
@@ -626,6 +637,7 @@ void SkGPipeCanvas::onClipRect(const SkRect& rect, SkRegion::Op rgnOp,
 
 void SkGPipeCanvas::onClipRRect(const SkRRect& rrect, SkRegion::Op rgnOp,
                                 ClipEdgeStyle edgeStyle) {
+    NOTIFY_SETUP(this);
     if (this->needOpBytes(kSizeOfFlatRRect)) {
         unsigned flags = 0;
         if (kSoft_ClipEdgeStyle == edgeStyle) {
@@ -639,6 +651,7 @@ void SkGPipeCanvas::onClipRRect(const SkRRect& rrect, SkRegion::Op rgnOp,
 
 void SkGPipeCanvas::onClipPath(const SkPath& path, SkRegion::Op rgnOp,
                                ClipEdgeStyle edgeStyle) {
+    NOTIFY_SETUP(this);
     if (this->needOpBytes(path.writeToMemory(NULL))) {
         unsigned flags = 0;
         if (kSoft_ClipEdgeStyle == edgeStyle) {
@@ -652,6 +665,7 @@ void SkGPipeCanvas::onClipPath(const SkPath& path, SkRegion::Op rgnOp,
 }
 
 void SkGPipeCanvas::onClipRegion(const SkRegion& region, SkRegion::Op rgnOp) {
+    NOTIFY_SETUP(this);
     if (this->needOpBytes(region.writeToMemory(NULL))) {
         this->writeOp(kClipRegion_DrawOp, 0, rgnOp);
         fWriter.writeRegion(region);
@@ -662,6 +676,7 @@ void SkGPipeCanvas::onClipRegion(const SkRegion& region, SkRegion::Op rgnOp) {
 ///////////////////////////////////////////////////////////////////////////////
 
 void SkGPipeCanvas::clear(SkColor color) {
+    NOTIFY_SETUP(this);
     unsigned flags = 0;
     if (color) {
         flags |= kClear_HasColor_DrawOpFlag;
@@ -675,6 +690,7 @@ void SkGPipeCanvas::clear(SkColor color) {
 }
 
 void SkGPipeCanvas::drawPaint(const SkPaint& paint) {
+    NOTIFY_SETUP(this);
     this->writePaint(paint);
     if (this->needOpBytes()) {
         this->writeOp(kDrawPaint_DrawOp);
@@ -684,6 +700,7 @@ void SkGPipeCanvas::drawPaint(const SkPaint& paint) {
 void SkGPipeCanvas::drawPoints(PointMode mode, size_t count,
                                const SkPoint pts[], const SkPaint& paint) {
     if (count) {
+        NOTIFY_SETUP(this);
         this->writePaint(paint);
         if (this->needOpBytes(4 + count * sizeof(SkPoint))) {
             this->writeOp(kDrawPoints_DrawOp, mode, 0);
@@ -694,6 +711,7 @@ void SkGPipeCanvas::drawPoints(PointMode mode, size_t count,
 }
 
 void SkGPipeCanvas::drawOval(const SkRect& rect, const SkPaint& paint) {
+    NOTIFY_SETUP(this);
     this->writePaint(paint);
     if (this->needOpBytes(sizeof(SkRect))) {
         this->writeOp(kDrawOval_DrawOp);
@@ -702,6 +720,7 @@ void SkGPipeCanvas::drawOval(const SkRect& rect, const SkPaint& paint) {
 }
 
 void SkGPipeCanvas::drawRect(const SkRect& rect, const SkPaint& paint) {
+    NOTIFY_SETUP(this);
     this->writePaint(paint);
     if (this->needOpBytes(sizeof(SkRect))) {
         this->writeOp(kDrawRect_DrawOp);
@@ -710,6 +729,7 @@ void SkGPipeCanvas::drawRect(const SkRect& rect, const SkPaint& paint) {
 }
 
 void SkGPipeCanvas::drawRRect(const SkRRect& rrect, const SkPaint& paint) {
+    NOTIFY_SETUP(this);
     this->writePaint(paint);
     if (this->needOpBytes(kSizeOfFlatRRect)) {
         this->writeOp(kDrawRRect_DrawOp);
@@ -719,6 +739,7 @@ void SkGPipeCanvas::drawRRect(const SkRRect& rrect, const SkPaint& paint) {
 
 void SkGPipeCanvas::onDrawDRRect(const SkRRect& outer, const SkRRect& inner,
                                  const SkPaint& paint) {
+    NOTIFY_SETUP(this);
     this->writePaint(paint);
     if (this->needOpBytes(kSizeOfFlatRRect * 2)) {
         this->writeOp(kDrawDRRect_DrawOp);
@@ -728,6 +749,7 @@ void SkGPipeCanvas::onDrawDRRect(const SkRRect& outer, const SkRRect& inner,
 }
 
 void SkGPipeCanvas::drawPath(const SkPath& path, const SkPaint& paint) {
+    NOTIFY_SETUP(this);
     this->writePaint(paint);
     if (this->needOpBytes(path.writeToMemory(NULL))) {
         this->writeOp(kDrawPath_DrawOp);
@@ -739,24 +761,16 @@ bool SkGPipeCanvas::commonDrawBitmap(const SkBitmap& bm, DrawOps op,
                                      unsigned flags,
                                      size_t opBytesNeeded,
                                      const SkPaint* paint) {
-    if (fDone) {
-        return false;
-    }
-
     if (paint != NULL) {
         flags |= kDrawBitmap_HasPaint_DrawOpFlag;
         this->writePaint(*paint);
     }
-
-    // This needs to run first so its calls to needOpBytes() and writes
-    // don't interlace with the needOpBytes() and writes below.
-    SkASSERT(fBitmapHeap != NULL);
-    int32_t bitmapIndex = fBitmapHeap->insert(bm);
-    if (SkBitmapHeap::INVALID_SLOT == bitmapIndex) {
-        return false;
-    }
-
     if (this->needOpBytes(opBytesNeeded)) {
+        SkASSERT(fBitmapHeap != NULL);
+        int32_t bitmapIndex = fBitmapHeap->insert(bm);
+        if (SkBitmapHeap::INVALID_SLOT == bitmapIndex) {
+            return false;
+        }
         this->writeOp(op, flags, bitmapIndex);
         return true;
     }
@@ -765,6 +779,7 @@ bool SkGPipeCanvas::commonDrawBitmap(const SkBitmap& bm, DrawOps op,
 
 void SkGPipeCanvas::drawBitmap(const SkBitmap& bm, SkScalar left, SkScalar top,
                                const SkPaint* paint) {
+    NOTIFY_SETUP(this);
     size_t opBytesNeeded = sizeof(SkScalar) * 2;
 
     if (this->commonDrawBitmap(bm, kDrawBitmap_DrawOp, 0, opBytesNeeded, paint)) {
@@ -776,6 +791,7 @@ void SkGPipeCanvas::drawBitmap(const SkBitmap& bm, SkScalar left, SkScalar top,
 void SkGPipeCanvas::drawBitmapRectToRect(const SkBitmap& bm, const SkRect* src,
                                          const SkRect& dst, const SkPaint* paint,
                                          DrawBitmapRectFlags dbmrFlags) {
+    NOTIFY_SETUP(this);
     size_t opBytesNeeded = sizeof(SkRect);
     bool hasSrc = src != NULL;
     unsigned flags;
@@ -799,6 +815,7 @@ void SkGPipeCanvas::drawBitmapRectToRect(const SkBitmap& bm, const SkRect* src,
 
 void SkGPipeCanvas::drawBitmapMatrix(const SkBitmap& bm, const SkMatrix& matrix,
                                      const SkPaint* paint) {
+    NOTIFY_SETUP(this);
     size_t opBytesNeeded = matrix.writeToMemory(NULL);
 
     if (this->commonDrawBitmap(bm, kDrawBitmapMatrix_DrawOp, 0, opBytesNeeded, paint)) {
@@ -808,6 +825,7 @@ void SkGPipeCanvas::drawBitmapMatrix(const SkBitmap& bm, const SkMatrix& matrix,
 
 void SkGPipeCanvas::drawBitmapNine(const SkBitmap& bm, const SkIRect& center,
                                    const SkRect& dst, const SkPaint* paint) {
+    NOTIFY_SETUP(this);
     size_t opBytesNeeded = sizeof(int32_t) * 4 + sizeof(SkRect);
 
     if (this->commonDrawBitmap(bm, kDrawBitmapNine_DrawOp, 0, opBytesNeeded, paint)) {
@@ -821,6 +839,7 @@ void SkGPipeCanvas::drawBitmapNine(const SkBitmap& bm, const SkIRect& center,
 
 void SkGPipeCanvas::drawSprite(const SkBitmap& bm, int left, int top,
                                    const SkPaint* paint) {
+    NOTIFY_SETUP(this);
     size_t opBytesNeeded = sizeof(int32_t) * 2;
 
     if (this->commonDrawBitmap(bm, kDrawSprite_DrawOp, 0, opBytesNeeded, paint)) {
@@ -832,6 +851,7 @@ void SkGPipeCanvas::drawSprite(const SkBitmap& bm, int left, int top,
 void SkGPipeCanvas::onDrawText(const void* text, size_t byteLength, SkScalar x, SkScalar y,
                                const SkPaint& paint) {
     if (byteLength) {
+        NOTIFY_SETUP(this);
         this->writePaint(paint);
         if (this->needOpBytes(4 + SkAlign4(byteLength) + 2 * sizeof(SkScalar))) {
             this->writeOp(kDrawText_DrawOp);
@@ -846,6 +866,7 @@ void SkGPipeCanvas::onDrawText(const void* text, size_t byteLength, SkScalar x,
 void SkGPipeCanvas::onDrawPosText(const void* text, size_t byteLength, const SkPoint pos[],
                                   const SkPaint& paint) {
     if (byteLength) {
+        NOTIFY_SETUP(this);
         this->writePaint(paint);
         int count = paint.textToGlyphs(text, byteLength, NULL);
         if (this->needOpBytes(4 + SkAlign4(byteLength) + 4 + count * sizeof(SkPoint))) {
@@ -861,6 +882,7 @@ void SkGPipeCanvas::onDrawPosText(const void* text, size_t byteLength, const SkP
 void SkGPipeCanvas::onDrawPosTextH(const void* text, size_t byteLength, const SkScalar xpos[],
                                    SkScalar constY, const SkPaint& paint) {
     if (byteLength) {
+        NOTIFY_SETUP(this);
         this->writePaint(paint);
         int count = paint.textToGlyphs(text, byteLength, NULL);
         if (this->needOpBytes(4 + SkAlign4(byteLength) + 4 + count * sizeof(SkScalar) + 4)) {
@@ -877,6 +899,7 @@ void SkGPipeCanvas::onDrawPosTextH(const void* text, size_t byteLength, const Sk
 void SkGPipeCanvas::onDrawTextOnPath(const void* text, size_t byteLength, const SkPath& path,
                                      const SkMatrix* matrix, const SkPaint& paint) {
     if (byteLength) {
+        NOTIFY_SETUP(this);
         unsigned flags = 0;
         size_t size = 4 + SkAlign4(byteLength) + path.writeToMemory(NULL);
         if (matrix) {
@@ -912,14 +935,10 @@ void SkGPipeCanvas::drawVertices(VertexMode vmode, int vertexCount,
         return;
     }
 
+    NOTIFY_SETUP(this);
+    size_t size = 4 + vertexCount * sizeof(SkPoint);
     this->writePaint(paint);
-
-    unsigned flags = 0;  // flags pack with the op, so don't need extra space themselves
-
-    size_t size = 0;
-    size += 4;                             // vmode
-    size += 4;                             // vertexCount
-    size += vertexCount * sizeof(SkPoint); // vertices
+    unsigned flags = 0;
     if (texs) {
         flags |= kDrawVertices_HasTexs_DrawOpFlag;
         size += vertexCount * sizeof(SkPoint);
@@ -928,14 +947,13 @@ void SkGPipeCanvas::drawVertices(VertexMode vmode, int vertexCount,
         flags |= kDrawVertices_HasColors_DrawOpFlag;
         size += vertexCount * sizeof(SkColor);
     }
-    if (xfer && !SkXfermode::IsMode(xfer, SkXfermode::kModulate_Mode)) {
-        flags |= kDrawVertices_HasXfermode_DrawOpFlag;
-        size += sizeof(int32_t);    // SkXfermode::Mode
-    }
     if (indices && indexCount > 0) {
         flags |= kDrawVertices_HasIndices_DrawOpFlag;
-        size += 4;                                        // indexCount
-        size += SkAlign4(indexCount * sizeof(uint16_t));  // indices
+        size += 4 + SkAlign4(indexCount * sizeof(uint16_t));
+    }
+    if (xfer && !SkXfermode::IsMode(xfer, SkXfermode::kModulate_Mode)) {
+        flags |= kDrawVertices_HasXfermode_DrawOpFlag;
+        size += sizeof(int32_t);    // mode enum
     }
 
     if (this->needOpBytes(size)) {
@@ -943,18 +961,18 @@ void SkGPipeCanvas::drawVertices(VertexMode vmode, int vertexCount,
         fWriter.write32(vmode);
         fWriter.write32(vertexCount);
         fWriter.write(vertices, vertexCount * sizeof(SkPoint));
-        if (flags & kDrawVertices_HasTexs_DrawOpFlag) {
+        if (texs) {
             fWriter.write(texs, vertexCount * sizeof(SkPoint));
         }
-        if (flags & kDrawVertices_HasColors_DrawOpFlag) {
+        if (colors) {
             fWriter.write(colors, vertexCount * sizeof(SkColor));
         }
         if (flags & kDrawVertices_HasXfermode_DrawOpFlag) {
             SkXfermode::Mode mode = SkXfermode::kModulate_Mode;
-            SkAssertResult(xfer->asMode(&mode));
+            (void)xfer->asMode(&mode);
             fWriter.write32(mode);
         }
-        if (flags & kDrawVertices_HasIndices_DrawOpFlag) {
+        if (indices && indexCount > 0) {
             fWriter.write32(indexCount);
             fWriter.writePad(indices, indexCount * sizeof(uint16_t));
         }
@@ -963,6 +981,7 @@ void SkGPipeCanvas::drawVertices(VertexMode vmode, int vertexCount,
 
 void SkGPipeCanvas::drawData(const void* ptr, size_t size) {
     if (size && ptr) {
+        NOTIFY_SETUP(this);
         unsigned data = 0;
         if (size < (1 << DRAWOPS_DATA_BITS)) {
             data = (unsigned)size;
@@ -990,7 +1009,7 @@ void SkGPipeCanvas::endCommentGroup() {
 }
 
 void SkGPipeCanvas::flushRecording(bool detachCurrentBlock) {
-    this->doNotify();
+    doNotify();
     if (detachCurrentBlock) {
         // force a new block to be requested for the next recorded command
         fBlockSize = 0;

diff --git a/src/pipe/utils/SamplePipeControllers.cpp b/src/pipe/utils/SamplePipeControllers.cpp
index de26346..1e25cb6 100644 (file)
--- a/src/pipe/utils/SamplePipeControllers.cpp
+++ b/src/pipe/utils/SamplePipeControllers.cpp
@@ -13,16 +13,23 @@
 #include "SkMatrix.h"
 
 PipeController::PipeController(SkCanvas* target, SkPicture::InstallPixelRefProc proc)
-    : fReader(target), fBlockSize(0), fBytesWritten(0) {
+:fReader(target) {
+    fBlock = NULL;
+    fBlockSize = fBytesWritten = 0;
     fReader.setBitmapDecoder(proc);
 }
 
-void* PipeController::requestBlock(size_t minRequest, size_t* actual) {
-    fBlockSize = minRequest;
-    fBlock.reset(fBlockSize);
+PipeController::~PipeController() {
+    sk_free(fBlock);
+}
+
+void* PipeController::requestBlock(size_t minRequest, size_t *actual) {
+    sk_free(fBlock);
+    fBlockSize = minRequest * 4;
+    fBlock = sk_malloc_throw(fBlockSize);
     fBytesWritten = 0;
     *actual = fBlockSize;
-    return fBlock.get();
+    return fBlock;
 }
 
 void PipeController::notifyWritten(size_t bytes) {

diff --git a/src/pipe/utils/SamplePipeControllers.h b/src/pipe/utils/SamplePipeControllers.h
index e8cc23b..35cfba7 100644 (file)
--- a/src/pipe/utils/SamplePipeControllers.h
+++ b/src/pipe/utils/SamplePipeControllers.h
@@ -17,14 +17,14 @@ class SkMatrix;
 class PipeController : public SkGPipeController {
 public:
     PipeController(SkCanvas* target, SkPicture::InstallPixelRefProc proc = NULL);
-
+    virtual ~PipeController();
     virtual void* requestBlock(size_t minRequest, size_t* actual) SK_OVERRIDE;
     virtual void notifyWritten(size_t bytes) SK_OVERRIDE;
 protected:
-    const void* getData() { return (const char*) fBlock.get() + fBytesWritten; }
+    const void* getData() { return (const char*) fBlock + fBytesWritten; }
     SkGPipeReader fReader;
 private:
-    SkAutoMalloc fBlock;
+    void* fBlock;
     size_t fBlockSize;
     size_t fBytesWritten;
     SkGPipeReader::Status fStatus;

diff --git a/tests/DeferredCanvasTest.cpp b/tests/DeferredCanvasTest.cpp
index f279c21..b61ae2e 100644 (file)
--- a/tests/DeferredCanvasTest.cpp
+++ b/tests/DeferredCanvasTest.cpp
@@ -547,11 +547,7 @@ static void TestDeferredCanvasBitmapCaching(skiatest::Reporter* reporter) {
     canvas->drawBitmap(sourceImages[0], 0, 0, NULL);
     REPORTER_ASSERT(reporter, 2 == notificationCounter.fStorageAllocatedChangedCount);
     canvas->drawBitmap(sourceImages[0], 0, 0, NULL);
-#ifdef SK_DEBUG
     REPORTER_ASSERT(reporter, 2 == notificationCounter.fStorageAllocatedChangedCount);
-#else
-    REPORTER_ASSERT(reporter, 3 == notificationCounter.fStorageAllocatedChangedCount);
-#endif
     REPORTER_ASSERT(reporter, 1 == notificationCounter.fFlushedDrawCommandsCount);
     REPORTER_ASSERT(reporter, canvas->storageAllocatedForRecording() < 2 * bitmapSize);