Advanced resource loading WIP stage 2

BansheeCore/Include/BsResourceHandleRTTI.h

@@ -30,9 +30,10 @@ namespace BansheeEngine
 
 			if(resourceHandle->mData && resourceHandle->mData->mUUID != "")
 			{
-				// NOTE: This will cause Resources::load to be called recursively with resources that contain other
-				// resources. This might cause problems. Keep this note here as a warning until I prove otherwise.
-				HResource loadedResource = gResources().loadFromUUID(resourceHandle->mData->mUUID);
+				// Note: Resource system needs to be aware of this handle before this point is reached so it
+				// can queue the load. This is handled automatically by Resources but is something to be aware of
+				// if you are loading resources in some other way.
+				HResource loadedResource = gResources()._getResourceHandle(resourceHandle->mData->mUUID);
 
 				if(loadedResource)
 					resourceHandle->_setHandleData(loadedResource.getHandleData());

BansheeCore/Include/BsResources.h

@@ -16,6 +16,17 @@ namespace BansheeEngine
 	 */
 	class BS_CORE_EXPORT Resources : public Module<Resources>
 	{
+		struct ResourceLoadData
+		{
+			ResourceLoadData(const HResource& resource, UINT32 numDependencies)
+				:resource(resource), remainingDependencies(numDependencies)
+			{ }
+
+			HResource resource;
+			ResourcePtr loadedData;
+			UINT32 remainingDependencies;
+		};
+
 	public:
 		Resources();
 		~Resources();
@@ -57,23 +68,17 @@ namespace BansheeEngine
 
 		/**
 		 * @brief	Loads the resource with the given UUID. Returns an empty handle if resource can't be loaded.
-		 *			Resource is loaded synchronously.
-		 */
-		HResource loadFromUUID(const String& uuid);
-
-		/**
-		 * @brief	Loads the resource with the given UUID asynchronously. Initially returned resource handle will be invalid
-		 *			until resource loading is done.
 		 *
-		 * @param	uuid	UUID of the resource to load. 
-		 *
-		 * @note	You can use returned invalid handle in engine systems as the engine will check for handle
-		 *			validity before using it.
+		 * @param	uuid	UUID of the resource to load.
+		 * @param	async	If true resource will be loaded asynchronously. Handle to non-loaded
+		 *					resource will be returned immediately while loading will continue in the background.
 		 */
-		HResource loadFromUUIDAsync(const String& uuid);
+		HResource loadFromUUID(const String& uuid, bool async = false);
 
 		/**
-		 * @brief	Unloads the resource that is referenced by the handle. 
+		 * @brief	Unloads the resource that is referenced by the handle. Resource is unloaded regardless if it is 
+		 *			referenced or not. Any dependencies held by the resource will also be unloaded, but only if the
+		 *			resource is holding the last reference to them.
 		 *
 		 * @param	resourceHandle	Handle of the resource to unload.
 		 * 							
@@ -111,6 +116,12 @@ namespace BansheeEngine
 		 */
 		HResource _createResourceHandle(const ResourcePtr& obj);
 
+		/**
+		 * @brief	Returns an existing handle of a resource that has already been loaded,
+		 *			or is currently being loaded.
+		 */
+		HResource _getResourceHandle(const String& uuid);
+
 		/**
 		 * @brief	Allows you to set a resource manifest containing UUID <-> file path mapping that is
 		 * 			used when resolving resource references.
@@ -179,6 +190,11 @@ namespace BansheeEngine
 		 */
 		ResourcePtr loadFromDiskAndDeserialize(const Path& filePath);
 
+		/**
+		 * @brief	Triggered when individual resource has finished loading.
+		 */
+		void loadComplete(HResource& resource);
+
 		/**
 		 * @brief	Callback triggered when the task manager is ready to process the loading task.
 		 */
@@ -192,7 +208,8 @@ namespace BansheeEngine
 		BS_MUTEX(mLoadedResourceMutex);
 
 		UnorderedMap<String, HResource> mLoadedResources;
-		UnorderedMap<String, HResource> mInProgressResources; // Resources that are being asynchronously loaded
+		UnorderedMap<String, ResourceLoadData*> mInProgressResources; // Resources that are being asynchronously loaded
+		UnorderedMap<String, Vector<ResourceLoadData*>> mDependantLoads;
 	};
 
 	/**

BansheeCore/Include/BsSavedResourceData.h

@@ -15,12 +15,12 @@ namespace BansheeEngine
 	{
 	public:
 		SavedResourceData();
-		SavedResourceData(const Vector<HResource>& dependencies, bool allowAsync);
+		SavedResourceData(const Vector<String>& dependencies, bool allowAsync);
 
 		/**
 		 * @brief	Returns a list of all resource dependencies.
 		 */
-		const Vector<HResource>& getDependencies() const { return mDependencies; }
+		const Vector<String>& getDependencies() const { return mDependencies; }
 
 		/**
 		 * @brief	Returns true if this resource is allow to be asynchronously loaded.
@@ -28,7 +28,7 @@ namespace BansheeEngine
 		bool allowAsyncLoading() const { return mAllowAsync; }
 
 	private:
-		Vector<HResource> mDependencies;
+		Vector<String> mDependencies;
 		bool mAllowAsync;
 
 	/************************************************************************/

BansheeCore/Include/BsSavedResourceDataRTTI.h

@@ -9,12 +9,12 @@ namespace BansheeEngine
 	class BS_CORE_EXPORT SavedResourceDataRTTI : public RTTIType <SavedResourceData, IReflectable, SavedResourceDataRTTI>
 	{
 	private:
-		HResource& getDependency(SavedResourceData* obj, UINT32 arrayIdx)
+		String& getDependency(SavedResourceData* obj, UINT32 arrayIdx)
 		{
 			return obj->mDependencies[arrayIdx];
 		}
 
-		void setDependency(SavedResourceData* obj, UINT32 arrayIdx, HResource& val)
+		void setDependency(SavedResourceData* obj, UINT32 arrayIdx, String& val)
 		{
 			obj->mDependencies[arrayIdx] = val;
 		}
@@ -26,7 +26,7 @@ namespace BansheeEngine
 
 		void setNumDependencies(SavedResourceData* obj, UINT32 numEntries)
 		{
-			obj->mDependencies = Vector<HResource>(numEntries);
+			obj->mDependencies = Vector<String>(numEntries);
 		}
 
 		bool& getAllowAsyncLoading(SavedResourceData* obj)
@@ -42,14 +42,14 @@ namespace BansheeEngine
 	public:
 		SavedResourceDataRTTI()
 		{
-			addReflectableArrayField("mDependencies", 0, &SavedResourceDataRTTI::getDependency, &SavedResourceDataRTTI::getNumDependencies,
+			addPlainArrayField("mDependencies", 0, &SavedResourceDataRTTI::getDependency, &SavedResourceDataRTTI::getNumDependencies,
 				&SavedResourceDataRTTI::setDependency, &SavedResourceDataRTTI::setNumDependencies);
 			addPlainField("mAllowAsync", 1, &SavedResourceDataRTTI::getAllowAsyncLoading, &SavedResourceDataRTTI::setAllowAsyncLoading);
 		}
 
 		virtual const String& getRTTIName()
 		{
-			static String name = "Resource";
+			static String name = "ResourceDependencies";
 			return name;
 		}
 

BansheeCore/Include/BsUtility.h

@@ -4,6 +4,20 @@
 
 namespace BansheeEngine
 {
+	/**
+	 * @brief	Contains information about a resource dependency, including
+	 *			the dependant resource and number of references to it.
+	 */
+	struct ResourceDependency
+	{
+		ResourceDependency()
+			:numReferences(0)
+		{ }
+
+		HResource resource;
+		UINT32 numReferences;
+	};
+
 	/**
 	 * @brief	Static class containing various utility methods that do not
 	 *			fit anywhere else.
@@ -20,7 +34,7 @@ namespace BansheeEngine
 		 *
 		 * @returns	A list of unique, non-null resources.
 		 */
-		static Vector<HResource> findResourceDependencies(IReflectable& object, bool recursive = true);
+		static Vector<ResourceDependency> findResourceDependencies(IReflectable& object, bool recursive = true);
 
 	private:
 		/**
@@ -28,6 +42,6 @@ namespace BansheeEngine
 		 *
 		 * @see	findDependencies
 		 */
-		static void findResourceDependenciesInternal(IReflectable& object, bool recursive, Map<String, HResource>& dependencies);
+		static void findResourceDependenciesInternal(IReflectable& object, bool recursive, Map<String, ResourceDependency>& dependencies);
 	};
 }

BansheeCore/Source/BsResources.cpp

@@ -43,7 +43,7 @@ namespace BansheeEngine
 		return loadInternal(filePath, false);
 	}
 
-	HResource Resources::loadFromUUID(const String& uuid)
+	HResource Resources::loadFromUUID(const String& uuid, bool async)
 	{
 		Path filePath;
 		bool foundPath = false;
@@ -65,30 +65,7 @@ namespace BansheeEngine
 			return HResource();
 		}
 
-		return load(filePath);
-	}
-
-	HResource Resources::loadFromUUIDAsync(const String& uuid)
-	{
-		Path filePath;
-		bool foundPath = false;
-
-		for(auto iter = mResourceManifests.rbegin(); iter != mResourceManifests.rend(); ++iter) 
-		{
-			if((*iter)->uuidToFilePath(uuid, filePath))
-			{
-				foundPath = true;
-				break;
-			}
-		}
-
-		if(!foundPath)
-		{
-			gDebug().logWarning("Cannot load resource. Resource with UUID '" + uuid + "' doesn't exist.");
-			return HResource();
-		}
-
-		return loadAsync(filePath);
+		return loadInternal(filePath, !async);
 	}
 
 	HResource Resources::loadInternal(const Path& filePath, bool synchronous)
