From f5ea4d513cf176ce2ae8f82f081a7fc740d1c3bb Mon Sep 17 00:00:00 2001 From: William Moberg Date: Fri, 15 Jan 2016 15:25:01 +0100 Subject: [PATCH] ResourceManager::Load never returns null, throws exceptions instead. --- include/Engine/Core/ResourceManager.h | 118 +++++++++++++------------- src/Engine/Collision/Collision.cpp | 4 +- src/Engine/Core/ResourceManager.cpp | 70 +++++++++------ src/Engine/Rendering/Model.cpp | 6 +- src/Engine/Rendering/RenderSystem.cpp | 16 +++- 5 files changed, 117 insertions(+), 97 deletions(-) diff --git a/include/Engine/Core/ResourceManager.h b/include/Engine/Core/ResourceManager.h index 9ebf545a..cb78ce13 100644 --- a/include/Engine/Core/ResourceManager.h +++ b/include/Engine/Core/ResourceManager.h @@ -21,6 +21,23 @@ protected: Resource() { } public: + //Should be thrown in a Resource's constructor if it cannot complete because another resource is still loading. + //Not actually an error, just a message to the ResourceManager. + struct StillLoadingException : public std::exception + { + virtual const char* what() const throw() + { + return "Resource is still loading."; + } + }; + struct FailedLoadingException : public std::exception + { + virtual const char* what() const throw() + { + return "Resource is failed to load."; + } + }; + // Pretend that this is a pure virtual function that you have to implement // FIXME: Why did we do this again instead of just using the constructor? // static Resource* Create(std::string resourceName); @@ -39,16 +56,6 @@ public: class ThreadUnsafeResource : public Resource { friend class ResourceManager; -protected: - //Should be thrown in a Resource's constructor if it cannot complete because another resource is still loading. - //Not actually an error, just a message to the ResourceManager. - struct StillLoadingException : public std::exception - { - virtual const char* what() const throw() - { - return "Resource is still loading."; - } - }; }; /** Singleton resource manager to keep track of and cache any external engine assets */ @@ -75,18 +82,18 @@ public: // TODO: Templateify static bool IsResourceLoaded(std::string resourceType, std::string resourceName); - /** If Async is false: Hot-loads a resource and caches it for future use. - Fairly safe to assume that return value is always a valid pointer, will only return nullptr on error. - + /** Return value should always be a valid pointer, will throw an exception on error. + If the resource has been loaded already, returns a pointer to it. + + If Async is false: Hot-loads a resource, caches it for future use, and returns a pointer to it. If Async is true: If the resource is not loaded yet, starts loading the resource - in the background and returns nullptr immediately. - If the resource has been loaded already, return a pointer to it. + in the background and throws Resource::StillLoadingException immediately. @tparam T Resource type. - @tparam Async Set this to true if the resource should be loaded asyncronously. + @tparam async Set this to true if the resource should be loaded asyncronously. @param resourceName Fully qualified name of the resource to load. */ - template + template static T* Load(std::string resourceName, Resource* parent = nullptr); /** Reloads an already loaded resource, keeping its resource ID intact. @@ -101,22 +108,6 @@ public: static void Update(); private: - //Represents a pointer value to signify that a resource haven't failed, but is not fully loaded. - class SpecialResourcePointer - { - public: - SpecialResourcePointer() - : m_Val(new Resource()) - {} - ~SpecialResourcePointer() - { - delete m_Val; - } - //Make this class implicitly convertible to the Resource*. - operator Resource* const() const { return m_Val; } - private: - Resource* const m_Val; - }; //This is a haxy way to make sure that IsMainThread is run when the program starts, so the master thread id is set. struct MasterThreadChecker { @@ -125,7 +116,6 @@ private: ResourceManager::IsMainThread(); } }; - const static SpecialResourcePointer m_StillLoading; const static MasterThreadChecker m_Checker; static std::unordered_map m_CompilerTypenameToResourceType; @@ -135,6 +125,7 @@ private: static std::unordered_map m_ResourceParents; // resource -> parent resource static std::unordered_map, boost::thread> m_LoadingThreads; // (type, name) -> loading thread + static std::unordered_map, std::exception_ptr> m_LoadingThreadExceptions; // (type, name) -> exceptions static boost::recursive_mutex m_Mutex; // TODO: Getters for IDs @@ -150,7 +141,9 @@ private: static unsigned int GetNewResourceID(unsigned int typeID); // Internal: Create a resource and cache it - static Resource* createResource(std::string resourceType, std::string resourceName, Resource* parent); + static Resource* createResourceThrowing(std::string resourceType, std::string resourceName, Resource* parent); + static Resource* createResource(std::string resourceType, std::string resourceName, Resource* parent, std::exception_ptr& exception); + static Resource* cacheResource(Resource* resource, std::string resourceType, std::string resourceName, Resource* parent); static bool IsMainThread(); }; @@ -169,14 +162,14 @@ static T* ResourceManager::Load(std::string resourceName, Resource* parent /* = auto iter = m_CompilerTypenameToResourceType.find(resourceTypename); if (iter == m_CompilerTypenameToResourceType.end()) { LOG_ERROR("Failed to load resource \"%s\" of type \"%s\": type not registered", resourceName.c_str(), resourceTypename); - return nullptr; + throw Resource::FailedLoadingException(); } std::string resourceType = iter->second; constexpr bool mustNotLoadInThread = std::is_base_of::value; if (mustNotLoadInThread && !IsMainThread()) { LOG_ERROR("Failed to Load \"%s\": ThreadUnsafeResource type \"%s\" load in the constructor of another resource that is loaded asyncronously.", resourceName.c_str(), iter->second.c_str()); - return nullptr; + throw Resource::FailedLoadingException(); } auto cacheKey = std::make_pair(resourceType, resourceName); @@ -185,9 +178,9 @@ static T* ResourceManager::Load(std::string resourceName, Resource* parent /* = auto tIt = m_LoadingThreads.find(cacheKey); if (tIt != m_LoadingThreads.end()) { if (async) { - //Return null if the thread is still working. + //Throw StillLoadingException if the thread is still working. if (!tIt->second.try_join_for(boost::chrono::nanoseconds(1))) { - return nullptr; + throw Resource::StillLoadingException(); } //Else we know the thread has completed. } else { @@ -196,42 +189,51 @@ static T* ResourceManager::Load(std::string resourceName, Resource* parent /* = } //When the thread is done, delete the thread. m_LoadingThreads.erase(tIt); - //Find the resource that the thread loaded. - it = m_ResourceCache.find(cacheKey); - if (it != m_ResourceCache.end()) { - //Threads should not be able to throw StillLoadingException, so no check should be needed. - return static_cast(it->second); - } else { - //If cacheKey does not exist after thread finishes, it failed. - return nullptr; + //Rethrow the thread exception if it threw any. + auto excIt = m_LoadingThreadExceptions.find(cacheKey); + std::exception_ptr exception = excIt->second; + m_LoadingThreadExceptions.erase(excIt); + if (exception) { + std::rethrow_exception(exception); } } //If resource has already been cached and completely loaded. it = m_ResourceCache.find(cacheKey); - if (it != m_ResourceCache.end() && it->second != m_StillLoading) { - return static_cast(it->second); + if (it != m_ResourceCache.end()) { + if (it->second != nullptr) { + return static_cast(it->second); + } else { + //Don't return null on failure, exception instead. + throw Resource::FailedLoadingException(); + } } Resource* res = nullptr; //If resource is not cached.. if (async) { if (mustNotLoadInThread) { - res = createResource(resourceType, resourceName, parent); - if (res != m_StillLoading) { - return static_cast(res); + try { + return static_cast(createResourceThrowing(resourceType, resourceName, parent)); + } catch (const Resource::StillLoadingException&) { + throw; } } else { //Create a thread that loads the resource into cache. - m_LoadingThreads[cacheKey] = boost::thread(createResource, resourceType, resourceName, parent); + m_LoadingThreads[cacheKey] = boost::thread(createResource, resourceType, resourceName, parent, m_LoadingThreadExceptions[cacheKey]); + throw Resource::StillLoadingException(); } - return nullptr; } else { //load and return the resource. - do { - res = createResource(resourceType, resourceName, parent); - } while (res == m_StillLoading); - return static_cast(res); + while (true) { + try { + return static_cast(createResourceThrowing(resourceType, resourceName, parent)); + } catch (const Resource::StillLoadingException&) { + continue; + } catch (const std::exception&) { + throw; + } + } } } diff --git a/src/Engine/Collision/Collision.cpp b/src/Engine/Collision/Collision.cpp index c0b2c658..c74af3ad 100644 --- a/src/Engine/Collision/Collision.cpp +++ b/src/Engine/Collision/Collision.cpp @@ -220,7 +220,7 @@ bool attachAABBComponentFromModel(World* world, EntityID id) } ComponentWrapper model = world->GetComponent(id, "Model"); ComponentWrapper collision = world->AttachComponent(id, "AABB"); - Model* modelRes = ResourceManager::Load(model["Resource"]); + Model* modelRes = ResourceManager::Load(model["Resource"]); if (modelRes == nullptr) { return false; } @@ -247,7 +247,7 @@ bool GetEntityBox(World* world, ComponentWrapper& AABBComponent, AABB& outBox) { ComponentWrapper& cTrans = world->GetComponent(AABBComponent.EntityID, "Transform"); ComponentWrapper model = world->GetComponent(AABBComponent.EntityID, "Model"); - Model* modelRes = ResourceManager::Load(model["Resource"]); + Model* modelRes = ResourceManager::Load(model["Resource"]); outBox.CreateFromCenter(AABBComponent["BoxCenter"], AABBComponent["BoxSize"]); glm::vec3 mini = outBox.MinCorner(); glm::vec3 maxi = outBox.MaxCorner(); diff --git a/src/Engine/Core/ResourceManager.cpp b/src/Engine/Core/ResourceManager.cpp index 30614e5c..ed79e4f7 100644 --- a/src/Engine/Core/ResourceManager.cpp +++ b/src/Engine/Core/ResourceManager.cpp @@ -13,8 +13,8 @@ std::unordered_map ResourceManager::m_ResourceTypeIDs std::unordered_map ResourceManager::m_ResourceCount; FileWatcher ResourceManager::m_FileWatcher; std::unordered_map, boost::thread> ResourceManager::m_LoadingThreads; +std::unordered_map, std::exception_ptr> ResourceManager::m_LoadingThreadExceptions; boost::recursive_mutex ResourceManager::m_Mutex; -const ResourceManager::SpecialResourcePointer ResourceManager::m_StillLoading; unsigned int ResourceManager::GetTypeID(std::string resourceType) { @@ -79,47 +79,61 @@ void ResourceManager::Update() m_FileWatcher.Check(); } -Resource* ResourceManager::createResource(std::string resourceType, std::string resourceName, Resource* parent) +Resource* ResourceManager::createResource(std::string resourceType, std::string resourceName, Resource* parent, std::exception_ptr& exception) { auto facIt = m_FactoryFunctions.find(resourceType); if (facIt == m_FactoryFunctions.end()) { LOG_ERROR("Failed to load resource \"%s\" of type \"%s\": type not registered", resourceName.c_str(), resourceType.c_str()); - return nullptr; + cacheResource(nullptr, resourceType, resourceName, parent); + //This basically throws an exception. + exception = std::make_exception_ptr(Resource::FailedLoadingException()); return nullptr; } // Call the factory function - Resource* resource = nullptr; try { - resource = facIt->second(resourceName); - } catch (const ThreadUnsafeResource::StillLoadingException&) { - resource = m_StillLoading; + return cacheResource(facIt->second(resourceName), resourceType, resourceName, parent); + } catch (const Resource::StillLoadingException&) { + exception = std::current_exception(); return nullptr; } catch (const std::exception& e) { LOG_ERROR("Failed to load resource \"%s\" of type \"%s\": %s", resourceName.c_str(), resourceType.c_str(), e.what()); + cacheResource(nullptr, resourceType, resourceName, parent); + exception = std::current_exception(); return nullptr; + } +} + + +Resource* ResourceManager::createResourceThrowing(std::string resourceType, std::string resourceName, Resource* parent) +{ + std::exception_ptr exception; + Resource* res = createResource(resourceType, resourceName, parent, exception); + if (exception) { + std::rethrow_exception(exception); + } + return res; +} + +Resource* ResourceManager::cacheResource(Resource* resource, std::string resourceType, std::string resourceName, Resource* parent) +{ + //Lock the mutex immediately, and unlock it when leaving the code block. + boost::lock_guard guard(m_Mutex); + if (resource != nullptr) { + // Store IDs + resource->TypeID = GetTypeID(resourceType); + resource->ResourceID = GetNewResourceID(resource->TypeID); } - { - //Lock the mutex immediately, and unlock it when leaving the code block. - boost::lock_guard guard(m_Mutex); - if (resource != nullptr) { - // Store IDs - resource->TypeID = GetTypeID(resourceType); - resource->ResourceID = GetNewResourceID(resource->TypeID); - } - - // Cache - m_ResourceCache[std::make_pair(resourceType, resourceName)] = resource; - m_ResourceFromName[resourceName] = resource; - if (parent != nullptr) { - m_ResourceParents[resource] = parent; - } - - if (!boost::filesystem::is_directory(resourceName)) { - LOG_DEBUG("Adding watch for %s", resourceName.c_str()); - m_FileWatcher.AddWatch(resourceName, fileWatcherCallback); - } + // Cache + m_ResourceCache[std::make_pair(resourceType, resourceName)] = resource; + m_ResourceFromName[resourceName] = resource; + if (parent != nullptr) { + m_ResourceParents[resource] = parent; } - return resource; + //if (!boost::filesystem::is_directory(resourceName)) { + // LOG_DEBUG("Adding watch for %s", resourceName.c_str()); + // m_FileWatcher.AddWatch(resourceName, fileWatcherCallback); + //} + return resource; } bool ResourceManager::IsMainThread() diff --git a/src/Engine/Rendering/Model.cpp b/src/Engine/Rendering/Model.cpp index c1afa037..33539a14 100644 --- a/src/Engine/Rendering/Model.cpp +++ b/src/Engine/Rendering/Model.cpp @@ -2,12 +2,8 @@ Model::Model(std::string fileName) { - //Load the RawModel asyncronously, this will be done on a separate thread in the background. + //Try loading the model asyncronously, if it throws any exceptions then let it propagate back to caller. m_RawModel = ResourceManager::Load(fileName); - //If it is null, the model is not done yet, so tell resourceManager to try constructing me again later. - if (m_RawModel == nullptr) { - throw StillLoadingException(); - } for (auto& group : m_RawModel->MaterialGroups) { if (!group.TexturePath.empty()) { diff --git a/src/Engine/Rendering/RenderSystem.cpp b/src/Engine/Rendering/RenderSystem.cpp index e228b33e..6431bde2 100644 --- a/src/Engine/Rendering/RenderSystem.cpp +++ b/src/Engine/Rendering/RenderSystem.cpp @@ -84,13 +84,21 @@ void RenderSystem::fillModels(std::list>& jobs, World continue; } - Model* model = ResourceManager::Load<::Model, true>(resource); - if (model == nullptr) { - model = ResourceManager::Load<::Model>("Models/Core/Error.obj"); + Model* model; + try { + model = ResourceManager::Load<::Model, true>(resource); + } catch (const Resource::StillLoadingException&) { + //continue; + model = ResourceManager::Load<::Model>("Models/Core/UnitRaptor.obj"); + } catch (const std::exception&) { + try { + model = ResourceManager::Load<::Model>("Models/Core/Error.obj"); + } catch (const std::exception&) { + continue; + } } glm::mat4 modelMatrix = Transform::ModelMatrix(modelComponent.EntityID, world); - for (auto matGroup : model->MaterialGroups()) { std::shared_ptr modelJob = std::shared_ptr(new ModelJob(model, m_Camera, modelMatrix, matGroup, modelComponent, world)); jobs.push_back(modelJob);