Merge branch 'killmutex' into 'master'

Remove mutex from PtrHolder

See merge request OpenMW/openmw!1110

apps/openmw/mwphysics/actor.cpp

@@ -21,7 +21,7 @@ namespace MWPhysics
 
 Actor::Actor(const MWWorld::Ptr& ptr, const Resource::BulletShape* shape, PhysicsTaskScheduler* scheduler, bool canWaterWalk)
   : mStandingOnPtr(nullptr), mCanWaterWalk(canWaterWalk), mWalkingOnWater(false)
-  , mCollisionObject(nullptr), mMeshTranslation(shape->mCollisionBox.center), mOriginalHalfExtents(shape->mCollisionBox.extents)
+  , mMeshTranslation(shape->mCollisionBox.center), mOriginalHalfExtents(shape->mCollisionBox.extents)
   , mVelocity(0,0,0), mStuckFrames(0), mLastStuckPosition{0, 0, 0}
   , mForce(0.f, 0.f, 0.f), mOnGround(true), mOnSlope(false)
   , mInternalCollisionMode(true)

apps/openmw/mwphysics/actor.hpp

@@ -132,11 +132,6 @@ namespace MWPhysics
             return mInternalCollisionMode && mOnSlope;
         }
 
-        btCollisionObject* getCollisionObject() const
-        {
-            return mCollisionObject.get();
-        }
-
         /// Sets whether this actor should be able to collide with the water surface
         void setCanWaterWalk(bool waterWalk);
 
@@ -188,8 +183,6 @@ namespace MWPhysics
         std::unique_ptr<btCollisionShape> mShape;
         btConvexShape* mConvexShape;
 
-        std::unique_ptr<btCollisionObject> mCollisionObject;
-
         osg::Vec3f mMeshTranslation;
         osg::Vec3f mOriginalHalfExtents;
         osg::Vec3f mHalfExtents;

apps/openmw/mwphysics/actorconvexcallback.cpp

@@ -77,10 +77,8 @@ namespace MWPhysics
             auto* projectileHolder = static_cast<Projectile*>(convexResult.m_hitCollisionObject->getUserPointer());
             if (!projectileHolder->isActive())
                 return btScalar(1);
-            auto* targetHolder = static_cast<PtrHolder*>(mMe->getUserPointer());
+            if (projectileHolder->isValidTarget(mMe))
-            const MWWorld::Ptr target = targetHolder->getPtr();
+                projectileHolder->hit(mMe, convexResult.m_hitPointLocal, convexResult.m_hitNormalLocal);
-            if (projectileHolder->isValidTarget(target))
-                projectileHolder->hit(target, convexResult.m_hitPointLocal, convexResult.m_hitNormalLocal);
             return btScalar(1);
         }
 

apps/openmw/mwphysics/closestnotmerayresultcallback.cpp

@@ -7,6 +7,7 @@
 
 #include "../mwworld/class.hpp"
 
+#include "collisiontype.hpp"
 #include "ptrholder.hpp"
 
 namespace MWPhysics
@@ -19,17 +20,14 @@ namespace MWPhysics
 
     btScalar ClosestNotMeRayResultCallback::addSingleResult(btCollisionWorld::LocalRayResult& rayResult, bool normalInWorldSpace)
     {
-        if (rayResult.m_collisionObject == mMe)
+        const auto* hitObject = rayResult.m_collisionObject;
+        if (hitObject == mMe)
             return 1.f;
 
-        if (!mTargets.empty())
+        if (hitObject->getBroadphaseHandle()->m_collisionFilterGroup == CollisionType_Actor && !mTargets.empty())
         {
-            if ((std::find(mTargets.begin(), mTargets.end(), rayResult.m_collisionObject) == mTargets.end()))
+            if ((std::find(mTargets.begin(), mTargets.end(), hitObject) == mTargets.end()))
-            {
+                return 1.f;
-                auto* holder = static_cast<PtrHolder*>(rayResult.m_collisionObject->getUserPointer());
-                if (holder && !holder->getPtr().isEmpty() && holder->getPtr().getClass().isActor())
-                    return 1.f;
-            }
         }
 
         return btCollisionWorld::ClosestRayResultCallback::addSingleResult(rayResult, normalInWorldSpace);

apps/openmw/mwphysics/deepestnotmecontacttestresultcallback.cpp

@@ -6,6 +6,7 @@
 
 #include "../mwworld/class.hpp"
 
+#include "collisiontype.hpp"
 #include "ptrholder.hpp"
 
 namespace MWPhysics
@@ -23,14 +24,10 @@ namespace MWPhysics
         const btCollisionObject* collisionObject = col1Wrap->m_collisionObject;
         if (collisionObject != mMe)
         {
-            if (!mTargets.empty())
+            if (collisionObject->getBroadphaseHandle()->m_collisionFilterGroup == CollisionType_Actor && !mTargets.empty())
             {
                 if ((std::find(mTargets.begin(), mTargets.end(), collisionObject) == mTargets.end()))
-                {
+                    return 0.f;
-                    PtrHolder* holder = static_cast<PtrHolder*>(collisionObject->getUserPointer());
-                    if (holder && !holder->getPtr().isEmpty() && holder->getPtr().getClass().isActor())
-                        return 0.f;
-                }
             }
 
             btScalar distsqr = mOrigin.distance2(cp.getPositionWorldOnA());

apps/openmw/mwphysics/object.cpp

@@ -83,16 +83,6 @@ namespace MWPhysics
         }
     }
 