@@ -107,73 +84,97 @@ namespace BansheeEngine
 		if(!foundUUID)
 			uuid = UUIDGenerator::instance().generateRandom();
 
-		// Load saved resource data
-		{
-
-		}
-
+		HResource outputResource;
+		bool alreadyLoading = false;
 		{
 			BS_LOCK_MUTEX(mLoadedResourceMutex);
 			auto iterFind = mLoadedResources.find(uuid);
 			if(iterFind != mLoadedResources.end()) // Resource is already loaded
 			{
-				return iterFind->second;
+				outputResource = iterFind->second;
+				alreadyLoading = true;
 			}
 		}
 
-		bool resourceLoadingInProgress = false;
-		HResource existingResource;
-
+		if (!alreadyLoading) // If not already detected as loaded
 		{
 			BS_LOCK_MUTEX(mInProgressResourcesMutex);
 			auto iterFind2 = mInProgressResources.find(uuid);
 			if(iterFind2 != mInProgressResources.end()) 
 			{
-				existingResource = iterFind2->second;
-				resourceLoadingInProgress = true;
-			}
-		}
+				outputResource = iterFind2->second->resource;
 
-		if(resourceLoadingInProgress) // We're already loading this resource
-		{
-			if(!synchronous)
-				return existingResource;
-			else
-			{
 				// Previously being loaded as async but now we want it synced, so we wait
-				existingResource.blockUntilLoaded();
+				if (synchronous)
+					outputResource.blockUntilLoaded();
 
-				return existingResource;
+				alreadyLoading = true;
 			}
 		}
 
