SkBitmapHasher: use 64-bit-truncated MD5 instead of 64-bit CityHash
authorepoger@google.com <epoger@google.com@2bbb7eff-a529-9590-31e7-b0007b416f81>
Fri, 3 May 2013 17:35:39 +0000 (17:35 +0000)
committerepoger@google.com <epoger@google.com@2bbb7eff-a529-9590-31e7-b0007b416f81>
Fri, 3 May 2013 17:35:39 +0000 (17:35 +0000)
BUG=https://code.google.com/p/skia/issues/detail?id=1257

(if we change our mind within the next few days, we can toggle the
BITMAPHASHER_USES_TRUNCATED_MD5 #ifdef ; at some point, we'll remove that
option so we can delete our CityHash implementation entirely)

R=bungeman@google.com

Review URL: https://codereview.chromium.org/14054012

git-svn-id: http://skia.googlecode.com/svn/trunk@8992 2bbb7eff-a529-9590-31e7-b0007b416f81

16 files changed:
gm/tests/outputs/compared-against-different-pixels-images/output-expected/json-summary.txt
gm/tests/outputs/compared-against-different-pixels-json/output-expected/json-summary.txt
gm/tests/outputs/compared-against-empty-dir/output-expected/json-summary.txt
gm/tests/outputs/compared-against-identical-bytes-images/output-expected/json-summary.txt
gm/tests/outputs/compared-against-identical-bytes-json/output-expected/json-summary.txt
gm/tests/outputs/compared-against-identical-pixels-images/output-expected/json-summary.txt
gm/tests/outputs/compared-against-identical-pixels-json/output-expected/json-summary.txt
gm/tests/outputs/ignore-expectations-mismatch/output-expected/json-summary.txt
gm/tests/outputs/intentionally-skipped-tests/output-expected/json-summary.txt
gm/tests/outputs/no-readpath/output-expected/json-summary.txt
gm/tests/outputs/nonverbose/output-expected/json-summary.txt
gm/tests/outputs/pipe-playback-failure/output-expected/json-summary.txt
include/core/SkEndian.h
src/utils/SkBitmapHasher.cpp
src/utils/SkBitmapHasher.h
tests/BitmapHasherTest.cpp

index 5214f7a..b8d43a3 100644 (file)
@@ -2,10 +2,10 @@
    "actual-results" : {
       "failed" : {
          "565/selftest1" : {
-            "bitmap-cityhash" : 9512553915271796906
+            "bitmap-cityhash" : 12927999507540085554
          },
          "8888/selftest1" : {
-            "bitmap-cityhash" : 14022967492765711532
+            "bitmap-cityhash" : 1209453360120438698
          }
       },
       "failure-ignored" : null,
    },
    "expected-results" : {
       "565/selftest1" : {
-         "allowed-bitmap-cityhashes" : [ 11071285354315388429 ],
+         "allowed-bitmap-cityhashes" : [ 8863920166200910451 ],
          "ignore-failure" : false
       },
       "8888/selftest1" : {
-         "allowed-bitmap-cityhashes" : [ 16527650414256125612 ],
+         "allowed-bitmap-cityhashes" : [ 13451349865803053525 ],
          "ignore-failure" : false
       }
    }
index 5214f7a..b8d43a3 100644 (file)
@@ -2,10 +2,10 @@
    "actual-results" : {
       "failed" : {
          "565/selftest1" : {
-            "bitmap-cityhash" : 9512553915271796906
+            "bitmap-cityhash" : 12927999507540085554
          },
          "8888/selftest1" : {
-            "bitmap-cityhash" : 14022967492765711532
+            "bitmap-cityhash" : 1209453360120438698
          }
       },
       "failure-ignored" : null,
    },
    "expected-results" : {
       "565/selftest1" : {
-         "allowed-bitmap-cityhashes" : [ 11071285354315388429 ],
+         "allowed-bitmap-cityhashes" : [ 8863920166200910451 ],
          "ignore-failure" : false
       },
       "8888/selftest1" : {
-         "allowed-bitmap-cityhashes" : [ 16527650414256125612 ],
+         "allowed-bitmap-cityhashes" : [ 13451349865803053525 ],
          "ignore-failure" : false
       }
    }
