Fix stroke width calculation

[sapics]

From #1929, strokeWidth calculation dose not depends on stroke.

closes #2177

[asturur]

I definitely missed this.
Inspecting for #2177 i noticed that, and i was about to fix this.

[sapics]

@asturur i found it from #2177, too!

[asturur]

@sapics if you remove the half part of strokewidth, and you scale will FULL strokewidth in both _setObjectScale and _scaleObjectEqually, you will notice that #2177 is fixed.

i mean

strokeWidth = target.strokeWidth;
transform.newScaleX = localMouse.x / (target.width + strokeWidth);
transform.newScaleY = localMouse.y / (target.height + strokeWidth);

For the third function you modified check if apply.

Obviously if we are doing something with width, we cannot just use half of stroke.
For vLine and hLine still some tweak is necessary, so leave strokewidth in a separate var.

Could you make that? if i do this we will have conflicting PR

[sapics]

Absolutely!
I have not understood about definition of localMouse yet, thus, I cannot judge HALF or FULL strokeWidth is right.
It would be better to close this PR to avoid conflict.

[asturur]

I think this PR is good, let's just finish it.
Exactly where you changed the strokeWidth definition, if you remove the use of strokeWidth / 2 and use strokeWidth, the object scaling is fixed 99% of time.

localMouse depends on the translate ( s) function, there are at least 3, and the fix is ugly as shit for them.
the fix would be:

          vLine = target.type === 'line' && target.width === 0,
          hLine = target.type === 'line' && target.height === 0,
          strokeWidthX = hLine ? 0 : target.strokeWidth,
          strokeWidthY = vLine ? 0 : target.strokeWidth;

and use strokeWidthX and strokeWidthY for the x and y instead of general strokeWidth.

If you try to resize this in current kitchensink on fabricJS.com:

canvas.add(new fabric.Rect({
 width:50, height:50,
      fill: 'red',
      stroke: 'blue',
      strokeWidth: 20
}))

You will notice that is wrong.
So the scaling function we have is just wrong, no one notice because large strokeWidth is not so popular.

[asturur]

@sapics thanks!

Can you also rebase the commit to have them nice?

[sapics]

@asturur I am sorry, I misunderstood you wrote.
I create jsfiddle to check this fix.
PREVIOUS: http://jsfiddle.net/sapics/tt5zjjot/8/
THIS PR: http://jsfiddle.net/sapics/tt5zjjot/7/

[sapics]

Sure!!

[asturur]

To fix the "translate effect" that still remain for just horizontal or vertical lines i would wait.

We should spam this code

          vLine = target.type === 'line' && target.width === 0,
          hLine = target.type === 'line' && target.height === 0,
          strokeWidthX = hLine ? 0 : target.strokeWidth,
          strokeWidthY = vLine ? 0 : target.strokeWidth;

All around the functions, while i want to do something more clean.
@kangax i think you can merge this after a quick look.

[sapics]

@asturur Thank you! I have fixed as you coded and rebase!

[kangax]

Awesome, thanks. Don't forget changelog please.