From 7cd3323c5d5eea8a6358df20d314e4a5ce2685ad Mon Sep 17 00:00:00 2001 From: HarshithJeevan-pixel Date: Sun, 1 Nov 2026 20:06:08 +0530 Subject: [PATCH 1/3] fix(graph): prevent crash when previewing node without outputs (#4883) --- .../node_graph/node_graph_message_handler.rs | 3 +- .../characterization_tests.rs | 15 ++++++ .../network_interface/mutations.rs | 4 +- editor/src/node_graph_executor/runtime.rs | 5 ++ node-graph/graph-craft/src/document.rs | 50 +++++++++++++++---- 5 files changed, 65 insertions(+), 12 deletions(-) diff --git a/editor/src/messages/portfolio/document/node_graph/node_graph_message_handler.rs b/editor/src/messages/portfolio/document/node_graph/node_graph_message_handler.rs index 8f82092bfe9..ebe67b8aacd 100644 --- a/editor/src/messages/portfolio/document/node_graph/node_graph_message_handler.rs +++ b/editor/src/messages/portfolio/document/node_graph/node_graph_message_handler.rs @@ -3410,7 +3410,8 @@ impl NodeGraphMessageHandler { .exports .iter() .any(|export| matches!(export, NodeInput::Node { node_id: export_node_id, .. } if *export_node_id == node_id)); - if selection_is_not_already_the_output && no_other_selections { + let has_outputs = network_interface.number_of_outputs(&node_id, breadcrumb_network_path) > 0; + if selection_is_not_already_the_output && no_other_selections && has_outputs { let button = TextButton::new("Preview") .icon("FrameAll") .tooltip_label("Preview") diff --git a/editor/src/messages/portfolio/document/utility_types/network_interface/characterization_tests.rs b/editor/src/messages/portfolio/document/utility_types/network_interface/characterization_tests.rs index aed5c315d3d..d19aaf5d26e 100644 --- a/editor/src/messages/portfolio/document/utility_types/network_interface/characterization_tests.rs +++ b/editor/src/messages/portfolio/document/utility_types/network_interface/characterization_tests.rs @@ -3534,3 +3534,18 @@ async fn a_session_preview_of_a_removed_node_is_dropped() { "The evaluated network should fall back to the document's own export" ); } + +#[tokio::test] +async fn cannot_preview_node_without_outputs() { + let mut editor = EditorTestUtils::create(); + editor.new_document().await; + + let custom_node = editor.create_node_by_name_at(DefinitionIdentifier::Network("Custom Node".to_string()), 0, 0).await; + let network_interface = &mut editor.active_document_mut().network_interface; + + assert_eq!(network_interface.number_of_outputs(&custom_node, &[]), 0); + + // Attempting to toggle preview on a node without outputs should not enable previewing + network_interface.toggle_preview(custom_node, &[]); + assert_eq!(network_interface.previewing(&[]), Previewing::No); +} diff --git a/editor/src/messages/portfolio/document/utility_types/network_interface/mutations.rs b/editor/src/messages/portfolio/document/utility_types/network_interface/mutations.rs index 8afa13d2d02..ad000c7f85a 100644 --- a/editor/src/messages/portfolio/document/utility_types/network_interface/mutations.rs +++ b/editor/src/messages/portfolio/document/utility_types/network_interface/mutations.rs @@ -1401,8 +1401,10 @@ impl NodeNetworkInterface { let primary_output = RootNode { node_id: toggle_id, output_index: 0 }; let previewing = if self.previewing(network_path).previews(toggle_id) || self.root_node(network_path) == Some(primary_output) { Previewing::No - } else { + } else if self.number_of_outputs(&toggle_id, network_path) > 0 { Previewing::Yes { previewed: primary_output } + } else { + return; }; self.set_previewing(previewing, network_path); } diff --git a/editor/src/node_graph_executor/runtime.rs b/editor/src/node_graph_executor/runtime.rs index 57e7ba3636f..f5edb3de0cb 100644 --- a/editor/src/node_graph_executor/runtime.rs +++ b/editor/src/node_graph_executor/runtime.rs @@ -105,6 +105,11 @@ pub struct GraphUpdate { pub(crate) fn apply_previews(network: &mut NodeNetwork, previewed: &[(Vec, RootNode)]) { for (network_path, previewed) in previewed { let Some(nested) = network.nested_network_mut(network_path) else { continue }; + if let Some(node) = nested.nodes.get(&previewed.node_id) { + if previewed.output_index >= node.implementation.output_count() { + continue; + } + } let Some(export) = nested.exports.first_mut() else { continue }; *export = NodeInput::node(previewed.node_id, previewed.output_index); } diff --git a/node-graph/graph-craft/src/document.rs b/node-graph/graph-craft/src/document.rs index c189b585e26..32d19eaaf2b 100644 --- a/node-graph/graph-craft/src/document.rs +++ b/node-graph/graph-craft/src/document.rs @@ -1308,7 +1308,9 @@ impl NodeNetwork { NodeInput::Node { node_id, output_index } => { nested_node.populate_first_network_input(node_id, output_index, nested_input_index); let input_node = self.nodes.get_mut(&node_id).unwrap_or_else(|| panic!("Unable to find input node {node_id:?}")); - input_node.original_location.dependants[output_index].push(nested_node_id); + if let Some(deps) = input_node.original_location.dependants.get_mut(output_index) { + deps.push(nested_node_id); + } } NodeInput::Import { import_index, .. } => { let parent_input_index = import_index; @@ -1331,8 +1333,9 @@ impl NodeNetwork { for parent_input in &node.inputs { if let NodeInput::Node { node_id, output_index } = *parent_input && let Some(input_node) = self.nodes.get_mut(&node_id) + && let Some(deps) = input_node.original_location.dependants.get_mut(output_index) { - input_node.original_location.dependants[output_index].retain(|&dependant| dependant != id); + deps.retain(|&dependant| dependant != id); } } @@ -1356,12 +1359,16 @@ impl NodeNetwork { } if let Some(new_output_node) = self.nodes.get_mut(node_id) { - for dep in &original_location.dependants[i] { - new_output_node.original_location.dependants[*output_index].push(*dep); + if let Some(dependants) = original_location.dependants.get(i) { + if let Some(new_deps) = new_output_node.original_location.dependants.get_mut(*output_index) { + for dep in dependants { + new_deps.push(*dep); + } + + // Remove references from dependants on wrapper + new_deps.retain(|&dependant| dependant != id); + } } - - // Remove references from dependants on wrapper - new_output_node.original_location.dependants[*output_index].retain(|&dependant| dependant != id); } } @@ -1437,10 +1444,12 @@ impl NodeNetwork { .. } = node.inputs[0] && let Some(upstream_node) = self.nodes.get_mut(&upstream_node_id) + && let Some(dependants_of_upstream) = upstream_node.original_location.dependants.get_mut(upstream_output_index) { - let dependants_of_upstream = &mut upstream_node.original_location.dependants[upstream_output_index]; // Add our dependants to the list of dependants - dependants_of_upstream.extend(node.original_location.dependants[0].iter().copied()); + if let Some(our_dependants) = node.original_location.dependants.first() { + dependants_of_upstream.extend(our_dependants.iter().copied()); + } // Remove ourselves from the list of dependants dependants_of_upstream.retain(|&dependant| dependant != passthrough_id); }; @@ -1453,7 +1462,8 @@ impl NodeNetwork { } // Look at all downstream nodes - for &dependant_id in &node.original_location.dependants[0] { + let downstream_dependants = node.original_location.dependants.first().map(|d| d.as_slice()).unwrap_or_default(); + for &dependant_id in downstream_dependants { let Some(dependant_node) = self.nodes.get_mut(&dependant_id) else { error!("remove_passthrough_node got a dependant id {dependant_id:?} that did not exist in the network\n{self:#?}"); continue; @@ -1840,6 +1850,26 @@ mod test { assert_eq!(expected, network); } + #[test] + fn flatten_node_without_exports() { + let mut network = NodeNetwork { + exports: vec![NodeInput::node(NodeId(1), 0)], + nodes: [( + NodeId(1), + DocumentNode { + inputs: vec![], + implementation: DocumentNodeImplementation::Network(NodeNetwork::default()), + ..Default::default() + }, + )] + .into_iter() + .collect(), + ..Default::default() + }; + network.populate_dependants(); + network.flatten(NodeId(1)); + } + #[test] fn resolve_proto_node_add() { let document_node = DocumentNode { From 29c114bb31fdcb6e37af0bae91f5d637f2787816 Mon Sep 17 00:00:00 2001 From: HarshithJeevan-pixel Date: Sun, 1 Nov 2026 20:22:00 +0530 Subject: [PATCH 2/3] fix(graph): address review feedback on definition output count and regression test --- editor/src/node_graph_executor/runtime.rs | 12 ++++---- node-graph/graph-craft/src/document.rs | 35 ++++++++++++++++++++--- 2 files changed, 38 insertions(+), 9 deletions(-) diff --git a/editor/src/node_graph_executor/runtime.rs b/editor/src/node_graph_executor/runtime.rs index f5edb3de0cb..f5864ac0e71 100644 --- a/editor/src/node_graph_executor/runtime.rs +++ b/editor/src/node_graph_executor/runtime.rs @@ -104,12 +104,14 @@ pub struct GraphUpdate { /// Replaces each network's primary export with its previewed node. pub(crate) fn apply_previews(network: &mut NodeNetwork, previewed: &[(Vec, RootNode)]) { for (network_path, previewed) in previewed { - let Some(nested) = network.nested_network_mut(network_path) else { continue }; - if let Some(node) = nested.nodes.get(&previewed.node_id) { - if previewed.output_index >= node.implementation.output_count() { - continue; - } + let output_count = network + .nested_network(network_path) + .and_then(|nested| nested.nodes.get(&previewed.node_id)) + .map(|node| network.implementation_output_count(&node.implementation)); + if output_count.is_some_and(|count| previewed.output_index >= count) { + continue; } + let Some(nested) = network.nested_network_mut(network_path) else { continue }; let Some(export) = nested.exports.first_mut() else { continue }; *export = NodeInput::node(previewed.node_id, previewed.output_index); } diff --git a/node-graph/graph-craft/src/document.rs b/node-graph/graph-craft/src/document.rs index 32d19eaaf2b..9f74f46516a 100644 --- a/node-graph/graph-craft/src/document.rs +++ b/node-graph/graph-craft/src/document.rs @@ -1851,14 +1851,37 @@ mod test { } #[test] - fn flatten_node_without_exports() { + fn flatten_nested_network_with_missing_dependant_index() { + let inner = NodeNetwork { + exports: vec![NodeInput::node(NodeId(10), 0)], + nodes: [( + NodeId(10), + DocumentNode { + inputs: vec![], + implementation: DocumentNodeImplementation::Network(NodeNetwork::default()), + original_location: OriginalLocation { + dependants: vec![], + ..Default::default() + }, + ..Default::default() + }, + )] + .into_iter() + .collect(), + ..Default::default() + }; + let mut network = NodeNetwork { exports: vec![NodeInput::node(NodeId(1), 0)], nodes: [( NodeId(1), DocumentNode { inputs: vec![], - implementation: DocumentNodeImplementation::Network(NodeNetwork::default()), + implementation: DocumentNodeImplementation::Network(inner), + original_location: OriginalLocation { + dependants: vec![], + ..Default::default() + }, ..Default::default() }, )] @@ -1866,8 +1889,12 @@ mod test { .collect(), ..Default::default() }; - network.populate_dependants(); - network.flatten(NodeId(1)); + + network.flatten_with_fns(NodeId(1), |_, inner_id| inner_id, NodeId::new); + + assert_eq!(network.exports, vec![NodeInput::node(NodeId(10), 0)]); + assert!(network.nodes.contains_key(&NodeId(10))); + assert!(!network.nodes.contains_key(&NodeId(1))); } #[test] From ffa37acc4efcc7bf1d50e69fa8ffa60516d5c4c8 Mon Sep 17 00:00:00 2001 From: HarshithJeevan-pixel Date: Tue, 3 Nov 2026 23:48:39 +0530 Subject: [PATCH 3/3] fix(graph): simplify to targeted fix without compiler changes --- editor/src/node_graph_executor/runtime.rs | 7 --- node-graph/graph-craft/src/document.rs | 77 +++-------------------- 2 files changed, 10 insertions(+), 74 deletions(-) diff --git a/editor/src/node_graph_executor/runtime.rs b/editor/src/node_graph_executor/runtime.rs index f5864ac0e71..57e7ba3636f 100644 --- a/editor/src/node_graph_executor/runtime.rs +++ b/editor/src/node_graph_executor/runtime.rs @@ -104,13 +104,6 @@ pub struct GraphUpdate { /// Replaces each network's primary export with its previewed node. pub(crate) fn apply_previews(network: &mut NodeNetwork, previewed: &[(Vec, RootNode)]) { for (network_path, previewed) in previewed { - let output_count = network - .nested_network(network_path) - .and_then(|nested| nested.nodes.get(&previewed.node_id)) - .map(|node| network.implementation_output_count(&node.implementation)); - if output_count.is_some_and(|count| previewed.output_index >= count) { - continue; - } let Some(nested) = network.nested_network_mut(network_path) else { continue }; let Some(export) = nested.exports.first_mut() else { continue }; *export = NodeInput::node(previewed.node_id, previewed.output_index); diff --git a/node-graph/graph-craft/src/document.rs b/node-graph/graph-craft/src/document.rs index 9f74f46516a..c189b585e26 100644 --- a/node-graph/graph-craft/src/document.rs +++ b/node-graph/graph-craft/src/document.rs @@ -1308,9 +1308,7 @@ impl NodeNetwork { NodeInput::Node { node_id, output_index } => { nested_node.populate_first_network_input(node_id, output_index, nested_input_index); let input_node = self.nodes.get_mut(&node_id).unwrap_or_else(|| panic!("Unable to find input node {node_id:?}")); - if let Some(deps) = input_node.original_location.dependants.get_mut(output_index) { - deps.push(nested_node_id); - } + input_node.original_location.dependants[output_index].push(nested_node_id); } NodeInput::Import { import_index, .. } => { let parent_input_index = import_index; @@ -1333,9 +1331,8 @@ impl NodeNetwork { for parent_input in &node.inputs { if let NodeInput::Node { node_id, output_index } = *parent_input && let Some(input_node) = self.nodes.get_mut(&node_id) - && let Some(deps) = input_node.original_location.dependants.get_mut(output_index) { - deps.retain(|&dependant| dependant != id); + input_node.original_location.dependants[output_index].retain(|&dependant| dependant != id); } } @@ -1359,16 +1356,12 @@ impl NodeNetwork { } if let Some(new_output_node) = self.nodes.get_mut(node_id) { - if let Some(dependants) = original_location.dependants.get(i) { - if let Some(new_deps) = new_output_node.original_location.dependants.get_mut(*output_index) { - for dep in dependants { - new_deps.push(*dep); - } - - // Remove references from dependants on wrapper - new_deps.retain(|&dependant| dependant != id); - } + for dep in &original_location.dependants[i] { + new_output_node.original_location.dependants[*output_index].push(*dep); } + + // Remove references from dependants on wrapper + new_output_node.original_location.dependants[*output_index].retain(|&dependant| dependant != id); } } @@ -1444,12 +1437,10 @@ impl NodeNetwork { .. } = node.inputs[0] && let Some(upstream_node) = self.nodes.get_mut(&upstream_node_id) - && let Some(dependants_of_upstream) = upstream_node.original_location.dependants.get_mut(upstream_output_index) { + let dependants_of_upstream = &mut upstream_node.original_location.dependants[upstream_output_index]; // Add our dependants to the list of dependants - if let Some(our_dependants) = node.original_location.dependants.first() { - dependants_of_upstream.extend(our_dependants.iter().copied()); - } + dependants_of_upstream.extend(node.original_location.dependants[0].iter().copied()); // Remove ourselves from the list of dependants dependants_of_upstream.retain(|&dependant| dependant != passthrough_id); }; @@ -1462,8 +1453,7 @@ impl NodeNetwork { } // Look at all downstream nodes - let downstream_dependants = node.original_location.dependants.first().map(|d| d.as_slice()).unwrap_or_default(); - for &dependant_id in downstream_dependants { + for &dependant_id in &node.original_location.dependants[0] { let Some(dependant_node) = self.nodes.get_mut(&dependant_id) else { error!("remove_passthrough_node got a dependant id {dependant_id:?} that did not exist in the network\n{self:#?}"); continue; @@ -1850,53 +1840,6 @@ mod test { assert_eq!(expected, network); } - #[test] - fn flatten_nested_network_with_missing_dependant_index() { - let inner = NodeNetwork { - exports: vec![NodeInput::node(NodeId(10), 0)], - nodes: [( - NodeId(10), - DocumentNode { - inputs: vec![], - implementation: DocumentNodeImplementation::Network(NodeNetwork::default()), - original_location: OriginalLocation { - dependants: vec![], - ..Default::default() - }, - ..Default::default() - }, - )] - .into_iter() - .collect(), - ..Default::default() - }; - - let mut network = NodeNetwork { - exports: vec![NodeInput::node(NodeId(1), 0)], - nodes: [( - NodeId(1), - DocumentNode { - inputs: vec![], - implementation: DocumentNodeImplementation::Network(inner), - original_location: OriginalLocation { - dependants: vec![], - ..Default::default() - }, - ..Default::default() - }, - )] - .into_iter() - .collect(), - ..Default::default() - }; - - network.flatten_with_fns(NodeId(1), |_, inner_id| inner_id, NodeId::new); - - assert_eq!(network.exports, vec![NodeInput::node(NodeId(10), 0)]); - assert!(network.nodes.contains_key(&NodeId(10))); - assert!(!network.nodes.contains_key(&NodeId(1))); - } - #[test] fn resolve_proto_node_add() { let document_node = DocumentNode {