Embed physics simulation results inside of actor class.

This gives finer control over reseting positions (switch off tcl is no
longer glitchy) and solve most of the erroneous usage of stale World::Ptr
indicated by:
"Error in frame: moveTo: object is not in this cell"

apps/openmw/mwphysics/actor.cpp

@@ -76,6 +76,7 @@ Actor::Actor(const MWWorld::Ptr& ptr, const Resource::BulletShape* shape, Physic
     updateScale();
     resetPosition();
     addCollisionMask(getCollisionMask());
+    updateCollisionObjectPosition();
 }
 
 Actor::~Actor()
@@ -139,7 +140,9 @@ osg::Vec3f Actor::getWorldPosition() const
 
 void Actor::setSimulationPosition(const osg::Vec3f& position)
 {
-    mSimulationPosition = position;
+    if (!mResetSimulation)
+        mSimulationPosition = position;
+    mResetSimulation = false;
 }
 
 osg::Vec3f Actor::getSimulationPosition() const
@@ -193,9 +196,9 @@ void Actor::resetPosition()
     mPreviousPosition = mWorldPosition;
     mPosition = mWorldPosition;
     mSimulationPosition = mWorldPosition;
-    updateCollisionObjectPositionUnsafe();
     mStandingOnPtr = nullptr;
     mWorldPositionChanged = false;
+    mResetSimulation = true;
 }
 
 osg::Vec3f Actor::getPosition() const

apps/openmw/mwphysics/actor.hpp

@@ -180,6 +180,7 @@ namespace MWPhysics
         osg::Vec3f mPreviousPosition;
         osg::Vec3f mPositionOffset;
         bool mWorldPositionChanged;
+        bool mResetSimulation;
         btTransform mLocalTransform;
         mutable std::mutex mPositionMutex;
 

apps/openmw/mwphysics/mtphysics.cpp

@@ -204,31 +204,31 @@ namespace MWPhysics
             thread.join();
     }
 
-    const PtrPositionList& PhysicsTaskScheduler::moveActors(int numSteps, float timeAccum, std::vector<ActorFrameData>&& actorsData, osg::Timer_t frameStart, unsigned int frameNumber, osg::Stats& stats)
+    const std::vector<MWWorld::Ptr>& PhysicsTaskScheduler::moveActors(int numSteps, float timeAccum, std::vector<ActorFrameData>&& actorsData, osg::Timer_t frameStart, unsigned int frameNumber, osg::Stats& stats)
     {
         // This function run in the main thread.
         // While the mSimulationMutex is held, background physics threads can't run.
 
         std::unique_lock lock(mSimulationMutex);
 
-        for (auto& data : actorsData)
+        mMovedActors.clear();
-            data.updatePosition();
 
         // start by finishing previous background computation
         if (mNumThreads != 0)
         {
             for (auto& data : mActorsFrameData)
             {
-                // Ignore actors that were deleted while the background thread was running
+                // Only return actors that are still part of the scene
-                if (!data.mActor.lock())
+                if (std::any_of(actorsData.begin(), actorsData.end(), [&data](const auto& newFrameData) { return data.mActorRaw->getPtr() == newFrameData.mActorRaw->getPtr(); }))
-                    continue;
+                {
+                    updateMechanics(data);
 
-                updateMechanics(data);
+                    // these variables are accessed directly from the main thread, update them here to prevent accessing "too new" values
-                if (mAdvanceSimulation)
+                    if (mAdvanceSimulation)
-                    data.mActorRaw->setStandingOnPtr(data.mStandingOn);
+                        data.mActorRaw->setStandingOnPtr(data.mStandingOn);
-
+                    data.mActorRaw->setSimulationPosition(interpolateMovements(data, mTimeAccum, mPhysicsDt));
-                if (mMovementResults.find(data.mPtr) != mMovementResults.end())
+                    mMovedActors.emplace_back(data.mActorRaw->getPtr());
-                    data.mActorRaw->setSimulationPosition(mMovementResults[data.mPtr]);
+                }
             }
 
             if (mFrameNumber == frameNumber - 1)
@@ -243,6 +243,8 @@ namespace MWPhysics
         }
 
         // init
+        for (auto& data : actorsData)
+            data.updatePosition();
         mRemainingSteps = numSteps;
         mTimeAccum = timeAccum;
         mActorsFrameData = std::move(actorsData);
@@ -257,52 +259,28 @@ namespace MWPhysics
 
         if (mNumThreads == 0)
         {
-            mMovementResults.clear();
             syncComputation();
-
+            return mMovedActors;
-            for (auto& data : mActorsFrameData)
-            {
-                if (mAdvanceSimulation)
-                    data.mActorRaw->setStandingOnPtr(data.mStandingOn);
-                if (mMovementResults.find(data.mPtr) != mMovementResults.end())
-                    data.mActorRaw->setSimulationPosition(mMovementResults[data.mPtr]);
-            }
-            return mMovementResults;
         }
 
-        // Remove actors that were deleted while the background thread was running
-        for (auto& data : mActorsFrameData)
-        {
-            if (!data.mActor.lock())
-                mMovementResults.erase(data.mPtr);
-        }
-        std::swap(mMovementResults, mPreviousMovementResults);
-
-        // mMovementResults is shared between all workers instance
-        // pre-allocate all nodes so that we don't need synchronization
-        mMovementResults.clear();
-        for (const auto& m : mActorsFrameData)
-            mMovementResults[m.mPtr] = m.mPosition;
-
         lock.unlock();
         mHasJob.notify_all();
-        return mPreviousMovementResults;
+        return mMovedActors;
     }
 
