Clean up. Make element instancer unique_ptr.

Include/RmlUi/Core/Context.h

@@ -326,8 +326,6 @@ private:
 	// Releases all unloaded documents pending destruction.
 	void ReleaseUnloadedDocuments();
 
-	void ElementRemovedFromContext(Element* element, Element* move_focus);
-
 	// Sends the specified event to all elements in new_items that don't appear in old_items.
 	static void SendEvents(const ElementSet& old_items, const ElementSet& new_items, EventId id, const Dictionary& parameters);
 

Include/RmlUi/Core/Element.h

@@ -501,13 +501,12 @@ public:
 	/// Replaces the second node with the first node.
 	/// @param[in] inserted_element The element that will be inserted and replace the other element.
 	/// @param[in] replaced_element The existing element that will be replaced. If this doesn't exist, inserted_element will be appended.
-	/// @return True if the replaced_element was found, false otherwise.
-	bool ReplaceChild(ElementPtr inserted_element, Element* replaced_element);
+	/// @return A unique pointer to the element if found, discard the result to immediately destroy;
+	ElementPtr ReplaceChild(ElementPtr inserted_element, Element* replaced_element);
 	/// Remove a child element from this element.
 	/// @param[in] The element to remove.
-	/// @returns True if the element was found and removed.
-	bool RemoveChild(Element* element);
-	ElementPtr ReleaseChild(Element* element);
+	/// @returns A unique pointer to the element if found, discard the result to immediately destroy;
+	ElementPtr RemoveChild(Element* element);
 	/// Returns whether or not this element has any DOM children.
 	/// @return True if the element has at least one DOM child, false otherwise.
 	bool HasChildNodes() const;
@@ -561,7 +560,7 @@ public:
 
 	/// Sets the instancer to use for releasing this element.
 	/// @param[in] instancer Instancer to set on this element.
-	void SetInstancer(const ElementInstancerPtr& instancer);
+	void SetInstancer(ElementInstancer* instancer);
 
 	/// Called for every event sent to this element or one of its descendants.
 	/// @param[in] event The event to process.
@@ -619,8 +618,6 @@ protected:
 private:
 	void SetParent(Element* parent);
 
-	void ReleaseElements();
-
 	void DirtyOffset();
 	void UpdateOffset();
 
@@ -661,7 +658,7 @@ private:
 	String id;
 
 	// Instancer that created us, used for destruction.
-	ElementInstancerPtr instancer;
+	ElementInstancer* instancer;
 
 	// Parent element.
 	Element* parent;
@@ -715,8 +712,6 @@ private:
 	OwnedElementList children;
 	int num_non_dom_children;
 
-	OwnedElementList deleted_children;
-
 	float z_index;
 	bool local_stacking_context;
 	bool local_stacking_context_forced;
@@ -756,7 +751,6 @@ private:
 	friend struct ElementDeleter;
 };
 
-
 }
 }
 

Include/RmlUi/Core/Factory.h

@@ -83,11 +83,11 @@ public:
 	/// @param[in] name Name of the instancer; elements with this as their tag will use this instancer.
 	/// @param[in] instancer The instancer to call when the tag is encountered.
 	/// @return The added instancer if the registration was successful, NULL otherwise.
-	static ElementInstancerPtr RegisterElementInstancer(const String& name, ElementInstancerPtr instancer);
+	static ElementInstancer* RegisterElementInstancer(const String& name, ElementInstancerPtr instancer);
 	/// Returns the element instancer for the specified tag.
 	/// @param[in] tag Name of the tag to get the instancer for.
 	/// @return The requested element instancer, or NULL if no such instancer is registered.
-	static ElementInstancerPtr GetElementInstancer(const String& tag);
+	static ElementInstancer* GetElementInstancer(const String& tag);
 	/// Instances a single element.
 	/// @param[in] parent The parent of the new element, or NULL for a root tag.
 	/// @param[in] instancer The name of the instancer to create the element with.

Include/RmlUi/Core/Types.h

@@ -98,12 +98,9 @@ typedef Matrix4< float, RowMajorStorage< float > > RowMajorMatrix4f;
 typedef ColumnMajorMatrix4f Matrix4f;
 
 class Element;
-struct ElementDeleter {
-	void operator()(Element* element) const;
-};
 class ElementInstancer;
 using ElementPtr = std::unique_ptr<Element>;
