From 2a8a2f1b39886ebd1b24e5555b1336618407b494 Mon Sep 17 00:00:00 2001 From: fleroviux Date: Sat, 4 May 2024 02:21:04 +0200 Subject: [PATCH] Zephyr: Renderer: fix a race condition when deleting a cached geometry --- app/next/src/main_window.cpp | 15 +++++++++++++ app/next/src/main_window.hpp | 2 ++ .../zephyr/renderer/engine/geometry_cache.hpp | 8 +++++-- zephyr/renderer/src/engine/geometry_cache.cpp | 22 +++++++++++++++---- zephyr/renderer/src/render_engine.cpp | 6 ++++- 5 files changed, 46 insertions(+), 7 deletions(-) diff --git a/app/next/src/main_window.cpp b/app/next/src/main_window.cpp index c0d5c50..7e0df49 100644 --- a/app/next/src/main_window.cpp +++ b/app/next/src/main_window.cpp @@ -44,6 +44,20 @@ namespace zephyr { if(event.type == SDL_QUIT) { return; } + + if(event.type == SDL_KEYUP) { + SDL_KeyboardEvent* key_event = (SDL_KeyboardEvent*)&event; + switch(key_event->keysym.sym) { + case SDLK_z: { + m_scene_root->Remove(m_behemoth_scene); + break; + } + case SDLK_x: { + m_behemoth_scene->SetVisible(!m_behemoth_scene->IsVisible()); + break; + } + } + } } RenderFrame(); @@ -119,6 +133,7 @@ namespace zephyr { gltf_scene_2->GetTransform().GetPosition() = Vector3{-1.0f, 0.0f, -5.0f}; gltf_scene_2->GetTransform().GetRotation().SetFromEuler(-M_PI * 0.5, M_PI, 0.0f); gltf_scene_2->GetTransform().GetScale() = {0.5f, 0.5f, 0.5f}; + m_behemoth_scene = gltf_scene_2.get(); m_scene_root->Add(std::move(gltf_scene_2)); } diff --git a/app/next/src/main_window.hpp b/app/next/src/main_window.hpp index 7a366ed..99cd37f 100644 --- a/app/next/src/main_window.hpp +++ b/app/next/src/main_window.hpp @@ -44,6 +44,8 @@ namespace zephyr { SDL_Window* m_window{}; std::shared_ptr m_vk_instance{}; VkSurfaceKHR m_vk_surface{VK_NULL_HANDLE}; + + SceneNode* m_behemoth_scene{}; }; } // namespace zephyr \ No newline at end of file diff --git a/zephyr/renderer/include/zephyr/renderer/engine/geometry_cache.hpp b/zephyr/renderer/include/zephyr/renderer/engine/geometry_cache.hpp index eb1f880..9983a29 100644 --- a/zephyr/renderer/include/zephyr/renderer/engine/geometry_cache.hpp +++ b/zephyr/renderer/include/zephyr/renderer/engine/geometry_cache.hpp @@ -13,7 +13,11 @@ namespace zephyr { public: explicit GeometryCache(std::shared_ptr render_backend) : m_render_backend{std::move(render_backend)} {} - void ScheduleUploadIfNeeded(const Geometry* geometry); + // Game Thread API: + void CommitPendingDeleteTaskList(); + void UpdateGeometry(const Geometry* geometry); + + // Render Thread API: void ProcessPendingUpdates(); RenderGeometry* GetCachedRenderGeometry(const Geometry* geometry); @@ -43,7 +47,7 @@ namespace zephyr { std::unordered_map m_geometry_state_table{}; std::unordered_map m_render_geometry_table{}; std::vector m_upload_tasks{}; - std::vector m_delete_tasks{}; + std::vector m_delete_tasks[2]{}; }; } // namespace zephyr diff --git a/zephyr/renderer/src/engine/geometry_cache.cpp b/zephyr/renderer/src/engine/geometry_cache.cpp index bd7b353..53938dd 100644 --- a/zephyr/renderer/src/engine/geometry_cache.cpp +++ b/zephyr/renderer/src/engine/geometry_cache.cpp @@ -1,10 +1,16 @@ #include +#include +#include #include namespace zephyr { - void GeometryCache::ScheduleUploadIfNeeded(const Geometry* geometry) { + void GeometryCache::CommitPendingDeleteTaskList() { + std::swap(m_delete_tasks[0], m_delete_tasks[1]); + } + + void GeometryCache::UpdateGeometry(const Geometry* geometry) { GeometryState& state = m_geometry_state_table[geometry]; if(!state.uploaded || state.current_version != geometry->CurrentVersion()) { @@ -26,8 +32,13 @@ namespace zephyr { if(!state.uploaded) { geometry->RegisterDeleteCallback([this](const Resource* geometry) { + /** + * This callback is called from the game thread and may be called outside the frame submission phase. + * To avoid deleting geometries too early, we have to push the delete task to an intermediate list, + * which is then committed for execution for the next frame submission. + */ + m_delete_tasks[1].push_back({.geometry = (const Geometry*)geometry}); m_geometry_state_table.erase((const Geometry*)geometry); - m_delete_tasks.push_back({.geometry = (const Geometry*)geometry}); }); } @@ -42,11 +53,14 @@ namespace zephyr { } RenderGeometry* GeometryCache::GetCachedRenderGeometry(const Geometry* geometry) { + if(!m_render_geometry_table.contains(geometry)) { + ZEPHYR_PANIC("Bad attempt to retrieve cached render geometry of a geometry which isn't cached.") + } return m_render_geometry_table[geometry]; } void GeometryCache::ProcessPendingDeletes() { - for(const auto& delete_task : m_delete_tasks) { + for(const auto& delete_task : m_delete_tasks[0]) { RenderGeometry* render_geometry = m_render_geometry_table[delete_task.geometry]; if(render_geometry) { m_render_backend->DestroyRenderGeometry(render_geometry); @@ -54,7 +68,7 @@ namespace zephyr { m_render_geometry_table.erase(delete_task.geometry); } - m_delete_tasks.clear(); + m_delete_tasks[0].clear(); } void GeometryCache::ProcessPendingUploads() { diff --git a/zephyr/renderer/src/render_engine.cpp b/zephyr/renderer/src/render_engine.cpp index d645af6..304b24f 100644 --- a/zephyr/renderer/src/render_engine.cpp +++ b/zephyr/renderer/src/render_engine.cpp @@ -21,6 +21,10 @@ namespace zephyr { m_game_thread_render_objects.clear(); + // Instruct the geometry cache to evict geometries which had been deleted in the submitted frame. + m_geometry_cache.CommitPendingDeleteTaskList(); + + // Build a list of objects to render and instruct the geometry cache to update (if necessary) any geometries we might need to render. scene_root->Traverse([&](SceneNode* node) -> bool { if(!node->IsVisible()) return false; @@ -29,7 +33,7 @@ namespace zephyr { const auto& geometry = mesh_component.geometry; if(geometry) { - m_geometry_cache.ScheduleUploadIfNeeded(geometry.get()); + m_geometry_cache.UpdateGeometry(geometry.get()); m_game_thread_render_objects.push_back({ .local_to_world = node->GetTransform().GetWorld(),