Make sure proper number of cells is preloaded

Limit the number of exterior cells around a cell to be preloded based on current
and max cache size not just max cache size. Avoid doing break from inner loop
only.

Log when truncation happens but only once during process lifetime to warn a user.

apps/openmw/mwworld/cellpreloader.cpp

@@ -225,8 +225,6 @@ namespace MWWorld
         , mTerrain(terrain)
         , mLandManager(landManager)
         , mExpiryDelay(0.0)
-        , mMinCacheSize(0)
-        , mMaxCacheSize(0)
         , mPreloadInstances(true)
         , mLastResourceCacheUpdate(0.0)
         , mLoadedTerrainTimestamp(0.0)
@@ -361,26 +359,11 @@ namespace MWWorld
         mExpiryDelay = expiryDelay;
     }
 
-    void CellPreloader::setMinCacheSize(unsigned int num)
-    {
-        mMinCacheSize = num;
-    }
-
-    void CellPreloader::setMaxCacheSize(unsigned int num)
-    {
-        mMaxCacheSize = num;
-    }
-
     void CellPreloader::setPreloadInstances(bool preload)
     {
         mPreloadInstances = preload;
     }
 
-    unsigned int CellPreloader::getMaxCacheSize() const
-    {
-        return mMaxCacheSize;
-    }
-
     void CellPreloader::setWorkQueue(osg::ref_ptr<SceneUtil::WorkQueue> workQueue)
     {
         mWorkQueue = workQueue;

apps/openmw/mwworld/cellpreloader.hpp

@@ -65,15 +65,17 @@ namespace MWWorld
         void setExpiryDelay(double expiryDelay);
 
         /// The minimum number of preloaded cells before unused cells get thrown out.
-        void setMinCacheSize(unsigned int num);
+        void setMinCacheSize(std::size_t value) { mMinCacheSize = value; }
 
         /// The maximum number of preloaded cells.
-        void setMaxCacheSize(unsigned int num);
+        void setMaxCacheSize(std::size_t value) { mMaxCacheSize = value; }
 
         /// Enables the creation of instances in the preloading thread.
         void setPreloadInstances(bool preload);
 
-        unsigned int getMaxCacheSize() const;
+        std::size_t getMaxCacheSize() const { return mMaxCacheSize; }
+
+        std::size_t getCacheSize() const { return mPreloadCells.size(); }
 
         void setWorkQueue(osg::ref_ptr<SceneUtil::WorkQueue> workQueue);
 
@@ -96,8 +98,8 @@ namespace MWWorld
         MWRender::LandManager* mLandManager;
         osg::ref_ptr<SceneUtil::WorkQueue> mWorkQueue;
         double mExpiryDelay;
-        unsigned int mMinCacheSize;
+        std::size_t mMinCacheSize = 0;
-        unsigned int mMaxCacheSize;
+        std::size_t mMaxCacheSize = 0;
         bool mPreloadInstances;
 
         double mLastResourceCacheUpdate;

apps/openmw/mwworld/scene.cpp

@@ -270,6 +270,29 @@ namespace
         pagedRefs.erase(it);
         return true;
     }
+
+    template <class Function>
+    void iterateOverCellsAround(int cellX, int cellY, int range, Function&& f)
+    {
+        for (int x = cellX - range, lastX = cellX + range; x <= lastX; ++x)
+            for (int y = cellY - range, lastY = cellY + range; y <= lastY; ++y)
+                f(x, y);
+    }
+
+    void sortCellsToLoad(int centerX, int centerY, std::vector<std::pair<int, int>>& cells)
+    {
+        const auto getDistanceToPlayerCell = [&](const std::pair<int, int>& cellPosition) {
+            return std::abs(cellPosition.first - centerX) + std::abs(cellPosition.second - centerY);
+        };
+
+        const auto getCellPositionPriority = [&](const std::pair<int, int>& cellPosition) {
+            return std::make_pair(getDistanceToPlayerCell(cellPosition), getCellPositionDistanceToOrigin(cellPosition));
+        };
+
+        std::sort(cells.begin(), cells.end(), [&](const std::pair<int, int>& lhs, const std::pair<int, int>& rhs) {
+            return getCellPositionPriority(lhs) < getCellPositionPriority(rhs);
+        });
+    }
 }
 
 namespace MWWorld
