Zephyr: Renderer: fix a race condition when deleting a cached geometry

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

app/next/src/main_window.hpp

@@ -44,6 +44,8 @@ namespace zephyr {
       SDL_Window* m_window{};
       std::shared_ptr<VulkanInstance> m_vk_instance{};
       VkSurfaceKHR m_vk_surface{VK_NULL_HANDLE};
+
+      SceneNode* m_behemoth_scene{};
   };
 
 } // namespace zephyr

zephyr/renderer/include/zephyr/renderer/engine/geometry_cache.hpp

@@ -13,7 +13,11 @@ namespace zephyr {
     public:
       explicit GeometryCache(std::shared_ptr<RenderBackend> 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<const Geometry*, GeometryState> m_geometry_state_table{};
       std::unordered_map<const Geometry*, RenderGeometry*> m_render_geometry_table{};
       std::vector<UploadTask> m_upload_tasks{};
-      std::vector<DeleteTask> m_delete_tasks{};
+      std::vector<DeleteTask> m_delete_tasks[2]{};
   };
 
 } // namespace zephyr

zephyr/renderer/src/engine/geometry_cache.cpp

@@ -1,10 +1,16 @@
 
 #include <zephyr/renderer/engine/geometry_cache.hpp>
+#include <zephyr/panic.hpp>
+#include <algorithm>
 #include <cstring>
 
 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,19 +53,22 @@ 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);
       }
       m_render_geometry_table.erase(delete_task.geometry);
     }
 
-    m_delete_tasks.clear();
+    m_delete_tasks[0].clear();
   }
 
   void GeometryCache::ProcessPendingUploads() {

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(),