Zephyr: Renderer: rework geometry cache and fix geometry update bug (f…

…ixes #10)

This reworks the geometry cache to be a bit cleaner internally and to interface this.
It also fixes a bug where a geometry wasn't updated anymore after removing a single instance of it from the scene,
even if more instances existed.

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

@@ -4,6 +4,7 @@
 #include <zephyr/renderer/backend/render_backend.hpp>
 #include <zephyr/renderer/resource/geometry.hpp>
 #include <EASTL/hash_map.h>
+#include <EASTL/hash_set.h>
 #include <memory>
 #include <vector>
 
@@ -16,11 +17,12 @@ namespace zephyr {
      ~GeometryCache();
 
       // Game Thread API:
-      void CommitPendingDeleteTaskList();
-      void UpdateGeometry(const Geometry* geometry);
+      void QueueTasksForRenderThread();
+      void IncrementGeometryRefCount(const Geometry* geometry);
+      void DecrementGeometryRefCount(const Geometry* geometry);
 
       // Render Thread API:
-      void ProcessPendingUpdates();
+      void ProcessQueuedTasks();
 
       RenderGeometry* GetCachedRenderGeometry(const Geometry* geometry) const {
         const auto match = m_render_geometry_table.find(geometry);
@@ -34,6 +36,7 @@ namespace zephyr {
       struct GeometryState {
         bool uploaded{false};
         u64 current_version{};
+        size_t ref_count{};
         VoidEvent::SubID destruct_event_subscription;
       };
 
@@ -51,10 +54,18 @@ namespace zephyr {
         const Geometry* geometry;
       };
 
-      void ProcessPendingDeletes();
-      void ProcessPendingUploads();
+      // Game Thread:
+      void QueueUploadTasksForUsedGeometries();
+      void QueueDeleteTaskFromPreviousFrame();
+      void QueueGeometryUploadTaskIfNeeded(const Geometry* geometry);
+      void QueueGeometryDeleteTaskForNextFrame(const Geometry* geometry);
+
+      // Render Thread:
+      void ProcessQueuedDeleteTasks();
+      void ProcessQueuedUploadTasks();
 
       std::shared_ptr<RenderBackend> m_render_backend;
+      eastl::hash_set<const Geometry*> m_used_geometry_set{};
       eastl::hash_map<const Geometry*, GeometryState> m_geometry_state_table{};
       mutable eastl::hash_map<const Geometry*, RenderGeometry*> m_render_geometry_table{};
       std::vector<UploadTask> m_upload_tasks{};

zephyr/renderer/include/zephyr/renderer/render_scene.hpp

@@ -84,7 +84,6 @@ namespace zephyr {
 
       std::shared_ptr<SceneGraph> m_current_scene_graph{};
       eastl::hash_map<const SceneNode*, EntityID> m_node_entity_map{};
-      eastl::hash_set<const Geometry*> m_active_geometry_set{};
       bool m_require_full_rebuild{};
 
       std::vector<Entity> m_entities{};

zephyr/renderer/src/backend/opengl/render_backend.cpp

@@ -48,7 +48,7 @@ namespace zephyr {
 
     m_render_geometry_manager = std::make_unique<OpenGLRenderGeometryManager>();
 
-    SDL_GL_SetSwapInterval(0);
+//    SDL_GL_SetSwapInterval(0);
   }
 
   void OpenGLRenderBackend::DestroyContext() {

zephyr/renderer/src/engine/geometry_cache.cpp

@@ -1,6 +1,5 @@
 
 #include <zephyr/renderer/engine/geometry_cache.hpp>
-#include <zephyr/panic.hpp>
 #include <algorithm>
 #include <cstring>
 
@@ -9,15 +8,43 @@ namespace zephyr {
   GeometryCache::~GeometryCache() {
     // Ensure that we do not receive any destruction callbacks from geometries that outlive this geometry cache.
     for(const auto& [geometry, state] : m_geometry_state_table) {
-      geometry->OnBeforeDestruct().Unsubscribe(state.destruct_event_subscription);
+      if(state.uploaded) {
+        geometry->OnBeforeDestruct().Unsubscribe(state.destruct_event_subscription);
+      }
+    }
+  }
+
+  void GeometryCache::QueueTasksForRenderThread() {
+    // Queue (re-)uploads for all geometries used in the submitted frame which are either new or have changed since the last frame.
+    QueueUploadTasksForUsedGeometries();
+
+    // Queue eviction of geometries which had been deleted in the previously submitted frame.
+    QueueDeleteTaskFromPreviousFrame();
+  }
+
+  void GeometryCache::IncrementGeometryRefCount(const Geometry* geometry) {
+    if(++m_geometry_state_table[geometry].ref_count == 1u) {
+      m_used_geometry_set.insert(geometry);
     }
   }
 
-  void GeometryCache::CommitPendingDeleteTaskList() {
+  void GeometryCache::DecrementGeometryRefCount(const Geometry* geometry) {
+    if(--m_geometry_state_table[geometry].ref_count == 0u) {
+      m_used_geometry_set.erase(geometry);
+    }
+  }
+
+  void GeometryCache::QueueUploadTasksForUsedGeometries() {
+    for(const Geometry* geometry : m_used_geometry_set) {
+      QueueGeometryUploadTaskIfNeeded(geometry);
+    }
+  }
+
+  void GeometryCache::QueueDeleteTaskFromPreviousFrame() {
     std::swap(m_delete_tasks[0], m_delete_tasks[1]);
   }
 
-  void GeometryCache::UpdateGeometry(const Geometry* geometry) {
+  void GeometryCache::QueueGeometryUploadTaskIfNeeded(const Geometry* geometry) {
     GeometryState& state = m_geometry_state_table[geometry];
 
     if(!state.uploaded || state.current_version != geometry->CurrentVersion()) {
@@ -39,28 +66,31 @@ namespace zephyr {
       });
 
       if(!state.uploaded) {
-        state.destruct_event_subscription = geometry->OnBeforeDestruct().Subscribe([this, 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);
-        });
+        state.destruct_event_subscription = geometry->OnBeforeDestruct().Subscribe(
+          std::bind(&GeometryCache::QueueGeometryDeleteTaskForNextFrame, this, geometry));
       }
 
       state.uploaded = true;
       state.current_version = geometry->CurrentVersion();
     }
   }
 
-  void GeometryCache::ProcessPendingUpdates() {
-    ProcessPendingDeletes();
-    ProcessPendingUploads();
+  void GeometryCache::QueueGeometryDeleteTaskForNextFrame(const Geometry* geometry) {
+    /**
+     * Queue the geometry for eviction from the cache.
+     * To avoid deleting the geometry before the current frame-in-flight has been rendered,
+     * these tasks will only be processed at the start of the *next* frame on the render thread.
+     */
+    m_delete_tasks[1].push_back({.geometry = (const Geometry*)geometry});
+    m_geometry_state_table.erase((const Geometry*)geometry);
+  }
+
+  void GeometryCache::ProcessQueuedTasks() {
+    ProcessQueuedDeleteTasks();
+    ProcessQueuedUploadTasks();
   }
 
-  void GeometryCache::ProcessPendingDeletes() {
+  void GeometryCache::ProcessQueuedDeleteTasks() {
     for(const auto& delete_task : m_delete_tasks[0]) {
       RenderGeometry* render_geometry = m_render_geometry_table[delete_task.geometry];
       if(render_geometry) {
@@ -72,7 +102,7 @@ namespace zephyr {
     m_delete_tasks[0].clear();
   }
 
-  void GeometryCache::ProcessPendingUploads() {
+  void GeometryCache::ProcessQueuedUploadTasks() {
     for(const auto& upload_task : m_upload_tasks) {
       const Geometry* geometry = upload_task.geometry;
       RenderGeometry* render_geometry = m_render_geometry_table[geometry];

zephyr/renderer/src/render_scene.cpp

@@ -26,13 +26,8 @@ namespace zephyr {
       PatchScene();
     }
 
-    // Instruct the geometry cache to evict geometries which had been deleted in the submitted frame.
-    m_geometry_cache.CommitPendingDeleteTaskList();
-
-    // Update all geometries which might be rendered in this frame.
-    for(const Geometry* geometry : m_active_geometry_set) {
-      m_geometry_cache.UpdateGeometry(geometry);
-    }
+    // Queue geometry cache updates and evictions to be processed on the render thread.
+    m_geometry_cache.QueueTasksForRenderThread();
   }
 
   void RenderScene::GetRenderCamera(RenderCamera& out_render_camera) {
@@ -54,7 +49,7 @@ namespace zephyr {
   }
 
   void RenderScene::UpdateStage2() {
-    m_geometry_cache.ProcessPendingUpdates();
+    m_geometry_cache.ProcessQueuedTasks();
 
     for(const RenderScenePatch& render_scene_patch : m_render_scene_patches) {
       switch(render_scene_patch.type) {
@@ -152,7 +147,7 @@ namespace zephyr {
       entity_mesh.geometry = node->GetComponent<MeshComponent>().geometry.get();
       m_entities[entity_id] |= COMPONENT_FLAG_MESH;
       m_view_mesh.push_back(entity_id);
-      m_active_geometry_set.insert(entity_mesh.geometry);
+      m_geometry_cache.IncrementGeometryRefCount(entity_mesh.geometry);
       m_render_scene_patches.push_back({.type = RenderScenePatch::Type::MeshMounted, .entity_id = entity_id});
     }
 
@@ -175,7 +170,7 @@ namespace zephyr {
       const EntityID entity_id = GetOrCreateEntityForNode(node);
       m_entities[entity_id] &= ~COMPONENT_FLAG_MESH;
       m_view_mesh.erase(std::ranges::find(m_view_mesh, entity_id));
-      m_active_geometry_set.erase(m_components_mesh[entity_id].geometry);
+      m_geometry_cache.DecrementGeometryRefCount(m_components_mesh[entity_id].geometry);
       m_render_scene_patches.push_back({.type = RenderScenePatch::Type::MeshRemoved, .entity_id = entity_id});
       did_remove_component = true;
     }