Make SkImageGenerator::getPixels() return an enum.

src/images/SkDecodingImageGenerator.cpp

Index: src/images/SkDecodingImageGenerator.cpp
diff --git a/src/images/SkDecodingImageGenerator.cpp b/src/images/SkDecodingImageGenerator.cpp
index e7a775e264b8e9181f5ddaad6e4f57299f10afbe..3219b742df67efd28fd3dd87b55aa3c458768771 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 onGetPixels(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::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.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;

[scroggo, 2015/02/11 16:01:25]

This mimics the old behavior, but it seems like this should be an assert? This
would mean the decoder did something wrong.

[djsollen, 2015/02/12 14:22:15]

I think we may want to have another error type that reports back if the input
params are invalid. We could then throw that error here as well as if they did
something like pass nullptr for the pixels

[scroggo, 2015/02/12 15:28:36]

We already have that: kInvalidParameters, which gets returned in
SkImageGenerator::getPixels (the non-virtual that always gets called).

But that's not what happened if we reached here. The input from the caller is
ctableEntries. If that was null, we would have returned kInvalidParameters in
getPixels.

Looking more closely at this, I realize that I was confused. The colorType we're
checking is the requested colorType. I thought it was the bitmap's colorType -
i.e. the bitmap was Index8 but did *not* have a colorTable, which would be a bug
in the decoder.

I still think we would never reach here though (predating my changes). If the
caller requested Index8 (this block), we have two possibilities:

A) The decoder gave us Index8 - if bitmap does *not* have a colorTable, there
was a bug in the decoder (my original concern).

B) The decoder gave us something else. In this case, the above block (if
allocator.isReady) would have attempted to copy to Index8 - which always fails -
you can only copy to Index8 from another Index8. So we would return
kInvalidConversion before reaching here. (The assert that canCopyTo returned
true just tells me that we aren't actually testing this path.)

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