Browse Source

Fixing an issue with out-of-order core object syncing that was causing renderables to be invisible in very specific situations

BearishSun 10 years ago
parent
commit
caabd3721e

+ 1 - 1
BansheeCore/Include/BsCoreObject.h

@@ -224,7 +224,7 @@ namespace BansheeEngine
 		/**
 		 * @brief	Populates the provided array with all core objects that this core object depends upon.
 		 */
-		virtual void getCoreDependencies(Vector<SPtr<CoreObject>>& dependencies) { }
+		virtual void getCoreDependencies(FrameVector<SPtr<CoreObject>>& dependencies) { }
 
 	protected:
 		SPtr<CoreObjectCore> mCoreSpecific;

+ 4 - 3
BansheeCore/Include/BsCoreObjectManager.h

@@ -26,12 +26,13 @@ namespace BansheeEngine
 				:destinationObj(nullptr)
 			{ }
 
-			CoreStoredSyncObjData(CoreObjectCore* destObj, const CoreSyncData& syncData)
-				:destinationObj(destObj), syncData(syncData)
+			CoreStoredSyncObjData(CoreObjectCore* destObj, UINT64 internalId, const CoreSyncData& syncData)
+				:destinationObj(destObj), syncData(syncData), internalId(internalId)
 			{ }
 
 			CoreObjectCore* destinationObj;
 			CoreSyncData syncData;
+			UINT64 internalId;
 		};
 
 		/**
@@ -42,7 +43,7 @@ namespace BansheeEngine
 		struct CoreStoredSyncData
 		{
 			FrameAlloc* alloc = nullptr;
-			Map<UINT64, CoreStoredSyncObjData> entries;
+			Vector<CoreStoredSyncObjData> entries;
 		};
 
 	public:

+ 1 - 1
BansheeCore/Include/BsFont.h

@@ -92,7 +92,7 @@ namespace BansheeEngine
 		/**
 		 * @copydoc	CoreObject::getCoreDependencies
 		 */
-		void getCoreDependencies(Vector<SPtr<CoreObject>>& dependencies) override;
+		void getCoreDependencies(FrameVector<SPtr<CoreObject>>& dependencies) override;
 
 	private:
 		Map<UINT32, FontData> mFontDataPerSize;

+ 1 - 1
BansheeCore/Include/BsMaterial.h

@@ -690,7 +690,7 @@ namespace BansheeEngine
 		/**
 		 * @copydoc	CoreObject::getCoreDependencies
 		 */
-		void getCoreDependencies(Vector<SPtr<CoreObject>>& dependencies) override;
+		void getCoreDependencies(FrameVector<SPtr<CoreObject>>& dependencies) override;
 
 		/**
 		 * @copydoc	CoreObject::markCoreDirty

+ 1 - 1
BansheeCore/Include/BsPass.h

@@ -197,7 +197,7 @@ namespace BansheeEngine
 		/**
 		 * @copydoc	CoreObject::syncToCore
 		 */
-		void getCoreDependencies(Vector<SPtr<CoreObject>>& dependencies) override;
+		void getCoreDependencies(FrameVector<SPtr<CoreObject>>& dependencies) override;
 
 		/**
 		 * @brief	Creates a new empty pass but doesn't initialize it.

+ 1 - 1
BansheeCore/Include/BsShader.h

@@ -466,7 +466,7 @@ namespace BansheeEngine
 		/**
 		 * @copydoc	CoreObject::getCoreDependencies
 		 */
-		void getCoreDependencies(Vector<SPtr<CoreObject>>& dependencies) override;
+		void getCoreDependencies(FrameVector<SPtr<CoreObject>>& dependencies) override;
 
 		/**
 		 * @copydoc	CoreObject::createCore

+ 2 - 2
BansheeCore/Include/BsTechnique.h

@@ -107,12 +107,12 @@ namespace BansheeEngine
 		/**
 		 * @copydoc	CoreObject::createCore
 		 */
-		SPtr<CoreObjectCore> createCore() const;
+		SPtr<CoreObjectCore> createCore() const override;
 
 		/**
 		 * @copydoc	CoreObject::getCoreDependencies
 		 */
-		void getCoreDependencies(Vector<SPtr<CoreObject>>& dependencies);
+		void getCoreDependencies(FrameVector<SPtr<CoreObject>>& dependencies);
 
 		/**
 		 * @brief	Creates a new technique but doesn't initialize it.

+ 1 - 1
BansheeCore/Include/BsViewport.h

@@ -260,7 +260,7 @@ namespace BansheeEngine
 		/**
 		 * @copydoc	CoreObject::getCoreDependencies
 		 */
