Modify skpdiff to write diffs directly to provided directories

gm/rebaseline_server/imagediffdb.py

Index: gm/rebaseline_server/imagediffdb.py
diff --git a/gm/rebaseline_server/imagediffdb.py b/gm/rebaseline_server/imagediffdb.py
index 167134aaf4b2a2efb9f41f8de1b2e7601bf60690..82d0cda76ccc67df808dd2abaa60fa310f26a782 100644
--- a/gm/rebaseline_server/imagediffdb.py
+++ b/gm/rebaseline_server/imagediffdb.py
@@ -125,8 +125,10 @@ class DiffRecord(object):
     try:
       skpdiff_summary_file = os.path.join(skpdiff_output_dir,
                                           'skpdiff-output.json')
-      skpdiff_rgbdiff_dir = os.path.join(skpdiff_output_dir, 'rgbDiff')
-      skpdiff_whitediff_dir = os.path.join(skpdiff_output_dir, 'whiteDiff')
+      skpdiff_rgbdiff_dir = os.path.join(storage_root, RGBDIFFS_SUBDIR)
+      skpdiff_whitediff_dir = os.path.join(storage_root, WHITEDIFFS_SUBDIR)
+      _mkdir_unless_exists(skpdiff_rgbdiff_dir)
+      _mkdir_unless_exists(skpdiff_rgbdiff_dir)
 
       # TODO(epoger): Consider calling skpdiff ONCE for all image pairs,
       # instead of calling it separately for each image pair.
@@ -134,9 +136,13 @@ class DiffRecord(object):
       # spinning up the skpdiff binary, etc.
       # Con: we would have to wait until all image pairs were loaded before
       # generating any of the diffs?
+      # Note(stephana): '--longnames' was added to allow for this 
+      # case (multiple files at once) versus specifying output diffs 
+      # directly.
       find_run_binary.run_command(
           [SKPDIFF_BINARY, '-p', expected_image_file, actual_image_file,
            '--jsonp', 'false',
+           '--longnames', 'true',
            '--output', skpdiff_summary_file,
            '--differs', 'perceptual', 'different_pixels',
            '--rgbDiffDir', skpdiff_rgbdiff_dir,
@@ -157,8 +163,6 @@ class DiffRecord(object):
       # See http://stackoverflow.com/a/626871
       self._max_diff_per_channel = [
           record['maxRedDiff'], record['maxGreenDiff'], record['maxBlueDiff']]
-      rgb_diff_path = record['rgbDiffPath']
-      white_diff_path = record['whiteDiffPath']
       per_differ_stats = record['diffs']
       for stats in per_differ_stats:
         differ_name = stats['differName']
@@ -175,25 +179,22 @@ class DiffRecord(object):
       if not 0 <= perceptual_similarity <= 1:
         perceptual_similarity = 0
       self._perceptual_difference = 100 - (perceptual_similarity * 100)
-
-      # Store the rgbdiff and whitediff images generated above.
-      diff_image_locator = _get_difference_locator(
-          expected_image_locator=expected_image_locator,
-          actual_image_locator=actual_image_locator)
-      basename = str(diff_image_locator) + image_suffix
-      _mkdir_unless_exists(os.path.join(storage_root, RGBDIFFS_SUBDIR))
-      _mkdir_unless_exists(os.path.join(storage_root, WHITEDIFFS_SUBDIR))
-      # TODO: Modify skpdiff's behavior so we can tell it exactly where to
-      # write the image files into, rather than having to move them around
-      # after skpdiff writes them out.
-      shutil.copyfile(rgb_diff_path,
-                      os.path.join(storage_root, RGBDIFFS_SUBDIR, basename))
-      shutil.copyfile(white_diff_path,
-                      os.path.join(storage_root, WHITEDIFFS_SUBDIR, basename))
-
     finally:
       shutil.rmtree(skpdiff_output_dir)
 
+
+
+  @staticmethod 
+  def get_diff_records(gs, storage_root,
+                       images_to_compare, 
+                       expected_images_subdir=DEFAULT_IMAGES_SUBDIR,
+                       actual_images_subdir=DEFAULT_IMAGES_SUBDIR,
+                       image_suffix=DEFAULT_IMAGE_SUFFIX):
+    # TODO: stub method to be filled in 

[epoger, 2014/08/12 18:03:40]

Do you intend to fill in this method before committing the CL?

[stephana, 2014/08/12 19:25:13]

No. That's an artifact. Will remove it. 

On 2014/08/12 18:03:40, epoger wrote:

+    diff_records = []
+    return diff_records
+
+  
   # TODO(epoger): Use properties instead of getters throughout.
   # See http://stackoverflow.com/a/6618176
   def get_num_pixels_differing(self):