Skip to content
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

bullet-featherstone: Use a single contact manifold for each convex decomposed mesh collision #664

Open
wants to merge 30 commits into
base: gz-physics7
Choose a base branch
from

Conversation

iche033
Copy link
Contributor

@iche033 iche033 commented Jul 3, 2024

🦟 Bug fix

Summary

Contact point optimization for convex decomposed mesh collisions in bullet-featherstone plugin. This PR reduces the number of contact points generated for convex decomposed mesh collision by using a single contact manifold.

Previously when a mesh collision is decomposed into multiple convex hull shape, each convex hull shape has a corresponding contact manifold which can generate up to 4 contact points (4 is the default limit in bullet). This slows down sim especially when querying contact points (via GetContactsFromLastStep). This PR overrides the btCollisionDispatcher class in order to update the contact manifolds stored in bullet. It post processes the contact points, generates a single contact manifold for each collision associated with convex hull shapes, and clears the exiting contact manifolds.

Note: it would be more efficient if we can do this inside bullet to avoid generating all the contact manifolds in the first place.

Before: many points can be generated from one convex decomposed mesh collision.
bullet_mesh_contacts

After: limit contact points to <= 4
bullet_mesh_contacts_single_manifold

Here're remotery graphs showing before and after the fix for a scene with multiple convex decomposed meshes. Note objects were auto-deactivated when these graphs were captured.

Before: >200 contact points
bullet_deactivated_profile_cropped

After: 20 contact points
bullet_deactivated_with_fix_profile_cropped

Notable speedups are:

  • PhysicsPrivate::Step: 0.153ms -> 0.097ms
  • PhysicsPrivate::UpdateCollision: 0.528ms -> 0.135ms

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

@github-actions github-actions bot added the 🎵 harmonic Gazebo Harmonic label Jul 3, 2024
@azeey
Copy link
Contributor

azeey commented Jul 10, 2024

Looks like there are several test failures in windows and I don't see any in the stable branch (https://build.osrfoundation.org/view/gz-harmonic/job/gz_physics-7-win/)

@iche033 iche033 marked this pull request as draft July 11, 2024 05:26
Signed-off-by: Ian Chen <[email protected]>
Signed-off-by: Ian Chen <[email protected]>
Signed-off-by: Ian Chen <[email protected]>
Signed-off-by: Ian Chen <[email protected]>
Signed-off-by: Ian Chen <[email protected]>
Signed-off-by: Ian Chen <[email protected]>
Signed-off-by: Ian Chen <[email protected]>
Signed-off-by: Ian Chen <[email protected]>
Signed-off-by: Ian Chen <[email protected]>
Signed-off-by: Ian Chen <[email protected]>
Signed-off-by: Ian Chen <[email protected]>
Signed-off-by: Ian Chen <[email protected]>
Signed-off-by: Ian Chen <[email protected]>
Signed-off-by: Ian Chen <[email protected]>
@iche033 iche033 marked this pull request as ready for review July 17, 2024 02:22
@iche033
Copy link
Contributor Author

iche033 commented Jul 17, 2024

Looks like there are several test failures in windows and I don't see any in the stable branch (https://build.osrfoundation.org/view/gz-harmonic/job/gz_physics-7-win/)

Fixed and this PR is ready for review again. The test failures were due to RTTI access violation when doing a dynamic_cast from btCollisionShape to btCompoundShape. I saw a similar issue reported in dartsim and fixed by adding pragmas in base classes but that's not viable here so I switched to static_cast.

@iche033 iche033 added the Bullet Bullet engine label Jul 22, 2024
@azeey azeey added the beta Targeting beta release of upcoming collection label Jul 29, 2024
@azeey azeey removed the beta Targeting beta release of upcoming collection label Sep 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bullet Bullet engine 🎵 harmonic Gazebo Harmonic
Projects
Status: In review
Development

Successfully merging this pull request may close these issues.

2 participants