-		void getCoreDependencies(Vector<SPtr<CoreObject>>& dependencies) override;
+		void getCoreDependencies(FrameVector<SPtr<CoreObject>>& dependencies) override;
 
 		/**
 		 * @copydoc	CoreObject::createCore

+ 25 - 22
BansheeCore/Source/BsCoreObject.cpp

@@ -88,38 +88,41 @@ namespace BansheeEngine
 		};
 
 		FrameAlloc* allocator = gCoreThread().getFrameAlloc();
-
-		Vector<SPtr<CoreObject>> dependencies;
 		Vector<IndividualCoreSyncData> syncData;
 
-		UINT32 stackPos = 0;
-
-		dependencies.push_back(getThisPtr());
-		while (stackPos < dependencies.size())
+		bs_frame_mark();
 		{
-			SPtr<CoreObject> curObj = dependencies[stackPos];
-			stackPos++;
+			FrameVector<SPtr<CoreObject>> dependencies;
+			UINT32 stackPos = 0;
 
-			if (curObj->isCoreDirty())
+			dependencies.push_back(getThisPtr());
+			while (stackPos < dependencies.size())
 			{
-				SPtr<CoreObjectCore> destObj = curObj->getCore();
-				if (destObj == nullptr)
-					return;
+				SPtr<CoreObject> curObj = dependencies[stackPos];
+				stackPos++;
 
-				IndividualCoreSyncData data;
-				data.allocator = allocator;
-				data.destination = destObj;
-				data.syncData = syncToCore(data.allocator);
+				if (curObj->isCoreDirty())
+				{
+					SPtr<CoreObjectCore> destObj = curObj->getCore();
+					if (destObj == nullptr)
+						return;
 
-				syncData.push_back(data);
+					IndividualCoreSyncData data;
+					data.allocator = allocator;
+					data.destination = destObj;
+					data.syncData = syncToCore(data.allocator);
 
-				curObj->markCoreClean();
-			}
+					syncData.push_back(data);
 
-			// Note: I don't check for recursion. Possible infinite loop if two objects
-			// are dependent on one another.
-			curObj->getCoreDependencies(dependencies);
+					curObj->markCoreClean();
+				}
+
+				// Note: I don't check for recursion. Possible infinite loop if two objects
+				// are dependent on one another.
+				curObj->getCoreDependencies(dependencies);
+			}
 		}
+		bs_frame_clear();
 
 		std::function<void(const Vector<IndividualCoreSyncData>&)> callback =
 			[](const Vector<IndividualCoreSyncData>& data)

+ 26 - 28
BansheeCore/Source/BsCoreObjectManager.cpp

@@ -53,10 +53,11 @@ namespace BansheeEngine
 
 		for (auto& syncData : mCoreSyncData)
 		{
-			auto iterFind = syncData.entries.find(internalId);
+			auto iterFind = std::find_if(syncData.entries.begin(), syncData.entries.end(), 
+				[&](const CoreStoredSyncObjData& data) { return data.internalId == internalId; });
 			if (iterFind != syncData.entries.end())
 			{
-				UINT8* data = iterFind->second.syncData.getBuffer();
+				UINT8* data = iterFind->syncData.getBuffer();
 
 				if (data != nullptr && syncData.alloc != nullptr)
 					syncData.alloc->dealloc(data);
@@ -81,44 +82,42 @@ namespace BansheeEngine
 
 		syncData.alloc = allocator;
 
-		Vector<SPtr<CoreObject>> dependencies;
-		Vector<CoreObject*> todo;
-		UINT32 stackPos = 0;
-
+		bs_frame_mark();
+		
+		// Order in which objects are recursed in matters, ones with lower ID will have been created before
+		// ones with higher ones and should be updated first.
 		for (auto& objectData : mObjects)
 		{
-			CoreObject* object = objectData.second;
-			
-			todo.push_back(object);
-			while (stackPos < todo.size())
+			std::function<void(CoreObject*)> syncObject = [&](CoreObject* curObj)
 			{
-				CoreObject* curObj = todo[stackPos];
-				stackPos++;
+				// Sync dependencies before dependants
+				// Note: I don't check for recursion. Possible infinite loop if two objects
+				// are dependent on one another.
+				
+				FrameVector<SPtr<CoreObject>> dependencies;
+				curObj->getCoreDependencies(dependencies);
+
+				for (auto& dependency : dependencies)
+					syncObject(dependency.get());
 
 				if (curObj->isCoreDirty())
 				{
 					SPtr<CoreObjectCore> objectCore = curObj->getCore();
 					if (objectCore == nullptr)
-						continue;
+						return;
 
 					CoreSyncData objSyncData = curObj->syncToCore(allocator);
 					curObj->markCoreClean();
 
-					syncData.entries[curObj->getInternalID()] = CoreStoredSyncObjData(objectCore.get(), objSyncData);
+					syncData.entries.push_back(CoreStoredSyncObjData(objectCore.get(),
+						curObj->getInternalID(), objSyncData));
 				}
+			};
 
-				// Note: I don't check for recursion. Possible infinite loop if two objects
-				// are dependent on one another.
-				curObj->getCoreDependencies(dependencies);
-				for (auto& dependency : dependencies)
-					todo.push_back(dependency.get());
-
-				dependencies.clear();
-			}
-
-			todo.clear();
-			stackPos = 0;
+			CoreObject* object = objectData.second;
+			syncObject(object);
 		}
+		bs_frame_clear();
 	}
 
 	void CoreObjectManager::syncUpload()
@@ -130,10 +129,9 @@ namespace BansheeEngine
 
 		CoreStoredSyncData& syncData = mCoreSyncData.front();
 
-		// Traverse in reverse to sync dependencies before dependants
-		for (auto& riter = syncData.entries.rbegin(); riter != syncData.entries.rend(); ++riter)
+		for (auto& iter = syncData.entries.begin(); iter != syncData.entries.end(); ++iter)
 		{
-			const CoreStoredSyncObjData& objSyncData = riter->second;
+			const CoreStoredSyncObjData& objSyncData = *iter;
 			objSyncData.destinationObj->syncToCore(objSyncData.syncData);
 
 			UINT8* data = objSyncData.syncData.getBuffer();

+ 1 - 1
BansheeCore/Source/BsFont.cpp

@@ -95,7 +95,7 @@ namespace BansheeEngine
 		}
 	}
 
-	void Font::getCoreDependencies(Vector<SPtr<CoreObject>>& dependencies)
+	void Font::getCoreDependencies(FrameVector<SPtr<CoreObject>>& dependencies)
 	{
 		for (auto& fontDataEntry : mFontDataPerSize)
 		{

+ 1 - 1
BansheeCore/Source/BsMaterial.cpp

@@ -1167,7 +1167,7 @@ namespace BansheeEngine
 		return CoreSyncData(buffer, size);
 	}
 
-	void Material::getCoreDependencies(Vector<SPtr<CoreObject>>& dependencies)
+	void Material::getCoreDependencies(FrameVector<SPtr<CoreObject>>& dependencies)
 	{
 		if (mShader.isLoaded())
 			dependencies.push_back(mShader.getInternalPtr());

+ 1 - 1
BansheeCore/Source/BsPass.cpp

@@ -124,7 +124,7 @@ namespace BansheeEngine
 		return CoreSyncData(data, size);
 	}
 
-	void Pass::getCoreDependencies(Vector<SPtr<CoreObject>>& dependencies)
+	void Pass::getCoreDependencies(FrameVector<SPtr<CoreObject>>& dependencies)
 	{
 		if (mData.blendState != nullptr)
 			dependencies.push_back(mData.blendState);

+ 1 - 1
BansheeCore/Source/BsShader.cpp

@@ -369,7 +369,7 @@ namespace BansheeEngine
 		return output;
 	}
 
-	void Shader::getCoreDependencies(Vector<SPtr<CoreObject>>& dependencies)
+	void Shader::getCoreDependencies(FrameVector<SPtr<CoreObject>>& dependencies)
 	{
 		for (auto& technique : mTechniques)
 			dependencies.push_back(technique);

+ 1 - 1
BansheeCore/Source/BsTechnique.cpp

@@ -90,7 +90,7 @@ namespace BansheeEngine
 		return techniquePtr;
 	}
 
-	void Technique::getCoreDependencies(Vector<SPtr<CoreObject>>& dependencies)
+	void Technique::getCoreDependencies(FrameVector<SPtr<CoreObject>>& dependencies)
 	{
 		for (auto& pass : mPasses)
 			dependencies.push_back(pass);

+ 1 - 1
BansheeCore/Source/BsViewport.cpp

@@ -200,7 +200,7 @@ namespace BansheeEngine
 		return CoreSyncData(buffer, size);
 	}
 
-	void Viewport::getCoreDependencies(Vector<SPtr<CoreObject>>& dependencies)
+	void Viewport::getCoreDependencies(FrameVector<SPtr<CoreObject>>& dependencies)
 	{
 		if (mTarget != nullptr)
 			dependencies.push_back(mTarget);

+ 1 - 0
BansheeEditor/Include/BsEditorApplication.h

@@ -181,6 +181,7 @@ namespace BansheeEngine
 		HMaterial mTestMaterial;
 		HTexture mTestTexRef;
 		HMesh mDbgMeshRef;
+		HSceneObject mTestModelGO;
 	};
 
 	/**

+ 5 - 4
BansheeEditor/Source/BsEditorApplication.cpp

@@ -46,6 +46,7 @@
 #include "BsMesh.h"
 #include "BsMath.h"
 #include "BsDebug.h"
+#include "BsInput.h"
 
 namespace BansheeEngine
 {
@@ -153,8 +154,8 @@ namespace BansheeEngine
 
 		RenderAPICore* renderAPI = RenderAPICore::instancePtr();
 
-		HSceneObject testModelGO = SceneObject::create("TestMesh");
-		HRenderable testRenderable = testModelGO->addComponent<CRenderable>();
+		mTestModelGO = SceneObject::create("TestMesh");
+		HRenderable testRenderable = mTestModelGO->addComponent<CRenderable>();
 
 		Path testShaderLoc = RUNTIME_DATA_PATH + L"Test.bsl";;
 
@@ -196,8 +197,8 @@ namespace BansheeEngine
 		testRenderable->setMesh(mDbgMeshRef);
 		testRenderable->setMaterial(0, mTestMaterial);
 
-		HSceneObject clone = testModelGO->clone();
-		testModelGO->destroy();
+		HSceneObject clone = mTestModelGO->clone();
+		mTestModelGO->destroy();
 
 		/************************************************************************/
 		/* 							END DEBUG CODE                      		*/

