Load LandData into the LandObject to avoid threading conflicts when the same data is being loaded by two threads

apps/opencs/view/render/terrainstorage.cpp

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

components/esm/loadland.cpp

@@ -2,8 +2,6 @@
 
 #include <utility>
 
-#include <OpenThreads/ScopedLock>
-
 #include "esmreader.hpp"
 #include "esmwriter.hpp"
 #include "defs.hpp"
@@ -177,45 +175,48 @@ namespace ESM
 
     }
 
-    void Land::loadData(int flags) const
+    void Land::loadData(int flags, LandData* target) const
     {
-        OpenThreads::ScopedLock<OpenThreads::Mutex> lock(mMutex);
+        // Create storage if nothing is loaded
+        if (!target && !mLandData)
+        {
+            mLandData = new LandData;
+        }
+
+        if (!target)
+            target = mLandData;
 
         // Try to load only available data
         flags = flags & mDataTypes;
         // Return if all required data is loaded
-        if (mLandData && (mLandData->mDataLoaded & flags) == flags) {
+        if ((target->mDataLoaded & flags) == flags) {
             return;
         }
-        // Create storage if nothing is loaded
-        if (mLandData == NULL) {
-            mLandData = new LandData;
-        }
 
         ESM::ESMReader reader;
         reader.restoreContext(mContext);
 
         if (reader.isNextSub("VNML")) {
-            condLoad(reader, flags, DATA_VNML, mLandData->mNormals, sizeof(mLandData->mNormals));
+            condLoad(reader, flags, target->mDataLoaded, DATA_VNML, target->mNormals, sizeof(target->mNormals));
         }
 
         if (reader.isNextSub("VHGT")) {
             VHGT vhgt;
-            if (condLoad(reader, flags, DATA_VHGT, &vhgt, sizeof(vhgt))) {
+            if (condLoad(reader, flags, target->mDataLoaded, DATA_VHGT, &vhgt, sizeof(vhgt))) {
                 float rowOffset = vhgt.mHeightOffset;
                 for (int y = 0; y < LAND_SIZE; y++) {
                     rowOffset += vhgt.mHeightData[y * LAND_SIZE];
 
-                    mLandData->mHeights[y * LAND_SIZE] = rowOffset * HEIGHT_SCALE;
+                    target->mHeights[y * LAND_SIZE] = rowOffset * HEIGHT_SCALE;
 
                     float colOffset = rowOffset;
                     for (int x = 1; x < LAND_SIZE; x++) {
                         colOffset += vhgt.mHeightData[y * LAND_SIZE + x];
-                        mLandData->mHeights[x + y * LAND_SIZE] = colOffset * HEIGHT_SCALE;
+                        target->mHeights[x + y * LAND_SIZE] = colOffset * HEIGHT_SCALE;
                     }
                 }
-                mLandData->mUnk1 = vhgt.mUnk1;
+                target->mUnk1 = vhgt.mUnk1;
-                mLandData->mUnk2 = vhgt.mUnk2;
+                target->mUnk2 = vhgt.mUnk2;
             }
         }
 
@@ -223,11 +224,11 @@ namespace ESM
             reader.skipHSub();
 
         if (reader.isNextSub("VCLR"))
-            condLoad(reader, flags, DATA_VCLR, mLandData->mColours, 3 * LAND_NUM_VERTS);
+            condLoad(reader, flags, target->mDataLoaded, DATA_VCLR, target->mColours, 3 * LAND_NUM_VERTS);
         if (reader.isNextSub("VTEX")) {
             uint16_t vtex[LAND_NUM_TEXTURES];
-            if (condLoad(reader, flags, DATA_VTEX, vtex, sizeof(vtex))) {
+            if (condLoad(reader, flags, target->mDataLoaded, DATA_VTEX, vtex, sizeof(vtex))) {
-                transposeTextureData(vtex, mLandData->mTextures);
+                transposeTextureData(vtex, target->mTextures);
             }
         }
     }
@@ -241,11 +242,11 @@ namespace ESM
         }
     }
 
-    bool Land::condLoad(ESM::ESMReader& reader, int flags, int dataFlag, void *ptr, unsigned int size) const
+    bool Land::condLoad(ESM::ESMReader& reader, int flags, int& targetFlags, int dataFlag, void *ptr, unsigned int size) const
     {
-        if ((mLandData->mDataLoaded & dataFlag) == 0 && (flags & dataFlag) != 0) {
+        if ((targetFlags & dataFlag) == 0 && (flags & dataFlag) != 0) {
             reader.getHExact(ptr, size);
-            mLandData->mDataLoaded |= dataFlag;
+            targetFlags |= dataFlag;
             return true;
         }
         reader.skipHSubSize(size);
@@ -254,7 +255,6 @@ namespace ESM
 
     bool Land::isDataLoaded(int flags) const
     {
-        OpenThreads::ScopedLock<OpenThreads::Mutex> lock(mMutex);
         return mLandData && (mLandData->mDataLoaded & flags) == (flags & mDataTypes);
     }
 

components/esm/loadland.hpp

@@ -3,8 +3,6 @@
 
 #include <stdint.h>
 
-#include <OpenThreads/Mutex>
-
 #include "esmcommon.hpp"
 
 namespace ESM
@@ -117,12 +115,13 @@ struct Land
     void blank() {}
 
     /**
-     * Actually loads data
+     * Actually loads data into target
+     * If target is NULL, assumed target is mLandData
      */
-    void loadData(int flags) const;
+    void loadData(int flags, LandData* target = NULL) const;
 
     /**
-     * Frees memory allocated for land data
+     * Frees memory allocated for mLandData
      */
     void unloadData() const;
 
@@ -160,9 +159,7 @@ struct Land
         /// Loads data and marks it as loaded
         /// \return true if data is actually loaded from file, false otherwise
         /// including the case when data is already loaded
-        bool condLoad(ESM::ESMReader& reader, int flags, int dataFlag, void *ptr, unsigned int size) const;
+        bool condLoad(ESM::ESMReader& reader, int flags, int& targetFlags, int dataFlag, void *ptr, unsigned int size) const;
-
-        mutable OpenThreads::Mutex mMutex;
 
         mutable LandData *mLandData;
 };

components/esmterrain/storage.cpp

@@ -25,7 +25,7 @@ namespace ESMTerrain
         : mLand(land)
         , mLoadFlags(loadFlags)
     {
-        mLand->loadData(mLoadFlags);
+        mLand->loadData(mLoadFlags, &mData);
     }
 
     LandObject::LandObject(const LandObject &copy, const osg::CopyOp &copyop)
@@ -34,13 +34,13 @@ namespace ESMTerrain
 
     LandObject::~LandObject()
     {
-        if (mLand && mLoadFlags) // only unload if we were responsible for loading to begin with.
-            mLand->unloadData();
     }
 
     const ESM::Land::LandData *LandObject::getData(int flags) const
     {
-        return mLand->getLandData(flags);
+        if ((mData.mDataLoaded & flags) != flags)
+            return NULL;
+        return &mData;
     }
 
     int LandObject::getPlugin() const

components/esmterrain/storage.hpp

@@ -16,24 +16,25 @@ namespace VFS
 namespace ESMTerrain
 {
 
-    /// @brief Wrapper around ESM::Land with reference counting. The wrapper needs to be held as long as the data is still in use
+    /// @brief Wrapper around Land Data with reference counting. The wrapper needs to be held as long as the data is still in use
-    /// Data will be unloaded when wrapper object is deleted
     class LandObject : public osg::Object
     {
-    private:
-        const ESM::Land* mLand;
-        int mLoadFlags;
-
     public:
+        LandObject();
+        LandObject(const ESM::Land* land, int loadFlags);
+        LandObject(const LandObject& copy, const osg::CopyOp& copyop);
+        virtual ~LandObject();
+
         META_Object(ESMTerrain, LandObject)
 
         const ESM::Land::LandData* getData(int flags) const;
         int getPlugin() const;
 
-        LandObject();
+    private:
-        LandObject(const ESM::Land* land, int loadFlags);
+        const ESM::Land* mLand;
-        LandObject(const LandObject& copy, const osg::CopyOp& copyop);
+        int mLoadFlags;
-        virtual ~LandObject();
+
+        ESM::Land::LandData mData;
     };
 
     /// @brief Feeds data from ESM terrain records (ESM::Land, ESM::LandTexture)