[transform_hierarchy][mesh_renderer][animation_renderer] removal of o…

…bjects. all use free-lists under-the-hood to maintain index stability

src/tz/core/data/transform_hierarchy.hpp

@@ -43,6 +43,8 @@ namespace tz
 			remove_children,
 			/// When the node is removed, any children are detached, becoming root nodes - even if the original node had a parent.
 			detach_children,
+			/// When the node is removed, the children are completely untouched, meaning they continue to refer to a dead parent. You almost never want this.
+			impl_do_nothing,
 		};
 		/// Create a new, empty hierarchy.
 		transform_hierarchy() = default;
@@ -166,6 +168,7 @@ namespace tz
 	private:
 		bool dbgui_node(unsigned int node_id, bool display_gizmo);
 		std::vector<transform_node<T>> nodes = {};
+		std::vector<std::size_t> node_free_list = {};
 		mutable std::vector<std::size_t> node_local_transform_hashes = {};
 		mutable std::vector<tz::trs> node_global_transform_cache = {};
 	};

src/tz/core/data/transform_hierarchy.inl

@@ -93,20 +93,33 @@ namespace tz
 	template<typename T>
 	unsigned int transform_hierarchy<T>::add_node(tz::trs local_transform, T data, std::optional<unsigned int> parent)
 	{
-		auto id = this->nodes.size();
-		this->nodes.push_back({.data = data, .local_transform = local_transform, .parent = parent});
+		std::size_t id;
+		if(this->node_free_list.size())
+		{
+			id = this->node_free_list.front();
+			tz::report("Use recycled node id %zu", id);
+			this->node_free_list.erase(this->node_free_list.begin());
+			this->nodes[id] = {.data = data, .local_transform = local_transform, .parent = parent};
+		}
+		else
+		{
+			id = this->nodes.size();
+			this->nodes.push_back({.data = data, .local_transform = local_transform, .parent = parent});
+			this->node_local_transform_hashes.push_back(std::numeric_limits<std::size_t>::max());
+			this->node_global_transform_cache.push_back({});
+		}
 		if(parent.has_value())
 		{
 			this->nodes[parent.value()].children.push_back(id);
 		}
-		this->node_local_transform_hashes.push_back(std::numeric_limits<std::size_t>::max());
-		this->node_global_transform_cache.push_back({});
 		return id;
 	}
 
 	template<typename T>
 	void transform_hierarchy<T>::remove_node(unsigned int node_id, remove_strategy strategy)
 	{
+		tz::assert(std::find(this->node_free_list.begin(), this->node_free_list.end(), node_id) == this->node_free_list.end(), "Double remove detected on node %u", node_id);
+		tz::report("Remove node %u", node_id);
 		tz::assert(node_id < this->nodes.size(), "Invalid node_id %zu (node count: %zu)", node_id, this->nodes.size());
 		// what if the node had a parent?
 		const auto& node = this->get_node(node_id);
@@ -133,19 +146,26 @@ namespace tz
 				}
 			break;
 			case remove_strategy::remove_children:
-				for(unsigned int child_node_idx : node.children)
+			{
+				// node.children will change each iteration, so work on a copy.
+				while(node.children.size())
 				{
-					this->remove_node(child_node_idx, strategy);
+					this->remove_node(node.children.front(), strategy);
 				}
+			}
 			break;
 			case remove_strategy::detach_children:
 				for(unsigned int child_node_idx : node.children)
 				{
 					this->nodes[child_node_idx].parent = std::nullopt;
 				}
 			break;
+			case remove_strategy::impl_do_nothing:
+			break;
 		}
-		this->nodes.erase(this->nodes.begin() + node_id);
+		this->nodes[node_id] = {};
+		this->node_free_list.push_back(node_id);
+		//this->nodes.erase(this->nodes.begin() + node_id);
 	}
 
 	template<typename T>
