Merge branch 'async_delete_animation' into 'master'

Destruct animation asynchronously when unloading a cell

See merge request OpenMW/openmw!2177

apps/openmw/engine.cpp

@@ -46,6 +46,7 @@
 #include <components/sceneutil/depth.hpp>
 #include <components/sceneutil/color.hpp>
 #include <components/sceneutil/util.hpp>
+#include <components/sceneutil/unrefqueue.hpp>
 
 #include <components/settings/shadermanager.hpp>
 
@@ -419,7 +420,14 @@ bool OMW::Engine::frame(float frametime)
             mWindowManager->update(frametime);
         }
 
-        if (stats->collectStats("resource"))
+        const bool reportResource = stats->collectStats("resource");
+
+        if (reportResource)
+            stats->setAttribute(frameNumber, "UnrefQueue", mUnrefQueue->getSize());
+
+        mUnrefQueue->flush(*mWorkQueue);
+
+        if (reportResource)
         {
             stats->setAttribute(frameNumber, "FrameNumber", frameNumber);
 
@@ -492,6 +500,7 @@ OMW::Engine::~Engine()
 
     mScriptContext = nullptr;
 
+    mUnrefQueue = nullptr;
     mWorkQueue = nullptr;
 
     mViewer = nullptr;
@@ -751,6 +760,7 @@ void OMW::Engine::prepareEngine()
     if (numThreads <= 0)
         throw std::runtime_error("Invalid setting: 'preload num threads' must be >0");
     mWorkQueue = new SceneUtil::WorkQueue(numThreads);
+    mUnrefQueue = std::make_unique<SceneUtil::UnrefQueue>();
 
     mScreenCaptureOperation = new SceneUtil::AsyncScreenCaptureOperation(
         mWorkQueue,
@@ -842,7 +852,7 @@ void OMW::Engine::prepareEngine()
     }
 
     // Create the world
-    mWorld = std::make_unique<MWWorld::World>(mViewer, rootNode, mResourceSystem.get(), mWorkQueue.get(),
+    mWorld = std::make_unique<MWWorld::World>(mViewer, rootNode, mResourceSystem.get(), mWorkQueue.get(), *mUnrefQueue,
         mFileCollections, mContentFiles, mGroundcoverFiles, mEncoder.get(), mActivationDistanceOverride, mCellName,
         mStartupScript, mResDir.string(), mCfgMgr.getUserDataPath().string());
     mWorld->setupPlayer();

apps/openmw/engine.hpp

@@ -22,6 +22,7 @@ namespace SceneUtil
 {
     class WorkQueue;
     class AsyncScreenCaptureOperation;
+    class UnrefQueue;
 }
 
 namespace VFS
@@ -120,6 +121,7 @@ namespace OMW
             std::unique_ptr<VFS::Manager> mVFS;
             std::unique_ptr<Resource::ResourceSystem> mResourceSystem;
             osg::ref_ptr<SceneUtil::WorkQueue> mWorkQueue;
+            std::unique_ptr<SceneUtil::UnrefQueue> mUnrefQueue;
             std::unique_ptr<MWWorld::World> mWorld;
             std::unique_ptr<MWSound::SoundManager> mSoundManager;
             std::unique_ptr<MWScript::ScriptManager> mScriptManager;

apps/openmw/mwrender/actoranimation.cpp

@@ -58,16 +58,7 @@ ActorAnimation::ActorAnimation(const MWWorld::Ptr& ptr, osg::ref_ptr<osg::Group>
     removeEffects();
 }
 
-ActorAnimation::~ActorAnimation()
+ActorAnimation::~ActorAnimation() = default;
-{
-    for (ItemLightMap::iterator iter = mItemLights.begin(); iter != mItemLights.end(); ++iter)
-    {
-        mInsert->removeChild(iter->second);
-    }
-
-    mScabbard.reset();
-    mHolsteredShield.reset();
-}
 
 PartHolderPtr ActorAnimation::attachMesh(const std::string& model, const std::string& bonename, bool enchantedGlow, osg::Vec4f* glowColor)
 {
@@ -597,4 +588,11 @@ void ActorAnimation::removeHiddenItemLight(const MWWorld::ConstPtr& item)
     mItemLights.erase(iter);
 }
 
+void ActorAnimation::removeFromScene()
+{
+    for (const auto& [k, v] : mItemLights)
+        mInsert->removeChild(v);
+    Animation::removeFromScene();
+}
+
 }

apps/openmw/mwrender/actoranimation.hpp

@@ -40,6 +40,8 @@ class ActorAnimation : public Animation, public MWWorld::ContainerStoreListener
         bool useShieldAnimations() const override;
         bool updateCarriedLeftVisible(const int weaptype) const override;
 
+        void removeFromScene() override;
+
     protected:
         osg::Group* getBoneByName(const std::string& boneName) const;
         virtual void updateHolsteredWeapon(bool showHolsteredWeapons);

apps/openmw/mwrender/animation.cpp

@@ -527,23 +527,7 @@ namespace MWRender
         mLightListCallback = new SceneUtil::LightListCallback;
     }
 
