From 72abcd8e9c91256fe45d098f71dfef194ced0328 Mon Sep 17 00:00:00 2001 From: David Williams Date: Sun, 8 Mar 2015 23:30:12 +0100 Subject: [PATCH 01/22] Chunks are now stored with unique_ptr rather than shared_ptr. --- include/PolyVox/PagedVolume.h | 10 ++++--- include/PolyVox/PagedVolume.inl | 53 ++++++++++++++++++++------------- 2 files changed, 38 insertions(+), 25 deletions(-) diff --git a/include/PolyVox/PagedVolume.h b/include/PolyVox/PagedVolume.h index 6bdf6331..8af3a3d0 100644 --- a/include/PolyVox/PagedVolume.h +++ b/include/PolyVox/PagedVolume.h @@ -281,7 +281,7 @@ namespace PolyVox PagedVolume& operator=(const PagedVolume& rhs); private: - std::shared_ptr getChunk(int32_t uChunkX, int32_t uChunkY, int32_t uChunkZ) const; + Chunk* getChunk(int32_t uChunkX, int32_t uChunkY, int32_t uChunkZ) const; void purgeNullPtrsFromAllChunks(void) const; @@ -299,14 +299,16 @@ namespace PolyVox // // 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; + /*typedef std::unordered_map > WeakPtrChunkMap; mutable WeakPtrChunkMap m_pAllChunks; typedef std::unordered_map > SharedPtrChunkMap; - mutable SharedPtrChunkMap m_pRecentlyUsedChunks; + mutable SharedPtrChunkMap m_pRecentlyUsedChunks;*/ + + mutable std::unordered_map > m_mapChunks; mutable uint32_t m_uTimestamper = 0; mutable Vector3DInt32 m_v3dLastAccessedChunkPos = Vector3DInt32(0, 0, 0); //There are no invalid positions, but initially the m_pLastAccessedChunk pointer will be null - mutable std::shared_ptr m_pLastAccessedChunk = nullptr; + mutable Chunk* m_pLastAccessedChunk = nullptr; uint32_t m_uChunkCountLimit = 0; // The size of the volume diff --git a/include/PolyVox/PagedVolume.inl b/include/PolyVox/PagedVolume.inl index 50841c48..0a902d96 100644 --- a/include/PolyVox/PagedVolume.inl +++ b/include/PolyVox/PagedVolume.inl @@ -217,14 +217,19 @@ namespace PolyVox // Clear this pointer so it doesn't hang on to any chunks. m_pLastAccessedChunk = nullptr; + /*for (auto chunk : m_mapChunks) + { + delete chunk.second; + }*/ + // Erase all the most recently used chunks. - m_pRecentlyUsedChunks.clear(); + m_mapChunks.clear(); // Remove deleted chunks from the list of all loaded chunks. - purgeNullPtrsFromAllChunks(); + //purgeNullPtrsFromAllChunks(); // If there are still some chunks left then this is a cause for concern. Perhaps samplers are holding on to them? - POLYVOX_LOG_WARNING_IF(m_pAllChunks.size() > 0, "Chunks still exist after performing flushAll()! Perhaps you have samplers attached?"); + //POLYVOX_LOG_WARNING_IF(m_pAllChunks.size() > 0, "Chunks still exist after performing flushAll()! Perhaps you have samplers attached?"); } //////////////////////////////////////////////////////////////////////////////// @@ -256,7 +261,7 @@ namespace PolyVox { for(int32_t z = v3dStart.getZ(); z <= v3dEnd.getZ(); z++) { - m_pRecentlyUsedChunks.erase(Vector3DInt32(x, y, z)); + m_mapChunks.erase(Vector3DInt32(x, y, z)); } } } @@ -268,7 +273,7 @@ namespace PolyVox } template - std::shared_ptr::Chunk> PagedVolume::getChunk(int32_t uChunkX, int32_t uChunkY, int32_t uChunkZ) const + typename PagedVolume::Chunk* PagedVolume::getChunk(int32_t uChunkX, int32_t uChunkY, int32_t uChunkZ) const { Vector3DInt32 v3dChunkPos(uChunkX, uChunkY, uChunkZ); @@ -282,18 +287,21 @@ namespace PolyVox } // The chunk was not the same as last time, but we can now hope it is in the set of most recently used chunks. - std::shared_ptr::Chunk> pChunk = nullptr; - typename SharedPtrChunkMap::iterator itChunk = m_pRecentlyUsedChunks.find(v3dChunkPos); + //std::shared_ptr::Chunk> pChunk = nullptr; + //typename SharedPtrChunkMap::iterator itChunk = m_pRecentlyUsedChunks.find(v3dChunkPos); + + Chunk* pChunk = nullptr; + auto itChunk = m_mapChunks.find(v3dChunkPos); // Check whether the chunk was found. - if ((itChunk) != m_pRecentlyUsedChunks.end()) + if ((itChunk) != m_mapChunks.end()) { // The chunk was found so we can use it. - pChunk = itChunk->second; + pChunk = itChunk->second.get(); POLYVOX_ASSERT(pChunk, "Recent chunk list shold never contain a null pointer."); } - if (!pChunk) + /*if (!pChunk) { // Although it's not in our recently use chunks, there's some (slim) chance that it // exists in the list of all loaded chunks, because a sampler may be holding on to it. @@ -314,21 +322,22 @@ namespace PolyVox m_pRecentlyUsedChunks.insert(std::make_pair(v3dChunkPos, pChunk)); } } - } + }*/ // 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) { // The chunk was not found so we will create a new one. - pChunk = std::make_shared< PagedVolume::Chunk >(v3dChunkPos, m_uChunkSideLength, m_pPager); + //pChunk = std::make_shared< PagedVolume::Chunk >(v3dChunkPos, m_uChunkSideLength, m_pPager); + pChunk = new PagedVolume::Chunk(v3dChunkPos, m_uChunkSideLength, m_pPager); // As we are loading a new chunk we should try to ensure we don't go over our target memory usage. bool erasedChunk = false; - while (m_pRecentlyUsedChunks.size() + 1 > m_uChunkCountLimit) // +1 ready for new chunk we will add next. + while (m_mapChunks.size() + 1 > m_uChunkCountLimit) // +1 ready for new chunk we will add next. { // Find the least recently used chunk. Hopefully this isn't too slow. - typename SharedPtrChunkMap::iterator itUnloadChunk = m_pRecentlyUsedChunks.begin(); - for (typename SharedPtrChunkMap::iterator i = m_pRecentlyUsedChunks.begin(); i != m_pRecentlyUsedChunks.end(); i++) + auto itUnloadChunk = m_mapChunks.begin(); + for (auto i = m_mapChunks.begin(); i != m_mapChunks.end(); i++) { if (i->second->m_uChunkLastAccessed < itUnloadChunk->second->m_uChunkLastAccessed) { @@ -337,7 +346,8 @@ namespace PolyVox } // Erase the least recently used chunk - m_pRecentlyUsedChunks.erase(itUnloadChunk); + //delete itUnloadChunk->second; + m_mapChunks.erase(itUnloadChunk); erasedChunk = true; } @@ -349,8 +359,9 @@ namespace PolyVox } // Add our new chunk to the maps. - m_pAllChunks.insert(std::make_pair(v3dChunkPos, pChunk)); - m_pRecentlyUsedChunks.insert(std::make_pair(v3dChunkPos, pChunk)); + //m_pAllChunks.insert(std::make_pair(v3dChunkPos, pChunk)); + //m_pRecentlyUsedChunks.insert(std::make_pair(v3dChunkPos, pChunk)); + m_mapChunks.insert(std::make_pair(v3dChunkPos, std::unique_ptr(pChunk))); } pChunk->m_uChunkLastAccessed = ++m_uTimestamper; @@ -371,13 +382,13 @@ namespace PolyVox // Note: We disregard the size of the other class members as they are likely to be very small compared to the size of the // allocated voxel data. This also keeps the reported size as a power of two, which makes other memory calculations easier. - return PagedVolume::Chunk::calculateSizeInBytes(m_uChunkSideLength) * m_pAllChunks.size(); + return PagedVolume::Chunk::calculateSizeInBytes(m_uChunkSideLength) * m_mapChunks.size(); } template void PagedVolume::purgeNullPtrsFromAllChunks(void) const { - for (auto chunkIter = m_pAllChunks.begin(); chunkIter != m_pAllChunks.end();) + /*for (auto chunkIter = m_pAllChunks.begin(); chunkIter != m_pAllChunks.end();) { if (chunkIter->second.expired()) { @@ -387,7 +398,7 @@ namespace PolyVox { chunkIter++; } - } + }*/ } } From 99d0a226c8f534d7b3e1b3a4fe6d7213c08ad83e Mon Sep 17 00:00:00 2001 From: David Williams Date: Sun, 8 Mar 2015 23:48:55 +0100 Subject: [PATCH 02/22] Tidying up. --- include/PolyVox/PagedVolume.h | 21 -------- include/PolyVox/PagedVolume.inl | 88 +++------------------------------ 2 files changed, 6 insertions(+), 103 deletions(-) diff --git a/include/PolyVox/PagedVolume.h b/include/PolyVox/PagedVolume.h index 8af3a3d0..c58e909b 100644 --- a/include/PolyVox/PagedVolume.h +++ b/include/PolyVox/PagedVolume.h @@ -283,27 +283,6 @@ namespace PolyVox private: Chunk* getChunk(int32_t uChunkX, int32_t uChunkY, int32_t uChunkZ) const; - void purgeNullPtrsFromAllChunks(void) const; - - // The chunk data is stored in the pair of maps below. This will often hold the same set of chunks but there are occasions - // when they can differ (more on that in a moment). The main store is the set of 'recently used chunks' which holds shared_ptr's - // to the chunk data. When memory is low chunks can be removed from this list and, assuming there are no more references to - // them, they will be deleted. However, it is also possible that a Sampler is pointing at a given chunk, and in this case the - // reference counting will ensure that the chunk survives until the sampler has finished with it. - // - // However, this leaves us open to a subtle bug. If a chunk is removed from the recently used set, continues to exist due to a - // sampler using it, and then the user tries to access it through the volume interface, then the volume will page the chunk - // 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; - mutable SharedPtrChunkMap m_pRecentlyUsedChunks;*/ - 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 0a902d96..b2a37ca8 100644 --- a/include/PolyVox/PagedVolume.inl +++ b/include/PolyVox/PagedVolume.inl @@ -214,22 +214,11 @@ namespace PolyVox template void PagedVolume::flushAll() { - // Clear this pointer so it doesn't hang on to any chunks. + // Clear this pointer as all chunks are about to be removed. m_pLastAccessedChunk = nullptr; - /*for (auto chunk : m_mapChunks) - { - delete chunk.second; - }*/ - // Erase all the most recently used chunks. m_mapChunks.clear(); - - // Remove deleted chunks from the list of all loaded chunks. - //purgeNullPtrsFromAllChunks(); - - // If there are still some chunks left then this is a cause for concern. Perhaps samplers are holding on to them? - //POLYVOX_LOG_WARNING_IF(m_pAllChunks.size() > 0, "Chunks still exist after performing flushAll()! Perhaps you have samplers attached?"); } //////////////////////////////////////////////////////////////////////////////// @@ -238,7 +227,7 @@ namespace PolyVox template void PagedVolume::flush(Region regFlush) { - // Clear this pointer so it doesn't hang on to any chunks. + // Clear this pointer in case the chunk it points at is flushed. m_pLastAccessedChunk = nullptr; // Convert the start and end positions into chunk space coordinates @@ -265,11 +254,6 @@ namespace PolyVox } } } - - m_pLastAccessedChunk = nullptr; - - // We might now have so null pointers in the 'all chunks' list so clean them up. - purgeNullPtrsFromAllChunks(); } template @@ -287,9 +271,6 @@ namespace PolyVox } // The chunk was not the same as last time, but we can now hope it is in the set of most recently used chunks. - //std::shared_ptr::Chunk> pChunk = nullptr; - //typename SharedPtrChunkMap::iterator itChunk = m_pRecentlyUsedChunks.find(v3dChunkPos); - Chunk* pChunk = nullptr; auto itChunk = m_mapChunks.find(v3dChunkPos); @@ -301,39 +282,16 @@ namespace PolyVox POLYVOX_ASSERT(pChunk, "Recent chunk list shold never contain a null pointer."); } - /*if (!pChunk) - { - // Although it's not in our recently use chunks, there's some (slim) chance that it - // exists in the list of all loaded chunks, because a sampler may be holding on to it. - typename WeakPtrChunkMap::iterator itWeakChunk = m_pAllChunks.find(v3dChunkPos); - if (itWeakChunk != m_pAllChunks.end()) - { - // We've found an entry in the 'all chunks' list, but it can be null. This happens if a sampler was the - // last thing to be keeping it alive and then the sampler let it go. In this case we remove it from the - // list, and it will get added again soon when we page it in and fill it with valid data. - if (itWeakChunk->second.expired()) - { - m_pAllChunks.erase(itWeakChunk); - } - else - { - // The chunk is valid. We know it's not in the recently used list (we checked earlier) so it should be added. - pChunk = std::shared_ptr< PagedVolume::Chunk >(itWeakChunk->second); - m_pRecentlyUsedChunks.insert(std::make_pair(v3dChunkPos, pChunk)); - } - } - }*/ - // 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) { // The chunk was not found so we will create a new one. - //pChunk = std::make_shared< PagedVolume::Chunk >(v3dChunkPos, m_uChunkSideLength, m_pPager); 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(v3dChunkPos, 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. - bool erasedChunk = false; - while (m_mapChunks.size() + 1 > m_uChunkCountLimit) // +1 ready for new chunk we will add next. + while (m_mapChunks.size() > m_uChunkCountLimit) { // Find the least recently used chunk. Hopefully this isn't too slow. auto itUnloadChunk = m_mapChunks.begin(); @@ -346,25 +304,10 @@ namespace PolyVox } // Erase the least recently used chunk - //delete itUnloadChunk->second; m_mapChunks.erase(itUnloadChunk); - erasedChunk = true; } - - // If we've deleted any chunks from the recently used list then this - // seems like a good place to purge the 'all chunks' list as well. - if (erasedChunk) - { - purgeNullPtrsFromAllChunks(); - } - - // Add our new chunk to the maps. - //m_pAllChunks.insert(std::make_pair(v3dChunkPos, pChunk)); - //m_pRecentlyUsedChunks.insert(std::make_pair(v3dChunkPos, pChunk)); - m_mapChunks.insert(std::make_pair(v3dChunkPos, std::unique_ptr(pChunk))); } - - pChunk->m_uChunkLastAccessed = ++m_uTimestamper; + m_pLastAccessedChunk = pChunk; m_v3dLastAccessedChunkPos = v3dChunkPos; @@ -377,28 +320,9 @@ namespace PolyVox template uint32_t PagedVolume::calculateSizeInBytes(void) { - // Purge null chunks so we know that all chunks are used. - purgeNullPtrsFromAllChunks(); - // Note: We disregard the size of the other class members as they are likely to be very small compared to the size of the // allocated voxel data. This also keeps the reported size as a power of two, which makes other memory calculations easier. return PagedVolume::Chunk::calculateSizeInBytes(m_uChunkSideLength) * m_mapChunks.size(); } - - template - void PagedVolume::purgeNullPtrsFromAllChunks(void) const - { - /*for (auto chunkIter = m_pAllChunks.begin(); chunkIter != m_pAllChunks.end();) - { - if (chunkIter->second.expired()) - { - chunkIter = m_pAllChunks.erase(chunkIter); - } - else - { - chunkIter++; - } - }*/ - } } From 741234e4a589247db7adbcc7aebbdda935f7e391 Mon Sep 17 00:00:00 2001 From: David Williams Date: Mon, 9 Mar 2015 23:52:56 +0100 Subject: [PATCH 03/22] Small speed improvement by storing variables separately (rather than in Vector3D) to void construction/comparison overhead. --- include/PolyVox/PagedVolume.h | 12 ++++++++++-- include/PolyVox/PagedVolume.inl | 14 ++++++++++---- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/include/PolyVox/PagedVolume.h b/include/PolyVox/PagedVolume.h index c58e909b..4e7d7fb2 100644 --- a/include/PolyVox/PagedVolume.h +++ b/include/PolyVox/PagedVolume.h @@ -283,11 +283,19 @@ namespace PolyVox private: Chunk* getChunk(int32_t uChunkX, int32_t uChunkY, int32_t uChunkZ) const; + // Storing these properties individually has proved to be faster than keeping + // them in a Vector3DInt32 as it avoids constructions and comparison overheads. + // They are also at the start of the class in the hope that they will be pulled + // into cache - I've got no idea if this actually makes a difference. + mutable int32_t m_v3dLastAccessedChunkX = 0; + mutable int32_t m_v3dLastAccessedChunkY = 0; + mutable int32_t m_v3dLastAccessedChunkZ = 0; + mutable Chunk* m_pLastAccessedChunk = nullptr; + mutable std::unordered_map > m_mapChunks; mutable uint32_t m_uTimestamper = 0; - mutable Vector3DInt32 m_v3dLastAccessedChunkPos = Vector3DInt32(0, 0, 0); //There are no invalid positions, but initially the m_pLastAccessedChunk pointer will be null - mutable Chunk* m_pLastAccessedChunk = nullptr; + uint32_t m_uChunkCountLimit = 0; // The size of the volume diff --git a/include/PolyVox/PagedVolume.inl b/include/PolyVox/PagedVolume.inl index b2a37ca8..5d1288c0 100644 --- a/include/PolyVox/PagedVolume.inl +++ b/include/PolyVox/PagedVolume.inl @@ -259,17 +259,20 @@ namespace PolyVox template typename PagedVolume::Chunk* PagedVolume::getChunk(int32_t uChunkX, int32_t uChunkY, int32_t uChunkZ) const { - Vector3DInt32 v3dChunkPos(uChunkX, uChunkY, uChunkZ); - //Check if we have the same chunk as last time, if so there's no need to even update //the time stamp. If we updated it everytime then that would be every time we touched //a voxel, which would overflow a uint32_t and require us to use a uint64_t instead. //This check should also provide a significant speed boost as usually it is true. - if((v3dChunkPos == m_v3dLastAccessedChunkPos) && (m_pLastAccessedChunk != 0)) + if ((uChunkX == m_v3dLastAccessedChunkX) && + (uChunkY == m_v3dLastAccessedChunkY) && + (uChunkZ == m_v3dLastAccessedChunkZ) && + (m_pLastAccessedChunk != 0)) { return m_pLastAccessedChunk; } + Vector3DInt32 v3dChunkPos(uChunkX, uChunkY, uChunkZ); + // 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(v3dChunkPos); @@ -309,7 +312,10 @@ namespace PolyVox } m_pLastAccessedChunk = pChunk; - m_v3dLastAccessedChunkPos = v3dChunkPos; + //m_v3dLastAccessedChunkPos = v3dChunkPos; + m_v3dLastAccessedChunkX = uChunkX; + m_v3dLastAccessedChunkY = uChunkY; + m_v3dLastAccessedChunkZ = uChunkZ; return pChunk; } From d305038c27514efe0ee986f5995f03ce218e94dc Mon Sep 17 00:00:00 2001 From: David Williams Date: Wed, 11 Mar 2015 23:47:32 +0100 Subject: [PATCH 04/22] Replaced loop with fill. --- include/PolyVox/Vector.h | 2 ++ include/PolyVox/Vector.inl | 5 +---- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/include/PolyVox/Vector.h b/include/PolyVox/Vector.h index 695ce8c0..d8918516 100644 --- a/include/PolyVox/Vector.h +++ b/include/PolyVox/Vector.h @@ -29,10 +29,12 @@ freely, subject to the following restrictions: #include "PolyVoxForwardDeclarations.h" +#include #include #include #include #include +#include namespace PolyVox { diff --git a/include/PolyVox/Vector.inl b/include/PolyVox/Vector.inl index d03ae025..562c0fca 100644 --- a/include/PolyVox/Vector.inl +++ b/include/PolyVox/Vector.inl @@ -39,10 +39,7 @@ namespace PolyVox template Vector::Vector(StorageType tFillValue) { - for(uint32_t ct = 0; ct < Size; ct++) - { - m_tElements[ct] = tFillValue; - } + std::fill(m_tElements, m_tElements + Size, tFillValue); } /** From e82d6beca1a5cf7e81c546e6dd0243f54ff5d3e6 Mon Sep 17 00:00:00 2001 From: David Williams Date: Sun, 15 Mar 2015 09:32:42 +0100 Subject: [PATCH 05/22] Replaced Vector3D with integer as key to map. Chunks of voxel data are stored in a map, and it is quite common to need to search the map for a particular chunk. The key type used to be a Vector3D (i.e. the position of the chunk in 3D space) which makes conceptual sense but is relatively slow. Using a Vector3D as a key seems to have overhead, probably in terms of copying and performing comparisons. It seems to be significantly faster to use an integer as a key, so we now take the 3D position and pack it into a single integer by bitshifting. Naturally this reduces the range of positions we can store - a 32-bit int can only encode 3 x 10-bit values, which means our volume can only be 1024 chunks in each direction (with a chunk often being 32x32x32 voxels). This should still be large enough for most uses, but an upcoming change will allow 64-bit keys to be used (at least on 64-bit builds) which then allows 21 bits of precision per component. This is so large that it's almost infinite for any practical purposes. --- include/PolyVox/PagedVolume.h | 39 ++++++++++--- include/PolyVox/PagedVolume.inl | 76 +++++++++++++++----------- include/PolyVox/PagedVolumeSampler.inl | 20 +++---- tests/testvolume.cpp | 4 +- tests/testvolume.h | 4 +- 5 files changed, 86 insertions(+), 57 deletions(-) diff --git a/include/PolyVox/PagedVolume.h b/include/PolyVox/PagedVolume.h index 4e7d7fb2..f792e67f 100644 --- a/include/PolyVox/PagedVolume.h +++ b/include/PolyVox/PagedVolume.h @@ -281,18 +281,30 @@ namespace PolyVox PagedVolume& operator=(const PagedVolume& rhs); private: - Chunk* getChunk(int32_t uChunkX, int32_t uChunkY, int32_t uChunkZ) const; + Chunk* getChunk(int32_t key) const; - // Storing these properties individually has proved to be faster than keeping - // them in a Vector3DInt32 as it avoids constructions and comparison overheads. - // They are also at the start of the class in the hope that they will be pulled - // into cache - I've got no idea if this actually makes a difference. - mutable int32_t m_v3dLastAccessedChunkX = 0; - mutable int32_t m_v3dLastAccessedChunkY = 0; - mutable int32_t m_v3dLastAccessedChunkZ = 0; + uint32_t posToChunkKey(int32_t iXPos, int32_t iYPos, int32_t iZPos) const + { + // 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); + + 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); + + const uint32_t key = 0x80000000 | xKey | yKey | zKey; + + return key; + } + + mutable int32_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; @@ -310,6 +322,15 @@ namespace PolyVox }; } +/*#define POS_TO_CHUNK_KEY(x, y, z, uChunkSideLengthPower) \ +{ \ + const int32_t chunkX = ((x >> uChunkSideLengthPower) & 0x3FF) << 20; \ + const int32_t chunkY = ((y >> uChunkSideLengthPower) & 0x3FF) << 10; \ + const int32_t chunkZ = ((z >> uChunkSideLengthPower) & 0x3FF); \ + const int32_t key = (-2147483648) | chunkX | chunkY | chunkZ; \ +}*/ + + #include "PolyVox/PagedVolume.inl" #include "PolyVox/PagedVolumeChunk.inl" #include "PolyVox/PagedVolumeSampler.inl" diff --git a/include/PolyVox/PagedVolume.inl b/include/PolyVox/PagedVolume.inl index 5d1288c0..077159d5 100644 --- a/include/PolyVox/PagedVolume.inl +++ b/include/PolyVox/PagedVolume.inl @@ -115,15 +115,15 @@ namespace PolyVox template VoxelType PagedVolume::getVoxel(int32_t uXPos, int32_t uYPos, int32_t uZPos) const { - const int32_t chunkX = uXPos >> m_uChunkSideLengthPower; - const int32_t chunkY = uYPos >> m_uChunkSideLengthPower; - const int32_t chunkZ = uZPos >> m_uChunkSideLengthPower; + const int32_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); const uint16_t xOffset = static_cast(uXPos & m_iChunkMask); const uint16_t yOffset = static_cast(uYPos & m_iChunkMask); const uint16_t zOffset = static_cast(uZPos & m_iChunkMask); - auto pChunk = getChunk(chunkX, chunkY, chunkZ); return pChunk->getVoxel(xOffset, yOffset, zOffset); } @@ -147,15 +147,15 @@ namespace PolyVox template void PagedVolume::setVoxel(int32_t uXPos, int32_t uYPos, int32_t uZPos, VoxelType tValue) { - const int32_t chunkX = uXPos >> m_uChunkSideLengthPower; - const int32_t chunkY = uYPos >> m_uChunkSideLengthPower; - const int32_t chunkZ = uZPos >> m_uChunkSideLengthPower; + const int32_t key = posToChunkKey(uXPos, uYPos, uZPos); - const uint16_t xOffset = static_cast(uXPos - (chunkX << m_uChunkSideLengthPower)); - const uint16_t yOffset = static_cast(uYPos - (chunkY << m_uChunkSideLengthPower)); - const uint16_t zOffset = static_cast(uZPos - (chunkZ << m_uChunkSideLengthPower)); + // 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); + + const uint16_t xOffset = static_cast(uXPos & m_iChunkMask); + const uint16_t yOffset = static_cast(uYPos & m_iChunkMask); + const uint16_t zOffset = static_cast(uZPos & m_iChunkMask); - auto pChunk = getChunk(chunkX, chunkY, chunkZ); pChunk->setVoxel(xOffset, yOffset, zOffset, tValue); } @@ -201,8 +201,12 @@ namespace PolyVox for(int32_t y = v3dStart.getY(); y <= v3dEnd.getY(); y++) { for(int32_t z = v3dStart.getZ(); z <= v3dEnd.getZ(); z++) - { - getChunk(x,y,z); + { + const int32_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. + getChunk(key); } } } @@ -216,6 +220,7 @@ namespace PolyVox { // Clear this pointer as all chunks are about to be removed. m_pLastAccessedChunk = nullptr; + m_v3dLastAccessedChunkKey = 0; // Erase all the most recently used chunks. m_mapChunks.clear(); @@ -229,6 +234,7 @@ namespace PolyVox { // Clear this pointer in case the chunk it points at is flushed. m_pLastAccessedChunk = nullptr; + m_v3dLastAccessedChunkKey = 0; // Convert the start and end positions into chunk space coordinates Vector3DInt32 v3dStart; @@ -250,32 +256,31 @@ namespace PolyVox { for(int32_t z = v3dStart.getZ(); z <= v3dEnd.getZ(); z++) { - m_mapChunks.erase(Vector3DInt32(x, y, z)); + const int32_t key = posToChunkKey(x, y, z); + m_mapChunks.erase(key); } } } } template - typename PagedVolume::Chunk* PagedVolume::getChunk(int32_t uChunkX, int32_t uChunkY, int32_t uChunkZ) const + typename PagedVolume::Chunk* PagedVolume::getChunk(int32_t key) const { - //Check if we have the same chunk as last time, if so there's no need to even update - //the time stamp. If we updated it everytime then that would be every time we touched - //a voxel, which would overflow a uint32_t and require us to use a uint64_t instead. - //This check should also provide a significant speed boost as usually it is true. - if ((uChunkX == m_v3dLastAccessedChunkX) && - (uChunkY == m_v3dLastAccessedChunkY) && - (uChunkZ == m_v3dLastAccessedChunkZ) && - (m_pLastAccessedChunk != 0)) - { - return m_pLastAccessedChunk; - } - - Vector3DInt32 v3dChunkPos(uChunkX, uChunkY, uChunkZ); + // 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 + // is). We could (and previously did) include a simple check for this in this function. However, this function would then usually return + // immediatly (which was good) but we still paid the overhead of a function call, probably because this function is not inlined due to + // being quite large. Therefore we decided the check against the previous accessed chunk should always be done *before* calling this + // function, and we add an assert here to try and catch if the user forgets to do it. Note that this is an internal function so the + // 'user' here is actually PolyVox developers and not the developers of applications using 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(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(v3dChunkPos); + Chunk* pChunk = nullptr; + auto itChunk = m_mapChunks.find(key); // Check whether the chunk was found. if ((itChunk) != m_mapChunks.end()) @@ -288,10 +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; // The chunk was not found so we will create a new one. + 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(v3dChunkPos, 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) @@ -313,9 +322,10 @@ namespace PolyVox m_pLastAccessedChunk = pChunk; //m_v3dLastAccessedChunkPos = v3dChunkPos; - m_v3dLastAccessedChunkX = uChunkX; + /*m_v3dLastAccessedChunkX = uChunkX; m_v3dLastAccessedChunkY = uChunkY; - m_v3dLastAccessedChunkZ = uChunkZ; + m_v3dLastAccessedChunkZ = uChunkZ;*/ + m_v3dLastAccessedChunkKey = key; return pChunk; } diff --git a/include/PolyVox/PagedVolumeSampler.inl b/include/PolyVox/PagedVolumeSampler.inl index 3de9ca10..cdcfde3c 100644 --- a/include/PolyVox/PagedVolumeSampler.inl +++ b/include/PolyVox/PagedVolumeSampler.inl @@ -97,20 +97,18 @@ namespace PolyVox // Base version updates position and validity flags. BaseVolume::template Sampler< PagedVolume >::setPosition(xPos, yPos, zPos); - // Then we update the voxel pointer - const int32_t uXChunk = this->mXPosInVolume >> this->mVolume->m_uChunkSideLengthPower; - const int32_t uYChunk = this->mYPosInVolume >> this->mVolume->m_uChunkSideLengthPower; - const int32_t uZChunk = this->mZPosInVolume >> this->mVolume->m_uChunkSideLengthPower; + const int32_t key = this->mVolume->posToChunkKey(xPos, yPos, zPos); - const uint16_t uXPosInChunk = static_cast(this->mXPosInVolume - (uXChunk << this->mVolume->m_uChunkSideLengthPower)); - const uint16_t uYPosInChunk = static_cast(this->mYPosInVolume - (uYChunk << this->mVolume->m_uChunkSideLengthPower)); - const uint16_t uZPosInChunk = static_cast(this->mZPosInVolume - (uZChunk << this->mVolume->m_uChunkSideLengthPower)); + // Then we update the voxel pointer. + auto pCurrentChunk = (key == this->mVolume->m_v3dLastAccessedChunkKey) ? this->mVolume->m_pLastAccessedChunk : this->mVolume->getChunk(key); - const uint32_t uVoxelIndexInChunk = uXPosInChunk + - uYPosInChunk * this->mVolume->m_uChunkSideLength + - uZPosInChunk * this->mVolume->m_uChunkSideLength * this->mVolume->m_uChunkSideLength; + const uint16_t xOffset = static_cast(xPos & this->mVolume->m_iChunkMask); + const uint16_t yOffset = static_cast(yPos & this->mVolume->m_iChunkMask); + const uint16_t zOffset = static_cast(zPos & this->mVolume->m_iChunkMask); - auto pCurrentChunk = this->mVolume->getChunk(uXChunk, uYChunk, uZChunk); + const uint32_t uVoxelIndexInChunk = xOffset + + yOffset * this->mVolume->m_uChunkSideLength + + zOffset * this->mVolume->m_uChunkSideLength * this->mVolume->m_uChunkSideLength; mCurrentVoxel = pCurrentChunk->m_tData + uVoxelIndexInChunk; } diff --git a/tests/testvolume.cpp b/tests/testvolume.cpp index 27d10a57..702d9ab1 100644 --- a/tests/testvolume.cpp +++ b/tests/testvolume.cpp @@ -278,7 +278,7 @@ TestVolume::~TestVolume() * RawVolume Tests */ -void TestVolume::testRawVolumeDirectAccessAllInternalForwards() +/*void TestVolume::testRawVolumeDirectAccessAllInternalForwards() { int32_t result = 0; @@ -364,7 +364,7 @@ void TestVolume::testRawVolumeSamplersWithExternalBackwards() result = testSamplersWithWrappingBackwards(m_pRawVolume, m_regExternal); } QCOMPARE(result, static_cast(-993539594)); -} +}*/ /* * PagedVolume Tests diff --git a/tests/testvolume.h b/tests/testvolume.h index 79bab98d..1e3b8406 100644 --- a/tests/testvolume.h +++ b/tests/testvolume.h @@ -38,14 +38,14 @@ public: ~TestVolume(); private slots: - void testRawVolumeDirectAccessAllInternalForwards(); + /*void testRawVolumeDirectAccessAllInternalForwards(); void testRawVolumeSamplersAllInternalForwards(); void testRawVolumeDirectAccessWithExternalForwards(); void testRawVolumeSamplersWithExternalForwards(); void testRawVolumeDirectAccessAllInternalBackwards(); void testRawVolumeSamplersAllInternalBackwards(); void testRawVolumeDirectAccessWithExternalBackwards(); - void testRawVolumeSamplersWithExternalBackwards(); + void testRawVolumeSamplersWithExternalBackwards();*/ void testPagedVolumeDirectAccessAllInternalForwards(); void testPagedVolumeSamplersAllInternalForwards(); From 8bd8f8ba7a91de21fd56d4350f3bf8403a84c4b3 Mon Sep 17 00:00:00 2001 From: David Williams Date: Fri, 20 Mar 2015 15:48:46 +0100 Subject: [PATCH 06/22] Work on using a bitfield to set up chunk key. --- include/PolyVox/Impl/Utility.h | 8 ++++++ include/PolyVox/PagedVolume.h | 48 +++++++++++++++++++++++---------- include/PolyVox/PagedVolume.inl | 21 ++++++++------- 3 files changed, 54 insertions(+), 23 deletions(-) diff --git a/include/PolyVox/Impl/Utility.h b/include/PolyVox/Impl/Utility.h index 2fb3e59e..8cacc771 100644 --- a/include/PolyVox/Impl/Utility.h +++ b/include/PolyVox/Impl/Utility.h @@ -30,6 +30,14 @@ 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 f792e67f..c53a8426 100644 --- a/include/PolyVox/PagedVolume.h +++ b/include/PolyVox/PagedVolume.h @@ -281,26 +281,46 @@ namespace PolyVox PagedVolume& operator=(const PagedVolume& rhs); private: - Chunk* getChunk(int32_t key) const; - - uint32_t posToChunkKey(int32_t iXPos, int32_t iYPos, int32_t iZPos) const + struct ChunkKey { - // 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); + 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; - 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); + // 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 key = 0x80000000 | xKey | yKey | zKey; + 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; - return key; + 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; } + 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 077159d5..5223e49b 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 key) const + typename PagedVolume::Chunk* PagedVolume::getChunk(int32_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 @@ -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(key != m_v3dLastAccessedChunkKey, "This should have been checked as an optimization before calling getChunk()."); + POLYVOX_ASSERT(iKeyAsInt32 != 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(key); + Chunk* pChunk = nullptr; + auto itChunk = m_mapChunks.find(iKeyAsInt32); // Check whether the chunk was found. if ((itChunk) != m_mapChunks.end()) @@ -293,14 +293,17 @@ 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. - Vector3DInt32 v3dChunkPos(uChunkX, uChunkY, uChunkZ); + /*ChunkKeyConverter converter; + converter.i = key;*/ + ChunkKey realKey = force_cast(iKeyAsInt32); + Vector3DInt32 v3dChunkPos(realKey.iXPos, realKey.iYPos, realKey.iZPos); 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(key, std::unique_ptr(pChunk))); + m_mapChunks.insert(std::make_pair(iKeyAsInt32, 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) @@ -325,7 +328,7 @@ namespace PolyVox /*m_v3dLastAccessedChunkX = uChunkX; m_v3dLastAccessedChunkY = uChunkY; m_v3dLastAccessedChunkZ = uChunkZ;*/ - m_v3dLastAccessedChunkKey = key; + m_v3dLastAccessedChunkKey = iKeyAsInt32; return pChunk; } From 6419c5827bc99d91dd5e96acd17bd507a30dc106 Mon Sep 17 00:00:00 2001 From: David Williams Date: Fri, 20 Mar 2015 16:59:25 +0100 Subject: [PATCH 07/22] Added typedef for chunk key type. --- include/PolyVox/Impl/Config.h | 4 ++++ include/PolyVox/PagedVolume.h | 18 +++++++++--------- include/PolyVox/PagedVolume.inl | 10 +++++----- 3 files changed, 18 insertions(+), 14 deletions(-) diff --git a/include/PolyVox/Impl/Config.h b/include/PolyVox/Impl/Config.h index 1959a6d2..388f25d6 100644 --- a/include/PolyVox/Impl/Config.h +++ b/include/PolyVox/Impl/Config.h @@ -24,6 +24,8 @@ freely, subject to the following restrictions: #ifndef __PolyVox_Config_H__ #define __PolyVox_Config_H__ +#include + //#define POLYVOX_LOG_TRACE_ENABLED //#define POLYVOX_LOG_DEBUG_ENABLED #define POLYVOX_LOG_INFO_ENABLED @@ -34,4 +36,6 @@ freely, subject to the following restrictions: //#define POLYVOX_ASSERTS_ENABLED #define POLYVOX_THROW_ENABLED +typedef int32_t PagedVolumeChunkKeyIntType; + #endif diff --git a/include/PolyVox/PagedVolume.h b/include/PolyVox/PagedVolume.h index c53a8426..5477b6d1 100644 --- a/include/PolyVox/PagedVolume.h +++ b/include/PolyVox/PagedVolume.h @@ -284,17 +284,17 @@ namespace PolyVox struct ChunkKey { 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; + PagedVolumeChunkKeyIntType iZPos : 10; + PagedVolumeChunkKeyIntType iYPos : 10; + PagedVolumeChunkKeyIntType iXPos : 10; // 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; + PagedVolumeChunkKeyIntType iValid : 2; }; - static_assert(sizeof(ChunkKey) == sizeof(int32_t), ""); + static_assert(sizeof(ChunkKey) == sizeof(PagedVolumeChunkKeyIntType), ""); - int32_t posToChunkKey(int32_t iXPos, int32_t iYPos, int32_t iZPos) const + PagedVolumeChunkKeyIntType posToChunkKey(int32_t iXPos, int32_t iYPos, int32_t iZPos) const { iXPos = iXPos >> m_uChunkSideLengthPower; iYPos = iYPos >> m_uChunkSideLengthPower; @@ -314,14 +314,14 @@ namespace PolyVox // If this kind of casting ever causes problems there are // other solutions here: http://stackoverflow.com/a/2468738 - int32_t iKeyAsInt32 = force_cast(chunkKey); + PagedVolumeChunkKeyIntType iKeyAsInt32 = force_cast(chunkKey); return iKeyAsInt32; } - Chunk* getChunk(int32_t iKeyAsInt32) const; + Chunk* getChunk(PagedVolumeChunkKeyIntType iKeyAsInt32) const; - mutable int32_t m_v3dLastAccessedChunkKey = 0; + mutable PagedVolumeChunkKeyIntType m_v3dLastAccessedChunkKey = 0; mutable Chunk* m_pLastAccessedChunk = nullptr; mutable std::unordered_map > m_mapChunks; diff --git a/include/PolyVox/PagedVolume.inl b/include/PolyVox/PagedVolume.inl index 5223e49b..15c831fd 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 int32_t key = posToChunkKey(uXPos, uYPos, uZPos); + const PagedVolumeChunkKeyIntType 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 int32_t key = posToChunkKey(uXPos, uYPos, uZPos); + const PagedVolumeChunkKeyIntType 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 int32_t key = posToChunkKey(x, y, z); + const PagedVolumeChunkKeyIntType 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 int32_t key = posToChunkKey(x, y, z); + const PagedVolumeChunkKeyIntType key = posToChunkKey(x, y, z); m_mapChunks.erase(key); } } @@ -264,7 +264,7 @@ namespace PolyVox } template - typename PagedVolume::Chunk* PagedVolume::getChunk(int32_t iKeyAsInt32) const + typename PagedVolume::Chunk* PagedVolume::getChunk(PagedVolumeChunkKeyIntType 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 From 69f6f4ac3713e7a9eee804773a516cc5c3412183 Mon Sep 17 00:00:00 2001 From: David Williams Date: Fri, 20 Mar 2015 23:09:38 +0100 Subject: [PATCH 08/22] Decided to always use a 64-bit chunk key, rather than trying to make it configurable. --- include/PolyVox/Impl/Config.h | 2 -- include/PolyVox/PagedVolume.h | 24 ++++++++++++------------ include/PolyVox/PagedVolume.inl | 10 +++++----- 3 files changed, 17 insertions(+), 19 deletions(-) diff --git a/include/PolyVox/Impl/Config.h b/include/PolyVox/Impl/Config.h index 388f25d6..1a46222e 100644 --- a/include/PolyVox/Impl/Config.h +++ b/include/PolyVox/Impl/Config.h @@ -36,6 +36,4 @@ freely, subject to the following restrictions: //#define POLYVOX_ASSERTS_ENABLED #define POLYVOX_THROW_ENABLED -typedef int32_t PagedVolumeChunkKeyIntType; - #endif diff --git a/include/PolyVox/PagedVolume.h b/include/PolyVox/PagedVolume.h index 5477b6d1..066ff927 100644 --- a/include/PolyVox/PagedVolume.h +++ b/include/PolyVox/PagedVolume.h @@ -284,17 +284,17 @@ namespace PolyVox struct ChunkKey { ChunkKey(int32_t x, int32_t y, int32_t z) : iZPos(z), iYPos(y), iXPos(x), iValid(~0) {} - PagedVolumeChunkKeyIntType iZPos : 10; - PagedVolumeChunkKeyIntType iYPos : 10; - PagedVolumeChunkKeyIntType iXPos : 10; + int64_t iZPos : 10; + int64_t iYPos : 10; + int64_t iXPos : 10; // 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. - PagedVolumeChunkKeyIntType iValid : 2; + int64_t iValid : 2; }; - static_assert(sizeof(ChunkKey) == sizeof(PagedVolumeChunkKeyIntType), ""); + static_assert(sizeof(ChunkKey) == sizeof(int64_t), ""); - PagedVolumeChunkKeyIntType posToChunkKey(int32_t iXPos, int32_t iYPos, int32_t iZPos) const + int64_t posToChunkKey(int32_t iXPos, int32_t iYPos, int32_t iZPos) const { iXPos = iXPos >> m_uChunkSideLengthPower; iYPos = iYPos >> m_uChunkSideLengthPower; @@ -306,22 +306,22 @@ namespace PolyVox // 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"); + 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 - PagedVolumeChunkKeyIntType iKeyAsInt32 = force_cast(chunkKey); + int64_t iKeyAsInt64 = force_cast(chunkKey); - return iKeyAsInt32; + return iKeyAsInt64; } - Chunk* getChunk(PagedVolumeChunkKeyIntType iKeyAsInt32) const; + Chunk* getChunk(int64_t iKeyAsInt32) const; - mutable PagedVolumeChunkKeyIntType m_v3dLastAccessedChunkKey = 0; + mutable int64_t m_v3dLastAccessedChunkKey = 0; mutable Chunk* m_pLastAccessedChunk = nullptr; mutable std::unordered_map > m_mapChunks; diff --git a/include/PolyVox/PagedVolume.inl b/include/PolyVox/PagedVolume.inl index 15c831fd..5e2a8152 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 PagedVolumeChunkKeyIntType key = posToChunkKey(uXPos, uYPos, uZPos); + const int64_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 PagedVolumeChunkKeyIntType key = posToChunkKey(uXPos, uYPos, uZPos); + const int64_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 PagedVolumeChunkKeyIntType key = posToChunkKey(x, y, z); + const int64_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 PagedVolumeChunkKeyIntType key = posToChunkKey(x, y, z); + const int64_t key = posToChunkKey(x, y, z); m_mapChunks.erase(key); } } @@ -264,7 +264,7 @@ namespace PolyVox } template - typename PagedVolume::Chunk* PagedVolume::getChunk(PagedVolumeChunkKeyIntType iKeyAsInt32) const + typename PagedVolume::Chunk* PagedVolume::getChunk(int64_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 From 0e995b514097dae6df5109f708fe3db8d6476239 Mon Sep 17 00:00:00 2001 From: David Williams Date: Sat, 21 Mar 2015 07:40:32 +0100 Subject: [PATCH 09/22] Fixed some compiler warnings. --- CMakeLists.txt | 4 ++++ examples/OpenGL/Shapes.cpp | 4 ++-- tests/TestVolumeSubclass.cpp | 8 ++++++-- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 785531d0..0b4738bc 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -37,6 +37,10 @@ IF(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang") #Maybe "OR ADD_DEFINITIONS(-std=c++0x) #Enable C++0x mode ENDIF() +IF(CMAKE_CXX_COMPILER_ID MATCHES "MSVC") + ADD_DEFINITIONS(-D_CRT_SECURE_NO_WARNINGS) +ENDIF() + ADD_SUBDIRECTORY(include) OPTION(ENABLE_EXAMPLES "Should the examples be built" ON) diff --git a/examples/OpenGL/Shapes.cpp b/examples/OpenGL/Shapes.cpp index 8821e7fb..77628f22 100644 --- a/examples/OpenGL/Shapes.cpp +++ b/examples/OpenGL/Shapes.cpp @@ -57,8 +57,8 @@ void createSphereInVolume(RawVolume& volData, float fRadi void createCubeInVolume(RawVolume& volData, Vector3DInt32 lowerCorner, Vector3DInt32 upperCorner, uint8_t uValue) { - uint8_t maxDen = MaterialDensityPair88::getMaxDensity(); - uint8_t minDen = MaterialDensityPair88::getMinDensity(); + uint8_t maxDen = static_cast(MaterialDensityPair88::getMaxDensity()); + uint8_t minDen = static_cast(MaterialDensityPair88::getMinDensity()); //This three-level for loop iterates over every voxel between the specified corners for (int z = lowerCorner.getZ(); z <= upperCorner.getZ(); z++) { diff --git a/tests/TestVolumeSubclass.cpp b/tests/TestVolumeSubclass.cpp index eb5238ca..0cccb3a2 100644 --- a/tests/TestVolumeSubclass.cpp +++ b/tests/TestVolumeSubclass.cpp @@ -73,7 +73,9 @@ public: /// Gets a voxel at the position given by x,y,z coordinates VoxelType getVoxel(int32_t uXPos, int32_t uYPos, int32_t uZPos) const { - if ((uXPos < mVolumeData.getDimension(0)) && (uYPos < mVolumeData.getDimension(1)) && (uZPos < mVolumeData.getDimension(2))) + if ((uXPos >= 0) && (uXPos < static_cast(mVolumeData.getDimension(0))) && + (uYPos >= 0) && (uYPos < static_cast(mVolumeData.getDimension(1))) && + (uZPos >= 0) && (uZPos < static_cast(mVolumeData.getDimension(2)))) { return mVolumeData(uXPos, uYPos, uZPos); } @@ -94,7 +96,9 @@ public: /// Sets the voxel at the position given by x,y,z coordinates bool setVoxel(int32_t uXPos, int32_t uYPos, int32_t uZPos, VoxelType tValue) { - if ((uXPos < mVolumeData.getDimension(0)) && (uYPos < mVolumeData.getDimension(1)) && (uZPos < mVolumeData.getDimension(2))) + if( (uXPos >= 0) && (uXPos < static_cast(mVolumeData.getDimension(0))) && + (uYPos >= 0) && (uYPos < static_cast(mVolumeData.getDimension(1))) && + (uZPos >= 0) && (uZPos < static_cast(mVolumeData.getDimension(2)))) { mVolumeData(uXPos, uYPos, uZPos) = tValue; return true; From 0d638f98370907b5ffed6b3460e320f4f9b2cf52 Mon Sep 17 00:00:00 2001 From: David Williams Date: Sat, 21 Mar 2015 08:05:58 +0100 Subject: [PATCH 10/22] 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); From 905ec27f47a3e2dc1cba3a3a384b2c7532dab03d Mon Sep 17 00:00:00 2001 From: David Williams Date: Sat, 21 Mar 2015 08:12:02 +0100 Subject: [PATCH 11/22] Added typedef for ChunkKey. --- include/PolyVox/PagedVolume.h | 35 +++++++++----------------- include/PolyVox/PagedVolume.inl | 10 ++++---- include/PolyVox/PagedVolumeSampler.inl | 2 +- 3 files changed, 18 insertions(+), 29 deletions(-) diff --git a/include/PolyVox/PagedVolume.h b/include/PolyVox/PagedVolume.h index cf18e4a3..852ebe60 100644 --- a/include/PolyVox/PagedVolume.h +++ b/include/PolyVox/PagedVolume.h @@ -281,42 +281,31 @@ namespace PolyVox PagedVolume& operator=(const PagedVolume& rhs); private: - /*struct ChunkKey - { - ChunkKey(int32_t x, int32_t y, int32_t z) : iZPos(z), iYPos(y), iXPos(x), iValid(~0) {} - int64_t iZPos : 10; - int64_t iYPos : 10; - int64_t iXPos : 10; + typedef uint64_t ChunkKey; - // 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. - int64_t iValid : 2; - }; - static_assert(sizeof(ChunkKey) == sizeof(int64_t), "");*/ - - uint64_t posToChunkKey(int32_t iXPos, int32_t iYPos, int32_t iZPos) const + ChunkKey posToChunkKey(int32_t iXPos, int32_t iYPos, int32_t iZPos) const { // 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 ChunkKey uXPos = static_cast(iXPos); + const ChunkKey uYPos = static_cast(iYPos); + const ChunkKey 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 ChunkKey xKey = ((uXPos >> m_uChunkSideLengthPower) & 0x3FF) << 20; + const ChunkKey yKey = ((uYPos >> m_uChunkSideLengthPower) & 0x3FF) << 10; + const ChunkKey zKey = ((uZPos >> m_uChunkSideLengthPower) & 0x3FF); - const uint64_t key = 0x80000000 | xKey | yKey | zKey; + const ChunkKey key = 0x80000000 | xKey | yKey | zKey; return key; } - Chunk* getChunk(uint64_t iKeyAsInt32) const; + Chunk* getChunk(ChunkKey iKeyAsInt32) const; - mutable uint64_t m_v3dLastAccessedChunkKey = 0; + mutable ChunkKey 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 80622bea..b93fc2de 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 uint64_t key = posToChunkKey(uXPos, uYPos, uZPos); + const ChunkKey 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 uint64_t key = posToChunkKey(uXPos, uYPos, uZPos); + const ChunkKey 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 uint64_t key = posToChunkKey(x, y, z); + const ChunkKey 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 uint64_t key = posToChunkKey(x, y, z); + const ChunkKey key = posToChunkKey(x, y, z); m_mapChunks.erase(key); } } @@ -264,7 +264,7 @@ namespace PolyVox } template - typename PagedVolume::Chunk* PagedVolume::getChunk(uint64_t iKeyAsInt32) const + typename PagedVolume::Chunk* PagedVolume::getChunk(ChunkKey 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 diff --git a/include/PolyVox/PagedVolumeSampler.inl b/include/PolyVox/PagedVolumeSampler.inl index a9dc9502..3f7a3622 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 uint64_t key = this->mVolume->posToChunkKey(xPos, yPos, zPos); + const ChunkKey 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); From fd451be2dd7b65671274791105e76bd6a31a0f79 Mon Sep 17 00:00:00 2001 From: David Williams Date: Sat, 21 Mar 2015 08:48:45 +0100 Subject: [PATCH 12/22] New, safer method of packing which makes careful use of casting to avoid problems with e.g. signed integer sign extension. --- include/PolyVox/PagedVolume.h | 25 ++++++++++++++++++++++--- include/PolyVox/PagedVolume.inl | 25 ++++++++++++++++++++++--- 2 files changed, 44 insertions(+), 6 deletions(-) diff --git a/include/PolyVox/PagedVolume.h b/include/PolyVox/PagedVolume.h index 852ebe60..105e50df 100644 --- a/include/PolyVox/PagedVolume.h +++ b/include/PolyVox/PagedVolume.h @@ -285,17 +285,36 @@ namespace PolyVox ChunkKey posToChunkKey(int32_t iXPos, int32_t iYPos, int32_t iZPos) const { + int32_t iBlockX = iXPos >> m_uChunkSideLengthPower; + int32_t iBlockY = iYPos >> m_uChunkSideLengthPower; + int32_t iBlockZ = iZPos >> m_uChunkSideLengthPower; + + int16_t iBlockXAsInt16 = static_cast(iBlockX); + int16_t iBlockYAsInt16 = static_cast(iBlockY); + int16_t iBlockZAsInt16 = static_cast(iBlockZ); + + uint16_t uBlockXAsUint16 = reinterpret_cast(iBlockXAsInt16); + uint16_t uBlockYAsUint16 = reinterpret_cast(iBlockYAsInt16); + uint16_t uBlockZAsUint16 = reinterpret_cast(iBlockZAsInt16); + + uint64_t uBlockXAsUint64 = static_cast(uBlockXAsUint16); + uint64_t uBlockYAsUint64 = static_cast(uBlockYAsUint16); + uint64_t uBlockZAsUint64 = static_cast(uBlockZAsUint16); + + uBlockXAsUint64 = uBlockXAsUint64 << 32; + uBlockYAsUint64 = uBlockYAsUint64 << 16; + // 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 ChunkKey uXPos = static_cast(iXPos); + /*const ChunkKey uXPos = static_cast(iXPos); const ChunkKey uYPos = static_cast(iYPos); const ChunkKey uZPos = static_cast(iZPos); const ChunkKey xKey = ((uXPos >> m_uChunkSideLengthPower) & 0x3FF) << 20; const ChunkKey yKey = ((uYPos >> m_uChunkSideLengthPower) & 0x3FF) << 10; - const ChunkKey zKey = ((uZPos >> m_uChunkSideLengthPower) & 0x3FF); + const ChunkKey zKey = ((uZPos >> m_uChunkSideLengthPower) & 0x3FF);*/ - const ChunkKey key = 0x80000000 | xKey | yKey | zKey; + const ChunkKey key = 0xFFFF000000000000 | uBlockXAsUint64 | uBlockYAsUint64 | uBlockZAsUint64; return key; } diff --git a/include/PolyVox/PagedVolume.inl b/include/PolyVox/PagedVolume.inl index b93fc2de..8d80c0f1 100644 --- a/include/PolyVox/PagedVolume.inl +++ b/include/PolyVox/PagedVolume.inl @@ -293,14 +293,33 @@ 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 = (iKeyAsInt32 >> 20) & 0x3FF; + uint64_t uBlockXAsUint64 = iKeyAsInt32; + uint64_t uBlockYAsUint64 = iKeyAsInt32; + uint64_t uBlockZAsUint64 = iKeyAsInt32; + + uBlockXAsUint64 = uBlockXAsUint64 >> 32; + uBlockYAsUint64 = uBlockYAsUint64 >> 16; + + uBlockXAsUint64 = uBlockXAsUint64 & 0xFFFF; + uBlockYAsUint64 = uBlockYAsUint64 & 0xFFFF; + uBlockZAsUint64 = uBlockZAsUint64 & 0xFFFF; + + uint16_t uBlockXAsUint16 = static_cast(uBlockXAsUint64); + uint16_t uBlockYAsUint16 = static_cast(uBlockYAsUint64); + uint16_t uBlockZAsUint16 = static_cast(uBlockZAsUint64); + + int16_t uBlockXAsInt16 = reinterpret_cast(uBlockXAsUint16); + int16_t uBlockYAsInt16 = reinterpret_cast(uBlockYAsUint16); + int16_t uBlockZAsInt16 = reinterpret_cast(uBlockZAsUint16); + + /*const int32_t uChunkX = (iKeyAsInt32 >> 20) & 0x3FF; const int32_t uChunkY = (iKeyAsInt32 >> 10) & 0x3FF; - const int32_t uChunkZ = (iKeyAsInt32) & 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(uChunkX, uChunkY, uChunkZ); + Vector3DInt32 v3dChunkPos(uBlockXAsInt16, uBlockYAsInt16, uBlockZAsInt16); 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))); From f574563672beff04389bb16934d90c8befe6bb87 Mon Sep 17 00:00:00 2001 From: David Williams Date: Sat, 21 Mar 2015 14:40:11 +0100 Subject: [PATCH 13/22] Revert "New, safer method of packing which makes careful use of casting to avoid problems with e.g. signed integer sign extension." This reverts commit fd451be2dd7b65671274791105e76bd6a31a0f79. --- include/PolyVox/PagedVolume.h | 25 +++---------------------- include/PolyVox/PagedVolume.inl | 25 +++---------------------- 2 files changed, 6 insertions(+), 44 deletions(-) diff --git a/include/PolyVox/PagedVolume.h b/include/PolyVox/PagedVolume.h index 105e50df..852ebe60 100644 --- a/include/PolyVox/PagedVolume.h +++ b/include/PolyVox/PagedVolume.h @@ -285,36 +285,17 @@ namespace PolyVox ChunkKey posToChunkKey(int32_t iXPos, int32_t iYPos, int32_t iZPos) const { - int32_t iBlockX = iXPos >> m_uChunkSideLengthPower; - int32_t iBlockY = iYPos >> m_uChunkSideLengthPower; - int32_t iBlockZ = iZPos >> m_uChunkSideLengthPower; - - int16_t iBlockXAsInt16 = static_cast(iBlockX); - int16_t iBlockYAsInt16 = static_cast(iBlockY); - int16_t iBlockZAsInt16 = static_cast(iBlockZ); - - uint16_t uBlockXAsUint16 = reinterpret_cast(iBlockXAsInt16); - uint16_t uBlockYAsUint16 = reinterpret_cast(iBlockYAsInt16); - uint16_t uBlockZAsUint16 = reinterpret_cast(iBlockZAsInt16); - - uint64_t uBlockXAsUint64 = static_cast(uBlockXAsUint16); - uint64_t uBlockYAsUint64 = static_cast(uBlockYAsUint16); - uint64_t uBlockZAsUint64 = static_cast(uBlockZAsUint16); - - uBlockXAsUint64 = uBlockXAsUint64 << 32; - uBlockYAsUint64 = uBlockYAsUint64 << 16; - // 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 ChunkKey uXPos = static_cast(iXPos); + const ChunkKey uXPos = static_cast(iXPos); const ChunkKey uYPos = static_cast(iYPos); const ChunkKey uZPos = static_cast(iZPos); const ChunkKey xKey = ((uXPos >> m_uChunkSideLengthPower) & 0x3FF) << 20; const ChunkKey yKey = ((uYPos >> m_uChunkSideLengthPower) & 0x3FF) << 10; - const ChunkKey zKey = ((uZPos >> m_uChunkSideLengthPower) & 0x3FF);*/ + const ChunkKey zKey = ((uZPos >> m_uChunkSideLengthPower) & 0x3FF); - const ChunkKey key = 0xFFFF000000000000 | uBlockXAsUint64 | uBlockYAsUint64 | uBlockZAsUint64; + const ChunkKey key = 0x80000000 | xKey | yKey | zKey; return key; } diff --git a/include/PolyVox/PagedVolume.inl b/include/PolyVox/PagedVolume.inl index 8d80c0f1..b93fc2de 100644 --- a/include/PolyVox/PagedVolume.inl +++ b/include/PolyVox/PagedVolume.inl @@ -293,33 +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) { - uint64_t uBlockXAsUint64 = iKeyAsInt32; - uint64_t uBlockYAsUint64 = iKeyAsInt32; - uint64_t uBlockZAsUint64 = iKeyAsInt32; - - uBlockXAsUint64 = uBlockXAsUint64 >> 32; - uBlockYAsUint64 = uBlockYAsUint64 >> 16; - - uBlockXAsUint64 = uBlockXAsUint64 & 0xFFFF; - uBlockYAsUint64 = uBlockYAsUint64 & 0xFFFF; - uBlockZAsUint64 = uBlockZAsUint64 & 0xFFFF; - - uint16_t uBlockXAsUint16 = static_cast(uBlockXAsUint64); - uint16_t uBlockYAsUint16 = static_cast(uBlockYAsUint64); - uint16_t uBlockZAsUint16 = static_cast(uBlockZAsUint64); - - int16_t uBlockXAsInt16 = reinterpret_cast(uBlockXAsUint16); - int16_t uBlockYAsInt16 = reinterpret_cast(uBlockYAsUint16); - int16_t uBlockZAsInt16 = reinterpret_cast(uBlockZAsUint16); - - /*const int32_t uChunkX = (iKeyAsInt32 >> 20) & 0x3FF; + const int32_t uChunkX = (iKeyAsInt32 >> 20) & 0x3FF; const int32_t uChunkY = (iKeyAsInt32 >> 10) & 0x3FF; - const int32_t uChunkZ = (iKeyAsInt32) & 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(uBlockXAsInt16, uBlockYAsInt16, uBlockZAsInt16); + 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))); From 0c619ebec7991dc7f09a02a94290d4e16392fe1f Mon Sep 17 00:00:00 2001 From: David Williams Date: Sat, 21 Mar 2015 14:40:30 +0100 Subject: [PATCH 14/22] Revert "Added typedef for ChunkKey." This reverts commit 905ec27f47a3e2dc1cba3a3a384b2c7532dab03d. --- include/PolyVox/PagedVolume.h | 35 +++++++++++++++++--------- include/PolyVox/PagedVolume.inl | 10 ++++---- include/PolyVox/PagedVolumeSampler.inl | 2 +- 3 files changed, 29 insertions(+), 18 deletions(-) diff --git a/include/PolyVox/PagedVolume.h b/include/PolyVox/PagedVolume.h index 852ebe60..cf18e4a3 100644 --- a/include/PolyVox/PagedVolume.h +++ b/include/PolyVox/PagedVolume.h @@ -281,31 +281,42 @@ namespace PolyVox PagedVolume& operator=(const PagedVolume& rhs); private: - typedef uint64_t 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; + int64_t iYPos : 10; + int64_t iXPos : 10; - ChunkKey posToChunkKey(int32_t iXPos, int32_t iYPos, int32_t iZPos) const + // 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. + int64_t iValid : 2; + }; + static_assert(sizeof(ChunkKey) == sizeof(int64_t), "");*/ + + uint64_t posToChunkKey(int32_t iXPos, int32_t iYPos, int32_t iZPos) const { // 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 ChunkKey uXPos = static_cast(iXPos); - const ChunkKey uYPos = static_cast(iYPos); - const ChunkKey uZPos = static_cast(iZPos); + const uint64_t uXPos = static_cast(iXPos); + const uint64_t uYPos = static_cast(iYPos); + const uint64_t uZPos = static_cast(iZPos); - const ChunkKey xKey = ((uXPos >> m_uChunkSideLengthPower) & 0x3FF) << 20; - const ChunkKey yKey = ((uYPos >> m_uChunkSideLengthPower) & 0x3FF) << 10; - const ChunkKey zKey = ((uZPos >> m_uChunkSideLengthPower) & 0x3FF); + 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 ChunkKey key = 0x80000000 | xKey | yKey | zKey; + const uint64_t key = 0x80000000 | xKey | yKey | zKey; return key; } - Chunk* getChunk(ChunkKey iKeyAsInt32) const; + Chunk* getChunk(uint64_t iKeyAsInt32) const; - mutable ChunkKey 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 b93fc2de..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 ChunkKey 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 ChunkKey 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 ChunkKey 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 ChunkKey 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(ChunkKey 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 diff --git a/include/PolyVox/PagedVolumeSampler.inl b/include/PolyVox/PagedVolumeSampler.inl index 3f7a3622..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 ChunkKey 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); From ceeb8f70ce9403169c1bb62ee89fb8bfb1c2b850 Mon Sep 17 00:00:00 2001 From: David Williams Date: Sat, 21 Mar 2015 14:40:49 +0100 Subject: [PATCH 15/22] Revert "Going back to building key by shifting instead of using bitfield." This reverts commit 0d638f98370907b5ffed6b3460e320f4f9b2cf52. --- include/PolyVox/PagedVolume.h | 46 +++++++++++++++----------- include/PolyVox/PagedVolume.inl | 20 +++++------ include/PolyVox/PagedVolumeSampler.inl | 2 +- 3 files changed, 38 insertions(+), 30 deletions(-) diff --git a/include/PolyVox/PagedVolume.h b/include/PolyVox/PagedVolume.h index cf18e4a3..066ff927 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,31 +292,39 @@ 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), ""); - uint64_t posToChunkKey(int32_t iXPos, int32_t iYPos, int32_t iZPos) const + int64_t posToChunkKey(int32_t iXPos, int32_t iYPos, int32_t iZPos) const { - // 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; + 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; } - Chunk* getChunk(uint64_t iKeyAsInt32) const; + Chunk* getChunk(int64_t iKeyAsInt32) const; - mutable uint64_t m_v3dLastAccessedChunkKey = 0; + mutable int64_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 80622bea..5e2a8152 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 uint64_t key = posToChunkKey(uXPos, uYPos, uZPos); + const int64_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 uint64_t key = posToChunkKey(uXPos, uYPos, uZPos); + const int64_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 uint64_t key = posToChunkKey(x, y, z); + const int64_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 uint64_t key = posToChunkKey(x, y, z); + const int64_t key = posToChunkKey(x, y, z); m_mapChunks.erase(key); } } @@ -264,7 +264,7 @@ namespace PolyVox } template - typename PagedVolume::Chunk* PagedVolume::getChunk(uint64_t iKeyAsInt32) const + typename PagedVolume::Chunk* PagedVolume::getChunk(int64_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 = (iKeyAsInt32 >> 20) & 0x3FF; - const int32_t uChunkY = (iKeyAsInt32 >> 10) & 0x3FF; - const int32_t uChunkZ = (iKeyAsInt32) & 0x3FF; + /*const int32_t uChunkX = (key >> 20) & 0x3FF; + const int32_t uChunkY = (key >> 10) & 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(uChunkX, uChunkY, uChunkZ); + ChunkKey realKey = force_cast(iKeyAsInt32); + Vector3DInt32 v3dChunkPos(realKey.iXPos, realKey.iYPos, realKey.iZPos); 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 a9dc9502..cdcfde3c 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 uint64_t key = this->mVolume->posToChunkKey(xPos, yPos, zPos); + const int32_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); From 92eaaae7659f1caa1a885d593e086f52cad58479 Mon Sep 17 00:00:00 2001 From: David Williams Date: Sat, 21 Mar 2015 14:40:57 +0100 Subject: [PATCH 16/22] Revert "Decided to always use a 64-bit chunk key, rather than trying to make it configurable." This reverts commit 69f6f4ac3713e7a9eee804773a516cc5c3412183. --- include/PolyVox/Impl/Config.h | 2 ++ include/PolyVox/PagedVolume.h | 24 ++++++++++++------------ include/PolyVox/PagedVolume.inl | 10 +++++----- 3 files changed, 19 insertions(+), 17 deletions(-) diff --git a/include/PolyVox/Impl/Config.h b/include/PolyVox/Impl/Config.h index 1a46222e..388f25d6 100644 --- a/include/PolyVox/Impl/Config.h +++ b/include/PolyVox/Impl/Config.h @@ -36,4 +36,6 @@ freely, subject to the following restrictions: //#define POLYVOX_ASSERTS_ENABLED #define POLYVOX_THROW_ENABLED +typedef int32_t PagedVolumeChunkKeyIntType; + #endif diff --git a/include/PolyVox/PagedVolume.h b/include/PolyVox/PagedVolume.h index 066ff927..5477b6d1 100644 --- a/include/PolyVox/PagedVolume.h +++ b/include/PolyVox/PagedVolume.h @@ -284,17 +284,17 @@ namespace PolyVox struct ChunkKey { ChunkKey(int32_t x, int32_t y, int32_t z) : iZPos(z), iYPos(y), iXPos(x), iValid(~0) {} - int64_t iZPos : 10; - int64_t iYPos : 10; - int64_t iXPos : 10; + PagedVolumeChunkKeyIntType iZPos : 10; + PagedVolumeChunkKeyIntType iYPos : 10; + PagedVolumeChunkKeyIntType iXPos : 10; // 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. - int64_t iValid : 2; + PagedVolumeChunkKeyIntType iValid : 2; }; - static_assert(sizeof(ChunkKey) == sizeof(int64_t), ""); + static_assert(sizeof(ChunkKey) == sizeof(PagedVolumeChunkKeyIntType), ""); - int64_t posToChunkKey(int32_t iXPos, int32_t iYPos, int32_t iZPos) const + PagedVolumeChunkKeyIntType posToChunkKey(int32_t iXPos, int32_t iYPos, int32_t iZPos) const { iXPos = iXPos >> m_uChunkSideLengthPower; iYPos = iYPos >> m_uChunkSideLengthPower; @@ -306,22 +306,22 @@ namespace PolyVox // 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"); + 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 - int64_t iKeyAsInt64 = force_cast(chunkKey); + PagedVolumeChunkKeyIntType iKeyAsInt32 = force_cast(chunkKey); - return iKeyAsInt64; + return iKeyAsInt32; } - Chunk* getChunk(int64_t iKeyAsInt32) const; + Chunk* getChunk(PagedVolumeChunkKeyIntType iKeyAsInt32) const; - mutable int64_t m_v3dLastAccessedChunkKey = 0; + mutable PagedVolumeChunkKeyIntType m_v3dLastAccessedChunkKey = 0; mutable Chunk* m_pLastAccessedChunk = nullptr; mutable std::unordered_map > m_mapChunks; diff --git a/include/PolyVox/PagedVolume.inl b/include/PolyVox/PagedVolume.inl index 5e2a8152..15c831fd 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 PagedVolumeChunkKeyIntType 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 PagedVolumeChunkKeyIntType 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 PagedVolumeChunkKeyIntType 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 PagedVolumeChunkKeyIntType 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(PagedVolumeChunkKeyIntType 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 From 5fc0317260d98e470f35a36f2f0f1223f1096ed1 Mon Sep 17 00:00:00 2001 From: David Williams Date: Sat, 21 Mar 2015 14:41:04 +0100 Subject: [PATCH 17/22] Revert "Added typedef for chunk key type." This reverts commit 6419c5827bc99d91dd5e96acd17bd507a30dc106. --- include/PolyVox/Impl/Config.h | 4 ---- include/PolyVox/PagedVolume.h | 18 +++++++++--------- include/PolyVox/PagedVolume.inl | 10 +++++----- 3 files changed, 14 insertions(+), 18 deletions(-) diff --git a/include/PolyVox/Impl/Config.h b/include/PolyVox/Impl/Config.h index 388f25d6..1959a6d2 100644 --- a/include/PolyVox/Impl/Config.h +++ b/include/PolyVox/Impl/Config.h @@ -24,8 +24,6 @@ freely, subject to the following restrictions: #ifndef __PolyVox_Config_H__ #define __PolyVox_Config_H__ -#include - //#define POLYVOX_LOG_TRACE_ENABLED //#define POLYVOX_LOG_DEBUG_ENABLED #define POLYVOX_LOG_INFO_ENABLED @@ -36,6 +34,4 @@ freely, subject to the following restrictions: //#define POLYVOX_ASSERTS_ENABLED #define POLYVOX_THROW_ENABLED -typedef int32_t PagedVolumeChunkKeyIntType; - #endif diff --git a/include/PolyVox/PagedVolume.h b/include/PolyVox/PagedVolume.h index 5477b6d1..c53a8426 100644 --- a/include/PolyVox/PagedVolume.h +++ b/include/PolyVox/PagedVolume.h @@ -284,17 +284,17 @@ namespace PolyVox struct ChunkKey { ChunkKey(int32_t x, int32_t y, int32_t z) : iZPos(z), iYPos(y), iXPos(x), iValid(~0) {} - PagedVolumeChunkKeyIntType iZPos : 10; - PagedVolumeChunkKeyIntType iYPos : 10; - PagedVolumeChunkKeyIntType iXPos : 10; + int32_t iZPos : 10; + int32_t iYPos : 10; + int32_t iXPos : 10; // 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. - PagedVolumeChunkKeyIntType iValid : 2; + int32_t iValid : 2; }; - static_assert(sizeof(ChunkKey) == sizeof(PagedVolumeChunkKeyIntType), ""); + static_assert(sizeof(ChunkKey) == sizeof(int32_t), ""); - PagedVolumeChunkKeyIntType posToChunkKey(int32_t iXPos, int32_t iYPos, int32_t iZPos) const + int32_t posToChunkKey(int32_t iXPos, int32_t iYPos, int32_t iZPos) const { iXPos = iXPos >> m_uChunkSideLengthPower; iYPos = iYPos >> m_uChunkSideLengthPower; @@ -314,14 +314,14 @@ namespace PolyVox // If this kind of casting ever causes problems there are // other solutions here: http://stackoverflow.com/a/2468738 - PagedVolumeChunkKeyIntType iKeyAsInt32 = force_cast(chunkKey); + int32_t iKeyAsInt32 = force_cast(chunkKey); return iKeyAsInt32; } - Chunk* getChunk(PagedVolumeChunkKeyIntType iKeyAsInt32) const; + Chunk* getChunk(int32_t iKeyAsInt32) const; - mutable PagedVolumeChunkKeyIntType m_v3dLastAccessedChunkKey = 0; + mutable int32_t m_v3dLastAccessedChunkKey = 0; mutable Chunk* m_pLastAccessedChunk = nullptr; mutable std::unordered_map > m_mapChunks; diff --git a/include/PolyVox/PagedVolume.inl b/include/PolyVox/PagedVolume.inl index 15c831fd..5223e49b 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 PagedVolumeChunkKeyIntType key = posToChunkKey(uXPos, uYPos, uZPos); + const int32_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 PagedVolumeChunkKeyIntType key = posToChunkKey(uXPos, uYPos, uZPos); + const int32_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 PagedVolumeChunkKeyIntType key = posToChunkKey(x, y, z); + const int32_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 PagedVolumeChunkKeyIntType key = posToChunkKey(x, y, z); + const int32_t key = posToChunkKey(x, y, z); m_mapChunks.erase(key); } } @@ -264,7 +264,7 @@ namespace PolyVox } template - typename PagedVolume::Chunk* PagedVolume::getChunk(PagedVolumeChunkKeyIntType iKeyAsInt32) const + typename PagedVolume::Chunk* PagedVolume::getChunk(int32_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 From 672c375a7aed5f1e1515f8a56260bb10ffdd82f8 Mon Sep 17 00:00:00 2001 From: David Williams Date: Sat, 21 Mar 2015 14:41:10 +0100 Subject: [PATCH 18/22] Revert "Work on using a bitfield to set up chunk key." This reverts commit 8bd8f8ba7a91de21fd56d4350f3bf8403a84c4b3. --- include/PolyVox/Impl/Utility.h | 8 ------ include/PolyVox/PagedVolume.h | 48 ++++++++++----------------------- include/PolyVox/PagedVolume.inl | 21 +++++++-------- 3 files changed, 23 insertions(+), 54 deletions(-) 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; } From d477bec540ff62e2ef5efd31a9bb48df5287a298 Mon Sep 17 00:00:00 2001 From: David Williams Date: Sat, 21 Mar 2015 14:41:15 +0100 Subject: [PATCH 19/22] Revert "Replaced Vector3D with integer as key to map." This reverts commit e82d6beca1a5cf7e81c546e6dd0243f54ff5d3e6. --- include/PolyVox/PagedVolume.h | 39 +++---------- include/PolyVox/PagedVolume.inl | 76 +++++++++++--------------- include/PolyVox/PagedVolumeSampler.inl | 20 ++++--- tests/testvolume.cpp | 4 +- tests/testvolume.h | 4 +- 5 files changed, 57 insertions(+), 86 deletions(-) diff --git a/include/PolyVox/PagedVolume.h b/include/PolyVox/PagedVolume.h index f792e67f..4e7d7fb2 100644 --- a/include/PolyVox/PagedVolume.h +++ b/include/PolyVox/PagedVolume.h @@ -281,30 +281,18 @@ namespace PolyVox PagedVolume& operator=(const PagedVolume& rhs); private: - Chunk* getChunk(int32_t key) const; + Chunk* getChunk(int32_t uChunkX, int32_t uChunkY, int32_t uChunkZ) const; - uint32_t posToChunkKey(int32_t iXPos, int32_t iYPos, int32_t iZPos) const - { - // 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); - - 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); - - const uint32_t key = 0x80000000 | xKey | yKey | zKey; - - return key; - } - - mutable int32_t m_v3dLastAccessedChunkKey = 0; + // Storing these properties individually has proved to be faster than keeping + // them in a Vector3DInt32 as it avoids constructions and comparison overheads. + // They are also at the start of the class in the hope that they will be pulled + // into cache - I've got no idea if this actually makes a difference. + mutable int32_t m_v3dLastAccessedChunkX = 0; + mutable int32_t m_v3dLastAccessedChunkY = 0; + mutable int32_t m_v3dLastAccessedChunkZ = 0; mutable Chunk* m_pLastAccessedChunk = nullptr; - mutable std::unordered_map > m_mapChunks; + mutable std::unordered_map > m_mapChunks; mutable uint32_t m_uTimestamper = 0; @@ -322,15 +310,6 @@ namespace PolyVox }; } -/*#define POS_TO_CHUNK_KEY(x, y, z, uChunkSideLengthPower) \ -{ \ - const int32_t chunkX = ((x >> uChunkSideLengthPower) & 0x3FF) << 20; \ - const int32_t chunkY = ((y >> uChunkSideLengthPower) & 0x3FF) << 10; \ - const int32_t chunkZ = ((z >> uChunkSideLengthPower) & 0x3FF); \ - const int32_t key = (-2147483648) | chunkX | chunkY | chunkZ; \ -}*/ - - #include "PolyVox/PagedVolume.inl" #include "PolyVox/PagedVolumeChunk.inl" #include "PolyVox/PagedVolumeSampler.inl" diff --git a/include/PolyVox/PagedVolume.inl b/include/PolyVox/PagedVolume.inl index 077159d5..5d1288c0 100644 --- a/include/PolyVox/PagedVolume.inl +++ b/include/PolyVox/PagedVolume.inl @@ -115,15 +115,15 @@ namespace PolyVox template VoxelType PagedVolume::getVoxel(int32_t uXPos, int32_t uYPos, int32_t uZPos) const { - const int32_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); + const int32_t chunkX = uXPos >> m_uChunkSideLengthPower; + const int32_t chunkY = uYPos >> m_uChunkSideLengthPower; + const int32_t chunkZ = uZPos >> m_uChunkSideLengthPower; const uint16_t xOffset = static_cast(uXPos & m_iChunkMask); const uint16_t yOffset = static_cast(uYPos & m_iChunkMask); const uint16_t zOffset = static_cast(uZPos & m_iChunkMask); + auto pChunk = getChunk(chunkX, chunkY, chunkZ); return pChunk->getVoxel(xOffset, yOffset, zOffset); } @@ -147,15 +147,15 @@ namespace PolyVox template void PagedVolume::setVoxel(int32_t uXPos, int32_t uYPos, int32_t uZPos, VoxelType tValue) { - const int32_t key = posToChunkKey(uXPos, uYPos, uZPos); + const int32_t chunkX = uXPos >> m_uChunkSideLengthPower; + const int32_t chunkY = uYPos >> m_uChunkSideLengthPower; + const int32_t chunkZ = uZPos >> m_uChunkSideLengthPower; - // 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); - - const uint16_t xOffset = static_cast(uXPos & m_iChunkMask); - const uint16_t yOffset = static_cast(uYPos & m_iChunkMask); - const uint16_t zOffset = static_cast(uZPos & m_iChunkMask); + const uint16_t xOffset = static_cast(uXPos - (chunkX << m_uChunkSideLengthPower)); + const uint16_t yOffset = static_cast(uYPos - (chunkY << m_uChunkSideLengthPower)); + const uint16_t zOffset = static_cast(uZPos - (chunkZ << m_uChunkSideLengthPower)); + auto pChunk = getChunk(chunkX, chunkY, chunkZ); pChunk->setVoxel(xOffset, yOffset, zOffset, tValue); } @@ -201,12 +201,8 @@ namespace PolyVox for(int32_t y = v3dStart.getY(); y <= v3dEnd.getY(); y++) { for(int32_t z = v3dStart.getZ(); z <= v3dEnd.getZ(); z++) - { - const int32_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. - getChunk(key); + { + getChunk(x,y,z); } } } @@ -220,7 +216,6 @@ namespace PolyVox { // Clear this pointer as all chunks are about to be removed. m_pLastAccessedChunk = nullptr; - m_v3dLastAccessedChunkKey = 0; // Erase all the most recently used chunks. m_mapChunks.clear(); @@ -234,7 +229,6 @@ namespace PolyVox { // Clear this pointer in case the chunk it points at is flushed. m_pLastAccessedChunk = nullptr; - m_v3dLastAccessedChunkKey = 0; // Convert the start and end positions into chunk space coordinates Vector3DInt32 v3dStart; @@ -256,31 +250,32 @@ namespace PolyVox { for(int32_t z = v3dStart.getZ(); z <= v3dEnd.getZ(); z++) { - const int32_t key = posToChunkKey(x, y, z); - m_mapChunks.erase(key); + m_mapChunks.erase(Vector3DInt32(x, y, z)); } } } } template - typename PagedVolume::Chunk* PagedVolume::getChunk(int32_t key) const + typename PagedVolume::Chunk* PagedVolume::getChunk(int32_t uChunkX, int32_t uChunkY, int32_t uChunkZ) 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 - // is). We could (and previously did) include a simple check for this in this function. However, this function would then usually return - // immediatly (which was good) but we still paid the overhead of a function call, probably because this function is not inlined due to - // being quite large. Therefore we decided the check against the previous accessed chunk should always be done *before* calling this - // function, and we add an assert here to try and catch if the user forgets to do it. Note that this is an internal function so the - // 'user' here is actually PolyVox developers and not the developers of applications using 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(key != m_v3dLastAccessedChunkKey, "This should have been checked as an optimization before calling getChunk()."); + //Check if we have the same chunk as last time, if so there's no need to even update + //the time stamp. If we updated it everytime then that would be every time we touched + //a voxel, which would overflow a uint32_t and require us to use a uint64_t instead. + //This check should also provide a significant speed boost as usually it is true. + if ((uChunkX == m_v3dLastAccessedChunkX) && + (uChunkY == m_v3dLastAccessedChunkY) && + (uChunkZ == m_v3dLastAccessedChunkZ) && + (m_pLastAccessedChunk != 0)) + { + return m_pLastAccessedChunk; + } + + Vector3DInt32 v3dChunkPos(uChunkX, uChunkY, uChunkZ); // 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(key); + Chunk* pChunk = nullptr; + auto itChunk = m_mapChunks.find(v3dChunkPos); // Check whether the chunk was found. if ((itChunk) != m_mapChunks.end()) @@ -293,14 +288,10 @@ 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; // The chunk was not found so we will create a new one. - 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(key, std::unique_ptr(pChunk))); + m_mapChunks.insert(std::make_pair(v3dChunkPos, 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) @@ -322,10 +313,9 @@ namespace PolyVox m_pLastAccessedChunk = pChunk; //m_v3dLastAccessedChunkPos = v3dChunkPos; - /*m_v3dLastAccessedChunkX = uChunkX; + m_v3dLastAccessedChunkX = uChunkX; m_v3dLastAccessedChunkY = uChunkY; - m_v3dLastAccessedChunkZ = uChunkZ;*/ - m_v3dLastAccessedChunkKey = key; + m_v3dLastAccessedChunkZ = uChunkZ; return pChunk; } diff --git a/include/PolyVox/PagedVolumeSampler.inl b/include/PolyVox/PagedVolumeSampler.inl index cdcfde3c..3de9ca10 100644 --- a/include/PolyVox/PagedVolumeSampler.inl +++ b/include/PolyVox/PagedVolumeSampler.inl @@ -97,18 +97,20 @@ 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); + // Then we update the voxel pointer + const int32_t uXChunk = this->mXPosInVolume >> this->mVolume->m_uChunkSideLengthPower; + const int32_t uYChunk = this->mYPosInVolume >> this->mVolume->m_uChunkSideLengthPower; + const int32_t uZChunk = this->mZPosInVolume >> this->mVolume->m_uChunkSideLengthPower; - // Then we update the voxel pointer. - auto pCurrentChunk = (key == this->mVolume->m_v3dLastAccessedChunkKey) ? this->mVolume->m_pLastAccessedChunk : this->mVolume->getChunk(key); + const uint16_t uXPosInChunk = static_cast(this->mXPosInVolume - (uXChunk << this->mVolume->m_uChunkSideLengthPower)); + const uint16_t uYPosInChunk = static_cast(this->mYPosInVolume - (uYChunk << this->mVolume->m_uChunkSideLengthPower)); + const uint16_t uZPosInChunk = static_cast(this->mZPosInVolume - (uZChunk << this->mVolume->m_uChunkSideLengthPower)); - const uint16_t xOffset = static_cast(xPos & this->mVolume->m_iChunkMask); - const uint16_t yOffset = static_cast(yPos & this->mVolume->m_iChunkMask); - const uint16_t zOffset = static_cast(zPos & this->mVolume->m_iChunkMask); + const uint32_t uVoxelIndexInChunk = uXPosInChunk + + uYPosInChunk * this->mVolume->m_uChunkSideLength + + uZPosInChunk * this->mVolume->m_uChunkSideLength * this->mVolume->m_uChunkSideLength; - const uint32_t uVoxelIndexInChunk = xOffset + - yOffset * this->mVolume->m_uChunkSideLength + - zOffset * this->mVolume->m_uChunkSideLength * this->mVolume->m_uChunkSideLength; + auto pCurrentChunk = this->mVolume->getChunk(uXChunk, uYChunk, uZChunk); mCurrentVoxel = pCurrentChunk->m_tData + uVoxelIndexInChunk; } diff --git a/tests/testvolume.cpp b/tests/testvolume.cpp index 702d9ab1..27d10a57 100644 --- a/tests/testvolume.cpp +++ b/tests/testvolume.cpp @@ -278,7 +278,7 @@ TestVolume::~TestVolume() * RawVolume Tests */ -/*void TestVolume::testRawVolumeDirectAccessAllInternalForwards() +void TestVolume::testRawVolumeDirectAccessAllInternalForwards() { int32_t result = 0; @@ -364,7 +364,7 @@ void TestVolume::testRawVolumeSamplersWithExternalBackwards() result = testSamplersWithWrappingBackwards(m_pRawVolume, m_regExternal); } QCOMPARE(result, static_cast(-993539594)); -}*/ +} /* * PagedVolume Tests diff --git a/tests/testvolume.h b/tests/testvolume.h index 1e3b8406..79bab98d 100644 --- a/tests/testvolume.h +++ b/tests/testvolume.h @@ -38,14 +38,14 @@ public: ~TestVolume(); private slots: - /*void testRawVolumeDirectAccessAllInternalForwards(); + void testRawVolumeDirectAccessAllInternalForwards(); void testRawVolumeSamplersAllInternalForwards(); void testRawVolumeDirectAccessWithExternalForwards(); void testRawVolumeSamplersWithExternalForwards(); void testRawVolumeDirectAccessAllInternalBackwards(); void testRawVolumeSamplersAllInternalBackwards(); void testRawVolumeDirectAccessWithExternalBackwards(); - void testRawVolumeSamplersWithExternalBackwards();*/ + void testRawVolumeSamplersWithExternalBackwards(); void testPagedVolumeDirectAccessAllInternalForwards(); void testPagedVolumeSamplersAllInternalForwards(); From 778238d11dc0292e8331cbd29f896428f10e12da Mon Sep 17 00:00:00 2001 From: David Williams Date: Sat, 21 Mar 2015 14:57:48 +0100 Subject: [PATCH 20/22] Moved the test for whether we are accessing the same voxel as last time. --- include/PolyVox/PagedVolume.inl | 32 ++++++++++++++++++++++---- include/PolyVox/PagedVolumeSampler.inl | 13 ++++++++++- 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/include/PolyVox/PagedVolume.inl b/include/PolyVox/PagedVolume.inl index 5d1288c0..081e9b17 100644 --- a/include/PolyVox/PagedVolume.inl +++ b/include/PolyVox/PagedVolume.inl @@ -123,7 +123,19 @@ namespace PolyVox const uint16_t yOffset = static_cast(uYPos & m_iChunkMask); const uint16_t zOffset = static_cast(uZPos & m_iChunkMask); - auto pChunk = getChunk(chunkX, chunkY, chunkZ); + Chunk* pChunk; + if ((chunkX == m_v3dLastAccessedChunkX) && + (chunkY == m_v3dLastAccessedChunkY) && + (chunkZ == m_v3dLastAccessedChunkZ) && + (m_pLastAccessedChunk != 0)) + { + pChunk = m_pLastAccessedChunk; + } + else + { + pChunk = getChunk(chunkX, chunkY, chunkZ); + } + return pChunk->getVoxel(xOffset, yOffset, zOffset); } @@ -155,7 +167,19 @@ namespace PolyVox const uint16_t yOffset = static_cast(uYPos - (chunkY << m_uChunkSideLengthPower)); const uint16_t zOffset = static_cast(uZPos - (chunkZ << m_uChunkSideLengthPower)); - auto pChunk = getChunk(chunkX, chunkY, chunkZ); + Chunk* pChunk; + if ((chunkX == m_v3dLastAccessedChunkX) && + (chunkY == m_v3dLastAccessedChunkY) && + (chunkZ == m_v3dLastAccessedChunkZ) && + (m_pLastAccessedChunk != 0)) + { + pChunk = m_pLastAccessedChunk; + } + else + { + pChunk = getChunk(chunkX, chunkY, chunkZ); + } + pChunk->setVoxel(xOffset, yOffset, zOffset, tValue); } @@ -263,13 +287,13 @@ namespace PolyVox //the time stamp. If we updated it everytime then that would be every time we touched //a voxel, which would overflow a uint32_t and require us to use a uint64_t instead. //This check should also provide a significant speed boost as usually it is true. - if ((uChunkX == m_v3dLastAccessedChunkX) && + /*if ((uChunkX == m_v3dLastAccessedChunkX) && (uChunkY == m_v3dLastAccessedChunkY) && (uChunkZ == m_v3dLastAccessedChunkZ) && (m_pLastAccessedChunk != 0)) { return m_pLastAccessedChunk; - } + }*/ Vector3DInt32 v3dChunkPos(uChunkX, uChunkY, uChunkZ); diff --git a/include/PolyVox/PagedVolumeSampler.inl b/include/PolyVox/PagedVolumeSampler.inl index 3de9ca10..71cb633a 100644 --- a/include/PolyVox/PagedVolumeSampler.inl +++ b/include/PolyVox/PagedVolumeSampler.inl @@ -110,7 +110,18 @@ namespace PolyVox uYPosInChunk * this->mVolume->m_uChunkSideLength + uZPosInChunk * this->mVolume->m_uChunkSideLength * this->mVolume->m_uChunkSideLength; - auto pCurrentChunk = this->mVolume->getChunk(uXChunk, uYChunk, uZChunk); + Chunk* pCurrentChunk; + if ((uXChunk == this->mVolume->m_v3dLastAccessedChunkX) && + (uYChunk == this->mVolume->m_v3dLastAccessedChunkY) && + (uZChunk == this->mVolume->m_v3dLastAccessedChunkZ) && + (this->mVolume->m_pLastAccessedChunk != 0)) + { + pCurrentChunk = this->mVolume->m_pLastAccessedChunk; + } + else + { + pCurrentChunk = this->mVolume->getChunk(uXChunk, uYChunk, uZChunk); + } mCurrentVoxel = pCurrentChunk->m_tData + uVoxelIndexInChunk; } From b027cf1a0c454bb6321331a454156009fcb9420f Mon Sep 17 00:00:00 2001 From: David Williams Date: Sat, 21 Mar 2015 16:22:23 +0100 Subject: [PATCH 21/22] Moved common code into function. --- include/PolyVox/PagedVolume.h | 3 ++- include/PolyVox/PagedVolume.inl | 35 ++++++++------------------ include/PolyVox/PagedVolumeSampler.inl | 14 ++--------- 3 files changed, 15 insertions(+), 37 deletions(-) diff --git a/include/PolyVox/PagedVolume.h b/include/PolyVox/PagedVolume.h index 4e7d7fb2..c4e306ed 100644 --- a/include/PolyVox/PagedVolume.h +++ b/include/PolyVox/PagedVolume.h @@ -280,7 +280,8 @@ namespace PolyVox /// Assignment operator PagedVolume& operator=(const PagedVolume& rhs); - private: + private: + bool canReuseLastAccessedChunk(int32_t iChunkX, int32_t iChunkY, int32_t iChunkZ) const; Chunk* getChunk(int32_t uChunkX, int32_t uChunkY, int32_t uChunkZ) const; // Storing these properties individually has proved to be faster than keeping diff --git a/include/PolyVox/PagedVolume.inl b/include/PolyVox/PagedVolume.inl index 081e9b17..38d8dc91 100644 --- a/include/PolyVox/PagedVolume.inl +++ b/include/PolyVox/PagedVolume.inl @@ -123,18 +123,7 @@ namespace PolyVox const uint16_t yOffset = static_cast(uYPos & m_iChunkMask); const uint16_t zOffset = static_cast(uZPos & m_iChunkMask); - Chunk* pChunk; - if ((chunkX == m_v3dLastAccessedChunkX) && - (chunkY == m_v3dLastAccessedChunkY) && - (chunkZ == m_v3dLastAccessedChunkZ) && - (m_pLastAccessedChunk != 0)) - { - pChunk = m_pLastAccessedChunk; - } - else - { - pChunk = getChunk(chunkX, chunkY, chunkZ); - } + auto pChunk = canReuseLastAccessedChunk(chunkX, chunkY, chunkZ) ? m_pLastAccessedChunk : getChunk(chunkX, chunkY, chunkZ); return pChunk->getVoxel(xOffset, yOffset, zOffset); } @@ -167,18 +156,7 @@ namespace PolyVox const uint16_t yOffset = static_cast(uYPos - (chunkY << m_uChunkSideLengthPower)); const uint16_t zOffset = static_cast(uZPos - (chunkZ << m_uChunkSideLengthPower)); - Chunk* pChunk; - if ((chunkX == m_v3dLastAccessedChunkX) && - (chunkY == m_v3dLastAccessedChunkY) && - (chunkZ == m_v3dLastAccessedChunkZ) && - (m_pLastAccessedChunk != 0)) - { - pChunk = m_pLastAccessedChunk; - } - else - { - pChunk = getChunk(chunkX, chunkY, chunkZ); - } + auto pChunk = canReuseLastAccessedChunk(chunkX, chunkY, chunkZ) ? m_pLastAccessedChunk : getChunk(chunkX, chunkY, chunkZ); pChunk->setVoxel(xOffset, yOffset, zOffset, tValue); } @@ -280,6 +258,15 @@ namespace PolyVox } } + template + bool PagedVolume::canReuseLastAccessedChunk(int32_t iChunkX, int32_t iChunkY, int32_t iChunkZ) const + { + return ((iChunkX == m_v3dLastAccessedChunkX) && + (iChunkY == m_v3dLastAccessedChunkY) && + (iChunkZ == m_v3dLastAccessedChunkZ) && + (m_pLastAccessedChunk)); + } + template typename PagedVolume::Chunk* PagedVolume::getChunk(int32_t uChunkX, int32_t uChunkY, int32_t uChunkZ) const { diff --git a/include/PolyVox/PagedVolumeSampler.inl b/include/PolyVox/PagedVolumeSampler.inl index 71cb633a..8173bacc 100644 --- a/include/PolyVox/PagedVolumeSampler.inl +++ b/include/PolyVox/PagedVolumeSampler.inl @@ -110,18 +110,8 @@ namespace PolyVox uYPosInChunk * this->mVolume->m_uChunkSideLength + uZPosInChunk * this->mVolume->m_uChunkSideLength * this->mVolume->m_uChunkSideLength; - Chunk* pCurrentChunk; - if ((uXChunk == this->mVolume->m_v3dLastAccessedChunkX) && - (uYChunk == this->mVolume->m_v3dLastAccessedChunkY) && - (uZChunk == this->mVolume->m_v3dLastAccessedChunkZ) && - (this->mVolume->m_pLastAccessedChunk != 0)) - { - pCurrentChunk = this->mVolume->m_pLastAccessedChunk; - } - else - { - pCurrentChunk = this->mVolume->getChunk(uXChunk, uYChunk, uZChunk); - } + auto pCurrentChunk = this->mVolume->canReuseLastAccessedChunk(uXChunk, uYChunk, uZChunk) ? + this->mVolume->m_pLastAccessedChunk : this->mVolume->getChunk(uXChunk, uYChunk, uZChunk); mCurrentVoxel = pCurrentChunk->m_tData + uVoxelIndexInChunk; } From 3facd4df4163401ca9325fda38dd4c2f7f88bb70 Mon Sep 17 00:00:00 2001 From: David Williams Date: Sat, 21 Mar 2015 16:27:43 +0100 Subject: [PATCH 22/22] Removed commented out code. --- include/PolyVox/PagedVolume.inl | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/include/PolyVox/PagedVolume.inl b/include/PolyVox/PagedVolume.inl index 38d8dc91..545f017e 100644 --- a/include/PolyVox/PagedVolume.inl +++ b/include/PolyVox/PagedVolume.inl @@ -270,18 +270,6 @@ namespace PolyVox template typename PagedVolume::Chunk* PagedVolume::getChunk(int32_t uChunkX, int32_t uChunkY, int32_t uChunkZ) const { - //Check if we have the same chunk as last time, if so there's no need to even update - //the time stamp. If we updated it everytime then that would be every time we touched - //a voxel, which would overflow a uint32_t and require us to use a uint64_t instead. - //This check should also provide a significant speed boost as usually it is true. - /*if ((uChunkX == m_v3dLastAccessedChunkX) && - (uChunkY == m_v3dLastAccessedChunkY) && - (uChunkZ == m_v3dLastAccessedChunkZ) && - (m_pLastAccessedChunk != 0)) - { - return m_pLastAccessedChunk; - }*/ - Vector3DInt32 v3dChunkPos(uChunkX, uChunkY, uChunkZ); // The chunk was not the same as last time, but we can now hope it is in the set of most recently used chunks.