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
This commit is contained in:
scroggo 2015-02-13 11:13:34 -08:00 committed by Commit bot
parent 9bafc30c79
commit 0864908ca5
8 changed files with 151 additions and 63 deletions

View File

@ -14,6 +14,7 @@
#
'skia_for_chromium_defines': [
'SK_LEGACY_DRAWPICTURECALLBACK',
'SK_SUPPORT_LEGACY_IMAGE_GENERATOR_RETURN',
],
},
}

View File

@ -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
@ -73,6 +75,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 *
@ -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);

View File

@ -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;
}

View File

@ -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],

View File

@ -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(

View File

@ -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.

View File

@ -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],

View File

@ -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: