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

Triangulator robustness #154

Merged
merged 7 commits into from
Jul 18, 2022
Merged

Triangulator robustness #154

merged 7 commits into from
Jul 18, 2022

Conversation

elalish
Copy link
Owner

@elalish elalish commented Jul 17, 2022

So, this doesn't actually solve #148, but it does fix part of it. I found my triangulator was failing on certain completely colinear polygons. The fix is fairly simple, since they just need to be identified - the simple monotone triangulator already works for them since any triangulation is admissible (all possible triangles are degenerate).

In the process of debugging I realized I didn't have as many intermediate checks operating in our tests as I had planned. I've enabled them now, which help the tests to catch more errors at the cost of a little slowdown (mostly on Sponge4, since most of its time is in triangulation). I also found and fixed a couple of bugs where I was accidentally recalculating normals instead of passing them along.

I've added the test from #148 as Disabled, since it still doesn't pass. Currently the triangulator fails on a simple polygon that is in fact ε-overlapped. I believe the issue is that the precision of the new intersection verts is worse than our precision calculation since nearly-parallel intersections can drive the error up to the size of the triangle overlaps. I'm not sure the best way to handle that - driving up the overall precision band with each operation will quickly make solutions impractical.

However, the good news is that I believe I now have a fairly simple way to create manifold (though geometrically invalid) triangulations from overlapped polygons. This should allow us to make all the triangulator exceptions optional, which was the biggest thing blocking #141. This may "solve" #148 too; the main question is whether small overlaps remain small or grow unacceptably large.

@codecov-commenter
Copy link

codecov-commenter commented Jul 17, 2022

Codecov Report

Merging #154 (e63e9b3) into master (6247784) will increase coverage by 0.08%.
The diff coverage is 93.93%.

@@            Coverage Diff             @@
##           master     #154      +/-   ##
==========================================
+ Coverage   93.72%   93.81%   +0.08%     
==========================================
  Files          32       32              
  Lines        2838     2861      +23     
==========================================
+ Hits         2660     2684      +24     
+ Misses        178      177       -1     
Impacted Files Coverage Δ
manifold/src/face_op.cpp 93.97% <ø> (-0.08%) ⬇️
polygon/src/polygon.cpp 88.07% <84.61%> (+0.47%) ⬆️
manifold/src/edge_op.cpp 99.21% <100.00%> (+0.02%) ⬆️
manifold/src/smoothing.cpp 96.17% <100.00%> (+0.03%) ⬆️
manifold/src/sort.cpp 100.00% <100.00%> (ø)
samples/src/rounded_frame.cpp 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6247784...e63e9b3. Read the comment docs.

@elalish elalish self-assigned this Jul 17, 2022
@elalish elalish merged commit cea3d22 into master Jul 18, 2022
@elalish elalish deleted the triangulatorPrecision branch July 18, 2022 05:09
@elalish elalish mentioned this pull request Jul 20, 2022
3 tasks
cartesian-theatrics pushed a commit to SovereignShop/manifold that referenced this pull request Mar 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants