Properly clean up C# GUIPanels when window is closed or when assembly refresh occurs

Fixed Gizmo and Handle manager so they don't keep a handle to non-core RenderTarget

BansheeEditor/Include/BsGizmoManager.h

@@ -218,12 +218,12 @@ namespace BansheeEngine
 		void renderGizmos(Matrix4 viewMatrix, Matrix4 projMatrix, SPtr<MeshCoreBase> mesh, GizmoManager::GizmoMaterial material);
 		void renderIconGizmos(Rect2I screenArea, SPtr<MeshCoreBase> mesh, GizmoManager::IconRenderDataVecPtr renderData, bool usePickingMaterial);
 
-		void updateData(const RenderTargetPtr& rt, const SPtr<MeshCoreBase>& solidMesh, const SPtr<MeshCoreBase>& wireMesh,
+		void updateData(const SPtr<RenderTargetCore>& rt, const SPtr<MeshCoreBase>& solidMesh, const SPtr<MeshCoreBase>& wireMesh,
 			const SPtr<MeshCoreBase>& iconMesh, const GizmoManager::IconRenderDataVecPtr& iconRenderData);
 
 		static const float PICKING_ALPHA_CUTOFF;
 
-		RenderTargetPtr mSceneRenderTarget;
+		SPtr<RenderTargetCore> mSceneRenderTarget;
 
 		SPtr<MeshCoreBase> mSolidMesh;
 		SPtr<MeshCoreBase> mWireMesh;

BansheeEditor/Include/BsHandleDrawManager.h

@@ -84,10 +84,10 @@ namespace BansheeEngine
 
 		void initialize(const SPtr<MaterialCore>& wireMat, const SPtr<MaterialCore>& solidMat);
 
-		void updateData(const RenderTargetPtr& rt, const Vector<MeshData>& meshes);
+		void updateData(const SPtr<RenderTargetCore>& rt, const Vector<MeshData>& meshes);
 		void render(const CameraHandlerCore& camera);
 
-		RenderTargetPtr mSceneRenderTarget;
+		SPtr<RenderTargetCore> mSceneRenderTarget;
 		Vector<MeshData> mMeshes;
 
 		// Immutable

BansheeEditor/Source/BsGizmoManager.cpp

@@ -264,7 +264,8 @@ namespace BansheeEngine
 		mIconMesh = buildIconMesh(camera, mIconData, false, iconRenderData);
 		SPtr<MeshCoreBase> iconMesh = mIconMesh->getCore();
 
-		gCoreAccessor().queueCommand(std::bind(&GizmoManagerCore::updateData, mCore, rt, solidMesh, wireMesh, iconMesh, iconRenderData));
+		gCoreAccessor().queueCommand(std::bind(&GizmoManagerCore::updateData, mCore, rt->getCore(), 
+			solidMesh, wireMesh, iconMesh, iconRenderData));
 	}
 
 	void GizmoManager::renderForPicking(const CameraHandlerPtr& camera, std::function<Color(UINT32)> idxToColorCallback)
@@ -703,7 +704,7 @@ namespace BansheeEngine
 		activeRenderer->onCorePostRenderViewport.connect(std::bind(&GizmoManagerCore::render, this, _1));
 	}
 
