From c71a21c9f8cd7f226a2fd739d496ba3a8b6f9754 Mon Sep 17 00:00:00 2001
From: ravi688 <rp0412204@gmail.com>
Date: Sat, 9 Nov 2024 18:21:51 +0530
Subject: [PATCH] [SUTK] Various fixes, and debugging improvements (more
 diagnostic messages upon invalid usage of the API)

---
 sutk/include/sutk/ButtonGraphic.hpp       |  1 +
 sutk/include/sutk/Container.hpp           |  3 ++-
 sutk/include/sutk/ContainerUtility.hpp    |  2 ++
 sutk/include/sutk/ImageContainer.hpp      | 21 +++++++++++++++++++++
 sutk/include/sutk/NotebookView.hpp        |  3 +++
 sutk/include/sutk/Renderable.hpp          |  5 ++---
 sutk/include/sutk/RenderableContainer.hpp |  5 +++--
 sutk/include/sutk/TextGroupContainer.hpp  |  2 ++
 sutk/makefile                             |  2 +-
 sutk/source/ButtonGraphic.cpp             |  5 +++++
 sutk/source/Container.cpp                 |  5 ++++-
 sutk/source/ContainerUtility.cpp          | 17 +++++++++++++++++
 sutk/source/ImageContainer.cpp            | 20 ++++++++++++++++++++
 sutk/source/NotebookView.cpp              |  5 +++--
 sutk/source/Renderable.cpp                | 10 ++++++++++
 sutk/source/RenderableContainer.cpp       |  4 ++++
 sutk/source/TextGroupContainer.cpp        |  7 +++----
 17 files changed, 103 insertions(+), 14 deletions(-)
 create mode 100644 sutk/include/sutk/ImageContainer.hpp
 create mode 100644 sutk/source/ImageContainer.cpp

diff --git a/sutk/include/sutk/ButtonGraphic.hpp b/sutk/include/sutk/ButtonGraphic.hpp
index 77018a3e..061ecff3 100644
--- a/sutk/include/sutk/ButtonGraphic.hpp
+++ b/sutk/include/sutk/ButtonGraphic.hpp
@@ -120,6 +120,7 @@ namespace SUTK
 
 	public:
 		DefaultButtonGraphicNoLabel(UIDriver& driver, Container* parent) noexcept;
+		~DefaultButtonGraphicNoLabel() noexcept;
 
 		RenderRectFillRound& getRenderRect() noexcept { return *m_renderRect; }
 	};
diff --git a/sutk/include/sutk/Container.hpp b/sutk/include/sutk/Container.hpp
index fc915e1d..ce3725d1 100644
--- a/sutk/include/sutk/Container.hpp
+++ b/sutk/include/sutk/Container.hpp
@@ -70,7 +70,6 @@ namespace SUTK
 	protected:
 		// this can only be called by SUTK::UIDriver
 		Container(SUTK::UIDriver& driver, Container* parent = NULL, com::Bool isLayoutIgnore = com::Bool::False(), Layer layer = InvalidLayer);
-		virtual ~Container();
 
 		friend class UIDriver;
 
@@ -110,6 +109,7 @@ namespace SUTK
 		void setParentChildRelation(Container* parent, std::size_t index = std::numeric_limits<std::size_t>::max()) noexcept;
 
 	public:
+		virtual ~Container();
 
 		// IMPLEMENTATION of IDebuggable
 		virtual void enableDebug(bool isEnable = true, Color4 color = Color4::green()) noexcept override;
@@ -136,6 +136,7 @@ namespace SUTK
 		// childs getters
 		std::vector<Container*>& getChilds() noexcept { return getContainerList(); }
 		const std::vector<Container*>& getChilds() const noexcept { return getContainerList(); }
+		std::size_t getChildCount() const noexcept { return getChilds().size(); }
 
 
 		// parent getters
