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

fix: multiple polygons to support holes #13

Closed
wants to merge 5 commits into from

Conversation

mvarendorff
Copy link

This PR fixes a bug with paths that contained multiple polygons which is easiest displayed with an example. Consider the following SVG:

<?xml version="1.0" encoding="utf-8" standalone="no" ?>
<svg viewBox="0 0 240 200" height="180" width="220" version="1.1" xmlns="http://www.w3.org/2000/svg">
<g fill="blue" fill-rule="evenodd">
    <path d="M20 20 h200 v160 h-200 Z M80 40 v100 l100 -50z" />
</g>
</svg>

grafik

This SVG currently only creates a single Polygon:

grafik

This PR fixes this by keeping track of the current polygon and adding to it in case multiple Z commands are found in the current path. With this change, the demo correctly displays a cut-out:

grafik

Notes

  • Husky regenerated hooks to point to a blob on my fork, I am not sure how to remedy that but I suppose it's gonna be rebuilt anyway after merge? If I have to fix that in some shape or form, let me know!
  • This project is awesome and saved us a lot of time. Thanks a bunch for your effort to create this!

@mvarendorff mvarendorff changed the title fix: mulitple polygons to support holes fix: multiple polygons to support holes Jul 10, 2023
@mvarendorff mvarendorff closed this Aug 8, 2023
@mvarendorff
Copy link
Author

Let me know if you are interested in including this change into the repo; we are using my master branch now internally, so this PR will start to include more stuff that's going to be unrelated to this bugfix :')

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.

None yet

1 participant