-	void GizmoManagerCore::updateData(const RenderTargetPtr& rt, const SPtr<MeshCoreBase>& solidMesh, const SPtr<MeshCoreBase>& wireMesh,
+	void GizmoManagerCore::updateData(const SPtr<RenderTargetCore>& rt, const SPtr<MeshCoreBase>& solidMesh, const SPtr<MeshCoreBase>& wireMesh,
 		const SPtr<MeshCoreBase>& iconMesh, const GizmoManager::IconRenderDataVecPtr& iconRenderData)
 	{
 		mSceneRenderTarget = rt;
@@ -718,13 +719,11 @@ namespace BansheeEngine
 		if (mSceneRenderTarget == nullptr)
 			return;
 
-		SPtr<RenderTargetCore> sceneRenderTarget = mSceneRenderTarget->getCore();
-
-		if (camera.getViewport()->getTarget() != sceneRenderTarget)
+		if (camera.getViewport()->getTarget() != mSceneRenderTarget)
 			return;
 
-		float width = (float)sceneRenderTarget->getProperties().getWidth();
-		float height = (float)sceneRenderTarget->getProperties().getHeight();
+		float width = (float)mSceneRenderTarget->getProperties().getWidth();
+		float height = (float)mSceneRenderTarget->getProperties().getHeight();
 
 		Rect2 normArea = camera.getViewport()->getNormArea();
 

BansheeEditor/Source/BsHandleDrawManager.cpp

@@ -178,7 +178,7 @@ namespace BansheeEngine
 		RenderTargetPtr sceneRenderTarget = camera->getViewport()->getTarget();
 
 		gCoreAccessor().queueCommand(std::bind(&HandleDrawManagerCore::updateData, mCore, 
-			sceneRenderTarget, proxyData));
+			sceneRenderTarget->getCore(), proxyData));
 
 		mDrawHelper->clear();
 	}
@@ -204,7 +204,7 @@ namespace BansheeEngine
 		activeRenderer->onCorePostRenderViewport.connect(std::bind(&HandleDrawManagerCore::render, this, _1));
 	}
 
-	void HandleDrawManagerCore::updateData(const RenderTargetPtr& rt, const Vector<MeshData>& meshes)
+	void HandleDrawManagerCore::updateData(const SPtr<RenderTargetCore>& rt, const Vector<MeshData>& meshes)
 	{
 		mSceneRenderTarget = rt;
 		mMeshes = meshes;
@@ -217,12 +217,11 @@ namespace BansheeEngine
 		if (mSceneRenderTarget == nullptr)
 			return;
 
-		SPtr<RenderTargetCore> sceneRenderTarget = mSceneRenderTarget->getCore();
-		if (camera.getViewport()->getTarget() != sceneRenderTarget)
+		if (camera.getViewport()->getTarget() != mSceneRenderTarget)
 			return;
 
-		float width = (float)sceneRenderTarget->getProperties().getWidth();
-		float height = (float)sceneRenderTarget->getProperties().getHeight();
+		float width = (float)mSceneRenderTarget->getProperties().getWidth();
+		float height = (float)mSceneRenderTarget->getProperties().getHeight();
 
 		Rect2 normArea = camera.getViewport()->getNormArea();
 

MBansheeEditor/EditorWindow.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Collections.Generic;
 using System.Runtime.CompilerServices;
 using BansheeEngine;
 
@@ -11,6 +12,7 @@ namespace BansheeEditor
         public bool HasFocus { get { return Internal_HasFocus(mCachedPtr); } }
 
         protected GUIPanel GUI;
+        private List<GUIPanel> panels = new List<GUIPanel>();
 
         public static T OpenWindow<T>() where T : EditorWindow
         {
@@ -36,6 +38,14 @@ namespace BansheeEditor
             GUI = CreatePanel(0, 0, Width, Height);
         }
 
+        private void OnDestroyInternal()
+        {
+            GUIPanel[] panelsCopy = panels.ToArray();
+
+            for (int i = 0; i < panelsCopy.Length; i++)
+                DestroyPanel(panelsCopy[i]);
+        }
+
         protected virtual void WindowResized(int width, int height)
         {
             GUI.SetArea(0, 0, width, height);
@@ -53,15 +63,26 @@ namespace BansheeEditor
             newPanel.Initialize();
             newPanel.SetArea(x, y, width, height);
 
+            panels.Add(newPanel);
             return newPanel;
         }
 
+        internal void DestroyPanel(GUIPanel panel)
+        {
+            panel.Destroy();
+            panels.Remove(panel);
+            Internal_DestroyGUIPanel(mCachedPtr, panel);
+        }
+
         [MethodImpl(MethodImplOptions.InternalCall)]
         private static extern EditorWindow Internal_CreateOrGetInstance(string ns, string typeName);
 
         [MethodImpl(MethodImplOptions.InternalCall)]
         private static extern void Internal_InitializeGUIPanel(IntPtr nativeInstance, GUIPanel panel);
 
+        [MethodImpl(MethodImplOptions.InternalCall)]
+        private static extern void Internal_DestroyGUIPanel(IntPtr nativeInstance, GUIPanel panel);
+
         [MethodImpl(MethodImplOptions.InternalCall)]
         private static extern int Internal_GetWidth(IntPtr nativeInstance);
 

MBansheeEditor/Inspector/Inspector.cs

@@ -12,11 +12,14 @@ namespace BansheeEditor
         protected GUILayoutY layout;
         protected object referencedObject;
 
-        internal void Initialize(GUIPanel gui, object instance)
+        private InspectorWindow parentWindow;
+
+        internal void Initialize(InspectorWindow parentWindow, GUIPanel gui, object instance)
         {
             GUI = gui;
             layout = gui.layout.AddLayoutY();
             referencedObject = instance;
+            this.parentWindow = parentWindow;
         }
 
         internal void SetArea(int x, int y, int width, int height)
@@ -32,7 +35,7 @@ namespace BansheeEditor
         internal void Destroy()
         {
             layout.Destroy();
-            GUI.Destroy();
+            parentWindow.DestroyPanel(GUI);
         }
 
         internal abstract bool Refresh();

MBansheeEditor/Inspector/InspectorWindow.cs

@@ -40,7 +40,7 @@ namespace BansheeEditor
                 inspectorLayout.AddElement(data.container);
 
                 data.inspector = GetInspector(allComponents[i].GetType());
-                data.inspector.Initialize(inspectorPanel, allComponents[i]);
+                data.inspector.Initialize(this, inspectorPanel, allComponents[i]);
 
                 data.foldout.SetExpanded(true);
                 data.foldout.OnToggled += (bool expanded) => OnComponentFoldoutToggled(data, expanded);

MBansheeEngine/GUI/GUIArea.cs

@@ -28,6 +28,7 @@ namespace BansheeEngine
                 parent.childAreas.Add(this);
         }
 
