rebaseline_server: improve pixel diff reporting

- fix differing pixels count (we weren't always doing this right) - report number of differing pixels, as well as percentage - report worst difference per RGB channel - apply alpha mask to difference image (changed pixels = opaque, unchanged = transparent) - add tooltips with further explanation of pixel diffs (SkipBuildbotRuns) R=mtklein@google.com Review URL: https://codereview.chromium.org/82823005 git-svn-id: http://skia.googlecode.com/svn/trunk@12366 2bbb7eff-a529-9590-31e7-b0007b416f81
2013-11-22 19:26:18 +00:00 · 2013-11-22 19:26:18 +00:00 · 214a024e8c
commit 214a024e8c
parent 1950d5f58f
5 changed files with 138 additions and 58 deletions
--- a/gm/rebaseline_server/imagediffdb.py
+++ b/gm/rebaseline_server/imagediffdb.py
@ -21,13 +21,13 @@ except ImportError:
                    + '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'

+VALUES_PER_BAND = 256
+

 class DiffRecord(object):
  """ Record of differences between two images. """
@ -65,33 +65,43 @@ class DiffRecord(object):
                     str(actual_image_locator) + IMAGE_SUFFIX),
        actual_image_url)

-    # Store the diff image (absolute diff at each pixel).
+    # Generate the diff image (absolute diff at each pixel) and
+    # max_diff_per_channel.
    diff_image = _generate_image_diff(actual_image, expected_image)
-    self._weighted_diff_measure = _calculate_weighted_diff_metric(diff_image)
+    diff_histogram = diff_image.histogram()
+    (diff_width, diff_height) = diff_image.size
+    self._weighted_diff_measure = _calculate_weighted_diff_metric(
+        diff_histogram, diff_width * diff_height)
+    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))
+
+    # 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)
-    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)
+    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()[255]
+    self._num_pixels_differing = (
+        whitediff_image.histogram()[VALUES_PER_BAND - 1])

  def get_num_pixels_differing(self):
    """Returns the absolute number of pixels that differ."""
@ -108,6 +118,11 @@ class DiffRecord(object):
    (inclusive)."""
    return self._weighted_diff_measure

+  def get_max_diff_per_channel(self):
+    """Returns the maximum difference between the expected and actual images
+    for each R/G/B channel, as a list."""
+    return self._max_diff_per_channel
+

 class ImageDiffDB(object):
  """ Calculates differences between image pairs, maintaining a database of
@ -175,26 +190,55 @@ class ImageDiffDB(object):

 # 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).
+def _calculate_weighted_diff_metric(histogram, num_pixels):
+  """Given the histogram of 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
+    histogram: PIL histogram of a per-channel diff between two images
+    num_pixels: integer; the total number of pixels in the diff image

  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.
+  # TODO(epoger): As a wild guess at an appropriate metric, weight each
+  # different pixel by the square of its delta value.  (The more different
+  # a pixel is from its expectation, the more we care about it.)
  # 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
+  assert(len(histogram) % VALUES_PER_BAND == 0)
+  num_bands = len(histogram) / VALUES_PER_BAND
+  max_diff = num_pixels * num_bands * (VALUES_PER_BAND - 1)**2
+  total_diff = 0
+  for index in xrange(len(histogram)):
+    total_diff += histogram[index] * (index % VALUES_PER_BAND)**2
+  return float(100 * total_diff) / max_diff
+
+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
@ -248,6 +292,18 @@ def _open_image(filepath):
    logging.error('IOError loading image file %s' % 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):
  """Unless path refers to an already-existing directory, create it.

--- a/gm/rebaseline_server/imagediffdb_test.py
+++ b/gm/rebaseline_server/imagediffdb_test.py
@ -30,12 +30,22 @@ def main():
  # 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)
+  # 6. expected max_diff_per_channel
  selftests = [
-      ['16206093933823793653',
-       IMAGE_URL_BASE + 'arcofzorro/16206093933823793653.png',
-       '13786535001616823825',
-       IMAGE_URL_BASE + 'arcofzorro/13786535001616823825.png',
-       '0.0653', '0.0113'],
+      [
+          '16206093933823793653',
+          IMAGE_URL_BASE + 'arcofzorro/16206093933823793653.png',
+          '13786535001616823825',
+          IMAGE_URL_BASE + 'arcofzorro/13786535001616823825.png',
+          '0.0662', '0.0113', [255, 255, 247],
+      ],
+      [
+          '10552995703607727960',
+          IMAGE_URL_BASE + 'gradients_degenerate_2pt/10552995703607727960.png',
+          '11198253335583713230',
+          IMAGE_URL_BASE + 'gradients_degenerate_2pt/11198253335583713230.png',
+          '100.0000', '66.6667', [255, 0, 255],
+      ],
  ]

  # Add all image pairs to the database
@ -51,7 +61,7 @@ def main():
                                actual_image_locator=selftest[2])
    assert (('%.4f' % record.get_percent_pixels_differing()) == selftest[4])
    assert (('%.4f' % record.get_weighted_diff_measure()) == selftest[5])
