GM: make behavior of make_filename() more late-binding

gm/gmmain.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.
  */
 
 /*
  * Code for the "gm" (Golden Master) rendering comparison tool.
  *
@@ skipping 174 matching lines @@
 
 const static ErrorCombination kDefaultIgnorableErrorTypes = ErrorCombination()
     .plus(kMissingExpectations_ErrorType)
     .plus(kIntentionallySkipped_ErrorType);
 
 class GMMain {
 public:
     GMMain() : fUseFileHierarchy(false), fIgnorableErrorTypes(kDefaultIgnorableErrorTypes),
                fMismatchPath(NULL), fTestsRun(0), fRenderModesEncountered(1) {}
 
-    SkString make_name(const char shortName[], const char configName[]) {
+    SkString make_filename(const char *path,
+                           const char *renderModeDescriptor,
+                           const char *shortNamePlusConfig,
+                           const char *suffix) {
+        SkString filename(shortNamePlusConfig);
+        filename.append(renderModeDescriptor);
+        filename.appendUnichar('.');
+        filename.append(suffix);
+        return SkPathJoin(path, filename.c_str());
+    }
+
+    /**
+     * Assemble shortNamePlusConfig from (surprise!) shortName and configName.
+     *
+     * The method for doing so depends on whether we are using hierarchical naming.
+     * For example, shortName "selftest1" and configName "8888" could be assembled into
+     * either "selftest1_8888" or "8888/selftest1".
+     */
+    SkString make_shortname_plus_config(const char *shortName, const char *configName) {

[epoger, 2013/05/19 09:00:43]

Patchset 2 renames some variables, parameters, and this method to make it
clearer where we are passing around "config/shortName" vs just "shortName". 
Still no behavioral changes, as verified using the self-tests.

         SkString name;
         if (0 == strlen(configName)) {
             name.append(shortName);
         } else if (fUseFileHierarchy) {
             name.appendf("%s%c%s", configName, SkPATH_SEPARATOR, shortName);
         } else {
             name.appendf("%s_%s", shortName, configName);
         }
         return name;
     }
@@ skipping 56 matching lines @@
             }
         }
     }
 
     /**
      * Records the results of this test in fTestsRun and fFailedTests.
      *
      * We even record successes, and errors that we regard as
      * "ignorable"; we can filter them out later.
      */
