From b263985850a7a74ccd5fda2abb057b04f7254e41 Mon Sep 17 00:00:00 2001 From: "commit-bot@chromium.org" Date: Wed, 28 May 2014 16:01:55 +0000 Subject: [PATCH] add colortable support to imagegenerator BUG=skia: R=halcanary@google.com, scroggo@google.com Author: reed@google.com Review URL: https://codereview.chromium.org/304443003 git-svn-id: http://skia.googlecode.com/svn/trunk@14916 2bbb7eff-a529-9590-31e7-b0007b416f81 --- gyp/images.gyp | 1 + gyp/skia_for_chromium_defines.gypi | 1 + include/core/SkImageGenerator.h | 37 ++++++++++++--- src/images/SkDecodingImageGenerator.cpp | 73 ++++++++++++++---------------- src/images/SkImageGenerator.cpp | 79 +++++++++++++++++++++++++++++++++ src/lazy/SkDiscardablePixelRef.cpp | 19 +++++++- src/lazy/SkDiscardablePixelRef.h | 5 +-- tests/CachedDecodingPixelRefTest.cpp | 10 +++-- tests/DrawBitmapRectTest.cpp | 12 ++--- 9 files changed, 174 insertions(+), 63 deletions(-) create mode 100644 src/images/SkImageGenerator.cpp diff --git a/gyp/images.gyp b/gyp/images.gyp index 0062f24..18a18be 100644 --- a/gyp/images.gyp +++ b/gyp/images.gyp @@ -71,6 +71,7 @@ '../src/images/SkImageEncoder.cpp', '../src/images/SkImageEncoder_Factory.cpp', '../src/images/SkImageEncoder_argb.cpp', + '../src/images/SkImageGenerator.cpp', '../src/images/SkImageRef.cpp', '../src/images/SkImageRefPool.cpp', '../src/images/SkImageRefPool.h', diff --git a/gyp/skia_for_chromium_defines.gypi b/gyp/skia_for_chromium_defines.gypi index c114b71..a47b391 100644 --- a/gyp/skia_for_chromium_defines.gypi +++ b/gyp/skia_for_chromium_defines.gypi @@ -16,6 +16,7 @@ 'SK_SUPPORT_LEGACY_GETTOPDEVICE', 'SK_SUPPORT_LEGACY_ASIMAGEINFO', 'SK_SUPPORT_LEGACY_N32_NAME', + 'SK_SUPPORT_LEGACY_IMAGEGENERATORAPI', ], }, } diff --git a/include/core/SkImageGenerator.h b/include/core/SkImageGenerator.h index f399ab5..21ebcbb 100644 --- a/include/core/SkImageGenerator.h +++ b/include/core/SkImageGenerator.h @@ -9,6 +9,7 @@ #define SkImageGenerator_DEFINED #include "SkImageInfo.h" +#include "SkColor.h" class SkBitmap; class SkData; @@ -47,6 +48,13 @@ public: */ virtual ~SkImageGenerator() { } +#ifdef SK_SUPPORT_LEGACY_IMAGEGENERATORAPI + virtual bool refEncodedData() { return this->onRefEncodedData(); } + virtual bool getInfo(SkImageInfo* info) { return this->onGetInfo(info); } + virtual bool getPixels(const SkImageInfo& info, void* pixels, size_t rowBytes) { + return this->onGetPixels(info, pixels, rowBytes, NULL, NULL); + } +#else /** * Return a ref to the encoded (i.e. compressed) representation, * of this data. @@ -54,7 +62,7 @@ public: * If non-NULL is returned, the caller is responsible for calling * unref() on the data when it is finished. */ - virtual SkData* refEncodedData() { return NULL; } + SkData* refEncodedData() { return this->onRefEncodedData(); } /** * Return some information about the image, allowing the owner of @@ -65,7 +73,7 @@ public: * * @return false if anything goes wrong. */ - virtual bool getInfo(SkImageInfo* info) = 0; + bool getInfo(SkImageInfo* info); /** * Decode into the given pixels, a block of memory of size at @@ -83,12 +91,31 @@ public: * different output-configs, which the implementation can * decide to support or not. * + * If info is kIndex8_SkColorType, then the caller must provide storage for up to 256 + * SkPMColor values in ctable. On success the generator must copy N colors into that storage, + * (where N is the logical number of table entries) and set ctableCount to N. + * + * If info is not kIndex8_SkColorType, then the last two parameters may be NULL. If ctableCount + * is not null, it will be set to 0. + * * @return false if anything goes wrong or if the image info is * unsupported. */ - virtual bool getPixels(const SkImageInfo& info, - void* pixels, - size_t rowBytes) = 0; + bool getPixels(const SkImageInfo& info, void* pixels, size_t rowBytes, + SkPMColor ctable[], int* ctableCount); + + /** + * Simplified version of getPixels() that asserts that info is NOT kIndex8_SkColorType. + */ + bool getPixels(const SkImageInfo& info, void* pixels, size_t rowBytes); +#endif + +protected: + virtual SkData* onRefEncodedData(); + virtual bool onGetInfo(SkImageInfo* info); + virtual bool onGetPixels(const SkImageInfo& info, + void* pixels, size_t rowBytes, + SkPMColor ctable[], int* ctableCount); }; #endif // SkImageGenerator_DEFINED diff --git a/src/images/SkDecodingImageGenerator.cpp b/src/images/SkDecodingImageGenerator.cpp index b3924a7..1f0f706 100644 --- a/src/images/SkDecodingImageGenerator.cpp +++ b/src/images/SkDecodingImageGenerator.cpp @@ -23,12 +23,6 @@ bool equal_modulo_alpha(const SkImageInfo& a, const SkImageInfo& b) { class DecodingImageGenerator : public SkImageGenerator { public: virtual ~DecodingImageGenerator(); - virtual SkData* refEncodedData() SK_OVERRIDE; - // This implementaion of getInfo() always returns true. - virtual bool getInfo(SkImageInfo* info) SK_OVERRIDE; - virtual bool getPixels(const SkImageInfo& info, - void* pixels, - size_t rowBytes) SK_OVERRIDE; SkData* fData; SkStreamRewindable* fStream; @@ -41,6 +35,18 @@ public: const SkImageInfo& info, int sampleSize, bool ditherImage); + +protected: + virtual SkData* onRefEncodedData() SK_OVERRIDE; + virtual bool onGetInfo(SkImageInfo* info) SK_OVERRIDE { + *info = fInfo; + return true; + } + virtual bool onGetPixels(const SkImageInfo& info, + void* pixels, size_t rowBytes, + SkPMColor ctable[], int* ctableCount) SK_OVERRIDE; + +private: typedef SkImageGenerator INHERITED; }; @@ -123,14 +129,7 @@ DecodingImageGenerator::~DecodingImageGenerator() { fStream->unref(); } -bool DecodingImageGenerator::getInfo(SkImageInfo* info) { - if (info != NULL) { - *info = fInfo; - } - return true; -} - -SkData* DecodingImageGenerator::refEncodedData() { +SkData* DecodingImageGenerator::onRefEncodedData() { // This functionality is used in `gm --serialize` // Does not encode options. if (fData != NULL) { @@ -151,22 +150,15 @@ SkData* DecodingImageGenerator::refEncodedData() { return SkSafeRef(fData); } -bool DecodingImageGenerator::getPixels(const SkImageInfo& info, - void* pixels, - size_t rowBytes) { - if (NULL == pixels) { - return false; - } +bool DecodingImageGenerator::onGetPixels(const SkImageInfo& info, + void* pixels, size_t rowBytes, + SkPMColor ctableEntries[], int* ctableCount) { if (fInfo != info) { // The caller has specified a different info. This is an // error for this kind of SkImageGenerator. Use the Options // to change the settings. return false; } - if (info.minRowBytes() > rowBytes) { - // The caller has specified a bad rowBytes. - return false; - } SkAssertResult(fStream->rewind()); SkAutoTDelete decoder(SkImageDecoder::Factory(fStream)); @@ -202,6 +194,20 @@ bool DecodingImageGenerator::getPixels(const SkImageInfo& info, } else { SkASSERT(check_alpha(info.alphaType(), bitmap.alphaType())); } + + if (kIndex_8_SkColorType == info.colorType()) { + if (kIndex_8_SkColorType != bitmap.colorType()) { + return false; // they asked for Index8, but we didn't receive that from decoder + } + SkColorTable* ctable = bitmap.getColorTable(); + if (NULL == ctable) { + return false; + } + const int count = ctable->count(); + memcpy(ctableEntries, ctable->lockColors(), count * sizeof(SkPMColor)); + ctable->unlockColors(); + *ctableCount = count; + } return true; } @@ -214,11 +220,6 @@ SkImageGenerator* CreateDecodingImageGenerator( const SkDecodingImageGenerator::Options& opts) { SkASSERT(stream); SkAutoTUnref autoStream(stream); // always unref this. - if (opts.fUseRequestedColorType && - (kIndex_8_SkColorType == opts.fRequestedColorType)) { - // We do not support indexed color with SkImageGenerators, - return NULL; - } SkAssertResult(autoStream->rewind()); SkAutoTDelete decoder(SkImageDecoder::Factory(autoStream)); if (NULL == decoder.get()) { @@ -227,24 +228,16 @@ SkImageGenerator* CreateDecodingImageGenerator( SkBitmap bitmap; decoder->setSampleSize(opts.fSampleSize); decoder->setRequireUnpremultipliedColors(opts.fRequireUnpremul); - if (!decoder->decode(stream, &bitmap, - SkImageDecoder::kDecodeBounds_Mode)) { + if (!decoder->decode(stream, &bitmap, SkImageDecoder::kDecodeBounds_Mode)) { return NULL; } - if (bitmap.config() == SkBitmap::kNo_Config) { + if (kUnknown_SkColorType == bitmap.colorType()) { return NULL; } SkImageInfo info = bitmap.info(); - if (!opts.fUseRequestedColorType) { - // Use default - if (kIndex_8_SkColorType == bitmap.colorType()) { - // We don't support kIndex8 because we don't support - // colortables in this workflow. - info.fColorType = kN32_SkColorType; - } - } else { + if (opts.fUseRequestedColorType && (opts.fRequestedColorType != info.colorType())) { if (!bitmap.canCopyTo(opts.fRequestedColorType)) { SkASSERT(bitmap.colorType() != opts.fRequestedColorType); return NULL; // Can not translate to needed config. diff --git a/src/images/SkImageGenerator.cpp b/src/images/SkImageGenerator.cpp new file mode 100644 index 0000000..b898764 --- /dev/null +++ b/src/images/SkImageGenerator.cpp @@ -0,0 +1,79 @@ +/* + * Copyright 2014 Google Inc. + * + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +#include "SkImageGenerator.h" + +#ifndef SK_SUPPORT_LEGACY_IMAGEGENERATORAPI +bool SkImageGenerator::getInfo(SkImageInfo* info) { + SkImageInfo dummy; + if (NULL == info) { + info = &dummy; + } + return this->onGetInfo(info); +} + +bool SkImageGenerator::getPixels(const SkImageInfo& info, void* pixels, size_t rowBytes, + SkPMColor ctable[], int* ctableCount) { + // Dummy (out-of-range) value to catch subclasses that forgot to respect ctable params. + int localCTableCount = -1; + int* localCTableCountPtr = NULL; + + if (kIndex_8_SkColorType == info.colorType()) { + if (NULL == ctable || NULL == ctableCount) { + return false; + } + } else { + if (ctableCount) { + *ctableCount = 0; + } + ctableCount = NULL; + ctable = NULL; + } + + if (kUnknown_SkColorType == info.colorType()) { + return false; + } + if (NULL == pixels) { + return false; + } + if (rowBytes < info.minRowBytes()) { + return false; + } + + bool success = this->onGetPixels(info, pixels, rowBytes, ctable, localCTableCountPtr); + + if (success && localCTableCountPtr) { + SkASSERT(localCTableCount >= 0 && localCTableCount <= 256); + if (ctableCount) { + *ctableCount = localCTableCount; + } + } + return success; +} + +bool SkImageGenerator::getPixels(const SkImageInfo& info, void* pixels, size_t rowBytes) { + SkASSERT(kIndex_8_SkColorType != info.colorType()); + if (kIndex_8_SkColorType == info.colorType()) { + return false; + } + return this->getPixels(info, pixels, rowBytes, NULL, NULL); +} +#endif + +///////////////////////////////////////////////////////////////////////////////////////////// + +SkData* SkImageGenerator::onRefEncodedData() { + return NULL; +} + +bool SkImageGenerator::onGetInfo(SkImageInfo*) { + return false; +} + +bool SkImageGenerator::onGetPixels(const SkImageInfo&, void*, size_t, SkPMColor*, int*) { + return false; +} diff --git a/src/lazy/SkDiscardablePixelRef.cpp b/src/lazy/SkDiscardablePixelRef.cpp index ccf812c..b0bbd27 100644 --- a/src/lazy/SkDiscardablePixelRef.cpp +++ b/src/lazy/SkDiscardablePixelRef.cpp @@ -60,15 +60,30 @@ bool SkDiscardablePixelRef::onNewLockPixels(LockRec* rec) { } void* pixels = fDiscardableMemory->data(); - if (!fGenerator->getPixels(this->info(), pixels, fRowBytes)) { + const SkImageInfo& info = this->info(); + SkPMColor colors[256]; + int colorCount = 0; + + if (!fGenerator->getPixels(info, pixels, fRowBytes, colors, &colorCount)) { fDiscardableMemory->unlock(); SkDELETE(fDiscardableMemory); fDiscardableMemory = NULL; return false; } + // Note: our ctable is not purgable, as it is not stored in the discardablememory block. + // This is because SkColorTable is refcntable, and therefore our caller could hold onto it + // beyond the scope of a lock/unlock. If we change the API/lifecycle for SkColorTable, we + // could move it into the block, but then again perhaps it is small enough that this doesn't + // really matter. + if (colorCount > 0) { + fCTable.reset(SkNEW_ARGS(SkColorTable, (colors, colorCount))); + } else { + fCTable.reset(NULL); + } + rec->fPixels = pixels; - rec->fColorTable = NULL; + rec->fColorTable = fCTable.get(); rec->fRowBytes = fRowBytes; return true; } diff --git a/src/lazy/SkDiscardablePixelRef.h b/src/lazy/SkDiscardablePixelRef.h index e5e1c9f..52a1d6c 100644 --- a/src/lazy/SkDiscardablePixelRef.h +++ b/src/lazy/SkDiscardablePixelRef.h @@ -17,10 +17,6 @@ * A PixelRef backed by SkDiscardableMemory, with the ability to * re-generate the pixels (via a SkImageGenerator) if the DM is * purged. - * - * Since SkColorTable is reference-counted, we do not support indexed - * color with this class; there would be no way for the discardable - * memory system to unref the color table. */ class SkDiscardablePixelRef : public SkPixelRef { public: @@ -46,6 +42,7 @@ private: // PixelRef, since the SkBitmap doesn't expect them to change. SkDiscardableMemory* fDiscardableMemory; + SkAutoTUnref fCTable; /* Takes ownership of SkImageGenerator. */ SkDiscardablePixelRef(const SkImageInfo&, SkImageGenerator*, diff --git a/tests/CachedDecodingPixelRefTest.cpp b/tests/CachedDecodingPixelRefTest.cpp index 10e33ba..eff77e2 100644 --- a/tests/CachedDecodingPixelRefTest.cpp +++ b/tests/CachedDecodingPixelRefTest.cpp @@ -183,7 +183,9 @@ public: SkASSERT((fType <= kLast_TestType) && (fType >= 0)); } virtual ~TestImageGenerator() { } - virtual bool getInfo(SkImageInfo* info) SK_OVERRIDE { + +protected: + virtual bool onGetInfo(SkImageInfo* info) SK_OVERRIDE { REPORTER_ASSERT(fReporter, NULL != info); if ((NULL == info) || (kFailGetInfo_TestType == fType)) { return false; @@ -194,9 +196,9 @@ public: info->fAlphaType = kOpaque_SkAlphaType; return true; } - virtual bool getPixels(const SkImageInfo& info, - void* pixels, - size_t rowBytes) SK_OVERRIDE { + + virtual bool onGetPixels(const SkImageInfo& info, void* pixels, size_t rowBytes, + SkPMColor ctable[], int* ctableCount) SK_OVERRIDE { REPORTER_ASSERT(fReporter, pixels != NULL); size_t minRowBytes = static_cast(info.fWidth * info.bytesPerPixel()); diff --git a/tests/DrawBitmapRectTest.cpp b/tests/DrawBitmapRectTest.cpp index f294ae7..aa69de4 100644 --- a/tests/DrawBitmapRectTest.cpp +++ b/tests/DrawBitmapRectTest.cpp @@ -22,20 +22,16 @@ class FailureImageGenerator : public SkImageGenerator { public: FailureImageGenerator() { } virtual ~FailureImageGenerator() { } - virtual bool getInfo(SkImageInfo* info) SK_OVERRIDE { + +protected: + virtual bool onGetInfo(SkImageInfo* info) SK_OVERRIDE { info->fWidth = 100; info->fHeight = 100; info->fColorType = kN32_SkColorType; info->fAlphaType = kPremul_SkAlphaType; return true; } - virtual bool getPixels(const SkImageInfo& info, - void* pixels, - size_t rowBytes) SK_OVERRIDE { - // this will deliberately return false if they are asking us - // to decode into pixels. - return false; - } + // default onGetPixels() returns false, which is what we want. }; // crbug.com/295895 -- 2.7.4