Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions editor/src/messages/portfolio/document/document_message_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3908,6 +3908,37 @@ mod document_message_handler_tests {
editor.handle_message(DocumentMessage::CreateEmptyFolder).await;
assert!(true, "Application didn't crash after folder move operation");
}

// Merging nodes whose output isn't wired downstream produces an encapsulating subnetwork with no exports.
// Inspecting it via the Data panel (which splices in a monitor node) must not leave a dangling reference that crashes compilation.
#[tokio::test]
async fn merge_selected_nodes_while_inspecting_does_not_crash() {
let mut editor = EditorTestUtils::create();
editor.new_document().await;
editor.draw_rect(0., 0., 100., 100.).await;

let node_a = editor.create_node_by_name(DefinitionIdentifier::ProtoNode(graphene_std::transform_nodes::transform::IDENTIFIER)).await;
let node_b = editor.create_node_by_name(DefinitionIdentifier::ProtoNode(graphene_std::transform_nodes::transform::IDENTIFIER)).await;
editor.handle_message(NodeGraphMessage::SelectedNodesSet { nodes: vec![node_a, node_b] }).await;
editor.handle_message(NodeGraphMessage::MergeSelectedNodes).await;

let merged = editor.active_document().network_interface.selected_nodes_in_nested_network(&[]).unwrap().0.clone();
assert_eq!(merged.len(), 1, "merge should leave one encapsulating node selected");

// Simulate the Data panel inspecting the merged node, which compiles the graph with a monitor node spliced in
let portfolio = &mut editor.editor.dispatcher.message_handlers.portfolio_message_handler;
let document_id = portfolio.active_document_id.unwrap();
let document = portfolio.documents.get_mut(&document_id).unwrap();
portfolio
.executor
.submit_node_graph_evaluation(document, document_id, glam::UVec2::ONE, 1., Default::default(), merged, true, DVec2::ZERO)
.unwrap();
editor.runtime.run().await;

let mut messages = VecDeque::new();
editor.editor.poll_node_graph_evaluation(&mut messages).unwrap();
}

#[tokio::test]
async fn test_moving_folder_with_children() {
let mut editor = EditorTestUtils::create();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -838,11 +838,17 @@ impl<'a> MessageHandler<NodeGraphMessage, NodeGraphMessageContext<'a>> for NodeG
let currently_is_node = !network_interface.is_layer(&node_id, breadcrumb_network_path);
let can_be_layer = network_interface.is_eligible_to_be_layer(&node_id, breadcrumb_network_path);

let selected_nodes = network_interface.selected_nodes_in_nested_network(selection_network_path);
let is_clicked_selected = selected_nodes.as_ref().is_some_and(|selected| selected.selected_nodes().any(|id| *id == node_id));

// Right-clicking an unselected node selects just it, so selection-based actions like Merge Selected Nodes target it
if !is_clicked_selected {
responses.add(NodeGraphMessage::SelectedNodesSet { nodes: vec![node_id] });
}

// Determine which layers the Lock/Unlock action would affect:
// - If the right-clicked node is in the selection, it affects all selected layers
// - If the right-clicked node is not in the selection, it affects just the right-clicked node
let selected_nodes = network_interface.selected_nodes_in_nested_network(selection_network_path);
let is_clicked_selected = selected_nodes.as_ref().is_some_and(|selected| selected.selected_nodes().any(|id| *id == node_id));
let affected_layer_ids = if is_clicked_selected {
selected_nodes.map(|selected| selected.selected_nodes().copied().filter(|id| network_interface.is_layer(id, selection_network_path)).collect())
} else {
Expand Down
14 changes: 13 additions & 1 deletion editor/src/node_graph_executor/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -609,11 +609,23 @@ impl PartialEq for InspectResult {

impl InspectState {
/// Insert the monitor node alongside the inspect node identified by `inspect_path` (full path from root, last element is the target).
/// Returns `None` if the path is empty or doesn't resolve to a node inside a reachable subnetwork.
/// Returns `None` if the path is empty, doesn't resolve to a node inside a reachable subnetwork, or the target has no
/// flatten-safe primary output to monitor (e.g. an empty merged subnetwork), which would otherwise leave the monitor's
/// input dangling once the subnetwork is flattened away.
pub fn monitor_inspect_node(network: &mut NodeNetwork, inspect_path: &[NodeId]) -> Option<Self> {
let (inspect_node, parent_path) = inspect_path.split_last()?;
let inspect_node = *inspect_node;
let target_network = navigate_to_network_mut(network, parent_path)?;

// A subnetwork's primary output only survives flattening if its first export is a node
let monitorable = match &target_network.nodes.get(&inspect_node)?.implementation {
DocumentNodeImplementation::Network(inner) => matches!(inner.exports.first(), Some(NodeInput::Node { .. })),
_ => true,
};
if !monitorable {
return None;
}

let monitor_id = NodeId::new();

// It is necessary to replace the inputs before inserting the monitor node to avoid changing the input of the new monitor node
Expand Down
Loading