Merge branch 'terrain_cleanup' into 'master'

Terrain releated code cleanup

See merge request OpenMW/openmw!3353

apps/esmtool/labels.cpp

@@ -759,7 +759,7 @@ std::string enchantmentFlags(int flags)
     return properties;
 }
 
-std::string landFlags(int flags)
+std::string landFlags(std::uint32_t flags)
 {
     std::string properties;
     // The ESM component says that this first four bits are used, but

apps/esmtool/labels.hpp

@@ -1,6 +1,7 @@
 #ifndef OPENMW_ESMTOOL_LABELS_H
 #define OPENMW_ESMTOOL_LABELS_H
 
+#include <cstdint>
 #include <string>
 #include <string_view>
 
@@ -53,7 +54,7 @@ std::string cellFlags(int flags);
 std::string containerFlags(int flags);
 std::string creatureFlags(int flags);
 std::string enchantmentFlags(int flags);
-std::string landFlags(int flags);
+std::string landFlags(std::uint32_t flags);
 std::string creatureListFlags(int flags);
 std::string itemListFlags(int flags);
 std::string lightFlags(int flags);

apps/navmeshtool/worldspacedata.cpp

@@ -206,7 +206,7 @@ namespace NavMeshTool
                     ESM::Land::DEFAULT_HEIGHT };
 
             ESM::Land::LandData& landData = *landDatas.emplace_back(std::make_unique<ESM::Land::LandData>());
-            land->loadData(ESM::Land::DATA_VHGT, &landData);
+            land->loadData(ESM::Land::DATA_VHGT, landData);
             heightfields.push_back(std::vector<float>(std::begin(landData.mHeights), std::end(landData.mHeights)));
             HeightfieldSurface surface;
             surface.mHeights = heightfields.back().data();

apps/opencs/view/render/terrainstorage.cpp

@@ -39,7 +39,7 @@ namespace CSVRender
 
         const ESM::Land& land = mData.getLand().getRecord(index).get();
         return new ESMTerrain::LandObject(
-            &land, ESM::Land::DATA_VHGT | ESM::Land::DATA_VNML | ESM::Land::DATA_VCLR | ESM::Land::DATA_VTEX);
+            land, ESM::Land::DATA_VHGT | ESM::Land::DATA_VNML | ESM::Land::DATA_VCLR | ESM::Land::DATA_VTEX);
     }
 
     const ESM::LandTexture* TerrainStorage::getLandTexture(int index, short plugin)

apps/openmw/mwrender/globalmap.cpp

@@ -18,6 +18,7 @@
 #include <components/sceneutil/workqueue.hpp>
 
 #include <components/esm3/globalmap.hpp>
+#include <components/esm3/loadland.hpp>
 
 #include "../mwbase/environment.hpp"
 

apps/openmw/mwrender/landmanager.cpp

@@ -15,39 +15,34 @@ namespace MWRender
         : GenericResourceManager<ESM::ExteriorCellLocation>(nullptr)
         , mLoadFlags(loadFlags)
     {
-        mCache = new CacheType;
     }
 
     osg::ref_ptr<ESMTerrain::LandObject> LandManager::getLand(ESM::ExteriorCellLocation cellIndex)
     {
-        osg::ref_ptr<osg::Object> obj = mCache->getRefFromObjectCache(cellIndex);
+        const osg::ref_ptr<osg::Object> obj = mCache->getRefFromObjectCache(cellIndex);
-        if (obj)
+        if (obj != nullptr)
             return static_cast<ESMTerrain::LandObject*>(obj.get());
+
+        const MWBase::World& world = *MWBase::Environment::get().getWorld();
+        osg::ref_ptr<ESMTerrain::LandObject> landObj = nullptr;
+
+        if (ESM::isEsm4Ext(cellIndex.mWorldspace))
+        {
+            const ESM4::Land* land = world.getStore().get<ESM4::Land>().search(cellIndex);
+            if (land == nullptr)
+                return nullptr;
+            landObj = new ESMTerrain::LandObject(*land, mLoadFlags);
+        }
         else
         {
-            const auto world = MWBase::Environment::get().getWorld();
+            const ESM::Land* land = world.getStore().get<ESM::Land>().search(cellIndex.mX, cellIndex.mY);
-            if (!world)
+            if (land == nullptr)
                 return nullptr;
-
+            landObj = new ESMTerrain::LandObject(*land, mLoadFlags);
-            if (ESM::isEsm4Ext(cellIndex.mWorldspace))
-            {
-                const ESM4::Land* land = world->getStore().get<ESM4::Land>().search(cellIndex);
-                if (!land)
-                    return nullptr;
-                osg::ref_ptr<ESMTerrain::LandObject> landObj(new ESMTerrain::LandObject(land, mLoadFlags));
-                mCache->addEntryToObjectCache(cellIndex, landObj.get());
-                return landObj;
-            }
-            else
-            {
-                const ESM::Land* land = world->getStore().get<ESM::Land>().search(cellIndex.mX, cellIndex.mY);
-                if (!land)
-                    return nullptr;
-                osg::ref_ptr<ESMTerrain::LandObject> landObj(new ESMTerrain::LandObject(land, mLoadFlags));
-                mCache->addEntryToObjectCache(cellIndex, landObj.get());
-                return landObj;
-            }
         }
+
+        mCache->addEntryToObjectCache(cellIndex, landObj.get());
+        return landObj;
     }
 
     void LandManager::reportStats(unsigned int frameNumber, osg::Stats* stats) const

apps/openmw/mwrender/terrainstorage.cpp

@@ -1,5 +1,7 @@
 #include "terrainstorage.hpp"
 
+#include <components/esm3/loadland.hpp>
+
 #include "../mwbase/environment.hpp"
 #include "../mwworld/esmstore.hpp"
 

apps/openmw/mwworld/cellpreloader.cpp

@@ -7,6 +7,7 @@
 #include <components/debug/debuglog.hpp>
 #include <components/esm3/loadcell.hpp>
 #include <components/loadinglistener/reporter.hpp>
+#include <components/misc/constants.hpp>
 #include <components/misc/resourcehelpers.hpp>
 #include <components/misc/strings/lower.hpp>
 #include <components/resource/bulletshapemanager.hpp>