-using ElementInstancerPtr = std::shared_ptr<ElementInstancer>;
+using ElementInstancerPtr = std::unique_ptr<ElementInstancer>;
 class ElementAnimation;
 class Property;
 class Variant;

Samples/basic/drag/src/DragListener.cpp

@@ -48,7 +48,7 @@ void DragListener::ProcessEvent(Rml::Core::Event& event)
 		if (dest_container == dest_element)
 		{
 			// The dragged element was dragged directly onto a container.
-			Rml::Core::ElementPtr element = drag_element->GetParentNode()->ReleaseChild(drag_element);
+			Rml::Core::ElementPtr element = drag_element->GetParentNode()->RemoveChild(drag_element);
 			dest_container->AppendChild(std::move(element));
 		}
 		else
@@ -77,7 +77,7 @@ void DragListener::ProcessEvent(Rml::Core::Event& event)
 				}
 			}
 
-			Rml::Core::ElementPtr element = drag_element->GetParentNode()->ReleaseChild(drag_element);
+			Rml::Core::ElementPtr element = drag_element->GetParentNode()->RemoveChild(drag_element);
 			dest_container->InsertBefore(std::move(element), insert_before);
 		}
 	}

Samples/invaders/src/main.cpp

@@ -121,7 +121,7 @@ int main(int RMLUI_UNUSED_PARAMETER(argc), char** RMLUI_UNUSED_PARAMETER(argv))
 	Shell::LoadFonts("assets/");
 
 	// Register Invader's custom element and decorator instancers.
-	Rml::Core::Factory::RegisterElementInstancer("game", std::make_shared<Rml::Core::ElementInstancerGeneric< ElementGame >>());
+	Rml::Core::Factory::RegisterElementInstancer("game", std::make_unique<Rml::Core::ElementInstancerGeneric< ElementGame >>());
 
 	Rml::Core::Factory::RegisterDecoratorInstancer("starfield", std::make_unique<DecoratorInstancerStarfield>());
 	Rml::Core::Factory::RegisterDecoratorInstancer("defender", std::make_unique<DecoratorInstancerDefender>());

Source/Controls/Controls.cpp

@@ -45,27 +45,27 @@ namespace Controls {
 // Registers the custom element instancers.
 void RegisterElementInstancers()
 {
-	Core::Factory::RegisterElementInstancer("form", std::make_shared<Core::ElementInstancerGeneric< ElementForm >>());
+	Core::Factory::RegisterElementInstancer("form", std::make_unique<Core::ElementInstancerGeneric< ElementForm >>());
 
-	Core::Factory::RegisterElementInstancer("input", std::make_shared<Core::ElementInstancerGeneric< ElementFormControlInput >>());
+	Core::Factory::RegisterElementInstancer("input", std::make_unique<Core::ElementInstancerGeneric< ElementFormControlInput >>());
 
-	Core::Factory::RegisterElementInstancer("dataselect", std::make_shared<Core::ElementInstancerGeneric< ElementFormControlDataSelect >>());
+	Core::Factory::RegisterElementInstancer("dataselect", std::make_unique<Core::ElementInstancerGeneric< ElementFormControlDataSelect >>());
 
-	Core::Factory::RegisterElementInstancer("select", std::make_shared<Core::ElementInstancerGeneric< ElementFormControlSelect >>());
+	Core::Factory::RegisterElementInstancer("select", std::make_unique<Core::ElementInstancerGeneric< ElementFormControlSelect >>());
 
-	Core::Factory::RegisterElementInstancer("textarea", std::make_shared<Core::ElementInstancerGeneric< ElementFormControlTextArea >>());
+	Core::Factory::RegisterElementInstancer("textarea", std::make_unique<Core::ElementInstancerGeneric< ElementFormControlTextArea >>());
 
-	Core::Factory::RegisterElementInstancer("#selection", std::make_shared<Core::ElementInstancerGeneric< ElementTextSelection >>());
+	Core::Factory::RegisterElementInstancer("#selection", std::make_unique<Core::ElementInstancerGeneric< ElementTextSelection >>());
 
-	Core::Factory::RegisterElementInstancer("tabset", std::make_shared<Core::ElementInstancerGeneric< ElementTabSet >>());
+	Core::Factory::RegisterElementInstancer("tabset", std::make_unique<Core::ElementInstancerGeneric< ElementTabSet >>());
 
-	Core::Factory::RegisterElementInstancer("datagrid", std::make_shared<Core::ElementInstancerGeneric< ElementDataGrid >>());
+	Core::Factory::RegisterElementInstancer("datagrid", std::make_unique<Core::ElementInstancerGeneric< ElementDataGrid >>());
 
-	Core::Factory::RegisterElementInstancer("datagridexpand", std::make_shared<Core::ElementInstancerGeneric< ElementDataGridExpandButton >>());
+	Core::Factory::RegisterElementInstancer("datagridexpand", std::make_unique<Core::ElementInstancerGeneric< ElementDataGridExpandButton >>());
 
-	Core::Factory::RegisterElementInstancer("#rmlctl_datagridcell", std::make_shared<Core::ElementInstancerGeneric< ElementDataGridCell >>());
+	Core::Factory::RegisterElementInstancer("#rmlctl_datagridcell", std::make_unique<Core::ElementInstancerGeneric< ElementDataGridCell >>());
 
-	Core::Factory::RegisterElementInstancer("#rmlctl_datagridrow", std::make_shared<Core::ElementInstancerGeneric< ElementDataGridRow >>());
+	Core::Factory::RegisterElementInstancer("#rmlctl_datagridrow", std::make_unique<Core::ElementInstancerGeneric< ElementDataGridRow >>());
 }
 
 void RegisterXMLNodeHandlers()

Source/Controls/ElementFormControlSelect.cpp

@@ -139,7 +139,7 @@ void ElementFormControlSelect::OnUpdate()
 		// Pull the inner RML and add the option.
 		Rml::Core::String rml;
 		child->GetInnerRML(rml);
-		widget->AddOption(rml, attribute_value, -1, child->GetAttribute("selected") != NULL, child->GetAttribute("unselectable") == NULL);
+		widget->AddOption(rml, attribute_value, -1, child->GetAttribute("selected"), !child->GetAttribute("unselectable"));
 
 		RemoveChild(child);
 	}