+ 1 - 1
BansheeEngine/Include/BsCamera.h

@@ -607,7 +607,7 @@ namespace BansheeEngine
 		/**
 		 * @copydoc	CoreObject::getCoreDependencies
 		 */
-		void getCoreDependencies(Vector<SPtr<CoreObject>>& dependencies) override;
+		void getCoreDependencies(FrameVector<SPtr<CoreObject>>& dependencies) override;
 
 		/**
 		 * @brief	Creates a new camera without initializing it.

+ 1 - 1
BansheeEngine/Include/BsRenderable.h

@@ -243,7 +243,7 @@ namespace BansheeEngine
 		/**
 		 * @copydoc	CoreObject::getCoreDependencies
 		 */
-		void getCoreDependencies(Vector<SPtr<CoreObject>>& dependencies) override;
+		void getCoreDependencies(FrameVector<SPtr<CoreObject>>& dependencies) override;
 
 		/**
 		 * @copydoc	IResourceListener::getListenerResources

+ 1 - 1
BansheeEngine/Include/BsSpriteTexture.h

@@ -72,7 +72,7 @@ namespace BansheeEngine
 		/**
 		 * @copydoc	CoreObject::getCoreDependencies
 		 */
-		void getCoreDependencies(Vector<SPtr<CoreObject>>& dependencies) override;
+		void getCoreDependencies(FrameVector<SPtr<CoreObject>>& dependencies) override;
 
 		HTexture mAtlasTexture;
 		Vector2 mUVOffset;

