From e0c9a46ed31a7316aba6248bc3e12a1ebca7116e Mon Sep 17 00:00:00 2001
From: jdescottes <julian.descottes@gmail.com>
Date: Fri, 7 Aug 2015 08:37:13 +0200
Subject: [PATCH 1/5] wip : needs tests

---
 src/js/model/Frame.js                   |  2 +-
 src/js/rendering/CanvasRenderer.js      | 22 ++-------------
 src/js/rendering/frame/FrameRenderer.js | 28 ++-----------------
 src/js/selection/BaseSelection.js       |  6 ++++
 src/js/utils/FrameUtils.js              | 37 +++++++++++++++++++++++++
 5 files changed, 48 insertions(+), 47 deletions(-)

diff --git a/src/js/model/Frame.js b/src/js/model/Frame.js
index 677b7eab..51958fa7 100644
--- a/src/js/model/Frame.js
+++ b/src/js/model/Frame.js
@@ -88,7 +88,7 @@
     if (this.containsPixel(x, y)) {
       var p = this.pixels[x][y];
       if (p !== color) {
-        this.pixels[x][y] = color;
+        this.pixels[x][y] = color || Constants.TRANSPARENT_COLOR;
         this.version++;
       }
     }
diff --git a/src/js/rendering/CanvasRenderer.js b/src/js/rendering/CanvasRenderer.js
index 434de61b..8e38ecdd 100644
--- a/src/js/rendering/CanvasRenderer.js
+++ b/src/js/rendering/CanvasRenderer.js
@@ -18,19 +18,9 @@
 
   ns.CanvasRenderer.prototype.render = function  () {
     var canvas = this.createCanvas_();
-    var context = canvas.getContext('2d');
 
-    for (var x = 0, width = this.frame.getWidth(); x < width; x++) {
-      for (var y = 0, height = this.frame.getHeight(); y < height; y++) {
-        var color = this.frame.getPixel(x, y);
-        var w = 1;
-        while (color === this.frame.getPixel(x, y + w)) {
-          w++;
-        }
-        this.renderLine_(color, x, y, w, context);
-        y = y + w - 1;
-      }
-    }
+    // Draw in canvas
+    pskl.utils.FrameUtils.drawToCanvas(this.frame, canvas, this.transparentColor_);
 
     var scaledCanvas = this.createCanvas_(this.zoom);
     var scaledContext = scaledCanvas.getContext('2d');
@@ -49,14 +39,6 @@
     context.fillRect(x, y, 1, 1);
   };
 
-  ns.CanvasRenderer.prototype.renderLine_ = function (color, x, y, width, context) {
-    if (color == Constants.TRANSPARENT_COLOR) {
-      color = this.transparentColor_;
-    }
-    context.fillStyle = color;
-    context.fillRect(x, y, 1, width);
-  };
-
   ns.CanvasRenderer.prototype.createCanvas_ = function (zoom) {
     zoom = zoom || 1;
     var width = this.frame.getWidth() * zoom;
diff --git a/src/js/rendering/frame/FrameRenderer.js b/src/js/rendering/frame/FrameRenderer.js
index e200f3fe..6d584594 100644
--- a/src/js/rendering/frame/FrameRenderer.js
+++ b/src/js/rendering/frame/FrameRenderer.js
@@ -219,18 +219,8 @@
       this.canvas = pskl.utils.CanvasUtils.createCanvas(frame.getWidth(), frame.getHeight());
     }
 
-    var context = this.canvas.getContext('2d');
-    for (var x = 0, width = frame.getWidth() ; x < width ; x++) {
-      for (var y = 0, height = frame.getHeight() ; y < height ; y++) {
-        var color = frame.getPixel(x, y);
-        var w = 1;
-        while (color === frame.getPixel(x, y + w)) {
-          w++;
-        }
-        this.renderLine_(color, x, y, w, context);
-        y = y + w - 1;
-      }
-    }
+    // Draw in canvas
+    pskl.utils.FrameUtils.drawToCanvas(frame, this.canvas);
 
     this.updateMargins_(frame);
 
@@ -264,18 +254,4 @@
     }
     displayContext.restore();
   };
-
-  ns.FrameRenderer.prototype.renderPixel_ = function (color, x, y, context) {
-    if (color != Constants.TRANSPARENT_COLOR) {
-      context.fillStyle = color;
-      context.fillRect(x, y, 1, 1);
-    }
-  };
-
-  ns.FrameRenderer.prototype.renderLine_ = function (color, x, y, width, context) {
-    if (color != Constants.TRANSPARENT_COLOR) {
-      context.fillStyle = color;
-      context.fillRect(x, y, 1, width);
-    }
-  };
 })();
