From f0ed4927e8fd75e99fc3b26e104164f722709eea Mon Sep 17 00:00:00 2001 From: jdescottes Date: Thu, 26 Nov 2015 23:35:33 +0100 Subject: [PATCH] Issue #258 : Move resize method to utils + add unit test --- src/js/service/pensize/PenSizeService.js | 43 +++----- src/js/tools/drawing/BaseTool.js | 14 +-- src/js/tools/drawing/Circle.js | 15 +-- src/js/tools/drawing/DitheringTool.js | 7 +- src/js/tools/drawing/Lighten.js | 9 +- src/js/tools/drawing/Rectangle.js | 14 +-- src/js/tools/drawing/ShapeTool.js | 11 +- src/js/tools/drawing/SimplePen.js | 7 +- src/js/tools/drawing/Stroke.js | 102 +++++++----------- src/js/utils/PixelUtils.js | 47 ++++++++ test/js/service/pensize/PenSizeServiceTest.js | 73 +++++++++++++ 11 files changed, 206 insertions(+), 136 deletions(-) create mode 100644 test/js/service/pensize/PenSizeServiceTest.js diff --git a/src/js/service/pensize/PenSizeService.js b/src/js/service/pensize/PenSizeService.js index 1fe2669d..5866cff2 100644 --- a/src/js/service/pensize/PenSizeService.js +++ b/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], - ]; - } - }; })(); diff --git a/src/js/tools/drawing/BaseTool.js b/src/js/tools/drawing/BaseTool.js index f306496c..4b58408a 100644 --- a/src/js/tools/drawing/BaseTool.js +++ b/src/js/tools/drawing/BaseTool.js @@ -49,15 +49,11 @@ } var frameColor = frame.getPixel(col, row); - - if (this.supportsDynamicPenSize()) { - var pixels = pskl.app.penSizeService.getPixelsForPenSize(col, row); - pixels.forEach(function (p) { - overlay.setPixel(p[0], p[1], this.getHighlightColor_(frameColor)); - }.bind(this)); - } else { - overlay.setPixel(col, row, this.getHighlightColor_(frameColor)); - } + var highlightColor = this.getHighlightColor_(frameColor); + var size = this.supportsDynamicPenSize() ? pskl.app.penSizeService.getPenSize() : 1; + pskl.PixelUtils.resizePixel(col, row, size).forEach(function (point) { + overlay.setPixel(point[0], point[1], highlightColor); + }); this.highlightedPixelCol = col; this.highlightedPixelRow = row; diff --git a/src/js/tools/drawing/Circle.js b/src/js/tools/drawing/Circle.js index bf0ec702..05cfb1e7 100644 --- a/src/js/tools/drawing/Circle.js +++ b/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 applyDraw = function (p) { - 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); - } + var circlePixels = this.getCirclePixels_(this.startCol, this.startRow, col, row); + pskl.PixelUtils.resizePixels(circlePixels, penSize).forEach(function (point) { + targetFrame.setPixel(point[0], point[1], color); + }); }; ns.Circle.prototype.getCirclePixels_ = function (x0, y0, x1, y1) { diff --git a/src/js/tools/drawing/DitheringTool.js b/src/js/tools/drawing/DitheringTool.js index 12b65d3a..fa469c03 100644 --- a/src/js/tools/drawing/DitheringTool.js +++ b/src/js/tools/drawing/DitheringTool.js @@ -26,9 +26,10 @@ this.previousCol = col; this.previousRow = row; - var pixels = pskl.app.penSizeService.getPixelsForPenSize(col, row); - pixels.forEach(function (p) { - this.applyToolOnPixel(p[0], p[1], frame, overlay, event); + var penSize = pskl.app.penSizeService.getPenSize(); + var points = pskl.PixelUtils.resizePixel(col, row, penSize); + points.forEach(function (point) { + this.applyToolOnPixel(point[0], point[1], frame, overlay, event); }.bind(this)); }; diff --git a/src/js/tools/drawing/Lighten.js b/src/js/tools/drawing/Lighten.js index f2820b16..2ad6e3c3 100644 --- a/src/js/tools/drawing/Lighten.js +++ b/src/js/tools/drawing/Lighten.js @@ -30,10 +30,11 @@ this.previousCol = col; this.previousRow = row; - var pixels = pskl.app.penSizeService.getPixelsForPenSize(col, row); - pixels.forEach(function (p) { - var modifiedColor = this.getModifiedColor_(p[0], p[1], frame, overlay, event); - this.draw(modifiedColor, p[0], p[1], frame, overlay); + var penSize = pskl.app.penSizeService.getPenSize(); + var points = pskl.PixelUtils.resizePixel(col, row, penSize); + points.forEach(function (point) { + var modifiedColor = this.getModifiedColor_(point[0], point[1], frame, overlay, event); + this.draw(modifiedColor, point[0], point[1], frame, overlay); }.bind(this)); }; diff --git a/src/js/tools/drawing/Rectangle.js b/src/js/tools/drawing/Rectangle.js index e1b8fe28..9b83d1bf 100644 --- a/src/js/tools/drawing/Rectangle.js +++ b/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) { - targetFrame.setPixel(p[0], p[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); - } + pskl.PixelUtils.resizePixels(rectanglePixels, penSize).forEach(function (point) { + targetFrame.setPixel(point[0], point[1], color); + }); }; })(); diff --git a/src/js/tools/drawing/ShapeTool.js b/src/js/tools/drawing/ShapeTool.js index be33f95a..837809f5 100644 --- a/src/js/tools/drawing/ShapeTool.js +++ b/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 }); }; diff --git a/src/js/tools/drawing/SimplePen.js b/src/js/tools/drawing/SimplePen.js index 12b450c0..f4d3fe81 100644 --- a/src/js/tools/drawing/SimplePen.js +++ b/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); - pixels.forEach(function (p) { - this.draw(color, p[0], p[1], frame, overlay); + var penSize = pskl.app.penSizeService.getPenSize(); + var points = pskl.PixelUtils.resizePixel(col, row, penSize); + points.forEach(function (point) { + this.draw(color, point[0], point[1], frame, overlay); }.bind(this)); }; diff --git a/src/js/tools/drawing/Stroke.js b/src/js/tools/drawing/Stroke.js index 529dbd5c..527d327e 100644 --- a/src/js/tools/drawing/Stroke.js +++ b/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 coords = this.getStraightLineCoordinates_(col, row); - 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 penSize = pskl.app.penSizeService.getPenSize(); + var isStraight = event.shiftKey; var color = this.getToolColor(); - - var setPixel = function (p) { - overlay.setPixel(p[0], p[1], color); - }.bind(this); - - for (var i = 0; i < strokePoints.length; i++) { - - if (color == Constants.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); + if (color == Constants.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; } + + 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) { - frame.setPixel(p[0], p[1], color); - }.bind(this); - - // 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() + var linePixels = this.getLinePixels_(this.startCol, col, this.startRow, row); + pskl.PixelUtils.resizePixels(linePixels, penSize).forEach(function (point) { + targetFrame.setPixel(point[0], point[1], color); }); }; ns.Stroke.prototype.replay = function(frame, replayData) { - var color = replayData.color; - var setPixel = function (p) { - frame.setPixel(p[0], p[1], color); - }.bind(this); - - replayData.pixels.forEach(function (pixel) { - var pixels = pskl.app.penSizeService.getPixelsForPenSize(pixel.col, pixel.row, replayData.penSize); - pixels.forEach(setPixel); - }); + this.startCol = replayData.startCol; + this.startRow = replayData.startRow; + this.draw_(replayData.col, replayData.row, replayData.color, frame, replayData.penSize, replayData.isStraight); }; /** @@ -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); diff --git a/src/js/utils/PixelUtils.js b/src/js/utils/PixelUtils.js index ae42e736..d6d77fe5 100644 --- a/src/js/utils/PixelUtils.js +++ b/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. diff --git a/test/js/service/pensize/PenSizeServiceTest.js b/test/js/service/pensize/PenSizeServiceTest.js new file mode 100644 index 00000000..3fd48c13 --- /dev/null +++ b/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); + }); +}); \ No newline at end of file