skdiff: recurse over subdirectories, unless --norecurse option is given

in anticipation of https://code.google.com/p/skia/issues/detail?id=743 ('move gm baselines outside of trunk, and modify naming convention')
Review URL: https://codereview.appspot.com/6465053

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

tools/skdiff_main.cpp

@@ -26,7 +26,7 @@
  * third directory for each pair.
  * Creates an index.html in the current third directory to compare each
  * pair that does not match exactly.
- * Does *not* recursively descend directories.
+ * Recursively descends directories, unless run with --norecurse.
  *
  * Returns zero exit code if all images match across baseDir and comparisonDir.
  */
@@ -612,6 +612,27 @@ static void compute_diff(DiffRecord* dr,
     dr->fAverageMismatchB = ((float) totalMismatchB) / pixelCount;
 }
 
+/// Return a copy of the "input" string, within which we have replaced all instances
+/// of oldSubstring with newSubstring.
+///
+/// TODO: If we like this, we should move it into the core SkString implementation,
+/// adding more checks and ample test cases, and paying more attention to efficiency.
+static SkString replace_all(const SkString &input,
+                            const char oldSubstring[], const char newSubstring[]) {
+    SkString output;
+    const char *input_cstr = input.c_str();
+    const char *first_char = input_cstr;
+    const char *match_char;
+    int oldSubstringLen = strlen(oldSubstring);
+    while (NULL != (match_char = strstr(first_char, oldSubstring))) {
+        output.append(first_char, (match_char - first_char));
+        output.append(newSubstring);
+        first_char = match_char + oldSubstringLen;
+    }
+    output.append(first_char);
+    return output;
+}
+
 static SkString filename_to_derived_filename (const SkString& filename,
                                               const char *suffix) {
     SkString diffName (filename);
@@ -619,6 +640,10 @@ static SkString filename_to_derived_filename (const SkString& filename,
     int dotOffset = strrchr(cstring, '.') - cstring;
     diffName.remove(dotOffset, diffName.size() - dotOffset);
     diffName.append(suffix);
+
+    // In case we recursed into subdirectories, replace slashes with something else
+    // so the diffs will all be written into a single flat directory.
+    diffName = replace_all(diffName, PATH_DIV_STR, "_");
     return diffName;
 }
 
@@ -686,22 +711,70 @@ static bool string_contains_any_of(const SkString& string,
     return false;
 }
 
-/// Iterate over dir and get all files that:
-///  - match any of the substrings in matchSubstrings, but...
-///  - DO NOT match any of the substrings in nomatchSubstrings
-/// Returns the list of files in *files.
+/// Internal (potentially recursive) implementation of get_file_list.
+static void get_file_list_subdir(const SkString& rootDir, const SkString& subDir,
+                                 const StringArray& matchSubstrings,
+                                 const StringArray& nomatchSubstrings,
+                                 bool recurseIntoSubdirs, FileArray *files) {
+    bool isSubDirEmpty = subDir.isEmpty();
+    SkString dir(rootDir);
+    if (!isSubDirEmpty) {
+        dir.append(PATH_DIV_STR);
+        dir.append(subDir);
+    }
+
+    // Iterate over files (not directories) within dir.
+    SkOSFile::Iter fileIterator(dir.c_str());
+    SkString fileName;
+    while (fileIterator.next(&fileName, false)) {
+        if (fileName.startsWith(".")) {
+            continue;
+        }
+        SkString pathRelativeToRootDir(subDir);
+        if (!isSubDirEmpty) {
+            pathRelativeToRootDir.append(PATH_DIV_STR);
+        }
+        pathRelativeToRootDir.append(fileName);
+        if (string_contains_any_of(pathRelativeToRootDir, matchSubstrings) &&
+            !string_contains_any_of(pathRelativeToRootDir, nomatchSubstrings)) {
+            files->push(new SkString(pathRelativeToRootDir));
+        }
+    }
+
+    // Recurse into any non-ignored subdirectories.
+    if (recurseIntoSubdirs) {
+        SkOSFile::Iter dirIterator(dir.c_str());
+        SkString dirName;
+        while (dirIterator.next(&dirName, true)) {
+            if (dirName.startsWith(".")) {
+                continue;
+            }
+            SkString pathRelativeToRootDir(subDir);
+            if (!isSubDirEmpty) {
+                pathRelativeToRootDir.append(PATH_DIV_STR);
+            }
+            pathRelativeToRootDir.append(dirName);
+            if (!string_contains_any_of(pathRelativeToRootDir, nomatchSubstrings)) {
+                get_file_list_subdir(rootDir, pathRelativeToRootDir,
+                                     matchSubstrings, nomatchSubstrings, recurseIntoSubdirs,
+                                     files);
+            }
+        }
+    }
+}
+
+/// Iterate over dir and get all files whose filename:
+///  - matches any of the substrings in matchSubstrings, but...
+///  - DOES NOT match any of the substrings in nomatchSubstrings
+///  - DOES NOT start with a dot (.)
+/// Adds the matching files to the list in *files.
 static void get_file_list(const SkString& dir,
                           const StringArray& matchSubstrings,
                           const StringArray& nomatchSubstrings,
-                          FileArray *files) {
-    SkOSFile::Iter it(dir.c_str());
-    SkString filename;
-    while (it.next(&filename)) {
-        if (string_contains_any_of(filename, matchSubstrings) &&
-            !string_contains_any_of(filename, nomatchSubstrings)) {
-            files->push(new SkString(filename));
-        }
-    }
+                          bool recurseIntoSubdirs, FileArray *files) {
+    get_file_list_subdir(dir, SkString(""),
+                         matchSubstrings, nomatchSubstrings, recurseIntoSubdirs,
+                         files);
 }
 
 static void release_file_list(FileArray *files) {
@@ -723,6 +796,7 @@ static void create_diff_images (DiffMetricProc dmp,
                                 const SkString& outputDir,
                                 const StringArray& matchSubstrings,
                                 const StringArray& nomatchSubstrings,
+                                bool recurseIntoSubdirs,
                                 DiffSummary* summary) {
     SkASSERT(!baseDir.isEmpty());
     SkASSERT(!comparisonDir.isEmpty());
@@ -730,8 +804,8 @@ static void create_diff_images (DiffMetricProc dmp,
     FileArray baseFiles;
     FileArray comparisonFiles;
 
-    get_file_list(baseDir, matchSubstrings, nomatchSubstrings, &baseFiles);
-    get_file_list(comparisonDir, matchSubstrings, nomatchSubstrings,
+    get_file_list(baseDir, matchSubstrings, nomatchSubstrings, recurseIntoSubdirs, &baseFiles);
+    get_file_list(comparisonDir, matchSubstrings, nomatchSubstrings, recurseIntoSubdirs,
                   &comparisonFiles);
 
     if (!baseFiles.isEmpty()) {
@@ -1146,6 +1220,7 @@ static void usage (char * argv0) {
 "\n                           filenames contain this substring."
 "\n                           this flag may be repeated."
 "\n    --noprintdirs: do not print the directories used."
+"\n    --norecurse: do not recurse into subdirectories."
 "\n    --sortbymaxmismatch: sort by worst color channel mismatch;"
 "\n                         break ties with -sortbymismatch"
 "\n    --sortbymismatch: sort by average color channel mismatch"
@@ -1180,7 +1255,8 @@ int main (int argc, char ** argv) {
 
     bool generateDiffs = true;
     bool listFilenames = false;
-    bool printDirs = true;
+    bool printDirNames = true;
+    bool recurseIntoSubdirs = true;
 
     RecordArray differences;
     DiffSummary summary;
@@ -1219,7 +1295,11 @@ int main (int argc, char ** argv) {
             continue;
         }
         if (!strcmp(argv[i], "--noprintdirs")) {
-            printDirs = false;
+            printDirNames = false;
+            continue;
+        }
+        if (!strcmp(argv[i], "--norecurse")) {
+            recurseIntoSubdirs = false;
             continue;
         }
         if (!strcmp(argv[i], "--sortbymaxmismatch")) {
@@ -1271,14 +1351,14 @@ int main (int argc, char ** argv) {
     if (!baseDir.endsWith(PATH_DIV_STR)) {
         baseDir.append(PATH_DIV_STR);
     }
-    if (printDirs) {
+    if (printDirNames) {
         printf("baseDir is [%s]\n", baseDir.c_str());
     }
 
     if (!comparisonDir.endsWith(PATH_DIV_STR)) {
         comparisonDir.append(PATH_DIV_STR);
     }
-    if (printDirs) {
+    if (printDirNames) {
         printf("comparisonDir is [%s]\n", comparisonDir.c_str());
     }
 
@@ -1286,11 +1366,11 @@ int main (int argc, char ** argv) {
         outputDir.append(PATH_DIV_STR);
     }
     if (generateDiffs) {
-        if (printDirs) {
+        if (printDirNames) {
             printf("writing diffs to outputDir is [%s]\n", outputDir.c_str());
         }
     } else {
-        if (printDirs) {
+        if (printDirNames) {
             printf("not writing any diffs to outputDir [%s]\n", outputDir.c_str());
         }
         outputDir.set("");
@@ -1304,7 +1384,7 @@ int main (int argc, char ** argv) {
 
     create_diff_images(diffProc, colorThreshold, &differences,
                        baseDir, comparisonDir, outputDir,
-                       matchSubstrings, nomatchSubstrings, &summary);
+                       matchSubstrings, nomatchSubstrings, recurseIntoSubdirs, &summary);
     summary.print(listFilenames, failOnResultType);
 
     if (differences.count()) {

tools/tests/skdiff/test1/output-expected/index.html

@@ -8,25 +8,25 @@
 <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>
+<td><b>missing-files/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-files/missing-from-baseDir.png"><img src="../../../../../tools/tests/skdiff/comparisonDir/missing-files/missing-from-baseDir.png" height="240px"></a></td></tr>
 <tr>
-<td><b>missing-from-baseDir.xyz</b><br>Missing from baseDir</td><td>N/A</td><td>N/A</td><td>N/A</td><td>N/A</td></tr>
+<td><b>missing-files/missing-from-baseDir.xyz</b><br>Missing from baseDir</td><td>N/A</td><td>N/A</td><td>N/A</td><td>N/A</td></tr>
 <tr>
-<td><b>missing-from-comparisonDir.png</b><br>Missing from comparisonDir</td><td>N/A</td><td>N/A</td><td><a href="../../../../../tools/tests/skdiff/baseDir/missing-from-comparisonDir.png"><img src="../../../../../tools/tests/skdiff/baseDir/missing-from-comparisonDir.png" height="240px"></a></td><td>N/A</td></tr>
+<td><b>missing-files/missing-from-comparisonDir.png</b><br>Missing from comparisonDir</td><td>N/A</td><td>N/A</td><td><a href="../../../../../tools/tests/skdiff/baseDir/missing-files/missing-from-comparisonDir.png"><img src="../../../../../tools/tests/skdiff/baseDir/missing-files/missing-from-comparisonDir.png" height="240px"></a></td><td>N/A</td></tr>
 <tr>
-<td><b>missing-from-comparisonDir.xyz</b><br>Missing from comparisonDir</td><td>N/A</td><td>N/A</td><td>N/A</td><td>N/A</td></tr>
+<td><b>missing-files/missing-from-comparisonDir.xyz</b><br>Missing from comparisonDir</td><td>N/A</td><td>N/A</td><td>N/A</td><td>N/A</td></tr>
 <tr>
-<td><b>different-bits-unknown-format.xyz</b><br>Files differ; unable to parse one or both files</td><td>N/A</td><td>N/A</td><td>N/A</td><td>N/A</td></tr>
+<td><b>different-bits/different-bits-unknown-format.xyz</b><br>Files differ; unable to parse one or both files</td><td>N/A</td><td>N/A</td><td>N/A</td><td>N/A</td></tr>
 <tr>
-<td><b>slightly-different-sizes.png</b><br>Image sizes differ</td><td>N/A</td><td>N/A</td><td><a href="../../../../../tools/tests/skdiff/baseDir/slightly-different-sizes.png"><img src="../../../../../tools/tests/skdiff/baseDir/slightly-different-sizes.png" height="240px"></a></td><td><a href="../../../../../tools/tests/skdiff/comparisonDir/slightly-different-sizes.png"><img src="../../../../../tools/tests/skdiff/comparisonDir/slightly-different-sizes.png" height="240px"></a></td></tr>
+<td><b>different-bits/slightly-different-sizes.png</b><br>Image sizes differ</td><td>N/A</td><td>N/A</td><td><a href="../../../../../tools/tests/skdiff/baseDir/different-bits/slightly-different-sizes.png"><img src="../../../../../tools/tests/skdiff/baseDir/different-bits/slightly-different-sizes.png" height="240px"></a></td><td><a href="../../../../../tools/tests/skdiff/comparisonDir/different-bits/slightly-different-sizes.png"><img src="../../../../../tools/tests/skdiff/comparisonDir/different-bits/slightly-different-sizes.png" height="240px"></a></td></tr>
 <tr>
-<td><b>very-different-sizes.png</b><br>Image sizes differ</td><td>N/A</td><td>N/A</td><td><a href="../../../../../tools/tests/skdiff/baseDir/very-different-sizes.png"><img src="../../../../../tools/tests/skdiff/baseDir/very-different-sizes.png" height="240px"></a></td><td><a href="../../../../../tools/tests/skdiff/comparisonDir/very-different-sizes.png"><img src="../../../../../tools/tests/skdiff/comparisonDir/very-different-sizes.png" height="128px"></a></td></tr>
+<td><b>different-bits/very-different-sizes.png</b><br>Image sizes differ</td><td>N/A</td><td>N/A</td><td><a href="../../../../../tools/tests/skdiff/baseDir/different-bits/very-different-sizes.png"><img src="../../../../../tools/tests/skdiff/baseDir/different-bits/very-different-sizes.png" height="240px"></a></td><td><a href="../../../../../tools/tests/skdiff/comparisonDir/different-bits/very-different-sizes.png"><img src="../../../../../tools/tests/skdiff/comparisonDir/different-bits/very-different-sizes.png" height="128px"></a></td></tr>
 <tr>
-<td><b>very-different-pixels-same-size.png</b><br>     97.9926% of pixels differ
-  (     42.8911% weighted)<br>Average color mismatch 89<br>Max color mismatch 239</td><td><a href="very-different-pixels-same-size-white.png"><img src="very-different-pixels-same-size-white.png" height="240px"></a></td><td><a href="very-different-pixels-same-size-diff.png"><img src="very-different-pixels-same-size-diff.png" height="240px"></a></td><td><a href="../../../../../tools/tests/skdiff/baseDir/very-different-pixels-same-size.png"><img src="../../../../../tools/tests/skdiff/baseDir/very-different-pixels-same-size.png" height="240px"></a></td><td><a href="../../../../../tools/tests/skdiff/comparisonDir/very-different-pixels-same-size.png"><img src="../../../../../tools/tests/skdiff/comparisonDir/very-different-pixels-same-size.png" height="240px"></a></td></tr>
+<td><b>different-bits/very-different-pixels-same-size.png</b><br>     97.9926% of pixels differ
+  (     42.8911% weighted)<br>Average color mismatch 89<br>Max color mismatch 239</td><td><a href="different-bits_very-different-pixels-same-size-white.png"><img src="different-bits_very-different-pixels-same-size-white.png" height="240px"></a></td><td><a href="different-bits_very-different-pixels-same-size-diff.png"><img src="different-bits_very-different-pixels-same-size-diff.png" height="240px"></a></td><td><a href="../../../../../tools/tests/skdiff/baseDir/different-bits/very-different-pixels-same-size.png"><img src="../../../../../tools/tests/skdiff/baseDir/different-bits/very-different-pixels-same-size.png" height="240px"></a></td><td><a href="../../../../../tools/tests/skdiff/comparisonDir/different-bits/very-different-pixels-same-size.png"><img src="../../../../../tools/tests/skdiff/comparisonDir/different-bits/very-different-pixels-same-size.png" height="240px"></a></td></tr>
 <tr>
-<td><b>slightly-different-pixels-same-size.png</b><br>      0.6630% of pixels differ
-  (      0.1904% weighted)<br>(2164 pixels)<br>Average color mismatch 0<br>Max color mismatch 213</td><td><a href="slightly-different-pixels-same-size-white.png"><img src="slightly-different-pixels-same-size-white.png" height="240px"></a></td><td><a href="slightly-different-pixels-same-size-diff.png"><img src="slightly-different-pixels-same-size-diff.png" height="240px"></a></td><td><a href="../../../../../tools/tests/skdiff/baseDir/slightly-different-pixels-same-size.png"><img src="../../../../../tools/tests/skdiff/baseDir/slightly-different-pixels-same-size.png" height="240px"></a></td><td><a href="../../../../../tools/tests/skdiff/comparisonDir/slightly-different-pixels-same-size.png"><img src="../../../../../tools/tests/skdiff/comparisonDir/slightly-different-pixels-same-size.png" height="240px"></a></td></tr>
+<td><b>different-bits/slightly-different-pixels-same-size.png</b><br>      0.6630% of pixels differ
+  (      0.1904% weighted)<br>(2164 pixels)<br>Average color mismatch 0<br>Max color mismatch 213</td><td><a href="different-bits_slightly-different-pixels-same-size-white.png"><img src="different-bits_slightly-different-pixels-same-size-white.png" height="240px"></a></td><td><a href="different-bits_slightly-different-pixels-same-size-diff.png"><img src="different-bits_slightly-different-pixels-same-size-diff.png" height="240px"></a></td><td><a href="../../../../../tools/tests/skdiff/baseDir/different-bits/slightly-different-pixels-same-size.png"><img src="../../../../../tools/tests/skdiff/baseDir/different-bits/slightly-different-pixels-same-size.png" height="240px"></a></td><td><a href="../../../../../tools/tests/skdiff/comparisonDir/different-bits/slightly-different-pixels-same-size.png"><img src="../../../../../tools/tests/skdiff/comparisonDir/different-bits/slightly-different-pixels-same-size.png" height="240px"></a></td></tr>
 </table>
 </body>
 </html>

tools/tests/skdiff/test1/output-expected/stdout

@@ -1,8 +1,8 @@
-ERROR: no codec found for basePath <tools/tests/skdiff/baseDir/different-bits-unknown-format.xyz>
-ERROR: no codec found for <tools/tests/skdiff/comparisonDir/missing-from-baseDir.xyz>
-ERROR: no codec found for <tools/tests/skdiff/baseDir/missing-from-comparisonDir.xyz>
-ERROR: no codec found for <tools/tests/skdiff/baseDir/different-bits-unknown-format.xyz>
-ERROR: no codec found for <tools/tests/skdiff/comparisonDir/different-bits-unknown-format.xyz>
+ERROR: no codec found for basePath <tools/tests/skdiff/baseDir/different-bits/different-bits-unknown-format.xyz>
+ERROR: no codec found for <tools/tests/skdiff/comparisonDir/missing-files/missing-from-baseDir.xyz>
+ERROR: no codec found for <tools/tests/skdiff/baseDir/missing-files/missing-from-comparisonDir.xyz>
+ERROR: no codec found for <tools/tests/skdiff/baseDir/different-bits/different-bits-unknown-format.xyz>
+ERROR: no codec found for <tools/tests/skdiff/comparisonDir/different-bits/different-bits-unknown-format.xyz>
 baseDir is [tools/tests/skdiff/baseDir/]
 comparisonDir is [tools/tests/skdiff/comparisonDir/]
 writing diffs to outputDir is [tools/tests/skdiff/test1/output-actual/]

tools/tests/skdiff/test2/output-expected/stdout

@@ -1,16 +1,16 @@
-ERROR: no codec found for basePath <tools/tests/skdiff/baseDir/different-bits-unknown-format.xyz>
+ERROR: no codec found for basePath <tools/tests/skdiff/baseDir/different-bits/different-bits-unknown-format.xyz>
 baseDir is [tools/tests/skdiff/baseDir/]
 comparisonDir is [tools/tests/skdiff/comparisonDir/]
 not writing any diffs to outputDir [tools/tests/skdiff/test2/output-actual/]
 
 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 
+[_] 2 file pairs contain exactly the same bits: identical-bits/identical-bits-unknown-format.xyz identical-bits/identical-bits.png 
+[_] 1 file pairs contain the same pixel values, but not the same bits: different-bits/different-bits-identical-pixels.png 
+[*] 2 file pairs have identical dimensions but some differing pixels: different-bits/slightly-different-pixels-same-size.png different-bits/very-different-pixels-same-size.png 
+[*] 2 file pairs have differing dimensions: different-bits/slightly-different-sizes.png different-bits/very-different-sizes.png 
+[*] 1 file pairs contain different bits and are not parsable images: different-bits/different-bits-unknown-format.xyz 
+[_] 2 file pairs missing from comparisonDir: missing-files/missing-from-comparisonDir.png missing-files/missing-from-comparisonDir.xyz 
+[_] 2 file pairs missing from baseDir: missing-files/missing-from-baseDir.png missing-files/missing-from-baseDir.xyz 
 [*] 0 file pairs not compared yet: 
 (results marked with [*] will cause nonzero return value)