rebaseline_server: add pixel diffs, and sorting by diff metrics

(SkipBuildbotRuns) R=bsalomon@google.com, jcgregorio@google.com Review URL: https://codereview.chromium.org/59283006 git-svn-id: http://skia.googlecode.com/svn/trunk@12193 2bbb7eff-a529-9590-31e7-b0007b416f81
2013-11-08 16:25:25 +00:00 · 2013-11-08 16:25:25 +00:00 · 9dddf6f6b9
commit 9dddf6f6b9
parent bc25dfc798
6 changed files with 454 additions and 7 deletions
--- a/gm/rebaseline_server/imagediffdb.py
+++ b/gm/rebaseline_server/imagediffdb.py
@ -0,0 +1,271 @@
+#!/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.
+"""
+
+import contextlib
+import logging
+import os
+import shutil
+import urllib
+try:
+  from PIL import Image, ImageChops
+except ImportError:
+  raise ImportError('Requires PIL to be installed; see '
+                    + 'http://www.pythonware.com/products/pil/')
+
+IMAGE_SUFFIX = '.png'
+IMAGE_FORMAT = 'PNG'   # must match one of the PIL image formats, listed at
+                       # http://effbot.org/imagingbook/formats.htm
+
+IMAGES_SUBDIR = 'images'
+DIFFS_SUBDIR = 'diffs'
+WHITEDIFFS_SUBDIR = 'whitediffs'
+
+
+class DiffRecord(object):
+  """ Record of differences between two images. """
+
+  def __init__(self, storage_root,
+               expected_image_url, expected_image_locator,
+               actual_image_url, actual_image_locator):
+    """Download this pair of images (unless we already have them on local disk),
+    and prepare a DiffRecord for them.
+
+    TODO(epoger): Make this asynchronously download images, rather than blocking
+    until the images have been downloaded and processed.
+
+    Args:
+      storage_root: root directory on local disk within which we store all
+          images
+      expected_image_url: file or HTTP url from which we will download the
+          expected image
+      expected_image_locator: a unique ID string under which we will store the
+          expected image within storage_root (probably including a checksum to
+          guarantee uniqueness)
+      actual_image_url: file or HTTP url from which we will download the
+          actual image
+      actual_image_locator: a unique ID string under which we will store the
+          actual image within storage_root (probably including a checksum to
+          guarantee uniqueness)
+    """
+    # Download the expected/actual images, if we don't have them already.
+    expected_image = _download_and_open_image(
+        os.path.join(storage_root, IMAGES_SUBDIR,
+                     str(expected_image_locator) + IMAGE_SUFFIX),
+        expected_image_url)
+    actual_image = _download_and_open_image(
+        os.path.join(storage_root, IMAGES_SUBDIR,
+                     str(actual_image_locator) + IMAGE_SUFFIX),
+        actual_image_url)
+
+    # Store the diff image (absolute diff at each pixel).
+    diff_image = _generate_image_diff(actual_image, expected_image)
+    self._weighted_diff_measure = _calculate_weighted_diff_metric(diff_image)
+    diff_image_locator = _get_difference_locator(
+        expected_image_locator=expected_image_locator,
+        actual_image_locator=actual_image_locator)
+    diff_image_filepath = os.path.join(
+        storage_root, DIFFS_SUBDIR, str(diff_image_locator) + IMAGE_SUFFIX)
+    _mkdir_unless_exists(os.path.join(storage_root, DIFFS_SUBDIR))
+    diff_image.save(diff_image_filepath, IMAGE_FORMAT)
+
+    # Store the whitediff image (any differing pixels show as white).
+    #
+    # TODO(epoger): From http://effbot.org/imagingbook/image.htm , it seems
+    # like we should be able to use im.point(function, mode) to perform both
+    # the point() and convert('1') operations simultaneously, but I couldn't
+    # get it to work.
+    whitediff_image = (diff_image.point(lambda p: (0, 256)[p!=0])
+                                 .convert('1'))
+    whitediff_image_filepath = os.path.join(
+        storage_root, WHITEDIFFS_SUBDIR, str(diff_image_locator) + IMAGE_SUFFIX)
+    _mkdir_unless_exists(os.path.join(storage_root, WHITEDIFFS_SUBDIR))
+    whitediff_image.save(whitediff_image_filepath, IMAGE_FORMAT)
+
+    # Calculate difference metrics.
+    (self._width, self._height) = diff_image.size
+    self._num_pixels_differing = whitediff_image.histogram()[255]
+
+  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):
+    """Returns the percentage of pixels that differ, as a float between
+    0 and 100 (inclusive)."""
+    return ((float(self._num_pixels_differing) * 100) /
+            (self._width * self._height))
+
+  def get_weighted_diff_measure(self):
+    """Returns a weighted measure of image diffs, as a float between 0 and 100
+    (inclusive)."""
+    return self._weighted_diff_measure
+
+
+class ImageDiffDB(object):
+  """ Calculates differences between image pairs, maintaining a database of
+  them for download."""
+
+  def __init__(self, storage_root):
+    """
+    Args:
+      storage_root: string; root path within the DB will store all of its stuff
+    """
+    self._storage_root = storage_root
+
+    # Dictionary of DiffRecords, keyed by (expected_image_locator,
+    # actual_image_locator) tuples.
+    self._diff_dict = {}
+
+  def add_image_pair(self,
+                     expected_image_url, expected_image_locator,
+                     actual_image_url, actual_image_locator):
+    """Download this pair of images (unless we already have them on local disk),
+    and prepare a DiffRecord for them.
+
+    TODO(epoger): Make this asynchronously download images, rather than blocking
+    until the images have been downloaded and processed.
+    When we do that, we should probably add a new method that will block
+    until all of the images have been downloaded and processed.  Otherwise,
+    we won't know when it's safe to start calling get_diff_record().
+    jcgregorio notes: maybe just make ImageDiffDB thread-safe and create a
+    thread-pool/worker queue at a higher level that just uses ImageDiffDB?
+
+    Args:
+      expected_image_url: file or HTTP url from which we will download the
+          expected image
+      expected_image_locator: a unique ID string under which we will store the
+          expected image within storage_root (probably including a checksum to
+          guarantee uniqueness)
+      actual_image_url: file or HTTP url from which we will download the
+          actual image
+      actual_image_locator: a unique ID string under which we will store the
+          actual image within storage_root (probably including a checksum to
+          guarantee uniqueness)
+    """
+    key = (expected_image_locator, actual_image_locator)
+    if not key in self._diff_dict:
+      try:
+        new_diff_record = DiffRecord(
+            self._storage_root,
+            expected_image_url=expected_image_url,
+            expected_image_locator=expected_image_locator,
+            actual_image_url=actual_image_url,
+            actual_image_locator=actual_image_locator)
+      except:
+        logging.exception('got exception while creating new DiffRecord')
+        return
+      self._diff_dict[key] = new_diff_record
+
+  def get_diff_record(self, expected_image_locator, actual_image_locator):
+    """Returns the DiffRecord for this image pair.
+
+    Raises a KeyError if we don't have a DiffRecord for this image pair.
+    """
+    key = (expected_image_locator, actual_image_locator)
+    return self._diff_dict[key]
+
+
+# Utility functions
+
+def _calculate_weighted_diff_metric(image):
+  """Given a diff image (per-channel diff at each pixel between two images),
+  calculate the weighted diff metric (a stab at how different the two images
+  really are).
+
+  Args:
+    image: PIL image; a per-channel diff between two images
+
+  Returns: a weighted diff metric, as a float between 0 and 100 (inclusive).
+  """
+  # TODO(epoger): This is just a wild guess at an appropriate metric.
+  # In the long term, we will probably use some metric generated by
+  # skpdiff anyway.
+  (width, height) = image.size
+  maxdiff = 3 * (width * height) * 255**2
+  h = image.histogram()
+  assert(len(h) % 256 == 0)
+  totaldiff = sum(map(lambda index,value: value * (index%256)**2,
+                      range(len(h)), h))
+  return float(100 * totaldiff) / maxdiff
+
+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.
+
+  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:
+    logging.error('IOError loading image file %s' % filepath)
+    raise
+
+def _mkdir_unless_exists(path):
+  """Unless path refers to an already-existing directory, create it.
+
+  Args:
+    path: path on local disk
+  """
+  if not os.path.isdir(path):
+    os.makedirs(path)
+
+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.
+
+  Args:
+    expected_image_locator: locator string pointing at expected image
+    actual_image_locator: locator string pointing at actual image
+
+  Returns: locator where the diffs between expected and actual images can be
+      found
+  """
+  return "%s-vs-%s" % (expected_image_locator, actual_image_locator)
--- a/gm/rebaseline_server/imagediffdb_test.py
+++ b/gm/rebaseline_server/imagediffdb_test.py
@ -0,0 +1,59 @@
+#!/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.
+
+Test imagediffdb.py
+
+TODO(epoger): Modify to use Python's unittest framework.
+"""
+
+# System-level imports
+import logging
+
+# Local imports
+import imagediffdb
+
+
+IMAGE_URL_BASE = 'http://chromium-skia-gm.commondatastorage.googleapis.com/gm/bitmap-64bitMD5/'
+
+def main():
+  logging.basicConfig(level=logging.INFO)
+
+  # params for each self-test:
+  # 0. expected image locator
+  # 1. expected image URL
+  # 2. actual image locator
+  # 3. actual image URL
+  # 4. expected percent_pixels_differing (as a string, to 4 decimal places)
+  # 5. expected weighted_diff_measure (as a string, to 4 decimal places)
+  selftests = [
+      ['16206093933823793653',
+       IMAGE_URL_BASE + 'arcofzorro/16206093933823793653.png',
+       '13786535001616823825',
+       IMAGE_URL_BASE + 'arcofzorro/13786535001616823825.png',
+       '0.0653', '0.0113'],
+  ]
+
+  # Add all image pairs to the database
+  db = imagediffdb.ImageDiffDB('/tmp/ImageDiffDB')
+  for selftest in selftests:
+    retval = db.add_image_pair(
+        expected_image_locator=selftest[0], expected_image_url=selftest[1],
+        actual_image_locator=selftest[2],   actual_image_url=selftest[3])
+
+  # Fetch each image pair from the database
+  for selftest in selftests:
+    record = db.get_diff_record(expected_image_locator=selftest[0],
+                                actual_image_locator=selftest[2])
+    assert (('%.4f' % record.get_percent_pixels_differing()) == selftest[4])
+    assert (('%.4f' % record.get_weighted_diff_measure()) == selftest[5])
+
+  logging.info("Self-test completed successfully!")
+
+
+if __name__ == '__main__':
+  main()
--- a/gm/rebaseline_server/results.py
+++ b/gm/rebaseline_server/results.py
@ -29,6 +29,7 @@ GM_DIRECTORY = os.path.dirname(os.path.dirname(os.path.realpath(__file__)))
 if GM_DIRECTORY not in sys.path:
  sys.path.append(GM_DIRECTORY)
 import gm_json
+import imagediffdb

 IMAGE_FILENAME_RE = re.compile(gm_json.IMAGE_FILENAME_PATTERN)
 IMAGE_FILENAME_FORMATTER = '%s_%s.png'  # pass in (testname, config)
@ -55,12 +56,15 @@ class Results(object):
  are immutable.  If you want to update the results based on updated JSON
  file contents, you will need to create a new Results object."""

