skdiff: add --failonmismatches and --listfilename options, plus cleanup

These changes are needed in order to switch the buildbots from using "gm -r" to "skdiff" to compare gm results, and should be generally good for humans too.
Review URL: https://codereview.appspot.com/6392054

git-svn-id: http://skia.googlecode.com/svn/trunk@4579 2bbb7eff-a529-9590-31e7-b0007b416f81

11 files changed, 148 insertions, 98 deletions

diff --git a/tools/skdiff_main.cpp b/tools/skdiff_main.cpp
index 8a6162bc5b..c53dcf52dd 100644
--- a/tools/skdiff_main.cpp
+++ b/tools/skdiff_main.cpp
@@ -42,18 +42,42 @@
 // Result of comparison for each pair of files.
 // Listed from "better" to "worse", for sorting of results.
 enum Result {
-    kEqualBits,      // both files in the pair contain exactly the same bits
-    kEqualPixels,    // not bitwise equal, but their pixels are exactly the same
-    kDifferentPixels,// both are images we can parse, but with different pixels
-    kDifferentSizes, // both are images we can parse, but of different sizes
-    kDifferentOther, // files have different bits but are not parsable images
-    kComparisonMissing,// missing from comparisonDir
-    kBaseMissing,      // missing from baseDir
-    kUnknown,        // not compared yet
+    kEqualBits,
+    kEqualPixels,
+    kDifferentPixels,
+    kDifferentSizes,
+    kDifferentOther,
+    kComparisonMissing,
+    kBaseMissing,
+    kUnknown,
     //
     kNumResultTypes  // NOT A VALID VALUE--used to set up arrays. Must be last.
 };
 
