-
Notifications
You must be signed in to change notification settings - Fork 39
Remove SimpleBVH dependency and references #213
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
Conversation
Drop the SimpleBVH third-party integration and all references to it. - Removed cmake/recipes/simple_bvh.cmake and stopped including/linking simple_bvh in the top-level CMakeLists.txt - Deleted BVH source files under python/src/broad_phase and src/ipc/broad_phase - Updated dependency diagrams and docs (dependencies.dot/.svg and related rst) - Adjusted bindings/CMakeLists/tests to reflect the removal. This simplifies the build by eliminating the SimpleBVH dependency and cleaning up related artifacts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Removes the SimpleBVH third-party integration and the associated BVH broad-phase implementation from the C++ core and Python bindings, updating tests and documentation accordingly to simplify the build/dependency surface.
Changes:
- Removed the C++
BVHbroad-phase implementation (SimpleBVH-backed) and its factory/enum wiring. - Removed Python bindings/exposed API for
ipctk.BVHand updated examples/notebooks/tests to use other broad phases (notablyLBVH). - Updated dependency documentation/diagrams to drop SimpleBVH from the published dependency graph and docs.
Reviewed changes
Copilot reviewed 22 out of 23 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| CMakeLists.txt | Stops including/linking the SimpleBVH third-party target. |
| cmake/recipes/simple_bvh.cmake | Removes the CPM recipe for SimpleBVH. |
| src/ipc/broad_phase/bvh.cpp | Deletes the SimpleBVH-backed BVH implementation. |
| src/ipc/broad_phase/bvh.hpp | Deletes the BVH class declaration (previously deprecated). |
| src/ipc/broad_phase/CMakeLists.txt | Removes BVH sources from the broad-phase build. |
| src/ipc/broad_phase/create_broad_phase.cpp | Removes BVH include and factory case. |
| src/ipc/broad_phase/create_broad_phase.hpp | Removes BroadPhaseMethod::BVH enum entry. |
| tests/src/tests/utils.cpp | Drops BVH from the list of broad phases exercised by tests. |
| tests/src/tests/broad_phase/test_stq.cpp | Removes unused BVH include. |
| tests/src/tests/broad_phase/test_lbvh.cpp | Replaces BVH baseline comparisons with SpatialHash baseline in LBVH candidate tests. |
| python/src/bindings.cpp | Removes define_bvh() registration. |
| python/src/broad_phase/bindings.hpp | Removes define_bvh() declaration. |
| python/src/broad_phase/bvh.cpp | Deletes BVH Python binding implementation. |
| python/src/broad_phase/CMakeLists.txt | Removes BVH binding source from Python extension build. |
| python/tests/utils.py | Updates Python test broad-phase enumeration (drops ipctk.BVH, uses ipctk.LBVH). |
| python/examples/profiler.py | Removes runtime selection of BVH vs LBVH; uses LBVH only. |
| notebooks/ogc.py | Replaces ipctk.BVH() usage with ipctk.LBVH() and reformats. |
| docs/source/tutorials/getting_started.rst | Removes BVH/SimpleBVH from the documented broad_phase options list. |
| docs/source/python-api/broad_phase.rst | Removes ipctk.BVH API section. |
| docs/source/cpp-api/broad_phase.rst | Removes ipc::BVH API section. |
| docs/source/about/dependencies.rst | Removes SimpleBVH from the dependency listing table. |
| docs/source/_static/graphviz/dependencies.dot | Removes SimpleBVH node/edges from the dependency graph source. |
| docs/source/_static/graphviz/dependencies.svg | Updates the rendered dependency graph to match the new DOT. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #213 +/- ##
==========================================
- Coverage 96.96% 96.95% -0.01%
==========================================
Files 161 159 -2
Lines 24747 24666 -81
Branches 893 882 -11
==========================================
- Hits 23997 23916 -81
Misses 750 750
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:
|
Description
Drop the SimpleBVH third-party integration and all references to it.
This simplifies the build by eliminating the SimpleBVH dependency and cleaning up related artifacts.
Type of change