2d-operations via Clipper2 #341

geoffder · 2023-02-27T23:07:24Z

Fixes #127

Minimal proof of concept of integrating Clipper2 in order to flesh out Manifold's 2d capabilities.

elalish

Thank you, this is excellent - I'm also really glad to see how much functionality is here with not that much code. @pca006132 what do you think regarding API? Anything we should consider regarding our various bindings?

src/clippoly/src/clippoly.cpp

src/clippoly/include/clippoly.h

src/manifold/include/manifold.h

src/clippoly/include/clippoly.h

src/clippoly/src/clippoly.cpp

elalish

Since all the comments are now marked outdated due to the rename, would you mind resolving the ones you've finished so we know what's left to do?

src/cross_section/CMakeLists.txt

geoffder · 2023-02-28T22:12:07Z

Now that usage of CrossSection is forced, 11 of the tests are failing. I'm afraid that I'm not sure which of them are acceptable changes (if any) from my brief looking around.

elalish

Looks like the tests are doing their job! Do you feel comfortable debugging? The simplest thing is probably just to dump out the input polygons and ToPolygon result and see what changed. Let me know if you get stumped.

src/collider/src/collider.cpp

geoffder · 2023-02-28T22:53:17Z

Looks like the tests are doing their job! Do you feel comfortable debugging? The simplest thing is probably just to dump out the input polygons and ToPolygon result and see what changed. Let me know if you get stumped.

Ok, I'll try to get some in later and report on what's happening to the inputs. I'm suspecting that at least some of them are due to the precision rounding step, but there could of course be deeper things going on with the clipping. The outputs are still manifold and the exported model glbs seem fine, but obviously some vertices are moving around to throw off the geometry checks.

src/cross_section/src/cross_section.cpp

geoffder · 2023-03-01T04:01:01Z

All tests now passing on my machine 👍.

codecov · 2023-03-01T04:21:35Z

Codecov Report

Patch coverage: 60.86% and project coverage change: -2.02 ⚠️

Comparison is base (7c8ce3c) 91.52% compared to head (9289f18) 89.50%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #341      +/-   ##
==========================================
- Coverage   91.52%   89.50%   -2.02%     
==========================================
  Files          32       33       +1     
  Lines        3730     3965     +235     
==========================================
+ Hits         3414     3549     +135     
- Misses        316      416     +100

