From 84a1802b072d76657261eada3018955283ab29d2 Mon Sep 17 00:00:00 2001 From: "epoger@google.com" Date: Fri, 1 Feb 2013 20:39:15 +0000 Subject: gm: report max-pixel-error if comparing against PNG files (not checksums) Review URL: https://codereview.appspot.com/7241064 git-svn-id: http://skia.googlecode.com/svn/trunk@7526 2bbb7eff-a529-9590-31e7-b0007b416f81 --- gm/gm_expectations.h | 23 +++++++++++---- gm/gmmain.cpp | 79 ++++++++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 91 insertions(+), 11 deletions(-) diff --git a/gm/gm_expectations.h b/gm/gm_expectations.h index 59a0c1c697..07f0e60dbd 100644 --- a/gm/gm_expectations.h +++ b/gm/gm_expectations.h @@ -8,6 +8,7 @@ #define gm_expectations_DEFINED #include "gm.h" +#include "SkBitmap.h" #include "SkBitmapChecksummer.h" #include "SkImageDecoder.h" #include "SkOSFile.h" @@ -62,14 +63,15 @@ namespace skiagm { } /** - * Allow exactly one checksum (appropriate for the case when we + * Expect exactly one image (appropriate for the case when we * are comparing against a single PNG file). * * By default, DO NOT ignore failures. */ - Expectations(Checksum singleChecksum, bool ignoreFailure=false) { + Expectations(const SkBitmap& bitmap, bool ignoreFailure=false) { + fBitmap = bitmap; fIgnoreFailure = ignoreFailure; - fAllowedChecksums.push_back() = singleChecksum; + fAllowedChecksums.push_back() = SkBitmapChecksummer::Compute64(bitmap); } /** @@ -97,6 +99,16 @@ namespace skiagm { return false; } + /** + * If this Expectation is based on a single SkBitmap, return a + * pointer to that SkBitmap. Otherwise (if the Expectation is + * empty, or if it was based on a list of checksums rather + * than a single bitmap), returns NULL. + */ + const SkBitmap *asBitmap() const { + return (SkBitmap::kNo_Config == fBitmap.config()) ? NULL : &fBitmap; + } + /** * Return a JSON representation of the allowed checksums. * This does NOT include any information about whether to @@ -116,6 +128,7 @@ namespace skiagm { private: SkTArray fAllowedChecksums; bool fIgnoreFailure; + SkBitmap fBitmap; }; /** @@ -154,9 +167,7 @@ namespace skiagm { SkImageDecoder::kDecodePixels_Mode, NULL); if (decodedReferenceBitmap) { - Checksum checksum = SkBitmapChecksummer::Compute64( - referenceBitmap); - return Expectations(checksum); + return Expectations(referenceBitmap); } else { if (fNotifyOfMissingFiles) { fprintf(stderr, "FAILED to read %s\n", path.c_str()); diff --git a/gm/gmmain.cpp b/gm/gmmain.cpp index 8ded7c1662..bbeb443d9f 100644 --- a/gm/gmmain.cpp +++ b/gm/gmmain.cpp @@ -80,6 +80,12 @@ extern bool gSkSuppressFontCachePurgeSpew; #define CAN_IMAGE_PDF 0 #endif +// TODO(epoger): We created this ErrorBitfield so that we could record +// multiple error types for the same comparison. But in practice, we +// process its final value in switch() statements, which inherently +// assume that only one error type will be set. +// I think we should probably change this to be an enum, and thus +// constrain ourselves to a single error type per comparison. typedef int ErrorBitfield; const static ErrorBitfield ERROR_NONE = 0x00; const static ErrorBitfield ERROR_NO_GPU_CONTEXT = 0x01; @@ -259,7 +265,7 @@ public: // of this type. void RecordError(ErrorBitfield errorType, const SkString& name, const char renderModeDescriptor []) { - bool isPixelError = false; + bool isPixelError; switch (errorType) { case ERROR_NONE: return; @@ -536,6 +542,61 @@ public: } } + /** + * Log more detail about the mistmatch between expectedBitmap and + * actualBitmap. + */ + void report_bitmap_diffs(const SkBitmap& expectedBitmap, const SkBitmap& actualBitmap, + const char *testName) { + const int expectedWidth = expectedBitmap.width(); + const int expectedHeight = expectedBitmap.height(); + const int width = actualBitmap.width(); + const int height = actualBitmap.height(); + if ((expectedWidth != width) || (expectedHeight != height)) { + SkDebugf("---- %s: dimension mismatch -- expected [%d %d], actual [%d %d]\n", + testName, expectedWidth, expectedHeight, width, height); + return; + } + + if ((SkBitmap::kARGB_8888_Config != expectedBitmap.config()) || + (SkBitmap::kARGB_8888_Config != actualBitmap.config())) { + SkDebugf("---- %s: not computing max per-channel pixel mismatch because non-8888\n", + testName); + return; + } + + SkAutoLockPixels alp0(expectedBitmap); + SkAutoLockPixels alp1(actualBitmap); + int errR = 0; + int errG = 0; + int errB = 0; + int errA = 0; + int differingPixels = 0; + + for (int y = 0; y < height; ++y) { + const SkPMColor* expectedPixelPtr = expectedBitmap.getAddr32(0, y); + const SkPMColor* actualPixelPtr = actualBitmap.getAddr32(0, y); + for (int x = 0; x < width; ++x) { + SkPMColor expectedPixel = *expectedPixelPtr++; + SkPMColor actualPixel = *actualPixelPtr++; + if (expectedPixel != actualPixel) { + differingPixels++; + errR = SkMax32(errR, SkAbs32((int)SkGetPackedR32(expectedPixel) - + (int)SkGetPackedR32(actualPixel))); + errG = SkMax32(errG, SkAbs32((int)SkGetPackedG32(expectedPixel) - + (int)SkGetPackedG32(actualPixel))); + errB = SkMax32(errB, SkAbs32((int)SkGetPackedB32(expectedPixel) - + (int)SkGetPackedB32(actualPixel))); + errA = SkMax32(errA, SkAbs32((int)SkGetPackedA32(expectedPixel) - + (int)SkGetPackedA32(actualPixel))); + } + } + } + SkDebugf("---- %s: %d (of %d) differing pixels, max per-channel mismatch" + " R=%d G=%d B=%d A=%d\n", + testName, differingPixels, width*height, errR, errG, errB, errA); + } + /** * Compares actual checksum to expectations. * Returns ERROR_NONE if they match, or some particular error code otherwise @@ -572,12 +633,22 @@ public: retval = ERROR_NONE; } else { retval = ERROR_IMAGE_MISMATCH; + + // Write out the "actuals" for any mismatches, if we have + // been directed to do so. if (fMismatchPath) { SkString path = make_filename(fMismatchPath, renderModeDescriptor, baseNameString.c_str(), "png"); write_bitmap(path, actualBitmap); } + + // If we have access to a single expected bitmap, log more + // detail about the mismatch. + const SkBitmap *expectedBitmapPtr = expectations.asBitmap(); + if (NULL != expectedBitmapPtr) { + report_bitmap_diffs(*expectedBitmapPtr, actualBitmap, completeName); + } } RecordError(retval, baseNameString, renderModeDescriptor); @@ -609,7 +680,7 @@ public: // TODO: Once we have added the ability to compare // actual results against expectations in a JSON file // (where we can set ignore-failure to either true or - // false), add tests cases that exercise ignored + // false), add test cases that exercise ignored // failures (both for ERROR_READING_REFERENCE_IMAGE // and ERROR_IMAGE_MISMATCH). this->fJsonActualResults_FailureIgnored[testName] = @@ -733,9 +804,7 @@ public: SkASSERT(referenceBitmap); SkString name = make_name(gm->shortName(), gRec.fName); - Checksum referenceChecksum = - SkBitmapChecksummer::Compute64(*referenceBitmap); - Expectations expectations(referenceChecksum); + Expectations expectations(*referenceBitmap); return compare_to_expectations(expectations, actualBitmap, name, renderModeDescriptor); } -- cgit v1.2.3