From cee9dcb8377e1f85a7a232822a894464ea6ccddc Mon Sep 17 00:00:00 2001 From: "reed@google.com" Date: Fri, 13 Sep 2013 16:04:49 +0000 Subject: [PATCH] start to remove lockPixels from bitmapshader BUG= R=scroggo@google.com Review URL: https://codereview.chromium.org/23591030 git-svn-id: http://skia.googlecode.com/svn/trunk@11258 2bbb7eff-a529-9590-31e7-b0007b416f81 --- include/core/SkPixelRef.h | 36 ++++++++++++ src/core/SkBitmapProcShader.cpp | 30 +++++++--- src/core/SkBitmapProcState.cpp | 98 ++++++++++++++++++++++----------- src/core/SkBitmapProcState.h | 9 ++- src/core/SkPixelRef.cpp | 8 +++ src/lazy/SkLazyPixelRef.cpp | 53 ++++++++++++++++++ src/lazy/SkLazyPixelRef.h | 2 + 7 files changed, 195 insertions(+), 41 deletions(-) diff --git a/include/core/SkPixelRef.h b/include/core/SkPixelRef.h index 0487e42610..64e08765be 100644 --- a/include/core/SkPixelRef.h +++ b/include/core/SkPixelRef.h @@ -141,6 +141,37 @@ public: return this->onRefEncodedData(); } + /** + * Experimental -- tells the caller if it is worth it to call decodeInto(). + * Just an optimization at this point, to avoid checking the cache first. + * We may remove/change this call in the future. + */ + bool implementsDecodeInto() { + return this->onImplementsDecodeInto(); + } + + /** + * Return a decoded instance of this pixelRef in bitmap. If this cannot be + * done, return false and the bitmap parameter is ignored/unchanged. + * + * pow2 is the requeste power-of-two downscale that the caller needs. This + * can be ignored, and the "original" size can be returned, but if the + * underlying codec can efficiently return a smaller size, that should be + * done. Some examples: + * + * To request the "base" version (original scale), pass 0 for pow2 + * To request 1/2 scale version (1/2 width, 1/2 height), pass 1 for pow2 + * To request 1/4 scale version (1/4 width, 1/4 height), pass 2 for pow2 + * ... + * + * If this returns true, then bitmap must be "locked" such that + * bitmap->getPixels() will return the correct address. + */ + bool decodeInto(int pow2, SkBitmap* bitmap) { + SkASSERT(pow2 >= 0); + return this->onDecodeInto(pow2, bitmap); + } + /** Are we really wrapping a texture instead of a bitmap? */ virtual GrTexture* getTexture() { return NULL; } @@ -190,6 +221,11 @@ protected: /** Default impl returns true */ virtual bool onLockPixelsAreWritable() const; + // returns false; + virtual bool onImplementsDecodeInto(); + // returns false; + virtual bool onDecodeInto(int pow2, SkBitmap* bitmap); + /** * For pixelrefs that don't have access to their raw pixels, they may be * able to make a copy of them (e.g. if the pixels are on the GPU). diff --git a/src/core/SkBitmapProcShader.cpp b/src/core/SkBitmapProcShader.cpp index 3f657156f4..ded1b72009 100644 --- a/src/core/SkBitmapProcShader.cpp +++ b/src/core/SkBitmapProcShader.cpp @@ -80,24 +80,37 @@ bool SkBitmapProcShader::isOpaque() const { return fRawBitmap.isOpaque(); } +static bool valid_for_drawing(const SkBitmap& bm) { + if (0 == bm.width() || 0 == bm.height()) { + return false; // nothing to draw + } + if (NULL == bm.pixelRef()) { + return false; // no pixels to read + } + if (SkBitmap::kIndex8_Config == bm.config()) { + // ugh, I have to lock-pixels to inspect the colortable + SkAutoLockPixels alp(bm); + if (!bm.getColorTable()) { + return false; + } + } + return true; +} + bool SkBitmapProcShader::setContext(const SkBitmap& device, const SkPaint& paint, const SkMatrix& matrix) { + if (!fRawBitmap.getTexture() && !valid_for_drawing(fRawBitmap)) { + return false; + } + // do this first, so we have a correct inverse matrix if (!this->INHERITED::setContext(device, paint, matrix)) { return false; } fState.fOrigBitmap = fRawBitmap; - fState.fOrigBitmap.lockPixels(); - if (!fState.fOrigBitmap.getTexture() && !fState.fOrigBitmap.readyToDraw()) { - fState.fOrigBitmap.unlockPixels(); - this->INHERITED::endContext(); - return false; - } - if (!fState.chooseProcs(this->getTotalInverse(), paint)) { - fState.fOrigBitmap.unlockPixels(); this->INHERITED::endContext(); return false; } @@ -147,7 +160,6 @@ bool SkBitmapProcShader::setContext(const SkBitmap& device, } void SkBitmapProcShader::endContext() { - fState.fOrigBitmap.unlockPixels(); fState.endContext(); this->INHERITED::endContext(); } diff --git a/src/core/SkBitmapProcState.cpp b/src/core/SkBitmapProcState.cpp index 25f90d60af..9ecfc2280b 100644 --- a/src/core/SkBitmapProcState.cpp +++ b/src/core/SkBitmapProcState.cpp @@ -13,6 +13,7 @@ #include "SkUtilsArm.h" #include "SkBitmapScaler.h" #include "SkMipMap.h" +#include "SkPixelRef.h" #include "SkScaledImageCache.h" #if !SK_ARM_NEON_IS_NONE @@ -109,15 +110,14 @@ static SkScalar effective_matrix_scale_sqrd(const SkMatrix& mat) { // the portion of the image that we're going to need. This will complicate // the interface to the cache, but might be well worth it. -void SkBitmapProcState::possiblyScaleImage() { +bool SkBitmapProcState::possiblyScaleImage() { + SkASSERT(NULL == fBitmap); + SkASSERT(NULL == fScaledCacheID); if (fFilterLevel <= SkPaint::kLow_FilterLevel) { - // none or low (bilerp) does not need to look any further - return; + return false; } - // STEP 1: Highest quality direct scale? - // Check to see if the transformation matrix is simple, and if we're // doing high quality scaling. If so, do the bitmap scale here and // remove the scaling component from the matrix. @@ -129,7 +129,6 @@ void SkBitmapProcState::possiblyScaleImage() { SkScalar invScaleX = fInvMatrix.getScaleX(); SkScalar invScaleY = fInvMatrix.getScaleY(); - SkASSERT(NULL == fScaledCacheID); fScaledCacheID = SkScaledImageCache::FindAndLock(fOrigBitmap, invScaleX, invScaleY, &fScaledBitmap); @@ -151,7 +150,7 @@ void SkBitmapProcState::possiblyScaleImage() { simd)) { // we failed to create fScaledBitmap, so just return and let // the scanline proc handle it. - return; + return true; } fScaledCacheID = SkScaledImageCache::AddAndLock(fOrigBitmap, @@ -159,25 +158,19 @@ void SkBitmapProcState::possiblyScaleImage() { invScaleY, fScaledBitmap); } - fScaledBitmap.lockPixels(); - + fScaledBitmap.lockPixels(); // wonder if Resize() should have locked this fBitmap = &fScaledBitmap; // set the inv matrix type to translate-only; - fInvMatrix.setTranslate(fInvMatrix.getTranslateX() / fInvMatrix.getScaleX(), fInvMatrix.getTranslateY() / fInvMatrix.getScaleY()); // no need for any further filtering; we just did it! - fFilterLevel = SkPaint::kNone_FilterLevel; - - return; + return true; } /* - * If we get here, the caller has requested either Med or High filter-level - * * If High, then our special-case for scale-only did not take, and so we * have to make a choice: * 1. fall back on mipmaps + bilerp @@ -202,7 +195,7 @@ void SkBitmapProcState::possiblyScaleImage() { const SkScalar bicubicLimit = SkFloatToScalar(4.0f); const SkScalar bicubicLimitSqd = bicubicLimit * bicubicLimit; if (scaleSqd < bicubicLimitSqd) { // use bicubic scanline - return; + return false; } // else set the filter-level to Medium, since we're scaling down and @@ -247,15 +240,61 @@ void SkBitmapProcState::possiblyScaleImage() { level.fRowBytes); fScaledBitmap.setPixels(level.fPixels); fBitmap = &fScaledBitmap; + fFilterLevel = SkPaint::kLow_FilterLevel; + return true; } } } + return false; +} + +static bool get_locked_pixels(const SkBitmap& src, int pow2, SkBitmap* dst) { + SkPixelRef* pr = src.pixelRef(); + if (pr && pr->decodeInto(pow2, dst)) { + return true; + } + /* - * At this point, we may or may not have built a mipmap. Regardless, we - * now fall back on Low so will bilerp whatever fBitmap now points at. + * If decodeInto() fails, it is possibe that we have an old subclass that + * does not, or cannot, implement that. In that case we fall back to the + * older protocol of having the pixelRef handle the caching for us. */ - fFilterLevel = SkPaint::kLow_FilterLevel; + *dst = src; + dst->lockPixels(); + return SkToBool(dst->getPixels()); +} + +bool SkBitmapProcState::lockBaseBitmap() { + SkPixelRef* pr = fOrigBitmap.pixelRef(); + + if (pr->isLocked() || !pr->implementsDecodeInto()) { + // fast-case, no need to look in our cache + fScaledBitmap = fOrigBitmap; + } else { + fScaledCacheID = SkScaledImageCache::FindAndLock(fOrigBitmap, + SK_Scalar1, SK_Scalar1, + &fScaledBitmap); + if (NULL == fScaledCacheID) { + if (!get_locked_pixels(fOrigBitmap, 0, &fScaledBitmap)) { + return false; + } + + // TODO: if fScaled comes back at a different width/height than fOrig, + // we need to update the matrix we are using to sample from this guy. + + fScaledCacheID = SkScaledImageCache::AddAndLock(fOrigBitmap, + SK_Scalar1, SK_Scalar1, + fScaledBitmap); + if (!fScaledCacheID) { + fScaledBitmap.reset(); + return false; + } + } + } + fScaledBitmap.lockPixels(); // just 'cause the cache made a copy :( + fBitmap = &fScaledBitmap; + return true; } void SkBitmapProcState::endContext() { @@ -277,17 +316,10 @@ SkBitmapProcState::~SkBitmapProcState() { } bool SkBitmapProcState::chooseProcs(const SkMatrix& inv, const SkPaint& paint) { - if (fOrigBitmap.width() == 0 || fOrigBitmap.height() == 0) { - return false; - } + SkASSERT(fOrigBitmap.width() && fOrigBitmap.height()); - fBitmap = &fOrigBitmap; + fBitmap = NULL; fInvMatrix = inv; - - // initialize our filter quality to the one requested by the caller. - // We may downgrade it later if we determine that we either don't need - // or can't provide as high a quality filtering as the user requested. - fFilterLevel = paint.getFilterLevel(); // possiblyScaleImage will look to see if it can rescale the image as a @@ -295,8 +327,13 @@ bool SkBitmapProcState::chooseProcs(const SkMatrix& inv, const SkPaint& paint) { // a nearby mipmap level. If it does, it will adjust the working // matrix as well as the working bitmap. It may also adjust the filter // quality to avoid re-filtering an already perfectly scaled image. - - this->possiblyScaleImage(); + if (!this->possiblyScaleImage()) { + if (!this->lockBaseBitmap()) { + return false; + } + } + + SkASSERT(fBitmap); bool trivialMatrix = (fInvMatrix.getType() & ~SkMatrix::kTranslate_Mask) == 0; bool clampClamp = SkShader::kClamp_TileMode == fTileModeX && @@ -322,7 +359,6 @@ bool SkBitmapProcState::chooseProcs(const SkMatrix& inv, const SkPaint& paint) { SkScalar tx = -SkScalarRoundToScalar(forward.getTranslateX()); SkScalar ty = -SkScalarRoundToScalar(forward.getTranslateY()); fInvMatrix.setTranslate(tx, ty); - } } } diff --git a/src/core/SkBitmapProcState.h b/src/core/SkBitmapProcState.h index 2522a69e7f..d5a354e36a 100644 --- a/src/core/SkBitmapProcState.h +++ b/src/core/SkBitmapProcState.h @@ -13,6 +13,7 @@ #include "SkBitmap.h" #include "SkBitmapFilter.h" #include "SkMatrix.h" +#include "SkPaint.h" #include "SkScaledImageCache.h" #define FractionalInt_IS_64BIT @@ -160,7 +161,13 @@ private: bool chooseProcs(const SkMatrix& inv, const SkPaint&); ShaderProc32 chooseShaderProc32(); - void possiblyScaleImage(); + // returns false if we did not try to scale the image. In that case, we + // will need to "lock" its pixels some other way. + bool possiblyScaleImage(); + + // returns false if we failed to "lock" the pixels at all. Typically this + // means we have to abort the shader. + bool lockBaseBitmap(); SkBitmapFilter* fBitmapFilter; diff --git a/src/core/SkPixelRef.cpp b/src/core/SkPixelRef.cpp index bc548a22a8..08775f24de 100644 --- a/src/core/SkPixelRef.cpp +++ b/src/core/SkPixelRef.cpp @@ -167,6 +167,14 @@ bool SkPixelRef::onLockPixelsAreWritable() const { return true; } +bool SkPixelRef::onImplementsDecodeInto() { + return false; +} + +bool SkPixelRef::onDecodeInto(int pow2, SkBitmap* bitmap) { + return false; +} + uint32_t SkPixelRef::getGenerationID() const { if (0 == fGenerationID) { fGenerationID = SkNextPixelRefGenerationID(); diff --git a/src/lazy/SkLazyPixelRef.cpp b/src/lazy/SkLazyPixelRef.cpp index dc9aef9f06..870056091f 100644 --- a/src/lazy/SkLazyPixelRef.cpp +++ b/src/lazy/SkLazyPixelRef.cpp @@ -145,3 +145,56 @@ SkData* SkLazyPixelRef::onRefEncodedData() { fData->ref(); return fData; } + +#include "SkImagePriv.h" + +static bool init_from_info(SkBitmap* bm, const SkImage::Info& info, + size_t rowBytes) { + bool isOpaque; + SkBitmap::Config config = SkImageInfoToBitmapConfig(info, &isOpaque); + if (SkBitmap::kNo_Config == config) { + return false; + } + + bm->setConfig(config, info.fWidth, info.fHeight, rowBytes); + bm->setIsOpaque(isOpaque); + return bm->allocPixels(); +} + +bool SkLazyPixelRef::onImplementsDecodeInto() { + return true; +} + +bool SkLazyPixelRef::onDecodeInto(int pow2, SkBitmap* bitmap) { + SkASSERT(fData != NULL && fData->size() > 0); + if (fErrorInDecoding) { + return false; + } + + SkImage::Info info; + // Determine the size of the image in order to determine how much memory to allocate. + // FIXME: As an optimization, only do this part once. + fErrorInDecoding = !fDecodeProc(fData->data(), fData->size(), &info, NULL); + if (fErrorInDecoding) { + return false; + } + + SkBitmapFactory::Target target; + (void)ComputeMinRowBytesAndSize(info, &target.fRowBytes); + + SkBitmap tmp; + if (!init_from_info(&tmp, info, target.fRowBytes)) { + return false; + } + + target.fAddr = tmp.getPixels(); + fErrorInDecoding = !fDecodeProc(fData->data(), fData->size(), &info, &target); + if (fErrorInDecoding) { + return false; + } + + *bitmap = tmp; + return true; +} + + diff --git a/src/lazy/SkLazyPixelRef.h b/src/lazy/SkLazyPixelRef.h index fd41dd4682..c51675dd5f 100644 --- a/src/lazy/SkLazyPixelRef.h +++ b/src/lazy/SkLazyPixelRef.h @@ -61,6 +61,8 @@ protected: virtual void onUnlockPixels() SK_OVERRIDE; virtual bool onLockPixelsAreWritable() const SK_OVERRIDE { return false; } virtual SkData* onRefEncodedData() SK_OVERRIDE; + virtual bool onImplementsDecodeInto() SK_OVERRIDE; + virtual bool onDecodeInto(int pow2, SkBitmap*) SK_OVERRIDE; private: bool fErrorInDecoding;