GM: add --writeChecksumBasedFilenames option

gm/gmmain.cpp

Index: gm/gmmain.cpp
===================================================================
--- gm/gmmain.cpp	(revision 9460)
+++ gm/gmmain.cpp	(working copy)
@@ -188,7 +188,8 @@
 
 class GMMain {
 public:
-    GMMain() : fUseFileHierarchy(false), fIgnorableErrorTypes(kDefaultIgnorableErrorTypes),
+    GMMain() : fUseFileHierarchy(false), fWriteChecksumBasedFilenames(false),
+               fIgnorableErrorTypes(kDefaultIgnorableErrorTypes),
                fMismatchPath(NULL), fTestsRun(0), fRenderModesEncountered(1) {}
 
     /**
@@ -225,6 +226,30 @@
         return SkOSPath::SkPathJoin(path, filename.c_str());
     }
 
+    /**
+     * Assemble filename suitable for writing out an SkBitmap.
+     */
+    SkString make_bitmap_filename(const char *path,
+                                  const char *shortName,
+                                  const char *configName,
+                                  const char *renderModeDescriptor,
+                                  const GmResultDigest &bitmapDigest) {
+        if (fWriteChecksumBasedFilenames) {
+            SkString filename;
+            filename.append(bitmapDigest.getHashType());
+            filename.appendUnichar('_');

[epoger, 2013/06/18 07:21:03]

In patchset 4, I decided that it will be easier across the board if we flatten
the checksum-based filenames so that they can all be written into a single
directory.

+            filename.append(shortName);
+            filename.appendUnichar('_');
+            filename.append(bitmapDigest.getDigestValue());
+            filename.appendUnichar('.');
+            filename.append(kPNG_FileExtension);
+            return SkOSPath::SkPathJoin(path, filename.c_str());
+        } else {
+            return make_filename(path, shortName, configName, renderModeDescriptor,
+                                 kPNG_FileExtension);
+        }
+    }
+
     /* since PNG insists on unpremultiplying our alpha, we take no
        precision chances and force all pixels to be 100% opaque,
        otherwise on compare we may not get a perfect match.
@@ -635,6 +660,7 @@
     ErrorCombination write_reference_image(const ConfigData& gRec, const char writePath [],
                                            const char renderModeDescriptor [],
                                            const char *shortName, SkBitmap& bitmap,
+                                           GmResultDigest& bitmapDigest,
                                            SkDynamicMemoryWStream* document) {
         SkString path;
         bool success = false;
@@ -642,8 +668,8 @@
             gRec.fBackend == kGPU_Backend ||
             (gRec.fBackend == kPDF_Backend && CAN_IMAGE_PDF)) {
 
-            path = make_filename(writePath, shortName, gRec.fName, renderModeDescriptor,
-                                 kPNG_FileExtension);
+            path = make_bitmap_filename(writePath, shortName, gRec.fName, renderModeDescriptor,
+                                        bitmapDigest);
             success = write_bitmap(path, bitmap);
         }
         if (kPDF_Backend == gRec.fBackend) {
@@ -741,6 +767,7 @@
      *
      * @param expectations what expectations to compare actualBitmap against
      * @param actualBitmap the image we actually generated
+     * @param actualResultDigest GmResultDigest of actualBitmap
      * @param shortName name of test, e.g. "selftest1"
      * @param configName name of config, e.g. "8888"
      * @param renderModeDescriptor e.g., "-rtree", "-deferred"
@@ -753,11 +780,11 @@
      */
     ErrorCombination compare_to_expectations(Expectations expectations,
                                              const SkBitmap& actualBitmap,
+                                             const GmResultDigest& actualResultDigest,

[scroggo, 2013/06/19 14:55:46]

I ran into this earlier when I was modifying skimage - I hate passing in
actualResultDigest, which depends on another parameter being passed in to the
same function. The caller wants the GmResultDigest too, regardless of whether
this function gets called.

Maybe this function should assert that actualResultDigest matches actualBitmap?

[epoger, 2013/06/19 15:02:16]

As discussed live... I will change the code to just pass the bitmaps around, so
we don't have a chance of the bitmap and digest getting out of sync.

I will add TODOs saying something like: we can optimize this later by creating a
new object type that encapsulates both const SkBitmap and const GmResultDigest,
guaranteeing that they stay in sync.

[epoger, 2013/06/19 15:44:08]

Heck, it's just as easy to write it now.

                                              const char *shortName, const char *configName,
                                              const char *renderModeDescriptor,
                                              bool addToJsonSummary) {
         ErrorCombination errors;
-        GmResultDigest actualResultDigest(actualBitmap);
         SkString shortNamePlusConfig = make_shortname_plus_config(shortName, configName);
         SkString completeNameString(shortNamePlusConfig);
         completeNameString.append(renderModeDescriptor);
@@ -780,9 +807,8 @@
             // Write out the "actuals" for any mismatches, if we have
             // been directed to do so.
             if (fMismatchPath) {
-                SkString path =
-                    make_filename(fMismatchPath, shortName, configName, renderModeDescriptor,
-                                  kPNG_FileExtension);
+                SkString path = make_bitmap_filename(fMismatchPath, shortName, configName,
+                                                     renderModeDescriptor, actualResultDigest);
                 write_bitmap(path, actualBitmap);
             }
 
@@ -871,6 +897,7 @@
         GM* gm, const ConfigData& gRec, const char writePath[],
         SkBitmap& actualBitmap, SkDynamicMemoryWStream* pdf) {
 
+        GmResultDigest actualResultDigest(actualBitmap);
         SkString shortNamePlusConfig = make_shortname_plus_config(gm->shortName(), gRec.fName);
         SkString nameWithExtension(shortNamePlusConfig);
         nameWithExtension.append(".");
@@ -893,12 +920,11 @@
              * See comments above complete_bitmap() for more detail.
              */
             Expectations expectations = expectationsSource->get(nameWithExtension.c_str());
-            errors.add(compare_to_expectations(expectations, actualBitmap,
+            errors.add(compare_to_expectations(expectations, actualBitmap, actualResultDigest,
                                                gm->shortName(), gRec.fName, "", true));
         } else {
             // If we are running without expectations, we still want to
             // record the actual results.
-            GmResultDigest actualResultDigest(actualBitmap);
             add_actual_results_to_json_summary(nameWithExtension.c_str(), actualResultDigest,
                                                ErrorCombination(kMissingExpectations_ErrorType),
                                                false);
@@ -912,7 +938,7 @@
         // renderModes of all tests!  That would be a lot of files.
         if (writePath && (gRec.fFlags & kWrite_ConfigFlag)) {
             errors.add(write_reference_image(gRec, writePath, "", gm->shortName(),
-                                             actualBitmap, pdf));
+                                             actualBitmap, actualResultDigest, pdf));
         }
 
         return errors;
@@ -933,7 +959,8 @@
 
         SkASSERT(referenceBitmap);
         Expectations expectations(*referenceBitmap);
-        return compare_to_expectations(expectations, actualBitmap, shortName,
+        GmResultDigest actualResultDigest(actualBitmap);
+        return compare_to_expectations(expectations, actualBitmap, actualResultDigest, shortName,
                                        configName, renderModeDescriptor, false);
     }
 
@@ -1143,7 +1170,7 @@
     // They are public for now, to allow easier setting by tool_main().
     //
 
-    bool fUseFileHierarchy;
+    bool fUseFileHierarchy, fWriteChecksumBasedFilenames;
     ErrorCombination fIgnorableErrorTypes;
 
     const char* fMismatchPath;
@@ -1303,9 +1330,11 @@
 DEFINE_bool(tileGrid, true, "Exercise the tile grid variant of SkPicture.");
 DEFINE_string(tileGridReplayScales, "", "Space separated list of floating-point scale "
               "factors to be used for tileGrid playback testing. Default value: 1.0");
-DEFINE_string(writeJsonSummaryPath, "", "Write a JSON-formatted result summary to this file.");
 DEFINE_bool2(verbose, v, false, "Give more detail (e.g. list all GMs run, more info about "
              "each test).");
+DEFINE_bool(writeChecksumBasedFilenames, false, "When writing out actual images, use checksum-"
+            "based filenames, as rebaseline.py will use when downloading them from Google Storage");
+DEFINE_string(writeJsonSummaryPath, "", "Write a JSON-formatted result summary to this file.");
 DEFINE_string2(writePath, w, "",  "Write rendered images into this directory.");
 DEFINE_string2(writePicturePath, p, "", "Write .skp files into this directory.");
 DEFINE_int32(pdfJpegQuality, -1, "Encodes images in JPEG at quality level N, "
@@ -1762,6 +1791,7 @@
     SkCommandLineFlags::Parse(argc, argv);
 
     gmmain.fUseFileHierarchy = FLAGS_hierarchy;
+    gmmain.fWriteChecksumBasedFilenames = FLAGS_writeChecksumBasedFilenames;
     if (FLAGS_mismatchPath.count() == 1) {
         gmmain.fMismatchPath = FLAGS_mismatchPath[0];
     }