-    Animation::~Animation()
+    Animation::~Animation() = default;
-    {
-        Animation::setLightEffect(0.f);
-
-        if (mObjectRoot)
-            mInsert->removeChild(mObjectRoot);
-    }
-
-    MWWorld::ConstPtr Animation::getPtr() const
-    {
-        return mPtr;
-    }
-
-    MWWorld::Ptr Animation::getPtr()
-    {
-        return mPtr;
-    }
 
     void Animation::setActive(int active)
     {
@@ -1779,6 +1763,15 @@ namespace MWRender
         return mHeadYawRadians;
     }
 
+    void Animation::removeFromScene()
+    {
+        if (mGlowLight != nullptr)
+            mInsert->removeChild(mGlowLight);
+
+        if (mObjectRoot != nullptr)
+            mInsert->removeChild(mObjectRoot);
+    }
+
     // ------------------------------------------------------
 
     float Animation::AnimationTime::getValue(osg::NodeVisitor*)

apps/openmw/mwrender/animation.hpp

@@ -351,9 +351,9 @@ public:
     /// Must be thread safe
     virtual ~Animation();
 
-    MWWorld::ConstPtr getPtr() const;
+    MWWorld::ConstPtr getPtr() const { return mPtr; }
 
-    MWWorld::Ptr getPtr();
+    MWWorld::Ptr getPtr() { return mPtr; }
 
     /// Set active flag on the object skeleton, if one exists.
     /// @see SceneUtil::Skeleton::setActive
@@ -497,6 +497,8 @@ public:
     virtual void setAccurateAiming(bool enabled) {}
     virtual bool canBeHarvested() const { return false; }
 
+    virtual void removeFromScene();
+
 private:
     Animation(const Animation&);
     void operator=(Animation&);

apps/openmw/mwrender/objects.cpp

@@ -4,6 +4,7 @@
 #include <osg/UserDataContainer>
 
 #include <components/sceneutil/positionattitudetransform.hpp>
+#include <components/sceneutil/unrefqueue.hpp>
 
 #include "../mwworld/ptr.hpp"
 #include "../mwworld/class.hpp"
@@ -17,9 +18,11 @@
 namespace MWRender
 {
 
-Objects::Objects(Resource::ResourceSystem* resourceSystem, osg::ref_ptr<osg::Group> rootNode)
+Objects::Objects(Resource::ResourceSystem* resourceSystem, const osg::ref_ptr<osg::Group>& rootNode,
+    SceneUtil::UnrefQueue& unrefQueue)
     : mRootNode(rootNode)
     , mResourceSystem(resourceSystem)
+    , mUnrefQueue(unrefQueue)
 {
 }
 
@@ -115,6 +118,8 @@ bool Objects::removeObject (const MWWorld::Ptr& ptr)
     const auto iter = mObjects.find(ptr.mRef);
     if(iter != mObjects.end())
     {
+        iter->second->removeFromScene();
+        mUnrefQueue.push(std::move(iter->second));
         mObjects.erase(iter);
 
         if (ptr.getClass().isActor())
@@ -148,7 +153,9 @@ void Objects::removeCell(const MWWorld::CellStore* store)
                 ptr.getClass().getContainerStore(ptr).setContListener(nullptr);
             }
 
-            mObjects.erase(iter++);
+            iter->second->removeFromScene();
+            mUnrefQueue.push(std::move(iter->second));
+            iter = mObjects.erase(iter);
         }
         else
             ++iter;

apps/openmw/mwrender/objects.hpp

@@ -24,6 +24,11 @@ namespace MWWorld
     class CellStore;
 }
 
