rough-stuff / rough

Create graphics with a hand-drawn, sketchy, appearance
http://roughjs.com
MIT License
19.83k stars 613 forks source link

Wrong filling for particular path in 4.x #153

Open Oreilles opened 4 years ago

Oreilles commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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