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

Smooth Path utils #7140

Merged
merged 9 commits into from
Jun 27, 2021
Merged

Conversation

ShaMan123
Copy link
Contributor

@ShaMan123 ShaMan123 commented Jun 21, 2021

This PR replaces most of #7082
It focuses on smooth path utils.

Changes:

  1. Extracted convertPointsToSVGPath to path util and renamed to getSmoothPathFromPoints
  2. Added default correction value to getSmoothPathFromPoints
  3. Adapted pencil brush and eraser brush
  4. Adjusted tests

@ShaMan123
Copy link
Contributor Author

ShaMan123 commented Jun 21, 2021

Should I add the path builder to this PR?
It is now very simple and minor because of the refactoring + tests.

fabric.Path.pointsToSmoothPath = function (points, options) {
    return new fabric.Path(fabric.util.getSmoothPathFromPoints(points), options);
  };

or fromPointsToSmoothPath

If not I will wait for this to merge

@ShaMan123 ShaMan123 changed the title Refactor convertPointsToSVGPath Smooth Path utils Jun 21, 2021
@asturur
Copy link
Member

asturur commented Jun 25, 2021

here i am late again!

Copy link
Member

@asturur asturur left a comment

Choose a reason for hiding this comment

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

If we remove that extra if cycle we are good to go with this pr!

src/util/path.js Outdated Show resolved Hide resolved
src/util/path.js Outdated Show resolved Hide resolved
@asturur asturur merged commit f9011cd into fabricjs:master Jun 27, 2021
@ShaMan123 ShaMan123 deleted the refactor-convertPointsToSVGPath branch July 1, 2021 10:01
@asturur asturur mentioned this pull request Aug 27, 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