From 596fe56bfdfe95dbc1a8783f65bf33c73694cd7e Mon Sep 17 00:00:00 2001 From: scrawl Date: Tue, 9 Feb 2016 20:14:16 +0100 Subject: [PATCH] Make Land::loadData thread safe --- apps/opencs/model/tools/mergestages.cpp | 2 -- apps/openmw/mwrender/terrainstorage.cpp | 3 ++ components/esm/loadland.cpp | 42 ++++++++++++++----------- components/esm/loadland.hpp | 7 +++-- 4 files changed, 31 insertions(+), 23 deletions(-) diff --git a/apps/opencs/model/tools/mergestages.cpp b/apps/opencs/model/tools/mergestages.cpp index 52e1e6964..f52e8886f 100644 --- a/apps/opencs/model/tools/mergestages.cpp +++ b/apps/opencs/model/tools/mergestages.cpp @@ -224,8 +224,6 @@ void CSMTools::MergeLandStage::perform (int stage, CSMDoc::Messages& messages) CSMWorld::Land newLand (land); - newLand.mEsm = 0; // avoid potential dangling pointer (ESMReader isn't needed anyway, - // because record is already fully loaded) newLand.mPlugin = 0; if (land.mDataTypes & ESM::Land::DATA_VTEX) diff --git a/apps/openmw/mwrender/terrainstorage.cpp b/apps/openmw/mwrender/terrainstorage.cpp index ed1d8b677..02947b5dd 100644 --- a/apps/openmw/mwrender/terrainstorage.cpp +++ b/apps/openmw/mwrender/terrainstorage.cpp @@ -62,6 +62,9 @@ namespace MWRender const int flags = ESM::Land::DATA_VCLR|ESM::Land::DATA_VHGT|ESM::Land::DATA_VNML|ESM::Land::DATA_VTEX; if (!land->isDataLoaded(flags)) land->loadData(flags); + + // TODO: unload land data when it's no longer needed + return land; } diff --git a/components/esm/loadland.cpp b/components/esm/loadland.cpp index e7be03321..b0b072232 100644 --- a/components/esm/loadland.cpp +++ b/components/esm/loadland.cpp @@ -2,6 +2,8 @@ #include +#include + #include "esmreader.hpp" #include "esmwriter.hpp" #include "defs.hpp" @@ -60,7 +62,6 @@ namespace ESM , mX(0) , mY(0) , mPlugin(0) - , mEsm(NULL) , mDataTypes(0) , mDataLoaded(false) , mLandData(NULL) @@ -86,8 +87,7 @@ namespace ESM { isDeleted = false; - mEsm = &esm; - mPlugin = mEsm->getIndex(); + mPlugin = esm.getIndex(); bool hasLocation = false; bool isLoaded = false; @@ -180,6 +180,8 @@ namespace ESM void Land::loadData(int flags) const { + OpenThreads::ScopedLock lock(mMutex); + // Try to load only available data flags = flags & mDataTypes; // Return if all required data is loaded @@ -191,15 +193,17 @@ namespace ESM mLandData = new LandData; mLandData->mDataTypes = mDataTypes; } - mEsm->restoreContext(mContext); - if (mEsm->isNextSub("VNML")) { - condLoad(flags, DATA_VNML, mLandData->mNormals, sizeof(mLandData->mNormals)); + ESM::ESMReader reader; + reader.restoreContext(mContext); + + if (reader.isNextSub("VNML")) { + condLoad(reader, flags, DATA_VNML, mLandData->mNormals, sizeof(mLandData->mNormals)); } - if (mEsm->isNextSub("VHGT")) { + if (reader.isNextSub("VHGT")) { static VHGT vhgt; - if (condLoad(flags, DATA_VHGT, &vhgt, sizeof(vhgt))) { + if (condLoad(reader, flags, DATA_VHGT, &vhgt, sizeof(vhgt))) { float rowOffset = vhgt.mHeightOffset; for (int y = 0; y < LAND_SIZE; y++) { rowOffset += vhgt.mHeightData[y * LAND_SIZE]; @@ -217,14 +221,14 @@ namespace ESM } } - if (mEsm->isNextSub("WNAM")) { - condLoad(flags, DATA_WNAM, mLandData->mWnam, 81); + if (reader.isNextSub("WNAM")) { + condLoad(reader, flags, DATA_WNAM, mLandData->mWnam, 81); } - if (mEsm->isNextSub("VCLR")) - condLoad(flags, DATA_VCLR, mLandData->mColours, 3 * LAND_NUM_VERTS); - if (mEsm->isNextSub("VTEX")) { + if (reader.isNextSub("VCLR")) + condLoad(reader, flags, DATA_VCLR, mLandData->mColours, 3 * LAND_NUM_VERTS); + if (reader.isNextSub("VTEX")) { static uint16_t vtex[LAND_NUM_TEXTURES]; - if (condLoad(flags, DATA_VTEX, vtex, sizeof(vtex))) { + if (condLoad(reader, flags, DATA_VTEX, vtex, sizeof(vtex))) { LandData::transposeTextureData(vtex, mLandData->mTextures); } } @@ -240,25 +244,26 @@ namespace ESM } } - bool Land::condLoad(int flags, int dataFlag, void *ptr, unsigned int size) const + bool Land::condLoad(ESM::ESMReader& reader, int flags, int dataFlag, void *ptr, unsigned int size) const { if ((mDataLoaded & dataFlag) == 0 && (flags & dataFlag) != 0) { - mEsm->getHExact(ptr, size); + reader.getHExact(ptr, size); mDataLoaded |= dataFlag; return true; } - mEsm->skipHSubSize(size); + reader.skipHSubSize(size); return false; } bool Land::isDataLoaded(int flags) const { + OpenThreads::ScopedLock lock(mMutex); return (mDataLoaded & flags) == (flags & mDataTypes); } Land::Land (const Land& land) : mFlags (land.mFlags), mX (land.mX), mY (land.mY), mPlugin (land.mPlugin), - mEsm (land.mEsm), mContext (land.mContext), mDataTypes (land.mDataTypes), + mContext (land.mContext), mDataTypes (land.mDataTypes), mDataLoaded (land.mDataLoaded), mLandData (land.mLandData ? new LandData (*land.mLandData) : 0) {} @@ -275,7 +280,6 @@ namespace ESM std::swap (mX, land.mX); std::swap (mY, land.mY); std::swap (mPlugin, land.mPlugin); - std::swap (mEsm, land.mEsm); std::swap (mContext, land.mContext); std::swap (mDataTypes, land.mDataTypes); std::swap (mDataLoaded, land.mDataLoaded); diff --git a/components/esm/loadland.hpp b/components/esm/loadland.hpp index 65ac88cda..8a8d6fdd2 100644 --- a/components/esm/loadland.hpp +++ b/components/esm/loadland.hpp @@ -3,6 +3,8 @@ #include +#include + #include "esmcommon.hpp" namespace ESM @@ -31,7 +33,6 @@ struct Land // File context. This allows the ESM reader to be 'reset' to this // location later when we are ready to load the full data set. - ESMReader* mEsm; ESM_Context mContext; int mDataTypes; @@ -155,7 +156,9 @@ 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(int flags, int dataFlag, void *ptr, unsigned int size) const; + bool condLoad(ESM::ESMReader& reader, int flags, int dataFlag, void *ptr, unsigned int size) const; + + mutable OpenThreads::Mutex mMutex; mutable int mDataLoaded;