+		// Not loaded and not in progress, start loading of new resource
+		// (or if already loaded or in progress, load any dependencies)
+		if (!alreadyLoading)
+			outputResource = HResource(uuid);
+
 		if(!FileSystem::isFile(filePath))
 		{
 			gDebug().logWarning("Specified file: " + filePath.toString() + " doesn't exist.");
-			return HResource();
+
+			loadComplete(outputResource);
+			return outputResource;
 		}
 
-		HResource newResource(uuid);
+		// Load saved resource data
+		FileDecoder fs(filePath);
+		SPtr<SavedResourceData> savedResourceData = std::static_pointer_cast<SavedResourceData>(fs.decode());
 
+		// If already loading keep the old load operation active, 
+		// otherwise create a new one
+		if (!alreadyLoading)
 		{
 			BS_LOCK_MUTEX(mInProgressResourcesMutex);
-			mInProgressResources[uuid] = newResource;
+
+			ResourceLoadData* loadData = bs_new<ResourceLoadData>(outputResource, 0);
+			mInProgressResources[uuid] = loadData;
+			loadData->resource = outputResource;
+
+			for (auto& dependency : savedResourceData->getDependencies())
+			{
+				if (dependency != uuid)
+				{
+					mDependantLoads[dependency].push_back(loadData);
+					loadData->remainingDependencies++;
+				}
+			}
 		}
 
