Merge branch 'physics_clean' into 'master'

Close 2 data races with async physics

See merge request OpenMW/openmw!695

apps/openmw/mwphysics/movementsolver.cpp

@@ -459,8 +459,7 @@ namespace MWPhysics
         const auto verticalHalfExtent = osg::Vec3f(0.0, 0.0, physicActor->getHalfExtents().z());
 
         // use a 3d approximation of the movement vector to better judge player intent
-        const ESM::Position& refpos = ptr.getRefData().getPosition();
+        auto velocity = (osg::Quat(actor.mRefpos.rot[0], osg::Vec3f(-1, 0, 0)) * osg::Quat(actor.mRefpos.rot[2], osg::Vec3f(0, 0, -1))) * actor.mMovement;
-        auto velocity = (osg::Quat(refpos.rot[0], osg::Vec3f(-1, 0, 0)) * osg::Quat(refpos.rot[2], osg::Vec3f(0, 0, -1))) * actor.mMovement;
         // try to pop outside of the world before doing anything else if we're inside of it
         if (!physicActor->getOnGround() || physicActor->getOnSlope())
                 velocity += physicActor->getInertialForce();

apps/openmw/mwphysics/mtphysics.cpp

@@ -9,6 +9,7 @@
 #include "components/settings/settings.hpp"
 #include "../mwmechanics/actorutil.hpp"
 #include "../mwmechanics/movement.hpp"
+#include "../mwrender/bulletdebugdraw.hpp"
 #include "../mwworld/class.hpp"
 #include "../mwworld/player.hpp"
 
@@ -137,11 +138,12 @@ namespace
 
 namespace MWPhysics
 {
-    PhysicsTaskScheduler::PhysicsTaskScheduler(float physicsDt, std::shared_ptr<btCollisionWorld> collisionWorld)
+    PhysicsTaskScheduler::PhysicsTaskScheduler(float physicsDt, btCollisionWorld *collisionWorld, MWRender::DebugDrawer* debugDrawer)
           : mDefaultPhysicsDt(physicsDt)
           , mPhysicsDt(physicsDt)
           , mTimeAccum(0.f)
-          , mCollisionWorld(std::move(collisionWorld))
+          , mCollisionWorld(collisionWorld)
+          , mDebugDrawer(debugDrawer)
           , mNumJobs(0)
           , mRemainingSteps(0)
           , mLOSCacheExpiry(Settings::Manager::getInt("lineofsight keep inactive cache", "Physics"))
@@ -185,7 +187,7 @@ namespace MWPhysics
                 if (data.mActor.lock())
                 {
                     std::unique_lock lock(mCollisionWorldMutex);
-                    MovementSolver::unstuck(data, mCollisionWorld.get());
+                    MovementSolver::unstuck(data, mCollisionWorld);
                 }
             });
 
@@ -381,7 +383,7 @@ namespace MWPhysics
     void PhysicsTaskScheduler::contactTest(btCollisionObject* colObj, btCollisionWorld::ContactResultCallback& resultCallback)
     {
         std::shared_lock lock(mCollisionWorldMutex);
-        ContactTestWrapper::contactTest(mCollisionWorld.get(), colObj, resultCallback);
+        ContactTestWrapper::contactTest(mCollisionWorld, colObj, resultCallback);
     }
 
     std::optional<btVector3> PhysicsTaskScheduler::getHitPoint(const btTransform& from, btCollisionObject* target)
@@ -532,7 +534,7 @@ namespace MWPhysics
                 if(const auto actor = mActorsFrameData[job].mActor.lock())
                 {
                     MaybeSharedLock lockColWorld(mCollisionWorldMutex, mThreadSafeBullet);
-                    MovementSolver::move(mActorsFrameData[job], mPhysicsDt, mCollisionWorld.get(), *mWorldFrameData);
+                    MovementSolver::move(mActorsFrameData[job], mPhysicsDt, mCollisionWorld, *mWorldFrameData);
                 }
             }
 
@@ -594,8 +596,8 @@ namespace MWPhysics
         {
             for (auto& actorData : mActorsFrameData)
             {
-                MovementSolver::unstuck(actorData, mCollisionWorld.get());
+                MovementSolver::unstuck(actorData, mCollisionWorld);
-                MovementSolver::move(actorData, mPhysicsDt, mCollisionWorld.get(), *mWorldFrameData);
+                MovementSolver::move(actorData, mPhysicsDt, mCollisionWorld, *mWorldFrameData);
             }
 
             updateActorsPositions();
@@ -626,4 +628,10 @@ namespace MWPhysics
         mTimeBegin = mTimer->tick();
         mFrameNumber = frameNumber;
     }
