rename SkBitmapChecksummer as SkBitmapHasher, and prepare for it to possibly use
some algorithm other than CityHash
Review URL: https://codereview.chromium.org/14170010
git-svn-id: http://skia.googlecode.com/svn/trunk@8639 2bbb7eff-a529-9590-31e7-b0007b416f81
diff --git a/gm/gm_expectations.h b/gm/gm_expectations.h
index 3d3f2b5..84cb7fe2 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 @@
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 da2bf53..91aef10 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 @@
* 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 @@
*
* 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 @@
}
/**
- * 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 @@
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 @@
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 @@
* 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 @@
} 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 @@
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 @@
} 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 @@
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 683255e..68764cd 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 0048111..a14f585 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 63ac726..0000000
--- 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
similarity index 79%
rename from src/utils/SkBitmapChecksummer.cpp
rename to src/utils/SkBitmapHasher.cpp
index 883210c..6df3ab9 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*/ 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 @@
// 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 @@
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 @@
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 0000000..dc8685b
--- /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 0000000..6aa464d
--- /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 0319490..81e7ef3 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 @@
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 @@
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;