From b4976354a522f2f0aa20254d159204dbf8cc781a Mon Sep 17 00:00:00 2001 From: elsid Date: Wed, 24 Apr 2024 11:09:13 +0200 Subject: [PATCH 1/2] Use correct frame and stats for lua worker When a loading screen appears during the frame processing, the frame number returned by the viewer is incremented and the stats reporting goes into the wrong frame. Pass frame number and stats object from the main thread to avoid this. --- apps/openmw/engine.cpp | 7 ++++--- apps/openmw/mwlua/worker.cpp | 33 ++++++++++++++++----------------- apps/openmw/mwlua/worker.hpp | 25 +++++++++++++++++-------- 3 files changed, 37 insertions(+), 28 deletions(-) diff --git a/apps/openmw/engine.cpp b/apps/openmw/engine.cpp index 179dbcdc32a..26d8af83044 100644 --- a/apps/openmw/engine.cpp +++ b/apps/openmw/engine.cpp @@ -340,11 +340,12 @@ bool OMW::Engine::frame(float frametime) mWorld->updateWindowManager(); } - mLuaWorker->allowUpdate(); // if there is a separate Lua thread, it starts the update now + // if there is a separate Lua thread, it starts the update now + mLuaWorker->allowUpdate(frameStart, frameNumber, *stats); mViewer->renderingTraversals(); - mLuaWorker->finishUpdate(); + mLuaWorker->finishUpdate(frameStart, frameNumber, *stats); return true; } @@ -910,7 +911,7 @@ void OMW::Engine::prepareEngine() mLuaManager->init(); // starts a separate lua thread if "lua num threads" > 0 - mLuaWorker = std::make_unique(*mLuaManager, *mViewer); + mLuaWorker = std::make_unique(*mLuaManager); } // Initialise and enter main loop. diff --git a/apps/openmw/mwlua/worker.cpp b/apps/openmw/mwlua/worker.cpp index 193d340208b..5639cc89edf 100644 --- a/apps/openmw/mwlua/worker.cpp +++ b/apps/openmw/mwlua/worker.cpp @@ -7,13 +7,12 @@ #include #include -#include +#include namespace MWLua { - Worker::Worker(LuaManager& manager, osgViewer::Viewer& viewer) + Worker::Worker(LuaManager& manager) : mManager(manager) - , mViewer(viewer) { if (Settings::lua().mLuaNumThreads > 0) mThread = std::thread([this] { run(); }); @@ -29,26 +28,26 @@ namespace MWLua } } - void Worker::allowUpdate() + void Worker::allowUpdate(osg::Timer_t frameStart, unsigned frameNumber, osg::Stats& stats) { if (!mThread) return; { std::lock_guard lk(mMutex); - mUpdateRequest = true; + mUpdateRequest = UpdateRequest{ .mFrameStart = frameStart, .mFrameNumber = frameNumber, .mStats = &stats }; } mCV.notify_one(); } - void Worker::finishUpdate() + void Worker::finishUpdate(osg::Timer_t frameStart, unsigned frameNumber, osg::Stats& stats) { if (mThread) { std::unique_lock lk(mMutex); - mCV.wait(lk, [&] { return !mUpdateRequest; }); + mCV.wait(lk, [&] { return !mUpdateRequest.has_value(); }); } else - update(); + update(frameStart, frameNumber, stats); } void Worker::join() @@ -64,12 +63,10 @@ namespace MWLua } } - void Worker::update() + void Worker::update(osg::Timer_t frameStart, unsigned frameNumber, osg::Stats& stats) { - const osg::Timer_t frameStart = mViewer.getStartTick(); - const unsigned int frameNumber = mViewer.getFrameStamp()->getFrameNumber(); - OMW::ScopedProfile profile( - frameStart, frameNumber, *osg::Timer::instance(), *mViewer.getViewerStats()); + const osg::Timer* const timer = osg::Timer::instance(); + OMW::ScopedProfile profile(frameStart, frameNumber, *timer, stats); mManager.update(); } @@ -79,20 +76,22 @@ namespace MWLua while (true) { std::unique_lock lk(mMutex); - mCV.wait(lk, [&] { return mUpdateRequest || mJoinRequest; }); + mCV.wait(lk, [&] { return mUpdateRequest.has_value() || mJoinRequest; }); if (mJoinRequest) break; + assert(mUpdateRequest.has_value()); + try { - update(); + update(mUpdateRequest->mFrameStart, mUpdateRequest->mFrameNumber, *mUpdateRequest->mStats); } - catch (std::exception& e) + catch (const std::exception& e) { Log(Debug::Error) << "Failed to update LuaManager: " << e.what(); } - mUpdateRequest = false; + mUpdateRequest.reset(); lk.unlock(); mCV.notify_one(); } diff --git a/apps/openmw/mwlua/worker.hpp b/apps/openmw/mwlua/worker.hpp index fed625e1f13..58d69afe71a 100644 --- a/apps/openmw/mwlua/worker.hpp +++ b/apps/openmw/mwlua/worker.hpp @@ -1,14 +1,17 @@ #ifndef OPENMW_MWLUA_WORKER_H #define OPENMW_MWLUA_WORKER_H +#include +#include + #include #include #include #include -namespace osgViewer +namespace osg { - class Viewer; + class Stats; } namespace MWLua @@ -18,26 +21,32 @@ namespace MWLua class Worker { public: - explicit Worker(LuaManager& manager, osgViewer::Viewer& viewer); + explicit Worker(LuaManager& manager); ~Worker(); - void allowUpdate(); + void allowUpdate(osg::Timer_t frameStart, unsigned int frameNumber, osg::Stats& stats); - void finishUpdate(); + void finishUpdate(osg::Timer_t frameStart, unsigned int frameNumber, osg::Stats& stats); void join(); private: - void update(); + struct UpdateRequest + { + osg::Timer_t mFrameStart; + unsigned mFrameNumber; + osg::ref_ptr mStats; + }; + + void update(osg::Timer_t frameStart, unsigned frameNumber, osg::Stats& stats); void run() noexcept; LuaManager& mManager; - osgViewer::Viewer& mViewer; std::mutex mMutex; std::condition_variable mCV; - bool mUpdateRequest = false; + std::optional mUpdateRequest; bool mJoinRequest = false; std::optional mThread; }; From 41d5d3bf090189cfa0cd763732c61764782698f4 Mon Sep 17 00:00:00 2001 From: elsid Date: Wed, 24 Apr 2024 11:30:21 +0200 Subject: [PATCH 2/2] Report osg stats for all frames showing loading screens --- CHANGELOG.md | 1 + apps/openmw/engine.cpp | 30 ++++++++++++++++++++---------- apps/openmw/engine.hpp | 2 +- 3 files changed, 22 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e935364264e..b4fd840ace8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -145,6 +145,7 @@ Bug #7723: Assaulting vampires and werewolves shouldn't be a crime Bug #7724: Guards don't help vs werewolves Bug #7733: Launcher shows incorrect data paths when there's two plugins with the same name + Bug #7737: OSG stats are missing some data on loading screens Bug #7742: Governing attribute training limit should use the modified attribute Bug #7753: Editor: Actors Don't Scale According to Their Race Bug #7758: Water walking is not taken into account to compute path cost on the water diff --git a/apps/openmw/engine.cpp b/apps/openmw/engine.cpp index 26d8af83044..cf9c45f54ea 100644 --- a/apps/openmw/engine.cpp +++ b/apps/openmw/engine.cpp @@ -165,6 +165,15 @@ namespace private: int mMaxTextureImageUnits = 0; }; + + void reportStats(unsigned frameNumber, osgViewer::Viewer& viewer, std::ostream& stream) + { + viewer.getViewerStats()->report(stream, frameNumber); + osgViewer::Viewer::Cameras cameras; + viewer.getCameras(cameras); + for (osg::Camera* camera : cameras) + camera->getStats()->report(stream, frameNumber); + } } void OMW::Engine::executeLocalScripts() @@ -180,10 +189,9 @@ void OMW::Engine::executeLocalScripts() } } -bool OMW::Engine::frame(float frametime) +bool OMW::Engine::frame(unsigned frameNumber, float frametime) { const osg::Timer_t frameStart = mViewer->getStartTick(); - const unsigned int frameNumber = mViewer->getFrameStamp()->getFrameNumber(); const osg::Timer* const timer = osg::Timer::instance(); osg::Stats* const stats = mViewer->getViewerStats(); @@ -1021,7 +1029,9 @@ void OMW::Engine::go() mViewer->advance(timeManager.getRenderingSimulationTime()); - if (!frame(dt)) + const unsigned frameNumber = mViewer->getFrameStamp()->getFrameNumber(); + + if (!frame(frameNumber, dt)) { std::this_thread::sleep_for(std::chrono::milliseconds(5)); continue; @@ -1035,16 +1045,16 @@ void OMW::Engine::go() if (stats) { + // The delay is required because rendering happens in parallel to the main thread and stats from there is + // available with delay. constexpr unsigned statsReportDelay = 3; - const auto frameNumber = mViewer->getFrameStamp()->getFrameNumber(); if (frameNumber >= statsReportDelay) { - const unsigned reportFrameNumber = frameNumber - statsReportDelay; - mViewer->getViewerStats()->report(stats, reportFrameNumber); - osgViewer::Viewer::Cameras cameras; - mViewer->getCameras(cameras); - for (auto camera : cameras) - camera->getStats()->report(stats, reportFrameNumber); + // Viewer frame number can be different from frameNumber because of loading screens which render new + // frames inside a simulation frame. + const unsigned currentFrameNumber = mViewer->getFrameStamp()->getFrameNumber(); + for (unsigned i = frameNumber; i <= currentFrameNumber; ++i) + reportStats(i - statsReportDelay, *mViewer, stats); } } diff --git a/apps/openmw/engine.hpp b/apps/openmw/engine.hpp index 2cd224785b1..bf7bf7441b0 100644 --- a/apps/openmw/engine.hpp +++ b/apps/openmw/engine.hpp @@ -188,7 +188,7 @@ namespace OMW void executeLocalScripts(); - bool frame(float dt); + bool frame(unsigned frameNumber, float dt); /// Prepare engine for game play void prepareEngine();