diff options
-rw-r--r-- | gm/gm_expectations.h | 10 | ||||
-rw-r--r-- | gm/gmmain.cpp | 51 | ||||
-rw-r--r-- | gyp/tests.gyp | 2 | ||||
-rw-r--r-- | gyp/utils.gyp | 4 | ||||
-rw-r--r-- | src/utils/SkBitmapChecksummer.h | 37 | ||||
-rw-r--r-- | src/utils/SkBitmapHasher.cpp (renamed from src/utils/SkBitmapChecksummer.cpp) | 19 | ||||
-rw-r--r-- | src/utils/SkBitmapHasher.h | 42 | ||||
-rw-r--r-- | tests/BitmapHasherTest.cpp | 64 | ||||
-rw-r--r-- | tests/ChecksumTest.cpp | 31 |
9 files changed, 158 insertions, 102 deletions
diff --git a/gm/gm_expectations.h b/gm/gm_expectations.h index 3d3f2b55fa..84cb7fe255 100644 --- a/gm/gm_expectations.h +++ b/gm/gm_expectations.h @@ -10,7 +10,7 @@ #include <stdarg.h> #include "gm.h" #include "SkBitmap.h" -#include "SkBitmapChecksummer.h" +#include "SkBitmapHasher.h" #include "SkData.h" #include "SkImageDecoder.h" #include "SkOSFile.h" @@ -94,7 +94,13 @@ namespace skiagm { Expectations(const SkBitmap& bitmap, bool ignoreFailure=kDefaultIgnoreFailure) { fBitmap = bitmap; fIgnoreFailure = ignoreFailure; - fAllowedChecksums.push_back() = SkBitmapChecksummer::Compute64(bitmap); + SkHashDigest digest; + // TODO(epoger): Better handling for error returned by ComputeDigest()? + // For now, we just report a digest of 0 in error cases, like before. + if (!SkBitmapHasher::ComputeDigest(bitmap, &digest)) { + digest = 0; + } + fAllowedChecksums.push_back() = digest; } /** diff --git a/gm/gmmain.cpp b/gm/gmmain.cpp index da2bf53fcd..91aef10188 100644 --- a/gm/gmmain.cpp +++ b/gm/gmmain.cpp @@ -18,7 +18,7 @@ #include "gm_expectations.h" #include "system_preferences.h" #include "SkBitmap.h" -#include "SkBitmapChecksummer.h" +#include "SkBitmapHasher.h" #include "SkColorPriv.h" #include "SkCommandLineFlags.h" #include "SkData.h" @@ -401,24 +401,24 @@ public: * will be written to, or compared against, PNG files. * PRO: Preserve/compare alpha channel info for the non-PNG cases * (comparing different renderModes in-memory) - * CON: The bitmaps (and checksums) for these non-PNG cases would be + * CON: The bitmaps (and hash digests) for these non-PNG cases would be * different than those for the PNG-compared cases, and in the * case of a failed renderMode comparison, how would we write the * image to disk for examination? * - * 2. Always compute image checksums from PNG format (either + * 2. Always compute image hash digests from PNG format (either * directly from the the bytes of a PNG file, or capturing the * bytes we would have written to disk if we were writing the * bitmap out as a PNG). * PRO: I think this would allow us to never force opaque, and to * the extent that alpha channel data can be preserved in a PNG * file, we could observe it. - * CON: If we read a bitmap from disk, we need to take its checksum + * CON: If we read a bitmap from disk, we need to take its hash digest * from the source PNG (we can't compute it from the bitmap we * read out of the PNG, because we will have already premultiplied * the alpha). * CON: Seems wasteful to convert a bitmap to PNG format just to take - * its checksum. (Although we're wasting lots of effort already + * its hash digest. (Although we're wasting lots of effort already * calling force_all_opaque().) * * 3. Make the alpha premultiply/unpremultiply routines 100% consistent, @@ -428,7 +428,7 @@ public: * * 4. Perform a "close enough" comparison of bitmaps (+/- 1 bit in each * channel), rather than demanding absolute equality. - * CON: Can't do this with checksums. + * CON: Can't do this with hash digests. */ static void complete_bitmap(SkBitmap* bitmap) { force_all_opaque(*bitmap); @@ -690,7 +690,7 @@ public: } /** - * Compares actual checksum to expectations, returning the set of errors + * Compares actual hash digest to expectations, returning the set of errors * (if any) that we saw along the way. * * If fMismatchPath has been set, and there are pixel diffs, then the @@ -713,14 +713,19 @@ public: const char renderModeDescriptor[], bool addToJsonSummary) { ErrorCombination errors; - Checksum actualChecksum = SkBitmapChecksummer::Compute64(actualBitmap); + SkHashDigest actualBitmapHash; + // TODO(epoger): Better handling for error returned by ComputeDigest()? + // For now, we just report a digest of 0 in error cases, like before. + if (!SkBitmapHasher::ComputeDigest(actualBitmap, &actualBitmapHash)) { + actualBitmapHash = 0; + } SkString completeNameString = baseNameString; completeNameString.append(renderModeDescriptor); const char* completeName = completeNameString.c_str(); if (expectations.empty()) { errors.add(kMissingExpectations_ErrorType); - } else if (!expectations.match(actualChecksum)) { + } else if (!expectations.match(actualBitmapHash)) { addToJsonSummary = true; // The error mode we record depends on whether this was running // in a non-standard renderMode. @@ -749,7 +754,7 @@ public: RecordTestResults(errors, baseNameString, renderModeDescriptor); if (addToJsonSummary) { - add_actual_results_to_json_summary(completeName, actualChecksum, errors, + add_actual_results_to_json_summary(completeName, actualBitmapHash, errors, expectations.ignoreFailure()); add_expected_results_to_json_summary(completeName, expectations); } @@ -762,12 +767,12 @@ public: * depending on status. */ void add_actual_results_to_json_summary(const char testName[], - Checksum actualChecksum, + const SkHashDigest& actualBitmapHash, ErrorCombination result, bool ignoreFailure) { Json::Value actualResults; actualResults[kJsonKey_ActualResults_AnyStatus_Checksum] = - asJsonValue(actualChecksum); + asJsonValue(actualBitmapHash); if (result.isEmpty()) { this->fJsonActualResults_Succeeded[testName] = actualResults; } else { @@ -783,7 +788,7 @@ public: } else { if (result.includes(kMissingExpectations_ErrorType)) { // TODO: What about the case where there IS an - // expected image checksum, but that gm test + // expected image hash digest, but that gm test // doesn't actually run? For now, those cases // will always be ignored, because gm only looks // at expectations that correspond to gm tests @@ -841,14 +846,14 @@ public: if (expectationsSource && (gRec.fFlags & kRead_ConfigFlag)) { /* * Get the expected results for this test, as one or more allowed - * checksums. The current implementation of expectationsSource - * get this by computing the checksum of a single PNG file on disk. + * hash digests. The current implementation of expectationsSource + * get this by computing the hash digest of a single PNG file on disk. * * TODO(epoger): This relies on the fact that * force_all_opaque() was called on the bitmap before it * was written to disk as a PNG in the first place. If - * not, the checksum returned here may not match the - * checksum of actualBitmap, which *has* been run through + * not, the hash digest returned here may not match the + * hash digest of actualBitmap, which *has* been run through * force_all_opaque(). * See comments above complete_bitmap() for more detail. */ @@ -858,9 +863,13 @@ public: } else { // If we are running without expectations, we still want to // record the actual results. - Checksum actualChecksum = - SkBitmapChecksummer::Compute64(actualBitmap); - add_actual_results_to_json_summary(name.c_str(), actualChecksum, + SkHashDigest actualBitmapHash; + // TODO(epoger): Better handling for error returned by ComputeDigest()? + // For now, we just report a digest of 0 in error cases, like before. + if (!SkBitmapHasher::ComputeDigest(actualBitmap, &actualBitmapHash)) { + actualBitmapHash = 0; + } + add_actual_results_to_json_summary(name.c_str(), actualBitmapHash, ErrorCombination(kMissingExpectations_ErrorType), false); RecordTestResults(ErrorCombination(kMissingExpectations_ErrorType), name, ""); @@ -1105,7 +1114,7 @@ public: int fTestsRun; SkTDict<int> fRenderModesEncountered; - // Where to read expectations (expected image checksums, etc.) from. + // Where to read expectations (expected image hash digests, etc.) from. // If unset, we don't do comparisons. SkAutoTUnref<ExpectationsSource> fExpectationsSource; diff --git a/gyp/tests.gyp b/gyp/tests.gyp index 683255eca7..68764cd1a2 100644 --- a/gyp/tests.gyp +++ b/gyp/tests.gyp @@ -23,6 +23,7 @@ '../tests/BitmapCopyTest.cpp', '../tests/BitmapFactoryTest.cpp', '../tests/BitmapGetColorTest.cpp', + '../tests/BitmapHasherTest.cpp', '../tests/BitmapHeapTest.cpp', '../tests/BitmapTransformerTest.cpp', '../tests/BitSetTest.cpp', @@ -143,6 +144,7 @@ # TODO(borenet): Find a way to either provide this dependency or # replace it. 'sources!': [ + '../tests/BitmapHasherTest.cpp', '../tests/ChecksumTest.cpp', ], }], diff --git a/gyp/utils.gyp b/gyp/utils.gyp index 00481112ee..a14f585dfe 100644 --- a/gyp/utils.gyp +++ b/gyp/utils.gyp @@ -57,8 +57,8 @@ '../src/utils/SkBase64.cpp', '../src/utils/SkBase64.h', - '../src/utils/SkBitmapChecksummer.cpp', - '../src/utils/SkBitmapChecksummer.h', + '../src/utils/SkBitmapHasher.cpp', + '../src/utils/SkBitmapHasher.h', '../src/utils/SkBitmapTransformer.cpp', '../src/utils/SkBitmapTransformer.h', '../src/utils/SkBitSet.cpp', diff --git a/src/utils/SkBitmapChecksummer.h b/src/utils/SkBitmapChecksummer.h deleted file mode 100644 index 63ac726c2e..0000000000 --- a/src/utils/SkBitmapChecksummer.h +++ /dev/null @@ -1,37 +0,0 @@ - -/* - * Copyright 2012 Google Inc. - * - * Use of this source code is governed by a BSD-style license that can be - * found in the LICENSE file. - */ - -#ifndef SkBitmapChecksummer_DEFINED -#define SkBitmapChecksummer_DEFINED - -#include "SkBitmap.h" -#include "SkBitmapTransformer.h" - -/** - * Static class that can generate checksums from SkBitmaps. - */ -class SkBitmapChecksummer { -public: - /** - * Returns a 64-bit checksum of the pixels in this bitmap. - * - * If this is unable to compute the checksum for some reason, - * it returns 0. - * - * Note: depending on the bitmap config, we may need to create an - * intermediate SkBitmap and copy the pixels over to it... so in some - * cases, performance and memory usage can suffer. - */ - static uint64_t Compute64(const SkBitmap& bitmap); - -private: - static uint64_t Compute64Internal(const SkBitmap& bitmap, - const SkBitmapTransformer& transformer); -}; - -#endif diff --git a/src/utils/SkBitmapChecksummer.cpp b/src/utils/SkBitmapHasher.cpp index 883210c360..6df3ab9f27 100644 --- a/src/utils/SkBitmapChecksummer.cpp +++ b/src/utils/SkBitmapHasher.cpp @@ -7,7 +7,7 @@ */ #include "SkBitmap.h" -#include "SkBitmapChecksummer.h" +#include "SkBitmapHasher.h" #include "SkBitmapTransformer.h" #include "SkCityHash.h" #include "SkEndian.h" @@ -23,8 +23,8 @@ static void write_int_to_buffer(int val, char* buf) { } } -/*static*/ uint64_t SkBitmapChecksummer::Compute64Internal( - const SkBitmap& bitmap, const SkBitmapTransformer& transformer) { +/*static*/ bool SkBitmapHasher::ComputeDigestInternal( + const SkBitmap& bitmap, const SkBitmapTransformer& transformer, SkHashDigest *result) { size_t pixelBufferSize = transformer.bytesNeededTotal(); size_t totalBufferSize = pixelBufferSize + 8; // leave room for x/y dimensions @@ -39,12 +39,13 @@ static void write_int_to_buffer(int val, char* buf) { // add all the pixel data if (!transformer.copyBitmapToPixelBuffer(bufPtr, pixelBufferSize)) { - return 0; + return false; } - return SkCityHash::Compute64(bufferStart, totalBufferSize); + *result = SkCityHash::Compute64(bufferStart, totalBufferSize); + return true; } -/*static*/ uint64_t SkBitmapChecksummer::Compute64(const SkBitmap& bitmap) { +/*static*/ bool SkBitmapHasher::ComputeDigest(const SkBitmap& bitmap, SkHashDigest *result) { const SkBitmapTransformer::PixelFormat kPixelFormat = SkBitmapTransformer::kARGB_8888_Premul_PixelFormat; @@ -52,7 +53,7 @@ static void write_int_to_buffer(int val, char* buf) { const SkBitmapTransformer transformer = SkBitmapTransformer(bitmap, kPixelFormat); if (transformer.isValid(false)) { - return Compute64Internal(bitmap, transformer); + return ComputeDigestInternal(bitmap, transformer, result); } // Hmm, that didn't work. Maybe if we create a new @@ -62,8 +63,8 @@ static void write_int_to_buffer(int val, char* buf) { const SkBitmapTransformer copyTransformer = SkBitmapTransformer(copyBitmap, kPixelFormat); if (copyTransformer.isValid(true)) { - return Compute64Internal(copyBitmap, copyTransformer); + return ComputeDigestInternal(copyBitmap, copyTransformer, result); } else { - return 0; + return false; } } diff --git a/src/utils/SkBitmapHasher.h b/src/utils/SkBitmapHasher.h new file mode 100644 index 0000000000..dc8685bddb --- /dev/null +++ b/src/utils/SkBitmapHasher.h @@ -0,0 +1,42 @@ + +/* + * Copyright 2012 Google Inc. + * + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +#ifndef SkBitmapHasher_DEFINED +#define SkBitmapHasher_DEFINED + +#include "SkBitmap.h" +#include "SkBitmapTransformer.h" + +// TODO(epoger): Soon, SkHashDigest will become a real class of its own, +// and callers won't be able to assume it converts to/from a uint64_t. +typedef uint64_t SkHashDigest; + +/** + * Static class that can generate an SkHashDigest from an SkBitmap. + */ +class SkBitmapHasher { +public: + /** + * Fills in "result" with a hash of the pixels in this bitmap. + * + * If this is unable to compute the hash for some reason, + * it returns false. + * + * Note: depending on the bitmap config, we may need to create an + * intermediate SkBitmap and copy the pixels over to it... so in some + * cases, performance and memory usage can suffer. + */ + static bool ComputeDigest(const SkBitmap& bitmap, SkHashDigest *result); + +private: + static bool ComputeDigestInternal(const SkBitmap& bitmap, + const SkBitmapTransformer& transformer, + SkHashDigest *result); +}; + +#endif diff --git a/tests/BitmapHasherTest.cpp b/tests/BitmapHasherTest.cpp new file mode 100644 index 0000000000..6aa464d106 --- /dev/null +++ b/tests/BitmapHasherTest.cpp @@ -0,0 +1,64 @@ + +/* + * Copyright 2013 Google Inc. + * + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ +#include "Test.h" + +#include "SkBitmap.h" +#include "SkBitmapHasher.h" +#include "SkColor.h" + +// Word size that is large enough to hold results of any checksum type. +typedef uint64_t checksum_result; + +namespace skiatest { + class BitmapHasherTestClass : public Test { + public: + static Test* Factory(void*) {return SkNEW(BitmapHasherTestClass); } + protected: + virtual void onGetName(SkString* name) { name->set("BitmapHasher"); } + virtual void onRun(Reporter* reporter) { + this->fReporter = reporter; + RunTest(); + } + private: + + // Fill in bitmap with test data. + void CreateTestBitmap(SkBitmap &bitmap, SkBitmap::Config config, int width, int height, + SkColor color) { + bitmap.setConfig(config, width, height); + REPORTER_ASSERT(fReporter, bitmap.allocPixels()); + bitmap.setIsOpaque(true); + bitmap.eraseColor(color); + } + + void RunTest() { + // Test SkBitmapHasher + SkBitmap bitmap; + SkHashDigest digest; + // initial test case + CreateTestBitmap(bitmap, SkBitmap::kARGB_8888_Config, 333, 555, SK_ColorBLUE); + REPORTER_ASSERT(fReporter, SkBitmapHasher::ComputeDigest(bitmap, &digest)); + REPORTER_ASSERT(fReporter, digest == 0x18f9df68b1b02f38ULL); + // same pixel data but different dimensions should yield a different checksum + CreateTestBitmap(bitmap, SkBitmap::kARGB_8888_Config, 555, 333, SK_ColorBLUE); + REPORTER_ASSERT(fReporter, SkBitmapHasher::ComputeDigest(bitmap, &digest)); + REPORTER_ASSERT(fReporter, digest == 0x6b0298183f786c8eULL); + // same dimensions but different color should yield a different checksum + CreateTestBitmap(bitmap, SkBitmap::kARGB_8888_Config, 555, 333, SK_ColorGREEN); + REPORTER_ASSERT(fReporter, SkBitmapHasher::ComputeDigest(bitmap, &digest)); + REPORTER_ASSERT(fReporter, digest == 0xc6b4b3f6fadaaf37ULL); + // same pixel colors in a different config should yield the same checksum + CreateTestBitmap(bitmap, SkBitmap::kARGB_4444_Config, 555, 333, SK_ColorGREEN); + REPORTER_ASSERT(fReporter, SkBitmapHasher::ComputeDigest(bitmap, &digest)); + REPORTER_ASSERT(fReporter, digest == 0xc6b4b3f6fadaaf37ULL); + } + + Reporter* fReporter; + }; + + static TestRegistry gReg(BitmapHasherTestClass::Factory); +} diff --git a/tests/ChecksumTest.cpp b/tests/ChecksumTest.cpp index 03194907f5..81e7ef396c 100644 --- a/tests/ChecksumTest.cpp +++ b/tests/ChecksumTest.cpp @@ -7,11 +7,8 @@ */ #include "Test.h" -#include "SkBitmap.h" -#include "SkBitmapChecksummer.h" #include "SkChecksum.h" #include "SkCityHash.h" -#include "SkColor.h" // Word size that is large enough to hold results of any checksum type. typedef uint64_t checksum_result; @@ -107,15 +104,6 @@ namespace skiatest { return result; } - // Fill in bitmap with test data. - void CreateTestBitmap(SkBitmap &bitmap, SkBitmap::Config config, int width, int height, - SkColor color) { - bitmap.setConfig(config, width, height); - REPORTER_ASSERT(fReporter, bitmap.allocPixels()); - bitmap.setIsOpaque(true); - bitmap.eraseColor(color); - } - void RunTest() { // Test self-consistency of checksum algorithms. fWhichAlgorithm = kSkChecksum; @@ -156,25 +144,6 @@ namespace skiatest { GetTestDataChecksum(128) == GetTestDataChecksum(256)); REPORTER_ASSERT(fReporter, GetTestDataChecksum(132) == GetTestDataChecksum(260)); - - // Test SkBitmapChecksummer - SkBitmap bitmap; - // initial test case - CreateTestBitmap(bitmap, SkBitmap::kARGB_8888_Config, 333, 555, SK_ColorBLUE); - REPORTER_ASSERT(fReporter, - SkBitmapChecksummer::Compute64(bitmap) == 0x18f9df68b1b02f38ULL); - // same pixel data but different dimensions should yield a different checksum - CreateTestBitmap(bitmap, SkBitmap::kARGB_8888_Config, 555, 333, SK_ColorBLUE); - REPORTER_ASSERT(fReporter, - SkBitmapChecksummer::Compute64(bitmap) == 0x6b0298183f786c8eULL); - // same dimensions but different color should yield a different checksum - CreateTestBitmap(bitmap, SkBitmap::kARGB_8888_Config, 555, 333, SK_ColorGREEN); - REPORTER_ASSERT(fReporter, - SkBitmapChecksummer::Compute64(bitmap) == 0xc6b4b3f6fadaaf37ULL); - // same pixel colors in a different config should yield the same checksum - CreateTestBitmap(bitmap, SkBitmap::kARGB_4444_Config, 555, 333, SK_ColorGREEN); - REPORTER_ASSERT(fReporter, - SkBitmapChecksummer::Compute64(bitmap) == 0xc6b4b3f6fadaaf37ULL); } Reporter* fReporter; |