-  def __init__(self, actuals_root, expected_root):
+  def __init__(self, actuals_root, expected_root, generated_images_root):
    """
    Args:
      actuals_root: root directory containing all actual-results.json files
      expected_root: root directory containing all expected-results.json files
+      generated_images_root: directory within which to create all pixels diffs;
+          if this directory does not yet exist, it will be created
    """
+    self._image_diff_db = imagediffdb.ImageDiffDB(generated_images_root)
    self._actuals_root = actuals_root
    self._expected_root = expected_root
    self._load_actual_and_expected()
@ -249,6 +253,36 @@ class Results(object):
          'for builders %s' % (
              expected_builders_written, actual_builders_written))

+  def _generate_pixel_diffs_if_needed(self, test, expected_image, actual_image):
+    """If expected_image and actual_image both exist but are different,
+    add the image pair to self._image_diff_db and generate pixel diffs.
+
+    Args:
+      test: string; name of test
+      expected_image: (hashType, hashDigest) tuple describing the expected image
+      actual_image: (hashType, hashDigest) tuple describing the actual image
+    """
+    if expected_image == actual_image:
+      return
+
+    (expected_hashtype, expected_hashdigest) = expected_image
+    (actual_hashtype, actual_hashdigest) = actual_image
+    if None in [expected_hashtype, expected_hashdigest,
+                actual_hashtype, actual_hashdigest]:
+      return
+
+    expected_url = gm_json.CreateGmActualUrl(
+        test_name=test, hash_type=expected_hashtype,
+        hash_digest=expected_hashdigest)
+    actual_url = gm_json.CreateGmActualUrl(
+        test_name=test, hash_type=actual_hashtype,
+        hash_digest=actual_hashdigest)
+    self._image_diff_db.add_image_pair(
+        expected_image_locator=expected_hashdigest,
+        expected_image_url=expected_url,
+        actual_image_locator=actual_hashdigest,
+        actual_image_url=actual_url)
+
  def _load_actual_and_expected(self):
    """Loads the results of all tests, across all builders (based on the
    files within self._actuals_root and self._expected_root),
@ -343,6 +377,9 @@ class Results(object):
            updated_result_type = result_type

          (test, config) = IMAGE_FILENAME_RE.match(image_name).groups()
+          self._generate_pixel_diffs_if_needed(
+              test=test, expected_image=expected_image,
+              actual_image=actual_image)
          results_for_this_test = {
              'resultType': updated_result_type,
              'builder': builder,
@ -359,6 +396,26 @@ class Results(object):
          if expectations_per_test:
            for field in FIELDS_PASSED_THRU_VERBATIM:
              results_for_this_test[field] = expectations_per_test.get(field)
+
+          if updated_result_type == gm_json.JSONKEY_ACTUALRESULTS_NOCOMPARISON:
+            pass # no diff record to calculate at all
+          elif updated_result_type == gm_json.JSONKEY_ACTUALRESULTS_SUCCEEDED:
+            results_for_this_test['percentDifferingPixels'] = 0
+            results_for_this_test['weightedDiffMeasure'] = 0
+          else:
+            try:
+              diff_record = self._image_diff_db.get_diff_record(
+                  expected_image_locator=expected_image[1],
+                  actual_image_locator=actual_image[1])
+              results_for_this_test['percentDifferingPixels'] = (
+                  diff_record.get_percent_pixels_differing())
+              results_for_this_test['weightedDiffMeasure'] = (
+                  diff_record.get_weighted_diff_measure())
+            except KeyError:
+              logging.warning('unable to find diff_record for ("%s", "%s")' %
+                              (expected_image[1], actual_image[1]))
+              pass
+
          Results._add_to_category_dict(categories_all, results_for_this_test)
          data_all.append(results_for_this_test)

--- a/gm/rebaseline_server/server.py
+++ b/gm/rebaseline_server/server.py
@ -45,6 +45,8 @@ EXPECTATIONS_SVN_REPO = 'http://skia.googlecode.com/svn/trunk/expectations/gm'
 PATHSPLIT_RE = re.compile('/([^/]+)/(.+)')
 TRUNK_DIRECTORY = os.path.dirname(os.path.dirname(os.path.dirname(
    os.path.realpath(__file__))))
+GENERATED_IMAGES_ROOT = os.path.join(PARENT_DIRECTORY, 'static',
+                                     'generated-images')

 # A simple dictionary of file name extensions to MIME types. The empty string
 # entry is used as the default when no extension was given or if the extension
@ -155,11 +157,13 @@ class Server(object):
          expectations_repo.Update('.')

      logging.info(
-          'Parsing results from actuals in %s and expectations in %s ...' % (
+          ('Parsing results from actuals in %s and expectations in %s, '
+          + 'and generating pixel diffs (may take a while) ...') % (
          self._actuals_dir, self._expectations_dir))
      self.results = results.Results(
        actuals_root=self._actuals_dir,
-        expected_root=self._expectations_dir)
+        expected_root=self._expectations_dir,
+        generated_images_root=GENERATED_IMAGES_ROOT)

  def _result_reloader(self):
    """ If --reload argument was specified, reload results at the appropriate
