dm changes

dm/DMSrcSink.cpp

Index: dm/DMSrcSink.cpp
diff --git a/dm/DMSrcSink.cpp b/dm/DMSrcSink.cpp
index 090a24d86a62590f123881274578b0468e59a01d..2679df28a6371a9010787bab5726d72d78c6c7d6 100644
--- a/dm/DMSrcSink.cpp
+++ b/dm/DMSrcSink.cpp
@@ -141,6 +141,7 @@ Error CodecSrc::draw(SkCanvas* canvas) const {
                     // Everything else is considered a failure.
                     return SkStringPrintf("Couldn't getPixels %s.", fPath.c_str());
             }
+            canvas->drawBitmap(bitmap, 0, 0);
             break;
         case kScanline_Mode: {
             SkScanlineDecoder* scanlineDecoder = codec->getScanlineDecoder(decodeInfo, NULL,
@@ -160,10 +161,109 @@ Error CodecSrc::draw(SkCanvas* canvas) const {
                                               fPath.c_str(), y-1, (int) result);
                 }
             }
+            canvas->drawBitmap(bitmap, 0, 0);
+            break;
+        }
+        case kScanline_Subset_Mode: {
+            //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 SkStringPrintf("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;
+            /*
+            * if w or h are not evenly divided by divisor need to adjust width and height of end 
+            * subsets to cover entire image.
+            * Add w%divisor and h%divisor 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 + w%divisor, subsetHeight + h%divisor);
+            SkBitmap largestSubsetBm;
+            if (!largestSubsetBm.tryAllocPixels(largestSubsetDecodeInfo, NULL, colorTable.get())) {
+                return SkStringPrintf("Image(%s) is too large (%d x %d)\n", fPath.c_str(),
+                        largestSubsetDecodeInfo.width(), largestSubsetDecodeInfo.height());
+            }
+            char* line = SkNEW_ARRAY(char, decodeInfo.minRowBytes());
+            SkAutoTDeleteArray<char> lineDeleter(line);
+            SkIRect bounds = SkIRect::MakeWH(subsetWidth, subsetHeight);
+            //used if image w and h are not evenly divided by divisor 
+            int extraX = 0;
+            int extraY = 0;
+            //currentSubsetWidth may be larger than subsetWidth for end subsets 
+            int currentSubsetWidth = subsetWidth + extraX;
+            int currentSubsetHeight = subsetHeight + extraY;
+            for (int x = 0; x < subsetWidth*divisor; x += subsetWidth) {
+                //consider extraX for rightmost subset 
+                if (x + 2*subsetWidth > w) {
+                    extraX = w - subsetWidth*divisor;

[scroggo, 2015/05/22 14:38:26]

Can we remove extraX and extraY entirely? I think you can just say:

currentSubsetWidth = subsetWidth + w - subsetWidth*divisor;

+                    currentSubsetWidth = subsetWidth + extraX;
+                }         
+                for (int y = 0; y < subsetHeight*divisor; y += subsetHeight) {
+                    //consider extraY for bottom subset
+                    if (y + 2*subsetHeight > h) {
+                        extraY = h - subsetHeight*divisor;
+                        currentSubsetHeight = subsetHeight + extraY;
+                    }
+                    //create scanline decoder for each subset
+                    SkScanlineDecoder* subsetScanlineDecoder = codec->getScanlineDecoder(decodeInfo, 
+                            NULL, 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");
+                        } else {
+                            return "Error scanline decoder is NULL";
+                        }
+                    }
+                    //skip to first line of subset
+                    const SkImageGenerator::Result skipResult = 
+                            subsetScanlineDecoder->skipScanlines(y);
+                    switch (skipResult) {
+                        case SkImageGenerator::kSuccess:
+                        case SkImageGenerator::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;
+                    bounds.setXYWH(0, 0, currentSubsetWidth, currentSubsetHeight);

[scroggo, 2015/05/22 14:38:26]

I think you can move the declaration of bounds here, right before it is used.

[emmaleeroach, 2015/05/22 15:58:54]

Acknowledged.

+                    SkASSERT(largestSubsetBm.extractSubset(&subsetBm, bounds));

[scroggo, 2015/05/22 14:38:26]

This is a little tricky: In Release mode, SkASSERT(something) gets removed
completely. There are a couple ways you can do this so the extract is still done
in Release:

SkDEBUGCODE(bool result =) largestSubsetBm.extractSubset(&subsetBm, bounds);
SkASSERT(result);

or

SkAssertResult(largestSubsetBm.extractSubset(&subsetBm, bounds));

[emmaleeroach, 2015/05/22 15:58:55]

Acknowledged.

+                    SkAutoLockPixels autlockSubsetBm(subsetBm, true);
+                    for (int subsetY = 0; subsetY < currentSubsetHeight; ++subsetY) {
+                        const SkImageGenerator::Result subsetResult = 
+                                subsetScanlineDecoder->getScanlines(line, 1, 0); 
+                        //copy section of line based on x value   
+                        memcpy(subsetBm.getAddr(0, subsetY), 
+                                line + x*largestSubsetDecodeInfo.bytesPerPixel(), 

[scroggo, 2015/05/22 14:38:26]

nit: since largestSubsetDecodeInfo.bytesPerPixel() is the same as
decodeInfo.bytesPerPixel(), this might be more readable if you use the latter.
It might even be better to declare a variable for bytesPerPixel, so this can fit
on one line:

const size_t bpp = decodeInfo.bytesPerPixel();
memcpy(substBm.getAddr(0, subsetY), line + x * bpp, currentSubsetWidth * bpp);

[emmaleeroach, 2015/05/22 15:58:55]

Acknowledged.

+                                currentSubsetWidth*largestSubsetDecodeInfo.bytesPerPixel());
+                        switch (subsetResult) {
+                            case SkImageGenerator::kSuccess:
+                            case SkImageGenerator::kIncompleteInput:
+                                break;
+                            default:
+                                return SkStringPrintf("%s failed after %d scanlines with error" 
+                                        "message %d", fPath.c_str(), y-1, (int) subsetResult);
+                        }
+                    }
+                    canvas->drawBitmap(subsetBm, x, y);
+                }
+                //reset currentSubsetHeight for each new column
+                extraY = 0;
+                currentSubsetHeight = subsetHeight + extraY;

[scroggo, 2015/05/22 14:38:26]

Alternatively, where you set currentSubsetHeight, you can say:

if (/* bottom subset */) {
  currentSubsetHeight = subsetHeight + /* extraY */;
} else {
  currentSubsetHeight = subsetHeight;
}

