Derive node ids for resources and animation curves deterministically

editor/src/node_graph_executor.rs

@@ -22,6 +22,8 @@ use std::any::Any;
 use std::sync::Arc;
 use std::sync::mpsc::{Receiver, Sender};
 
+#[cfg(test)]
+mod recompile_tests;
 mod runtime;
 pub use runtime::*;
 

editor/src/node_graph_executor/recompile_tests.rs

@@ -0,0 +1,56 @@
+use crate::messages::portfolio::document::utility_types::network_interface::FlowType;
+use crate::test_utils::test_prelude::*;
+use glam::UVec2;
+use graph_craft::document::NodeId;
+use graphene_std::application_io::TimingInformation;
+use std::collections::HashSet;
+
+async fn compile_and_run(editor: &mut EditorTestUtils) {
+	let portfolio = editor.portfolio_message_handler_mut();
+	let document_id = portfolio.active_document_id.unwrap();
+	let (executor, documents) = (&mut portfolio.executor, &mut portfolio.documents);
+	let document = documents.get_mut(&document_id).unwrap();
+	executor
+		.submit_node_graph_evaluation(document, document_id, UVec2::ONE, 1., TimingInformation::default(), Vec::new(), true, DVec2::ZERO)
+		.unwrap();
+	editor.runtime.run().await;
+	editor.dispatch(Message::Wake);
+}
+
+/// The proto node IDs compiled for the document node paths that pass `keep`. The executor keeps the nodes an update drops
+/// for one more update, so a first update clears those of the test harness's instrumented graph.
+async fn compiled_ids(editor: &mut EditorTestUtils, keep: impl Fn(&[NodeId]) -> bool) -> HashSet<NodeId> {
+	compile_and_run(editor).await;
+	compile_and_run(editor).await;
+	let source_map = editor.runtime.executor.tree().source_map();
+	source_map.iter().filter(|(path, _)| keep(path)).map(|(_, (id, _))| *id).collect()
+}
+
+#[tokio::test]
+async fn recompiling_keeps_the_ids_of_an_image_chain() {
+	let mut editor = EditorTestUtils::create();
+	editor.new_document().await;
+	editor.create_raster_image(Image::new(16, 16, Color::BLUE), Some((0., 0.))).await;
+	let layer = editor.get_selected_layer().await.expect("the dropped image should be selected").to_node();
+
+	// The nodes feeding the image layer's content, which the resource node feeds
+	let network_interface = &editor.active_document().network_interface;
+	let chain: Vec<_> = network_interface
+		.upstream_flow_back_from_nodes(vec![layer], &[], FlowType::HorizontalFlow)
+		.filter(|&node| node != layer)
+		.collect();
+	let in_chain = |path: &[NodeId]| path.iter().any(|node| chain.contains(node));
+
+	let all = compiled_ids(&mut editor, |_| true).await;
+	assert_eq!(compiled_ids(&mut editor, |_| true).await, all, "compiling the same document again changed its proto node IDs");
+	let image_chain = compiled_ids(&mut editor, in_chain).await;
+	assert!(!image_chain.is_empty(), "the image layer has no compiled content");
+
+	// The new layer above takes the image layer as its input, a change downstream of the image
+	editor.draw_rect(0., 0., 10., 10.).await;
+	assert_eq!(
+		compiled_ids(&mut editor, in_chain).await,
+		image_chain,
+		"an edit downstream of the image changed the image chain's proto node IDs"
+	);
+}

node-graph/preprocessor/src/lib.rs

@@ -13,6 +13,16 @@ use graphene_std::*;
 use std::collections::{HashMap, HashSet};
 use std::hash::{DefaultHasher, Hash, Hasher};
 
+// Derives the ids for generated nodes from their contents / context to be deterministic arcoss graph compilations.
+// The identifier is needed such that different nodes with the same hash bytes get different node ids.
+fn producer_node_id(ident: &ProtoNodeIdentifier, key: impl Hash) -> NodeId {
+	let mut hasher = DefaultHasher::new();
+	"preprocessor producer".hash(&mut hasher);
+	ident.hash(&mut hasher);
+	key.hash(&mut hasher);
+	NodeId(hasher.finish())
+}
+
 #[derive(Debug, Clone)]
 struct Substitution {
 	node: DocumentNode,
@@ -53,13 +63,13 @@ impl Preprocessor {
 		let mut hash_to_node_id: HashMap<ResourceHash, NodeId> = HashMap::new();
 		let mut new_nodes: Vec<(NodeId, DocumentNode)> = Vec::new();
 
-		for node in network.nodes.values_mut() {
+		for (&consumer_id, node) in &mut network.nodes {
 			if let DocumentNodeImplementation::Network(nested) = &mut node.implementation {
 				self.replace_inputs_with_producer_nodes(nested, resolve_resource)?;
 				continue;
 			}
 
-			for input in node.inputs.iter_mut() {
+			for (input_index, input) in node.inputs.iter_mut().enumerate() {
 				let NodeInput::Value { tagged_value, .. } = input else { continue };
 
 				let node_id = match &**tagged_value {
@@ -70,7 +80,7 @@ impl Preprocessor {
 						};
 
 						*hash_to_node_id.entry(hash).or_insert_with(|| {
-							let id = NodeId::new();
+							let id = producer_node_id(&platform_application_io::editor_api::IDENTIFIER, resource_id);
 							let resource_node = DocumentNode {
 								inputs: vec![
 									NodeInput::scope(platform_application_io::editor_api::IDENTIFIER),
@@ -84,7 +94,7 @@ impl Preprocessor {
 						})
 					}
 					TaggedValue::AnimationCurve(curve) => {
-						let id = NodeId::new();
+						let id = producer_node_id(&graphene_core::animation::evaluate_curve::IDENTIFIER, (consumer_id, input_index));
 						let curve_node = DocumentNode {
 							inputs: vec![NodeInput::value(TaggedValue::None, false), NodeInput::value(TaggedValue::AnimationCurve(curve.clone()), false)],
 							implementation: DocumentNodeImplementation::ProtoNode(graphene_core::animation::evaluate_curve::IDENTIFIER),