index 31892c9..e55c38c 100644 (file)
@@ -4,10 +4,10 @@
       "failure-ignored" : null,
       "no-comparison" : {
          "565/selftest1" : {
-            "bitmap-cityhash" : 9512553915271796906
+            "bitmap-cityhash" : 12927999507540085554
          },
          "8888/selftest1" : {
-            "bitmap-cityhash" : 14022967492765711532
+            "bitmap-cityhash" : 1209453360120438698
          }
       },
       "succeeded" : null
index 417e3d8..e050124 100644 (file)
@@ -5,20 +5,20 @@
       "no-comparison" : null,
       "succeeded" : {
          "565/selftest1" : {
-            "bitmap-cityhash" : 9512553915271796906
+            "bitmap-cityhash" : 12927999507540085554
          },
          "8888/selftest1" : {
-            "bitmap-cityhash" : 14022967492765711532
+            "bitmap-cityhash" : 1209453360120438698
          }
       }
    },
    "expected-results" : {
       "565/selftest1" : {
-         "allowed-bitmap-cityhashes" : [ 9512553915271796906 ],
+         "allowed-bitmap-cityhashes" : [ 12927999507540085554 ],
          "ignore-failure" : false
       },
       "8888/selftest1" : {
-         "allowed-bitmap-cityhashes" : [ 14022967492765711532 ],
+         "allowed-bitmap-cityhashes" : [ 1209453360120438698 ],
          "ignore-failure" : false
       }
    }
index 417e3d8..e050124 100644 (file)
@@ -5,20 +5,20 @@
       "no-comparison" : null,
       "succeeded" : {
          "565/selftest1" : {
-            "bitmap-cityhash" : 9512553915271796906
+            "bitmap-cityhash" : 12927999507540085554
          },
          "8888/selftest1" : {
-            "bitmap-cityhash" : 14022967492765711532
+            "bitmap-cityhash" : 1209453360120438698
          }
       }
    },
    "expected-results" : {
       "565/selftest1" : {
-         "allowed-bitmap-cityhashes" : [ 9512553915271796906 ],
+         "allowed-bitmap-cityhashes" : [ 12927999507540085554 ],
          "ignore-failure" : false
       },
       "8888/selftest1" : {
-         "allowed-bitmap-cityhashes" : [ 14022967492765711532 ],
+         "allowed-bitmap-cityhashes" : [ 1209453360120438698 ],
          "ignore-failure" : false
       }
    }
index 417e3d8..e050124 100644 (file)
@@ -5,20 +5,20 @@
       "no-comparison" : null,
       "succeeded" : {
          "565/selftest1" : {
-            "bitmap-cityhash" : 9512553915271796906
+            "bitmap-cityhash" : 12927999507540085554
          },
          "8888/selftest1" : {
-            "bitmap-cityhash" : 14022967492765711532
+            "bitmap-cityhash" : 1209453360120438698
          }
       }
    },
    "expected-results" : {
       "565/selftest1" : {
-         "allowed-bitmap-cityhashes" : [ 9512553915271796906 ],
+         "allowed-bitmap-cityhashes" : [ 12927999507540085554 ],
          "ignore-failure" : false
       },
       "8888/selftest1" : {
-         "allowed-bitmap-cityhashes" : [ 14022967492765711532 ],
+         "allowed-bitmap-cityhashes" : [ 1209453360120438698 ],
          "ignore-failure" : false
       }
    }
index 417e3d8..e050124 100644 (file)
@@ -5,20 +5,20 @@
       "no-comparison" : null,
       "succeeded" : {
          "565/selftest1" : {
-            "bitmap-cityhash" : 9512553915271796906
+            "bitmap-cityhash" : 12927999507540085554
          },
          "8888/selftest1" : {
-            "bitmap-cityhash" : 14022967492765711532
+            "bitmap-cityhash" : 1209453360120438698
          }
       }
    },
    "expected-results" : {
       "565/selftest1" : {
-         "allowed-bitmap-cityhashes" : [ 9512553915271796906 ],
+         "allowed-bitmap-cityhashes" : [ 12927999507540085554 ],
          "ignore-failure" : false
       },
       "8888/selftest1" : {
-         "allowed-bitmap-cityhashes" : [ 14022967492765711532 ],
+         "allowed-bitmap-cityhashes" : [ 1209453360120438698 ],
          "ignore-failure" : false
       }
    }
