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

gm/rebaseline_server/compare_rendered_pictures.py

 #!/usr/bin/python
 
 """
 Copyright 2014 Google Inc.
 
 Use of this source code is governed by a BSD-style license that can be
 found in the LICENSE file.
 
 Compare results of two render_pictures runs.
 
 TODO(epoger): Start using this module to compare ALL images (whether they
 were generated from GMs or SKPs), and rename it accordingly.
 """
 
 # System-level imports
 import logging
 import os
 import shutil
 import subprocess
 import tempfile
 import time
 
 # Must fix up PYTHONPATH before importing from within Skia
-import fix_pythonpath  # pylint: disable=W0611
+import rs_fixpypath  # pylint: disable=W0611
 
 # Imports from within Skia
 from py.utils import git_utils
 from py.utils import gs_utils
 from py.utils import url_utils
 import buildbot_globals
 import column
 import gm_json
 import imagediffdb
 import imagepair
 import imagepairset
 import results
 
 # URL under which all render_pictures images can be found in Google Storage.
 #
 # TODO(epoger): In order to allow live-view of GMs and other images, read this
 # from the input summary files, or allow the caller to set it within the
 # GET_live_results call.
 DEFAULT_IMAGE_BASE_GS_URL = 'gs://' + buildbot_globals.Get('skp_images_bucket')
 
 # Column descriptors, and display preferences for them.
 COLUMN__RESULT_TYPE = results.KEY__EXTRACOLUMNS__RESULT_TYPE
 COLUMN__SOURCE_SKP = 'sourceSkpFile'
 COLUMN__TILED_OR_WHOLE = 'tiledOrWhole'
 COLUMN__TILENUM = 'tilenum'
+COLUMN__BUILDER_A = 'builderA'

[epoger, 2014/08/18 19:56:31]

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.

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

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:

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

SGTM.

+COLUMN__RENDER_MODE_A = 'renderModeA'
+COLUMN__BUILDER_B = 'builderB'
+COLUMN__RENDER_MODE_B = 'renderModeB'
+
 FREEFORM_COLUMN_IDS = [
     COLUMN__SOURCE_SKP,
     COLUMN__TILENUM,
 ]
 ORDERED_COLUMN_IDS = [
     COLUMN__RESULT_TYPE,
     COLUMN__SOURCE_SKP,
     COLUMN__TILED_OR_WHOLE,
     COLUMN__TILENUM,
+    COLUMN__BUILDER_A,
+    COLUMN__RENDER_MODE_A,
+    COLUMN__BUILDER_B,
+    COLUMN__RENDER_MODE_B,
 ]
 
 # A special "repo:" URL type that we use to refer to Skia repo contents.
 # (Useful for comparing against expectations files we store in our repo.)
 REPO_URL_PREFIX = 'repo:'
 REPO_BASEPATH = os.path.abspath(os.path.join(
     os.path.dirname(os.path.abspath(__file__)), os.pardir, os.pardir))
 
 # Which sections within a JSON summary file can contain results.
 ALLOWED_SECTION_NAMES = [
@@ skipping 186 matching lines @@
           (dict_num, num_union_dict_paths, dict_path))
 
       dictA = self.get_default(setA_dicts, None, dict_path)
       self._validate_dict_version(dictA)
       dictA_results = self.get_default(dictA, {}, setA_section)
 
       dictB = self.get_default(setB_dicts, None, dict_path)
       self._validate_dict_version(dictB)
       dictB_results = self.get_default(dictB, {}, setB_section)
 
+      get2 = lambda dic, k: None if not dic else \

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

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.

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

Done.

