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

feat(fabric.Path): Change the way path is parsed and drawn #6504

Merged
merged 4 commits into from
Aug 8, 2020

Conversation

asturur
Copy link
Member

@asturur asturur commented Aug 7, 2020

We want to simplify path code in order to make space for text on path ( curved text ) 🎉

First step of this work is to remove from the path:
all the non absolute commands, all the Arcs, and V and H.

Less command types will also mean less drawing and bounding box code.

var _join = Array.prototype.join;

function segmentToBezier(th2, th3, cosTh, sinTh, rx, ry, cx1, cy1, mT, fromX, fromY) {
var costh2 = fabric.util.cos(th2),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, just for sure, this PR means the fabric.js will support Bezier Curve soon?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean? fabric always drawn beizer curves but does not give you the ability to interact and modify them.

Copy link
Contributor

@juzhiyuan juzhiyuan Aug 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yes, I mean if this PR could give us ability to modify shapes with Becker Curves :DD hoping someone would create a sample 🎊

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR aims at removing everything that is not M L C Q from complex path. so that is easier to write code to handle paths manually.
So yes, it will help definitely with you text/path thing.

@asturur
Copy link
Member Author

asturur commented Aug 8, 2020

OK UT pass, but i first need a visual regression test PR to be merged on the current master. and run this gain

]);
x = current[6];
y = current[7];
break;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@melchiar i wrote code in the file util/path.js to handle 'a' and 'A' at parse time, and also all the lower case commands.
I would like to get rid in this function also of case 'T' and case 'S'. and keep 'C' and 'Q'.
An 'S' would become a paricular different 'C' and a 'T' a particular different 'Q'.

The drawing code in this function explains how. and the code should be added in util/path.js function makePathSimpler. The transtion from s to S and to t to T is already handled.

@asturur asturur merged commit 1c2164b into master Aug 8, 2020
@asturur asturur deleted the path-semplification branch August 9, 2020 04:56
@asturur asturur mentioned this pull request Aug 23, 2020
shanicerae pushed a commit to shanicerae/fabric.js that referenced this pull request Jan 16, 2021
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