Fix graphics resource free bug / Scene reload crash

assets/scripts/tests/scene-reload.txt

@@ -0,0 +1,40 @@
+x.PhysicsFPS 120
+g.LogicFPS 120
+loadscene test1
+screenshot reload-before1.png
+stepgraphics
+steplogic
+stepphysics
+stepgraphics 120
+screenshot reload-before2.png
+stepgraphics
+assert_position test1:animbox 0 1.5 -4.875
+
+reloadscene
+screenshot reload-after1.png
+stepgraphics
+assert_position test1:animbox 0 1.5 -4.875
+steplogic
+stepphysics
+stepgraphics
+
+reloadscene
+reloadscene
+reloadscene
+reloadscene
+screenshot reload-after2.png
+stepgraphics
+assert_position test1:animbox 0 1.5 -4.875
+steplogic 2700
+stepphysics 2
+stepgraphics 120
+screenshot reload-after3.png
+stepgraphics
+assert_position test1:animbox 0 1.5 -4.875
+stepphysics 120
+assert_position test1:animbox 0 2 -4.875
+stepphysics 120
+assert_position test1:animbox 0 5 -4.875
+stepgraphics 120
+screenshot reload-after4.png
+stepgraphics

src/core/ecs/EcsImpl.hh

@@ -63,7 +63,7 @@ namespace ecs {
         ECS staging;
         ECS live;
         EntityReferenceManager refManager;
-        sp::DispatchQueue transactionQueue = sp::DispatchQueue("ECSTransactionQueue", 2, std::chrono::milliseconds(1));
+        sp::DispatchQueue transactionQueue = sp::DispatchQueue("ECSTransactionQueue", 1, std::chrono::milliseconds(1));
     };
 
     // Define these special components here to solve circular includes

src/core/ecs/EventQueue.cc

@@ -246,13 +246,13 @@ namespace ecs {
         return true;
     }
 
