Various improvements to CodecSrc testing in dm

dm/DMSrcSink.cpp

 /*
  * Copyright 2015 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "DMSrcSink.h"
 #include "SamplePipeControllers.h"
 #include "SkCodec.h"
@@ skipping 67 matching lines @@
     return flags.type != SinkFlags::kRaster
         || flags.approach != SinkFlags::kDirect;
 }
 
 SkScanlineDecoder* start_scanline_decoder(SkData* encoded, const SkImageInfo& info,
         SkPMColor* colorPtr, int* colorCountPtr) {
     SkAutoTDelete<SkScanlineDecoder> scanlineDecoder(SkScanlineDecoder::NewFromData(encoded));
     if (nullptr == scanlineDecoder) {
         return nullptr;
     }
-    // DM scanline test assume kTopDown scanline ordering.  Other orderings are
-    // tested from within SkScaledCodec.
-    // TODO (msarett): Redesign the CodecSrc tests to improve our coverage of SkCodec and
-    //                 SkScanlineDecoder functionality.  Maybe we should write code to explicitly
-    //                 test kNone, kOutOfOrder, and kBottomUp.
-    if (SkScanlineDecoder::kTopDown_SkScanlineOrder != scanlineDecoder->getScanlineOrder()) {
-        return nullptr;
-    }
     if (SkCodec::kSuccess != scanlineDecoder->start(info, NULL, colorPtr, colorCountPtr)) {
         return nullptr;
     }
     return scanlineDecoder.detach();
 }
 
 Error CodecSrc::draw(SkCanvas* canvas) const {
     SkAutoTUnref<SkData> encoded(SkData::NewFromFileName(fPath.c_str()));
     if (!encoded) {
         return SkStringPrintf("Couldn't read %s.", fPath.c_str());
     }
-    SkAutoTDelete<SkCodec> codec(SkScaledCodec::NewFromData(encoded));
+    SkAutoTDelete<SkCodec> codec(NULL);
+    if (kScaledCodec_Mode == fMode) {
+        codec.reset(SkScaledCodec::NewFromData(encoded));
+        // TODO (msarett): This should fall throught to a fatal error once we support scaled

[scroggo, 2015/08/31 19:35:24]

nit: through*

[msarett, 2015/08/31 21:05:28]

Done.

+        //                 codecs for all image types.
+        if (nullptr == codec.get()) {
+            return Error::Nonfatal(SkStringPrintf("Couldn't create scaled codec for %s.",
+                    fPath.c_str()));
+        }
+    } else {
+        codec.reset(SkCodec::NewFromData(encoded));
+    }
     if (nullptr == codec.get()) {
-        // scaledCodec not supported, try normal codec
-        codec.reset(SkCodec::NewFromData(encoded));
-        if (nullptr == codec.get()) {
-            return SkStringPrintf("Couldn't create codec for %s.", fPath.c_str());
-        }
+        return SkStringPrintf("Couldn't create codec for %s.", fPath.c_str());
     }
 
     // Choose the color type to decode to
     SkImageInfo decodeInfo = codec->getInfo();
     SkColorType canvasColorType = canvas->imageInfo().colorType();
     switch (fDstColorType) {
         case kIndex8_Always_DstColorType:
             decodeInfo = codec->getInfo().makeColorType(kIndex_8_SkColorType);
             if (kRGB_565_SkColorType == canvasColorType) {
                 return Error::Nonfatal("Testing non-565 to 565 is uninteresting.");
@@ skipping 40 matching lines @@
         decodeInfo = decodeInfo.makeAlphaType(kPremul_SkAlphaType);
     }
 
     SkBitmap bitmap;
     if (!bitmap.tryAllocPixels(decodeInfo, nullptr, colorTable.get())) {
         return SkStringPrintf("Image(%s) is too large (%d x %d)\n", fPath.c_str(),
                               decodeInfo.width(), decodeInfo.height());
     }
 
     switch (fMode) {
-        case kNormal_Mode: {
+        case kScaledCodec_Mode:
+        case kCodec_Mode: {
             switch (codec->getPixels(decodeInfo, bitmap.getPixels(), bitmap.rowBytes(), nullptr,
                     colorPtr, colorCountPtr)) {
                 case SkCodec::kSuccess:
                     // We consider incomplete to be valid, since we should still decode what is
                     // available.
                 case SkCodec::kIncompleteInput:
                     break;
                 case SkCodec::kInvalidConversion:
                     return Error::Nonfatal("Incompatible colortype conversion");
                 default:
                     // Everything else is considered a failure.
                     return SkStringPrintf("Couldn't getPixels %s.", fPath.c_str());
             }
             canvas->drawBitmap(bitmap, 0, 0);
             break;
         }
         case kScanline_Mode: {
             SkAutoTDelete<SkScanlineDecoder> scanlineDecoder(
                     start_scanline_decoder(encoded.get(), decodeInfo, colorPtr, colorCountPtr));
             if (nullptr == scanlineDecoder) {
-                return Error::Nonfatal("Could not start top-down scanline decoder");
+                return Error::Nonfatal("Could not start scanline decoder");
             }
 
             const SkCodec::Result result = scanlineDecoder->getScanlines(
                     bitmap.getAddr(0, 0), decodeInfo.height(), bitmap.rowBytes());
             switch (result) {
                 case SkCodec::kSuccess:
                 case SkCodec::kIncompleteInput:
                     break;
                 default:
                     return SkStringPrintf("%s failed with error message %d",
                                           fPath.c_str(), (int) result);
             }
             canvas->drawBitmap(bitmap, 0, 0);
             break;
         }
         case kScanline_Subset_Mode: {

[msarett, 2015/08/31 18:05:52]

I'm wondering if maybe we should eliminate the scanline subset test after we
have implemented scanline subsetting in SkScaledCodec?  Any thoughts?

[scroggo, 2015/08/31 19:35:24]

Yes, I think we should. This serves as a proof-of-concept test.

             //this mode decodes the image in divisor*divisor subsets, using a scanline decoder
             const int divisor = 2;
             const int w = decodeInfo.width();
             const int h = decodeInfo.height();
             if (divisor > w || divisor > h) {
                 return Error::Nonfatal(SkStringPrintf("Cannot decode subset: divisor %d is too big"
                         "for %s with dimensions (%d x %d)", divisor, fPath.c_str(), w, h));
             }
             const int subsetWidth = w/divisor;
             const int subsetHeight = h/divisor;
             // One of our subsets will be larger to contain any pixels that do not divide evenly.
             const int extraX = w % divisor;
             const int extraY = h % divisor;
             /*
             * if w or h are not evenly divided by divisor need to adjust width and height of end
             * subsets to cover entire image.
             * Add extraX and extraY to largestSubsetBm's width and height to adjust width
             * and height of end subsets.
             * subsetBm is extracted from largestSubsetBm.
             * subsetBm's size is determined based on the current subset and may be larger for end
             * subsets.
             */
             SkImageInfo largestSubsetDecodeInfo =
                     decodeInfo.makeWH(subsetWidth + extraX, subsetHeight + extraY);
             SkBitmap largestSubsetBm;
