dm changes

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 123 matching lines @@
                     // We consider incomplete to be valid, since we should still decode what is
                     // available.
                 case SkImageGenerator::kIncompleteInput:
                     break;
                 case SkImageGenerator::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: {
             SkScanlineDecoder* scanlineDecoder = codec->getScanlineDecoder(decodeInfo, NULL,
                     colorPtr, colorCountPtr);
             if (NULL == scanlineDecoder) {
                 return Error::Nonfatal("Cannot use scanline decoder for all images");
             }
             for (int y = 0; y < decodeInfo.height(); ++y) {
                 const SkImageGenerator::Result result = scanlineDecoder->getScanlines(
                         bitmap.getAddr(0, y), 1, 0);
                 switch (result) {
                     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) 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 "";
 }
 
 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();
     } else {
         return SkISize::Make(0, 0);
@@ skipping 603 matching lines @@
             skr.visit<void>(i, drawsAsSingletonPictures);
         }
         SkAutoTUnref<SkPicture> macroPic(macroRec.endRecordingAsPicture());
 
         canvas->drawPicture(macroPic);
         return "";
     });
 }
 
 }  // namespace DM