From 36d08c5c90c7607cd559769f7a9c2b3364eeba85 Mon Sep 17 00:00:00 2001 From: "halcanary@google.com" Date: Thu, 5 Dec 2013 14:00:03 +0000 Subject: [PATCH] SkCachingPixelRef to use SkImageGenerator - Remove SkLazyCachingPixelRef class. - Refactor unit tests. BUG= R=reed@google.com, scroggo@google.com Review URL: https://codereview.chromium.org/84083002 git-svn-id: http://skia.googlecode.com/svn/trunk@12505 2bbb7eff-a529-9590-31e7-b0007b416f81 --- gyp/core.gypi | 2 - src/lazy/SkCachingPixelRef.cpp | 100 +++++++++---------- src/lazy/SkCachingPixelRef.h | 77 +++++++-------- src/lazy/SkLazyCachingPixelRef.cpp | 61 ------------ src/lazy/SkLazyCachingPixelRef.h | 98 ------------------- tests/CachedDecodingPixelRefTest.cpp | 179 ++++++++++------------------------- 6 files changed, 127 insertions(+), 390 deletions(-) delete mode 100644 src/lazy/SkLazyCachingPixelRef.cpp delete mode 100644 src/lazy/SkLazyCachingPixelRef.h diff --git a/gyp/core.gypi b/gyp/core.gypi index b3f76ea..4bb4d51 100644 --- a/gyp/core.gypi +++ b/gyp/core.gypi @@ -322,8 +322,6 @@ '<(skia_src_path)/lazy/SkCachingPixelRef.cpp', '<(skia_src_path)/lazy/SkCachingPixelRef.h', - '<(skia_src_path)/lazy/SkLazyCachingPixelRef.cpp', - '<(skia_src_path)/lazy/SkLazyCachingPixelRef.h', # Path ops '<(skia_include_path)/pathops/SkPathOps.h', diff --git a/src/lazy/SkCachingPixelRef.cpp b/src/lazy/SkCachingPixelRef.cpp index d12b7cf..6c9da8f 100644 --- a/src/lazy/SkCachingPixelRef.cpp +++ b/src/lazy/SkCachingPixelRef.cpp @@ -8,66 +8,71 @@ #include "SkCachingPixelRef.h" #include "SkScaledImageCache.h" -SkCachingPixelRef::SkCachingPixelRef() - : fErrorInDecoding(false) - , fScaledCacheId(NULL) { - memset(&fInfo, 0xFF, sizeof(fInfo)); -} -SkCachingPixelRef::~SkCachingPixelRef() { - SkASSERT(NULL == fScaledCacheId); - // Assert always unlock before unref. -} -bool SkCachingPixelRef::getInfo(SkImageInfo* info) { - SkASSERT(info != NULL); - if (fErrorInDecoding) { - return false; // Don't try again. - } - if (fInfo.fWidth < 0) { - SkImageInfo tmp; - if (!this->onDecodeInfo(&tmp)) { - fErrorInDecoding = true; - return false; - } - SkASSERT(tmp.fWidth >= 0); - fInfo = tmp; +bool SkCachingPixelRef::Install(SkImageGenerator* generator, + SkBitmap* dst) { + SkImageInfo info; + SkASSERT(generator != NULL); + SkASSERT(dst != NULL); + if ((NULL == generator) + || !(generator->getInfo(&info)) + || !dst->setConfig(info, 0)) { + SkDELETE(generator); + return false; } - *info = fInfo; + SkAutoTUnref ref(SkNEW_ARGS(SkCachingPixelRef, + (generator, + info, + dst->rowBytes()))); + dst->setPixelRef(ref); return true; } -bool SkCachingPixelRef::configure(SkBitmap* bitmap) { - SkASSERT(bitmap != NULL); - SkImageInfo info; - if (!this->getInfo(&info)) { - return false; - } - return bitmap->setConfig(info, 0); +SkCachingPixelRef::SkCachingPixelRef(SkImageGenerator* generator, + const SkImageInfo& info, + size_t rowBytes) + : fImageGenerator(generator) + , fErrorInDecoding(false) + , fScaledCacheId(NULL) + , fInfo(info) + , fRowBytes(rowBytes) { + SkASSERT(fImageGenerator != NULL); +} +SkCachingPixelRef::~SkCachingPixelRef() { + SkDELETE(fImageGenerator); + SkASSERT(NULL == fScaledCacheId); + // Assert always unlock before unref. } void* SkCachingPixelRef::onLockPixels(SkColorTable** colorTable) { (void)colorTable; - SkImageInfo info; - if (!this->getInfo(&info)) { - return NULL; + if (fErrorInDecoding) { + return NULL; // don't try again. } SkBitmap bitmap; - + SkASSERT(NULL == fScaledCacheId); fScaledCacheId = SkScaledImageCache::FindAndLock(this->getGenerationID(), - info.fWidth, - info.fHeight, + fInfo.fWidth, + fInfo.fHeight, &bitmap); if (NULL == fScaledCacheId) { // Cache has been purged, must re-decode. - if (!this->onDecodeInto(0, &bitmap)) { + if ((!bitmap.setConfig(fInfo, fRowBytes)) || !bitmap.allocPixels()) { + fErrorInDecoding = true; + return NULL; + } + SkAutoLockPixels autoLockPixels(bitmap); + if (!fImageGenerator->getPixels(fInfo, bitmap.getPixels(), fRowBytes)) { + fErrorInDecoding = true; return NULL; } fScaledCacheId = SkScaledImageCache::AddAndLock(this->getGenerationID(), - info.fWidth, - info.fHeight, + fInfo.fWidth, + fInfo.fHeight, bitmap); SkASSERT(fScaledCacheId != NULL); } + // Now bitmap should contain a concrete PixelRef of the decoded // image. SkAutoLockPixels autoLockPixels(bitmap); @@ -92,20 +97,3 @@ void SkCachingPixelRef::onUnlockPixels() { } } -bool SkCachingPixelRef::onDecodeInto(int pow2, SkBitmap* bitmap) { - SkASSERT(bitmap != NULL); - SkBitmap tmp; - SkImageInfo info; - // TODO(halcanary) - Enable SkCachingPixelRef to use a custom - // allocator. `tmp.allocPixels(fAllocator, NULL)` - if (!(this->configure(&tmp) && tmp.allocPixels())) { - return false; - } - SkAssertResult(this->getInfo(&info)); // since configure() succeeded. - if (!this->onDecodePixels(info, tmp.getPixels(), tmp.rowBytes())) { - fErrorInDecoding = true; - return false; - } - *bitmap = tmp; - return true; -} diff --git a/src/lazy/SkCachingPixelRef.h b/src/lazy/SkCachingPixelRef.h index db968df..81092dc 100644 --- a/src/lazy/SkCachingPixelRef.h +++ b/src/lazy/SkCachingPixelRef.h @@ -8,7 +8,8 @@ #ifndef SkCachingPixelRef_DEFINED #define SkCachingPixelRef_DEFINED -#include "SkImage.h" +#include "SkImageInfo.h" +#include "SkImageGenerator.h" #include "SkPixelRef.h" class SkColorTable; @@ -20,61 +21,51 @@ class SkColorTable; * or be destroyed before the next lock. If so, onLockPixels will * attempt to re-decode. * - * Decoding is handled by the pure-virtual functions onDecodeInfo() - * and onDecodePixels(). Subclasses of this class need only provide - * those two functions. + * Decoding is handled by the SkImageGenerator */ class SkCachingPixelRef : public SkPixelRef { public: - SkCachingPixelRef(); - virtual ~SkCachingPixelRef(); + /** + * Takes ownership of SkImageGenerator. If this method fails for + * whatever reason, it will return false and immediatetely delete + * the generator. If it succeeds, it will modify destination + * bitmap. + * + * If Install fails or when the SkCachingPixelRef that is + * installed into destination is destroyed, it will call + * SkDELETE() on the generator. Therefore, generator should be + * allocated with SkNEW() or SkNEW_ARGS(). + */ + static bool Install(SkImageGenerator* gen, SkBitmap* dst); protected: + virtual ~SkCachingPixelRef(); virtual void* onLockPixels(SkColorTable** colorTable) SK_OVERRIDE; virtual void onUnlockPixels() SK_OVERRIDE; virtual bool onLockPixelsAreWritable() const SK_OVERRIDE { return false; } - virtual bool onImplementsDecodeInto() SK_OVERRIDE { return true; } - virtual bool onDecodeInto(int pow2, SkBitmap*) SK_OVERRIDE; - /** - * Configure the supplied bitmap for this pixelRef, based on - * information provided by onDecodeInfo(). Does not set the - * bitmap's pixelRef. */ - bool configure(SkBitmap* bitmap); - - /** - * Cache info from onDecodeInfo(). Returns false on failure. - */ - bool getInfo(SkImageInfo* info); - - /** - * Return some information about the pixels, allowing this class - * to allocate pixels. @return false if anything goes wrong. - */ - virtual bool onDecodeInfo(SkImageInfo* info) = 0; - /** - * Decode into the given pixels, a block of memory of size - * (info.fHeight - 1) * rowBytes + (info.fWidth * bytesPerPixel) - * - * @param info Should be identical to the info returned by - * onDecodeInfo so that the implementation can confirm - * that the caller knows what it is asking for (config, - * size). Thiscontract also allows the caller to specify - * different output-configs, which the implementation can - * decide to support or not. - * - * @return false if anything goes wrong. - */ - virtual bool onDecodePixels(const SkImageInfo& info, - void* pixels, - size_t rowBytes) = 0; + virtual SkData* onRefEncodedData() SK_OVERRIDE { + return fImageGenerator->refEncodedData(); + } + // No need to flatten this object. When flattening an SkBitmap, + // SkOrderedWriteBuffer will check the encoded data and write that + // instead. + // Future implementations of SkFlattenableWriteBuffer will need to + // special case for onRefEncodedData as well. + SK_DECLARE_UNFLATTENABLE_OBJECT() private: - bool fErrorInDecoding; - void* fScaledCacheId; - SkImageInfo fInfo; + SkImageGenerator* const fImageGenerator; + bool fErrorInDecoding; + void* fScaledCacheId; + const SkImageInfo fInfo; + const size_t fRowBytes; + SkCachingPixelRef(SkImageGenerator* imageGenerator, + const SkImageInfo& info, + size_t rowBytes); typedef SkPixelRef INHERITED; }; #endif // SkCachingPixelRef_DEFINED + diff --git a/src/lazy/SkLazyCachingPixelRef.cpp b/src/lazy/SkLazyCachingPixelRef.cpp deleted file mode 100644 index 730b5f7..0000000 --- a/src/lazy/SkLazyCachingPixelRef.cpp +++ /dev/null @@ -1,61 +0,0 @@ -/* - * Copyright 2013 Google Inc. - * - * Use of this source code is governed by a BSD-style license that can be - * found in the LICENSE file. - */ - -#include "Sk64.h" -#include "SkColorTable.h" -#include "SkData.h" -#include "SkImageDecoder.h" -#include "SkImagePriv.h" -#include "SkLazyCachingPixelRef.h" -#include "SkPostConfig.h" - -SkLazyCachingPixelRef::SkLazyCachingPixelRef(SkData* data, - SkBitmapFactory::DecodeProc proc) - : fDecodeProc(proc) { - if (NULL == data) { - fData = SkData::NewEmpty(); - } else { - fData = data; - fData->ref(); - } - if (NULL == fDecodeProc) { // use a reasonable default. - fDecodeProc = SkImageDecoder::DecodeMemoryToTarget; - } - this->setImmutable(); -} - -SkLazyCachingPixelRef::~SkLazyCachingPixelRef() { - SkASSERT(fData != NULL); - fData->unref(); -} - -bool SkLazyCachingPixelRef::onDecodeInfo(SkImageInfo* info) { - SkASSERT(info); - return fDecodeProc(fData->data(), fData->size(), info, NULL); -} - -bool SkLazyCachingPixelRef::onDecodePixels(const SkImageInfo& passedInfo, - void* pixels, size_t rowBytes) { - SkASSERT(pixels); - SkImageInfo info; - if (!this->getInfo(&info)) { - return false; - } - if (passedInfo != info) { - return false; // This implementation can not handle this case. - } - SkBitmapFactory::Target target = {pixels, rowBytes}; - return fDecodeProc(fData->data(), fData->size(), &info, &target); -} - -bool SkLazyCachingPixelRef::Install(SkBitmapFactory::DecodeProc proc, - SkData* data, - SkBitmap* destination) { - SkAutoTUnref ref( - SkNEW_ARGS(SkLazyCachingPixelRef, (data, proc))); - return ref->configure(destination) && destination->setPixelRef(ref); -} diff --git a/src/lazy/SkLazyCachingPixelRef.h b/src/lazy/SkLazyCachingPixelRef.h deleted file mode 100644 index a9d2dad..0000000 --- a/src/lazy/SkLazyCachingPixelRef.h +++ /dev/null @@ -1,98 +0,0 @@ -/* - * Copyright 2013 Google Inc. - * - * Use of this source code is governed by a BSD-style license that can be - * found in the LICENSE file. - */ - -#ifndef SkLazyCachingPixelRef_DEFINED -#define SkLazyCachingPixelRef_DEFINED - -#include "SkBitmapFactory.h" -#include "SkCachingPixelRef.h" - -class SkData; - -/** - * PixelRef which defers decoding until SkBitmap::lockPixels() is - * called. Makes use of a supplied decode procedure. Will decode at - * the procedure's preferred size. - */ -class SkLazyCachingPixelRef : public SkCachingPixelRef { -public: - /** - * @param data Encoded data representing the pixels. NULL is - * equivalent to an empty data, and will be passed to - * DecodeProc with length zero. - * - * @param procedure Called to decode the pixels when - * needed. If NULL, use SkImageDecoder::DecodeMemoryToTarget. - */ - SkLazyCachingPixelRef(SkData* data, - SkBitmapFactory::DecodeProc procedure); - - virtual ~SkLazyCachingPixelRef(); - - virtual SkData* onRefEncodedData() SK_OVERRIDE { return SkSafeRef(fData); } - - /** - * A simplified version of SkBitmapFactory. Installs a new - * SkLazyCachingPixelRef into the provided bitmap. Will - * immediately call onDecodeInfo() to configure the bitmap, but - * will defer decoding until the first time the bitmap's pixels - * are locked. - * - * @param data Encoded data representing the pixels. NULL is - * equivalent to an empty data, and will be passed to - * DecodeProc with length zero. - * - * @param procedure Called to decode the pixels when - * needed. If NULL, use SkImageDecoder::DecodeMemoryToTarget. - * - * @param destination Bitmap that will be modified on success. - * - * @returns true on success. - */ - static bool Install(SkBitmapFactory::DecodeProc procedure, - SkData* data, - SkBitmap* destination); - - // No need to flatten this object. When flattening an SkBitmap, - // SkOrderedWriteBuffer will check the encoded data and write that - // instead. - // Future implementations of SkFlattenableWriteBuffer will need to - // special case for onRefEncodedData as well. - SK_DECLARE_UNFLATTENABLE_OBJECT() - -protected: - /** - * Return some information about the pixels, allowing this class - * to allocate pixels. @return false if anything goes wrong. - * - * This implementation calls SkBitmapFactory::DecodeProc with a - * NULL target. - */ - virtual bool onDecodeInfo(SkImageInfo* info) SK_OVERRIDE; - /** - * Decode into the given pixels, a block of memory of size - * (info.fHeight * rowBytes) bytes. - * - * @param info Should be identical to the info returned by - * onDecodeInfo so that the implementation can confirm - * that the caller knows what its asking for (config, - * size). - * - * @return false if anything goes wrong. - */ - virtual bool onDecodePixels(const SkImageInfo& info, - void* pixels, - size_t rowBytes) SK_OVERRIDE; - -private: - SkData* fData; - SkBitmapFactory::DecodeProc fDecodeProc; - - typedef SkCachingPixelRef INHERITED; -}; - -#endif // SkLazyCachingPixelRef_DEFINED diff --git a/tests/CachedDecodingPixelRefTest.cpp b/tests/CachedDecodingPixelRefTest.cpp index 2d8a0e7..af1df10 100644 --- a/tests/CachedDecodingPixelRefTest.cpp +++ b/tests/CachedDecodingPixelRefTest.cpp @@ -12,8 +12,8 @@ #include "SkForceLinking.h" #include "SkImageDecoder.h" #include "SkImagePriv.h" -#include "SkLazyCachingPixelRef.h" #include "SkLazyPixelRef.h" +#include "SkCachingPixelRef.h" #include "SkScaledImageCache.h" #include "SkStream.h" @@ -109,15 +109,13 @@ static void compare_bitmaps(skiatest::Reporter* reporter, } -typedef void(*CompareEncodedToOriginal)(skiatest::Reporter* reporter, - SkData* encoded, - const SkBitmap& original, - bool pixelPerfect); +typedef bool (*InstallEncoded)(SkData* encoded, SkBitmap* dst); + /** - this function tests three differently encoded images against the - original bitmap */ + This function tests three differently encoded images against the + original bitmap */ static void test_three_encodings(skiatest::Reporter* reporter, - CompareEncodedToOriginal comp) { + InstallEncoded install) { SkBitmap original; make_test_image(&original); REPORTER_ASSERT(reporter, !original.empty()); @@ -134,146 +132,67 @@ static void test_three_encodings(skiatest::Reporter* reporter, SkImageEncoder::Type type = types[i]; SkAutoDataUnref encoded(create_data_from_bitmap(original, type)); REPORTER_ASSERT(reporter, encoded.get() != NULL); - if (NULL != encoded.get()) { - bool comparePixels = (SkImageEncoder::kPNG_Type == type); - comp(reporter, encoded, original, comparePixels); + if (NULL == encoded.get()) { + continue; + } + SkBitmap lazy; + bool installSuccess = install(encoded.get(), &lazy); + REPORTER_ASSERT(reporter, installSuccess); + if (!installSuccess) { + continue; } + REPORTER_ASSERT(reporter, NULL == lazy.getPixels()); + { + SkAutoLockPixels autoLockPixels(lazy); // now pixels are good. + REPORTER_ASSERT(reporter, NULL != lazy.getPixels()); + if (NULL == lazy.getPixels()) { + continue; + } + } + // pixels should be gone! + REPORTER_ASSERT(reporter, NULL == lazy.getPixels()); + { + SkAutoLockPixels autoLockPixels(lazy); // now pixels are good. + REPORTER_ASSERT(reporter, NULL != lazy.getPixels()); + if (NULL == lazy.getPixels()) { + continue; + } + } + bool comparePixels = (SkImageEncoder::kPNG_Type == type); + compare_bitmaps(reporter, original, lazy, comparePixels); } } + +//////////////////////////////////////////////////////////////////////////////// /** * This checks to see that a SkLazyPixelRef works as advertised. */ -static void compare_with_skLazyPixelRef(skiatest::Reporter* reporter, - SkData* encoded, - const SkBitmap& original, - bool comparePixels) { - SkBitmap lazy; +bool install_skLazyPixelRef(SkData* encoded, SkBitmap* dst) { static const SkBitmapFactory::DecodeProc decoder = &(SkImageDecoder::DecodeMemoryToTarget); - bool success = simple_bitmap_factory(decoder, encoded, &lazy); - REPORTER_ASSERT(reporter, success); - - REPORTER_ASSERT(reporter, NULL == lazy.getPixels()); - { - SkAutoLockPixels autoLockPixels(lazy); // now pixels are good. - REPORTER_ASSERT(reporter, NULL != lazy.getPixels()); - } - // pixels should be gone! - REPORTER_ASSERT(reporter, NULL == lazy.getPixels()); - { - SkAutoLockPixels autoLockPixels(lazy); // now pixels are good. - REPORTER_ASSERT(reporter, NULL != lazy.getPixels()); - } - compare_bitmaps(reporter, original, lazy, comparePixels); + return simple_bitmap_factory(decoder, encoded, dst); } DEF_TEST(LazyPixelRef, reporter) { - test_three_encodings(reporter, compare_with_skLazyPixelRef); + test_three_encodings(reporter, install_skLazyPixelRef); } - - +//////////////////////////////////////////////////////////////////////////////// /** - * This checks to see that a SkLazyCachedPixelRef works as advertised. + * This checks to see that a SkCachingPixelRef works as advertised. */ - -static void compare_with_skLazyCachedPixelRef(skiatest::Reporter* reporter, - SkData* encoded, - const SkBitmap& original, - bool comparePixels) { - SkBitmap lazy; - static const SkBitmapFactory::DecodeProc decoder = - &(SkImageDecoder::DecodeMemoryToTarget); - bool success = SkLazyCachingPixelRef::Install(decoder, encoded, &lazy); - REPORTER_ASSERT(reporter, success); - - REPORTER_ASSERT(reporter, NULL == lazy.getPixels()); - { - SkAutoLockPixels autoLockPixels(lazy); // now pixels are good. - REPORTER_ASSERT(reporter, NULL != lazy.getPixels()); - } - // pixels should be gone! - REPORTER_ASSERT(reporter, NULL == lazy.getPixels()); - { - SkAutoLockPixels autoLockPixels(lazy); // now pixels are good. - REPORTER_ASSERT(reporter, NULL != lazy.getPixels()); - } - compare_bitmaps(reporter, original, lazy, comparePixels); +bool install_skCachingPixelRef(SkData* encoded, SkBitmap* dst) { + return SkCachingPixelRef::Install( + SkNEW_ARGS(SkDecodingImageGenerator, (encoded)), dst); } -DEF_TEST(LazyCachedPixelRef, reporter) { - test_three_encodings(reporter, compare_with_skLazyCachedPixelRef); -} - -class TestPixelRef : public SkCachingPixelRef { -public: - TestPixelRef(int x) : fX(x) { } - virtual ~TestPixelRef() { } - static bool Install(SkBitmap* destination, int x) { - SkAutoTUnref ref(SkNEW_ARGS(TestPixelRef, (x))); - return ref->configure(destination) && destination->setPixelRef(ref); - } - SK_DECLARE_UNFLATTENABLE_OBJECT() -protected: - virtual bool onDecodeInfo(SkImageInfo* info) SK_OVERRIDE { - if (fX == 0) { - return false; - } - SkASSERT(info); - info->fWidth = 10; - info->fHeight = 10; - info->fColorType = kRGBA_8888_SkColorType; - info->fAlphaType = kOpaque_SkAlphaType; - return true; - } - virtual bool onDecodePixels(const SkImageInfo& info, - void* pixels, - size_t rowBytes) SK_OVERRIDE { - return false; - } -private: - int fX; // controls where the failure happens - typedef SkCachingPixelRef INHERITED; -}; - DEF_TEST(CachingPixelRef, reporter) { - SkBitmap lazy; - // test the error handling - REPORTER_ASSERT(reporter, !TestPixelRef::Install(&lazy, 0)); - // onDecodeInfo should succeed, allowing installation - REPORTER_ASSERT(reporter, TestPixelRef::Install(&lazy, 1)); - SkAutoLockPixels autoLockPixels(lazy); // now pixels are good. - // onDecodePixels should fail, so getting pixels will fail. - REPORTER_ASSERT(reporter, NULL == lazy.getPixels()); + test_three_encodings(reporter, install_skCachingPixelRef); } -static void compare_with_SkDecodingImageGenerator(skiatest::Reporter* reporter, - SkData* encoded, - const SkBitmap& original, - bool comparePixels) { - - SkBitmap lazy; - bool success = SkDecodingImageGenerator::Install(encoded, &lazy); - REPORTER_ASSERT(reporter, success); - if (!success) { - return; - } - - REPORTER_ASSERT(reporter, NULL == lazy.getPixels()); - { - SkAutoLockPixels autoLockPixels(lazy); // now pixels are good. - REPORTER_ASSERT(reporter, NULL != lazy.getPixels()); - if (NULL == lazy.getPixels()) { - return; - } - } - // pixels should be gone! - REPORTER_ASSERT(reporter, NULL == lazy.getPixels()); - { - SkAutoLockPixels autoLockPixels(lazy); // now pixels are good. - REPORTER_ASSERT(reporter, NULL != lazy.getPixels()); - } - compare_bitmaps(reporter, original, lazy, comparePixels); -} +//////////////////////////////////////////////////////////////////////////////// +/** + * This checks to see that a SkDecodingImageGenerator works as advertised. + */ DEF_TEST(DecodingImageGenerator, reporter) { - test_three_encodings(reporter, compare_with_SkDecodingImageGenerator); + test_three_encodings(reporter, SkDecodingImageGenerator::Install); } -- 2.7.4