Issue #258 : Move resize method to utils + add unit test

src/js/service/pensize/PenSizeService.js

@@ -4,10 +4,15 @@
   var MIN_PENSIZE = 1;
   var MAX_PENSIZE = 4;
 
-  ns.PenSizeService = function () {};
+  /**
+   * Service to retrieve and modify the current pen size.
+   */
+  ns.PenSizeService = function () {
+    this.size = MIN_PENSIZE;
+  };
 
   ns.PenSizeService.prototype.init = function () {
-    this.size = pskl.UserSettings.get('PEN_SIZE');
+    this.size = pskl.UserSettings.get(pskl.UserSettings.PEN_SIZE);
 
     var shortcuts = pskl.service.keyboard.Shortcuts;
     pskl.app.shortcutService.registerShortcut(shortcuts.MISC.INCREASE_PENSIZE, this.increasePenSize_.bind(this));
@@ -22,10 +27,14 @@
     this.setPenSize(this.size - 1);
   };
 
+  ns.PenSizeService.prototype.getPenSize = function () {
+    return this.size;
+  };
+
   ns.PenSizeService.prototype.setPenSize = function (size) {
     if (this.isPenSizeValid_(size) && size != this.size) {
       this.size = size;
-      pskl.UserSettings.set('PEN_SIZE', size);
+      pskl.UserSettings.set(pskl.UserSettings.PEN_SIZE, size);
       $.publish(Events.PEN_SIZE_CHANGED);
     }
   };
@@ -38,32 +47,4 @@
     return size >= MIN_PENSIZE && size <= MAX_PENSIZE;
   };
 
-  ns.PenSizeService.prototype.getPenSize = function () {
-    return this.size;
-  };
-
-  ns.PenSizeService.prototype.getPixelsForPenSize = function (col, row, penSize) {
-    var size = penSize || this.size;
-    if (size == 1) {
-      return [[col, row]];
-    } else if (size == 2) {
-      return [
-        [col, row], [col + 1, row],
-        [col, row + 1], [col + 1, row + 1]
-      ];
-    } else if (size == 3) {
-      return [
-        [col - 1, row - 1], [col, row - 1], [col + 1, row - 1],
-        [col - 1, row + 0], [col, row + 0], [col + 1, row + 0],
-        [col - 1, row + 1], [col, row + 1], [col + 1, row + 1],
-      ];
-    } else if (size == 4) {
-      return [
-        [col - 1, row - 1], [col, row - 1], [col + 1, row - 1], [col + 2, row - 1],
-        [col - 1, row + 0], [col, row + 0], [col + 1, row + 0], [col + 2, row + 0],
-        [col - 1, row + 1], [col, row + 1], [col + 1, row + 1], [col + 2, row + 1],
-        [col - 1, row + 2], [col, row + 2], [col + 1, row + 2], [col + 2, row + 2],
-      ];
-    }
-  };
 })();

src/js/tools/drawing/BaseTool.js

@@ -49,15 +49,11 @@
     }
 
     var frameColor = frame.getPixel(col, row);
-
+    var highlightColor = this.getHighlightColor_(frameColor);
-    if (this.supportsDynamicPenSize()) {
+    var size = this.supportsDynamicPenSize() ? pskl.app.penSizeService.getPenSize() : 1;
-      var pixels = pskl.app.penSizeService.getPixelsForPenSize(col, row);
+    pskl.PixelUtils.resizePixel(col, row, size).forEach(function (point) {
-      pixels.forEach(function (p) {
+      overlay.setPixel(point[0], point[1], highlightColor);
-        overlay.setPixel(p[0], p[1], this.getHighlightColor_(frameColor));
+    });
-      }.bind(this));
-    } else {
-      overlay.setPixel(col, row, this.getHighlightColor_(frameColor));
-    }
 
     this.highlightedPixelCol = col;
     this.highlightedPixelRow = row;

src/js/tools/drawing/Circle.js

