From 93bc0e706650f0d0256ec48e9c8efb1a2c9ca9f6 Mon Sep 17 00:00:00 2001 From: EmaMaker Date: Wed, 4 Oct 2023 13:55:32 +0200 Subject: [PATCH 1/3] renderer: do not send empty meshes to the gpu only at creation I feel never sending empty meshes to the GPU is the cause of the bug causing floating quads near chunk borders when a block is placed and then destroyed. When destroying a block in a chunk, if nearby empty chunk meshes are not updated, the old mesh is kept, which includes a quad at the border --- src/renderer.cpp | 52 ++++++++++++++++++++++++++---------------------- 1 file changed, 28 insertions(+), 24 deletions(-) diff --git a/src/renderer.cpp b/src/renderer.cpp index 5daa398..c5420f9 100644 --- a/src/renderer.cpp +++ b/src/renderer.cpp @@ -151,6 +151,11 @@ namespace renderer{ render_info = a->second; render_info->position = m->position; render_info->num_vertices = m->num_vertices; + + // Always updated the mesh, even if it's empty + // This should solve the problem of having floating quads when destroying a block + // near chunk borders + send_chunk_to_gpu(m, render_info); }else{ render_info = new RenderInfo(); render_info->index = m->index; @@ -158,9 +163,11 @@ namespace renderer{ render_info->num_vertices = m->num_vertices; ChunksToRender.emplace(a, std::make_pair(render_info->index, render_info)); + + // Only send the mesh to the GPU if it's not empty + if(render_info->num_vertices > 0) send_chunk_to_gpu(m, render_info); } - send_chunk_to_gpu(m, render_info); chunkmesher::getMeshDataQueue().push(m); } @@ -254,35 +261,32 @@ namespace renderer{ void send_chunk_to_gpu(ChunkMeshData* mesh_data, RenderInfo* render_info) { - if (render_info->num_vertices > 0) - { - if(!render_info->buffers_allocated) render_info->allocateBuffers(); + if(!render_info->buffers_allocated) render_info->allocateBuffers(); - // bind the Vertex Array Object first, then bind and set vertex buffer(s), and then configure vertex attributes(s). - glBindVertexArray(render_info->VAO); + // bind the Vertex Array Object first, then bind and set vertex buffer(s), and then configure vertex attributes(s). + glBindVertexArray(render_info->VAO); - // TODO: change GL_STATIC_DRAW to the one that means "few redraws and further in between" + // TODO: change GL_STATIC_DRAW to the one that means "few redraws and further in between" - // position attribute - glBindBuffer(GL_ARRAY_BUFFER, render_info->VBO); - glBufferData(GL_ARRAY_BUFFER, mesh_data->vertices.size() * sizeof(GLfloat), &(mesh_data->vertices[0]), GL_STATIC_DRAW); - glVertexAttribPointer(0, 3, GL_FLOAT, GL_FALSE, 3 * sizeof(float), (void *)0); - glEnableVertexAttribArray(0); + // position attribute + glBindBuffer(GL_ARRAY_BUFFER, render_info->VBO); + glBufferData(GL_ARRAY_BUFFER, mesh_data->vertices.size() * sizeof(GLfloat), &(mesh_data->vertices[0]), GL_STATIC_DRAW); + glVertexAttribPointer(0, 3, GL_FLOAT, GL_FALSE, 3 * sizeof(float), (void *)0); + glEnableVertexAttribArray(0); - // normal attribute - glBindBuffer(GL_ARRAY_BUFFER, render_info->extentsBuffer); - glBufferData(GL_ARRAY_BUFFER, mesh_data->extents.size() * sizeof(GLfloat), &(mesh_data->extents[0]), GL_STATIC_DRAW); - glVertexAttribPointer(1, 3, GL_FLOAT, GL_FALSE, 3 * sizeof(float), (void *)(0)); - glEnableVertexAttribArray(1); + // normal attribute + glBindBuffer(GL_ARRAY_BUFFER, render_info->extentsBuffer); + glBufferData(GL_ARRAY_BUFFER, mesh_data->extents.size() * sizeof(GLfloat), &(mesh_data->extents[0]), GL_STATIC_DRAW); + glVertexAttribPointer(1, 3, GL_FLOAT, GL_FALSE, 3 * sizeof(float), (void *)(0)); + glEnableVertexAttribArray(1); - // texcoords attribute - glBindBuffer(GL_ARRAY_BUFFER, render_info->texinfoBuffer); - glBufferData(GL_ARRAY_BUFFER, mesh_data->texinfo.size() * sizeof(GLfloat), &(mesh_data->texinfo[0]), GL_STATIC_DRAW); - glEnableVertexAttribArray(2); - glVertexAttribPointer(2, 2, GL_FLOAT, GL_FALSE, 2 * sizeof(float), (void *)0); + // texcoords attribute + glBindBuffer(GL_ARRAY_BUFFER, render_info->texinfoBuffer); + glBufferData(GL_ARRAY_BUFFER, mesh_data->texinfo.size() * sizeof(GLfloat), &(mesh_data->texinfo[0]), GL_STATIC_DRAW); + glEnableVertexAttribArray(2); + glVertexAttribPointer(2, 2, GL_FLOAT, GL_FALSE, 2 * sizeof(float), (void *)0); - glBindVertexArray(0); - } + glBindVertexArray(0); } From afdd622ec28670ebea7332c695213d41a6583149 Mon Sep 17 00:00:00 2001 From: EmaMaker Date: Tue, 3 Oct 2023 18:43:29 +0200 Subject: [PATCH 2/3] refactor message system between main and update threads restores blockpicking with new multithread system --- include/chunkmanager.hpp | 9 ++++---- include/controls.hpp | 2 +- include/worldupdatemessage.h | 21 ++++++++++++++++++ src/chunkmanager.cpp | 43 +++++++++++++++++++++++++++--------- src/controls.cpp | 42 ++++++++++++++++++++++++++--------- src/main.cpp | 4 ++++ 6 files changed, 93 insertions(+), 28 deletions(-) create mode 100644 include/worldupdatemessage.h diff --git a/include/chunkmanager.hpp b/include/chunkmanager.hpp index 9aa2d19..d5fd627 100644 --- a/include/chunkmanager.hpp +++ b/include/chunkmanager.hpp @@ -1,14 +1,14 @@ #ifndef CHUNKMANAGER_H #define CHUNKMANAGER_H -#include "chunk.hpp" - #include #include #include #include +#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 ChunkPriorityQueue; void init(); - //void blockpick(bool place); - + void update(); void stop(); void destroy(); + WorldUpdateMsgQueue& getWorldUpdateQueue(); std::array, chunks_volume>& getChunksIndices(); Block getBlockAtPos(int x, int y, int z); - void update(); } #endif diff --git a/include/controls.hpp b/include/controls.hpp index 490dbbd..6703f25 100644 --- a/include/controls.hpp +++ b/include/controls.hpp @@ -4,7 +4,7 @@ #include #include -#define BLOCKPICK_TIMEOUT 0.15f +#define BLOCKPICK_TIMEOUT 0.1f namespace controls{ void init(); diff --git a/include/worldupdatemessage.h b/include/worldupdatemessage.h new file mode 100644 index 0000000..200f8d9 --- /dev/null +++ b/include/worldupdatemessage.h @@ -0,0 +1,21 @@ +#ifndef WORLD_UPDATE_MSG_H +#define WORLD_UPDATE_MSG_H + +#include +#include + +enum class WorldUpdateMsgType{ + BLOCKPICK_PLACE, + BLOCKPICK_BREAK +}; + +typedef struct WorldUpdateMsg{ + WorldUpdateMsgType msg_type; + glm::vec3 cameraPos; + glm::vec3 cameraFront; + float time; +} WorldUpdateMsg; + +typedef oneapi::tbb::concurrent_queue WorldUpdateMsgQueue; + +#endif diff --git a/src/chunkmanager.cpp b/src/chunkmanager.cpp index 1addbce..5983d5b 100644 --- a/src/chunkmanager.cpp +++ b/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, 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,8 +43,11 @@ namespace chunkmanager // Queue of chunks to be meshed ChunkPriorityQueue chunks_to_mesh_queue; + /* Block picking */ int block_to_place{2}; + WorldUpdateMsgQueue& getWorldUpdateQueue(){ return WorldUpdateQueue; } + // Init chunkmanager. Chunk indices and start threads void init(){ int index{0}; @@ -97,15 +106,27 @@ namespace chunkmanager std::atomic_int chunkY=static_cast(theCamera.getAtomicPosY() / CHUNK_SIZE); std::atomic_int chunkZ=static_cast(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 +276,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(), - theCamera.getAtomicPosZ()); + glm::vec3 pos = msg.cameraPos; 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 +306,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 @@ -313,8 +333,9 @@ namespace chunkmanager 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_block_type", (int)(Block::STONE)); + debug::window::set_parameter("block_last_action", (int)msg.msg_type); + debug::window::set_parameter("block_last_action_block_type", + (int)(block_to_place)); 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 +360,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", (int)msg.msg_type); 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 +370,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/controls.cpp b/src/controls.cpp index 235ed96..2b2b82d 100644 --- a/src/controls.cpp +++ b/src/controls.cpp @@ -1,4 +1,8 @@ #include "controls.hpp" + +#include "camera.hpp" +#include "chunkmanager.hpp" +#include "globals.hpp" #include "renderer.hpp" namespace controls{ @@ -12,23 +16,39 @@ namespace controls{ 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){ - //chunkmanager::blockpick(false); - blockpick=true; - lastBlockPick=glfwGetTime(); - } - - if(glfwGetMouseButton(window, GLFW_MOUSE_BUTTON_1) == GLFW_PRESS && !blockpick){ - //chunkmanager::blockpick(true); - blockpick=true; - lastBlockPick=glfwGetTime(); + // Start timeout for next block pick action + blockpick = true; + lastBlockPick = current_time; + + // Construct the message to send to chunkmanager + + // WorldUpdateMsg is allocated on the stack + // unlike ChunkMeshData, the fields of WorldUpdateMsg are few and light, so there's no + // problem in passing them by value each time. + // 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; + + // 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) { diff --git a/src/main.cpp b/src/main.cpp index 7bc51f9..f3f16ff 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1,6 +1,8 @@ #include #include +#include "main.hpp" + #include #include @@ -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 From 255460892d89876bf257f12b5746974117bfef0d Mon Sep 17 00:00:00 2001 From: EmaMaker Date: Wed, 4 Oct 2023 14:41:44 +0200 Subject: [PATCH 3/3] blockpick control belongs to control, not chunkmgr --- include/worldupdatemessage.h | 3 +++ src/chunkmanager.cpp | 14 ++++---------- src/controls.cpp | 7 +++++++ src/debugwindow.cpp | 10 +++++----- 4 files changed, 19 insertions(+), 15 deletions(-) diff --git a/include/worldupdatemessage.h b/include/worldupdatemessage.h index 200f8d9..3c9275e 100644 --- a/include/worldupdatemessage.h +++ b/include/worldupdatemessage.h @@ -4,6 +4,8 @@ #include #include +#include "block.hpp" + enum class WorldUpdateMsgType{ BLOCKPICK_PLACE, BLOCKPICK_BREAK @@ -14,6 +16,7 @@ typedef struct WorldUpdateMsg{ glm::vec3 cameraPos; glm::vec3 cameraFront; float time; + Block block; } WorldUpdateMsg; typedef oneapi::tbb::concurrent_queue WorldUpdateMsgQueue; diff --git a/src/chunkmanager.cpp b/src/chunkmanager.cpp index 5983d5b..9204e38 100644 --- a/src/chunkmanager.cpp +++ b/src/chunkmanager.cpp @@ -43,9 +43,6 @@ namespace chunkmanager // Queue of chunks to be meshed ChunkPriorityQueue chunks_to_mesh_queue; - /* Block picking */ - int block_to_place{2}; - WorldUpdateMsgQueue& getWorldUpdateQueue(){ return WorldUpdateQueue; } // Init chunkmanager. Chunk indices and start threads @@ -66,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) @@ -327,15 +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", (int)msg.msg_type); - debug::window::set_parameter("block_last_action_block_type", - (int)(block_to_place)); + debug::window::set_parameter("block_last_action", true); + 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); @@ -360,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", (int)msg.msg_type); + 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); diff --git a/src/controls.cpp b/src/controls.cpp index 2b2b82d..27ba313 100644 --- a/src/controls.cpp +++ b/src/controls.cpp @@ -2,15 +2,21 @@ #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){ @@ -43,6 +49,7 @@ namespace controls{ 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); diff --git a/src/debugwindow.cpp b/src/debugwindow.cpp index 256dd2e..1db0880 100644 --- a/src/debugwindow.cpp +++ b/src/debugwindow.cpp @@ -79,6 +79,11 @@ namespace debug{ ImGui::Text("Pointing in direction: %f, %f, %f", std::any_cast(parameters.at("lx")),std::any_cast(parameters.at("ly")),std::any_cast(parameters.at("lz")) ); + ImGui::SliderInt("Crosshair type", + std::any_cast(parameters.at("crosshair_type_return")), 0, 1); + ImGui::SliderInt("Block to place", + std::any_cast(parameters.at("block_type_return")), 2, 6); + if(parameters.find("block_last_action") != parameters.end()){ ImGui::Text("Last Block action: %s", std::any_cast(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(parameters.at("block_last_action_x")),std::any_cast(parameters.at("block_last_action_y")),std::any_cast(parameters.at("block_last_action_z")) ); } - - ImGui::SliderInt("Crosshair type", - std::any_cast(parameters.at("crosshair_type_return")), 0, 1); - ImGui::SliderInt("Block to place", - std::any_cast(parameters.at("block_type_return")), 2, 6); } if(ImGui::CollapsingHeader("Mesh")){