Source/Core/Context.cpp

@@ -319,8 +319,8 @@ void Context::UnloadDocument(ElementDocument* _document)
 		document->DispatchEvent(EventId::Unload, Dictionary());
 		PluginRegistry::NotifyDocumentUnload(document);
 
-		// Remove the document from the context.
-		unloaded_documents.push_back( root->ReleaseChild(document) );
+		// Move document to a temporary location to be released later.
+		unloaded_documents.push_back( root->RemoveChild(document) );
 	}
 
 	// Remove the item from the focus history.
@@ -363,10 +363,6 @@ void Context::UnloadAllDocuments()
 	while (root->GetNumChildren(true) > 0)
 		UnloadDocument(root->GetChild(0)->GetOwnerDocument());
 
-	// Force cleanup of child elements now, reference counts must hit zero so that python (if it's in use) cleans up
-	// before we exit this method.
-	root->ReleaseElements();
-
 	// Also need to clear containers that keep ElementReference pointers to elements belonging to removed documents,
 	// essentially preventing them from being released in correct order (before context destroys render interface,
 	// which causes access violation for elements that try to release geometry after context is released)
@@ -1189,11 +1185,6 @@ void Context::ReleaseUnloadedDocuments()
 	}
 }
 
-void Context::ElementRemovedFromContext(Element* element, Element* move_focus)
-{
-
-}
-
 // Sends the specified event to all elements in new_items that don't appear in old_items.
 void Context::SendEvents(const ElementSet& old_items, const ElementSet& new_items, EventId id, const Dictionary& parameters)
 {

Source/Core/Element.cpp

@@ -189,25 +189,9 @@ Element::~Element()
 		child->SetOwnerDocument(nullptr);
 	}
 
-	if (deleted_children.empty())
-	{
-		deleted_children = std::move(children);
-		children.clear();
-	}
-	else
-	{
-		deleted_children.reserve(deleted_children.size() + children.size());
-		for (auto& child : children)
-			deleted_children.push_back(std::move(child));
-		children.clear();
-	}
-	
 	children.clear();
 	num_non_dom_children = 0;
 
-	// Release all deleted children.
-	ReleaseElements();
-
 	delete element_meta;
 
 	if (font_face_handle != NULL)
@@ -216,8 +200,6 @@ Element::~Element()
 
 void Element::Update(float dp_ratio)
 {
-	ReleaseElements();
-
 	OnUpdate();
 
 	UpdateStructure();
@@ -1496,7 +1478,7 @@ Element* Element::InsertBefore(ElementPtr child, Element* adjacent_element)
 }
 
 // Replaces the second node with the first node.