@@ -20,17 +20,10 @@
    * @override
    */
   ns.Circle.prototype.draw = function (col, row, color, targetFrame, penSize) {
-    var circlePoints = this.getCirclePixels_(this.startCol, this.startRow, col, row);
+    var circlePixels = this.getCirclePixels_(this.startCol, this.startRow, col, row);
-
+    pskl.PixelUtils.resizePixels(circlePixels, penSize).forEach(function (point) {
-    var applyDraw = function (p) {
+      targetFrame.setPixel(point[0], point[1], color);
-      targetFrame.setPixel(p[0], p[1], color);
+    });
-    }.bind(this);
-
-    for (var i = 0 ; i < circlePoints.length ; i++) {
-      // Change model:
-      var pixels = pskl.app.penSizeService.getPixelsForPenSize(circlePoints[i].col, circlePoints[i].row, penSize);
-      pixels.forEach(applyDraw);
-    }
   };
 
   ns.Circle.prototype.getCirclePixels_ = function (x0, y0, x1, y1) {

src/js/tools/drawing/DitheringTool.js

@@ -26,9 +26,10 @@
     this.previousCol = col;
     this.previousRow = row;
 
-    var pixels = pskl.app.penSizeService.getPixelsForPenSize(col, row);
+    var penSize = pskl.app.penSizeService.getPenSize();
-    pixels.forEach(function (p) {
+    var points = pskl.PixelUtils.resizePixel(col, row, penSize);
-      this.applyToolOnPixel(p[0], p[1], frame, overlay, event);
+    points.forEach(function (point) {
+      this.applyToolOnPixel(point[0], point[1], frame, overlay, event);
     }.bind(this));
   };
 

src/js/tools/drawing/Lighten.js

@@ -30,10 +30,11 @@
     this.previousCol = col;
     this.previousRow = row;
 
-    var pixels = pskl.app.penSizeService.getPixelsForPenSize(col, row);
+    var penSize = pskl.app.penSizeService.getPenSize();
-    pixels.forEach(function (p) {
+    var points = pskl.PixelUtils.resizePixel(col, row, penSize);
-      var modifiedColor = this.getModifiedColor_(p[0], p[1], frame, overlay, event);
+    points.forEach(function (point) {
-      this.draw(modifiedColor, p[0], p[1], frame, overlay);
+      var modifiedColor = this.getModifiedColor_(point[0], point[1], frame, overlay, event);
+      this.draw(modifiedColor, point[0], point[1], frame, overlay);
     }.bind(this));
   };
 

src/js/tools/drawing/Rectangle.js

@@ -20,16 +20,10 @@
    * @override
    */
   ns.Rectangle.prototype.draw = function (col, row, color, targetFrame, penSize) {
-    var strokePoints = pskl.PixelUtils.getBoundRectanglePixels(this.startCol, this.startRow, col, row);
+    var rectanglePixels = pskl.PixelUtils.getBoundRectanglePixels(this.startCol, this.startRow, col, row);
 
-    var applyDraw = function (p) {
+    pskl.PixelUtils.resizePixels(rectanglePixels, penSize).forEach(function (point) {
-      targetFrame.setPixel(p[0], p[1], color);
+      targetFrame.setPixel(point[0], point[1], color);
-    }.bind(this);
+    });
-
-    for (var i = 0 ; i < strokePoints.length ; i++) {
-      // Change model:
-      var pixels = pskl.app.penSizeService.getPixelsForPenSize(strokePoints[i].col, strokePoints[i].row, penSize);
-      pixels.forEach(applyDraw);
-    }
   };
 })();

src/js/tools/drawing/ShapeTool.js

@@ -29,7 +29,8 @@
     this.startRow = row;
 
     // Drawing the first point of the rectangle in the fake overlay canvas:
-    overlay.setPixel(col, row, this.getToolColor());
+    var penSize = pskl.app.penSizeService.getPenSize();
+    this.draw(col, row, this.getToolColor(), overlay, penSize);
   };
 
   ns.ShapeTool.prototype.moveToolAt = function(col, row, frame, overlay, event) {
@@ -43,7 +44,8 @@
     }
 
     // draw in overlay