-    const PtrPositionList& PhysicsTaskScheduler::resetSimulation(const ActorMap& actors)
+    const std::vector<MWWorld::Ptr>& PhysicsTaskScheduler::resetSimulation(const ActorMap& actors)
     {
         std::unique_lock lock(mSimulationMutex);
-        mMovementResults.clear();
+        mMovedActors.clear();
-        mPreviousMovementResults.clear();
         mActorsFrameData.clear();
-
         for (const auto& [_, actor] : actors)
         {
             actor->resetPosition();
-            actor->setStandingOnPtr(nullptr);
+            actor->setSimulationPosition(actor->getWorldPosition()); // resetPosition skip next simulation, now we need to "consume" it
-            mMovementResults[actor->getPtr()] = actor->getWorldPosition();
+            actor->updateCollisionObjectPosition();
+            mMovedActors.emplace_back(actor->getPtr());
         }
-        return mMovementResults;
+        return mMovedActors;
     }
 
     void PhysicsTaskScheduler::rayTest(const btVector3& rayFromWorld, const btVector3& rayToWorld, btCollisionWorld::RayResultCallback& resultCallback) const
@@ -370,18 +348,18 @@ namespace MWPhysics
         mCollisionWorld->removeCollisionObject(collisionObject);
     }
 
-    void PhysicsTaskScheduler::updateSingleAabb(std::weak_ptr<PtrHolder> ptr)
+    void PhysicsTaskScheduler::updateSingleAabb(std::weak_ptr<PtrHolder> ptr, bool immediate)
     {
-        if (mDeferAabbUpdate)
+        if (!mDeferAabbUpdate || immediate)
-        {
-            std::unique_lock lock(mUpdateAabbMutex);
-            mUpdateAabb.insert(std::move(ptr));
-        }
-        else
         {
             std::unique_lock lock(mCollisionWorldMutex);
             updatePtrAabb(ptr);
         }
+        else
+        {
+            std::unique_lock lock(mUpdateAabbMutex);
+            mUpdateAabb.insert(std::move(ptr));
+        }
     }
 
     bool PhysicsTaskScheduler::getLineOfSight(const std::weak_ptr<Actor>& actor1, const std::weak_ptr<Actor>& actor2)
@@ -484,7 +462,6 @@ namespace MWPhysics
                     {
                         auto& actorData = mActorsFrameData[job];
                         handleFall(actorData, mAdvanceSimulation);
-                        mMovementResults[actorData.mPtr] = interpolateMovements(actorData, mTimeAccum, mPhysicsDt);
                     }
                 }
 
