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

Revamp Boolean Operations #761

Closed
lehni opened this issue Aug 25, 2015 · 31 comments
Closed

Revamp Boolean Operations #761

lehni opened this issue Aug 25, 2015 · 31 comments

Comments

@lehni
Copy link
Member

lehni commented Aug 25, 2015

Despite all the recent improvements on boolean operations in the https://github.com/paperjs/paper.js/tree/boolean-fix branch, built on top of @iconexperience's code that checks for curve overlaps, I start to agree with @hkrish in his observation that the current approach is too complex to fix for all failing edge cases:

@hkrish and @kuribas seem to think that using a sweep-line based algorithm for beziers is the way to go:

#371 (comment)
#371 (comment)

I have found an open-source implementation in Objective C that I believe is based on sweep-line, outlined here:
http://losingfight.com/blog/2011/07/09/how-to-implement-boolean-operations-on-bezier-paths-part-3/

These two posts lead up to that final post:
http://losingfight.com/blog/2011/07/07/how-to-implement-boolean-operations-on-bezier-paths-part-1/
http://losingfight.com/blog/2011/07/08/how-to-implement-boolean-operations-on-bezier-paths-part-2/

The code lives here:
https://bitbucket.org/andyfinnell/vectorboolean/

And in 2014, @BohemianCoding sponsored him to improve the algorithm for inclusion in Sketch app:
http://losingfight.com/blog/2014/01/24/fixes-and-performance-enhancements-for-vectorboolean/

We could attempt porting this to JavaScript? But first we should perform some tests with the complex shapes that our current implementation chokes on, directly in the Objective C code base.

@kuribas
Copy link

kuribas commented Aug 25, 2015

@lehni I test for overlap before the bezier clip algorithm, because it would return an infinite (or very large) amount of points. The source path is first split into curves that increase in the x direction, so there can be no self overlap. The winding contribution is based on the direction of that segment.
A sweep line algorithm still has to take care of all these issues, but the advantage is that it needs to do less intersection tests for a typical drawing, O(log(n+m)(n+m)) versus O(n²), where m is the number of intersections.
I am very much interested in the edge cases where your code chokes on, or also difficult cases that it does correctly.

@lehni lehni self-assigned this Aug 26, 2015
@lehni
Copy link
Member Author

lehni commented Aug 26, 2015

@kuribas we do exactly the same, see here:

https://github.com/paperjs/paper.js/blob/develop/src/path/Curve.js#L1488

The edge cases turned out to be a bug in the code that sorted the found intersections before merging duplicates, and due to variations in precision, the sequence of the time parameters wasn't always the same (tiny variations lead to a different sequence of the found intersections). That's all fixed now!

And I start feeling much more confident in the current solution again. The only part missing now is handling self-intersecting source paths, and more testing... But what you say about the performance still holds, I am sure. Still better to have a working implementation than none : )

@iconexperience
Copy link
Contributor

@lehni

While I understand the algorithms for finding intersections quite well, I am still trying to get into the boolean operations algorithm. Are there any resources/papers that describe the used algorithm? There is of course the Vatti algorithm, and I found this resource:

http://davis.wpi.edu/~matt/courses/clipping/

Do you have any other links?

Regarding performance: I have already seen a bunch of potential improvements in the code, so I wouldn't worry too much. Let's make it work first.

@kuribas
Copy link

kuribas commented Sep 3, 2015

I have documented sweep line implementation: http://kuribas.hcoop.net/Overlap.html

@kuribas
Copy link

kuribas commented Sep 3, 2015

For clarity: this is another boolean operations algorithm than the one implemented in paperjs.

@iconexperience
Copy link
Contributor

@kuribas This is great, but I think Jürg is correct, we should first try to make the current algorithm work, because it already works pretty well in the implementation is more or less finished. The big question is: Will is be possible to resolve the remaining cases where the current implementations still fails or are we on a dead end?

