Explorar o código

Fixed a nasty threading issue where objects were being destroyed in the wrong queue so they would get destroyed while they could still be used in another queue

Marko Pintera %!s(int64=12) %!d(string=hai) anos
pai
achega
3129cf8b5a

+ 1 - 0
BansheeEditor/Include/BsDockManager.h

@@ -102,6 +102,7 @@ namespace BansheeEditor
 
 		CM::RenderWindow* mParentWindow;
 		DockContainer mRootContainer;
+		CM::RectI mArea;
 
 		CM::HMesh mDropOverlayMesh;
 		CM::HMaterial mDropOverlayMat;

+ 3 - 0
BansheeEditor/Source/BsDockManager.cpp

@@ -466,6 +466,7 @@ namespace BansheeEditor
 	void DockManager::setArea(INT32 x, INT32 y, UINT32 width, UINT32 height)
 	{
 		mRootContainer.setArea(x, y, width, height);
+		mArea = RectI(x, y, width, height);
 
 		updateDropOverlay(x, y, width, height);
 	}
@@ -678,6 +679,8 @@ namespace BansheeEditor
 			if(widget->_getParent() == nullptr)
 				widget->close();
 		}
+
+		setArea(mArea.x, mArea.y, mArea.width, mArea.height);
 	}
 
 	void DockManager::updateClippedBounds()

+ 3 - 0
CamelotCore/Include/CmCoreObject.h

@@ -154,6 +154,9 @@ o		 *
 		CM_STATIC_THREAD_SYNCHRONISER(mCoreGpuObjectLoadedCondition)
 		CM_STATIC_MUTEX(mCoreGpuObjectLoadedMutex)
 
+		static void queueInitializeGpuCommand(std::shared_ptr<CoreObject>& obj);
+		static void queueDestroyGpuCommand(std::shared_ptr<CoreObject>& obj);
+
 		static void executeGpuCommand(std::shared_ptr<CoreObject>& obj, boost::function<void()> func);
 		static void executeReturnGpuCommand(std::shared_ptr<CoreObject>& obj, boost::function<void(AsyncOp&)> func, AsyncOp& op); 
 	};

+ 28 - 4
CamelotCore/Include/CmCoreThread.h

@@ -9,6 +9,19 @@ namespace CamelotFramework
 	/**
 	 * @brief	Manager for the core thread. Takes care of starting, running, queuing commands
 	 * 			and shutting down the core thread.
+	 * 			
+	 * @note	How threading works:
+	 * 			 - This class contains a queue which is filled by commands from other threads via queueCommand and queueReturnCommand  
+	 * 			 - Commands are executed on the core thread as soon as they are queued (if core thread is not busy with previous commands)  
+	 * 			 - Core thread accessors are helpers for queuing commands. They serve two purposes:  
+	 * 				- They contain helper methods for various common Core thread commands.
+	 * 				- They perform better than queuing each command directly using queueCommand or queueReturnCommand    
+	 * 			 - Accessors contain a command queue of their own, and queuing commands in them will not automatically start executing the commands  
+	 * 			   like with queueCommand or queueReturnCommand. Instead you must manually call "submitAccessors" when you are ready to send their
+	 * 			   commands to the core thread.
+	 * 			 - Synced accessor is a special type of accessor which may be accessed from any thread. Its commands are always executed after all other  
+	 * 			   non-synced accessors. It is primarily useful when multiple threads are managing the same resource and you must ensure proper order of operations.
+	 * 			   You should use normal accessors whenever possible as synced accessors involve potentially slow synchronization operations.
 	 */
 	class CoreThread : public Module<CoreThread>
 	{
@@ -30,9 +43,10 @@ public:
 
 	/**
 		* @brief	Creates or retrieves an accessor that you can use for executing commands on the core thread from 
-		* 			a non-core thread. You can have as many of these as you wish, the only limitation
-		* 			is that you do not use a single instance on more than one thread. Each thread
-		* 			requires its own accessor. The accessor will be bound to the thread you call this method on.
+		* 			a non-core thread. The accessor will be bound to the thread you call this method on.
+		* 			
+		* @note		Accessors contain their own command queue and their commands will only start to get executed once that queue is submitted
+		* 			to the core thread via "submitAccessors" method.
 		*/
 	CM_EXPORT CoreAccessorPtr getAccessor();
 
@@ -41,9 +55,19 @@ public:
 	* 			a non-core thread. There is only one synchronized accessor and you may access it from any thread you wish.
 	* 			Note however that it is much more efficient to create a separate non-synchronized accessor using
 	* 			"createCoreAccessor" for each thread you will be using it on.
-		*/
+	* 			
+	* @note		Accessors contain their own command queue and their commands will only start to get executed once that queue is submitted
+	* 			to the core thread via "submitAccessors" method.
+	* 			
+	*			Synced accessor commands are sent after all non-synced accessor commands are sent.
+	*/
 	CM_EXPORT SyncedCoreAccessor& getSyncedAccessor();
 
+	/**
+	 * @brief	Queues all the accessor commands and starts executing them on the core thread.
+	 */
+	CM_EXPORT void submitAccessors(bool blockUntilComplete = false);
+
 	/**
 		* @brief	Queues a new command that will be added to the global command queue. You are allowed to call this from any thread,
 		* 			however be aware that it involves possibly slow synchronization primitives, so limit your usage.

+ 2 - 0
CamelotCore/Include/CmRenderSystem.h

@@ -406,6 +406,8 @@ namespace CamelotFramework
 		 */
 		RenderWindowPtr initialize(const RENDER_WINDOW_DESC& primaryWindowDesc);
 		virtual void initialize_internal(AsyncOp& asyncOp);
+
+		void destroy();
 		virtual void destroy_internal();
 
 		/// Internal method used to set the underlying clip planes when needed

+ 4 - 0
CamelotCore/Include/CmRenderSystemManager.h

@@ -11,11 +11,15 @@ namespace CamelotFramework
 	class CM_EXPORT RenderSystemManager : public Module<RenderSystemManager>
 	{
 	public:
+		RenderSystemManager();
+		~RenderSystemManager();
+
 		RenderWindowPtr initialize(const String& name, RENDER_WINDOW_DESC& primaryWindowDesc);
 
 		void registerRenderSystemFactory(RenderSystemFactoryPtr factory);
 	private:
 		Vector<RenderSystemFactoryPtr>::type mAvailableFactories;
+		bool mRenderSystemInitialized;
 	};
 }
 

