From b4041856efe3f2a6916df42d01cae1f573beaf5b Mon Sep 17 00:00:00 2001 From: swinston Date: Wed, 19 Aug 2026 15:47:48 -0700 Subject: [PATCH] Implement reference counting in ResourceManager (#320) refCount was documented as part of the resource management design but never implemented: LoadResource never incremented it and UnloadResource unloaded and erased a resource unconditionally, even with other outstanding handles to it. Also drop the `final` on Resource and ResourceManager: as written, neither could ever be subclassed, so none of the concrete resource types the tutorial documents (Texture, Mesh, Shader) could actually exist. --- .../simple_engine/resource_manager.cpp | 2 +- attachments/simple_engine/resource_manager.h | 63 ++++++++++++++++--- 2 files changed, 54 insertions(+), 11 deletions(-) diff --git a/attachments/simple_engine/resource_manager.cpp b/attachments/simple_engine/resource_manager.cpp index 006c16b46..3e4c3575f 100644 --- a/attachments/simple_engine/resource_manager.cpp +++ b/attachments/simple_engine/resource_manager.cpp @@ -43,7 +43,7 @@ void ResourceManager::UnloadAllResources() for (auto &innerKv : val) { auto &loadedResource = innerKv.second; - loadedResource->Unload(); + loadedResource.resource->Unload(); } val.clear(); } diff --git a/attachments/simple_engine/resource_manager.h b/attachments/simple_engine/resource_manager.h index 2e0b4c539..ec4cfecd8 100644 --- a/attachments/simple_engine/resource_manager.h +++ b/attachments/simple_engine/resource_manager.h @@ -26,7 +26,7 @@ /** * @brief Base class for all resources. */ -class Resource final +class Resource { protected: std::string resourceId; @@ -157,10 +157,19 @@ class ResourceHandle * This class implements the resource management system as described in the Engine_Architecture chapter: * @see en/Building_a_Simple_Engine/Engine_Architecture/04_resource_management.adoc */ -class ResourceManager final +class ResourceManager { private: - std::unordered_map>> resources; + /** + * @brief A stored resource together with how many outstanding ResourceHandles reference it. + */ + struct ResourceData + { + std::unique_ptr resource; + int refCount = 0; + }; + + std::unordered_map> resources; public: /** @@ -191,6 +200,8 @@ class ResourceManager final auto it = typeResources.find(id); if (it != typeResources.end()) { + // Resource already loaded: another handle now references it too. + ++it->second.refCount; return ResourceHandle(id, this); } @@ -201,8 +212,8 @@ class ResourceManager final throw std::runtime_error("Failed to load resource: " + id); } - // Store the resource - typeResources[id] = std::move(resource); + // Store the resource with an initial reference count of 1 + typeResources[id] = ResourceData{std::move(resource), 1}; return ResourceHandle(id, this); } @@ -230,7 +241,34 @@ class ResourceManager final return nullptr; } - return static_cast(resourceIt->second.get()); + return static_cast(resourceIt->second.resource.get()); + } + + /** + * @brief Get the current reference count of a resource. + * @tparam T The type of resource. + * @param id The resource ID. + * @return The number of outstanding references, or 0 if the resource doesn't exist. + */ + template + int GetRefCount(const std::string &id) const + { + static_assert(std::is_base_of::value, "T must derive from Resource"); + + auto typeIt = resources.find(std::type_index(typeid(T))); + if (typeIt == resources.end()) + { + return 0; + } + + auto &typeResources = typeIt->second; + auto resourceIt = typeResources.find(id); + if (resourceIt == typeResources.end()) + { + return 0; + } + + return resourceIt->second.refCount; } /** @@ -255,10 +293,12 @@ class ResourceManager final } /** - * @brief Unload a resource. + * @brief Release a reference to a resource. Only actually unloads it once its + * reference count drops to zero, i.e. once every ResourceHandle that was + * loaded against this id has released its reference. * @tparam T The type of resource. * @param id The resource ID. - * @return True if the resource was unloaded, false otherwise. + * @return True if the resource existed and a reference was released, false otherwise. */ template bool UnloadResource(const std::string &id) @@ -278,8 +318,11 @@ class ResourceManager final return false; } - resourceIt->second->Unload(); - typeResources.erase(resourceIt); + if (--resourceIt->second.refCount <= 0) + { + resourceIt->second.resource->Unload(); + typeResources.erase(resourceIt); + } return true; }