From 22dc383f638402a9a4cb54375349257b30e12b35 Mon Sep 17 00:00:00 2001 From: fteppe Date: Sat, 20 May 2023 00:31:38 +0200 Subject: [PATCH] fixes errors and warnings Applies review comments getWorldspaceTerrain => returns a reference because never null crashfix in navigator updateLandPositions fixes naming of it const ESM4::Land* MWWorld::Store::search(ESM::ExteriorCellLocation cellLocation) const removes useless else ExteriorCellLocation uses default initializers get terrain height returns -MAX_FLOAT when there is no esm4 terrain. applied review comments use default initlializer when possible factorise code uses pattern matching in for loop. --- apps/opencs/view/render/terrainstorage.cpp | 2 +- apps/opencs/view/render/terrainstorage.hpp | 2 +- apps/openmw/mwrender/renderingmanager.cpp | 17 +++++----- apps/openmw/mwrender/renderingmanager.hpp | 2 +- apps/openmw/mwrender/terrainstorage.cpp | 36 ++++++++++------------ apps/openmw/mwworld/scene.cpp | 2 +- apps/openmw/mwworld/store.cpp | 15 +++++---- apps/openmw/mwworld/store.hpp | 1 + components/esm/util.hpp | 12 +++----- components/esm3terrain/storage.cpp | 4 +-- components/terrain/quadtreeworld.hpp | 13 +++----- 11 files changed, 47 insertions(+), 59 deletions(-) diff --git a/apps/opencs/view/render/terrainstorage.cpp b/apps/opencs/view/render/terrainstorage.cpp index a5b1797daf..a9c1c5e5cf 100644 --- a/apps/opencs/view/render/terrainstorage.cpp +++ b/apps/opencs/view/render/terrainstorage.cpp @@ -82,7 +82,7 @@ namespace CSVRender return &mAlteredHeight[inCellY * ESM::Land::LAND_SIZE + inCellX]; } - void TerrainStorage::getBounds(float& minX, float& maxX, float& minY, float& maxY) + void TerrainStorage::getBounds(float& minX, float& maxX, float& minY, float& maxY, ESM::RefId worldspace) { // not needed at the moment - this returns the bounds of the whole world, but we only edit individual cells throw std::runtime_error("getBounds not implemented"); diff --git a/apps/opencs/view/render/terrainstorage.hpp b/apps/opencs/view/render/terrainstorage.hpp index 500309dc84..bce5d94156 100644 --- a/apps/opencs/view/render/terrainstorage.hpp +++ b/apps/opencs/view/render/terrainstorage.hpp @@ -40,7 +40,7 @@ namespace CSVRender osg::ref_ptr getLand(ESM::ExteriorCellLocation cellLocation) override; const ESM::LandTexture* getLandTexture(int index, short plugin) override; - void getBounds(float& minX, float& maxX, float& minY, float& maxY) override; + void getBounds(float& minX, float& maxX, float& minY, float& maxY, ESM::RefId worldspace) override; int getThisHeight(int col, int row, const ESM::Land::LandData* heightData) const; int getLeftHeight(int col, int row, const ESM::Land::LandData* heightData) const; diff --git a/apps/openmw/mwrender/renderingmanager.cpp b/apps/openmw/mwrender/renderingmanager.cpp index e8fbee1a09..066f73514a 100644 --- a/apps/openmw/mwrender/renderingmanager.cpp +++ b/apps/openmw/mwrender/renderingmanager.cpp @@ -458,7 +458,7 @@ namespace MWRender mTerrainStorage = std::make_unique(mResourceSystem, normalMapPattern, heightMapPattern, useTerrainNormalMaps, specularMapPattern, useTerrainSpecularMaps); - mTerrain = getWorldspaceTerrain(ESM::Cell::sDefaultWorldspaceId); + mTerrain = &getWorldspaceTerrain(ESM::Cell::sDefaultWorldspaceId); bool groundcover = Settings::Manager::getBool("enabled", "Groundcover"); if (groundcover) @@ -758,7 +758,7 @@ namespace MWRender if (store->getCell()->isExterior()) { getWorldspaceTerrain(store->getCell()->getWorldSpace()) - ->unloadCell(store->getCell()->getGridX(), store->getCell()->getGridY()); + .unloadCell(store->getCell()->getGridX(), store->getCell()->getGridY()); } mWater->removeCell(store); @@ -770,7 +770,7 @@ namespace MWRender mWater->setCullCallback(nullptr); else { - Terrain::World* newTerrain = getWorldspaceTerrain(worldspace); + Terrain::World* newTerrain = &getWorldspaceTerrain(worldspace); if (newTerrain != mTerrain) { mTerrain->enable(false); @@ -1328,11 +1328,11 @@ namespace MWRender mStateUpdater->setFogColor(color); } - Terrain::World* RenderingManager::getWorldspaceTerrain(ESM::RefId worldspace) + Terrain::World& RenderingManager::getWorldspaceTerrain(ESM::RefId worldspace) { auto existingTerrain = mWorldspaceTerrains.find(worldspace); if (existingTerrain != mWorldspaceTerrains.end()) - return existingTerrain->second.get(); + return *existingTerrain->second.get(); std::unique_ptr newTerrain; const float lodFactor = Settings::Manager::getFloat("lod factor", "Terrain"); @@ -1366,8 +1366,9 @@ namespace MWRender float distanceMult = std::cos(osg::DegreesToRadians(std::min(mFieldOfView, 140.f)) / 2.f); newTerrain->setViewDistance(mViewDistance * (distanceMult ? 1.f / distanceMult : 1.f)); - mWorldspaceTerrains[worldspace] = std::move(newTerrain); - return mWorldspaceTerrains[worldspace].get(); + Terrain::World* result = newTerrain.get(); + mWorldspaceTerrains.emplace(worldspace, std::move(newTerrain)); + return *result; } void RenderingManager::reportStats() const @@ -1473,7 +1474,7 @@ namespace MWRender float RenderingManager::getTerrainHeightAt(const osg::Vec3f& pos, ESM::RefId worldspace) { - return getWorldspaceTerrain(worldspace)->getHeightAt(pos); + return getWorldspaceTerrain(worldspace).getHeightAt(pos); } void RenderingManager::overrideFieldOfView(float val) diff --git a/apps/openmw/mwrender/renderingmanager.hpp b/apps/openmw/mwrender/renderingmanager.hpp index 818df3b150..5811387284 100644 --- a/apps/openmw/mwrender/renderingmanager.hpp +++ b/apps/openmw/mwrender/renderingmanager.hpp @@ -282,7 +282,7 @@ namespace MWRender void updateAmbient(); void setFogColor(const osg::Vec4f& color); void updateThirdPersonViewMode(); - Terrain::World* getWorldspaceTerrain(ESM::RefId worldspace); + Terrain::World& getWorldspaceTerrain(ESM::RefId worldspace); void reportStats() const; diff --git a/apps/openmw/mwrender/terrainstorage.cpp b/apps/openmw/mwrender/terrainstorage.cpp index 871f6fd70e..899c035cc6 100644 --- a/apps/openmw/mwrender/terrainstorage.cpp +++ b/apps/openmw/mwrender/terrainstorage.cpp @@ -34,6 +34,18 @@ namespace MWRender return land != nullptr; } + static void BoundUnion(float& minX, float& maxX, float& minY, float& maxY, float x, float y) + { + if (x < minX) + minX = x; + if (x > maxX) + maxX = x; + if (y < minY) + minY = y; + if (y > maxY) + maxY = y; + } + void TerrainStorage::getBounds(float& minX, float& maxX, float& minY, float& maxY, ESM::RefId worldspace) { minX = 0; @@ -46,20 +58,11 @@ namespace MWRender if (ESM::isEsm4Ext(worldspace)) { const auto& lands = esmStore.get().getLands(); - for (const auto& it : lands) + for (const auto& [landPos, _] : lands) { - if (it.first.mWorldspace == worldspace) + if (landPos.mWorldspace == worldspace) { - int x = it.first.mX; - int y = it.first.mY; - if (x < minX) - minX = static_cast(x); - if (x > maxX) - maxX = static_cast(x); - if (y < minY) - minY = static_cast(y); - if (y > maxY) - maxY = static_cast(y); + BoundUnion(minX, maxX, minY, maxY, static_cast(landPos.mX), static_cast(landPos.mY)); } } } @@ -68,14 +71,7 @@ namespace MWRender MWWorld::Store::iterator it = esmStore.get().begin(); for (; it != esmStore.get().end(); ++it) { - if (it->mX < minX) - minX = static_cast(it->mX); - if (it->mX > maxX) - maxX = static_cast(it->mX); - if (it->mY < minY) - minY = static_cast(it->mY); - if (it->mY > maxY) - maxY = static_cast(it->mY); + BoundUnion(minX, maxX, minY, maxY, static_cast(it->mX), static_cast(it->mY)); } } // since grid coords are at cell origin, we need to add 1 cell diff --git a/apps/openmw/mwworld/scene.cpp b/apps/openmw/mwworld/scene.cpp index 4bfc48ce6f..3c172d3d4f 100644 --- a/apps/openmw/mwworld/scene.cpp +++ b/apps/openmw/mwworld/scene.cpp @@ -433,7 +433,7 @@ namespace MWWorld return heights; } }(); - mNavigator.addHeightfield(cellPosition, data->getSize(), shape, navigatorUpdateGuard); + mNavigator.addHeightfield(cellPosition, worldsize, shape, navigatorUpdateGuard); } } diff --git a/apps/openmw/mwworld/store.cpp b/apps/openmw/mwworld/store.cpp index 3d8f741573..d9520dc3b3 100644 --- a/apps/openmw/mwworld/store.cpp +++ b/apps/openmw/mwworld/store.cpp @@ -1199,15 +1199,15 @@ namespace MWWorld void Store::updateLandPositions(const Store& cells) { - for (auto& it : mStatic) + for (const auto& [id, value] : mStatic) { - const ESM4::Cell* cell = cells.find(it.second.mCell); - mLands[cell->getExteriorCellLocation()] = &it.second; + const ESM4::Cell* cell = cells.find(value.mCell); + mLands[cell->getExteriorCellLocation()] = &value; } - for (auto& it : mDynamic) + for (const auto& [id, value] : mDynamic) { - const ESM4::Cell* cell = cells.find(it.second.mCell); - mLands[cell->getExteriorCellLocation()] = &it.second; + const ESM4::Cell* cell = cells.find(value.mCell); + mLands[cell->getExteriorCellLocation()] = &value; } } @@ -1216,8 +1216,7 @@ namespace MWWorld auto foundLand = mLands.find(cellLocation); if (foundLand == mLands.end()) return nullptr; - else - return foundLand->second; + return foundLand->second; } // ESM4 Reference diff --git a/apps/openmw/mwworld/store.hpp b/apps/openmw/mwworld/store.hpp index ed8165ab96..c496d1ee0f 100644 --- a/apps/openmw/mwworld/store.hpp +++ b/apps/openmw/mwworld/store.hpp @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include diff --git a/components/esm/util.hpp b/components/esm/util.hpp index 8b0087b881..d7ebcf8f6f 100644 --- a/components/esm/util.hpp +++ b/components/esm/util.hpp @@ -51,14 +51,10 @@ namespace ESM struct ExteriorCellLocation { - int mX, mY; - ESM::RefId mWorldspace; - ExteriorCellLocation() - : mX(0) - , mY(0) - , mWorldspace(ESM::Cell::sDefaultWorldspaceId) - { - } + int mX = 0; + int mY = 0; + ESM::RefId mWorldspace = ESM::Cell::sDefaultWorldspaceId; + ExteriorCellLocation() = default; ExteriorCellLocation(int x, int y, ESM::RefId worldspace) : mX(x) diff --git a/components/esm3terrain/storage.cpp b/components/esm3terrain/storage.cpp index 2268d06c07..474d171426 100644 --- a/components/esm3terrain/storage.cpp +++ b/components/esm3terrain/storage.cpp @@ -337,7 +337,7 @@ namespace ESMTerrain if (!validHeightDataExists && ESM::isEsm4Ext(worldspace)) { - for (int iVert = 0; iVert < numVerts * numVerts; iVert++) + for (unsigned int iVert = 0; iVert < numVerts * numVerts; iVert++) { (*positions)[static_cast(iVert)] = osg::Vec3f(0.f, 0.f, 0.f); } @@ -485,7 +485,7 @@ namespace ESMTerrain osg::ref_ptr land = getLand(ESM::ExteriorCellLocation(cellX, cellY, worldspace)); if (!land) - return defaultHeight; + return ESM::isEsm4Ext(worldspace) ? std::numeric_limits::lowest() : defaultHeight; const ESM::LandData* data = land->getData(ESM::Land::DATA_VHGT); if (!data) diff --git a/components/terrain/quadtreeworld.hpp b/components/terrain/quadtreeworld.hpp index fb7a078d7c..2524dc046f 100644 --- a/components/terrain/quadtreeworld.hpp +++ b/components/terrain/quadtreeworld.hpp @@ -60,12 +60,7 @@ namespace Terrain { public: virtual ~ChunkManager() {} - ChunkManager() - : mWorldspace(ESM::RefId()) - , mViewDistance(0.f) - , mMaxLodLevel(~0u) - { - } + ChunkManager() = default; ChunkManager(ESM::RefId worldspace) : ChunkManager() { @@ -84,11 +79,11 @@ namespace Terrain void setMaxLodLevel(unsigned int level) { mMaxLodLevel = level; } protected: - ESM::RefId mWorldspace; + ESM::RefId mWorldspace = ESM::RefId(); private: - float mViewDistance; - unsigned int mMaxLodLevel; + float mViewDistance = 0.f; + unsigned int mMaxLodLevel = ~0u; }; void addChunkManager(ChunkManager*);