Make CurvatureAngles more robust to degenerate faces#1620
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1620 +/- ##
=======================================
Coverage 95.15% 95.16%
=======================================
Files 34 34
Lines 7701 7707 +6
=======================================
+ Hits 7328 7334 +6
Misses 373 373 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| // degenerate faces | ||
|
|
||
| TEST(Properties, DegenerateCurvature) { | ||
| Manifold::Impl impl; |
There was a problem hiding this comment.
In general we test Manifold rather than Impl to stick with testing the public API surface as much as possible. It's also a good opportunity to demonstrate how to read properties from MeshGL.
There was a problem hiding this comment.
I'm a little confused on what to do. When using the Manifold constructor it automatically cleans up the degenerate faces, but I need them to test the CurvatureAngles fixes.
There was a problem hiding this comment.
Ah, fair point. I guess that's why we never saw this bug in practice. Okay, you can leave the Impl test then.
| impl.faceNormal_[0] = vec3(0.0, 0.0, 1.0); | ||
| impl.faceNormal_[1] = vec3(0.0, 0.0, 1.0); | ||
|
|
||
| impl.CalculateCurvature(-1, 0); |
There was a problem hiding this comment.
Let's test both curvatures as long as we're here.
| vertGaussianCurvature[vert] *= factor; | ||
| // Degenerate vertices can have zero area, which can cause the | ||
| // curvature to become NaN | ||
| if (vertArea[vert] > 0.0) { |
There was a problem hiding this comment.
What will the curvature be if the area is 0? It might be good to explicitly set it to zero here for clarity.
2 participants
Fixes 7 in #1474. It implements a some checks for degenerate edges and faces within Curvature Angles, and creates a test case to ensure no NANs are produced when degenerate faces are inputted.