+ 2 - 3
CamelotCore/Source/CmApplication.cpp

@@ -131,8 +131,7 @@ namespace CamelotFramework
 			}
 
 			gCoreThread().queueCommand(&Platform::coreUpdate);
-			PROFILE_CALL(mPrimaryCoreAccessor->submitToCoreThread(), "CommandSubmit");
-			PROFILE_CALL(mPrimarySyncedCoreAccessor->submitToCoreThread(), "SyncCommandSubmit");
+			gCoreThread().submitAccessors();
 			gCoreThread().queueCommand(boost::bind(&Application::endCoreProfiling, this));
 			gCoreThread().queueCommand(boost::bind(&Application::frameRenderingFinishedCallback, this));
 
@@ -181,7 +180,7 @@ namespace CamelotFramework
 		SceneManager::shutDown();
 
 		RendererManager::shutDown();
-		RenderSystem::shutDown();
+		RenderSystemManager::shutDown();
 		CoreThread::shutDown();
 		Input::shutDown();
 

+ 25 - 5
CamelotCore/Source/CmCoreObject.cpp

@@ -42,7 +42,10 @@ namespace CamelotFramework
 		{
 			setScheduledToBeDeleted(true);
 
-			queueGpuCommand(mThis.lock(), boost::bind(&CoreObject::destroy_internal, this));
+			if(CM_THREAD_CURRENT_ID == CoreThread::instance().getCoreThreadId())
+				mThis.lock()->destroy_internal();
+			else
+				queueDestroyGpuCommand(mThis.lock());
 		}
 		else
 		{
@@ -71,7 +74,10 @@ namespace CamelotFramework
 		{
 			setScheduledToBeInitialized(true);
 
-			queueGpuCommand(mThis.lock(), boost::bind(&CoreObject::initialize_internal, this));
+			if(CM_THREAD_CURRENT_ID == CoreThread::instance().getCoreThreadId())
+				mThis.lock()->initialize_internal();
+			else
+				queueInitializeGpuCommand(mThis.lock());
 		}
 		else
 		{
@@ -158,14 +164,28 @@ namespace CamelotFramework
 		// reference to the obj (saved in the bound function).
 		// We could have called the function directly using "this" pointer but then we couldn't have used a shared_ptr for the object,
 		// in which case there is a possibility that the object would be released and deleted while still being in the command queue.
-		CoreThread::instance().queueCommand(boost::bind(&CoreObject::executeGpuCommand, obj, func));
-
+		CoreThread::instance().getAccessor()->queueCommand(boost::bind(&CoreObject::executeGpuCommand, obj, func));
 	}
 
 	AsyncOp CoreObject::queueReturnGpuCommand(std::shared_ptr<CoreObject>& obj, boost::function<void(AsyncOp&)> func)
 	{
 		// See queueGpuCommand
-		return CoreThread::instance().queueReturnCommand(boost::bind(&CoreObject::executeReturnGpuCommand, obj, func, _1));
+		return CoreThread::instance().getAccessor()->queueReturnCommand(boost::bind(&CoreObject::executeReturnGpuCommand, obj, func, _1));
+	}
+
+	void CoreObject::queueInitializeGpuCommand(std::shared_ptr<CoreObject>& obj)
+	{
+		boost::function<void()> func = boost::bind(&CoreObject::initialize_internal, obj.get());
+
+		CoreThread::instance().queueCommand(boost::bind(&CoreObject::executeGpuCommand, obj, func));
+	}
+
+	void CoreObject::queueDestroyGpuCommand(std::shared_ptr<CoreObject>& obj)
+	{
+		boost::function<void()> func = boost::bind(&CoreObject::destroy_internal, obj.get());
+
+		CoreThread::instance().getAccessor()->queueCommand(
+			boost::bind(&CoreObject::executeGpuCommand, obj, func));
 	}
 
 	void CoreObject::executeGpuCommand(std::shared_ptr<CoreObject>& obj, boost::function<void()> func)

+ 17 - 0
CamelotCore/Source/CmCoreThread.cpp

@@ -30,6 +30,7 @@ namespace CamelotFramework
 
 		{
 			CM_LOCK_MUTEX(mAccessorMutex);
+
 			for(auto& accessor : mAccessors)
 			{
 				cm_delete(accessor);
@@ -162,6 +163,22 @@ namespace CamelotFramework
 		return *mSyncedCoreAccessor;
 	}
 
+	void CoreThread::submitAccessors(bool blockUntilComplete)
+	{
+		Vector<AccessorContainer*>::type accessorCopies;
+
+		{
+			CM_LOCK_MUTEX(mAccessorMutex);
+
+			accessorCopies = mAccessors;
+		}
+
+		for(auto& accessor : accessorCopies)
+			accessor->accessor->submitToCoreThread(blockUntilComplete);
+
+		mSyncedCoreAccessor->submitToCoreThread(blockUntilComplete);
+	}
+
 	AsyncOp CoreThread::queueReturnCommand(boost::function<void(AsyncOp&)> commandCallback, bool blockUntilComplete)
 	{
 		AsyncOp op;

+ 7 - 0
CamelotCore/Source/CmRenderSystem.cpp

@@ -75,6 +75,7 @@ namespace CamelotFramework {
 		mPrimaryWindowDesc = primaryWindowDesc;
 
 		AsyncOp op = gCoreThread().queueReturnCommand(boost::bind(&RenderSystem::initialize_internal, this, _1), true);
+
 		return op.getReturnValue<RenderWindowPtr>();
 	}
 
@@ -87,6 +88,12 @@ namespace CamelotFramework {
 		mFragmentProgramBound = false;
 	}
 
+	void RenderSystem::destroy()
+	{
+		gCoreThread().getAccessor()->queueCommand(boost::bind(&RenderSystem::destroy_internal, this));
+		gCoreThread().submitAccessors(true);
+	}
+
 	void RenderSystem::destroy_internal()
 	{
 		mActiveRenderTarget = nullptr;

+ 17 - 0
CamelotCore/Source/CmRenderSystemManager.cpp

@@ -6,8 +6,24 @@
 
 namespace CamelotFramework
 {
+	RenderSystemManager::RenderSystemManager()
+		:mRenderSystemInitialized(false)
+	{ }
+
+	RenderSystemManager::~RenderSystemManager()
+	{
+		if(mRenderSystemInitialized)
+		{
+			RenderSystem::instance().destroy();
+			RenderSystem::shutDown();
+		}
+	}
+
 	RenderWindowPtr RenderSystemManager::initialize(const String& pluginFilename, RENDER_WINDOW_DESC& primaryWindowDesc)
 	{
+		if(mRenderSystemInitialized)
+			return nullptr;
+
 		DynLib* loadedLibrary = gDynLibManager().load(pluginFilename);
 		String name = "";
 
@@ -24,6 +40,7 @@ namespace CamelotFramework
 			if((*iter)->name() == name)
 			{
 				(*iter)->create();		
+				mRenderSystemInitialized = true;
 				return RenderSystem::instance().initialize(primaryWindowDesc);
 			}
 		}

+ 15 - 0
CamelotCore/Source/CmRenderWindowManager.cpp

@@ -37,6 +37,11 @@ namespace CamelotFramework
 				CM_EXCEPT(InternalErrorException, "Trying to destroy a window that is not in the created windows list.");
 
 			mCreatedWindows.erase(iterFind);
+
+			auto iterFind2 = std::find(begin(mMovedOrResizedWindows), end(mMovedOrResizedWindows), window);
+
+			if(iterFind2 != mMovedOrResizedWindows.end())
+				mMovedOrResizedWindows.erase(iterFind2);
 		}
 	}
 
@@ -58,6 +63,16 @@ namespace CamelotFramework
 
 	void RenderWindowManager::windowMovedOrResized(RenderWindow* window)
 	{
+		bool isValidWindow = false;
+		{
+			CM_LOCK_MUTEX(mWindowMutex);
+
+			isValidWindow = std::find(begin(mCreatedWindows), end(mCreatedWindows), window) != mCreatedWindows.end();
+		}
+
+		if(!isValidWindow)
+			return;
+
 		window->_windowMovedOrResized();
 
 		CM_LOCK_MUTEX(mWindowMutex);

+ 1 - 2
CamelotD3D11RenderSystem/Source/CmD3D11RenderSystem.cpp

@@ -40,8 +40,7 @@ namespace CamelotFramework
 
 	D3D11RenderSystem::~D3D11RenderSystem()
 	{
-		// This needs to be called from the child class, since destroy_internal is virtual
-		gCoreThread().queueCommand(boost::bind(&D3D11RenderSystem::destroy_internal, this), true);
+
 	}
 
 	const String& D3D11RenderSystem::getName() const

+ 2 - 8
CamelotD3D9Renderer/Source/CmD3D9RenderSystem.cpp

@@ -77,11 +77,6 @@ namespace CamelotFramework
 {
 	D3D9RenderSystem* D3D9RenderSystem::msD3D9RenderSystem = NULL;
 
-	/************************************************************************/
-	/* 								PUBLIC INTERFACE                      	*/
-	/************************************************************************/
-
-	//---------------------------------------------------------------------
 	D3D9RenderSystem::D3D9RenderSystem( HINSTANCE hInstance )
 		: mTexStageDesc(nullptr)
 		, mNumTexStages(0)
@@ -109,11 +104,10 @@ namespace CamelotFramework
 		mScissorRect.top = 0;
 		mScissorRect.bottom = 720;
 	}
-	//---------------------------------------------------------------------
+
 	D3D9RenderSystem::~D3D9RenderSystem()
 	{
-		// This needs to be called from the child class, since destroy_internal is virtual
-		gCoreThread().queueCommand(boost::bind(&D3D9RenderSystem::destroy_internal, this), true);
+
 	}
 
 	const String& D3D9RenderSystem::getName() const

+ 1 - 2
CamelotGLRenderer/Source/CmGLRenderSystem.cpp

@@ -116,8 +116,7 @@ namespace CamelotFramework
 
 	GLRenderSystem::~GLRenderSystem()
 	{
-		// This needs to be called from the child class, since destroy_internal is virtual
-		gCoreThread().queueCommand(boost::bind(&GLRenderSystem::destroy_internal, this), true);
+
 	}
 
 	const String& GLRenderSystem::getName(void) const

+ 6 - 9
EditorWindowDock.txt

@@ -2,26 +2,23 @@ TODO:
  - Closing all docked widgets causes an exception
  - Test out minimum dock container size and maybe increase it a bit
 
-Fix up DockManager::setLayout and ensure it opens widgets properly
-Check SVN modifications and re-evaluate my changes
-
 I should consider adding multiple C# EditorWindow types (via an enum)
  - Normal dockable EditorWindow, a tool window (unockable, no tabs), a modal window or maybe others
    - Dockable EditorWindow would internally be EditorWidgetBase as it is now and I would create different types for other windows
    - (Other windows should not need layout)
 
-I should try to get rid of two-step EditorWidget initialization
- - Both C++ and C# scripts require additional calls to initialize() after construction
- - SOLUTION: Update GUI so it can accept a "null" parent widget. Then it just needs to reintialize on _chageParentWidget which it does anyway
 
 Polish TOOD:
  - Change cursor icon when window is dragged
  - Prevent docking if available size is less than 20 pixels, otherwise there might be some weirdness
 
 ------------------------
-Layout saving/loading:
-TODO:
- Register a bunch of duplicate widgets and open and dock them
+
+Docked widgets are not shown anymore
+
+LATER:
+ get rid of CmApplication gMainCA and similar methods. Users may access CoreThread singleton directly now
+ get rid of CoreAccessor::submitToCoreThread and instead leave that functionally only available to CoreThread
 
 ------------------------
 

+ 2 - 0
SBansheeEditor/Source/BsScriptEditorWindow.cpp

@@ -89,6 +89,8 @@ namespace BansheeEditor
 					const String& className = curClass->getFullName();
 					EditorWidgetManager::instance().registerWidget(className, 
 						std::bind(&ScriptEditorWindow::openEditorWidgetCallback, curClass->getNamespace(), curClass->getTypeName(), std::placeholders::_1));
+
+					EditorWidgetManager::instance().open(className);
 				}
 			}
 		}