rebaseline_server: use just skpdiff, not Python Image Library

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 6b684143b03f256a6a4eee87d43447eabfe713f8..985017af1c28b862525957588df4e8482ca10541 100644
--- a/gm/rebaseline_server/imagediffdb.py
+++ b/gm/rebaseline_server/imagediffdb.py
@@ -10,7 +10,7 @@ Calulate differences between image pairs, and store them in a database.
 """
 
 import contextlib
-import csv
+import json
 import logging
 import os
 import re
@@ -18,11 +18,6 @@ import shutil
 import sys
 import tempfile
 import urllib
-try:
-  from PIL import Image, ImageChops
-except ImportError:
-  raise ImportError('Requires PIL to be installed; see '
-                    + 'http://www.pythonware.com/products/pil/')
 
 # Set the PYTHONPATH to include the tools directory.
 sys.path.append(
@@ -38,11 +33,9 @@ DEFAULT_IMAGES_SUBDIR = 'images'
 
 DISALLOWED_FILEPATH_CHAR_REGEX = re.compile('[^\w\-]')
 
-DIFFS_SUBDIR = 'diffs'
+RGBDIFFS_SUBDIR = 'diffs'
 WHITEDIFFS_SUBDIR = 'whitediffs'
 
-VALUES_PER_BAND = 256
-
 # Keys used within DiffRecord dictionary representations.
 # NOTE: Keep these in sync with static/constants.js
 KEY__DIFFERENCES__MAX_DIFF_PER_CHANNEL = 'maxDiffPerChannel'
@@ -87,9 +80,8 @@ class DiffRecord(object):
     actual_image_locator = _sanitize_locator(actual_image_locator)
 
     # Download the expected/actual images, if we don't have them already.
-    # TODO(rmistry): Add a parameter that makes _download_and_open_image raise
-    # an exception if images are not found locally (instead of trying to
-    # download them).
+    # TODO(rmistry): Add a parameter that just tries to use already-present
+    # image files rather than downloading them.
     expected_image_file = os.path.join(
         storage_root, expected_images_subdir,
         str(expected_image_locator) + image_suffix)
@@ -97,80 +89,91 @@ class DiffRecord(object):
         storage_root, actual_images_subdir,
         str(actual_image_locator) + image_suffix)
     try:
-      expected_image = _download_and_open_image(
-          expected_image_file, expected_image_url)
+      _download_file(expected_image_file, expected_image_url)
     except Exception:
       logging.exception('unable to download expected_image_url %s to file %s' %
                         (expected_image_url, expected_image_file))
       raise
     try:
-      actual_image = _download_and_open_image(
-          actual_image_file, actual_image_url)
+      _download_file(actual_image_file, actual_image_url)
     except Exception:
       logging.exception('unable to download actual_image_url %s to file %s' %
                         (actual_image_url, actual_image_file))
       raise
 
-    # Generate the diff image (absolute diff at each pixel) and
-    # max_diff_per_channel.
-    diff_image = _generate_image_diff(actual_image, expected_image)
-    diff_histogram = diff_image.histogram()
-    (diff_width, diff_height) = diff_image.size
-    self._max_diff_per_channel = _max_per_band(diff_histogram)
-
-    # Generate the whitediff image (any differing pixels show as white).
-    # This is tricky, because when you convert color images to grayscale or
-    # black & white in PIL, it has its own ideas about thresholds.
-    # We have to force it: if a pixel has any color at all, it's a '1'.
-    bands = diff_image.split()
-    graydiff_image = ImageChops.lighter(ImageChops.lighter(
-        bands[0], bands[1]), bands[2])
-    whitediff_image = (graydiff_image.point(lambda p: p > 0 and VALUES_PER_BAND)
-                                     .convert('1', dither=Image.NONE))
-
-    # Calculate the perceptual difference percentage.
-    skpdiff_csv_dir = tempfile.mkdtemp()
+    # Get all diff images and values from skpdiff binary.
+    skpdiff_output_dir = tempfile.mkdtemp()
     try:
-      skpdiff_csv_output = os.path.join(skpdiff_csv_dir, 'skpdiff-output.csv')
+      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')
       expected_img = os.path.join(storage_root, expected_images_subdir,
                                   str(expected_image_locator) + image_suffix)
       actual_img = os.path.join(storage_root, actual_images_subdir,
                                 str(actual_image_locator) + image_suffix)
+
+      # TODO: Call skpdiff ONCE for all image pairs, instead of calling it
+      # repeatedly.  This will allow us to parallelize a lot more work.
       find_run_binary.run_command(
           [SKPDIFF_BINARY, '-p', expected_img, actual_img,
-           '--csv', skpdiff_csv_output, '-d', 'perceptual'])
-      with contextlib.closing(open(skpdiff_csv_output)) as csv_file:
-        for row in csv.DictReader(csv_file):
-          perceptual_similarity = float(row[' perceptual'].strip())
-          if not 0 <= perceptual_similarity <= 1:
-            # skpdiff outputs -1 if the images are different sizes. Treat any
-            # output that does not lie in [0, 1] as having 0% perceptual
-            # similarity.
-            perceptual_similarity = 0
-          # skpdiff returns the perceptual similarity, convert it to get the
-          # perceptual difference percentage.
-          self._perceptual_difference = 100 - (perceptual_similarity * 100)
+           '--jsonp', 'false',
+           '--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_csv_dir)
-
-    # Final touches on diff_image: use whitediff_image as an alpha mask.
-    # Unchanged pixels are transparent; differing pixels are opaque.
-    diff_image.putalpha(whitediff_image)
-
-    # Store the diff 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
-    _save_image(diff_image, os.path.join(
-        storage_root, DIFFS_SUBDIR, basename))
-    _save_image(whitediff_image, os.path.join(
-        storage_root, WHITEDIFFS_SUBDIR, basename))
-
-    # Calculate difference metrics.
-    (self._width, self._height) = diff_image.size
-    self._num_pixels_differing = (
-        whitediff_image.histogram()[VALUES_PER_BAND - 1])
+      shutil.rmtree(skpdiff_output_dir)
 
   def get_num_pixels_differing(self):
     """Returns the absolute number of pixels that differ."""
@@ -278,102 +281,18 @@ class ImageDiffDB(object):
 
 # Utility functions
 
-def _max_per_band(histogram):
-  """Given the histogram of an image, return the maximum value of each band
-  (a.k.a. "color channel", such as R/G/B) across the entire image.
-
-  Args:
-    histogram: PIL histogram
-
-  Returns the maximum value of each band within the image histogram, as a list.
-  """
-  max_per_band = []
-  assert(len(histogram) % VALUES_PER_BAND == 0)
-  num_bands = len(histogram) / VALUES_PER_BAND
-  for band in xrange(num_bands):
-    # Assuming that VALUES_PER_BAND is 256...
-    #  the 'R' band makes up indices 0-255 in the histogram,
-    #  the 'G' band makes up indices 256-511 in the histogram,
-    #  etc.
-    min_index = band * VALUES_PER_BAND
-    index = min_index + VALUES_PER_BAND
-    while index > min_index:
-      index -= 1
-      if histogram[index] > 0:
-        max_per_band.append(index - min_index)
-        break
-  return max_per_band
-
-
-def _generate_image_diff(image1, image2):
-  """Wrapper for ImageChops.difference(image1, image2) that will handle some
-  errors automatically, or at least yield more useful error messages.
-
-  TODO(epoger): Currently, some of the images generated by the bots are RGBA
-  and others are RGB.  I'm not sure why that is.  For now, to avoid confusion
-  within the UI, convert all to RGB when diffing.
-
-  Args:
-    image1: a PIL image object
-    image2: a PIL image object
-
-  Returns: per-pixel diffs between image1 and image2, as a PIL image object
-  """
-  try:
-    return ImageChops.difference(image1.convert('RGB'), image2.convert('RGB'))
-  except ValueError:
-    logging.error('Error diffing image1 [%s] and image2 [%s].' % (
-        repr(image1), repr(image2)))
-    raise
-
-
-def _download_and_open_image(local_filepath, url):
-  """Open the image at local_filepath; if there is no file at that path,
-  download it from url to that path and then open it.
+def _download_file(local_filepath, url):
+  """Download a file from url to local_filepath, unless it is already there.
 
   Args:
     local_filepath: path on local disk where the image should be stored
     url: URL from which we can download the image if we don't have it yet
-
-  Returns: a PIL image object
   """
   if not os.path.exists(local_filepath):
     _mkdir_unless_exists(os.path.dirname(local_filepath))
     with contextlib.closing(urllib.urlopen(url)) as url_handle:
       with open(local_filepath, 'wb') as file_handle:
         shutil.copyfileobj(fsrc=url_handle, fdst=file_handle)
-  return _open_image(local_filepath)
-
-
-def _open_image(filepath):
-  """Wrapper for Image.open(filepath) that yields more useful error messages.
-
-  Args:
-    filepath: path on local disk to load image from
-
-  Returns: a PIL image object
-  """
-  try:
-    return Image.open(filepath)
-  except IOError:
-    # If we are unable to load an image from the file, delete it from disk
-    # and we will try to fetch it again next time.  Fixes http://skbug.com/2247
-    logging.error('IOError loading image file %s ; deleting it.' % filepath)
-    os.remove(filepath)
-    raise
-
-
-def _save_image(image, filepath, format='PNG'):
-  """Write an image to disk, creating any intermediate directories as needed.
-
-  Args:
-    image: a PIL image object
-    filepath: path on local disk to write image to
-    format: one of the PIL image formats, listed at
-            http://effbot.org/imagingbook/formats.htm
-  """
-  _mkdir_unless_exists(os.path.dirname(filepath))
-  image.save(filepath, format)
 
 
 def _mkdir_unless_exists(path):