From df68b1fe7abc4d1b7800c866c1b9dc17523652aa Mon Sep 17 00:00:00 2001 From: David Williams Date: Fri, 10 Apr 2009 08:56:37 +0000 Subject: [PATCH] Work on volume memory management. --- examples/OpenGL/main.cpp | 2 + .../PolyVoxCore/include/PolyVoxImpl/Block.h | 1 - .../include/PolyVoxImpl/BlockData.inl | 8 +-- library/PolyVoxCore/include/Volume.h | 15 ++++- library/PolyVoxCore/include/Volume.inl | 67 ++++++++++--------- 5 files changed, 53 insertions(+), 40 deletions(-) diff --git a/examples/OpenGL/main.cpp b/examples/OpenGL/main.cpp index 684c13b1..93c05f8d 100644 --- a/examples/OpenGL/main.cpp +++ b/examples/OpenGL/main.cpp @@ -66,6 +66,8 @@ int main(int argc, char *argv[]) createCubeInVolume(volData, Vector3DUint16(midPos+1, minPos, midPos+1), Vector3DUint16(maxPos, midPos-1, maxPos), 0); createCubeInVolume(volData, Vector3DUint16(minPos, midPos+1, midPos+1), Vector3DUint16(midPos-1, maxPos, maxPos), 0); + volData.idle(0); + QApplication app(argc, argv); OpenGLWidget openGLWidget(0); diff --git a/library/PolyVoxCore/include/PolyVoxImpl/Block.h b/library/PolyVoxCore/include/PolyVoxImpl/Block.h index 92c84312..d6ee0821 100644 --- a/library/PolyVoxCore/include/PolyVoxImpl/Block.h +++ b/library/PolyVoxCore/include/PolyVoxImpl/Block.h @@ -34,7 +34,6 @@ namespace PolyVox { public: POLYVOX_SHARED_PTR< BlockData > m_pBlockData; - bool m_bIsPotentiallySharable; }; } diff --git a/library/PolyVoxCore/include/PolyVoxImpl/BlockData.inl b/library/PolyVoxCore/include/PolyVoxImpl/BlockData.inl index b5fe1913..76745761 100644 --- a/library/PolyVoxCore/include/PolyVoxImpl/BlockData.inl +++ b/library/PolyVoxCore/include/PolyVoxImpl/BlockData.inl @@ -152,14 +152,12 @@ namespace PolyVox template bool BlockData::isHomogeneous(void) { - VoxelType currentVoxel = m_tData; - VoxelType firstVal = *currentVoxel; + const VoxelType tFirstVoxel = m_tData[0]; + const uint32_t uNoOfVoxels = m_uSideLength * m_uSideLength * m_uSideLength; - uint32_t uNoOfVoxels = m_uSideLength * m_uSideLength * m_uSideLength; for(uint32_t ct = 1; ct < uNoOfVoxels; ++ct) { - ++currentVoxel; - if(*currentVoxel != firstVal) + if(m_tData[ct] != tFirstVoxel) { return false; } diff --git a/library/PolyVoxCore/include/Volume.h b/library/PolyVoxCore/include/Volume.h index cb5371ff..3b7b7c01 100644 --- a/library/PolyVoxCore/include/Volume.h +++ b/library/PolyVoxCore/include/Volume.h @@ -29,6 +29,8 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. #include "PolyVoxImpl/CPlusPlusZeroXSupport.h" #include +#include + #pragma endregion namespace PolyVox @@ -60,8 +62,15 @@ namespace PolyVox private: POLYVOX_SHARED_PTR< BlockData > getHomogenousBlockData(VoxelType tHomogenousValue) const; - Block* m_pBlocks; - static std::map > > m_pHomogenousBlockData; + std::vector< Block > m_pBlocks; + std::vector m_vecBlockIsPotentiallyHomogenous; + + //Note: We were once storing weak_ptr's in this map, so that the blocks would be deleted once they + //were not being referenced by anyone else. However, this made it difficult to know when a block was + //shared. A call to shared_ptr::unique() from within setVoxel was not sufficient as weak_ptr's did + //not contribute to the reference count. Instead we store shared_ptr's here, and check if they + //are used by anyone else (i.e are non-unique) when we tidy the volume. + static std::map > > m_pHomogenousBlockData; uint32_t m_uNoOfBlocksInVolume; uint16_t m_uSideLengthInBlocks; @@ -74,7 +83,7 @@ namespace PolyVox }; //Required for the static member - template std::map > > Volume::m_pHomogenousBlockData; + template std::map > > Volume::m_pHomogenousBlockData; //Some handy typedefs diff --git a/library/PolyVoxCore/include/Volume.inl b/library/PolyVoxCore/include/Volume.inl index 160c60be..33aabb98 100644 --- a/library/PolyVoxCore/include/Volume.inl +++ b/library/PolyVoxCore/include/Volume.inl @@ -28,6 +28,7 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. #include #include //For memcpy +#include #include //For invalid_argument #pragma endregion @@ -72,11 +73,12 @@ namespace PolyVox m_uNoOfBlocksInVolume = m_uSideLengthInBlocks * m_uSideLengthInBlocks * m_uSideLengthInBlocks; //Create the blocks - m_pBlocks = new Block[m_uNoOfBlocksInVolume]; + m_pBlocks.resize(m_uNoOfBlocksInVolume); + m_vecBlockIsPotentiallyHomogenous.resize(m_uNoOfBlocksInVolume); for(uint32_t i = 0; i < m_uNoOfBlocksInVolume; ++i) { m_pBlocks[i].m_pBlockData = getHomogenousBlockData(0); - m_pBlocks[i].m_bIsPotentiallySharable = false; + m_vecBlockIsPotentiallyHomogenous[i] = false; } } @@ -89,7 +91,6 @@ namespace PolyVox template Volume::~Volume() { - delete[] m_pBlocks; } #pragma endregion @@ -162,12 +163,12 @@ namespace PolyVox const uint16_t yOffset = uYPos - (blockY << m_uBlockSideLengthPower); const uint16_t zOffset = uZPos - (blockZ << m_uBlockSideLengthPower); - Block& block = m_pBlocks - [ - blockX + - blockY * m_uSideLengthInBlocks + - blockZ * m_uSideLengthInBlocks * m_uSideLengthInBlocks - ]; + uint32_t uBlockIndex = + blockX + + blockY * m_uSideLengthInBlocks + + blockZ * m_uSideLengthInBlocks * m_uSideLengthInBlocks; + + Block& block = m_pBlocks[uBlockIndex]; //It's quite possible that the user might attempt to set a voxel to it's current value. //We test for this case firstly because it could help performance, but more importantly @@ -177,15 +178,15 @@ namespace PolyVox if(block.m_pBlockData.unique()) { block.m_pBlockData->setVoxelAt(xOffset,yOffset,zOffset, tValue); - //There is a chance that setting this voxel makes the block homogenous and therefore shareable. But checking - //this will take some time, so for now just set a flag. - block.m_bIsPotentiallySharable = true; + //There is a chance that setting this voxel makes the block homogenous and therefore shareable. + //But checking this will take some time, so for now just set a flag. + m_vecBlockIsPotentiallyHomogenous[uBlockIndex] = true; } else { POLYVOX_SHARED_PTR< BlockData > pNewBlockData(new BlockData(*(block.m_pBlockData))); block.m_pBlockData = pNewBlockData; - block.m_bIsPotentiallySharable = false; + m_vecBlockIsPotentiallyHomogenous[uBlockIndex] = false; block.m_pBlockData->setVoxelAt(xOffset,yOffset,zOffset, tValue); } } @@ -202,32 +203,36 @@ namespace PolyVox template void Volume::idle(uint32_t uAmount) { - //This function performs two roles. Firstly, it examines all of the blocks which are marked as - //'potentially sharable' to determine whether they really are sharable or not. For those which - //are sharable, it adjusts the pointer and deletes tho old data. Secondly, it determines which - //homogeneous regions are not actually being used (by thier reference count) and frees them. - for(uint32_t i = 0; i < m_uNoOfBlocksInVolume; ++i) { - Block block = m_pBlocks[i]; - if(block.m_bIsPotentiallySharable) + if(m_vecBlockIsPotentiallyHomogenous[i]) { - bool isSharable = block.m_pBlockData->isHomogeneous(); - if(isSharable) + if(m_pBlocks[i].m_pBlockData->isHomogeneous()) { - VoxelType homogeneousValue = block.m_pBlockData->getVoxelAt(0,0,0); - delete block.m_pBlockData; + VoxelType homogeneousValue = m_pBlocks[i].m_pBlockData->getVoxelAt(0,0,0); - block.m_pBlockData = getHomogenousBlockData(homogeneousValue); + m_pBlocks[i].m_pBlockData = getHomogenousBlockData(homogeneousValue); } //Either way, we have now determined whether the block was sharable. So it's not *potentially* sharable. - block.m_bIsPotentiallySharable = false; + m_vecBlockIsPotentiallyHomogenous[i] = false; } } - //Note - this second step should probably happen immediatly, rather than in this function. - //Use of a shared pointer system would allow this. + //FIXME - Better way to do this? Maybe using for_each()? + std::list< std::map > >::iterator > itersToDelete; + for(std::map > >::iterator iter = m_pHomogenousBlockData.begin(); iter != m_pHomogenousBlockData.end(); ++iter) + { + if(iter->second.unique()) + { + itersToDelete.push_back(iter); + } + } + + for(std::list< std::map > >::iterator >::iterator listIter = itersToDelete.begin(); listIter != itersToDelete.end(); ++listIter) + { + m_pHomogenousBlockData.erase(*listIter); + } } template @@ -257,7 +262,7 @@ namespace PolyVox template POLYVOX_SHARED_PTR< BlockData > Volume::getHomogenousBlockData(VoxelType tHomogenousValue) const { - typename std::map > >::iterator iterResult = m_pHomogenousBlockData.find(tHomogenousValue); + std::map > >::iterator iterResult = m_pHomogenousBlockData.find(tHomogenousValue); if(iterResult == m_pHomogenousBlockData.end()) { Block block; @@ -271,8 +276,8 @@ namespace PolyVox else { //iterResult->second.m_uReferenceCount++; - POLYVOX_SHARED_PTR< BlockData > result(iterResult->second); - return result; + //POLYVOX_SHARED_PTR< BlockData > result(iterResult->second); + return iterResult->second; } } #pragma endregion