Improve sorting of skdiff output, and make it consistent across platforms
BUG=https://code.google.com/p/skia/issues/detail?id=677
Review URL: https://codereview.appspot.com/6351045
git-svn-id: http://skia.googlecode.com/svn/trunk@4388 2bbb7eff-a529-9590-31e7-b0007b416f81
diff --git a/tools/skdiff_main.cpp b/tools/skdiff_main.cpp
index 3ff7c25..34fc975 100644
--- a/tools/skdiff_main.cpp
+++ b/tools/skdiff_main.cpp
@@ -40,14 +40,15 @@
#endif
// 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
- kDifferentSizes, // both are images we can parse, but of different sizes
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
- kBaseMissing, // missing from baseDir
kComparisonMissing,// missing from comparisonDir
+ kBaseMissing, // missing from baseDir
kUnknown, // not compared yet
//
kNumResultTypes // NOT A VALID VALUE--used to set up arrays. Must be last.
@@ -177,7 +178,6 @@
break;
case kDifferentSizes:
fNumMismatches++;
- drp->fFractionDifference = 2;// sort as if 200% of pixels differed
break;
case kDifferentPixels:
fNumMismatches++;
@@ -192,7 +192,6 @@
break;
case kDifferentOther:
fNumMismatches++;
- drp->fFractionDifference = 3;// sort as if 300% of pixels differed
break;
case kBaseMissing:
fNumMismatches++;
@@ -212,63 +211,100 @@
typedef SkTDArray<DiffRecord*> RecordArray;
-/// Comparison routine for qsort; sorts by fFractionDifference
-/// from largest to smallest.
-static int compare_diff_metrics (DiffRecord** lhs, DiffRecord** rhs) {
- if ((*lhs)->fFractionDifference < (*rhs)->fFractionDifference) {
- return 1;
+/// A wrapper for any sortProc (comparison routine) which applies a first-order
+/// sort beforehand, and a tiebreaker if the sortProc returns 0.
+template<typename T>
+static int compare(const void* untyped_lhs, const void* untyped_rhs) {
+ const DiffRecord* lhs = *reinterpret_cast<DiffRecord* const*>(untyped_lhs);
+ const DiffRecord* rhs = *reinterpret_cast<DiffRecord* const*>(untyped_rhs);
+
+ // First-order sort... these comparisons should be applied before comparing
+ // pixel values, no matter what.
+ if (lhs->fResult != rhs->fResult) {
+ return (lhs->fResult < rhs->fResult) ? 1 : -1;
}
- if ((*rhs)->fFractionDifference < (*lhs)->fFractionDifference) {
- return -1;
+
+ // Passed first-order sort, so call the pixel comparison routine.
+ int result = T::comparePixels(lhs, rhs);
+ if (result != 0) {
+ return result;
}
- return 0;
+
+ // Tiebreaker... if we got to this point, we don't really care
+ // which order they are sorted in, but let's at least be consistent.
+ return strcmp(lhs->fFilename.c_str(), rhs->fFilename.c_str());
}
-static int compare_diff_weighted (DiffRecord** lhs, DiffRecord** rhs) {
- if ((*lhs)->fWeightedFraction < (*rhs)->fWeightedFraction) {
- return 1;
+/// Comparison routine for qsort; sorts by fFractionDifference
+/// from largest to smallest.
+class CompareDiffMetrics {
+public:
+ static int comparePixels(const DiffRecord* lhs, const DiffRecord* rhs) {
+ if (lhs->fFractionDifference < rhs->fFractionDifference) {
+ return 1;
+ }
+ if (rhs->fFractionDifference < lhs->fFractionDifference) {
+ return -1;
+ }
+ return 0;
}
- if ((*lhs)->fWeightedFraction > (*rhs)->fWeightedFraction) {
- return -1;
+};
+
+class CompareDiffWeighted {
+public:
+ static int comparePixels(const DiffRecord* lhs, const DiffRecord* rhs) {
+ if (lhs->fWeightedFraction < rhs->fWeightedFraction) {
+ return 1;
+ }
+ if (lhs->fWeightedFraction > rhs->fWeightedFraction) {
+ return -1;
+ }
+ return 0;
}
- return 0;
-}
+};
/// Comparison routine for qsort; sorts by max(fAverageMismatch{RGB})
/// from largest to smallest.
-static int compare_diff_mean_mismatches (DiffRecord** lhs, DiffRecord** rhs) {
- float leftValue = MAX3((*lhs)->fAverageMismatchR,
- (*lhs)->fAverageMismatchG,
- (*lhs)->fAverageMismatchB);
- float rightValue = MAX3((*rhs)->fAverageMismatchR,
- (*rhs)->fAverageMismatchG,
- (*rhs)->fAverageMismatchB);
- if (leftValue < rightValue) {
- return 1;
+class CompareDiffMeanMismatches {
+public:
+ static int comparePixels(const DiffRecord* lhs, const DiffRecord* rhs) {
+ float leftValue = MAX3(lhs->fAverageMismatchR,
+ lhs->fAverageMismatchG,
+ lhs->fAverageMismatchB);
+ float rightValue = MAX3(rhs->fAverageMismatchR,
+ rhs->fAverageMismatchG,
+ rhs->fAverageMismatchB);
+ if (leftValue < rightValue) {
+ return 1;
+ }
+ if (rightValue < leftValue) {
+ return -1;
+ }
+ return 0;
}
- if (rightValue < leftValue) {
- return -1;
- }
- return 0;
-}
+};
/// Comparison routine for qsort; sorts by max(fMaxMismatch{RGB})
/// from largest to smallest.
-static int compare_diff_max_mismatches (DiffRecord** lhs, DiffRecord** rhs) {
- uint32_t leftValue = MAX3((*lhs)->fMaxMismatchR,
- (*lhs)->fMaxMismatchG,
- (*lhs)->fMaxMismatchB);
- uint32_t rightValue = MAX3((*rhs)->fMaxMismatchR,
- (*rhs)->fMaxMismatchG,
- (*rhs)->fMaxMismatchB);
- if (leftValue < rightValue) {
- return 1;
+class CompareDiffMaxMismatches {
+public:
+ static int comparePixels(const DiffRecord* lhs, const DiffRecord* rhs) {
+ uint32_t leftValue = MAX3(lhs->fMaxMismatchR,
+ lhs->fMaxMismatchG,
+ lhs->fMaxMismatchB);
+ uint32_t rightValue = MAX3(rhs->fMaxMismatchR,
+ rhs->fMaxMismatchG,
+ rhs->fMaxMismatchB);
+ if (leftValue < rightValue) {
+ return 1;
+ }
+ if (rightValue < leftValue) {
+ return -1;
+ }
+
+ return CompareDiffMeanMismatches::comparePixels(lhs, rhs);
}
- if (rightValue < leftValue) {
- return -1;
- }
- return compare_diff_mean_mismatches(lhs, rhs);
-}
+};
@@ -1045,7 +1081,7 @@
int main (int argc, char ** argv) {
DiffMetricProc diffProc = compute_diff_pmcolor;
- int (*sortProc)(const void*, const void*) = SkCastForQSort(compare_diff_metrics);
+ int (*sortProc)(const void*, const void*) = compare<CompareDiffMetrics>;
// Maximum error tolerated in any one color channel in any one pixel before
// a difference is reported.
@@ -1085,15 +1121,15 @@
continue;
}
if (!strcmp(argv[i], "--sortbymismatch")) {
- sortProc = SkCastForQSort(compare_diff_mean_mismatches);
+ sortProc = compare<CompareDiffMeanMismatches>;
continue;
}
if (!strcmp(argv[i], "--sortbymaxmismatch")) {
- sortProc = SkCastForQSort(compare_diff_max_mismatches);
+ sortProc = compare<CompareDiffMaxMismatches>;
continue;
}
if (!strcmp(argv[i], "--weighted")) {
- sortProc = SkCastForQSort(compare_diff_weighted);
+ sortProc = compare<CompareDiffWeighted>;
continue;
}
if (argv[i][0] != '-') {
diff --git a/tools/tests/skdiff/test1/output-expected/index.html b/tools/tests/skdiff/test1/output-expected/index.html
index 2cef548..9f3ce45 100644
--- a/tools/tests/skdiff/test1/output-expected/index.html
+++ b/tools/tests/skdiff/test1/output-expected/index.html
@@ -8,6 +8,14 @@
<th>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>
+<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>
+<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>
+<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>
+<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>
<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>
@@ -19,14 +27,6 @@
<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>
-<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>
-<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>
-<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>
-<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>
</table>
</body>
</html>
diff --git a/tools/tests/skdiff/test1/output-expected/stdout b/tools/tests/skdiff/test1/output-expected/stdout
index 862b5be..3d337d3 100644
--- a/tools/tests/skdiff/test1/output-expected/stdout
+++ b/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/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 <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>
baseDir is [tools/tests/skdiff/baseDir/]
comparisonDir is [tools/tests/skdiff/comparisonDir/]
writing diffs to outputDir is [tools/tests/skdiff/test1/output-actual/]