index 5214f7a..b8d43a3 100644 (file)
@@ -2,10 +2,10 @@
    "actual-results" : {
       "failed" : {
          "565/selftest1" : {
-            "bitmap-cityhash" : 9512553915271796906
+            "bitmap-cityhash" : 12927999507540085554
          },
          "8888/selftest1" : {
-            "bitmap-cityhash" : 14022967492765711532
+            "bitmap-cityhash" : 1209453360120438698
          }
       },
       "failure-ignored" : null,
    },
    "expected-results" : {
       "565/selftest1" : {
-         "allowed-bitmap-cityhashes" : [ 11071285354315388429 ],
+         "allowed-bitmap-cityhashes" : [ 8863920166200910451 ],
          "ignore-failure" : false
       },
       "8888/selftest1" : {
-         "allowed-bitmap-cityhashes" : [ 16527650414256125612 ],
+         "allowed-bitmap-cityhashes" : [ 13451349865803053525 ],
          "ignore-failure" : false
       }
    }
index 1ee2207..3d3e1e4 100644 (file)
@@ -4,16 +4,16 @@
       "failure-ignored" : null,
       "no-comparison" : {
          "565/selftest1" : {
-            "bitmap-cityhash" : 9512553915271796906
+            "bitmap-cityhash" : 12927999507540085554
          },
          "565/selftest2" : {
-            "bitmap-cityhash" : 11071285354315388429
+            "bitmap-cityhash" : 8863920166200910451
          },
          "8888/selftest1" : {
-            "bitmap-cityhash" : 14022967492765711532
+            "bitmap-cityhash" : 1209453360120438698
          },
          "8888/selftest2" : {
-            "bitmap-cityhash" : 16527650414256125612
+            "bitmap-cityhash" : 13451349865803053525
          }
       },
       "succeeded" : null
index c2325d0..5140616 100644 (file)
@@ -4,10 +4,10 @@
       "failure-ignored" : null,
       "no-comparison" : {
          "565/selftest1" : {
-            "bitmap-cityhash" : 9512553915271796906
+            "bitmap-cityhash" : 12927999507540085554
          },
          "8888/selftest1" : {
-            "bitmap-cityhash" : 14022967492765711532
+            "bitmap-cityhash" : 1209453360120438698
          }
       },
       "succeeded" : null
index 31892c9..e55c38c 100644 (file)
@@ -4,10 +4,10 @@
       "failure-ignored" : null,
       "no-comparison" : {
          "565/selftest1" : {
-            "bitmap-cityhash" : 9512553915271796906
+            "bitmap-cityhash" : 12927999507540085554
          },
          "8888/selftest1" : {
-            "bitmap-cityhash" : 14022967492765711532
+            "bitmap-cityhash" : 1209453360120438698
          }
       },
       "succeeded" : null
index 54e0bde..506191d 100644 (file)
@@ -2,31 +2,31 @@
    "actual-results" : {
       "failed" : {
          "comparison/selftest1-pipe" : {
-            "bitmap-cityhash" : 4259036727585789440
+            "bitmap-cityhash" : 6140979239232854774
          }
       },
       "failure-ignored" : null,
       "no-comparison" : null,
       "succeeded" : {
          "565/selftest1" : {
-            "bitmap-cityhash" : 9512553915271796906
+            "bitmap-cityhash" : 12927999507540085554
          },
          "8888/selftest1" : {
-            "bitmap-cityhash" : 14022967492765711532
+            "bitmap-cityhash" : 1209453360120438698
          }
       }
    },
    "expected-results" : {
       "565/selftest1" : {
-         "allowed-bitmap-cityhashes" : [ 9512553915271796906 ],
+         "allowed-bitmap-cityhashes" : [ 12927999507540085554 ],
          "ignore-failure" : false
       },
       "8888/selftest1" : {
-         "allowed-bitmap-cityhashes" : [ 14022967492765711532 ],
+         "allowed-bitmap-cityhashes" : [ 1209453360120438698 ],
          "ignore-failure" : false
       },
       "comparison/selftest1-pipe" : {
-         "allowed-bitmap-cityhashes" : [ 14022967492765711532 ],
+         "allowed-bitmap-cityhashes" : [ 1209453360120438698 ],
          "ignore-failure" : false
       }
    }
index 01d2195..6eba297 100644 (file)
@@ -65,7 +65,7 @@ template<uint32_t N> struct SkTEndianSwap32 {
                                   (N >> 24);
 };
 
-/** Vector version of SkEndianSwap16(), which swaps the
+/** Vector version of SkEndianSwap32(), which swaps the
     bytes of each value in the array.
 */
 static inline void SkEndianSwap32s(uint32_t array[], int count) {
@@ -77,26 +77,70 @@ static inline void SkEndianSwap32s(uint32_t array[], int count) {
     }
 }
 
