Use separate subset directories in skimage.

tools/skimage_main.cpp

 /*
  * Copyright 2011 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "gm_expectations.h"
 #include "SkBitmap.h"
 #include "SkBitmapHasher.h"
@@ skipping 281 matching lines @@
                     if (compare_to_expectations_if_necessary(bitmapFromDecodeSubset,
                                                              subsetName.c_str(),
                                                              &gFailedSubsetDecodes)) {
                         gSuccessfulSubsetDecodes.push_back().printf("Decoded subset %s from %s",
                                                               subsetDim.c_str(), srcPath);
                     }
 
                     write_expectations(bitmapFromDecodeSubset, subsetName.c_str());
 
                     if (writePath != NULL) {
-                        // Write the region to a file whose name includes the dimensions.
-                        SkString suffix = SkStringPrintf("_%s.png", subsetDim.c_str());
-                        SkString outPath;
-                        make_outname(&outPath, writePath->c_str(), srcPath, suffix.c_str());
-                        SkDEBUGCODE(bool success =)
-                        write_bitmap(outPath.c_str(), &bitmapFromDecodeSubset);
-                        SkASSERT(success);
-                        gSuccessfulSubsetDecodes.push_back().printf("\twrote %s", outPath.c_str());
-                        // Also use extractSubset from the original for visual comparison.
-                        SkBitmap extractedSubset;
-                        if (bitmap.extractSubset(&extractedSubset, rect)) {
-                            suffix.printf("_%s_extracted.png", subsetDim.c_str());
-                            make_outname(&outPath, writePath->c_str(), srcPath, suffix.c_str());
-                            SkDEBUGCODE(success =) write_bitmap(outPath.c_str(), &extractedSubset);
-                            SkASSERT(success);
+                        // Create a subdirectory to hold the results of decodeSubset.
+                        SkString dir(*writePath);
+                        dir.appendf("%s%c", "subsets", SkPATH_SEPARATOR);
+                        if (sk_mkdir(dir.c_str())) {
+                            // Write the subset to a file whose name includes the dimensions.
+                            SkString suffix = SkStringPrintf("_%s.png", subsetDim.c_str());
+                            SkString outPath;
+                            make_outname(&outPath, dir.c_str(), srcPath, suffix.c_str());
+                            SkAssertResult(write_bitmap(outPath.c_str(), &bitmapFromDecodeSubset));
+                            gSuccessfulSubsetDecodes.push_back().printf("\twrote %s",
+                                                                        outPath.c_str());
+                            // Also use extractSubset from the original for visual comparison.
+                            // Write the result to a file in a separate subdirectory.
+                            SkBitmap extractedSubset;
+                            if (bitmap.extractSubset(&extractedSubset, rect)) {
+                                SkString dirExtracted(*writePath);
+                                dirExtracted.appendf("%s%c", "extracted", SkPATH_SEPARATOR);
+                                if (sk_mkdir(dirExtracted.c_str())) {
+                                    make_outname(&outPath, dirExtracted.c_str(), srcPath,
+                                                 suffix.c_str());
+                                    SkAssertResult(write_bitmap(outPath.c_str(),
+                                                                &extractedSubset));
+                                } else {
+                                    gFailedSubsetDecodes.push_back().printf("Successfully decoded "
+                                                                            "%s from %s, but "
+                                                                            "failed to create a "
+                                                                            "directory for "
+                                                                            "extractSubset "
+                                                                            "comparison.",
+                                                                            subsetDim.c_str(),
+                                                                            filename);
+                                }
+                            } else {
+                                gFailedSubsetDecodes.push_back().printf("Successfully decoded %s "
+                                                                        "from %s, but failed to "
+                                                                        "extract a similar subset "
+                                                                        "for comparison.",
+                                                                        subsetDim.c_str(),
+                                                                        filename);
+                            }
+                        } else {
+                            gFailedSubsetDecodes.push_back().printf("Successfully decoded %s from "

[epoger, 2013/05/14 15:50:31]

As we've discussed before, I think it's a lot cleaner to exit early in failure
cases like this:

1. The debug message is constructed much closer to the failed test (not 38 lines
later as in this case)
2. It saves you from multi-level indent hell

Maybe something like this:

/**
 * Copy of the method in gm/gm_expectations.cpp
 * TODO: move it into SkOSFile.h in a future CL
 */
SkString SkPathJoin(const char *rootPath, const char *relativePath) {
  blahblah;
}

bool write_subsets(const char *writePath, SkString *failureReason) {
  if (writePath == NULL) {
    return true;
  }

  SkString dir = SkPathJoin(writePath, "subsets");
  if (!sk_mkdir(dir.c_str())) {
    failureReason.printf("blah blah blah");
    return false;
  }

  blah blah;

  if (!bitmap.extractSubset(&extractedSubset, rect)) {
    failureReason.printf("blah blah blah");
    return false;
  }

  blah blah;
  return true;
}

[scroggo, 2013/05/14 20:13:13]

Done.

+                                                                    "%s, but failed to create a "
+                                                                    "directory to write to.",
+                                                                    subsetDim.c_str(), filename);
                         }
                     }
                 } else {
-                    gFailedSubsetDecodes.push_back().printf("Failed to decode region %s from %s\n",
+                    gFailedSubsetDecodes.push_back().printf("Failed to decode region %s from %s",
                                                             subsetDim.c_str(), srcPath);
                 }
             }
         }
     }
     if (FLAGS_reencode) {
         // Encode to the format the file was originally in, or PNG if the encoder for the same
         // format is unavailable.
         SkImageDecoder::Format format = codec->getFormat();
         if (SkImageDecoder::kUnknown_Format == format) {
@@ skipping 174 matching lines @@
     }
 
     return failed ? -1 : 0;
 }
 
 #if !defined SK_BUILD_FOR_IOS
 int main(int argc, char * const argv[]) {
     return tool_main(argc, (char**) argv);
 }
 #endif