Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lockRotation and object:modified event #1927

Closed
lukejagodzinski opened this issue Jan 12, 2015 · 2 comments · Fixed by #2890
Closed

lockRotation and object:modified event #1927

lukejagodzinski opened this issue Jan 12, 2015 · 2 comments · Fixed by #2890
Labels

Comments

@lukejagodzinski
Copy link

Hi, I've noticed strange behavior when trying to rotate object that has lockRotation set to true. At first it performs object:rotating event on object even that it's not rotating. And what is even more unwanted when rotation is finished it fires object:modified but nothing has changed. When I investigated what properties has changed using this code:

fabric.Object.prototype.hasStateChanged = function() {
  this.stateProperties.forEach(function(prop) {
    if (this.get(prop) !== this.originalState[prop]) {
      console.log('hasStateChanged(' + prop + ')')
      console.log('    current:  ' + this.get(prop));
      console.log('    original: ' + this.originalState[prop]);
    }
  }.bind(this));

  return this.stateProperties.some(function(prop) {
    return this.get(prop) !== this.originalState[prop];
  }, this);
};

it appeared to be left, top, originX, originY properties. I understand that it's because of changing origin point to center, center while rotating. But origin change shouldn't happen at all because rotation had been locked. When I set lockRotation to false it's detecting change of angle property and those four previously mentioned. But it should only show angle. My conclusion is that it shouldn't detect properties change if those properties were modified only as a side effect of transformation. Probably the same situation has place if it goes about scaling.

// From console
hasStateChanged(top) fabric.js:12846
    current:  196.06459398155516 fabric.js:12847
    original: 71.06459398155516 fabric.js:12848
hasStateChanged(left) fabric.js:12846
    current:  627.9755859375 fabric.js:12847
    original: 559.55517578125 fabric.js:12848
hasStateChanged(originX) fabric.js:12846
    current:  center fabric.js:12847
    original: left fabric.js:12848
hasStateChanged(originY) fabric.js:12846
    current:  center fabric.js:12847
    original: top 

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

@lukejagodzinski
Copy link
Author

Problem can be solved by moving the line this._restoreOriginXY(target); before the target.hasStateChanged() function. But this change will probably cause another error - lack of change detection of originX and originY. But is it a problem?

Actually fabric.Canvas.prototype._finalizeCurrentTransform function is only executed on mouseup event. Is there any chance that origin will change permanently (without going to previous state) during object transformation?

fabric.Canvas.prototype._finalizeCurrentTransform: function() {

  var transform = this._currentTransform,
      target = transform.target;

  if (target._scaling) {
    target._scaling = false;
  }

  target.setCoords();

  this._restoreOriginXY(target); // THIS LINE WAS MOVED HERE FROM... ~~~~~~~~

  // only fire :modified event if target coordinates were changed during mousedown-mouseup
  if (this.stateful && target.hasStateChanged()) {
    this.fire('object:modified', {
      target: target
    });
    target.fire('modified');
  }

  // ... THIS POSITION ~~~~~~~~
};

@asturur
Copy link
Member

asturur commented Apr 13, 2016

Hi @jagi, sorry for not seeing this, but you were right about the position of restoreOriginXY.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants