From b8fcd6d3ba0a3a91879ae7a98b22f75c852433c1 Mon Sep 17 00:00:00 2001 From: elsid Date: Sun, 11 Jul 2021 14:43:52 +0200 Subject: [PATCH] Manage work item lifetime on the client side Instead of explicit work queue stop before any possibly used engine manager is destructed. Based on an assumption that any engine manager can be destructed independently from the work queue destruction. This model is already used in CellPreloader that conflicts with explicit work queue stop. After the work queue is requested to be stopped, any client waiting for a not started work item to be done will wait forever because the work item is dropped from the queue. Therefore either clients should not wait for own work items to be completed in destructor or the work queue should not drop items before clients are destructed. Other approaches are possible but are not considered due to increasing complexity. CellPreloader already tries to wait for all created work items to be done so keep it that way and extend the model to AsyncScreenCaptureOperation and Scene. Additionally abort all scheduled work items when owner is destructed. This prevents a long exit when multiple screenshots are scheduled right before exiting the game. --- apps/openmw/engine.cpp | 3 ++- apps/openmw/engine.hpp | 3 ++- apps/openmw/mwworld/scene.cpp | 24 +++++++++++++++++++- apps/openmw/mwworld/scene.hpp | 9 ++++++++ components/sceneutil/screencapture.cpp | 31 +++++++++++++++++++++++++- components/sceneutil/screencapture.hpp | 9 ++++++++ 6 files changed, 75 insertions(+), 4 deletions(-) diff --git a/apps/openmw/engine.cpp b/apps/openmw/engine.cpp index f554821b3d..b0047fd1df 100644 --- a/apps/openmw/engine.cpp +++ b/apps/openmw/engine.cpp @@ -427,7 +427,8 @@ OMW::Engine::Engine(Files::ConfigurationManager& configurationManager) OMW::Engine::~Engine() { - mWorkQueue->stop(); + if (mScreenCaptureOperation != nullptr) + mScreenCaptureOperation->stop(); mEnvironment.cleanup(); diff --git a/apps/openmw/engine.hpp b/apps/openmw/engine.hpp index 180e06bcbc..290fd890a6 100644 --- a/apps/openmw/engine.hpp +++ b/apps/openmw/engine.hpp @@ -21,6 +21,7 @@ namespace Resource namespace SceneUtil { class WorkQueue; + class AsyncScreenCaptureOperation; } namespace VFS @@ -67,7 +68,7 @@ namespace OMW boost::filesystem::path mResDir; osg::ref_ptr mViewer; osg::ref_ptr mScreenCaptureHandler; - osg::ref_ptr mScreenCaptureOperation; + osg::ref_ptr mScreenCaptureOperation; std::string mCellName; std::vector mContentFiles; std::vector mGroundcoverFiles; diff --git a/apps/openmw/mwworld/scene.cpp b/apps/openmw/mwworld/scene.cpp index 7d3a6c7893..c43ee76d1b 100644 --- a/apps/openmw/mwworld/scene.cpp +++ b/apps/openmw/mwworld/scene.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #include #include @@ -867,6 +868,11 @@ namespace MWWorld Scene::~Scene() { + for (const osg::ref_ptr& v : mWorkItems) + v->abort(); + + for (const osg::ref_ptr& v : mWorkItems) + v->waitTillDone(); } bool Scene::hasCellChanged() const @@ -1061,6 +1067,9 @@ namespace MWWorld void doWork() override { + if (mAborted) + return; + try { mSceneManager->getTemplate(mMesh); @@ -1069,9 +1078,16 @@ namespace MWWorld { } } + + void abort() override + { + mAborted = true; + } + private: std::string mMesh; Resource::SceneManager* mSceneManager; + std::atomic_bool mAborted {false}; }; void Scene::preload(const std::string &mesh, bool useAnim) @@ -1081,7 +1097,13 @@ namespace MWWorld mesh_ = Misc::ResourceHelpers::correctActorModelPath(mesh_, mRendering.getResourceSystem()->getVFS()); if (!mRendering.getResourceSystem()->getSceneManager()->checkLoaded(mesh_, mRendering.getReferenceTime())) - mRendering.getWorkQueue()->addWorkItem(new PreloadMeshItem(mesh_, mRendering.getResourceSystem()->getSceneManager())); + { + osg::ref_ptr item(new PreloadMeshItem(mesh_, mRendering.getResourceSystem()->getSceneManager())); + mRendering.getWorkQueue()->addWorkItem(item); + const auto isDone = [] (const osg::ref_ptr& v) { return v->isDone(); }; + mWorkItems.erase(std::remove_if(mWorkItems.begin(), mWorkItems.end(), isDone), mWorkItems.end()); + mWorkItems.emplace_back(std::move(item)); + } } void Scene::preloadCells(float dt) diff --git a/apps/openmw/mwworld/scene.hpp b/apps/openmw/mwworld/scene.hpp index bc9c2386bb..75c070dd1e 100644 --- a/apps/openmw/mwworld/scene.hpp +++ b/apps/openmw/mwworld/scene.hpp @@ -3,6 +3,7 @@ #include #include +#include #include "ptr.hpp" #include "globals.hpp" @@ -10,6 +11,7 @@ #include #include #include +#include #include @@ -49,6 +51,11 @@ namespace MWPhysics class PhysicsSystem; } +namespace SceneUtil +{ + class WorkItem; +} + namespace MWWorld { class Player; @@ -91,6 +98,8 @@ namespace MWWorld std::set mPagedRefs; + std::vector> mWorkItems; + void insertCell (CellStore &cell, Loading::Listener* loadingListener, bool onlyObjects, bool test = false); osg::Vec2i mCurrentGridCenter; diff --git a/components/sceneutil/screencapture.cpp b/components/sceneutil/screencapture.cpp index 47119f3644..99cb535326 100644 --- a/components/sceneutil/screencapture.cpp +++ b/components/sceneutil/screencapture.cpp @@ -15,6 +15,7 @@ #include #include #include +#include namespace { @@ -32,6 +33,9 @@ namespace void doWork() override { + if (mAborted) + return; + try { (*mImpl)(*mImage, mContextId); @@ -42,10 +46,16 @@ namespace } } + void abort() override + { + mAborted = true; + } + private: const osg::ref_ptr mImpl; const osg::ref_ptr mImage; const unsigned int mContextId; + std::atomic_bool mAborted {false}; }; } @@ -130,8 +140,27 @@ namespace SceneUtil assert(mImpl != nullptr); } + AsyncScreenCaptureOperation::~AsyncScreenCaptureOperation() + { + stop(); + } + + void AsyncScreenCaptureOperation::stop() + { + for (const osg::ref_ptr& item : *mWorkItems.lockConst()) + item->abort(); + + for (const osg::ref_ptr& item : *mWorkItems.lockConst()) + item->waitTillDone(); + } + void AsyncScreenCaptureOperation::operator()(const osg::Image& image, const unsigned int context_id) { - mQueue->addWorkItem(new ScreenCaptureWorkItem(mImpl, image, context_id)); + osg::ref_ptr item(new ScreenCaptureWorkItem(mImpl, image, context_id)); + mQueue->addWorkItem(item); + const auto isDone = [] (const osg::ref_ptr& v) { return v->isDone(); }; + const auto workItems = mWorkItems.lock(); + workItems->erase(std::remove_if(workItems->begin(), workItems->end(), isDone), workItems->end()); + workItems->emplace_back(std::move(item)); } } diff --git a/components/sceneutil/screencapture.hpp b/components/sceneutil/screencapture.hpp index 6395d989d8..87e396b020 100644 --- a/components/sceneutil/screencapture.hpp +++ b/components/sceneutil/screencapture.hpp @@ -1,10 +1,13 @@ #ifndef OPENMW_COMPONENTS_SCENEUTIL_SCREENCAPTURE_H #define OPENMW_COMPONENTS_SCENEUTIL_SCREENCAPTURE_H +#include + #include #include #include +#include namespace osg { @@ -14,6 +17,7 @@ namespace osg namespace SceneUtil { class WorkQueue; + class WorkItem; std::string writeScreenshotToFile(const std::string& screenshotPath, const std::string& screenshotFormat, const osg::Image& image); @@ -38,11 +42,16 @@ namespace SceneUtil AsyncScreenCaptureOperation(osg::ref_ptr queue, osg::ref_ptr impl); + ~AsyncScreenCaptureOperation(); + + void stop(); + void operator()(const osg::Image& image, const unsigned int context_id) override; private: const osg::ref_ptr mQueue; const osg::ref_ptr mImpl; + Misc::ScopeGuarded>> mWorkItems; }; }