Use separate subset directories in skimage.

tools/skimage_main.cpp

Index: tools/skimage_main.cpp
diff --git a/tools/skimage_main.cpp b/tools/skimage_main.cpp
index 2041f7bbcc5a4991fc81791519b65f4de86ea068..e3a1c86543eddaa6494972e794b4bf6bdc5c3053 100644
--- a/tools/skimage_main.cpp
+++ b/tools/skimage_main.cpp
@@ -299,25 +299,55 @@ static void decodeFileAndWrite(const char srcPath[], const SkString* writePath)
                     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);
                 }
             }