This should also mean that you can declare it inside the loop.

[emmaleeroach, 2015/05/22 15:58:55]

even when that is inside a loop?
instead of once if declared outside a loop
trade offs for doing this

[scroggo, 2015/05/22 18:39:53]

I am hesitant to say "always", but I will say that most of the time it is better
to declare a variable right before it is used.

In this case, IIUC, you are concerned that each time through the loop the
program will need to do extra work to create the variable if it's inside the
loop, whereas it would have only had to create the variable once if it is
outside the loop.

If it is faster to declare it outside the loop, you are giving up some
maintainability. The code may be harder to follow, and a larger scope means that
you could accidentally change it in more places.

I *think* the compiler will generate the same assembly either way. (Someone else
in the office could probably give you a better answer than this. You can also
use a disassembler to compare, which might be interesting.) If not, I think this
could be at most a small performance difference. When it comes to this kind of
micro-optimization, my general rule is to go with clarity unless a profile shows
it to be a problem.

Follow-up: A Google search led me to this excellent discussion on StackOverflow
(StackOverflow is your friend):
http://stackoverflow.com/questions/7959573/declaring-variables-inside-loops-g...

According to that page, it actually may be *faster* to declare the variable
inside the loop, since the compiler has more information about when the variable
is used.

One exception that was wisely pointed out is that it's a different question if
we're talking about a class with a constructor, which would mean calling the
constructor (and destructor) each time through the loop. In this case, we're
just talking about a primitive type, so that is not a problem.

+            }
             break;
         }
     }
-    canvas->drawBitmap(bitmap, 0, 0);
     return "";
 }