Bullet margin fix and Aabb query #399
Conversation
…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.
facebook-github-bot
added
the
CLA Signed
|
Ready for review. |
Codecov Report
@@ Coverage Diff @@
## master #399 +/- ##
==========================================
+ Coverage 56.97% 57.53% +0.56%
==========================================
Files 175 175
Lines 8153 8181 +28
Branches 84 84
==========================================
+ Hits 4645 4707 +62
+ Misses 3508 3474 -34
Continue to review full report at Codecov.
|
| @@ -177,6 +177,8 @@ bool BulletRigidObject::initializeObject( | |||
| if (joinCollisionMeshes) { | |||
| btTransform t; | |||
| t.setIdentity(); | |||
| bObjectConvexShapes_.back()->setMargin(0.0); | |||
There was a problem hiding this comment.
I haven't dug too deep in the physics code yet but why is this here and not in constructBulletCompoundFromMeshes
There was a problem hiding this comment.
constructBulletCompoundFromMeshes is a recursive function.
With the join flag, as in this case, it is building up a single convex shape from all components. This will not be complete until after the recursion is done.
Without the join flag, each level of recursion builds a new object, so it makes sense to set margin upon construction in that case.
There was a problem hiding this comment.
The constructBulletCompoundFromMeshes code might be easier to read with two functions. A top-level function and a recursive helper. It feels like functionality that should be part of constructBulletCompoundFromMeshes is part of initializeObject.
There was a problem hiding this comment.
Fair point. 👍
A refactor like this should probably be a separate PR after this bug fix is merged.
| @@ -177,6 +177,8 @@ bool BulletRigidObject::initializeObject( | |||
| if (joinCollisionMeshes) { | |||
| btTransform t; | |||
| t.setIdentity(); | |||
| bObjectConvexShapes_.back()->setMargin(0.0); | |||
There was a problem hiding this comment.
The constructBulletCompoundFromMeshes code might be easier to read with two functions. A top-level function and a recursive helper. It feels like functionality that should be part of constructBulletCompoundFromMeshes is part of initializeObject.
Cleaned up agent state checks as np.close method can cause issues and possible breaks. As well as logic changed: we tolerate that new agent's position can differ from the one requested by user as the agent should stand on the plane and can't intersect with other meshes.
* Added Aabb query to BulletPhysicsManager and BulletRigidObject. * Added C++ test for margin setting between joined and unjoined objects and the scene. * Fixed margin setting bugs. * Bug fix: make PhysicsTest.cpp build without --bullet
Motivation and Context
Bullet collision shapes have a default margin on 0.04.
We allow the user to assign a single collision margin value for each rigid body. We assign this to the root of the btCompoundShape object which holds all sub-components of the collision shape. We have not been explicitly overriding the default margins for these sub-objects. This caused a difference between joined and component hierarchy objects in the actual collision margins of the whole, as sub-component margins are accumulated additively (i.e. default margins (0.04) on sub-components would be added to the user defined margin).
This PR fixes said issue.
Sub-thesis:
PhysicsTest.cppbuild without--bulletissue. (identified by @matthewjmay ).How Has This Been Tested
This PR introduces a test to be sure that different collision shape modes (scene, joined, hierarchal) will result in the correct margin. To do so, we add functionality for querying collision Aabb from Bullet and check this against ground truth for a known test asset composed of multiple parts. This test will be extended to encompass additional collision modes such as bounding boxes when added.
Types of changes
Checklist