@@ -334,6 +354,11 @@ namespace tz
 	bool transform_hierarchy<T>::dbgui_node(unsigned int node_id, bool display_gizmo)
 	{
 		const transform_node<T>& node = this->get_node(node_id);
+		if(std::find(this->node_free_list.begin(), this->node_free_list.end(), node_id) != this->node_free_list.end())
+		{
+			// its on the free-list. this is a deleted node. skip
+			return true;
+		}
 		std::string node_name;
 		if constexpr(is_printable<T>)
 		{

src/tz/ren/animation.cpp

@@ -121,30 +121,42 @@ namespace tz::ren
 	{
 		// maintain offsets so we can support multiple gltfs.
 		// add the new gltf.
-		this->gltfs.push_back
-		({
-			.data = gltf,
-			.object_offset = static_cast<unsigned int>(mesh_renderer::draw_count()),
-		});
-		auto& this_gltf = this->gltfs.back();
-		this_gltf.assets.gltfh = static_cast<tz::hanval>(this->gltfs.size() - 1);
+		gltf_info* this_gltf = nullptr;
+		if(this->gltf_free_list.size())
+		{
+			auto hanval = static_cast<std::size_t>(static_cast<tz::hanval>(this->gltf_free_list.front()));
+			tz::report("Re-use GLTF spot %zu", hanval);
+			this->gltf_free_list.erase(this->gltf_free_list.begin());
+			this_gltf = &this->gltfs[hanval];
+			this_gltf->assets.gltfh = static_cast<tz::hanval>(hanval);
+		}
+		else
+		{
+			this->gltfs.push_back
+			({
+				.data = gltf,
+				.object_offset = static_cast<unsigned int>(mesh_renderer::draw_count()),
+			});
+			this_gltf = &this->gltfs.back();
+			this_gltf->assets.gltfh = static_cast<tz::hanval>(this->gltfs.size() - 1);
+		}
 
 		// skins
-		this->node_handle_skins(this_gltf);
+		this->node_handle_skins(*this_gltf);
 		// meshes
 		std::vector<mesh_handle> meshes;
 		if(opkg.overrides.contains(override_flag::mesh))
 		{
 			meshes = opkg.pkg.meshes;
-			this->shallow_patch_meshes(this_gltf);
+			this->shallow_patch_meshes(*this_gltf);
 		}
 		else
 		{
-			meshes = this->node_handle_meshes(this_gltf);
+			meshes = this->node_handle_meshes(*this_gltf);
 		}
 		for(mesh_handle mesh : meshes)
 		{
-			this_gltf.assets.meshes.push_back(mesh);
+			this_gltf->assets.meshes.push_back(mesh);
 		}
 		// textures
 		std::vector<texture_handle> materials;
@@ -154,11 +166,11 @@ namespace tz::ren
 		}
 		else
 		{
-			materials = this->node_handle_materials(this_gltf);
+			materials = this->node_handle_materials(*this_gltf);
 		}
 		for(texture_handle texture : materials)
 		{
-			this_gltf.assets.textures.push_back(texture);
+			this_gltf->assets.textures.push_back(texture);
 		}
 
 		// objects
@@ -173,26 +185,52 @@ namespace tz::ren
 			std::size_t node_id = std::distance(all_nodes.begin(), iter);
 			tz::assert(node.id == node_id);
 
-			this->expand_current_gltf_node(this_gltf, node_id, std::nullopt, parent);
+			this->expand_current_gltf_node(*this_gltf, node_id, std::nullopt, parent);
 		}
 
-		this->write_inverse_bind_matrices(this_gltf);
+		this->write_inverse_bind_matrices(*this_gltf);
 
 		std::optional<tz::vec2ui32> maybe_joint_span = std::nullopt;
-		if(this_gltf.data.get_skins().size())
+		if(this_gltf->data.get_skins().size())
 		{
-			maybe_joint_span = this->write_skin_object_data(this_gltf);
+			maybe_joint_span = this->write_skin_object_data(*this_gltf);
 		}
 		//this->resource_write_joint_indices(this_gltf);
 		if(maybe_joint_span.has_value())
 		{
-			for(object_handle oh : this_gltf.assets.objects)
+			for(object_handle oh : this_gltf->assets.objects)
 			{
 				mesh_renderer::get_object_data(oh).extra_indices = maybe_joint_span.value().with_more(0u).with_more(0u);
 			}
 		}
 
-		return this_gltf.assets;
+		return this_gltf->assets;
+	}
+
+	void animation_renderer::remove_objects(asset_package pkg, transform_hierarchy::remove_strategy strategy)
+	{
+		// its safe to do this! even if override packages are used to share meshes/textures,
+		// each new gltf still has their own copy of everything.
+		// if gltfh is nullhand, it could mean many things:
+		// - double remove of pkg. not good
+		// - someones cleared/didnt think about gltfh and just wants to delete objects. that's fine.
+
+		// we cant just outright delete these because previous pkgs have been sent out with fixed gltfhs.
+		// if we delete a gltfh, it invalidates all other gltfhs. a free-list works fine here though.
+		if(pkg.gltfh != tz::nullhand)
+		{
+			auto gltf_hanval = static_cast<std::size_t>(static_cast<tz::hanval>(pkg.gltfh));
+			// note that we can no longer tell the difference between a used, empty gltf, and a deleted, no-longer-used gltf
+			this->gltfs[gltf_hanval] = {};
+			// thats what the free-list is for. recycle this spot for someone else.
+			this->gltf_free_list.push_back(pkg.gltfh);
+		}
+
+		for(object_handle oh : pkg.objects)
+		{
+			mesh_renderer::remove_object(oh, strategy);
+			this->object_extras[static_cast<std::size_t>(static_cast<tz::hanval>(oh))] = {};
+		}
 	}
 
 	std::size_t animation_renderer::get_animation_count(const asset_package& pkg) const
