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 <keavon@keavon.com>
This commit is contained in:
James Lindsay 2025-08-02 23:53:05 +01:00 committed by GitHub
parent c42011f8e2
commit 1e3c3da3fe
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 32 additions and 22 deletions

View File

@ -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)

View File

@ -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

View File

@ -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))
}

View File

@ -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<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));