diff --git a/sutk/include/sutk/ContainerUtility.hpp b/sutk/include/sutk/ContainerUtility.hpp
index 8306c3fd..73d14d8b 100644
--- a/sutk/include/sutk/ContainerUtility.hpp
+++ b/sutk/include/sutk/ContainerUtility.hpp
@@ -27,6 +27,8 @@ namespace SUTK
 		static void RenderablesVisit(Container* container, const std::function<void(Renderable*)>& visitor) noexcept;
 		static void ContainersWalkUpUntil(Container* container, const std::function<bool(Container*)>& visitor) noexcept;
 		static void ContainersVisit(Container* container, const std::function<void(Container*)>& visitor) noexcept;
+		static void ContainersDestroyRecursive(Container* container) noexcept;
+		static void ContainersVisitChildEnd(Container* container, const std::function<void(Container*)>& visitor) noexcept;
 		static GfxDriverObjectHandleType findTextGroupHandle(Container* container) noexcept;
 
 		typedef void (*IInputEventHandlerObjectsVisitor)(std::vector<IInputEventHandlerObject*>& eventHandlerObj);
diff --git a/sutk/include/sutk/ImageContainer.hpp b/sutk/include/sutk/ImageContainer.hpp
new file mode 100644
index 00000000..df3ad6bc
--- /dev/null
+++ b/sutk/include/sutk/ImageContainer.hpp
@@ -0,0 +1,21 @@
+#pragma once
+
+#include <sutk/UIDriver.hpp> // for UIDriver::ImageReference
+#include <sutk/RenderableContainer.hpp>
+
+namespace SUTK
+{
+	class RenderImage;
+
+	class SUTK_API ImageContainer : public RenderableContainer
+	{
+	private:
+		RenderImage* m_renderImage;
+	public:
+		ImageContainer(UIDriver& driver, Container* parent, com::Bool isLayoutIgnore = com::Bool::False(), Layer layer = InvalidLayer) noexcept;
+		~ImageContainer() noexcept;
+
+		void setImage(UIDriver::ImageReference image) noexcept;
+		RenderImage* getRenderImage() noexcept { return m_renderImage; }
+	};
+}
\ No newline at end of file
diff --git a/sutk/include/sutk/NotebookView.hpp b/sutk/include/sutk/NotebookView.hpp
index 00f76713..4f3a73d5 100644
--- a/sutk/include/sutk/NotebookView.hpp
+++ b/sutk/include/sutk/NotebookView.hpp
@@ -8,6 +8,7 @@
 
 #include <common/Concepts.hpp> // for com::CompareFunction
 