-    btCollisionObject* Object::getCollisionObject()
-    {
-        return mCollisionObject.get();
-    }
-
-    const btCollisionObject* Object::getCollisionObject() const
-    {
-        return mCollisionObject.get();
-    }
-
     btTransform Object::getTransform() const
     {
         std::unique_lock<std::mutex> lock(mPositionMutex);

apps/openmw/mwphysics/object.hpp

@@ -33,8 +33,6 @@ namespace MWPhysics
         void setRotation(osg::Quat quat);
         void updatePosition();
         void commitPositionChange();
-        btCollisionObject* getCollisionObject();
-        const btCollisionObject* getCollisionObject() const;
         btTransform getTransform() const;
         /// Return solid flag. Not used by the object itself, true by default.
         bool isSolid() const;
@@ -45,7 +43,6 @@ namespace MWPhysics
         bool animateCollisionShapes();
 
     private:
-        std::unique_ptr<btCollisionObject> mCollisionObject;
         osg::ref_ptr<Resource::BulletShapeInstance> mShapeInstance;
         std::map<int, osg::NodePath> mRecIndexToNodePath;
         bool mSolid;

apps/openmw/mwphysics/physicssystem.cpp

@@ -635,19 +635,7 @@ namespace MWPhysics
         if (btFrom == btTo)
             return;
 
-        const auto casterPtr = projectile->getCaster();
+        ProjectileConvexCallback resultCallback(projectile->getCasterCollisionObject(), projectile->getCollisionObject(), btFrom, btTo, projectile);
-        const auto* caster = [this,&casterPtr]() -> const btCollisionObject*
-        {
-            const Actor* actor = getActor(casterPtr);
-            if (actor)
-                return actor->getCollisionObject();
-            const Object* object = getObject(casterPtr);
-            if (object)
-                return object->getCollisionObject();
-            return nullptr;
-        }();
-
-        ProjectileConvexCallback resultCallback(caster, btFrom, btTo, projectile);
         resultCallback.m_collisionFilterMask = 0xff;
         resultCallback.m_collisionFilterGroup = CollisionType_Projectile;
 

apps/openmw/mwphysics/projectile.cpp

@@ -7,8 +7,10 @@
 
 #include "../mwworld/class.hpp"
 
+#include "actor.hpp"
 #include "collisiontype.hpp"
 #include "mtphysics.hpp"
+#include "object.hpp"
 #include "projectile.hpp"
 
 namespace MWPhysics
@@ -17,7 +19,7 @@ Projectile::Projectile(const MWWorld::Ptr& caster, const osg::Vec3f& position, f
     : mCanCrossWaterSurface(canCrossWaterSurface)
     , mCrossedWaterSurface(false)
     , mActive(true)
-    , mCaster(caster)
+    , mHitTarget(nullptr)
     , mWaterHitPosition(std::nullopt)
     , mPhysics(physicssystem)
     , mTaskScheduler(scheduler)
@@ -32,6 +34,7 @@ Projectile::Projectile(const MWWorld::Ptr& caster, const osg::Vec3f& position, f
     mCollisionObject->setUserPointer(this);
 
     setPosition(position);
+    setCaster(caster);
 
     const int collisionMask = CollisionType_World | CollisionType_HeightMap |
         CollisionType_Actor | CollisionType_Door | CollisionType_Water | CollisionType_Projectile;
@@ -77,54 +80,67 @@ bool Projectile::canTraverseWater() const
     return mCanCrossWaterSurface;
 }
 
-void Projectile::hit(const MWWorld::Ptr& target, btVector3 pos, btVector3 normal)
+void Projectile::hit(const btCollisionObject* target, btVector3 pos, btVector3 normal)
 {
-    if (!mActive.load(std::memory_order_acquire))
+    bool active = true;
+    if (!mActive.compare_exchange_strong(active, false, std::memory_order_relaxed) || !active)
         return;
-    std::scoped_lock lock(mMutex);
     mHitTarget = target;
     mHitPosition = pos;
     mHitNormal = normal;
-    mActive.store(false, std::memory_order_release);
+}
+
+MWWorld::Ptr Projectile::getTarget() const
+{
+    assert(!mActive);
+    auto* target = static_cast<PtrHolder*>(mHitTarget->getUserPointer());
+    return target ? target->getPtr() : MWWorld::Ptr();
 }
 
 MWWorld::Ptr Projectile::getCaster() const
 {
-    std::scoped_lock lock(mMutex);
     return mCaster;
 }
 
 void Projectile::setCaster(const MWWorld::Ptr& caster)
 {
-    std::scoped_lock lock(mMutex);
     mCaster = caster;
+    mCasterColObj = [this,&caster]() -> const btCollisionObject*
+    {
+        const Actor* actor = mPhysics->getActor(caster);
+        if (actor)
+            return actor->getCollisionObject();
+        const Object* object = mPhysics->getObject(caster);
+        if (object)
+            return object->getCollisionObject();
+        return nullptr;
+    }();
 }
 
 void Projectile::setValidTargets(const std::vector<MWWorld::Ptr>& targets)
 {
     std::scoped_lock lock(mMutex);
-    mValidTargets = targets;
+    mValidTargets.clear();
+    for (const auto& ptr : targets)
+    {
+        const auto* physicActor = mPhysics->getActor(ptr);
+        if (physicActor)
+            mValidTargets.push_back(physicActor->getCollisionObject());
+    }
 }
 
-bool Projectile::isValidTarget(const MWWorld::Ptr& target) const
+bool Projectile::isValidTarget(const btCollisionObject* target) const
 {
+    assert(target);
     std::scoped_lock lock(mMutex);
-    if (mCaster == target)
+    if (mCasterColObj == target)
         return false;
 
-    if (target.isEmpty() || mValidTargets.empty())
+    if (mValidTargets.empty())
         return true;
 
-    bool validTarget = false;
+    return std::any_of(mValidTargets.begin(), mValidTargets.end(),
-    for (const auto& targetActor : mValidTargets)
+            [target](const btCollisionObject* actor) { return target == actor; });
-    {
-        if (targetActor == target)
-        {
-            validTarget = true;
-            break;
-        }
-    }
-    return validTarget;
 }
 
 std::optional<btVector3> Projectile::getWaterHitPosition()

apps/openmw/mwphysics/projectile.hpp

@@ -42,31 +42,26 @@ namespace MWPhysics
         void setPosition(const osg::Vec3f& position);
         osg::Vec3f getPosition() const;
 
-        btCollisionObject* getCollisionObject() const
-        {
-            return mCollisionObject.get();
-        }
-
         bool isActive() const
         {
             return mActive.load(std::memory_order_acquire);
         }
 
-        MWWorld::Ptr getTarget() const
+        MWWorld::Ptr getTarget() const;
-        {
-            assert(!mActive);
-            return mHitTarget;
-        }
 
         MWWorld::Ptr getCaster() const;
         void setCaster(const MWWorld::Ptr& caster);
+        const btCollisionObject* getCasterCollisionObject() const
+        {
+            return mCasterColObj;
+        }
 
         bool canTraverseWater() const;
 
-        void hit(const MWWorld::Ptr& target, btVector3 pos, btVector3 normal);
+        void hit(const btCollisionObject* target, btVector3 pos, btVector3 normal);
 
         void setValidTargets(const std::vector<MWWorld::Ptr>& targets);
-        bool isValidTarget(const MWWorld::Ptr& target) const;
+        bool isValidTarget(const btCollisionObject* target) const;
 
         std::optional<btVector3> getWaterHitPosition();
         void setWaterHitPosition(btVector3 pos);
@@ -76,19 +71,19 @@ namespace MWPhysics
         std::unique_ptr<btCollisionShape> mShape;
         btConvexShape* mConvexShape;
 
-        std::unique_ptr<btCollisionObject> mCollisionObject;
         bool mTransformUpdatePending;
         bool mCanCrossWaterSurface;
         bool mCrossedWaterSurface;
         std::atomic<bool> mActive;
         MWWorld::Ptr mCaster;
-        MWWorld::Ptr mHitTarget;
+        const btCollisionObject* mCasterColObj;
+        const btCollisionObject* mHitTarget;
         std::optional<btVector3> mWaterHitPosition;
         osg::Vec3f mPosition;
         btVector3 mHitPosition;
         btVector3 mHitNormal;
 
-        std::vector<MWWorld::Ptr> mValidTargets;
+        std::vector<const btCollisionObject*> mValidTargets;
 
         mutable std::mutex mMutex;
 

apps/openmw/mwphysics/projectileconvexcallback.cpp

@@ -1,3 +1,5 @@
+#include <BulletCollision/CollisionDispatch/btCollisionObject.h>
+
 #include "../mwworld/class.hpp"
 
 #include "actor.hpp"
@@ -8,41 +10,41 @@
 
 namespace MWPhysics
 {
-    ProjectileConvexCallback::ProjectileConvexCallback(const btCollisionObject* me, const btVector3& from, const btVector3& to, Projectile* proj)
+    ProjectileConvexCallback::ProjectileConvexCallback(const btCollisionObject* caster, const btCollisionObject* me, const btVector3& from, const btVector3& to, Projectile* proj)
         : btCollisionWorld::ClosestConvexResultCallback(from, to)
-        , mMe(me), mProjectile(proj)
+        , mCaster(caster)
+        , mMe(me)
+        , mProjectile(proj)
     {
         assert(mProjectile);
     }
 
     btScalar ProjectileConvexCallback::addSingleResult(btCollisionWorld::LocalConvexResult& result, bool normalInWorldSpace)
     {
+        const auto* hitObject = result.m_hitCollisionObject;
         // don't hit the caster
-        if (result.m_hitCollisionObject == mMe)
+        if (hitObject == mCaster)
             return 1.f;
 
         // don't hit the projectile
-        if (result.m_hitCollisionObject == mProjectile->getCollisionObject())
+        if (hitObject == mMe)
             return 1.f;
 
         btCollisionWorld::ClosestConvexResultCallback::addSingleResult(result, normalInWorldSpace);
-        switch (result.m_hitCollisionObject->getBroadphaseHandle()->m_collisionFilterGroup)
+        switch (hitObject->getBroadphaseHandle()->m_collisionFilterGroup)
         {
             case CollisionType_Actor:
                 {
-                    auto* target = static_cast<Actor*>(result.m_hitCollisionObject->getUserPointer());
+                    if (!mProjectile->isValidTarget(hitObject))
-                    if (!mProjectile->isValidTarget(target->getPtr()))
                         return 1.f;
-                    mProjectile->hit(target->getPtr(), result.m_hitPointLocal, result.m_hitNormalLocal);
                     break;
                 }
             case CollisionType_Projectile:
                 {
-                    auto* target = static_cast<Projectile*>(result.m_hitCollisionObject->getUserPointer());
+                    auto* target = static_cast<Projectile*>(hitObject->getUserPointer());
-                    if (!mProjectile->isValidTarget(target->getCaster()))
+                    if (!mProjectile->isValidTarget(target->getCasterCollisionObject()))
                         return 1.f;
-                    target->hit(mProjectile->getPtr(), m_hitPointWorld, m_hitNormalWorld);
+                    target->hit(mMe, m_hitPointWorld, m_hitNormalWorld);
-                    mProjectile->hit(target->getPtr(), m_hitPointWorld, m_hitNormalWorld);
                     break;
                 }
             case CollisionType_Water:
@@ -50,17 +52,10 @@ namespace MWPhysics
                     mProjectile->setWaterHitPosition(m_hitPointWorld);
                     if (mProjectile->canTraverseWater())
                         return 1.f;
-                    mProjectile->hit(MWWorld::Ptr(), m_hitPointWorld, m_hitNormalWorld);
-                    break;
-                }
-            default:
-                {
-                    auto* target = static_cast<PtrHolder*>(result.m_hitCollisionObject->getUserPointer());
-                    auto ptr = target ? target->getPtr() : MWWorld::Ptr();
-                    mProjectile->hit(ptr, m_hitPointWorld, m_hitNormalWorld);
                     break;
                 }
         }
+        mProjectile->hit(hitObject, m_hitPointWorld, m_hitNormalWorld);
 
         return result.m_hitFraction;
     }

apps/openmw/mwphysics/projectileconvexcallback.hpp

@@ -12,11 +12,12 @@ namespace MWPhysics
     class ProjectileConvexCallback : public btCollisionWorld::ClosestConvexResultCallback
     {
     public:
-        ProjectileConvexCallback(const btCollisionObject* me, const btVector3& from, const btVector3& to, Projectile* proj);
+        ProjectileConvexCallback(const btCollisionObject* caster, const btCollisionObject* me, const btVector3& from, const btVector3& to, Projectile* proj);
 
         btScalar addSingleResult(btCollisionWorld::LocalConvexResult& result, bool normalInWorldSpace) override;
 
     private:
+        const btCollisionObject* mCaster;
         const btCollisionObject* mMe;
         Projectile* mProjectile;
     };

apps/openmw/mwphysics/ptrholder.hpp

@@ -2,6 +2,9 @@
 #define OPENMW_MWPHYSICS_PTRHOLDER_H
 
 #include <mutex>
+#include <memory>
+
+#include <BulletCollision/CollisionDispatch/btCollisionObject.h>
 
 #include "../mwworld/ptr.hpp"
 
@@ -10,31 +13,26 @@ namespace MWPhysics
     class PtrHolder
     {
     public:
-        virtual ~PtrHolder() {}
+        virtual ~PtrHolder() = default;
 
         void updatePtr(const MWWorld::Ptr& updated)
         {
-            std::scoped_lock lock(mMutex);
             mPtr = updated;
         }
 
         MWWorld::Ptr getPtr()
         {
-            std::scoped_lock lock(mMutex);
             return mPtr;
         }
 
-        MWWorld::ConstPtr getPtr() const
+        btCollisionObject* getCollisionObject() const
         {
-            std::scoped_lock lock(mMutex);
+            return mCollisionObject.get();
-            return mPtr;
         }
 
     protected:
         MWWorld::Ptr mPtr;
-
+        std::unique_ptr<btCollisionObject> mCollisionObject;
-    private:
-        mutable std::mutex mMutex;
     };
 }
 

apps/openmw/mwworld/projectilemanager.cpp

@@ -521,7 +521,7 @@ namespace MWWorld
             }
 
             MWMechanics::projectileHit(caster, target, bow, projectileRef.getPtr(), pos, projectileState.mAttackStrength);
-            cleanupProjectile(projectileState);
+            projectileState.mToDelete = true;
         }
         for (auto& magicBoltState : mMagicBolts)
         {
@@ -550,7 +550,19 @@ namespace MWWorld
             cast.inflict(target, caster, magicBoltState.mEffects, ESM::RT_Target, false, true);
 
             MWBase::Environment::get().getWorld()->explodeSpell(pos, magicBoltState.mEffects, caster, target, ESM::RT_Target, magicBoltState.mSpellId, magicBoltState.mSourceName);
-            cleanupMagicBolt(magicBoltState);
+            magicBoltState.mToDelete = true;
+        }
+
+        for (auto& projectileState : mProjectiles)
+        {
+            if (projectileState.mToDelete)
+                cleanupProjectile(projectileState);
+        }
+
+        for (auto& magicBoltState : mMagicBolts)
+        {
+            if (magicBoltState.mToDelete)
+                cleanupMagicBolt(magicBoltState);
         }
         mProjectiles.erase(std::remove_if(mProjectiles.begin(), mProjectiles.end(), [](const State& state) { return state.mToDelete; }),
                 mProjectiles.end());