@@ -539,8 +516,11 @@ namespace MWPhysics
         for (auto& actorData : mActorsFrameData)
         {
             handleFall(actorData, mAdvanceSimulation);
-            mMovementResults[actorData.mPtr] = interpolateMovements(actorData, mTimeAccum, mPhysicsDt);
+            actorData.mActorRaw->setSimulationPosition(interpolateMovements(actorData, mTimeAccum, mPhysicsDt));
             updateMechanics(actorData);
+            mMovedActors.emplace_back(actorData.mActorRaw->getPtr());
+            if (mAdvanceSimulation)
+                actorData.mActorRaw->setStandingOnPtr(actorData.mStandingOn);
         }
     }
 }

apps/openmw/mwphysics/mtphysics.hpp

@@ -32,9 +32,9 @@ namespace MWPhysics
             /// @param timeAccum accumulated time from previous run to interpolate movements
             /// @param actorsData per actor data needed to compute new positions
             /// @return new position of each actor
-            const PtrPositionList& moveActors(int numSteps, float timeAccum, std::vector<ActorFrameData>&& actorsData, osg::Timer_t frameStart, unsigned int frameNumber, osg::Stats& stats);
+            const std::vector<MWWorld::Ptr>& moveActors(int numSteps, float timeAccum, std::vector<ActorFrameData>&& actorsData, osg::Timer_t frameStart, unsigned int frameNumber, osg::Stats& stats);
 
-            const PtrPositionList& resetSimulation(const ActorMap& actors);
+            const std::vector<MWWorld::Ptr>& resetSimulation(const ActorMap& actors);
 
             // Thread safe wrappers
             void rayTest(const btVector3& rayFromWorld, const btVector3& rayToWorld, btCollisionWorld::RayResultCallback& resultCallback) const;
@@ -46,7 +46,7 @@ namespace MWPhysics
             void setCollisionFilterMask(btCollisionObject* collisionObject, int collisionFilterMask);
             void addCollisionObject(btCollisionObject* collisionObject, int collisionFilterGroup, int collisionFilterMask);
             void removeCollisionObject(btCollisionObject* collisionObject);
-            void updateSingleAabb(std::weak_ptr<PtrHolder> ptr);
+            void updateSingleAabb(std::weak_ptr<PtrHolder> ptr, bool immediate=false);
             bool getLineOfSight(const std::weak_ptr<Actor>& actor1, const std::weak_ptr<Actor>& actor2);
 
         private:
@@ -60,8 +60,7 @@ namespace MWPhysics
 
             std::unique_ptr<WorldFrameData> mWorldFrameData;
             std::vector<ActorFrameData> mActorsFrameData;
-            PtrPositionList mMovementResults;
+            std::vector<MWWorld::Ptr> mMovedActors;
-            PtrPositionList mPreviousMovementResults;
             const float mPhysicsDt;
             float mTimeAccum;
             std::shared_ptr<btCollisionWorld> mCollisionWorld;

apps/openmw/mwphysics/physicssystem.cpp

@@ -437,7 +437,7 @@ namespace MWPhysics
         ActorMap::iterator found = mActors.find(ptr);
         if (found ==  mActors.end())
             return ptr.getRefData().getPosition().asVec3();
-        found->second->resetPosition();
+        resetPosition(ptr);
         return MovementSolver::traceDown(ptr, position, found->second.get(), mCollisionWorld.get(), maxHeight);
     }
 
@@ -647,6 +647,17 @@ namespace MWPhysics
         }
     }
 