+#include <optional> // for std::optional
 
 namespace SUTK
 {
@@ -20,6 +21,7 @@ namespace SUTK
 	private:
 		void* m_data;
 		void (*m_dataDeleter)(void*);
+		std::optional<std::function<void(NotebookPage*)>> m_onPageRemove;
 		TabView* m_tabView;
 		Container* m_container;
 
@@ -31,6 +33,7 @@ namespace SUTK
 	public:
 		NotebookPage(Container* container) noexcept;
 		~NotebookPage() noexcept;
+		void setOnRemove(std::function<void(NotebookPage*)> onPageRemove) noexcept { m_onPageRemove = onPageRemove; }
 		void setLabel(const std::string_view str) noexcept;
 		Container* getContainer() noexcept { return m_container; }
 		u32 getIndex() const noexcept;
diff --git a/sutk/include/sutk/Renderable.hpp b/sutk/include/sutk/Renderable.hpp
index 67a90a9e..74ca5de6 100644
--- a/sutk/include/sutk/Renderable.hpp
+++ b/sutk/include/sutk/Renderable.hpp
@@ -14,16 +14,15 @@ namespace SUTK
 	class Renderable : public UIDriverObject, public Activatable
 	{
 		friend class UIDriver;
+		friend class Container;
+		friend class RenderableContainer;
 
 	private:
 		RenderableContainer* m_container;
 		u32 m_drawOrder;
 		bool m_isDrawOrderDirty;
 
-		friend class Container;
-
 	protected:
-		friend class RenderableContainer;
 		// Rendering Geometries (or Text) requires conversion from local coordinates to global coordinates (target rendering backend, SGE)
 		// So, whenever any of the Anscestor containers change their position, the old transformed coordinates become invalid (dirty),
 		// and this requires to walk recursively up the container tree to finally calculate the new global coordiantes with possibly same
diff --git a/sutk/include/sutk/RenderableContainer.hpp b/sutk/include/sutk/RenderableContainer.hpp
index de74c132..36603369 100644
--- a/sutk/include/sutk/RenderableContainer.hpp
+++ b/sutk/include/sutk/RenderableContainer.hpp
@@ -8,14 +8,15 @@ namespace SUTK
 
 	class RenderableContainer : public Container
 	{
+		friend class Renderable;
+		friend class UIDriver;
 	private:
 		Renderable* m_renderable;
 
-		friend class UIDriver;
-
 	protected:
 
 		RenderableContainer(UIDriver& driver, Container* parent, com::Bool isLayoutIgnore = com::Bool::False(), Layer layer = InvalidLayer) noexcept;
+		~RenderableContainer() noexcept;
 		
 		virtual void onAdd(Container* parent) override;
 		virtual void onParentResize(const Rect2Df& newRect, bool isPositionChanged, bool isSizeChanged) override;
diff --git a/sutk/include/sutk/TextGroupContainer.hpp b/sutk/include/sutk/TextGroupContainer.hpp
index 25042bc3..8c44c1e9 100644
--- a/sutk/include/sutk/TextGroupContainer.hpp
+++ b/sutk/include/sutk/TextGroupContainer.hpp
@@ -9,6 +9,8 @@ namespace SUTK
 	class TextGroup;
 	class SUTK_API TextGroupContainer : public RenderableContainer
 	{
+	private:
+		TextGroup* m_textGroup;
 	public:
 		TextGroupContainer(UIDriver& driver, Container* parent = NULL, com::Bool isLayoutIgnore = com::Bool::False(), Layer layer = InvalidLayer) noexcept;
 
diff --git a/sutk/makefile b/sutk/makefile
index 43a1deee..c8dc3e80 100644
--- a/sutk/makefile
+++ b/sutk/makefile
@@ -171,7 +171,7 @@ OBJECTS= $(addsuffix .o, $(SOURCES))
 LIBS = $(EXTERNAL_LIBRARIES)
 
 #Flags and Defines
-DEBUG_DEFINES =  -DGLOBAL_DEBUG -DDEBUG -DLOG_DEBUG
+DEBUG_DEFINES =  -DGLOBAL_DEBUG -DDEBUG -DLOG_DEBUG -DCONTAINER_DEBUG
 RELEASE_DEFINES =  -DGLOBAL_RELEASE -DRELEASE -DLOG_RELEASE
 DEFINES = $(BUILD_DEFINES)
 
diff --git a/sutk/source/ButtonGraphic.cpp b/sutk/source/ButtonGraphic.cpp
index 8cb015d7..ed16bcdd 100644
--- a/sutk/source/ButtonGraphic.cpp
+++ b/sutk/source/ButtonGraphic.cpp
@@ -85,6 +85,11 @@ namespace SUTK
 		m_renderRect = driver.createRenderable<RenderRectFillRound>(this);
 	}
 
+	DefaultButtonGraphicNoLabel::~DefaultButtonGraphicNoLabel() noexcept
+	{
+		getUIDriver().destroyRenderable<RenderRectFillRound>(m_renderRect);
+	}
+
 	void DefaultButtonGraphicNoLabel::onColorChange(Color4 color) noexcept
 	{
 		m_renderRect->setColor(color);
diff --git a/sutk/source/Container.cpp b/sutk/source/Container.cpp
index 225932bf..f8d9e604 100644
--- a/sutk/source/Container.cpp
+++ b/sutk/source/Container.cpp
@@ -31,15 +31,18 @@ namespace SUTK
 
 	Container::~Container()
 	{
+		com_assert(COM_DESCRIPTION(getChildCount() == 0), "You must destroy child containers first!");
+		
 		if(m_parent)
 			m_parent->remove(this);
 		
 		#ifdef CONTAINER_DEBUG
 		if(m_renderRectCont)
 		{
-			getUIDriver().destroyContainer<RenderableContainer>(m_renderRectCont);
+			// Renderable should be destroyed first.
 			_com_assert(m_renderRect != com::null_pointer<RenderRectOutline>());
 			getUIDriver().destroyRenderable<RenderRectOutline>(m_renderRect);
+			getUIDriver().destroyContainer<RenderableContainer>(m_renderRectCont);
 		}
 		#endif
 
diff --git a/sutk/source/ContainerUtility.cpp b/sutk/source/ContainerUtility.cpp
index a7a93fe4..77a63f80 100644
--- a/sutk/source/ContainerUtility.cpp
+++ b/sutk/source/ContainerUtility.cpp
@@ -97,6 +97,23 @@ namespace SUTK
 			ContainersVisit(child, visitor);
 	}
 
+	void ContainerUtility::ContainersDestroyRecursive(Container* container) noexcept
+	{
+		ContainersVisitChildEnd(container, [](Container* container) noexcept
+		{
+			container->getUIDriver().destroyContainer<Container>(container);
+		});
+	}
+
+	void ContainerUtility::ContainersVisitChildEnd(Container* container, const std::function<void(Container*)>& visitor) noexcept
+	{
+		if(!container) return;
+		std::vector<Container*>& childs = container->getChilds();
+		while(childs.size() > 0)
+			ContainersVisitChildEnd(childs.back(), visitor);
+		visitor(container);
+	}
+
 	GfxDriverObjectHandleType ContainerUtility::findTextGroupHandle(Container* container) noexcept
 	{
 		GfxDriverObjectHandleType handle = GFX_DRIVER_OBJECT_NULL_HANDLE;
diff --git a/sutk/source/ImageContainer.cpp b/sutk/source/ImageContainer.cpp
new file mode 100644
index 00000000..e5385a57
--- /dev/null
+++ b/sutk/source/ImageContainer.cpp
@@ -0,0 +1,20 @@
+#include <sutk/ImageContainer.hpp>
+#include <sutk/RenderImage.hpp>
+
+namespace SUTK
+{
+	ImageContainer::ImageContainer(UIDriver& driver, Container* parent, com::Bool isLayoutIgnore, Layer layer) noexcept : RenderableContainer(driver, parent, isLayoutIgnore, layer)
+	{
+		m_renderImage = driver.createRenderable<RenderImage>(this);
+	}
+
+	ImageContainer::~ImageContainer() noexcept
+	{
+		getUIDriver().destroyRenderable<RenderImage>(m_renderImage);
+	}
+
+	void ImageContainer::setImage(UIDriver::ImageReference image) noexcept
+	{
+		m_renderImage->setImage(image);
+	}
+}
\ No newline at end of file
diff --git a/sutk/source/NotebookView.cpp b/sutk/source/NotebookView.cpp
index b5c653cf..9e27c540 100644
--- a/sutk/source/NotebookView.cpp
+++ b/sutk/source/NotebookView.cpp
@@ -84,8 +84,8 @@ namespace SUTK
 	TabView::~TabView() noexcept
 	{
 		auto& driver = getUIDriver();
-		driver.destroyContainer<Button>(m_closeButton);
 		driver.destroyContainer<ImageButtonGraphic>(m_closeButtonGraphic);
+		driver.destroyContainer<Button>(m_closeButton);
 	}
 
 	void TabView::setLabel(const std::string_view str) noexcept
@@ -320,7 +320,8 @@ namespace SUTK
 			m_head = page->getNext();
 		if(tabView->m_next)
 			tabView->m_next->m_prev = tabView->m_prev;
-		m_tabContainer->remove(tabView);
+		if(page->m_onPageRemove.has_value())
+			page->m_onPageRemove.value() (page);
 		driver.destroyContainer<Container>(page->m_container);
 		driver.destroyContainer<TabView>(page->m_tabView);
 		delete page;
diff --git a/sutk/source/Renderable.cpp b/sutk/source/Renderable.cpp
index 92530951..4a23b78a 100644
--- a/sutk/source/Renderable.cpp
+++ b/sutk/source/Renderable.cpp
@@ -7,7 +7,12 @@ namespace SUTK
 	Renderable::Renderable(UIDriver& driver, RenderableContainer* container) noexcept : UIDriverObject(driver), m_container(container), m_drawOrder(0), m_isDrawOrderDirty(true)
 	{
 		if(container != NULL)
+		{
+			// NOTE: We can't call RenderableContainer::getRenderable() here because it is virtual, and can be overriden
+			// And in that override, one may instantiate this Renderable() which will lead to non-terminating recursion!
+			com_assert(COM_DESCRIPTION(!container->m_renderable), "Only one Renderable is allowed per RenderableContainer");
 			container->setRenderable(this);
+		}
 		driver.addRenderable(this);
 
 		setDrawOrder((container == NULL) ? 0 : container->getDepth());
@@ -15,6 +20,11 @@ namespace SUTK
 
 	Renderable::~Renderable() noexcept
 	{
+		if(getContainer())
+		{
+			getContainer()->setRenderable(com::null_pointer<Renderable>());
+			m_container = com::null_pointer<RenderableContainer>();
+		}
 		getUIDriver().removeRenderable(this);
 	}
 
diff --git a/sutk/source/RenderableContainer.cpp b/sutk/source/RenderableContainer.cpp
index 063981ac..2e70047c 100644
--- a/sutk/source/RenderableContainer.cpp
+++ b/sutk/source/RenderableContainer.cpp
@@ -6,6 +6,10 @@ namespace SUTK
 	RenderableContainer::RenderableContainer(UIDriver& driver, Container* parent, com::Bool isLayoutIgnore, Layer layer) noexcept : Container(driver, parent, isLayoutIgnore, layer), m_renderable(NULL)
 	{
 
+	}
+	RenderableContainer::~RenderableContainer() noexcept
+	{
+		com_assert(COM_DESCRIPTION(!getRenderable()), "You must destroy Renderable before RenderableContainer!");
 	}
 	void RenderableContainer::onAdd(Container* parent)
 	{
diff --git a/sutk/source/TextGroupContainer.cpp b/sutk/source/TextGroupContainer.cpp
index ac1038ab..e7e51efb 100644
--- a/sutk/source/TextGroupContainer.cpp
+++ b/sutk/source/TextGroupContainer.cpp
@@ -3,10 +3,9 @@
 
 namespace SUTK
 {
-	TextGroupContainer::TextGroupContainer(UIDriver& driver, Container* parent, com::Bool isLayoutIgnore, Layer layer) noexcept : RenderableContainer(driver, parent, isLayoutIgnore, layer)
+	TextGroupContainer::TextGroupContainer(UIDriver& driver, Container* parent, com::Bool isLayoutIgnore, Layer layer) noexcept : RenderableContainer(driver, parent, isLayoutIgnore, layer), m_textGroup(com::null_pointer<TextGroup>())
 	{
-		TextGroup* textGroup = driver.createRenderable<TextGroup>(this);
-		setRenderable(textGroup);
+		m_textGroup = driver.createRenderable<TextGroup>(this);
 	}
 
 	GfxDriverObjectHandleType TextGroupContainer::getGfxDriverObjectHandle() noexcept
@@ -16,6 +15,6 @@ namespace SUTK
 
  	TextGroup* TextGroupContainer::getTextGroup() noexcept
  	{ 
- 		return com::iknow_down_cast<TextGroup*>(getRenderable()); 
+ 		return m_textGroup;
  	}
 }
\ No newline at end of file