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

Wrong filling for particular path in 4.x #153

Open
Oreilles opened this issue Apr 22, 2020 · 5 comments
Open

Wrong filling for particular path in 4.x #153

Oreilles opened this issue Apr 22, 2020 · 5 comments

Comments

@Oreilles
Copy link
Contributor

It's a checkerboard; The filling algorithms are giving expected result in 3.1.0, but failing in newer version (tested on 4.2.3).Fiddle to replicate:
https://jsfiddle.net/8qscjo1n/

The bug affects ell fillStyle algorithms. But careful, path is quite long, so using dots can crash the page.

@pshihn
Copy link
Collaborator

pshihn commented Apr 22, 2020

This may be related to your other bug with 00 in the path.

Also, the way you are drawing is just not efficient. Why don't you just draw 64 squares as opposed to a non-optimized path?

Anyways I will see why it's happening. It could also be that your path is actually multiple sections so it gets really tricky to fill .

@Oreilles
Copy link
Contributor Author

Also, the way you are drawing is just not efficient. Why don't you just draw 64 squares as opposed to a non-optimized path?

I'm not the author, and I also don't quite understand why he choosed to do it that way. (here's the source)

It might well be related to the other bug, however this isn't the exact same path, this one hasn't been minified with svgo.

@pshihn
Copy link
Collaborator

pshihn commented Apr 22, 2020

The new algorithm estimates a single polygon out of a SVG path. Your Path has several unconnected paths and some paths are nested within the outer path. Since they are all in a single path, it is a single polygon. I will have to create a special case for this.

Sorry about this. but it may take some time to figure out the best way.

I would suggest draw individual squares or revert to the older version of roughjs as it may work better for you.

The new version only has new svg optimization which is a problem for your case, so you wont be missing out on anything by using an older version.

In the meantime I will keep this bug open to resolve it

@apennamen
Copy link

Hello @pshihn
I noticed this bug in version 4 of roughjs, while building the wired icon generator
So there's a real use case for icons which uses complex paths.
I can provide tests cases if you want

Regards,
AP

@apennamen
Copy link

If it can ease your tests, I made a version of the wired icon generator with rough 4.3.0, available here:
https://deploy-preview-17--wired-icon-generator.netlify.app/

You can compare with the rough 3.1.0:
https://wired-icon-generator.netlify.app/

For instance if you load the provided example (it's the "build" icon from the Material SVG iconset), here are some differences:

Fill: solid

version 3.1.0
image

version 4.3.0
image

Fill: zigazg

version 3.1.0
image

version 4.3.0
image

Hope it can help you, wish you all the best!

Regards,
AP

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

No branches or pull requests

3 participants