vm_run_skia_try/write_json_summary: run skpdiff within imagediffdb, not on its own

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 50c44b20e1eb20633a96447d4a0f83d4c8cc70e9..5e88b206ae138939c3f4694bb003b57d73768ec9 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
@@ -18,22 +18,22 @@ sys.path.append(
 import json_summary_constants
 
 
-def WriteJsonSummary(img_root, nopatch_json, nopatch_img_dir_name,
-                     withpatch_json, withpatch_img_dir_name, output_file_path,
-                     gs_output_dir, gs_skp_dir, slave_num, gm_json_path,
-                     imagediffdb_path, skpdiff_output_csv):
+def WriteJsonSummary(img_root, nopatch_json, nopatch_images_base_url,
+                     withpatch_json, withpatch_images_base_url,
+                     output_file_path, gs_output_dir, gs_skp_dir, slave_num,
+                     gm_json_path, imagediffdb_path):
   """Outputs the JSON summary of image comparisions.
 
   Args:
     img_root: (str) The root directory on local disk where we store all images.
     nopatch_json: (str) Location of the nopatch render_pictures JSON summary
         file.
-    nopatch_img_dir_name: (str) Name of the directory within img_root that
-        contains all nopatch images.
+    nopatch_images_base_url: (str) URL of directory containing all nopatch

[epoger, 2014/03/03 21:51:05]

This changes because ImageDiffDB expects a URL from which it can download source
images.  A file:// URL is OK.

[rmistry, 2014/03/04 13:14:19]

Acknowledged.

+        images.
     withpatch_json: (str) Location of the withpatch render_pictures JSON summary
         file.
-    withpatch_img_dir_name: (str) Name of the directory within img_root that
-        contains all withpatch images.
+    withpatch_images_base_url: (str) URL of directory containing all withpatch
+        images.
     output_file_path: (str) The local path to the JSON file that will be
         created by this function which will contain a summary of all file
         differences for this slave.
@@ -45,7 +45,6 @@ def WriteJsonSummary(img_root, nopatch_json, nopatch_img_dir_name,
         this script.
     gm_json_path: (str) Local complete path to gm_json.py in Skia trunk.
     imagediffdb_path: (str) Local complete path to imagediffdb.py in Skia trunk.
-    skpdiff_output_csv: (str) Local complete path to the CSV output of skpdiff.
   """
 
   assert os.path.isfile(gm_json_path), 'Must specify a valid path to gm_json.py'
@@ -54,13 +53,15 @@ def WriteJsonSummary(img_root, nopatch_json, nopatch_img_dir_name,
       'Must specify a valid path to imagediffdb.py')
   imagediffdb_mod = imp.load_source(imagediffdb_path, imagediffdb_path)
 
-  files_to_checksums1 =  GetFilesAndChecksums(nopatch_json, gm_json_mod)
-  files_to_checksums2 =  GetFilesAndChecksums(withpatch_json, gm_json_mod)
+  files_to_checksums_nopatch = GetFilesAndChecksums(nopatch_json, gm_json_mod)
+  files_to_checksums_withpatch = GetFilesAndChecksums(
+      withpatch_json, gm_json_mod)
 
-  assert len(files_to_checksums1) == len(files_to_checksums2), (
+  assert len(files_to_checksums_nopatch) == len(files_to_checksums_withpatch), (
       'Number of images in both JSON summary files are different')
-  assert files_to_checksums1.keys() == files_to_checksums2.keys(), (
-      'File names 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')
 
   # Compare checksums in both directories and output differences.
   file_differences = []
@@ -79,40 +80,38 @@ def WriteJsonSummary(img_root, nopatch_json, nopatch_img_dir_name,
   json_summary = {
       'slave%s' % slave_num: slave_dict
   }
-  # Read the skpdiff CSV output.
-  page_to_perceptual_similarity = {}
-  for row in csv.DictReader(open(skpdiff_output_csv, 'r')):
-    page_to_perceptual_similarity[row['key']] = float(
-        row[' perceptual'].strip())
-
-  for file1 in files_to_checksums1:
-    algo1, checksum1 = files_to_checksums1[file1]
-    algo2, checksum2 = files_to_checksums2[file1]
-    assert algo1 == algo2, 'Different algorithms found'
-    if checksum1 != checksum2:
-      # Call imagediffdb and then the diffs and metrics.
-      image_diff = imagediffdb_mod.DiffRecord(
-          storage_root=img_root,
-          expected_image_url=None,  # Do not need to download any img.
-          expected_image_locator=os.path.splitext(file1)[0],
-          actual_image_url=None,  # Do not need to download any img.
-          actual_image_locator=os.path.splitext(file1)[0],
-          expected_images_subdir=nopatch_img_dir_name,
-          actual_images_subdir=withpatch_img_dir_name)
+
+  image_diff_db = imagediffdb_mod.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'
+    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_diff_db.add_image_pair(
+          expected_image_url=posixpath.join(nopatch_images_base_url, filename),
+          expected_image_locator=checksum_nopatch,
+          actual_image_url=posixpath.join(withpatch_images_base_url, filename),
+          actual_image_locator=checksum_withpatch)
+      diff_record = image_diff_db.get_diff_record(
+          expected_image_locator=checksum_nopatch,
+          actual_image_locator=checksum_withpatch)
       file_differences.append({
-          json_summary_constants.JSONKEY_FILE_NAME: file1,
+          json_summary_constants.JSONKEY_FILE_NAME: filename,
           json_summary_constants.JSONKEY_SKP_LOCATION: posixpath.join(
-              gs_skp_dir, GetSkpFileName(file1)),
+              gs_skp_dir, GetSkpFileName(filename)),
           json_summary_constants.JSONKEY_NUM_PIXELS_DIFFERING:
-              image_diff.get_num_pixels_differing(),
+              diff_record.get_num_pixels_differing(),
           json_summary_constants.JSONKEY_PERCENT_PIXELS_DIFFERING:
-              image_diff.get_percent_pixels_differing(),
+              diff_record.get_percent_pixels_differing(),
           json_summary_constants.JSONKEY_WEIGHTED_DIFF_MEASURE:
-              image_diff.get_weighted_diff_measure(),
+              diff_record.get_weighted_diff_measure(),
           json_summary_constants.JSONKEY_MAX_DIFF_PER_CHANNEL:
-              image_diff.get_max_diff_per_channel(),
+              diff_record.get_max_diff_per_channel(),
           json_summary_constants.JSONKEY_PERCEPTUAL_SIMILARITY:
-              page_to_perceptual_similarity[file1],
+              diff_record.get_perceptual_difference(),
       })
   if file_differences:
     slave_dict[json_summary_constants.JSONKEY_FAILED_FILES_COUNT] = len(
@@ -147,16 +146,14 @@ if '__main__' == __name__:
       '', '--nopatch_json',
       help='Location of the nopatch render_pictures JSON summary file.')
   option_parser.add_option(
-      '', '--nopatch_img_dir_name',
-      help='Name of the directory within img_root that contains all nopatch '
-           'images.')
+      '', '--nopatch_images_base_url',
+      help='URL of directory containing all nopatch images.')
   option_parser.add_option(
       '', '--withpatch_json',
       help='Location of the withpatch render_pictures JSON summary file.')
   option_parser.add_option(
-      '', '--withpatch_img_dir_name',
-      help='Name of the directory within img_root that contains all withpatch '
-           'images.')
+      '', '--withpatch_images_base_url',
+      help='URL of directory containing all withpatch images.')
   option_parser.add_option(
       '', '--output_file_path',
       help='The local path to the JSON file that will be created by this '
@@ -180,25 +177,22 @@ if '__main__' == __name__:
   option_parser.add_option(
       '', '--imagediffdb_path',
       help='Local complete path to imagediffdb.py in Skia trunk.')
-  option_parser.add_option(
-      '', '--skpdiff_output_csv',
-      help='Local complete path to the CSV output of skpdiff.')
   options, unused_args = option_parser.parse_args()
   if (not options.nopatch_json or not options.withpatch_json
       or not options.output_file_path or not options.gs_output_dir
       or not options.gs_skp_dir or not options.slave_num
       or not options.gm_json_path or not options.img_root
-      or not options.nopatch_img_dir_name or not options.withpatch_img_dir_name
-      or not options.imagediffdb_path or not options.skpdiff_output_csv):
+      or not options.nopatch_images_base_url
+      or not options.withpatch_images_base_url
+      or not options.imagediffdb_path):
     option_parser.error(
-        'Must specify img_root, nopatch_json, nopatch_img_dir_name, '
-        'withpatch_json, withpatch_img_dir_name, output_file_path, '
+        'Must specify img_root, nopatch_json, nopatch_images_base_url, '
+        'withpatch_json, withpatch_images_base_url, output_file_path, '
         'gs_output_dir, gs_skp_dir, slave_num, gm_json_path, '
-        'imagediffdb_path and skpdiff_output_csv.')
+        'and imagediffdb_path.')
 
   WriteJsonSummary(options.img_root, options.nopatch_json,
-                   options.nopatch_img_dir_name, options.withpatch_json,
-                   options.withpatch_img_dir_name, options.output_file_path,
+                   options.nopatch_images_base_url, options.withpatch_json,
+                   options.withpatch_images_base_url, options.output_file_path,
                    options.gs_output_dir, options.gs_skp_dir, options.slave_num,
-                   options.gm_json_path, options.imagediffdb_path,
-                   options.skpdiff_output_csv)
+                   options.gm_json_path, options.imagediffdb_path)