rename SkBitmapChecksummer as SkBitmapHasher, and prepare for it to possibly use

rename SkBitmapChecksummer as SkBitmapHasher, and prepare for it to possibly use
some algorithm other than CityHash
Committed: https://code.google.com/p/skia/source/detail?r=8639

[epoger, 2013-04-11 17:39:40]

Ben, please review at patchset 2...
CCing Eric so he sees the nacl change to tests.gyp.

https://codereview.chromium.org/14170010/diff/8001/gyp/tests.gyp
File gyp/tests.gyp (right):

https://codereview.chromium.org/14170010/diff/8001/gyp/tests.gyp#newcode145
gyp/tests.gyp:145: '../tests/BitmapHasherTest.cpp',
ChecksumTest directly uses CityHash, and BitmapHasherTest uses CityHash by way
of SkBitmapHasher.  So I suppose they both need to be disabled for NaCl...

[epoger, 2013-04-11 17:41:58]

https://codereview.chromium.org/14170010/diff/8001/gm/gm_expectations.h
File gm/gm_expectations.h (right):

https://codereview.chromium.org/14170010/diff/8001/gm/gm_expectations.h#newcode2
gm/gm_expectations.h:2: * Copyright 2013 Google Inc.
Toplevel comment: the reason for making this change is that, as noted in
https://goto.google.com/ChecksumRemainingSteps , we may want to switch from
CityHash to some other bitmap hashing algorithm.  This CL is a step toward
allowing such a switch.

[borenet, 2013-04-11 17:43:59]

On 2013/04/11 17:41:58, epoger wrote:
> https://codereview.chromium.org/14170010/diff/8001/gm/gm_expectations.h
> File gm/gm_expectations.h (right):
> 
>
https://codereview.chromium.org/14170010/diff/8001/gm/gm_expectations.h#newcode2
> gm/gm_expectations.h:2: * Copyright 2013 Google Inc.
> Toplevel comment: the reason for making this change is that, as noted in
> https://goto.google.com/ChecksumRemainingSteps , we may want to switch from
> CityHash to some other bitmap hashing algorithm.  This CL is a step toward
> allowing such a switch.

Gyp changes look fine to me.

[bungeman-skia, 2013-04-11 20:49:37]

Other than the one naming nit, lgtm.

https://codereview.chromium.org/14170010/diff/8001/src/utils/SkBitmapHasher.h
File src/utils/SkBitmapHasher.h (right):

https://codereview.chromium.org/14170010/diff/8001/src/utils/SkBitmapHasher.h...
src/utils/SkBitmapHasher.h:34: static bool Compute64(const SkBitmap& bitmap,
SkHashDigest *result);
Any reason not to change the name of this right now? Not sure what to call it
(ComputeDigest? make this class a functor?), but the 64 seems a bit out of place
considering everything else has changed.

[epoger, 2013-04-12 02:20:48]

https://codereview.chromium.org/14170010/diff/8001/src/utils/SkBitmapHasher.h
File src/utils/SkBitmapHasher.h (right):

https://codereview.chromium.org/14170010/diff/8001/src/utils/SkBitmapHasher.h...
src/utils/SkBitmapHasher.h:34: static bool Compute64(const SkBitmap& bitmap,
SkHashDigest *result);
On 2013/04/11 20:49:37, bungeman1 wrote:
> Any reason not to change the name of this right now? Not sure what to call it
> (ComputeDigest? make this class a functor?), but the 64 seems a bit out of
place
> considering everything else has changed.

Good idea, thanks.  Done in patchset 3.

[epoger, 2013-04-12 02:24:04]

Committed patchset #4 manually as r8639 (presubmit successful).