fix(controls): Fix flip and controls and skewY and controls.

src/mixins/object_geometry.mixin.js

@@ -468,17 +468,16 @@
     },
 
     calcOCoords: function() {
-      var matrix = this.calcTransformMatrix(),
+      var rotateMatrix = this._calcRotateMatrix(),
+          translateMatrix = this._calcTranslateMatrix(),
           vpt = this.getViewportTransform(),
-          options = util.qrDecompose(multiplyMatrices(vpt, matrix));
-      options.scaleX = 1;
-      options.scaleY = 1;
-      options.skewX = 0;
-      var finalMatrix = util.composeMatrix(options),
+          startMatrix = multiplyMatrices(vpt, translateMatrix),
+          finalMatrix = multiplyMatrices(startMatrix, rotateMatrix),
+          finalMatrix = multiplyMatrices(finalMatrix, [1 / vpt[0], 0, 0, 1 / vpt[3], 0, 0]),
           dim = this._calculateCurrentDimensions(),
           coords = {};
       this.forEachControl(function(control, key, fabricObject) {
-        coords[key] = control.positionHandler(dim, finalMatrix, fabricObject, control);
+        coords[key] = control.positionHandler(dim, finalMatrix, fabricObject);
       });
 
       // debug code
@@ -548,33 +547,36 @@
       return [1, 0, 0, 1, center.x, center.y];
     },
 