+namespace SceneUtil
+{
+    class UnrefQueue;
+}
+
 namespace MWRender{
 
 class Animation;
@@ -57,15 +62,15 @@ class Objects
     typedef std::map<const MWWorld::CellStore*, osg::ref_ptr<osg::Group> > CellMap;
     CellMap mCellSceneNodes;
     PtrAnimationMap mObjects;
-
     osg::ref_ptr<osg::Group> mRootNode;
-
     Resource::ResourceSystem* mResourceSystem;
+    SceneUtil::UnrefQueue& mUnrefQueue;
 
     void insertBegin(const MWWorld::Ptr& ptr);
 
 public:
-    Objects(Resource::ResourceSystem* resourceSystem, osg::ref_ptr<osg::Group> rootNode);
+    Objects(Resource::ResourceSystem* resourceSystem, const osg::ref_ptr<osg::Group>& rootNode,
+        SceneUtil::UnrefQueue& unrefQueue);
     ~Objects();
 
     /// @param animated Attempt to load separate keyframes from a .kf file matching the model file?

apps/openmw/mwrender/renderingmanager.cpp

@@ -367,8 +367,9 @@ namespace MWRender
     };
 
     RenderingManager::RenderingManager(osgViewer::Viewer* viewer, osg::ref_ptr<osg::Group> rootNode,
-                                       Resource::ResourceSystem* resourceSystem, SceneUtil::WorkQueue* workQueue,
+        Resource::ResourceSystem* resourceSystem, SceneUtil::WorkQueue* workQueue, const std::string& resourcePath,
-                                       const std::string& resourcePath, DetourNavigator::Navigator& navigator, const MWWorld::GroundcoverStore& groundcoverStore)
+        DetourNavigator::Navigator& navigator, const MWWorld::GroundcoverStore& groundcoverStore,
+        SceneUtil::UnrefQueue& unrefQueue)
         : mSkyBlending(Settings::Manager::getBool("sky blending", "Fog"))
         , mViewer(viewer)
         , mRootNode(rootNode)
@@ -479,7 +480,7 @@ namespace MWRender
         mRecastMesh = std::make_unique<RecastMesh>(mRootNode, Settings::Manager::getBool("enable recast mesh render", "Navigator"));
         mPathgrid = std::make_unique<Pathgrid>(mRootNode);
 
-        mObjects = std::make_unique<Objects>(mResourceSystem, sceneRoot);
+        mObjects = std::make_unique<Objects>(mResourceSystem, sceneRoot, unrefQueue);
 
         if (getenv("OPENMW_DONT_PRECOMPILE") == nullptr)
         {

apps/openmw/mwrender/renderingmanager.hpp

@@ -60,6 +60,7 @@ namespace SceneUtil
     class ShadowManager;
     class WorkQueue;
     class LightManager;
+    class UnrefQueue;
 }
 
 namespace DetourNavigator
@@ -101,8 +102,9 @@ namespace MWRender
     {
     public:
         RenderingManager(osgViewer::Viewer* viewer, osg::ref_ptr<osg::Group> rootNode,
-                         Resource::ResourceSystem* resourceSystem, SceneUtil::WorkQueue* workQueue,
+            Resource::ResourceSystem* resourceSystem, SceneUtil::WorkQueue* workQueue, const std::string& resourcePath,
-                         const std::string& resourcePath, DetourNavigator::Navigator& navigator, const MWWorld::GroundcoverStore& groundcoverStore);
+            DetourNavigator::Navigator& navigator, const MWWorld::GroundcoverStore& groundcoverStore,
+            SceneUtil::UnrefQueue& unrefQueue);
         ~RenderingManager();
 
         osgUtil::IncrementalCompileOperation* getIncrementalCompileOperation();

apps/openmw/mwworld/worldimp.cpp

@@ -28,6 +28,7 @@
 
 #include <components/sceneutil/positionattitudetransform.hpp>
 #include <components/sceneutil/lightmanager.hpp>
+#include <components/sceneutil/workqueue.hpp>
 
 #include <components/detournavigator/navigator.hpp>
 #include <components/detournavigator/settings.hpp>
@@ -139,6 +140,7 @@ namespace MWWorld
         osgViewer::Viewer* viewer,
         osg::ref_ptr<osg::Group> rootNode,
         Resource::ResourceSystem* resourceSystem, SceneUtil::WorkQueue* workQueue,
+        SceneUtil::UnrefQueue& unrefQueue,
         const Files::Collections& fileCollections,
         const std::vector<std::string>& contentFiles,
         const std::vector<std::string>& groundcoverFiles,
@@ -187,7 +189,8 @@ namespace MWWorld
             mNavigator = DetourNavigator::makeNavigatorStub();
         }
 
