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

dimensions code refactor #2206

Merged
merged 1 commit into from
May 18, 2015
Merged

dimensions code refactor #2206

merged 1 commit into from
May 18, 2015

Conversation

asturur
Copy link
Member

@asturur asturur commented May 18, 2015

Fixes handling of strokewidth limit cases.

I did this to move the logic of calculating how big is an object and put under a function _getUntransformedDimensions() to reuse when necessary.

Lot of functions like:
translateToCenterPoint
translateToOriginPoint
getPointByOrigin

relies on a wrong calculation of the object unscaled dimensions, assuming that
width * scaleX + strokewidth * scaleX
is the solution for everything.

While this is not true for

  • horizontal lines with big strokewidth,
  • object with strokewidth bigger than width
  • and it won't work in situation where the object inside has a complex transform

For example you can try to paste this code in the current kitchensink and try to resize the resulting object:

// clear canvas
canvas.clear();

// add red rectangle
canvas.add(new fabric.Line([100,100,100,200],{
      fill: 'red',
      stroke: 'red',
      strokeWidth: 500,
    }));

You will see that the object moves while you resize it.
This pr fixes that behaviour.

And make easier handle objects where there is a transformMatrix or a skew

closes #2187

@kangax
Copy link
Member

kangax commented May 18, 2015

I love this cleanup and simplification! One thing — untransformed sounds strange to my ear. I would prefer nontransformed or even better withoutTransform.

@kangax
Copy link
Member

kangax commented May 18, 2015

Actually, no, getNonTransformedDimensions and getTransformedDimensions would be best, I think.

@asturur
Copy link
Member Author

asturur commented May 18, 2015

Done.

i will make more changes in little steps.

@kangax
Copy link
Member

kangax commented May 18, 2015

This is not ready yet? More changes coming?

@asturur
Copy link
Member Author

asturur commented May 18, 2015

no no, different PRs with different cleaning ( setCoords is the next one )

kangax added a commit that referenced this pull request May 18, 2015
@kangax kangax merged commit 106b0a5 into fabricjs:master May 18, 2015
@kangax
Copy link
Member

kangax commented May 18, 2015

+changelog

@asturur asturur deleted the boundingboxes branch May 18, 2015 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Changing line stroke width shifts it to the right
2 participants