GM: specify that currently used checksums are CityHashes of SkBitmaps

GM: specify that currently used checksums are CityHashes of SkBitmaps
(distinct from other hash algorithms we may switch to, or hashes of
files such as PDFs that cannot be read into an SkBitmap)

Committed: https://code.google.com/p/skia/source/detail?r=8877

[epoger, 2013-04-25 16:48:30]

https://codereview.chromium.org/14284018/diff/1/gm/tests/outputs/compared-aga...
File
gm/tests/outputs/compared-against-different-pixels-images/output-expected/json-summary.txt
(right):

https://codereview.chromium.org/14284018/diff/1/gm/tests/outputs/compared-aga...
gm/tests/outputs/compared-against-different-pixels-images/output-expected/json-summary.txt:5:
"bitmap-cityhash" : 9512553915271796906
patchset 1 changes the names used in the JSON representation

[epoger, 2013-04-25 17:23:20]

https://codereview.chromium.org/14284018/diff/4001/gm/gm_expectations.cpp
File gm/gm_expectations.cpp (right):

https://codereview.chromium.org/14284018/diff/4001/gm/gm_expectations.cpp#new...
gm/gm_expectations.cpp:21: Expectations::Expectations(const SkBitmap& bitmap,
bool ignoreFailure) {
patchset 2: moved most implementation details out of gm_expectations.h into new
gm_expectations.cpp file.  

This will allow us to hide future changes within gm_expectations.cpp, confident
that gmmain.cpp and other source files will not depend on implementation details
that used to be stored in the header file.

All of the implementations were moved in VERBATIM from gm_expectations.h.  If I
make any changes to the implementation logic, that will be in a separate
patchset.

[epoger, 2013-04-25 19:17:42]

https://codereview.chromium.org/14284018/diff/9001/gm/gm_expectations.cpp
File gm/gm_expectations.cpp (right):

https://codereview.chromium.org/14284018/diff/9001/gm/gm_expectations.cpp#new...
gm/gm_expectations.cpp:14: const static char kJsonKey_ActualResults[]   =
"actual-results";
patchset 3 moves all the json field names into gm_expectations.cpp, so that we
can change them later and no other code will care.

[epoger, 2013-04-25 21:15:35]

Ready to review at patchset 4.

[borenet, 2013-04-26 14:00:22]

On 2013/04/25 21:15:35, epoger wrote:
> Ready to review at patchset 4.

Nice!  This will make it easy to switch between hash functions if we find it
necessary.  One thing that is not clear to me is what will happen if we do
switch, and the expectations file has cityhashes while the new method is md5 or
some such.  Will this be interpreted as no-expectations, or will it be a
mismatch (indicating that the expectations need rebaselining)?

[epoger, 2013-04-26 14:49:11]

On 2013/04/26 14:00:22, borenet wrote:
> On 2013/04/25 21:15:35, epoger wrote:
> > Ready to review at patchset 4.
> 
> Nice!  This will make it easy to switch between hash functions if we find it
> necessary.  One thing that is not clear to me is what will happen if we do
> switch, and the expectations file has cityhashes while the new method is md5
or
> some such.  Will this be interpreted as no-expectations, or will it be a
> mismatch (indicating that the expectations need rebaselining)?

A good question... here's my plan.  Tell me what you think:


1. Here's what the actual and expected results look like now:

{
   "actual-results" : {
      "failed" : null,
      "failure-ignored" : null,
      "no-comparison" : null,
      "succeeded" : {
         "8888/selftest1" : {
            "bitmap-cityhash" : 14022967492765711532
         }
      }
   },
   "expected-results" : {
      "8888/selftest1" : {
         "allowed-bitmap-cityhashes" : [ 14022967492765711532 ],
         "ignore-failure" : false
      }
   }
}


2. After switching to MD5 (for example), the results of the first GenerateGMs
run would look like this, and thus the CompareGMs step would go red:

{
   "actual-results" : {
      "failed" : {
         "8888/selftest1" : {
            "bitmap-md5" : 12345
         }
      },
      "failure-ignored" : null,
      "no-comparison" : null,
      "succeeded" : null,
   },
   "expected-results" : {
      "8888/selftest1" : {
         "allowed-bitmap-cityhashes" : [ 14022967492765711532 ],
         "ignore-failure" : false
      }
   }
}


3. Then a human being would run the rebaselining tool (not yet written, but the
same rebaselining tool we would use for any pixel changes).  That tool would
download and compare the following two images:

http://some.root/gm/bitmap-cityhash/selftest1/14022967492765711532.png
http://some.root/gm/bitmap-md5/selftest1/12345.png

The tool would see that the images were pixel-identical, and thus generate new
expectations automatically:

   "expected-results" : {
      "8888/selftest1" : {
         "allowed-bitmap-md5" : [ 12345 ],
         "ignore-failure" : false
      }
   }


4. After the human committed those new expectations, the next run of GenerateGM
would succeed and yield these results:

{
   "actual-results" : {
      "failed" : null,
      "failure-ignored" : null,
      "no-comparison" : null,
      "succeeded" : {
         "8888/selftest1" : {
            "bitmap-md5" : 12345
         }
      }
   },
   "expected-results" : {
      "8888/selftest1" : {
         "allowed-bitmap-md5s" : [ 12345 ],
         "ignore-failure" : false
      }
   }
}


5. At some point, we would want to go through and delete all of the images
stored under http://some.root/gm/bitmap-cityhash/ .

[borenet, 2013-04-26 14:53:13]

On 2013/04/26 14:49:11, epoger wrote:
> On 2013/04/26 14:00:22, borenet wrote:
> > On 2013/04/25 21:15:35, epoger wrote:
> > > Ready to review at patchset 4.
> > 
> > Nice!  This will make it easy to switch between hash functions if we find it
> > necessary.  One thing that is not clear to me is what will happen if we do
> > switch, and the expectations file has cityhashes while the new method is md5
> or
> > some such.  Will this be interpreted as no-expectations, or will it be a
> > mismatch (indicating that the expectations need rebaselining)?
> 
> A good question... here's my plan.  Tell me what you think:
> 
> 
> 1. Here's what the actual and expected results look like now:
> 
> {
>    "actual-results" : {
>       "failed" : null,
>       "failure-ignored" : null,
>       "no-comparison" : null,
>       "succeeded" : {
>          "8888/selftest1" : {
>             "bitmap-cityhash" : 14022967492765711532
>          }
>       }
>    },
>    "expected-results" : {
>       "8888/selftest1" : {
>          "allowed-bitmap-cityhashes" : [ 14022967492765711532 ],
>          "ignore-failure" : false
>       }
>    }
> }
> 
> 
> 2. After switching to MD5 (for example), the results of the first GenerateGMs
> run would look like this, and thus the CompareGMs step would go red:
> 
> {
>    "actual-results" : {
>       "failed" : {
>          "8888/selftest1" : {
>             "bitmap-md5" : 12345
>          }
>       },
>       "failure-ignored" : null,
>       "no-comparison" : null,
>       "succeeded" : null,
>    },
>    "expected-results" : {
>       "8888/selftest1" : {
>          "allowed-bitmap-cityhashes" : [ 14022967492765711532 ],
>          "ignore-failure" : false
>       }
>    }
> }
> 
> 
> 3. Then a human being would run the rebaselining tool (not yet written, but
the
> same rebaselining tool we would use for any pixel changes).  That tool would
> download and compare the following two images:
> 
> http://some.root/gm/bitmap-cityhash/selftest1/14022967492765711532.png
> http://some.root/gm/bitmap-md5/selftest1/12345.png
> 
> The tool would see that the images were pixel-identical, and thus
generate new
> expectations automatically:
> 
>    "expected-results" : {
>       "8888/selftest1" : {
>          "allowed-bitmap-md5" : [ 12345 ],
>          "ignore-failure" : false
>       }
>    }
> 
> 
> 4. After the human committed those new expectations, the next run of
GenerateGM
> would succeed and yield these results:
> 
> {
>    "actual-results" : {
>       "failed" : null,
>       "failure-ignored" : null,
>       "no-comparison" : null,
>       "succeeded" : {
>          "8888/selftest1" : {
>             "bitmap-md5" : 12345
>          }
>       }
>    },
>    "expected-results" : {
>       "8888/selftest1" : {
>          "allowed-bitmap-md5s" : [ 12345 ],
>          "ignore-failure" : false
>       }
>    }
> }
> 
> 
> 5. At some point, we would want to go through and delete all of the images
> stored under http://some.root/gm/bitmap-cityhash/ .

Sounds great.  LGTM

[epoger, 2013-04-26 15:06:55]

Committed patchset #5 manually as r8877 (presubmit successful).