-        mRendering = std::make_unique<MWRender::RenderingManager>(viewer, rootNode, resourceSystem, workQueue, resourcePath, *mNavigator, mGroundcoverStore);
+        mRendering = std::make_unique<MWRender::RenderingManager>(viewer, rootNode, resourceSystem, workQueue,
+            resourcePath, *mNavigator, mGroundcoverStore, unrefQueue);
         mProjectileManager = std::make_unique<ProjectileManager>(mRendering->getLightRoot()->asGroup(), resourceSystem, mRendering.get(), mPhysics.get());
         mRendering->preloadCommonAssets();
 

apps/openmw/mwworld/worldimp.hpp

@@ -38,6 +38,7 @@ namespace Resource
 namespace SceneUtil
 {
     class WorkQueue;
+    class UnrefQueue;
 }
 
 namespace ESM
@@ -192,6 +193,7 @@ namespace MWWorld
                 osgViewer::Viewer* viewer,
                 osg::ref_ptr<osg::Group> rootNode,
                 Resource::ResourceSystem* resourceSystem, SceneUtil::WorkQueue* workQueue,
+                SceneUtil::UnrefQueue& unrefQueue,
                 const Files::Collections& fileCollections,
                 const std::vector<std::string>& contentFiles,
                 const std::vector<std::string>& groundcoverFiles,

components/CMakeLists.txt

@@ -61,7 +61,7 @@ add_component_dir (sceneutil
     clone attach visitor util statesetupdater controller skeleton riggeometry morphgeometry lightcontroller
     lightmanager lightutil positionattitudetransform workqueue pathgridutil waterutil writescene serialize optimizer
     actorutil detourdebugdraw navmesh agentpath shadow mwshadowtechnique recastmesh shadowsbin osgacontroller rtt
-    screencapture depth color riggeometryosgaextension extradata
+    screencapture depth color riggeometryosgaextension extradata unrefqueue
     )
 
 add_component_dir (nif

components/resource/stats.cpp

@@ -417,6 +417,7 @@ void StatsHandler::setUpScene(osgViewer::ViewerBase *viewer)
             "Compiling",
             "WorkQueue",
             "WorkThread",
+            "UnrefQueue",
             "",
             "Texture",
             "StateSet",

components/sceneutil/unrefqueue.cpp

@@ -0,0 +1,28 @@
+#include "unrefqueue.hpp"
+
+namespace SceneUtil
+{
+    namespace
+    {
+        struct ClearVector final : SceneUtil::WorkItem
+        {
+            std::vector<osg::ref_ptr<osg::Referenced>> mObjects;
+
+            explicit ClearVector(std::vector<osg::ref_ptr<osg::Referenced>>&& objects)
+                : mObjects(std::move(objects)) {}
+
+            void doWork() override { mObjects.clear(); }
+        };
+    }
+
+    void UnrefQueue::flush(SceneUtil::WorkQueue& workQueue)
+    {
+        if (mObjects.empty())
+            return;
+
+        // Move only objects to keep allocated storage in mObjects
+        workQueue.addWorkItem(new ClearVector(std::vector<osg::ref_ptr<osg::Referenced>>(
+            std::move_iterator(mObjects.begin()), std::move_iterator(mObjects.end()))));
+        mObjects.clear();
+    }
+}

components/sceneutil/unrefqueue.hpp

@@ -0,0 +1,36 @@
+#ifndef OPENMW_COMPONENTS_UNREFQUEUE_H
+#define OPENMW_COMPONENTS_UNREFQUEUE_H
+
+#include "workqueue.hpp"
+
+#include <osg/ref_ptr>
+#include <osg/Referenced>
+
+#include <vector>
+
+namespace SceneUtil
+{
+    class WorkQueue;
+
+    /// @brief Handles unreferencing of objects through the WorkQueue. Typical use scenario
+    /// would be the main thread pushing objects that are no longer needed, and the background thread deleting them.
+    class UnrefQueue
+    {
+    public:
+        /// Adds an object to the list of objects to be unreferenced. Call from the main thread.
+        void push(osg::ref_ptr<osg::Referenced>&& obj) { mObjects.push_back(std::move(obj)); }
+
+        void push(const osg::ref_ptr<osg::Referenced>& obj) { mObjects.push_back(obj); }
+
+        /// Adds a WorkItem to the given WorkQueue that will clear the list of objects in a worker thread,
+        /// thus unreferencing them. Call from the main thread.
+        void flush(SceneUtil::WorkQueue& workQueue);
+
+        std::size_t getSize() const { return mObjects.size(); }
+
+    private:
+        std::vector<osg::ref_ptr<osg::Referenced>> mObjects;
+    };
+}
+
+#endif