Fix the Path tool adding a Path node but not applying the change until a second attempt due to wrong segment IDs (#3727)

* Fix SegmentIds lost when no Path node exists

* fix import order

* Use Arc<Vector> in vector_data for regression
This commit is contained in:
Ayush Amawate
2026-02-13 14:40:38 -08:00
committed by GitHub
parent e606efd008
commit 119a554260
6 changed files with 32 additions and 5 deletions
@@ -15,6 +15,7 @@ use graphene_std::Color;
use graphene_std::raster::BlendMode; use graphene_std::raster::BlendMode;
use graphene_std::raster::Image; use graphene_std::raster::Image;
use graphene_std::transform::Footprint; use graphene_std::transform::Footprint;
use graphene_std::vector::Vector;
use graphene_std::vector::click_target::ClickTarget; use graphene_std::vector::click_target::ClickTarget;
use graphene_std::vector::style::RenderMode; use graphene_std::vector::style::RenderMode;
@@ -209,6 +210,9 @@ pub enum DocumentMessage {
UpdateClipTargets { UpdateClipTargets {
clip_targets: HashSet<NodeId>, clip_targets: HashSet<NodeId>,
}, },
UpdateVectorData {
vector_data: HashMap<NodeId, Arc<Vector>>,
},
Undo, Undo,
UngroupSelectedLayers, UngroupSelectedLayers,
UngroupLayer { UngroupLayer {
@@ -1424,6 +1424,20 @@ impl MessageHandler<DocumentMessage, DocumentMessageContext<'_>> for DocumentMes
DocumentMessage::UpdateClipTargets { clip_targets } => { DocumentMessage::UpdateClipTargets { clip_targets } => {
self.network_interface.update_clip_targets(clip_targets); self.network_interface.update_clip_targets(clip_targets);
} }
DocumentMessage::UpdateVectorData { vector_data } => {
// Convert NodeId keys to LayerNodeIdentifier keys, filtering to only layers
let layer_vector_data = vector_data
.into_iter()
.filter(|(node_id, _)| self.network_interface.document_network().nodes.contains_key(node_id))
.filter_map(|(node_id, vector)| {
self.network_interface.is_layer(&node_id, &[]).then(|| {
let layer = LayerNodeIdentifier::new(node_id, &self.network_interface);
(layer, vector)
})
})
.collect();
self.network_interface.update_vector_data(layer_vector_data);
}
DocumentMessage::Undo => { DocumentMessage::Undo => {
if self.network_interface.transaction_status() != TransactionStatus::Finished { if self.network_interface.transaction_status() != TransactionStatus::Finished {
return; return;
@@ -30,6 +30,9 @@ pub struct DocumentMetadata {
pub click_targets: HashMap<LayerNodeIdentifier, Vec<Arc<ClickTarget>>>, pub click_targets: HashMap<LayerNodeIdentifier, Vec<Arc<ClickTarget>>>,
pub clip_targets: HashSet<NodeId>, pub clip_targets: HashSet<NodeId>,
pub vector_modify: HashMap<NodeId, Vector>, pub vector_modify: HashMap<NodeId, Vector>,
/// Vector data keyed by layer ID, used as fallback when no Path node exists.
/// This provides accurate SegmentIds for layers without explicit Path nodes.
pub layer_vector_data: HashMap<LayerNodeIdentifier, Arc<Vector>>,
/// Transform from document space to viewport space. /// Transform from document space to viewport space.
pub document_to_viewport: DAffine2, pub document_to_viewport: DAffine2,
} }
@@ -3076,11 +3076,7 @@ impl NodeNetworkInterface {
return Some(modified); return Some(modified);
} }
self.document_metadata self.document_metadata.layer_vector_data.get(&layer).map(|arc| arc.as_ref().clone())
.click_targets
.get(&layer)
.map(|click| click.iter().map(|x| x.target_type()))
.map(|target_types| Vector::from_target_types(target_types, true))
} }
/// Loads the structure of layer nodes from a node graph. /// Loads the structure of layer nodes from a node graph.
@@ -3194,6 +3190,11 @@ impl NodeNetworkInterface {
pub fn update_vector_modify(&mut self, new_vector_modify: HashMap<NodeId, Vector>) { pub fn update_vector_modify(&mut self, new_vector_modify: HashMap<NodeId, Vector>) {
self.document_metadata.vector_modify = new_vector_modify; self.document_metadata.vector_modify = new_vector_modify;
} }
/// Update the layer vector data (for layers without Path nodes)
pub fn update_vector_data(&mut self, new_layer_vector_data: HashMap<LayerNodeIdentifier, Arc<Vector>>) {
self.document_metadata.layer_vector_data = new_layer_vector_data;
}
} }
// Public mutable methods // Public mutable methods
+2
View File
@@ -412,6 +412,7 @@ impl NodeGraphExecutor {
first_element_source_id, first_element_source_id,
click_targets, click_targets,
clip_targets, clip_targets,
vector_data,
} = render_output.metadata; } = render_output.metadata;
// Run these update state messages immediately // Run these update state messages immediately
@@ -422,6 +423,7 @@ impl NodeGraphExecutor {
}); });
responses.add(DocumentMessage::UpdateClickTargets { click_targets }); responses.add(DocumentMessage::UpdateClickTargets { click_targets });
responses.add(DocumentMessage::UpdateClipTargets { clip_targets }); responses.add(DocumentMessage::UpdateClipTargets { clip_targets });
responses.add(DocumentMessage::UpdateVectorData { vector_data });
responses.add(DocumentMessage::RenderScrollbars); responses.add(DocumentMessage::RenderScrollbars);
responses.add(DocumentMessage::RenderRulers); responses.add(DocumentMessage::RenderRulers);
responses.add(OverlaysMessage::Draw); responses.add(OverlaysMessage::Draw);
@@ -249,6 +249,7 @@ pub struct RenderMetadata {
pub first_element_source_id: HashMap<NodeId, Option<NodeId>>, pub first_element_source_id: HashMap<NodeId, Option<NodeId>>,
pub click_targets: HashMap<NodeId, Vec<Arc<ClickTarget>>>, pub click_targets: HashMap<NodeId, Vec<Arc<ClickTarget>>>,
pub clip_targets: HashSet<NodeId>, pub clip_targets: HashSet<NodeId>,
pub vector_data: HashMap<NodeId, Arc<Vector>>,
} }
impl RenderMetadata { impl RenderMetadata {
@@ -1187,6 +1188,8 @@ impl Render for Table<Vector> {
.collect::<Vec<_>>(); .collect::<Vec<_>>();
metadata.click_targets.entry(element_id).or_insert(click_targets); metadata.click_targets.entry(element_id).or_insert(click_targets);
// Store the full vector data including segment IDs for accurate segment modification
metadata.vector_data.entry(element_id).or_insert_with(|| Arc::new(vector.clone()));
} }
if let Some(upstream_nested_layers) = &vector.upstream_data { if let Some(upstream_nested_layers) = &vector.upstream_data {