Modify skpdiff to write diffs directly to provided directories

gm/rebaseline_server/imagediffdb.py

 #!/usr/bin/python
 
 """
 Copyright 2013 Google Inc.
 
 Use of this source code is governed by a BSD-style license that can be
 found in the LICENSE file.
 
 Calulate differences between image pairs, and store them in a database.
 """
@@ skipping 107 matching lines @@
     # Return early if we do not need to generate diffs.
     if (expected_image_url == actual_image_url or
         not expected_image_url or not actual_image_url):
       return
 
     # Get all diff images and values using the skpdiff binary.
     skpdiff_output_dir = tempfile.mkdtemp()
     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.
       # Pro: we'll incur less overhead from making repeated system calls,
       # 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,
            '--whiteDiffDir', skpdiff_whitediff_dir,
            ])
 
       # Get information out of the skpdiff_summary_file.
       with contextlib.closing(open(skpdiff_summary_file)) as fp:
         data = json.load(fp)
 
       # For now, we can assume there is only one record in the output summary,
       # since we passed skpdiff only one pair of images.
       record = data['records'][0]
       self._width = record['width']
       self._height = record['height']
       # TODO: make max_diff_per_channel a tuple instead of a list, because the
       # structure is meaningful (first element is red, second is green, etc.)
       # 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']
         if differ_name == 'different_pixels':
           self._num_pixels_differing = stats['pointsOfInterest']
         elif differ_name == 'perceptual':
           perceptual_similarity = stats['result']
 
       # skpdiff returns the perceptual similarity; convert it to get the
       # perceptual difference percentage.
       # skpdiff outputs -1 if the images are different sizes. Treat any
       # output that does not lie in [0, 1] as having 0% perceptual
       # similarity.
       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)
 
   # TODO(epoger): Use properties instead of getters throughout.
   # See http://stackoverflow.com/a/6618176
   def get_num_pixels_differing(self):
     """Returns the absolute number of pixels that differ."""
     return self._num_pixels_differing
 
   def get_percent_pixels_differing(self):
@@ skipping 268 matching lines @@
 
   Args:
     locator: string, or something that can be represented as a string.
         If None or '', it is returned without modification, because empty
         locators have a particular meaning ("there is no image for this")
   """
   if locator:
     return DISALLOWED_FILEPATH_CHAR_REGEX.sub('_', str(locator))
   else:
     return locator
-
-def _get_difference_locator(expected_image_locator, actual_image_locator):
-  """Returns the locator string used to look up the diffs between expected_image
-  and actual_image.
-
-  We must keep this function in sync with getImageDiffRelativeUrl() in
-  static/loader.js
-
-  Args:
-    expected_image_locator: locator string pointing at expected image
-    actual_image_locator: locator string pointing at actual image
-
-  Returns: already-sanitized locator where the diffs between expected and
-      actual images can be found
-  """
-  return "%s-vs-%s" % (_sanitize_locator(expected_image_locator),
-                       _sanitize_locator(actual_image_locator))