From 586ea8b1dda2e543834d67712d661f79d0032de1 Mon Sep 17 00:00:00 2001 From: Keavon Chambers Date: Fri, 10 Jul 2026 21:58:13 -0700 Subject: [PATCH 1/5] Route the remaining view queries through the logging query helper --- .../network_interface/queries.rs | 29 +++++++++---------- 1 file changed, 13 insertions(+), 16 deletions(-) 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 e677d26f21..2ea19dfb74 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 @@ -80,14 +80,11 @@ impl NodeNetworkInterface { /// Runs an encapsulating-node query, staying silent for the document network which has no encapsulating node. fn query_encapsulating<'a, 'p, T>(&'a self, network_path: &'p [NodeId], caller: &str, query: impl FnOnce(NetworkView<'a, 'p>) -> Result) -> Option { - match self.view(network_path).and_then(query) { - Ok(value) => Some(value), - Err(NetworkError::NoEncapsulatingNode) => None, - Err(error) => { - log::error!("{error} in {caller}"); - None - } - } + self.query(network_path, caller, |view| match query(view) { + Err(NetworkError::NoEncapsulatingNode) => Ok(None), + result => result.map(Some), + }) + .flatten() } /// Get the network which the encapsulating node of the currently viewed network is part of. Will always be None in the document network. @@ -153,11 +150,11 @@ impl NodeNetworkInterface { } pub fn number_of_imports(&self, network_path: &[NodeId]) -> usize { - self.view(network_path).map(|view| view.number_of_imports()).unwrap_or_default() + self.query(network_path, "number_of_imports", |view| Ok(view.number_of_imports())).unwrap_or_default() } pub fn number_of_exports(&self, network_path: &[NodeId]) -> usize { - self.view(network_path).map(|view| view.number_of_exports()).unwrap_or_default() + self.query(network_path, "number_of_exports", |view| Ok(view.number_of_exports())).unwrap_or_default() } pub(crate) fn number_of_displayed_inputs(&self, node_id: &NodeId, network_path: &[NodeId]) -> usize { @@ -172,7 +169,7 @@ impl NodeNetworkInterface { /// Whether the node has an exposed input at index 0 to accept the horizontal flow from upstream. /// A node without one (e.g. a generator) can only be the most-upstream node in a chain. pub fn has_primary_input(&self, node_id: &NodeId, network_path: &[NodeId]) -> bool { - self.view(network_path).and_then(|view| view.has_primary_input(node_id)).unwrap_or_default() + self.query(network_path, "has_primary_input", |view| view.has_primary_input(node_id)).unwrap_or_default() } pub fn number_of_outputs(&self, node_id: &NodeId, network_path: &[NodeId]) -> usize { @@ -837,7 +834,7 @@ impl NodeNetworkInterface { /// The given network's pinned nodes in display order: pinning appends, dragging rearranges, and any not yet recorded go last. pub fn ordered_pinned_nodes(&self, network_path: &[NodeId]) -> Vec { - self.view(network_path).map(|view| view.ordered_pinned_nodes()).unwrap_or_default() + self.query(network_path, "ordered_pinned_nodes", |view| Ok(view.ordered_pinned_nodes())).unwrap_or_default() } pub fn is_visible(&self, node_id: &NodeId, network_path: &[NodeId]) -> bool { @@ -871,15 +868,15 @@ impl NodeNetworkInterface { } pub fn hidden_primary_export(&self, network_path: &[NodeId]) -> bool { - self.view(network_path).map(|view| view.hidden_primary_export()).unwrap_or_default() + self.query(network_path, "hidden_primary_export", |view| Ok(view.hidden_primary_export())).unwrap_or_default() } pub fn hidden_primary_output(&self, node_id: &NodeId, network_path: &[NodeId]) -> bool { - self.view(network_path).and_then(|view| view.hidden_primary_output(node_id)).unwrap_or_default() + self.query(network_path, "hidden_primary_output", |view| view.hidden_primary_output(node_id)).unwrap_or_default() } pub fn hidden_primary_import(&self, network_path: &[NodeId]) -> bool { - self.view(network_path).map(|view| view.hidden_primary_import()).unwrap_or_default() + self.query(network_path, "hidden_primary_import", |view| Ok(view.hidden_primary_import())).unwrap_or_default() } pub fn is_absolute(&self, node_id: &NodeId, network_path: &[NodeId]) -> bool { @@ -897,7 +894,7 @@ impl NodeNetworkInterface { /// Whether the node is an Artboard node by identity, regardless of whether it currently participates in the scene. /// Callers that care about scene membership should source their layers from the document structure or check connectivity separately. pub fn is_artboard(&self, node_id: &NodeId, network_path: &[NodeId]) -> bool { - self.view(network_path).map(|view| view.is_artboard(node_id)).unwrap_or_default() + self.query(network_path, "is_artboard", |view| Ok(view.is_artboard(node_id))).unwrap_or_default() } /// All artboard layers that participate in the scene, excluding disconnected Artboard nodes. From 62b1ee01ea71a81cc3a87957beaabc14543cd353 Mon Sep 17 00:00:00 2001 From: Keavon Chambers Date: Fri, 10 Jul 2026 21:58:42 -0700 Subject: [PATCH 2/5] Name the cache readers by their loading contract and stop cloning the owned nodes per collision check --- .../utility_types/network_interface/caches.rs | 24 +++-- .../network_interface/hit_tests.rs | 14 +-- .../utility_types/network_interface/layout.rs | 91 ++++++++++--------- 3 files changed, 68 insertions(+), 61 deletions(-) diff --git a/editor/src/messages/portfolio/document/utility_types/network_interface/caches.rs b/editor/src/messages/portfolio/document/utility_types/network_interface/caches.rs index d8d606c541..77ccd3c0e8 100644 --- a/editor/src/messages/portfolio/document/utility_types/network_interface/caches.rs +++ b/editor/src/messages/portfolio/document/utility_types/network_interface/caches.rs @@ -91,7 +91,7 @@ impl NodeNetworkInterface { } /// Reads the stack dependents through &self if they are already loaded. - pub(crate) fn with_stack_dependents(&self, network_path: &[NodeId], read: impl FnOnce(&HashMap) -> R) -> Option { + pub(crate) fn with_stack_dependents_if_loaded(&self, network_path: &[NodeId], read: impl FnOnce(&HashMap) -> R) -> Option { self.network_metadata(network_path)?.transient_metadata.stack_dependents.with_loaded(read) } @@ -454,7 +454,7 @@ impl NodeNetworkInterface { } /// Reads the owned nodes of a layer through &self if they are loaded. - pub(crate) fn with_owned_nodes(&self, node_id: &NodeId, network_path: &[NodeId], read: impl FnOnce(&HashSet) -> R) -> Option { + pub(crate) fn with_owned_nodes_if_loaded(&self, node_id: &NodeId, network_path: &[NodeId], read: impl FnOnce(&HashSet) -> R) -> Option { let layer_node = self.node_metadata(node_id, network_path)?; if !layer_node.persistent_metadata.is_layer() { return None; @@ -938,25 +938,29 @@ impl NodeNetworkInterface { } /// Loads the node click targets if needed, then reads them through &self. - pub(crate) fn with_loaded_node_click_targets(&self, node_id: &NodeId, network_path: &[NodeId], read: impl FnOnce(&DocumentNodeClickTargets) -> R) -> Option { + pub(crate) fn with_node_click_targets(&self, node_id: &NodeId, network_path: &[NodeId], read: impl FnOnce(&DocumentNodeClickTargets) -> R) -> Option { self.try_load_node_click_targets(node_id, network_path); - self.with_node_click_targets(node_id, network_path, read) + self.with_node_click_targets_if_loaded(node_id, network_path, read) } /// Reads the modify import/export click targets through &self, loading them first if needed. pub(crate) fn with_modify_import_export(&self, network_path: &[NodeId], read: impl FnOnce(&ModifyImportExportClickTarget) -> R) -> Option { + self.try_load_modify_import_export(network_path); + self.network_metadata(network_path)?.transient_metadata.modify_import_export.with_loaded(read) + } + + fn try_load_modify_import_export(&self, network_path: &[NodeId]) { let Some(network_metadata) = self.network_metadata(network_path) else { log::error!("Could not get nested network_metadata in modify_import_export"); - return None; + return; }; if !network_metadata.transient_metadata.modify_import_export.is_loaded() { self.load_modify_import_export(network_path); } - self.network_metadata(network_path)?.transient_metadata.modify_import_export.with_loaded(read) } /// Reads the node click targets through &self if they are already loaded. - pub(crate) fn with_node_click_targets(&self, node_id: &NodeId, network_path: &[NodeId], read: impl FnOnce(&DocumentNodeClickTargets) -> R) -> Option { + pub(crate) fn with_node_click_targets_if_loaded(&self, node_id: &NodeId, network_path: &[NodeId], read: impl FnOnce(&DocumentNodeClickTargets) -> R) -> Option { let node_metadata = self.node_metadata(node_id, network_path)?; let result = node_metadata.transient_metadata.click_targets.with_loaded(read); if result.is_none() { @@ -1095,8 +1099,8 @@ impl NodeNetworkInterface { let name_left = node_top_left.x + NAME_LEFT_OFFSET; let icons_reserve = VISIBILITY_INSET_FROM_LAYER_RIGHT + icons_width + GRIP_WIDTH; let name_right_max = node_top_left.x + width as f64 - icons_reserve; - let text_w = text_width(&display_name, FONT_SIZE); - let name_right = (name_left + text_w).min(name_right_max); + let name_width = text_width(&display_name, FONT_SIZE); + let name_right = (name_left + name_width).min(name_right_max); if name_right > name_left { // The 1-grid-tall name strip is centered vertically in the 2-grid-tall layer. let name_top = node_top_left.y + HALF_GRID_SIZE as f64; @@ -1142,7 +1146,7 @@ impl NodeNetworkInterface { } pub fn try_get_node_bounding_box(&self, node_id: &NodeId, network_path: &[NodeId]) -> Option<[DVec2; 2]> { - self.with_node_click_targets(node_id, network_path, |click_targets| click_targets.node_click_target.bounding_box()) + self.with_node_click_targets_if_loaded(node_id, network_path, |click_targets| click_targets.node_click_target.bounding_box()) .flatten() } diff --git a/editor/src/messages/portfolio/document/utility_types/network_interface/hit_tests.rs b/editor/src/messages/portfolio/document/utility_types/network_interface/hit_tests.rs index b829fa54f5..14879febb5 100644 --- a/editor/src/messages/portfolio/document/utility_types/network_interface/hit_tests.rs +++ b/editor/src/messages/portfolio/document/utility_types/network_interface/hit_tests.rs @@ -39,7 +39,7 @@ impl NodeNetworkInterface { } }); nodes.into_iter().for_each(|node_id| { - self.with_loaded_node_click_targets(&node_id, network_path, |node_click_targets| { + self.with_node_click_targets(&node_id, network_path, |node_click_targets| { let mut node_path = String::new(); if let ClickTargetType::Subpath(subpath) = node_click_targets.node_click_target.target_type() { @@ -146,7 +146,7 @@ impl NodeNetworkInterface { let clicked_nodes = nodes .iter() .filter(|node_id| { - self.with_loaded_node_click_targets(node_id, network_path, |transient_node_metadata| { + self.with_node_click_targets(node_id, network_path, |transient_node_metadata| { transient_node_metadata.node_click_target.intersect_point_no_stroke(point) }) == Some(true) }) @@ -181,7 +181,7 @@ impl NodeNetworkInterface { node_ids .iter() .filter_map(|node_id| { - self.with_loaded_node_click_targets(node_id, network_path, |transient_node_metadata| { + self.with_node_click_targets(node_id, network_path, |transient_node_metadata| { if let NodeTypeClickTargets::Layer(layer) = &transient_node_metadata.node_type_metadata { match click_target_type { LayerClickTargetTypes::Visibility => layer.visibility_click_target.intersect_point_no_stroke(point).then_some(*node_id), @@ -216,7 +216,7 @@ impl NodeNetworkInterface { .collect::>() .iter() .filter_map(|node_id| { - self.with_loaded_node_click_targets(node_id, network_path, |transient_node_metadata| { + self.with_node_click_targets(node_id, network_path, |transient_node_metadata| { transient_node_metadata .port_click_targets .clicked_input_port_from_point(point) @@ -246,7 +246,7 @@ impl NodeNetworkInterface { nodes .iter() .filter_map(|node_id| { - self.with_loaded_node_click_targets(node_id, network_path, |transient_node_metadata| { + self.with_node_click_targets(node_id, network_path, |transient_node_metadata| { transient_node_metadata .port_click_targets .clicked_output_port_from_point(point) @@ -266,7 +266,7 @@ impl NodeNetworkInterface { pub fn input_position(&self, input_connector: &InputConnector, network_path: &[NodeId]) -> Option { match input_connector { InputConnector::Node { node_id, input_index } => self - .with_loaded_node_click_targets(node_id, network_path, |transient_node_metadata| { + .with_node_click_targets(node_id, network_path, |transient_node_metadata| { transient_node_metadata.port_click_targets.input_port_position(*input_index) }) .flatten(), @@ -279,7 +279,7 @@ impl NodeNetworkInterface { pub fn output_position(&self, output_connector: &OutputConnector, network_path: &[NodeId]) -> Option { match output_connector { OutputConnector::Node { node_id, output_index } => self - .with_loaded_node_click_targets(node_id, network_path, |transient_node_metadata| { + .with_node_click_targets(node_id, network_path, |transient_node_metadata| { transient_node_metadata.port_click_targets.output_port_position(*output_index) }) .flatten(), 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 af47bc5903..249c074745 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 @@ -76,7 +76,6 @@ impl NodeNetworkInterface { else { 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()); 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) { @@ -282,7 +281,7 @@ impl NodeNetworkInterface { self.try_load_stack_dependents(network_path); for node_id in node_ids.clone() { if self.is_layer(&node_id, network_path) { - self.with_owned_nodes(&node_id, network_path, |owned_nodes| { + self.with_owned_nodes_if_loaded(&node_id, network_path, |owned_nodes| { for owned_node in owned_nodes { node_ids.remove(owned_node); } @@ -405,7 +404,7 @@ impl NodeNetworkInterface { shifted_nodes.insert(*node_id); self.shift_node(node_id, IVec2::new(0, shift_sign), network_path); - if self.with_stack_dependents(network_path, |stack_dependents| matches!(stack_dependents.get(node_id), Some(LayerOwner::None))) == Some(true) { + if self.with_stack_dependents_if_loaded(network_path, |stack_dependents| matches!(stack_dependents.get(node_id), Some(LayerOwner::None))) == Some(true) { self.add_drag_offset(node_id, shift_sign, network_path); } @@ -445,7 +444,7 @@ impl NodeNetworkInterface { } if seed_nodes .iter() - .any(|seed_node| seed_node == node_id || self.with_owned_nodes(node_id, network_path, |owned_nodes| owned_nodes.contains(seed_node)) == Some(true)) + .any(|seed_node| seed_node == node_id || self.with_owned_nodes_if_loaded(node_id, network_path, |owned_nodes| owned_nodes.contains(seed_node)) == Some(true)) { return None; }; @@ -499,7 +498,7 @@ impl NodeNetworkInterface { self.shift_node(node_id, IVec2::new(0, shift_sign), network_path); - match self.with_stack_dependents(network_path, |stack_dependents| stack_dependents.get(node_id).cloned()) { + match self.with_stack_dependents_if_loaded(network_path, |stack_dependents| stack_dependents.get(node_id).cloned()) { Some(Some(LayerOwner::None)) => self.add_drag_offset(node_id, shift_sign, network_path), Some(Some(LayerOwner::Layer(_))) => log::error!("Node being shifted with a push should not be owned"), Some(None) => log::error!("Could not get layer owner in vertical_shift_with_push for node {node_id}"), @@ -519,7 +518,7 @@ impl NodeNetworkInterface { } // Shift the nodes that are owned by the layer (if any) - if let Some(owned_nodes) = self.with_owned_nodes(node_id, network_path, |owned_nodes| owned_nodes.clone()) { + if let Some(owned_nodes) = self.with_owned_nodes_if_loaded(node_id, network_path, |owned_nodes| owned_nodes.clone()) { for owned_node in owned_nodes { if self.is_absolute(&owned_node, network_path) { self.try_shift_node(&owned_node, IVec2::new(0, shift_sign), shifted_nodes, network_path); @@ -533,50 +532,54 @@ impl NodeNetworkInterface { self.try_load_stack_dependents(network_path); // Check collisions and for all owned nodes and recursively shift them - let nodes_to_shift = self.with_stack_dependents(network_path, |stack_dependents| { - let mut nodes_to_shift = Vec::new(); - - let owned_nodes = self.with_owned_nodes(node_id, network_path, |owned_nodes| owned_nodes.clone()).unwrap_or_default(); - - for current_node in owned_nodes.iter().chain(std::iter::once(node_id)) { - for node_to_check_collision in stack_dependents { - // Do not check collision between any of the owned nodes or the shifted node - if owned_nodes.contains(node_to_check_collision.0) || node_to_check_collision.0 == node_id { - continue; - } - - if node_to_check_collision.0 == current_node { - continue; - } - let Some(mut current_node_bounding_box) = self.try_get_node_bounding_box(current_node, network_path) else { - log::error!("Could not get bounding box for node {node_id} in shift_selected_nodes"); - continue; - }; + let nodes_to_shift = self.with_stack_dependents_if_loaded(network_path, |stack_dependents| { + // Hot path during drags: borrow the owned nodes for the sweep rather than cloning them per call + let collect_collisions = |owned_nodes: &HashSet| { + let mut nodes_to_shift = Vec::new(); + + for current_node in owned_nodes.iter().chain(std::iter::once(node_id)) { + for node_to_check_collision in stack_dependents { + // Do not check collision between any of the owned nodes or the shifted node + if owned_nodes.contains(node_to_check_collision.0) || node_to_check_collision.0 == node_id { + continue; + } - let Some(node_bounding_box) = self.try_get_node_bounding_box(node_to_check_collision.0, network_path) else { - log::error!("Could not get bounding box for node {node_to_check_collision:?} in shift_selected_nodes"); - continue; - }; - // If the nodes do not intersect horizontally, then there is no collision - if current_node_bounding_box[1].x < node_bounding_box[0].x || current_node_bounding_box[0].x > node_bounding_box[1].x { - continue; - } - // Do not check collision if the nodes are currently intersecting - if current_node_bounding_box[1].y >= node_bounding_box[0].y - 0.1 && current_node_bounding_box[0].y <= node_bounding_box[1].y + 0.1 { - continue; - } + if node_to_check_collision.0 == current_node { + continue; + } + let Some(mut current_node_bounding_box) = self.try_get_node_bounding_box(current_node, network_path) else { + log::error!("Could not get bounding box for node {node_id} in shift_selected_nodes"); + continue; + }; + + let Some(node_bounding_box) = self.try_get_node_bounding_box(node_to_check_collision.0, network_path) else { + log::error!("Could not get bounding box for node {node_to_check_collision:?} in shift_selected_nodes"); + continue; + }; + // If the nodes do not intersect horizontally, then there is no collision + if current_node_bounding_box[1].x < node_bounding_box[0].x || current_node_bounding_box[0].x > node_bounding_box[1].x { + continue; + } + // Do not check collision if the nodes are currently intersecting + if current_node_bounding_box[1].y >= node_bounding_box[0].y - 0.1 && current_node_bounding_box[0].y <= node_bounding_box[1].y + 0.1 { + continue; + } - current_node_bounding_box[1].y += GRID_SIZE as f64 * shift_sign as f64; - current_node_bounding_box[0].y += GRID_SIZE as f64 * shift_sign as f64; + current_node_bounding_box[1].y += GRID_SIZE as f64 * shift_sign as f64; + current_node_bounding_box[0].y += GRID_SIZE as f64 * shift_sign as f64; - let collision = current_node_bounding_box[1].y >= node_bounding_box[0].y - 0.1 && current_node_bounding_box[0].y <= node_bounding_box[1].y + 0.1; - if collision { - nodes_to_shift.push((*node_to_check_collision.0, node_to_check_collision.1.clone())); + let collision = current_node_bounding_box[1].y >= node_bounding_box[0].y - 0.1 && current_node_bounding_box[0].y <= node_bounding_box[1].y + 0.1; + if collision { + nodes_to_shift.push((*node_to_check_collision.0, node_to_check_collision.1.clone())); + } } } - } - nodes_to_shift + nodes_to_shift + }; + + self.with_owned_nodes_if_loaded(node_id, network_path, collect_collisions) + .unwrap_or_else(|| collect_collisions(&HashSet::new())) }); let Some(nodes_to_shift) = nodes_to_shift else { From 32d5a169b9c5dcd6a61688cb406abd8ff1a70ab2 Mon Sep 17 00:00:00 2001 From: Keavon Chambers Date: Fri, 10 Jul 2026 21:59:13 -0700 Subject: [PATCH 3/5] Deduplicate the port center, output count, and upstream connector helpers and drop dead accessors --- .../utility_types/network_interface/caches.rs | 34 +++++-------------- .../network_interface/template.rs | 5 --- .../utility_types/network_interface/types.rs | 20 ++++------- .../utility_types/network_interface/view.rs | 22 ++++-------- 4 files changed, 21 insertions(+), 60 deletions(-) diff --git a/editor/src/messages/portfolio/document/utility_types/network_interface/caches.rs b/editor/src/messages/portfolio/document/utility_types/network_interface/caches.rs index 77ccd3c0e8..d0463b9fd3 100644 --- a/editor/src/messages/portfolio/document/utility_types/network_interface/caches.rs +++ b/editor/src/messages/portfolio/document/utility_types/network_interface/caches.rs @@ -679,38 +679,20 @@ impl NodeNetworkInterface { } pub fn get_input_center(&self, input: &InputConnector, network_path: &[NodeId]) -> Option { - fn port_center(ports: &Ports, index: usize) -> Option { - ports - .input_ports - .iter() - .find_map(|(input_index, click_target)| if index == *input_index { click_target.bounding_box_center() } else { None }) - } - match input { - InputConnector::Node { node_id, input_index } => { - self.try_load_node_click_targets(node_id, network_path); - self.with_node_click_targets(node_id, network_path, |click_targets| port_center(&click_targets.port_click_targets, *input_index)) - .flatten() - } - InputConnector::Export(export_index) => self.with_import_export_ports(network_path, |ports| port_center(ports, *export_index)).flatten(), + InputConnector::Node { node_id, input_index } => self + .with_node_click_targets(node_id, network_path, |click_targets| click_targets.port_click_targets.input_port_position(*input_index)) + .flatten(), + InputConnector::Export(export_index) => self.with_import_export_ports(network_path, |ports| ports.input_port_position(*export_index)).flatten(), } } pub fn get_output_center(&self, output: &OutputConnector, network_path: &[NodeId]) -> Option { - fn port_center(ports: &Ports, index: usize) -> Option { - ports - .output_ports - .iter() - .find_map(|(output_index, click_target)| if index == *output_index { click_target.bounding_box_center() } else { None }) - } - match output { - OutputConnector::Node { node_id, output_index } => { - self.try_load_node_click_targets(node_id, network_path); - self.with_node_click_targets(node_id, network_path, |click_targets| port_center(&click_targets.port_click_targets, *output_index)) - .flatten() - } - OutputConnector::Import(import_index) => self.with_import_export_ports(network_path, |ports| port_center(ports, *import_index)).flatten(), + OutputConnector::Node { node_id, output_index } => self + .with_node_click_targets(node_id, network_path, |click_targets| click_targets.port_click_targets.output_port_position(*output_index)) + .flatten(), + OutputConnector::Import(import_index) => self.with_import_export_ports(network_path, |ports| ports.output_port_position(*import_index)).flatten(), } } diff --git a/editor/src/messages/portfolio/document/utility_types/network_interface/template.rs b/editor/src/messages/portfolio/document/utility_types/network_interface/template.rs index 3d13f7858b..cac31868c0 100644 --- a/editor/src/messages/portfolio/document/utility_types/network_interface/template.rs +++ b/editor/src/messages/portfolio/document/utility_types/network_interface/template.rs @@ -195,11 +195,6 @@ impl NodeTemplate { (document_node, persistent_node_metadata) } - /// The [`DocumentNode`] half alone, for callers performing raw network surgery. - pub fn into_document_node(self) -> DocumentNode { - self.into_parts().0 - } - /// Resizes `input_metadata` to match `inputs` at every nesting level, filling gaps with defaults. pub fn normalize_input_metadata(&mut self) { self.input_metadata.resize_with(self.inputs.len(), InputMetadata::default); 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 2f82a1229f..dc9b845054 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 @@ -239,23 +239,15 @@ impl Ports { } pub fn input_port_position(&self, index: usize) -> Option { - self.input_ports.iter().find_map(|(port_index, click_target)| { - if *port_index == index { - click_target.bounding_box().map(|bounds| bounds[0] + DVec2::new(8., 8.)) - } else { - None - } - }) + self.input_ports + .iter() + .find_map(|(port_index, click_target)| if *port_index == index { click_target.bounding_box_center() } else { None }) } pub fn output_port_position(&self, index: usize) -> Option { - self.output_ports.iter().find_map(|(port_index, click_target)| { - if *port_index == index { - click_target.bounding_box().map(|bounds| bounds[0] + DVec2::new(8., 8.)) - } else { - None - } - }) + self.output_ports + .iter() + .find_map(|(port_index, click_target)| if *port_index == index { click_target.bounding_box_center() } else { None }) } } diff --git a/editor/src/messages/portfolio/document/utility_types/network_interface/view.rs b/editor/src/messages/portfolio/document/utility_types/network_interface/view.rs index 3fb723d64d..737878f8f9 100644 --- a/editor/src/messages/portfolio/document/utility_types/network_interface/view.rs +++ b/editor/src/messages/portfolio/document/utility_types/network_interface/view.rs @@ -49,10 +49,6 @@ impl NodeNetworkInterface { } impl<'a, 'p> NetworkView<'a, 'p> { - pub fn network(&self) -> &'a NodeNetwork { - self.network - } - pub fn network_metadata(&self) -> &'a NodeNetworkMetadata { self.metadata } @@ -291,11 +287,7 @@ impl<'a, 'p> NetworkView<'a, 'p> { } pub fn upstream_output_connector(&self, input_connector: &InputConnector) -> Result, NetworkError> { - Ok(match self.input(input_connector)? { - NodeInput::Node { node_id, output_index, .. } => Some(OutputConnector::node(*node_id, *output_index)), - NodeInput::Import { import_index, .. } => Some(OutputConnector::Import(*import_index)), - _ => None, - }) + Ok(OutputConnector::from_input(self.input(input_connector)?)) } /// Whether the node reaches the exports by following wires downstream. @@ -326,16 +318,16 @@ impl<'a, 'p> NetworkView<'a, 'p> { while let Some(node) = stack.pop() { for input in &node.inputs { - if let &NodeInput::Node { node_id: ref_id, .. } = input { - if already_visited.contains(&ref_id) { + if let &NodeInput::Node { node_id: upstream_id, .. } = input { + if already_visited.contains(&upstream_id) { continue; } - if ref_id == *target_node_id { + if upstream_id == *target_node_id { return true; } - let Some(ref_node) = self.network.nodes.get(&ref_id) else { continue }; - already_visited.insert(ref_id); - stack.push(ref_node); + let Some(upstream_node) = self.network.nodes.get(&upstream_id) else { continue }; + already_visited.insert(upstream_id); + stack.push(upstream_node); } } } From 81925fd64e6e9c3dae4230fd4c49122ea7a28793 Mon Sep 17 00:00:00 2001 From: Keavon Chambers Date: Fri, 10 Jul 2026 21:59:14 -0700 Subject: [PATCH 4/5] Collapse the TransientMetadata enum into the Option inside TransientCache --- .../utility_types/network_interface.rs | 2 +- .../utility_types/network_interface/types.rs | 39 ++++--------------- 2 files changed, 8 insertions(+), 33 deletions(-) diff --git a/editor/src/messages/portfolio/document/utility_types/network_interface.rs b/editor/src/messages/portfolio/document/utility_types/network_interface.rs index a223222911..5558d114a8 100644 --- a/editor/src/messages/portfolio/document/utility_types/network_interface.rs +++ b/editor/src/messages/portfolio/document/utility_types/network_interface.rs @@ -64,7 +64,7 @@ pub struct NodeNetworkInterface { network: MemoNetwork, /// Stores all editor information for a NodeNetwork. Should automatically kept in sync by the setter methods when changes to the document network are made. network_metadata: NodeNetworkMetadata, - // TODO: Wrap in TransientMetadata Option + // TODO: Wrap in a TransientCache /// Stores the document network's structural topology. Should automatically kept in sync by the setter methods when changes to the document network are made. #[serde(skip)] document_metadata: DocumentMetadata, 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 dc9b845054..716708ef0e 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 @@ -350,62 +350,37 @@ pub struct NodeNetworkPersistentMetadata { pub selection_redo_history: VecDeque, } -/// This is the same as Option, but more clear in the context of having cached metadata either being loaded or unloaded -#[derive(Debug, Default, Clone)] -pub enum TransientMetadata { - Loaded(T), - #[default] - Unloaded, -} - -impl TransientMetadata { - /// Set the current transient metadata to unloaded - pub fn unload(&mut self) { - *self = TransientMetadata::Unloaded; - } - - pub fn is_loaded(&self) -> bool { - matches!(self, TransientMetadata::Loaded(_)) - } -} - /// A lazily computed cache slot whose load and read paths work through &self, with interior mutability guarding the stored value. #[derive(Debug, Clone)] -pub(crate) struct TransientCache(std::cell::RefCell>); +pub(crate) struct TransientCache(std::cell::RefCell>); impl Default for TransientCache { fn default() -> Self { - TransientCache(std::cell::RefCell::new(TransientMetadata::Unloaded)) + TransientCache(std::cell::RefCell::new(None)) } } impl TransientCache { pub(crate) fn is_loaded(&self) -> bool { - self.0.borrow().is_loaded() + self.0.borrow().is_some() } pub(crate) fn store(&self, value: T) { - *self.0.borrow_mut() = TransientMetadata::Loaded(value); + *self.0.borrow_mut() = Some(value); } pub(crate) fn unload(&self) { - *self.0.borrow_mut() = TransientMetadata::Unloaded; + *self.0.borrow_mut() = None; } /// Runs `read` on the cached value if it is loaded. pub(crate) fn with_loaded(&self, read: impl FnOnce(&T) -> R) -> Option { - match &*self.0.borrow() { - TransientMetadata::Loaded(value) => Some(read(value)), - TransientMetadata::Unloaded => None, - } + self.0.borrow().as_ref().map(read) } /// Direct access without runtime borrow tracking, for callers already holding exclusive access. pub(crate) fn get_loaded_mut(&mut self) -> Option<&mut T> { - match self.0.get_mut() { - TransientMetadata::Loaded(value) => Some(value), - TransientMetadata::Unloaded => None, - } + self.0.get_mut().as_mut() } } From 9fe7caeb4006925aac884cabd72627c52598a5f5 Mon Sep 17 00:00:00 2001 From: Keavon Chambers Date: Fri, 10 Jul 2026 21:59:14 -0700 Subject: [PATCH 5/5] Extract the shared history snapshot install from undo and redo --- .../document/document_message_handler.rs | 31 +++++++------------ 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/editor/src/messages/portfolio/document/document_message_handler.rs b/editor/src/messages/portfolio/document/document_message_handler.rs index 41a358a84f..1bc20e191c 100644 --- a/editor/src/messages/portfolio/document/document_message_handler.rs +++ b/editor/src/messages/portfolio/document/document_message_handler.rs @@ -2414,22 +2414,26 @@ impl DocumentMessageHandler { self.drive_storage_undo_redo(document_id, resource_storage, legacy_applied, true, responses); } - pub fn undo(&mut self, viewport: &ViewportMessageHandler, responses: &mut VecDeque) -> Option { - // If there is no history return and don't broadcast SelectionChanged - let mut network_interface = self.history.pop_undo()?; - + /// Installs a history snapshot as the active network interface, carrying over the current view state and structure load, and returns the replaced interface. + fn install_history_snapshot(&mut self, mut network_interface: NodeNetworkInterface, viewport: &ViewportMessageHandler) -> NodeNetworkInterface { // Set the previous network navigation metadata to the current navigation metadata network_interface.copy_all_navigation_metadata(&self.network_interface); std::mem::swap(&mut network_interface.resolved_types, &mut self.network_interface.resolved_types); - //Update the metadata transform based on document PTZ + // Update the metadata transform based on document PTZ let transform = self.navigation_handler.calculate_offset_transform(viewport.center_in_viewport_space().into(), &self.document_ptz); network_interface.set_document_to_viewport_transform(transform); // Ensure document structure is loaded so that updating the selected nodes has the correct metadata network_interface.load_structure(); - let previous_network = std::mem::replace(&mut self.network_interface, network_interface); + std::mem::replace(&mut self.network_interface, network_interface) + } + + pub fn undo(&mut self, viewport: &ViewportMessageHandler, responses: &mut VecDeque) -> Option { + // If there is no history return and don't broadcast SelectionChanged + let network_interface = self.history.pop_undo()?; + let previous_network = self.install_history_snapshot(network_interface, viewport); // Push the UpdateOpenDocumentsList message to the bus in order to update the save status of the open documents responses.add(PortfolioMessage::UpdateOpenDocumentsList); @@ -2454,20 +2458,9 @@ impl DocumentMessageHandler { pub fn redo(&mut self, viewport: &ViewportMessageHandler, responses: &mut VecDeque) -> Option { // If there is no history return and don't broadcast SelectionChanged - let mut network_interface = self.history.pop_redo()?; - - // Set the previous network navigation metadata to the current navigation metadata - network_interface.copy_all_navigation_metadata(&self.network_interface); - std::mem::swap(&mut network_interface.resolved_types, &mut self.network_interface.resolved_types); - - //Update the metadata transform based on document PTZ - let transform = self.navigation_handler.calculate_offset_transform(viewport.center_in_viewport_space().into(), &self.document_ptz); - network_interface.set_document_to_viewport_transform(transform); - - // Ensure document structure is loaded so that updating the selected nodes has the correct metadata - network_interface.load_structure(); + let network_interface = self.history.pop_redo()?; + let previous_network = self.install_history_snapshot(network_interface, viewport); - let previous_network = std::mem::replace(&mut self.network_interface, network_interface); // Push the UpdateOpenDocumentsList message to the bus in order to update the save status of the open documents responses.add(PortfolioMessage::UpdateOpenDocumentsList); responses.add(NodeGraphMessage::SelectedNodesUpdated);