-		if(synchronous)
+		// Load dependencies
 		{
-			loadCallback(filePath, newResource);
+			for (auto& dependency : savedResourceData->getDependencies())
+			{
+				loadFromUUID(dependency, !synchronous);
+			}
 		}
-		else
+
+		// Actually queue the load
+		if (!alreadyLoading)
 		{
-			String fileName = filePath.getFilename();
-			String taskName = "Resource load: " + fileName;
+			if (synchronous || !savedResourceData->allowAsyncLoading())
+			{
+				loadCallback(filePath, outputResource);
+			}
+			else
+			{
+				String fileName = filePath.getFilename();
+				String taskName = "Resource load: " + fileName;
 
-			TaskPtr task = Task::create(taskName, std::bind(&Resources::loadCallback, this, filePath, newResource));
-			TaskScheduler::instance().addTask(task);
+				TaskPtr task = Task::create(taskName, std::bind(&Resources::loadCallback, this, filePath, outputResource));
+				TaskScheduler::instance().addTask(task);
+			}
 		}
 
-		return newResource;
+		return outputResource;
 	}
 
 	ResourcePtr Resources::loadFromDiskAndDeserialize(const Path& filePath)
@@ -194,8 +195,17 @@ namespace BansheeEngine
 
 	void Resources::unload(HResource resource)
 	{
-		if(!resource.isLoaded()) // If it's still loading wait until that finishes
+		if (resource == nullptr)
+			return;
+
+		if (!resource.isLoaded()) // If it's still loading wait until that finishes
+		{
+			LOGWRN("Performance warning: Unloading a resource that is still in process of loading \
+				   causes a stall until resource finishes loading.");
 			resource.blockUntilLoaded();
+		}
+
+		Vector<ResourceDependency> dependencies = Utility::findResourceDependencies(*resource.get(), false);
 
 		// Call this before we actually destroy it
 		onResourceDestroyed(resource);
@@ -208,6 +218,21 @@ namespace BansheeEngine
 		}
 
 		resource._setHandleData(nullptr, "");
+
+		for (auto& dependency : dependencies)
+		{
+			HResource dependantResource = dependency.resource;
+
+			// Last reference was kept by the unloaded resource, so unload the dependency too
+			if ((UINT32)dependantResource.mData.use_count() == (dependency.numReferences + 1))
+			{
+				// TODO - Use count is not thread safe. Meaning it might increase after above check, in
+				// which case we will be unloading a resource that is in use. I don't see a way around 
+				// it at the moment.
+
+				unload(dependantResource);
+			}
+		}
 	}
 
 	void Resources::unloadAllUnused()
@@ -223,6 +248,9 @@ namespace BansheeEngine
 			}
 		}
 
+		// Note: When unloading multiple resources it's possible that unloading one will also unload
+		// another resource in "resourcesToUnload". This is fine because "unload" deals with invalid
+		// handles gracefully.
 		for(auto iter = resourcesToUnload.begin(); iter != resourcesToUnload.end(); ++iter)
 		{
 			unload(*iter);
@@ -245,8 +273,13 @@ namespace BansheeEngine
 
 		mDefaultResourceManifest->registerResource(resource.getUUID(), filePath);
 
-		Vector<HResource> dependencyList = Utility::findResourceDependencies(*resource.get(), false);
-		SPtr<SavedResourceData> resourceData = bs_shared_ptr<SavedResourceData>(dependencyList, resource->allowAsyncLoading());
+		Vector<ResourceDependency> dependencyList = Utility::findResourceDependencies(*resource.get(), false);
+
+		Vector<String> dependencyUUIDs(dependencyList.size());
+		for (UINT32 i = 0; i < (UINT32)dependencyList.size(); i++)
+			dependencyUUIDs[i] = dependencyList[i].resource.getUUID();
+
+		SPtr<SavedResourceData> resourceData = bs_shared_ptr<SavedResourceData>(dependencyUUIDs, resource->allowAsyncLoading());
 
 		FileEncoder fs(filePath);
 		fs.encode(resourceData.get());
@@ -290,6 +323,29 @@ namespace BansheeEngine
 		return newHandle;
 	}
 