+        // Note: Should only ever be called by its parent GUIPanel
         internal static GUIArea Create(GUIPanel parent, int x, int y, int width, int height, short depth)
         {
             GUIArea newArea = new GUIArea();

MBansheeEngine/GUI/GUIPanel.cs

@@ -50,6 +50,7 @@ namespace BansheeEngine
             mainArea.SetArea(0, 0, width, height);
         }
 
+        // Note: Only to be called from EditorWindow.DestroyPanel
         internal void Destroy()
         {
             GUIArea[] tempAreas = childAreas.ToArray();
@@ -57,8 +58,6 @@ namespace BansheeEngine
                 tempAreas[i].Destroy();
 
             childAreas.Clear();
-
-            Internal_Destroy(mCachedPtr);
         }
 
         [MethodImpl(MethodImplOptions.InternalCall)]
@@ -66,8 +65,5 @@ namespace BansheeEngine
 
         [MethodImpl(MethodImplOptions.InternalCall)]
         private static extern void Internal_CreateInstance(GUIPanel instance);
-
-        [MethodImpl(MethodImplOptions.InternalCall)]
-        private static extern void Internal_Destroy(IntPtr nativeInstance);
     }
 }

SBansheeEditor/Include/BsScriptEditorWindow.h

@@ -40,6 +40,7 @@ namespace BansheeEngine
 		static UINT32 internal_getHeight(ScriptEditorWindow* thisPtr);
 
 		static void internal_initializeGUIPanel(ScriptEditorWindow* thisPtr, MonoObject* panel);
+		static void internal_destroyGUIPanel(ScriptEditorWindow* thisPtr, MonoObject* panel);
 
 		void onWidgetMoved(INT32 x, INT32 y);
 		void onWidgetResized(UINT32 width, UINT32 height);
@@ -65,6 +66,7 @@ namespace BansheeEngine
 		static MonoMethod* onResizedMethod;
 		static MonoMethod* onFocusChangedMethod;
 		static MonoMethod* onInitializedInternalMethod;