@lehni
Copy link
Member Author

lehni commented Sep 5, 2015

@iconexperience The algorithm that @hkrish fleshed out in 2013 is based on this video here about Path Operations by Google Chrome engineer Cary Clark who is working on Skia:
https://www.youtube.com/watch?v=OmfliNQsk88
I found the subtitles for this video here, may be of use for searching:
https://github.com/samdutton/chromesearch/blob/master/tracks/OmfliNQsk88.srt

@kuribas that's really great, thanks for putting it online! It sounds quite a bit smarter than what we're doing currently. I don't think I have the skills to port this over to JS though, unfortunately, having never written a single line of Haskell so far.

@lehni
Copy link
Member Author

lehni commented Sep 5, 2015

PS: I find it interesting that in order to find intersections, Cary suggests approximating cubics with multiple quadratics, solving their intersections mathematically and then going back to the cubics from there.

@iconexperience
Copy link
Contributor

@lehni Just for the record, replacing cubic beziers with quadratic ones is what all tools that convert svgs to fonts have to do, because TrueType only supports quadratic bezier curves. So in case we need code for doing this, we could take a look at Fontello or similar projects.

@lehni
Copy link
Member Author

lehni commented Sep 5, 2015

@iconexperience yes I'm aware of that. I'm quite happy with our fat line clipping code currently though...

@iconexperience
Copy link
Contributor

@lehni I agree, finding intersections has become pretty good recently.

I watched the video and luckily the algorithm does not seem to be very difficult. It would be nice to have the canvs he uses for PaperJS. Let's see if I can produce something that will help with debugging.

@kuribas
Copy link

kuribas commented Sep 6, 2015

I watched the whole video. At 19:40 he states that it is hard to solve a cubic using math. However Sederberg and Nishita have shown that it is possible using implicitization. According to the paper this method is faster than the bezier clip algorithm. It reduces to a maximum 9th degree polynomial equation, which can be solved numerically. For example using Polynomial Real Root Finding in Bernstein Form

@kuribas
Copy link

kuribas commented Sep 6, 2015

It seems he wasn't aware of this research, or his method predates it.

@lehni
Copy link
Member Author

lehni commented Sep 6, 2015

@kuribas neither was I : ) What did you think of his approach otherwise? How does it compare to what you are doing?

@kuribas
Copy link

kuribas commented Sep 7, 2015

@lehni I linked to the wrong paper, implicitization is explained in cagd. I am using bezier clipping like you do. According to the paper imlicitization is the fastest method for curves of degree 2 and 3, so I assume it will be faster than approximating a quadratic bezier, but I haven't tested it.

The algorithm in the video does O(n*n) comparisons, while my one does O((n+m)log(n+m)) comparisons (n = number of segments, m = number of intersections). For a large number of segments, this may give a significant performance improvement. For a few segments I don't know, that would need to be tested.

I actually look at all points, since that is necessary for the sweepline algorithm to test intersections. For the rest both methods seem to be the very similar. I actually test if two curves are overlapping, or if a point comes near a curve.

@nicholaswmin
Copy link

So, hate to be late to the party but my 2 cents is this:

Has anyone thought about:

  • Flattening the paths
  • Using a polygon-clipping algorithm
  • Reconstructing the curves

I'm under the impression that some info about the curves was kept, just before flattening, that would help on the accuracy of reconstructing them back after clipping has been done.

AFAIK, this was discussed at a point in Angus Johnson's Clipper Lib, somewhere here

Clipper is pretty much based on Vatti which as far as I know is the most 'input tolerant' clipping algorithm. It handles self-intersections/shapes-with-holes just fine but it works only on polygons.

lehni added a commit that referenced this issue Sep 9, 2015
@kuribas
Copy link

kuribas commented Sep 9, 2015

@nicholaswmin according to sederbergh recursive subdivision is much slower than implicitisation. He gives the following relative speed for cubic beziers:

