Support multiple PDF rendering backends in the GM

gm/gmmain.cpp

Index: gm/gmmain.cpp
diff --git a/gm/gmmain.cpp b/gm/gmmain.cpp
index 958a511eb7446dc07e12313b6b73684508aca76c..abdcb39ea83e9f5c0c6c7ae321b352eae40ed43c 100644
--- a/gm/gmmain.cpp
+++ b/gm/gmmain.cpp
@@ -30,6 +30,7 @@
 #include "SkImageDecoder.h"
 #include "SkImageEncoder.h"
 #include "SkOSFile.h"
+#include "SkPDFRasterizer.h"
 #include "SkPicture.h"
 #include "SkRefCnt.h"
 #include "SkStream.h"
@@ -83,9 +84,6 @@ extern bool gSkSuppressFontCachePurgeSpew;
 
 #ifdef SK_BUILD_FOR_MAC
     #include "SkCGUtils.h"
-    #define CAN_IMAGE_PDF   1
-#else
-    #define CAN_IMAGE_PDF   0
 #endif
 
 using namespace skiagm;
@@ -160,6 +158,12 @@ struct ConfigData {
     bool                            fRunByDefault;
 };
 
+struct PDFRasterizerData {

[vandebo (ex-Chrome), 2013/08/09 16:26:34]

PDFRasterizer PDFRasterizerInfo ?

[ducky, 2013/08/09 23:29:41]

Naming was to be consistent with ConfigData, which serves the same purpose for
configs to run.

+    bool (*fRasterizerFunction)(SkStream*, SkBitmap*);
+    const char*                     fName;

[vandebo (ex-Chrome), 2013/08/09 16:26:34]

spacing

[ducky, 2013/08/09 23:29:41]

Shrunk. This was to be consistent with ConfigData above.

[vandebo (ex-Chrome), 2013/08/12 17:41:56]

To be consistent, you'd line up fRasterizerFunction, since that's the name of
the field.

[ducky, 2013/08/12 21:57:09]

Done.

+    bool                            fRunByDefault;
+};
+
 class BWTextDrawFilter : public SkDrawFilter {
 public:
     virtual bool filter(SkPaint*, Type) SK_OVERRIDE;
@@ -282,14 +286,21 @@ public:
         }
     }
 
