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

[3.x] bullet: Sync with upstream 3.17 #48300

Merged
merged 1 commit into from
Sep 29, 2021

Conversation

akien-mga
Copy link
Member

@akien-mga akien-mga commented Apr 29, 2021

Backport of #48299.

Fixes #43868.

Increase minimum version for distro packages to 2.90 (this was never released
as the "next" version after 2.89 was 3.05... but that covers it too).


This might change Bullet behavior, so I wouldn't cherry-pick for 3.3 in any case, but this might be a good update for 3.4+? WDYT @pouleyKetchoupp?

Note that the major version change from 2.89 to 3.05 (and now 3.09) might seem a bit risky... but from what I've seen over the years I can't derive any kind of logic behind Bullet version numbers so we need to assess the API itself I guess, not trust the numbers.

@akien-mga akien-mga added this to the 3.4 milestone Apr 29, 2021
@akien-mga akien-mga requested review from a team as code owners April 29, 2021 13:58
@akien-mga akien-mga changed the title bullet: Sync with upstream 3.09 [3.x] bullet: Sync with upstream 3.09 Apr 29, 2021
@akien-mga
Copy link
Member Author

akien-mga commented Apr 29, 2021

Sounds like I'll have to finally do that non-trivial backport of #44457 which I've been postponing, as the bullet upgrade generates a broken binary when building incrementally from a previous version (it doesn't properly rebuild everything it seems).

Edit: Done.

@akien-mga
Copy link
Member Author

Interesting backtrace on the regression test project:

[1] bin/godot.x11.tools.64s() [0x172867a] (/home/runner/work/godot/godot/platform/x11/crash_handler_x11.cpp:54)
[2] /lib/x86_64-linux-gnu/libc.so.6(+0x46210) [0x7feccc486210] (??:0)
[3] btHashedOverlappingPairCache::processAllOverlappingPairs(btOverlapCallback*, btDispatcher*, btDispatcherInfo const&) (/home/runner/work/godot/godot/thirdparty/bullet/BulletCollision/BroadphaseCollision/btOverlappingPairCache.cpp:374)
[4] btDbvtBroadphase::performDeferredRemoval(btDispatcher*) (/home/runner/work/godot/godot/thirdparty/bullet/BulletCollision/BroadphaseCollision/btDbvtBroadphase.cpp:445)
[5] btDbvtBroadphase::calculateOverlappingPairs(btDispatcher*) (/home/runner/work/godot/godot/thirdparty/bullet/BulletCollision/BroadphaseCollision/btDbvtBroadphase.cpp:441)
[6] btCollisionWorld::computeOverlappingPairs() (/home/runner/work/godot/godot/thirdparty/bullet/BulletCollision/CollisionDispatch/btCollisionWorld.cpp:217)
[7] btCollisionWorld::performDiscreteCollisionDetection() (/home/runner/work/godot/godot/thirdparty/bullet/BulletCollision/CollisionDispatch/btCollisionWorld.cpp:231)
[8] btDiscreteDynamicsWorld::internalSingleStepSimulation(float) (/home/runner/work/godot/godot/thirdparty/bullet/BulletDynamics/Dynamics/btDiscreteDynamicsWorld.cpp:475)
[9] btSoftRigidDynamicsWorld::internalSingleStepSimulation(float) (/home/runner/work/godot/godot/thirdparty/bullet/BulletSoftBody/btSoftRigidDynamicsWorld.cpp:91)
[10] btDiscreteDynamicsWorld::stepSimulation(float, int, float) (/home/runner/work/godot/godot/thirdparty/bullet/BulletDynamics/Dynamics/btDiscreteDynamicsWorld.cpp:435 (discriminator 2))
[11] SpaceBullet::step(float) (/home/runner/work/godot/godot/modules/bullet/space_bullet.cpp:384)
[12] BulletPhysicsServer::step(float) (/home/runner/work/godot/godot/modules/bullet/bullet_physics_server.cpp:1549 (discriminator 2))
[13] Main::iteration() (/home/runner/work/godot/godot/main/main.cpp:2113)
[14] OS_X11::run() (/home/runner/work/godot/godot/platform/x11/os_x11.cpp:3641)
[15] bin/godot.x11.tools.64s(main+0x329) [0x171f06f] (/home/runner/work/godot/godot/platform/x11/godot_x11.cpp:57)
[16] /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf3) [0x7feccc4670b3] (??:0)
[17] bin/godot.x11.tools.64s(_start+0x2e) [0x171ec8e] (??:?)
-- END OF BACKTRACE --