-    bool EventQueue::Add(const Event &event, size_t transactionId) {
+    bool EventQueue::Add(const Event &event, uint64_t transactionId) {
         AsyncEvent asyncEvent(event.name, event.source, event.data);
         asyncEvent.transactionId = transactionId;
         return Add(asyncEvent);
     }
 
-    bool EventQueue::Poll(Event &eventOut, size_t transactionId) {
+    bool EventQueue::Poll(Event &eventOut, uint64_t transactionId) {
         bool outputSet = false;
         while (!outputSet) {
             State s = state.load();
@@ -291,7 +291,7 @@ namespace ecs {
         return s.head == s.tail;
     }
 
-    size_t EventQueue::Size() {
+    uint32_t EventQueue::Size() {
         State s = state.load();
         if (s.head > s.tail) {
             return s.tail + events.size() - s.head;

src/core/ecs/EventQueue.hh

@@ -251,7 +251,7 @@ namespace ecs {
         Entity source;
         sp::AsyncPtr<EventData> data;
 
-        size_t transactionId = 0;
+        uint64_t transactionId = 0;
 
         AsyncEvent() {}
         AsyncEvent(std::string_view name, const Entity &source, const sp::AsyncPtr<EventData> &data)
@@ -280,16 +280,16 @@ namespace ecs {
         // Returns false if the queue is full
         bool Add(const AsyncEvent &event);
         // Returns false if the queue is full
-        bool Add(const Event &event, size_t transactionId = 0);
+        bool Add(const Event &event, uint64_t transactionId = 0);
 
         // Returns false if the queue is empty
-        bool Poll(Event &eventOut, size_t transactionId = 0);
+        bool Poll(Event &eventOut, uint64_t transactionId = 0);
 
         bool Empty();
         void Clear();
-        size_t Size();
+        uint32_t Size();
 
-        size_t Capacity() const {
+        uint32_t Capacity() const {
             return events.size();
         }
 

src/core/ecs/components/Events.hh

@@ -35,8 +35,8 @@ namespace ecs {
          * A transactionId can be provided to synchronize event visibility with transactions.
          * If no transactionId is provided, this event will be immediately visible to all transactions.
          */
-        size_t Add(const Event &event, size_t transactionId = 0) const;
-        size_t Add(const AsyncEvent &event) const;
+        uint64_t Add(const Event &event, uint64_t transactionId = 0) const;
+        uint64_t Add(const AsyncEvent &event) const;
         static bool Poll(Lock<Read<EventInput>> lock, const EventQueueRef &queue, Event &eventOut);
 
         robin_hood::unordered_map<EventName, std::vector<EventQueueWeakRef>, sp::StringHash, sp::StringEqual> events;

src/game/game/SceneManager.cc

@@ -12,8 +12,6 @@
 #include "assets/JsonHelpers.hh"
 #include "common/Logging.hh"
 #include "common/Tracing.hh"
-#include "console/Console.hh"
-#include "console/ConsoleBindingManager.hh"
 #include "ecs/EcsImpl.hh"
 #include "ecs/EntityReferenceManager.hh"
 #include "ecs/ScriptManager.hh"
@@ -23,7 +21,6 @@
 
 #include <algorithm>
 #include <filesystem>
-#include <fstream>
 #include <glm/glm.hpp>
 #include <picojson.h>
 #include <robin_hood.h>

src/graphics/graphics/vulkan/render_graph/PassBuilder.cc

@@ -7,6 +7,8 @@
 
 #include "PassBuilder.hh"
 
+#include "graphics/vulkan/render_graph/Resources.hh"
+
 namespace sp::vulkan::render_graph {
     void PassBuilder::Read(ResourceID id, Access access) {
         pass.AddAccess(id, access);

src/graphics/graphics/vulkan/render_graph/Resources.cc

@@ -455,7 +455,7 @@ namespace sp::vulkan::render_graph {
             auto it = frame.resourceNames.begin();
             while (it != frame.resourceNames.end()) {
                 if (it->second == id) {
-                    frame.resourceNames.erase(it++);
+                    it = frame.resourceNames.erase(it);
                 } else {
                     it++;
                 }

src/graphics/graphics/vulkan/render_passes/Blur.cc

@@ -36,6 +36,7 @@ namespace sp::vulkan::renderer {
                 auto desc = builder.DeriveImage(sourceID);
                 desc.extent.width = std::max(desc.extent.width / downsample, 1u);
                 desc.extent.height = std::max(desc.extent.height / downsample, 1u);
+                if (desc.mipLevels > 1) desc.mipLevels = CalculateMipmapLevels(desc.extent);
                 builder.OutputColorAttachment(0, "", desc, {LoadOp::DontCare, StoreOp::Store});
             })
             .Execute([sourceID, constants](Resources &resources, CommandContext &cmd) {

src/scripts/guis/SignalDisplayGui.cc

@@ -63,8 +63,10 @@ namespace sp::scripts {
 
         void Destroy(ScriptState &state) {
             Debugf("Destroying signal display: %llu", state.GetInstanceId());
-            ImGui::DestroyContext(imCtx);
-            imCtx = nullptr;
+            if (imCtx) {
+                ImGui::DestroyContext(imCtx);
+                imCtx = nullptr;
+            }
         }
 
         bool PreDefine(ScriptState &state, Entity ent) {

tests/unit/core-ecs.cc

@@ -44,30 +44,32 @@ namespace CoreEcsTests {
     void TryQueueTransaction() {
         Timer t("Test ecs::QueueTransaction");
 
-        auto entFuture = ecs::QueueTransaction<ecs::AddRemove>([](auto lock) {
-            return lock.NewEntity();
-        });
-        ecs::QueueTransaction<ecs::AddRemove>([entFuture](auto &lock) {
+        for (size_t i = 0; i < 1000; i++) {
+            auto entFuture = ecs::QueueTransaction<ecs::AddRemove>([](auto lock) {
+                return lock.NewEntity();
+            });
+            ecs::QueueTransaction<ecs::AddRemove>([entFuture](auto &lock) {
+                AssertTrue(entFuture->Ready(), "Expected result of first transaction to be available");
+                auto entPtr = entFuture->Get();
+                AssertTrue(entPtr != nullptr, "Expected future to contain a value");
+                ecs::Entity ent = *entPtr;
+                AssertTrue(ent.Exists(lock), "Expected entity to be available in second transaction");
+                ent.Set<ecs::Name>(lock, "test", "entity");
+            });
+            auto result = ecs::QueueTransaction<ecs::AddRemove>([entFuture](auto &lock) {
+                ecs::Entity ent = *entFuture->Get();
+                AssertTrue(ent.Exists(lock), "Expected entity to be available in third transaction");
+                ent.Destroy(lock);
+            });
+            result->Get();
             AssertTrue(entFuture->Ready(), "Expected result of first transaction to be available");
-            auto entPtr = entFuture->Get();
-            AssertTrue(entPtr != nullptr, "Expected future to contain a value");
-            ecs::Entity ent = *entPtr;
-            AssertTrue(ent.Exists(lock), "Expected entity to be available in second transaction");
-            ent.Set<ecs::Name>(lock, "test", "entity");
-        });
-        auto result = ecs::QueueTransaction<ecs::AddRemove>([entFuture](auto &lock) {
-            ecs::Entity ent = *entFuture->Get();
-            AssertTrue(ent.Exists(lock), "Expected entity to be available in third transaction");
-            ent.Destroy(lock);
-        });
-        result->Get();
-        AssertTrue(entFuture->Ready(), "Expected result of first transaction to be available");
 
-        {
-            auto lock = ecs::StartTransaction<>();
-            ecs::Entity ent = *entFuture->Get();
-            AssertTrue((bool)ent, "Expected entity to be available in third transaction");
-            AssertTrue(!ent.Exists(lock), "Expected entity to removed after third transaction");
+            {
+                auto lock = ecs::StartTransaction<>();
+                ecs::Entity ent = *entFuture->Get();
+                AssertTrue((bool)ent, "Expected entity to be available in third transaction");
+                AssertTrue(!ent.Exists(lock), "Expected entity to removed after third transaction");
+            }
         }
     }