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

Specific boolean intersection case fail since v0.10.4 #1619

Closed
sasensi opened this issue Nov 29, 2018 · 11 comments
Closed

Specific boolean intersection case fail since v0.10.4 #1619

sasensi opened this issue Nov 29, 2018 · 11 comments

Comments

@sasensi
Copy link
Contributor

sasensi commented Nov 29, 2018

Description/Steps to reproduce

Do boolean intersection between 2 specific shapes.

Link to reproduction test-case

The bug is reproduced in this sketch.

Expected result

screenshot-sketch paperjs org-2018 11 29-10-00-55

Actual result

screenshot-sketch paperjs org-2018 11 29-10-54-37

Additional information

This bug only shows up in versions 0.10.4 and later.
It was originally reported here.

@lehni
Copy link
Member

lehni commented Nov 29, 2018

The way boolean operations are currently implemented, unfortunately there will likely always be some edge cases that fail. @hkrish has some good ideas about a better approach to boolean operations, but implementing it in paper.js is far from trivial. I am very reluctant to spending more time on ironing out kinks in edge cases, but maybe @iconexperience could take a look? He helped a lot with the past iterations of improvements...

@sasensi
Copy link
Contributor Author

sasensi commented Nov 30, 2018

I've read passed discussion about boolean operations but I didn't get far enough to understand how they are implemented in detail.
Have somebody already considered porting Inkscape implementation (https://gitlab.com/inkscape/inkscape) which seems to be working well ?
I had a quick look at it but I don't know much about C++ so I can't estimate how much work this would be.

@lehni
Copy link
Member

lehni commented Dec 3, 2018

I've looked at InkScape in the past, but I think the code is rather massive... Size is a concern here too. Not sure what's best to do at this point, as we've progressed pretty far with the current approach. It just feels like edge cases don't stop to pop up!

@hkrish
Copy link
Contributor

hkrish commented Dec 3, 2018

Is it such a crazy idea to deprecate boolean ops altogether within the core library, and inherit from some thing like https://skia.org/user/modules/pathkit?

@lehni
Copy link
Member

lehni commented Dec 3, 2018

Oh wow, I wasn't aware of this! Not such a crazy idea, but the asm.js is almost twice the size of the whole paper.js library, and the wasm version is about the same size... I guess outsourcing would make sense though. @hkrish did you test it? Is it good? I remember Skia's boolean ops having some glitches and side-effects in the early days?

@hkrish
Copy link
Contributor

hkrish commented Dec 3, 2018

It is quite a large library for sure (gzipped download size, for wasm seems to be comparable to paperjs, was it ~130K?). I haven't tried it yet, so no idea on glitches or accuracy. It seems they have been working on it though --https://skia.org/dev/present/pathops.
Also they have a "canvaskit" pending release which is a canvas based implementation of whole skia?! It all depends on what will people use paperjs for, for building interactive editors and such, this might not be a bad idea.

@sasensi
Copy link
Contributor Author

sasensi commented Dec 4, 2018

I've set up a little fiddle running Skia and Paper.js in parallel to test boolean operations results.
It can be used as a first step to check if it performs better or not.

@sasensi
Copy link
Contributor Author

sasensi commented Dec 4, 2018

Actually, for this specific case, Skia has the same intersection bug 😞 (fiddle).
And by tweeking path data just a little, Paper.js result is better (fiddle).
It seems that they have problems with edge cases too.

@lehni
Copy link
Member

lehni commented Dec 4, 2018

Wow... That's pretty crazy :) And kinda good news for our code! It means we're not doing that badly...

@hkrish
Copy link
Contributor

hkrish commented Dec 4, 2018 via email

@lehni
Copy link
Member

lehni commented Jun 22, 2019

I've reduced this case to this sketch for further analysis.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants