From f95cc6bfcac0284ddad573883196405b841b96df Mon Sep 17 00:00:00 2001 From: David Williams Date: Sun, 21 Sep 2014 17:50:35 +0200 Subject: [PATCH] Added comments and warnings. --- documentation/Threading.rst | 4 +++- .../PolyVoxCore/include/PolyVoxCore/LargeVolume.h | 2 ++ .../PolyVoxCore/include/PolyVoxCore/PagedVolume.h | 13 +++++++++---- .../PolyVoxCore/include/PolyVoxCore/SimpleVolume.h | 2 ++ 4 files changed, 16 insertions(+), 5 deletions(-) diff --git a/documentation/Threading.rst b/documentation/Threading.rst index c2d8991a..fc4d2245 100644 --- a/documentation/Threading.rst +++ b/documentation/Threading.rst @@ -27,8 +27,10 @@ C++ does provide the 'volatile' keyword which can be used to ensure a variable i Lastly, note that PolyVox volumes are templatised which means the voxel type might be something other than a simple int. However we don't think this actually makes a difference given that so few guarantees are made anyway, and it should still be safe to perform multiple concurrent reads for more complex types. -LargeVolume +PagedVolume ----------- +NOTE: The info below is based on LargeVolume, which PagedVolume has replaced. It is likely that the same limitations apply but this has not been well tested. We do intend to improve the thread safty of PagedVolume in the future. + The LargeVolume provides even less thread safety than the RawVolume, in that even concurrent read operations can cause problems. The reason for this is the more complex memory management which is performed behind the scenes, and which allows pieces of volume data to be moved around and deleted. For example, a read of a single voxel may mean that the block of data associated with that voxel has to be paged in to memory, which in turn may mean that another block of data has to be paged out of memory. If second thread was halfway through reading a voxel in this second block of data then a problem will occur. In the future we may do a more comprehensive analysis of thread safety in the LargeVolume, but for now you should assume that any multithreaded access can cause problems. diff --git a/library/PolyVoxCore/include/PolyVoxCore/LargeVolume.h b/library/PolyVoxCore/include/PolyVoxCore/LargeVolume.h index ff60e49f..a018e67c 100644 --- a/library/PolyVoxCore/include/PolyVoxCore/LargeVolume.h +++ b/library/PolyVoxCore/include/PolyVoxCore/LargeVolume.h @@ -1,6 +1,8 @@ #ifndef __PolyVox_LargeVolume_H__ #define __PolyVox_LargeVolume_H__ +#pragma message("WARNING - The LargeVolume class has been replaced by PagedVolume. Please use that instead.") + #include "PagedVolume.h" #include "PolyVoxForwardDeclarations.h" diff --git a/library/PolyVoxCore/include/PolyVoxCore/PagedVolume.h b/library/PolyVoxCore/include/PolyVoxCore/PagedVolume.h index b014c9a3..848486e2 100644 --- a/library/PolyVoxCore/include/PolyVoxCore/PagedVolume.h +++ b/library/PolyVoxCore/include/PolyVoxCore/PagedVolume.h @@ -118,6 +118,8 @@ namespace PolyVox class Chunk { + friend class PagedVolume; + public: Chunk(Vector3DInt32 v3dPosition, uint16_t uSideLength, Pager* pPager = nullptr); ~Chunk(); @@ -131,7 +133,7 @@ namespace PolyVox void setVoxelAt(uint16_t uXPos, uint16_t uYPos, uint16_t uZPos, VoxelType tValue); void setVoxelAt(const Vector3DUint16& v3dPos, VoxelType tValue); - public: + private: /// Private copy constructor to prevent accisdental copying Chunk(const Chunk& /*rhs*/) {}; @@ -152,6 +154,8 @@ namespace PolyVox uint16_t m_uSideLength; uint8_t m_uSideLengthPower; Pager* m_pPager; + + // Note: Do we really need to store this position here as well as in the block maps? Vector3DInt32 m_v3dChunkSpacePosition; }; @@ -300,9 +304,7 @@ namespace PolyVox private: - typedef std::unordered_map > SharedPtrChunkMap; - typedef std::unordered_map > WeakPtrChunkMap; - + // FIXME - We can probably ove this into the constructor void initialise(); // A trick to implement specialization of template member functions in template classes. See http://stackoverflow.com/a/4951057 @@ -328,6 +330,9 @@ namespace PolyVox // back in (not knowing the sampler still has it). This would result in two chunks in memory with the same position is space. // To avoid this, the 'all chunks' set tracks chunks with are used by the volume *and/or* the samplers. It holds weak pointers // so does not cause chunks to persist. + // + // TODO: Do we really need maps here? It means we are storing the chunk positions in the map, but they are also stored in the + // chunks themselves (so they can be passed to the pager from the chunks destructor). Can we use a set here? What is a better approach? typedef std::unordered_map > WeakPtrChunkMap; mutable WeakPtrChunkMap m_pAllChunks; typedef std::unordered_map > SharedPtrChunkMap; diff --git a/library/PolyVoxCore/include/PolyVoxCore/SimpleVolume.h b/library/PolyVoxCore/include/PolyVoxCore/SimpleVolume.h index 9967d874..84ecd46b 100644 --- a/library/PolyVoxCore/include/PolyVoxCore/SimpleVolume.h +++ b/library/PolyVoxCore/include/PolyVoxCore/SimpleVolume.h @@ -1,6 +1,8 @@ #ifndef __PolyVox_SimpleVolume_H__ #define __PolyVox_SimpleVolume_H__ +#pragma message("WARNING - The LargeVolume class has been replaced by PagedVolume. Please use that instead.") + #include "PagedVolume.h" #include "PolyVoxForwardDeclarations.h"