Refactor Nonlinear CCD Implementation into a Class#218
Conversation
- Introduced a new NonlinearCCD class to encapsulate nonlinear continuous collision detection (CCD) methods. - Updated point-point, point-edge, edge-edge, and point-triangle CCD methods to use the new class structure. - Modified the test cases to utilize the new NonlinearCCD class, ensuring consistency in the testing approach. - Enhanced readability by applying consistent formatting and style changes across the CCD methods. - Updated function signatures to accept Eigen::ConstRef types for better performance and clarity. - Adjusted the conservative rescaling parameter handling in the CCD methods for improved accuracy. - Removed deprecated function signatures and ensured all tests reflect the new method signatures.
0460835 to
918b54d
Compare
There was a problem hiding this comment.
Pull request overview
This PR refactors nonlinear continuous collision detection (CCD) from a set of free functions into a NonlinearCCD class, updating C++ and Python call sites/tests and adjusting several signatures to use Eigen::ConstRef for consistency with the rest of the codebase.
Changes:
- Introduced
ipc::NonlinearCCD(stateful tolerance/max-iterations/conservative-rescaling) and moved nonlinear CCD entry points onto it. - Updated C++ tests and Python tests/bindings to call
NonlinearCCDmethods instead of the removed free functions. - Updated tutorial snippet includes/signatures for interval arithmetic usage.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/src/tests/utils.hpp | Switch edge_normal args to Eigen::ConstRef. |
| tests/src/tests/test_has_intersections.cpp | Switch combine_meshes rotation args to Eigen::ConstRef. |
| tests/src/tests/ccd/test_point_edge_ccd.cpp | Switch helper arg types to Eigen::ConstRef<VectorMax3d>. |
| tests/src/tests/ccd/test_nonlinear_ccd.cpp | Update trajectory constructors to Eigen::ConstRef and migrate tests to NonlinearCCD. |
| src/ipc/ccd/nonlinear_ccd.hpp | Add NonlinearCCD class API and expose conservative piecewise-linear CCD as a static method. |
| src/ipc/ccd/nonlinear_ccd.cpp | Implement NonlinearCCD methods and refactor old free-function logic into the class. |
| python/tests/test_ccd.py | Update nonlinear CCD usage to instantiate/call ipctk.NonlinearCCD. |
| python/src/ccd/nonlinear_ccd.cpp | Replace module-level nonlinear CCD functions with NonlinearCCD Python class bindings. |
| docs/source/tutorials/nonlinear_ccd.rst | Update interval include path and example function signatures. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/ipc/ccd/nonlinear_ccd.hpp
Outdated
| bool point_triangle_ccd( | ||
| const NonlinearTrajectory& p, | ||
| const NonlinearTrajectory& t0, | ||
| const NonlinearTrajectory& t1, | ||
| const NonlinearTrajectory& t2, | ||
| double& toi, | ||
| const double min_distance = 0.0, | ||
| const double tmax = 1.0) const; |
There was a problem hiding this comment.
NonlinearCCD::point_triangle_ccd is the only CCD query in this class that is not declared virtual, unlike point_point_ccd, point_edge_ccd, and edge_edge_ccd. This asymmetry makes subclassing/overriding inconsistent and can be surprising for users extending the class. Consider making point_triangle_ccd virtual as well (or document why it is intentionally non-virtual).
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #218 +/- ##
==========================================
- Coverage 97.18% 97.17% -0.01%
==========================================
Files 160 161 +1
Lines 24734 24748 +14
Branches 890 890
==========================================
+ Hits 24037 24050 +13
- Misses 697 698 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Description
Type of change