diff --git a/src/js/selection/BaseSelection.js b/src/js/selection/BaseSelection.js
index fd733e13..1551a922 100644
--- a/src/js/selection/BaseSelection.js
+++ b/src/js/selection/BaseSelection.js
@@ -24,9 +24,15 @@
   };
 
   ns.BaseSelection.prototype.fillSelectionFromFrame = function (targetFrame) {
+    // on copy trim the selection if out of bounds
+    // this.pixels = this.pixels.filter(function (pixel) {
+    //   return targetFrame.containsPixel(pixel.col, pixel.row);
+    // });
+
     this.pixels.forEach(function (pixel) {
       pixel.color = targetFrame.getPixel(pixel.col, pixel.row);
     });
+
     this.hasPastedContent = true;
   };
 })();
diff --git a/src/js/utils/FrameUtils.js b/src/js/utils/FrameUtils.js
index 11e50986..54ebb096 100644
--- a/src/js/utils/FrameUtils.js
+++ b/src/js/utils/FrameUtils.js
@@ -16,6 +16,43 @@
       return canvasRenderer.render();
     },
 
+    /**
+     * Draw the provided frame in a 2d canvas
+     *
+     * @param  {pskl.model.Frame} frame the frame to draw
+     * @param  {Canvas} canvas the canvas target
+     * @param  {String} transparentColor (optional) color to use to represent transparent pixels.
+     */
+    drawToCanvas : function (frame, canvas, transparentColor) {
+      var context = canvas.getContext('2d');
+
+      transparentColor = transparentColor || Constants.TRANSPARENT_COLOR;
+      for (var x = 0, width = frame.getWidth() ; x < width ; x++) {
+        for (var y = 0, height = frame.getHeight() ; y < height ; y++) {
+          var color = frame.getPixel(x, y);
+          var w = 1;
+          while (color === frame.getPixel(x, y + w) && (y + w) < height) {
+            w++;
+          }
+
+          if (color == Constants.TRANSPARENT_COLOR) {
+            color = transparentColor;
+          }
+
+          pskl.utils.FrameUtils.renderLine_(color, x, y, w, context);
+          y = y + w - 1;
+        }
+      }
+
+    },
+
+    renderLine_ : function (color, x, y, width, context) {
+      if (color != Constants.TRANSPARENT_COLOR) {
+        context.fillStyle = color;
+        context.fillRect(x, y, 1, width);
+      }
+    },
+
     merge : function (frames) {
       var merged = null;
       if (frames.length) {

From c0742170476c6e9c0512822045507b63ca0d8643 Mon Sep 17 00:00:00 2001
From: juliandescottes <julian.descottes@gmail.com>
Date: Sun, 9 Aug 2015 01:22:25 +0200
Subject: [PATCH 2/5] Add macos specific nodewebkit configuration

---
 Gruntfile.js | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/Gruntfile.js b/Gruntfile.js
index c7b10200..f52e29dc 100644
--- a/Gruntfile.js
+++ b/Gruntfile.js
@@ -217,15 +217,24 @@ module.exports = function(grunt) {
       }
     },
     nodewebkit: {
-      options: {
-        version : "0.11.5",
-        build_dir: './dest/desktop/', // destination folder of releases.
-        mac: true,
-        win: true,
-        linux32: true,
-        linux64: true
+      windows : {
+        options: {
+          version : "0.11.5",
+          build_dir: './dest/desktop/', // destination folder of releases.
+          win: true,
+          linux32: true,
+          linux64: true
+        },
+        src: ['./dest/**/*', "./package.json", "!./dest/desktop/"]
       },
-      src: ['./dest/**/*', "./package.json", "!./dest/desktop/"]
+      macos : {
+        options: {
+          platforms : ['osx64'],
+          version : "0.10.5",
+          build_dir: './dest/desktop/'
+        },
+        src: ['./dest/**/*', "./package.json", "!./dest/desktop/"]
+      }
     }
   });
 

From 1208324d4d4ccee8d6a9a9a27712672f824dc37f Mon Sep 17 00:00:00 2001
From: juliandescottes <julian.descottes@gmail.com>
Date: Sun, 9 Aug 2015 12:37:03 +0200
Subject: [PATCH 3/5] Copy paste bug : add unit tests for FrameUtils with null
 value

