Hash .pngs instead of SkBitmaps.
This has the nice property of being able to double-check hashes after the fact.
mtklein@mtklein ~/skia (hash-png)> md5sum bad/8888/3x3bitmaprect.png
deede70ab2f34067d461fb4a93332d4c bad/8888/3x3bitmaprect.png
mtklein@mtklein ~/skia (hash-png)> grep 3x3bitmaprect_8888 bad/dm.json
"3x3bitmaprect_8888" : "deede70ab2f34067d461fb4a93332d4c",
I have checked that no two premultiplied colors map to the same unpremultiplied
color (math nerds: unpremultiplication is injective), so a change in
premultiplied SkBitmap will always imply a change in the encoded
unpremultiplied .png. This means, it's safe to hash .pngs; we won't miss
subtle changes.
BUG=skia:
R=jcgregorio@google.com, stephana@google.com, mtklein@google.com
Author: mtklein@chromium.org
Review URL: https://codereview.chromium.org/549203003
diff --git a/dm/DMWriteTask.cpp b/dm/DMWriteTask.cpp
index 00dfffc..efc1415 100644
--- a/dm/DMWriteTask.cpp
+++ b/dm/DMWriteTask.cpp
@@ -67,59 +67,37 @@
}
}
-static bool save_bitmap_to_file(SkBitmap bitmap, const char* path) {
- SkFILEWStream stream(path);
- if (!stream.isValid() ||
- !SkImageEncoder::EncodeStream(&stream, bitmap, SkImageEncoder::kPNG_Type, 100)) {
- SkDebugf("Can't write a PNG to %s.\n", path);
- return false;
+static SkStreamAsset* encode_to_png(const SkBitmap& bitmap) {
+ SkDynamicMemoryWStream png;
+ if (!SkImageEncoder::EncodeStream(&png, bitmap, SkImageEncoder::kPNG_Type, 100)) {
+ return NULL;
}
- return true;
+ png.copyToData()->unref(); // Forces detachAsStream() to be contiguous.
+ return png.detachAsStream();
}
-// Does not take ownership of data.
-static bool save_data_to_file(SkStreamAsset* data, const char* path) {
- data->rewind();
- SkFILEWStream stream(path);
- if (!stream.isValid() || !stream.writeStream(data, data->getLength())) {
- SkDebugf("Can't write data to %s.\n", path);
- return false;
- }
- return true;
-}
-
-static SkString finish_hash(SkMD5* hasher) {
- SkMD5::Digest digest;
- hasher->finish(digest);
-
- SkString out;
- for (int i = 0; i < 16; i++) {
- out.appendf("%02x", digest.data[i]);
- }
- return out;
-}
-
-static SkString hash(const SkBitmap& src) {
- SkMD5 hasher;
- {
- SkAutoLockPixels lock(src);
- hasher.write(src.getPixels(), src.getSize());
- }
- return finish_hash(&hasher);
-}
-
-static SkString hash(SkStreamAsset* src) {
+static SkString get_md5(SkStreamAsset* src) {
SkMD5 hasher;
hasher.write(src->getMemoryBase(), src->getLength());
- return finish_hash(&hasher);
+ SkMD5::Digest digest;
+ hasher.finish(digest);
+
+ SkString md5;
+ for (int i = 0; i < 16; i++) {
+ md5.appendf("%02x", digest.data[i]);
+ }
+ return md5;
}
void WriteTask::draw() {
- JsonData entry = {
- fFullName,
- fData ? hash(fData) : hash(fBitmap),
- };
+ if (!fData.get()) {
+ fData.reset(encode_to_png(fBitmap));
+ if (!fData.get()) {
+ this->fail("Can't encode to PNG.");
+ }
+ }
+ JsonData entry = { fFullName, get_md5(fData) };
{
SkAutoMutexAcquire lock(&gJsonDataLock);
gJsonData.push_back(entry);
@@ -156,10 +134,14 @@
// If already present we overwrite here, since the content may have changed.
}
- const bool ok = fData.get() ? save_data_to_file(fData.get(), path.c_str())
- : save_bitmap_to_file(fBitmap, path.c_str());
- if (!ok) {
- this->fail();
+ SkFILEWStream file(path.c_str());
+ if (!file.isValid()) {
+ return this->fail("Can't open file.");
+ }
+
+ fData->rewind();
+ if (!file.writeStream(fData, fData->getLength())) {
+ return this->fail("Can't write to file.");
}
}
@@ -213,7 +195,8 @@
}
const char* expected = fJson[name.c_str()].asCString();
- SkString actual = hash(bitmap);
+ SkAutoTDelete<SkStreamAsset> png(encode_to_png(bitmap));
+ SkString actual = get_md5(png);
return actual.equals(expected);
}
diff --git a/gyp/tests.gypi b/gyp/tests.gypi
index cf78e41..15f5d04 100644
--- a/gyp/tests.gypi
+++ b/gyp/tests.gypi
@@ -205,6 +205,7 @@
'../tests/TracingTest.cpp',
'../tests/TypefaceTest.cpp',
'../tests/UnicodeTest.cpp',
+ '../tests/UnpremultiplyTest.cpp',
'../tests/UtilsTest.cpp',
'../tests/WArrayTest.cpp',
'../tests/WritePixelsTest.cpp',
diff --git a/tests/UnpremultiplyTest.cpp b/tests/UnpremultiplyTest.cpp
new file mode 100644
index 0000000..7846a1b
--- /dev/null
+++ b/tests/UnpremultiplyTest.cpp
@@ -0,0 +1,38 @@
+/*
+ * Copyright 2014 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "SkColorPriv.h"
+#include "SkUnPreMultiply.h"
+#include "Test.h"
+
+DEF_TEST(Unpremultiply, reporter) {
+ // Here we test that unpremultiplication is injective:
+ // no two distinct premul colors map to the same unpremul color.
+
+ // DM exploits this fact to safely hash .pngs instead of the original bitmaps.
+
+ // It is sufficient to test red. Green and blue follow the same rules.
+ // This means we have at most 256*256 possible colors to deal with.
+ int hits[256*256];
+ for (size_t i = 0; i < SK_ARRAY_COUNT(hits); i++) {
+ hits[i] = 0;
+ }
+
+ for (int a = 0; a < 256; a++) {
+ for (int r = 0; r <= a; r++) {
+ SkPMColor pm = SkPackARGB32(a, r, 0, 0);
+ SkColor upm = SkUnPreMultiply::PMColorToColor(pm);
+
+ // ARGB -> AR
+ hits[upm >> 16]++;
+ }
+ }
+
+ for (size_t i = 0; i < SK_ARRAY_COUNT(hits); i++) {
+ REPORTER_ASSERT(reporter, hits[i] < 2);
+ }
+}