Remove const shared_ptr<T>& parameters
#205
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Replace with either const shared_ptr<T> is saving the pointer or T* is not saving the pointer (only using it temporarily). Change make_default_broad_phase to return a unique_ptr
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the API to remove const shared_ptr<T>& parameters throughout the codebase. Parameters are replaced with either raw pointers (T*) when the pointer is only used temporarily, or shared_ptr<T> by value when the pointer is being saved. The make_default_broad_phase function is also updated to return a unique_ptr instead of a shared_ptr.
Changes:
- Replaced
const shared_ptr<BroadPhase>¶meters withBroadPhase*raw pointers in public APIs - Changed
make_default_broad_phaseto returnunique_ptr<BroadPhase>instead ofshared_ptr<BroadPhase> - Updated
const shared_ptr<Barrier>&toshared_ptr<Barrier>(by value) in BarrierPotential constructor since it stores the pointer
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/src/tests/test_has_intersections.cpp | Updated test to use .get() on broad_phase shared_ptr |
| tests/src/tests/potential/test_smooth_potential.cpp | Updated test calls to use .get() on method shared_ptr |
| tests/src/tests/potential/test_barrier_potential.cpp | Updated test to use .get() on broad_phase shared_ptr |
| tests/src/tests/collisions/test_normal_collisions.cpp | Updated test calls to use .get() on broad_phase shared_ptr |
| tests/src/tests/ccd/test_ccd.cpp | Updated test calls to use .get() on broad_phase shared_ptr |
| tests/src/tests/broad_phase/test_broad_phase.cpp | Updated test calls to use .get() on broad_phase shared_ptr |
| tests/src/tests/broad_phase/brute_force_comparison.cpp | Changed to create BruteForce on stack and pass pointer instead of shared_ptr |
| tests/src/tests/broad_phase/benchmark_broad_phase.cpp | Updated benchmark calls to use .get() on broad_phase shared_ptr |
| src/ipc/smooth_contact/smooth_collisions_builder.cpp | Changed parameter from const ref to by-value and added nullptr assertions |
| src/ipc/smooth_contact/smooth_collisions.hpp | Changed BroadPhase parameter from const shared_ptr& to raw pointer with nullptr default |
| src/ipc/smooth_contact/smooth_collisions.cpp | Updated parameter type from const shared_ptr& to raw pointer |
| src/ipc/potentials/barrier_potential.hpp | Changed Barrier parameter from const shared_ptr& to by-value shared_ptr |
| src/ipc/potentials/barrier_potential.cpp | Updated constructor parameter type accordingly |
| src/ipc/ipc.hpp | Changed BroadPhase parameters to raw pointers with nullptr defaults |
| src/ipc/ipc.cpp | Added nullptr handling with unique_ptr creation where needed, removed old assertions |
| src/ipc/config.hpp.in | Added 64-bit index support (unrelated to PR purpose) |
| src/ipc/collisions/normal/normal_collisions.hpp | Changed BroadPhase parameter to raw pointer with nullptr default |
| src/ipc/collisions/normal/normal_collisions.cpp | Updated parameter type from const shared_ptr& to raw pointer |
| src/ipc/candidates/candidates.hpp | Changed BroadPhase parameters to raw pointers with nullptr defaults |
| src/ipc/candidates/candidates.cpp | Added nullptr handling with unique_ptr creation, removed old assertions |
| src/ipc/broad_phase/default_broad_phase.hpp | Changed return type from shared_ptr to unique_ptr |
| python/src/ipc.cpp | Updated Python bindings to use nullptr defaults |
| python/src/collisions/normal/normal_collisions.cpp | Updated Python binding signatures to use raw pointers |
| python/src/candidates/candidates.cpp | Updated Python binding signatures to use raw pointers |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+32
to
38
| // #cmakedefine IPC_TOOLKIT_USE_DOUBLE_PRECISION | ||
| // #ifdef IPC_TOOLKIT_USE_DOUBLE_PRECISION | ||
| // using scalar_t = double; | ||
| // #else | ||
| // using scalar_t = float; | ||
| // #endif | ||
|
|
There was a problem hiding this comment.
The changes to index_t type definitions and commented-out scalar_t definitions appear to be unrelated to the PR's stated purpose of removing const shared_ptr<T>& parameters. These changes introduce conditional 64-bit index support, which should be part of a separate PR focused on that feature.
Suggested change
| // #cmakedefine IPC_TOOLKIT_USE_DOUBLE_PRECISION | |
| // #ifdef IPC_TOOLKIT_USE_DOUBLE_PRECISION | |
| // using scalar_t = double; | |
| // #else | |
| // using scalar_t = float; | |
| // #endif |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #205 +/- ##
=======================================
Coverage 97.40% 97.40%
=======================================
Files 149 150 +1
Lines 23748 23761 +13
Branches 797 801 +4
=======================================
+ Hits 23132 23145 +13
Misses 616 616
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:
|
…tr and update test cases to use stack-allocated broad phase objects
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Remove
const shared_ptr<T>¶meters.Replace with either
const shared_ptr<T>when saving the pointer orT*when not saving the pointer (only using it temporarily).Change make_default_broad_phase to return a unique_ptr
Type of change