From 40c2ba27b6e5c6a4b6c073264f8c0a7c86355abe Mon Sep 17 00:00:00 2001 From: "reed@google.com" Date: Mon, 25 Jul 2011 19:41:22 +0000 Subject: [PATCH] don/t modify const paint, since it could be used in different threads git-svn-id: http://skia.googlecode.com/svn/trunk@1953 2bbb7eff-a529-9590-31e7-b0007b416f81 --- src/core/SkDraw.cpp | 109 +++++++++++++++++++++----------------------- 1 file changed, 51 insertions(+), 58 deletions(-) diff --git a/src/core/SkDraw.cpp b/src/core/SkDraw.cpp index 6d28044883..235c61fb63 100644 --- a/src/core/SkDraw.cpp +++ b/src/core/SkDraw.cpp @@ -67,17 +67,12 @@ private: SkBounder* fBounder; }; -static SkPoint* rect_points(SkRect& r, int index) { - SkASSERT((unsigned)index < 2); - return &((SkPoint*)(void*)&r)[index]; -} - /** Helper for allocating small blitters on the stack. */ #define kBlitterStorageLongCount (sizeof(SkBitmapProcShader) >> 2) -class SkAutoBlitterChoose { +class SkAutoBlitterChoose : SkNoncopyable { public: SkAutoBlitterChoose() { fBlitter = NULL; @@ -113,23 +108,30 @@ SkAutoBlitterChoose::~SkAutoBlitterChoose() { } } -class SkAutoBitmapShaderInstall { +/** + * Since we are providing the storage for the shader (to avoid the perf cost + * of calling new) we insist that in our destructor we can account for all + * owners of the shader. + */ +class SkAutoBitmapShaderInstall : SkNoncopyable { public: - SkAutoBitmapShaderInstall(const SkBitmap& src, const SkPaint* paint) - : fPaint((SkPaint*)paint) { - fPrevShader = paint->getShader(); - SkSafeRef(fPrevShader); - fPaint->setShader(SkShader::CreateBitmapShader( src, + SkAutoBitmapShaderInstall(const SkBitmap& src, const SkPaint& paint) + : fPaint(paint) /* makes a copy of the paint */ { + fPaint.setShader(SkShader::CreateBitmapShader(src, SkShader::kClamp_TileMode, SkShader::kClamp_TileMode, fStorage, sizeof(fStorage))); + // we deliberately left the shader with an owner-count of 2 + SkASSERT(2 == fPaint.getShader()->getRefCnt()); } ~SkAutoBitmapShaderInstall() { - SkShader* shader = fPaint->getShader(); + SkShader* shader = fPaint.getShader(); + // since we manually destroy shader, we insist that owners == 2 + SkASSERT(2 == shader->getRefCnt()); - fPaint->setShader(fPrevShader); - SkSafeUnref(fPrevShader); + fPaint.setShader(NULL); // unref the shader by 1 + // now destroy to take care of the 2nd owner-count if ((void*)shader == (void*)fStorage) { shader->~SkShader(); } else { @@ -137,33 +139,14 @@ public: } } + // return the new paint that has the shader applied + const SkPaint& paintWithShader() const { return fPaint; } + private: - SkPaint* fPaint; - SkShader* fPrevShader; + SkPaint fPaint; // copy of caller's paint (which we then modify) uint32_t fStorage[kBlitterStorageLongCount]; }; -class SkAutoPaintStyleRestore { -public: - SkAutoPaintStyleRestore(const SkPaint& paint, SkPaint::Style style) - : fPaint((SkPaint&)paint) { - fStyle = paint.getStyle(); // record the old - fPaint.setStyle(style); // change it to the specified style - } - - ~SkAutoPaintStyleRestore() { - fPaint.setStyle(fStyle); // restore the old - } - -private: - SkPaint& fPaint; - SkPaint::Style fStyle; - - // illegal - SkAutoPaintStyleRestore(const SkAutoPaintStyleRestore&); - SkAutoPaintStyleRestore& operator=(const SkAutoPaintStyleRestore&); -}; - /////////////////////////////////////////////////////////////////////////////// SkDraw::SkDraw() { @@ -628,12 +611,13 @@ void SkDraw::drawPoints(SkCanvas::PointMode mode, size_t count, switch (mode) { case SkCanvas::kPoints_PointMode: { // temporarily mark the paint as filling. - SkAutoPaintStyleRestore restore(paint, SkPaint::kFill_Style); + SkPaint newPaint(paint); + newPaint.setStyle(SkPaint::kFill_Style); - SkScalar width = paint.getStrokeWidth(); + SkScalar width = newPaint.getStrokeWidth(); SkScalar radius = SkScalarHalf(width); - if (paint.getStrokeCap() == SkPaint::kRound_Cap) { + if (newPaint.getStrokeCap() == SkPaint::kRound_Cap) { SkPath path; SkMatrix preMatrix; @@ -643,10 +627,11 @@ void SkDraw::drawPoints(SkCanvas::PointMode mode, size_t count, // pass true for the last point, since we can modify // then path then if (fDevice) { - fDevice->drawPath(*this, path, paint, &preMatrix, + fDevice->drawPath(*this, path, newPaint, &preMatrix, (count-1) == i); } else { - this->drawPath(path, paint, &preMatrix, (count-1) == i); + this->drawPath(path, newPaint, &preMatrix, + (count-1) == i); } } } else { @@ -658,9 +643,9 @@ void SkDraw::drawPoints(SkCanvas::PointMode mode, size_t count, r.fRight = r.fLeft + width; r.fBottom = r.fTop + width; if (fDevice) { - fDevice->drawRect(*this, r, paint); + fDevice->drawRect(*this, r, newPaint); } else { - this->drawRect(r, paint); + this->drawRect(r, newPaint); } } } @@ -740,6 +725,11 @@ SkDraw::RectType SkDraw::ComputeRectType(const SkPaint& paint, return rtype; } +static SkPoint* rect_points(SkRect& r, int index) { + SkASSERT((unsigned)index < 2); + return &((SkPoint*)(void*)&r)[index]; +} + void SkDraw::drawRect(const SkRect& rect, const SkPaint& paint) const { SkDEBUGCODE(this->validate();) @@ -1097,11 +1087,11 @@ void SkDraw::drawBitmapAsMask(const SkBitmap& bitmap, // we manually build a shader and draw that into our new mask SkPaint tmpPaint; tmpPaint.setFlags(paint.getFlags()); - SkAutoBitmapShaderInstall install(bitmap, &tmpPaint); + SkAutoBitmapShaderInstall install(bitmap, tmpPaint); SkRect rr; rr.set(0, 0, SkIntToScalar(bitmap.width()), SkIntToScalar(bitmap.height())); - c.drawRect(rr, tmpPaint); + c.drawRect(rr, install.paintWithShader()); } this->drawDevMask(mask, paint); } @@ -1125,14 +1115,14 @@ static bool clipped_out(const SkMatrix& matrix, const SkRegion& clip, } void SkDraw::drawBitmap(const SkBitmap& bitmap, const SkMatrix& prematrix, - const SkPaint& paint) const { + const SkPaint& origPaint) const { SkDEBUGCODE(this->validate();) // nothing to draw if (fClip->isEmpty() || bitmap.width() == 0 || bitmap.height() == 0 || bitmap.getConfig() == SkBitmap::kNo_Config || - (paint.getAlpha() == 0 && paint.getXfermode() == NULL)) { + (origPaint.getAlpha() == 0 && origPaint.getXfermode() == NULL)) { return; } @@ -1143,7 +1133,8 @@ void SkDraw::drawBitmap(const SkBitmap& bitmap, const SkMatrix& prematrix, } #endif - SkAutoPaintStyleRestore restore(paint, SkPaint::kFill_Style); + SkPaint paint(origPaint); + paint.setStyle(SkPaint::kFill_Style); SkMatrix matrix; if (!matrix.setConcat(*fMatrix, prematrix)) { @@ -1208,25 +1199,25 @@ void SkDraw::drawBitmap(const SkBitmap& bitmap, const SkMatrix& prematrix, if (bitmap.getConfig() == SkBitmap::kA8_Config) { draw.drawBitmapAsMask(bitmap, paint); } else { - SkAutoBitmapShaderInstall install(bitmap, &paint); + SkAutoBitmapShaderInstall install(bitmap, paint); SkRect r; r.set(0, 0, SkIntToScalar(bitmap.width()), SkIntToScalar(bitmap.height())); // is this ok if paint has a rasterizer? - draw.drawRect(r, paint); + draw.drawRect(r, install.paintWithShader()); } } void SkDraw::drawSprite(const SkBitmap& bitmap, int x, int y, - const SkPaint& paint) const { + const SkPaint& origPaint) const { SkDEBUGCODE(this->validate();) // nothing to draw if (fClip->isEmpty() || bitmap.width() == 0 || bitmap.height() == 0 || bitmap.getConfig() == SkBitmap::kNo_Config || - (paint.getAlpha() == 0 && paint.getXfermode() == NULL)) { + (origPaint.getAlpha() == 0 && origPaint.getXfermode() == NULL)) { return; } @@ -1237,7 +1228,8 @@ void SkDraw::drawSprite(const SkBitmap& bitmap, int x, int y, return; // nothing to draw } - SkAutoPaintStyleRestore restore(paint, SkPaint::kFill_Style); + SkPaint paint(origPaint); + paint.setStyle(SkPaint::kFill_Style); if (NULL == paint.getColorFilter()) { uint32_t storage[kBlitterStorageLongCount]; @@ -1262,7 +1254,8 @@ void SkDraw::drawSprite(const SkBitmap& bitmap, int x, int y, } } - SkAutoBitmapShaderInstall install(bitmap, &paint); + SkAutoBitmapShaderInstall install(bitmap, paint); + const SkPaint& shaderPaint = install.paintWithShader(); SkMatrix matrix; SkRect r; @@ -1272,14 +1265,14 @@ void SkDraw::drawSprite(const SkBitmap& bitmap, int x, int y, // tell the shader our offset matrix.setTranslate(r.fLeft, r.fTop); - paint.getShader()->setLocalMatrix(matrix); + shaderPaint.getShader()->setLocalMatrix(matrix); SkDraw draw(*this); matrix.reset(); draw.fMatrix = &matrix; // call ourself with a rect // is this OK if paint has a rasterizer? - draw.drawRect(r, paint); + draw.drawRect(r, shaderPaint); } ///////////////////////////////////////////////////////////////////////////////