Bullet collision from bounding box #423
Conversation
… to default.phys_scene_config.json. Modified inertia computation for bounding box objects.
…l tests to avoid code duplication.
… test confirming bug in margin setting between joined and unjoined objects.
…e to margin test. Fixed margin setting bugs.
…ollision-from-bounding-box
…entation assertion to account for jittering.
facebook-github-bot
added
the
CLA Signed
Codecov Report
@@ Coverage Diff @@
## master #423 +/- ##
=========================================
+ Coverage 56.63% 56.83% +0.2%
=========================================
Files 155 156 +1
Lines 6832 6872 +40
Branches 84 84
=========================================
+ Hits 3869 3906 +37
- Misses 2963 2966 +3
Continue to review full report at Codecov.
|
| btVector3 dim(cumulativeBB_.size() / 2.0); | ||
| bObjectShape_->removeChildShape(bGenericShapes_.back().get()); | ||
| bGenericShapes_.clear(); | ||
| bGenericShapes_.emplace_back(std::make_unique<btBoxShape>(dim)); |
There was a problem hiding this comment.
Why is bGenericShapes needed? Couldn't a local variable have been used instead?
There was a problem hiding this comment.
We need to store the collision shapes since Bullet does not replicate them internally, this is also consistent with how we handle other collision shapes.
There was a problem hiding this comment.
Why is bGenericShapes_ a vector? I can't seem to find anywhere else that adds something to it, i.e. bGenericShapes_.size() == 1 is always true.
There was a problem hiding this comment.
Next step is to enable component level bounding box hierarchies for approximate concavities. This will be used soon.
There was a problem hiding this comment.
We need to store the collision shapes since Bullet does not replicate them internally, this is also consistent with how we handle other collision shapes.
Understand. But bGenericShape is only ever added as a ChildShape to bObjectShape. From your later comment it sounds like bGenericShape is needed for future work.
|
|
||
| setInertiaVector(Magnum::Vector3(bInertia)); | ||
| } | ||
| } |
There was a problem hiding this comment.
The logic above is now duplicated. Could it be put into its own function?
There was a problem hiding this comment.
These two instances are different enough that this wouldn't give us much. Since one is before initialization of the bObjectRigidBody_ and the other is after, most of the get/set code would still need to stay for each.
| // Then set the collision shape to the cumulativeBB_ if necessary | ||
| if (objID != ID_UNDEFINED) { | ||
| BulletRigidObject* bro = | ||
| static_cast<BulletRigidObject*>(existingObjects_.at(objID)); |
There was a problem hiding this comment.
Do we ever expect to have multiple types of rigid objects in existingObjects_? (i.e. some objects are bullet objects, others are simple kinematic ones?). If so, a dynamic cast would be safer here.
There was a problem hiding this comment.
Current design is to set the MotionType of the object to specify kinematic, dynamic or static. To enable collisions between all types, they must still be BulletRigidObject type, so I think this is fine. If the user has enabled the BulletPhysicsManager then all objects in this list are BulletRigidObject. Will likely setup a different PhysicsManager type for dealing with multi-bodies.
| btVector3 dim(cumulativeBB_.size() / 2.0); | ||
| bObjectShape_->removeChildShape(bGenericShapes_.back().get()); | ||
| bGenericShapes_.clear(); | ||
| bGenericShapes_.emplace_back(std::make_unique<btBoxShape>(dim)); |
There was a problem hiding this comment.
Why is bGenericShapes_ a vector? I can't seem to find anywhere else that adds something to it, i.e. bGenericShapes_.size() == 1 is always true.
| btVector3 dim(cumulativeBB_.size() / 2.0); | ||
| bObjectShape_->removeChildShape(bGenericShapes_.back().get()); | ||
| bGenericShapes_.clear(); | ||
| bGenericShapes_.emplace_back(std::make_unique<btBoxShape>(dim)); |
There was a problem hiding this comment.
We need to store the collision shapes since Bullet does not replicate them internally, this is also consistent with how we handle other collision shapes.
Understand. But bGenericShape is only ever added as a ChildShape to bObjectShape. From your later comment it sounds like bGenericShape is needed for future work.
* Enabled bounding box collision for BulletRigidObjects * Added bounding box C++ test and test asset sphere.glb.
Motivation and Context
Primitive box shapes are much more efficient and stable than meshes for collision.
This PR enables the use of an object's accumulated mesh bounding box to serve as its collision shape. It also allows this option to be configured in an object's physics config file. This option will override other options such as choice of collision mesh and mesh joining.
TODO in future PR:
How Has This Been Tested
C++ CI test.
Local testing in viewer:
Types of changes
Checklist