Fix free draw positioning (fabricjs#5718)

src/brushes/base_brush.class.js

@@ -112,6 +112,11 @@ fabric.BaseBrush = fabric.util.createClass(/** @lends fabric.BaseBrush.prototype
     ctx.shadowOffsetY = this.shadow.offsetY * zoom;
   },
 
+  needsFullRender: function() {
+    var color = new fabric.Color(this.color);
+    return color.getAlpha() < 1 || !!this.shadow;
+  },
+
   /**
    * Removes brush shadow styles
    * @private

src/brushes/circle_brush.class.js

@@ -29,13 +29,16 @@ fabric.CircleBrush = fabric.util.createClass(fabric.BaseBrush, /** @lends fabric
     var point = this.addPoint(pointer),
         ctx = this.canvas.contextTop;
     this._saveAndTransform(ctx);
+    this.dot(ctx, point);
+    ctx.restore();
+  },
+
+  dot: function(ctx, point) {
     ctx.fillStyle = point.fill;
     ctx.beginPath();
     ctx.arc(point.x, point.y, point.radius, 0, Math.PI * 2, false);
     ctx.closePath();
     ctx.fill();
-
-    ctx.restore();
   },
 
   /**
@@ -54,15 +57,10 @@ fabric.CircleBrush = fabric.util.createClass(fabric.BaseBrush, /** @lends fabric
    */
   _render: function() {
     var ctx  = this.canvas.contextTop, i, len,
-        points = this.points, point;
+        points = this.points;
     this._saveAndTransform(ctx);
     for (i = 0, len = points.length; i < len; i++) {
-      point = points[i];
-      ctx.fillStyle = point.fill;
-      ctx.beginPath();
-      ctx.arc(point.x, point.y, point.radius, 0, Math.PI * 2, false);
-      ctx.closePath();
-      ctx.fill();
+      this.dot(ctx, points[i]);
     }
     ctx.restore();
   },
@@ -72,7 +70,14 @@ fabric.CircleBrush = fabric.util.createClass(fabric.BaseBrush, /** @lends fabric
    * @param {Object} pointer
    */
   onMouseMove: function(pointer) {
-    this.drawDot(pointer);
+    if (this.needsFullRender()) {
+      this.canvas.clearContext(this.canvas.contextTop);
+      this.addPoint(pointer);
+      this._render();
+    }
+    else {
+      this.drawDot(pointer);
+    }
   },
 
   /**

src/brushes/pencil_brush.class.js

@@ -1,12 +1,18 @@
 (function() {
-
   /**
    * PencilBrush class
    * @class fabric.PencilBrush
    * @extends fabric.BaseBrush
    */
   fabric.PencilBrush = fabric.util.createClass(fabric.BaseBrush, /** @lends fabric.PencilBrush.prototype */ {
 
+    /**
+     * Discard points that are less than `decimate` pixel distant from each other
+     * @type Number
+     * @default 0.4
+     */
+    decimate: 0.4,
+
     /**
      * Constructor
      * @param {fabric.Canvas} canvas
@@ -45,7 +51,7 @@
      */
     onMouseMove: function(pointer) {
       if (this._captureDrawingPath(pointer) && this._points.length > 1) {
-        if (this.needsFullRender) {
+        if (this.needsFullRender()) {
           // redraw curve
           // clear top canvas
           this.canvas.clearContext(this.canvas.contextTop);
@@ -106,8 +112,6 @@
     _reset: function() {
       this._points.length = 0;
       this._setBrushStyles();
-      var color = new fabric.Color(this.color);
-      this.needsFullRender = (color.getAlpha() < 1);
       this._setShadow();
     },
 
@@ -168,7 +172,7 @@
       var path = [], i, width = this.width / 1000,
           p1 = new fabric.Point(points[0].x, points[0].y),
           p2 = new fabric.Point(points[1].x, points[1].y),
-          len = points.length, multSignX = 1, multSignY = 1, manyPoints = len > 2;
+          len = points.length, multSignX = 1, multSignY = 0, manyPoints = len > 2;
 
       if (manyPoints) {
         multSignX = points[2].x < p2.x ? -1 : points[2].x === p2.x ? 0 : 1;
@@ -211,10 +215,6 @@
         strokeLineJoin: this.strokeLineJoin,
         strokeDashArray: this.strokeDashArray,
       });
-      var position = new fabric.Point(path.left + path.width / 2, path.top + path.height / 2);
-      position = path.translateToGivenOrigin(position, 'center', 'center', path.originX, path.originY);
-      path.top = position.y;
-      path.left = position.x;
       if (this.shadow) {
         this.shadow.affectStroke = true;
         path.setShadow(this.shadow);
@@ -223,6 +223,26 @@
       return path;
     },
 
+    /**
+     * Decimate poins array with the decimate value
+     */
+    decimatePoints: function(points, distance) {
+      if (points.length <= 2) {
+        return points;
+      }
+      var zoom = this.canvas.getZoom(), adjustedDistance = Math.pow(distance / zoom, 2),
+          i, l = points.length - 1, lastPoint = points[0], newPoints = [lastPoint],
+          cDistance;
+      for (i = 1; i < l; i++) {
+        cDistance = Math.pow(lastPoint.x - points[i].x, 2) + Math.pow(lastPoint.y - points[i].y, 2);
+        if (cDistance >= adjustedDistance) {
+          lastPoint = points[i];
+          newPoints.push(lastPoint);
+        }
+      }
+      return newPoints;
+    },
+
     /**
      * On mouseup after drawing the path on contextTop canvas
      * we use the points captured to create an new fabric path object
@@ -231,7 +251,9 @@
     _finalizeAndAddPath: function() {
       var ctx = this.canvas.contextTop;
       ctx.closePath();
-
+      if (this.decimate) {
+        this._points = this.decimatePoints(this._points, this.decimate);
+      }
       var pathData = this.convertPointsToSVGPath(this._points).join('');
       if (pathData === 'M 0 0 Q 0 0 0 0 L 0 0') {
         // do not create 0 width/height paths, as they are

test/lib/visualTestLoop.js

@@ -19,15 +19,16 @@
     }
   };
 
-  function createCanvasForTest(width, height) {
+  function createCanvasForTest(opts) {
+    var fabricClass = opts.fabricClass || 'StaticCanvas';
     var options = { enableRetinaScaling: false, renderOnAddRemove: false, width: 200, height: 200 };
-    if (width) {
-      options.width = width;
+    if (opts.width) {
+      options.width = opts.width;
     }
-    if (height) {
-      options.height = height;
+    if (opts.height) {
+      options.height = opts.height;
     }
-    return new fabric.StaticCanvas(null, options);
+    return new fabric[fabricClass](null, options);
   };
 
   function getAbsolutePath(path) {
@@ -127,7 +128,7 @@
       }
       QUnit.test(testName, function(assert) {
         var done = assert.async();
-        var fabricCanvas = createCanvasForTest(testObj.width, testObj.height);
+        var fabricCanvas = createCanvasForTest(testObj);
         code(fabricCanvas, function(renderedCanvas) {
           var width = renderedCanvas.width;
           var height = renderedCanvas.height;

test/unit/brushes.js

@@ -26,7 +26,7 @@
     var pointer = canvas.getPointer({ clientX: 10, clientY: 10});
     brush.onMouseDown(pointer);
     var pathData = brush.convertPointsToSVGPath(brush._points).join('');
-    assert.equal(pathData, 'M 9.999 9.999 L 10.001 10.001', 'path data create a small line that looks like a point');
+    assert.equal(pathData, 'M 9.999 10 L 10.001 10', 'path data create a small line that looks like a point');
   });
   QUnit.test('fabric pencil brush multiple points', function(assert) {
     var brush = new fabric.PencilBrush(canvas);
@@ -37,7 +37,7 @@
     brush.onMouseMove(pointer);
     brush.onMouseMove(pointer);
     var pathData = brush.convertPointsToSVGPath(brush._points).join('');
-    assert.equal(pathData, 'M 9.999 9.999 L 10.001 10.001', 'path data create a small line that looks like a point');
+    assert.equal(pathData, 'M 9.999 10 L 10.001 10', 'path data create a small line that looks like a point');
     assert.equal(brush._points.length, 2, 'concident points are discarded');
   });
   QUnit.test('fabric pencil brush multiple points not discarded', function(assert) {