teach rebaseline_server how to compare results of multiple render_pictures runs

teach rebaseline_server how to compare results of multiple render_pictures runs

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

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

[epoger, 2014-04-03 21:52:53]

Please review at patchset 1.

[epoger, 2014-04-03 21:58:42]

On 2014/04/03 21:52:53, epoger wrote:
> Please review at patchset 1.

The Housekeeper trybot failed because of ApplyPatch problem with binary image
files, same as Leon saw earlier today.

[epoger, 2014-04-03 22:03:40]

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

https://codereview.chromium.org/216103004/diff/20001/gm/rebaseline_server/com...
gm/rebaseline_server/compare_rendered_pictures.py:135: # The image name may
contain funny characters or be ridiculously long
Actually, rather than sanitizing/truncating the image name here, I think I will
probably do it in render_pictures instead.

Reason: we will need to be consistent in applying this rule, so that the process
which uploads the files to GS using these paths will match the paths created by
downstream processes.

In a coming CL, I will make render_pictures write out images to paths that are
"ready to upload" to Google Storage, like the ones that come out of gm.

Anyway, this shouldn't block review of this CL.

[rmistry, 2014-04-04 14:22:20]

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

https://codereview.chromium.org/216103004/diff/20001/gm/rebaseline_server/com...
gm/rebaseline_server/compare_rendered_pictures.py:47: BASE_IMAGE_URL =
'http://chromium-skia-gm.commondatastorage.googleapis.com/render_pictures/images'
Should this be the default value for a parameter in the constructor?

https://codereview.chromium.org/216103004/diff/20001/gm/rebaseline_server/com...
gm/rebaseline_server/compare_rendered_pictures.py:47: BASE_IMAGE_URL =
'http://chromium-skia-gm.commondatastorage.googleapis.com/render_pictures/images'
What do you think about adding this link to global variables and reading it from
there?

https://codereview.chromium.org/216103004/diff/20001/gm/rebaseline_server/com...
gm/rebaseline_server/compare_rendered_pictures.py:135: # The image name may
contain funny characters or be ridiculously long
On 2014/04/03 22:03:41, epoger wrote:
> Actually, rather than sanitizing/truncating the image name here, I think I
will
> probably do it in render_pictures instead.
> 
> Reason: we will need to be consistent in applying this rule, so that the
process
> which uploads the files to GS using these paths will match the paths created
by
> downstream processes.
> 
> In a coming CL, I will make render_pictures write out images to paths that are
> "ready to upload" to Google Storage, like the ones that come out of gm.
> 
> Anyway, this shouldn't block review of this CL.

Should we add a TODO here to keep track of this (making render_pictures write
out the right images)?

[epoger, 2014-04-04 15:31:53]

Thanks... please see patchset 3.

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

https://codereview.chromium.org/216103004/diff/20001/gm/rebaseline_server/com...
gm/rebaseline_server/compare_rendered_pictures.py:47: BASE_IMAGE_URL =
'http://chromium-skia-gm.commondatastorage.googleapis.com/render_pictures/images'
On 2014/04/04 14:22:20, rmistry wrote:
> Should this be the default value for a parameter in the constructor?

Done.

https://codereview.chromium.org/216103004/diff/20001/gm/rebaseline_server/com...
gm/rebaseline_server/compare_rendered_pictures.py:47: BASE_IMAGE_URL =
'http://chromium-skia-gm.commondatastorage.googleapis.com/render_pictures/images'
On 2014/04/04 14:22:20, rmistry wrote:
> What do you think about adding this link to global variables and reading it
from
> there?

Good idea.  Added as a TODO for now.

https://codereview.chromium.org/216103004/diff/20001/gm/rebaseline_server/com...
gm/rebaseline_server/compare_rendered_pictures.py:135: # The image name may
contain funny characters or be ridiculously long
On 2014/04/04 14:22:20, rmistry wrote:
> On 2014/04/03 22:03:41, epoger wrote:
> > Actually, rather than sanitizing/truncating the image name here, I think I
> will
> > probably do it in render_pictures instead.
> > 
> > Reason: we will need to be consistent in applying this rule, so that the
> process
> > which uploads the files to GS using these paths will match the paths created
> by
> > downstream processes.
> > 
> > In a coming CL, I will make render_pictures write out images to paths that
are
> > "ready to upload" to Google Storage, like the ones that come out of gm.
> > 
> > Anyway, this shouldn't block review of this CL.
> 
> Should we add a TODO here to keep track of this (making render_pictures write
> out the right images)?

Done.

https://codereview.chromium.org/216103004/diff/40001/gm/rebaseline_server/com...
File gm/rebaseline_server/compare_rendered_pictures_test.py (right):

https://codereview.chromium.org/216103004/diff/40001/gm/rebaseline_server/com...
gm/rebaseline_server/compare_rendered_pictures_test.py:35: # TODO(epoger):
Specify image_base_url pointing at the directory on local
Added a TODO here, after noticing that the unittest isn't actually loading the
test images.  It just notes which ones are different and which ones are the
same, based on checksum-based filenames.

[rmistry, 2014-04-04 16:14:56]

LGTM

[epoger, 2014-04-04 16:39:54]

The CQ bit was checked by epoger@google.com

[commit-bot: I haz the power, 2014-04-04 16:39:57]

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

[commit-bot: I haz the power, 2014-04-04 16:40:34]

Change committed as 14062