Scanline decoding for bmp

dm/DMSrcSink.cpp

Index: dm/DMSrcSink.cpp
diff --git a/dm/DMSrcSink.cpp b/dm/DMSrcSink.cpp
index a7240b8579e636d29e27efa93147a95efa5a4319..f9fa818a7826e305e5d443f2eec8705d5b194e32 100644
--- a/dm/DMSrcSink.cpp
+++ b/dm/DMSrcSink.cpp
@@ -79,6 +79,23 @@ bool CodecSrc::veto(SinkFlags flags) const {
         || flags.approach != SinkFlags::kDirect;
 }
 
+SkScanlineDecoder* start_scanline_decoder(SkData* encoded, const SkImageInfo& info,
+        SkPMColor* colorPtr, int* colorCountPtr) {
+    SkAutoTDelete<SkScanlineDecoder> scanlineDecoder(SkScanlineDecoder::NewFromData(encoded));
+    if (NULL == scanlineDecoder) {
+        return NULL;
+    }
+    // DM scanline test assume kTopDown scanline ordering.  Other orderings are
+    // tested from within SkScaledCodec.
+    if (SkScanlineDecoder::kTopDown_SkScanlineOrder != scanlineDecoder->getScanlineOrder()) {
+        return NULL;
+    }
+    if (SkCodec::kSuccess != scanlineDecoder->start(info, NULL, colorPtr, colorCountPtr)) {
+        return NULL;
+    }
+    return scanlineDecoder.detach();
+}
+
 Error CodecSrc::draw(SkCanvas* canvas) const {
     SkAutoTUnref<SkData> encoded(SkData::NewFromFileName(fPath.c_str()));
     if (!encoded) {
@@ -170,10 +187,9 @@ Error CodecSrc::draw(SkCanvas* canvas) const {
         }
         case kScanline_Mode: {
             SkAutoTDelete<SkScanlineDecoder> scanlineDecoder(
-                    SkScanlineDecoder::NewFromData(encoded));
-            if (NULL == scanlineDecoder || SkCodec::kSuccess !=
-                    scanlineDecoder->start(decodeInfo, NULL, colorPtr, colorCountPtr)) {
-                return Error::Nonfatal("Cannot use scanline decoder for all images");
+                    start_scanline_decoder(encoded.get(), decodeInfo, colorPtr, colorCountPtr));
+            if (NULL == scanlineDecoder) {
+                return Error::Nonfatal("Could not start top-down scanline decoder");
             }
 
             const SkCodec::Result result = scanlineDecoder->getScanlines(
@@ -234,18 +250,20 @@ Error CodecSrc::draw(SkCanvas* canvas) const {
                     const int y = row * subsetHeight;
                     //create scanline decoder for each subset
                     SkAutoTDelete<SkScanlineDecoder> subsetScanlineDecoder(
-                            SkScanlineDecoder::NewFromData(encoded));
-                    if (NULL == subsetScanlineDecoder || SkCodec::kSuccess !=
-                            subsetScanlineDecoder->start(
-                            decodeInfo, NULL, colorPtr, colorCountPtr))
-                    {
+                            start_scanline_decoder(encoded.get(), decodeInfo,
+                                    colorPtr, colorCountPtr));
+                    if (NULL == subsetScanlineDecoder) {
                         if (x == 0 && y == 0) {
                             //first try, image may not be compatible
-                            return Error::Nonfatal("Cannot use scanline decoder for all images");
+                            return Error::Nonfatal("Could not start top-down scanline decoder");
                         } else {
                             return "Error scanline decoder is NULL";
                         }
                     }
+                    if (SkScanlineDecoder::kTopDown_SkScanlineOrder !=
+                            subsetScanlineDecoder->getScanlineOrder()) {
+                        return Error::Nonfatal("Test mode not supported");

[scroggo, 2015/08/27 20:14:22]

Wait, why do we require kTopDown? Don't we know how many scanlines we need? It
seems like for kNone we should not have to do anything to support this. For
kOutOfOrder, we would need to write separate code for this, but maybe that could
be a TODO? (FWIW, we might want some common code somewhere that handles that
case.)

[msarett, 2015/08/27 21:21:46]

First, this check should be deleted because it is a duplicate of the one in
start_scanline_decoder().

However, I am adding a TODO.  I think you're right that we want common code to
deal with kOutOfOrder etc.  I think you can find that code in SkScaledCodec in
the gif scanline decoder review :/

I'm hesitant to make the change in this patchset because I think we need a more
serious redesign of our test cases here.  If we share the code used in
SkScaledCodec throughout these tests, we may have a bit of duplication of our
test cases.

[scroggo, 2015/08/28 13:28:46]

sgtm

+                    }
                     //skip to first line of subset
                     const SkCodec::Result skipResult =
                             subsetScanlineDecoder->skipScanlines(y);
@@ -304,10 +322,10 @@ Error CodecSrc::draw(SkCanvas* canvas) const {
             const int numStripes = (height + stripeHeight - 1) / stripeHeight;
 
             // Decode odd stripes
-            SkAutoTDelete<SkScanlineDecoder> decoder(SkScanlineDecoder::NewFromData(encoded));
-            if (NULL == decoder || SkCodec::kSuccess !=
-                    decoder->start(decodeInfo, NULL, colorPtr, colorCountPtr)) {
-                return Error::Nonfatal("Cannot use scanline decoder for all images");
+            SkAutoTDelete<SkScanlineDecoder> decoder(
+                    start_scanline_decoder(encoded.get(), decodeInfo, colorPtr, colorCountPtr));
+            if (NULL == decoder) {
+                return Error::Nonfatal("Could not start top-down scanline decoder");
             }
             for (int i = 0; i < numStripes; i += 2) {
                 // Skip a stripe