Skip to content

Commit

Permalink
Removed the code for swapping origin for rotation (fabricjs#4878)
Browse files Browse the repository at this point in the history 
* lots of code deleted

* rotation working

* a change

* added simple test

* resolve conflict
  • Loading branch information
@asturur
asturur authored Apr 1, 2018
1 parent 581fd2f commit 9cf88ec
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 102 deletions.
73 changes: 39 additions & 34 deletions src/canvas.class.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -425,30 +425,25 @@
});

if (this._shouldCenterTransform(t.target)) {
if (t.action === 'rotate') {
this._setOriginToCenter(t.target);
if (t.originX !== 'center') {
if (t.originX === 'right') {
t.mouseXSign = -1;
}
else {
t.mouseXSign = 1;
}
}
else {
if (t.originX !== 'center') {
if (t.originX === 'right') {
t.mouseXSign = -1;
}
else {
t.mouseXSign = 1;
}
if (t.originY !== 'center') {
if (t.originY === 'bottom') {
t.mouseYSign = -1;
}
if (t.originY !== 'center') {
if (t.originY === 'bottom') {
t.mouseYSign = -1;
}
else {
t.mouseYSign = 1;
}
else {
t.mouseYSign = 1;
}

t.originX = 'center';
t.originY = 'center';
}

t.originX = 'center';
t.originY = 'center';
}
else {
t.originX = t.original.originX;
Expand Down Expand Up @@ -571,6 +566,8 @@
},

/**
* centeredScaling from object can't override centeredScaling from canvas.
* this should be fixed, since object setting should take precedence over canvas.
* @private
* @param {fabric.Object} target
*/
Expand Down Expand Up @@ -663,6 +660,7 @@
scaleY: target.scaleY,
skewX: target.skewX,
skewY: target.skewY,
// used by transation
offsetX: pointer.x - target.left,
offsetY: pointer.y - target.top,
originX: origin.x,
Expand All @@ -671,9 +669,11 @@
ey: pointer.y,
lastX: pointer.x,
lastY: pointer.y,
left: target.left,
top: target.top,
// unsure they are usefull anymore.
// left: target.left,
// top: target.top,
theta: degreesToRadians(target.angle),
// end of unsure
width: target.width * target.scaleX,
mouseXSign: 1,
mouseYSign: 1,
Expand Down Expand Up @@ -843,9 +843,9 @@
_scaleObject: function (x, y, by) {
var t = this._currentTransform,
target = t.target,
lockScalingX = target.get('lockScalingX'),
lockScalingY = target.get('lockScalingY'),
lockScalingFlip = target.get('lockScalingFlip');
lockScalingX = target.lockScalingX,
lockScalingY = target.lockScalingY,
lockScalingFlip = target.lockScalingFlip;

if (lockScalingX && lockScalingY) {
return false;
Expand Down Expand Up @@ -1015,20 +1015,22 @@
*/
_rotateObject: function (x, y) {

var t = this._currentTransform;
var t = this._currentTransform,
target = t.target, constraintPosition,
constraintPosition = target.translateToOriginPoint(target.getCenterPoint(), t.originX, t.originY);

if (t.target.get('lockRotation')) {
if (target.lockRotation) {
return false;
}

var lastAngle = atan2(t.ey - t.top, t.ex - t.left),
curAngle = atan2(y - t.top, x - t.left),
var lastAngle = atan2(t.ey - constraintPosition.y, t.ex - constraintPosition.x),
curAngle = atan2(y - constraintPosition.y, x - constraintPosition.x),
angle = radiansToDegrees(curAngle - lastAngle + t.theta),
hasRotated = true;

if (t.target.snapAngle > 0) {
var snapAngle = t.target.snapAngle,
snapThreshold = t.target.snapThreshold || snapAngle,
if (target.snapAngle > 0) {
var snapAngle = target.snapAngle,
snapThreshold = target.snapThreshold || snapAngle,
rightAngleLocked = Math.ceil(angle / snapAngle) * snapAngle,
leftAngleLocked = Math.floor(angle / snapAngle) * snapAngle;

Expand All @@ -1046,11 +1048,14 @@
}
angle %= 360;

if (t.target.angle === angle) {
if (target.angle === angle) {
hasRotated = false;
}
else {
t.target.angle = angle;
// rotation only happen here
target.angle = angle;
// Make sure the constraints apply
target.setPositionByOrigin(constraintPosition, t.originX, t.originY);
}

return hasRotated;
Expand Down
64 changes: 0 additions & 64 deletions src/mixins/canvas_events.mixin.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -467,37 +467,13 @@
}

target.setCoords();
this._restoreOriginXY(target);

if (transform.actionPerformed || (this.stateful && target.hasStateChanged())) {
this.fire('object:modified', { target: target, e: e });
target.fire('modified', { e: e });
}
},

/**
* @private
* @param {Object} target Object to restore
*/
_restoreOriginXY: function(target) {
if (this._previousOriginX && this._previousOriginY) {

var originPoint = target.translateToOriginPoint(
target.getCenterPoint(),
this._previousOriginX,
this._previousOriginY);

target.originX = this._previousOriginX;
target.originY = this._previousOriginY;

target.left = originPoint.x;
target.top = originPoint.y;

this._previousOriginX = null;
this._previousOriginY = null;
}
},

/**
* @private
* @param {Event} e Event object fired on mousedown
Expand Down Expand Up @@ -628,46 +604,6 @@

},

/**
* @private
* @param {Object} target Object for that origin is set to center
*/
_setOriginToCenter: function(target) {
this._previousOriginX = this._currentTransform.target.originX;
this._previousOriginY = this._currentTransform.target.originY;

var center = target.getCenterPoint();

target.originX = 'center';
target.originY = 'center';

target.left = center.x;
target.top = center.y;

this._currentTransform.left = target.left;
this._currentTransform.top = target.top;
},

/**
* @private
* @param {Object} target Object for that center is set to origin
*/
_setCenterToOrigin: function(target) {
var originPoint = target.translateToOriginPoint(
target.getCenterPoint(),
this._previousOriginX,
this._previousOriginY);

target.originX = this._previousOriginX;
target.originY = this._previousOriginY;

target.left = originPoint.x;
target.top = originPoint.y;

this._previousOriginX = null;
this._previousOriginY = null;
},

/**
* Method that defines the actions when mouse is hovering the canvas.
* The currentTransform parameter will definde whether the user is rotating/scaling/translating
Expand Down
5 changes: 1 addition & 4 deletions src/mixins/canvas_gestures.mixin.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@

t.action = 'scale';
t.originX = t.originY = 'center';
this._setOriginToCenter(t.target);

this._scaleObjectBy(self.scale, e);

Expand All @@ -62,8 +61,6 @@
this._rotateObjectByAngle(self.rotation, e);
}

this._setCenterToOrigin(t.target);

this.requestRenderAll();
t.action = 'drag';
},
Expand Down Expand Up @@ -155,7 +152,7 @@
if (t.target.get('lockRotation')) {
return;
}
t.target.angle = radiansToDegrees(degreesToRadians(curAngle) + t.theta);
t.target.rotate(radiansToDegrees(degreesToRadians(curAngle) + t.theta));
this._fire('rotating', t.target, e);
}
});
Expand Down
36 changes: 36 additions & 0 deletions test/unit/canvas.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1980,9 +1980,45 @@
assert.equal(t.target, rect, 'should have rect as a target');
assert.equal(t.action, 'rotate', 'should target a corner and setup rotate');
assert.equal(t.corner, 'mtr', 'mtr selected');
assert.equal(t.originX, 'center', 'origin in center');
assert.equal(t.originY, 'center', 'origin in center');
canvas._currentTransform = false;
});

QUnit.test('_rotateObject', function(assert) {
assert.ok(typeof canvas._rotateObject === 'function');
var rect = new fabric.Rect({ left: 75, top: 75, width: 50, height: 50 });
canvas.add(rect);
var canvasEl = canvas.getElement(),
canvasOffset = fabric.util.getElementOffset(canvasEl);
var eventStub = {
clientX: canvasOffset.left + rect.oCoords.mtr.x,
clientY: canvasOffset.top + rect.oCoords.mtr.y,
target: rect,
};
canvas._setupCurrentTransform(eventStub, rect);
var rotated = canvas._rotateObject(30, 30, 'equally');
assert.equal(rotated, true, 'return true if a rotation happened');
rotated = canvas._rotateObject(30, 30);
assert.equal(rotated, false, 'return true if no rotation happened');
});

QUnit.test('_rotateObject do not change origins', function(assert) {
assert.ok(typeof canvas._rotateObject === 'function');
var rect = new fabric.Rect({ left: 75, top: 75, width: 50, height: 50, originX: 'right', originY: 'bottom' });
canvas.add(rect);
var canvasEl = canvas.getElement(),
canvasOffset = fabric.util.getElementOffset(canvasEl);
var eventStub = {
clientX: canvasOffset.left + rect.oCoords.mtr.x,
clientY: canvasOffset.top + rect.oCoords.mtr.y,
target: rect,
};
canvas._setupCurrentTransform(eventStub, rect);
assert.equal(rect.originX, 'right');
assert.equal(rect.originY, 'bottom');
});

QUnit.test('_scaleObject', function(assert) {
assert.ok(typeof canvas._scaleObject === 'function');
var rect = new fabric.Rect({ left: 75, top: 75, width: 50, height: 50 });
Expand Down

0 comments on commit 9cf88ec

Please sign in to comment.