@@ -277,7 +315,11 @@ namespace tz::ren
 		for(std::size_t i = 0; i < mesh_renderer::draw_count(); i++)
 		{
 			auto maybe_tree_id = mesh_renderer::object_tree.find_node(i);
-			tz::assert(maybe_tree_id.has_value());
+			if(!maybe_tree_id.has_value())
+			{
+				// node will be nullopt if an object was removed (and is thus an empty). we just skip those.
+				continue;
+			}
 			auto& tree_node = mesh_renderer::object_tree.get_node(maybe_tree_id.value());
 			auto& extra = this->object_extras[i];
 			if(extra.is_animated)
@@ -767,6 +809,10 @@ namespace tz::ren
 					}
 				}
 				ImGui::EndChild();
+				if(ImGui::Button("Delete"))
+				{
+					this->remove_objects(anim.assets, transform_hierarchy::remove_strategy::remove_children);
+				}
 			}
 			static int object_id = 0;
 			if(mesh_renderer::draw_count() > 0 && ImGui::CollapsingHeader("Objects", ImGuiTreeNodeFlags_DefaultOpen))

src/tz/ren/animation.hpp

@@ -57,6 +57,7 @@ namespace tz::ren
 		asset_package add_gltf(tz::io::gltf gltf, override_package opkg);
 		asset_package add_gltf(tz::io::gltf gltf, object_handle parent);
 		asset_package add_gltf(tz::io::gltf gltf, object_handle parent, override_package opkg);
+		void remove_objects(asset_package pkg, transform_hierarchy::remove_strategy strategy);
 
 		std::size_t get_animation_count(const asset_package& pkg) const;
 		std::optional<std::size_t> get_playing_animation(const asset_package& pkg) const;
@@ -126,6 +127,7 @@ namespace tz::ren
 
 		std::vector<gltf_info> gltfs = {};
 		std::vector<object_extra_info> object_extras = {};
+		std::vector<gltf_handle> gltf_free_list = {};
 	};
 }
 

src/tz/ren/mesh.cpp

@@ -171,10 +171,22 @@ namespace tz::ren
 		this->free_list.push_back(oh);
 		// set to front mesh (which needs to be a null locator);
 		this->compute_pass.get_draw_list_meshes()[hanval] = {};
+
+		// we rely on the object tree keeping children alive until its their turn to die
 		auto maybe_node = this->object_tree.find_node(hanval);
 		if(maybe_node.has_value())
 		{
-			this->object_tree.remove_node(maybe_node.value(), strategy);
+			// for that reason, keep the children intact and recurse manually. hence use of impl_do_nothing
+			if(strategy == transform_hierarchy::remove_strategy::remove_children)
+			{
+				auto this_node = this->object_tree.get_node(maybe_node.value());	
+				this->object_tree.remove_node(maybe_node.value(), transform_hierarchy::remove_strategy::impl_do_nothing);
+				for(unsigned int childid : this_node.children)
+				{
+					auto child = this->object_tree.get_node(childid);
+					this->remove_object(static_cast<tz::hanval>(child.data), strategy);
+				}
+			}
 		}
 	}