From caf6f2c3b58299e0c9e7560e496be97252271bb7 Mon Sep 17 00:00:00 2001 From: Keavon Chambers Date: Fri, 10 Jul 2026 04:48:04 -0700 Subject: [PATCH 1/6] Remove commented-out code and fix stale names and doc comments --- .../portfolio/document/document_history.rs | 18 +++++------ .../node_graph/document_node_definitions.rs | 2 +- .../utility_types/network_interface/layout.rs | 32 ------------------- .../network_interface/mutations.rs | 16 ---------- .../network_interface/queries.rs | 15 --------- .../utility_types/network_interface/types.rs | 6 ---- 6 files changed, 10 insertions(+), 79 deletions(-) diff --git a/editor/src/messages/portfolio/document/document_history.rs b/editor/src/messages/portfolio/document/document_history.rs index e1d3ef7812..9758fd3846 100644 --- a/editor/src/messages/portfolio/document/document_history.rs +++ b/editor/src/messages/portfolio/document/document_history.rs @@ -17,9 +17,9 @@ use super::utility_types::network_interface::storage_metadata::{StorageMetadataV #[derivative(Clone, Debug, Default)] pub struct DocumentHistory { /// Stack of document network snapshots for previous history states. - lagacy_undo_stack: VecDeque, + legacy_undo_stack: VecDeque, /// Stack of document network snapshots for future history states. - lagacy_redo_stack: VecDeque, + legacy_redo_stack: VecDeque, /// The `Gdd` working copy: owns the CRDT `Session` and mirrors edits to disk. `None` until the mount /// future built by `load_document` resolves. #[derivative(Debug = "ignore")] @@ -31,38 +31,38 @@ impl DocumentHistory { /// Push a snapshot onto the undo stack, evicting the oldest entry past the history cap. pub fn push_undo(&mut self, snapshot: NodeNetworkInterface) { - Self::push_capped(&mut self.lagacy_undo_stack, snapshot); + Self::push_capped(&mut self.legacy_undo_stack, snapshot); } /// Push a snapshot onto the redo stack, evicting the oldest entry past the history cap. pub fn push_redo(&mut self, snapshot: NodeNetworkInterface) { - Self::push_capped(&mut self.lagacy_redo_stack, snapshot); + Self::push_capped(&mut self.legacy_redo_stack, snapshot); } /// Pop the most recent undo snapshot, or `None` when the stack is empty. pub fn pop_undo(&mut self) -> Option { - self.lagacy_undo_stack.pop_back() + self.legacy_undo_stack.pop_back() } /// Pop the most recent redo snapshot, or `None` when the stack is empty. pub fn pop_redo(&mut self) -> Option { - self.lagacy_redo_stack.pop_back() + self.legacy_redo_stack.pop_back() } /// Drop the most recently pushed undo snapshot (used to cancel a transaction that ended up unmodified). pub fn discard_last_undo(&mut self) { - self.lagacy_undo_stack.pop_back(); + self.legacy_undo_stack.pop_back(); } /// Clear the redo stack, called when a fresh edit invalidates the redo future. pub fn clear_redo(&mut self) { - self.lagacy_redo_stack.clear(); + self.legacy_redo_stack.clear(); } /// Add the resources referenced by every snapshot in both history stacks into `resources`, so /// history-only resources stay alive for legacy undo/redo. pub fn collect_used_resources(&self, resources: &mut HashSet) { - for interface in self.lagacy_undo_stack.iter().chain(&self.lagacy_redo_stack) { + for interface in self.legacy_undo_stack.iter().chain(&self.legacy_redo_stack) { interface.collect_used_resources(resources); } } diff --git a/editor/src/messages/portfolio/document/node_graph/document_node_definitions.rs b/editor/src/messages/portfolio/document/node_graph/document_node_definitions.rs index 00351bf6b9..8c3ac9637d 100644 --- a/editor/src/messages/portfolio/document/node_graph/document_node_definitions.rs +++ b/editor/src/messages/portfolio/document/node_graph/document_node_definitions.rs @@ -102,7 +102,7 @@ pub struct DocumentNodeDefinition { /// Used to create the [`DefinitionIdentifier::Network`] identifier. pub identifier: &'static str, - /// All data required to construct a [`DocumentNode`] and [`DocumentNodeMetadata`] + /// The default [`NodeTemplate`] this definition instantiates. pub node_template: NodeTemplate, /// Definition specific data. In order for the editor to access this data, the reference will be used. diff --git a/editor/src/messages/portfolio/document/utility_types/network_interface/layout.rs b/editor/src/messages/portfolio/document/utility_types/network_interface/layout.rs index c7c9c655a2..af47bc5903 100644 --- a/editor/src/messages/portfolio/document/utility_types/network_interface/layout.rs +++ b/editor/src/messages/portfolio/document/utility_types/network_interface/layout.rs @@ -77,44 +77,12 @@ impl NodeNetworkInterface { log::error!("Could not set chain position for layer node {node_id}"); } // let previous_upstream_node = self.upstream_output_connector(&InputConnector::node(*node_id, 0), network_path).and_then(|output| output.node_id()); - // let Some(previous_upstream_node_position) = previous_upstream_node.and_then(|upstream| self.position_from_downstream_node(&upstream, network_path)) else { - // log::error!("Could not get previous_upstream_node_position"); - // return; - // }; self.unload_upstream_node_click_targets(vec![*node_id], network_path); // Reload click target of the layer which encapsulate the chain if let Some(downstream_layer) = self.downstream_layer_for_chain_node(node_id, network_path) { self.unload_node_click_targets(&downstream_layer, network_path); } self.unload_all_nodes_bounding_box(network_path); - - // let Some(new_upstream_node_position) = previous_upstream_node.and_then(|upstream| self.position_from_downstream_node(&upstream, network_path)) else { - // log::error!("Could not get new_upstream_node_position"); - // return; - // }; - // if let Some(previous_upstream_node) = { - // let x_delta = new_upstream_node_position.x - previous_upstream_node_position.x; - // // Upstream node got shifted to left, so shift all upstream absolute sole dependents - // if x_delta != 0 { - // let upstream_absolute_nodes = SelectedNodes( - // self.upstream_flow_back_from_nodes(vec![previous_upstream_node], network_path, FlowType::UpstreamFlow) - // .into_iter() - // .filter(|node_id| self.is_absolute(node_id, network_path)) - // .collect::>(), - // ); - // let old_selected_nodes = std::mem::replace(self.selected_nodes_mut(network_path).unwrap(), upstream_absolute_nodes); - // if x_delta < 0 { - // for _ in 0..x_delta.abs() { - // self.shift_selected_nodes(Direction::Left, false, network_path); - // } - // } else { - // for _ in 0..x_delta.abs() { - // self.shift_selected_nodes(Direction::Right, false, network_path); - // } - // } - // let _ = std::mem::replace(self.selected_nodes_mut(network_path).unwrap(), old_selected_nodes); - // } - // } } pub(crate) fn valid_upstream_chain_nodes(&self, input_connector: &InputConnector, network_path: &[NodeId]) -> Vec { 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 116f2903bc..1ac62e1fb7 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 @@ -1313,22 +1313,6 @@ impl NodeNetworkInterface { network_metadata.persistent_metadata.previewing = Previewing::No; } - // /// Sets the root node only if a node is being previewed - // pub fn update_root_node(&mut self, node_id: NodeId, output_index: usize) { - // if let Previewing::Yes { root_node_to_restore } = self.previewing { - // // Only continue previewing if the new root node is not the same as the primary export. If it is the same, end the preview - // if let Some(root_node_to_restore) = root_node_to_restore { - // if root_node_to_restore.id != node_id { - // self.start_previewing(node_id, output_index); - // } else { - // self.stop_preview(); - // } - // } else { - // self.stop_preview(); - // } - // } - // } - pub fn set_display_name(&mut self, node_id: &NodeId, display_name: String, network_path: &[NodeId]) { let Some(node_metadata) = self.node_metadata_mut(node_id, network_path) else { log::error!("Could not get node {node_id} in set_visibility"); diff --git a/editor/src/messages/portfolio/document/utility_types/network_interface/queries.rs b/editor/src/messages/portfolio/document/utility_types/network_interface/queries.rs index 0ad94edf99..e677d26f21 100644 --- a/editor/src/messages/portfolio/document/utility_types/network_interface/queries.rs +++ b/editor/src/messages/portfolio/document/utility_types/network_interface/queries.rs @@ -1210,21 +1210,6 @@ impl NodeNetworkInterface { Some(parent_metadata) } - // /// Mutably get the node which encapsulates the currently viewed network. Will always be None in the document network. - // fn encapsulating_node_mut(&mut self, network_path: &[NodeId]) -> Option<&mut DocumentNode> { - // let mut encapsulating_path = network_path.to_vec(); - // let encapsulating_node_id = encapsulating_path.pop()?; - // let Some(parent_network) = self.network_mut(&encapsulating_path) else { - // log::error!("Could not get parent network in encapsulating_node_mut"); - // return None; - // }; - // let Some(encapsulating_node) = parent_network.nodes.mut(&encapsulating_node_id) else { - // log::error!("Could not get encapsulating node in encapsulating_node_mut"); - // return None; - // }; - // Some(encapsulating_node) - // } - /// Get the node metadata for the node which encapsulates the currently viewed network. Will always be None in the document network. pub(crate) fn encapsulating_node_metadata_mut(&mut self, network_path: &[NodeId]) -> Option<&mut DocumentNodeMetadata> { let mut encapsulating_path = network_path.to_vec(); diff --git a/editor/src/messages/portfolio/document/utility_types/network_interface/types.rs b/editor/src/messages/portfolio/document/utility_types/network_interface/types.rs index e162d98999..2f82a1229f 100644 --- a/editor/src/messages/portfolio/document/utility_types/network_interface/types.rs +++ b/editor/src/messages/portfolio/document/utility_types/network_interface/types.rs @@ -426,8 +426,6 @@ pub struct NodeNetworkTransientMetadata { pub(crate) stack_dependents: TransientCache>, /// Cache for the bounding box around all nodes in node graph space. pub(crate) all_nodes_bounding_box: TransientCache<[DVec2; 2]>, - // /// Cache bounding box for all "groups of nodes", which will be used to prevent overlapping nodes - // node_group_bounding_box: Vec<(Subpath, Vec)>, /// Cache for all outward wire connections pub(crate) outward_wires: TransientCache>>, /// All export connector click targets @@ -704,8 +702,6 @@ impl NodeTypePersistentMetadata { /// All fields in LayerMetadata should automatically be updated by using the network interface API #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] pub struct LayerPersistentMetadata { - // TODO: Store click target for the preview button, which will appear when the node is a selected/(hovered?) layer node - // preview_click_target: Option, /// Stores the position of a layer node, which can either be Absolute or Stack pub position: LayerPosition, } @@ -790,8 +786,6 @@ pub struct LayerClickTargets { /// to skip the drill-into-subgraph behavior when the click lands on the name itself. /// `None` for layers whose display name is empty. pub name_click_target: Option, - // TODO: Store click target for the preview button, which will appear when the node is a selected/(hovered?) layer node - // preview_click_target: ClickTarget, } pub enum LayerClickTargetTypes { From cf23c01a9f9502039a89d4010dca04bd817258c3 Mon Sep 17 00:00:00 2001 From: Keavon Chambers Date: Fri, 10 Jul 2026 04:50:03 -0700 Subject: [PATCH 2/6] Validate that the pinned node order only references existing nodes --- .../document/utility_types/network_interface/validation.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/editor/src/messages/portfolio/document/utility_types/network_interface/validation.rs b/editor/src/messages/portfolio/document/utility_types/network_interface/validation.rs index 60d03969d9..59e7407297 100644 --- a/editor/src/messages/portfolio/document/utility_types/network_interface/validation.rs +++ b/editor/src/messages/portfolio/document/utility_types/network_interface/validation.rs @@ -28,6 +28,13 @@ fn validate_network(network: &NodeNetwork, network_metadata: &NodeNetworkMetadat } } + // The pinned display order may only reference existing nodes + for node_id in &network_metadata.persistent_metadata.pinned_node_order { + if !network.nodes.contains_key(node_id) { + violations.push(format!("Pinned node order in network {path:?} references nonexistent node {node_id}")); + } + } + // Every wire must reference an existing endpoint within this network let validate_input = |input: &NodeInput, location: &str, violations: &mut Vec| match input { NodeInput::Node { node_id, output_index, .. } => match network.nodes.get(node_id) { From 970b1373196ac0732ed88412c4ff5fc5f2472f39 Mon Sep 17 00:00:00 2001 From: Keavon Chambers Date: Fri, 10 Jul 2026 21:57:37 -0700 Subject: [PATCH 3/6] Abort remove_import before any mutation when the import's wires cannot be resolved --- .../network_interface/mutations.rs | 36 ++++++++----------- 1 file changed, 15 insertions(+), 21 deletions(-) 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 1ac62e1fb7..206dfa2700 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 @@ -226,34 +226,26 @@ impl NodeNetworkInterface { self.unload_modify_import_export(network_path); } - /// Disconnects every wire fed by the given import within the network. - pub(crate) fn disconnect_import_wires(&mut self, import_index: usize, network_path: &[NodeId]) { - let Some(wires_for_import) = self.with_outward_wires(network_path, |outward_wires| outward_wires.get(&OutputConnector::Import(import_index)).cloned()) else { - log::error!("Could not get outward wires in disconnect_import_wires"); - return; - }; - let Some(wires_for_import) = wires_for_import else { - log::error!("Could not get outward wires for import in disconnect_import_wires"); - return; - }; - for downstream_connection in wires_for_import { - self.disconnect_input(&downstream_connection, network_path); - } + /// Disconnects every wire fed by the given import within the network. Returns false without mutating if the import's wires cannot be resolved. + pub(crate) fn disconnect_import_wires(&mut self, import_index: usize, network_path: &[NodeId]) -> bool { + self.disconnect_output_wires(&OutputConnector::Import(import_index), network_path) } - /// Disconnects every wire fed by the given output within the network. - pub(crate) fn disconnect_output_wires(&mut self, output_connector: &OutputConnector, network_path: &[NodeId]) { + /// Disconnects every wire fed by the given output within the network. Returns false without mutating if the output's wires cannot be resolved. + pub(crate) fn disconnect_output_wires(&mut self, output_connector: &OutputConnector, network_path: &[NodeId]) -> bool { let Some(downstream_connections) = self.with_outward_wires(network_path, |outward_wires| outward_wires.get(output_connector).cloned()) else { log::error!("Could not get outward wires in disconnect_output_wires"); - return; + return false; }; let Some(downstream_connections) = downstream_connections else { - log::error!("Could not get downstream connections in disconnect_output_wires"); - return; + log::error!("Could not get downstream connections for {output_connector:?} in disconnect_output_wires"); + return false; }; for downstream_connection in downstream_connections { self.disconnect_input(&downstream_connection, network_path); } + + true } /// Refreshes the metadata invalidated when the encapsulating node's signature changes, demoting it from a layer if it is no longer eligible. @@ -346,9 +338,11 @@ impl NodeNetworkInterface { } } - // Disconnect all upstream connections - self.disconnect_import_wires(import_index, network_path); - // Shift inputs connected to to imports at a higher index down one + // Disconnect all upstream connections, aborting before any mutation if the import's wires cannot be resolved + if !self.disconnect_import_wires(import_index, network_path) { + return; + } + // Shift inputs connected to imports at a higher index down one for (output_connector, input_wire) in new_import_mapping { self.create_wire(&output_connector, &input_wire, network_path); } From c546887ea91f7bbbac0c28f825a9d9a05ba2ced2 Mon Sep 17 00:00:00 2001 From: Keavon Chambers Date: Fri, 10 Jul 2026 21:57:38 -0700 Subject: [PATCH 4/6] Purge the deleted nodes' cached wire paths in delete_nodes --- .../utility_types/network_interface/mutations.rs | 9 +++++++++ 1 file changed, 9 insertions(+) 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 206dfa2700..5dc2ca8c68 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 @@ -1199,6 +1199,15 @@ impl NodeNetworkInterface { network_metadata.persistent_metadata.pinned_node_order.retain(|node_id| surviving_nodes.contains(node_id)); } + // Purge the deleted nodes' cached wire paths, since the per-node unload can no longer reach them once the nodes are gone + if let Some(network_metadata) = self.network_metadata(network_path) { + network_metadata + .transient_metadata + .wires + .borrow_mut() + .retain(|connector, _| connector.node_id().is_none_or(|node_id| !delete_nodes.contains(&node_id))); + } + self.unload_all_nodes_bounding_box(network_path); // Instead of unloaded all node click targets, just unload the nodes upstream from the deleted nodes. unload_upstream_node_click_targets will not work since the nodes have been deleted. self.unload_all_nodes_click_targets(network_path); From f1a8d1ced10965e3ad029d69a0629a0d3c5d4243 Mon Sep 17 00:00:00 2001 From: Keavon Chambers Date: Fri, 10 Jul 2026 21:57:56 -0700 Subject: [PATCH 5/6] Disconnect a deleted node's inputs by position so hidden inputs cannot mask a later exposed wire --- .../document/utility_types/network_interface/mutations.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 5dc2ca8c68..f5d3cac675 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 @@ -1169,7 +1169,8 @@ impl NodeNetworkInterface { continue; } - for input_index in 0..self.number_of_displayed_inputs(delete_node_id, network_path) { + // Disconnect every input by position, since hidden inputs make the displayed count undershoot the index of a later exposed wire + for input_index in 0..self.number_of_inputs(delete_node_id, network_path) { self.disconnect_input(&InputConnector::node(*delete_node_id, input_index), network_path); } From e096de6811fa2b2be5502cb61229f5f99c07bda7 Mon Sep 17 00:00:00 2001 From: Keavon Chambers Date: Fri, 10 Jul 2026 21:57:56 -0700 Subject: [PATCH 6/6] Gather the delete_nodes layer sweep only when deleting children needs it --- .../network_interface/mutations.rs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) 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 f5d3cac675..f4a1b598d5 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 @@ -1111,13 +1111,16 @@ impl NodeNetworkInterface { } // Layer membership is fixed during the expansion phase, so gather it once for the sole-dependent closure - let layer_nodes = self - .nested_network(network_path) - .map(|network| network.nodes.keys().copied().collect::>()) - .unwrap_or_default() - .into_iter() - .filter(|candidate| self.is_layer(candidate, network_path)) - .collect::>(); + let layer_nodes = if delete_children { + self.nested_network(network_path) + .map(|network| network.nodes.keys().copied().collect::>()) + .unwrap_or_default() + .into_iter() + .filter(|candidate| self.is_layer(candidate, network_path)) + .collect::>() + } else { + HashSet::new() + }; let mut delete_nodes = HashSet::new(); for node_id in &nodes_to_delete {