-    static bool write_bitmap(const SkString& path, const SkBitmap& bitmap) {
+    static ErrorCombination write_bitmap(const SkString& path, const SkBitmap& bitmap) {
         // TODO(epoger): Now that we have removed force_all_opaque()
         // from this method, we should be able to get rid of the
         // transformation to 8888 format also.
         SkBitmap copy;
         bitmap.copyTo(&copy, SkBitmap::kARGB_8888_Config);
-        return SkImageEncoder::EncodeFile(path.c_str(), copy,
-                                          SkImageEncoder::kPNG_Type, 100);
+        bool success = SkImageEncoder::EncodeFile(path.c_str(), copy,
+                                                  SkImageEncoder::kPNG_Type,
+                                                  100);
+        if (success) {
+            return kEmpty_ErrorCombination;
+        } else {
+            gm_fprintf(stderr, "FAILED to write bitmap: %s\n", path.c_str());
+            return ErrorCombination(kWritingReferenceImage_ErrorType);
+        }
     }
 
     /**
@@ -410,9 +421,15 @@ public:
         gm_fprintf(stdout, "(results marked with [*] will cause nonzero return value)\n");
     }
 
-    static bool write_document(const SkString& path, SkStreamAsset* asset) {
+    static ErrorCombination write_document(const SkString& path, SkStreamAsset* asset) {
         SkFILEWStream stream(path.c_str());
-        return stream.writeStream(asset, asset->getLength());
+        bool success = stream.writeStream(asset, asset->getLength());
+        if (success) {
+            return kEmpty_ErrorCombination;
+        } else {
+            gm_fprintf(stderr, "FAILED to write document: %s\n", path.c_str());
+            return ErrorCombination(kWritingReferenceImage_ErrorType);
+        }
     }
 
     /**
@@ -659,51 +676,6 @@ public:
 #endif
     }
 
-    ErrorCombination write_reference_image(const ConfigData& gRec, const char writePath [],
-                                           const char renderModeDescriptor [],
-                                           const char *shortName,
-                                           const BitmapAndDigest* bitmapAndDigest,
-                                           SkStreamAsset* document) {
-        SkString path;
-        bool success = false;
-        if (gRec.fBackend == kRaster_Backend ||
-            gRec.fBackend == kGPU_Backend ||
-            (gRec.fBackend == kPDF_Backend && CAN_IMAGE_PDF)) {
-
-            path = make_bitmap_filename(writePath, shortName, gRec.fName, renderModeDescriptor,
-                                        bitmapAndDigest->fDigest);
-            success = write_bitmap(path, bitmapAndDigest->fBitmap);
-        }
-        if (kPDF_Backend == gRec.fBackend) {
-            path = make_filename(writePath, shortName, gRec.fName, renderModeDescriptor,
-                                 "pdf");
-            success = write_document(path, document);
-        }
-        if (kXPS_Backend == gRec.fBackend) {
-            path = make_filename(writePath, shortName, gRec.fName, renderModeDescriptor,
-                                 "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, make_shortname_plus_config(shortName, gRec.fName),
-                              renderModeDescriptor);
-            return errors;
-        }
-    }
-
     /**
      * Log more detail about the mistmatch between expectedBitmap and
      * actualBitmap.
@@ -908,13 +880,10 @@ public:
      * @param document pdf or xps representation, if appropriate
      */
     ErrorCombination compare_test_results_to_stored_expectations(
-        GM* gm, const ConfigData& gRec, const char writePath[],
-        const BitmapAndDigest* actualBitmapAndDigest, SkStreamAsset* document) {
+        GM* gm, const ConfigData& gRec,
+        const BitmapAndDigest* actualBitmapAndDigest) {
 
         SkString shortNamePlusConfig = make_shortname_plus_config(gm->shortName(), gRec.fName);
-        SkString nameWithExtension(shortNamePlusConfig);
-        nameWithExtension.append(".");
-        nameWithExtension.append(kPNG_FileExtension);
 
         ErrorCombination errors;
 
@@ -922,18 +891,20 @@ public:
             // Note that we intentionally skipped validating the results for
             // this test, because we don't know how to generate an SkBitmap
             // version of the output.
-            RecordTestResults(ErrorCombination(kIntentionallySkipped_ErrorType),
-                              shortNamePlusConfig, "");
+            errors.add(ErrorCombination(kIntentionallySkipped_ErrorType));
         } else if (!(gRec.fFlags & kWrite_ConfigFlag)) {
             // We don't record the results for this test or compare them
             // against any expectations, because the output image isn't
             // meaningful.
             // See https://code.google.com/p/skia/issues/detail?id=1410 ('some
             // GM result images not available for download from Google Storage')
-            RecordTestResults(ErrorCombination(kIntentionallySkipped_ErrorType),
-                              shortNamePlusConfig, "");
+            errors.add(ErrorCombination(kIntentionallySkipped_ErrorType));
         } else {
             ExpectationsSource *expectationsSource = this->fExpectationsSource.get();
+            SkString nameWithExtension(shortNamePlusConfig);
+            nameWithExtension.append(".");
+            nameWithExtension.append(kPNG_FileExtension);
+
             if (expectationsSource && (gRec.fFlags & kRead_ConfigFlag)) {
                 /*
                  * Get the expected results for this test, as one or more allowed
@@ -958,20 +929,9 @@ public:
                                                    actualBitmapAndDigest->fDigest,
                                                    ErrorCombination(kMissingExpectations_ErrorType),
                                                    false);
-                RecordTestResults(ErrorCombination(kMissingExpectations_ErrorType),
-                                  shortNamePlusConfig, "");
+                errors.add(ErrorCombination(kMissingExpectations_ErrorType));
             }
         }
-
-        // 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, "", gm->shortName(),
-                                             actualBitmapAndDigest, document));
-        }
-
         return errors;
     }
 
@@ -1032,46 +992,88 @@ public:
 
     // Test: draw into a bitmap or pdf.
     // Depending on flags, possibly compare to an expected image.
-    ErrorCombination test_drawing(GM* gm,
-                                  const ConfigData& gRec,
+    ErrorCombination test_drawing(GM* gm, const ConfigData& gRec,
+                                  const SkTDArray<const PDFRasterizerData*> &pdfRasterizers,
                                   const char writePath [],
                                   GrSurface* gpuTarget,
                                   SkBitmap* bitmap) {
+        ErrorCombination errors;
         SkDynamicMemoryWStream document;
+        SkString path;
 
         if (gRec.fBackend == kRaster_Backend ||
             gRec.fBackend == kGPU_Backend) {
             // Early exit if we can't generate the image.
-            ErrorCombination errors = generate_image(gm, gRec, gpuTarget, bitmap, false);
+            errors.add(generate_image(gm, gRec, gpuTarget, bitmap, false));
             if (!errors.isEmpty()) {
                 // TODO: Add a test to exercise what the stdout and
                 // JSON look like if we get an "early error" while
                 // trying to generate the image.
                 return errors;
             }
+            BitmapAndDigest bitmapAndDigest(*bitmap);
+            errors.add(compare_test_results_to_stored_expectations(
+                           gm, gRec, &bitmapAndDigest));
+
+            if (gRec.fFlags & kWrite_ConfigFlag) {
+                path = make_bitmap_filename(writePath, gm->shortName(), gRec.fName,
+                                            "", bitmapAndDigest.fDigest);
+                errors.add(write_bitmap(path, bitmapAndDigest.fBitmap));
+            }
         } else if (gRec.fBackend == kPDF_Backend) {
             generate_pdf(gm, document);
-#if CAN_IMAGE_PDF
-            SkAutoDataUnref data(document.copyToData());
-            SkMemoryStream stream(data->data(), data->size());
-            SkPDFDocumentToBitmap(&stream, bitmap);
-#else
-            bitmap = NULL;  // we don't generate a bitmap rendering of the PDF file
-#endif
+
+            SkAutoTUnref<SkStreamAsset> documentStream(document.detachAsStream());
+            if (gRec.fFlags & kWrite_ConfigFlag) {
+                path = make_filename(writePath, gm->shortName(), gRec.fName, "", "pdf");
+                errors.add(write_document(path, documentStream));
+            }
+
+            for (int i = 0; i < pdfRasterizers.count(); i++) {
+                SkASSERT(documentStream->rewind());
+                bool success = (*pdfRasterizers[i]->fRasterizerFunction)
+                                        (documentStream.get(), bitmap);
+                if (!success) {
+                    gm_fprintf(stderr, "FAILED to render PDF for %s using renderer %s\n",
+                               gm->shortName(),
+                               pdfRasterizers[i]->fName);
+                    continue;
+                }
+
+                BitmapAndDigest bitmapAndDigest(*bitmap);
+                errors.add(compare_test_results_to_stored_expectations(
+                           gm, gRec, &bitmapAndDigest));
+
+                SkString configName(gRec.fName);
+                configName.append("_");
+                configName.append(pdfRasterizers[i]->fName);
+
+                if (gRec.fFlags & kWrite_ConfigFlag) {
+                    path = make_bitmap_filename(writePath, gm->shortName(), configName.c_str(),
+                                                "", bitmapAndDigest.fDigest);
+                    errors.add(write_bitmap(path, bitmapAndDigest.fBitmap));
+                }
+            }
+
+            // Though we may have a rasterizer and it may output a bitmap,

[vandebo (ex-Chrome), 2013/08/09 16:26:34]

Instead of having this comment and clearing the bitmap, just use a locally
scoped (and differently named) bitmap. Line 1033

[ducky, 2013/08/09 23:29:41]

Done.

+            // we may also have multiple rasterizers outputting different
+            // bitmaps. Since the output bitmap isn't used by anything except
+            // raster and GPU, the output bitmap is cleared.
+            bitmap = NULL;
         } else if (gRec.fBackend == kXPS_Backend) {
             generate_xps(gm, document);
-            bitmap = NULL;  // we don't generate a bitmap rendering of the XPS file
-        }
+            SkAutoTUnref<SkStreamAsset> documentStream(document.detachAsStream());
+
+            if (gRec.fFlags & kWrite_ConfigFlag) {
+                path = make_filename(writePath, gm->shortName(), gRec.fName, "", "xps");
+                errors.add(write_document(path, documentStream));
+            }
 
-        SkAutoTUnref<SkStreamAsset> documentStream(document.detachAsStream());

[vandebo (ex-Chrome), 2013/08/09 16:26:34]

It looks like you can leave this block of code here.... by that I mean there is
common code in the pdf, xps, and raster case, pull it to down here.

[ducky, 2013/08/09 23:29:41]

Not really. The PDF case can now have multiple bitmaps, and the old code doesn't
really support that. It's also ugly in how the document is passed along with the
bitmap just so it can be written two functions (or so) down.

The newer code is a bit less DRY, but I think the repetition is minimal and
overall is more elegant than the alternatives.

[vandebo (ex-Chrome), 2013/08/12 17:41:56]

Hmm, I see.  In that sense, different PDF renders should be be different
configs.  Of course that would mean generating the PDF multiple times. Maybe the
simplified framework (you wouldn't need to pass around the list of rasterizers)
would be worth the extra computation?  Probably not worth it though.... any
thoughts?

[ducky, 2013/08/12 21:57:09]

I don't think it would roll into configs easily, either - configs doesn't have a
way to specify functions to be called, just backend enums. You could specify
kPDFPoppler_Backend, kPDFMac_Backend, kPDFNative_Backend, but I think the code
to handle those would be messier and less elegant.

-        if (NULL == bitmap) {
-            return compare_test_results_to_stored_expectations(
-                gm, gRec, writePath, NULL, documentStream);
+            bitmap = NULL;
         } else {
-            BitmapAndDigest bitmapAndDigest(*bitmap);
-            return compare_test_results_to_stored_expectations(
-                gm, gRec, writePath, &bitmapAndDigest, documentStream);
+            SkASSERT(false);
         }
+        return errors;
     }
 
     ErrorCombination test_deferred_drawing(GM* gm,
@@ -1223,6 +1225,11 @@ static const GLContextType kDontCare_GLContextType = GrContextFactory::kNative_G
 static const GLContextType kDontCare_GLContextType = 0;
 #endif
 
+#if defined(SK_BUILD_FOR_MAC) || defined(SK_BUILD_POPPLER)
+#define CAN_IMAGE_PDF 1
+#else
+#define CAN_IMAGE_PDF 0
+#endif
 // If the platform does not support writing PNGs of PDFs then there will be no
 // reference images to read. However, we can always write the .pdf files
 static const ConfigFlags kPDFConfigFlags = CAN_IMAGE_PDF ? kRW_ConfigFlag :
@@ -1263,6 +1270,15 @@ static const ConfigData gRec[] = {
 #endif // SK_SUPPORT_PDF
 };
 
+static const PDFRasterizerData kPDFRasterizers[] = {
+#ifdef SK_BUILD_FOR_MAC
+    { &SkPDFDocumentToBitmap,   "macnative",    true },
+#endif
+#ifdef SK_BUILD_POPPLER
+    { &SkPopplerRasterizePDF,   "poppler",      true },
+#endif
+};
+
 static const char kDefaultsConfigStr[] = "defaults";
 static const char kExcludeConfigChar = '~';
 
@@ -1314,6 +1330,29 @@ static SkString configUsage() {
     return result;
 }
 
+static SkString pdfRasterizerUsage() {
+    SkString result;
+    result.appendf("Space delimited list of which PDF rasterizers to run. Possible options: [");
+    for (size_t i = 0; i < SK_ARRAY_COUNT(kPDFRasterizers); ++i) {
+        if (i > 0) {
+            result.append("|");
+        }
+        result.append(kPDFRasterizers[i].fName);
+    }
+    result.append("]\n");
+    result.append("The default value is: \"");
+    for (size_t i = 0; i < SK_ARRAY_COUNT(kPDFRasterizers); ++i) {
+        if (kPDFRasterizers[i].fRunByDefault) {
+            if (i > 0) {
+                result.append(" ");
+            }
+            result.append(kPDFRasterizers[i].fName);
+        }
+    }
+    result.append("\"");
+    return result;
+}
+
 // Macro magic to convert a numeric preprocessor token into a string.
 // Adapted from http://stackoverflow.com/questions/240353/convert-a-preprocessor-token-to-a-string
 // This should probably be moved into one of our common headers...
@@ -1322,6 +1361,7 @@ static SkString configUsage() {
 
 // Alphabetized ignoring "no" prefix ("readPath", "noreplay", "resourcePath").
 DEFINE_string(config, "", configUsage().c_str());
+DEFINE_string(pdfRasterizers, "", pdfRasterizerUsage().c_str());
 DEFINE_bool(deferred, true, "Exercise the deferred rendering test pass.");
 DEFINE_string(excludeConfig, "", "Space delimited list of configs to skip.");
 DEFINE_bool(forceBWtext, false, "Disable text anti-aliasing.");
@@ -1410,6 +1450,15 @@ static int findConfig(const char config[]) {
     return -1;
 }
 
+static const PDFRasterizerData* findPDFRasterizer(const char rasterizer[]) {
+    for (size_t i = 0; i < SK_ARRAY_COUNT(kPDFRasterizers); i++) {
+        if (!strcmp(rasterizer, kPDFRasterizers[i].fName)) {
+            return &kPDFRasterizers[i];
+        }
+    }
+    return NULL;
+}
+
 namespace skiagm {
 #if SK_SUPPORT_GPU
 SkAutoTUnref<GrContext> gGrContext;
@@ -1466,9 +1515,13 @@ template <typename T> void appendUnique(SkTDArray<T>* array, const T& value) {
  *
  * Returns all errors encountered while doing so.
  */
-ErrorCombination run_multiple_configs(GMMain &gmmain, GM *gm, const SkTDArray<size_t> &configs,
+ErrorCombination run_multiple_configs(GMMain &gmmain, GM *gm,
+                                      const SkTDArray<size_t> &configs,
+                                      const SkTDArray<const PDFRasterizerData*> &pdfRasterizers,
                                       GrContextFactory *grFactory);
-ErrorCombination run_multiple_configs(GMMain &gmmain, GM *gm, const SkTDArray<size_t> &configs,
+ErrorCombination run_multiple_configs(GMMain &gmmain, GM *gm,
+                                      const SkTDArray<size_t> &configs,
+                                      const SkTDArray<const PDFRasterizerData*> &pdfRasterizers,
                                       GrContextFactory *grFactory) {
     const char renderModeDescriptor[] = "";
     ErrorCombination errorsForAllConfigs;
@@ -1559,9 +1612,12 @@ ErrorCombination run_multiple_configs(GMMain &gmmain, GM *gm, const SkTDArray<si
         } else {
             writePath = NULL;
         }
+
         if (errorsForThisConfig.isEmpty()) {
-            errorsForThisConfig.add(gmmain.test_drawing(gm,config, writePath, gpuTarget,
+            errorsForThisConfig.add(gmmain.test_drawing(gm, config, pdfRasterizers,
+                                                        writePath, gpuTarget,
                                                         &comparisonBitmap));
+            gmmain.RecordTestResults(errorsForThisConfig, shortNamePlusConfig, "");
         }
 
         if (FLAGS_deferred && errorsForThisConfig.isEmpty() &&
@@ -1767,41 +1823,12 @@ bool prepare_subdirectories(const char *root, bool useFileHierarchy,
     return true;
 }
 
-int tool_main(int argc, char** argv);
-int tool_main(int argc, char** argv) {
-
-#if SK_ENABLE_INST_COUNT
-    gPrintInstCount = true;
-#endif
-
-    SkGraphics::Init();
-    // we don't need to see this during a run
-    gSkSuppressFontCachePurgeSpew = true;
-
-    setSystemPreferences();
-    GMMain gmmain;
-
-    SkTDArray<size_t> configs;
+static bool parse_flags_configs(SkTDArray<size_t>* outConfigs,
+                         GrContextFactory* grFactory) {
     SkTDArray<size_t> excludeConfigs;
-    bool userConfig = false;
-
-    SkString usage;
-    usage.printf("Run the golden master tests.\n");
-    SkCommandLineFlags::SetUsage(usage.c_str());
-    SkCommandLineFlags::Parse(argc, argv);
-
-    gmmain.fUseFileHierarchy = FLAGS_hierarchy;
-    gmmain.fWriteChecksumBasedFilenames = FLAGS_writeChecksumBasedFilenames;
-    if (FLAGS_mismatchPath.count() == 1) {
-        gmmain.fMismatchPath = FLAGS_mismatchPath[0];
-    }
-    if (FLAGS_missingExpectationsPath.count() == 1) {
-        gmmain.fMissingExpectationsPath = FLAGS_missingExpectationsPath[0];
-    }
 
     for (int i = 0; i < FLAGS_config.count(); i++) {
         const char* config = FLAGS_config[i];
-        userConfig = true;
         bool exclude = false;
         if (*config == kExcludeConfigChar) {
             exclude = true;
@@ -1812,23 +1839,22 @@ int tool_main(int argc, char** argv) {
             if (exclude) {
                 *excludeConfigs.append() = index;
             } else {
-                appendUnique<size_t>(&configs, index);
+                appendUnique<size_t>(outConfigs, index);
             }
         } else if (0 == strcmp(kDefaultsConfigStr, config)) {
+            if (exclude) {
+                gm_fprintf(stderr, "%c%s is not allowed.\n",
+                           kExcludeConfigChar, kDefaultsConfigStr);
+                return false;
+            }
             for (size_t c = 0; c < SK_ARRAY_COUNT(gRec); ++c) {
                 if (gRec[c].fRunByDefault) {
-                    if (exclude) {
-                        gm_fprintf(stderr, "%c%s is not allowed.\n",
-                                   kExcludeConfigChar, kDefaultsConfigStr);
-                        return -1;
-                    } else {
-                        appendUnique<size_t>(&configs, c);
-                    }
+                    appendUnique<size_t>(outConfigs, c);
                 }
             }
         } else {
             gm_fprintf(stderr, "unrecognized config %s\n", config);
-            return -1;
+            return false;
         }
     }
 
@@ -1838,147 +1864,271 @@ int tool_main(int argc, char** argv) {
             *excludeConfigs.append() = index;
         } else {
             gm_fprintf(stderr, "unrecognized excludeConfig %s\n", FLAGS_excludeConfig[i]);
-            return -1;
+            return false;
         }
     }
 
-    int moduloRemainder = -1;
-    int moduloDivisor = -1;
-
-    if (FLAGS_modulo.count() == 2) {
-        moduloRemainder = atoi(FLAGS_modulo[0]);
-        moduloDivisor = atoi(FLAGS_modulo[1]);
-        if (moduloRemainder < 0 || moduloDivisor <= 0 || moduloRemainder >= moduloDivisor) {
-            gm_fprintf(stderr, "invalid modulo values.");
-            return -1;
-        }
-    }
-
-    if (FLAGS_ignoreErrorTypes.count() > 0) {
-        gmmain.fIgnorableErrorTypes = ErrorCombination();
-        for (int i = 0; i < FLAGS_ignoreErrorTypes.count(); i++) {
-            ErrorType type;
-            const char *name = FLAGS_ignoreErrorTypes[i];
-            if (!getErrorTypeByName(name, &type)) {
-                gm_fprintf(stderr, "cannot find ErrorType with name '%s'\n", name);
-                return -1;
-            } else {
-                gmmain.fIgnorableErrorTypes.add(type);
-            }
-        }
-    }
-
-#if SK_SUPPORT_GPU
-    if (FLAGS_gpuCacheSize.count() > 0) {
-        if (FLAGS_gpuCacheSize.count() != 2) {
-            gm_fprintf(stderr, "--gpuCacheSize requires two arguments\n");
-            return -1;
-        }
-        gGpuCacheSizeBytes = atoi(FLAGS_gpuCacheSize[0]);
-        gGpuCacheSizeCount = atoi(FLAGS_gpuCacheSize[1]);
-    } else {
-        gGpuCacheSizeBytes = DEFAULT_CACHE_VALUE;
-        gGpuCacheSizeCount = DEFAULT_CACHE_VALUE;
-    }
-#endif
-
-    SkTDArray<SkScalar> tileGridReplayScales;
-    *tileGridReplayScales.append() = SK_Scalar1; // By default only test at scale 1.0
-    if (FLAGS_tileGridReplayScales.count() > 0) {
-        tileGridReplayScales.reset();
-        for (int i = 0; i < FLAGS_tileGridReplayScales.count(); i++) {
-            double val = atof(FLAGS_tileGridReplayScales[i]);
-            if (0 < val) {
-                *tileGridReplayScales.append() = SkDoubleToScalar(val);
-            }
-        }
-        if (0 == tileGridReplayScales.count()) {
-            // Should have at least one scale
-            gm_fprintf(stderr, "--tileGridReplayScales requires at least one scale.\n");
-            return -1;
-        }
-    }
-
-    if (!userConfig) {
+    if (outConfigs->count() == 0) {
         // if no config is specified by user, add the defaults
         for (size_t i = 0; i < SK_ARRAY_COUNT(gRec); ++i) {
             if (gRec[i].fRunByDefault) {
-                *configs.append() = i;
+                *outConfigs->append() = i;
             }
         }
     }
     // now remove any explicitly excluded configs
     for (int i = 0; i < excludeConfigs.count(); ++i) {
-        int index = configs.find(excludeConfigs[i]);
+        int index = outConfigs->find(excludeConfigs[i]);
         if (index >= 0) {
-            configs.remove(index);
+            outConfigs->remove(index);
             // now assert that there was only one copy in configs[]
-            SkASSERT(configs.find(excludeConfigs[i]) < 0);
+            SkASSERT(outConfigs->find(excludeConfigs[i]) < 0);
         }
     }
 
 #if SK_SUPPORT_GPU
-    GrContextFactory* grFactory = new GrContextFactory;
-    for (int i = 0; i < configs.count(); ++i) {
-        size_t index = configs[i];
+    SkASSERT(grFactory != NULL);
+    for (int i = 0; i < outConfigs->count(); ++i) {
+        size_t index = (*outConfigs)[i];
         if (kGPU_Backend == gRec[index].fBackend) {
             GrContext* ctx = grFactory->get(gRec[index].fGLContextType);
             if (NULL == ctx) {
                 gm_fprintf(stderr, "GrContext could not be created for config %s."
                            " Config will be skipped.\n", gRec[index].fName);
-                configs.remove(i);
+                outConfigs->remove(i);
                 --i;
                 continue;
             }
             if (gRec[index].fSampleCnt > ctx->getMaxSampleCount()) {
                 gm_fprintf(stderr, "Sample count (%d) of config %s is not supported."
-                           " Config will be skipped.\n", gRec[index].fSampleCnt, gRec[index].fName);
-                configs.remove(i);
+                           " Config will be skipped.\n",
+                           gRec[index].fSampleCnt, gRec[index].fName);
+                outConfigs->remove(i);
                 --i;
             }
         }
     }
-#else
-    GrContextFactory* grFactory = NULL;
 #endif
 
-    if (configs.isEmpty()) {
+    if (outConfigs->isEmpty()) {
         gm_fprintf(stderr, "No configs to run.");
-        return -1;
+        return false;
     }
 
     // now show the user the set of configs that will be run.
     SkString configStr("These configs will be run: ");
     // show the user the config that will run.
-    for (int i = 0; i < configs.count(); ++i) {
-        configStr.appendf("%s%s", gRec[configs[i]].fName, (i == configs.count() - 1) ? "\n" : " ");
+    for (int i = 0; i < outConfigs->count(); ++i) {
+        configStr.appendf("%s ", gRec[(*outConfigs)[i]].fName);
     }
-    gm_fprintf(stdout, "%s", configStr.c_str());
+    gm_fprintf(stdout, "%s\n", configStr.c_str());
 
-    if (FLAGS_resourcePath.count() == 1) {
-        GM::SetResourcePath(FLAGS_resourcePath[0]);
+    return true;
+}
+
+static bool parse_flags_pdf_rasterizers(SkTDArray<const PDFRasterizerData*>* outRasterizers) {
+    for (int i = 0; i < FLAGS_pdfRasterizers.count(); i++) {
+        const char* rasterizer = FLAGS_pdfRasterizers[i];
+        const PDFRasterizerData* rasterizerPtr = findPDFRasterizer(rasterizer);
+
+        if (rasterizer != NULL) {
+            appendUnique<const PDFRasterizerData*>(outRasterizers,
+                                                   rasterizerPtr);
+        } else {
+            gm_fprintf(stderr, "unrecognized rasterizer %s\n", rasterizer);
+            return false;
+        }
+    }
+
+    if (outRasterizers->count() == 0) {
+        // if no config is specified by user, add the defaults
+        for (size_t i = 0; i < SK_ARRAY_COUNT(kPDFRasterizers); ++i) {
+            if (kPDFRasterizers[i].fRunByDefault) {
+                *outRasterizers->append() = &kPDFRasterizers[i];
+            }
+        }
+    }
+
+    // now show the user the set of configs that will be run.
+    SkString configStr("These PDF rasterizers will be run: ");
+    // show the user the config that will run.
+    for (int i = 0; i < outRasterizers->count(); ++i) {
+        configStr.appendf("%s ", (*outRasterizers)[i]->fName);
+    }
+    gm_fprintf(stdout, "%s\n", configStr.c_str());
+
+    return true;
+}
+
+static bool parse_flags_ignore_error_types(ErrorCombination* outErrorTypes) {
+    if (FLAGS_ignoreErrorTypes.count() > 0) {
+        *outErrorTypes = ErrorCombination();
+        for (int i = 0; i < FLAGS_ignoreErrorTypes.count(); i++) {
+            ErrorType type;
+            const char *name = FLAGS_ignoreErrorTypes[i];
+            if (!getErrorTypeByName(name, &type)) {
+                gm_fprintf(stderr, "cannot find ErrorType with name '%s'\n", name);
+                return false;
+            } else {
+                outErrorTypes->add(type);
+            }
+        }
+    }
+    return true;
+}
+
+static bool parse_flags_modulo(int* moduloRemainder, int* moduloDivisor) {
+    if (FLAGS_modulo.count() == 2) {
+        *moduloRemainder = atoi(FLAGS_modulo[0]);
+        *moduloDivisor = atoi(FLAGS_modulo[1]);
+        if (*moduloRemainder < 0 || *moduloDivisor <= 0 ||
+                *moduloRemainder >= *moduloDivisor) {
+            gm_fprintf(stderr, "invalid modulo values.");
+            return false;
+        }
+    }
+    return true;
+}
+
+#if SK_SUPPORT_GPU
+static bool parse_flags_gpu_cache(int* sizeBytes, int* sizeCount) {
+    if (FLAGS_gpuCacheSize.count() > 0) {
+        if (FLAGS_gpuCacheSize.count() != 2) {
+            gm_fprintf(stderr, "--gpuCacheSize requires two arguments\n");
+            return false;
+        }
+        *sizeBytes = atoi(FLAGS_gpuCacheSize[0]);
+        *sizeCount = atoi(FLAGS_gpuCacheSize[1]);
+    } else {
+        *sizeBytes = DEFAULT_CACHE_VALUE;
+        *sizeCount = DEFAULT_CACHE_VALUE;
+    }
+    return true;
+}
+#endif
+
+static bool parse_flags_tile_grid_replay_scales(SkTDArray<SkScalar>* outScales) {
+    *outScales->append() = SK_Scalar1; // By default only test at scale 1.0
+    if (FLAGS_tileGridReplayScales.count() > 0) {
+        outScales->reset();
+        for (int i = 0; i < FLAGS_tileGridReplayScales.count(); i++) {
+            double val = atof(FLAGS_tileGridReplayScales[i]);
+            if (0 < val) {
+                *outScales->append() = SkDoubleToScalar(val);
+            }
+        }
+        if (0 == outScales->count()) {
+            // Should have at least one scale
+            gm_fprintf(stderr, "--tileGridReplayScales requires at least one scale.\n");
+            return false;
+        }
+    }
+    return true;
+}
+
+static bool parse_flags_gmmain_paths(GMMain* gmmain) {
+    gmmain->fUseFileHierarchy = FLAGS_hierarchy;
+    gmmain->fWriteChecksumBasedFilenames = FLAGS_writeChecksumBasedFilenames;
+
+    if (FLAGS_mismatchPath.count() == 1) {
+        gmmain->fMismatchPath = FLAGS_mismatchPath[0];
+    }
+
+    if (FLAGS_missingExpectationsPath.count() == 1) {
+        gmmain->fMissingExpectationsPath = FLAGS_missingExpectationsPath[0];
     }
 
     if (FLAGS_readPath.count() == 1) {
         const char* readPath = FLAGS_readPath[0];
         if (!sk_exists(readPath)) {
             gm_fprintf(stderr, "readPath %s does not exist!\n", readPath);
-            return -1;
+            return false;
         }
         if (sk_isdir(readPath)) {
             if (FLAGS_verbose) {
                 gm_fprintf(stdout, "reading from %s\n", readPath);
             }
-            gmmain.fExpectationsSource.reset(SkNEW_ARGS(
+            gmmain->fExpectationsSource.reset(SkNEW_ARGS(
                 IndividualImageExpectationsSource, (readPath)));
         } else {
             if (FLAGS_verbose) {
                 gm_fprintf(stdout, "reading expectations from JSON summary file %s\n", readPath);
             }
-            gmmain.fExpectationsSource.reset(SkNEW_ARGS(
-                JsonExpectationsSource, (readPath)));
+            gmmain->fExpectationsSource.reset(SkNEW_ARGS(JsonExpectationsSource, (readPath)));
         }
     }
+    return true;
+}
+
+static bool parse_flags_match_strs(SkTDArray<const char*>* matchStrs) {
+    for (int i = 0; i < FLAGS_match.count(); ++i) {
+        matchStrs->push(FLAGS_match[i]);
+    }
+    return true;
+}
+
+static bool parse_flags_resource_path() {
+    if (FLAGS_resourcePath.count() == 1) {
+        GM::SetResourcePath(FLAGS_resourcePath[0]);
+    }
+    return true;
+}
+
+static bool parse_flags_jpeg_quality() {
+    if (FLAGS_pdfJpegQuality < -1 || FLAGS_pdfJpegQuality > 100) {
+        gm_fprintf(stderr, "%s\n", "pdfJpegQuality must be in [-1 .. 100] range.");
+        return false;
+    }
+    return true;
+}
+
+int tool_main(int argc, char** argv);
+int tool_main(int argc, char** argv) {
+
+#if SK_ENABLE_INST_COUNT
+    gPrintInstCount = true;
+#endif
+
+    SkGraphics::Init();
+    // we don't need to see this during a run
+    gSkSuppressFontCachePurgeSpew = true;
+
+    setSystemPreferences();
+    GMMain gmmain;
+
+    SkString usage;
+    usage.printf("Run the golden master tests.\n");
+    SkCommandLineFlags::SetUsage(usage.c_str());
+    SkCommandLineFlags::Parse(argc, argv);
+
+    SkTDArray<size_t> configs;
+
+    int moduloRemainder = -1;
+    int moduloDivisor = -1;
+    SkTDArray<const PDFRasterizerData*> pdfRasterizers;
+    SkTDArray<SkScalar> tileGridReplayScales;
+#if SK_SUPPORT_GPU
+    GrContextFactory* grFactory = new GrContextFactory;
+#else
+    GrContextFactory* grFactory = NULL;
+#endif
+    SkTDArray<const char*> matchStrs;
+
+    if (!parse_flags_modulo(&moduloRemainder, &moduloDivisor) ||

[vandebo (ex-Chrome), 2013/08/09 16:26:34]

It looks like your flags refactor has leaked into this CL...

[ducky, 2013/08/09 23:29:41]

Looks like I set the wrong cl upload base...

+        !parse_flags_ignore_error_types(&gmmain.fIgnorableErrorTypes) ||
+#if SK_SUPPORT_GPU
+        !parse_flags_gpu_cache(&gGpuCacheSizeBytes, &gGpuCacheSizeCount) ||
+#endif
+        !parse_flags_tile_grid_replay_scales(&tileGridReplayScales) ||
+        !parse_flags_gmmain_paths(&gmmain) ||
+        !parse_flags_resource_path() ||
+        !parse_flags_match_strs(&matchStrs) ||
+        !parse_flags_jpeg_quality() ||
+        !parse_flags_configs(&configs, grFactory) ||
+        !parse_flags_pdf_rasterizers(&pdfRasterizers)) {
+        return -1;
+    }
+
     if (FLAGS_verbose) {
         if (FLAGS_writePath.count() == 1) {
             gm_fprintf(stdout, "writing to %s\n", FLAGS_writePath[0]);
@@ -1998,13 +2148,6 @@ int tool_main(int argc, char** argv) {
         }
     }
 
-    if (moduloDivisor <= 0) {
-        moduloRemainder = -1;
-    }
-    if (moduloRemainder < 0 || moduloRemainder >= moduloDivisor) {
-        moduloRemainder = -1;
-    }
-
     int gmsRun = 0;
     int gmIndex = -1;
     SkString moduloStr;
@@ -2027,10 +2170,6 @@ int tool_main(int argc, char** argv) {
         }
     }
 
-    if (FLAGS_pdfJpegQuality < -1 || FLAGS_pdfJpegQuality > 100) {
-        gm_fprintf(stderr, "%s\n", "pdfJpegQuality must be in [-1 .. 100] range.");
-    }
-
     Iter iter;
     GM* gm;
     while ((gm = iter.next()) != NULL) {
@@ -2045,10 +2184,6 @@ int tool_main(int argc, char** argv) {
 
         const char* shortName = gm->shortName();
 
-        SkTDArray<const char*> matchStrs;
-        for (int i = 0; i < FLAGS_match.count(); ++i) {
-            matchStrs.push(FLAGS_match[i]);
-        }
         if (SkCommandLineFlags::ShouldSkip(matchStrs, shortName)) {
             continue;
         }
@@ -2058,7 +2193,7 @@ int tool_main(int argc, char** argv) {
         gm_fprintf(stdout, "%sdrawing... %s [%d %d]\n", moduloStr.c_str(), shortName,
                    size.width(), size.height());
 
-        run_multiple_configs(gmmain, gm, configs, grFactory);
+        run_multiple_configs(gmmain, gm, configs, pdfRasterizers, grFactory);
 
         SkBitmap comparisonBitmap;
         const ConfigData compareConfig =