-    void RecordTestResults(const ErrorCombination& errorCombination, const SkString& name,
+    void RecordTestResults(const ErrorCombination& errorCombination,
+                           const SkString& shortNamePlusConfig,
                            const char renderModeDescriptor []) {
         // Things to do regardless of errorCombination.
         fTestsRun++;
         int renderModeCount = 0;
         this->fRenderModesEncountered.find(renderModeDescriptor, &renderModeCount);
         renderModeCount++;
         this->fRenderModesEncountered.set(renderModeDescriptor, renderModeCount);
 
         if (errorCombination.isEmpty()) {
             return;
         }
 
         // Things to do only if there is some error condition.
-        SkString fullName = name;
+        SkString fullName = shortNamePlusConfig;
         fullName.append(renderModeDescriptor);
         for (int typeInt = 0; typeInt <= kLast_ErrorType; typeInt++) {
             ErrorType type = static_cast<ErrorType>(typeInt);
             if (errorCombination.includes(type)) {
                 fFailedTests[type].push_back(fullName);
             }
         }
     }
 
     /**
@@ skipping 299 matching lines @@
         dev->beginPortfolio(&xps);
         dev->beginSheet(unitsPerMeter, pixelsPerMeter, trimSize);
         invokeGM(gm, &c, false, false);
         dev->endSheet();
         dev->endPortfolio();
 
 #endif
     }
 
     ErrorCombination write_reference_image(const ConfigData& gRec, const char writePath [],
-                                           const char renderModeDescriptor [], const SkString& name,
+                                           const char renderModeDescriptor [],
+                                           const SkString& shortNamePlusConfig,
                                            SkBitmap& bitmap, SkDynamicMemoryWStream* document) {
         SkString path;
         bool success = false;
         if (gRec.fBackend == kRaster_Backend ||
             gRec.fBackend == kGPU_Backend ||
             (gRec.fBackend == kPDF_Backend && CAN_IMAGE_PDF)) {
 
-            path = make_filename(writePath, renderModeDescriptor, name.c_str(),
+            path = make_filename(writePath, renderModeDescriptor, shortNamePlusConfig.c_str(),
                                  kPNG_FileExtension);
             success = write_bitmap(path, bitmap);
         }
         if (kPDF_Backend == gRec.fBackend) {
-            path = make_filename(writePath, renderModeDescriptor, name.c_str(),
+            path = make_filename(writePath, renderModeDescriptor, shortNamePlusConfig.c_str(),
                                  "pdf");
             success = write_document(path, *document);
         }
         if (kXPS_Backend == gRec.fBackend) {
-            path = make_filename(writePath, renderModeDescriptor, name.c_str(),
+            path = make_filename(writePath, renderModeDescriptor, shortNamePlusConfig.c_str(),
                                  "xps");
             success = write_document(path, *document);
         }
         if (success) {
             return kEmpty_ErrorCombination;
         } else {
             gm_fprintf(stderr, "FAILED to write %s\n", path.c_str());
             ErrorCombination errors(kWritingReferenceImage_ErrorType);
             // TODO(epoger): Don't call RecordTestResults() here...
             // Instead, we should make sure to call RecordTestResults
             // exactly ONCE per test.  (Otherwise, gmmain.fTestsRun
             // will be incremented twice for this test: once in
             // compare_test_results_to_stored_expectations() before
             // that method calls this one, and again here.)
             //
             // When we make that change, we should probably add a
             // WritingReferenceImage test to the gm self-tests.)
-            RecordTestResults(errors, name, renderModeDescriptor);
+            RecordTestResults(errors, shortNamePlusConfig, renderModeDescriptor);
             return errors;
         }
     }
 
     /**
      * Log more detail about the mistmatch between expectedBitmap and
      * actualBitmap.
      */
     void report_bitmap_diffs(const SkBitmap& expectedBitmap, const SkBitmap& actualBitmap,
                              const char *testName) {
@@ skipping 49 matching lines @@
 
     /**
      * Compares actual hash digest to expectations, returning the set of errors
      * (if any) that we saw along the way.
      *
      * If fMismatchPath has been set, and there are pixel diffs, then the
      * actual bitmap will be written out to a file within fMismatchPath.
      *
      * @param expectations what expectations to compare actualBitmap against
      * @param actualBitmap the image we actually generated
-     * @param baseNameString name of test without renderModeDescriptor added
+     * @param shortNamePlusConfig name of test without renderModeDescriptor added
      * @param renderModeDescriptor e.g., "-rtree", "-deferred"
      * @param addToJsonSummary whether to add these results (both actual and
      *        expected) to the JSON summary. Regardless of this setting, if
      *        we find an image mismatch in this test, we will write these
      *        results to the JSON summary.  (This is so that we will always
      *        report errors across rendering modes, such as pipe vs tiled.
      *        See https://codereview.chromium.org/13650002/ )
      */
     ErrorCombination compare_to_expectations(Expectations expectations,
                                              const SkBitmap& actualBitmap,
-                                             const SkString& baseNameString,
+                                             const SkString& shortNamePlusConfig,
                                              const char renderModeDescriptor[],
                                              bool addToJsonSummary) {
         ErrorCombination errors;
         SkHashDigest actualBitmapHash;
         // TODO(epoger): Better handling for error returned by ComputeDigest()?
         // For now, we just report a digest of 0 in error cases, like before.
         if (!SkBitmapHasher::ComputeDigest(actualBitmap, &actualBitmapHash)) {
             actualBitmapHash = 0;
         }
-        SkString completeNameString = baseNameString;
+        SkString completeNameString = shortNamePlusConfig;
         completeNameString.append(renderModeDescriptor);
         completeNameString.append(".");
         completeNameString.append(kPNG_FileExtension);
         const char* completeName = completeNameString.c_str();
 
         if (expectations.empty()) {
             errors.add(kMissingExpectations_ErrorType);
         } else if (!expectations.match(actualBitmapHash)) {
             addToJsonSummary = true;
             // The error mode we record depends on whether this was running
             // in a non-standard renderMode.
             if ('\0' == *renderModeDescriptor) {
                 errors.add(kExpectationsMismatch_ErrorType);
             } else {
                 errors.add(kRenderModeMismatch_ErrorType);
             }
 
             // Write out the "actuals" for any mismatches, if we have
             // been directed to do so.
             if (fMismatchPath) {
                 SkString path =
                     make_filename(fMismatchPath, renderModeDescriptor,
-                                  baseNameString.c_str(), kPNG_FileExtension);
+                                  shortNamePlusConfig.c_str(), kPNG_FileExtension);
                 write_bitmap(path, actualBitmap);
             }
 
             // If we have access to a single expected bitmap, log more
             // detail about the mismatch.
             const SkBitmap *expectedBitmapPtr = expectations.asBitmap();
             if (NULL != expectedBitmapPtr) {
                 report_bitmap_diffs(*expectedBitmapPtr, actualBitmap, completeName);
             }
         }
-        RecordTestResults(errors, baseNameString, renderModeDescriptor);
+        RecordTestResults(errors, shortNamePlusConfig, renderModeDescriptor);
 
         if (addToJsonSummary) {
             add_actual_results_to_json_summary(completeName, actualBitmapHash, errors,
                                                expectations.ignoreFailure());
             add_expected_results_to_json_summary(completeName, expectations);
         }
 
         return errors;
     }
 
@@ skipping 57 matching lines @@
      * @param gRec
      * @param writePath unless this is NULL, write out actual images into this
      *        directory
      * @param actualBitmap bitmap generated by this run
      * @param pdf
      */
     ErrorCombination compare_test_results_to_stored_expectations(
         GM* gm, const ConfigData& gRec, const char writePath[],
         SkBitmap& actualBitmap, SkDynamicMemoryWStream* pdf) {
 
-        SkString name = make_name(gm->shortName(), gRec.fName);
+        SkString shortNamePlusConfig = make_shortname_plus_config(gm->shortName(), gRec.fName);
+        SkString nameWithExtension(shortNamePlusConfig);
+        nameWithExtension.append(".");
+        nameWithExtension.append(kPNG_FileExtension);
+
         ErrorCombination errors;
-
         ExpectationsSource *expectationsSource = this->fExpectationsSource.get();
         if (expectationsSource && (gRec.fFlags & kRead_ConfigFlag)) {
             /*
              * Get the expected results for this test, as one or more allowed
              * hash digests. The current implementation of expectationsSource
              * get this by computing the hash digest of a single PNG file on disk.
              *
              * TODO(epoger): This relies on the fact that
              * force_all_opaque() was called on the bitmap before it
              * was written to disk as a PNG in the first place. If
              * not, the hash digest returned here may not match the
              * hash digest of actualBitmap, which *has* been run through
              * force_all_opaque().
              * See comments above complete_bitmap() for more detail.
              */
-            SkString nameWithExtension(name);
-            nameWithExtension.append(".");
-            nameWithExtension.append(kPNG_FileExtension);
             Expectations expectations = expectationsSource->get(nameWithExtension.c_str());
             errors.add(compare_to_expectations(expectations, actualBitmap,
-                                               name, "", true));
+                                               shortNamePlusConfig, "", true));
         } else {
             // If we are running without expectations, we still want to
             // record the actual results.
             SkHashDigest actualBitmapHash;
             // TODO(epoger): Better handling for error returned by ComputeDigest()?
             // For now, we just report a digest of 0 in error cases, like before.
             if (!SkBitmapHasher::ComputeDigest(actualBitmap, &actualBitmapHash)) {
                 actualBitmapHash = 0;
             }
-            SkString nameWithExtension(name);
-            nameWithExtension.append(".");
-            nameWithExtension.append(kPNG_FileExtension);
             add_actual_results_to_json_summary(nameWithExtension.c_str(), actualBitmapHash,
                                                ErrorCombination(kMissingExpectations_ErrorType),
                                                false);
-            RecordTestResults(ErrorCombination(kMissingExpectations_ErrorType), name, "");
+            RecordTestResults(ErrorCombination(kMissingExpectations_ErrorType),
+                              shortNamePlusConfig, "");
         }
 
         // TODO: Consider moving this into compare_to_expectations(),
         // similar to fMismatchPath... for now, we don't do that, because
         // we don't want to write out the actual bitmaps for all
         // renderModes of all tests!  That would be a lot of files.
         if (writePath && (gRec.fFlags & kWrite_ConfigFlag)) {
             errors.add(write_reference_image(gRec, writePath, "",
-                                             name, actualBitmap, pdf));
+                                             shortNamePlusConfig, actualBitmap, pdf));
         }
 
         return errors;
     }
 
     /**
      * Compare actualBitmap to referenceBitmap.
      *
-     * @param baseNameString name of test without renderModeDescriptor added
+     * @param shortNamePlusConfig name of test without renderModeDescriptor added
      * @param renderModeDescriptor
      * @param actualBitmap actual bitmap generated by this run
      * @param referenceBitmap bitmap we expected to be generated
      */
     ErrorCombination compare_test_results_to_reference_bitmap(
-        const SkString& baseNameString, const char renderModeDescriptor[],
+        const SkString& shortNamePlusConfig, const char renderModeDescriptor[],
         SkBitmap& actualBitmap, const SkBitmap* referenceBitmap) {
 
         SkASSERT(referenceBitmap);
         Expectations expectations(*referenceBitmap);
         return compare_to_expectations(expectations, actualBitmap,
-                                       baseNameString, renderModeDescriptor, false);
+                                       shortNamePlusConfig, renderModeDescriptor, false);
     }
 
     static SkPicture* generate_new_picture(GM* gm, BbhType bbhType, uint32_t recordFlags,
                                            SkScalar scale = SK_Scalar1) {
         // Pictures are refcounted so must be on heap
         SkPicture* pict;
         int width = SkScalarCeilToInt(SkScalarMul(SkIntToScalar(gm->getISize().width()), scale));
         int height = SkScalarCeilToInt(SkScalarMul(SkIntToScalar(gm->getISize().height()), scale));
 
         if (kTileGrid_BbhType == bbhType) {
@@ skipping 96 matching lines @@
             // https://codereview.chromium.org/12992003/ is unblocked.
             //
             // Filed as https://code.google.com/p/skia/issues/detail?id=1180
             // ('image-surface gm test is failing in "deferred" mode,
             // and gm is not reporting the failure')
             if (errors.isEmpty()) {
                 // TODO(epoger): Report this as a new ErrorType,
                 // something like kImageGeneration_ErrorType?
                 return kEmpty_ErrorCombination;
             }
-            const SkString name = make_name(gm->shortName(), gRec.fName);
+            const SkString shortNamePlusConfig = make_shortname_plus_config(gm->shortName(),
+                                                                            gRec.fName);
             return compare_test_results_to_reference_bitmap(
-                name, renderModeDescriptor, bitmap, &referenceBitmap);
+                shortNamePlusConfig, renderModeDescriptor, bitmap, &referenceBitmap);
         }
         return kEmpty_ErrorCombination;
     }
 
     ErrorCombination test_pipe_playback(GM* gm, const ConfigData& gRec,
                                         const SkBitmap& referenceBitmap, bool simulateFailure) {
-        const SkString name = make_name(gm->shortName(), gRec.fName);
+        const SkString shortNamePlusConfig = make_shortname_plus_config(gm->shortName(),
+                                                                        gRec.fName);
         ErrorCombination errors;
         for (size_t i = 0; i < SK_ARRAY_COUNT(gPipeWritingFlagCombos); ++i) {
             SkString renderModeDescriptor("-pipe");
             renderModeDescriptor.append(gPipeWritingFlagCombos[i].name);
 
             if (gm->getFlags() & GM::kSkipPipe_Flag) {
-                RecordTestResults(kIntentionallySkipped_ErrorType, name,
+                RecordTestResults(kIntentionallySkipped_ErrorType, shortNamePlusConfig,
                                   renderModeDescriptor.c_str());
                 errors.add(kIntentionallySkipped_ErrorType);
             } else {
                 SkBitmap bitmap;
                 SkISize size = gm->getISize();
                 setup_bitmap(gRec, size, &bitmap);
                 SkCanvas canvas(bitmap);
                 installFilter(&canvas);
                 // Pass a decoding function so the factory GM (which has an SkBitmap
                 // with encoded data) will not fail playback.
                 PipeController pipeController(&canvas, &SkImageDecoder::DecodeMemory);
                 SkGPipeWriter writer;
                 SkCanvas* pipeCanvas = writer.startRecording(&pipeController,
                                                              gPipeWritingFlagCombos[i].flags,
                                                              size.width(), size.height());
                 if (!simulateFailure) {
                     invokeGM(gm, pipeCanvas, false, false);
                 }
                 complete_bitmap(&bitmap);
                 writer.endRecording();
                 errors.add(compare_test_results_to_reference_bitmap(
-                    name, renderModeDescriptor.c_str(), bitmap, &referenceBitmap));
+                    shortNamePlusConfig, renderModeDescriptor.c_str(), bitmap, &referenceBitmap));
                 if (!errors.isEmpty()) {
                     break;
                 }
             }
         }
         return errors;
     }
 
     ErrorCombination test_tiled_pipe_playback(GM* gm, const ConfigData& gRec,
                                               const SkBitmap& referenceBitmap) {
-        const SkString name = make_name(gm->shortName(), gRec.fName);
+        const SkString shortNamePlusConfig = make_shortname_plus_config(gm->shortName(),
+                                                                        gRec.fName);
         ErrorCombination errors;
         for (size_t i = 0; i < SK_ARRAY_COUNT(gPipeWritingFlagCombos); ++i) {
             SkString renderModeDescriptor("-tiled pipe");
             renderModeDescriptor.append(gPipeWritingFlagCombos[i].name);
 
             if ((gm->getFlags() & GM::kSkipPipe_Flag) ||
                 (gm->getFlags() & GM::kSkipTiled_Flag)) {
-                RecordTestResults(kIntentionallySkipped_ErrorType, name,
+                RecordTestResults(kIntentionallySkipped_ErrorType, shortNamePlusConfig,
                                   renderModeDescriptor.c_str());
                 errors.add(kIntentionallySkipped_ErrorType);
             } else {
                 SkBitmap bitmap;
                 SkISize size = gm->getISize();
                 setup_bitmap(gRec, size, &bitmap);
                 SkCanvas canvas(bitmap);
                 installFilter(&canvas);
                 TiledPipeController pipeController(bitmap, &SkImageDecoder::DecodeMemory);
                 SkGPipeWriter writer;
                 SkCanvas* pipeCanvas = writer.startRecording(&pipeController,
                                                              gPipeWritingFlagCombos[i].flags,
                                                              size.width(), size.height());
                 invokeGM(gm, pipeCanvas, false, false);
                 complete_bitmap(&bitmap);
                 writer.endRecording();
-                errors.add(compare_test_results_to_reference_bitmap(name,
+                errors.add(compare_test_results_to_reference_bitmap(shortNamePlusConfig,
                                                                     renderModeDescriptor.c_str(),
                                                                     bitmap, &referenceBitmap));
                 if (!errors.isEmpty()) {
                     break;
                 }
             }
         }
         return errors;
     }
 
@@ skipping 299 matching lines @@
 ErrorCombination run_multiple_configs(GMMain &gmmain, GM *gm, const SkTDArray<size_t> &configs,
                                       GrContextFactory *grFactory);
 ErrorCombination run_multiple_configs(GMMain &gmmain, GM *gm, const SkTDArray<size_t> &configs,
                                       GrContextFactory *grFactory) {
     const char renderModeDescriptor[] = "";
     ErrorCombination errorsForAllConfigs;
     uint32_t gmFlags = gm->getFlags();
 
     for (int i = 0; i < configs.count(); i++) {
         ConfigData config = gRec[configs[i]];
-        const SkString name = gmmain.make_name(gm->shortName(), config.fName);
+        const SkString shortNamePlusConfig = gmmain.make_shortname_plus_config(gm->shortName(),
+                                                                               config.fName);
 
         // Skip any tests that we don't even need to try.
         // If any of these were skipped on a per-GM basis, record them as
         // kIntentionallySkipped.
         if (kPDF_Backend == config.fBackend) {
             if (!FLAGS_pdf) {
                 continue;
             }
             if (gmFlags & GM::kSkipPDF_Flag) {
-                gmmain.RecordTestResults(kIntentionallySkipped_ErrorType, name,
+                gmmain.RecordTestResults(kIntentionallySkipped_ErrorType, shortNamePlusConfig,
                                          renderModeDescriptor);
                 errorsForAllConfigs.add(kIntentionallySkipped_ErrorType);
                 continue;
             }
         }
         if ((gmFlags & GM::kSkip565_Flag) &&
             (kRaster_Backend == config.fBackend) &&
             (SkBitmap::kRGB_565_Config == config.fConfig)) {
-            gmmain.RecordTestResults(kIntentionallySkipped_ErrorType, name,
+            gmmain.RecordTestResults(kIntentionallySkipped_ErrorType, shortNamePlusConfig,
                                      renderModeDescriptor);
             errorsForAllConfigs.add(kIntentionallySkipped_ErrorType);
             continue;
         }
         if ((gmFlags & GM::kSkipGPU_Flag) &&
             kGPU_Backend == config.fBackend) {
-            gmmain.RecordTestResults(kIntentionallySkipped_ErrorType, name,
+            gmmain.RecordTestResults(kIntentionallySkipped_ErrorType, shortNamePlusConfig,
                                      renderModeDescriptor);
             errorsForAllConfigs.add(kIntentionallySkipped_ErrorType);
             continue;
         }
 
         // Now we know that we want to run this test and record its
         // success or failure.
         ErrorCombination errorsForThisConfig;
         GrSurface* gpuTarget = NULL;
 #if SK_SUPPORT_GPU
@@ skipping 66 matching lines @@
  * Returns all errors encountered while doing so.
  */
 ErrorCombination run_multiple_modes(GMMain &gmmain, GM *gm, const ConfigData &compareConfig,
                                     const SkBitmap &comparisonBitmap,
                                     const SkTDArray<SkScalar> &tileGridReplayScales);
 ErrorCombination run_multiple_modes(GMMain &gmmain, GM *gm, const ConfigData &compareConfig,
                                     const SkBitmap &comparisonBitmap,
                                     const SkTDArray<SkScalar> &tileGridReplayScales) {
     ErrorCombination errorsForAllModes;
     uint32_t gmFlags = gm->getFlags();
-    const SkString name = gmmain.make_name(gm->shortName(), compareConfig.fName);
+    const SkString shortNamePlusConfig = gmmain.make_shortname_plus_config(gm->shortName(),
+                                                                           compareConfig.fName);
 
     SkPicture* pict = gmmain.generate_new_picture(gm, kNone_BbhType, 0);
     SkAutoUnref aur(pict);
     if (FLAGS_replay) {
         const char renderModeDescriptor[] = "-replay";
         if (gmFlags & GM::kSkipPicture_Flag) {
-            gmmain.RecordTestResults(kIntentionallySkipped_ErrorType, name, renderModeDescriptor);
+            gmmain.RecordTestResults(kIntentionallySkipped_ErrorType, shortNamePlusConfig,
+                                     renderModeDescriptor);
             errorsForAllModes.add(kIntentionallySkipped_ErrorType);
         } else {
             SkBitmap bitmap;
             gmmain.generate_image_from_picture(gm, compareConfig, pict, &bitmap);
             errorsForAllModes.add(gmmain.compare_test_results_to_reference_bitmap(
-                name, renderModeDescriptor, bitmap, &comparisonBitmap));
+                shortNamePlusConfig, renderModeDescriptor, bitmap, &comparisonBitmap));
         }
     }
 
     if (FLAGS_serialize) {
         const char renderModeDescriptor[] = "-serialize";
         if (gmFlags & GM::kSkipPicture_Flag) {
-            gmmain.RecordTestResults(kIntentionallySkipped_ErrorType, name, renderModeDescriptor);
+            gmmain.RecordTestResults(kIntentionallySkipped_ErrorType, shortNamePlusConfig,
+                                     renderModeDescriptor);
             errorsForAllModes.add(kIntentionallySkipped_ErrorType);
         } else {
             SkPicture* repict = gmmain.stream_to_new_picture(*pict);
             SkAutoUnref aurr(repict);
             SkBitmap bitmap;
             gmmain.generate_image_from_picture(gm, compareConfig, repict, &bitmap);
             errorsForAllModes.add(gmmain.compare_test_results_to_reference_bitmap(
-                name, renderModeDescriptor, bitmap, &comparisonBitmap));
+                shortNamePlusConfig, renderModeDescriptor, bitmap, &comparisonBitmap));
         }
     }
 
     if ((1 == FLAGS_writePicturePath.count()) &&
         !(gmFlags & GM::kSkipPicture_Flag)) {
         const char* pictureSuffix = "skp";
-        SkString path = make_filename(FLAGS_writePicturePath[0], "",
-                                      gm->shortName(), pictureSuffix);
+        // TODO(epoger): In this case, we call make_filename() with

[epoger, 2013/05/19 09:00:43]

Any insights on this?  I think it's worth looking into in a separate CL...

+        // just the shortName instead of shortNamePlusConfig.  I
+        // suspect that is incorrect, because runs with different
+        // configs will write out to the same filename.
+        SkString path = gmmain.make_filename(FLAGS_writePicturePath[0], "",
+                                             gm->shortName(), pictureSuffix);
         SkFILEWStream stream(path.c_str());
         pict->serialize(&stream);
     }
 
     if (FLAGS_rtree) {
         const char renderModeDescriptor[] = "-rtree";
         if (gmFlags & GM::kSkipPicture_Flag) {
-            gmmain.RecordTestResults(kIntentionallySkipped_ErrorType, name, renderModeDescriptor);
+            gmmain.RecordTestResults(kIntentionallySkipped_ErrorType, shortNamePlusConfig,
+                                     renderModeDescriptor);
             errorsForAllModes.add(kIntentionallySkipped_ErrorType);
         } else {
             SkPicture* pict = gmmain.generate_new_picture(
                 gm, kRTree_BbhType, SkPicture::kUsePathBoundsForClip_RecordingFlag);
             SkAutoUnref aur(pict);
             SkBitmap bitmap;
             gmmain.generate_image_from_picture(gm, compareConfig, pict, &bitmap);
             errorsForAllModes.add(gmmain.compare_test_results_to_reference_bitmap(
-                name, renderModeDescriptor, bitmap, &comparisonBitmap));
+                shortNamePlusConfig, renderModeDescriptor, bitmap, &comparisonBitmap));
         }
     }
 
     if (FLAGS_tileGrid) {
         for(int scaleIndex = 0; scaleIndex < tileGridReplayScales.count(); ++scaleIndex) {
             SkScalar replayScale = tileGridReplayScales[scaleIndex];
             SkString renderModeDescriptor("-tilegrid");
             if (SK_Scalar1 != replayScale) {
                 renderModeDescriptor += "-scale-";
                 renderModeDescriptor.appendScalar(replayScale);
             }
 
             if ((gmFlags & GM::kSkipPicture_Flag) ||
                 ((gmFlags & GM::kSkipScaledReplay_Flag) && replayScale != 1)) {
-                gmmain.RecordTestResults(kIntentionallySkipped_ErrorType, name,
+                gmmain.RecordTestResults(kIntentionallySkipped_ErrorType, shortNamePlusConfig,
                                          renderModeDescriptor.c_str());
                 errorsForAllModes.add(kIntentionallySkipped_ErrorType);
             } else {
                 // We record with the reciprocal scale to obtain a replay
                 // result that can be validated against comparisonBitmap.
                 SkScalar recordScale = SkScalarInvert(replayScale);
                 SkPicture* pict = gmmain.generate_new_picture(
                     gm, kTileGrid_BbhType, SkPicture::kUsePathBoundsForClip_RecordingFlag,
                     recordScale);
                 SkAutoUnref aur(pict);
                 SkBitmap bitmap;
                 // We cannot yet pass 'true' to generate_image_from_picture to
                 // perform actual tiled rendering (see Issue 1198 -
                 // https://code.google.com/p/skia/issues/detail?id=1198)
                 gmmain.generate_image_from_picture(gm, compareConfig, pict, &bitmap,
                                                    replayScale /*, true */);
                 errorsForAllModes.add(gmmain.compare_test_results_to_reference_bitmap(
-                    name, renderModeDescriptor.c_str(), bitmap, &comparisonBitmap));
+                    shortNamePlusConfig, renderModeDescriptor.c_str(), bitmap, &comparisonBitmap));
             }
         }
     }
 
     // run the pipe centric GM steps
     if (FLAGS_pipe) {
         errorsForAllModes.add(gmmain.test_pipe_playback(gm, compareConfig, comparisonBitmap,
                                                         FLAGS_simulatePipePlaybackFailure));
         if (FLAGS_tiledPipe) {
             errorsForAllModes.add(gmmain.test_tiled_pipe_playback(gm, compareConfig,
@@ skipping 410 matching lines @@
     if (FLAGS_forceBWtext) {
         canvas->setDrawFilter(SkNEW(BWTextDrawFilter))->unref();
     }
 }
 
 #if !defined(SK_BUILD_FOR_IOS) && !defined(SK_BUILD_FOR_NACL)
 int main(int argc, char * const argv[]) {
     return tool_main(argc, (char**) argv);
 }
 #endif