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

5 files changed, 138 insertions, 58 deletions

diff --git a/gm/rebaseline_server/imagediffdb.py b/gm/rebaseline_server/imagediffdb.py
index 69d282f1d9..6f7a1e60db 100644
--- 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.
 
diff --git a/gm/rebaseline_server/imagediffdb_test.py b/gm/rebaseline_server/imagediffdb_test.py
index 034ac51b3e..9d0dc0de06 100755
--- 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!")
 
 
diff --git a/gm/rebaseline_server/results.py b/gm/rebaseline_server/results.py
index 90a532a594..aa06ec0925 100755
--- 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]))
diff --git a/gm/rebaseline_server/static/view.css b/gm/rebaseline_server/static/view.css
index f848220325..f32c2ec6e9 100644
--- 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;
+}
diff --git a/gm/rebaseline_server/static/view.html b/gm/rebaseline_server/static/view.html
index bd6cffd376..fa5a0c635b 100644
--- 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">