+    void PhysicsSystem::resetPosition(const MWWorld::ConstPtr &ptr)
+    {
+        ActorMap::iterator foundActor = mActors.find(ptr);
+        if (foundActor != mActors.end())
+        {
+            foundActor->second->resetPosition();
+            mTaskScheduler->updateSingleAabb(foundActor->second, true);
+            return;
+        }
+    }
+
     void PhysicsSystem::addActor (const MWWorld::Ptr& ptr, const std::string& mesh)
     {
         osg::ref_ptr<const Resource::BulletShape> shape = mShapeManager->getShape(mesh);
@@ -683,6 +694,8 @@ namespace MWPhysics
         if (found != mActors.end())
         {
             bool cmode = found->second->getCollisionMode();
+            if (cmode)
+                resetPosition(found->first);
             cmode = !cmode;
             found->second->enableCollisionMode(cmode);
             // NB: Collision body isn't disabled for vanilla TCL compatibility
@@ -711,7 +724,7 @@ namespace MWPhysics
         mMovementQueue.clear();
     }
 
-    const PtrPositionList& PhysicsSystem::applyQueuedMovement(float dt, bool skipSimulation, osg::Timer_t frameStart, unsigned int frameNumber, osg::Stats& stats)
+    const std::vector<MWWorld::Ptr>& PhysicsSystem::applyQueuedMovement(float dt, bool skipSimulation, osg::Timer_t frameStart, unsigned int frameNumber, osg::Stats& stats)
     {
         mTimeAccum += dt;
 
@@ -930,7 +943,6 @@ namespace MWPhysics
     void ActorFrameData::updatePosition()
     {
         mActorRaw->updatePosition();
-        mOrigin = mActorRaw->getSimulationPosition();
         mPosition = mActorRaw->getPosition();
         if (mMoveToWaterSurface)
         {

apps/openmw/mwphysics/physicssystem.hpp

@@ -50,8 +50,6 @@ class btVector3;
 
 namespace MWPhysics
 {
-    using PtrPositionList = std::map<MWWorld::Ptr, osg::Vec3f>;
-
     class HeightField;
     class Object;
     class Actor;
@@ -99,7 +97,6 @@ namespace MWPhysics
         float mOldHeight;
         float mFallHeight;
         osg::Vec3f mMovement;
-        osg::Vec3f mOrigin;
         osg::Vec3f mPosition;
         ESM::Position mRefpos;
     };
@@ -147,6 +144,7 @@ namespace MWPhysics
             void updateScale (const MWWorld::Ptr& ptr);
             void updateRotation (const MWWorld::Ptr& ptr);
             void updatePosition (const MWWorld::Ptr& ptr);
+            void resetPosition(const MWWorld::ConstPtr &ptr);
 
             void addHeightField (const float* heights, int x, int y, float triSize, float sqrtVerts, float minH, float maxH, const osg::Object* holdObject);
 
@@ -210,7 +208,7 @@ namespace MWPhysics
             void queueObjectMovement(const MWWorld::Ptr &ptr, const osg::Vec3f &velocity);
 
             /// Apply all queued movements, then clear the list.
-            const PtrPositionList& applyQueuedMovement(float dt, bool skipSimulation, osg::Timer_t frameStart, unsigned int frameNumber, osg::Stats& stats);
+            const std::vector<MWWorld::Ptr>& applyQueuedMovement(float dt, bool skipSimulation, osg::Timer_t frameStart, unsigned int frameNumber, osg::Stats& stats);
 
             /// Clear the queued movements list without applying.
             void clearQueuedMovement();

apps/openmw/mwworld/worldimp.cpp

@@ -1356,12 +1356,7 @@ namespace MWWorld
         }
 
         moveObject(ptr, ptr.getCell(), pos.x(), pos.y(), pos.z());
-        if (ptr.getClass().isActor())
+        mPhysics->resetPosition(ptr);
-        {
-            MWPhysics::Actor* actor = mPhysics->getActor(ptr);
-            if (actor)
-                actor->resetPosition();
-        }
     }
 
     void World::fixPosition()
@@ -1512,16 +1507,26 @@ namespace MWWorld
         mProjectileManager->processHits();
         mDiscardMovements = false;
 
-        for(const auto& [actor, position]: results)
+        for(const auto& actor : results)
         {
             // Handle player last, in case a cell transition occurs
             if(actor != getPlayerPtr())
+            {
+                auto* physactor = mPhysics->getActor(actor);
+                assert(physactor);
+                const auto position = physactor->getSimulationPosition();
                 moveObjectImp(actor, position.x(), position.y(), position.z(), false);
+            }
         }
 
-        const auto player = results.find(getPlayerPtr());
+        const auto player = std::find(results.begin(), results.end(), getPlayerPtr());
         if (player != results.end())
-            moveObjectImp(player->first, player->second.x(), player->second.y(), player->second.z(), false);
+        {
+            auto* physactor = mPhysics->getActor(*player);
+            assert(physactor);
+            const auto position = physactor->getSimulationPosition();
+            moveObjectImp(*player, position.x(), position.y(), position.z(), false);
+        }
     }
 
     void World::updateNavigator()