-    transformMatrixKey: function(skipGroup) {
+    transformMatrixKey: function(skipGroup, skipFlip) {
       var sep = '_', prefix = '';
       if (!skipGroup && this.group) {
         prefix = this.group.transformMatrixKey(skipGroup) + sep;
       };
       return prefix + this.top + sep + this.left + sep + this.scaleX + sep + this.scaleY +
         sep + this.skewX + sep + this.skewY + sep + this.angle + sep + this.originX + sep + this.originY +
-        sep + this.width + sep + this.height + sep + this.strokeWidth + this.flipX + this.flipY;
+        sep + this.width + sep + this.height + sep + this.strokeWidth +
+        (skipFlip ? 'falsefalse' : this.flipX + this.flipY);
     },
 
     /**
      * calculate transform matrix that represents the current transformations from the
      * object's properties.
      * @param {Boolean} [skipGroup] return transform matrix for object not counting parent transformations
+     * @param {Boolean} [skipFlip] return transform matrix for object not the flipping.
+     * There are some situation in which this is useful to avoid the fake rotation.
      * @return {Array} transform matrix for the object
      */
-    calcTransformMatrix: function(skipGroup) {
-      if (skipGroup) {
-        return this.calcOwnMatrix();
+    calcTransformMatrix: function(skipGroup, skipFlip) {
+      var matrix = this.calcOwnMatrix(skipFlip);
+      if (skipGroup || !this.group) {
+        return matrix;
       }
-      var key = this.transformMatrixKey(), cache = this.matrixCache || (this.matrixCache = {});
+      var key = this.transformMatrixKey(skipGroup, skipFlip), cache = this.matrixCache || (this.matrixCache = {});
       if (cache.key === key) {
         return cache.value;
       }
-      var matrix = this.calcOwnMatrix();
       if (this.group) {
-        matrix = multiplyMatrices(this.group.calcTransformMatrix(), matrix);
+        matrix = multiplyMatrices(this.group.calcTransformMatrix(false, skipFlip), matrix);
       }
       cache.key = key;
       cache.value = matrix;
@@ -584,18 +586,30 @@
     /**
      * calculate transform matrix that represents the current transformations from the
      * object's properties, this matrix does not include the group transformation
+     * @param {Boolean} [skipFlip] return transform matrix for object not the flipping.
      * @return {Array} transform matrix for the object
      */
-    calcOwnMatrix: function() {
-      var key = this.transformMatrixKey(true), cache = this.ownMatrixCache || (this.ownMatrixCache = {});
+    calcOwnMatrix: function(skipFlip) {
+      var key = this.transformMatrixKey(true, skipFlip), cache = this.ownMatrixCache || (this.ownMatrixCache = {});
       if (cache.key === key) {
         return cache.value;
       }
-      var tMatrix = this._calcTranslateMatrix();
-      this.translateX = tMatrix[4];
-      this.translateY = tMatrix[5];
+      var tMatrix = this._calcTranslateMatrix(),
+          options = {
+            angle: this.angle,
+            translateX: tMatrix[4],
+            translateY: tMatrix[5],
+            scaleX: this.scaleX,
+            scaleY: this.scaleY,
+            skewX: this.skewX,
+            skewY: this.skewY,
+          };
+      if (!skipFlip) {
+        options.flipX = this.flipX;
+        options.flipY = this.flipY;
+      }
       cache.key = key;
-      cache.value = util.composeMatrix(this);
+      cache.value = util.composeMatrix(options);
       return cache.value;
     },
 
@@ -658,35 +672,13 @@
       if (noSkew) {
         return this._finalizeDimensions(dimX * this.scaleX, dimY * this.scaleY);
       }
-      else {
-        dimX /= 2;
-        dimY /= 2;
-      }
-      var points = [
-            {
-              x: -dimX,
-              y: -dimY
-            },
-            {
-              x: dimX,
-              y: -dimY
-            },
-            {
-              x: -dimX,
-              y: dimY
-            },
-            {
-              x: dimX,
-              y: dimY
-            }],
-          transformMatrix = util.calcDimensionsMatrix({
-            scaleX: this.scaleX,
-            scaleY: this.scaleY,
-            skewX: skewX,
-            skewY: skewY,
-          }),
-          bbox = util.makeBoundingBoxFromPoints(points, transformMatrix);
-      return this._finalizeDimensions(bbox.width, bbox.height);
+      var bbox = util.sizeAfterTransform(dimX, dimY, {
+        scaleX: this.scaleX,
+        scaleY: this.scaleY,
+        skewX: skewX,
+        skewY: skewY,
+      });
+      return this._finalizeDimensions(bbox.x, bbox.y);
     },
 
     /*
@@ -703,8 +695,10 @@
         :
         { x: width, y: height };
     },
+
     /*
-     * Calculate object dimensions for controls, including padding and canvas zoom.
+     * Calculate object dimensions for controls box, including padding and canvas zoom.
+     * and active selection
      * private
      */
     _calculateCurrentDimensions: function()  {

src/mixins/object_interactivity.mixin.js

@@ -222,23 +222,17 @@
      */
     drawBordersInGroup: function(ctx, options, styleOverride) {
       styleOverride = styleOverride || {};
-      var p = { x: this.width, y: this.height },
-          matrix = fabric.util.composeMatrix({
-            scaleX: options.scaleX,
-            scaleY: options.scaleY,
-            skewX: options.skewX
-          }),
-          wh = fabric.util.transformPoint(p, matrix),
+      var bbox = fabric.util.sizeAfterTransform(this.width, this.height, options),
           strokeWidth = this.strokeWidth,
+          strokeUniform = this.strokeUniform,
           borderScaleFactor = this.borderScaleFactor,
           width =
-            wh.x + strokeWidth * (this.strokeUniform ? this.canvas.getZoom() : options.scaleX) + borderScaleFactor,
+            bbox.x + strokeWidth * (strokeUniform ? this.canvas.getZoom() : options.scaleX) + borderScaleFactor,
           height =
-            wh.y + strokeWidth * (this.strokeUniform ? this.canvas.getZoom() : options.scaleY) + borderScaleFactor;
+            bbox.y + strokeWidth * (strokeUniform ? this.canvas.getZoom() : options.scaleY) + borderScaleFactor;
       ctx.save();
       this._setLineDash(ctx, styleOverride.borderDashArray || this.borderDashArray, null);
       ctx.strokeStyle = styleOverride.borderColor || this.borderColor;
-
       ctx.strokeRect(
         -width / 2,
         -height / 2,

src/util/misc.js

@@ -659,10 +659,10 @@
       var angle = atan2(a[1], a[0]),
           denom = pow(a[0], 2) + pow(a[1], 2),
           scaleX = sqrt(denom),
-          scaleY = (a[0] * a[3] - a[2] * a [1]) / scaleX,
+          scaleY = (a[0] * a[3] - a[2] * a[1]) / scaleX,
           skewX = atan2(a[0] * a[2] + a[1] * a [3], denom);
       return {
-        angle: angle  / PiBy180,
+        angle: angle / PiBy180,
         scaleX: scaleX,
         scaleY: scaleY,
         skewX: skewX / PiBy180,
@@ -973,6 +973,49 @@
       return 'matrix(' + transform.map(function(value) {
         return fabric.util.toFixed(value, fabric.Object.NUM_FRACTION_DIGITS);
       }).join(' ') + ')';
+    },
+
+    /**
+     * given a width and height, return the size of the bounding box
+     * that can contains the box with width/height with applied transform
+     * described in options.
+     * Use to calculate the boxes around objects for controls.
+     * @memberOf fabric.util
+     * @param {Number} width
+     * @param {Number} height
+     * @param {Object} options
+     * @param {Number} options.scaleX
+     * @param {Number} options.scaleY
+     * @param {Number} options.skewX
+     * @param {Number} options.skewY
+     * @return {Object.x} width of containing
+     * @return {Object.y} height of containing
+     */
+    sizeAfterTransform: function(width, height, options) {
+      var dimX = width / 2, dimY = height / 2,
+          points = [
+            {
+              x: -dimX,
+              y: -dimY
+            },
+            {
+              x: dimX,
+              y: -dimY
+            },
+            {
+              x: -dimX,
+              y: dimY
+            },
+            {
+              x: dimX,
+              y: dimY
+            }],
+          transformMatrix = fabric.util.calcDimensionsMatrix(options),
+          bbox = fabric.util.makeBoundingBoxFromPoints(points, transformMatrix);
+      return {
+        x: bbox.width,
+        y: bbox.height,
+      };
     }
   };
 })(typeof exports !== 'undefined' ? exports : this);

test/unit/object_geometry.js

@@ -287,6 +287,20 @@
     assert.equal(cObj.oCoords.br.y, 350);
     assert.equal(cObj.oCoords.mtr.x, 300);
     assert.equal(cObj.oCoords.mtr.y, 210);
+
+    cObj.set('padding', 25);
+    cObj.setCoords();
+    // coords should still correspond to initial one, even after invoking `set`
+    assert.equal(cObj.oCoords.tl.x, 225, 'setCoords tl.x padding');
+    assert.equal(cObj.oCoords.tl.y, 225, 'setCoords tl.y padding');
+    assert.equal(cObj.oCoords.tr.x, 375, 'setCoords tr.x padding');
+    assert.equal(cObj.oCoords.tr.y, 225, 'setCoords tr.y padding');
+    assert.equal(cObj.oCoords.bl.x, 225, 'setCoords bl.x padding');
+    assert.equal(cObj.oCoords.bl.y, 375, 'setCoords bl.y padding');
+    assert.equal(cObj.oCoords.br.x, 375, 'setCoords br.x padding');
+    assert.equal(cObj.oCoords.br.y, 375, 'setCoords br.y padding');
+    assert.equal(cObj.oCoords.mtr.x, 300, 'setCoords mtr.x padding');
+    assert.equal(cObj.oCoords.mtr.y, 185, 'setCoords mtr.y padding');
   });
 
   QUnit.test('setCoords and aCoords', function(assert) {
@@ -296,16 +310,16 @@
     };
     cObj.setCoords();
 
-    assert.equal(cObj.oCoords.tl.x, 300, 'oCoords are modified by viewportTransform');
-    assert.equal(cObj.oCoords.tl.y, 300, 'oCoords are modified by viewportTransform');
-    assert.equal(cObj.oCoords.tr.x, 500, 'oCoords are modified by viewportTransform');
-    assert.equal(cObj.oCoords.tr.y, 300, 'oCoords are modified by viewportTransform');
-    assert.equal(cObj.oCoords.bl.x, 300, 'oCoords are modified by viewportTransform');
-    assert.equal(cObj.oCoords.bl.y, 500, 'oCoords are modified by viewportTransform');
-    assert.equal(cObj.oCoords.br.x, 500, 'oCoords are modified by viewportTransform');
-    assert.equal(cObj.oCoords.br.y, 500, 'oCoords are modified by viewportTransform');
-    assert.equal(cObj.oCoords.mtr.x, 400, 'oCoords are modified by viewportTransform');
-    assert.equal(cObj.oCoords.mtr.y, 260, 'oCoords are modified by viewportTransform');
+    assert.equal(cObj.oCoords.tl.x, 300, 'oCoords are modified by viewportTransform tl.x');
+    assert.equal(cObj.oCoords.tl.y, 300, 'oCoords are modified by viewportTransform tl.y');
+    assert.equal(cObj.oCoords.tr.x, 500, 'oCoords are modified by viewportTransform tr.x');
+    assert.equal(cObj.oCoords.tr.y, 300, 'oCoords are modified by viewportTransform tr.y');
+    assert.equal(cObj.oCoords.bl.x, 300, 'oCoords are modified by viewportTransform bl.x');
+    assert.equal(cObj.oCoords.bl.y, 500, 'oCoords are modified by viewportTransform bl.y');
+    assert.equal(cObj.oCoords.br.x, 500, 'oCoords are modified by viewportTransform br.x');
+    assert.equal(cObj.oCoords.br.y, 500, 'oCoords are modified by viewportTransform br.y');
+    assert.equal(cObj.oCoords.mtr.x, 400, 'oCoords are modified by viewportTransform mtr.x');
+    assert.equal(cObj.oCoords.mtr.y, 260, 'oCoords are modified by viewportTransform mtr.y');
 
     assert.equal(cObj.aCoords.tl.x, 150, 'aCoords do not interfere with viewportTransform');
     assert.equal(cObj.aCoords.tl.y, 150, 'aCoords do not interfere with viewportTransform');