+/** Reverse all 8 bytes in a 64bit value.
+    e.g. 0x1122334455667788 -> 0x8877665544332211
+*/
+static inline uint64_t SkEndianSwap64(uint64_t value) {
+    return (((value & 0x00000000000000FFULL) << (8*7)) |
+            ((value & 0x000000000000FF00ULL) << (8*5)) |
+            ((value & 0x0000000000FF0000ULL) << (8*3)) |
+            ((value & 0x00000000FF000000ULL) << (8*1)) |
+            ((value & 0x000000FF00000000ULL) >> (8*1)) |
+            ((value & 0x0000FF0000000000ULL) >> (8*3)) |
+            ((value & 0x00FF000000000000ULL) >> (8*5)) |
+            ((value)                         >> (8*7)));
+}
+template<uint64_t N> struct SkTEndianSwap64 {
+    static const uint64_t value = (((N & 0x00000000000000FFULL) << (8*7)) |
+                                   ((N & 0x000000000000FF00ULL) << (8*5)) |
+                                   ((N & 0x0000000000FF0000ULL) << (8*3)) |
+                                   ((N & 0x00000000FF000000ULL) << (8*1)) |
+                                   ((N & 0x000000FF00000000ULL) >> (8*1)) |
+                                   ((N & 0x0000FF0000000000ULL) >> (8*3)) |
+                                   ((N & 0x00FF000000000000ULL) >> (8*5)) |
+                                   ((N)                         >> (8*7)));
+};
+
+/** Vector version of SkEndianSwap64(), which swaps the
+    bytes of each value in the array.
+*/
+static inline void SkEndianSwap64s(uint64_t array[], int count) {
+    SkASSERT(count == 0 || array != NULL);
+
+    while (--count >= 0) {
+        *array = SkEndianSwap64(*array);
+        array += 1;
+    }
+}
+
 #ifdef SK_CPU_LENDIAN
     #define SkEndian_SwapBE16(n)    SkEndianSwap16(n)
     #define SkEndian_SwapBE32(n)    SkEndianSwap32(n)
+    #define SkEndian_SwapBE64(n)    SkEndianSwap64(n)
     #define SkEndian_SwapLE16(n)    (n)
     #define SkEndian_SwapLE32(n)    (n)
+    #define SkEndian_SwapLE64(n)    (n)
 
     #define SkTEndian_SwapBE16(n)    SkTEndianSwap16<n>::value
     #define SkTEndian_SwapBE32(n)    SkTEndianSwap32<n>::value
+    #define SkTEndian_SwapBE64(n)    SkTEndianSwap64<n>::value
     #define SkTEndian_SwapLE16(n)    (n)
     #define SkTEndian_SwapLE32(n)    (n)
+    #define SkTEndian_SwapLE64(n)    (n)
 #else   // SK_CPU_BENDIAN
     #define SkEndian_SwapBE16(n)    (n)
     #define SkEndian_SwapBE32(n)    (n)
+    #define SkEndian_SwapBE64(n)    (n)
     #define SkEndian_SwapLE16(n)    SkEndianSwap16(n)
     #define SkEndian_SwapLE32(n)    SkEndianSwap32(n)
+    #define SkEndian_SwapLE64(n)    SkEndianSwap64(n)
 
     #define SkTEndian_SwapBE16(n)    (n)
     #define SkTEndian_SwapBE32(n)    (n)
+    #define SkTEndian_SwapBE64(n)    (n)
     #define SkTEndian_SwapLE16(n)    SkTEndianSwap16<n>::value
     #define SkTEndian_SwapLE32(n)    SkTEndianSwap32<n>::value
+    #define SkTEndian_SwapLE64(n)    SkTEndianSwap64<n>::value
 #endif
 
 // When a bytestream is embedded in a 32-bit word, how far we need to
index e8352e5..82cbe9f 100644 (file)
@@ -7,15 +7,20 @@
 
 #include "SkBitmap.h"
 #include "SkBitmapHasher.h"
-#include "SkCityHash.h"
 #include "SkEndian.h"
 #include "SkImageEncoder.h"
+
+#ifdef BITMAPHASHER_USES_TRUNCATED_MD5
+#include "SkMD5.h"
+#else
+#include "SkCityHash.h"
 #include "SkStream.h"
