Merge pull request 'Refactor Main/Update thread communication' (#14) from multithread-refactor into main

Reviewed-on: #14

include/chunkmanager.hpp

@@ -1,14 +1,14 @@
 #ifndef CHUNKMANAGER_H
 #define CHUNKMANAGER_H
 
-#include "chunk.hpp"
-
 #include <oneapi/tbb/concurrent_hash_map.h>
 #include <oneapi/tbb/concurrent_queue.h>
 #include <oneapi/tbb/concurrent_priority_queue.h>
 #include <thread>
 
+#include "chunk.hpp"
 #include "globals.hpp"
+#include "worldupdatemessage.h"
 
 // Seconds to be passed outside of render distance for a chunk to be destroyed
 #define UNLOAD_TIMEOUT 10
@@ -30,13 +30,12 @@ namespace chunkmanager
     typedef oneapi::tbb::concurrent_priority_queue<ChunkPQEntry, compare_f> ChunkPriorityQueue;
 
     void init();
-    //void blockpick(bool place);
+    void update();
-    
     void stop();
     void destroy();
+    WorldUpdateMsgQueue& getWorldUpdateQueue();
     std::array<std::array<chunk_intcoord_t, 3>, chunks_volume>& getChunksIndices();
     Block getBlockAtPos(int x, int y, int z);
-    void update();
 }
 
 #endif

include/controls.hpp

@@ -4,7 +4,7 @@
 #include <glad/glad.h>
 #include <GLFW/glfw3.h>
 
