add ui for mutli-rebaselining
R=epoger@google.com
Review URL: https://codereview.chromium.org/22580004
git-svn-id: http://skia.googlecode.com/svn/trunk@10618 2bbb7eff-a529-9590-31e7-b0007b416f81
diff --git a/tools/skpdiff/diff_viewer.js b/tools/skpdiff/diff_viewer.js
index e7156b3..9c33f84 100644
--- a/tools/skpdiff/diff_viewer.js
+++ b/tools/skpdiff/diff_viewer.js
@@ -76,7 +76,11 @@
};
});
-function DiffListController($scope, $http, $timeout) {
+function DiffListController($scope, $http, $location, $timeout, $parse) {
+ // Detect if we are running the web server version of the viewer. If so, we set a flag and
+ // enable some extra functionality of the website for rebaselining.
+ $scope.isDynamic = ($location.protocol() == "http" || $location.protocol() == "https");
+
// Label each kind of differ for the sort buttons.
$scope.differs = [
{
@@ -90,12 +94,20 @@
// Puts the records within AngularJS scope
$scope.records = SkPDiffRecords.records;
+ // Keep track of the index of the last record to change so that shift clicking knows what range
+ // of records to apply the action to.
+ $scope.lastSelectedIndex = undefined;
+
// Indicates which diff metric is used for sorting
$scope.sortIndex = 1;
// Called by the sort buttons to adjust the metric used for sorting
$scope.setSortIndex = function(idx) {
$scope.sortIndex = idx;
+
+ // Because the index of things has most likely changed, the ranges of shift clicking no
+ // longer make sense from the user's point of view. We reset it to avoid confusion.
+ $scope.lastSelectedIndex = undefined;
};
// A predicate for pulling out the number used for sorting
@@ -103,30 +115,58 @@
return record.diffs[$scope.sortIndex].result;
};
- // Flash status indicators on the rows, and then remove them so the style can potentially be
+ // Flash status indicator on the page, and then remove it so the style can potentially be
// reapplied later.
- $scope.flashRowStatus = function(success, record) {
+ $scope.flashStatus = function(success) {
var flashStyle = success ? "success-flash" : "failure-flash";
var flashDurationMillis = success ? 500 : 800;
// Store the style in the record. The row will pick up the style this way instead of through
// index because index can change with sort order.
- record.cssClasses = flashStyle;
+ $scope.statusClass = flashStyle;
// The animation cannot be repeated unless the class is removed the element.
$timeout(function() {
- record.cssClasses = "";
+ $scope.statusClass = "";
}, flashDurationMillis);
- }
+ };
- $scope.setHashOf = function(imagePath, record) {
- $http.post("/set_hash", {
- "path": imagePath
+ $scope.selectedRebaseline = function(index, event) {
+ // Retrieve the records in the same order they are displayed.
+ var recordsInOrder = $parse("records | orderBy:sortingDiffer")($scope);
+
+ // If the user is shift clicking, apply the last tick/untick to all elements in between this
+ // record, and the last one they ticked/unticked.
+ if (event.shiftKey && $scope.lastSelectedIndex !== undefined) {
+ var currentAction = recordsInOrder[index].isRebaselined;
+ var smallerIndex = Math.min($scope.lastSelectedIndex, index);
+ var largerIndex = Math.max($scope.lastSelectedIndex, index);
+ for (var recordIndex = smallerIndex; recordIndex <= largerIndex; recordIndex++) {
+ recordsInOrder[recordIndex].isRebaselined = currentAction;
+ }
+ $scope.lastSelectedIndex = index;
+ }
+ else
+ {
+ $scope.lastSelectedIndex = index;
+ }
+
+ };
+
+ $scope.commitRebaselines = function() {
+ // Gather up all records that have the rebaseline set.
+ var rebaselines = [];
+ for (var recordIndex = 0; recordIndex < $scope.records.length; recordIndex++) {
+ if ($scope.records[recordIndex].isRebaselined) {
+ rebaselines.push($scope.records[recordIndex].testPath);
+ }
+ }
+ $http.post("/commit_rebaselines", {
+ "rebaselines": rebaselines
}).success(function(data) {
- $scope.flashRowStatus(data.success, record);
+ $scope.flashStatus(data.success);
}).error(function() {
- $scope.flashRowStatus(false, record);
+ $scope.flashStatus(false);
});
-
};
}
diff --git a/tools/skpdiff/skpdiff_server.py b/tools/skpdiff/skpdiff_server.py
index 1403540..053291a 100755
--- a/tools/skpdiff/skpdiff_server.py
+++ b/tools/skpdiff/skpdiff_server.py
@@ -99,6 +99,8 @@
@param image_path Path to place the image.
@param hash_val The hash value of the image.
"""
+ if hash_val is None:
+ return
# Separate the test name from a image name
image_match = IMAGE_FILENAME_RE.match(image_name)
@@ -174,14 +176,18 @@
- image_name = the GM test name and config
- expected_hash = the current expected hash value
- actual_hash = the actual hash value
+ - is_rebaselined = True if actual_hash is what is currently in the expected
+ results file, False otherwise.
"""
def __init__(self,
device_name, image_name,
- expected_hash, actual_hash):
+ expected_hash, actual_hash,
+ is_rebaselined):
self.device_name = device_name
self.image_name = image_name
self.expected_hash = expected_hash
self.actual_hash = actual_hash
+ self.is_rebaselined = is_rebaselined
class ExpectationsManager:
@@ -228,13 +234,14 @@
# Invoke skpdiff with our downloaded images and place its results in the
# temporary directory.
- self.skpdiff_output_path = os.path.join(image_output_dir,
+ self._skpdiff_output_path = os.path.join(image_output_dir,
'skpdiff_output.json')
skpdiff_cmd = SKPDIFF_INVOKE_FORMAT.format(self._skpdiff_path,
- self.skpdiff_output_path,
+ self._skpdiff_output_path,
expected_image_dir,
actual_image_dir)
os.system(skpdiff_cmd)
+ self._load_skpdiff_output()
def _get_expectations(self):
@@ -267,11 +274,25 @@
with open(updated_file_path, 'rb') as updated_file:
updated_contents = updated_file.read()
+ # Read the expected results on disk to determine what we've
+ # already rebaselined.
+ commited_contents = None
+ with open(expected_file_path, 'rb') as expected_file:
+ commited_contents = expected_file.read()
+
# Find all expectations that did not match.
expected_diff = differ.GenerateDiffDictFromStrings(
expected_contents,
updated_contents)
+ # Generate a set of images that have already been rebaselined
+ # onto disk.
+ rebaselined_diff = differ.GenerateDiffDictFromStrings(
+ expected_contents,
+ commited_contents)
+
+ rebaselined_set = set(rebaselined_diff.keys())
+
# The name of the device corresponds to the name of the folder
# we are in.
device_name = os.path.basename(root)
@@ -280,7 +301,18 @@
for image_name, hashes in expected_diff.iteritems():
self._expectations.append(
GMInstance(device_name, image_name,
- hashes['old'], hashes['new']))
+ hashes['old'], hashes['new'],
+ image_name in rebaselined_set))
+
+ def _load_skpdiff_output(self):
+ """Loads the results of skpdiff and annotates them with whether they
+ have already been rebaselined or not. The resulting data is store in
+ self.skpdiff_records."""
+ self.skpdiff_records = None
+ with open(self._skpdiff_output_path, 'rb') as skpdiff_output_file:
+ self.skpdiff_records = json.load(skpdiff_output_file)['records']
+ for record in self.skpdiff_records:
+ record['isRebaselined'] = self.image_map[record['baselinePath']][1].is_rebaselined
def _download_expectation_images(self, expected_image_dir, actual_image_dir):
@@ -346,23 +378,36 @@
# Write it out to disk using gm_json to keep the formatting consistent.
gm_json.WriteToFile(expectations, json_path)
- def use_hash_of(self, image_path):
- """Determine the hash of the image at the path using the records, and
- set it as the expected hash for its device and image config.
+ def commit_rebaselines(self, rebaselines):
+ """Sets the expected results file to use the hashes of the images in
+ the rebaselines list. If a expected result image is not in rebaselines
+ at all, the old hash will be used.
- @param image_path The path of the image as it was stored in the output
- of skpdiff_path
+ @param rebaselines A list of image paths to use the hash of.
"""
+ # Reset all expectations to their old hashes because some of them may
+ # have been set to the new hash by a previous call to this function.
+ for expectation in self._expectations:
+ expectation.is_rebaselined = False
+ self._set_expected_hash(expectation.device_name,
+ expectation.image_name,
+ expectation.expected_hash)
- # Get the metadata about the image at the path.
- is_actual, expectation = self.image_map[image_path]
+ # Take all the images to rebaseline
+ for image_path in rebaselines:
+ # Get the metadata about the image at the path.
+ is_actual, expectation = self.image_map[image_path]
- expectation_hash = expectation.actual_hash if is_actual else\
- expectation.expected_hash
+ expectation.is_rebaselined = is_actual
+ expectation_hash = expectation.actual_hash if is_actual else\
+ expectation.expected_hash
- # Write out that image's hash directly to the expected results file.
- self._set_expected_hash(expectation.device_name, expectation.image_name,
- expectation_hash)
+ # Write out that image's hash directly to the expected results file.
+ self._set_expected_hash(expectation.device_name,
+ expectation.image_name,
+ expectation_hash)
+
+ self._load_skpdiff_output()
class SkPDiffHandler(BaseHTTPServer.BaseHTTPRequestHandler):
@@ -410,7 +455,15 @@
self.send_response(200)
self.send_header('Content-type', MIME_TYPE_MAP['json'])
self.end_headers()
- self.wfile.write(self.server.skpdiff_output_json)
+
+ # Add JSONP padding to the JSON because the web page expects it. It
+ # expects it because it was designed to run with or without a web
+ # server. Without a web server, the only way to load JSON is with
+ # JSONP.
+ skpdiff_records = self.server.expectations_manager.skpdiff_records
+ self.wfile.write('var SkPDiffRecords = ')
+ json.dump({'records': skpdiff_records}, self.wfile)
+ self.wfile.write(';')
return
# Attempt to send static asset files first.
@@ -427,10 +480,11 @@
self.send_error(404)
def do_POST(self):
- if self.path == '/set_hash':
+ if self.path == '/commit_rebaselines':
content_length = int(self.headers['Content-length'])
request_data = json.loads(self.rfile.read(content_length))
- self.server.expectations_manager.use_hash_of(request_data['path'])
+ rebaselines = request_data['rebaselines']
+ self.server.expectations_manager.commit_rebaselines(rebaselines)
self.send_response(200)
self.send_header('Content-type', 'application/json')
self.end_headers()
@@ -442,23 +496,11 @@
def run_server(expectations_manager, port=8080):
- # Preload the skpdiff results file. This is so we can perform some
- # processing on it.
- skpdiff_output_json = ''
- with open(expectations_manager.skpdiff_output_path, 'rb') as records_file:
- skpdiff_output_json = records_file.read()
-
# It's important to parse the results file so that we can make a set of
# images that the web page might request.
- skpdiff_records = json.loads(skpdiff_output_json)['records']
+ skpdiff_records = expectations_manager.skpdiff_records
image_set = get_image_set_from_skpdiff(skpdiff_records)
- # Add JSONP padding to the JSON because the web page expects it. It expects
- # it because it was designed to run with or without a web server. Without a
- # web server, the only way to load JSON is with JSONP.
- skpdiff_output_json = ('var SkPDiffRecords = ' +
- json.dumps({'records': skpdiff_records}) + ';')
-
# Do not bind to interfaces other than localhost because the server will
# attempt to serve files relative to the root directory as a last resort
# before 404ing. This means all of your files can be accessed from this
@@ -466,7 +508,6 @@
server_address = ('127.0.0.1', port)
http_server = BaseHTTPServer.HTTPServer(server_address, SkPDiffHandler)
http_server.image_set = image_set
- http_server.skpdiff_output_json = skpdiff_output_json
http_server.expectations_manager = expectations_manager
print('Navigate thine browser to: http://{}:{}/'.format(*server_address))
http_server.serve_forever()
diff --git a/tools/skpdiff/viewer.html b/tools/skpdiff/viewer.html
index 46c5871..f4b8fd7 100644
--- a/tools/skpdiff/viewer.html
+++ b/tools/skpdiff/viewer.html
@@ -15,50 +15,63 @@
us to use a webserver.
-->
<script type="text/ng-template" id="/diff_list.html">
- <!-- Give a choice of how to order the differs -->
- <div style="margin:8px">
- Show me the worst by metric:
- <button ng-repeat="differ in differs" ng-click="setSortIndex($index)">
- <span class="result-{{ $index }}" style="padding-left:12px;"> </span>
- {{ differ.title }}
- </button>
+ <div ng-class="statusClass">
+ <!-- The commit button -->
+ <div ng-show="isDynamic" class="commit">
+ <button ng-click="commitRebaselines()">Commit</button>
+ </div>
+ <!-- Give a choice of how to order the differs -->
+ <div style="margin:8px">
+ Show me the worst by metric:
+ <button ng-repeat="differ in differs" ng-click="setSortIndex($index)">
+ <span class="result-{{ $index }}" style="padding-left:12px;"> </span>
+ {{ differ.title }}
+ </button>
+ </div>
+ <!-- Begin list of differences -->
+ <table>
+ <thead>
+ <tr>
+ <td ng-show="isDynamic">Rebaseline?</td>
+ <td>Expected Image</td>
+ <td>Actual Image</td>
+ <td>Results</td>
+ </tr>
+ </thead>
+ <tbody>
+ <!--
+ Loops through every record and crates a row for it. This sorts based on the whichever
+ metric the user chose, and places a limit on the max number of records to show.
+ -->
+ <tr ng-repeat="record in records | orderBy:sortingDiffer | limitTo:500"
+ ng-class="{selected: record.isRebaselined}">
+ <td ng-show="isDynamic">
+ <input type="checkbox"
+ ng-model="record.isRebaselined"
+ ng-click="selectedRebaseline($index, $event)"
+ ng-class="{lastselected: lastSelectedIndex == $index}" />
+ </td>
+ <td>
+ <swap-img left-src="{{ record.baselinePath }}"
+ right-src="{{ record.testPath }}"
+ side="left"
+ class="gm-image left-image" /></td>
+ <td>
+ <swap-img left-src="{{ record.baselinePath }}"
+ right-src="{{ record.testPath }}"
+ side="right"
+ class="gm-image right-image" /></td>
+ <td>
+ <div ng-repeat="diff in record.diffs"
+ ng-mouseover="highlight(diff.differName)"
+ class="result result-{{$index}}">
+ <span class="result-button">{{ diff.result }}</span>
+ </div>
+ </td>
+ </tr>
+ </tbody>
+ </table>
</div>
- <!-- Begin list of differences -->
- <table>
- <thead>
- <tr>
- <td>Expected Image</td>
- <td>Actual Image</td>
- <td>Results</td>
- </tr>
- </thead>
- <tbody>
- <!--
- Loops through every record and crates a row for it. This sorts based on the whichever
- metric the user chose, and places a limit on the max number of records to show.
- -->
- <tr ng-repeat="record in records | orderBy:sortingDiffer | limitTo:500"
- class="{{ record.cssClasses }}">
- <td ng-click="setHashOf(record.baselinePath, record)">
- <swap-img left-src="{{ record.baselinePath }}"
- right-src="{{ record.testPath }}"
- side="left"
- class="gm-image left-image" /></td>
- <td ng-click="setHashOf(record.testPath, record)">
- <swap-img left-src="{{ record.baselinePath }}"
- right-src="{{ record.testPath }}"
- side="right"
- class="gm-image right-image" /></td>
- <td>
- <div ng-repeat="diff in record.diffs"
- ng-mouseover="highlight(diff.differName)"
- class="result result-{{$index}}">
- <span class="result-button">{{ diff.result }}</span>
- </div>
- </td>
- </tr>
- </tbody>
- </table>
</script>
<!-- Whatever template is used is rendered in the following div -->
<div ng-view></div>
diff --git a/tools/skpdiff/viewer_style.css b/tools/skpdiff/viewer_style.css
index 81b4f90..e172667 100644
--- a/tools/skpdiff/viewer_style.css
+++ b/tools/skpdiff/viewer_style.css
@@ -19,6 +19,60 @@
border: none;
}
+button {
+ font-family: Verdana;
+ font-weight: 900;
+}
+
+input[type="checkbox"] {
+ -webkit-appearance: none;
+ -moz-appearance: none;
+ width: 20px;
+ height:20px;
+ padding: 2px;
+ background: #EEE;
+ border-radius: 2px;
+ box-shadow: inset 0 0 8px -2px black;
+}
+
+input[type="checkbox"]:checked {
+ background: #a9db80;
+ background: -webkit-linear-gradient(top, #00b7ea 0%,#009ec3 100%);
+}
+
+input[type="checkbox"]:active {
+ background: #a9db80;
+ background: -webkit-linear-gradient(top, #009ec3 0%,#00b7ea 100%);
+}
+
+input[type="checkbox"].lastselected {
+ padding: 0;
+ border: 2px solid #009ec3;
+ box-shadow: inset 0 0 8px -2px black, 0 0 6px black;
+}
+
+.commit {
+ position: absolute;
+ top: 8px;
+ right: 8px;
+}
+
+.commit > button {
+ border: 1px solid #00687F;
+ border-radius: 4px;
+ padding: 8px;
+ color: white;
+ text-shadow: 0 0 4px black;
+ box-shadow: 0 0 8px black;
+ background: #a9db80;
+ background: -webkit-linear-gradient(top, #a9db80 0%,#96c56f 100%);
+}
+
+.commit > button:active {
+ background: #96c56f;
+ background: -webkit-linear-gradient(top, #96c56f 0%,#a9db80 100%);
+}
+
.gm-image {
border: 1px dotted black;
}
@@ -27,6 +81,10 @@
border: 1px dashed black;
}
+.selected {
+ background: #ffff88;
+}
+
.left-image {
float: right;
}
@@ -68,13 +126,13 @@
.result {
padding: 8px;
cursor: default;
- opacity: 0.7;
+ -webkit-filter: grayscale(30%)
}
.result:hover {
border: 2px dotted #DDD;
padding: 6px;
- opacity: 1.0;
+ -webkit-filter: grayscale(0)
}
.result-0 {