-    this.draw(coords.col, coords.row, color, overlay);
+    var penSize = pskl.app.penSizeService.getPenSize();
+    this.draw(coords.col, coords.row, color, overlay, penSize);
   };
 
   /**
@@ -53,7 +55,8 @@
     overlay.clear();
     var coords = this.getCoordinates_(col, row, event);
     var color = this.getToolColor();
-    this.draw(coords.col, coords.row, color, frame);
+    var penSize = pskl.app.penSizeService.getPenSize();
+    this.draw(coords.col, coords.row, color, frame, penSize);
 
     $.publish(Events.DRAG_END);
     this.raiseSaveStateEvent({
@@ -62,7 +65,7 @@
       startCol : this.startCol,
       startRow : this.startRow,
       color : color,
-      penSize : pskl.app.penSizeService.getPenSize()
+      penSize : penSize
     });
   };
 

src/js/tools/drawing/SimplePen.js

@@ -36,9 +36,10 @@
   };
 
   ns.SimplePen.prototype.drawUsingPenSize = function(color, col, row, frame, overlay) {
-    var pixels = pskl.app.penSizeService.getPixelsForPenSize(col, row);
+    var penSize = pskl.app.penSizeService.getPenSize();
-    pixels.forEach(function (p) {
+    var points = pskl.PixelUtils.resizePixel(col, row, penSize);
-      this.draw(color, p[0], p[1], frame, overlay);
+    points.forEach(function (point) {
+      this.draw(color, point[0], point[1], frame, overlay);
     }.bind(this));
   };
 

src/js/tools/drawing/Stroke.js

@@ -47,85 +47,65 @@
   ns.Stroke.prototype.moveToolAt = function(col, row, frame, overlay, event) {
     overlay.clear();
 
-    if (event.shiftKey) {
+    var penSize = pskl.app.penSizeService.getPenSize();
-      var coords = this.getStraightLineCoordinates_(col, row);
+    var isStraight = event.shiftKey;
-      col = coords.col;
-      row = coords.row;
-    }
-
-    // When the user moussemove (before releasing), we dynamically compute the
-    // pixel to draw the line and draw this line in the overlay canvas:
-    var strokePoints = this.getLinePixels_(this.startCol, col, this.startRow, row);
-
-    // Drawing current stroke:
     var color = this.getToolColor();
-
+    if (color == Constants.TRANSPARENT_COLOR) {
-    var setPixel = function (p) {
+      // When mousemoving the stroke tool, we draw in the canvas overlay above the drawing canvas.
-      overlay.setPixel(p[0], p[1], color);
+      // If the stroke color is transparent, we won't be
-    }.bind(this);
+      // able to see it during the movement.
-
+      // We set it to a semi-opaque white during the tool mousemove allowing to see colors below the stroke.
-    for (var i = 0; i < strokePoints.length; i++) {
+      // When the stroke tool will be released, It will draw a transparent stroke,
-
+      // eg deleting the equivalent of a stroke.
-      if (color == Constants.TRANSPARENT_COLOR) {
+      color = Constants.SELECTION_TRANSPARENT_COLOR;
-        // When mousemoving the stroke tool, we draw in the canvas overlay above the drawing canvas.
-        // If the stroke color is transparent, we won't be
-        // able to see it during the movement.
-        // We set it to a semi-opaque white during the tool mousemove allowing to see colors below the stroke.
-        // When the stroke tool will be released, It will draw a transparent stroke,
-        // eg deleting the equivalent of a stroke.
-        color = Constants.SELECTION_TRANSPARENT_COLOR;
-      }
-
-      var pixels = pskl.app.penSizeService.getPixelsForPenSize(strokePoints[i].col, strokePoints[i].row);
-      pixels.forEach(setPixel);
     }
+
+    this.draw_(col, row, color, overlay, penSize, isStraight);
   };
 
   /**
    * @override
    */
   ns.Stroke.prototype.releaseToolAt = function(col, row, frame, overlay, event) {
+    var penSize = pskl.app.penSizeService.getPenSize();
+    var isStraight = event.shiftKey;
     var color = this.getToolColor();
 
-    if (event.shiftKey) {
+    // The user released the tool to draw a line. We will compute the pixel coordinate, impact
+    // the model and draw them in the drawing canvas (not the fake overlay anymore)
+    this.draw_(col, row, color, frame, penSize, isStraight);
+
+    // For now, we are done with the stroke tool and don't need an overlay anymore:
+    overlay.clear();
+
+    this.raiseSaveStateEvent({
+      col : col,
+      row : row,
+      startCol : this.startCol,
+      startRow : this.startRow,
+      color : color,
+      penSize : penSize,
+      isStraight : isStraight
+    });
+  };
+
+  ns.Stroke.prototype.draw_ = function (col, row, color, targetFrame, penSize, isStraight) {
+    if (isStraight) {
       var coords = this.getStraightLineCoordinates_(col, row);
       col = coords.col;
       row = coords.row;
     }
 
-    var setPixel = function (p) {
+    var linePixels = this.getLinePixels_(this.startCol, col, this.startRow, row);
-      frame.setPixel(p[0], p[1], color);
+    pskl.PixelUtils.resizePixels(linePixels, penSize).forEach(function (point) {
-    }.bind(this);
+      targetFrame.setPixel(point[0], point[1], color);
-
-    // The user released the tool to draw a line. We will compute the pixel coordinate, impact
-    // the model and draw them in the drawing canvas (not the fake overlay anymore)
-    var strokePoints = this.getLinePixels_(this.startCol, col, this.startRow, row);
-    for (var i = 0; i < strokePoints.length; i++) {
-      // Change model:
-
-      var pixels = pskl.app.penSizeService.getPixelsForPenSize(strokePoints[i].col, strokePoints[i].row);
-      pixels.forEach(setPixel);
-    }
-    // For now, we are done with the stroke tool and don't need an overlay anymore:
-    overlay.clear();
-
-    this.raiseSaveStateEvent({
-      pixels : strokePoints,
-      color : color,
-      penSize : pskl.app.penSizeService.getPenSize()
     });
   };
 
   ns.Stroke.prototype.replay = function(frame, replayData) {
-    var color = replayData.color;
+    this.startCol = replayData.startCol;
-    var setPixel = function (p) {
+    this.startRow = replayData.startRow;
-      frame.setPixel(p[0], p[1], color);
+    this.draw_(replayData.col, replayData.row, replayData.color, frame, replayData.penSize, replayData.isStraight);
-    }.bind(this);
-
-    replayData.pixels.forEach(function (pixel) {
-      var pixels = pskl.app.penSizeService.getPixelsForPenSize(pixel.col, pixel.row, replayData.penSize);
-      pixels.forEach(setPixel);
-    });
   };
 
   /**
@@ -139,7 +119,7 @@
     var dist = Math.sqrt(Math.pow(tCol, 2) + Math.pow(tRow, 2));
 
     var axisDistance = Math.round(dist);
-    var diagDistance = Math.round(Math.sqrt(Math.pow(dist, 2) / 2));
+    var diagDistance = Math.round(dist / Math.sqrt(2));
 
     var PI8 = Math.PI / 8;
     var angle = Math.atan2(tRow, tCol);

src/js/utils/PixelUtils.js

@@ -70,6 +70,53 @@
       return paintedPixels;
     },
 
+    /**
+     * Resize the pixel at {col, row} for the provided size. Will return the array of pixels centered
+     * around the original pixel, forming a pixel square of side=size
+     *
+     * @param  {Number} row  x-coordinate of the original pixel
+     * @param  {Number} col  y-coordinate of the original pixel
+     * @param  {Number} size >= 1 && <= 4
+     * @return {Array}  array of arrays of 2 Numbers (eg. [[0,0], [0,1], [1,0], [1,1]])
+     */
+    resizePixel : function (col, row, size) {
+      if (size == 1) {
+        return [[col, row]];
+      } else if (size == 2) {
+        return [
+          [col, row], [col + 1, row],
+          [col, row + 1], [col + 1, row + 1]
+        ];
+      } else if (size == 3) {
+        return [
+          [col - 1, row - 1], [col, row - 1], [col + 1, row - 1],
+          [col - 1, row + 0], [col, row + 0], [col + 1, row + 0],
+          [col - 1, row + 1], [col, row + 1], [col + 1, row + 1],
+        ];
+      } else if (size == 4) {
+        return [
+          [col - 1, row - 1], [col, row - 1], [col + 1, row - 1], [col + 2, row - 1],
+          [col - 1, row + 0], [col, row + 0], [col + 1, row + 0], [col + 2, row + 0],
+          [col - 1, row + 1], [col, row + 1], [col + 1, row + 1], [col + 2, row + 1],
+          [col - 1, row + 2], [col, row + 2], [col + 1, row + 2], [col + 2, row + 2],
+        ];
+      } else {
+        console.error('Unsupported size : ' + size);
+      }
+    },
+
+    /**
+     * Shortcut to reduce the output of pskl.PixelUtils.resizePixel for several pixels
+     * @param  {Array}  pixels Array of pixels (objects {col:Number, row:Number})
+     * @param  {Number} >= 1 && <= 4
+     * @return {Array}  array of arrays of 2 Numbers (eg. [[0,0], [0,1], [1,0], [1,1]])
+     */
+    resizePixels : function (pixels, size) {
+      return pixels.reduce(function (p, pixel) {
+        return p.concat(pskl.PixelUtils.resizePixel(pixel.col, pixel.row, size));
+      }, []);
+    },
+
     /**
      * Apply the paintbucket tool in a frame at the (col, row) initial position
      * with the replacement color.

test/js/service/pensize/PenSizeServiceTest.js

@@ -0,0 +1,73 @@
+describe("PenSize test suite", function() {
+  var penSizeService;
+  var userSettingsBackup;
+  var userSettingsPenSize;
+
+  beforeEach(function() {
+    userSettingsBackup = pskl.UserSettings;
+
+    pskl.UserSettings = {
+      PEN_SIZE : 'PEN_SIZE_TEST_KEY',
+      get : function () {
+        return userSettingsPenSize;
+      },
+
+      set : function (size) {
+        userSettingsPenSize = size;
+      }
+    };
+
+    spyOn(pskl.UserSettings, 'get').and.callThrough();
+    spyOn(pskl.UserSettings, 'set').and.callThrough();
+    spyOn($, 'publish').and.callThrough();
+
+
+    penSizeService = new pskl.service.pensize.PenSizeService();
+  });
+
+  afterEach(function () {
+    pskl.UserSettings = userSettingsBackup;
+  });
+
+  it("gets initial value from user settings", function() {
+    console.log('[PenSizeService] gets initial value from user settings');
+    userSettingsPenSize = 2;
+
+    penSizeService.init();
+    expect(penSizeService.getPenSize()).toBe(2);
+    expect(pskl.UserSettings.get).toHaveBeenCalledWith('PEN_SIZE_TEST_KEY');
+  });
+
+  it("saves valid value to user settings", function() {
+    console.log('[PenSizeService] saves valid value to user settings');
+    userSettingsPenSize = 1;
+
+    penSizeService.init();
+    penSizeService.setPenSize(3);
+    expect(penSizeService.getPenSize()).toBe(3);
+
+    expect(pskl.UserSettings.set).toHaveBeenCalledWith('PEN_SIZE_TEST_KEY', 3);
+    expect($.publish).toHaveBeenCalledWith(Events.PEN_SIZE_CHANGED);
+  });
+
+  it("skips invalid value (outside of [1, 4])", function() {
+    console.log('[PenSizeService] skips invalid value (outside of [1, 4])');
+    userSettingsPenSize = 1;
+
+    penSizeService.init();
+    // MAX_VALUE is 4
+    penSizeService.setPenSize(5);
+    expect(penSizeService.getPenSize()).toBe(1);
+    // MIN_VALUE is 1
+    penSizeService.setPenSize(0);
+    expect(penSizeService.getPenSize()).toBe(1);
+    // value should be a number
+    penSizeService.setPenSize("test");
+    expect(penSizeService.getPenSize()).toBe(1);
+
+    // nothing set in usersettings
+    expect(pskl.UserSettings.set.calls.any()).toBe(false);
+    // no event fired
+    expect($.publish.calls.any()).toBe(false);
+  });
+});