+#endif
 
 /**
- * Write an integer value to a stream in little-endian order.
+ * Write an int32 value to a stream in little-endian order.
  */
-static void write_int_to_buffer(uint32_t val, SkWStream* out) {
+static void write_int32_to_buffer(uint32_t val, SkWStream* out) {
     val = SkEndian_SwapLE32(val);
     for (size_t byte = 0; byte < 4; ++byte) {
         out->write8((uint8_t)(val & 0xff));
@@ -23,18 +28,29 @@ static void write_int_to_buffer(uint32_t val, SkWStream* out) {
     }
 }
 
+/**
+ * Return the first 8 bytes of a bytearray, encoded as a little-endian uint64.
+ */
+static inline uint64_t first_8_bytes_as_uint64(const uint8_t *bytearray) {
+    return SkEndian_SwapLE64(*(reinterpret_cast<const uint64_t *>(bytearray)));
+}
+
 /*static*/ bool SkBitmapHasher::ComputeDigestInternal(const SkBitmap& bitmap,
                                                       SkHashDigest *result) {
+#ifdef BITMAPHASHER_USES_TRUNCATED_MD5
+    SkMD5 out;
+#else
     size_t pixelBufferSize = bitmap.width() * bitmap.height() * 4;
     size_t totalBufferSize = pixelBufferSize + 2 * sizeof(uint32_t);
 
     SkAutoMalloc bufferManager(totalBufferSize);
     char *bufferStart = static_cast<char *>(bufferManager.get());
     SkMemoryWStream out(bufferStart, totalBufferSize);
+#endif
 
     // start with the x/y dimensions
-    write_int_to_buffer(SkToU32(bitmap.width()), &out);
-    write_int_to_buffer(SkToU32(bitmap.height()), &out);
+    write_int32_to_buffer(SkToU32(bitmap.width()), &out);
+    write_int32_to_buffer(SkToU32(bitmap.height()), &out);
 
     // add all the pixel data
     SkAutoTDelete<SkImageEncoder> enc(CreateARGBImageEncoder());
@@ -42,7 +58,13 @@ static void write_int_to_buffer(uint32_t val, SkWStream* out) {
         return false;
     }
 
+#ifdef BITMAPHASHER_USES_TRUNCATED_MD5
+    SkMD5::Digest digest;
+    out.finish(digest);
+    *result = first_8_bytes_as_uint64(digest.data);
+#else
     *result = SkCityHash::Compute64(bufferStart, totalBufferSize);
+#endif
     return true;
 }
 
index 165da3d..c99421a 100644 (file)
@@ -11,6 +11,8 @@
 
 #include "SkBitmap.h"
 
+#define BITMAPHASHER_USES_TRUNCATED_MD5
+
 // 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;
index 6aa464d..2823698 100644 (file)
@@ -42,19 +42,35 @@ namespace skiatest {
             // initial test case
             CreateTestBitmap(bitmap, SkBitmap::kARGB_8888_Config, 333, 555, SK_ColorBLUE);
             REPORTER_ASSERT(fReporter, SkBitmapHasher::ComputeDigest(bitmap, &digest));
+#ifdef BITMAPHASHER_USES_TRUNCATED_MD5
+            REPORTER_ASSERT(fReporter, digest == 0xfb2903562766ef87ULL);
+#else
             REPORTER_ASSERT(fReporter, digest == 0x18f9df68b1b02f38ULL);
+#endif
             // 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));
+#ifdef BITMAPHASHER_USES_TRUNCATED_MD5
+            REPORTER_ASSERT(fReporter, digest == 0xfe04023fb97d0f61ULL);
+#else
             REPORTER_ASSERT(fReporter, digest == 0x6b0298183f786c8eULL);
+#endif
             // 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));
+#ifdef BITMAPHASHER_USES_TRUNCATED_MD5
+            REPORTER_ASSERT(fReporter, digest == 0x2423c51cad6d1edcULL);
+#else
             REPORTER_ASSERT(fReporter, digest == 0xc6b4b3f6fadaaf37ULL);
+#endif
             // 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));
+#ifdef BITMAPHASHER_USES_TRUNCATED_MD5
+            REPORTER_ASSERT(fReporter, digest == 0x2423c51cad6d1edcULL);
+#else
             REPORTER_ASSERT(fReporter, digest == 0xc6b4b3f6fadaaf37ULL);
+#endif
         }
 
         Reporter* fReporter;