Will have to test further and see if it's a bug in bullet or in our code.

@akien-mga akien-mga marked this pull request as draft April 29, 2021 15:38
@pouleyKetchoupp
Copy link
Contributor

pouleyKetchoupp commented Apr 29, 2021

This might change Bullet behavior, so I wouldn't cherry-pick for 3.3 in any case, but this might be a good update for 3.4+? WDYT @pouleyKetchoupp?

Yeah it can be worth a try at least. Once the crash you spotted is fixed and more regression tests are done we'll need to make sure it's well tested by the community, so an update for 3.4+ seems like the way to go.

I haven't checked everything in details because it's too many files, but it seems there are interesting changes in soft bodies and in the multi-threaded collision dispatcher (it might allow us to revive #40073 at some point).

@akien-mga akien-mga changed the title [3.x] bullet: Sync with upstream 3.09 [3.x] bullet: Sync with upstream 3.17 May 24, 2021
@akien-mga
Copy link
Member Author

I could reproduce the crash from #48300 (comment) locally when building the updated bullet module on top of a pre-existing 3.x build.

So despite my backport of buildsystem improvements in #48301, there's still this issue with bullet where it doesn't properly rebuild everything that needs to be rebuilt for the new code to be compatible with old objects.

Not sure what to do, it's been like this since the beginning. Merging this would likely result in a few bug reports from users who see their 3.x build crashing until they do a clean build (like for every previous Bullet update).

Might be too late for 3.4 anyway? We could merge when starting development for 3.5 maybe.

@akien-mga
Copy link
Member Author

So despite my backport of buildsystem improvements in #48301, there's still this issue with bullet where it doesn't properly rebuild everything that needs to be rebuilt for the new code to be compatible with old objects.

I investigated a bit, and indeed SCons doesn't properly rebuild all files of the thirdparty bullet code which are (supposedly) affected by the update. I guess it properly rebuilds the modified .cpp files, but not necessarily the ones which are impacted by modified .h files.

I wonder if it's because we treat them as system headers to avoid raising warnings in Bullet headers?

godot/modules/bullet/SCsub

Lines 203 to 205 in 3db672c

# Treat Bullet headers as system headers to avoid raising warnings. Not supported on MSVC.
if not env.msvc:
env_bullet.Append(CPPFLAGS=["-isystem", Dir(thirdparty_dir).path])

This is the diff between the files which are compiled when building this PR on top of a full build of its parent commit (broken binary), and the same PR after having cleaned all build objects in thirdparty/bullet:

--- tatas       2021-09-29 14:40:05.415131101 +0200
+++ titis       2021-09-29 14:40:10.357138653 +0200
@@ -21,63 +21,125 @@
 Compiling ==> modules/bullet/soft_body_bullet.cpp
 Compiling ==> modules/bullet/space_bullet.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/BroadphaseCollision/btAxisSweep3.cpp
+Compiling ==> thirdparty/bullet/BulletCollision/BroadphaseCollision/btBroadphaseProxy.cpp
+Compiling ==> thirdparty/bullet/BulletCollision/BroadphaseCollision/btCollisionAlgorithm.cpp
+Compiling ==> thirdparty/bullet/BulletCollision/BroadphaseCollision/btDbvtBroadphase.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/BroadphaseCollision/btDbvt.cpp
+Compiling ==> thirdparty/bullet/BulletCollision/BroadphaseCollision/btDispatcher.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/BroadphaseCollision/btOverlappingPairCache.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/BroadphaseCollision/btQuantizedBvh.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/BroadphaseCollision/btSimpleBroadphase.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/CollisionDispatch/btActivatingCollisionAlgorithm.cpp
+Compiling ==> thirdparty/bullet/BulletCollision/CollisionDispatch/btBox2dBox2dCollisionAlgorithm.cpp
+Compiling ==> thirdparty/bullet/BulletCollision/CollisionDispatch/btBoxBoxCollisionAlgorithm.cpp
+Compiling ==> thirdparty/bullet/BulletCollision/CollisionDispatch/btBoxBoxDetector.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/CollisionDispatch/btCollisionDispatcher.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/CollisionDispatch/btCollisionDispatcherMt.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/CollisionDispatch/btCollisionObject.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/CollisionDispatch/btCollisionWorld.cpp
+Compiling ==> thirdparty/bullet/BulletCollision/CollisionDispatch/btCollisionWorldImporter.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/CollisionDispatch/btCompoundCollisionAlgorithm.cpp
+Compiling ==> thirdparty/bullet/BulletCollision/CollisionDispatch/btCompoundCompoundCollisionAlgorithm.cpp
+Compiling ==> thirdparty/bullet/BulletCollision/CollisionDispatch/btConvex2dConvex2dAlgorithm.cpp
+Compiling ==> thirdparty/bullet/BulletCollision/CollisionDispatch/btConvexConcaveCollisionAlgorithm.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/CollisionDispatch/btConvexConvexAlgorithm.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/CollisionDispatch/btConvexPlaneCollisionAlgorithm.cpp
+Compiling ==> thirdparty/bullet/BulletCollision/CollisionDispatch/btDefaultCollisionConfiguration.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/CollisionDispatch/btEmptyCollisionAlgorithm.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/CollisionDispatch/btGhostObject.cpp
+Compiling ==> thirdparty/bullet/BulletCollision/CollisionDispatch/btHashedSimplePairCache.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/CollisionDispatch/btInternalEdgeUtility.cpp
+Compiling ==> thirdparty/bullet/BulletCollision/CollisionDispatch/btManifoldResult.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/CollisionDispatch/btSimulationIslandManager.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/CollisionDispatch/btSphereBoxCollisionAlgorithm.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/CollisionDispatch/btSphereSphereCollisionAlgorithm.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/CollisionDispatch/btSphereTriangleCollisionAlgorithm.cpp
+Compiling ==> thirdparty/bullet/BulletCollision/CollisionDispatch/btUnionFind.cpp
+Compiling ==> thirdparty/bullet/BulletCollision/CollisionDispatch/SphereTriangleDetector.cpp
+Compiling ==> thirdparty/bullet/BulletCollision/CollisionShapes/btBox2dShape.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/CollisionShapes/btBoxShape.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/CollisionShapes/btBvhTriangleMeshShape.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/CollisionShapes/btCapsuleShape.cpp
+Compiling ==> thirdparty/bullet/BulletCollision/CollisionShapes/btCollisionShape.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/CollisionShapes/btCompoundShape.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/CollisionShapes/btConcaveShape.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/CollisionShapes/btConeShape.cpp
+Compiling ==> thirdparty/bullet/BulletCollision/CollisionShapes/btConvex2dShape.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/CollisionShapes/btConvexHullShape.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/CollisionShapes/btConvexInternalShape.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/CollisionShapes/btConvexPointCloudShape.cpp
+Compiling ==> thirdparty/bullet/BulletCollision/CollisionShapes/btConvexPolyhedron.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/CollisionShapes/btConvexShape.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/CollisionShapes/btConvexTriangleMeshShape.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/CollisionShapes/btCylinderShape.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/CollisionShapes/btEmptyShape.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/CollisionShapes/btHeightfieldTerrainShape.cpp
+Compiling ==> thirdparty/bullet/BulletCollision/CollisionShapes/btMiniSDF.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/CollisionShapes/btMinkowskiSumShape.cpp
+Compiling ==> thirdparty/bullet/BulletCollision/CollisionShapes/btMultimaterialTriangleMeshShape.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/CollisionShapes/btMultiSphereShape.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/CollisionShapes/btOptimizedBvh.cpp
+Compiling ==> thirdparty/bullet/BulletCollision/CollisionShapes/btPolyhedralConvexShape.cpp
+Compiling ==> thirdparty/bullet/BulletCollision/CollisionShapes/btScaledBvhTriangleMeshShape.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/CollisionShapes/btSdfCollisionShape.cpp
+Compiling ==> thirdparty/bullet/BulletCollision/CollisionShapes/btShapeHull.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/CollisionShapes/btSphereShape.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/CollisionShapes/btStaticPlaneShape.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/CollisionShapes/btStridingMeshInterface.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/CollisionShapes/btTetrahedronShape.cpp
+Compiling ==> thirdparty/bullet/BulletCollision/CollisionShapes/btTriangleBuffer.cpp
+Compiling ==> thirdparty/bullet/BulletCollision/CollisionShapes/btTriangleCallback.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/CollisionShapes/btTriangleIndexVertexArray.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/CollisionShapes/btTriangleIndexVertexMaterialArray.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/CollisionShapes/btTriangleMesh.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/CollisionShapes/btTriangleMeshShape.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/CollisionShapes/btUniformScalingShape.cpp
+Compiling ==> thirdparty/bullet/BulletCollision/Gimpact/btContactProcessing.cpp
+Compiling ==> thirdparty/bullet/BulletCollision/Gimpact/btGenericPoolAllocator.cpp
+Compiling ==> thirdparty/bullet/BulletCollision/Gimpact/btGImpactBvh.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/Gimpact/btGImpactCollisionAlgorithm.cpp
+Compiling ==> thirdparty/bullet/BulletCollision/Gimpact/btGImpactQuantizedBvh.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/Gimpact/btGImpactShape.cpp
+Compiling ==> thirdparty/bullet/BulletCollision/Gimpact/btTriangleShapeEx.cpp
+Compiling ==> thirdparty/bullet/BulletCollision/Gimpact/gim_box_set.cpp
+Compiling ==> thirdparty/bullet/BulletCollision/Gimpact/gim_contact.cpp
+Compiling ==> thirdparty/bullet/BulletCollision/Gimpact/gim_memory.cpp
+Compiling ==> thirdparty/bullet/BulletCollision/Gimpact/gim_tri_collision.cpp
+Compiling ==> thirdparty/bullet/BulletCollision/NarrowPhaseCollision/btContinuousConvexCollision.cpp
+Compiling ==> thirdparty/bullet/BulletCollision/NarrowPhaseCollision/btConvexCast.cpp
+Compiling ==> thirdparty/bullet/BulletCollision/NarrowPhaseCollision/btGjkConvexCast.cpp
 Compiling ==> thirdparty/bullet/BulletCollision/NarrowPhaseCollision/btGjkEpa2.cpp
+Compiling ==> thirdparty/bullet/BulletCollision/NarrowPhaseCollision/btGjkEpaPenetrationDepthSolver.cpp
+Compiling ==> thirdparty/bullet/BulletCollision/NarrowPhaseCollision/btGjkPairDetector.cpp
+Compiling ==> thirdparty/bullet/BulletCollision/NarrowPhaseCollision/btMinkowskiPenetrationDepthSolver.cpp
+Compiling ==> thirdparty/bullet/BulletCollision/NarrowPhaseCollision/btPersistentManifold.cpp
+Compiling ==> thirdparty/bullet/BulletCollision/NarrowPhaseCollision/btPolyhedralContactClipping.cpp
+Compiling ==> thirdparty/bullet/BulletCollision/NarrowPhaseCollision/btRaycastCallback.cpp
+Compiling ==> thirdparty/bullet/BulletCollision/NarrowPhaseCollision/btSubSimplexConvexCast.cpp
+Compiling ==> thirdparty/bullet/BulletCollision/NarrowPhaseCollision/btVoronoiSimplexSolver.cpp
+Compiling ==> thirdparty/bullet/BulletDynamics/Character/btKinematicCharacterController.cpp
 Compiling ==> thirdparty/bullet/BulletDynamics/ConstraintSolver/btBatchedConstraints.cpp
+Compiling ==> thirdparty/bullet/BulletDynamics/ConstraintSolver/btConeTwistConstraint.cpp
 Compiling ==> thirdparty/bullet/BulletDynamics/ConstraintSolver/btContactConstraint.cpp
 Compiling ==> thirdparty/bullet/BulletDynamics/ConstraintSolver/btFixedConstraint.cpp
+Compiling ==> thirdparty/bullet/BulletDynamics/ConstraintSolver/btGearConstraint.cpp
+Compiling ==> thirdparty/bullet/BulletDynamics/ConstraintSolver/btGeneric6DofConstraint.cpp
 Compiling ==> thirdparty/bullet/BulletDynamics/ConstraintSolver/btGeneric6DofSpring2Constraint.cpp
+Compiling ==> thirdparty/bullet/BulletDynamics/ConstraintSolver/btGeneric6DofSpringConstraint.cpp
 Compiling ==> thirdparty/bullet/BulletDynamics/ConstraintSolver/btHinge2Constraint.cpp
+Compiling ==> thirdparty/bullet/BulletDynamics/ConstraintSolver/btHingeConstraint.cpp
+Compiling ==> thirdparty/bullet/BulletDynamics/ConstraintSolver/btNNCGConstraintSolver.cpp
+Compiling ==> thirdparty/bullet/BulletDynamics/ConstraintSolver/btPoint2PointConstraint.cpp
 Compiling ==> thirdparty/bullet/BulletDynamics/ConstraintSolver/btSequentialImpulseConstraintSolver.cpp
+Compiling ==> thirdparty/bullet/BulletDynamics/ConstraintSolver/btSequentialImpulseConstraintSolverMt.cpp
+Compiling ==> thirdparty/bullet/BulletDynamics/ConstraintSolver/btSliderConstraint.cpp
+Compiling ==> thirdparty/bullet/BulletDynamics/ConstraintSolver/btSolve2LinearConstraint.cpp
+Compiling ==> thirdparty/bullet/BulletDynamics/ConstraintSolver/btTypedConstraint.cpp
+Compiling ==> thirdparty/bullet/BulletDynamics/ConstraintSolver/btUniversalConstraint.cpp
 Compiling ==> thirdparty/bullet/BulletDynamics/Dynamics/btDiscreteDynamicsWorld.cpp
+Compiling ==> thirdparty/bullet/BulletDynamics/Dynamics/btDiscreteDynamicsWorldMt.cpp
 Compiling ==> thirdparty/bullet/BulletDynamics/Dynamics/btRigidBody.cpp
+Compiling ==> thirdparty/bullet/BulletDynamics/Dynamics/btSimpleDynamicsWorld.cpp
 Compiling ==> thirdparty/bullet/BulletDynamics/Dynamics/btSimulationIslandManagerMt.cpp
 Compiling ==> thirdparty/bullet/BulletDynamics/Featherstone/btMultiBodyConstraint.cpp
 Compiling ==> thirdparty/bullet/BulletDynamics/Featherstone/btMultiBodyConstraintSolver.cpp
@@ -87,27 +149,36 @@
 Compiling ==> thirdparty/bullet/BulletDynamics/Featherstone/btMultiBodyGearConstraint.cpp
 Compiling ==> thirdparty/bullet/BulletDynamics/Featherstone/btMultiBodyJointLimitConstraint.cpp
 Compiling ==> thirdparty/bullet/BulletDynamics/Featherstone/btMultiBodyJointMotor.cpp
+Compiling ==> thirdparty/bullet/BulletDynamics/Featherstone/btMultiBodyMLCPConstraintSolver.cpp
 Compiling ==> thirdparty/bullet/BulletDynamics/Featherstone/btMultiBodyPoint2Point.cpp
 Compiling ==> thirdparty/bullet/BulletDynamics/Featherstone/btMultiBodySliderConstraint.cpp
 Compiling ==> thirdparty/bullet/BulletDynamics/Featherstone/btMultiBodySphericalJointMotor.cpp
+Compiling ==> thirdparty/bullet/BulletDynamics/MLCPSolvers/btDantzigLCP.cpp
+Compiling ==> thirdparty/bullet/BulletDynamics/MLCPSolvers/btLemkeAlgorithm.cpp
 Compiling ==> thirdparty/bullet/BulletDynamics/MLCPSolvers/btMLCPSolver.cpp
+Compiling ==> thirdparty/bullet/BulletDynamics/Vehicle/btRaycastVehicle.cpp
+Compiling ==> thirdparty/bullet/BulletDynamics/Vehicle/btWheelInfo.cpp
 Compiling ==> thirdparty/bullet/BulletInverseDynamics/details/MultiBodyTreeImpl.cpp
 Compiling ==> thirdparty/bullet/BulletInverseDynamics/details/MultiBodyTreeInitCache.cpp
 Compiling ==> thirdparty/bullet/BulletInverseDynamics/IDMath.cpp
 Compiling ==> thirdparty/bullet/BulletInverseDynamics/MultiBodyTree.cpp
+Compiling ==> thirdparty/bullet/BulletSoftBody/btDefaultSoftBodySolver.cpp
 Compiling ==> thirdparty/bullet/BulletSoftBody/btDeformableBackwardEulerObjective.cpp
 Compiling ==> thirdparty/bullet/BulletSoftBody/btDeformableBodySolver.cpp
 Compiling ==> thirdparty/bullet/BulletSoftBody/btDeformableContactConstraint.cpp
 Compiling ==> thirdparty/bullet/BulletSoftBody/btDeformableContactProjection.cpp
 Compiling ==> thirdparty/bullet/BulletSoftBody/btDeformableMultiBodyConstraintSolver.cpp
 Compiling ==> thirdparty/bullet/BulletSoftBody/btDeformableMultiBodyDynamicsWorld.cpp
+Compiling ==> thirdparty/bullet/BulletSoftBody/btSoftBodyConcaveCollisionAlgorithm.cpp
 Compiling ==> thirdparty/bullet/BulletSoftBody/btSoftBody.cpp
 Compiling ==> thirdparty/bullet/BulletSoftBody/btSoftBodyHelpers.cpp
+Compiling ==> thirdparty/bullet/BulletSoftBody/btSoftBodyRigidBodyCollisionConfiguration.cpp
 Compiling ==> thirdparty/bullet/BulletSoftBody/btSoftMultiBodyDynamicsWorld.cpp
 Compiling ==> thirdparty/bullet/BulletSoftBody/btSoftRigidCollisionAlgorithm.cpp
 Compiling ==> thirdparty/bullet/BulletSoftBody/btSoftRigidDynamicsWorld.cpp
 Compiling ==> thirdparty/bullet/BulletSoftBody/btSoftSoftCollisionAlgorithm.cpp
 Compiling ==> thirdparty/bullet/BulletSoftBody/poly34.cpp
+Compiling ==> thirdparty/bullet/clew/clew.c
 Compiling ==> thirdparty/bullet/LinearMath/btAlignedAllocator.cpp
 Compiling ==> thirdparty/bullet/LinearMath/btConvexHullComputer.cpp
 Compiling ==> thirdparty/bullet/LinearMath/btConvexHull.cpp
@@ -115,5 +186,10 @@
 Compiling ==> thirdparty/bullet/LinearMath/btPolarDecomposition.cpp
 Compiling ==> thirdparty/bullet/LinearMath/btQuickprof.cpp
 Compiling ==> thirdparty/bullet/LinearMath/btReducedVector.cpp
+Compiling ==> thirdparty/bullet/LinearMath/btSerializer64.cpp
+Compiling ==> thirdparty/bullet/LinearMath/btSerializer.cpp
 Compiling ==> thirdparty/bullet/LinearMath/btThreads.cpp
 Compiling ==> thirdparty/bullet/LinearMath/btVector3.cpp
+Compiling ==> thirdparty/bullet/LinearMath/TaskScheduler/btTaskScheduler.cpp
+Compiling ==> thirdparty/bullet/LinearMath/TaskScheduler/btThreadSupportPosix.cpp
+Compiling ==> thirdparty/bullet/LinearMath/TaskScheduler/btThreadSupportWin32.cpp

CC @dmoody256 if you have any idea about this.

@akien-mga
Copy link
Member Author

akien-mga commented Sep 29, 2021

I wonder if it's because we treat them as system headers to avoid raising warnings in Bullet headers?

Seems to be it, this solves it:

diff --git a/modules/bullet/SCsub b/modules/bullet/SCsub
index 12639f6d34..00ab7730ea 100644
--- a/modules/bullet/SCsub
+++ b/modules/bullet/SCsub
@@ -199,11 +199,7 @@ if env["builtin_bullet"]:
 
     thirdparty_sources = [thirdparty_dir + file for file in bullet2_src]
 
-    # Treat Bullet headers as system headers to avoid raising warnings. Not supported on MSVC.
-    if not env.msvc:
-        env_bullet.Append(CPPFLAGS=["-isystem", Dir(thirdparty_dir).path])
-    else:
-        env_bullet.Prepend(CPPPATH=[thirdparty_dir])
+    env_bullet.Prepend(CPPPATH=[thirdparty_dir])
     # if env['target'] == "debug" or env['target'] == "release_debug":
     #     env_bullet.Append(CPPDEFINES=['BT_DEBUG'])

Of course it makes Bullet headers spew warnings which are not our responsibility to fix (we already disable warnings when building Bullet itself, but those warnings get raised when building Godot's own code that includes Bullet headers).

Sounds like there's no clean way in C++ to keep a proper separation of concerns here... I'll guess I'll bite the bullet (heh) and fix those warnings.

@akien-mga akien-mga force-pushed the 3.x-bullet-3.09 branch 4 times, most recently from a96c395 to d6bb397 Compare September 29, 2021 14:04
Stop include Bullet headers using `-isystem` for GCC/Clang as it misleads
SCons into not properly rebuilding all files when headers change.

This means we also need to make sure Bullet builds without warning, and
current version fares fairly well, there were just a couple to fix (patch
included).

Increase minimum version for distro packages to 2.90 (this was never released
as the "next" version after 2.89 was 3.05... but that covers it too).
@akien-mga akien-mga marked this pull request as ready for review September 29, 2021 14:39
@akien-mga
Copy link
Member Author

akien-mga commented Sep 29, 2021

Alright, I fixed the buildsystem as discussed above by no longer treating Bullet headers as system headers. This means that I had to fix warnings in those headers, but thankfully there were only a couple (it used to be much worse but I guess others have been cleaning up warning upstream).

I PR'ed the warning fixes upstream: bulletphysics/bullet3#3980 bulletphysics/bullet3#3981

So now this builds fine incrementally and runs fine.

@pouleyKetchoupp Should we take the chance and merge now for future 3.4 beta builds, or do you think we'd need more time to assess whether the upgrade is indeed safe?

@dmoody256
Copy link
Contributor

dmoody256 commented Sep 29, 2021

Regarding making the scanner work and having a custom include prefix, the easiest thing to do would be to overwrite the bullet_env['_CPPINCFLAGS'] var to your own custom concat function which can look at the specific include paths can add the isystem conditionally instead of the standard include prefix.

Normally the _CPPINCFLAGS is defined as:

'${_concat(INCPREFIX, CPPPATH, INCSUFFIX, __env__, RDirs, TARGET, SOURCE, affect_signature=False)}'

You can overwrite it with a custom _concat function which does whatever you want. The implementation would look something like this:

def custom_concat(prefix, items_iter, suffix, env, **kwargs):
    result = []
    for item in items_iter:
        if 'thirdparty/bullet' in env.subst(item, **kwargs) and not env.msvc:
            result += [env['_concat']('-isystem', [item], suffix, env, **kwargs)]
        else:
            result += [env['_concat'](prefix, [item], suffix, env, **kwargs)]
    return result

env_bullet['custom_concat'] = custom_concat
env_bullet['_CPPINCFLAGS'] = '${custom_concat(INCPREFIX, CPPPATH, INCSUFFIX, __env__, RDirs, TARGET, SOURCE, affect_signature=False)}'

I haven't tested it and it might need some tweaking but should be something like that.

But if you already fixed the warnings thats good too!

@pouleyKetchoupp
Copy link
Contributor

@pouleyKetchoupp Should we take the chance and merge now for future 3.4 beta builds, or do you think we'd need more time to assess whether the upgrade is indeed safe?

I just tested on all the 3D physics tests and didn't spot anything.
If we have at least a few weeks for more beta testing before releasing 3.4, I would be up for trying to merge this and #53183 as well.
But if at this point the plan is to release 3.4 as soon as possible, better wait for 3.5 to merge changes in Bullet.

@akien-mga
Copy link
Member Author

@dmoody256 Thanks for the details! For bullet we should be good as is now, but I'll have to assess what to do with other deps where we currently use -isystem to silence warnings (glslang, basis_universal), so this might come in handy.

@pouleyKetchoupp I think we'll have at least one more beta, maybe two if needed. Then a couple RCs or more, so probably 3 weeks until the 3.4 release I think. Might be good enough?

@pouleyKetchoupp
Copy link
Contributor

@akien-mga Yeah 3 weeks sounds good, so depending when you plan the next beta release it can be safe enough.

Copy link
Contributor

@pouleyKetchoupp pouleyKetchoupp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved since 3D physics seems to work well.

@dmoody256
Copy link
Contributor

@akien-mga Just to note, that solution is a bit off-the-cuff hack. A more complete and systemic solution would be to create a new CPPSYSPATH, _INCSYSFLAGS and new instance of CScanner looking at CPPSYSPATH. Then there is no checking the include path specifically for a certain string and it can be shared across all environments and modules.

I can write it up in a PR if its something your interested in, and it sounds like there are real cases where its needed.

@akien-mga akien-mga merged commit 585a9c2 into godotengine:3.x Sep 29, 2021
@akien-mga akien-mga deleted the 3.x-bullet-3.09 branch September 29, 2021 19:26
@akien-mga
Copy link
Member Author

@dmoody256 I'd say let's way to see if the -isystem use in glslang and basis_universal is really needed, or if I just copied it from bullet for consistency. If we can have everything build fine without this workaround that's probably best.

But I remember at the time that I wished SCons itself would have a feature to add "system" include paths and handle everything seamlessly behind the scenes. I guess that's what you mean with CPPSYSPATH? If so that's probably a discussion worth having upstream if we can confirm the need (and the issue with the manual workaround).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants