From 135e16cd8ebb2ba30458b284d4a17e1eea81ad35 Mon Sep 17 00:00:00 2001 From: "reed@google.com" Date: Thu, 13 Dec 2012 21:53:41 +0000 Subject: [PATCH] revert 6798 (assert in GM) git-svn-id: http://skia.googlecode.com/svn/trunk@6802 2bbb7eff-a529-9590-31e7-b0007b416f81 --- include/core/SkComposeShader.h | 9 ++-- include/core/SkShader.h | 29 ++++------- src/core/SkBitmapProcShader.cpp | 18 ++++--- src/core/SkBitmapProcShader.h | 3 +- src/core/SkBlitter.cpp | 52 ++++++++----------- src/core/SkColorFilter.cpp | 28 +++++----- src/core/SkComposeShader.cpp | 38 ++++++-------- src/core/SkDraw.cpp | 20 ++++--- src/core/SkFilterShader.h | 14 ++--- src/core/SkShader.cpp | 24 +++++---- src/effects/gradients/SkGradientShader.cpp | 2 - src/effects/gradients/SkGradientShaderPriv.h | 1 + .../gradients/SkTwoPointRadialGradient.cpp | 13 ++--- 13 files changed, 115 insertions(+), 136 deletions(-) diff --git a/include/core/SkComposeShader.h b/include/core/SkComposeShader.h index b0790bf393..a8a8e0bb60 100644 --- a/include/core/SkComposeShader.h +++ b/include/core/SkComposeShader.h @@ -34,10 +34,11 @@ public: SkComposeShader(SkShader* sA, SkShader* sB, SkXfermode* mode = NULL); virtual ~SkComposeShader(); - virtual bool setContext(const SkBitmap&, const SkPaint&, - const SkMatrix&) SK_OVERRIDE; - virtual void endContext() SK_OVERRIDE; - virtual void shadeSpan(int x, int y, SkPMColor[], int count) SK_OVERRIDE; + // override + virtual bool setContext(const SkBitmap& device, const SkPaint& paint, const SkMatrix& matrix); + virtual void shadeSpan(int x, int y, SkPMColor result[], int count); + virtual void beginSession(); + virtual void endSession(); SK_DECLARE_PUBLIC_FLATTENABLE_DESERIALIZATION_PROCS(SkComposeShader) diff --git a/include/core/SkShader.h b/include/core/SkShader.h index 7edbe6fd4b..1286177271 100644 --- a/include/core/SkShader.h +++ b/include/core/SkShader.h @@ -139,28 +139,11 @@ public: /** * Called once before drawing, with the current paint and device matrix. * Return true if your shader supports these parameters, or false if not. - * If false is returned, nothing will be drawn. If true is returned, then - * a balancing call to endContext() will be made before the next call to - * setContext. - * - * Subclasses should be sure to call their INHERITED::setContext() if they - * override this method. + * If false is returned, nothing will be drawn. */ virtual bool setContext(const SkBitmap& device, const SkPaint& paint, const SkMatrix& matrix); - /** - * Assuming setContext returned true, endContext() will be called when - * the draw using the shader has completed. It is an error for setContext - * to be called twice w/o an intervening call to endContext(). - * - * Subclasses should be sure to call their INHERITED::endContext() if they - * override this method. - */ - virtual void endContext(); - - SkDEBUGCODE(bool setContextHasBeenCalled() const { return fInSetContext; }) - /** * Called for each span of the object being drawn. Your subclass should * set the appropriate colors (with premultiplied alpha) that correspond @@ -199,6 +182,14 @@ public: return (flags & kHasSpan16_Flag) != 0; } + /** + * Called before a session using the shader begins. Some shaders override + * this to defer some of their work (like calling bitmap.lockPixels()). + * Must be balanced by a call to endSession. + */ + virtual void beginSession(); + virtual void endSession(); + /** Gives method bitmap should be read to implement a shader. Also determines number and interpretation of "extra" parameters returned @@ -364,7 +355,7 @@ private: uint8_t fPaintAlpha; uint8_t fDeviceConfig; uint8_t fTotalInverseClass; - SkDEBUGCODE(SkBool8 fInSetContext;) + SkDEBUGCODE(SkBool8 fInSession;) static SkShader* CreateBitmapShader(const SkBitmap& src, TileMode, TileMode, diff --git a/src/core/SkBitmapProcShader.cpp b/src/core/SkBitmapProcShader.cpp index 3e4116651d..df2caee2ca 100644 --- a/src/core/SkBitmapProcShader.cpp +++ b/src/core/SkBitmapProcShader.cpp @@ -41,6 +41,18 @@ SkBitmapProcShader::SkBitmapProcShader(SkFlattenableReadBuffer& buffer) fFlags = 0; // computed in setContext } +void SkBitmapProcShader::beginSession() { + this->INHERITED::beginSession(); + + fRawBitmap.lockPixels(); +} + +void SkBitmapProcShader::endSession() { + fRawBitmap.unlockPixels(); + + this->INHERITED::endSession(); +} + SkShader::BitmapType SkBitmapProcShader::asABitmap(SkBitmap* texture, SkMatrix* texM, TileMode xy[]) const { @@ -90,7 +102,6 @@ bool SkBitmapProcShader::setContext(const SkBitmap& device, } if (!fState.chooseProcs(this->getTotalInverse(), paint)) { - fState.fOrigBitmap.unlockPixels(); return false; } @@ -138,11 +149,6 @@ bool SkBitmapProcShader::setContext(const SkBitmap& device, return true; } -void SkBitmapProcShader::endContext() { - fState.fOrigBitmap.unlockPixels(); - this->INHERITED::endContext(); -} - #define BUF_MAX 128 #define TEST_BUFFER_OVERRITEx diff --git a/src/core/SkBitmapProcShader.h b/src/core/SkBitmapProcShader.h index cb791d0c92..23e099250b 100644 --- a/src/core/SkBitmapProcShader.h +++ b/src/core/SkBitmapProcShader.h @@ -20,11 +20,12 @@ public: // overrides from SkShader virtual bool isOpaque() const SK_OVERRIDE; virtual bool setContext(const SkBitmap&, const SkPaint&, const SkMatrix&); - virtual void endContext(); virtual uint32_t getFlags() { return fFlags; } virtual void shadeSpan(int x, int y, SkPMColor dstC[], int count); virtual ShadeProc asAShadeProc(void** ctx) SK_OVERRIDE; virtual void shadeSpan16(int x, int y, uint16_t dstC[], int count); + virtual void beginSession(); + virtual void endSession(); virtual BitmapType asABitmap(SkBitmap*, SkMatrix*, TileMode*) const; static bool CanDo(const SkBitmap&, TileMode tx, TileMode ty); diff --git a/src/core/SkBlitter.cpp b/src/core/SkBlitter.cpp index 1234f43c3f..85331c23c3 100644 --- a/src/core/SkBlitter.cpp +++ b/src/core/SkBlitter.cpp @@ -572,30 +572,16 @@ public: void setMask(const SkMask* mask) { fMask = mask; } virtual bool setContext(const SkBitmap& device, const SkPaint& paint, - const SkMatrix& matrix) SK_OVERRIDE { - if (!this->INHERITED::setContext(device, paint, matrix)) { - return false; - } + const SkMatrix& matrix) { if (fProxy) { - if (!fProxy->setContext(device, paint, matrix)) { - // must keep our set/end context calls balanced - this->INHERITED::endContext(); - return false; - } + return fProxy->setContext(device, paint, matrix); } else { fPMColor = SkPreMultiplyColor(paint.getColor()); + return this->INHERITED::setContext(device, paint, matrix); } - return true; - } - - virtual void endContext() SK_OVERRIDE { - if (fProxy) { - fProxy->endContext(); - } - this->INHERITED::endContext(); } - virtual void shadeSpan(int x, int y, SkPMColor span[], int count) SK_OVERRIDE { + virtual void shadeSpan(int x, int y, SkPMColor span[], int count) { if (fProxy) { fProxy->shadeSpan(x, y, span, count); } @@ -659,6 +645,20 @@ public: } } + virtual void beginSession() { + this->INHERITED::beginSession(); + if (fProxy) { + fProxy->beginSession(); + } + } + + virtual void endSession() { + if (fProxy) { + fProxy->endSession(); + } + this->INHERITED::endSession(); + } + SK_DECLARE_PUBLIC_FLATTENABLE_DESERIALIZATION_PROCS(Sk3DShader) protected: @@ -907,17 +907,8 @@ SkBlitter* SkBlitter::Choose(const SkBitmap& device, // if there is one, the shader will take care of it. } - /* - * We need to have balanced calls to the shader: - * setContext - * endContext - * We make the first call here, in case it fails we can abort the draw. - * The endContext() call is made by the blitter (assuming setContext did - * not fail) in its destructor. - */ if (shader && !shader->setContext(device, *paint, matrix)) { - SK_PLACEMENT_NEW(blitter, SkNullBlitter, storage, storageSize); - return blitter; + return SkNEW(SkNullBlitter); } switch (device.getConfig()) { @@ -987,15 +978,14 @@ SkShaderBlitter::SkShaderBlitter(const SkBitmap& device, const SkPaint& paint) : INHERITED(device) { fShader = paint.getShader(); SkASSERT(fShader); - SkASSERT(fShader->setContextHasBeenCalled()); fShader->ref(); + fShader->beginSession(); fShaderFlags = fShader->getFlags(); } SkShaderBlitter::~SkShaderBlitter() { - SkASSERT(fShader->setContextHasBeenCalled()); - fShader->endContext(); + fShader->endSession(); fShader->unref(); } diff --git a/src/core/SkColorFilter.cpp b/src/core/SkColorFilter.cpp index 156fb8fb96..6d9f4ba3a6 100644 --- a/src/core/SkColorFilter.cpp +++ b/src/core/SkColorFilter.cpp @@ -62,6 +62,16 @@ SkFilterShader::~SkFilterShader() { fShader->unref(); } +void SkFilterShader::beginSession() { + this->INHERITED::beginSession(); + fShader->beginSession(); +} + +void SkFilterShader::endSession() { + fShader->endSession(); + this->INHERITED::endSession(); +} + void SkFilterShader::flatten(SkFlattenableWriteBuffer& buffer) const { this->INHERITED::flatten(buffer); buffer.writeFlattenable(fShader); @@ -86,22 +96,8 @@ uint32_t SkFilterShader::getFlags() { bool SkFilterShader::setContext(const SkBitmap& device, const SkPaint& paint, const SkMatrix& matrix) { - // we need to keep the setContext/endContext calls balanced. If we return - // false, our endContext() will not be called. - - if (!this->INHERITED::setContext(device, paint, matrix)) { - return false; - } - if (!fShader->setContext(device, paint, matrix)) { - this->INHERITED::endContext(); - return false; - } - return true; -} - -void SkFilterShader::endContext() { - fShader->endContext(); - this->INHERITED::endContext(); + return this->INHERITED::setContext(device, paint, matrix) && + fShader->setContext(device, paint, matrix); } void SkFilterShader::shadeSpan(int x, int y, SkPMColor result[], int count) { diff --git a/src/core/SkComposeShader.cpp b/src/core/SkComposeShader.cpp index 92ffaf7d91..2fe7a20980 100644 --- a/src/core/SkComposeShader.cpp +++ b/src/core/SkComposeShader.cpp @@ -43,6 +43,18 @@ SkComposeShader::~SkComposeShader() { fShaderA->unref(); } +void SkComposeShader::beginSession() { + this->INHERITED::beginSession(); + fShaderA->beginSession(); + fShaderB->beginSession(); +} + +void SkComposeShader::endSession() { + fShaderA->endSession(); + fShaderB->endSession(); + this->INHERITED::endSession(); +} + class SkAutoAlphaRestore { public: SkAutoAlphaRestore(SkPaint* paint, uint8_t newAlpha) { @@ -69,10 +81,7 @@ void SkComposeShader::flatten(SkFlattenableWriteBuffer& buffer) const { /* We call setContext on our two worker shaders. However, we always let them see opaque alpha, and if the paint really is translucent, then we apply that after the fact. - - We need to keep the calls to setContext/endContext balanced, since if we - return false, our endContext() will not be called. - */ +*/ bool SkComposeShader::setContext(const SkBitmap& device, const SkPaint& paint, const SkMatrix& matrix) { @@ -89,25 +98,8 @@ bool SkComposeShader::setContext(const SkBitmap& device, SkAutoAlphaRestore restore(const_cast(&paint), 0xFF); - bool setContextA = fShaderA->setContext(device, paint, tmpM); - bool setContextB = fShaderB->setContext(device, paint, tmpM); - if (!setContextA || !setContextB) { - if (setContextB) { - fShaderB->endContext(); - } - else if (setContextA) { - fShaderA->endContext(); - } - this->INHERITED::endContext(); - return false; - } - return true; -} - -void SkComposeShader::endContext() { - fShaderB->endContext(); - fShaderA->endContext(); - this->INHERITED::endContext(); + return fShaderA->setContext(device, paint, tmpM) && + fShaderB->setContext(device, paint, tmpM); } // larger is better (fewer times we have to loop), but we shouldn't diff --git a/src/core/SkDraw.cpp b/src/core/SkDraw.cpp index f71187e74a..3035afe1ef 100644 --- a/src/core/SkDraw.cpp +++ b/src/core/SkDraw.cpp @@ -1228,6 +1228,13 @@ void SkDraw::drawBitmap(const SkBitmap& bitmap, const SkMatrix& prematrix, } } + // only lock the pixels if we passed the clip and bounder tests + SkAutoLockPixels alp(bitmap); + // after the lock, check if we are valid + if (!bitmap.readyToDraw()) { + return; + } + if (bitmap.getConfig() != SkBitmap::kA8_Config && just_translate(matrix, bitmap)) { int ix = SkScalarRound(matrix.getTranslateX()); @@ -2272,7 +2279,7 @@ public: bool setup(const SkPoint pts[], const SkColor colors[], int, int, int); - virtual void shadeSpan(int x, int y, SkPMColor dstC[], int count) SK_OVERRIDE; + virtual void shadeSpan(int x, int y, SkPMColor dstC[], int count); SK_DECLARE_PUBLIC_FLATTENABLE_DESERIALIZATION_PROCS(SkTriColorShader) @@ -2399,7 +2406,7 @@ void SkDraw::drawVertices(SkCanvas::VertexMode vmode, int count, if (NULL != colors) { if (NULL == textures) { // just colors (no texture) - shader = p.setShader(&triShader); + p.setShader(&triShader); } else { // colors * texture SkASSERT(shader); @@ -2414,7 +2421,6 @@ void SkDraw::drawVertices(SkCanvas::VertexMode vmode, int count, if (releaseMode) { xmode->unref(); } - shader = compose; } } @@ -2430,17 +2436,10 @@ void SkDraw::drawVertices(SkCanvas::VertexMode vmode, int count, savedLocalM = shader->getLocalMatrix(); } - // do we need this? triShader should have been installed in p, either - // directly or indirectly (using compose shader), so its setContext - // should have already been called. if (NULL != colors) { - SkASSERT(triShader.setContextHasBeenCalled()); -#if 0 - if (!triShader.setContext(*fBitmap, p, *fMatrix)) { colors = NULL; } -#endif } while (vertProc(&state)) { @@ -2449,7 +2448,6 @@ void SkDraw::drawVertices(SkCanvas::VertexMode vmode, int count, tempM.postConcat(savedLocalM); shader->setLocalMatrix(tempM); // need to recal setContext since we changed the local matrix - shader->endContext(); if (!shader->setContext(*fBitmap, p, *fMatrix)) { continue; } diff --git a/src/core/SkFilterShader.h b/src/core/SkFilterShader.h index ded3125adc..f637584718 100644 --- a/src/core/SkFilterShader.h +++ b/src/core/SkFilterShader.h @@ -17,12 +17,14 @@ public: SkFilterShader(SkShader* shader, SkColorFilter* filter); virtual ~SkFilterShader(); - virtual uint32_t getFlags() SK_OVERRIDE; - virtual bool setContext(const SkBitmap&, const SkPaint&, - const SkMatrix&) SK_OVERRIDE; - virtual void endContext() SK_OVERRIDE; - virtual void shadeSpan(int x, int y, SkPMColor[], int count) SK_OVERRIDE; - virtual void shadeSpan16(int x, int y, uint16_t[], int count) SK_OVERRIDE; + // override + virtual uint32_t getFlags(); + virtual bool setContext(const SkBitmap& device, const SkPaint& paint, + const SkMatrix& matrix); + virtual void shadeSpan(int x, int y, SkPMColor result[], int count); + virtual void shadeSpan16(int x, int y, uint16_t result[], int count); + virtual void beginSession(); + virtual void endSession(); SK_DECLARE_PUBLIC_FLATTENABLE_DESERIALIZATION_PROCS(SkFilterShader) diff --git a/src/core/SkShader.cpp b/src/core/SkShader.cpp index 706c1b79f5..2b20e3d31f 100644 --- a/src/core/SkShader.cpp +++ b/src/core/SkShader.cpp @@ -17,7 +17,7 @@ SK_DEFINE_INST_COUNT(SkShader) SkShader::SkShader() { fLocalMatrix.reset(); - SkDEBUGCODE(fInSetContext = false;) + SkDEBUGCODE(fInSession = false;) } SkShader::SkShader(SkFlattenableReadBuffer& buffer) @@ -28,11 +28,21 @@ SkShader::SkShader(SkFlattenableReadBuffer& buffer) fLocalMatrix.reset(); } - SkDEBUGCODE(fInSetContext = false;) + SkDEBUGCODE(fInSession = false;) } SkShader::~SkShader() { - SkASSERT(!fInSetContext); + SkASSERT(!fInSession); +} + +void SkShader::beginSession() { + SkASSERT(!fInSession); + SkDEBUGCODE(fInSession = true;) +} + +void SkShader::endSession() { + SkASSERT(fInSession); + SkDEBUGCODE(fInSession = false;) } void SkShader::flatten(SkFlattenableWriteBuffer& buffer) const { @@ -47,8 +57,6 @@ void SkShader::flatten(SkFlattenableWriteBuffer& buffer) const { bool SkShader::setContext(const SkBitmap& device, const SkPaint& paint, const SkMatrix& matrix) { - SkASSERT(!this->setContextHasBeenCalled()); - const SkMatrix* m = &matrix; SkMatrix total; @@ -60,17 +68,11 @@ bool SkShader::setContext(const SkBitmap& device, } if (m->invert(&fTotalInverse)) { fTotalInverseClass = (uint8_t)ComputeMatrixClass(fTotalInverse); - SkDEBUGCODE(fInSetContext = true;) return true; } return false; } -void SkShader::endContext() { - SkASSERT(fInSetContext); - SkDEBUGCODE(fInSetContext = false;) -} - SkShader::ShadeProc SkShader::asAShadeProc(void** ctx) { return NULL; } diff --git a/src/effects/gradients/SkGradientShader.cpp b/src/effects/gradients/SkGradientShader.cpp index 5b2a60e944..15c751052c 100644 --- a/src/effects/gradients/SkGradientShader.cpp +++ b/src/effects/gradients/SkGradientShader.cpp @@ -213,8 +213,6 @@ bool SkGradientShaderBase::setContext(const SkBitmap& device, const SkMatrix& inverse = this->getTotalInverse(); if (!fDstToIndex.setConcat(fPtsToUnit, inverse)) { - // need to keep our set/end context calls balanced. - this->INHERITED::endContext(); return false; } diff --git a/src/effects/gradients/SkGradientShaderPriv.h b/src/effects/gradients/SkGradientShaderPriv.h index 829d153d77..792cf6640f 100644 --- a/src/effects/gradients/SkGradientShaderPriv.h +++ b/src/effects/gradients/SkGradientShaderPriv.h @@ -91,6 +91,7 @@ public: int colorCount, SkShader::TileMode mode, SkUnitMapper* mapper); virtual ~SkGradientShaderBase(); + // overrides virtual bool setContext(const SkBitmap&, const SkPaint&, const SkMatrix&) SK_OVERRIDE; virtual uint32_t getFlags() SK_OVERRIDE { return fFlags; } virtual bool isOpaque() const SK_OVERRIDE; diff --git a/src/effects/gradients/SkTwoPointRadialGradient.cpp b/src/effects/gradients/SkTwoPointRadialGradient.cpp index 9aa923b2db..71b31260da 100644 --- a/src/effects/gradients/SkTwoPointRadialGradient.cpp +++ b/src/effects/gradients/SkTwoPointRadialGradient.cpp @@ -292,15 +292,16 @@ void SkTwoPointRadialGradient::shadeSpan(int x, int y, SkPMColor* dstCParam, } } -bool SkTwoPointRadialGradient::setContext( const SkBitmap& device, - const SkPaint& paint, - const SkMatrix& matrix){ - // For now, we might have divided by zero, so detect that - if (0 == fDiffRadius) { +bool SkTwoPointRadialGradient::setContext( + const SkBitmap& device, + const SkPaint& paint, + const SkMatrix& matrix){ + if (!this->INHERITED::setContext(device, paint, matrix)) { return false; } - if (!this->INHERITED::setContext(device, paint, matrix)) { + // For now, we might have divided by zero, so detect that + if (0 == fDiffRadius) { return false; }