Integrate the node graph as a Node Graph Frame layer type (#812)

* Add node graph frame tool

* Add a brighten

* Use the node graph

* Fix topological_sort

* Update UI

* Add icons for the tool and layer type

* Avoid serde & use bitmaps to improve performance

* Allow serialising a node graph

* Fix missing ..Default::default()

* Fix incorrect comments

* Cache node graph output image

* Suppress no-cycle import warning

Co-authored-by: Keavon Chambers <keavon@keavon.com>

node-graph/graph-craft/Cargo.toml

@@ -14,3 +14,8 @@ num-traits = "0.2"
 borrow_stack = { path = "../borrow_stack" }
 dyn-clone = "1.0"
 rand_chacha = "0.3.1"
+
+[dependencies.serde]
+version = "1.0"
+optional = true
+features = ["derive"]

node-graph/graph-craft/src/document.rs

@@ -28,9 +28,8 @@ fn merge_ids(a: u64, b: u64) -> u64 {
 	hasher.finish()
 }
 
-type Fqn = NodeIdentifier<'static>;
-
-#[derive(Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq)]
+#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 pub struct DocumentNode {
 	pub name: String,
 	pub inputs: Vec<NodeInput>,
@@ -55,9 +54,9 @@ impl DocumentNode {
 		let first = self.inputs.remove(0);
 		if let DocumentNodeImplementation::Unresolved(fqn) = self.implementation {
 			let (input, mut args) = match first {
-				NodeInput::Value(value) => {
+				NodeInput::Value(tagged_value) => {
 					assert_eq!(self.inputs.len(), 0);
-					(ProtoNodeInput::None, ConstructionArgs::Value(value))
+					(ProtoNodeInput::None, ConstructionArgs::Value(tagged_value.to_value()))
 				}
 				NodeInput::Node(id) => (ProtoNodeInput::Node(id), ConstructionArgs::Nodes(vec![])),
 				NodeInput::Network => (ProtoNodeInput::Network, ConstructionArgs::Nodes(vec![])),
@@ -82,10 +81,11 @@ impl DocumentNode {
 	}
 }
 
-#[derive(Debug)]
+#[derive(Clone, Debug)]
+#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 pub enum NodeInput {
 	Node(NodeId),
-	Value(value::Value),
+	Value(value::TaggedValue),
 	Network,
 }
 
@@ -107,13 +107,15 @@ impl PartialEq for NodeInput {
 	}
 }
 
-#[derive(Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq)]
+#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 pub enum DocumentNodeImplementation {
 	Network(NodeNetwork),
-	Unresolved(Fqn),
+	Unresolved(NodeIdentifier),
 }
 
-#[derive(Debug, Default, PartialEq)]
+#[derive(Clone, Debug, Default, PartialEq)]
+#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 pub struct NodeNetwork {
 	pub inputs: Vec<NodeId>,
 	pub output: NodeId,
@@ -160,7 +162,7 @@ impl NodeNetwork {
 							network_input.populate_first_network_input(node, *offset);
 						}
 						NodeInput::Value(value) => {
-							let name = format!("Value: {:?}", value);
+							let name = format!("Value: {:?}", value.clone().to_value());
 							let new_id = map_ids(id, gen_id());
 							let value_node = DocumentNode {
 								name: name.clone(),
@@ -284,7 +286,7 @@ mod test {
 				1,
 				DocumentNode {
 					name: "Inc".into(),
-					inputs: vec![NodeInput::Network, NodeInput::Value(2_u32.into_any())],
+					inputs: vec![NodeInput::Network, NodeInput::Value(value::TaggedValue::U32(2))],
 					implementation: DocumentNodeImplementation::Network(add_network()),
 				},
 			)]
@@ -384,7 +386,7 @@ mod test {
 					14,
 					DocumentNode {
 						name: "Value: 2".into(),
-						inputs: vec![NodeInput::Value(2_u32.into_any())],
+						inputs: vec![NodeInput::Value(value::TaggedValue::U32(2))],
 						implementation: DocumentNodeImplementation::Unresolved(NodeIdentifier::new("graphene_core::value::ValueNode", &[Type::Generic])),
 					},
 				),

node-graph/graph-craft/src/document/value.rs

@@ -4,6 +4,26 @@ use dyn_clone::DynClone;
 
 use dyn_any::{DynAny, Upcast};
 
+/// A type that is known, allowing serialization (serde::Deserialize is not object safe)
+#[derive(Clone, Debug, PartialEq)]
+#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
+pub enum TaggedValue {
+	String(String),
+	U32(u32),
+	//Image(graphene_std::raster::Image),
+	Color(graphene_core::raster::color::Color),
+}
+
+impl TaggedValue {
+	pub fn to_value(self) -> Value {
+		match self {
+			TaggedValue::String(x) => Box::new(x),
+			TaggedValue::U32(x) => Box::new(x),
+			TaggedValue::Color(x) => Box::new(x),
+		}
+	}
+}
+
 pub type Value = Box<dyn ValueTrait>;
 
 pub trait ValueTrait: DynAny<'static> + Upcast<dyn DynAny<'static>> + std::fmt::Debug + DynClone {}

node-graph/graph-craft/src/lib.rs

@@ -11,7 +11,6 @@ mod tests {
 	use graphene_core::value::ValueNode;
 	use graphene_core::{structural::*, RefNode};
 
-	use crate::document::value::IntoValue;
 	use borrow_stack::BorrowStack;
 	use borrow_stack::FixedSizeStack;
 	use dyn_any::{downcast, IntoDynAny};
@@ -68,7 +67,10 @@ mod tests {
 						DocumentNode {
 							name: "cons".into(),
 							inputs: vec![NodeInput::Network, NodeInput::Network],
-							implementation: DocumentNodeImplementation::Unresolved(NodeIdentifier::new("graphene_core::structural::ConsNode", &[Type::Concrete("u32"), Type::Concrete("u32")])),
+							implementation: DocumentNodeImplementation::Unresolved(NodeIdentifier::new(
+								"graphene_core::structural::ConsNode",
+								&[Type::Concrete(std::borrow::Cow::Borrowed("u32")), Type::Concrete(std::borrow::Cow::Borrowed("u32"))],
+							)),
 						},
 					),
 					(
@@ -76,7 +78,10 @@ mod tests {
 						DocumentNode {
 							name: "add".into(),
 							inputs: vec![NodeInput::Node(0)],
-							implementation: DocumentNodeImplementation::Unresolved(NodeIdentifier::new("graphene_core::ops::AddNode", &[Type::Concrete("u32"), Type::Concrete("u32")])),
+							implementation: DocumentNodeImplementation::Unresolved(NodeIdentifier::new(
+								"graphene_core::ops::AddNode",
+								&[Type::Concrete(std::borrow::Cow::Borrowed("u32")), Type::Concrete(std::borrow::Cow::Borrowed("u32"))],
+							)),
 						},
 					),
 				]
@@ -92,7 +97,7 @@ mod tests {
 				0,
 				DocumentNode {
 					name: "Inc".into(),
-					inputs: vec![NodeInput::Network, NodeInput::Value(1_u32.into_any())],
+					inputs: vec![NodeInput::Network, NodeInput::Value(value::TaggedValue::U32(1))],
 					implementation: DocumentNodeImplementation::Network(add_network()),
 				},
 			)]

node-graph/graph-craft/src/node_registry.rs

@@ -1,11 +1,9 @@
-use std::marker::PhantomData;
-
 use borrow_stack::FixedSizeStack;
 use graphene_core::generic::FnNode;
-use graphene_core::ops::{AddNode, IdNode};
+use graphene_core::ops::AddNode;
 use graphene_core::raster::color::Color;
 use graphene_core::structural::{ConsNode, Then};
-use graphene_core::{AsRefNode, Node};
+use graphene_core::Node;
 use graphene_std::any::DowncastBothNode;
 use graphene_std::any::{Any, DowncastNode, DynAnyNode, IntoTypeErasedNode, TypeErasedNode};
 use graphene_std::raster::Image;
@@ -13,17 +11,12 @@ use graphene_std::raster::Image;
 use crate::proto::Type;
 use crate::proto::{ConstructionArgs, NodeIdentifier, ProtoNode, ProtoNodeInput, Type::Concrete};
 
-use dyn_any::Upcast;
-
 type NodeConstructor = fn(ProtoNode, &FixedSizeStack<TypeErasedNode<'static>>);
 
 //TODO: turn into hasmap
-static NODE_REGISTRY: &[(NodeIdentifier<'static>, NodeConstructor)] = &[
+static NODE_REGISTRY: &[(NodeIdentifier, NodeConstructor)] = &[
 	(
-		NodeIdentifier {
-			name: "graphene_core::ops::IdNode",
-			types: &[Concrete("Any<'_>")],
-		},
+		NodeIdentifier::new("graphene_core::ops::IdNode", &[Concrete(std::borrow::Cow::Borrowed("Any<'_>"))]),
 		|proto_node, stack| {
 			stack.push_fn(|nodes| {
 				let pre_node = nodes.get(proto_node.input.unwrap_node() as usize).unwrap();
@@ -32,24 +25,18 @@ static NODE_REGISTRY: &[(NodeIdentifier<'static>, NodeConstructor)] = &[
 			})
 		},
 	),
+	(NodeIdentifier::new("graphene_core::ops::IdNode", &[Type::Generic]), |proto_node, stack| {
+		stack.push_fn(|nodes| {
+			let pre_node = nodes.get(proto_node.input.unwrap_node() as usize).unwrap();
+			let node = pre_node.then(graphene_core::ops::IdNode);
+			node.into_type_erased()
+		})
+	}),
 	(
-		NodeIdentifier {
-			name: "graphene_core::ops::IdNode",
-			types: &[Type::Generic],
-		},
-		|proto_node, stack| {
-			stack.push_fn(|nodes| {
-				let pre_node = nodes.get(proto_node.input.unwrap_node() as usize).unwrap();
-				let node = pre_node.then(graphene_core::ops::IdNode);
-				node.into_type_erased()
-			})
-		},
-	),
-	(
-		NodeIdentifier {
-			name: "graphene_core::ops::AddNode",
-			types: &[Concrete("u32"), Concrete("u32")],
-		},
+		NodeIdentifier::new(
+			"graphene_core::ops::AddNode",
+			&[Concrete(std::borrow::Cow::Borrowed("u32")), Concrete(std::borrow::Cow::Borrowed("u32"))],
+		),
 		|proto_node, stack| {
 			stack.push_fn(|nodes| {
 				let pre_node = nodes.get(proto_node.input.unwrap_node() as usize).unwrap();
@@ -61,10 +48,10 @@ static NODE_REGISTRY: &[(NodeIdentifier<'static>, NodeConstructor)] = &[
 		},
 	),
 	(
-		NodeIdentifier {
-			name: "graphene_core::ops::AddNode",
-			types: &[Concrete("&u32"), Concrete("&u32")],
-		},
+		NodeIdentifier::new(
+			"graphene_core::ops::AddNode",
+			&[Concrete(std::borrow::Cow::Borrowed("&u32")), Concrete(std::borrow::Cow::Borrowed("&u32"))],
+		),
 		|proto_node, stack| {
 			stack.push_fn(|nodes| {
 				let pre_node = nodes.get(proto_node.input.unwrap_node() as usize).unwrap();
@@ -76,10 +63,10 @@ static NODE_REGISTRY: &[(NodeIdentifier<'static>, NodeConstructor)] = &[
 		},
 	),
 	(
-		NodeIdentifier {
-			name: "graphene_core::ops::AddNode",
-			types: &[Concrete("&u32"), Concrete("u32")],
-		},
+		NodeIdentifier::new(
+			"graphene_core::ops::AddNode",
+			&[Concrete(std::borrow::Cow::Borrowed("&u32")), Concrete(std::borrow::Cow::Borrowed("u32"))],
+		),
 		|proto_node, stack| {
 			stack.push_fn(|nodes| {
 				let pre_node = nodes.get(proto_node.input.unwrap_node() as usize).unwrap();
@@ -91,10 +78,10 @@ static NODE_REGISTRY: &[(NodeIdentifier<'static>, NodeConstructor)] = &[
 		},
 	),
 	(
-		NodeIdentifier {
-			name: "graphene_core::structural::ConsNode",
-			types: &[Concrete("&u32"), Concrete("u32")],
-		},
+		NodeIdentifier::new(
+			"graphene_core::structural::ConsNode",
+			&[Concrete(std::borrow::Cow::Borrowed("&u32")), Concrete(std::borrow::Cow::Borrowed("u32"))],
+		),
 		|proto_node, stack| {
 			if let ConstructionArgs::Nodes(cons_node_arg) = proto_node.construction_args {
 				stack.push_fn(move |nodes| {
@@ -118,10 +105,10 @@ static NODE_REGISTRY: &[(NodeIdentifier<'static>, NodeConstructor)] = &[
 		},
 	),
 	(
-		NodeIdentifier {
-			name: "graphene_core::structural::ConsNode",
-			types: &[Concrete("u32"), Concrete("u32")],
-		},
+		NodeIdentifier::new(
+			"graphene_core::structural::ConsNode",
+			&[Concrete(std::borrow::Cow::Borrowed("u32")), Concrete(std::borrow::Cow::Borrowed("u32"))],
+		),
 		|proto_node, stack| {
 			if let ConstructionArgs::Nodes(cons_node_arg) = proto_node.construction_args {
 				stack.push_fn(move |nodes| {
@@ -146,10 +133,10 @@ static NODE_REGISTRY: &[(NodeIdentifier<'static>, NodeConstructor)] = &[
 	),
 	// TODO: create macro to impl for all types
 	(
-		NodeIdentifier {
-			name: "graphene_core::structural::ConsNode",
-			types: &[Concrete("&u32"), Concrete("&u32")],
-		},
+		NodeIdentifier::new(
+			"graphene_core::structural::ConsNode",
+			&[Concrete(std::borrow::Cow::Borrowed("&u32")), Concrete(std::borrow::Cow::Borrowed("&u32"))],
+		),
 		|proto_node, stack| {
 			let node_id = proto_node.input.unwrap_node() as usize;
 			if let ConstructionArgs::Nodes(cons_node_arg) = proto_node.construction_args {
@@ -168,10 +155,7 @@ static NODE_REGISTRY: &[(NodeIdentifier<'static>, NodeConstructor)] = &[
 		},
 	),
 	(
-		NodeIdentifier {
-			name: "graphene_core::any::DowncastNode",
-			types: &[Concrete("&u32")],
-		},
+		NodeIdentifier::new("graphene_core::any::DowncastNode", &[Concrete(std::borrow::Cow::Borrowed("&u32"))]),
 		|proto_node, stack| {
 			stack.push_fn(|nodes| {
 				let pre_node = nodes.get(proto_node.input.unwrap_node() as usize).unwrap();
@@ -181,10 +165,7 @@ static NODE_REGISTRY: &[(NodeIdentifier<'static>, NodeConstructor)] = &[
 		},
 	),
 	(
-		NodeIdentifier {
-			name: "graphene_core::value::ValueNode",
-			types: &[Concrete("Any<'_>")],
-		},
+		NodeIdentifier::new("graphene_core::value::ValueNode", &[Concrete(std::borrow::Cow::Borrowed("Any<'_>"))]),
 		|proto_node, stack| {
 			stack.push_fn(|_nodes| {
 				if let ConstructionArgs::Value(value) = proto_node.construction_args {
@@ -197,69 +178,48 @@ static NODE_REGISTRY: &[(NodeIdentifier<'static>, NodeConstructor)] = &[
 			})
 		},
 	),
-	(
-		NodeIdentifier {
-			name: "graphene_core::value::ValueNode",
-			types: &[Type::Generic],
-		},
-		|proto_node, stack| {
-			stack.push_fn(|_nodes| {
-				if let ConstructionArgs::Value(value) = proto_node.construction_args {
-					let node = FnNode::new(move |_| value.clone().up_box() as Any<'static>);
-					node.into_type_erased()
-				} else {
-					unreachable!()
-				}
-			})
-		},
-	),
-	(
-		NodeIdentifier {
-			name: "graphene_core::raster::GrayscaleNode",
-			types: &[],
-		},
-		|proto_node, stack| {
-			stack.push_fn(|nodes| {
-				let node = DynAnyNode::new(graphene_core::raster::GrayscaleNode);
-
-				if let ProtoNodeInput::Node(pre_id) = proto_node.input {
-					let pre_node = nodes.get(pre_id as usize).unwrap();
-					(pre_node).then(node).into_type_erased()
-				} else {
-					node.into_type_erased()
-				}
-			})
-		},
-	),
-	(
-		NodeIdentifier {
-			name: "graphene_std::raster::MapImageNode",
-			types: &[],
-		},
-		|proto_node, stack| {
-			let node_id = proto_node.input.unwrap_node() as usize;
-			if let ConstructionArgs::Nodes(operation_node_id) = proto_node.construction_args {
-				stack.push_fn(move |nodes| {
-					let pre_node = nodes.get(node_id).unwrap();
-
-					let operation_node = nodes.get(operation_node_id[0] as usize).unwrap();
-					let operation_node: DowncastBothNode<_, Color, Color> = DowncastBothNode::new(operation_node);
-					let map_node = DynAnyNode::new(graphene_std::raster::MapImageNode::new(operation_node));
-
-					let node = (pre_node).then(map_node);
-
-					node.into_type_erased()
-				})
+	(NodeIdentifier::new("graphene_core::value::ValueNode", &[Type::Generic]), |proto_node, stack| {
+		stack.push_fn(|_nodes| {
+			if let ConstructionArgs::Value(value) = proto_node.construction_args {
+				let node = FnNode::new(move |_| value.clone().up_box() as Any<'static>);
+				node.into_type_erased()
 			} else {
-				unimplemented!()
+				unreachable!()
 			}
-		},
-	),
+		})
+	}),
+	(NodeIdentifier::new("graphene_core::raster::GrayscaleNode", &[]), |proto_node, stack| {
+		stack.push_fn(|nodes| {
+			let node = DynAnyNode::new(graphene_core::raster::GrayscaleNode);
+
+			if let ProtoNodeInput::Node(pre_id) = proto_node.input {
+				let pre_node = nodes.get(pre_id as usize).unwrap();
+				(pre_node).then(node).into_type_erased()
+			} else {
+				node.into_type_erased()
+			}
+		})
+	}),
+	(NodeIdentifier::new("graphene_std::raster::MapImageNode", &[]), |proto_node, stack| {
+		if let ConstructionArgs::Nodes(operation_node_id) = proto_node.construction_args {
+			stack.push_fn(move |nodes| {
+				let operation_node = nodes.get(operation_node_id[0] as usize).unwrap();
+				let operation_node: DowncastBothNode<_, Color, Color> = DowncastBothNode::new(operation_node);
+				let map_node = DynAnyNode::new(graphene_std::raster::MapImageNode::new(operation_node));
+
+				if let ProtoNodeInput::Node(node_id) = proto_node.input {
+					let pre_node = nodes.get(node_id as usize).unwrap();
+					(pre_node).then(map_node).into_type_erased()
+				} else {
+					map_node.into_type_erased()
+				}
+			})
+		} else {
+			unimplemented!()
+		}
+	}),
 	(
-		NodeIdentifier {
-			name: "graphene_std::raster::ImageNode",
-			types: &[Concrete("&str")],
-		},
+		NodeIdentifier::new("graphene_std::raster::ImageNode", &[Concrete(std::borrow::Cow::Borrowed("&str"))]),
 		|_proto_node, stack| {
 			stack.push_fn(|_nodes| {
 				let image = FnNode::new(|s: &str| graphene_std::raster::image_node::<&str>().eval(s).unwrap());
@@ -269,10 +229,7 @@ static NODE_REGISTRY: &[(NodeIdentifier<'static>, NodeConstructor)] = &[
 		},
 	),
 	(
-		NodeIdentifier {
-			name: "graphene_std::raster::ExportImageNode",
-			types: &[Concrete("&str")],
-		},
+		NodeIdentifier::new("graphene_std::raster::ExportImageNode", &[Concrete(std::borrow::Cow::Borrowed("&str"))]),
 		|proto_node, stack| {
 			stack.push_fn(|nodes| {
 				let pre_node = nodes.get(proto_node.input.unwrap_node() as usize).unwrap();
@@ -285,10 +242,10 @@ static NODE_REGISTRY: &[(NodeIdentifier<'static>, NodeConstructor)] = &[
 		},
 	),
 	(
-		NodeIdentifier {
-			name: "graphene_core::structural::ConsNode",
-			types: &[Concrete("Image"), Concrete("&str")],
-		},
+		NodeIdentifier::new(
+			"graphene_core::structural::ConsNode",
+			&[Concrete(std::borrow::Cow::Borrowed("Image")), Concrete(std::borrow::Cow::Borrowed("&str"))],
+		),
 		|proto_node, stack| {
 			let node_id = proto_node.input.unwrap_node() as usize;
 			if let ConstructionArgs::Nodes(cons_node_arg) = proto_node.construction_args {
@@ -322,11 +279,6 @@ mod protograph_testing {
 
 	use super::*;
 
-	/// Lookup a node by th suffix of the name (for testing only)
-	fn simple_lookup(suffix: &str) -> &(NodeIdentifier, fn(ProtoNode, &FixedSizeStack<TypeErasedNode<'static>>)) {
-		NODE_REGISTRY.iter().find(|node| node.0.name.ends_with(suffix)).unwrap()
-	}
-
 	#[test]
 	fn add_values() {
 		let stack = FixedSizeStack::new(256);
@@ -339,14 +291,20 @@ mod protograph_testing {
 		let cons_protonode = ProtoNode {
 			construction_args: ConstructionArgs::Nodes(vec![1]),
 			input: ProtoNodeInput::Node(0),
-			identifier: NodeIdentifier::new("graphene_core::structural::ConsNode", &[Concrete("u32"), Concrete("u32")]),
+			identifier: NodeIdentifier::new(
+				"graphene_core::structural::ConsNode",
+				&[Concrete(std::borrow::Cow::Borrowed("u32")), Concrete(std::borrow::Cow::Borrowed("u32"))],
+			),
 		};
 		push_node(cons_protonode, &stack);
 
 		let add_protonode = ProtoNode {
 			construction_args: ConstructionArgs::Nodes(vec![]),
 			input: ProtoNodeInput::Node(2),
-			identifier: NodeIdentifier::new("graphene_core::ops::AddNode", &[Concrete("u32"), Concrete("u32")]),
+			identifier: NodeIdentifier::new(
+				"graphene_core::ops::AddNode",
+				&[Concrete(std::borrow::Cow::Borrowed("u32")), Concrete(std::borrow::Cow::Borrowed("u32"))],
+			),
 		};
 		push_node(add_protonode, &stack);
 
@@ -379,7 +337,7 @@ mod protograph_testing {
 		let image_protonode = ProtoNode {
 			construction_args: ConstructionArgs::Nodes(vec![]),
 			input: ProtoNodeInput::None,
-			identifier: NodeIdentifier::new("graphene_std::raster::ImageNode", &[Concrete("&str")]),
+			identifier: NodeIdentifier::new("graphene_std::raster::ImageNode", &[Concrete(std::borrow::Cow::Borrowed("&str"))]),
 		};
 		push_node(image_protonode, &stack);
 
@@ -394,7 +352,7 @@ mod protograph_testing {
 		let image_protonode = ProtoNode {
 			construction_args: ConstructionArgs::Nodes(vec![]),
 			input: ProtoNodeInput::None,
-			identifier: NodeIdentifier::new("graphene_std::raster::ImageNode", &[Concrete("&str")]),
+			identifier: NodeIdentifier::new("graphene_std::raster::ImageNode", &[Concrete(std::borrow::Cow::Borrowed("&str"))]),
 		};
 		push_node(image_protonode, &stack);
 

node-graph/graph-craft/src/proto.rs

@@ -1,35 +1,49 @@
 use std::collections::HashMap;
-use std::collections::HashSet;
 
 use crate::document::value;
 use crate::document::NodeId;
 
-#[derive(Debug, PartialEq)]
-pub struct NodeIdentifier<'a> {
-	pub name: &'a str,
-	pub types: &'a [Type<'a>],
+#[derive(Clone, Debug, PartialEq)]
+#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
+pub struct NodeIdentifier {
+	pub name: std::borrow::Cow<'static, str>,
+	pub types: std::borrow::Cow<'static, [Type]>,
 }
 
-#[derive(Debug, PartialEq)]
-pub enum Type<'a> {
+#[derive(Clone, Debug, PartialEq)]
+#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
+pub enum Type {
 	Generic,
-	Concrete(&'a str),
+	Concrete(std::borrow::Cow<'static, str>),
 }
 
-impl<'a> From<&'a str> for Type<'a> {
-	fn from(s: &'a str) -> Self {
-		Type::Concrete(s)
-	}
-}
-impl<'a> From<&'a str> for NodeIdentifier<'a> {
-	fn from(s: &'a str) -> Self {
-		NodeIdentifier { name: s, types: &[] }
+impl From<&'static str> for Type {
+	fn from(s: &'static str) -> Self {
+		Type::Concrete(std::borrow::Cow::Borrowed(s))
 	}
 }
 
-impl<'a> NodeIdentifier<'a> {
-	pub fn new(name: &'a str, types: &'a [Type<'a>]) -> Self {
-		NodeIdentifier { name, types }
+impl Type {
+	pub const fn from_str(concrete: &'static str) -> Self {
+		Type::Concrete(std::borrow::Cow::Borrowed(concrete))
+	}
+}
+
+impl From<&'static str> for NodeIdentifier {
+	fn from(s: &'static str) -> Self {
+		NodeIdentifier {
+			name: std::borrow::Cow::Borrowed(s),
+			types: std::borrow::Cow::Borrowed(&[]),
+		}
+	}
+}
+
+impl NodeIdentifier {
+	pub const fn new(name: &'static str, types: &'static [Type]) -> Self {
+		NodeIdentifier {
+			name: std::borrow::Cow::Borrowed(name),
+			types: std::borrow::Cow::Borrowed(types),
+		}
 	}
 }
 
@@ -60,7 +74,7 @@ impl PartialEq for ConstructionArgs {
 pub struct ProtoNode {
 	pub construction_args: ConstructionArgs,
 	pub input: ProtoNodeInput,
-	pub identifier: NodeIdentifier<'static>,
+	pub identifier: NodeIdentifier,
 }
 
 #[derive(Debug, Default, PartialEq, Eq)]
@@ -83,10 +97,7 @@ impl ProtoNodeInput {
 impl ProtoNode {
 	pub fn value(value: ConstructionArgs) -> Self {
 		Self {
-			identifier: NodeIdentifier {
-				name: "graphene_core::value::ValueNode",
-				types: &[Type::Generic],
-			},
+			identifier: NodeIdentifier::new("graphene_core::value::ValueNode", &[Type::Generic]),
 			construction_args: value,
 			input: ProtoNodeInput::None,
 		}
@@ -110,7 +121,22 @@ impl ProtoNode {
 }
 
 impl ProtoNetwork {
-	fn reverse_edges(&self) -> HashMap<NodeId, Vec<NodeId>> {
+	pub fn collect_outwards_edges(&self) -> HashMap<NodeId, Vec<NodeId>> {
+		let mut edges: HashMap<NodeId, Vec<NodeId>> = HashMap::new();
+		for (id, node) in &self.nodes {
+			if let ProtoNodeInput::Node(ref_id) = &node.input {
+				edges.entry(*ref_id).or_default().push(*id)
+			}
+			if let ConstructionArgs::Nodes(ref_nodes) = &node.construction_args {
+				for ref_id in ref_nodes {
+					edges.entry(*ref_id).or_default().push(*id)
+				}
+			}
+		}
+		edges
+	}
+
+	pub fn collect_inwards_edges(&self) -> HashMap<NodeId, Vec<NodeId>> {
 		let mut edges: HashMap<NodeId, Vec<NodeId>> = HashMap::new();
 		for (id, node) in &self.nodes {
 			if let ProtoNodeInput::Node(ref_id) = &node.input {
@@ -125,38 +151,28 @@ impl ProtoNetwork {
 		edges
 	}
 
+	// Based on https://en.wikipedia.org/wiki/Topological_sorting#Kahn's_algorithm
 	pub fn topological_sort(&self) -> Vec<NodeId> {
-		let mut visited = HashSet::new();
-		let mut stack = Vec::new();
 		let mut sorted = Vec::new();
-		let graph = self.reverse_edges();
-		// TODO: remove
-		println!("{:#?}", graph);
+		let outwards_edges = self.collect_outwards_edges();
+		let mut inwards_edges = self.collect_inwards_edges();
+		let mut no_incoming_edges: Vec<_> = self.nodes.iter().map(|entry| entry.0).filter(|id| !inwards_edges.contains_key(id)).collect();
 
-		for (id, _) in &self.nodes {
-			if !visited.contains(id) {
-				stack.push(*id);
+		assert_ne!(no_incoming_edges.len(), 0, "Acyclic graphs must have at least one node with no incoming edge");
 
-				while let Some(id) = stack.pop() {
-					//TODO remove
-					println!("{:?}", stack);
-					if !visited.contains(&id) {
-						visited.insert(id);
-						if let Some(refs) = graph.get(&id) {
-							for ref_id in refs {
-								if !visited.contains(ref_id) {
-									stack.push(id);
-									stack.push(*ref_id);
-									break;
-								}
-							}
-						}
-						sorted.push(id);
+		while let Some(node_id) = no_incoming_edges.pop() {
+			sorted.push(node_id);
+
+			if let Some(outwards_edges) = outwards_edges.get(&node_id) {
+				for &ref_id in outwards_edges {
+					let dependencies = inwards_edges.get_mut(&ref_id).unwrap();
+					dependencies.retain(|&id| id != node_id);
+					if dependencies.is_empty() {
+						no_incoming_edges.push(ref_id)
 					}
 				}
 			}
 		}
-		sorted.reverse();
 		sorted
 	}