resize mask canvas for viewer.html in skpdiff tool

tools/skpdiff/diff_viewer.js

 var MAX_SWAP_IMG_SIZE = 400;
 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
 var MAGNIFIER_SCALE_FACTOR = 2.0;
 
 angular.module('diff_viewer', []).
 directive('imgCompare', function() {
   // Custom directive for comparing (3-way) images
   return {
       restrict: 'E', // The directive can be used as an element name
       replace: true, // The directive replaces itself with the template
       template: '<canvas/>',
       scope: true,
       link: function(scope, elm, attrs, ctrl) {
           var image = new Image();
           var canvas = elm[0];
           var ctx = canvas.getContext('2d');
 
           var magnifyContent = 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;
+                    scope.setHWRatio4MaskCanvas(canvas.height / canvas.width);

[djsollen, 2013/12/09 16:40:12]

I think you can remove this.  The width and height of the canvas will not be
that of the image until the image.onload completes so I don't see the benefit
here.

[yunchao, 2013/12/10 05:58:31]

Agree!

                     break;
                 case "baseline":
                     image.src = scope.record.baselinePath;
                     magnifyContent = true;
                     break;
                 case "test":
                     image.src = scope.record.testPath;
                     magnifyContent = true;
                     break;
                 default:
                     console.log("Unknown type attribute on <img-compare>: " + value);
                     return;
               }
 
               image.onload = function() {
                   // compute the scaled image width/height for image and canvas
                   var divisor = 1;
                   // Make it so the maximum size of an image is MAX_SWAP_IMG_SIZE,
                   // and the images are scaled down in halves.
                   while ((image.width / divisor) > MAX_SWAP_IMG_SIZE) {
                       divisor *= 2;
                   }
 
                   scope.setImgScaleFactor(1 / divisor);
 
                   // Set canvas to correct size
                   canvas.width = image.width * scope.imgScaleFactor;
                   canvas.height = image.height * scope.imgScaleFactor;
 
+                  // Set the ratio of height / width for mask canvas according
+                  // to baseline and test canvas
+                  scope.setHWRatio4MaskCanvas(canvas.height / canvas.width);

[djsollen, 2013/12/09 16:40:12]

it seems like we only want to do this for the non-alphaMask canvas.  I think
your missing some kind of conditional check here.

[yunchao, 2013/12/10 05:58:31]

Done.

+
                   // render the image onto the canvas
                   scope.renderImage();
               }
           });
 
           // When the magnify attribute changes, render the magnified rect at
           // the default zoom level.
           scope.$watch('magnifyCenter', function(magCenter) {
               if (!magnifyContent) {
                   return;
@@ skipping 59 matching lines @@
 
           // event handler for mouse events that triggers the magnification
           // effect across the 3 images being compared.
           scope.MagnifyDraw = function(event, startMagnify) {
               if (startMagnify) {
                   scope.setMagnifierState(true);
               }  else if (!scope.magnifierOn) {
                   return;
               }
 
+              canvas.height = canvas.width * scope.HWRatio4MaskCanvas;

[djsollen, 2013/12/09 16:40:12]

why does the canvas height need to be set each time the magnifier is drawn. It
seems like it should only need to be done one time.

[yunchao, 2013/12/10 05:58:31]

Done, resize non-alphaMask canvas when loading image and resizing baseline
canvas.

               scope.renderImage();
 
               // render the magnifier outline rect
               var rect = scope.computeMagnifierOutline(event);
               ctx.save();
               ctx.beginPath();
               ctx.rect(rect.x, rect.y, rect.width, rect.height);
               ctx.lineWidth = 2;
               ctx.strokeStyle = 'red';
               ctx.stroke();
@@ skipping 12 matching lines @@
               scope.setMagnifyCenter(undefined);
         };
       }
   };
 });
 
 function ImageController($scope, $http, $location, $timeout, $parse) {
   $scope.imgScaleFactor = 1.0;
   $scope.magnifierOn = false;
   $scope.magnifyCenter = undefined;
+  $scope.HWRatio4MaskCanvas = 0;
 
   $scope.setImgScaleFactor = function(scaleFactor) {
     $scope.imgScaleFactor = scaleFactor;
   }
 
   $scope.setMagnifierState = function(magnifierOn) {
     $scope.magnifierOn = magnifierOn;
   }
 
   $scope.setMagnifyCenter = function(magnifyCenter) {
       $scope.magnifyCenter = magnifyCenter;
   }
+
+  $scope.setHWRatio4MaskCanvas = function(ratio) {
+      if (ratio > $scope.HWRatio4MaskCanvas) {

[djsollen, 2013/12/09 16:40:12]

I don't see why the setter should be conditional. I think you are just calling
the setter too many times.

[yunchao, 2013/12/10 05:58:31]

Agree. have updated this patch accordingly.

+          $scope.HWRatio4MaskCanvas = ratio;
+      }
+  }
 }
 
 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 = [
         {
@@ skipping 76 matching lines @@
         }
         $http.post("/commit_rebaselines", {
             "rebaselines": rebaselines
         }).success(function(data) {
             $scope.flashStatus(data.success);
         }).error(function() {
             $scope.flashStatus(false);
         });
     };
 }