Impacted Files	Coverage Δ
samples/src/rounded_frame.cpp	`100.00% <ø> (ø)`
src/manifold/src/manifold.cpp	`92.01% <ø> (+3.04%)`	⬆️
src/utilities/include/public.h	`69.69% <43.75%> (-5.01%)`	⬇️
src/cross_section/src/cross_section.cpp	`59.16% <59.16%> (ø)`
src/manifold/src/constructors.cpp	`95.05% <94.11%> (-1.06%)`	⬇️
samples/src/bracelet.cpp	`100.00% <100.00%> (ø)`
samples/src/knot.cpp	`100.00% <100.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

geoffder · 2023-03-01T04:44:26Z

I haven't converted Manifold (and the rest of the codebase) over yet, but I lifted the circular quality static globals into a class Quality that can be shared by CrossSection and Manifold. If that sounds reasonable to you, I can remove the originals from Manifold and replace the usages with Quality (or whatever you'd like it renamed as).

elalish · 2023-03-01T05:33:39Z

I haven't converted Manifold (and the rest of the codebase) over yet, but I lifted the circular quality static globals into a class Quality that can be shared by CrossSection and Manifold. If that sounds reasonable to you, I can remove the originals from Manifold and replace the usages with Quality (or whatever you'd like it renamed as).

Yes, that sounds like a great idea. Also, how do you want to approach this PR? We'll want to update various bindings and examples to make use of what you've built. Would you like to keep working here, or get this PR merged and look at follow-ons? I'm not too picky, but I'm working on getting a release out, and I think it's best for this to come after.

geoffder · 2023-03-01T05:55:41Z

Yes, that sounds like a great idea.

Roger, I'll change things over then.

Also, how do you want to approach this PR? We'll want to update various bindings and examples to make use of what you've built. Would you like to keep working here, or get this PR merged and look at follow-ons? I'm not too picky, but I'm working on getting a release out, and I think it's best for this to come after.

I think merging first, then updating bindings works fine. Since Polygons has a constructor in CrossSection and everything is working without "knowing" that CrossSections exists, the bindings haven't been broken and from here the changes they need are additive.

src/cross_section/include/quality.h

src/cross_section/include/cross_section.h

samples/src/knot.cpp

src/cross_section/src/cross_section.cpp

pca006132 · 2023-03-02T16:07:44Z

Just curious: Why do we name this as cross section instead of polygon? Other than this I don't think I have anything to comment on, this looks pretty good to me!

elalish · 2023-03-02T16:56:43Z

Just curious: Why do we name this as cross section instead of polygon? Other than this I don't think I have anything to comment on, this looks pretty good to me!

We still have Polygons which is just a vector of vectors. I think of Polygons as the 2D version of Mesh and CrossSection as the 2D version of Manifold. With polygons you have to ask yourself if they are self-intersected or not, while a cross section is meant to imply geometric sanity. Someday I hope to get there with Manifold too. And certainly its usage so far (in Extrude and Revolve) is as a cross-section.

elalish

This is looking great! What else would you like to do before we merge?

src/cross_section/include/cross_section.h

src/cross_section/src/cross_section.cpp

src/utilities/include/public.h

geoffder · 2023-03-02T22:03:50Z

This is looking great! What else would you like to do before we merge?

I can't think of anything at the moment that needs to be added in the first go. Probably a good place to call it?

This reverts commit af8f6c8.

geoffder · 2023-03-02T22:25:54Z

This is looking great! What else would you like to do before we merge?

I can't think of anything at the moment that needs to be added in the first go. Probably a good place to call it?

Actually, the last potential piece would be wrapping Clipper2Lib::Rect as well. Giving us a 2d bounding box and access to "rect clipping" which is a fast special case of intersection.

elalish

Excellent work! Ready to merge?

geoffder · 2023-03-03T17:21:08Z

Looking at it again I've remembered that I haven't added Transform yet for the 2d types. That would bring them another step closer to the 3d counterparts and make it ready I think.

geoffder · 2023-03-03T22:24:24Z

Ok, Transform is in (along with a test like the one for Manifold) 👍

elalish

Looks great, thanks!

elalish · 2023-03-06T16:25:35Z

@geoffder I would still like to see the equivalent Mirror function in Manifold, not to mention exposing CrossSection in our various bindings. Are you interested in taking some of that on? Also, do you want to add yourself to our AUTHORS file? I can also add you as a collaborator on Github if you'd like to stay involved. I appreciate your contributions!

geoffder · 2023-03-06T19:14:13Z

@elalish
I wrote out the new C bindings on the weekend, just need to test them out via the ocaml bindings to make sure that I didn't miss something and that they work as expected. I can add the Manifold mirroring to my list as well.

Also, do you want to add yourself to our AUTHORS file? I can also add you as a collaborator on Github if you'd like to stay involved. I appreciate your contributions!

Sure I'll add myself on there soon, thanks for the offer 🙏. Continuing to collaborate and stay involved sounds good to me.

* WIP Clipper2 wrapper for 2d subsystem * Working example in clippoly_test * Add clean_ property to clippoly * Square center=false by default * Square defaults to center=false * Shorten union example * + simplification and minkowski functions * + Circle and note on Minkowski * + Offset (InflatePaths) * Clippoly -> CrossSection renaming * Switch from NonZero to Positive fill rule * -(minkowski, simplifs, clean_) + pre-alloc vecs Remove minkowski, simplification algorithms other than simplify, clean (union) arbitrary contours on construction, and pre-allocate vectors when length is known. * - index from polygon conversion + missed vec prealloc * Use Polygon shorthand over std::vector * Update copyright date * Add Rotate * Update copyrights to 2023 * Extrude and Revolve accept CrossSection only * Fix incorrect use of vector constructor * Fix remaining incorrect vector construction * Revert "Update copyrights to 2023" This reverts commit af8f6c8. * Re-update copyright notice in cross_section files * Add missing precision_ argument to cleaning Unions * Lift circular resolution globals into Quality * Replace uses of Manifold globals with Quality * + Ellipse, Mirror, Area * Include mirror in Union example and test area * + Rewind (reverse winding of all contours) * + fillrule/IsEmpty - Ellipse/Rewind * Move Quality into public.h * Remove bad definition * Replace manual circles with CrossSection::Circle * Lists of empty contours are empty * + Rect class, RectClip, + const params/methods * Add Transform to CrossSection and Rect

geoffder mentioned this pull request Feb 27, 2023

Flesh out 2D subsystem #127

Closed

elalish reviewed Feb 28, 2023

View reviewed changes

elalish marked this pull request as ready for review February 28, 2023 20:04

elalish reviewed Feb 28, 2023

View reviewed changes

src/cross_section/CMakeLists.txt Outdated Show resolved Hide resolved

elalish requested changes Feb 28, 2023

View reviewed changes

src/collider/src/collider.cpp Outdated Show resolved Hide resolved

elalish reviewed Feb 28, 2023

View reviewed changes

src/cross_section/src/cross_section.cpp Outdated Show resolved Hide resolved

elalish requested changes Mar 2, 2023

View reviewed changes

pca006132 reviewed Mar 2, 2023

View reviewed changes

src/cross_section/src/cross_section.cpp Outdated Show resolved Hide resolved

elalish reviewed Mar 2, 2023

View reviewed changes

src/cross_section/include/cross_section.h Show resolved Hide resolved

src/cross_section/src/cross_section.cpp Show resolved Hide resolved

src/cross_section/src/cross_section.cpp Outdated Show resolved Hide resolved

src/utilities/include/public.h Show resolved Hide resolved

geoffder added 11 commits March 2, 2023 14:05

WIP Clipper2 wrapper for 2d subsystem

a44bdba

Working example in clippoly_test

b38edae

Add clean_ property to clippoly

7c30eae

Square center=false by default

ab4c45b

Square defaults to center=false

022dba3

Shorten union example

5cb7914

+ simplification and minkowski functions

7645fe7

+ Circle and note on Minkowski

2bd5b61

+ Offset (InflatePaths)

4d56828

Clippoly -> CrossSection renaming

9d006fa

Switch from NonZero to Positive fill rule

fa0bebb

geoffder added 17 commits March 2, 2023 14:05

Update copyrights to 2023

55f7dd2

Extrude and Revolve accept CrossSection only

d9ad39d

Fix incorrect use of vector constructor

c5dccd6

Fix remaining incorrect vector construction

2ed4403

Revert "Update copyrights to 2023"

8af1967

This reverts commit af8f6c8.

Re-update copyright notice in cross_section files

9b2aab9

Add missing precision_ argument to cleaning Unions

997414f

Lift circular resolution globals into Quality

53f1d99

Replace uses of Manifold globals with Quality

5cf0ab7

+ Ellipse, Mirror, Area

70386cf

Include mirror in Union example and test area

9fbc3ad

+ Rewind (reverse winding of all contours)

88b4d9f

+ fillrule/IsEmpty - Ellipse/Rewind

4e3a3dd

Move Quality into public.h

d112824

Remove bad definition

f9ce6bc

Replace manual circles with CrossSection::Circle

e485a55

Lists of empty contours are empty

9487335

+ Rect class, RectClip, + const params/methods

7f9abfb

elalish approved these changes Mar 3, 2023

View reviewed changes

Add Transform to CrossSection and Rect

9289f18

elalish approved these changes Mar 3, 2023

View reviewed changes

elalish merged commit a97c666 into elalish:master Mar 4, 2023

geoffder deleted the 2d-via-clipper branch March 4, 2023 01:29

elalish mentioned this pull request Nov 3, 2023

V2.2.1 #589

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

2d-operations via Clipper2 #341

2d-operations via Clipper2 #341

geoffder commented Feb 27, 2023 •

edited by elalish

Loading

elalish left a comment

elalish left a comment

geoffder commented Feb 28, 2023 •

edited

Loading

elalish left a comment

geoffder commented Feb 28, 2023

geoffder commented Mar 1, 2023

codecov bot commented Mar 1, 2023 •

edited

Loading

geoffder commented Mar 1, 2023

elalish commented Mar 1, 2023

geoffder commented Mar 1, 2023

pca006132 commented Mar 2, 2023

elalish commented Mar 2, 2023

elalish left a comment

geoffder commented Mar 2, 2023

geoffder commented Mar 2, 2023

elalish left a comment

geoffder commented Mar 3, 2023

geoffder commented Mar 3, 2023 •

edited

Loading

elalish left a comment

elalish commented Mar 6, 2023

geoffder commented Mar 6, 2023

2d-operations via Clipper2 #341

2d-operations via Clipper2 #341

Conversation

geoffder commented Feb 27, 2023 • edited by elalish Loading

elalish left a comment

Choose a reason for hiding this comment

elalish left a comment

Choose a reason for hiding this comment

geoffder commented Feb 28, 2023 • edited Loading

elalish left a comment

Choose a reason for hiding this comment

geoffder commented Feb 28, 2023

geoffder commented Mar 1, 2023

codecov bot commented Mar 1, 2023 • edited Loading

Codecov Report

geoffder commented Mar 1, 2023

elalish commented Mar 1, 2023

geoffder commented Mar 1, 2023

pca006132 commented Mar 2, 2023

elalish commented Mar 2, 2023

elalish left a comment

Choose a reason for hiding this comment

geoffder commented Mar 2, 2023

geoffder commented Mar 2, 2023

elalish left a comment

Choose a reason for hiding this comment

geoffder commented Mar 3, 2023

geoffder commented Mar 3, 2023 • edited Loading

elalish left a comment

Choose a reason for hiding this comment

elalish commented Mar 6, 2023

geoffder commented Mar 6, 2023

geoffder commented Feb 27, 2023 •

edited by elalish

Loading

geoffder commented Feb 28, 2023 •

edited

Loading

codecov bot commented Mar 1, 2023 •

edited

Loading

geoffder commented Mar 3, 2023 •

edited

Loading