From f700fb2f139cc38f6ba3571db6cfe0a2e1bbc8c0 Mon Sep 17 00:00:00 2001 From: "commit-bot@chromium.org" Date: Wed, 14 May 2014 14:50:57 +0000 Subject: [PATCH] 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 --- src/pipe/SkGPipeWrite.cpp | 109 ++++++++++++++++++------------- src/pipe/utils/SamplePipeControllers.cpp | 17 +++-- src/pipe/utils/SamplePipeControllers.h | 6 +- tests/DeferredCanvasTest.cpp | 4 -- 4 files changed, 79 insertions(+), 57 deletions(-) diff --git a/src/pipe/SkGPipeWrite.cpp b/src/pipe/SkGPipeWrite.cpp index 5b1257a..82848f8 100644 --- 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(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 --- 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 --- 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 --- 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); -- 2.7.4