From 3983c296458520d93a8e79275ef5cc7925f3e431 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lars=20M=C3=BCller?=
 <34514239+appgurueu@users.noreply.github.com>
Date: Mon, 8 Jan 2024 18:43:24 +0100
Subject: [PATCH] Optimize scene node child removal to constant time (#275)

---
 include/ISceneNode.h | 54 ++++++++++++++++++++++----------------------
 1 file changed, 27 insertions(+), 27 deletions(-)

diff --git a/include/ISceneNode.h b/include/ISceneNode.h
index 8d99d27..f963aca 100644
--- a/include/ISceneNode.h
+++ b/include/ISceneNode.h
@@ -15,6 +15,7 @@
 #include "matrix4.h"
 #include "IAttributes.h"
 #include <list>
+#include <optional>
 
 namespace irr
 {
@@ -275,31 +276,31 @@ namespace scene
 
 				child->grab();
 				child->remove(); // remove from old parent
-				Children.push_back(child);
+				// Note: This iterator is not invalidated until we erase it.
+				child->ThisIterator = Children.insert(Children.end(), child);
 				child->Parent = this;
 			}
 		}
 
 
 		//! Removes a child from this scene node.
-		/** If found in the children list, the child pointer is also
-		dropped and might be deleted if no other grab exists.
+		/**
 		\param child A pointer to the child which shall be removed.
 		\return True if the child was removed, and false if not,
-		e.g. because it couldn't be found in the children list. */
+		e.g. because it belongs to a different parent or no parent. */
 		virtual bool removeChild(ISceneNode* child)
 		{
-			ISceneNodeList::iterator it = Children.begin();
-			for (; it != Children.end(); ++it)
-				if ((*it) == child)
-				{
-					(*it)->Parent = 0;
-					(*it)->drop();
-					Children.erase(it);
-					return true;
-				}
+			if (child->Parent != this)
+				return false;
 
-			return false;
+			// The iterator must be set since the parent is not null.
+			_IRR_DEBUG_BREAK_IF(!child->ThisIterator.has_value());
+			auto it = *child->ThisIterator;
+			child->ThisIterator = std::nullopt;
+			child->Parent = nullptr;
+			child->drop();
+			Children.erase(it);
+			return true;
 		}
 
 
@@ -309,13 +310,11 @@ namespace scene
 		*/
 		virtual void removeAll()
 		{
-			ISceneNodeList::iterator it = Children.begin();
-			for (; it != Children.end(); ++it)
-			{
-				(*it)->Parent = 0;
-				(*it)->drop();
+			for (auto &child : Children) {
+				child->Parent = nullptr;
+				child->ThisIterator = std::nullopt;
+				child->drop();
 			}
-
 			Children.clear();
 		}
 
@@ -508,10 +507,8 @@ namespace scene
 			grab();
 			remove();
 
-			Parent = newParent;
-
-			if (Parent)
-				Parent->addChild(this);
+			if (newParent)
+				newParent->addChild(this);
 
 			drop();
 		}
@@ -618,12 +615,15 @@ namespace scene
 		//! Relative scale of the scene node.
 		core::vector3df RelativeScale;
 
-		//! Pointer to the parent
-		ISceneNode* Parent;
-
 		//! List of all children of this node
 		std::list<ISceneNode*> Children;
 
+		//! Iterator pointing to this node in the parent's child list.
+		std::optional<ISceneNodeList::iterator> ThisIterator;
+
+		//! Pointer to the parent
+		ISceneNode* Parent;
+
 		//! Pointer to the scene manager
 		ISceneManager* SceneManager;