Bmp Image Decoding

dm/DMSrcSink.cpp

 #include "DMSrcSink.h"
 #include "SamplePipeControllers.h"
 #include "SkCommonFlags.h"
 #include "SkCodec.h"
 #include "SkDocument.h"
 #include "SkMultiPictureDraw.h"
 #include "SkNullCanvas.h"
 #include "SkOSFile.h"
 #include "SkPictureRecorder.h"
 #include "SkRandom.h"
@@ skipping 27 matching lines @@
 /*~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~*/
 
 ImageSrc::ImageSrc(Path path, int divisor) : fPath(path), fDivisor(divisor) {}
 
 Error ImageSrc::draw(SkCanvas* canvas) const {
     SkAutoTUnref<SkData> encoded(SkData::NewFromFileName(fPath.c_str()));
     if (!encoded) {
         return SkStringPrintf("Couldn't read %s.", fPath.c_str());
     }
     const SkColorType dstColorType = canvas->imageInfo().colorType();
+    SkString error;
     if (fDivisor == 0) {
         // Decode the full image.
         SkBitmap bitmap;
         if (FLAGS_codec) {
             SkAutoTDelete<SkCodec> codec(SkCodec::NewFromData(encoded));
             if (!codec) {
                 return SkStringPrintf("Couldn't decode %s.", fPath.c_str());
             }
             SkImageInfo info;
             if (!codec->getInfo(&info)) {
                 return SkStringPrintf("Couldn't getInfo %s.", fPath.c_str());
             }
             info = info.makeColorType(dstColorType);
             if (info.alphaType() == kUnpremul_SkAlphaType) {
                 // FIXME: Currently we cannot draw unpremultiplied sources.
                 info = info.makeAlphaType(kPremul_SkAlphaType);
             }
             bitmap.allocPixels(info);
             SkAutoLockPixels alp(bitmap);
             const SkImageGenerator::Result result = codec->getPixels(info, bitmap.getPixels(),
                                                                      bitmap.rowBytes());
-            if (result != SkImageGenerator::kSuccess) {
+            if (result == SkImageGenerator::kIncompleteInput) {

[scroggo, 2015/03/04 17:10:29]

I think this would read better as a switch statement.

That said, I'm not sure what the result of drawing to the bitmap and reporting
an error is (as opposed to one or the other).

[msarett, 2015/03/05 23:13:17]

Doesn't feel to me like a switch statement is necessary here now that we've
revised the behavior to be simpler again.  Let me know if you feel otherwise.

+                error = SkStringPrintf("Warning incomplete getPixels %s.", fPath.c_str());

[scroggo, 2015/03/04 17:10:29]

I'm not sure what the right behavior is here. Note that DM does not distinguish
between errors and warnings. So these will be reported as failures. If we expect
to run our tests on incomplete images, this will mean we'll turn the bot red
(and make people think there is a problem with the code).

On the other hand, these are potentially worth investigating, since the decoder
*could* have a bug leading to only partially decoding. It may just be enough to
look at the results in Gold, and if the image is incorrectly incomplete, that
tells us we have a problem.

[msarett, 2015/03/05 23:13:17]

The decision here is to accept kSuccess or kIncompleteInput and draw without
signaling an error.  The decoder should print a warning before returning
kIncompleteInput.  Other return values will signal an error.

It might be worth noting that a few paths in the bmp decoder are able to decode
partial images before hitting an invalid input that forces the decode to halt
(ex: invalid RLE code).  I still return kIncompleteInput here (instead of
kInvalidInput) because we want to signal that there is a valid partially decoded
image.  Let me know if this is confusing.

+            } else if (result != SkImageGenerator::kSuccess) {
                 return SkStringPrintf("Couldn't getPixels %s.", fPath.c_str());
+            } else {
+                error = "";

[scroggo, 2015/03/04 17:10:29]

Maybe set error to "" initially?

             }
         } else {
             if (!SkImageDecoder::DecodeMemory(encoded->data(), encoded->size(), &bitmap,
                                               dstColorType, SkImageDecoder::kDecodePixels_Mode)) {
                 return SkStringPrintf("Couldn't decode %s.", fPath.c_str());
+            } else {
+                error = "";
             }
         }
         encoded.reset((SkData*)NULL);  // Might as well drop this when we're done with it.
         canvas->drawBitmap(bitmap, 0,0);
-        return "";
+        return error;
     }
     // Decode subsets.  This is a little involved.
     SkAutoTDelete<SkMemoryStream> stream(new SkMemoryStream(encoded));
     SkAutoTDelete<SkImageDecoder> decoder(SkImageDecoder::Factory(stream.get()));
     if (!decoder) {
         return SkStringPrintf("Can't find a good decoder for %s.", fPath.c_str());
     }
     stream->rewind();
     int w,h;
     if (!decoder->buildTileIndex(stream.detach(), &w, &h) || w*h == 1) {
@@ skipping 16 matching lines @@
                                       x, y, x+subsetWidth, y+subsetHeight);
             }
             canvas->drawBitmap(subset, SkIntToScalar(x), SkIntToScalar(y));
         }
     }
     return "";
 }
 
 SkISize ImageSrc::size() const {
     SkAutoTUnref<SkData> encoded(SkData::NewFromFileName(fPath.c_str()));
-    SkBitmap bitmap;
-    if (!encoded || !SkImageDecoder::DecodeMemory(encoded->data(),
-                                                  encoded->size(),
-                                                  &bitmap,
-                                                  kUnknown_SkColorType,
-                                                  SkImageDecoder::kDecodeBounds_Mode)) {
-        return SkISize::Make(0,0);
+    if (FLAGS_codec) {
+        SkAutoTDelete<SkCodec> codec(SkCodec::NewFromData(encoded));
+        if (!codec) {
+            return SkISize::Make(0,0);
+        }
+        SkImageInfo info;
+        if (!codec->getInfo(&info)) {
+            return SkISize::Make(0,0);
+        }
+        return info.dimensions();
+    } else {
+        SkBitmap bitmap;
+        if (!encoded || !SkImageDecoder::DecodeMemory(encoded->data(),
+                                                      encoded->size(),
+                                                      &bitmap,
+                                                      kUnknown_SkColorType,
+                                                      SkImageDecoder::kDecodeBounds_Mode)) {
+            return SkISize::Make(0,0);
+        }
+        return bitmap.dimensions();
     }
-    return bitmap.dimensions();
 }
 
 Name ImageSrc::name() const {
     return SkOSPath::Basename(fPath.c_str());
 }
 
 /*~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~*/
 
 static const SkRect kSKPViewport = {0,0, 1000,1000};
 
@@ skipping 347 matching lines @@
             surfaces.unrefAll();
             return "";
         }
         SkISize size() const SK_OVERRIDE { return fSize; }
         Name name() const SK_OVERRIDE { sk_throw(); return ""; }  // No one should be calling this.
     } proxy(fW, fH, pic, src.size());
     return fSink->draw(proxy, bitmap, stream, log);
 }
 
 }  // namespace DM