+// Returns a text description of the given Result type.
+const char *getResultDescription(Result result) {
+    switch (result) {
+      case kEqualBits:
+        return "contain exactly the same bits";
+      case kEqualPixels:
+        return "contain the same pixel values, but not the same bits";
+      case kDifferentPixels:
+        return "have identical dimensions but some differing pixels";
+      case kDifferentSizes:
+        return "have differing dimensions";
+      case kDifferentOther:
+        return "contain different bits and are not parsable images";
+      case kBaseMissing:
+        return "missing from baseDir";
+      case kComparisonMissing:
+        return "missing from comparisonDir";
+      case kUnknown:
+        return "not compared yet";
+      default:
+        return NULL;
+    }
+}
+
 struct DiffRecord {
     DiffRecord (const SkString filename,
                 const SkString basePath,
@@ -138,31 +162,32 @@ struct DiffSummary {
 
     FileArray fResultsOfType[kNumResultTypes];
 
-    // Print the contents of this FileArray, if any, to stdout.
-    // (If the FileArray is empty, print nothing.)
-    void printContents(const FileArray& fileArray, const char* headerText) {
+    // Print a line about the contents of this FileArray to stdout; if the FileArray is empty,
+    // print nothing.
+    void printContents(const FileArray& fileArray, const char* headerText, bool listFilenames) {
         int n = fileArray.count();
         if (n > 0) {
-            printf("%s:\n", headerText);
-            for (int i = 0; i < n; ++i) {
-                printf("\t%s\n", fileArray[i]->c_str());
+            printf(" %d file pairs %s", n, headerText);
+            if (listFilenames) {
+                printf(": ");
+                for (int i = 0; i < n; ++i) {
+                    printf("%s ", fileArray[i]->c_str());
+                }
             }
+            printf("\n");
         }
     }
 
-    void print () {
-        printContents(fResultsOfType[kBaseMissing],
-                      "Missing in baseDir");
-        printContents(fResultsOfType[kComparisonMissing],
-                      "Missing in comparisonDir");
-        printf("%d of %d images matched.\n", fNumMatches,
-               fNumMatches + fNumMismatches);
+    void print(bool listFilenames) {
+        printf("compared %d file pairs:\n", fNumMatches + fNumMismatches);
+        for (int resultInt = 0; resultInt < kNumResultTypes; resultInt++) {
+            Result result = static_cast<Result>(resultInt);
+            printContents(fResultsOfType[result], getResultDescription(result), listFilenames);
+        }
         if (fNumMismatches > 0) {
             printf("Maximum pixel intensity mismatch %d\n", fMaxMismatchV);
-            printf("Largest area mismatch was %.2f%% of pixels\n",
-                   fMaxMismatchPercent);
+            printf("Largest area mismatch was %.2f%% of pixels\n",fMaxMismatchPercent);
         }
-
     }
 
     void add (DiffRecord* drp) {
@@ -831,9 +856,9 @@ static void print_table_header (SkFILEWStream* stream,
     stream->writeText("every different pixel shown in white");
     stream->writeText("</th>\n<th>");
     stream->writeText("color difference at each pixel");
-    stream->writeText("</th>\n<th>");
+    stream->writeText("</th>\n<th>baseDir: ");
     stream->writeText(baseDir.c_str());
-    stream->writeText("</th>\n<th>");
+    stream->writeText("</th>\n<th>comparisonDir: ");
     stream->writeText(comparisonDir.c_str());
     stream->writeText("</th>\n");
     stream->writeText("</tr>\n");
@@ -1052,35 +1077,40 @@ static void usage (char * argv0) {
 "Usage: \n"
 "    %s <baseDir> <comparisonDir> [outputDir] \n"
 , argv0, argv0);
-    SkDebugf("\n"
-"Arguments: \n"
-"    --nodiffs: don't write out image diffs or index.html, just generate \n"
-"               report on stdout \n"
-"    --threshold <n>: only report differences > n (per color channel) [default 0]\n"
-"    --match: compare files whose filenames contain this substring; if \n"
-"             unspecified, compare ALL files. \n"
-"             this flag may be repeated to add more matching substrings. \n"
-"    --nomatch: regardless of --match, DO NOT compare files whose filenames \n"
-"               contain this substring. \n"
-"               this flag may be repeated to add more forbidden substrings. \n"
-"    --sortbymismatch: sort by average color channel mismatch\n");
-    SkDebugf(
-"    --sortbymaxmismatch: sort by worst color channel mismatch;\n"
-"                         break ties with -sortbymismatch\n"
-"    [default sort is by fraction of pixels mismatching]\n");
-    SkDebugf(
-"    --weighted: sort by # pixels different weighted by color difference\n");
-    SkDebugf(
-"    --noprintdirs: do not print the directories used.");
     SkDebugf(
-"    baseDir: directory to read baseline images from.\n");
-    SkDebugf(
-"    comparisonDir: directory to read comparison images from\n");
-    SkDebugf(
-"    outputDir: directory to write difference images and index.html to; \n"
-"               defaults to comparisonDir \n");
+"\nArguments:"
+"\n    --help: display this info"
+"\n    --failonmismatches: exit with nonzero return code (number of mismatching"
+"\n                        image pairs) if any pairs differ by more than threshold;"
+"\n                        otherwise, only return nonzero if the tool itself fails"
+"\n    --listfilenames: list all filenames for each result type in stdout"
+"\n    --match: compare files whose filenames contain this substring; if"
+"\n             unspecified, compare ALL files."
+"\n             this flag may be repeated to add more matching substrings."
+"\n    --nodiffs: don't write out image diffs or index.html, just generate"
+"\n               report on stdout"
+"\n    --nomatch: regardless of --match, DO NOT compare files whose filenames"
+"\n               contain this substring."
+"\n               this flag may be repeated to add more forbidden substrings."
+"\n    --noprintdirs: do not print the directories used."
+"\n    --sortbymaxmismatch: sort by worst color channel mismatch;"
+"\n                         break ties with -sortbymismatch"
+"\n    --sortbymismatch: sort by average color channel mismatch"
+"\n    --threshold <n>: only report differences > n (per color channel) [default 0]"
+"\n    --weighted: sort by # pixels different weighted by color difference"
+"\n"
+"\n    baseDir: directory to read baseline images from."
+"\n    comparisonDir: directory to read comparison images from"
+"\n    outputDir: directory to write difference images and index.html to;"
+"\n               defaults to comparisonDir"
+"\n"
+"\nIf no sort is specified, it will sort by fraction of pixels mismatching."
+"\n");
 }
 
+const int NO_ERROR = 0;
+const int GENERIC_ERROR = -1;
+
 int main (int argc, char ** argv) {
     DiffMetricProc diffProc = compute_diff_pmcolor;
     int (*sortProc)(const void*, const void*) = compare<CompareDiffMetrics>;
@@ -1094,7 +1124,9 @@ int main (int argc, char ** argv) {
     StringArray matchSubstrings;
     StringArray nomatchSubstrings;
 
+    bool failOnMismatches = false;
     bool generateDiffs = true;
+    bool listFilenames = false;
     bool printDirs = true;
 
     RecordArray differences;
@@ -1103,40 +1135,48 @@ int main (int argc, char ** argv) {
     int i;
     int numUnflaggedArguments = 0;
     for (i = 1; i < argc; i++) {
+        if (!strcmp(argv[i], "--failonmismatches")) {
+            failOnMismatches = true;
+            continue;
+        }
         if (!strcmp(argv[i], "--help")) {
             usage(argv[0]);
-            return 0;
-        }
-        if (!strcmp(argv[i], "--nodiffs")) {
-            generateDiffs = false;
-            continue;
+            return NO_ERROR;
         }
-        if (!strcmp(argv[i], "--threshold")) {
-            colorThreshold = atoi(argv[++i]);
+        if (!strcmp(argv[i], "--listfilenames")) {
+            listFilenames = true;
             continue;
         }
         if (!strcmp(argv[i], "--match")) {
             matchSubstrings.push(new SkString(argv[++i]));
             continue;
         }
+        if (!strcmp(argv[i], "--nodiffs")) {
+            generateDiffs = false;
+            continue;
+        }
         if (!strcmp(argv[i], "--nomatch")) {
             nomatchSubstrings.push(new SkString(argv[++i]));
             continue;
         }
-        if (!strcmp(argv[i], "--sortbymismatch")) {
-            sortProc = compare<CompareDiffMeanMismatches>;
+        if (!strcmp(argv[i], "--noprintdirs")) {
+            printDirs = false;
             continue;
         }
         if (!strcmp(argv[i], "--sortbymaxmismatch")) {
             sortProc = compare<CompareDiffMaxMismatches>;
             continue;
         }
-        if (!strcmp(argv[i], "--weighted")) {
-            sortProc = compare<CompareDiffWeighted>;
+        if (!strcmp(argv[i], "--sortbymismatch")) {
+            sortProc = compare<CompareDiffMeanMismatches>;
             continue;
         }
-        if (!strcmp(argv[i], "--noprintdirs")) {
-            printDirs = false;
+        if (!strcmp(argv[i], "--threshold")) {
+            colorThreshold = atoi(argv[++i]);
+            continue;
+        }
+        if (!strcmp(argv[i], "--weighted")) {
+            sortProc = compare<CompareDiffWeighted>;
             continue;
         }
         if (argv[i][0] != '-') {
@@ -1153,20 +1193,20 @@ int main (int argc, char ** argv) {
                 default:
                     SkDebugf("extra unflagged argument <%s>\n", argv[i]);
                     usage(argv[0]);
-                    return 0;
+                    return GENERIC_ERROR;
             }
         }
 
         SkDebugf("Unrecognized argument <%s>\n", argv[i]);
         usage(argv[0]);
-        return 0;
+        return GENERIC_ERROR;
     }
 
     if (numUnflaggedArguments == 2) {
         outputDir = comparisonDir;
     } else if (numUnflaggedArguments != 3) {
         usage(argv[0]);
-        return 0;
+        return GENERIC_ERROR;
     }
 
     if (!baseDir.endsWith(PATH_DIV_STR)) {
@@ -1206,7 +1246,7 @@ int main (int argc, char ** argv) {
     create_diff_images(diffProc, colorThreshold, &differences,
                        baseDir, comparisonDir, outputDir,
                        matchSubstrings, nomatchSubstrings, &summary);
-    summary.print();
+    summary.print(listFilenames);
 
     if (differences.count()) {
         qsort(differences.begin(), differences.count(),
@@ -1224,5 +1264,9 @@ int main (int argc, char ** argv) {
     matchSubstrings.deleteAll();
     nomatchSubstrings.deleteAll();
 
-    return summary.fNumMismatches;
+    if (failOnMismatches) {
+        return summary.fNumMismatches;
+    } else {
+        return NO_ERROR;
+    }
 }
diff --git a/tools/tests/run.sh b/tools/tests/run.sh
index f6dbe27ca8..25baf1987a 100755
--- a/tools/tests/run.sh
+++ b/tools/tests/run.sh
@@ -42,19 +42,20 @@ SKDIFF_TESTDIR=tools/tests/skdiff
 
 # Run skdiff over a variety of file pair types: identical bits, identical
 # pixels, missing from baseDir, etc.
-# TODO: In the near future, skdiff will return a nonzero exit code in this case.
 skdiff_test "$SKDIFF_TESTDIR/baseDir $SKDIFF_TESTDIR/comparisonDir" "$SKDIFF_TESTDIR/test1"
 
-# Same as above but without generating HTML output files.
-# TODO: In the near future, skdiff will return a nonzero exit code in this case.
-skdiff_test "--nodiffs $SKDIFF_TESTDIR/baseDir $SKDIFF_TESTDIR/comparisonDir" "$SKDIFF_TESTDIR/test2"
+# Same as above, except:
+# - return the number of mismatching file pairs
+# - list filenames with each result type to stdout
+# - don't generate HTML output files
+skdiff_test "--failonmismatches --listfilenames --nodiffs $SKDIFF_TESTDIR/baseDir $SKDIFF_TESTDIR/comparisonDir" "$SKDIFF_TESTDIR/test2"
 
 # Run skdiff over just the files that have identical bits, to validate any
 # behavior/return value differences in this case.
-skdiff_test "--nodiffs --match identical-bits $SKDIFF_TESTDIR/baseDir $SKDIFF_TESTDIR/comparisonDir" "$SKDIFF_TESTDIR/identical-bits"
+skdiff_test "--failonmismatches --nodiffs --match identical-bits $SKDIFF_TESTDIR/baseDir $SKDIFF_TESTDIR/comparisonDir" "$SKDIFF_TESTDIR/identical-bits"
 
 # Run skdiff over just the files that have identical bits or identical pixels,
 # to validate any behavior/return value differences in this case.
-skdiff_test "--nodiffs --match identical-bits --match identical-pixels $SKDIFF_TESTDIR/baseDir $SKDIFF_TESTDIR/comparisonDir" "$SKDIFF_TESTDIR/identical-bits-or-pixels"
+skdiff_test "--failonmismatches --nodiffs --match identical-bits --match identical-pixels $SKDIFF_TESTDIR/baseDir $SKDIFF_TESTDIR/comparisonDir" "$SKDIFF_TESTDIR/identical-bits-or-pixels"
 
 echo "All tests passed."
diff --git a/tools/tests/skdiff/identical-bits-or-pixels/output-expected/command_line b/tools/tests/skdiff/identical-bits-or-pixels/output-expected/command_line
index 8a20667901..eed0759ac5 100644
--- a/tools/tests/skdiff/identical-bits-or-pixels/output-expected/command_line
+++ b/tools/tests/skdiff/identical-bits-or-pixels/output-expected/command_line
@@ -1 +1 @@
-out/Debug/skdiff --nodiffs --match identical-bits --match identical-pixels tools/tests/skdiff/baseDir tools/tests/skdiff/comparisonDir tools/tests/skdiff/identical-bits-or-pixels/output-actual
+out/Debug/skdiff --failonmismatches --nodiffs --match identical-bits --match identical-pixels tools/tests/skdiff/baseDir tools/tests/skdiff/comparisonDir tools/tests/skdiff/identical-bits-or-pixels/output-actual
diff --git a/tools/tests/skdiff/identical-bits-or-pixels/output-expected/stdout b/tools/tests/skdiff/identical-bits-or-pixels/output-expected/stdout
index 937f8c77c4..c5d12ec3da 100644
--- a/tools/tests/skdiff/identical-bits-or-pixels/output-expected/stdout
+++ b/tools/tests/skdiff/identical-bits-or-pixels/output-expected/stdout
@@ -1,4 +1,6 @@
 baseDir is [tools/tests/skdiff/baseDir/]
 comparisonDir is [tools/tests/skdiff/comparisonDir/]
 not writing any diffs to outputDir [tools/tests/skdiff/identical-bits-or-pixels/output-actual/]
-3 of 3 images matched.
+compared 3 file pairs:
+ 2 file pairs contain exactly the same bits
+ 1 file pairs contain the same pixel values, but not the same bits
diff --git a/tools/tests/skdiff/identical-bits/output-expected/command_line b/tools/tests/skdiff/identical-bits/output-expected/command_line
index 49f9ad5e74..5c8c27bbb6 100644
--- a/tools/tests/skdiff/identical-bits/output-expected/command_line
+++ b/tools/tests/skdiff/identical-bits/output-expected/command_line
@@ -1 +1 @@
-out/Debug/skdiff --nodiffs --match identical-bits tools/tests/skdiff/baseDir tools/tests/skdiff/comparisonDir tools/tests/skdiff/identical-bits/output-actual
+out/Debug/skdiff --failonmismatches --nodiffs --match identical-bits tools/tests/skdiff/baseDir tools/tests/skdiff/comparisonDir tools/tests/skdiff/identical-bits/output-actual
diff --git a/tools/tests/skdiff/identical-bits/output-expected/stdout b/tools/tests/skdiff/identical-bits/output-expected/stdout
index ccb24f8be4..c40ae334c9 100644
--- a/tools/tests/skdiff/identical-bits/output-expected/stdout
+++ b/tools/tests/skdiff/identical-bits/output-expected/stdout
@@ -1,4 +1,5 @@
 baseDir is [tools/tests/skdiff/baseDir/]
 comparisonDir is [tools/tests/skdiff/comparisonDir/]
 not writing any diffs to outputDir [tools/tests/skdiff/identical-bits/output-actual/]
-2 of 2 images matched.
+compared 2 file pairs:
+ 2 file pairs contain exactly the same bits
diff --git a/tools/tests/skdiff/test1/output-expected/index.html b/tools/tests/skdiff/test1/output-expected/index.html
index 9f3ce45bae..88b64edc17 100644
--- a/tools/tests/skdiff/test1/output-expected/index.html
+++ b/tools/tests/skdiff/test1/output-expected/index.html
@@ -4,8 +4,8 @@
 <tr><th>3 of 12 images matched exactly.<br></th>
 <th>every different pixel shown in white</th>
 <th>color difference at each pixel</th>
-<th>tools/tests/skdiff/baseDir/</th>
-<th>tools/tests/skdiff/comparisonDir/</th>
+<th>baseDir: tools/tests/skdiff/baseDir/</th>
+<th>comparisonDir: tools/tests/skdiff/comparisonDir/</th>
 </tr>
 <tr>
 <td><b>missing-from-baseDir.png</b><br>Missing from baseDir</td><td>N/A</td><td>N/A</td><td>N/A</td><td><a href="../../../../../tools/tests/skdiff/comparisonDir/missing-from-baseDir.png"><img src="../../../../../tools/tests/skdiff/comparisonDir/missing-from-baseDir.png" height="240px"></a></td></tr>
diff --git a/tools/tests/skdiff/test1/output-expected/return_value b/tools/tests/skdiff/test1/output-expected/return_value
index ec635144f6..573541ac97 100644
--- a/tools/tests/skdiff/test1/output-expected/return_value
+++ b/tools/tests/skdiff/test1/output-expected/return_value
@@ -1 +1 @@
-9
+0
diff --git a/tools/tests/skdiff/test1/output-expected/stdout b/tools/tests/skdiff/test1/output-expected/stdout
index 3d337d3376..142093f1dd 100644
--- a/tools/tests/skdiff/test1/output-expected/stdout
+++ b/tools/tests/skdiff/test1/output-expected/stdout
@@ -6,12 +6,13 @@ ERROR: no codec found for <tools/tests/skdiff/comparisonDir/different-bits-unkno
 baseDir is [tools/tests/skdiff/baseDir/]
 comparisonDir is [tools/tests/skdiff/comparisonDir/]
 writing diffs to outputDir is [tools/tests/skdiff/test1/output-actual/]
-Missing in baseDir:
-	missing-from-baseDir.png
-	missing-from-baseDir.xyz
-Missing in comparisonDir:
-	missing-from-comparisonDir.png
-	missing-from-comparisonDir.xyz
-3 of 12 images matched.
+compared 12 file pairs:
+ 2 file pairs contain exactly the same bits
+ 1 file pairs contain the same pixel values, but not the same bits
+ 2 file pairs have identical dimensions but some differing pixels
+ 2 file pairs have differing dimensions
+ 1 file pairs contain different bits and are not parsable images
+ 2 file pairs missing from comparisonDir
+ 2 file pairs missing from baseDir
 Maximum pixel intensity mismatch 239
 Largest area mismatch was 97.99% of pixels
diff --git a/tools/tests/skdiff/test2/output-expected/command_line b/tools/tests/skdiff/test2/output-expected/command_line
index b0bdd6291e..22dcf74a76 100644
--- a/tools/tests/skdiff/test2/output-expected/command_line
+++ b/tools/tests/skdiff/test2/output-expected/command_line
@@ -1 +1 @@
-out/Debug/skdiff --nodiffs tools/tests/skdiff/baseDir tools/tests/skdiff/comparisonDir tools/tests/skdiff/test2/output-actual
+out/Debug/skdiff --failonmismatches --listfilenames --nodiffs tools/tests/skdiff/baseDir tools/tests/skdiff/comparisonDir tools/tests/skdiff/test2/output-actual
diff --git a/tools/tests/skdiff/test2/output-expected/stdout b/tools/tests/skdiff/test2/output-expected/stdout
index eae392fc54..855deaee6e 100644
--- a/tools/tests/skdiff/test2/output-expected/stdout
+++ b/tools/tests/skdiff/test2/output-expected/stdout
@@ -2,12 +2,13 @@ ERROR: no codec found for basePath <tools/tests/skdiff/baseDir/different-bits-un
 baseDir is [tools/tests/skdiff/baseDir/]
 comparisonDir is [tools/tests/skdiff/comparisonDir/]
 not writing any diffs to outputDir [tools/tests/skdiff/test2/output-actual/]
-Missing in baseDir:
-	missing-from-baseDir.png
-	missing-from-baseDir.xyz
-Missing in comparisonDir:
-	missing-from-comparisonDir.png
-	missing-from-comparisonDir.xyz
-3 of 12 images matched.
+compared 12 file pairs:
+ 2 file pairs contain exactly the same bits: identical-bits-unknown-format.xyz identical-bits.png 
+ 1 file pairs contain the same pixel values, but not the same bits: different-bits-identical-pixels.png 
+ 2 file pairs have identical dimensions but some differing pixels: slightly-different-pixels-same-size.png very-different-pixels-same-size.png 
+ 2 file pairs have differing dimensions: slightly-different-sizes.png very-different-sizes.png 
+ 1 file pairs contain different bits and are not parsable images: different-bits-unknown-format.xyz 
+ 2 file pairs missing from comparisonDir: missing-from-comparisonDir.png missing-from-comparisonDir.xyz 
+ 2 file pairs missing from baseDir: missing-from-baseDir.png missing-from-baseDir.xyz 
 Maximum pixel intensity mismatch 239
 Largest area mismatch was 97.99% of pixels