Add cotangent-based Laplace-Beltrami and mean curvature support to fv_surface_mesh#12
Add cotangent-based Laplace-Beltrami and mean curvature support to fv_surface_mesh#12
fv_surface_mesh#12Conversation
Co-authored-by: hdclark <934858+hdclark@users.noreply.github.com>
Co-authored-by: hdclark <934858+hdclark@users.noreply.github.com>
Co-authored-by: hdclark <934858+hdclark@users.noreply.github.com>
fv_surface_mesh
There was a problem hiding this comment.
Pull request overview
This PR adds differential geometry operators to the fv_surface_mesh class, implementing cotangent-weight based Laplace-Beltrami operator and mean curvature computation. The implementation follows established codebase patterns for error handling, template specialization, and testing.
Changes:
- Added three new methods to
fv_surface_mesh:cotangent_weights,laplace_beltrami_operator, andmean_curvature - Implemented cotangent weight computation with validation for degenerate triangles and manifold constraints
- Added comprehensive unit tests for the new functionality using a regular tetrahedral mesh
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| src/YgorMath.h | Declares three new public methods for differential geometry operations |
| src/YgorMath.cc | Implements cotangent weights, Laplace-Beltrami operator with manifold/degeneracy checks, and mean curvature magnitude computation with proper template specializations |
| tests2/YgorMath/fv_surface_mesh.cc | Adds three test subcases validating cotangent weights, Laplace-Beltrami operator, mean curvature, and error handling for non-triangular meshes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| std::map<std::pair<I,I>, std::vector<I>> edge_to_opposing_vertices; | ||
| for(const auto &fv : this->faces){ | ||
| if(fv.size() < 3) continue; | ||
| if(fv.size() != 3){ |
There was a problem hiding this comment.
The logic at line 6330 skips faces with fewer than 3 vertices with 'continue', but then line 6331 throws an error if the face doesn't have exactly 3 vertices. This means faces with exactly 3 vertices are processed, faces with fewer than 3 are skipped silently, but faces with more than 3 vertices throw an error. However, the skip at line 6330 is redundant since faces with size < 3 would fail the size != 3 check at line 6331 anyway. Consider removing line 6330 for clarity, or ensuring faces with < 3 vertices should genuinely be skipped rather than trigger an error.
| if(fv.size() != 3){ | |
| if(fv.size() > 3){ |
There was a problem hiding this comment.
Remove line 6330 as only faces with 3 vertices should be accepted.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This PR extends
fv_surface_meshinsrc/YgorMath.{h,cc}with core differential-geometry operators requested in the issue: efficient cotangent-weight helpers, per-vertex Laplace-Beltrami application, and per-vertex mean curvature derived from that operator. It also adds focused unit coverage to validate correctness and enforce algorithm preconditions.API additions (
fv_surface_mesh)cotangent_weights(I v1, I v2, I v3, I v4) constlaplace_beltrami_operator() constmean_curvature() constCotangent helper implementation
Laplace-Beltrami operator
Mean curvature
Targeted test coverage
tests2/YgorMath/fv_surface_mesh.ccsubcases for:Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.