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(convertPathData): convert c to q #1892

Merged
merged 9 commits into from
Jan 1, 2024
Merged

feat(convertPathData): convert c to q #1892

merged 9 commits into from
Jan 1, 2024

Conversation

KTibow
Copy link
Contributor

@KTibow KTibow commented Dec 20, 2023

Some cubic beziers are quadratic beziers in disguise! They may have been degree elevated or just happen to be approximatable. Whatever the case is, we can find a smaller, faster representation as a quadratic bezier.
See https://stackoverflow.com/questions/2009160/how-do-i-convert-the-2-control-points-of-a-cubic-curve-to-the-single-control-poi for a better explanation.

Results

Closes #1891
Compared to current SVGO, removes 6452 bytes on Isometric Madness w/o multipass and 6500 with.
This would probably help some icon sets a lot, haven't tested the effects yet though.

@SethFalco

This comment was marked as off-topic.

@KTibow

This comment was marked as off-topic.

dont worry that it now looks different, it's still under the error so its really just doing what svgo is supposed to do
@SethFalco

This comment was marked as off-topic.

@KTibow

This comment was marked as off-topic.

@SethFalco

This comment was marked as off-topic.

@KTibow

This comment was marked as off-topic.

@SethFalco

This comment was marked as off-topic.

@KTibow

This comment was marked as off-topic.

@KTibow

This comment was marked as off-topic.

@SethFalco

This comment was marked as off-topic.

Copy link
Member

@SethFalco SethFalco left a comment

Choose a reason for hiding this comment

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

I think the math can be simplified, which is mentioned in the Stack Overflow post as well, but otherwise this looks great. Nice idea!

See any issues with the suggestion?

test/plugins/convertPathData.16.svg Show resolved Hide resolved
plugins/convertPathData.js Outdated Show resolved Hide resolved
KTibow and others added 2 commits January 1, 2024 12:29
Co-authored-by: Seth Falco <seth@falco.fun>
Copy link
Member

@SethFalco SethFalco left a comment

Choose a reason for hiding this comment

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

You can ignore the failing tests, you probably noticed, but we have an intermittent issue atm. Will look into that later, nothing to do with this PR.

Thanks for the contribution!

@SethFalco SethFalco merged commit 8644cf3 into svg:main Jan 1, 2024
8 of 10 checks passed
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.

Convert c to q
2 participants