+ 1 - 1
BansheeEngine/Source/BsCamera.cpp

@@ -818,7 +818,7 @@ namespace BansheeEngine
 		return CoreSyncData(buffer, size);
 	}
 
-	void Camera::getCoreDependencies(Vector<SPtr<CoreObject>>& dependencies)
+	void Camera::getCoreDependencies(FrameVector<SPtr<CoreObject>>& dependencies)
 	{
 		dependencies.push_back(mViewport);
 	}

+ 1 - 1
BansheeEngine/Source/BsRenderable.cpp

@@ -281,7 +281,7 @@ namespace BansheeEngine
 		return CoreSyncData(data, size);
 	}
 
-	void Renderable::getCoreDependencies(Vector<SPtr<CoreObject>>& dependencies)
+	void Renderable::getCoreDependencies(FrameVector<SPtr<CoreObject>>& dependencies)
 	{
 		if (mMesh.isLoaded())
 			dependencies.push_back(mMesh.getInternalPtr());

+ 1 - 1
BansheeEngine/Source/BsSpriteTexture.cpp

@@ -49,7 +49,7 @@ namespace BansheeEngine
 			dependencies.push_back(mAtlasTexture);
 	}
 
-	void SpriteTexture::getCoreDependencies(Vector<SPtr<CoreObject>>& dependencies)
+	void SpriteTexture::getCoreDependencies(FrameVector<SPtr<CoreObject>>& dependencies)
 	{
 		if (mAtlasTexture.isLoaded())
 			dependencies.push_back(mAtlasTexture.getInternalPtr());

+ 1 - 2
TODO.txt

@@ -53,8 +53,7 @@ Code quality improvements:
 Polish
 
 Ribek use:
- - When performing core sync it doesn't handle it in hierarchical order. E.g. if a camera depends on viewport, then it should
-   sync the viewport first, then camera
+ - Renderables don't render - seems to be caused by core sync
  - Opening complex array entries causes inspector to get screwed up
  - Hook up color picker to guicolor field
  - Camera, Renderable, Material, Texture inspector