From d1b151f92f6ce0c98cbfd9189761f9c14297c743 Mon Sep 17 00:00:00 2001 From: EmaMaker Date: Wed, 4 Oct 2023 11:14:18 +0200 Subject: [PATCH 1/4] chunk: typedef for chunk state --- include/chunk.hpp | 24 ++++++++++++------------ src/chunk.cpp | 2 +- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/include/chunk.hpp b/include/chunk.hpp index 26a7f86..81339a3 100644 --- a/include/chunk.hpp +++ b/include/chunk.hpp @@ -23,6 +23,7 @@ // int32_t is fine, since i'm limiting the coordinate to only use up to ten bits (1023). There's actually two spare bits typedef int32_t chunk_index_t; typedef int16_t chunk_intcoord_t; +typedef uint16_t chunk_state_t; namespace Chunk { @@ -30,13 +31,13 @@ namespace Chunk chunk_index_t calculateIndex(chunk_intcoord_t i, chunk_intcoord_t j, chunk_intcoord_t k); chunk_index_t calculateIndex(glm::vec3 pos); - constexpr uint8_t CHUNK_STATE_GENERATED = 1; - constexpr uint8_t CHUNK_STATE_MESHED = 2; - constexpr uint8_t CHUNK_STATE_MESH_LOADED = 4; - constexpr uint8_t CHUNK_STATE_LOADED = 8; - constexpr uint8_t CHUNK_STATE_OUTOFVISION = 16; - constexpr uint8_t CHUNK_STATE_UNLOADED = 32; - constexpr uint8_t CHUNK_STATE_EMPTY = 64; + constexpr chunk_state_t CHUNK_STATE_GENERATED = 1; + constexpr chunk_state_t CHUNK_STATE_MESHED = 2; + constexpr chunk_state_t CHUNK_STATE_MESH_LOADED = 4; + constexpr chunk_state_t CHUNK_STATE_LOADED = 8; + constexpr chunk_state_t CHUNK_STATE_OUTOFVISION = 16; + constexpr chunk_state_t CHUNK_STATE_UNLOADED = 32; + constexpr chunk_state_t CHUNK_STATE_EMPTY = 64; int coord3DTo1D(int x, int y, int z); @@ -52,9 +53,9 @@ namespace Chunk void deleteBuffers(); glm::vec3 getPosition() { return this->position; } - uint8_t getTotalState() { return this->state; } - bool getState(uint8_t n) { return (this->state & n) == n; } - void setState(uint8_t nstate, bool value); + chunk_state_t getTotalState() { return this->state; } + bool getState(chunk_state_t n) { return (this->state & n) == n; } + void setState(chunk_state_t nstate, bool value); void setBlock(Block b, int x, int y, int z); void setBlocks(int start, int end, Block b); @@ -71,8 +72,7 @@ namespace Chunk glm::vec3 position{}; IntervalMap blocks{}; - std::atomic_uint8_t state{0}; - + std::atomic state{0}; chunk_index_t index; }; }; diff --git a/src/chunk.cpp b/src/chunk.cpp index 982edf8..6eebecd 100644 --- a/src/chunk.cpp +++ b/src/chunk.cpp @@ -69,7 +69,7 @@ namespace Chunk this->blocks.insert(start < 0 ? 0 : start, end >= CHUNK_VOLUME ? CHUNK_VOLUME : end, b); } - void Chunk::setState(uint8_t nstate, bool value) + void Chunk::setState(chunk_state_t nstate, bool value) { if (value) this->state.fetch_or(nstate); -- 2.40.1 From 4e7fadd2b9f5be2a750bc48a7ab983022ee39f5c Mon Sep 17 00:00:00 2001 From: EmaMaker Date: Wed, 4 Oct 2023 11:23:38 +0200 Subject: [PATCH 2/4] chunk: use chunk state to mark presence of chunk in thread communication queues --- include/chunk.hpp | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/include/chunk.hpp b/include/chunk.hpp index 81339a3..19a9dc6 100644 --- a/include/chunk.hpp +++ b/include/chunk.hpp @@ -38,6 +38,9 @@ namespace Chunk constexpr chunk_state_t CHUNK_STATE_OUTOFVISION = 16; constexpr chunk_state_t CHUNK_STATE_UNLOADED = 32; constexpr chunk_state_t CHUNK_STATE_EMPTY = 64; + constexpr chunk_state_t CHUNK_STATE_IN_GENERATION_QUEUE = 128; + constexpr chunk_state_t CHUNK_STATE_IN_MESHING_QUEUE = 256; + constexpr chunk_state_t CHUNK_STATE_IN_DELETING_QUEUE = 512; int coord3DTo1D(int x, int y, int z); @@ -53,9 +56,14 @@ namespace Chunk void deleteBuffers(); glm::vec3 getPosition() { return this->position; } - chunk_state_t getTotalState() { return this->state; } - bool getState(chunk_state_t n) { return (this->state & n) == n; } void setState(chunk_state_t nstate, bool value); + bool getState(chunk_state_t n) { return (this->state & n) == n; } + bool isFree(){ return !( + this->getState(CHUNK_STATE_IN_GENERATION_QUEUE) || + this->getState(CHUNK_STATE_IN_MESHING_QUEUE) || + this->getState(CHUNK_STATE_IN_DELETING_QUEUE) + ); } + chunk_state_t getTotalState() { return this->state; } void setBlock(Block b, int x, int y, int z); void setBlocks(int start, int end, Block b); -- 2.40.1 From f4947d5f705532c62a49b332b0cbf03f170f5469 Mon Sep 17 00:00:00 2001 From: EmaMaker Date: Wed, 4 Oct 2023 11:27:53 +0200 Subject: [PATCH 3/4] debugwindow: catch exception to avoid crash when missing parameters --- src/debugwindow.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/debugwindow.cpp b/src/debugwindow.cpp index 70ab802..f6e7977 100644 --- a/src/debugwindow.cpp +++ b/src/debugwindow.cpp @@ -121,7 +121,9 @@ namespace debug{ std::any_cast(parameters.at("update_chunks_bucket"))); } }catch(const std::bad_any_cast& e){ - std::cout << e.what(); + std::cout << e.what() << std::endl; + }catch(const std::out_of_range& e){ + std::cout << e.what() << std::endl; } ImGui::End(); -- 2.40.1 From 88abf215023546dbac0fd33d4df787ae1ee7bff3 Mon Sep 17 00:00:00 2001 From: EmaMaker Date: Wed, 4 Oct 2023 11:33:00 +0200 Subject: [PATCH 4/4] multithread: refactor update thread, communication between mesh/gen/upd threads - Use parallel_for to iterate over all the stored chunks - Only push a chunk to a queue if it is not already present, using chunk state bitfield (solves memory leak, continously adding new elements to a queue) - Properly delete an element from chunks concurrent_hash_map using accessor, then free the memory (solves memory leak: not being able to delete old elements and always adding new ones) Breaks: - Rendering (will be properly refactored in a future commit) - Block picking (will be refactored in a future commit) --- include/chunkmanager.hpp | 3 +- src/chunkmanager.cpp | 198 ++++++++++++++++++++++++++++----------- src/renderer.cpp | 8 +- 3 files changed, 150 insertions(+), 59 deletions(-) diff --git a/include/chunkmanager.hpp b/include/chunkmanager.hpp index b393760..9aa2d19 100644 --- a/include/chunkmanager.hpp +++ b/include/chunkmanager.hpp @@ -30,11 +30,10 @@ namespace chunkmanager typedef oneapi::tbb::concurrent_priority_queue ChunkPriorityQueue; void init(); - void blockpick(bool place); + //void blockpick(bool place); void stop(); void destroy(); - oneapi::tbb::concurrent_queue& getDeleteVector(); std::array, chunks_volume>& getChunksIndices(); Block getBlockAtPos(int x, int y, int z); void update(); diff --git a/src/chunkmanager.cpp b/src/chunkmanager.cpp index 6f0d6d9..1eb021d 100644 --- a/src/chunkmanager.cpp +++ b/src/chunkmanager.cpp @@ -8,6 +8,8 @@ #include #include +#include + #include "block.hpp" #include "chunk.hpp" #include "chunkgenerator.hpp" @@ -29,6 +31,7 @@ namespace chunkmanager /* Multithreading */ std::atomic_bool should_run; std::thread gen_thread, mesh_thread, update_thread; + // Queue of chunks to be generated ChunkPriorityQueue chunks_to_generate_queue; // Queue of chunks to be meshed @@ -66,7 +69,11 @@ namespace chunkmanager void generate(){ while(should_run){ ChunkPQEntry entry; - if(chunks_to_generate_queue.try_pop(entry)) generateChunk(entry.first); + if(chunks_to_generate_queue.try_pop(entry)){ + Chunk::Chunk* chunk = entry.first; + generateChunk(chunk); + chunk->setState(Chunk::CHUNK_STATE_IN_GENERATION_QUEUE, false); + } } chunks_to_generate_queue.clear(); } @@ -77,78 +84,160 @@ namespace chunkmanager ChunkPQEntry entry; if(chunks_to_mesh_queue.try_pop(entry)){ Chunk::Chunk* chunk = entry.first; - if(chunk->getState(Chunk::CHUNK_STATE_GENERATED)){ - chunkmesher::mesh(chunk); - renderer::getChunksToRender().insert(chunk); - } + chunkmesher::mesh(chunk); + renderer::getChunksToRender().insert(chunk); + chunk->setState(Chunk::CHUNK_STATE_IN_MESHING_QUEUE, false); } } chunks_to_mesh_queue.clear(); } - oneapi::tbb::concurrent_queue chunks_todelete; - int nUnloaded{0}; + oneapi::tbb::concurrent_queue chunks_todelete; void update(){ while(should_run) { - int chunkX=static_cast(theCamera.getAtomicPosX() / CHUNK_SIZE); - int chunkY=static_cast(theCamera.getAtomicPosY() / CHUNK_SIZE); - int chunkZ=static_cast(theCamera.getAtomicPosZ() / CHUNK_SIZE); + /* Setup variables for the whole loop */ + // Atomic is needed by parallel_for + std::atomic_int nUnloaded{0}, nMarkUnload{0}, nExplored{0}, nMeshed{0}, nGenerated{0}; + std::atomic_int chunkX=static_cast(theCamera.getAtomicPosX() / CHUNK_SIZE); + std::atomic_int chunkY=static_cast(theCamera.getAtomicPosY() / CHUNK_SIZE); + std::atomic_int chunkZ=static_cast(theCamera.getAtomicPosZ() / CHUNK_SIZE); - // Update other chunks + /* Delete old chunks */ + // In my head it makes sense to first delete old chunks, then create new ones + // I think it's easier for memory allocator to re-use the memory that was freed just + // before, but this isn't backed be any evidence and I might be wrong. Anyway this way + // works fine so I'm gonna keep it. + int i; + ChunkTable::accessor a; + while(chunks_todelete.try_pop(i)){ + const int index = i; + if(chunks.find(a, index)){ + Chunk::Chunk* c = a->second; + // Use the accessor to erase the element + // Using the key doesn't work + if(chunks.erase(a)){ + nUnloaded++; + delete c; + } else { + c->setState(Chunk::CHUNK_STATE_IN_DELETING_QUEUE, false); + std::cout << "failed to delete " << index << std::endl; + } + } else std::cout << "no such element found to delete\n"; + } + + /* Create new chunks around the player */ for(int i = 0; i < chunks_volume; i++) { const chunk_intcoord_t x = chunks_indices[i][0] + chunkX; const chunk_intcoord_t y = chunks_indices[i][1] + chunkY; const chunk_intcoord_t z = chunks_indices[i][2] + chunkZ; if(x < 0 || y < 0 || z < 0 || x > 1023 || y > 1023 || z > 1023) continue; + nExplored++; + const chunk_index_t index = Chunk::calculateIndex(x, y, z); + ChunkTable::accessor a; + if(!chunks.find(a, index)) chunks.emplace(a, std::make_pair(index, new + Chunk::Chunk(glm::vec3(x,y,z)))); + } - ChunkTable::accessor a, a1, a2, b1, b2, c1, c2; - if(!chunks.find(a, index)) chunks.emplace(a, std::make_pair(index, new Chunk::Chunk(glm::vec3(x,y,z)))); + /* Update all the chunks */ + oneapi::tbb::parallel_for(chunks.range(), [&](ChunkTable::range_type &r){ + for(ChunkTable::iterator a = r.begin(); a != r.end(); a++){ + Chunk::Chunk* c = a->second; + + int x = c->getPosition().x; + int y = c->getPosition().y; + int z = c->getPosition().z; + int distx = x - chunkX; + int disty = y - chunkY; + int distz = z - chunkZ; + + // Local variables avoid continously having to call atomic variables + int gen{0}, mesh{0}, unload{0}; - if(! (a->second->getState(Chunk::CHUNK_STATE_GENERATED))) { - chunks_to_generate_queue.push(std::make_pair(a->second, GENERATION_PRIORITY_NORMAL)); - }else if(! (a->second->getState(Chunk::CHUNK_STATE_MESHED))){ if( - (x + 1 > 1023 || (chunks.find(a1, Chunk::calculateIndex(x+1, y, z)) && - a1->second->getState(Chunk::CHUNK_STATE_GENERATED))) && - (x - 1 < 0|| (chunks.find(a1, Chunk::calculateIndex(x-1, y, z)) && - a1->second->getState(Chunk::CHUNK_STATE_GENERATED))) && - (y + 1 > 1023 || (chunks.find(a1, Chunk::calculateIndex(x, y+1, z)) && - a1->second->getState(Chunk::CHUNK_STATE_GENERATED))) && - (y - 1 < 0|| (chunks.find(a1, Chunk::calculateIndex(x, y-1, z)) && - a1->second->getState(Chunk::CHUNK_STATE_GENERATED))) && - (z + 1 > 1023 || (chunks.find(a1, Chunk::calculateIndex(x, y, z+1)) && - a1->second->getState(Chunk::CHUNK_STATE_GENERATED))) && - (z - 1 < 0|| (chunks.find(a1, Chunk::calculateIndex(x, y, z-1)) && - a1->second->getState(Chunk::CHUNK_STATE_GENERATED))) - ) - chunks_to_mesh_queue.push(std::make_pair(a->second, MESHING_PRIORITY_NORMAL)); + distx >= -RENDER_DISTANCE && distx < RENDER_DISTANCE && + disty >= -RENDER_DISTANCE && disty < RENDER_DISTANCE && + distz >= -RENDER_DISTANCE && distz < RENDER_DISTANCE + ){ + + // If within distance + // Reset out-of-view flags + c->setState(Chunk::CHUNK_STATE_OUTOFVISION, false); + c->setState(Chunk::CHUNK_STATE_UNLOADED, false); + + // If not yet generated + if(!c->getState(Chunk::CHUNK_STATE_GENERATED)){ + if(c->isFree()){ + // Generate + + // Mark as present in the queue before sending to avoid strange + // a chunk being marked as in the queue after it was already + // processed + c->setState(Chunk::CHUNK_STATE_IN_GENERATION_QUEUE, true); + chunks_to_generate_queue.push(std::make_pair(c, GENERATION_PRIORITY_NORMAL)); + } + }else{ + gen++; + // If generated but not yet meshed + if(!c->getState(Chunk::CHUNK_STATE_MESHED)){ + ChunkTable::accessor a1; + + // Checking if nearby chunks have been generated allows for seamless + // borders between chunks + if(c->isFree() && + (distx+1 >= RENDER_DISTANCE || x + 1 > 1023 || (chunks.find(a1, Chunk::calculateIndex(x+1, y, z)) && + a1->second->getState(Chunk::CHUNK_STATE_GENERATED))) && + (distx-1 < -RENDER_DISTANCE || x - 1 < 0 || (chunks.find(a1, Chunk::calculateIndex(x-1, y, z)) && + a1->second->getState(Chunk::CHUNK_STATE_GENERATED))) && + (disty+1 >= RENDER_DISTANCE || y + 1 > 1023 || (chunks.find(a1, Chunk::calculateIndex(x, y+1, z)) && + a1->second->getState(Chunk::CHUNK_STATE_GENERATED))) && + (disty-1 < -RENDER_DISTANCE || y - 1 < 0|| (chunks.find(a1, Chunk::calculateIndex(x, y-1, z)) && + a1->second->getState(Chunk::CHUNK_STATE_GENERATED))) && + (distz+1 >= RENDER_DISTANCE || z + 1 > 1023 || (chunks.find(a1, Chunk::calculateIndex(x, y, z+1)) && + a1->second->getState(Chunk::CHUNK_STATE_GENERATED))) && + (distz-1 < -RENDER_DISTANCE || z - 1 < 0|| (chunks.find(a1, Chunk::calculateIndex(x, y, z-1)) && + a1->second->getState(Chunk::CHUNK_STATE_GENERATED))) + ) + { + // Mesh + + // Mark as present in the queue before sending to avoid strange + // a chunk being marked as in the queue after it was already + // processed + c->setState(Chunk::CHUNK_STATE_IN_MESHING_QUEUE, true); + chunks_to_mesh_queue.push(std::make_pair(c, MESHING_PRIORITY_NORMAL)); + } + }else mesh++; + } + + }else{ + // If not within distance + if(c->getState(Chunk::CHUNK_STATE_OUTOFVISION)){ + // If enough time has passed, set to be deleted + if(c->isFree() && glfwGetTime() - c->unload_timer >= UNLOAD_TIMEOUT){ + c->setState(Chunk::CHUNK_STATE_IN_DELETING_QUEUE, true); + chunks_todelete.push(c->getIndex()); + unload++; + } + }else{ + // Mark as out of view, and start waiting time + c->setState(Chunk::CHUNK_STATE_OUTOFVISION, true); + c->setState(Chunk::CHUNK_STATE_UNLOADED, false); + c->unload_timer = glfwGetTime(); + } + } + + // Update atomic variables only once at the end + nGenerated += gen; + nMeshed += mesh; + nMarkUnload += unload; } + }); - a.release(); - } - - debug::window::set_parameter("update_chunks_total", (int) (chunks.size())); - debug::window::set_parameter("update_chunks_bucket", (int) (chunks.max_size())); - - Chunk::Chunk* n; - nUnloaded = 0; - while(chunks_todelete.try_pop(n)){ - chunk_intcoord_t x = static_cast(n->getPosition().x); - chunk_intcoord_t y = static_cast(n->getPosition().y); - chunk_intcoord_t z = static_cast(n->getPosition().z); - if(x > 1023 || y > 1023 || z > 1023) continue; - const uint32_t index = Chunk::calculateIndex(x, y, z); - - chunks.erase(index); - //delete n; - nUnloaded++; - } } } - oneapi::tbb::concurrent_queue& getDeleteVector(){ return chunks_todelete; } std::array, chunks_volume>& getChunksIndices(){ return chunks_indices; } void stop() { @@ -164,12 +253,13 @@ namespace chunkmanager } void destroy(){ - /*for(const auto& n : chunks){ + for(const auto& n : chunks){ delete n.second; - }*/ + } + chunks.clear(); } - void blockpick(bool place){ + /*void blockpick(bool place){ // cast a ray from the camera in the direction pointed by the camera itself glm::vec3 pos = glm::vec3(theCamera.getAtomicPosX(), theCamera.getAtomicPosY(), theCamera.getAtomicPosZ()); @@ -263,7 +353,7 @@ namespace chunkmanager break; } } - } + }*/ Block getBlockAtPos(int x, int y, int z){ if(x < 0 || y < 0 || z < 0) return Block::NULLBLK; diff --git a/src/renderer.cpp b/src/renderer.cpp index ab8cd16..0dcd3d2 100644 --- a/src/renderer.cpp +++ b/src/renderer.cpp @@ -140,10 +140,11 @@ namespace renderer{ chunkmesher::MeshData* m; while(MeshDataQueue.try_pop(m)){ - chunkmesher::sendtogpu(m); + //chunkmesher::sendtogpu(m); chunkmesher::getMeshDataQueue().push(m); } + /* for(auto& c : chunks_torender){ float dist = glm::distance(c->getPosition(), cameraChunkPos); if(dist <= static_cast(RENDER_DISTANCE)){ @@ -211,6 +212,7 @@ namespace renderer{ } } + */ total = chunks_torender.size(); debug::window::set_parameter("render_chunks_total", total); @@ -220,14 +222,14 @@ namespace renderer{ debug::window::set_parameter("render_chunks_deleted", (int) (render_todelete.size())); debug::window::set_parameter("render_chunks_vertices", vertices); - for(auto& c : render_todelete){ + /*for(auto& c : render_todelete){ // we can get away with unsafe erase as access to the container is only done by this // thread c->deleteBuffers(); chunks_torender.unsafe_erase(c); chunkmanager::getDeleteVector().push(c); } - render_todelete.clear(); + render_todelete.clear();*/ /* DISPLAY TEXTURE ON A QUAD THAT FILLS THE SCREEN */ // Now to render the quad, with the texture on top -- 2.40.1