-bool Element::ReplaceChild(ElementPtr inserted_element, Element* replaced_element)
+ElementPtr Element::ReplaceChild(ElementPtr inserted_element, Element* replaced_element)
 {
 	RMLUI_ASSERT(inserted_element);
 	auto insertion_point = children.begin();
@@ -1510,13 +1492,13 @@ bool Element::ReplaceChild(ElementPtr inserted_element, Element* replaced_elemen
 	if (insertion_point == children.end())
 	{
 		AppendChild(std::move(inserted_element));
-		return false;
+		return nullptr;
 	}
 
 	inserted_element_ptr->SetParent(this);
 
 	children.insert(insertion_point, std::move(inserted_element));
-	RemoveChild(replaced_element);
+	ElementPtr result = RemoveChild(replaced_element);
 
 	inserted_element_ptr->GetStyle()->DirtyDefinition();
 
@@ -1524,22 +1506,11 @@ bool Element::ReplaceChild(ElementPtr inserted_element, Element* replaced_elemen
 	for (int i = 0; i <= ChildNotifyLevels && ancestor; i++, ancestor = ancestor->GetParentNode())
 		ancestor->OnChildAdd(inserted_element_ptr);
 
-	return true;
-}
-
-bool Element::RemoveChild(Element* child)
-{
-	ElementPtr released_child = ReleaseChild(child);
-	if (!released_child)
-		return false;
-
-	//deleted_children.push_back(std::move(released_child));
-
-	return true;
+	return result;
 }
 
 // Removes the specified child
-ElementPtr Element::ReleaseChild(Element* child)
+ElementPtr Element::RemoveChild(Element* child)
 {
 	size_t child_index = 0;
 
@@ -1715,7 +1686,7 @@ RenderInterface* Element::GetRenderInterface()
 	return Rml::Core::GetRenderInterface();
 }
 
-void Element::SetInstancer(const ElementInstancerPtr& _instancer)
+void Element::SetInstancer(ElementInstancer* _instancer)
 {
 	// Only record the first instancer being set as some instancers call other instancers to do their dirty work, in
 	// which case we don't want to update the lowest level instancer.
@@ -2151,47 +2122,6 @@ void Element::SetParent(Element* _parent)
 	SetOwnerDocument(parent ? parent->GetOwnerDocument() : nullptr);
 }
 
-void Element::ReleaseElements()
-{
-	//for (auto& element : deleted_children)
-	//{
-	//	// Set the parent to NULL unless it's been reparented already.
-	//	if (element->GetParentNode() == this)
-	//	{
-	//		element->parent = nullptr;
-	//		element->SetOwnerDocument(nullptr);
-	//	}
-	//}
-	
-	deleted_children.clear();
-
-	// Todo: See if some of the old functionality is required after change to unique pointers:
-
-	//// Remove deleted children from this element.
-	//while (!deleted_children.empty())
-	//{
-	//	ElementPtr element = std::move(deleted_children.back());
-	//	deleted_children.pop_back();
-
-	//	// If this element has been added back into our list, then we remove our previous oustanding reference on it
-	//	// and continue.
-	//	if (std::find(children.begin(), children.end(), element) != children.end())
-	//	{
-	//		element->RemoveReference();
-	//		continue;
-	//	}
-
-	//	// Set the parent to NULL unless it's been reparented already.
-	//	if (element->GetParentNode() == this)
-	//	{
-	//		element->parent = nullptr;
-	//		element->SetOwnerDocument(nullptr);
-	//	}
-
-	//	element->RemoveReference();
-	//}
-}
-
 void Element::DirtyOffset()
 {
 	if(!offset_dirty)
@@ -2850,15 +2780,5 @@ void Element::UpdateTransformState()
 	}
 }
 
-void ElementDeleter::operator()(Element* element) const 
-{
-	ElementInstancerPtr& instancer = element->instancer;
-	if (instancer)
-		instancer->ReleaseElement(element);
-	else
-		Log::Message(Log::LT_WARNING, "Leak detected: element %s not instanced via RmlUi Factory. Unable to release.", element->GetAddress().c_str());
-};
-
-
 }
 }

Source/Core/Factory.cpp

@@ -98,11 +98,11 @@ bool Factory::Initialise()
 		event_listener_instancer = NULL;
 
 	// Bind the default element instancers
