From da4a141c851ba31c34785ac65910a3dffa02b761 Mon Sep 17 00:00:00 2001 From: Kbz-8 Date: Tue, 3 Jun 2025 21:57:47 +0200 Subject: [PATCH] fixing some data races --- Application/Chunk.cpp | 15 +++++++++++---- Application/Chunk.h | 2 ++ Application/ThreadPool.h | 2 +- Application/World.cpp | 39 ++++++++++++++++++++++++--------------- Application/World.h | 8 +++++--- Makefile | 6 ++++++ 6 files changed, 49 insertions(+), 23 deletions(-) diff --git a/Application/Chunk.cpp b/Application/Chunk.cpp index 4d0a563..0677498 100644 --- a/Application/Chunk.cpp +++ b/Application/Chunk.cpp @@ -52,12 +52,18 @@ void Chunk::GenerateChunk() if(p_actor) return; for(auto& y: m_data) + { + std::unique_lock guard(m_data_mutex); std::memset(y.data(), 0, y.size() * sizeof(std::uint32_t)); + } for(std::uint32_t x = 0; x < CHUNK_SIZE.x; x++) { for(std::uint32_t z = 0; z < CHUNK_SIZE.z; z++) + { + std::unique_lock guard(m_data_mutex); std::memcpy(m_data[POS_TO_INDEX(x, z)].data(), m_world.GetNoiseGenerator().GetBlocks(m_position + Scop::Vec2i(x, z)).data(), CHUNK_SIZE.y * sizeof(std::uint32_t)); + } } } @@ -370,23 +376,24 @@ std::uint32_t Chunk::GetBlock(Scop::Vec3i position) const noexcept return static_cast(BlockType::Dirt); if(position.x < 0) [[unlikely]] { - Scop::NonOwningPtr neighbour = m_world.GetChunk(Scop::Vec2i{ m_offset.x - 1, m_offset.y }); + Scop::NonOwningPtr neighbour = m_world.GetChunk(Scop::Vec2i{ m_offset.x - 1, m_offset.y }); return neighbour ? neighbour->GetBlock(Scop::Vec3i(CHUNK_SIZE.x - 1, position.y, position.z)) : static_cast(BlockType::Dirt); } if(position.x >= CHUNK_SIZE.x) [[unlikely]] { - Scop::NonOwningPtr neighbour = m_world.GetChunk(Scop::Vec2i{ m_offset.x + 1, m_offset.y }); + Scop::NonOwningPtr neighbour = m_world.GetChunk(Scop::Vec2i{ m_offset.x + 1, m_offset.y }); return neighbour ? neighbour->GetBlock(Scop::Vec3i(0, position.y, position.z)) : static_cast(BlockType::Dirt); } if(position.z < 0) [[unlikely]] { - Scop::NonOwningPtr neighbour = m_world.GetChunk(Scop::Vec2i{ m_offset.x, m_offset.y - 1 }); + Scop::NonOwningPtr neighbour = m_world.GetChunk(Scop::Vec2i{ m_offset.x, m_offset.y - 1 }); return neighbour ? neighbour->GetBlock(Scop::Vec3i(position.x, position.y, CHUNK_SIZE.x - 1)) : static_cast(BlockType::Dirt); } if(position.z >= CHUNK_SIZE.z) [[unlikely]] { - Scop::NonOwningPtr neighbour = m_world.GetChunk(Scop::Vec2i{ m_offset.x, m_offset.y + 1 }); + Scop::NonOwningPtr neighbour = m_world.GetChunk(Scop::Vec2i{ m_offset.x, m_offset.y + 1 }); return neighbour ? neighbour->GetBlock(Scop::Vec3i(position.x, position.y, 0)) : static_cast(BlockType::Dirt); } + std::shared_lock guard(m_data_mutex); return m_data[POS_TO_INDEX(position.x, position.z)][position.y]; } diff --git a/Application/Chunk.h b/Application/Chunk.h index b7533cc..5b971b9 100644 --- a/Application/Chunk.h +++ b/Application/Chunk.h @@ -2,6 +2,7 @@ #define CHUNK_H #include +#include #include #include @@ -31,6 +32,7 @@ class Chunk std::vector m_water_mesh_data; std::vector m_water_mesh_index_data; std::array, CHUNK_SIZE.x * CHUNK_SIZE.z> m_data; + mutable std::shared_mutex m_data_mutex; Scop::Vec2i m_offset; // In chunks Scop::Vec2i m_position; // In blocks class World& m_world; diff --git a/Application/ThreadPool.h b/Application/ThreadPool.h index 5c0c0f6..52e0456 100644 --- a/Application/ThreadPool.h +++ b/Application/ThreadPool.h @@ -16,7 +16,7 @@ class ThreadPool inline void WaitForAllTasks() const { using namespace std::chrono_literals; - for(; m_waiting_count != m_concurency;) + for(; m_waiting_count != m_concurency && !m_tasks.IsEmpty();) std::this_thread::sleep_for(10ms); } ~ThreadPool(); diff --git a/Application/World.cpp b/Application/World.cpp index 8ba137c..4c554c3 100644 --- a/Application/World.cpp +++ b/Application/World.cpp @@ -63,12 +63,12 @@ World::World(Scop::Scene& scene) : m_noisecollection(42), p_water_pipeline(std:: static_cast(global_block_position.y < 0 ? std::floorf(global_block_position.y / static_cast(CHUNK_SIZE.z)) : static_cast(global_block_position.y) / static_cast(CHUNK_SIZE.z)), }; - if(Scop::NonOwningPtr current_chunk = GetChunk(m_current_chunk_position); current_chunk) + if(Scop::NonOwningPtr current_chunk = GetChunk(m_current_chunk_position); current_chunk) { Scop::Vec3i block_position = Scop::Vec3i{ - global_block_position.x - (m_current_chunk_position.x * static_cast(CHUNK_SIZE.x)), + global_block_position.x - (m_current_chunk_position.load().x * static_cast(CHUNK_SIZE.x)), static_cast(camera_position.y), - global_block_position.y - (m_current_chunk_position.y * static_cast(CHUNK_SIZE.z)), + global_block_position.y - (m_current_chunk_position.load().y * static_cast(CHUNK_SIZE.z)), }; post_process_data.underwater = current_chunk->GetBlock(block_position) == static_cast(BlockType::Water); } @@ -87,7 +87,7 @@ World::World(Scop::Scene& scene) : m_noisecollection(42), p_water_pipeline(std:: if(generate) { - if(m_generation_status != GenerationState::Ready || m_current_chunk_position != m_previous_chunk_position) + if(m_generation_status != GenerationState::Ready || m_current_chunk_position.load() != m_previous_chunk_position) { if(m_generation_status == GenerationState::Ready) { @@ -128,11 +128,13 @@ World::~World() using namespace std::chrono_literals; std::this_thread::sleep_for(16ms); } + m_thread_pool.WaitForAllTasks(); } -[[nodiscard]] Scop::NonOwningPtr World::GetChunk(Scop::Vec2i position) +[[nodiscard]] Scop::NonOwningPtr World::GetChunk(Scop::Vec2i position) const { - auto it = m_chunks.find(position); + std::shared_lock guard(m_chunk_mutex); + const auto it = m_chunks.find(position); if(it == m_chunks.end()) return nullptr; return &it->second; @@ -143,15 +145,18 @@ void World::UnloadChunks() for(auto it = m_chunks.begin(); it != m_chunks.end();) { Scop::Vec2i pos = it->first; - std::uint32_t x_dist = std::abs(pos.x - m_current_chunk_position.x); - std::uint32_t z_dist = std::abs(pos.y - m_current_chunk_position.y); + std::uint32_t x_dist = std::abs(pos.x - m_current_chunk_position.load().x); + std::uint32_t z_dist = std::abs(pos.y - m_current_chunk_position.load().y); if(RENDER_DISTANCE < x_dist || RENDER_DISTANCE < z_dist) { if(it->second.GetActor()) m_scene.RemoveActor(*it->second.GetActor()); if(it->second.GetWaterActor()) m_scene.RemoveActor(*it->second.GetWaterActor()); - it = m_chunks.erase(it); + { + std::unique_lock guard(m_chunk_mutex); + it = m_chunks.erase(it); + } } else ++it; @@ -176,8 +181,8 @@ void World::GenerateWorld() std::queue> mesh_generation_queue; - Scop::Vec2i x_range{ m_current_chunk_position.x - RENDER_DISTANCE - 1, m_current_chunk_position.x + RENDER_DISTANCE + 1 }; - Scop::Vec2i z_range{ m_current_chunk_position.y - RENDER_DISTANCE - 1, m_current_chunk_position.y + RENDER_DISTANCE + 1 }; + Scop::Vec2i x_range{ m_current_chunk_position.load().x - RENDER_DISTANCE - 1, m_current_chunk_position.load().x + RENDER_DISTANCE + 1 }; + Scop::Vec2i z_range{ m_current_chunk_position.load().y - RENDER_DISTANCE - 1, m_current_chunk_position.load().y + RENDER_DISTANCE + 1 }; std::size_t range = (RENDER_DISTANCE + RENDER_DISTANCE + 2) * 2; m_loading_progress = 0.0f; @@ -187,13 +192,17 @@ void World::GenerateWorld() for(std::int32_t z = z_range.x; z <= z_range.y; z++) { QUIT_CHECK(); - auto [chunk_data, _] = m_chunks.try_emplace(Scop::Vec2i{ x, z }, *this, Scop::Vec2i{ x, z }); + + std::unordered_map::iterator chunk_data; + { + std::unique_lock guard(m_chunk_mutex); + chunk_data = m_chunks.try_emplace(Scop::Vec2i{ x, z }, *this, Scop::Vec2i{ x, z }).first; + } chunk_data->second.GenerateChunk(); + if(!chunk_data->second.GetActor() && !chunk_data->second.GetWaterActor() && x > x_range.x && x < x_range.y && z > z_range.x && z < z_range.y) + mesh_generation_queue.push(std::ref(chunk_data->second)); m_loading_progress = std::min(m_loading_progress + (1.0f / LIMIT) * range, LIMIT); - - if(!chunk_data->second.GetActor() || !chunk_data->second.GetWaterActor() && x > x_range.x && x < x_range.y && z > z_range.x && z < z_range.y) - mesh_generation_queue.push(std::ref(chunk_data->second)); } } diff --git a/Application/World.h b/Application/World.h index c458bfd..4b519ae 100644 --- a/Application/World.h +++ b/Application/World.h @@ -3,6 +3,7 @@ #include #include +#include #include @@ -13,7 +14,7 @@ #include constexpr std::uint8_t RENDER_DISTANCE = 15; -constexpr std::uint8_t CHUNKS_UPLOAD_PER_FRAME = 12; +constexpr std::uint8_t CHUNKS_UPLOAD_PER_FRAME = 3; enum class GenerationState: std::uint8_t { @@ -38,7 +39,7 @@ class World [[nodiscard]] inline std::shared_ptr GetBlockMaterial() const { return p_block_material; } [[nodiscard]] inline std::shared_ptr GetWaterPipeline() const { return p_water_pipeline; } - [[nodiscard]] Scop::NonOwningPtr GetChunk(Scop::Vec2i position); + [[nodiscard]] Scop::NonOwningPtr GetChunk(Scop::Vec2i position) const; [[nodiscard]] NoiseCollection& GetNoiseGenerator() noexcept { return m_noisecollection; } ~World(); @@ -59,9 +60,10 @@ class World std::shared_ptr p_water_pipeline; std::shared_ptr p_water_vertex_shader; std::shared_ptr p_water_fragment_shader; + mutable std::shared_mutex m_chunk_mutex; Scop::Scene& m_scene; Scop::Vec2i m_previous_chunk_position; - Scop::Vec2i m_current_chunk_position; + std::atomic m_current_chunk_position; std::atomic m_generation_status = GenerationState::Ready; Scop::NonOwningPtr p_fps_text; Scop::NonOwningPtr p_loading_text; diff --git a/Makefile b/Makefile index e249974..25f9d8f 100644 --- a/Makefile +++ b/Makefile @@ -19,6 +19,7 @@ CXXFLAGS = -std=c++20 -I ScopEngine/Runtime/Includes -I Application -I ScopEngin LDFLAGS = -lSDL2 ScopEngine/Bin/engine.a DEBUG ?= false +TSAN ?= false MODE = "release" NZSLC = ./ScopEngine/Assets/Vendors/nzslc.x86_64 @@ -43,6 +44,11 @@ else COLOR := $(_GREEN) endif +ifeq ($(TSAN), true) + CXXFLAGS += -fsanitize=thread + LDFLAGS += -fsanitize=thread +endif + JOBS = $(patsubst -j%,%,$(filter -j%,$(MAKEFLAGS))) RM = rm -rf