Issue #311 : Cleanup and comments

src/js/controller/CursorCoordinatesController.js

@@ -58,7 +58,7 @@
     this.redraw();
   };
 
-  ns.CursorCoordinatesController.prototype.onDragEnd_ = function (event, x, y) {
+  ns.CursorCoordinatesController.prototype.onDragEnd_ = function (event) {
     this.origin = null;
     this.redraw();
   };

src/js/controller/ToolController.js

@@ -16,9 +16,9 @@
       toDescriptor('rectangle', 'R', new pskl.tools.drawing.Rectangle()),
       toDescriptor('circle', 'C', new pskl.tools.drawing.Circle()),
       toDescriptor('move', 'M', new pskl.tools.drawing.Move()),
+      toDescriptor('shapeSelect', 'Z', new pskl.tools.drawing.selection.ShapeSelect()),
       toDescriptor('rectangleSelect', 'S', new pskl.tools.drawing.selection.RectangleSelect()),
       toDescriptor('lassoSelect', 'H', new pskl.tools.drawing.selection.LassoSelect()),
-      toDescriptor('shapeSelect', 'Z', new pskl.tools.drawing.selection.ShapeSelect()),
       toDescriptor('lighten', 'U', new pskl.tools.drawing.Lighten()),
       toDescriptor('dithering', 'T', new pskl.tools.drawing.DitheringTool()),
       toDescriptor('colorPicker', 'O', new pskl.tools.drawing.ColorPicker())

src/js/tools/drawing/ShapeTool.js

@@ -51,7 +51,7 @@
     var color = this.getToolColor();
     this.draw(coords.col, coords.row, color, frame);
 
-    $.publish(Events.DRAG_END, [coords.col, coords.row]);
+    $.publish(Events.DRAG_END);
     this.raiseSaveStateEvent({
       col : coords.col,
       row : coords.row,

src/js/tools/drawing/selection/AbstractDragSelect.js

@@ -1,3 +1,8 @@
+/**
+ * Base class for all select tools that use a dragging mechanism to define the selection.
+ *
+ * @provide pskl.tools.drawing.selection.AbstractDragSelect
+ */
 (function () {
   var ns = $.namespace('pskl.tools.drawing.selection');
 
@@ -8,9 +13,7 @@
 
   pskl.utils.inherit(ns.AbstractDragSelect, ns.BaseSelect);
 
-  /**
+  /** @override */
-   * @override
-   */
   ns.AbstractDragSelect.prototype.onSelectStart_ = function (col, row, color, frame, overlay) {
     if (this.hasSelection) {
       this.hasSelection = false;
@@ -23,6 +26,7 @@
     }
   };
 
+  /** @override */
   ns.AbstractDragSelect.prototype.onSelect_ = function (col, row, color, frame, overlay) {
     if (!this.hasSelection && (this.startCol !== col || this.startRow !== row)) {
       this.hasSelection = true;
@@ -34,6 +38,7 @@
     }
   };
 
+  /** @override */
   ns.AbstractDragSelect.prototype.onSelectEnd_ = function (col, row, color, frame, overlay) {
     if (this.hasSelection) {
       this.endDragSelection_(col, row, color, frame, overlay);

src/js/tools/drawing/selection/BaseSelect.js

@@ -1,5 +1,5 @@
 /**
- * @provide pskl.tools.drawing.BaseSelect
+ * @provide pskl.tools.drawing.selection.BaseSelect
  *
  * @require pskl.utils
  */
@@ -40,12 +40,12 @@
     // mode to create a selection.
     // If the initial click is on a previous selection, we go in 'moveSelection'
     // mode to allow to move the selection by drag'n dropping it.
-    if (this.isInSelection(col, row)) {
+    if (!this.isInSelection(col, row)) {
-      this.mode = 'moveSelection';
-      this.onSelectionDragStart_(col, row, frame, overlay);
-    } else {
       this.mode = 'select';
-      this.onSelectStart_(col, row, frame, overlay);
+      this.onSelectStart_(col, row, color, frame, overlay);
+    } else {
+      this.mode = 'moveSelection';
+      this.onSelectionMoveStart_(col, row, color, frame, overlay);
     }
   };
 
@@ -56,7 +56,7 @@
     if (this.mode == 'select') {
       this.onSelect_(col, row, frame, overlay);
     } else if (this.mode == 'moveSelection') {
-      this.onSelectionDrag_(col, row, frame, overlay);
+      this.onSelectionMove_(col, row, color, frame, overlay);
     }
   };
 
@@ -67,7 +67,7 @@
     if (this.mode == 'select') {
       this.onSelectEnd_(col, row, frame, overlay);
     } else if (this.mode == 'moveSelection') {
-      this.onSelectionDragEnd_(col, row, frame, overlay);
+      this.onSelectionMoveEnd_(col, row, color, frame, overlay);
     }
   };
 
@@ -132,10 +132,11 @@
 
   // The list of callbacks that define the drag'n drop behavior of the selection.
   /** @private */
-  ns.BaseSelect.prototype.onSelectionDragStart_ = function (col, row, frame, overlay) {};
+
+  ns.BaseSelect.prototype.onSelectionMoveStart_ = function (col, row, color, frame, overlay) {};
 
   /** @private */
-  ns.BaseSelect.prototype.onSelectionDrag_ = function (col, row, frame, overlay) {
+  ns.BaseSelect.prototype.onSelectionMove_ = function (col, row, color, frame, overlay) {
     var deltaCol = col - this.lastCol;
     var deltaRow = row - this.lastRow;
 
@@ -152,7 +153,7 @@
   };
 
   /** @private */
-  ns.BaseSelect.prototype.onSelectionDragEnd_ = function (col, row, frame, overlay) {
+  ns.BaseSelect.prototype.onSelectionMoveEnd_ = function (col, row, color, frame, overlay) {
-    this.onSelectionDrag_(col, row, frame, overlay);
+    this.onSelectionMove_(col, row, color, frame, overlay);
   };
 })();

src/js/tools/drawing/selection/LassoSelect.js

@@ -1,5 +1,5 @@
 /**
- * @provide pskl.tools.drawing.ShapeSelect
+ * @provide pskl.tools.drawing.selection.LassoSelect
  *
  * @require pskl.utils
  */
@@ -15,6 +15,7 @@
 
   pskl.utils.inherit(ns.LassoSelect, ns.AbstractDragSelect);
 
+  /** @override */
   ns.LassoSelect.prototype.startDragSelection_ = function (col, row) {
     this.pixels = [{col : col, row : row}];
     this.previousCol = col;
@@ -22,41 +23,60 @@
     $.publish(Events.DRAG_START, [col, row]);
   };
 
+  /** @override */
   ns.LassoSelect.prototype.updateDragSelection_ = function (col, row, color, frame, overlay) {
-    col = pskl.utils.Math.minmax(col, 0, frame.getWidth() - 1);
+    this.addPixel_(col, row, frame);
-    row = pskl.utils.Math.minmax(row, 0, frame.getHeight() - 1);
+    // use ShapeSelection during selection, contains only the pixels hovered by the user
-    this.addPixelToSelection_(col, row, frame);
+    var selection = new pskl.selection.ShapeSelection(this.getLassoPixels_());
-    var additionnalPixels = this.getLinePixels_(col, this.startCol, row, this.startRow);
+    this.setSelection_(selection, overlay);
-
-    // during the selection, create simple ShapeSelection, containing only the pixels hovered by the user
-    this.selection = new pskl.selection.ShapeSelection(this.pixels.concat(additionnalPixels));
-    $.publish(Events.SELECTION_CREATED, [this.selection]);
-
-    overlay.clear();
-    this.drawSelectionOnOverlay_(overlay);
   };
 
+  /** @override */
   ns.LassoSelect.prototype.endDragSelection_ = function (col, row, color, frame, overlay) {
-    col = pskl.utils.Math.minmax(col, 0, frame.getWidth() - 1);
+    this.addPixel_(col, row, frame);
-    row = pskl.utils.Math.minmax(row, 0, frame.getHeight() - 1);
+    // use LassoSelection to finalize selection, includes pixels inside the lasso shape
-    this.addPixelToSelection_(col, row, frame);
+    var selection = new pskl.selection.LassoSelection(this.getLassoPixels_(), frame);
-    var additionnalPixels = this.getLinePixels_(col, this.startCol, row, this.startRow);
+    this.setSelection_(selection, overlay);
 
-    // finalize the selection, add all pixels contained inside the shape drawn by the user to the selection
+    $.publish(Events.DRAG_END);
-    this.selection = new pskl.selection.LassoSelection(this.pixels.concat(additionnalPixels), frame);
-    $.publish(Events.SELECTION_CREATED, [this.selection]);
-
-    overlay.clear();
-    this.drawSelectionOnOverlay_(overlay);
-
-    $.publish(Events.DRAG_END, [col, row]);
   };
 
-  ns.LassoSelect.prototype.addPixelToSelection_ = function (col, row, frame) {
+  /**
+   * Retrieve the lasso shape as an array of pixels. A line is added between the origin of the selection
+   * and the last known coordinate to make sure the shape is closed.
+   *
+   * @return {Array} array of pixels corresponding to the whole lasso shape
+   * @private
+   */
+  ns.LassoSelect.prototype.getLassoPixels_ = function () {
+    var line = this.getLinePixels_(this.previousCol, this.startCol, this.previousRow, this.startRow);
+    return this.pixels.concat(line);
+  };
+
+  /**
+   * Add the provided pixel to the lasso pixels Array.
+   * @private
+   */
+  ns.LassoSelect.prototype.addPixel_ = function (col, row, frame) {
+    // normalize coordinates to always remain inside the frame
+    col = pskl.utils.Math.minmax(col, 0, frame.getWidth() - 1);
+    row = pskl.utils.Math.minmax(row, 0, frame.getHeight() - 1);
+
+    // line interpolation needed in case mousemove was too fast
     var interpolatedPixels = this.getLinePixels_(col, this.previousCol, row, this.previousRow);
     this.pixels = this.pixels.concat(interpolatedPixels);
 
+    // update state
     this.previousCol = col;
     this.previousRow = row;
   };
+
+  /** @private */
+  ns.LassoSelect.prototype.setSelection_ = function (selection, overlay) {
+    this.selection = selection;
+
+    $.publish(Events.SELECTION_CREATED, [this.selection]);
+    overlay.clear();
+    this.drawSelectionOnOverlay_(overlay);
+  };
 })();

src/js/tools/drawing/selection/RectangleSelect.js

@@ -1,5 +1,5 @@
 /**
- * @provide pskl.tools.drawing.RectangleSelect
+ * @provide pskl.tools.drawing.selection.RectangleSelect
  *
  * @require pskl.utils
  */
@@ -15,6 +15,7 @@
 
   pskl.utils.inherit(ns.RectangleSelect, ns.AbstractDragSelect);
 
+  /** @override */
   ns.RectangleSelect.prototype.startDragSelection_ = function (col, row) {
     $.publish(Events.DRAG_START, [col, row]);
   };
@@ -32,9 +33,10 @@
     this.drawSelectionOnOverlay_(overlay);
   };
 
-  ns.RectangleSelect.prototype.onSelectEnd_ = function (col, row, color, frame, overlay) {
+  /** @override */
+  ns.RectangleSelect.prototype.endDragSelection_ = function (col, row, color, frame, overlay) {
     this.onSelect_(col, row, color, frame, overlay);
-    $.publish(Events.DRAG_END, [col, row]);
+    $.publish(Events.DRAG_END);
   };
 
 })();

src/js/tools/drawing/selection/ShapeSelect.js

@@ -1,5 +1,5 @@
 /**
- * @provide pskl.tools.drawing.ShapeSelect
+ * @provide pskl.tools.drawing.selection.ShapeSelect
  *
  * @require pskl.utils
  */