---
 src/js/app.js                                 |   4 +-
 .../controller/preview/PreviewController.js   |   3 -
 src/js/rendering/CanvasRenderer.js            |   8 --
 src/js/selection/BaseSelection.js             |   8 +-
 src/js/selection/SelectionManager.js          |  21 ++-
 src/js/utils/FrameUtils.js                    |  30 +++--
 src/js/utils/PixelUtils.js                    |   2 +-
 test/js/testutils/TestUtils.js                |  22 +++
 test/js/utils/FrameUtilsTest.js               | 125 +++++++++++-------
 9 files changed, 131 insertions(+), 92 deletions(-)

diff --git a/src/js/app.js b/src/js/app.js
index 23ea52a8..b8dbd171 100644
--- a/src/js/app.js
+++ b/src/js/app.js
@@ -192,9 +192,7 @@
 
     getFirstFrameAsPng : function () {
       var firstFrame = this.piskelController.getFrameAt(0);
-      var canvasRenderer = new pskl.rendering.CanvasRenderer(firstFrame, 1);
-      canvasRenderer.drawTransparentAs('rgba(0,0,0,0)');
-      var firstFrameCanvas = canvasRenderer.render();
+      var firstFrameCanvas = pskl.utils.FrameUtils.toImage(firstFrame);
       return firstFrameCanvas.toDataURL('image/png');
     },
 
diff --git a/src/js/controller/preview/PreviewController.js b/src/js/controller/preview/PreviewController.js
index 1c12ccdc..5526839c 100644
--- a/src/js/controller/preview/PreviewController.js
+++ b/src/js/controller/preview/PreviewController.js
@@ -25,9 +25,6 @@
   };
 
   ns.PreviewController.prototype.init = function () {
-    // the oninput event won't work on IE10 unfortunately, but at least will provide a
-    // consistent behavior across all other browsers that support the input type range
-    // see https://bugzilla.mozilla.org/show_bug.cgi?id=853670
     this.fpsRangeInput.on('input change', this.onFPSSliderChange.bind(this));
     document.querySelector('.right-column').style.width = Constants.ANIMATED_PREVIEW_WIDTH + 'px';
 
diff --git a/src/js/rendering/CanvasRenderer.js b/src/js/rendering/CanvasRenderer.js
index 8e38ecdd..9dfe9e07 100644
--- a/src/js/rendering/CanvasRenderer.js
+++ b/src/js/rendering/CanvasRenderer.js
@@ -31,14 +31,6 @@
     return scaledCanvas;
   };
 
