GM: run mode comparisons for all raster configs.

gm/gmmain.cpp

Index: gm/gmmain.cpp
diff --git a/gm/gmmain.cpp b/gm/gmmain.cpp
index 0e43c554c69829cd186c9ca723fd24e6d4e60cd9..07b5c006e4a8b7eb17a622ec1a8185c443491c66 100644
--- a/gm/gmmain.cpp
+++ b/gm/gmmain.cpp
@@ -1529,127 +1529,6 @@ template <typename T> void appendUnique(SkTDArray<T>* array, const T& value) {
 }
 
 /**
- * Run this test in a number of different configs (8888, 565, PDF,
- * etc.), confirming that the resulting bitmaps match expectations
- * (which may be different for each config).
- *
- * Returns all errors encountered while doing so.
- */
-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,
-                                      const SkTDArray<const PDFRasterizerData*> &pdfRasterizers,
-                                      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 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 (gmFlags & GM::kSkipPDF_Flag) {
-                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, shortNamePlusConfig,
-                                     renderModeDescriptor);
-            errorsForAllConfigs.add(kIntentionallySkipped_ErrorType);
-            continue;
-        }
-        if (((gmFlags & GM::kSkipGPU_Flag) && kGPU_Backend == config.fBackend) ||
-            ((gmFlags & GM::kGPUOnly_Flag) && kGPU_Backend != config.fBackend)) {
-            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
-        SkAutoTUnref<GrSurface> auGpuTarget;
-        if ((errorsForThisConfig.isEmpty()) && (kGPU_Backend == config.fBackend)) {
-            if (FLAGS_resetGpuContext) {
-                grFactory->destroyContexts();
-            }
-            GrContext* gr = grFactory->get(config.fGLContextType);
-            bool grSuccess = false;
-            if (gr) {
-                // create a render target to back the device
-                GrTextureDesc desc;
-                desc.fConfig = kSkia8888_GrPixelConfig;
-                desc.fFlags = kRenderTarget_GrTextureFlagBit;
-                desc.fWidth = gm->getISize().width();
-                desc.fHeight = gm->getISize().height();
-                desc.fSampleCnt = config.fSampleCnt;
-                auGpuTarget.reset(gr->createUncachedTexture(desc, NULL, 0));
-                if (NULL != auGpuTarget) {
-                    gpuTarget = auGpuTarget;
-                    grSuccess = true;
-                    // Set the user specified cache limits if non-default.
-                    size_t bytes;
-                    int count;
-                    gr->getTextureCacheLimits(&count, &bytes);
-                    if (DEFAULT_CACHE_VALUE != gGpuCacheSizeBytes) {
-                        bytes = static_cast<size_t>(gGpuCacheSizeBytes);
-                    }
-                    if (DEFAULT_CACHE_VALUE != gGpuCacheSizeCount) {
-                        count = gGpuCacheSizeCount;
-                    }
-                    gr->setTextureCacheLimits(count, bytes);
-                }
-            }
-            if (!grSuccess) {
-                errorsForThisConfig.add(kNoGpuContext_ErrorType);
-            }
-        }
-#endif
-
-        SkBitmap comparisonBitmap;
-
-        const char* writePath;
-        if (FLAGS_writePath.count() == 1) {
-            writePath = FLAGS_writePath[0];
-        } else {
-            writePath = NULL;
-        }
-
-        if (errorsForThisConfig.isEmpty()) {
-            errorsForThisConfig.add(gmmain.test_drawing(gm, config, pdfRasterizers,
-                                                        writePath, gpuTarget,
-                                                        &comparisonBitmap));
-            gmmain.RecordTestResults(errorsForThisConfig, shortNamePlusConfig, "");
-        }
-
-        if (FLAGS_deferred && errorsForThisConfig.isEmpty() &&
-            (kGPU_Backend == config.fBackend || kRaster_Backend == config.fBackend)) {
-            errorsForThisConfig.add(gmmain.test_deferred_drawing(gm, config, comparisonBitmap,
-                                                                 gpuTarget));
-        }
-
-        errorsForAllConfigs.add(errorsForThisConfig);
-    }
-    return errorsForAllConfigs;
-}
-
-/**
  * Run this test in a number of different drawing modes (pipe,
  * deferred, tiled, etc.), confirming that the resulting bitmaps all
  * *exactly* match comparisonBitmap.
@@ -1782,6 +1661,136 @@ ErrorCombination run_multiple_modes(GMMain &gmmain, GM *gm, const ConfigData &co
     return errorsForAllModes;
 }
 
+
+/**
+ * Run this test in a number of different configs (8888, 565, PDF,
+ * etc.), confirming that the resulting bitmaps match expectations
+ * (which may be different for each config).
+ *
+ * Returns all errors encountered while doing so.
+ */
+ErrorCombination run_multiple_configs(GMMain &gmmain, GM *gm,
+                                      const SkTDArray<size_t> &configs,
+                                      const SkTDArray<const PDFRasterizerData*> &pdfRasterizers,
+                                      const SkTDArray<SkScalar> &tileGridReplayScales,
+                                      GrContextFactory *grFactory);
+ErrorCombination run_multiple_configs(GMMain &gmmain, GM *gm,
+                                      const SkTDArray<size_t> &configs,
+                                      const SkTDArray<const PDFRasterizerData*> &pdfRasterizers,
+                                      const SkTDArray<SkScalar> &tileGridReplayScales,
+                                      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 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 (gmFlags & GM::kSkipPDF_Flag) {
+                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, shortNamePlusConfig,
+                                     renderModeDescriptor);
+            errorsForAllConfigs.add(kIntentionallySkipped_ErrorType);
+            continue;
+        }
+        if (((gmFlags & GM::kSkipGPU_Flag) && kGPU_Backend == config.fBackend) ||
+            ((gmFlags & GM::kGPUOnly_Flag) && kGPU_Backend != config.fBackend)) {
+            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
+        SkAutoTUnref<GrSurface> auGpuTarget;
+        if ((errorsForThisConfig.isEmpty()) && (kGPU_Backend == config.fBackend)) {
+            if (FLAGS_resetGpuContext) {
+                grFactory->destroyContexts();
+            }
+            GrContext* gr = grFactory->get(config.fGLContextType);
+            bool grSuccess = false;
+            if (gr) {
+                // create a render target to back the device
+                GrTextureDesc desc;
+                desc.fConfig = kSkia8888_GrPixelConfig;
+                desc.fFlags = kRenderTarget_GrTextureFlagBit;
+                desc.fWidth = gm->getISize().width();
+                desc.fHeight = gm->getISize().height();
+                desc.fSampleCnt = config.fSampleCnt;
+                auGpuTarget.reset(gr->createUncachedTexture(desc, NULL, 0));
+                if (NULL != auGpuTarget) {
+                    gpuTarget = auGpuTarget;
+                    grSuccess = true;
+                    // Set the user specified cache limits if non-default.
+                    size_t bytes;
+                    int count;
+                    gr->getTextureCacheLimits(&count, &bytes);
+                    if (DEFAULT_CACHE_VALUE != gGpuCacheSizeBytes) {
+                        bytes = static_cast<size_t>(gGpuCacheSizeBytes);
+                    }
+                    if (DEFAULT_CACHE_VALUE != gGpuCacheSizeCount) {
+                        count = gGpuCacheSizeCount;
+                    }
+                    gr->setTextureCacheLimits(count, bytes);
+                }
+            }
+            if (!grSuccess) {
+                errorsForThisConfig.add(kNoGpuContext_ErrorType);
+            }
+        }
+#endif
+
+        SkBitmap comparisonBitmap;
+
+        const char* writePath;
+        if (FLAGS_writePath.count() == 1) {
+            writePath = FLAGS_writePath[0];
+        } else {
+            writePath = NULL;
+        }
+
+        if (errorsForThisConfig.isEmpty()) {
+            errorsForThisConfig.add(gmmain.test_drawing(gm, config, pdfRasterizers,
+                                                        writePath, gpuTarget,
+                                                        &comparisonBitmap));
+            gmmain.RecordTestResults(errorsForThisConfig, shortNamePlusConfig, "");
+        }
+
+        // TODO: run only if gmmain.test_drawing succeeded.

[mtklein, 2014/01/08 20:05:53]

The diff is weird, but what's happened here is:
  - run_multiple_modes moves up the file verbatim to be visible to
run_multiple_configs
  - Only these next three lines (plus passing tileGridReplayScales) are new in
run_multiple_configs.  Nothing else changed.

[epoger, 2014/01/08 20:48:55]

Thanks for the explanation.

One way of avoiding the diff weirdness (at the expense of some time/effort) is
to upload the move as patchset 1, and the modifications as patchset 2.

+        if (kRaster_Backend == config.fBackend) {
+            run_multiple_modes(gmmain, gm, config, comparisonBitmap, tileGridReplayScales);
+        }
+
+        if (FLAGS_deferred && errorsForThisConfig.isEmpty() &&
+            (kGPU_Backend == config.fBackend || kRaster_Backend == config.fBackend)) {
+            errorsForThisConfig.add(gmmain.test_deferred_drawing(gm, config, comparisonBitmap,
+                                                                 gpuTarget));
+        }
+
+        errorsForAllConfigs.add(errorsForThisConfig);
+    }
+    return errorsForAllConfigs;
+}
+
+
 /**
  * Read individual lines from a file, pushing them into the given array.
  *
@@ -2299,16 +2308,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, pdfRasterizers, grFactory);
-
-        SkBitmap comparisonBitmap;
-        const ConfigData compareConfig =
-            { SkBitmap::kARGB_8888_Config, kRaster_Backend, kDontCare_GLContextType, 0, kRW_ConfigFlag, "comparison", false };
-        gmmain.generate_image(gm, compareConfig, NULL, &comparisonBitmap, false);
-
-        // TODO(epoger): only run this if gmmain.generate_image() succeeded?
-        // Otherwise, what are we comparing against?
-        run_multiple_modes(gmmain, gm, compareConfig, comparisonBitmap, tileGridReplayScales);
+        run_multiple_configs(gmmain, gm, configs, pdfRasterizers, tileGridReplayScales, grFactory);
     }
 
     SkTArray<SkString> modes;