@@ -585,44 +608,24 @@ namespace MWWorld
         mPagedRefs.clear();
         mRendering.getPagedRefnums(newGrid, mPagedRefs);
 
-        std::size_t refsToLoad = 0;
-        const auto cellsToLoad = [&](CellStoreCollection& collection, int range) -> std::vector<std::pair<int, int>> {
-            std::vector<std::pair<int, int>> cellsPositionsToLoad;
-            for (int x = playerCellX - range; x <= playerCellX + range; ++x)
-            {
-                for (int y = playerCellY - range; y <= playerCellY + range; ++y)
-                {
-                    if (!isCellInCollection(ESM::ExteriorCellLocation(x, y, playerCellIndex.mWorldspace), collection))
-                    {
-                        refsToLoad += mWorld.getWorldModel().getExterior(playerCellIndex).count();
-                        cellsPositionsToLoad.emplace_back(x, y);
-                    }
-                }
-            }
-            return cellsPositionsToLoad;
-        };
-
         addPostponedPhysicsObjects();
 
-        auto cellsPositionsToLoad = cellsToLoad(mActiveCells, mHalfGridSize);
+        std::size_t refsToLoad = 0;
+        std::vector<std::pair<int, int>> cellsPositionsToLoad;
+        iterateOverCellsAround(playerCellX, playerCellY, mHalfGridSize, [&](int x, int y) {
+            const ESM::ExteriorCellLocation location(x, y, playerCellIndex.mWorldspace);
+            if (isCellInCollection(location, mActiveCells))
+                return;
+            refsToLoad += mWorld.getWorldModel().getExterior(location).count();
+            cellsPositionsToLoad.emplace_back(x, y);
+        });
 
         Loading::Listener* loadingListener = MWBase::Environment::get().getWindowManager()->getLoadingScreen();
         Loading::ScopedLoad load(loadingListener);
         loadingListener->setLabel("#{OMWEngine:LoadingExterior}");
         loadingListener->setProgressRange(refsToLoad);
 
