bulletphysics / bullet3

Bullet Physics SDK: real-time collision detection and multi-physics simulation for VR, games, visual effects, robotics, machine learning etc.
http://bulletphysics.org
Other
12.61k stars 2.87k forks source link

Decouple InverseDynamics from using Bullet3Common by default #4306

Open akien-mga opened 2 years ago

akien-mga commented 2 years ago

In the Godot project, we're using Bullet2 only but for the longest time we've had to chug along Bullet3 libraries too as some Bullet2 libraries seemed to reach for Bullet3 headers.

I finally spent some time looking into it and this was fairly easy to decouple, the only coupled part is the BulletInverseDynamics module which uses a couple files from Bullet3Common:

https://github.com/bulletphysics/bullet3/blob/a1d96646c8ca28b99b2581dcfc4d74cc3b4de018/src/BulletInverseDynamics/IDErrorMessages.hpp#L8-L30

https://github.com/bulletphysics/bullet3/blob/a1d96646c8ca28b99b2581dcfc4d74cc3b4de018/src/BulletInverseDynamics/IDConfig.hpp#L72-L98

There's actually an undocumented BT_USE_INVERSE_DYNAMICS_WITH_BULLET2 define which can be set to disable using Bullet3Common headers and this seems to work fine, so I could use it to do a nice cleanup in Godot's repository: https://github.com/godotengine/godot/pull/63143

image

As this could be relevant for other users, I would suggest decouple BulletInverseDynamics from Bullet3Common by default, and only enable this code when BUILD_BULLET3 is enabled.

I.e. I'd do something like this:

https://github.com/bulletphysics/bullet3/blob/a1d96646c8ca28b99b2581dcfc4d74cc3b4de018/src/CMakeLists.txt#L7

https://github.com/bulletphysics/bullet3/blob/master/src/BulletInverseDynamics/CMakeLists.txt#L36

Or:

I'm happy to make a PR with either solution if there's interest.

erwincoumans commented 2 years ago

Are you actually using BulletInverseDynamics? You should be able to skip/ignore that library.

akien-mga commented 2 years ago

Are you actually using BulletInverseDynamics? You should be able to skip/ignore that library.

It seems like we don't indeed, I'll spend some more time reviewing what we do need and removing the components we don't.

But for our specific use case I already solved the coupling problem by defining BT_USE_INVERSE_DYNAMICS_WITH_BULLET2, I mostly opened that issue for discussion of potentially improving this upstream too if that's useful.