-	RegisterElementInstancer("*", std::make_shared<ElementInstancerGeneric< Element >>());
-	RegisterElementInstancer("img", std::make_shared < ElementInstancerGeneric< ElementImage >>());
-	RegisterElementInstancer("#text", std::make_shared < ElementInstancerGeneric< ElementTextDefault >>());
-	RegisterElementInstancer("handle", std::make_shared < ElementInstancerGeneric< ElementHandle >>());
-	RegisterElementInstancer("body", std::make_shared < ElementInstancerGeneric< ElementDocument >>());
+	RegisterElementInstancer("*", std::make_unique<ElementInstancerGeneric< Element >>());
+	RegisterElementInstancer("img", std::make_unique < ElementInstancerGeneric< ElementImage >>());
+	RegisterElementInstancer("#text", std::make_unique < ElementInstancerGeneric< ElementTextDefault >>());
+	RegisterElementInstancer("handle", std::make_unique < ElementInstancerGeneric< ElementHandle >>());
+	RegisterElementInstancer("body", std::make_unique < ElementInstancerGeneric< ElementDocument >>());
 
 	// Bind the default decorator instancers
 	RegisterDecoratorInstancer("tiled-horizontal", std::make_unique<DecoratorTiledHorizontalInstancer>());
@@ -166,15 +166,16 @@ Context* Factory::InstanceContext(const String& name)
 	return new_context;
 }
 
-ElementInstancerPtr Factory::RegisterElementInstancer(const String& name, ElementInstancerPtr instancer)
+ElementInstancer* Factory::RegisterElementInstancer(const String& name, ElementInstancerPtr instancer)
 {
+	ElementInstancer* result = instancer.get();
 	String lower_case_name = ToLower(name);
-	element_instancers[lower_case_name] = instancer;
-	return instancer;
+	element_instancers[lower_case_name] = std::move(instancer);
+	return result;
 }
 
 // Looks up the instancer for the given element
-ElementInstancerPtr Factory::GetElementInstancer(const String& tag)
+ElementInstancer* Factory::GetElementInstancer(const String& tag)
 {
 	ElementInstancerMap::iterator instancer_iterator = element_instancers.find(tag);
 	if (instancer_iterator == element_instancers.end())
@@ -184,13 +185,13 @@ ElementInstancerPtr Factory::GetElementInstancer(const String& tag)
 			return nullptr;
 	}
 
-	return (*instancer_iterator).second;
+	return instancer_iterator->second.get();
 }
 
 // Instances a single element.
 ElementPtr Factory::InstanceElement(Element* parent, const String& instancer_name, const String& tag, const XMLAttributes& attributes)
 {
-	ElementInstancerPtr instancer = GetElementInstancer(instancer_name);
+	ElementInstancer* instancer = GetElementInstancer(instancer_name);
 
 	if (instancer)
 	{

Source/Debugger/Plugin.cpp

@@ -83,7 +83,7 @@ bool Plugin::Initialise(Core::Context* context)
 		return false;
 	}
 
-	Core::Factory::RegisterElementInstancer("debug-hook", std::make_shared<Core::ElementInstancerGeneric< ElementContextHook >>());
+	Core::Factory::RegisterElementInstancer("debug-hook", std::make_unique<Core::ElementInstancerGeneric< ElementContextHook >>());
 
 	return true;
 }
@@ -336,7 +336,7 @@ bool Plugin::LoadMenuElement()
 
 bool Plugin::LoadInfoElement()
 {
-	Core::Factory::RegisterElementInstancer("debug-info", std::make_shared<Core::ElementInstancerGeneric< ElementInfo >>());
+	Core::Factory::RegisterElementInstancer("debug-info", std::make_unique<Core::ElementInstancerGeneric< ElementInfo >>());
 	info_element = dynamic_cast< ElementInfo* >(host_context->CreateDocument("debug-info"));
 	if (info_element == NULL)
 		return false;
@@ -356,7 +356,7 @@ bool Plugin::LoadInfoElement()
 
 bool Plugin::LoadLogElement()
 {
-	Core::Factory::RegisterElementInstancer("debug-log", std::make_shared<Core::ElementInstancerGeneric< ElementLog >>());
+	Core::Factory::RegisterElementInstancer("debug-log", std::make_unique<Core::ElementInstancerGeneric< ElementLog >>());
 	log_element = dynamic_cast< ElementLog* >(host_context->CreateDocument("debug-log"));
 	if (log_element == NULL)
 		return false;