-
+    assert (record.get_max_diff_per_channel() == selftest[6])
  logging.info("Self-test completed successfully!")


--- a/gm/rebaseline_server/results.py
+++ b/gm/rebaseline_server/results.py
@ -61,7 +61,7 @@ class Results(object):
    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;
+      generated_images_root: directory within which to create all pixel diffs;
          if this directory does not yet exist, it will be created
    """
    self._image_diff_db = imagediffdb.ImageDiffDB(generated_images_root)
@ -400,17 +400,23 @@ class Results(object):
          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['numDifferingPixels'] = 0
            results_for_this_test['percentDifferingPixels'] = 0
            results_for_this_test['weightedDiffMeasure'] = 0
+            results_for_this_test['maxDiffPerChannel'] = 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['numDifferingPixels'] = (
+                  diff_record.get_num_pixels_differing())
              results_for_this_test['percentDifferingPixels'] = (
                  diff_record.get_percent_pixels_differing())
              results_for_this_test['weightedDiffMeasure'] = (
                  diff_record.get_weighted_diff_measure())
+              results_for_this_test['maxDiffPerChannel'] = (
+                  diff_record.get_max_diff_per_channel())
            except KeyError:
              logging.warning('unable to find diff_record for ("%s", "%s")' %
                              (expected_image[1], actual_image[1]))
--- a/gm/rebaseline_server/static/view.css
+++ b/gm/rebaseline_server/static/view.css
@ -30,3 +30,7 @@
 .update-results-button {
    font-size: 30px;
 }
+
+.image-link {
+    text-decoration: none;
+}
--- a/gm/rebaseline_server/static/view.html
+++ b/gm/rebaseline_server/static/view.html
@ -232,7 +232,7 @@
                   ng-click="sortResultsBy('bugs')">
            bugs
          </th>
-          <th>
+          <th width="{{imageSize}}">
            <input type="radio"
                   name="sortColumnRadio"
                   value="expectedHashDigest"
@ -240,7 +240,7 @@
                   ng-click="sortResultsBy('expectedHashDigest')">
            expected image
          </th>
-          <th>
+          <th width="{{imageSize}}">
            <input type="radio"
                   name="sortColumnRadio"
                   value="actualHashDigest"
@ -248,21 +248,21 @@
                   ng-click="sortResultsBy('actualHashDigest')">
            actual image
          </th>
-          <th>
+          <th width="{{imageSize}}">
            <input type="radio"
                   name="sortColumnRadio"
                   value="percentDifferingPixels"
                   ng-checked="(sortColumn == 'percentDifferingPixels')"
                   ng-click="sortResultsBy('percentDifferingPixels')">
-            differing pixels
+            differing pixels in white
          </th>
-          <th>
+          <th width="{{imageSize}}">
            <input type="radio"
                   name="sortColumnRadio"
                   value="weightedDiffMeasure"
                   ng-checked="(sortColumn == 'weightedDiffMeasure')"
                   ng-click="sortResultsBy('weightedDiffMeasure')">
-            per-channel deltas
+            difference per pixel
          </th>
          <th>
            <!-- item-selection checkbox column -->
@ -295,26 +295,28 @@
          </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">
+          <td valign="top" width="{{imageSize}}">
+            <a class="image-link" 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>

          <!-- actual image -->
-          <td valign="top">
-            <a target="_blank" href="http://chromium-skia-gm.commondatastorage.googleapis.com/gm/{{result.actualHashType}}/{{result.test}}/{{result.actualHashDigest}}.png">
+          <td valign="top" width="{{imageSize}}">
+            <a class="image-link" 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">
+          <td valign="top" width="{{imageSize}}">
+            <div ng-hide="result.expectedHashDigest == result.actualHashDigest"
+                 title="{{result.numDifferingPixels | number:0}} of {{(100 * result.numDifferingPixels / result.percentDifferingPixels) | number:0}} pixels ({{result.percentDifferingPixels.toFixed(4)}}%) differ from expectation.">
+              <a class="image-link" 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)}}%
+              ({{result.numDifferingPixels}})
            </div>
            <div ng-hide="result.expectedHashDigest != result.actualHashDigest"
                 style="text-align:center">
@ -323,12 +325,14 @@
          </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">
+          <td valign="top" width="{{imageSize}}">
+            <div ng-hide="result.expectedHashDigest == result.actualHashDigest"
+                 title="Weighted difference measure is {{result.weightedDiffMeasure.toFixed(4)}}%.  Maximum difference per channel: R={{result.maxDiffPerChannel[0]}}, G={{result.maxDiffPerChannel[1]}}, B={{result.maxDiffPerChannel[2]}}">
+              <a class="image-link" 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)}}%
+              {{result.maxDiffPerChannel}}
            </div>
            <div ng-hide="result.expectedHashDigest != result.actualHashDigest"
                 style="text-align:center">