output max-pixel-err if we're comparing against known PNG files.

gm/gmmain.cpp

Index: gm/gmmain.cpp 
=================================================================== 
--- gm/gmmain.cpp	(revision 7412) 
+++ gm/gmmain.cpp	(working copy) 
@@ -195,6 +195,36 @@ 
 | SkGPipeWriter::kSharedAddressSpace_Flag } 
 }; 
 
+static int brute_force_bitmap_compare(const SkBitmap& a, const SkBitmap& b) { 
+ SkAutoLockPixels alp0(a); 
+ SkAutoLockPixels alp1(b); 
+ 
+ if (a.width() != b.width() || a.height() != b.height()) { 
+ return -1; 

[epoger, 2013/01/30 16:46:50]

This is OK, but it would be better to report ERROR_DIMENSION_MISMATCH like we did before https://code.google.com/p/skia/source/detail?r=7093 . Of course, it might be confusing for gm to know about expected image dimensions only *sometimes* (if it is comparing against PNGs, as opposed to comparing against a checksum file like https://code.google.com/p/skia-autogen/source/browse/gm-actual/base-shuttle-w... ) Here are some possibilities I can think of... which do you think is best? 1. Only report ERROR_DIMENSION_MISMATCH in the case when actual images are being compared against local PNG files (not when comparing against checksums). If comparing against checksums, a dimension mismatch will be reported as ERROR_IMAGE_MISMATCH (just like pixel mismatches would be) 2. Never report ERROR_DIMENSION_MISMATCH... even if we know the expected image dimensions (because we are comparing against PNGs), pretend that we don't, and report dimension mismatches as the generic ERROR_IMAGE_MISMATCH. 3. Report dimension mismatches even when comparing against checksums (JSON expectations). We could add new width/height fields to the JSON format, or we could embed the dimensions in the same string as the checksum ("640_480_deadbeef" instead of "deadbeef"). 

+ } 
+ if (a.config() != SkBitmap::kARGB_8888_Config || 
+ b.config() != SkBitmap::kARGB_8888_Config) { 
+ return -2; 

[epoger, 2013/01/30 16:46:50]

Do we not care at all about non-8888 images? 

+ } 
+ 
+ int err = 0; 
+ for (int y = 0; y < a.height(); ++y) { 
+ const SkPMColor* pa = a.getAddr32(0, y); 
+ const SkPMColor* pb = b.getAddr32(0, y); 
+ for (int x = 0; x < a.width(); ++x) { 
+ SkPMColor ca = *pa++; 
+ SkPMColor cb = *pb++; 
+ if (ca == cb) { 
+ continue; 
+ } 
+ err = SkMax32(err, SkAbs32((int)SkGetPackedR32(ca) - (int)SkGetPackedR32(cb))); 

[bsalomon, 2013/01/28 20:14:43]

We've had images differ only in alpha in the past. skdiff was updated recently to report alpha differences. Maybe we should track them here (either in err or separately). 

[epoger, 2013/01/30 16:46:50]

Right. See http://code.google.com/p/skia/source/detail?r=7003 ('Extended skdiff to report alpha channel differences.') Maybe we should move http://code.google.com/p/skia/source/browse/trunk/tools/skdiff.cpp 's compute_diff() function somewhere that it could be shared between gm and skdiff? 

+ err = SkMax32(err, SkAbs32((int)SkGetPackedG32(ca) - (int)SkGetPackedG32(cb))); 
+ err = SkMax32(err, SkAbs32((int)SkGetPackedB32(ca) - (int)SkGetPackedB32(cb))); 
+ } 
+ } 
+ return err; 
+} 
+ 
 class GMMain { 
 public: 
 GMMain() { 
@@ -676,7 +706,18 @@ 
 * force_all_opaque(). 
 * See comments above get_checksum() for more detail. 
 */ 
- Expectations expectations = expectationsSource->get(name.c_str()); 
+ SkBitmap expectedBitmap; 
+ Expectations expectations = expectationsSource->get(name.c_str(), 
+ &expectedBitmap); 
+ 
+ int maxErr = 0; 
+ if (!expectedBitmap.isNull()) { 
+ maxErr = brute_force_bitmap_compare(actualBitmap, expectedBitmap); 
+ } 
+ if (maxErr) { 
+ SkDebugf("--- bitmap compare failed with max component difference %d\n", maxErr); 
+ } 
+ 
 retval |= compare_to_expectations(expectations, actualBitmap, 
 name, "", true); 
 } else {