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

@@ -427,7 +427,8 @@ OMW::Engine::Engine(Files::ConfigurationManager& configurationManager)
 
 OMW::Engine::~Engine()
 {
-    mWorkQueue->stop();
+    if (mScreenCaptureOperation != nullptr)
+        mScreenCaptureOperation->stop();
 
     mEnvironment.cleanup();
 

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<osgViewer::Viewer> mViewer;
             osg::ref_ptr<osgViewer::ScreenCaptureHandler> mScreenCaptureHandler;
-            osg::ref_ptr<osgViewer::ScreenCaptureHandler::CaptureOperation> mScreenCaptureOperation;
+            osg::ref_ptr<SceneUtil::AsyncScreenCaptureOperation> mScreenCaptureOperation;
             std::string mCellName;
             std::vector<std::string> mContentFiles;
             std::vector<std::string> mGroundcoverFiles;

apps/openmw/mwworld/scene.cpp

@@ -3,6 +3,7 @@
 #include <limits>
 #include <chrono>
 #include <thread>
+#include <atomic>
 
 #include <BulletCollision/CollisionDispatch/btCollisionObject.h>
 #include <BulletCollision/CollisionShapes/btCompoundShape.h>
@@ -867,6 +868,11 @@ namespace MWWorld
 
     Scene::~Scene()
     {
+        for (const osg::ref_ptr<SceneUtil::WorkItem>& v : mWorkItems)
+            v->abort();
+
+        for (const osg::ref_ptr<SceneUtil::WorkItem>& 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<PreloadMeshItem> item(new PreloadMeshItem(mesh_, mRendering.getResourceSystem()->getSceneManager()));
+            mRendering.getWorkQueue()->addWorkItem(item);
+            const auto isDone = [] (const osg::ref_ptr<SceneUtil::WorkItem>& 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)

apps/openmw/mwworld/scene.hpp

@@ -3,6 +3,7 @@
 
 #include <osg/Vec4i>
 #include <osg/Vec2i>
+#include <osg/ref_ptr>
 
 #include "ptr.hpp"
 #include "globals.hpp"
@@ -10,6 +11,7 @@
 #include <set>
 #include <memory>
 #include <unordered_map>
+#include <vector>
 
 #include <components/misc/constants.hpp>
 
@@ -49,6 +51,11 @@ namespace MWPhysics
     class PhysicsSystem;
 }
 
+namespace SceneUtil
+{
+    class WorkItem;
+}
+
 namespace MWWorld
 {
     class Player;
@@ -91,6 +98,8 @@ namespace MWWorld
 
             std::set<ESM::RefNum> mPagedRefs;
 
+            std::vector<osg::ref_ptr<SceneUtil::WorkItem>> mWorkItems;
+
             void insertCell (CellStore &cell, Loading::Listener* loadingListener, bool onlyObjects, bool test = false);
             osg::Vec2i mCurrentGridCenter;
 

components/sceneutil/screencapture.cpp

@@ -15,6 +15,7 @@
 #include <iomanip>
 #include <sstream>
 #include <string>
+#include <atomic>
 
 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<osgViewer::ScreenCaptureHandler::CaptureOperation> mImpl;
             const osg::ref_ptr<const osg::Image> 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<SceneUtil::WorkItem>& item : *mWorkItems.lockConst())
+            item->abort();
+
+        for (const osg::ref_ptr<SceneUtil::WorkItem>& 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<SceneUtil::WorkItem> item(new ScreenCaptureWorkItem(mImpl, image, context_id));
+        mQueue->addWorkItem(item);
+        const auto isDone = [] (const osg::ref_ptr<SceneUtil::WorkItem>& 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));
     }
 }

components/sceneutil/screencapture.hpp

@@ -1,10 +1,13 @@
 #ifndef OPENMW_COMPONENTS_SCENEUTIL_SCREENCAPTURE_H
 #define OPENMW_COMPONENTS_SCENEUTIL_SCREENCAPTURE_H
 
+#include <components/misc/guarded.hpp>
+
 #include <osg/ref_ptr>
 #include <osgViewer/ViewerEventHandlers>
 
 #include <string>
+#include <vector>
 
 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<SceneUtil::WorkQueue> queue,
                                         osg::ref_ptr<osgViewer::ScreenCaptureHandler::CaptureOperation> impl);
 
+            ~AsyncScreenCaptureOperation();
+
+            void stop();
+
             void operator()(const osg::Image& image, const unsigned int context_id) override;
 
         private:
             const osg::ref_ptr<SceneUtil::WorkQueue> mQueue;
             const osg::ref_ptr<osgViewer::ScreenCaptureHandler::CaptureOperation> mImpl;
+            Misc::ScopeGuarded<std::vector<osg::ref_ptr<SceneUtil::WorkItem>>> mWorkItems;
     };
 }