+	HResource Resources::_getResourceHandle(const String& uuid)
+	{
+		{
+			BS_LOCK_MUTEX(mLoadedResourceMutex);
+			auto iterFind = mLoadedResources.find(uuid);
+			if (iterFind != mLoadedResources.end()) // Resource is already loaded
+			{
+				return iterFind->second;
+			}
+		}
+
+		{
+			BS_LOCK_MUTEX(mInProgressResourcesMutex);
+			auto iterFind2 = mInProgressResources.find(uuid);
+			if (iterFind2 != mInProgressResources.end())
+			{
+				return iterFind2->second->resource;
+			}
+		}
+
+		return HResource();
+	}
+
 	bool Resources::getFilePathFromUUID(const String& uuid, Path& filePath) const
 	{
 		for(auto iter = mResourceManifests.rbegin(); iter != mResourceManifests.rend(); ++iter) 
@@ -312,25 +368,68 @@ namespace BansheeEngine
 		return false;
 	}
 
-	void Resources::loadCallback(const Path& filePath, HResource& resource)
+	void Resources::loadComplete(HResource& resource)
 	{
-		ResourcePtr rawResource = loadFromDiskAndDeserialize(filePath);
+		String uuid = resource.getUUID();
 
+		ResourceLoadData* myLoadData = nullptr;
+		Vector<ResourceLoadData*> dependantLoads;
 		{
 			BS_LOCK_MUTEX(mInProgressResourcesMutex);
-			mInProgressResources.erase(resource.getUUID());
+			auto iterFind = mInProgressResources.find(uuid);
+			if (iterFind != mInProgressResources.end())
+			{
+				myLoadData = iterFind->second;
+				mInProgressResources.erase(iterFind);
+			}
+
+			dependantLoads = mDependantLoads[uuid];
+			mDependantLoads.erase(uuid);
 		}
 
-		resource._setHandleData(rawResource, resource.getUUID());
+		if (resource)
+		{
+			{
+				BS_LOCK_MUTEX(mLoadedResourceMutex);
+				mLoadedResources[resource.getUUID()] = resource;
+			}
 
-		onResourceLoaded(resource);
+			resource._setHandleData(myLoadData->loadedData, resource.getUUID());
+			onResourceLoaded(resource);
+		}
+
+		if (myLoadData != nullptr)
+			bs_delete(myLoadData);
 
+		for (auto& dependantLoad : dependantLoads)
 		{
-			BS_LOCK_MUTEX(mLoadedResourceMutex);
-			mLoadedResources[resource.getUUID()] = resource;
+			dependantLoad->remainingDependencies--;
+
+			if (dependantLoad->remainingDependencies == 0)
+				loadComplete(dependantLoad->resource);
 		}
 	}
 