--- a/gm/rebaseline_server/static/loader.js
+++ b/gm/rebaseline_server/static/loader.js
@ -58,7 +58,7 @@ Loader.controller(
        $scope.header = data.header;
        $scope.categories = data.categories;
        $scope.testData = data.testData;
-        $scope.sortColumn = 'test';
+        $scope.sortColumn = 'weightedDiffMeasure';
        $scope.showTodos = false;

        $scope.showSubmitAdvancedSettings = false;
@ -242,6 +242,13 @@ Loader.controller(
      // array copies?  (For better performance.)

      if ($scope.viewingTab == $scope.defaultTab) {
+
+        // TODO(epoger): Until we allow the user to reverse sort order,
+        // there are certain columns we want to sort in a different order.
+        var doReverse = (
+            ($scope.sortColumn == 'percentDifferingPixels') ||
+            ($scope.sortColumn == 'weightedDiffMeasure'));
+
        $scope.filteredTestData =
            $filter("orderBy")(
                $filter("removeHiddenItems")(
@ -252,7 +259,7 @@ Loader.controller(
                    $scope.categoryValueMatch.test,
                    $scope.viewingTab
                ),
-                $scope.sortColumn);
+                $scope.sortColumn, doReverse);
        $scope.limitedTestData = $filter("limitTo")(
            $scope.filteredTestData, $scope.displayLimit);
      } else {
--- a/gm/rebaseline_server/static/view.html
+++ b/gm/rebaseline_server/static/view.html
@ -248,6 +248,22 @@
                   ng-click="sortResultsBy('actualHashDigest')">
            actual image
          </th>
+          <th>
+            <input type="radio"
+                   name="sortColumnRadio"
+                   value="percentDifferingPixels"
+                   ng-checked="(sortColumn == 'percentDifferingPixels')"
+                   ng-click="sortResultsBy('percentDifferingPixels')">
+            differing pixels
+          </th>
+          <th>
+            <input type="radio"
+                   name="sortColumnRadio"
+                   value="weightedDiffMeasure"
+                   ng-checked="(sortColumn == 'weightedDiffMeasure')"
+                   ng-click="sortResultsBy('weightedDiffMeasure')">
+            per-channel deltas
+          </th>
          <th>
            <!-- item-selection checkbox column -->
          </th>
@ -277,16 +293,49 @@
              {{bug}}
            </a>
          </td>
-          <td>
+
+          <!-- expected image -->
+          <td valign="top">
            <a target="_blank" href="http://chromium-skia-gm.commondatastorage.googleapis.com/gm/{{result.expectedHashType}}/{{result.test}}/{{result.expectedHashDigest}}.png">
              <img width="{{imageSize}}" src="http://chromium-skia-gm.commondatastorage.googleapis.com/gm/{{result.expectedHashType}}/{{result.test}}/{{result.expectedHashDigest}}.png"/>
            </a>
          </td>
-          <td>
+
+          <!-- actual image -->
+          <td valign="top">
            <a target="_blank" href="http://chromium-skia-gm.commondatastorage.googleapis.com/gm/{{result.actualHashType}}/{{result.test}}/{{result.actualHashDigest}}.png">
              <img width="{{imageSize}}" src="http://chromium-skia-gm.commondatastorage.googleapis.com/gm/{{result.actualHashType}}/{{result.test}}/{{result.actualHashDigest}}.png"/>
            </a>
          </td>
+
+          <!-- whitediffs: every differing pixel shown in white -->
+          <td valign="top">
+            <div ng-hide="result.expectedHashDigest == result.actualHashDigest">
+              <a target="_blank" href="/static/generated-images/whitediffs/{{result.expectedHashDigest}}-vs-{{result.actualHashDigest}}.png">
+                <img width="{{imageSize}}" src="/static/generated-images/whitediffs/{{result.expectedHashDigest}}-vs-{{result.actualHashDigest}}.png"/>
+              </a><br>
+              {{result.percentDifferingPixels.toFixed(4)}}%
+            </div>
+            <div ng-hide="result.expectedHashDigest != result.actualHashDigest"
+                 style="text-align:center">
+              &ndash;none&ndash;
+            </div>
+          </td>
+
+          <!-- diffs: per-channel RGB deltas -->
+          <td valign="top">
+            <div ng-hide="result.expectedHashDigest == result.actualHashDigest">
+              <a target="_blank" href="/static/generated-images/diffs/{{result.expectedHashDigest}}-vs-{{result.actualHashDigest}}.png">
+                <img width="{{imageSize}}" src="/static/generated-images/diffs/{{result.expectedHashDigest}}-vs-{{result.actualHashDigest}}.png"/>
+              </a><br>
+              {{result.weightedDiffMeasure.toFixed(4)}}%
+            </div>
+            <div ng-hide="result.expectedHashDigest != result.actualHashDigest"
+                 style="text-align:center">
+              &ndash;none&ndash;
+            </div>
+          </td>
+
          <td>
            <input type="checkbox"
                   name="rowSelect"