make compare_rendered_pictures process render_pictures's new JSON output format

make compare_rendered_pictures process render_pictures's new JSON output format

BUG=skia:1942, skia:2230
NOTRY=True

Committed: http://code.google.com/p/skia/source/detail?r=14591

[epoger, 2014-05-02 21:24:52]

Ready for review at patchset 2.

https://codereview.chromium.org/265793013/diff/20001/gm/rebaseline_server/tes...
File
gm/rebaseline_server/testdata/inputs/render_pictures_output/after_patch/builder1/output.json
(right):

https://codereview.chromium.org/265793013/diff/20001/gm/rebaseline_server/tes...
gm/rebaseline_server/testdata/inputs/render_pictures_output/after_patch/builder1/output.json:2:
"header" : {
Updated render_pictures JSON summary format to reflect
https://github.com/google/skia/commit/24c568c1597de9f54df8cea3b46f2e93028a5aec

https://codereview.chromium.org/265793013/diff/20001/gm/rebaseline_server/tes...
gm/rebaseline_server/testdata/inputs/render_pictures_output/after_patch/builder1/output.json:3:
"type" : "ChecksummedImages",
In a future CL, I will make the unittest call render_pictures to generate test
input files, rather than using canned input files.  That way, any changes in
render_pictures will be instantly reflected in these unittest results.

[borenet, 2014-05-06 14:35:48]

LGTM with some concerns.

https://codereview.chromium.org/265793013/diff/20001/gm/gm_json.py
File gm/gm_json.py (right):

https://codereview.chromium.org/265793013/diff/20001/gm/gm_json.py#newcode26
gm/gm_json.py:26: # gm_expectations.cpp and tools/PictureRenderer.cpp !
This is scary!

https://codereview.chromium.org/265793013/diff/20001/gm/rebaseline_server/com...
File gm/rebaseline_server/compare_rendered_pictures.py (right):

https://codereview.chromium.org/265793013/diff/20001/gm/rebaseline_server/com...
gm/rebaseline_server/compare_rendered_pictures.py:140:
gm_json.JSONKEY_SOURCE_WHOLEIMAGE, None)
Is it possible that dictA contains SKPs not in dictB? If so, this will cause an
error.

https://codereview.chromium.org/265793013/diff/20001/gm/rebaseline_server/com...
gm/rebaseline_server/compare_rendered_pictures.py:225: imageB_checksum = None
An inline function might be nice to remove this duplication:

def _checksum_and_relative_url(d):
  if d:
    return (d[gm_json.JSONKEY_IMAGE_FILEPATH],
            (d[gm_json.JSONKEY_IMAGE_CHECKSUMALGORITHM],
             d[gm_json.JSONKEY_IMAGE_CHECKSUMVALUE]))
  else:
    return None, None

imageA_relative_url, imageA_checksum = _checksum_and_relative_url(image_dict_A)
imageB_relative_url, imageB_checksum = _checksum_and_relative_url(image_dict_B)

[epoger, 2014-05-06 15:13:52]

Great comments, Eric. Thanks.  PTAL at patchset 3.

https://codereview.chromium.org/265793013/diff/20001/gm/gm_json.py
File gm/gm_json.py (right):

https://codereview.chromium.org/265793013/diff/20001/gm/gm_json.py#newcode26
gm/gm_json.py:26: # gm_expectations.cpp and tools/PictureRenderer.cpp !
On 2014/05/06 14:35:48, borenet wrote:
> This is scary!

Agreed.

The good news is that in another CL I am moving these constants from
tools/PictureRenderer.cpp to tools/image_expectations.cpp , which will
eventually merge with gm/gm_expectations.cpp .

We'll still need to keep them in sync between Python and C++ (unless you maybe
have a clever idea to get around that; I'm sure something is POSSIBLE)

https://codereview.chromium.org/265793013/diff/20001/gm/rebaseline_server/com...
File gm/rebaseline_server/compare_rendered_pictures.py (right):

https://codereview.chromium.org/265793013/diff/20001/gm/rebaseline_server/com...
gm/rebaseline_server/compare_rendered_pictures.py:140:
gm_json.JSONKEY_SOURCE_WHOLEIMAGE, None)
On 2014/05/06 14:35:48, borenet wrote:
> Is it possible that dictA contains SKPs not in dictB? If so, this will cause
an
> error.

Good catch, thank you!

I added this case to the unittest data, and fixed the code to handle it.

https://codereview.chromium.org/265793013/diff/20001/gm/rebaseline_server/com...
gm/rebaseline_server/compare_rendered_pictures.py:225: imageB_checksum = None
On 2014/05/06 14:35:48, borenet wrote:
> An inline function might be nice to remove this duplication:
> 
> def _checksum_and_relative_url(d):
>   if d:
>     return (d[gm_json.JSONKEY_IMAGE_FILEPATH],
>             (d[gm_json.JSONKEY_IMAGE_CHECKSUMALGORITHM],
>              d[gm_json.JSONKEY_IMAGE_CHECKSUMVALUE]))
>   else:
>     return None, None
> 
> imageA_relative_url, imageA_checksum =
_checksum_and_relative_url(image_dict_A)
> imageB_relative_url, imageB_checksum =
_checksum_and_relative_url(image_dict_B)

Good idea, done!

[borenet, 2014-05-06 15:26:55]

LGTM at patch set 3.

https://codereview.chromium.org/265793013/diff/20001/gm/gm_json.py
File gm/gm_json.py (right):

https://codereview.chromium.org/265793013/diff/20001/gm/gm_json.py#newcode26
gm/gm_json.py:26: # gm_expectations.cpp and tools/PictureRenderer.cpp !
On 2014/05/06 15:13:53, epoger wrote:
> On 2014/05/06 14:35:48, borenet wrote:
> > This is scary!
> 
> Agreed.
> 
> The good news is that in another CL I am moving these constants from
> tools/PictureRenderer.cpp to tools/image_expectations.cpp , which will
> eventually merge with gm/gm_expectations.cpp .
> 
> We'll still need to keep them in sync between Python and C++ (unless you maybe
> have a clever idea to get around that; I'm sure something is POSSIBLE)

Off the top of my head - gm/gm_expectations_constants.h containing ONLY variable
declarations so as to be readable by both gm/gm_expectations.cpp and Python?

[epoger, 2014-05-06 15:31:06]

The CQ bit was checked by epoger@google.com

[commit-bot: I haz the power, 2014-05-06 15:31:15]

CQ is trying da patch. Follow status at
 https://skia-tree-status.appspot.com/cq/epoger@google.com/265793013/80001

[commit-bot: I haz the power, 2014-05-06 15:32:16]

Change committed as 14591