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

Update path.class.js #3774

Merged
merged 1 commit into from
Mar 14, 2017
Merged

Update path.class.js #3774

merged 1 commit into from
Mar 14, 2017

Conversation

by12
Copy link
Contributor

@by12 by12 commented Mar 9, 2017

Bug Fixed Request. Can you please fixed a small bug in path class.

@asturur
Copy link
Member

asturur commented Mar 14, 2017

Sorry i missed this PR.
What the bug fixes? i would be super happy to merge it if i know what does it change.
Any screenshot available?

@by12
Copy link
Contributor Author

by12 commented Mar 14, 2017

Hi,
Only the problem here is you are using two time the same x and y. But normally for the Bezier Curve we need the fabric.util.getBoundsOfCurve( x, y, current[1], current[2], current[3], current[4], current[5], current[6]);

But in your case you have
x = current[5];
y = current[6];
controlX = current[3];
controlY = current[4];

and then fabric.util.getBoundsOfCurve(x, y, current[1], current[2] , controlX, controlY, x, y);
so its mean you always have the same start and end points. current[5], current[6],

fabric.util.getBoundsOfCurve(current[5], current[6], current[1], current[2], current[3], current[4], current[5], current[6]);

which is making problems to get the write boundry box for the curve objects . in screen shot you can see
modified and nonmodified code results.

untitled

@asturur
Copy link
Member

asturur commented Mar 14, 2017

ohhh this could be the error i was searching for so long!!!!!

any idea why on the left screenshot the curve do not touch the left side of the bbox?

@asturur asturur merged commit a7ff845 into fabricjs:master Mar 14, 2017
asturur pushed a commit that referenced this pull request Mar 14, 2017
asturur added a commit that referenced this pull request Mar 14, 2017
asturur added a commit that referenced this pull request Mar 14, 2017
* Revert "Update path.class.js (#3774)"

This reverts commit a7ff845.

* Revert "switch to canvas prebuilt (#3757)"

This reverts commit b979bd5.

* Revert "Fix textarea focus bug when canvas is in an element that stops events from reaching the body (#3754)"

This reverts commit 3f9b69f.
asturur added a commit that referenced this pull request Mar 14, 2017
* Revert "Update path.class.js (#3774)"

This reverts commit a7ff845.

* Revert "switch to canvas prebuilt (#3757)"

This reverts commit b979bd5.

* Revert "Fix textarea focus bug when canvas is in an element that stops events from reaching the body (#3754)"

This reverts commit 3f9b69f.
@by12
Copy link
Contributor Author

by12 commented Mar 15, 2017

Hi,
Screen shots were from quick and dirty modifications. space on left side was some thing else not because of fabric. There are new screen shot with fabric 1.7.8.
untitled

So everything looked fine now.
Have a nice day

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.

2 participants