Merge pull request 'Refactor Secondary Threads' (#12) from multithread-refactor into main

Reviewed-on: #12

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,16 @@ 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 chunk_state_t CHUNK_STATE_GENERATED = 1;
-    constexpr uint8_t CHUNK_STATE_MESHED = 2;
+    constexpr chunk_state_t CHUNK_STATE_MESHED = 2;
-    constexpr uint8_t CHUNK_STATE_MESH_LOADED = 4;
+    constexpr chunk_state_t CHUNK_STATE_MESH_LOADED = 4;
-    constexpr uint8_t CHUNK_STATE_LOADED = 8;
+    constexpr chunk_state_t CHUNK_STATE_LOADED = 8;
-    constexpr uint8_t CHUNK_STATE_OUTOFVISION = 16;
+    constexpr chunk_state_t CHUNK_STATE_OUTOFVISION = 16;
-    constexpr uint8_t CHUNK_STATE_UNLOADED = 32;
+    constexpr chunk_state_t CHUNK_STATE_UNLOADED = 32;
-    constexpr uint8_t CHUNK_STATE_EMPTY = 64;
+    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);
 
@@ -52,9 +56,14 @@ namespace Chunk
 	void deleteBuffers();
 
         glm::vec3 getPosition() { return this->position; }
-        uint8_t getTotalState() { return this->state; }
+        void setState(chunk_state_t nstate, bool value);
-        bool getState(uint8_t n) { return (this->state & n) == n; }
+        bool getState(chunk_state_t n) { return (this->state & n) == n; }
-        void setState(uint8_t nstate, bool value);
+	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);
@@ -71,8 +80,7 @@ namespace Chunk
         glm::vec3 position{};
         IntervalMap<Block> blocks{};
         
-	std::atomic_uint8_t state{0};
+	std::atomic<chunk_state_t> state{0};
-
 	chunk_index_t index;
     };
 };

include/chunkmanager.hpp

@@ -30,11 +30,10 @@ namespace chunkmanager
     typedef oneapi::tbb::concurrent_priority_queue<ChunkPQEntry, compare_f> ChunkPriorityQueue;
 
     void init();
-    void blockpick(bool place);
+    //void blockpick(bool place);
     
     void stop();
     void destroy();
-    oneapi::tbb::concurrent_queue<Chunk::Chunk*>& getDeleteVector();
     std::array<std::array<chunk_intcoord_t, 3>, chunks_volume>& getChunksIndices();
     Block getBlockAtPos(int x, int y, int z);
     void update();

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);

src/chunkmanager.cpp

@@ -8,6 +8,8 @@
 #include <glm/glm.hpp>
 #include <glm/gtc/matrix_transform.hpp>
 
+#include <oneapi/tbb/parallel_for.h>
+
 #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);
-		}
+		chunk->setState(Chunk::CHUNK_STATE_IN_MESHING_QUEUE, false);
 	    }
 	}
 	chunks_to_mesh_queue.clear();
     }
 
-    oneapi::tbb::concurrent_queue<Chunk::Chunk*> chunks_todelete;
+    oneapi::tbb::concurrent_queue<chunk_index_t> chunks_todelete;
-    int nUnloaded{0};
     void update(){
 	while(should_run) {
-	    int chunkX=static_cast<int>(theCamera.getAtomicPosX() / CHUNK_SIZE);
+	    /* Setup variables for the whole loop */
-	    int chunkY=static_cast<int>(theCamera.getAtomicPosY() / CHUNK_SIZE);
+	    // Atomic is needed by parallel_for
-	    int chunkZ=static_cast<int>(theCamera.getAtomicPosZ() / CHUNK_SIZE);
+	    std::atomic_int nUnloaded{0}, nMarkUnload{0}, nExplored{0}, nMeshed{0}, nGenerated{0};
+	    std::atomic_int chunkX=static_cast<int>(theCamera.getAtomicPosX() / CHUNK_SIZE);
+	    std::atomic_int chunkY=static_cast<int>(theCamera.getAtomicPosY() / CHUNK_SIZE);
+	    std::atomic_int chunkZ=static_cast<int>(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;
+	    /* Update all the chunks */
-		if(!chunks.find(a, index)) chunks.emplace(a, std::make_pair(index, new Chunk::Chunk(glm::vec3(x,y,z))));
+	    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)) &&
+			    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))) &&
-			    (x - 1 < 0|| (chunks.find(a1, Chunk::calculateIndex(x-1, y, z)) &&
+				    (distx-1 < -RENDER_DISTANCE || 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)) &&
+				    (disty+1 >= RENDER_DISTANCE || 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)) &&
+				    (disty-1 < -RENDER_DISTANCE || 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)) &&
+				    (distz+1 >= RENDER_DISTANCE || 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)) &&
+				    (distz-1 < -RENDER_DISTANCE || 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));
+				{
+				    // 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++;
 			}
 
-		a.release();
+		    }else{
-	    }
+			// If not within distance
-
+			if(c->getState(Chunk::CHUNK_STATE_OUTOFVISION)){
-	    debug::window::set_parameter("update_chunks_total", (int) (chunks.size()));
+			    // If enough time has passed, set to be deleted
-	    debug::window::set_parameter("update_chunks_bucket", (int) (chunks.max_size()));
+			    if(c->isFree() && glfwGetTime() - c->unload_timer >= UNLOAD_TIMEOUT){
-
+				c->setState(Chunk::CHUNK_STATE_IN_DELETING_QUEUE, true);
-	    Chunk::Chunk* n;
+				chunks_todelete.push(c->getIndex());
-	    nUnloaded = 0;
+				unload++;
-	    while(chunks_todelete.try_pop(n)){
-		chunk_intcoord_t x = static_cast<chunk_intcoord_t>(n->getPosition().x);
-		chunk_intcoord_t y = static_cast<chunk_intcoord_t>(n->getPosition().y);
-		chunk_intcoord_t z = static_cast<chunk_intcoord_t>(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++;
 			    }
+			}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;
+		}
+	    });
+
 	}
     }
 
-    oneapi::tbb::concurrent_queue<Chunk::Chunk*>& getDeleteVector(){ return chunks_todelete; }
     std::array<std::array<chunk_intcoord_t, 3>, 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;

src/debugwindow.cpp

@@ -121,7 +121,9 @@ namespace debug{
 			std::any_cast<int>(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();

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<float>(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