From 0d638f98370907b5ffed6b3460e320f4f9b2cf52 Mon Sep 17 00:00:00 2001 From: David Williams Date: Sat, 21 Mar 2015 08:05:58 +0100 Subject: [PATCH] Going back to building key by shifting instead of using bitfield. --- include/PolyVox/PagedVolume.h | 46 +++++++++++--------------- include/PolyVox/PagedVolume.inl | 20 +++++------ include/PolyVox/PagedVolumeSampler.inl | 2 +- 3 files changed, 30 insertions(+), 38 deletions(-) diff --git a/include/PolyVox/PagedVolume.h b/include/PolyVox/PagedVolume.h index 066ff927..cf18e4a3 100644 --- a/include/PolyVox/PagedVolume.h +++ b/include/PolyVox/PagedVolume.h @@ -281,7 +281,7 @@ namespace PolyVox PagedVolume& operator=(const PagedVolume& rhs); private: - struct ChunkKey + /*struct ChunkKey { ChunkKey(int32_t x, int32_t y, int32_t z) : iZPos(z), iYPos(y), iXPos(x), iValid(~0) {} int64_t iZPos : 10; @@ -292,39 +292,31 @@ namespace PolyVox // so we don't need to have a seperate check for that before using it. int64_t iValid : 2; }; - static_assert(sizeof(ChunkKey) == sizeof(int64_t), ""); + static_assert(sizeof(ChunkKey) == sizeof(int64_t), "");*/ - int64_t posToChunkKey(int32_t iXPos, int32_t iYPos, int32_t iZPos) const + uint64_t posToChunkKey(int32_t iXPos, int32_t iYPos, int32_t iZPos) const { - iXPos = iXPos >> m_uChunkSideLengthPower; - iYPos = iYPos >> m_uChunkSideLengthPower; - iZPos = iZPos >> m_uChunkSideLengthPower; - - ChunkKey chunkKey(iXPos, iYPos, iZPos); - //chunkKey.iValid = ~0; - //chunkKey.iValid = 2; - // In general, bit shifting signed integers is dubious because of potential undefined or implementation defied behaviour. - // However, it seems that in practice most compilers and architectures work in the same way (http://stackoverflow.com/a/6488645). - - static_assert((int64_t(-3) / 4) == 0, "fdfs"); - static_assert((int64_t(-3) >> 2) == -1, "fdfs"); - /*chunkKey.iXPos = iXPos; - chunkKey.iYPos = iYPos; - chunkKey.iZPos = iZPos;*/ - - // If this kind of casting ever causes problems there are - // other solutions here: http://stackoverflow.com/a/2468738 - int64_t iKeyAsInt64 = force_cast(chunkKey); - - return iKeyAsInt64; + // Bit-shifting of signed integer values has various issues with undefined or implementation-defined behaviour. + // Therefore we cast to unsigned to avoid these (we only care about the bit pattern anyway, not the actual value). + const uint64_t uXPos = static_cast(iXPos); + const uint64_t uYPos = static_cast(iYPos); + const uint64_t uZPos = static_cast(iZPos); + + const uint64_t xKey = ((uXPos >> m_uChunkSideLengthPower) & 0x3FF) << 20; + const uint64_t yKey = ((uYPos >> m_uChunkSideLengthPower) & 0x3FF) << 10; + const uint64_t zKey = ((uZPos >> m_uChunkSideLengthPower) & 0x3FF); + + const uint64_t key = 0x80000000 | xKey | yKey | zKey; + + return key; } - Chunk* getChunk(int64_t iKeyAsInt32) const; + Chunk* getChunk(uint64_t iKeyAsInt32) const; - mutable int64_t m_v3dLastAccessedChunkKey = 0; + mutable uint64_t m_v3dLastAccessedChunkKey = 0; mutable Chunk* m_pLastAccessedChunk = nullptr; - mutable std::unordered_map > m_mapChunks; + mutable std::unordered_map > m_mapChunks; mutable uint32_t m_uTimestamper = 0; diff --git a/include/PolyVox/PagedVolume.inl b/include/PolyVox/PagedVolume.inl index 5e2a8152..80622bea 100644 --- a/include/PolyVox/PagedVolume.inl +++ b/include/PolyVox/PagedVolume.inl @@ -115,7 +115,7 @@ namespace PolyVox template VoxelType PagedVolume::getVoxel(int32_t uXPos, int32_t uYPos, int32_t uZPos) const { - const int64_t key = posToChunkKey(uXPos, uYPos, uZPos); + const uint64_t key = posToChunkKey(uXPos, uYPos, uZPos); // Only call get chunk if we can't reuse the chunk pointer from the last voxel access. auto pChunk = (key == m_v3dLastAccessedChunkKey) ? m_pLastAccessedChunk : getChunk(key); @@ -147,7 +147,7 @@ namespace PolyVox template void PagedVolume::setVoxel(int32_t uXPos, int32_t uYPos, int32_t uZPos, VoxelType tValue) { - const int64_t key = posToChunkKey(uXPos, uYPos, uZPos); + const uint64_t key = posToChunkKey(uXPos, uYPos, uZPos); // Only call get chunk if we can't reuse the chunk pointer from the last voxel access. auto pChunk = (key == m_v3dLastAccessedChunkKey) ? m_pLastAccessedChunk : getChunk(key); @@ -202,7 +202,7 @@ namespace PolyVox { for(int32_t z = v3dStart.getZ(); z <= v3dEnd.getZ(); z++) { - const int64_t key = posToChunkKey(x, y, z); + const uint64_t key = posToChunkKey(x, y, z); // Note that we don't check against the last chunk here. We're // not accessing the voxels, we just want to pull them into memory. @@ -256,7 +256,7 @@ namespace PolyVox { for(int32_t z = v3dStart.getZ(); z <= v3dEnd.getZ(); z++) { - const int64_t key = posToChunkKey(x, y, z); + const uint64_t key = posToChunkKey(x, y, z); m_mapChunks.erase(key); } } @@ -264,7 +264,7 @@ namespace PolyVox } template - typename PagedVolume::Chunk* PagedVolume::getChunk(int64_t iKeyAsInt32) const + typename PagedVolume::Chunk* PagedVolume::getChunk(uint64_t iKeyAsInt32) const { // This function is relatively large and slow because it involves searching for a chunk and creating it if it is not found. A natural // optimization is to only do this work if the chunk we are accessing is not the same as the last chunk we accessed (which it usually @@ -293,14 +293,14 @@ namespace PolyVox // If we still haven't found the chunk then it's time to create a new one and page it in from disk. if (!pChunk) { - /*const int32_t uChunkX = (key >> 20) & 0x3FF; - const int32_t uChunkY = (key >> 10) & 0x3FF; - const int32_t uChunkZ = (key ) & 0x3FF;*/ + const int32_t uChunkX = (iKeyAsInt32 >> 20) & 0x3FF; + const int32_t uChunkY = (iKeyAsInt32 >> 10) & 0x3FF; + const int32_t uChunkZ = (iKeyAsInt32) & 0x3FF; // The chunk was not found so we will create a new one. /*ChunkKeyConverter converter; converter.i = key;*/ - ChunkKey realKey = force_cast(iKeyAsInt32); - Vector3DInt32 v3dChunkPos(realKey.iXPos, realKey.iYPos, realKey.iZPos); + //ChunkKey realKey = force_cast(iKeyAsInt32); + Vector3DInt32 v3dChunkPos(uChunkX, uChunkY, uChunkZ); pChunk = new PagedVolume::Chunk(v3dChunkPos, m_uChunkSideLength, m_pPager); pChunk->m_uChunkLastAccessed = ++m_uTimestamper; // Important, as we may soon delete the oldest chunk m_mapChunks.insert(std::make_pair(iKeyAsInt32, std::unique_ptr(pChunk))); diff --git a/include/PolyVox/PagedVolumeSampler.inl b/include/PolyVox/PagedVolumeSampler.inl index cdcfde3c..a9dc9502 100644 --- a/include/PolyVox/PagedVolumeSampler.inl +++ b/include/PolyVox/PagedVolumeSampler.inl @@ -97,7 +97,7 @@ namespace PolyVox // Base version updates position and validity flags. BaseVolume::template Sampler< PagedVolume >::setPosition(xPos, yPos, zPos); - const int32_t key = this->mVolume->posToChunkKey(xPos, yPos, zPos); + const uint64_t key = this->mVolume->posToChunkKey(xPos, yPos, zPos); // Then we update the voxel pointer. auto pCurrentChunk = (key == this->mVolume->m_v3dLastAccessedChunkKey) ? this->mVolume->m_pLastAccessedChunk : this->mVolume->getChunk(key);