-            if (!largestSubsetBm.tryAllocPixels(largestSubsetDecodeInfo, nullptr, colorTable.get())) {
+            if (!largestSubsetBm.tryAllocPixels(largestSubsetDecodeInfo, nullptr,
+                    colorTable.get())) {
                 return SkStringPrintf("Image(%s) is too large (%d x %d)\n", fPath.c_str(),
                         largestSubsetDecodeInfo.width(), largestSubsetDecodeInfo.height());
             }
             const size_t rowBytes = decodeInfo.minRowBytes();
             char* buffer = new char[largestSubsetDecodeInfo.height() * rowBytes];
             SkAutoTDeleteArray<char> lineDeleter(buffer);
             for (int col = 0; col < divisor; col++) {
                 //currentSubsetWidth may be larger than subsetWidth for rightmost subsets
                 const int currentSubsetWidth = (col + 1 == divisor) ?
                         subsetWidth + extraX : subsetWidth;
                 const int x = col * subsetWidth;
                 for (int row = 0; row < divisor; row++) {
                     //currentSubsetHeight may be larger than subsetHeight for bottom subsets
                     const int currentSubsetHeight = (row + 1 == divisor) ?
                             subsetHeight + extraY : subsetHeight;
                     const int y = row * subsetHeight;
                     //create scanline decoder for each subset
-                    SkAutoTDelete<SkScanlineDecoder> subsetScanlineDecoder(
-                            start_scanline_decoder(encoded.get(), decodeInfo,
-                                    colorPtr, colorCountPtr));
-                    if (nullptr == subsetScanlineDecoder) {
+                    SkAutoTDelete<SkScanlineDecoder> decoder(start_scanline_decoder(encoded.get(),
+                            decodeInfo, colorPtr, colorCountPtr));
+                    // TODO (msarett): Support this mode for all scanline orderings.
+                    if (nullptr == decoder || SkScanlineDecoder::kTopDown_SkScanlineOrder !=
+                            decoder->getScanlineOrder()) {
                         if (x == 0 && y == 0) {
                             //first try, image may not be compatible
                             return Error::Nonfatal("Could not start top-down scanline decoder");
                         } else {
                             return "Error scanline decoder is nullptr";
                         }
                     }
                     //skip to first line of subset
-                    const SkCodec::Result skipResult =
-                            subsetScanlineDecoder->skipScanlines(y);
+                    const SkCodec::Result skipResult = decoder->skipScanlines(y);
                     switch (skipResult) {
                         case SkCodec::kSuccess:
                         case SkCodec::kIncompleteInput:
                             break;
                         default:
                             return SkStringPrintf("%s failed after attempting to skip %d scanlines"
                                     "with error message %d", fPath.c_str(), y, (int) skipResult);
                     }
                     //create and set size of subsetBm
                     SkBitmap subsetBm;
                     SkIRect bounds = SkIRect::MakeWH(subsetWidth, subsetHeight);
                     bounds.setXYWH(0, 0, currentSubsetWidth, currentSubsetHeight);
                     SkAssertResult(largestSubsetBm.extractSubset(&subsetBm, bounds));
                     SkAutoLockPixels autlockSubsetBm(subsetBm, true);
                     const SkCodec::Result subsetResult =
-                        subsetScanlineDecoder->getScanlines(buffer, currentSubsetHeight, rowBytes);
+                            decoder->getScanlines(buffer, currentSubsetHeight, rowBytes);
                     switch (subsetResult) {
                         case SkCodec::kSuccess:
                         case SkCodec::kIncompleteInput:
                             break;
                         default:
                             return SkStringPrintf("%s failed with error message %d",
                                     fPath.c_str(), (int) subsetResult);
                     }
                     const size_t bpp = decodeInfo.bytesPerPixel();
                     /*
@@ skipping 21 matching lines @@
         case kStripe_Mode: {
             const int height = decodeInfo.height();
             // This value is chosen arbitrarily.  We exercise more cases by choosing a value that
             // does not align with image blocks.
             const int stripeHeight = 37;
             const int numStripes = (height + stripeHeight - 1) / stripeHeight;
 
             // Decode odd stripes
             SkAutoTDelete<SkScanlineDecoder> decoder(
                     start_scanline_decoder(encoded.get(), decodeInfo, colorPtr, colorCountPtr));
-            if (nullptr == decoder) {
+            if (nullptr == decoder ||
+                    SkScanlineDecoder::kTopDown_SkScanlineOrder != decoder->getScanlineOrder()) {
+                // This mode was designed to test the new skip scanlines API in libjpeg-turbo.
+                // Jpegs have kTopDown_SkScanlineOrder, and at this time, it is not interesting
+                // to run this test for image types that do not have this scanline ordering.
                 return Error::Nonfatal("Could not start top-down scanline decoder");
             }
             for (int i = 0; i < numStripes; i += 2) {
                 // Skip a stripe
                 const int linesToSkip = SkTMin(stripeHeight, height - i * stripeHeight);
                 SkCodec::Result result = decoder->skipScanlines(linesToSkip);
                 switch (result) {
                     case SkCodec::kSuccess:
                     case SkCodec::kIncompleteInput:
                         break;
@@ skipping 13 matching lines @@
                             break;
                         default:
                             return SkStringPrintf("Cannot get scanlines for %s.", fPath.c_str());
                     }
                 }
             }
 
             // Decode even stripes
             const SkCodec::Result startResult = decoder->start(decodeInfo, nullptr, colorPtr,
                                                                colorCountPtr);
-            if (SkCodec::kSuccess != startResult) {
+            if (SkCodec::kSuccess != startResult ||
+                    SkScanlineDecoder::kTopDown_SkScanlineOrder != decoder->getScanlineOrder()) {

[scroggo, 2015/08/31 19:35:24]

It seems like we should never reach here if kTopDown is not the order?

[msarett, 2015/08/31 21:05:28]

Not unless there is a bug in start().  I agree that it's unnecessary.

                 return "Failed to restart scanline decoder with same parameters.";
             }
             for (int i = 0; i < numStripes; i += 2) {
                 // Read a stripe
                 const int startY = i * stripeHeight;
                 const int linesToRead = SkTMin(stripeHeight, height - startY);
                 SkCodec::Result result = decoder->getScanlines(bitmap.getAddr(0, startY),
                         linesToRead, bitmap.rowBytes());
                 switch (result) {
                     case SkCodec::kSuccess:
@@ skipping 710 matching lines @@
             skr.visit<void>(i, drawsAsSingletonPictures);
         }
         SkAutoTUnref<SkPicture> macroPic(macroRec.endRecordingAsPicture());
 
         canvas->drawPicture(macroPic);
         return "";
     });
 }
 
 }  // namespace DM