From 1e3c3da3fe092c2421530005e7eaa8201063827e Mon Sep 17 00:00:00 2001 From: James Lindsay <78500760+0HyperCube@users.noreply.github.com> Date: Sat, 2 Aug 2025 23:53:05 +0100 Subject: [PATCH] Fix the Path tool erroneously showing editable geometry overlays belonging to hidden Path nodes (#2932) * Ignore hidden path nodes * Use correct path node vector modification * Break test * Better fix for test * Fix rustfmt --------- Co-authored-by: Keavon Chambers --- .../utility_types/document_metadata.rs | 2 +- .../utility_types/network_interface.rs | 15 +++++++++----- .../graph_modification_utils.rs | 17 +++++++++++++--- .../tool/tool_messages/freehand_tool.rs | 20 +++++++------------ 4 files changed, 32 insertions(+), 22 deletions(-) diff --git a/editor/src/messages/portfolio/document/utility_types/document_metadata.rs b/editor/src/messages/portfolio/document/utility_types/document_metadata.rs index 600d4958..0f4286af 100644 --- a/editor/src/messages/portfolio/document/utility_types/document_metadata.rs +++ b/editor/src/messages/portfolio/document/utility_types/document_metadata.rs @@ -90,7 +90,7 @@ impl DocumentMetadata { let mut use_local = true; let graph_layer = graph_modification_utils::NodeGraphLayer::new(layer, network_interface); - if let Some(path_node) = graph_layer.upstream_node_id_from_name("Path") { + if let Some(path_node) = graph_layer.upstream_visible_node_id_from_name_in_layer("Path") { if let Some(&source) = self.first_instance_source_ids.get(&layer.to_node()) { if !network_interface .upstream_flow_back_from_nodes(vec![path_node], &[], FlowType::HorizontalFlow) 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 85bd54e2..3d557aea 100644 --- a/editor/src/messages/portfolio/document/utility_types/network_interface.rs +++ b/editor/src/messages/portfolio/document/utility_types/network_interface.rs @@ -3445,12 +3445,17 @@ impl NodeNetworkInterface { pub fn compute_modified_vector(&self, layer: LayerNodeIdentifier) -> Option { let graph_layer = graph_modification_utils::NodeGraphLayer::new(layer, self); - if let Some(vector_data) = graph_layer.upstream_node_id_from_name("Path").and_then(|node| self.document_metadata.vector_modify.get(&node)) { - let mut modified = vector_data.clone(); - if let Some(TaggedValue::VectorModification(modification)) = graph_layer.find_input("Path", 1) { - modification.apply(&mut modified); + if let Some(path_node) = graph_layer.upstream_visible_node_id_from_name_in_layer("Path") { + if let Some(vector_data) = self.document_metadata.vector_modify.get(&path_node) { + let mut modified = vector_data.clone(); + + let path_node = self.document_network().nodes.get(&path_node); + let modification_input = path_node.and_then(|node: &DocumentNode| node.inputs.get(1)).and_then(|input| input.as_value()); + if let Some(TaggedValue::VectorModification(modification)) = modification_input { + modification.apply(&mut modified); + } + return Some(modified); } - return Some(modified); } self.document_metadata diff --git a/editor/src/messages/tool/common_functionality/graph_modification_utils.rs b/editor/src/messages/tool/common_functionality/graph_modification_utils.rs index 3ba8350a..2bc1ca14 100644 --- a/editor/src/messages/tool/common_functionality/graph_modification_utils.rs +++ b/editor/src/messages/tool/common_functionality/graph_modification_utils.rs @@ -433,6 +433,16 @@ impl<'a> NodeGraphLayer<'a> { .find(|node_id| self.network_interface.reference(node_id, &[]).is_some_and(|reference| *reference == Some(node_name.to_string()))) } + /// Node id of a visible node if it exists in the layer's primary flow until another layer + pub fn upstream_visible_node_id_from_name_in_layer(&self, node_name: &str) -> Option { + // `.skip(1)` is used to skip self + self.horizontal_layer_flow() + .skip(1) + .take_while(|node_id| !self.network_interface.is_layer(node_id, &[])) + .filter(|node_id| self.network_interface.is_visible(node_id, &[])) + .find(|node_id| self.network_interface.reference(node_id, &[]).is_some_and(|reference| *reference == Some(node_name.to_string()))) + } + /// Node id of a protonode if it exists in the layer's primary flow pub fn upstream_node_id_from_protonode(&self, protonode_identifier: ProtoNodeIdentifier) -> Option { self.horizontal_layer_flow() @@ -447,10 +457,11 @@ impl<'a> NodeGraphLayer<'a> { /// Find all of the inputs of a specific node within the layer's primary flow, up until the next layer is reached. pub fn find_node_inputs(&self, node_name: &str) -> Option<&'a Vec> { + // `.skip(1)` is used to skip self self.horizontal_layer_flow() - .skip(1)// Skip self - .take_while(|node_id| !self.network_interface.is_layer(node_id,&[])) - .find(|node_id| self.network_interface.reference(node_id,&[]).is_some_and(|reference| *reference == Some(node_name.to_string()))) + .skip(1) + .take_while(|node_id| !self.network_interface.is_layer(node_id, &[])) + .find(|node_id| self.network_interface.reference(node_id, &[]).is_some_and(|reference| *reference == Some(node_name.to_string()))) .and_then(|node_id| self.network_interface.document_network().nodes.get(&node_id).map(|node| &node.inputs)) } diff --git a/editor/src/messages/tool/tool_messages/freehand_tool.rs b/editor/src/messages/tool/tool_messages/freehand_tool.rs index 8663180f..29775f0e 100644 --- a/editor/src/messages/tool/tool_messages/freehand_tool.rs +++ b/editor/src/messages/tool/tool_messages/freehand_tool.rs @@ -356,7 +356,7 @@ fn extend_path_with_next_segment(tool_data: &mut FreehandToolData, position: DVe mod test_freehand { use crate::messages::input_mapper::utility_types::input_mouse::{EditorMouseState, MouseKeys, ScrollDelta}; use crate::messages::portfolio::document::graph_operation::utility_types::TransformIn; - use crate::messages::tool::common_functionality::graph_modification_utils::get_stroke_width; + use crate::messages::tool::common_functionality::graph_modification_utils::{NodeGraphLayer, get_stroke_width}; use crate::messages::tool::tool_messages::freehand_tool::FreehandOptionsUpdate; use crate::test_utils::test_prelude::*; use glam::{DAffine2, DVec2}; @@ -368,6 +368,10 @@ mod test_freehand { layers .filter_map(|layer| { + let graph_layer = NodeGraphLayer::new(layer, &document.network_interface); + // Only get layers with path nodes + let _ = graph_layer.upstream_visible_node_id_from_name_in_layer("Path")?; + let vector_data = document.network_interface.compute_modified_vector(layer)?; let transform = document.metadata().transform_to_viewport(layer); Some((vector_data, transform)) @@ -376,9 +380,7 @@ mod test_freehand { } fn verify_path_points(vector_data_list: &[(VectorData, DAffine2)], expected_captured_points: &[DVec2], tolerance: f64) -> Result<(), String> { - if vector_data_list.len() == 0 { - return Err("No vector data found after drawing".to_string()); - } + assert_eq!(vector_data_list.len(), 1, "there should be one vector data"); let path_data = vector_data_list.iter().find(|(data, _)| data.point_domain.ids().len() > 0).ok_or("Could not find path data")?; @@ -386,15 +388,7 @@ mod test_freehand { let point_count = vector_data.point_domain.ids().len(); let segment_count = vector_data.segment_domain.ids().len(); - let actual_positions: Vec = vector_data - .point_domain - .ids() - .iter() - .filter_map(|&point_id| { - let position = vector_data.point_domain.position_from_id(point_id)?; - Some(transform.transform_point2(position)) - }) - .collect(); + let actual_positions: Vec = vector_data.point_domain.positions().iter().map(|&position| transform.transform_point2(position)).collect(); if segment_count != point_count - 1 { return Err(format!("Expected segments to be one less than points, got {} segments for {} points", segment_count, point_count));