GM: add --writeChecksumBasedFilenames option

GM: add --writeChecksumBasedFilenames option 

as needed for Step 2 of https://goto.google.com/ChecksumTransitionDetail :
make GenerateGMs upload actual images to Google Storage instead of skia-autogen

R=scroggo@google.com

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

[epoger, 2013-06-18 03:44:10]

https://codereview.chromium.org/17365002/diff/1/gm/gmmain.cpp
File gm/gmmain.cpp (right):

https://codereview.chromium.org/17365002/diff/1/gm/gmmain.cpp#newcode1308
gm/gmmain.cpp:1308: DEFINE_bool(writeChecksumBasedFilenames, false, "When
writing out actual images, use checksum-"
Patchset 1 adds the definition of the new flag, but doesn't actually modify GM's
behavior based on the presence of the flag.

https://codereview.chromium.org/17365002/diff/1/gm/tests/outputs/checksum-bas...
File
gm/tests/outputs/checksum-based-filenames/output-expected/mismatchPath/selftest1_565.png
(right):

https://codereview.chromium.org/17365002/diff/1/gm/tests/outputs/checksum-bas...
gm/tests/outputs/checksum-based-filenames/output-expected/mismatchPath/selftest1_565.png:1:
[contents of
gm/tests/outputs/checksum-based-filenames/output-actual/mismatchPath/selftest1_565.png]
As of patchset 1, you can see that the files written out to mismatchPath (and
writePath, for that matter) still have the old non-checksum-based filenames. 
That will change later in this CL.

[epoger, 2013-06-18 05:45:36]

https://codereview.chromium.org/17365002/diff/5001/gm/gmmain.cpp
File gm/gmmain.cpp (right):

https://codereview.chromium.org/17365002/diff/5001/gm/gmmain.cpp#newcode231
gm/gmmain.cpp:231: SkString make_bitmap_filename(const char *path,
Patchset 2 adds the make_bitmap_filename() wrapper for make_filename(), which we
can call whenever assembling the filename for an SkBitmap (as opposed to a PDF
file, SKP file, etc.)

No behavioral change yet... but this new method is where we will switch based on
the --writeChecksumBasedFilenames flag.

[epoger, 2013-06-18 06:58:38]

https://codereview.chromium.org/17365002/diff/11001/gm/gmmain.cpp
File gm/gmmain.cpp (right):

https://codereview.chromium.org/17365002/diff/11001/gm/gmmain.cpp#newcode237
gm/gmmain.cpp:237: if (fWriteChecksumBasedFilenames) {
At patchset 3, we are assembling the desired output paths, but because they
include multiple directory levels, we still need to create the intermediate
directories.

Here's the error I see when running the self-tests:

GM: FAILED to write
gm/tests/outputs/checksum-based-filenames/output-actual/writePath/bitmap-64bitMD5/selftest1/1209453360120438698.png
GM: FAILED to write
gm/tests/outputs/checksum-based-filenames/output-actual/writePath/bitmap-64bitMD5/selftest1/12927999507540085554.png
GM: expected 9 tests, but ran or skipped 11 tests

[epoger, 2013-06-18 07:21:03]

Ready for review at patchset 4!

https://codereview.chromium.org/17365002/diff/13002/gm/gmmain.cpp
File gm/gmmain.cpp (right):

https://codereview.chromium.org/17365002/diff/13002/gm/gmmain.cpp#newcode240
gm/gmmain.cpp:240: filename.appendUnichar('_');
In patchset 4, I decided that it will be easier across the board if we flatten
the checksum-based filenames so that they can all be written into a single
directory.

https://codereview.chromium.org/17365002/diff/13002/gm/tests/outputs/checksum...
File
gm/tests/outputs/checksum-based-filenames/output-expected/mismatchPath/bitmap-64bitMD5_selftest1_1209453360120438698.png
(right):

https://codereview.chromium.org/17365002/diff/13002/gm/tests/outputs/checksum...
gm/tests/outputs/checksum-based-filenames/output-expected/mismatchPath/bitmap-64bitMD5_selftest1_1209453360120438698.png:1:
[contents of
gm/tests/outputs/checksum-based-filenames/output-actual/mismatchPath/bitmap-64bitMD5_selftest1_1209453360120438698.png]
Note that the name of this file corresponds to a line in the JSON file, as we
would expect:

"selftest1_8888.png" : [ "bitmap-64bitMD5", 1209453360120438698 ]

https://codereview.chromium.org/17365002/diff/13002/gm/tests/outputs/checksum...
File gm/tests/outputs/checksum-based-filenames/output-expected/return_value
(right):

https://codereview.chromium.org/17365002/diff/13002/gm/tests/outputs/checksum...
gm/tests/outputs/checksum-based-filenames/output-expected/return_value:1: 255
nonzero return code, as expected, because the actuals don't match expectations.

[epoger, 2013-06-18 14:48:12]

Regarding the trybot failures... they don't seem to have anything to do with my
CL.  They're all very basic-looking compile failures in libwebp, like this:

[07:23:51.495079] make[1]: *** No rule to make target
`Release/obj.target/libwebp_dec/third_party/externals/libwebp/dec/alpha.o',
needed by `Release/obj.target/gyp/libwebp_dec.a'.  Stop.
[07:23:52.534867] make[1]: *** Waiting for unfinished jobs....

I'm going to ignore those failures... looks like there might be something wrong
with our trybots...

[scroggo, 2013-06-19 14:55:46]

lgtm

https://codereview.chromium.org/17365002/diff/13002/gm/gmmain.cpp
File gm/gmmain.cpp (right):

https://codereview.chromium.org/17365002/diff/13002/gm/gmmain.cpp#newcode783
gm/gmmain.cpp:783: const GmResultDigest& actualResultDigest,
I ran into this earlier when I was modifying skimage - I hate passing in
actualResultDigest, which depends on another parameter being passed in to the
same function. The caller wants the GmResultDigest too, regardless of whether
this function gets called.

Maybe this function should assert that actualResultDigest matches actualBitmap?

[epoger, 2013-06-19 15:02:16]

https://codereview.chromium.org/17365002/diff/13002/gm/gmmain.cpp
File gm/gmmain.cpp (right):

https://codereview.chromium.org/17365002/diff/13002/gm/gmmain.cpp#newcode783
gm/gmmain.cpp:783: const GmResultDigest& actualResultDigest,
On 2013/06/19 14:55:46, scroggo wrote:
> I ran into this earlier when I was modifying skimage - I hate passing in
> actualResultDigest, which depends on another parameter being passed in to the
> same function. The caller wants the GmResultDigest too, regardless of whether
> this function gets called.
> 
> Maybe this function should assert that actualResultDigest matches
actualBitmap?

As discussed live... I will change the code to just pass the bitmaps around, so
we don't have a chance of the bitmap and digest getting out of sync.

I will add TODOs saying something like: we can optimize this later by creating a
new object type that encapsulates both const SkBitmap and const GmResultDigest,
guaranteeing that they stay in sync.

[epoger, 2013-06-19 15:44:08]

Leon, please take a look...

https://codereview.chromium.org/17365002/diff/13002/gm/gmmain.cpp
File gm/gmmain.cpp (right):

https://codereview.chromium.org/17365002/diff/13002/gm/gmmain.cpp#newcode783
gm/gmmain.cpp:783: const GmResultDigest& actualResultDigest,
On 2013/06/19 15:02:16, epoger wrote:
> On 2013/06/19 14:55:46, scroggo wrote:
> > I ran into this earlier when I was modifying skimage - I hate passing in
> > actualResultDigest, which depends on another parameter being passed in to
the
> > same function. The caller wants the GmResultDigest too, regardless of
whether
> > this function gets called.
> > 
> > Maybe this function should assert that actualResultDigest matches
> actualBitmap?
> 
> As discussed live... I will change the code to just pass the bitmaps around,
so
> we don't have a chance of the bitmap and digest getting out of sync.
> 
> I will add TODOs saying something like: we can optimize this later by creating
a
> new object type that encapsulates both const SkBitmap and const
GmResultDigest,
> guaranteeing that they stay in sync.

Heck, it's just as easy to write it now.

https://codereview.chromium.org/17365002/diff/37001/gm/gm_expectations.h
File gm/gm_expectations.h (right):

https://codereview.chromium.org/17365002/diff/37001/gm/gm_expectations.h#newc...
gm/gm_expectations.h:97: class BitmapAndDigest {
added new encapsulating class here

[scroggo, 2013-06-19 15:46:49]

On 2013/06/19 15:44:08, epoger wrote:
> Leon, please take a look...
> 
> https://codereview.chromium.org/17365002/diff/13002/gm/gmmain.cpp
> File gm/gmmain.cpp (right):
> 
> https://codereview.chromium.org/17365002/diff/13002/gm/gmmain.cpp#newcode783
> gm/gmmain.cpp:783: const GmResultDigest& actualResultDigest,
> On 2013/06/19 15:02:16, epoger wrote:
> > On 2013/06/19 14:55:46, scroggo wrote:
> > > I ran into this earlier when I was modifying skimage - I hate passing in
> > > actualResultDigest, which depends on another parameter being passed in to
> the
> > > same function. The caller wants the GmResultDigest too, regardless of
> whether
> > > this function gets called.
> > > 
> > > Maybe this function should assert that actualResultDigest matches
> > actualBitmap?
> > 
> > As discussed live... I will change the code to just pass the bitmaps around,
> so
> > we don't have a chance of the bitmap and digest getting out of sync.
> > 
> > I will add TODOs saying something like: we can optimize this later by
creating
> a
> > new object type that encapsulates both const SkBitmap and const
> GmResultDigest,
> > guaranteeing that they stay in sync.
> 
> Heck, it's just as easy to write it now.
> 
> https://codereview.chromium.org/17365002/diff/37001/gm/gm_expectations.h
> File gm/gm_expectations.h (right):
> 
>
https://codereview.chromium.org/17365002/diff/37001/gm/gm_expectations.h#newc...
> gm/gm_expectations.h:97: class BitmapAndDigest {
> added new encapsulating class here

LGTM

[epoger, 2013-06-19 15:50:14]

Committed patchset #6 manually as r9682 (presubmit successful).