+
+    void PhysicsTaskScheduler::debugDraw()
+    {
+        std::shared_lock lock(mCollisionWorldMutex);
+        mDebugDrawer->step();
+    }
 }

apps/openmw/mwphysics/mtphysics.hpp

@@ -20,12 +20,17 @@ namespace Misc
     class Barrier;
 }
 
+namespace MWRender
+{
+    class DebugDrawer;
+}
+
 namespace MWPhysics
 {
     class PhysicsTaskScheduler
     {
         public:
-            PhysicsTaskScheduler(float physicsDt, std::shared_ptr<btCollisionWorld> collisionWorld);
+            PhysicsTaskScheduler(float physicsDt, btCollisionWorld* collisionWorld, MWRender::DebugDrawer* debugDrawer);
             ~PhysicsTaskScheduler();
 
             /// @brief move actors taking into account desired movements and collisions
@@ -49,6 +54,7 @@ namespace MWPhysics
             void removeCollisionObject(btCollisionObject* collisionObject);
             void updateSingleAabb(std::weak_ptr<PtrHolder> ptr, bool immediate=false);
             bool getLineOfSight(const std::weak_ptr<Actor>& actor1, const std::weak_ptr<Actor>& actor2);
+            void debugDraw();
 
         private:
             void syncComputation();
@@ -67,7 +73,8 @@ namespace MWPhysics
             float mDefaultPhysicsDt;
             float mPhysicsDt;
             float mTimeAccum;
-            std::shared_ptr<btCollisionWorld> mCollisionWorld;
+            btCollisionWorld* mCollisionWorld;
+            MWRender::DebugDrawer* mDebugDrawer;
             std::vector<LOSRequest> mLOSCache;
             std::set<std::weak_ptr<PtrHolder>, std::owner_less<std::weak_ptr<PtrHolder>>> mUpdateAabb;
 

apps/openmw/mwphysics/physicssystem.cpp

@@ -79,7 +79,7 @@ namespace MWPhysics
         mDispatcher = std::make_unique<btCollisionDispatcher>(mCollisionConfiguration.get());
         mBroadphase = std::make_unique<btDbvtBroadphase>();
 
-        mCollisionWorld = std::make_shared<btCollisionWorld>(mDispatcher.get(), mBroadphase.get(), mCollisionConfiguration.get());
+        mCollisionWorld = std::make_unique<btCollisionWorld>(mDispatcher.get(), mBroadphase.get(), mCollisionConfiguration.get());
 
         // Don't update AABBs of all objects every frame. Most objects in MW are static, so we don't need this.
         // Should a "static" object ever be moved, we have to update its AABB manually using DynamicsWorld::updateSingleAabb.
@@ -97,8 +97,8 @@ namespace MWPhysics
             }
         }
 
-        mTaskScheduler = std::make_unique<PhysicsTaskScheduler>(mPhysicsDt, mCollisionWorld);
         mDebugDrawer = std::make_unique<MWRender::DebugDrawer>(mParentNode, mCollisionWorld.get(), mDebugDrawEnabled);
+        mTaskScheduler = std::make_unique<PhysicsTaskScheduler>(mPhysicsDt, mCollisionWorld.get(), mDebugDrawer.get());
     }
 
     PhysicsSystem::~PhysicsSystem()
@@ -827,7 +827,7 @@ namespace MWPhysics
     void PhysicsSystem::debugDraw()
     {
         if (mDebugDrawEnabled)
-            mDebugDrawer->step();
+            mTaskScheduler->debugDraw();
     }
 
     bool PhysicsSystem::isActorStandingOn(const MWWorld::Ptr &actor, const MWWorld::ConstPtr &object) const

apps/openmw/mwphysics/physicssystem.hpp

@@ -259,7 +259,7 @@ namespace MWPhysics
             std::unique_ptr<btBroadphaseInterface> mBroadphase;
             std::unique_ptr<btDefaultCollisionConfiguration> mCollisionConfiguration;
             std::unique_ptr<btCollisionDispatcher> mDispatcher;
-            std::shared_ptr<btCollisionWorld> mCollisionWorld;
+            std::unique_ptr<btCollisionWorld> mCollisionWorld;
             std::unique_ptr<PhysicsTaskScheduler> mTaskScheduler;
 
             std::unique_ptr<Resource::BulletShapeManager> mShapeManager;