[Feature request] Better mouse support when lockScalingFlip is true

[ncou]

Hi,

I have set the parameter 'lockScalingFlip' to false in my app, but there is something not really "user friendly" when you change this parameter to true.
The scaling is stopped if the mouse cursor go over the oposite edge of the object (i suppose it should lanch the flip, but in this case with the parameter lock=true, it stop the current action, so the stretching is stopped :( ).

Use case :
http://jsfiddle.net/eA3xH/175/

If you select the bottom right corner of the rect and press you mouse button to stretch the rect, and move the cursor on the left, you can see the stretching is stopped if your mouse go over the left edge of the shape.

Not really sure if i am clear enough.
Let me know if this behavior could be better handled in the futur version.

Have a good day.

[asturur]

Yes is clear, but what should it do?
How would you handle that? do you have any clear idea?

[ncou]

Here is what i think :
we will use this example => click on the bottom right corner, maintain click and move the mouse horizontaly on the left.

Once the mouse cursor cross the left edge, it will be the vertical axis who will scale up or dow the shape.

[asturur]

That could work.
Do you want to try to make a PR for it? i can assist you.

[ncou]

Hi,

Yes i could try to do the changes, can you point me the functions i need to analyse ? thank you.

[asturur]

you should just search in the codebase for lockScalingFlip i bet is used in one or 2 places maximum.
You are interested in the area when the parameter 'by' is 'equally'

[ncou]

It was more easy to change the lib than what i was thinking, really a very good lib :)

1. I choose to mess with the localmouse/x and .y value instead of passing args to the _scaleObjectEqually() function. because localmouse is not used later in the main function. But i could change this and use args if you prefer.
2. I made some tests also with the textbox component because i see it override the scale function.

I made the changes in a temp lib before pushing a PR (if you are ok with point 1) ), here is a fiddle, the left object is locked for flipping, not the other, also a limitscale and a textbox example.
https://jsfiddle.net/wq3pyhhu/19/

Here is what i changed:

_setObjectScale: function(localMouse, transform, lockScalingX, lockScalingY, by, lockScalingFlip, _dim) {
      var target = transform.target, forbidScalingX = false, forbidScalingY = false, scaled = false,
          changeX, changeY, scaleX, scaleY;

      scaleX = localMouse.x * target.scaleX / _dim.x;
      scaleY = localMouse.y * target.scaleY / _dim.y;
      changeX = target.scaleX !== scaleX;
      changeY = target.scaleY !== scaleY;

      if (lockScalingFlip && scaleX <= 0 && scaleX < target.scaleX) {
        forbidScalingX = true;
       //added by NCOU
        localMouse.x = 0;
      }

      if (lockScalingFlip && scaleY <= 0 && scaleY < target.scaleY) {
        forbidScalingY = true;
       //added by NCOU
        localMouse.y = 0;
      }

      if (by === 'equally' && !lockScalingX && !lockScalingY) {
        // OLD code
        //forbidScalingX || forbidScalingY || (scaled = this._scaleObjectEqually(localMouse, target, transform, _dim));
       //changed by NCOU (i removed the check on forbidX&Y equals to false)
        scaled = this._scaleObjectEqually(localMouse, target, transform, _dim);
      }
      else if (!by) {
        forbidScalingX || lockScalingX || (target.set('scaleX', scaleX) && (scaled = scaled || changeX));
        forbidScalingY || lockScalingY || (target.set('scaleY', scaleY) && (scaled = scaled || changeY));
      }
      else if (by === 'x' && !target.get('lockUniScaling')) {
        forbidScalingX || lockScalingX || (target.set('scaleX', scaleX) && (scaled = scaled || changeX));
      }
      else if (by === 'y' && !target.get('lockUniScaling')) {
        forbidScalingY || lockScalingY || (target.set('scaleY', scaleY) && (scaled = scaled || changeY));
      }
      transform.newScaleX = scaleX;
      transform.newScaleY = scaleY;
      forbidScalingX || forbidScalingY || this._flipObject(transform, by);
      return scaled;
    }

I also found a bug with limitscale, i will open a ticket.

[asturur]

Yes so the change coudl be that you do not lock scale but you void the movement in that direction if you detect a flip.

This could work.

but scaleX and scaleY get calculated anyway, before your change and get set later.
Doesn't this influence the scaling anyway?

[ncou]

The change in the code need to apply only when you use a corner of the object to scale, and in this case "by" === "equally", so the scaleX and scaleY (calculated previously) are not used in this case.

All seems to work fine (check my above fiddle), i just found a small bug but it should already exist in the code.
If you set the misScaleLimit to 0 (it's not logical, min value should be 0.01), you got an error (the fabric.js code try to do a "constrain value" when using scaleX or scaleY). But the minScaleLimit parameter is not set by defaut to Math.min(0.01,....)

Something like line 12527 should be done before the constain function (line 12496). no ?
https://github.com/kangax/fabric.js/blob/master/dist/fabric.js#L12527

[awehring]

I'm jumping in here, because you are working on Flip.

There is a small bug in the Flip. You can try it at the Fabric.js homepage with the cat.
The cat flips perfectly when you do it with the top, bottom, left, or right controls.

But it does not flip, when you use the corner controls.

[asturur]

@ncou i m just saying that

      transform.newScaleX = scaleX;
      transform.newScaleY = scaleY;

get changed with some values.

But i also think that if localMouse is 0, the scaleX does not change from the frame before.
i think is good.
Open a pr if you feel like

[asturur]

That other flip issue is something else, unrelated and not so important.

[ncou]

ok, I will open a PR with this changes.

about the bug for the minScaleLimit <= 0, should i open a ticket ?

[asturur]

if you want yes.

[ncou]

ok i do the PR, i also made a build to test all was ok.

I will open the ticket about minScaleLimit <=0 tomorrow, i need to go outside.

Have a good day.