-        const auto getDistanceToPlayerCell = [&](const std::pair<int, int>& cellPosition) {
+        sortCellsToLoad(playerCellX, playerCellY, cellsPositionsToLoad);
-            return std::abs(cellPosition.first - playerCellX) + std::abs(cellPosition.second - playerCellY);
-        };
-
-        const auto getCellPositionPriority = [&](const std::pair<int, int>& cellPosition) {
-            return std::make_pair(getDistanceToPlayerCell(cellPosition), getCellPositionDistanceToOrigin(cellPosition));
-        };
-
-        std::sort(cellsPositionsToLoad.begin(), cellsPositionsToLoad.end(),
-            [&](const std::pair<int, int>& lhs, const std::pair<int, int>& rhs) {
-                return getCellPositionPriority(lhs) < getCellPositionPriority(rhs);
-            });
 
         for (const auto& [x, y] : cellsPositionsToLoad)
         {
@@ -1136,7 +1139,7 @@ namespace MWWorld
             {
                 try
                 {
-                    preloadCell(mWorld.getWorldModel().getCell(door.getCellRef().getDestCell()), true);
+                    preloadCellWithSurroundings(mWorld.getWorldModel().getCell(door.getCellRef().getDestCell()));
                 }
                 catch (std::exception&)
                 {
@@ -1183,27 +1186,47 @@ namespace MWWorld
         }
     }
 
-    void Scene::preloadCell(CellStore& cell, bool preloadSurrounding)
+    void Scene::preloadCellWithSurroundings(CellStore& cell)
     {
-        if (preloadSurrounding && cell.isExterior())
+        if (!cell.isExterior())
         {
-            int x = cell.getCell()->getGridX();
-            int y = cell.getCell()->getGridY();
-            unsigned int numpreloaded = 0;
-            for (int dx = -mHalfGridSize; dx <= mHalfGridSize; ++dx)
-            {
-                for (int dy = -mHalfGridSize; dy <= mHalfGridSize; ++dy)
-                {
-                    mPreloader->preload(mWorld.getWorldModel().getExterior(
-                                            ESM::ExteriorCellLocation(x + dx, y + dy, cell.getCell()->getWorldSpace())),
-                        mRendering.getReferenceTime());
-                    if (++numpreloaded >= mPreloader->getMaxCacheSize())
-                        break;
-                }
-            }
-        }
-        else
             mPreloader->preload(cell, mRendering.getReferenceTime());
+            return;
+        }
+
+        const int cellX = cell.getCell()->getGridX();
+        const int cellY = cell.getCell()->getGridY();
+
+        std::vector<std::pair<int, int>> cells;
+        const std::size_t gridSize = static_cast<std::size_t>(2 * mHalfGridSize + 1);
+        cells.reserve(gridSize * gridSize);
+
+        iterateOverCellsAround(cellX, cellY, mHalfGridSize, [&](int x, int y) { cells.emplace_back(x, y); });
+
+        sortCellsToLoad(cellX, cellY, cells);
+
+        const std::size_t leftCapacity = mPreloader->getMaxCacheSize() - mPreloader->getCacheSize();
+        if (cells.size() > leftCapacity)
+        {
+            static bool logged = [&] {
+                Log(Debug::Warning) << "Not enough cell preloader cache capacity to preload exterior cells, consider "
+                                       "increasing \"preload cell cache max\" up to "
+                                    << (mPreloader->getCacheSize() + cells.size());
+                return true;
+            }();
+            (void)logged;
+            cells.resize(leftCapacity);
+        }
+
+        const ESM::RefId worldspace = cell.getCell()->getWorldSpace();
+        for (const auto& [x, y] : cells)
+            mPreloader->preload(mWorld.getWorldModel().getExterior(ESM::ExteriorCellLocation(x, y, worldspace)),
+                mRendering.getReferenceTime());
+    }
+
+    void Scene::preloadCell(CellStore& cell)
+    {
+        mPreloader->preload(cell, mRendering.getReferenceTime());
     }
 
     void Scene::preloadTerrain(const osg::Vec3f& pos, ESM::RefId worldspace, bool sync)
@@ -1281,7 +1304,7 @@ namespace MWWorld
                 osg::Vec3f pos = dest.mPos.asVec3();
                 const ESM::ExteriorCellLocation cellIndex
                     = ESM::positionToExteriorCellLocation(pos.x(), pos.y(), extWorldspace);
-                preloadCell(mWorld.getWorldModel().getExterior(cellIndex), true);
+                preloadCellWithSurroundings(mWorld.getWorldModel().getExterior(cellIndex));
                 exteriorPositions.emplace_back(pos, gridCenterToBounds(getNewGridCenter(pos)));
             }
         }

apps/openmw/mwworld/scene.hpp

@@ -145,7 +145,8 @@ namespace MWWorld
 
         ~Scene();
 
-        void preloadCell(MWWorld::CellStore& cell, bool preloadSurrounding = false);
+        void preloadCellWithSurroundings(MWWorld::CellStore& cell);
+        void preloadCell(MWWorld::CellStore& cell);
         void preloadTerrain(const osg::Vec3f& pos, ESM::RefId worldspace, bool sync = false);
         void reloadTerrain();
 

apps/openmw/mwworld/worldimp.cpp

@@ -523,7 +523,7 @@ namespace MWWorld
                     if (getPlayerPtr().getCell()->isExterior())
                         mWorldScene->preloadTerrain(getPlayerPtr().getRefData().getPosition().asVec3(),
                             getPlayerPtr().getCell()->getCell()->getWorldSpace());
-                    mWorldScene->preloadCell(*getPlayerPtr().getCell(), true);
+                    mWorldScene->preloadCellWithSurroundings(*getPlayerPtr().getCell());
                 }
                 break;
             default: