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 <[email protected]>

Author: 0HyperCube

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)

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<VectorData> {
 		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

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<NodeId> {
+		// `.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<NodeId> {
 		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<NodeInput>> {
+		// `.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))
 	}
 

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,25 +380,15 @@ 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")?;
 
 		let (vector_data, transform) = path_data;
 		let point_count = vector_data.point_domain.ids().len();
 		let segment_count = vector_data.segment_domain.ids().len();
 
-		let actual_positions: Vec<DVec2> = 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<DVec2> = 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));