resize mask canvas for viewer.html in skpdiff tool

tools/skpdiff/diff_viewer.js

Index: tools/skpdiff/diff_viewer.js
diff --git a/tools/skpdiff/diff_viewer.js b/tools/skpdiff/diff_viewer.js
index 06a864edf4e219ade95c0c322cc14d935e3141b5..6083c2a19b5e55ce881845527eca984b57c2d61c 100644
--- a/tools/skpdiff/diff_viewer.js
+++ b/tools/skpdiff/diff_viewer.js
@@ -3,7 +3,6 @@ var MAGNIFIER_WIDTH = 200;
 var MAGNIFIER_HEIGHT = 200;
 var MAGNIFIER_HALF_WIDTH = MAGNIFIER_WIDTH * 0.5;
 var MAGNIFIER_HALF_HEIGHT = MAGNIFIER_HEIGHT * 0.5;
-// TODO add support for a magnified scale factor

[djsollen, 2013/12/10 14:18:15]

this TODO was to track making the strength of the magnifier user configurable.

 var MAGNIFIER_SCALE_FACTOR = 2.0;
 
 angular.module('diff_viewer', []).
@@ -20,12 +19,18 @@ directive('imgCompare', function() {
           var ctx = canvas.getContext('2d');
 
           var magnifyContent = false;
+          var maskCanvas = false;
 
           // When the type attribute changes, load the image and then render
           attrs.$observe('type', function(value) {
               switch(value) {
                 case "alphaMask":
                     image.src = scope.record.differencePath;
+                    maskCanvas = true;
+                    // need to change size for non-alphaMask canvas
+                    if (image.name == "") {

[djsollen, 2013/12/10 14:18:15]

This if check is unnecessary.  I don't think the alphaMask element ever has a
name assigned.

[yunchao, 2013/12/10 14:35:00]

Hey derek, how to depart a non-alphaMask element from an alphaMask element? I
found this error(can't magnify all part for baseline/test image) in GM results.
But all cases of skpdiff_output.json from GM results have no alphaMask element.
The differencePath for alphaMask element are all NULL. Then image.src is set to
PATH/viewer.html here. I can't find a case has alphaMask element.   

[djsollen, 2013/12/10 14:44:21]

You need to run the skpdiff tool with the --alphaDir option to have it produce
the alpha mask.

[yunchao, 2013/12/11 03:02:21]

Yes. Then I found this CL is a corner case, because in most cases, alphaDir
should be designated.

+                        scope.setNeedChangeMaskCanvasSize(true);
+                    }
                     break;
                 case "baseline":
                     image.src = scope.record.baselinePath;
@@ -55,11 +60,27 @@ directive('imgCompare', function() {
                   canvas.width = image.width * scope.imgScaleFactor;
                   canvas.height = image.height * scope.imgScaleFactor;
 
+                  // resize non-alphaMask canvas according to baseline canvas
+                  if (scope.needChangeMaskCanvasSize) {
+                      scope.resizeMaskCanvas({width: canvas.width, height: canvas.height});
+                      scope.setNeedChangeMaskCanvasSize(false);

[djsollen, 2013/12/10 14:18:15]

you should be passing true if you really only want this to run 1x.

[yunchao, 2013/12/10 14:35:00]

I suppose line 31-33 can pass true. And update canvas size for non-alphaMask
canvas when loading baseline image. Then set false here to prevent from resizing
the mask canvas again when loading test image.

[djsollen, 2013/12/10 14:44:21]

I don't think 31-33 needs to exist as the scope object will initialize that
variable to false.

[yunchao, 2013/12/11 03:02:21]

Yes, the original patch line 31-33 is unnecessary. But the code is executed
after scope object initialization. I have traced this in devtools.

+                  }
+
                   // render the image onto the canvas
                   scope.renderImage();
               }
           });
 
+          // when newMaskCanvas changes, resize mask canvas size.
+          scope.$watch('newMaskCanvas', function(newCanvas) {
+              if (!maskCanvas) {
+                  return;
+              }
+              
+              canvas.width = newCanvas.width;
+              canvas.height = newCanvas.height;
+          }) 

[djsollen, 2013/12/10 14:18:15]

this should be });

[yunchao, 2013/12/11 03:02:21]

Done.

+
           // When the magnify attribute changes, render the magnified rect at
           // the default zoom level.
           scope.$watch('magnifyCenter', function(magCenter) {
@@ -166,6 +187,8 @@ function ImageController($scope, $http, $location, $timeout, $parse) {
   $scope.imgScaleFactor = 1.0;
   $scope.magnifierOn = false;
   $scope.magnifyCenter = undefined;
+  $scope.newMaskCanvas = undefined;
+  $scope.needChangeMaskCanvasSize = false;

[djsollen, 2013/12/10 14:18:15]

I think it may be more clear to use names like

$scope.maskSizeUpdated = undefined;
$scope.maskSizeInitialized = false;

[yunchao, 2013/12/11 03:02:21]

Agree, the original variable names are not good. The mask Canvas size is
initialized to width=300, height=150 in line 18. So I didn't use variable
maskSizeInitialized. I use variable maskSizeUpdated as you suggested to record
the state(size updated or not), and use updatedMaskSize to record the new size
when updating.

 
   $scope.setImgScaleFactor = function(scaleFactor) {
     $scope.imgScaleFactor = scaleFactor;
@@ -178,6 +201,14 @@ function ImageController($scope, $http, $location, $timeout, $parse) {
   $scope.setMagnifyCenter = function(magnifyCenter) {
       $scope.magnifyCenter = magnifyCenter;
   }
+
+  $scope.resizeMaskCanvas = function(newMaskCanvas) {

[djsollen, 2013/12/10 14:18:15]

function(updatedSize)

+      $scope.newMaskCanvas = newMaskCanvas;
+  }
+
+  $scope.setNeedChangeMaskCanvasSize = function(flag) {
+      $scope.needChangeMaskCanvasSize = flag;
+  }
 }
 
 function DiffListController($scope, $http, $location, $timeout, $parse) {