Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Added Project() #638

Merged
merged 4 commits into from
Dec 10, 2023
Merged

Added Project() #638

merged 4 commits into from
Dec 10, 2023

Conversation

elalish
Copy link
Owner

@elalish elalish commented Dec 8, 2023

Addresses one part of #426

Pretty simple algorithm. However, I'm getting lots of micro holes in the bracelet projection, probably from rounding errors, which are messing up the triangulation. I haven't determined if the CrossSection is actually epsilon-valid, and either way, I'm surprised that Clipper's positive fill rule isn't getting rid of them.

@elalish elalish self-assigned this Dec 8, 2023
Copy link

codecov bot commented Dec 8, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (45826d6) 91.62% compared to head (04c1b13) 91.58%.
Report is 1 commits behind head on master.

Files Patch % Lines
src/manifold/src/face_op.cpp 95.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #638      +/-   ##
==========================================
- Coverage   91.62%   91.58%   -0.05%     
==========================================
  Files          36       36              
  Lines        4682     4693      +11     
==========================================
+ Hits         4290     4298       +8     
- Misses        392      395       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -194,17 +196,12 @@ void Manifold::Impl::Face2Tri(const Vec<int>& faceEdge,
* For the input face index, return a set of 2D polygons formed by the input
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to update documentation

@pca006132
Copy link
Collaborator

Yeah I envisioned this epsilon-valid issue when converting our 3D model into 2D, I think similar things will happen for slice as well. Not sure how clipper handles this.

}
glm::vec2 size = box.Size();
if (glm::abs(area) > glm::max(size.x, size.y) * epsilon) {
filtered.push_back(poly);
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is how I took care of the tiny sliver polygons we were getting in the bracelet.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also went ahead and updated Clipper2 to the latest release - I didn't actually notice any difference, but they said they fixed some bugs and I don't think we were on a release at all before.

Copy link
Collaborator

@pca006132 pca006132 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel like we may want to add more simplification to CrossSection later. LGTM for now.

@pca006132 pca006132 merged commit e15f4cf into master Dec 10, 2023
18 checks passed
@pca006132 pca006132 deleted the projection branch December 10, 2023 11:33
@elalish elalish mentioned this pull request Jan 7, 2024
cartesian-theatrics pushed a commit to SovereignShop/manifold that referenced this pull request Mar 11, 2024
* added Project

* updated Clipper2 to 1.3.0

* fix simplify to remove more cruft

* update docs
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants