From 29928b774de0b21cd497081aa5412ef34da96c81 Mon Sep 17 00:00:00 2001 From: David Williams Date: Tue, 15 Jun 2010 20:32:14 +0000 Subject: [PATCH] Bounds check added to get/setVoxelAt. --- TODO.txt | 8 ++ library/PolyVoxCore/include/Volume.h | 8 +- library/PolyVoxCore/include/Volume.inl | 125 ++++++++++++++----------- tests/testvolume.cpp | 12 +++ 4 files changed, 95 insertions(+), 58 deletions(-) diff --git a/TODO.txt b/TODO.txt index 9e0920cc..db2c0de1 100644 --- a/TODO.txt +++ b/TODO.txt @@ -1,3 +1,11 @@ +Short term +========== +Sort awkward use of 'offset' +Replace float with uchar for material. +Mesh smoothing (surface nets?) +Mesh filters/modifiers - A 'translate' modifier? + + For Version 1.0 =============== Implement Memory Pool diff --git a/library/PolyVoxCore/include/Volume.h b/library/PolyVoxCore/include/Volume.h index 35de2bf7..73ae75a0 100644 --- a/library/PolyVoxCore/include/Volume.h +++ b/library/PolyVoxCore/include/Volume.h @@ -132,11 +132,11 @@ namespace PolyVox uint16_t getShortestSideLength(void) const; ///Gets the length of the diagonal in voxels float getDiagonalLength(void) const; - VoxelType getVoxelAt(uint16_t uXPos, uint16_t uYPos, uint16_t uZPos) const; - VoxelType getVoxelAt(const Vector3DUint16& v3dPos) const; + VoxelType getVoxelAt(uint16_t uXPos, uint16_t uYPos, uint16_t uZPos, VoxelType tDefault = 0) const; + VoxelType getVoxelAt(const Vector3DUint16& v3dPos, VoxelType tDefault = 0) const; - void setVoxelAt(uint16_t uXPos, uint16_t uYPos, uint16_t uZPos, VoxelType tValue); - void setVoxelAt(const Vector3DUint16& v3dPos, VoxelType tValue); + bool setVoxelAt(uint16_t uXPos, uint16_t uYPos, uint16_t uZPos, VoxelType tValue); + bool setVoxelAt(const Vector3DUint16& v3dPos, VoxelType tValue); void tidyUpMemory(uint32_t uNoOfBlocksToProcess = (std::numeric_limits::max)()); diff --git a/library/PolyVoxCore/include/Volume.inl b/library/PolyVoxCore/include/Volume.inl index c595a907..879f255a 100644 --- a/library/PolyVoxCore/include/Volume.inl +++ b/library/PolyVoxCore/include/Volume.inl @@ -224,82 +224,99 @@ namespace PolyVox } template - VoxelType Volume::getVoxelAt(uint16_t uXPos, uint16_t uYPos, uint16_t uZPos) const + VoxelType Volume::getVoxelAt(uint16_t uXPos, uint16_t uYPos, uint16_t uZPos, VoxelType tDefault) const { - assert(uXPos < getWidth()); - assert(uYPos < getHeight()); - assert(uZPos < getDepth()); + //We don't use getEnclosingRegion here because we care + //about speed and don't need to check the lower bound. + if((uXPos < getWidth()) && (uYPos < getHeight()) && (uZPos < getDepth())) + { + const uint16_t blockX = uXPos >> m_uBlockSideLengthPower; + const uint16_t blockY = uYPos >> m_uBlockSideLengthPower; + const uint16_t blockZ = uZPos >> m_uBlockSideLengthPower; - const uint16_t blockX = uXPos >> m_uBlockSideLengthPower; - const uint16_t blockY = uYPos >> m_uBlockSideLengthPower; - const uint16_t blockZ = uZPos >> m_uBlockSideLengthPower; + const uint16_t xOffset = uXPos - (blockX << m_uBlockSideLengthPower); + const uint16_t yOffset = uYPos - (blockY << m_uBlockSideLengthPower); + const uint16_t zOffset = uZPos - (blockZ << m_uBlockSideLengthPower); - const uint16_t xOffset = uXPos - (blockX << m_uBlockSideLengthPower); - const uint16_t yOffset = uYPos - (blockY << m_uBlockSideLengthPower); - const uint16_t zOffset = uZPos - (blockZ << m_uBlockSideLengthPower); + const std::shared_ptr< Block< VoxelType > >& block = m_pBlocks + [ + blockX + + blockY * m_uWidthInBlocks + + blockZ * m_uWidthInBlocks * m_uHeightInBlocks + ]; - const std::shared_ptr< Block< VoxelType > >& block = m_pBlocks - [ - blockX + - blockY * m_uWidthInBlocks + - blockZ * m_uWidthInBlocks * m_uHeightInBlocks - ]; - - return block->getVoxelAt(xOffset,yOffset,zOffset); + return block->getVoxelAt(xOffset,yOffset,zOffset); + } + else + { + return tDefault; + } } template - VoxelType Volume::getVoxelAt(const Vector3DUint16& v3dPos) const + VoxelType Volume::getVoxelAt(const Vector3DUint16& v3dPos, VoxelType tDefault) const { - return getVoxelAt(v3dPos.getX(), v3dPos.getY(), v3dPos.getZ()); + return getVoxelAt(v3dPos.getX(), v3dPos.getY(), v3dPos.getZ(), tDefault); } #pragma endregion #pragma region Setters template - void Volume::setVoxelAt(uint16_t uXPos, uint16_t uYPos, uint16_t uZPos, VoxelType tValue) + bool Volume::setVoxelAt(uint16_t uXPos, uint16_t uYPos, uint16_t uZPos, VoxelType tValue) { - const uint16_t blockX = uXPos >> m_uBlockSideLengthPower; - const uint16_t blockY = uYPos >> m_uBlockSideLengthPower; - const uint16_t blockZ = uZPos >> m_uBlockSideLengthPower; - - const uint16_t xOffset = uXPos - (blockX << m_uBlockSideLengthPower); - const uint16_t yOffset = uYPos - (blockY << m_uBlockSideLengthPower); - const uint16_t zOffset = uZPos - (blockZ << m_uBlockSideLengthPower); - - uint32_t uBlockIndex = - blockX + - blockY * m_uWidthInBlocks + - blockZ * m_uWidthInBlocks * m_uHeightInBlocks; - - std::shared_ptr< 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 - //because it lets us avoid unsharing blocks unnecessarily. - if(block->getVoxelAt(xOffset, yOffset, zOffset) != tValue) + //We don't use getEnclosingRegion here because we care + //about speed and don't need to check the lower bound. + if((uXPos < getWidth()) && (uYPos < getHeight()) && (uZPos < getDepth())) { - if(block.unique()) + const uint16_t blockX = uXPos >> m_uBlockSideLengthPower; + const uint16_t blockY = uYPos >> m_uBlockSideLengthPower; + const uint16_t blockZ = uZPos >> m_uBlockSideLengthPower; + + const uint16_t xOffset = uXPos - (blockX << m_uBlockSideLengthPower); + const uint16_t yOffset = uYPos - (blockY << m_uBlockSideLengthPower); + const uint16_t zOffset = uZPos - (blockZ << m_uBlockSideLengthPower); + + uint32_t uBlockIndex = + blockX + + blockY * m_uWidthInBlocks + + blockZ * m_uWidthInBlocks * m_uHeightInBlocks; + + std::shared_ptr< 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 + //because it lets us avoid unsharing blocks unnecessarily. + if(block->getVoxelAt(xOffset, yOffset, zOffset) != tValue) { - block->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. - m_vecBlockIsPotentiallyHomogenous[uBlockIndex] = true; - } - else - { - std::shared_ptr< Block > pNewBlock(new Block(*(block))); - block = pNewBlock; - m_vecBlockIsPotentiallyHomogenous[uBlockIndex] = false; - block->setVoxelAt(xOffset,yOffset,zOffset, tValue); + if(block.unique()) + { + block->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. + m_vecBlockIsPotentiallyHomogenous[uBlockIndex] = true; + } + else + { + std::shared_ptr< Block > pNewBlock(new Block(*(block))); + block = pNewBlock; + m_vecBlockIsPotentiallyHomogenous[uBlockIndex] = false; + block->setVoxelAt(xOffset,yOffset,zOffset, tValue); + } } + //Return true to indicate that we modified a voxel. + return true; + } + else + { + //Return false to indicate that no voxel was modified. + return false; } } template - void Volume::setVoxelAt(const Vector3DUint16& v3dPos, VoxelType tValue) + bool Volume::setVoxelAt(const Vector3DUint16& v3dPos, VoxelType tValue) { - setVoxelAt(v3dPos.getX(), v3dPos.getY(), v3dPos.getZ(), tValue); + return setVoxelAt(v3dPos.getX(), v3dPos.getY(), v3dPos.getZ(), tValue); } #pragma endregion diff --git a/tests/testvolume.cpp b/tests/testvolume.cpp index 5d6d9dc7..ccbd0dc4 100644 --- a/tests/testvolume.cpp +++ b/tests/testvolume.cpp @@ -33,6 +33,18 @@ void TestVolume::testSize() { const uint16_t g_uVolumeSideLength = 128; Volume volData(g_uVolumeSideLength, g_uVolumeSideLength, g_uVolumeSideLength); + + //Note: Deliberatly go past each edge by one to test if the bounds checking works. + for (uint16_t z = 0; z < g_uVolumeSideLength + 1; z++) + { + for (uint16_t y = 0; y < g_uVolumeSideLength + 1; y++) + { + for (uint16_t x = 0; x < g_uVolumeSideLength + 1; x++) + { + volData.setVoxelAt(x,y,z,255); + } + } + } volData.tidyUpMemory(0);