+		static MonoMethod* onDestroyInternalMethod;
 
 		// Global editor window management methods
 		static void registerScriptEditorWindow(ScriptEditorWindow* editorWindow);

SBansheeEditor/Source/BsScriptEditorWindow.cpp

@@ -21,6 +21,7 @@ namespace BansheeEngine
 	MonoMethod* ScriptEditorWindow::onResizedMethod = nullptr;
 	MonoMethod* ScriptEditorWindow::onFocusChangedMethod = nullptr;
 	MonoMethod* ScriptEditorWindow::onInitializedInternalMethod = nullptr;
+	MonoMethod* ScriptEditorWindow::onDestroyInternalMethod = nullptr;
 
 	ScriptEditorWindow::ScriptEditorWindow(ScriptEditorWidget* editorWidget)
 		:ScriptObject(editorWidget->getManagedInstance()), mName(editorWidget->getName()), mEditorWidget(editorWidget), mRefreshInProgress(false)
@@ -48,6 +49,7 @@ namespace BansheeEngine
 	{
 		metaData.scriptClass->addInternalCall("Internal_CreateOrGetInstance", &ScriptEditorWindow::internal_createOrGetInstance);
 		metaData.scriptClass->addInternalCall("Internal_InitializeGUIPanel", &ScriptEditorWindow::internal_initializeGUIPanel);
+		metaData.scriptClass->addInternalCall("Internal_DestroyGUIPanel", &ScriptEditorWindow::internal_destroyGUIPanel);
 		metaData.scriptClass->addInternalCall("Internal_GetWidth", &ScriptEditorWindow::internal_getWidth);
 		metaData.scriptClass->addInternalCall("Internal_GetHeight", &ScriptEditorWindow::internal_getHeight);
 		metaData.scriptClass->addInternalCall("Internal_HasFocus", &ScriptEditorWindow::internal_hasFocus);
@@ -57,6 +59,7 @@ namespace BansheeEngine
 		onResizedMethod = metaData.scriptClass->getMethod("WindowResized", 2);
 		onFocusChangedMethod = metaData.scriptClass->getMethod("FocusChanged", 1);
 		onInitializedInternalMethod = metaData.scriptClass->getMethod("OnInitializeInternal", 0);
+		onDestroyInternalMethod = metaData.scriptClass->getMethod("OnDestroyInternal", 0);
 	}
 
 	MonoObject* ScriptEditorWindow::internal_createOrGetInstance(MonoString* ns, MonoString* typeName)
@@ -183,6 +186,15 @@ namespace BansheeEngine
 		scriptGUIPanel->setParentWidget(&thisPtr->mEditorWidget->_getParent()->getParentWidget());
 	}
 
