diff options
author | scroggo <scroggo@google.com> | 2015-02-13 11:13:34 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-02-13 11:13:34 -0800 |
commit | 0864908ca50049d3d907fc5c3749bc8a436b4738 (patch) | |
tree | a04d0629768ac04cbb13ba235e970fe46e8460c5 | |
parent | 9bafc30c7900375d316d47e24412ddfd8bd0b1f2 (diff) |
Make SkImageGenerator::getPixels() return an enum.
The new enum describes the nature of the failure. This is in
preparation for writing a replacement for SkImageDecoder, which will
use this interface.
Update the comments for getPixels() to specify what it means to pass
an SkImageInfo with a different size.
Make SkImageGenerator Noncopyable.
Leave onGetYUV8Planes alone, since we have separate discussions
regarding modifying that API.
Make callers of SkImageDecoder consistently handle kPartialSuccess.
Previously, some callers considered it a failure, and others considered
it a success.
BUG=skia:3257
Review URL: https://codereview.chromium.org/919693002
-rw-r--r-- | gyp/skia_for_chromium_defines.gypi | 1 | ||||
-rw-r--r-- | include/core/SkImageGenerator.h | 66 | ||||
-rw-r--r-- | src/core/SkImageGenerator.cpp | 41 | ||||
-rw-r--r-- | src/images/SkDecodingImageGenerator.cpp | 38 | ||||
-rw-r--r-- | src/lazy/SkCachingPixelRef.cpp | 12 | ||||
-rw-r--r-- | src/lazy/SkDiscardablePixelRef.cpp | 16 | ||||
-rw-r--r-- | src/ports/SkImageGenerator_skia.cpp | 22 | ||||
-rw-r--r-- | tests/CachedDecodingPixelRefTest.cpp | 18 |
8 files changed, 151 insertions, 63 deletions
diff --git a/gyp/skia_for_chromium_defines.gypi b/gyp/skia_for_chromium_defines.gypi index bcc6096a1d..bcb33a2796 100644 --- a/gyp/skia_for_chromium_defines.gypi +++ b/gyp/skia_for_chromium_defines.gypi @@ -14,6 +14,7 @@ # 'skia_for_chromium_defines': [ 'SK_LEGACY_DRAWPICTURECALLBACK', + 'SK_SUPPORT_LEGACY_IMAGE_GENERATOR_RETURN', ], }, } diff --git a/include/core/SkImageGenerator.h b/include/core/SkImageGenerator.h index 2967974c78..de58b68ce0 100644 --- a/include/core/SkImageGenerator.h +++ b/include/core/SkImageGenerator.h @@ -15,6 +15,8 @@ class SkBitmap; class SkData; class SkImageGenerator; +//#define SK_SUPPORT_LEGACY_IMAGE_GENERATOR_RETURN + /** * Takes ownership of SkImageGenerator. If this method fails for * whatever reason, it will return false and immediatetely delete @@ -45,7 +47,7 @@ SK_API bool SkInstallDiscardablePixelRef(SkData* encoded, SkBitmap* destination) * An interface that allows a purgeable PixelRef (such as a * SkDiscardablePixelRef) to decode and re-decode an image as needed. */ -class SK_API SkImageGenerator { +class SK_API SkImageGenerator : public SkNoncopyable { public: /** * The PixelRef which takes ownership of this SkImageGenerator @@ -74,6 +76,49 @@ public: bool getInfo(SkImageInfo* info); /** + * Used to describe the result of a call to getPixels(). + * + * Result is the union of possible results from subclasses. + */ + enum Result { + /** + * General return value for success. + */ + kSuccess, + /** + * The input is incomplete. A partial image was generated. + */ + kIncompleteInput, + /** + * The generator cannot convert to match the request, ignoring + * dimensions. + */ + kInvalidConversion, + /** + * The generator cannot scale to requested size. + */ + kInvalidScale, + /** + * Parameters (besides info) are invalid. e.g. NULL pixels, rowBytes + * too small, etc. + */ + kInvalidParameters, + /** + * The input did not contain a valid image. + */ + kInvalidInput, + /** + * Fulfilling this request requires rewinding the input, which is not + * supported for this input. + */ + kCouldNotRewind, + /** + * This method is not implemented by this generator. + */ + kUnimplemented, + }; + + /** * Decode into the given pixels, a block of memory of size at * least (info.fHeight - 1) * rowBytes + (info.fWidth * * bytesPerPixel) @@ -89,6 +134,10 @@ public: * different output-configs, which the implementation can * decide to support or not. * + * A size that does not match getInfo() implies a request + * to scale. If the generator cannot perform this scale, + * it will return kInvalidScale. + * * 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. @@ -96,16 +145,15 @@ public: * 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. + * @return Result kSuccess, or another value explaining the type of failure. */ - bool getPixels(const SkImageInfo& info, void* pixels, size_t rowBytes, - SkPMColor ctable[], int* ctableCount); + Result 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); + Result getPixels(const SkImageInfo& info, void* pixels, size_t rowBytes); /** * If planes or rowBytes is NULL or if any entry in planes is NULL or if any entry in rowBytes @@ -131,9 +179,15 @@ public: protected: virtual SkData* onRefEncodedData(); virtual bool onGetInfo(SkImageInfo* info); +#ifdef SK_SUPPORT_LEGACY_IMAGE_GENERATOR_RETURN virtual bool onGetPixels(const SkImageInfo& info, void* pixels, size_t rowBytes, SkPMColor ctable[], int* ctableCount); +#endif + // TODO (scroggo): rename to onGetPixels. + virtual Result onGetPixelsEnum(const SkImageInfo& info, + void* pixels, size_t rowBytes, + SkPMColor ctable[], int* ctableCount); virtual bool onGetYUV8Planes(SkISize sizes[3], void* planes[3], size_t rowBytes[3]); virtual bool onGetYUV8Planes(SkISize sizes[3], void* planes[3], size_t rowBytes[3], SkYUVColorSpace* colorSpace); diff --git a/src/core/SkImageGenerator.cpp b/src/core/SkImageGenerator.cpp index 0f63db50f6..c6167ff278 100644 --- a/src/core/SkImageGenerator.cpp +++ b/src/core/SkImageGenerator.cpp @@ -15,21 +15,22 @@ bool SkImageGenerator::getInfo(SkImageInfo* info) { return this->onGetInfo(info); } -bool SkImageGenerator::getPixels(const SkImageInfo& info, void* pixels, size_t rowBytes, - SkPMColor ctable[], int* ctableCount) { +SkImageGenerator::Result SkImageGenerator::getPixels(const SkImageInfo& info, void* pixels, + size_t rowBytes, SkPMColor ctable[], + int* ctableCount) { if (kUnknown_SkColorType == info.colorType()) { - return false; + return kInvalidConversion; } if (NULL == pixels) { - return false; + return kInvalidParameters; } if (rowBytes < info.minRowBytes()) { - return false; + return kInvalidParameters; } if (kIndex_8_SkColorType == info.colorType()) { if (NULL == ctable || NULL == ctableCount) { - return false; + return kInvalidParameters; } } else { if (ctableCount) { @@ -39,18 +40,19 @@ bool SkImageGenerator::getPixels(const SkImageInfo& info, void* pixels, size_t r ctable = NULL; } - bool success = this->onGetPixels(info, pixels, rowBytes, ctable, ctableCount); + const Result result = this->onGetPixelsEnum(info, pixels, rowBytes, ctable, ctableCount); - if (success && ctableCount) { + if ((kIncompleteInput == result || kSuccess == result) && ctableCount) { SkASSERT(*ctableCount >= 0 && *ctableCount <= 256); } - return success; + return result; } -bool SkImageGenerator::getPixels(const SkImageInfo& info, void* pixels, size_t rowBytes) { +SkImageGenerator::Result 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 kInvalidConversion; } return this->getPixels(info, pixels, rowBytes, NULL, NULL); } @@ -117,6 +119,19 @@ bool SkImageGenerator::onGetInfo(SkImageInfo*) { return false; } -bool SkImageGenerator::onGetPixels(const SkImageInfo&, void*, size_t, SkPMColor*, int*) { - return false; +#ifdef SK_SUPPORT_LEGACY_IMAGE_GENERATOR_RETURN +bool SkImageGenerator::onGetPixels(const SkImageInfo&, void*, size_t, + SkPMColor*, int*) { + return kUnimplemented; +} +#endif +SkImageGenerator::Result SkImageGenerator::onGetPixelsEnum(const SkImageInfo& info, void* pixels, + size_t rowBytes, SkPMColor* colors, + int* colorCount) { +#ifdef SK_SUPPORT_LEGACY_IMAGE_GENERATOR_RETURN + if (this->onGetPixels(info, pixels, rowBytes, colors, colorCount)) { + return kSuccess; + } +#endif + return kUnimplemented; } diff --git a/src/images/SkDecodingImageGenerator.cpp b/src/images/SkDecodingImageGenerator.cpp index e7a775e264..cce01d2832 100644 --- a/src/images/SkDecodingImageGenerator.cpp +++ b/src/images/SkDecodingImageGenerator.cpp @@ -42,9 +42,9 @@ protected: *info = fInfo; return true; } - virtual bool onGetPixels(const SkImageInfo& info, - void* pixels, size_t rowBytes, - SkPMColor ctable[], int* ctableCount) SK_OVERRIDE; + virtual Result onGetPixelsEnum(const SkImageInfo& info, + void* pixels, size_t rowBytes, + SkPMColor ctable[], int* ctableCount) SK_OVERRIDE; virtual bool onGetYUV8Planes(SkISize sizes[3], void* planes[3], size_t rowBytes[3], SkYUVColorSpace* colorSpace) SK_OVERRIDE; @@ -147,20 +147,22 @@ SkData* DecodingImageGenerator::onRefEncodedData() { return SkSafeRef(fData); } -bool DecodingImageGenerator::onGetPixels(const SkImageInfo& info, - void* pixels, size_t rowBytes, - SkPMColor ctableEntries[], int* ctableCount) { +SkImageGenerator::Result DecodingImageGenerator::onGetPixelsEnum(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.dimensions() != fInfo.dimensions()) { + return kInvalidScale; + } + return kInvalidConversion; } SkAssertResult(fStream->rewind()); SkAutoTDelete<SkImageDecoder> decoder(SkImageDecoder::Factory(fStream)); if (NULL == decoder.get()) { - return false; + return kInvalidInput; } decoder->setDitherImage(fDitherImage); decoder->setSampleSize(fSampleSize); @@ -169,11 +171,11 @@ bool DecodingImageGenerator::onGetPixels(const SkImageInfo& info, SkBitmap bitmap; TargetAllocator allocator(fInfo, pixels, rowBytes); decoder->setAllocator(&allocator); - bool success = decoder->decode(fStream, &bitmap, info.colorType(), - SkImageDecoder::kDecodePixels_Mode) != SkImageDecoder::kFailure; + const SkImageDecoder::Result decodeResult = decoder->decode(fStream, &bitmap, info.colorType(), + SkImageDecoder::kDecodePixels_Mode); decoder->setAllocator(NULL); - if (!success) { - return false; + if (SkImageDecoder::kFailure == decodeResult) { + return kInvalidInput; } if (allocator.isReady()) { // Did not use pixels! SkBitmap bm; @@ -182,7 +184,7 @@ bool DecodingImageGenerator::onGetPixels(const SkImageInfo& info, if (!copySuccess || allocator.isReady()) { SkDEBUGFAIL("bitmap.copyTo(requestedConfig) failed."); // Earlier we checked canCopyto(); we expect consistency. - return false; + return kInvalidConversion; } SkASSERT(check_alpha(info.alphaType(), bm.alphaType())); } else { @@ -191,17 +193,21 @@ bool DecodingImageGenerator::onGetPixels(const SkImageInfo& info, 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 + // they asked for Index8, but we didn't receive that from decoder + return kInvalidConversion; } SkColorTable* ctable = bitmap.getColorTable(); if (NULL == ctable) { - return false; + return kInvalidConversion; } const int count = ctable->count(); memcpy(ctableEntries, ctable->readColors(), count * sizeof(SkPMColor)); *ctableCount = count; } - return true; + if (SkImageDecoder::kPartialSuccess == decodeResult) { + return kIncompleteInput; + } + return kSuccess; } bool DecodingImageGenerator::onGetYUV8Planes(SkISize sizes[3], void* planes[3], diff --git a/src/lazy/SkCachingPixelRef.cpp b/src/lazy/SkCachingPixelRef.cpp index 5ab96562ec..570fc6fbd7 100644 --- a/src/lazy/SkCachingPixelRef.cpp +++ b/src/lazy/SkCachingPixelRef.cpp @@ -52,9 +52,15 @@ bool SkCachingPixelRef::onNewLockPixels(LockRec* rec) { fErrorInDecoding = true; return false; } - if (!fImageGenerator->getPixels(info, fLockedBitmap.getPixels(), fRowBytes)) { - fErrorInDecoding = true; - return false; + const SkImageGenerator::Result result = fImageGenerator->getPixels(info, + fLockedBitmap.getPixels(), fRowBytes); + switch (result) { + case SkImageGenerator::kIncompleteInput: + case SkImageGenerator::kSuccess: + break; + default: + fErrorInDecoding = true; + return false; } fLockedBitmap.setImmutable(); SkBitmapCache::Add( diff --git a/src/lazy/SkDiscardablePixelRef.cpp b/src/lazy/SkDiscardablePixelRef.cpp index b51daa6faa..50988587a7 100644 --- a/src/lazy/SkDiscardablePixelRef.cpp +++ b/src/lazy/SkDiscardablePixelRef.cpp @@ -64,11 +64,17 @@ bool SkDiscardablePixelRef::onNewLockPixels(LockRec* rec) { SkPMColor colors[256]; int colorCount = 0; - if (!fGenerator->getPixels(info, pixels, fRowBytes, colors, &colorCount)) { - fDiscardableMemory->unlock(); - SkDELETE(fDiscardableMemory); - fDiscardableMemory = NULL; - return false; + const SkImageGenerator::Result result = fGenerator->getPixels(info, pixels, fRowBytes, + colors, &colorCount); + switch (result) { + case SkImageGenerator::kSuccess: + case SkImageGenerator::kIncompleteInput: + break; + default: + fDiscardableMemory->unlock(); + SkDELETE(fDiscardableMemory); + fDiscardableMemory = NULL; + return false; } // Note: our ctable is not purgeable, as it is not stored in the discardablememory block. diff --git a/src/ports/SkImageGenerator_skia.cpp b/src/ports/SkImageGenerator_skia.cpp index 878a44de9c..079da56155 100644 --- a/src/ports/SkImageGenerator_skia.cpp +++ b/src/ports/SkImageGenerator_skia.cpp @@ -52,8 +52,8 @@ protected: return true; } - virtual bool onGetPixels(const SkImageInfo& info, void* pixels, size_t rowBytes, - SkPMColor ctableEntries[], int* ctableCount) SK_OVERRIDE { + virtual Result onGetPixelsEnum(const SkImageInfo& info, void* pixels, size_t rowBytes, + SkPMColor ctableEntries[], int* ctableCount) SK_OVERRIDE { SkMemoryStream stream(fData->data(), fData->size(), false); SkAutoTUnref<BareMemoryAllocator> allocator(SkNEW_ARGS(BareMemoryAllocator, (info, pixels, rowBytes))); @@ -61,13 +61,10 @@ protected: fDecoder->setRequireUnpremultipliedColors(kUnpremul_SkAlphaType == info.alphaType()); SkBitmap bm; - SkImageDecoder::Result result = fDecoder->decode(&stream, &bm, info.colorType(), - SkImageDecoder::kDecodePixels_Mode); - switch (result) { - case SkImageDecoder::kSuccess: - break; - default: - return false; + const SkImageDecoder::Result result = fDecoder->decode(&stream, &bm, info.colorType(), + SkImageDecoder::kDecodePixels_Mode); + if (SkImageDecoder::kFailure == result) { + return kInvalidInput; } SkASSERT(info.colorType() == bm.info().colorType()); @@ -77,13 +74,16 @@ protected: SkColorTable* ctable = bm.getColorTable(); if (NULL == ctable) { - return false; + return kInvalidConversion; } const int count = ctable->count(); memcpy(ctableEntries, ctable->readColors(), count * sizeof(SkPMColor)); *ctableCount = count; } - return true; + if (SkImageDecoder::kPartialSuccess == result) { + return kIncompleteInput; + } + return kSuccess; } bool onGetYUV8Planes(SkISize sizes[3], void* planes[3], size_t rowBytes[3], diff --git a/tests/CachedDecodingPixelRefTest.cpp b/tests/CachedDecodingPixelRefTest.cpp index e8fea2fdf2..51cb7ba38d 100644 --- a/tests/CachedDecodingPixelRefTest.cpp +++ b/tests/CachedDecodingPixelRefTest.cpp @@ -189,15 +189,15 @@ protected: return true; } - virtual bool onGetPixels(const SkImageInfo& info, void* pixels, size_t rowBytes, - SkPMColor ctable[], int* ctableCount) SK_OVERRIDE { + virtual Result onGetPixelsEnum(const SkImageInfo& info, void* pixels, size_t rowBytes, + SkPMColor ctable[], int* ctableCount) SK_OVERRIDE { REPORTER_ASSERT(fReporter, pixels != NULL); - size_t minRowBytes = static_cast<size_t>(info.width() * info.bytesPerPixel()); - REPORTER_ASSERT(fReporter, rowBytes >= minRowBytes); - if ((NULL == pixels) - || (fType != kSucceedGetPixels_TestType) - || (info.colorType() != kN32_SkColorType)) { - return false; + REPORTER_ASSERT(fReporter, rowBytes >= info.minRowBytes()); + if (fType != kSucceedGetPixels_TestType) { + return kUnimplemented; + } + if (info.colorType() != kN32_SkColorType) { + return kInvalidConversion; } char* bytePtr = static_cast<char*>(pixels); for (int y = 0; y < info.height(); ++y) { @@ -205,7 +205,7 @@ protected: TestImageGenerator::Color(), info.width()); bytePtr += rowBytes; } - return true; + return kSuccess; } private: |