Added 'renderMode' and 'builder' as columns to the SKP front-end.

This adds renderMode and builder as columns to the front end when looking at SKPs. 

BUG=skia:2833
NOTRY=true

Committed: https://skia.googlesource.com/skia/+/3b5c86c7a2db25f82a8415b6c79d1ac580fc64ae

[stephana, 2014-08-18 19:35:21]

The changes for this turned out to be much smaller than anticipated.

https://codereview.chromium.org/480153002/diff/1/gm/rebaseline_server/compare...
File gm/rebaseline_server/compare_configs.py (right):

https://codereview.chromium.org/480153002/diff/1/gm/rebaseline_server/compare...
gm/rebaseline_server/compare_configs.py:18: import rs_fixpypath  # pylint:
disable=W0611
This is a change in several files. There are some source files in
'gm/rebaseline_server' and 'tools' that have identical names. 
It can be confusing which one is actually imported. This is a first effort to
make that clearer.

[epoger, 2014-08-18 19:56:32]

Nice!  Just two suggested modifications.

https://codereview.chromium.org/480153002/diff/1/gm/rebaseline_server/compare...
File gm/rebaseline_server/compare_rendered_pictures.py (right):

https://codereview.chromium.org/480153002/diff/1/gm/rebaseline_server/compare...
gm/rebaseline_server/compare_rendered_pictures.py:50: COLUMN__BUILDER_A =
'builderA'
Interesting.  One thing that's potentially tricky: we USUALLY want to compare
image pairs that have the same builder (expectations vs. actuals for that same
builder).  But SOMETIMES we want to compare results of builderA against results
of builderB.

I'm not sure what the best way is to handle both cases, from a UI perspective.

https://codereview.chromium.org/480153002/diff/1/gm/rebaseline_server/compare...
gm/rebaseline_server/compare_rendered_pictures.py:274: get2 = lambda dic, k:
None if not dic else \
I think this provides a subset of functionality that you could obtain by calling
self.get_default(), which is defined in results.py.  Maybe call that instead?

If you decide to keep this one, I think it would be better to define it as a
separate function, with descriptive docstring.  But I don't feel strongly about
it.

https://codereview.chromium.org/480153002/diff/1/gm/rebaseline_server/compare...
gm/rebaseline_server/compare_rendered_pictures.py:373: source_skp_name: string;
name of the source SKP file
please document the new args here

[epoger, 2014-08-18 19:57:14]

CCing Ravi, who is sure to follow along while on vacation.

[rmistry, 2014-08-18 19:58:38]

On 2014/08/18 19:57:14, epoger wrote:
> CCing Ravi, who is sure to follow along while on vacation.

Ofcourse he is..

[stephana, 2014-08-18 20:24:53]

https://codereview.chromium.org/480153002/diff/1/gm/rebaseline_server/compare...
File gm/rebaseline_server/compare_rendered_pictures.py (right):

https://codereview.chromium.org/480153002/diff/1/gm/rebaseline_server/compare...
gm/rebaseline_server/compare_rendered_pictures.py:50: COLUMN__BUILDER_A =
'builderA'
IMO, if we are comparing output from two builders that are the same, then
builderA and builderB will be identical in the output. 
In that case we can (and should) account for it on the front-end by adjusting
the UI accordingly.

On 2014/08/18 19:56:31, epoger wrote:
> Interesting.  One thing that's potentially tricky: we USUALLY want to compare
> image pairs that have the same builder (expectations vs. actuals for that same
> builder).  But SOMETIMES we want to compare results of builderA against
results
> of builderB.
> 
> I'm not sure what the best way is to handle both cases, from a UI perspective.

https://codereview.chromium.org/480153002/diff/1/gm/rebaseline_server/compare...
gm/rebaseline_server/compare_rendered_pictures.py:274: get2 = lambda dic, k:
None if not dic else \
On 2014/08/18 19:56:32, epoger wrote:
> I think this provides a subset of functionality that you could obtain by
calling
> self.get_default(), which is defined in results.py.  Maybe call that instead?
> 
> If you decide to keep this one, I think it would be better to define it as a
> separate function, with descriptive docstring.  But I don't feel strongly
about
> it.

Done.

https://codereview.chromium.org/480153002/diff/1/gm/rebaseline_server/compare...
gm/rebaseline_server/compare_rendered_pictures.py:373: source_skp_name: string;
name of the source SKP file
On 2014/08/18 19:56:32, epoger wrote:
> please document the new args here

Done.

[epoger, 2014-08-18 20:34:49]

lgtm

https://codereview.chromium.org/480153002/diff/1/gm/rebaseline_server/compare...
File gm/rebaseline_server/compare_rendered_pictures.py (right):

https://codereview.chromium.org/480153002/diff/1/gm/rebaseline_server/compare...
gm/rebaseline_server/compare_rendered_pictures.py:50: COLUMN__BUILDER_A =
'builderA'
On 2014/08/18 20:24:53, stephana wrote:
> IMO, if we are comparing output from two builders that are the same, then
> builderA and builderB will be identical in the output. 
> In that case we can (and should) account for it on the front-end by adjusting
> the UI accordingly.

SGTM.

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

https://codereview.chromium.org/480153002/diff/20001/gm/rebaseline_server/com...
gm/rebaseline_server/compare_rendered_pictures.py:275: builder_A     =
self.get_default(dictA, None,
Ravi will cite
https://engdoc.corp.google.com/eng/doc/devguide/py/styleguide.shtml?cl=head#s...
and tell you not to add whitespace to line up the tokens in these rows.

I personally disagree with that item in the style guide, but I have been told
that I am not stylish.

[stephana, 2014-08-18 20:36:07]

The CQ bit was checked by stephana@google.com

[commit-bot: I haz the power, 2014-08-18 20:37:09]

CQ is trying da patch. Follow status at
 https://skia-tree-status.appspot.com/cq/stephana@google.com/480153002/20001

[commit-bot: I haz the power, 2014-08-18 20:38:06]

Committed patchset #2 (20001) as 3b5c86c7a2db25f82a8415b6c79d1ac580fc64ae