ex1: clipping 2.5, implicitisation 1, subdivision 15

ex2: clipping 1.8, implicitisation 1, subdivison 6

@lehni
Copy link
Member Author

lehni commented Sep 11, 2015

@nicholaswmin I've heard this suggestion before but your link is the first time I see it described in detail.

We've gone very far down the current road which works very well for a whole lot of cases (on the boolean-fix branch). The remaining edge-cases and the handling of self-intersection is what's left to fix, and there are plans in place for both. We should be good to go quite soon!

Ideally I'd like to see @kuribas approach based on the sweepline algorithm implemented in paper.js, but that's another project... I'm already happy when we can finally say that our current implementation is robust, which it almost is now : )

@nicholaswmin
Copy link

@lehni True - I also doubt the approach I described would be anyplace near as fast than what you already have in place anyway.

@lehni
Copy link
Member Author

lehni commented Sep 12, 2015

@nicholaswmin it looks like we just finally managed to resolve all known issues with boolean operations, except this one rare edge case (#773) Even operations on self-intersecting paths are now supported! What's left to do is more testing and some optimizations. This was a very long time in the making, very exciting news!

@petrbrzek
Copy link

Btw: Someone ported Andy Finnell's VectorBoolean to Swift (https://github.com/lrtitze/Swift-VectorBoolean). So it could be a little bit easier to port it to JS.

@lehni
Copy link
Member Author

lehni commented Sep 13, 2015

@petrbrzek thanks for pointing that out! In the meantime we've made so much progress on the current implementation that I don't think we require a port anymore. Our implementation also works with a whole lot less code, which is welcome in a browser environment.

But it's always good to compare edge cases. Did you already compile it? If so, could you compare and see how it handles cases where paths only touch or overlap along curves / lines? e.g. #450 and #449?

@iconexperience
Copy link
Contributor

The recent improvements on the boolean operations are quite impressive, but whenever a change to the code was made, we could see the effect of the change on some cases, but we had no way to quantize the effect of the change regarding the quality of the boolean operations.

Therefore I wrote a test case based on my first curve offset approach, which takes a curve, splits it into several subcurves, creates an offest path for reach subcurve, and finally merges the offset path using boolean operations. The test automatically runs through 8000 curves, making about 42000 calls to path.unite(). The curves are fairly random, but each time the same curves are used. Since the algorithm creating the offset paths is rather clumsy, there are many cases with small hooks, self intersections, tiny loops and many imprecisions, it is very useful for creating difficult test cases for finding hidden issues in the boolean operations. I ran this test for the most recent commits and recorded the number of glitches (error messages still creating a result) and errors (usually stack size exceeded errors). I also compared to the result before the commit to find the number of new and resolved cases. And here are the results:

Commit glitches absolute glitches relative (new/solved) errors absolute errors relative (new/solved) comment
9b883e5 43 +0/-0 0 +0/-0
5601e2 43 +0/-0 0 +0/0
807318 43 +2/-0 0 +0/-1 includes 5601e21
79cb21 unusable, skipped
86b1b74 41 +2/-2 1 +1/-3 includes 5601e21
2bed611 41 +0/-0 3 +0/-0 includes 5601e2
6fb4b7e 41 +0/-5 3 +0/-0 includes 5601e21
5d7a596 46 +0/-0 3 +0/-0 includes 5601e21
50c7473 46 +1/-15 3 +1/-1 includes 5601e21
2167e45 60 +0/-0 3 +0/-0
7496a7c 60 +0/-0 3 +0/-0
632eb25 60 +0/-2 3 +1/-0
c6de2f7 62 +0/-0 2 +0/-0
11611c8 62 +1/-0 2 +0/-0
a808aaf 61 +0/-0 2 +0/-0
de57a7f 61 +0/-0 2 +0/-0
c0bb689 61 +0/-0 2 +0/-0
4b4ccba 61 +21/-5 2 +2/-0
9b883e5 45 +3/-0 0 +0/-0
b8c6eb4 42 +1/-0 0 +0/-0 includes 72f9705
c77165b 41 +1/-27 0 +0/-0 includes 72f9705
75a0041 67 +3/-0 0 +0/-0
5f706a4 64 +1/-12 0 +0/-0
9bcf369 75 +0/-0 0 +0/-0
2a7d1c5 75 +0/-6 0 +0/-11
4e9bac1 81 +0/-0 11 +0/-0
ec70fa1 81 +8/-98 11 +4/-2
3fa810a 171 +0/-0 9 +0/-0
fd927cb 171 +3/-0 9 +0/-0
fc0b5a8 168 +0/-2 9 +0/-4
c79166a 170 +0/-2 13 +0/-0
cf5bf3 172 +0/-0 13 +0/-0
515d4f 172 +9/-11 13 +3/-1
cc7e60e 174 +2/-0 11 +0/-0
20f950a 172 +0/-0 11 +0/-0
db1ecdd 172 +6/-2 11 +1/-0
6a29f20 168 +0/-1 10 +0/-2
51c3444 169 +0/-0 12 +0/-0
84bcc53 169 +6/-7 12 +3/-6
0f61ce8 170 +0/-0 15 +0/-0
812ac63 170 +14/-44 15 +1/-3
812ac63 209 +9/-0 17 +16/-0
e36319b 200 +6/-1 1 +0/-0
f6f42fe 195 +0/-2048 1 +0/-6
19a17b2 2243 +0/-0 7 +0/-0
1ad95c9 2243 +2243/-0 7 +7/-0

You should note that the results are not very representative, but the give an indication of the effectiveness of each commit. I hope this is helpful.

I will make this test available here, but please be patient, as it need some cleaning first.

@lehni
Copy link
Member Author

lehni commented Oct 4, 2015

This is very useful, thanks! It'd be great if you could share the code for this with me, so I can immediately see the impact of my changes on the result.

@iconexperience
Copy link
Contributor

I will, but I need to clean it first, because it's a mess. I will post more examples if you do not mind?

@lehni
Copy link
Member Author

lehni commented Oct 4, 2015

Sure keep sending them : ) I'm curious about the +2 in 8073183... That doesn't really make sense, so I'd like to play with it to understand what happened.

