From 9dddf6f6b957166b6cd6080f405b762aab109948 Mon Sep 17 00:00:00 2001 From: "epoger@google.com" Date: Fri, 8 Nov 2013 16:25:25 +0000 Subject: 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 --- gm/rebaseline_server/imagediffdb.py | 271 +++++++++++++++++++++++++++++++ gm/rebaseline_server/imagediffdb_test.py | 59 +++++++ gm/rebaseline_server/results.py | 59 ++++++- gm/rebaseline_server/server.py | 8 +- gm/rebaseline_server/static/loader.js | 11 +- gm/rebaseline_server/static/view.html | 53 +++++- 6 files changed, 454 insertions(+), 7 deletions(-) create mode 100644 gm/rebaseline_server/imagediffdb.py create mode 100755 gm/rebaseline_server/imagediffdb_test.py (limited to 'gm') diff --git a/gm/rebaseline_server/imagediffdb.py b/gm/rebaseline_server/imagediffdb.py new file mode 100644 index 0000000000..69d282f1d9 --- /dev/null +++ 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) diff --git a/gm/rebaseline_server/imagediffdb_test.py b/gm/rebaseline_server/imagediffdb_test.py new file mode 100755 index 0000000000..034ac51b3e --- /dev/null +++ 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() diff --git a/gm/rebaseline_server/results.py b/gm/rebaseline_server/results.py index 59a8f30c50..90a532a594 100755 --- 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) diff --git a/gm/rebaseline_server/server.py b/gm/rebaseline_server/server.py index bc33c21ec0..a0f31f8ce8 100755 --- 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 diff --git a/gm/rebaseline_server/static/loader.js b/gm/rebaseline_server/static/loader.js index ab5ab480d2..3b16e2e439 100644 --- 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 { diff --git a/gm/rebaseline_server/static/view.html b/gm/rebaseline_server/static/view.html index 1a5faf50c1..bd6cffd376 100644 --- a/gm/rebaseline_server/static/view.html +++ b/gm/rebaseline_server/static/view.html @@ -248,6 +248,22 @@ ng-click="sortResultsBy('actualHashDigest')"> actual image + + + differing pixels + + + + per-channel deltas + @@ -277,16 +293,49 @@ {{bug}} - + + + - + + + + + + +
+ + +
+ {{result.percentDifferingPixels.toFixed(4)}}% +
+
+ –none– +
+ + + + +
+ + +
+ {{result.weightedDiffMeasure.toFixed(4)}}% +
+
+ –none– +
+ +