diff --git a/include/PolyVox/Impl/Utility.h b/include/PolyVox/Impl/Utility.h index 8cacc771..2fb3e59e 100644 --- a/include/PolyVox/Impl/Utility.h +++ b/include/PolyVox/Impl/Utility.h @@ -30,14 +30,6 @@ freely, subject to the following restrictions: namespace PolyVox { - // Cast any type to any other type with no safety checks. - // Should only be used if you really know what you are doing! - template - inline to force_cast(from input) - { - return *(reinterpret_cast(&input)); - } - inline bool isPowerOf2(uint32_t uInput) { if (uInput == 0) diff --git a/include/PolyVox/PagedVolume.h b/include/PolyVox/PagedVolume.h index c53a8426..f792e67f 100644 --- a/include/PolyVox/PagedVolume.h +++ b/include/PolyVox/PagedVolume.h @@ -281,46 +281,26 @@ namespace PolyVox PagedVolume& operator=(const PagedVolume& rhs); private: - struct ChunkKey + Chunk* getChunk(int32_t key) const; + + uint32_t posToChunkKey(int32_t iXPos, int32_t iYPos, int32_t iZPos) const { - ChunkKey(int32_t x, int32_t y, int32_t z) : iZPos(z), iYPos(y), iXPos(x), iValid(~0) {} - int32_t iZPos : 10; - int32_t iYPos : 10; - int32_t iXPos : 10; + // 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). + // See http://stackoverflow.com/a/4009922 for more details. + const uint32_t uXPos = static_cast(iXPos); + const uint32_t uYPos = static_cast(iYPos); + const uint32_t uZPos = static_cast(iZPos); - // Should only be true when the last access chunk pointer is valid, - // so we don't need to have a seperate check for that before using it. - int32_t iValid : 2; - }; - static_assert(sizeof(ChunkKey) == sizeof(int32_t), ""); + const uint32_t xKey = ((uXPos >> m_uChunkSideLengthPower) & 0x3FF) << 20; + const uint32_t yKey = ((uYPos >> m_uChunkSideLengthPower) & 0x3FF) << 10; + const uint32_t zKey = ((uZPos >> m_uChunkSideLengthPower) & 0x3FF); - int32_t posToChunkKey(int32_t iXPos, int32_t iYPos, int32_t iZPos) const - { - iXPos = iXPos >> m_uChunkSideLengthPower; - iYPos = iYPos >> m_uChunkSideLengthPower; - iZPos = iZPos >> m_uChunkSideLengthPower; + const uint32_t key = 0x80000000 | xKey | yKey | zKey; - 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((int32_t(-3) / 4) == 0, "fdfs"); - static_assert((int32_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 - int32_t iKeyAsInt32 = force_cast(chunkKey); - - return iKeyAsInt32; + return key; } - Chunk* getChunk(int32_t iKeyAsInt32) const; - mutable int32_t m_v3dLastAccessedChunkKey = 0; mutable Chunk* m_pLastAccessedChunk = nullptr; diff --git a/include/PolyVox/PagedVolume.inl b/include/PolyVox/PagedVolume.inl index 5223e49b..077159d5 100644 --- a/include/PolyVox/PagedVolume.inl +++ b/include/PolyVox/PagedVolume.inl @@ -264,7 +264,7 @@ namespace PolyVox } template - typename PagedVolume::Chunk* PagedVolume::getChunk(int32_t iKeyAsInt32) const + typename PagedVolume::Chunk* PagedVolume::getChunk(int32_t key) 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 @@ -276,11 +276,11 @@ namespace PolyVox // // A second benefit of only calling this function when really necessary is that we can minimize the number of times we update the // timestamp. This reduces the chance of timestamp overflow (particularly if it is only 32-bit). - POLYVOX_ASSERT(iKeyAsInt32 != m_v3dLastAccessedChunkKey, "This should have been checked as an optimization before calling getChunk()."); + POLYVOX_ASSERT(key != m_v3dLastAccessedChunkKey, "This should have been checked as an optimization before calling getChunk()."); // The chunk was not the same as last time, but we can now hope it is in the set of most recently used chunks. - Chunk* pChunk = nullptr; - auto itChunk = m_mapChunks.find(iKeyAsInt32); + Chunk* pChunk = nullptr; + auto itChunk = m_mapChunks.find(key); // Check whether the chunk was found. if ((itChunk) != m_mapChunks.end()) @@ -293,17 +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 uChunkX = (key >> 20) & 0x3FF; const int32_t uChunkY = (key >> 10) & 0x3FF; - const int32_t uChunkZ = (key ) & 0x3FF;*/ + const int32_t uChunkZ = (key ) & 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); + 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))); + m_mapChunks.insert(std::make_pair(key, std::unique_ptr(pChunk))); // As we are loading a new chunk we should try to ensure we don't go over our target memory usage. while (m_mapChunks.size() > m_uChunkCountLimit) @@ -328,7 +325,7 @@ namespace PolyVox /*m_v3dLastAccessedChunkX = uChunkX; m_v3dLastAccessedChunkY = uChunkY; m_v3dLastAccessedChunkZ = uChunkZ;*/ - m_v3dLastAccessedChunkKey = iKeyAsInt32; + m_v3dLastAccessedChunkKey = key; return pChunk; }