ARGB image encoder for checksums.

src/utils/SkBitmapHasher.cpp

Index: src/utils/SkBitmapHasher.cpp
===================================================================
--- src/utils/SkBitmapHasher.cpp	(revision 8777)
+++ src/utils/SkBitmapHasher.cpp	(working copy)
@@ -1,4 +1,3 @@
-
 /*
  * Copyright 2012 Google Inc.
  *
@@ -8,63 +7,75 @@
 
 #include "SkBitmap.h"
 #include "SkBitmapHasher.h"
-#include "SkBitmapTransformer.h"
 #include "SkCityHash.h"
 #include "SkEndian.h"
+#include "SkImageEncoder.h"
+#include "SkStream.h"
+#include "SkMD5.h"
 
+#define SK_DIGEST_CONST 0
+#define SK_DIGEST_CITYHASH 1
+#define SK_DIGEST_MD5 2
+#define SK_DIGEST SK_DIGEST_CITYHASH
+
+#if SK_DIGEST != SK_DIGEST_CONST
 /**
- * Write an integer value into a bytebuffer in little-endian order.
+ * Write an integer value to a stream in little-endian order.
  */
-static void write_int_to_buffer(int val, char* buf) {
+static void write_int_to_buffer(int val, SkWStream* out) {
     val = SkEndian_SwapLE32(val);
-    for (int byte=0; byte<4; byte++) {
-        *buf++ = (char)(val & 0xff);
+    for (size_t byte = 0; byte < sizeof(int); ++byte) {
+        out->write8((uint8_t)(val & 0xff));
         val = val >> 8;
     }
 }
+#endif
 
-/*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
+/*static*/ bool SkBitmapHasher::ComputeDigestInternal(const SkBitmap& bitmap,
+                                                      SkHashDigest *result) {
+#if SK_DIGEST == SK_DIGEST_CONST
+    *result = 0;
+#else
 
+#if SK_DIGEST == SK_DIGEST_CITYHASH
+    size_t pixelBufferSize = bitmap.width() * bitmap.height() * 4;
+    size_t totalBufferSize = pixelBufferSize + 2 * sizeof(int);
+
     SkAutoMalloc bufferManager(totalBufferSize);
     char *bufferStart = static_cast<char *>(bufferManager.get());
-    char *bufPtr = bufferStart;
+    SkMemoryWStream out(bufferStart, totalBufferSize);
+#elif SK_DIGEST == SK_DIGEST_MD5
+    SkMD5 out;
+#endif
     // start with the x/y dimensions
-    write_int_to_buffer(bitmap.width(), bufPtr);
-    bufPtr += 4;
-    write_int_to_buffer(bitmap.height(), bufPtr);
-    bufPtr += 4;
+    write_int_to_buffer(bitmap.width(), &out);
+    write_int_to_buffer(bitmap.height(), &out);
 
     // add all the pixel data
-    if (!transformer.copyBitmapToPixelBuffer(bufPtr, pixelBufferSize)) {
+    SkAutoTDelete<SkImageEncoder> enc(CreateARGBImageEncoder());
+    if (!enc->encodeStream(&out, bitmap, SkImageEncoder::kDefaultQuality)) {

[epoger, 2013/04/22 19:53:28]

So, I guess SkMemoryWStream is going to keep reallocating its memory buffer
until it is large enough to hold the entire transformed bitmap, right?

Doesn't that seem less efficient than my previous code, in which we call
transformer.bytesNeededTotal() and allocate a large enough chunk in one go?

Even better is the code in
https://codereview.chromium.org/14265010/diff2/53019:12005/src/utils/SkBitmap...
, where we only need to allocate a chunk large enough for a single row of
pixels.

[bungeman-skia, 2013/04/22 20:08:00]

No. We're only using the SkMemoryWStream in the CityHash case, and we're
pre-allocating the memory as ARGBImageEncoder advertises the amount of memory it
requires. Note that this is an SkMemoryWStream, not a SkDynamicMemoryWStream.
The MD5 case just passes the MD5 in, since it is a SkWStream. That's part of its
optimization / convenience, it's stream based, not a one shot hash.

         return false;
     }
+
+#if SK_DIGEST == SK_DIGEST_CITYHASH
     *result = SkCityHash::Compute64(bufferStart, totalBufferSize);
+#elif SK_DIGEST == SK_DIGEST_MD5
+    SkMD5::Digest digest;
+    out.finish(digest);
+    *result = *((SkHashDigest*)&digest);

[epoger, 2013/04/22 19:53:28]

Since SkHashDigest is just a typedef for uint64_t, this is going to truncate the
16-byte-long MD5 hash digest, right?  That may account for why you didn't see as
big of a runtime difference as I did in
https://code.google.com/p/skia/issues/detail?id=1257 ('deciding between
CityHash, MD5, and SHA1 for image hash algorithm')

[bungeman-skia, 2013/04/22 20:08:00]

I was interested in the difference between MD5 and CityHash, the rest should be
minimal (if it isn't, that needs to be fixed). Truncating a digest is just as
valid, you're just throwing away the rest of the entropy.

+#endif
+#endif
     return true;
 }
 
 /*static*/ bool SkBitmapHasher::ComputeDigest(const SkBitmap& bitmap, SkHashDigest *result) {
-    const SkBitmapTransformer::PixelFormat kPixelFormat =
-        SkBitmapTransformer::kARGB_8888_Premul_PixelFormat;
-
-    // First, try to transform the existing bitmap.
-    const SkBitmapTransformer transformer =
-        SkBitmapTransformer(bitmap, kPixelFormat);
-    if (transformer.isValid(false)) {
-        return ComputeDigestInternal(bitmap, transformer, result);
+    if (ComputeDigestInternal(bitmap, result)) {
+        return true;
     }
 
     // Hmm, that didn't work. Maybe if we create a new
     // kARGB_8888_Config version of the bitmap it will work better?
     SkBitmap copyBitmap;
     bitmap.copyTo(&copyBitmap, SkBitmap::kARGB_8888_Config);
-    const SkBitmapTransformer copyTransformer =
-        SkBitmapTransformer(copyBitmap, kPixelFormat);
-    if (copyTransformer.isValid(true)) {
-        return ComputeDigestInternal(copyBitmap, copyTransformer, result);
-    } else {
-        return false;
-    }
+    return ComputeDigestInternal(copyBitmap, result);
 }