+	void ScriptEditorWindow::internal_destroyGUIPanel(ScriptEditorWindow* thisPtr, MonoObject* panel)
+	{
+		ScriptGUIPanel* scriptGUIPanel = ScriptGUIPanel::toNative(panel);
+
+		auto findIter = std::find(thisPtr->mPanels.begin(), thisPtr->mPanels.end(), scriptGUIPanel);
+		if (findIter != thisPtr->mPanels.end())
+			thisPtr->mPanels.erase(findIter);
+	}
+
 	void ScriptEditorWindow::onWidgetMoved(INT32 x, INT32 y)
 	{
 		for(auto& panel : mPanels)
@@ -346,6 +358,9 @@ namespace BansheeEngine
 
 	void ScriptEditorWidget::triggerOnDestroy()
 	{
+		if (mManagedInstance != nullptr)
+			ScriptEditorWindow::onDestroyInternalMethod->invoke(mManagedInstance, nullptr);
+
 		if (mOnDestroyThunk != nullptr && mManagedInstance != nullptr)
 		{
 			MonoException* exception = nullptr;

SBansheeEditor/Source/BsScriptGUIPanelContainer.cpp

@@ -6,6 +6,7 @@
 #include "BsMonoManager.h"
 #include "BsMonoMethod.h"
 #include "BsGUIOptions.h"
+#include "BsDebug.h"
 
 namespace BansheeEngine
 {
@@ -30,6 +31,7 @@ namespace BansheeEngine
 			options.addOption(mono_array_get(guiOptions, GUIOption, i));
 
 		ScriptGUIPanel* guiPanel = ScriptGUIPanel::toNative(panel);
+		LOGWRN("Creating panel: " + toString((UINT64)guiPanel));
 		GUIPanelContainer* guiPanelContainer = GUIPanelContainer::create(*guiPanel, options);
 
 		ScriptGUIPanelContainer* nativeInstance = new (bs_alloc<ScriptGUIPanelContainer>()) ScriptGUIPanelContainer(instance, guiPanelContainer);

SBansheeEngine/Include/BsScriptGUIPanel.h

@@ -23,6 +23,8 @@ namespace BansheeEngine
 		const Vector<ScriptGUIArea*>& getAreas() const { return mAreas; }
 		Rect2I getArea() const { return mMyArea; }
 
+		void destroy();
+
 	protected:
 		ScriptGUIPanel(MonoObject* instance);
 
@@ -38,5 +40,7 @@ namespace BansheeEngine
 		Rect2I mParentArea;
 		Rect2I mMyArea;
 		Rect2I mClippedArea;
+
+		static MonoMethod* mDestroyMethod;
 	};
 }

SBansheeEngine/Source/BsScriptGUIPanel.cpp

@@ -6,9 +6,12 @@
 #include "BsScriptGUIArea.h"
 #include "BsGUIArea.h"
 #include "BsGUILayout.h"
+#include "BsMonoMethod.h"
 
 namespace BansheeEngine
 {
+	MonoMethod* ScriptGUIPanel::mDestroyMethod = nullptr;
+
 	ScriptGUIPanel::ScriptGUIPanel(MonoObject* instance)
 		:ScriptObject(instance)
 	{
@@ -19,6 +22,8 @@ namespace BansheeEngine
 	{
 		metaData.scriptClass->addInternalCall("Internal_CreateInstance", &ScriptGUIPanel::internal_createInstance);
 		metaData.scriptClass->addInternalCall("Internal_SetArea", &ScriptGUIPanel::internal_setArea);
+
+		mDestroyMethod = metaData.scriptClass->getMethod("Destroy", 0);
 	}
 
 	void ScriptGUIPanel::internal_createInstance(MonoObject* instance)
@@ -69,6 +74,13 @@ namespace BansheeEngine
 			mAreas.erase(iterFind);
 	}
 
+	void ScriptGUIPanel::destroy()
+	{
+		assert(mDestroyMethod != nullptr);
+
+		mDestroyMethod->invoke(mManagedInstance, nullptr);
+	}
+
 	void ScriptGUIPanel::updateArea()
 	{
 		mClippedArea = mMyArea;

TODO.txt

@@ -7,8 +7,15 @@ Possibly set up automatic refresh in debug mode after initialization? As an ad-h
 
 I don't think GUI elements are properly cleaned up after assembly refresh
  - GUIPanelContainer was holding an invalid reference to mGUIPanel
+ - GUI elements aren't destroyed when their managed reference is lost. Instead they remain owned by their GUIWidget.
+   - How to get around this when performing assembly refresh?
+   - Destroying them when reference is lost would be too clumsy as I would need to track when they're referenced by GUILayout or GUIArea and similar.
+   - I probably need to ensure that any GUIArea owned by EditorWindow gets destroyed properly.
+     - By definition this should be any area owned by a GUIPanel
+   - Later when I might have GUI elements in non-editor windows I can handle those on a case by case basis
 
 Scene grid disappears and scene view doesn't work after refresh
+ - This might be related to GUI issue above, as the old GUI still remains so it might be rendering above the new stuff
 
 <<<<Multi-resource saving>>>>:
  - Modify Font so it doesn't contain a texture, but instead keeps a handle to it