feat: add cotmatrix, grad, cut_mesh, face_components, rdp #41
Conversation
- cotmatrix: trimesh_cotmatrix, trimesh_cotmatrix_entries - grad: trimesh_grad (gradient operator) - meshing: trimesh_cut_mesh, trimesh_face_components - simplify: ramer_douglas_peucker (polyline simplification)
|
@jf--- could you record the changes in the log please? |
|
@tomvanmele sure see additional PR |
|
why additional PR? |
lazy, sorry... gotcha, will get used to it eventually |
There was a problem hiding this comment.
Pull request overview
This PR adds five new geometry processing functions to compas_libigl to enable the compas_slicer project to eliminate its direct dependency on raw libigl Python bindings. The additions provide COMPAS-style wrapped interfaces (mesh as tuple of vertices/faces) for:
- Cotangent matrix computations for Laplacian mesh operations
- Gradient operator computation for scalar field processing
- Mesh cutting along specified edges with vertex duplication
- Connected component detection for mesh faces
- Ramer-Douglas-Peucker polyline simplification
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/cotmatrix.hpp | C++ header defining cotangent matrix and cotmatrix entries functions |
| src/cotmatrix.cpp | C++ implementation wrapping igl::cotmatrix and igl::cotmatrix_entries |
| src/compas_libigl/cotmatrix.py | Python wrapper for cotangent matrix functions with COMPAS mesh interface |
| src/grad.hpp | C++ header defining gradient operator function |
| src/grad.cpp | C++ implementation wrapping igl::grad |
| src/compas_libigl/grad.py | Python wrapper for gradient operator with COMPAS mesh interface |
| src/meshing.hpp | Added C++ declarations for cut_mesh and face_components functions |
| src/meshing.cpp | Added C++ implementations wrapping igl::cut_mesh and igl::facet_components |
| src/compas_libigl/meshing.py | Added Python wrappers for trimesh_cut_mesh and trimesh_face_components |
| src/simplify.hpp | C++ header defining Ramer-Douglas-Peucker polyline simplification |
| src/simplify.cpp | C++ implementation wrapping igl::ramer_douglas_peucker |
| src/compas_libigl/simplify.py | Python wrapper for RDP algorithm with list-based interface |
| tests/test_cotmatrix.py | Tests verifying cotangent matrix shape and structure |
| tests/test_grad.py | Test verifying gradient operator dimensions |
| tests/test_meshing.py | Added tests for mesh cutting and component detection |
| tests/test_simplify.py | Tests for polyline simplification with various thresholds |
| CMakeLists.txt | Added nanobind module definitions for new C++ modules |
| src/compas_libigl/init.py | Added new modules to plugin list in alphabetical order |
| CHANGELOG.md | Documented new functions under Unreleased section |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| list[int] | ||
| Component ID per face. Faces sharing edges belong to the same component. | ||
| """ | ||
| V, F = M |
There was a problem hiding this comment.
Unnecessary unpacking: The variable V is extracted from M but never used. Since trimesh_face_components only requires face connectivity (not vertex positions), you should either remove the unpacking of V or only unpack F with _, F = M for clarity.
| V, F = M | |
| _, F = M |
| return C_row; | ||
| } | ||
|
|
||
| NB_MODULE(_cotmatrix, m) { |
There was a problem hiding this comment.
[nitpick] Missing module-level documentation: The _cotmatrix module should have a m.doc() statement for consistency with other modules like _simplify (line 20 of simplify.cpp) and _meshing (line 115 of meshing.cpp). Suggested addition after line 24: m.doc() = "Cotangent matrix computation functions using libigl";
| NB_MODULE(_cotmatrix, m) { | |
| NB_MODULE(_cotmatrix, m) { | |
| m.doc() = "Cotangent matrix computation functions using libigl"; |
| return G; | ||
| } | ||
|
|
||
| NB_MODULE(_grad, m) { |
There was a problem hiding this comment.
[nitpick] Missing module-level documentation: The _grad module should have a m.doc() statement for consistency with other modules like _simplify (line 20 of simplify.cpp) and _meshing (line 115 of meshing.cpp). Suggested addition after line 13: m.doc() = "Gradient operator computation functions using libigl";
| NB_MODULE(_grad, m) { | |
| NB_MODULE(_grad, m) { | |
| m.doc() = "Gradient operator computation functions using libigl"; |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
thx @tomvanmele for accepting this PR. a bit of context: I've been working whipping |
|
all good. my only suggestion is that we then try to scope our bindings similarly. where a possible stable solution from CGAL exists, we use that. otherwise we use an "experimental" function from IGL, if the functionality is available there... but that we try not to have the duplication is it may lead to misunderstandings, and it makes the plugin system (the way it works now) a bit less predictable. |
|
I share the CGAL preference. The plug-in concern should be manageable. What would it take for compas to have compas-cgal as a full on dependency rather than loading via the plug-in system? Rhino 8 isn't a showstopper? |
|
would be very cool indeed but @gonzalocasas rightfully pointed out that our licenses are not compatible... |
|
there are some things we can do to make the entire mechanism more transparent and a lot faster though |
2 participants
Summary
Add missing igl functions to enable compas_slicer to drop the
libigldependency entirely:trimesh_cotmatrix,trimesh_cotmatrix_entriestrimesh_grad(gradient operator)trimesh_cut_mesh,trimesh_face_componentsramer_douglas_peucker(polyline simplification)Motivation
Part of ongoing effort to modernize compas_slicer. Currently compas_slicer depends on both
compas_libigland rawlibiglpython bindings. These additions allow compas_slicer (and other packages) to use onlycompas_libigl, providing: