make CT skia_try_server work with updated render_pictures JSON

compute_engine_scripts/telemetry/telemetry_slave_scripts/write_json_summary.py

Index: compute_engine_scripts/telemetry/telemetry_slave_scripts/write_json_summary.py
diff --git a/compute_engine_scripts/telemetry/telemetry_slave_scripts/write_json_summary.py b/compute_engine_scripts/telemetry/telemetry_slave_scripts/write_json_summary.py
index 23a75adf8ab11dc10dbe52866a53545581ca287e..a4861a6012a7cb3546da3825137f8895871c784f 100644
--- a/compute_engine_scripts/telemetry/telemetry_slave_scripts/write_json_summary.py
+++ b/compute_engine_scripts/telemetry/telemetry_slave_scripts/write_json_summary.py
@@ -9,6 +9,7 @@ import json
 import optparse
 import os
 import posixpath
+import re
 import sys
 import traceback
 
@@ -16,6 +17,12 @@ sys.path.append(
     os.path.join(os.path.dirname(os.path.realpath(__file__)), os.pardir))
 import json_summary_constants
 
+# TODO(epoger): These constants must be kept in sync with the ones in
+# https://skia.googlesource.com/skia/+/master/tools/PictureRenderer.cpp
+JSONKEY_IMAGE_CHECKSUMALGORITHM = 'checksumAlgorithm'
+JSONKEY_IMAGE_CHECKSUMVALUE = 'checksumValue'
+JSONKEY_IMAGE_FILEPATH = 'filepath'
+
 
 def WriteJsonSummary(img_root, nopatch_json, nopatch_images_base_url,
                      withpatch_json, withpatch_images_base_url,
@@ -58,14 +65,16 @@ def WriteJsonSummary(img_root, nopatch_json, nopatch_images_base_url,
     traceback.print_exc()
     raise Exception('You need to add gm/ and gm/rebaseline_server to sys.path')
 
-  files_to_checksums_nopatch = GetFilesAndChecksums(gm_json, nopatch_json)
-  files_to_checksums_withpatch = GetFilesAndChecksums(gm_json, withpatch_json)
+  all_image_descriptions_nopatch = GetImageDescriptions(gm_json, nopatch_json)
+  all_image_descriptions_withpatch = GetImageDescriptions(
+      gm_json, withpatch_json)
 
-  assert len(files_to_checksums_nopatch) == len(files_to_checksums_withpatch), (
-      'Number of images in both JSON summary files are different')
-  assert files_to_checksums_nopatch.keys() == \
-         files_to_checksums_withpatch.keys(), (
-             'File names in both JSON summary files are different')
+  assert (len(all_image_descriptions_nopatch) ==
+          len(all_image_descriptions_withpatch)), \
+          'Number of images in the two JSON summary files are different'
+  assert (all_image_descriptions_nopatch.keys() ==
+          all_image_descriptions_withpatch.keys()), \
+          'SKP filenames in the two JSON summary files are different'
 
   # Compare checksums in both directories and output differences.
   file_differences = []
@@ -86,29 +95,41 @@ def WriteJsonSummary(img_root, nopatch_json, nopatch_images_base_url,
   }
 
   image_diff_db = imagediffdb.ImageDiffDB(storage_root=img_root)
-  for filename in files_to_checksums_nopatch:
-    algo_nopatch, checksum_nopatch = files_to_checksums_nopatch[filename]
-    algo_withpatch, checksum_withpatch = files_to_checksums_withpatch[filename]
-    assert algo_nopatch == algo_withpatch, 'Different checksum algorithms found'
+  for skp_file_and_tile in all_image_descriptions_nopatch:
+    image_desc_nopatch = all_image_descriptions_nopatch[skp_file_and_tile]
+    image_desc_withpatch = all_image_descriptions_withpatch[skp_file_and_tile]
+
+    algo_nopatch = image_desc_nopatch[JSONKEY_IMAGE_CHECKSUMALGORITHM]
+    algo_withpatch = image_desc_withpatch[JSONKEY_IMAGE_CHECKSUMALGORITHM]
+    assert algo_nopatch == algo_withpatch, 'Different checksum algorithms'
+
+    imagefile_nopatch = image_desc_nopatch[JSONKEY_IMAGE_FILEPATH]
+    imagefile_withpatch = image_desc_withpatch[JSONKEY_IMAGE_FILEPATH]
+    assert imagefile_nopatch == imagefile_withpatch, 'Different imagefile names'
+
+    checksum_nopatch = image_desc_nopatch[JSONKEY_IMAGE_CHECKSUMVALUE]
+    checksum_withpatch = image_desc_withpatch[JSONKEY_IMAGE_CHECKSUMVALUE]
     if checksum_nopatch != checksum_withpatch:
       # TODO(epoger): It seems silly that we add this DiffRecord to ImageDiffDB
       # and then pull it out again right away, but this is a stepping-stone
       # to using ImagePairSet instead of replicating its behavior here.
-      image_locator_base = os.path.splitext(filename)[0]
+      image_locator_base = os.path.splitext(imagefile_nopatch)[0]
       image_locator_nopatch = image_locator_base + '_nopatch'
       image_locator_withpatch = image_locator_base + '_withpatch'
       image_diff_db.add_image_pair(
-          expected_image_url=posixpath.join(nopatch_images_base_url, filename),
+          expected_image_url=posixpath.join(
+              nopatch_images_base_url, skp_file_and_tile),
           expected_image_locator=image_locator_nopatch,
-          actual_image_url=posixpath.join(withpatch_images_base_url, filename),
+          actual_image_url=posixpath.join(
+              withpatch_images_base_url, skp_file_and_tile),
           actual_image_locator=image_locator_withpatch)
       diff_record = image_diff_db.get_diff_record(
           expected_image_locator=image_locator_nopatch,
           actual_image_locator=image_locator_withpatch)
       file_differences.append({
-          json_summary_constants.JSONKEY_FILE_NAME: filename,
+          json_summary_constants.JSONKEY_FILE_NAME: imagefile_nopatch,
           json_summary_constants.JSONKEY_SKP_LOCATION: posixpath.join(
-              gs_skp_dir, GetSkpFileName(filename)),
+              gs_skp_dir, GetSkpFileName(skp_file_and_tile)),
           json_summary_constants.JSONKEY_NUM_PIXELS_DIFFERING:
               diff_record.get_num_pixels_differing(),
           json_summary_constants.JSONKEY_PERCENT_PIXELS_DIFFERING:
@@ -127,15 +148,23 @@ def WriteJsonSummary(img_root, nopatch_json, nopatch_images_base_url,
       f.write(json.dumps(json_summary, indent=4, sort_keys=True))
 
 
-def GetSkpFileName(img_file_name):
-  """Determine the SKP file name from the image's file name."""
+def GetSkpFileName(skp_file_and_tile):
+  """Determine the SKP file name from the combined skpFile and tile string."""
   # TODO(rmistry): The below relies too much on the current output of render
   # pictures to determine the root SKP.
-  return '%s_.skp' % '_'.join(img_file_name.split('_')[:-1])
+  return re.sub('-tile\d+$', '', skp_file_and_tile)

[epoger, 2014/04/07 22:08:01]

This still relies on a "magic" conversion algorithm!  I think this means that we
should re-evaluate the new JSON format we settled on in
https://codereview.chromium.org/226293002 , and make it explicit about which SKP
each tile image came from.

Let's discuss live on Tuesday...

 
 
-def GetFilesAndChecksums(gm_json_mod, json_location):
-  """Reads the JSON summary and returns dict of files to checksums."""
+def GetImageDescriptions(gm_json_mod, json_location):
+  """Reads the JSON summary and returns {SkpFileName: ImageDescription} dict.
+
+  Each ImageDescription is a dict of this form:
+  {
+    JSONKEY_IMAGE_CHECKSUMALGORITHM: blah,
+    JSONKEY_IMAGE_CHECKSUMVALUE: blah,
+    JSONKEY_IMAGE_FILEPATH: blah,
+  }
+  """
   data = gm_json_mod.LoadFromFile(json_location)
   if data:
     return data[gm_json_mod.JSONKEY_ACTUALRESULTS][