DM testing for skipping and scaling

dm/DMSrcSink.cpp

Index: dm/DMSrcSink.cpp
diff --git a/dm/DMSrcSink.cpp b/dm/DMSrcSink.cpp
index 0a90e03103dc23b087a3269f24434ec925f90d05..fbad320fc2eb4300cd2216871b835546d6077861 100644
--- a/dm/DMSrcSink.cpp
+++ b/dm/DMSrcSink.cpp
@@ -64,10 +64,11 @@ void GMSrc::modifyGrContextOptions(GrContextOptions* options) const {
 
 /*~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~*/
 
-CodecSrc::CodecSrc(Path path, Mode mode, DstColorType dstColorType)
+CodecSrc::CodecSrc(Path path, Mode mode, DstColorType dstColorType, float scale)
     : fPath(path)
     , fMode(mode)
     , fDstColorType(dstColorType)
+    , fScale(scale)
 {}
 
 Error CodecSrc::draw(SkCanvas* canvas) const {
@@ -108,6 +109,13 @@ Error CodecSrc::draw(SkCanvas* canvas) const {
             break;
     }
 
+    // Try to scale the image if it is desired
+    SkISize size = codec->getScaledDimensions(fScale);
+    if (size == decodeInfo.dimensions() && 1.0f != fScale) {
+        return Error::Nonfatal("Test without scaling is uninteresting.");
+    }
+    decodeInfo = decodeInfo.makeWH(size.width(), size.height());
+
     // Construct a color table for the decode if necessary
     SkAutoTUnref<SkColorTable> colorTable(NULL);
     SkPMColor* colorPtr = NULL;
@@ -195,7 +203,7 @@ Error CodecSrc::draw(SkCanvas* canvas) const {
             * subsetBm's size is determined based on the current subset and may be larger for end
             * subsets.
             */
-            SkImageInfo largestSubsetDecodeInfo = 
+            SkImageInfo largestSubsetDecodeInfo =
                     decodeInfo.makeWH(subsetWidth + extraX, subsetHeight + extraY);
             SkBitmap largestSubsetBm;
             if (!largestSubsetBm.tryAllocPixels(largestSubsetDecodeInfo, NULL, colorTable.get())) {
@@ -226,7 +234,7 @@ Error CodecSrc::draw(SkCanvas* canvas) const {
                         }
                     }
                     //skip to first line of subset
-                    const SkImageGenerator::Result skipResult = 
+                    const SkImageGenerator::Result skipResult =
                             subsetScanlineDecoder->skipScanlines(y);
                     switch (skipResult) {
                         case SkImageGenerator::kSuccess:
@@ -262,6 +270,54 @@ Error CodecSrc::draw(SkCanvas* canvas) const {
             }
             break;
         }
+        // This mode tests the skipping of scanlines.

[scroggo, 2015/06/11 20:12:48]

Maybe the description belongs in the header, by the declaration of kStripe_Mode?

[msarett, 2015/06/11 21:20:27]

Done.

+        case kStripe_Mode: {
+            int width = decodeInfo.width();

[scroggo, 2015/06/11 20:12:48]

Can these be const?

[msarett, 2015/06/11 21:20:27]

Done.

+            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;
+
+            // Create bitmap
+            SkBitmap bitmap;
+            if (!bitmap.tryAllocPixels(decodeInfo, NULL, colorTable.get())) {
+                return SkStringPrintf("Cannot allocate pixels for %s %dx%d", fPath.c_str(),
+                        width, height);
+            }
+
+            // Decode odd stripes
+            SkScanlineDecoder* decoder = codec->getScanlineDecoder(decodeInfo, NULL, colorPtr,
+                    colorCountPtr);
+            if (NULL == decoder) {
+                return Error::Nonfatal("Cannot use scanline decoder for all images");
+            }
+            for (int i = 0; i < numStripes; i += 2) {
+                // Skip a stripe
+                const int linesToSkip = SkTMin(stripeHeight, height - i * stripeHeight);

[scroggo, 2015/06/11 20:12:48]

Will this ever be less than stripeHeight? It seems like we would either have a
full stripe to skip, or there is less than a full stripe to skip, in which case
we won't have anything to read down below.

Ok, I think I see: down below, we might read 0 scanlines?

[msarett, 2015/06/11 21:20:27]

Yes, we might read 0 scanlines below.

+                decoder->skipScanlines(linesToSkip);

[scroggo, 2015/06/11 20:12:48]

I think we want to return an error if this or getScanlines fails.

[msarett, 2015/06/11 21:20:27]

Done.

+
+                // Read a stripe
+                const int startY = SkTMin((i + 1) * stripeHeight, height - 1);

[scroggo, 2015/06/11 20:12:48]

When will we use height - 1? It seems like if the first value is too large,
we've gone past the end. Or do we special case so that we'll read the last line?
Is that so the final skip was meaningful? Maybe add a comment; this seems a
little tricky to me.

[msarett, 2015/06/11 21:20:27]

Yes this is tricky and a bug.  The original code was:
const int startY = (i + 1) * stripeHeight;
Which can go past the end of the image, but works with the code below because
linesToRead would then be 0.

I added SkTMin becuase bitmap.getAddr() will fail in Debug mode if startY >=
height.  But now this will cause us to always read at least one line below,
which we don't want.

The fix should clear things up.

+                const int linesToRead = SkTMax(0, SkTMin(stripeHeight, height - startY));
+                decoder->getScanlines(bitmap.getAddr(0, startY), linesToRead, bitmap.rowBytes());
+            }
+
+            // Decode even stripes
+            decoder = codec->getScanlineDecoder(decodeInfo, NULL, colorPtr, colorCountPtr);

[scroggo, 2015/06/11 20:12:48]

I think we want to return an error if decoder is null. (Since it should always
succeed the second time).

[msarett, 2015/06/11 21:20:27]

Done.

+            for (int i = 0; i < numStripes; i += 2) {
+                // Read a stripe
+                const int startY = i * stripeHeight;
+                const int linesToRead = SkTMin(stripeHeight, height - startY);
+                decoder->getScanlines(bitmap.getAddr(0, startY), linesToRead, bitmap.rowBytes());
+
+                // Skip a stripe
+                const int linesToSkip = SkTMax(0, SkTMin(stripeHeight,
+                        height - (i + 1) * stripeHeight));
+                decoder->skipScanlines(linesToSkip);
+            }
+            canvas->drawBitmap(bitmap, 0, 0);
+        }
     }
     return "";
 }
@@ -270,14 +326,19 @@ SkISize CodecSrc::size() const {
     SkAutoTUnref<SkData> encoded(SkData::NewFromFileName(fPath.c_str()));
     SkAutoTDelete<SkCodec> codec(SkCodec::NewFromData(encoded));
     if (NULL != codec) {
-        return codec->getInfo().dimensions();
+        SkISize size = codec->getScaledDimensions(fScale);
+        return size;
     } else {
         return SkISize::Make(0, 0);
     }
 }
 
 Name CodecSrc::name() const {
-    return SkOSPath::Basename(fPath.c_str());
+    if (1.0f == fScale) {
+        return SkOSPath::Basename(fPath.c_str());
+    } else {
+        return SkStringPrintf("%s_%.3f", SkOSPath::Basename(fPath.c_str()).c_str(), fScale);
+    }
 }
 
 /*~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~*/