diff --git a/CHANGELOG.md b/CHANGELOG.md index 81223fd270..fd757b631c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -70,6 +70,7 @@ Bug #7472: Crash when enchanting last projectiles Bug #7505: Distant terrain does not support sample size greater than cell size Bug #7553: Faction reaction loading is incorrect + Bug #7557: Terrain::ChunkManager::createChunk is called twice for the same position, lod on initial loading Feature #3537: Shader-based water ripples Feature #5492: Let rain and snow collide with statics Feature #6149: Dehardcode Lua API_REVISION diff --git a/apps/openmw/mwworld/scene.cpp b/apps/openmw/mwworld/scene.cpp index 78c8ccab1d..b5aff875bd 100644 --- a/apps/openmw/mwworld/scene.cpp +++ b/apps/openmw/mwworld/scene.cpp @@ -834,9 +834,6 @@ namespace MWWorld mPreloader = std::make_unique(rendering.getResourceSystem(), physics->getShapeManager(), rendering.getTerrain(), rendering.getLandManager()); mPreloader->setWorkQueue(mRendering.getWorkQueue()); - - rendering.getResourceSystem()->setExpiryDelay(Settings::cells().mCacheExpiryDelay); - mPreloader->setExpiryDelay(Settings::cells().mPreloadCellExpiryDelay); mPreloader->setMinCacheSize(Settings::cells().mPreloadCellCacheMin); mPreloader->setMaxCacheSize(Settings::cells().mPreloadCellCacheMax); diff --git a/components/resource/niffilemanager.cpp b/components/resource/niffilemanager.cpp index ea95edf2ae..5e457cdfaa 100644 --- a/components/resource/niffilemanager.cpp +++ b/components/resource/niffilemanager.cpp @@ -32,7 +32,9 @@ namespace Resource }; NifFileManager::NifFileManager(const VFS::Manager* vfs) - : ResourceManager(vfs) + // NIF files aren't needed any more once the converted objects are cached in SceneManager / BulletShapeManager, + // so no point in using an expiry delay. + : ResourceManager(vfs, 0) { } diff --git a/components/resource/objectcache.hpp b/components/resource/objectcache.hpp index 3847129ed3..881729ffc4 100644 --- a/components/resource/objectcache.hpp +++ b/components/resource/objectcache.hpp @@ -62,9 +62,9 @@ namespace Resource { // If ref count is greater than 1, the object has an external reference. // If the timestamp is yet to be initialized, it needs to be updated too. - if ((itr->second.first != nullptr && itr->second.first->referenceCount() > 1) - || itr->second.second == 0.0) - itr->second.second = referenceTime; + if ((itr->second.mValue != nullptr && itr->second.mValue->referenceCount() > 1) + || itr->second.mLastUsage == 0.0) + itr->second.mLastUsage = referenceTime; } } @@ -81,10 +81,10 @@ namespace Resource typename ObjectCacheMap::iterator oitr = _objectCache.begin(); while (oitr != _objectCache.end()) { - if (oitr->second.second <= expiryTime) + if (oitr->second.mLastUsage <= expiryTime) { - if (oitr->second.first != nullptr) - objectsToRemove.push_back(oitr->second.first); + if (oitr->second.mValue != nullptr) + objectsToRemove.push_back(std::move(oitr->second.mValue)); _objectCache.erase(oitr++); } else @@ -106,7 +106,7 @@ namespace Resource void addEntryToObjectCache(const KeyType& key, osg::Object* object, double timestamp = 0.0) { std::lock_guard lock(_objectCacheMutex); - _objectCache[key] = ObjectTimeStampPair(object, timestamp); + _objectCache[key] = Item{ object, timestamp }; } /** Remove Object from cache.*/ @@ -124,7 +124,7 @@ namespace Resource std::lock_guard lock(_objectCacheMutex); typename ObjectCacheMap::iterator itr = _objectCache.find(key); if (itr != _objectCache.end()) - return itr->second.first; + return itr->second.mValue; else return nullptr; } @@ -135,7 +135,7 @@ namespace Resource const auto it = _objectCache.find(key); if (it == _objectCache.end()) return std::nullopt; - return it->second.first; + return it->second.mValue; } /** Check if an object is in the cache, and if it is, update its usage time stamp. */ @@ -145,7 +145,7 @@ namespace Resource typename ObjectCacheMap::iterator itr = _objectCache.find(key); if (itr != _objectCache.end()) { - itr->second.second = timeStamp; + itr->second.mLastUsage = timeStamp; return true; } else @@ -158,7 +158,7 @@ namespace Resource std::lock_guard lock(_objectCacheMutex); for (typename ObjectCacheMap::iterator itr = _objectCache.begin(); itr != _objectCache.end(); ++itr) { - osg::Object* object = itr->second.first.get(); + osg::Object* object = itr->second.mValue.get(); object->releaseGLObjects(state); } } @@ -169,8 +169,7 @@ namespace Resource std::lock_guard lock(_objectCacheMutex); for (typename ObjectCacheMap::iterator itr = _objectCache.begin(); itr != _objectCache.end(); ++itr) { - osg::Object* object = itr->second.first.get(); - if (object) + if (osg::Object* object = itr->second.mValue.get()) { osg::Node* node = dynamic_cast(object); if (node) @@ -185,7 +184,7 @@ namespace Resource { std::lock_guard lock(_objectCacheMutex); for (typename ObjectCacheMap::iterator it = _objectCache.begin(); it != _objectCache.end(); ++it) - f(it->first, it->second.first.get()); + f(it->first, it->second.mValue.get()); } /** Get the number of objects in the cache. */ @@ -195,11 +194,26 @@ namespace Resource return _objectCache.size(); } + template + std::optional>> lowerBound(K&& key) + { + const std::lock_guard lock(_objectCacheMutex); + const auto it = _objectCache.lower_bound(std::forward(key)); + if (it == _objectCache.end()) + return std::nullopt; + return std::pair(it->first, it->second.mValue); + } + protected: + struct Item + { + osg::ref_ptr mValue; + double mLastUsage; + }; + virtual ~GenericObjectCache() {} - typedef std::pair, double> ObjectTimeStampPair; - typedef std::map ObjectCacheMap; + using ObjectCacheMap = std::map>; ObjectCacheMap _objectCache; mutable std::mutex _objectCacheMutex; diff --git a/components/resource/resourcemanager.hpp b/components/resource/resourcemanager.hpp index fc5ecc7f03..09b99faf28 100644 --- a/components/resource/resourcemanager.hpp +++ b/components/resource/resourcemanager.hpp @@ -3,6 +3,8 @@ #include +#include + #include "objectcache.hpp" namespace VFS @@ -23,11 +25,11 @@ namespace Resource { public: virtual ~BaseResourceManager() = default; - virtual void updateCache(double referenceTime) {} - virtual void clearCache() {} - virtual void setExpiryDelay(double expiryDelay) {} - virtual void reportStats(unsigned int frameNumber, osg::Stats* stats) const {} - virtual void releaseGLObjects(osg::State* state) {} + virtual void updateCache(double referenceTime) = 0; + virtual void clearCache() = 0; + virtual void setExpiryDelay(double expiryDelay) = 0; + virtual void reportStats(unsigned int frameNumber, osg::Stats* stats) const = 0; + virtual void releaseGLObjects(osg::State* state) = 0; }; /// @brief Base class for managers that require a virtual file system and object cache. @@ -39,10 +41,11 @@ namespace Resource public: typedef GenericObjectCache CacheType; - GenericResourceManager(const VFS::Manager* vfs) + explicit GenericResourceManager( + const VFS::Manager* vfs, double expiryDelay = Settings::cells().mCacheExpiryDelay) : mVFS(vfs) , mCache(new CacheType) - , mExpiryDelay(0.0) + , mExpiryDelay(expiryDelay) { } @@ -59,7 +62,7 @@ namespace Resource void clearCache() override { mCache->clear(); } /// How long to keep objects in cache after no longer being referenced. - void setExpiryDelay(double expiryDelay) override { mExpiryDelay = expiryDelay; } + void setExpiryDelay(double expiryDelay) final { mExpiryDelay = expiryDelay; } double getExpiryDelay() const { return mExpiryDelay; } const VFS::Manager* getVFS() const { return mVFS; } @@ -77,10 +80,15 @@ namespace Resource class ResourceManager : public GenericResourceManager { public: - ResourceManager(const VFS::Manager* vfs) + explicit ResourceManager(const VFS::Manager* vfs) : GenericResourceManager(vfs) { } + + explicit ResourceManager(const VFS::Manager* vfs, double expiryDelay) + : GenericResourceManager(vfs, expiryDelay) + { + } }; } diff --git a/components/terrain/chunkmanager.cpp b/components/terrain/chunkmanager.cpp index 0364a0a4fd..6273041f72 100644 --- a/components/terrain/chunkmanager.cpp +++ b/components/terrain/chunkmanager.cpp @@ -21,7 +21,7 @@ namespace Terrain ChunkManager::ChunkManager(Storage* storage, Resource::SceneManager* sceneMgr, TextureManager* textureManager, CompositeMapRenderer* renderer, ESM::RefId worldspace) - : GenericResourceManager(nullptr) + : GenericResourceManager(nullptr) , QuadTreeWorld::ChunkManager(worldspace) , mStorage(storage) , mSceneManager(sceneMgr) @@ -39,38 +39,26 @@ namespace Terrain mMultiPassRoot->setAttributeAndModes(material, osg::StateAttribute::ON); } - struct FindChunkTemplate - { - void operator()(ChunkId id, osg::Object* obj) - { - if (std::get<0>(id) == std::get<0>(mId) && std::get<1>(id) == std::get<1>(mId)) - mFoundTemplate = obj; - } - ChunkId mId; - osg::ref_ptr mFoundTemplate; - }; - osg::ref_ptr ChunkManager::getChunk(float size, const osg::Vec2f& center, unsigned char lod, unsigned int lodFlags, bool activeGrid, const osg::Vec3f& viewPoint, bool compile) { // Override lod with the vertexLodMod adjusted value. // TODO: maybe we can refactor this code by moving all vertexLodMod code into this class. lod = static_cast(lodFlags >> (4 * 4)); - ChunkId id = std::make_tuple(center, lod, lodFlags); - osg::ref_ptr obj = mCache->getRefFromObjectCache(id); - if (obj) + + const ChunkKey key{ .mCenter = center, .mLod = lod, .mLodFlags = lodFlags }; + if (osg::ref_ptr obj = mCache->getRefFromObjectCache(key)) return static_cast(obj.get()); - else - { - FindChunkTemplate find; - find.mId = id; - mCache->call(find); - TerrainDrawable* templateGeometry - = find.mFoundTemplate ? static_cast(find.mFoundTemplate.get()) : nullptr; - osg::ref_ptr node = createChunk(size, center, lod, lodFlags, compile, templateGeometry); - mCache->addEntryToObjectCache(id, node.get()); - return node; - } + + const TerrainDrawable* templateGeometry = nullptr; + const TemplateKey templateKey{ .mCenter = center, .mLod = lod }; + const auto pair = mCache->lowerBound(templateKey); + if (pair.has_value() && templateKey == TemplateKey{ .mCenter = pair->first.mCenter, .mLod = pair->first.mLod }) + templateGeometry = static_cast(pair->second.get()); + + osg::ref_ptr node = createChunk(size, center, lod, lodFlags, compile, templateGeometry); + mCache->addEntryToObjectCache(key, node.get()); + return node; } void ChunkManager::reportStats(unsigned int frameNumber, osg::Stats* stats) const @@ -80,14 +68,14 @@ namespace Terrain void ChunkManager::clearCache() { - GenericResourceManager::clearCache(); + GenericResourceManager::clearCache(); mBufferCache.clearCache(); } void ChunkManager::releaseGLObjects(osg::State* state) { - GenericResourceManager::releaseGLObjects(state); + GenericResourceManager::releaseGLObjects(state); mBufferCache.releaseGLObjects(state); } @@ -202,7 +190,7 @@ namespace Terrain } osg::ref_ptr ChunkManager::createChunk(float chunkSize, const osg::Vec2f& chunkCenter, unsigned char lod, - unsigned int lodFlags, bool compile, TerrainDrawable* templateGeometry) + unsigned int lodFlags, bool compile, const TerrainDrawable* templateGeometry) { osg::ref_ptr geometry(new TerrainDrawable); diff --git a/components/terrain/chunkmanager.hpp b/components/terrain/chunkmanager.hpp index 4238f01c8b..a55dd15cc1 100644 --- a/components/terrain/chunkmanager.hpp +++ b/components/terrain/chunkmanager.hpp @@ -28,10 +28,51 @@ namespace Terrain class CompositeMap; class TerrainDrawable; - typedef std::tuple ChunkId; // Center, Lod, Lod Flags + struct TemplateKey + { + osg::Vec2f mCenter; + unsigned char mLod; + }; + + inline auto tie(const TemplateKey& v) + { + return std::tie(v.mCenter, v.mLod); + } + + inline bool operator<(const TemplateKey& l, const TemplateKey& r) + { + return tie(l) < tie(r); + } + + inline bool operator==(const TemplateKey& l, const TemplateKey& r) + { + return tie(l) == tie(r); + } + + struct ChunkKey + { + osg::Vec2f mCenter; + unsigned char mLod; + unsigned mLodFlags; + }; + + inline auto tie(const ChunkKey& v) + { + return std::tie(v.mCenter, v.mLod, v.mLodFlags); + } + + inline bool operator<(const ChunkKey& l, const ChunkKey& r) + { + return tie(l) < tie(r); + } + + inline bool operator<(const ChunkKey& l, const TemplateKey& r) + { + return TemplateKey{ .mCenter = l.mCenter, .mLod = l.mLod } < r; + } /// @brief Handles loading and caching of terrain chunks - class ChunkManager : public Resource::GenericResourceManager, public QuadTreeWorld::ChunkManager + class ChunkManager : public Resource::GenericResourceManager, public QuadTreeWorld::ChunkManager { public: ChunkManager(Storage* storage, Resource::SceneManager* sceneMgr, TextureManager* textureManager, @@ -55,7 +96,7 @@ namespace Terrain private: osg::ref_ptr createChunk(float size, const osg::Vec2f& center, unsigned char lod, - unsigned int lodFlags, bool compile, TerrainDrawable* templateGeometry); + unsigned int lodFlags, bool compile, const TerrainDrawable* templateGeometry); osg::ref_ptr createCompositeMapRTT(); diff --git a/components/terrain/quadtreeworld.cpp b/components/terrain/quadtreeworld.cpp index 8449f3df25..52edd96b9f 100644 --- a/components/terrain/quadtreeworld.cpp +++ b/components/terrain/quadtreeworld.cpp @@ -544,37 +544,16 @@ namespace Terrain vd->setViewPoint(viewPoint); vd->setActiveGrid(grid); - for (unsigned int pass = 0; pass < 3; ++pass) + DefaultLodCallback lodCallback(mLodFactor, mMinSize, mViewDistance, grid, cellWorldSize); + mRootNode->traverseNodes(vd, viewPoint, &lodCallback); + + reporter.addTotal(vd->getNumEntries()); + + for (unsigned int i = 0, n = vd->getNumEntries(); i < n && !abort; ++i) { - unsigned int startEntry = vd->getNumEntries(); - - float distanceModifier = 0.f; - if (pass == 1) - distanceModifier = 1024; - else if (pass == 2) - distanceModifier = -1024; - DefaultLodCallback lodCallback(mLodFactor, mMinSize, mViewDistance, grid, cellWorldSize, distanceModifier); - mRootNode->traverseNodes(vd, viewPoint, &lodCallback); - - if (pass == 0) - { - std::size_t progressTotal = 0; - for (unsigned int i = 0, n = vd->getNumEntries(); i < n; ++i) - progressTotal += vd->getEntry(i).mNode->getSize(); - - reporter.addTotal(progressTotal); - } - - for (unsigned int i = startEntry; i < vd->getNumEntries() && !abort; ++i) - { - ViewDataEntry& entry = vd->getEntry(i); - - loadRenderingNode(entry, vd, cellWorldSize, grid, true); - if (pass == 0) - reporter.addProgress(entry.mNode->getSize()); - vd->removeNodeFromIndex(entry.mNode); - entry.mNode = nullptr; // Clear node lest we break the neighbours search for the next pass - } + ViewDataEntry& entry = vd->getEntry(i); + loadRenderingNode(entry, vd, cellWorldSize, grid, true); + reporter.addProgress(1); } } diff --git a/components/terrain/terraindrawable.hpp b/components/terrain/terraindrawable.hpp index b94f47df97..3ce846ad73 100644 --- a/components/terrain/terraindrawable.hpp +++ b/components/terrain/terraindrawable.hpp @@ -60,7 +60,7 @@ namespace Terrain const osg::BoundingBox& getWaterBoundingBox() const { return mWaterBoundingBox; } void setCompositeMap(CompositeMap* map) { mCompositeMap = map; } - CompositeMap* getCompositeMap() { return mCompositeMap; } + CompositeMap* getCompositeMap() const { return mCompositeMap; } void setCompositeMapRenderer(CompositeMapRenderer* renderer) { mCompositeMapRenderer = renderer; } private: