Suggestions and merge in the other CL.

I had some suggestions on the subset CL, and took the opportunity to rebase it
against head and merge in the other color type CL.

BUG=skia:

Review URL: https://codereview.chromium.org/893703002

diff --git a/dm/DM.cpp b/dm/DM.cpp
index 6dc7e66..f087a32 100644 (file)
--- a/dm/DM.cpp
+++ b/dm/DM.cpp
@@ -16,7 +16,7 @@
 #include "Timer.h"
 
 DEFINE_string(images, "resources", "Images to decode.");
-DEFINE_string(src, "tests gm skp image", "Source types to test.");
+DEFINE_string(src, "tests gm skp image subset", "Source types to test.");
 DEFINE_bool(nameByHash, false,
             "If true, write to FLAGS_writePath[0]/<hash>.png instead of "
             "to FLAGS_writePath[0]/<config>/<sourceType>/<name>.png");
@@ -145,13 +145,13 @@ static void gather_srcs() {
                 for (SkString file; it.next(&file); ) {
                     SkString path = SkOSPath::Join(flag, file.c_str());
                     push_src("image", new ImageSrc(path));     // Decode entire image.
-                    push_src("image", new ImageSrc(path, 5));  // Decode 5 random subsets.
+                    push_src("subset", new ImageSrc(path, 2)); // Decode into 2 x 2 subsets
                 }
             }
         } else if (sk_exists(flag)) {
             // assume that FLAGS_images[i] is a valid image if it is a file.
             push_src("image", new ImageSrc(flag));     // Decode entire image.
-            push_src("image", new ImageSrc(flag, 5));  // Decode 5 random subsets.
+            push_src("subset", new ImageSrc(flag, 2)); // Decode into 2 x 2 subsets
         }
     }
 }

diff --git a/dm/DMSrcSink.cpp b/dm/DMSrcSink.cpp
index 39b92cd..f3ed574 100644 (file)
--- a/dm/DMSrcSink.cpp
+++ b/dm/DMSrcSink.cpp
@@ -31,24 +31,26 @@ Name GMSrc::name() const {
 
 /*~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~*/
 
-ImageSrc::ImageSrc(Path path, int subsets) : fPath(path), fSubsets(subsets) {}
+ImageSrc::ImageSrc(Path path, int divisor) : fPath(path), fDivisor(divisor) {}
 
 Error ImageSrc::draw(SkCanvas* canvas) const {
     SkAutoTUnref<SkData> encoded(SkData::NewFromFileName(fPath.c_str()));
     if (!encoded) {
         return SkStringPrintf("Couldn't read %s.", fPath.c_str());
     }
-    if (fSubsets == 0) {
+    const SkColorType dstColorType = canvas->imageInfo().colorType();
+    if (fDivisor == 0) {
         // Decode the full image.
         SkBitmap bitmap;
-        if (!SkImageDecoder::DecodeMemory(encoded->data(), encoded->size(), &bitmap)) {
+        if (!SkImageDecoder::DecodeMemory(encoded->data(), encoded->size(), &bitmap,
+                                          dstColorType, SkImageDecoder::kDecodePixels_Mode)) {
             return SkStringPrintf("Couldn't decode %s.", fPath.c_str());
         }
         encoded.reset((SkData*)NULL);  // Might as well drop this when we're done with it.
         canvas->drawBitmap(bitmap, 0,0);
         return "";
     }
-    // Decode random subsets.  This is a little involved.
+    // Decode subsets.  This is a little involved.
     SkAutoTDelete<SkMemoryStream> stream(new SkMemoryStream(encoded));
     SkAutoTDelete<SkImageDecoder> decoder(SkImageDecoder::Factory(stream.get()));
     if (!decoder) {
@@ -59,21 +61,24 @@ Error ImageSrc::draw(SkCanvas* canvas) const {
     if (!decoder->buildTileIndex(stream.detach(), &w, &h) || w*h == 1) {
         return "";  // Not an error.  Subset decoding is not always supported.
     }
-    SkRandom rand;
-    for (int i = 0; i < fSubsets; i++) {
-        SkIRect rect;
-        do {
-            rect.fLeft   = rand.nextULessThan(w);
-            rect.fTop    = rand.nextULessThan(h);
-            rect.fRight  = rand.nextULessThan(w);
-            rect.fBottom = rand.nextULessThan(h);
-            rect.sort();
-        } while (rect.isEmpty());
-        SkBitmap subset;
-        if (!decoder->decodeSubset(&subset, rect, kUnknown_SkColorType/*use best fit*/)) {
-            return SkStringPrintf("Could not decode subset %d.\n", i);
+
+    // Divide the image into subsets that cover the entire image.
+    if (fDivisor > w || fDivisor > h) {
+        return SkStringPrintf("divisor %d is too big for %s with dimensions (%d x %d)",
+                              fDivisor, fPath.c_str(), w, h);
+    }
+    const int subsetWidth  = w / fDivisor,
+              subsetHeight = h / fDivisor;
+    for (int y = 0; y < h; y += subsetHeight) {
+        for (int x = 0; x < w; x += subsetWidth) {
+            SkBitmap subset;
+            SkIRect rect = SkIRect::MakeXYWH(x, y, subsetWidth, subsetHeight);
+            if (!decoder->decodeSubset(&subset, rect, dstColorType)) {
+                return SkStringPrintf("Could not decode subset (%d, %d, %d, %d).",
+                                      x, y, x+subsetWidth, y+subsetHeight);
+            }
+            canvas->drawBitmap(subset, SkIntToScalar(x), SkIntToScalar(y));
         }
-        canvas->drawBitmap(subset, SkIntToScalar(rect.fLeft), SkIntToScalar(rect.fTop));
     }
     return "";
 }
@@ -92,11 +97,7 @@ SkISize ImageSrc::size() const {
 }
 
 Name ImageSrc::name() const {
-    Name name = SkOSPath::Basename(fPath.c_str());
-    if (fSubsets > 0) {
-        name.appendf("-%d-subsets", fSubsets);
-    }
-    return name;
+    return SkOSPath::Basename(fPath.c_str());
 }
 
 /*~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~*/

diff --git a/dm/DMSrcSink.h b/dm/DMSrcSink.h
index 16f7c14..bf8e7d6 100644 (file)
--- a/dm/DMSrcSink.h
+++ b/dm/DMSrcSink.h
@@ -61,14 +61,16 @@ private:
 
 class ImageSrc : public Src {
 public:
-    explicit ImageSrc(Path path, int subsets = 0);
+    // divisor == 0 means decode the whole image
+    // divisor > 0 means decode in subsets, dividing into a divisor x divisor grid.
+    explicit ImageSrc(Path path, int divisor = 0);
 
     Error draw(SkCanvas*) const SK_OVERRIDE;
     SkISize size() const SK_OVERRIDE;
     Name name() const SK_OVERRIDE;
 private:
     Path fPath;
-    int  fSubsets;
+    const int  fDivisor;
 };
 
 class SKPSrc : public Src {