@@ -394,7 +395,7 @@ namespace MWWorld
 
     void CellPreloader::abortTerrainPreloadExcept(const CellPreloader::PositionCellGrid* exceptPos)
     {
-        if (exceptPos && contains(mTerrainPreloadPositions, std::array{ *exceptPos }, ESM::Land::REAL_SIZE))
+        if (exceptPos && contains(mTerrainPreloadPositions, std::array{ *exceptPos }, Constants::CellSizeInUnits))
             return;
         if (mTerrainPreloadItem && !mTerrainPreloadItem->isDone())
         {
@@ -437,7 +438,7 @@ namespace MWWorld
     bool CellPreloader::isTerrainLoaded(const CellPreloader::PositionCellGrid& position, double referenceTime) const
     {
         return mLoadedTerrainTimestamp + mResourceSystem->getSceneManager()->getExpiryDelay() > referenceTime
-            && contains(mLoadedTerrainPositions, std::array{ position }, ESM::Land::REAL_SIZE);
+            && contains(mLoadedTerrainPositions, std::array{ position }, Constants::CellSizeInUnits);
     }
 
     void CellPreloader::setTerrain(Terrain::World* terrain)

apps/openmw/mwworld/store.hpp

@@ -37,11 +37,6 @@ namespace ESM
     class ESMWriter;
 }
 
-namespace ESM4
-{
-    struct Land;
-}
-
 namespace Loading
 {
     class Listener;

components/esm/esmterrain.cpp

@@ -1,67 +1,77 @@
-#include <components/misc/constants.hpp>
-
 #include "esmterrain.hpp"
 
+#include <components/esm3/loadland.hpp>
+#include <components/esm4/loadland.hpp>
+#include <components/misc/constants.hpp>
+
+namespace
+{
+    constexpr std::uint16_t textures[ESM::LandRecordData::sLandNumTextures]{ 0 };
+
+    std::unique_ptr<const ESM::LandRecordData> loadData(const ESM::Land& land, int loadFlags)
+    {
+        std::unique_ptr<ESM::LandRecordData> result = std::make_unique<ESM::LandRecordData>();
+        land.loadData(loadFlags, *result);
+        return result;
+    }
+}
+
+namespace ESM
+{
+    LandData::LandData() = default;
+}
+
 ESM::LandData::LandData(const ESM::Land& land, int loadFlags)
-    : mLoadFlags(loadFlags)
+    : mData(loadData(land, loadFlags))
+    , mLoadFlags(mData->mDataLoaded)
+    , mMinHeight(mData->mMinHeight)
+    , mMaxHeight(mData->mMaxHeight)
     , mSize(Constants::CellSizeInUnits)
     , mLandSize(ESM::Land::LAND_SIZE)
+    , mPlugin(land.getPlugin())
+    , mHeights(mData->mHeights)
+    , mNormals(mData->mNormals)
+    , mColors(mData->mColours)
+    , mTextures(mData->mTextures)
 {
-    ESM::Land::LandData data;
-    land.loadData(loadFlags, &data);
-    mLoadFlags = data.mDataLoaded;
-    std::span<const float> heights(data.mHeights);
-    mHeights = std::vector(heights.begin(), heights.end());
-
-    std::span<const VNML> normals(data.mNormals);
-    mNormals = std::vector(normals.begin(), normals.end());
-
-    std::span<const unsigned char> colors(data.mColours);
-    mColors = std::vector(colors.begin(), colors.end());
-
-    std::span<const uint16_t> textures(data.mTextures);
-    mTextures = std::vector(textures.begin(), textures.end());
-
-    mMinHeight = data.mMinHeight;
-    mMaxHeight = data.mMaxHeight;
 }
 
 ESM::LandData::LandData(const ESM4::Land& land, int /*loadFlags*/)
     : mLoadFlags(land.mDataTypes) // ESM4::Land is always fully loaded. TODO: implement lazy loading
+    , mHeightsData(ESM4::Land::sLandNumVerts)
+    , mMinHeight(std::numeric_limits<float>::max())
+    , mMaxHeight(std::numeric_limits<float>::lowest())
     , mSize(Constants::ESM4CellSizeInUnits)
-    , mLandSize(ESM4::Land::VERTS_PER_SIDE)
+    , mLandSize(ESM4::Land::sVertsPerSide)
+    , mNormals(land.mVertNorm)
+    , mColors(land.mVertColr)
+    , mTextures(textures)
 {
-
+    float rowOffset = land.mHeightMap.heightOffset;
-    mMinHeight = std::numeric_limits<float>::max();
-    mMaxHeight = std::numeric_limits<float>::lowest();
-    mHeights.resize(ESM4::Land::LAND_NUM_VERTS);
-    mTextures.resize(ESM::Land::LAND_NUM_TEXTURES);
-    std::fill(mTextures.begin(), mTextures.end(), 0);
-
-    float row_offset = land.mHeightMap.heightOffset;
     for (int y = 0; y < mLandSize; y++)
     {
-        row_offset += land.mHeightMap.gradientData[y * mLandSize];
+        rowOffset += land.mHeightMap.gradientData[y * mLandSize];
 
-        const float heightY = row_offset * ESM4::Land::HEIGHT_SCALE;
+        const float heightY = rowOffset * ESM4::Land::sHeightScale;
-        mHeights[y * mLandSize] = heightY;
+        mHeightsData[y * mLandSize] = heightY;
         mMinHeight = std::min(mMinHeight, heightY);
         mMaxHeight = std::max(mMaxHeight, heightY);
 
-        float colOffset = row_offset;
+        float colOffset = rowOffset;
         for (int x = 1; x < mLandSize; x++)
         {
             colOffset += land.mHeightMap.gradientData[y * mLandSize + x];
-            const float heightX = colOffset * ESM4::Land::HEIGHT_SCALE;
+            const float heightX = colOffset * ESM4::Land::sHeightScale;
             mMinHeight = std::min(mMinHeight, heightX);
             mMaxHeight = std::max(mMaxHeight, heightX);
-            mHeights[x + y * mLandSize] = heightX;
+            mHeightsData[x + y * mLandSize] = heightX;
         }
     }
 
-    std::span<const VNML> normals(land.mVertNorm);
+    mHeights = mHeightsData;
-    mNormals = std::vector(normals.begin(), normals.end());
+}
-
+
-    std::span<const unsigned char> colors(land.mVertColr);
+namespace ESM
-    mColors = std::vector(colors.begin(), colors.end());
+{
+    LandData::~LandData() = default;
 }

components/esm/esmterrain.hpp

@@ -1,45 +1,54 @@
 #ifndef COMPONENTS_ESM_ESMTERRAIN
 #define COMPONENTS_ESM_ESMTERRAIN
 
+#include <cstdint>
+#include <memory>
 #include <span>
 #include <vector>
 
-#include <components/esm3/loadland.hpp>
+namespace ESM4
-#include <components/esm4/loadland.hpp>
+{
+    struct Land;
+}
 
 namespace ESM
 {
+    struct Land;
+    struct LandRecordData;
+
     class LandData
     {
-
     public:
-        ~LandData() = default;
+        LandData();
-        LandData() = default;
+        LandData(const ESM::Land& Land, int loadFlags);
-        LandData(const ESM::Land& Land, int loadFLags);
+        LandData(const ESM4::Land& Land, int loadFlags);
-        LandData(const ESM4::Land& Land, int loadFLags);
 
-        typedef signed char VNML;
+        ~LandData();
 
         std::span<const float> getHeights() const { return mHeights; }
-        std::span<const VNML> getNormals() const { return mNormals; }
+        std::span<const std::int8_t> getNormals() const { return mNormals; }
-        std::span<const unsigned char> getColors() const { return mColors; }
+        std::span<const std::uint8_t> getColors() const { return mColors; }
-        std::span<const uint16_t> getTextures() const { return mTextures; }
+        std::span<const std::uint16_t> getTextures() const { return mTextures; }
         float getSize() const { return mSize; }
         float getMinHeight() const { return mMinHeight; }
         float getMaxHeight() const { return mMaxHeight; }
         int getLandSize() const { return mLandSize; }
-
+        int getLoadFlags() const { return mLoadFlags; }
-        int mLoadFlags = 0;
+        int getPlugin() const { return mPlugin; }
 
     private:
+        std::unique_ptr<const ESM::LandRecordData> mData;
+        int mLoadFlags = 0;
+        std::vector<float> mHeightsData;
         float mMinHeight = 0.f;
         float mMaxHeight = 0.f;
         float mSize = 0.f;
         int mLandSize = 0;
-        std::vector<float> mHeights;
+        int mPlugin = 0;
-        std::vector<VNML> mNormals;
+        std::span<const float> mHeights;
-        std::vector<unsigned char> mColors;
+        std::span<const std::int8_t> mNormals;
-        std::vector<uint16_t> mTextures;
+        std::span<const std::uint8_t> mColors;
+        std::span<const std::uint16_t> mTextures;
     };
 
 }

components/esm/util.cpp

@@ -19,5 +19,5 @@ osg::Vec2 ESM::indexToPosition(const ESM::ExteriorCellLocation& cellIndex, bool
 
 int ESM::getLandSize(ESM::RefId worldspaceId)
 {
-    return isEsm4Ext(worldspaceId) ? ESM4::Land::VERTS_PER_SIDE : ESM::Land::LAND_SIZE;
+    return isEsm4Ext(worldspaceId) ? ESM4::Land::sVertsPerSide : ESM::Land::LAND_SIZE;
 }

components/esm3/landrecorddata.hpp

@@ -0,0 +1,48 @@
+#ifndef OPENMW_COMPONENTS_ESM3_LANDRECORDDATA_H
+#define OPENMW_COMPONENTS_ESM3_LANDRECORDDATA_H
+
+#include <cstdint>
+
+namespace ESM
+{
+    struct LandRecordData
+    {
+        // number of vertices per side
+        static constexpr unsigned sLandSize = 65;
+
+        // total number of vertices
+        static constexpr unsigned sLandNumVerts = sLandSize * sLandSize;
+
+        // number of textures per side of land
+        static constexpr unsigned sLandTextureSize = 16;
+
+        // total number of textures per land
+        static constexpr unsigned sLandNumTextures = sLandTextureSize * sLandTextureSize;
+
+        // Initial reference height for the first vertex, only needed for filling mHeights
+        float mHeightOffset = 0;
+        // Height in world space for each vertex
+        float mHeights[sLandNumVerts];
+        float mMinHeight = 0;
+        float mMaxHeight = 0;
+
+        // 24-bit normals, these aren't always correct though. Edge and corner normals may be garbage.
+        std::int8_t mNormals[sLandNumVerts * 3];
+
+        // 2D array of texture indices. An index can be used to look up an LandTexture,
+        // but to do so you must subtract 1 from the index first!
+        // An index of 0 indicates the default texture.
+        std::uint16_t mTextures[sLandNumTextures];
+
+        // 24-bit RGB color for each vertex
+        std::uint8_t mColours[3 * sLandNumVerts];
+
+        // ???
+        std::uint16_t mUnk1 = 0;
+        std::uint8_t mUnk2 = 0;
+
+        int mDataLoaded = 0;
+    };
+}
+
+#endif

components/esm3/loadland.cpp

@@ -1,6 +1,7 @@
 #include "loadland.hpp"
 
 #include <algorithm>
+#include <cstdint>
 #include <limits>
 #include <utility>
 
@@ -10,28 +11,31 @@
 
 namespace ESM
 {
-    Land::Land()
+    namespace
-        : mFlags(0)
-        , mX(0)
-        , mY(0)
-        , mDataTypes(0)
-        , mLandData(nullptr)
     {
-    }
+        void transposeTextureData(const std::uint16_t* in, std::uint16_t* out)
+        {
+            int readPos = 0; // bit ugly, but it works
+            for (int y1 = 0; y1 < 4; y1++)
+                for (int x1 = 0; x1 < 4; x1++)
+                    for (int y2 = 0; y2 < 4; y2++)
+                        for (int x2 = 0; x2 < 4; x2++)
+                            out[(y1 * 4 + y2) * 16 + (x1 * 4 + x2)] = in[readPos++];
+        }
 
-    void transposeTextureData(const uint16_t* in, uint16_t* out)
+        // Loads data and marks it as loaded. Return true if data is actually loaded from reader, false otherwise
-    {
+        // including the case when data is already loaded.
-        int readPos = 0; // bit ugly, but it works
+        bool condLoad(ESMReader& reader, int flags, int& targetFlags, int dataFlag, void* ptr, unsigned int size)
-        for (int y1 = 0; y1 < 4; y1++)
+        {
-            for (int x1 = 0; x1 < 4; x1++)
+            if ((targetFlags & dataFlag) == 0 && (flags & dataFlag) != 0)
-                for (int y2 = 0; y2 < 4; y2++)
+            {
-                    for (int x2 = 0; x2 < 4; x2++)
+                reader.getHExact(ptr, size);
-                        out[(y1 * 4 + y2) * 16 + (x1 * 4 + x2)] = in[readPos++];
+                targetFlags |= dataFlag;
-    }
+                return true;
-
+            }
-    Land::~Land()
+            reader.skipHSubSize(size);
-    {
+            return false;
-        delete mLandData;
+        }
     }
 
     void Land::load(ESMReader& esm, bool& isDeleted)
@@ -49,8 +53,8 @@ namespace ESM
                     esm.getSubHeader();
                     if (esm.getSubSize() != 8)
                         esm.fail("Subrecord size is not equal to 8");
-                    esm.getT<int>(mX);
+                    esm.getT(mX);
-                    esm.getT<int>(mY);
+                    esm.getT(mY);
                     hasLocation = true;
                     break;
                 case fourCC("DATA"):
@@ -90,7 +94,7 @@ namespace ESM
                     mDataTypes |= DATA_VHGT;
                     break;
                 case fourCC("WNAM"):
-                    esm.getHExact(mWnam, sizeof(mWnam));
+                    esm.getHExact(mWnam.data(), mWnam.size());
                     mDataTypes |= DATA_WNAM;
                     break;
                 case fourCC("VCLR"):
@@ -141,7 +145,8 @@ namespace ESM
                 for (int i = 0; i < LAND_SIZE; ++i)
                 {
                     float diff = (mLandData->mHeights[number] - prevY) / HEIGHT_SCALE;
-                    offsets.mHeightData[number] = (diff >= 0) ? (int8_t)(diff + 0.5) : (int8_t)(diff - 0.5);
+                    offsets.mHeightData[number]
+                        = diff >= 0 ? static_cast<std::int8_t>(diff + 0.5) : static_cast<std::int8_t>(diff - 0.5);
 
                     float prevX = prevY = mLandData->mHeights[number];
                     ++number;
@@ -149,7 +154,8 @@ namespace ESM
                     for (int j = 1; j < LAND_SIZE; ++j)
                     {
                         diff = (mLandData->mHeights[number] - prevX) / HEIGHT_SCALE;
-                        offsets.mHeightData[number] = (diff >= 0) ? (int8_t)(diff + 0.5) : (int8_t)(diff - 0.5);
+                        offsets.mHeightData[number]
+                            = diff >= 0 ? static_cast<std::int8_t>(diff + 0.5) : static_cast<std::int8_t>(diff - 0.5);
 
                         prevX = mLandData->mHeights[number];
                         ++number;
@@ -160,18 +166,19 @@ namespace ESM
             if (mDataTypes & Land::DATA_WNAM)
             {
                 // Generate WNAM record
-                signed char wnam[LAND_GLOBAL_MAP_LOD_SIZE];
+                std::int8_t wnam[LAND_GLOBAL_MAP_LOD_SIZE];
-                constexpr float max = std::numeric_limits<signed char>::max();
+                constexpr float max = std::numeric_limits<std::int8_t>::max();
-                constexpr float min = std::numeric_limits<signed char>::min();
+                constexpr float min = std::numeric_limits<std::int8_t>::min();
                 constexpr float vertMult = static_cast<float>(Land::LAND_SIZE - 1) / LAND_GLOBAL_MAP_LOD_SIZE_SQRT;
                 for (int row = 0; row < LAND_GLOBAL_MAP_LOD_SIZE_SQRT; ++row)
                 {
                     for (int col = 0; col < LAND_GLOBAL_MAP_LOD_SIZE_SQRT; ++col)
                     {
-                        float height = mLandData->mHeights[int(row * vertMult) * Land::LAND_SIZE + int(col * vertMult)];
+                        float height = mLandData->mHeights[static_cast<int>(row * vertMult) * Land::LAND_SIZE
+                            + static_cast<int>(col * vertMult)];
                         height /= height > 0 ? 128.f : 16.f;
                         height = std::clamp(height, min, max);
-                        wnam[row * LAND_GLOBAL_MAP_LOD_SIZE_SQRT + col] = static_cast<signed char>(height);
+                        wnam[row * LAND_GLOBAL_MAP_LOD_SIZE_SQRT + col] = static_cast<std::int8_t>(height);
                     }
                 }
                 esm.writeHNT("WNAM", wnam);
@@ -195,8 +202,8 @@ namespace ESM
 
         std::fill(std::begin(mWnam), std::end(mWnam), 0);
 
-        if (!mLandData)
+        if (mLandData == nullptr)
-            mLandData = new LandData;
+            mLandData = std::make_unique<LandData>();
 
         mLandData->mHeightOffset = 0;
         std::fill(std::begin(mLandData->mHeights), std::end(mLandData->mHeights), 0);
@@ -220,31 +227,29 @@ namespace ESM
         mContext.filename.clear();
     }
 
-    void Land::loadData(int flags, LandData* target) const
+    void Land::loadData(int flags) const
     {
-        // Create storage if nothing is loaded
+        if (mLandData == nullptr)
-        if (!target && !mLandData)
+            mLandData = std::make_unique<LandData>();
-        {
-            mLandData = new LandData;
-        }
 
-        if (!target)
+        loadData(flags, *mLandData);
-            target = mLandData;
+    }
 
+    void Land::loadData(int flags, LandData& data) const
+    {
         // Try to load only available data
         flags = flags & mDataTypes;
         // Return if all required data is loaded
-        if ((target->mDataLoaded & flags) == flags)
+        if ((data.mDataLoaded & flags) == flags)
         {
             return;
         }
 
-        // Copy data to target if no file
         if (mContext.filename.empty())
         {
             // Make sure there is data, and that it doesn't point to the same object.
-            if (mLandData && mLandData != target)
+            if (mLandData != nullptr && mLandData.get() != &data)
-                *target = *mLandData;
+                data = *mLandData;
 
             return;
         }
@@ -254,41 +259,41 @@ namespace ESM
 
         if (reader.isNextSub("VNML"))
         {
-            condLoad(reader, flags, target->mDataLoaded, DATA_VNML, target->mNormals, sizeof(target->mNormals));
+            condLoad(reader, flags, data.mDataLoaded, DATA_VNML, data.mNormals, sizeof(data.mNormals));
         }
 
         if (reader.isNextSub("VHGT"))
         {
             VHGT vhgt;
-            if (condLoad(reader, flags, target->mDataLoaded, DATA_VHGT, &vhgt, sizeof(vhgt)))
+            if (condLoad(reader, flags, data.mDataLoaded, DATA_VHGT, &vhgt, sizeof(vhgt)))
             {
-                target->mMinHeight = std::numeric_limits<float>::max();
+                data.mMinHeight = std::numeric_limits<float>::max();
-                target->mMaxHeight = -std::numeric_limits<float>::max();
+                data.mMaxHeight = -std::numeric_limits<float>::max();
                 float rowOffset = vhgt.mHeightOffset;
                 for (int y = 0; y < LAND_SIZE; y++)
                 {
                     rowOffset += vhgt.mHeightData[y * LAND_SIZE];
 
-                    target->mHeights[y * LAND_SIZE] = rowOffset * HEIGHT_SCALE;
+                    data.mHeights[y * LAND_SIZE] = rowOffset * HEIGHT_SCALE;
-                    if (rowOffset * HEIGHT_SCALE > target->mMaxHeight)
+                    if (rowOffset * HEIGHT_SCALE > data.mMaxHeight)
-                        target->mMaxHeight = rowOffset * HEIGHT_SCALE;
+                        data.mMaxHeight = rowOffset * HEIGHT_SCALE;
-                    if (rowOffset * HEIGHT_SCALE < target->mMinHeight)
+                    if (rowOffset * HEIGHT_SCALE < data.mMinHeight)
-                        target->mMinHeight = rowOffset * HEIGHT_SCALE;
+                        data.mMinHeight = rowOffset * HEIGHT_SCALE;
 
                     float colOffset = rowOffset;
                     for (int x = 1; x < LAND_SIZE; x++)
                     {
                         colOffset += vhgt.mHeightData[y * LAND_SIZE + x];
-                        target->mHeights[x + y * LAND_SIZE] = colOffset * HEIGHT_SCALE;
+                        data.mHeights[x + y * LAND_SIZE] = colOffset * HEIGHT_SCALE;
 
-                        if (colOffset * HEIGHT_SCALE > target->mMaxHeight)
+                        if (colOffset * HEIGHT_SCALE > data.mMaxHeight)
-                            target->mMaxHeight = colOffset * HEIGHT_SCALE;
+                            data.mMaxHeight = colOffset * HEIGHT_SCALE;
-                        if (colOffset * HEIGHT_SCALE < target->mMinHeight)
+                        if (colOffset * HEIGHT_SCALE < data.mMinHeight)
-                            target->mMinHeight = colOffset * HEIGHT_SCALE;
+                            data.mMinHeight = colOffset * HEIGHT_SCALE;
                     }
                 }
-                target->mUnk1 = vhgt.mUnk1;
+                data.mUnk1 = vhgt.mUnk1;
-                target->mUnk2 = vhgt.mUnk2;
+                data.mUnk2 = vhgt.mUnk2;
             }
         }
 
@@ -296,37 +301,20 @@ namespace ESM
             reader.skipHSub();
 
         if (reader.isNextSub("VCLR"))
-            condLoad(reader, flags, target->mDataLoaded, DATA_VCLR, target->mColours, 3 * LAND_NUM_VERTS);
+            condLoad(reader, flags, data.mDataLoaded, DATA_VCLR, data.mColours, 3 * LAND_NUM_VERTS);
         if (reader.isNextSub("VTEX"))
         {
             uint16_t vtex[LAND_NUM_TEXTURES];
-            if (condLoad(reader, flags, target->mDataLoaded, DATA_VTEX, vtex, sizeof(vtex)))
+            if (condLoad(reader, flags, data.mDataLoaded, DATA_VTEX, vtex, sizeof(vtex)))
             {
-                transposeTextureData(vtex, target->mTextures);
+                transposeTextureData(vtex, data.mTextures);
             }
         }
     }
 
-    void Land::unloadData() const
+    void Land::unloadData()
     {
-        if (mLandData)
+        mLandData = nullptr;
-        {
-            delete mLandData;
-            mLandData = nullptr;
-        }
-    }
-
-    bool Land::condLoad(
-        ESMReader& reader, int flags, int& targetFlags, int dataFlag, void* ptr, unsigned int size) const
-    {
-        if ((targetFlags & dataFlag) == 0 && (flags & dataFlag) != 0)
-        {
-            reader.getHExact(ptr, size);
-            targetFlags |= dataFlag;
-            return true;
-        }
-        reader.skipHSubSize(size);
-        return false;
     }
 
     bool Land::isDataLoaded(int flags) const
@@ -340,70 +328,33 @@ namespace ESM
         , mY(land.mY)
         , mContext(land.mContext)
         , mDataTypes(land.mDataTypes)
-        , mLandData(land.mLandData ? new LandData(*land.mLandData) : nullptr)
+        , mWnam(land.mWnam)
+        , mLandData(land.mLandData != nullptr ? std::make_unique<LandData>(*land.mLandData) : nullptr)
     {
-        std::copy(land.mWnam, land.mWnam + LAND_GLOBAL_MAP_LOD_SIZE, mWnam);
     }
 
     Land& Land::operator=(const Land& land)
     {
-        Land tmp(land);
+        Land copy(land);
-        swap(tmp);
+        *this = std::move(copy);
         return *this;
     }
 
-    void Land::swap(Land& land)
-    {
-        std::swap(mFlags, land.mFlags);
-        std::swap(mX, land.mX);
-        std::swap(mY, land.mY);
-        std::swap(mContext, land.mContext);
-        std::swap(mDataTypes, land.mDataTypes);
-        std::swap(mLandData, land.mLandData);
-        std::swap(mWnam, land.mWnam);
-    }
-
     const Land::LandData* Land::getLandData(int flags) const
     {
         if (!(flags & mDataTypes))
             return nullptr;
 
         loadData(flags);
-        return mLandData;
+        return mLandData.get();
-    }
-
-    const Land::LandData* Land::getLandData() const
-    {
-        return mLandData;
-    }
-
-    Land::LandData* Land::getLandData()
-    {
-        return mLandData;
     }
 
     void Land::add(int flags)
     {
-        if (!mLandData)
+        if (mLandData == nullptr)
-            mLandData = new LandData;
+            mLandData = std::make_unique<LandData>();
 
         mDataTypes |= flags;
         mLandData->mDataLoaded |= flags;
     }
-
-    void Land::remove(int flags)
-    {
-        mDataTypes &= ~flags;
-
-        if (mLandData)
-        {
-            mLandData->mDataLoaded &= ~flags;
-
-            if (!mLandData->mDataLoaded)
-            {
-                delete mLandData;
-                mLandData = nullptr;
-            }
-        }
-    }
 }

components/esm3/loadland.hpp

@@ -1,13 +1,17 @@
 #ifndef OPENMW_ESM_LAND_H
 #define OPENMW_ESM_LAND_H
 
-#include <stdint.h>
+#include <array>
+#include <cstdint>
+#include <memory>
 
 #include <components/misc/constants.hpp>
 
 #include "components/esm/defs.hpp"
 #include "components/esm/esmcommon.hpp"
 
+#include "landrecorddata.hpp"
+
 namespace ESM
 {
 
@@ -25,12 +29,21 @@ namespace ESM
         /// Return a string descriptor for this record type. Currently used for debugging / error logs only.
         static std::string_view getRecordType() { return "Land"; }
 
-        Land();
+        Land() = default;
-        ~Land();
 
-        int mFlags; // Only first four bits seem to be used, don't know what
+        Land(const Land& land);
-        // they mean.
+
-        int mX, mY; // Map coordinates.
+        Land(Land&& other) = default;
+
+        Land& operator=(const Land& land);
+
+        Land& operator=(Land&& land) = default;
+
+        // Only first four bits seem to be used, don't know what they mean.
+        std::uint32_t mFlags = 0;
+        // Map coordinates.
+        std::int32_t mX = 0;
+        std::int32_t mY = 0;
 
         // Plugin index, used to reference the correct material palette.
         int getPlugin() const { return mContext.index; }
@@ -42,7 +55,7 @@ namespace ESM
         // in which case the filename will be empty.
         ESM_Context mContext;
 
-        int mDataTypes;
+        int mDataTypes = 0;
 
         enum
         {
@@ -57,21 +70,21 @@ namespace ESM
         static constexpr int DEFAULT_HEIGHT = -2048;
 
         // number of vertices per side
-        static constexpr int LAND_SIZE = 65;
+        static constexpr int LAND_SIZE = LandRecordData::sLandSize;
 
         // cell terrain size in world coords
         static constexpr int REAL_SIZE = Constants::CellSizeInUnits;
 
         // total number of vertices
-        static constexpr int LAND_NUM_VERTS = LAND_SIZE * LAND_SIZE;
+        static constexpr int LAND_NUM_VERTS = LandRecordData::sLandNumVerts;
 
         static constexpr int HEIGHT_SCALE = 8;
 
         // number of textures per side of land
-        static constexpr int LAND_TEXTURE_SIZE = 16;
+        static constexpr int LAND_TEXTURE_SIZE = LandRecordData::sLandTextureSize;
 
         // total number of textures per land
-        static constexpr int LAND_NUM_TEXTURES = LAND_TEXTURE_SIZE * LAND_TEXTURE_SIZE;
+        static constexpr int LAND_NUM_TEXTURES = LandRecordData::sLandNumTextures;
 
         static constexpr int LAND_GLOBAL_MAP_LOD_SIZE = 81;
 
@@ -81,108 +94,58 @@ namespace ESM
         struct VHGT
         {
             float mHeightOffset;
-            int8_t mHeightData[LAND_NUM_VERTS];
+            std::int8_t mHeightData[LAND_NUM_VERTS];
-            short mUnk1;
+            std::uint16_t mUnk1;
-            char mUnk2;
+            std::uint8_t mUnk2;
         };
 #pragma pack(pop)
 
-        struct LandData
+        using LandData = ESM::LandRecordData;
-        {
-            typedef signed char VNML;
-
-            LandData()
-                : mHeightOffset(0)
-                , mMinHeight(0)
-                , mMaxHeight(0)
-                , mUnk1(0)
-                , mUnk2(0)
-                , mDataLoaded(0)
-            {
-            }
-
-            // Initial reference height for the first vertex, only needed for filling mHeights
-            float mHeightOffset;
-            // Height in world space for each vertex
-            float mHeights[LAND_NUM_VERTS];
-            float mMinHeight;
-            float mMaxHeight;
-
-            // 24-bit normals, these aren't always correct though. Edge and corner normals may be garbage.
-            VNML mNormals[LAND_NUM_VERTS * 3];
-
-            // 2D array of texture indices. An index can be used to look up an LandTexture,
-            // but to do so you must subtract 1 from the index first!
-            // An index of 0 indicates the default texture.
-            uint16_t mTextures[LAND_NUM_TEXTURES];
-
-            // 24-bit RGB color for each vertex
-            unsigned char mColours[3 * LAND_NUM_VERTS];
-
-            // ???
-            short mUnk1;
-            uint8_t mUnk2;
-
-            int mDataLoaded;
-        };
 
         // low-LOD heightmap (used for rendering the global map)
-        signed char mWnam[LAND_GLOBAL_MAP_LOD_SIZE];
+        std::array<std::int8_t, LAND_GLOBAL_MAP_LOD_SIZE> mWnam;
 
         void load(ESMReader& esm, bool& isDeleted);
         void save(ESMWriter& esm, bool isDeleted = false) const;
 
         void blank();
 
-        /**
+        void loadData(int flags) const;
-         * Actually loads data into target
+
-         * If target is nullptr, assumed target is mLandData
+        void loadData(int flags, LandData& data) const;
-         */
-        void loadData(int flags, LandData* target = nullptr) const;
 
         /**
          * Frees memory allocated for mLandData
          */
-        void unloadData() const;
+        void unloadData();
 
         /// Check if given data type is loaded
         bool isDataLoaded(int flags) const;
 
-        /// Sets the flags and creates a LandData if needed
-        void setDataLoaded(int flags);
-
-        Land(const Land& land);
-
-        Land& operator=(const Land& land);
-
-        void swap(Land& land);
-
         /// Return land data with at least the data types specified in \a flags loaded (if they
         /// are available). Will return a 0-pointer if there is no data for any of the
         /// specified types.
         const LandData* getLandData(int flags) const;
 
         /// Return land data without loading first anything. Can return a 0-pointer.
-        const LandData* getLandData() const;
+        const LandData* getLandData() const
+        {
+            return mLandData.get();
+        }
 
         /// Return land data without loading first anything. Can return a 0-pointer.
-        LandData* getLandData();
+        LandData* getLandData()
+        {
+            return mLandData.get();
+        }
 
         /// \attention Must not be called on objects that aren't fully loaded.
         ///
         /// \note Added data fields will be uninitialised
         void add(int flags);
 
-        /// \attention Must not be called on objects that aren't fully loaded.
-        void remove(int flags);
-
     private:
-        /// Loads data and marks it as loaded
+        mutable std::unique_ptr<LandData> mLandData;
-        /// \return true if data is actually loaded from file, false otherwise
-        /// including the case when data is already loaded
-        bool condLoad(ESMReader& reader, int flags, int& targetFlags, int dataFlag, void* ptr, unsigned int size) const;
-
-        mutable LandData* mLandData;
     };
 
 }

components/esm4/loadland.cpp

@@ -117,7 +117,7 @@ void ESM4::Land::load(ESM4::Reader& reader)
                 if (currentAddQuad != -1)
                 {
                     // FIXME: sometimes there are no VTXT following an ATXT?  Just add a dummy one for now
-                    Log(Debug::Verbose) << "ESM4::Land VTXT empty layer " << (int)layer.texture.layerIndex;
+                    Log(Debug::Verbose) << "ESM4::Land VTXT empty layer " << layer.texture.layerIndex;
                     mTextures[currentAddQuad].layers.push_back(layer);
                 }
                 reader.get(layer.texture);
@@ -149,7 +149,7 @@ void ESM4::Land::load(ESM4::Reader& reader)
                 if (currentAddQuad == -1)
                     throw std::runtime_error("VTXT without ATXT found");
 
-                int count = (int)reader.subRecordHeader().dataSize / sizeof(ESM4::Land::VTXT);
+                const std::uint16_t count = reader.subRecordHeader().dataSize / sizeof(ESM4::Land::VTXT);
                 if ((reader.subRecordHeader().dataSize % sizeof(ESM4::Land::VTXT)) != 0)
                     throw std::runtime_error("ESM4::LAND VTXT data size error");
 
@@ -179,7 +179,7 @@ void ESM4::Land::load(ESM4::Reader& reader)
             }
             case ESM4::SUB_VTEX: // only in Oblivion?
             {
-                int count = (int)reader.subRecordHeader().dataSize / sizeof(ESM::FormId32);
+                const std::uint16_t count = reader.subRecordHeader().dataSize / sizeof(ESM::FormId32);
                 if ((reader.subRecordHeader().dataSize % sizeof(ESM::FormId32)) != 0)
                     throw std::runtime_error("ESM4::LAND VTEX data size error");
 
@@ -202,8 +202,8 @@ void ESM4::Land::load(ESM4::Reader& reader)
     if (currentAddQuad != -1)
     {
         // FIXME: not sure if it happens here as well
-        Log(Debug::Verbose) << "ESM4::Land VTXT empty layer " << (int)layer.texture.layerIndex << " quad "
+        Log(Debug::Verbose) << "ESM4::Land VTXT empty layer " << layer.texture.layerIndex << " quad "
-                            << (int)layer.texture.quadrant;
+                            << static_cast<unsigned>(layer.texture.quadrant);
         mTextures[currentAddQuad].layers.push_back(layer);
     }
 

components/esm4/loadland.hpp

@@ -49,27 +49,27 @@ namespace ESM4
         };
 
         // number of vertices per side
-        static const int VERTS_PER_SIDE = 33;
+        static constexpr unsigned sVertsPerSide = 33;
 
         // cell terrain size in world coords
-        static const int REAL_SIZE = 4096;
+        static constexpr unsigned sRealSize = 4096;
 
         // total number of vertices
-        static const int LAND_NUM_VERTS = VERTS_PER_SIDE * VERTS_PER_SIDE;
+        static constexpr unsigned sLandNumVerts = sVertsPerSide * sVertsPerSide;
 
-        static const int HEIGHT_SCALE = 8;
+        static constexpr unsigned sHeightScale = 8;
 
         // number of textures per side of a land quadrant
         // (for TES4 - based on vanilla observations)
-        static const int QUAD_TEXTURE_PER_SIDE = 6;
+        static constexpr unsigned sQuadTexturePerSide = 6;
 
 #pragma pack(push, 1)
         struct VHGT
         {
             float heightOffset;
-            std::int8_t gradientData[VERTS_PER_SIDE * VERTS_PER_SIDE];
+            std::int8_t gradientData[sVertsPerSide * sVertsPerSide];
             std::uint16_t unknown1;
-            unsigned char unknown2;
+            std::uint8_t unknown2;
         };
 
         struct BTXT
@@ -117,19 +117,20 @@ namespace ESM4
         // FIXME: lazy loading not yet implemented
         int mDataTypes; // which data types are loaded
 
-        float mHeights[VERTS_PER_SIDE * VERTS_PER_SIDE]; // Float value of compressed Heightmap
+        float mHeights[sVertsPerSide * sVertsPerSide]; // Float value of compressed Heightmap
-        float mMinHeight, mMaxHeight;
+        std::int8_t mVertNorm[sVertsPerSide * sVertsPerSide * 3]; // from VNML subrecord
-        signed char mVertNorm[VERTS_PER_SIDE * VERTS_PER_SIDE * 3]; // from VNML subrecord
+        std::uint8_t mVertColr[sVertsPerSide * sVertsPerSide * 3]; // from VCLR subrecord
-        unsigned char mVertColr[VERTS_PER_SIDE * VERTS_PER_SIDE * 3]; // from VCLR subrecord
         VHGT mHeightMap;
         Texture mTextures[4]; // 0 = bottom left, 1 = bottom right, 2 = top left, 3 = top right
         std::vector<ESM::FormId> mIds; // land texture (LTEX) formids
         ESM::FormId mCell;
+
         void load(Reader& reader);
-        Land() = default;
+
         // void save(Writer& writer) const;
 
         // void blank();
+
         static constexpr ESM::RecNameInts sRecordId = ESM::REC_LAND4;
     };
 }

components/esmterrain/storage.cpp

@@ -8,6 +8,7 @@
 
 #include <components/debug/debuglog.hpp>
 #include <components/esm/esmterrain.hpp>
+#include <components/esm3/loadland.hpp>
 #include <components/esm4/loadland.hpp>
 #include <components/misc/resourcehelpers.hpp>
 #include <components/misc/strings/algorithm.hpp>
@@ -46,30 +47,21 @@ namespace ESMTerrain
         Map mMap;
     };
 
-    LandObject::LandObject()
+    LandObject::LandObject(const ESM4::Land& land, int loadFlags)
-        : mLand(nullptr)
+        : mData(land, loadFlags)
     {
     }
 
-    LandObject::LandObject(const ESM4::Land* land, int loadFlags)
+    LandObject::LandObject(const ESM::Land& land, int loadFlags)
-        : mLand(nullptr)
+        : mData(land, loadFlags)
-        , mData(*land, loadFlags)
     {
     }
 
-    LandObject::LandObject(const ESM::Land* land, int loadFlags)
+    LandObject::LandObject(const LandObject& /*copy*/, const osg::CopyOp& /*copyOp*/)
-        : mLand(land)
-        , mData(*land, loadFlags)
     {
+        throw std::logic_error("LandObject copy constructor is not implemented");
     }
 
-    LandObject::LandObject(const LandObject& copy, const osg::CopyOp& copyop)
-        : mLand(nullptr)
-    {
-    }
-
-    LandObject::~LandObject() {}
-
     const float defaultHeight = ESM::Land::DEFAULT_HEIGHT;
 
     Storage::Storage(const VFS::Manager* vfs, const std::string& normalMapPattern,
@@ -232,20 +224,29 @@ namespace ESMTerrain
         const osg::Vec2f origin = center - osg::Vec2f(size, size) * 0.5f;
         const int startCellX = static_cast<int>(std::floor(origin.x()));
         const int startCellY = static_cast<int>(std::floor(origin.y()));
-        ESM::ExteriorCellLocation lastCellLocation(startCellX - 1, startCellY - 1, worldspace);
+        std::pair lastCell{ startCellX, startCellY };
-        const LandObject* land = nullptr;
+        const LandObject* land = getLand(ESM::ExteriorCellLocation(startCellX, startCellY, worldspace), cache);
         const ESM::LandData* heightData = nullptr;
         const ESM::LandData* normalData = nullptr;
         const ESM::LandData* colourData = nullptr;
         bool validHeightDataExists = false;
 
+        if (land != nullptr)
+        {
+            heightData = land->getData(ESM::Land::DATA_VHGT);
+            normalData = land->getData(ESM::Land::DATA_VNML);
+            colourData = land->getData(ESM::Land::DATA_VCLR);
+            validHeightDataExists = true;
+        }
+
         const auto handleSample = [&](std::size_t cellShiftX, std::size_t cellShiftY, std::size_t row, std::size_t col,
                                       std::size_t vertX, std::size_t vertY) {
             const int cellX = startCellX + cellShiftX;
             const int cellY = startCellY + cellShiftY;
+            const std::pair cell{ cellX, cellY };
             const ESM::ExteriorCellLocation cellLocation(cellX, cellY, worldspace);
 
-            if (lastCellLocation != cellLocation)
+            if (lastCell != cell)
             {
                 land = getLand(cellLocation, cache);
 
@@ -261,7 +262,7 @@ namespace ESMTerrain
                     validHeightDataExists = true;
                 }
 
-                lastCellLocation = cellLocation;
+                lastCell = cell;
             }
 
             float height = defaultHeight;

components/esmterrain/storage.hpp

@@ -8,7 +8,6 @@
 
 #include <components/esm/esmterrain.hpp>
 #include <components/esm/util.hpp>
-#include <components/esm3/loadland.hpp>
 #include <components/esm3/loadltex.hpp>
 
 namespace ESM4
@@ -36,30 +35,26 @@ namespace ESMTerrain
     class LandObject : public osg::Object
     {
     public:
-        LandObject();
+        LandObject() = default;
-        LandObject(const ESM::Land* land, int loadFlags);
+        LandObject(const ESM::Land& land, int loadFlags);
-        LandObject(const ESM4::Land* land, int loadFlags);
+        LandObject(const ESM4::Land& land, int loadFlags);
-
-        LandObject(const LandObject& copy, const osg::CopyOp& copyop);
-        virtual ~LandObject();
 
         META_Object(ESMTerrain, LandObject)
 
-        inline const ESM::LandData* getData(int flags) const
+        const ESM::LandData* getData(int flags) const
         {
-            if ((mData.mLoadFlags & flags) != flags)
+            if ((mData.getLoadFlags() & flags) != flags)
                 return nullptr;
 
             return &mData;
         }
-        inline int getPlugin() const { return mLand->getPlugin(); }
+
-        inline int getLandSize() const { return mData.getLandSize(); }
+        int getPlugin() const { return mData.getPlugin(); }
-        inline int getRealSize() const { return mData.getSize(); }
 
     private:
-        const ESM::Land* mLand;
-
         ESM::LandData mData;
+
+        LandObject(const LandObject& copy, const osg::CopyOp& copyOp);
     };
 
     // Since plugins can define new texture palettes, we need to know the plugin index too