+            (dic.get(gm_json.JSONKEY_DESCRIPTIONS, {}) or {}).get(k, None)
+      builder_A     = get2(dictA, gm_json.JSONKEY_DESCRIPTIONS_BUILDER)
+      render_mode_A = get2(dictA, gm_json.JSONKEY_DESCRIPTIONS_RENDER_MODE)
+      builder_B     = get2(dictB, gm_json.JSONKEY_DESCRIPTIONS_BUILDER)
+      render_mode_B = get2(dictB, gm_json.JSONKEY_DESCRIPTIONS_RENDER_MODE)
+
       skp_names = sorted(set(dictA_results.keys() + dictB_results.keys()))
       # Just for manual testing... truncate to an arbitrary subset.
       if self.truncate_results:
         skp_names = skp_names[1:3]
       for skp_name in skp_names:
         imagepairs_for_this_skp = []
 
         whole_image_A = self.get_default(
             dictA_results, None,
             skp_name, gm_json.JSONKEY_SOURCE_WHOLEIMAGE)
         whole_image_B = self.get_default(
             dictB_results, None,
             skp_name, gm_json.JSONKEY_SOURCE_WHOLEIMAGE)
         imagepairs_for_this_skp.append(self._create_image_pair(
             image_dict_A=whole_image_A, image_dict_B=whole_image_B,
+            builder_A=builder_A, render_mode_A=render_mode_A,
+            builder_B=builder_B, render_mode_B=render_mode_B,
             source_skp_name=skp_name, tilenum=None))
 
         tiled_images_A = self.get_default(
             dictA_results, [],
             skp_name, gm_json.JSONKEY_SOURCE_TILEDIMAGES)
         tiled_images_B = self.get_default(
             dictB_results, [],
             skp_name, gm_json.JSONKEY_SOURCE_TILEDIMAGES)
         if tiled_images_A or tiled_images_B:
           num_tiles_A = len(tiled_images_A)
           num_tiles_B = len(tiled_images_B)
           num_tiles = max(num_tiles_A, num_tiles_B)
           for tile_num in range(num_tiles):
             imagepairs_for_this_skp.append(self._create_image_pair(
                 image_dict_A=(tiled_images_A[tile_num]
                               if tile_num < num_tiles_A else None),
                 image_dict_B=(tiled_images_B[tile_num]
                               if tile_num < num_tiles_B else None),
+                builder_A=builder_A, render_mode_A=render_mode_A,
+                builder_B=builder_B, render_mode_B=render_mode_B,
                 source_skp_name=skp_name, tilenum=tile_num))
 
         for one_imagepair in imagepairs_for_this_skp:
           if one_imagepair:
             all_image_pairs.add_image_pair(one_imagepair)
             result_type = one_imagepair.extra_columns_dict\
                 [COLUMN__RESULT_TYPE]
             if result_type != results.KEY__RESULT_TYPE__SUCCEEDED:
               failing_image_pairs.add_image_pair(one_imagepair)
 
@@ skipping 25 matching lines @@
     header = result_dict[gm_json.JSONKEY_HEADER]
     header_type = header[gm_json.JSONKEY_HEADER_TYPE]
     if header_type != expected_header_type:
       raise Exception('expected header_type "%s", but got "%s"' % (
           expected_header_type, header_type))
     header_revision = header[gm_json.JSONKEY_HEADER_REVISION]
     if header_revision != expected_header_revision:
       raise Exception('expected header_revision %d, but got %d' % (
           expected_header_revision, header_revision))
 
-  def _create_image_pair(self, image_dict_A, image_dict_B, source_skp_name,
+  def _create_image_pair(self, image_dict_A, image_dict_B, 
+                         builder_A, render_mode_A, 
+                         builder_B, render_mode_B,
+                         source_skp_name,
                          tilenum):
     """Creates an ImagePair object for this pair of images.
 
     Args:
       image_dict_A: dict with JSONKEY_IMAGE_* keys, or None if no image
       image_dict_B: dict with JSONKEY_IMAGE_* keys, or None if no image
       source_skp_name: string; name of the source SKP file

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

please document the new args here

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

Done.

       tilenum: which tile, or None if a wholeimage
 
     Returns:
       An ImagePair object, or None if both image_dict_A and image_dict_B are
       None.
     """
     if (not image_dict_A) and (not image_dict_B):
       return None
 
     def _checksum_and_relative_url(dic):
@@ skipping 14 matching lines @@
     elif not imageB_checksum:
       result_type = results.KEY__RESULT_TYPE__NOCOMPARISON
     elif imageA_checksum == imageB_checksum:
       result_type = results.KEY__RESULT_TYPE__SUCCEEDED
     else:
       result_type = results.KEY__RESULT_TYPE__FAILED
 
     extra_columns_dict = {
         COLUMN__RESULT_TYPE: result_type,
         COLUMN__SOURCE_SKP: source_skp_name,
+        COLUMN__BUILDER_A: builder_A,
+        COLUMN__RENDER_MODE_A: render_mode_A,
+        COLUMN__BUILDER_B: builder_B,
+        COLUMN__RENDER_MODE_B: render_mode_B,
     }
     if tilenum == None:
       extra_columns_dict[COLUMN__TILED_OR_WHOLE] = 'whole'
       extra_columns_dict[COLUMN__TILENUM] = 'N/A'
     else:
       extra_columns_dict[COLUMN__TILED_OR_WHOLE] = 'tiled'
       extra_columns_dict[COLUMN__TILENUM] = str(tilenum)
 
     try:
       return imagepair.ImagePair(
@@ skipping 46 matching lines @@
     if source_dir.lower().startswith(REPO_URL_PREFIX):
       repo_dir = os.path.join(REPO_BASEPATH, source_dir[len(REPO_URL_PREFIX):])
       revision = subprocess.check_output(
           args=[git_utils.GIT, 'rev-parse', 'HEAD'], cwd=repo_dir).strip()
       if assert_if_not and revision != assert_if_not:
         raise Exception('found revision %s that did not match %s' % (
             revision, assert_if_not))
       return revision
     else:
       return None