-  ns.CanvasRenderer.prototype.renderPixel_ = function (color, x, y, context) {
-    if (color == Constants.TRANSPARENT_COLOR) {
-      color = this.transparentColor_;
-    }
-    context.fillStyle = color;
-    context.fillRect(x, y, 1, 1);
-  };
-
   ns.CanvasRenderer.prototype.createCanvas_ = function (zoom) {
     zoom = zoom || 1;
     var width = this.frame.getWidth() * zoom;
diff --git a/src/js/selection/BaseSelection.js b/src/js/selection/BaseSelection.js
index 1551a922..0907b5dd 100644
--- a/src/js/selection/BaseSelection.js
+++ b/src/js/selection/BaseSelection.js
@@ -24,13 +24,9 @@
   };
 
   ns.BaseSelection.prototype.fillSelectionFromFrame = function (targetFrame) {
-    // on copy trim the selection if out of bounds
-    // this.pixels = this.pixels.filter(function (pixel) {
-    //   return targetFrame.containsPixel(pixel.col, pixel.row);
-    // });
-
     this.pixels.forEach(function (pixel) {
-      pixel.color = targetFrame.getPixel(pixel.col, pixel.row);
+      var color = targetFrame.getPixel(pixel.col, pixel.row);
+      pixel.color  = color || Constants.TRANSPARENT_COLOR;
     });
 
     this.hasPastedContent = true;
diff --git a/src/js/selection/SelectionManager.js b/src/js/selection/SelectionManager.js
index c9ade102..ea64fc58 100644
--- a/src/js/selection/SelectionManager.js
+++ b/src/js/selection/SelectionManager.js
@@ -88,20 +88,18 @@
   };
 
   ns.SelectionManager.prototype.paste = function() {
-    if (this.currentSelection && this.currentSelection.hasPastedContent) {
-      var pixels = this.currentSelection.pixels;
-      var opaquePixels = pixels.filter(function (p) {
-        return p.color !== Constants.TRANSPARENT_COLOR;
-      });
-      this.pastePixels(opaquePixels);
+    if (!this.currentSelection || !this.currentSelection.hasPastedContent) {
+      return;
     }
-  };
 
-  ns.SelectionManager.prototype.pastePixels = function(pixels) {
-    var currentFrame = this.piskelController.getCurrentFrame();
+    var pixels = this.currentSelection.pixels;
+    var frame = this.piskelController.getCurrentFrame();
 
     pixels.forEach(function (pixel) {
-      currentFrame.setPixel(pixel.col, pixel.row, pixel.color);
+      if (pixel.color === Constants.TRANSPARENT_COLOR || pixel.color === null) {
+        return;
+      }
+      frame.setPixel(pixel.col, pixel.row, pixel.color);
     });
 
     $.publish(Events.PISKEL_SAVE_STATE, {
@@ -115,8 +113,7 @@
   };
 
   ns.SelectionManager.prototype.replay = function (frame, replayData) {
-    var pixels = replayData.pixels;
-    pixels.forEach(function (pixel) {
+    replayData.pixels.forEach(function (pixel) {
       var color = replayData.type === SELECTION_REPLAY.PASTE ? pixel.color : Constants.TRANSPARENT_COLOR;
       frame.setPixel(pixel.col, pixel.row, color);
     });
diff --git a/src/js/utils/FrameUtils.js b/src/js/utils/FrameUtils.js
index 54ebb096..01393d23 100644
--- a/src/js/utils/FrameUtils.js
+++ b/src/js/utils/FrameUtils.js
@@ -5,8 +5,8 @@
     /**
      * Render a Frame object as an image.
      * Can optionally scale it (zoom)
-     * @param  {Frame} frame
-     * @param  {Number} zoom
+     * @param frame {Frame} frame
+     * @param zoom {Number} zoom
      * @return {Image}
      */
     toImage : function (frame, zoom) {
@@ -19,9 +19,9 @@
     /**
      * Draw the provided frame in a 2d canvas
      *
-     * @param  {pskl.model.Frame} frame the frame to draw
-     * @param  {Canvas} canvas the canvas target
-     * @param  {String} transparentColor (optional) color to use to represent transparent pixels.
+     * @param frame {pskl.model.Frame} frame the frame to draw
+     * @param canvas {Canvas} canvas the canvas target
+     * @param transparentColor {String} transparentColor (optional) color to use to represent transparent pixels.
      */
     drawToCanvas : function (frame, canvas, transparentColor) {
       var context = canvas.getContext('2d');
@@ -30,6 +30,9 @@
       for (var x = 0, width = frame.getWidth() ; x < width ; x++) {
         for (var y = 0, height = frame.getHeight() ; y < height ; y++) {
           var color = frame.getPixel(x, y);
+
+          // accumulate all the pixels of the same color to speed up rendering
+          // by reducting fillRect calls
           var w = 1;
           while (color === frame.getPixel(x, y + w) && (y + w) < height) {
             w++;
@@ -43,14 +46,23 @@
           y = y + w - 1;
         }
       }
-
     },
 
+    /**
+     * Render a line of a single color in a given canvas 2D context.
+     * 
+     * @param  color {String} color to draw
+     * @param  x {Number} x coordinate 
+     * @param  y {Number} y coordinate
+     * @param  width {Number} width of the line to draw, in pixels
+     * @param  context {CanvasRenderingContext2D} context of the canvas target
+     */
     renderLine_ : function (color, x, y, width, context) {
-      if (color != Constants.TRANSPARENT_COLOR) {
-        context.fillStyle = color;
-        context.fillRect(x, y, 1, width);
+      if (color === Constants.TRANSPARENT_COLOR || color === null) {
+        return;
       }
+      context.fillStyle = color;
+      context.fillRect(x, y, 1, width);
     },
 
     merge : function (frames) {
diff --git a/src/js/utils/PixelUtils.js b/src/js/utils/PixelUtils.js
index 81b47e71..9400e709 100644
--- a/src/js/utils/PixelUtils.js
+++ b/src/js/utils/PixelUtils.js
@@ -128,7 +128,7 @@
           var nextCol = currentItem.col + dx[i];
           var nextRow = currentItem.row + dy[i];
           try {
-            if (frame.containsPixel(nextCol, nextRow)  && frame.getPixel(nextCol, nextRow) == targetColor) {
+            if (frame.containsPixel(nextCol, nextRow) && frame.getPixel(nextCol, nextRow) == targetColor) {
               queue.push({'col': nextCol, 'row': nextRow});
             }
           } catch (e) {
diff --git a/test/js/testutils/TestUtils.js b/test/js/testutils/TestUtils.js
index bf5de19f..5153bec4 100644
--- a/test/js/testutils/TestUtils.js
+++ b/test/js/testutils/TestUtils.js
@@ -1,6 +1,28 @@
 (function () {
   var ns = $.namespace('test.testutils');
 
+  /**
+   * Frame.createFromGrid accepts grids that are rotated by 90deg from
+   * the visual/usual way. (column-based grid)
+   *
+   * For testing, it's easier for be able to specify a row-based grid, because
+   * it visually matches what the image will look like.
+   *
+   * For instance :
+   *
+   * [[black, black, black],
+   *  [white, white, white]]
+   *
+   * we expect this to be a 3x2 image, one black line above a white line.
+   *
+   * However Frame.createFromGrid needs the following input to create such an image :
+   * 
+   * [[black, white],
+   *  [black, white],
+   *  [black, white]] 
+   *
+   * This helper will build the second array from the first array.
+   */
   ns.toFrameGrid = function (normalGrid) {
     var frameGrid = [];
     var w = normalGrid[0].length;
diff --git a/test/js/utils/FrameUtilsTest.js b/test/js/utils/FrameUtilsTest.js
index ea1b2d41..40313012 100644
--- a/test/js/utils/FrameUtilsTest.js
+++ b/test/js/utils/FrameUtilsTest.js
@@ -3,35 +3,41 @@ describe("FrameUtils suite", function() {
   var red = '#ff0000';
   var transparent = Constants.TRANSPARENT_COLOR;
 
+  // shortcuts
+  var toFrameGrid = test.testutils.toFrameGrid;
+  var frameEqualsGrid = test.testutils.frameEqualsGrid;
+
   it("merges 2 frames", function () {
+    var B = black, R = red, T = transparent;
     var frame1 = pskl.model.Frame.fromPixelGrid([
-      [black, transparent],
-      [transparent, black]
+      [B, T],
+      [T, B]
     ]);
 
     var frame2 = pskl.model.Frame.fromPixelGrid([
-      [transparent, red],
-      [red, transparent]
+      [T, R],
+      [R, T]
     ]);
 
     var mergedFrame = pskl.utils.FrameUtils.merge([frame1, frame2]);
-    expect(mergedFrame.getPixel(0,0)).toBe(black);
-    expect(mergedFrame.getPixel(0,1)).toBe(red);
-    expect(mergedFrame.getPixel(1,0)).toBe(red);
-    expect(mergedFrame.getPixel(1,1)).toBe(black);
+    frameEqualsGrid(mergedFrame, [
+      [B, R],
+      [R, B]
+    ]);
   });
 
   it("returns same frame when merging single frame", function () {
-    var frame1 = pskl.model.Frame.fromPixelGrid([
-      [black, transparent],
-      [transparent, black]
-    ]);
+    var B = black, T = transparent;
+    var frame1 = pskl.model.Frame.fromPixelGrid(toFrameGrid([
+      [B, T],
+      [B, T]
+    ]));
 
     var mergedFrame = pskl.utils.FrameUtils.merge([frame1]);
-    expect(mergedFrame.getPixel(0,0)).toBe(black);
-    expect(mergedFrame.getPixel(0,1)).toBe(transparent);
-    expect(mergedFrame.getPixel(1,0)).toBe(transparent);
-    expect(mergedFrame.getPixel(1,1)).toBe(black);
+    frameEqualsGrid(mergedFrame, [
+      [B, T],
+      [B, T]
+    ]);
   });
 
   var checkPixelsColor = function (frame, pixels, color) {
@@ -42,9 +48,10 @@ describe("FrameUtils suite", function() {
   };
 
   it ("converts an image to a frame", function () {
+    var B = black, T = transparent;
     var frame1 = pskl.model.Frame.fromPixelGrid([
-      [black, transparent],
-      [transparent, black]
+      [B, T],
+      [T, B]
     ]);
 
     var image = pskl.utils.FrameUtils.toImage(frame1);
@@ -57,48 +64,66 @@ describe("FrameUtils suite", function() {
 
     var biggerFrame = pskl.utils.FrameUtils.createFromImage(biggerImage);
 
-    checkPixelsColor(biggerFrame, [
-      [0,0],[0,1],[0,2],
-      [1,0],[1,1],[1,2],
-      [2,0],[2,1],[2,2],
-      [3,3],[3,4],[3,5],
-      [4,3],[4,4],[4,5],
-      [5,3],[5,4],[5,5]
-    ], black);
-
-    checkPixelsColor(biggerFrame, [
-      [0,3],[0,4],[0,5],
-      [1,3],[1,4],[1,5],
-      [2,3],[2,4],[2,5],
-      [3,0],[3,1],[3,2],
-      [4,0],[4,1],[4,2],
-      [5,0],[5,1],[5,2]
-    ], transparent);
+    frameEqualsGrid(biggerFrame, [
+      [B, B, B, T, T, T],
+      [B, B, B, T, T, T],
+      [B, B, B, T, T, T],
+      [T, T, T, B, B, B],
+      [T, T, T, B, B, B],
+      [T, T, T, B, B, B]
+    ]);
   });
 
   it ("[LayerUtils] creates a layer from a simple spritesheet", function () {
-    var frame = pskl.model.Frame.fromPixelGrid([
-      [black, red],
-      [red, black],
-      [black, black],
-      [red, red]
-    ]);
+    var B = black, R = red;
+
+    // original image in 4x2
+    var frame = pskl.model.Frame.fromPixelGrid(toFrameGrid([
+      [B, R, B, R],
+      [R, B, B, R]
+    ]));
+
     var spritesheet = pskl.utils.FrameUtils.toImage(frame);
 
+    // split the spritesheet by 4
     var frames = pskl.utils.LayerUtils.createLayerFromSpritesheet(spritesheet, 4);
+
+    // expect 4 frames of 1x2
     expect(frames.length).toBe(4);
 
-    expect(frames[0].getPixel(0,0)).toBe(black);
-    expect(frames[0].getPixel(0,1)).toBe(red);
+    // verify frame content
+    frameEqualsGrid(frames[0], [
+      [B],
+      [R]
+    ]);
+    frameEqualsGrid(frames[1], [
+      [R],
+      [B]
+    ]);
+    frameEqualsGrid(frames[2], [
+      [B],
+      [B]
+    ]);
+    frameEqualsGrid(frames[3], [
+      [R],
+      [R]
+    ]);
+  });
 
-    expect(frames[1].getPixel(0,0)).toBe(red);
-    expect(frames[1].getPixel(0,1)).toBe(black);
+  it ("supports null values in frame array", function () {
+    var B = black, T = transparent;
+    var frame = pskl.model.Frame.fromPixelGrid([
+      [B, null],
+      [null, B]
+    ]);
 
-    expect(frames[2].getPixel(0,0)).toBe(black);
-    expect(frames[2].getPixel(0,1)).toBe(black);
-
-    expect(frames[3].getPixel(0,0)).toBe(red);
-    expect(frames[3].getPixel(0,1)).toBe(red);
+    var image = pskl.utils.FrameUtils.toImage(frame);
 
+    // transform back to frame for ease of testing
+    var testFrame = pskl.utils.FrameUtils.createFromImage(image);
+    frameEqualsGrid(testFrame, [
+      [B, T],
+      [T, B]
+    ]);
   });
 });
\ No newline at end of file

From fdb5483e87e832cbaef8beca344aa71c7b096f5a Mon Sep 17 00:00:00 2001
From: juliandescottes <julian.descottes@gmail.com>
Date: Sun, 9 Aug 2015 12:51:25 +0200
Subject: [PATCH 4/5] JSCS fixes

---
 src/js/utils/FrameUtils.js | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/js/utils/FrameUtils.js b/src/js/utils/FrameUtils.js
index 01393d23..346abd45 100644
--- a/src/js/utils/FrameUtils.js
+++ b/src/js/utils/FrameUtils.js
@@ -50,9 +50,9 @@
 
     /**
      * Render a line of a single color in a given canvas 2D context.
-     * 
+     *
      * @param  color {String} color to draw
-     * @param  x {Number} x coordinate 
+     * @param  x {Number} x coordinate
      * @param  y {Number} y coordinate
      * @param  width {Number} width of the line to draw, in pixels
      * @param  context {CanvasRenderingContext2D} context of the canvas target

From 99da69553c41038ca9e5d487f20f9bf05df32cc3 Mon Sep 17 00:00:00 2001
From: juliandescottes <julian.descottes@gmail.com>
Date: Sun, 9 Aug 2015 15:42:46 +0200
Subject: [PATCH 5/5] Copy paste out of bounds : added SelectionManager unit
 tests

---
 karma.conf.js                                |   2 +
 src/js/tools/drawing/selection/BaseSelect.js |  10 +-
 test/js/selection/SelectionManagerTest.js    | 206 +++++++++++++++++++
 3 files changed, 213 insertions(+), 5 deletions(-)
 create mode 100644 test/js/selection/SelectionManagerTest.js

diff --git a/karma.conf.js b/karma.conf.js
index 8fb83788..4292a77f 100644
--- a/karma.conf.js
+++ b/karma.conf.js
@@ -6,7 +6,9 @@ module.exports = function(config) {
   var mapToSrcFolder = function (path) {return ['src', path].join('/');};
 
   var piskelScripts = require('./src/piskel-script-list.js').scripts.map(mapToSrcFolder);
+  piskelScripts.push('test/js/testutils/**/*.js');
   piskelScripts.push('test/js/**/*.js');
+  
   config.set({
 
     // base path that will be used to resolve all patterns (eg. files, exclude)
diff --git a/src/js/tools/drawing/selection/BaseSelect.js b/src/js/tools/drawing/selection/BaseSelect.js
index 60c61c1c..2c1aaf99 100644
--- a/src/js/tools/drawing/selection/BaseSelect.js
+++ b/src/js/tools/drawing/selection/BaseSelect.js
@@ -8,7 +8,7 @@
 
   ns.BaseSelect = function() {
     this.secondaryToolId = pskl.tools.drawing.Move.TOOL_ID;
-    this.BodyRoot = $('body');
+    this.bodyRoot = $('body');
 
     // Select's first point coordinates (set in applyToolAt)
     this.startCol = null;
@@ -81,12 +81,12 @@
     if (overlay.containsPixel(col, row)) {
       if (this.isInSelection(col, row)) {
         // We're hovering the selection, show the move tool:
-        this.BodyRoot.addClass(this.secondaryToolId);
-        this.BodyRoot.removeClass(this.toolId);
+        this.bodyRoot.addClass(this.secondaryToolId);
+        this.bodyRoot.removeClass(this.toolId);
       } else {
         // We're not hovering the selection, show create selection tool:
-        this.BodyRoot.addClass(this.toolId);
-        this.BodyRoot.removeClass(this.secondaryToolId);
+        this.bodyRoot.addClass(this.toolId);
+        this.bodyRoot.removeClass(this.secondaryToolId);
       }
     }
   };
diff --git a/test/js/selection/SelectionManagerTest.js b/test/js/selection/SelectionManagerTest.js
new file mode 100644
index 00000000..736fe984
--- /dev/null
+++ b/test/js/selection/SelectionManagerTest.js
@@ -0,0 +1,206 @@
+describe("SelectionManager suite", function() {
+  var black = '#000000';
+  var red = '#ff0000';
+  var transparent = Constants.TRANSPARENT_COLOR;
+  var B = black, R = red, T = transparent;
+
+  // shortcuts
+  var toFrameGrid = test.testutils.toFrameGrid;
+  var frameEqualsGrid = test.testutils.frameEqualsGrid;
+
+  // test objects
+  var selectionManager;
+  var selection;
+  var currentFrame;
+
+  /**
+   * @Mock
+   */
+  pskl.app.shortcutService = {
+    addShortcut : function () {}
+  };
+
+  /**
+   * @Mock
+   */
+  var piskelController = {
+    getCurrentFrame : function () {
+      return currentFrame;
+    }
+  };
+
+  beforeEach(function() {
+    currentFrame = pskl.model.Frame.fromPixelGrid([
+      [B, R, T],
+      [R, B, R],
+      [T, R, B]
+    ]);
+
+    selectionManager = new pskl.selection.SelectionManager(piskelController);
+    selectionManager.init();
+
+    selection = new pskl.selection.BaseSelection();
+    
+    selection.pixels = [];
+  });
+
+  /**
+   * Check a basic copy paste scenario
+   */
+  it("copy/paste OK", function () {
+    console.log('[SelectionManager] copy/paste OK');
+    selectMiddleLine();
+
+    console.log('[SelectionManager] ... copy');
+    selectionManager.copy();
+
+    console.log('[SelectionManager] ... check selection content after copy contains correct colors');
+    expect(selection.pixels.length).toBe(3); // or not to be ... lalalala ... french-only joke \o/
+    checkContainsPixel(selection.pixels, 1, 0, R);
+    checkContainsPixel(selection.pixels, 1, 1, B);
+    checkContainsPixel(selection.pixels, 1, 2, R);
+
+    console.log('[SelectionManager] ... move 1 row down');
+    selection.move(0, 1);
+
+    console.log('[SelectionManager] ... check pixels were shifted by two columns forward');
+    checkContainsPixel(selection.pixels, 2, 0, R);
+    checkContainsPixel(selection.pixels, 2, 1, B);
+    checkContainsPixel(selection.pixels, 2, 2, R);
+
+    console.log('[SelectionManager] ... paste');
+    selectionManager.paste();
+
+    console.log('[SelectionManager] ... check last line is identical to middle line after paste');
+    frameEqualsGrid(currentFrame, [
+      [B, R, T],
+      [R, B, R],
+      [R, B, R]
+    ]);
+  });
+
+  /**
+   * Check a basic cut paste scenario
+   */
+  it("cut OK", function () {
+    console.log('[SelectionManager] cut OK');
+    selectMiddleLine();
+
+    console.log('[SelectionManager] ... cut');
+    selectionManager.cut();
+
+    console.log('[SelectionManager] ... check middle line was cut in the source frame');
+    frameEqualsGrid(currentFrame, [
+      [B, R, T],
+      [T, T, T],
+      [T, R, B]
+    ]);
+
+    console.log('[SelectionManager] ... paste');
+    selectionManager.paste();
+
+    console.log('[SelectionManager] ... check middle line was restored by paste');
+    frameEqualsGrid(currentFrame, [
+      [B, R, T],
+      [R, B, R],
+      [T, R, B]
+    ]);
+  });
+
+  /**
+   * Check a copy paste scenario that goes out of the frame boundaries for copying and for pasting.
+   */
+  it("copy/paste OK out of bounds", function () {
+    console.log('[SelectionManager] copy/paste OK out of bounds');
+    selectMiddleLine();
+
+    console.log('[SelectionManager] ... move 2 columns to the right');
+    selection.move(2, 0);
+
+    console.log('[SelectionManager] ... copy out of bounds');
+    selectionManager.copy();
+    console.log('[SelectionManager] ... check out of bound pixels were replaced by transparent pixels');
+    checkContainsPixel(selection.pixels, 1, 2, R);
+    checkContainsPixel(selection.pixels, 1, 3, T);
+    checkContainsPixel(selection.pixels, 1, 4, T);
+
+    console.log('[SelectionManager] ... move one column to the left');
+    selection.move(-1, 0);
+
+    console.log('[SelectionManager] ... check pixels were shifted by one column back');
+    checkContainsPixel(selection.pixels, 1, 1, R);
+    checkContainsPixel(selection.pixels, 1, 2, T);
+    checkContainsPixel(selection.pixels, 1, 3, T);
+
+    console.log('[SelectionManager] ... paste out of bounds');
+    selectionManager.paste();
+
+    console.log('[SelectionManager] ... check pixel at (1,1) is red after paste');
+    frameEqualsGrid(currentFrame, [
+      [B, R, T],
+      [R, R, R],
+      [T, R, B]
+    ]);
+  });
+
+  /**
+   * Check a cut paste scenario that goes out of the frame boundaries for cutting and for pasting.
+   */
+  it("cut OK out of bounds", function () {
+    console.log('[SelectionManager] cut OK');
+    selectMiddleLine();
+
+    console.log('[SelectionManager] ... move 2 columns to the right');
+    selection.move(2, 0);
+
+    console.log('[SelectionManager] ... cut out of bounds');
+    selectionManager.cut();
+    console.log('[SelectionManager] ... check last pixel of midle line was cut in the source frame');
+    frameEqualsGrid(currentFrame, [
+      [B, R, T],
+      [R, B, T],
+      [T, R, B]
+    ]);
+    
+    selection.move(-1, 0);
+
+    console.log('[SelectionManager] ... paste out of bounds');
+    selectionManager.paste();
+
+    console.log('[SelectionManager] ... check middle line final state');
+    frameEqualsGrid(currentFrame, [
+      [B, R, T],
+      [R, R, T],
+      [T, R, B]
+    ]);
+  });
+
+  // Private helpers
+  var createPixel = function(row, col, color) {
+    return {
+      row : row,
+      col : col,
+      color : color
+    };
+  };
+
+  var selectMiddleLine = function () {
+    console.log('[SelectionManager] ... select middle line');
+    selection.pixels.push(createPixel(1, 0));
+    selection.pixels.push(createPixel(1, 1));
+    selection.pixels.push(createPixel(1, 2));
+
+    expect(selectionManager.currentSelection).toBe(null);
+    console.log('[SelectionManager] ... send SELECTION_CREATED event for the test selection');
+    $.publish(Events.SELECTION_CREATED, [selection]);
+    expect(selectionManager.currentSelection).toBe(selection);
+  };
+
+  var checkContainsPixel = function (pixels, row, col, color) {
+    var containsPixel = pixels.some(function (pixel) {
+      return pixel.row == row && pixel.col == col && pixel.color == color;
+    });
+    expect(containsPixel).toBe(true);
+  };
+
+});
\ No newline at end of file