@lehni
Copy link
Member Author

lehni commented Oct 21, 2015

So after a ton of more work, the current implementation works with all known edge cases from @iconexperience's test suite. It's been a long and bumpy road to get here, but the code is now very sturdy and clean. I'm super happy with how it all turned out. I am closing this issue now, as all that's left to do is implement some unit tests to protect against regressions, and roll it all out.

For those curious about the implementation, the code is mainly in these files:

https://github.com/paperjs/paper.js/blob/develop/src/path/PathItem.Boolean.js
https://github.com/paperjs/paper.js/blob/develop/src/path/CurveLocation.js
https://github.com/paperjs/paper.js/blob/develop/src/path/Curve.js

@lehni lehni closed this as completed Oct 21, 2015
@be5invis
Copy link

@lehni is it possible to create a separate library which deals with boolean operations only, since paper is a bit too large.

@lehni
Copy link
Member Author

lehni commented Oct 22, 2015

@be5invis too large for what? And without paper.js, on what kind of data-structures would it operate on? SVG? Many things are possible, but I don't have the urge work on this particular project. The boolean code relies heavily on many of the data structures in paper.js, which is one of the reasons why the library was built this way. I'm happy with that structure, because it makes working with bezier paths much more simple and enjoyable. I don't think that a 180kb library is too heavy for handling the complex math involved with such bezier curve operations. Having to maintain two separate libraries that do the same thing on the other hand would be too heavy in a whole different way.

@petrbrzek
Copy link

@lehni: Nice job man! 👍

@petrbrzek
Copy link

@lehni When you plan to release a new version?

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

6 participants