-#define BLOCKPICK_TIMEOUT 0.15f
+#define BLOCKPICK_TIMEOUT 0.1f
 
 namespace controls{
     void init();

include/worldupdatemessage.h

@@ -0,0 +1,24 @@
+#ifndef WORLD_UPDATE_MSG_H
+#define WORLD_UPDATE_MSG_H
+
+#include <glm/glm.hpp>
+#include <oneapi/tbb/concurrent_queue.h>
+
+#include "block.hpp"
+
+enum class WorldUpdateMsgType{
+    BLOCKPICK_PLACE,
+    BLOCKPICK_BREAK
+};
+
+typedef struct WorldUpdateMsg{
+    WorldUpdateMsgType msg_type;
+    glm::vec3 cameraPos;
+    glm::vec3 cameraFront;
+    float time;
+    Block block;
+} WorldUpdateMsg;
+
+typedef oneapi::tbb::concurrent_queue<WorldUpdateMsg> WorldUpdateMsgQueue;
+
+#endif

src/chunkmanager.cpp

@@ -20,14 +20,20 @@
 
 namespace chunkmanager
 {
+    void blockpick(WorldUpdateMsg& msg); // There's no need of passing by value again (check
+					 // controls.cpp)
     void generate();
     void mesh();
 
+    /* Chunk holding data structures */
     // Concurrent hash table of chunks
     ChunkTable chunks;
     // Chunk indices. Centered at (0,0,0), going in concentric sphere outwards
     std::array<std::array<chunk_intcoord_t, 3>, chunks_volume> chunks_indices;
 
+    /* World Update messaging data structure */
+    WorldUpdateMsgQueue WorldUpdateQueue;
+
     /* Multithreading */
     std::atomic_bool should_run;
     std::thread gen_thread, mesh_thread, update_thread;
@@ -37,7 +43,7 @@ namespace chunkmanager
     // Queue of chunks to be meshed
     ChunkPriorityQueue chunks_to_mesh_queue;
 
-    int block_to_place{2};
+    WorldUpdateMsgQueue& getWorldUpdateQueue(){ return WorldUpdateQueue; }
     
     // Init chunkmanager. Chunk indices and start threads
     void init(){
@@ -57,8 +63,6 @@ namespace chunkmanager
 	update_thread = std::thread(update);
 	gen_thread = std::thread(generate);
 	mesh_thread = std::thread(mesh);
-
-	debug::window::set_parameter("block_type_return", &block_to_place);
     }
 
     // Method for world generation thread(s)
@@ -97,15 +101,27 @@ namespace chunkmanager
 	    std::atomic_int chunkY=static_cast<int>(theCamera.getAtomicPosY() / CHUNK_SIZE);
 	    std::atomic_int chunkZ=static_cast<int>(theCamera.getAtomicPosZ() / CHUNK_SIZE);
 
+	    /* Process update messages before anything happens */
+	    WorldUpdateMsg msg;
+	    while(WorldUpdateQueue.try_pop(msg)){
+		switch(msg.msg_type){
+		    case WorldUpdateMsgType::BLOCKPICK_BREAK:
+		    case WorldUpdateMsgType::BLOCKPICK_PLACE:
+			blockpick(msg);
+			break;
+		}
+	    }
+
+
 	    /* 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;
+	    chunk_index_t i;
 	    ChunkTable::accessor a;
 	    while(chunks_todelete.try_pop(i)){
-		const int index = i;
+		const chunk_index_t index = i;
 		if(chunks.find(a, index)){
 		    Chunk::Chunk* c = a->second;
 		    // Use the accessor to erase the element
@@ -255,13 +271,12 @@ namespace chunkmanager
 	chunks.clear();
     }
 
-    /*void blockpick(bool place){
+    void blockpick(WorldUpdateMsg& msg){
 	// cast a ray from the camera in the direction pointed by the camera itself
-	glm::vec3 pos = glm::vec3(theCamera.getAtomicPosX(), theCamera.getAtomicPosY(),
+	glm::vec3 pos = msg.cameraPos;
-		theCamera.getAtomicPosZ());
 	for(float t = 0.0; t <= 10.0; t += 0.5){
 	    // traverse the ray a block at the time
-	    pos = theCamera.getPos() + t * theCamera.getFront();
+	    pos = msg.cameraPos + t*msg.cameraFront;
 
 	    // get which chunk and block the ray is at
 	    int px = ((int)(pos.x))/CHUNK_SIZE;
@@ -286,7 +301,7 @@ namespace chunkmanager
 	    if(b != Block::AIR){
 
 		// if placing a new block
-		if(place){
+		if(msg.msg_type == WorldUpdateMsgType::BLOCKPICK_PLACE){
 		    // Go half a block backwards on the ray, to check the block where the ray was
 		    // coming from
 		    // Doing this and not using normal adds the unexpected (and unwanted) ability to
@@ -307,14 +322,14 @@ namespace chunkmanager
 		    if(!chunks.find(a1, Chunk::calculateIndex(px1, py1, pz1))) return;
 		    Chunk::Chunk* c1 = a1->second;
 		    // place the new block (only stone for now)
-		    c1->setBlock((Block)block_to_place, bx1, by1, bz1);
+		    c1->setBlock(msg.block, bx1, by1, bz1);
 
 		    // mark the mesh of the chunk the be updated
 		    chunks_to_mesh_queue.push(std::make_pair(c1, MESHING_PRIORITY_PLAYER_EDIT));
 		    chunks_to_mesh_queue.push(std::make_pair(c, MESHING_PRIORITY_PLAYER_EDIT));
 
-		    debug::window::set_parameter("block_last_action", place);
+		    debug::window::set_parameter("block_last_action", true);
-		    debug::window::set_parameter("block_last_action_block_type", (int)(Block::STONE));
+		    debug::window::set_parameter("block_last_action_block_type", (int)(msg.block));
 		    debug::window::set_parameter("block_last_action_x", px1*CHUNK_SIZE + bx1);
 		    debug::window::set_parameter("block_last_action_y", px1*CHUNK_SIZE + by1);
 		    debug::window::set_parameter("block_last_action_z", px1*CHUNK_SIZE + bz1);
@@ -339,7 +354,7 @@ namespace chunkmanager
 		    if(bz == CHUNK_SIZE - 1 && pz +1 < 1024 && chunks.find(c2, Chunk::calculateIndex(px, py, pz +1)))
 		      chunkmesher::mesh(c2->second);
 
-		    debug::window::set_parameter("block_last_action", place);
+		    debug::window::set_parameter("block_last_action", false);
 		    debug::window::set_parameter("block_last_action_block_type", (int) (Block::AIR));
 		    debug::window::set_parameter("block_last_action_x", px*CHUNK_SIZE + bx);
 		    debug::window::set_parameter("block_last_action_y", py*CHUNK_SIZE + by);
@@ -349,7 +364,7 @@ namespace chunkmanager
 		break;
 	    }
 	}
-    }*/
+    }
 
     Block getBlockAtPos(int x, int y, int z){
 	if(x < 0 || y < 0 || z < 0) return Block::NULLBLK;

src/controls.cpp

@@ -1,34 +1,61 @@
 #include "controls.hpp"
+
+#include "camera.hpp"
+#include "chunkmanager.hpp"
+#include "debugwindow.hpp"
+#include "globals.hpp"
 #include "renderer.hpp"
 
 namespace controls{
+    /* Block picking */
+    int block_to_place{2};
     float lastBlockPick=0.0;
     bool blockpick = false;
+
+    /* Cursor */
     bool cursor = false;
 
     void init(){
+	debug::window::set_parameter("block_type_return", &block_to_place);
     }
     
     void update(GLFWwindow* window){
 	float current_time = glfwGetTime();
 
-	// Reset blockpicking timeout has passed
+	/* BlockPicking */
+	// Reset blockpicking if enough time has passed
 	if(current_time - lastBlockPick > BLOCKPICK_TIMEOUT) blockpick = false;
 	// Reset blockpicking if both mouse buttons are released
 	if(glfwGetMouseButton(window, GLFW_MOUSE_BUTTON_1) == GLFW_RELEASE && glfwGetMouseButton(window, GLFW_MOUSE_BUTTON_2) == GLFW_RELEASE) blockpick = false;
+	// Process block picking if a mouse button is pressed
+	if((glfwGetMouseButton(window, GLFW_MOUSE_BUTTON_1) == GLFW_PRESS ||
+		glfwGetMouseButton(window, GLFW_MOUSE_BUTTON_2 == GLFW_PRESS)) && !blockpick){
 
-	if(glfwGetMouseButton(window, GLFW_MOUSE_BUTTON_2) == GLFW_PRESS && !blockpick){
+	    // Start timeout for next block pick action
-	    //chunkmanager::blockpick(false);
+	    blockpick =  true;
-	    blockpick=true;
+	    lastBlockPick = current_time;
-	    lastBlockPick=glfwGetTime();
+
-	}
+	    // Construct the message to send to chunkmanager
-
+
-	if(glfwGetMouseButton(window, GLFW_MOUSE_BUTTON_1) == GLFW_PRESS && !blockpick){
+	    // WorldUpdateMsg is allocated on the stack
-	    //chunkmanager::blockpick(true);
+	    // unlike ChunkMeshData, the fields of WorldUpdateMsg are few and light, so there's no
-	    blockpick=true;
+	    // problem in passing them by value each time.
-	    lastBlockPick=glfwGetTime();
+	    // It also has the advantage of having less memory to manage, since I'm not allocating
+	    // anything on the heap
+
+	    WorldUpdateMsg msg{};
+	    msg.cameraPos = theCamera.getPos();
+	    msg.cameraFront = theCamera.getFront();
+	    msg.time = current_time;
+	    msg.msg_type = glfwGetMouseButton(window, GLFW_MOUSE_BUTTON_1) == GLFW_PRESS ?
+	    WorldUpdateMsgType::BLOCKPICK_PLACE : WorldUpdateMsgType::BLOCKPICK_BREAK;
+	    msg.block = (Block)(block_to_place);
+
+	    // Send to chunk manager
+	    chunkmanager::getWorldUpdateQueue().push(msg);
 	}
 
+	/* SCREENSHOTS */
 	if(glfwGetKey(window, GLFW_KEY_F2) == GLFW_PRESS) renderer::saveScreenshot();
 	if(glfwGetKey(window, GLFW_KEY_F3) == GLFW_PRESS) renderer::saveScreenshot(true);
 	if(glfwGetKey(window, GLFW_KEY_M) == GLFW_PRESS) {

src/debugwindow.cpp

@@ -79,6 +79,11 @@ namespace debug{
 		    ImGui::Text("Pointing in direction: %f, %f, %f", 
 			    std::any_cast<float>(parameters.at("lx")),std::any_cast<float>(parameters.at("ly")),std::any_cast<float>(parameters.at("lz"))  );
 
+		    ImGui::SliderInt("Crosshair type",
+			    std::any_cast<int*>(parameters.at("crosshair_type_return")), 0, 1);
+		    ImGui::SliderInt("Block to place",
+			    std::any_cast<int*>(parameters.at("block_type_return")), 2, 6);
+
 		    if(parameters.find("block_last_action") != parameters.end()){
 			ImGui::Text("Last Block action: %s",
 			    std::any_cast<bool>(parameters.at("block_last_action")) ? "place" : "destroy");
@@ -87,11 +92,6 @@ namespace debug{
 			ImGui::Text("Last Block action position: X: %d, Y: %d, Z: %d",
 			    std::any_cast<int>(parameters.at("block_last_action_x")),std::any_cast<int>(parameters.at("block_last_action_y")),std::any_cast<int>(parameters.at("block_last_action_z"))  );
 		    }
-
-		    ImGui::SliderInt("Crosshair type",
-			    std::any_cast<int*>(parameters.at("crosshair_type_return")), 0, 1);
-		    ImGui::SliderInt("Block to place",
-			    std::any_cast<int*>(parameters.at("block_type_return")), 2, 6);
 		}
 
 		if(ImGui::CollapsingHeader("Mesh")){

src/main.cpp

@@ -1,6 +1,8 @@
 #include <glad/glad.h>
 #include <GLFW/glfw3.h>
 
+#include "main.hpp"
+
 #include <iostream>
 #include <thread>
 
@@ -98,6 +100,8 @@ int main()
 	if (glfwGetKey(window, GLFW_KEY_ESCAPE) == GLFW_PRESS)
 	    glfwSetWindowShouldClose(window, true);
 	// the rest of input processing is handled by controls.cpp
+
+        // Input processing
 	controls::update(window);
 
         // Camera