+	void Resources::loadCallback(const Path& filePath, HResource& resource)
+	{
+		ResourcePtr rawResource = loadFromDiskAndDeserialize(filePath);
+
+		bool finishLoad = false;
+		{
+			BS_LOCK_MUTEX(mInProgressResourcesMutex);
+
+			// Check if all my dependencies are loaded
+			ResourceLoadData* myLoadData = mInProgressResources[resource.getUUID()];
+			myLoadData->loadedData = rawResource;
+			myLoadData->remainingDependencies--;
+
+			finishLoad = myLoadData->remainingDependencies == 0;
+		}
+
+		if (finishLoad)
+			loadComplete(resource);
+	}
+
 	BS_CORE_EXPORT Resources& gResources()
 	{
 		return Resources::instance();

BansheeCore/Source/BsSavedResourceData.cpp

@@ -7,8 +7,8 @@ namespace BansheeEngine
 		:mAllowAsync(true)
 	{ }
 
-	SavedResourceData::SavedResourceData(const Vector<HResource>& dependencies, bool allowAsync)
-		:mDependencies(dependencies), mAllowAsync(allowAsync)
+	SavedResourceData::SavedResourceData(const Vector<String>& dependencies, bool allowAsync)
+		: mAllowAsync(allowAsync), mDependencies(dependencies)
 	{
 
 	}

BansheeCore/Source/BsUtility.cpp

@@ -3,12 +3,12 @@
 
 namespace BansheeEngine
 {
-	Vector<HResource> Utility::findResourceDependencies(IReflectable& obj, bool recursive)
+	Vector<ResourceDependency> Utility::findResourceDependencies(IReflectable& obj, bool recursive)
 	{
-		Map<String, HResource> dependencies;
+		Map<String, ResourceDependency> dependencies;
 		findResourceDependenciesInternal(obj, recursive, dependencies);
 
-		Vector<HResource> dependencyList(dependencies.size());
+		Vector<ResourceDependency> dependencyList(dependencies.size());
 		UINT32 i = 0;
 		for (auto& entry : dependencies)
 		{
@@ -19,7 +19,7 @@ namespace BansheeEngine
 		return dependencyList;
 	}
 
-	void Utility::findResourceDependenciesInternal(IReflectable& obj, bool recursive, Map<String, HResource>& dependencies)
+	void Utility::findResourceDependenciesInternal(IReflectable& obj, bool recursive, Map<String, ResourceDependency>& dependencies)
 	{
 		RTTITypeBase* rtti = obj.getRTTI();
 		rtti->onSerializationStarted(&obj);
@@ -42,14 +42,22 @@ namespace BansheeEngine
 						{
 							HResource resource = (HResource&)reflectableField->getArrayValue(&obj, j);
 							if (resource != nullptr)
-								dependencies[resource.getUUID()] = resource;
+							{
+								ResourceDependency& dependency = dependencies[resource.getUUID()];
+								dependency.resource = resource;
+								dependency.numReferences++;
+							}
 						}
 					}
 					else
 					{
 						HResource resource = (HResource&)reflectableField->getValue(&obj);
 						if (resource != nullptr)
-							dependencies[resource.getUUID()] = resource;
+						{
+							ResourceDependency& dependency = dependencies[resource.getUUID()];
+							dependency.resource = resource;
+							dependency.numReferences++;
+						}
 					}
 				}
 				else if (recursive)

TODO.txt

@@ -10,24 +10,11 @@ related to lambdas not capturing types as I expect them to. It always happens in
 TODO - Material waits to Shader to be loaded but doesn't wait for shader GpuPrograms or state objects.
  - What's the best way to ensure initialization is done when all these are loaded?
 
- Modify Resources so it loads resource dependencies and that actual resource gets loaded from the proper offset in file
-Test if stuff compiles and runs - commit
-Remove loading code from ResourceHandle
-Add loading code to Resources
-
-When resource loading starts synchonously read all dependencies (and recurse over their dependencies too)
- - Create a ResourceLoadGroup that contains all the non-loaded resources
- - Cue async loads for all resources
- - Perform sync loads for resources that don't support async
- - return an unloaded handle
-
-Each time an async load completes check if the resource is part of a resource load group
- - If all resources in the load group are done (or there was just one resource), resolve all of their handles at once
-   and trigger resource listener events.
-
-TODO
- - Pay special care in case a resource in resource load group gets unloaded. I should ignore it to avoid a deadlock that never finishes the resource load.
- - Allow for a situation if resource is part of multiple load groups.
+Implement resource unloading:
+ - Once unload is called, create a unique list of all dependant resources (just direct children)
+ - Track how many times is each resource referenced and how many actual references it has
+ - If ref count is 1 (just held by Resources manager) unload it
+ - Recurse unload over all child resources
 
 I can get mono errors by checking g_print calls in goutput.c
  - Calling thunks incorrectly can cause those weird errors with no real callstack