From 4ac609c7862072ef767da5d7bc1f9f28cfa6cb32 Mon Sep 17 00:00:00 2001 From: Dennis Kobert Date: Sun, 24 Aug 2025 09:25:38 +0200 Subject: [PATCH 1/5] Don't clone messages during batch processing --- editor/src/dispatcher.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/editor/src/dispatcher.rs b/editor/src/dispatcher.rs index 7e73a54fcf..94aa76f7bf 100644 --- a/editor/src/dispatcher.rs +++ b/editor/src/dispatcher.rs @@ -236,7 +236,7 @@ impl Dispatcher { } Message::NoOp => {} Message::Batched { messages } => { - messages.iter().for_each(|message| self.handle_message(message.to_owned(), false)); + messages.into_iter().for_each(|message| self.handle_message(message, false)); } } From e67b1c06994225df953cf12c9924c0468bbc0e88 Mon Sep 17 00:00:00 2001 From: Dennis Kobert Date: Wed, 10 Sep 2025 19:51:57 +0200 Subject: [PATCH 2/5] Improve selected nodes perf and memoize network hash computation --- .../utility_types/network_interface.rs | 83 +++++++++++++++++-- .../portfolio/document/utility_types/nodes.rs | 4 +- editor/src/node_graph_executor.rs | 2 +- node-graph/graph-craft/src/document.rs | 7 +- 4 files changed, 81 insertions(+), 15 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 a42a7c5c25..d732ca44b0 100644 --- a/editor/src/messages/portfolio/document/utility_types/network_interface.rs +++ b/editor/src/messages/portfolio/document/utility_types/network_interface.rs @@ -26,6 +26,7 @@ use graphene_std::vector::{PointId, Vector, VectorModificationType}; use interpreted_executor::dynamic_executor::ResolvedDocumentNodeTypes; use interpreted_executor::node_registry::NODE_REGISTRY; use kurbo::BezPath; +use private::MemoNetwork; use serde_json::{Value, json}; use std::collections::{HashMap, HashSet, VecDeque}; use std::hash::{DefaultHasher, Hash, Hasher}; @@ -36,7 +37,7 @@ use std::ops::Deref; pub struct NodeNetworkInterface { /// The node graph that generates this document's artwork. It recursively stores its sub-graphs, so this root graph is the whole snapshot of the document content. /// A public mutable reference should never be created. It should only be mutated through custom setters which perform the necessary side effects to keep network_metadata in sync - network: NodeNetwork, + 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 @@ -68,10 +69,69 @@ impl PartialEq for NodeNetworkInterface { } } +mod private { + use std::cell::RefCell; + use std::hash::{Hash, Hasher}; + + use graph_craft::document::NodeNetwork; + + #[derive(Debug, Default, Clone, PartialEq)] + pub struct MemoNetwork { + network: NodeNetwork, + hash_code: RefCell>, + } + + impl<'de> serde::Deserialize<'de> for MemoNetwork { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + Ok(Self::new(NodeNetwork::deserialize(deserializer)?)) + } + } + + impl serde::Serialize for MemoNetwork { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + self.network.serialize(serializer) + } + } + + impl Hash for MemoNetwork { + fn hash(&self, state: &mut H) { + self.current_hash().hash(state); + } + } + + impl MemoNetwork { + pub fn network(&self) -> &NodeNetwork { + &self.network + } + pub fn network_mut(&mut self) -> &mut NodeNetwork { + self.hash_code.replace(None); + &mut self.network + } + + pub fn new(network: NodeNetwork) -> Self { + Self { network, hash_code: None.into() } + } + + pub fn current_hash(&self) -> u64 { + let mut hash_code = self.hash_code.borrow_mut(); + if hash_code.is_none() { + *hash_code = Some(self.network.current_hash()); + } + hash_code.unwrap() + } + } +} + impl NodeNetworkInterface { /// Add DocumentNodePath input to the PathModifyNode protonode pub fn migrate_path_modify_node(&mut self) { - fix_network(&mut self.network); + fix_network(self.document_network_mut()); fn fix_network(network: &mut NodeNetwork) { for node in network.nodes.values_mut() { if let Some(network) = node.implementation.get_network_mut() { @@ -91,18 +151,25 @@ impl NodeNetworkInterface { impl NodeNetworkInterface { /// Gets the network of the root document pub fn document_network(&self) -> &NodeNetwork { - &self.network + self.network.network() + } + pub fn document_network_mut(&mut self) -> &mut NodeNetwork { + self.network.network_mut() } /// Gets the nested network based on network_path pub fn nested_network(&self, network_path: &[NodeId]) -> Option<&NodeNetwork> { - let Some(network) = self.network.nested_network(network_path) else { + let Some(network) = self.document_network().nested_network(network_path) else { log::error!("Could not get nested network with path {network_path:?} in NodeNetworkInterface::network"); return None; }; Some(network) } + pub fn network_hash(&self) -> u64 { + self.network.current_hash() + } + /// Get the specified document node in the nested network based on node_id and network_path pub fn document_node(&self, node_id: &NodeId, network_path: &[NodeId]) -> Option<&DocumentNode> { let network = self.nested_network(network_path)?; @@ -161,7 +228,7 @@ impl NodeNetworkInterface { .back() .cloned() .unwrap_or_default() - .filtered_selected_nodes(network_metadata.persistent_metadata.node_metadata.keys().cloned().collect()), + .filtered_selected_nodes(|node_id| network_metadata.persistent_metadata.node_metadata.contains_key(node_id)), ) } @@ -1556,7 +1623,7 @@ impl NodeNetworkInterface { log::error!("Could not get network or network_metadata in upstream_flow_back_from_nodes"); return FlowIter { stack: Vec::new(), - network: &self.network, + network: &self.document_network(), network_metadata: &self.network_metadata, flow_type: FlowType::UpstreamFlow, }; @@ -1708,7 +1775,7 @@ impl NodeNetworkInterface { } } Self { - network: node_network, + network: MemoNetwork::new(node_network), network_metadata, document_metadata: DocumentMetadata::default(), resolved_types: ResolvedDocumentNodeTypes::default(), @@ -1744,7 +1811,7 @@ fn random_protonode_implementation(protonode: &graph_craft::ProtoNodeIdentifier) // Private mutable getters for use within the network interface impl NodeNetworkInterface { fn network_mut(&mut self, network_path: &[NodeId]) -> Option<&mut NodeNetwork> { - self.network.nested_network_mut(network_path) + self.document_network_mut().nested_network_mut(network_path) } fn network_metadata_mut(&mut self, network_path: &[NodeId]) -> Option<&mut NodeNetworkMetadata> { diff --git a/editor/src/messages/portfolio/document/utility_types/nodes.rs b/editor/src/messages/portfolio/document/utility_types/nodes.rs index c120938a80..0e70f90fff 100644 --- a/editor/src/messages/portfolio/document/utility_types/nodes.rs +++ b/editor/src/messages/portfolio/document/utility_types/nodes.rs @@ -167,8 +167,8 @@ impl SelectedNodes { std::mem::replace(&mut self.0, new) } - pub fn filtered_selected_nodes(&self, node_ids: std::collections::HashSet) -> SelectedNodes { - SelectedNodes(self.0.iter().filter(|node_id| node_ids.contains(node_id)).cloned().collect()) + pub fn filtered_selected_nodes(&self, filter: impl Fn(&NodeId) -> bool) -> SelectedNodes { + SelectedNodes(self.0.iter().copied().filter(filter).collect()) } } diff --git a/editor/src/node_graph_executor.rs b/editor/src/node_graph_executor.rs index ee63999155..ff73192ef5 100644 --- a/editor/src/node_graph_executor.rs +++ b/editor/src/node_graph_executor.rs @@ -115,7 +115,7 @@ impl NodeGraphExecutor { /// Update the cached network if necessary. fn update_node_graph(&mut self, document: &mut DocumentMessageHandler, node_to_inspect: Option, ignore_hash: bool) -> Result<(), String> { - let network_hash = document.network_interface.document_network().current_hash(); + let network_hash = document.network_interface.network_hash(); // Refresh the graph when it changes or the inspect node changes if network_hash != self.node_graph_hash || self.previous_node_to_inspect != node_to_inspect || ignore_hash { let network = document.network_interface.document_network().clone(); diff --git a/node-graph/graph-craft/src/document.rs b/node-graph/graph-craft/src/document.rs index be798346a8..dc6265a2a7 100644 --- a/node-graph/graph-craft/src/document.rs +++ b/node-graph/graph-craft/src/document.rs @@ -9,7 +9,7 @@ pub use graphene_core::uuid::NodeId; pub use graphene_core::uuid::generate_uuid; use graphene_core::{Context, ContextDependencies, Cow, MemoHash, ProtoNodeIdentifier, Type}; use log::Metadata; -use rustc_hash::FxHashMap; +use rustc_hash::{FxBuildHasher, FxHashMap}; use std::collections::HashMap; use std::collections::hash_map::DefaultHasher; use std::hash::{Hash, Hasher}; @@ -551,9 +551,8 @@ impl PartialEq for NodeNetwork { /// Graph modification functions impl NodeNetwork { pub fn current_hash(&self) -> u64 { - let mut hasher = DefaultHasher::new(); - self.hash(&mut hasher); - hasher.finish() + use std::hash::BuildHasher; + FxBuildHasher.hash_one(self) } pub fn value_network(node: DocumentNode) -> Self { From d9e8a71cd095971008b2e24fe90bbab9d6467115 Mon Sep 17 00:00:00 2001 From: Dennis Kobert Date: Wed, 10 Sep 2025 22:04:31 +0200 Subject: [PATCH 3/5] Reuse click target bounding boxes for document bounds --- .../document/utility_types/document_metadata.rs | 16 +--------------- .../document/utility_types/network_interface.rs | 3 +-- .../tool/common_functionality/snapping.rs | 2 +- node-graph/gcore/src/vector/click_target.rs | 2 +- 4 files changed, 4 insertions(+), 19 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 acef970228..c2dfd988f5 100644 --- a/editor/src/messages/portfolio/document/utility_types/document_metadata.rs +++ b/editor/src/messages/portfolio/document/utility_types/document_metadata.rs @@ -154,21 +154,7 @@ impl DocumentMetadata { pub fn bounding_box_with_transform(&self, layer: LayerNodeIdentifier, transform: DAffine2) -> Option<[DVec2; 2]> { self.click_targets(layer)? .iter() - .filter_map(|click_target| match click_target.target_type() { - ClickTargetType::Subpath(subpath) => subpath.bounding_box_with_transform(transform), - ClickTargetType::FreePoint(_) => click_target.bounding_box_with_transform(transform), - }) - .reduce(Quad::combine_bounds) - } - - /// Get the loose bounding box of the click target of the specified layer in the specified transform space - pub fn loose_bounding_box_with_transform(&self, layer: LayerNodeIdentifier, transform: DAffine2) -> Option<[DVec2; 2]> { - self.click_targets(layer)? - .iter() - .filter_map(|click_target| match click_target.target_type() { - ClickTargetType::Subpath(subpath) => subpath.loose_bounding_box_with_transform(transform), - ClickTargetType::FreePoint(_) => click_target.bounding_box_with_transform(transform), - }) + .filter_map(|click_target| click_target.bounding_box_with_transform(transform)) .reduce(Quad::combine_bounds) } 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 d732ca44b0..b6bd4c32ba 100644 --- a/editor/src/messages/portfolio/document/utility_types/network_interface.rs +++ b/editor/src/messages/portfolio/document/utility_types/network_interface.rs @@ -3564,8 +3564,7 @@ impl NodeNetworkInterface { } self.document_metadata - .click_targets - .get(&layer) + .click_targets(layer) .map(|click| click.iter().map(ClickTarget::target_type)) .map(|target_types| Vector::from_target_types(target_types, true)) } diff --git a/editor/src/messages/tool/common_functionality/snapping.rs b/editor/src/messages/tool/common_functionality/snapping.rs index 27b658e9a9..4be02cb5a2 100644 --- a/editor/src/messages/tool/common_functionality/snapping.rs +++ b/editor/src/messages/tool/common_functionality/snapping.rs @@ -333,7 +333,7 @@ impl SnapManager { return; } // We use a loose bounding box here since these are potential candidates which will be filtered later anyway - let Some(bounds) = document.metadata().loose_bounding_box_with_transform(layer, DAffine2::IDENTITY) else { + let Some(bounds) = document.metadata().bounding_box_with_transform(layer, DAffine2::IDENTITY) else { return; }; let layer_bounds = document.metadata().transform_to_document(layer) * Quad::from_box(bounds); diff --git a/node-graph/gcore/src/vector/click_target.rs b/node-graph/gcore/src/vector/click_target.rs index 6ab7c1860a..1b665144b3 100644 --- a/node-graph/gcore/src/vector/click_target.rs +++ b/node-graph/gcore/src/vector/click_target.rs @@ -40,7 +40,7 @@ pub struct ClickTarget { impl ClickTarget { pub fn new_with_subpath(subpath: Subpath, stroke_width: f64) -> Self { - let bounding_box = subpath.loose_bounding_box(); + let bounding_box = subpath.bounding_box(); Self { target_type: ClickTargetType::Subpath(subpath), stroke_width, From 56a5b363326d9663e56903f1e8283072c5c749ae Mon Sep 17 00:00:00 2001 From: Dennis Kobert Date: Thu, 11 Sep 2025 09:41:19 +0200 Subject: [PATCH 4/5] Early terminate computing the connected count --- node-graph/gcore/src/vector/vector_attributes.rs | 5 +++++ node-graph/gcore/src/vector/vector_types.rs | 5 +++++ node-graph/gsvg-renderer/src/renderer.rs | 4 ++-- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/node-graph/gcore/src/vector/vector_attributes.rs b/node-graph/gcore/src/vector/vector_attributes.rs index 215473e5cd..2817a7561d 100644 --- a/node-graph/gcore/src/vector/vector_attributes.rs +++ b/node-graph/gcore/src/vector/vector_attributes.rs @@ -426,6 +426,11 @@ impl SegmentDomain { self.all_connected(point).count() } + /// Enumerate the number of segments connected to a point. If a segment starts and ends at a point then it is counted twice. + pub(crate) fn any_connected(&self, point: usize) -> bool { + self.all_connected(point).next().is_some() + } + /// Iterates over segments in the domain. /// /// Tuple is: (id, start point, end point, handles) diff --git a/node-graph/gcore/src/vector/vector_types.rs b/node-graph/gcore/src/vector/vector_types.rs index 7f26e40935..c49b12be59 100644 --- a/node-graph/gcore/src/vector/vector_types.rs +++ b/node-graph/gcore/src/vector/vector_types.rs @@ -322,6 +322,11 @@ impl Vector { self.point_domain.resolve_id(point).map_or(0, |point| self.segment_domain.connected_count(point)) } + /// Enumerate the number of segments connected to a point. If a segment starts and ends at a point then it is counted twice. + pub fn any_connected(&self, point: PointId) -> bool { + self.point_domain.resolve_id(point).is_some_and(|point| self.segment_domain.any_connected(point)) + } + pub fn check_point_inside_shape(&self, transform: DAffine2, point: DVec2) -> bool { let number = self .stroke_bezpath_iter() diff --git a/node-graph/gsvg-renderer/src/renderer.rs b/node-graph/gsvg-renderer/src/renderer.rs index d67b3442b9..f7b6015f93 100644 --- a/node-graph/gsvg-renderer/src/renderer.rs +++ b/node-graph/gsvg-renderer/src/renderer.rs @@ -1117,7 +1117,7 @@ impl Render for Table { // For free-floating anchors, we need to add a click target for each let single_anchors_targets = vector.point_domain.ids().iter().filter_map(|&point_id| { - if vector.connected_count(point_id) == 0 { + if !vector.any_connected(point_id) { let anchor = vector.point_domain.position_from_id(point_id).unwrap_or_default(); let point = FreePoint::new(point_id, anchor); @@ -1162,7 +1162,7 @@ impl Render for Table { // For free-floating anchors, we need to add a click target for each let single_anchors_targets = row.element.point_domain.ids().iter().filter_map(|&point_id| { - if row.element.connected_count(point_id) > 0 { + if row.element.any_connected(point_id) { return None; } From e1945771cebee79326170fb51f20f70d1178e4e8 Mon Sep 17 00:00:00 2001 From: Dennis Kobert Date: Thu, 11 Sep 2025 11:10:52 +0200 Subject: [PATCH 5/5] Cleanup --- .../utility_types/network_interface.rs | 62 +------------------ .../network_interface/memo_network.rs | 57 +++++++++++++++++ 2 files changed, 59 insertions(+), 60 deletions(-) create mode 100644 editor/src/messages/portfolio/document/utility_types/network_interface/memo_network.rs 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 b6bd4c32ba..53d40ee6ec 100644 --- a/editor/src/messages/portfolio/document/utility_types/network_interface.rs +++ b/editor/src/messages/portfolio/document/utility_types/network_interface.rs @@ -1,4 +1,5 @@ mod deserialization; +mod memo_network; use super::document_metadata::{DocumentMetadata, LayerNodeIdentifier, NodeRelations}; use super::misc::PTZ; @@ -26,7 +27,7 @@ use graphene_std::vector::{PointId, Vector, VectorModificationType}; use interpreted_executor::dynamic_executor::ResolvedDocumentNodeTypes; use interpreted_executor::node_registry::NODE_REGISTRY; use kurbo::BezPath; -use private::MemoNetwork; +use memo_network::MemoNetwork; use serde_json::{Value, json}; use std::collections::{HashMap, HashSet, VecDeque}; use std::hash::{DefaultHasher, Hash, Hasher}; @@ -69,65 +70,6 @@ impl PartialEq for NodeNetworkInterface { } } -mod private { - use std::cell::RefCell; - use std::hash::{Hash, Hasher}; - - use graph_craft::document::NodeNetwork; - - #[derive(Debug, Default, Clone, PartialEq)] - pub struct MemoNetwork { - network: NodeNetwork, - hash_code: RefCell>, - } - - impl<'de> serde::Deserialize<'de> for MemoNetwork { - fn deserialize(deserializer: D) -> Result - where - D: serde::Deserializer<'de>, - { - Ok(Self::new(NodeNetwork::deserialize(deserializer)?)) - } - } - - impl serde::Serialize for MemoNetwork { - fn serialize(&self, serializer: S) -> Result - where - S: serde::Serializer, - { - self.network.serialize(serializer) - } - } - - impl Hash for MemoNetwork { - fn hash(&self, state: &mut H) { - self.current_hash().hash(state); - } - } - - impl MemoNetwork { - pub fn network(&self) -> &NodeNetwork { - &self.network - } - pub fn network_mut(&mut self) -> &mut NodeNetwork { - self.hash_code.replace(None); - &mut self.network - } - - pub fn new(network: NodeNetwork) -> Self { - Self { network, hash_code: None.into() } - } - - pub fn current_hash(&self) -> u64 { - let mut hash_code = self.hash_code.borrow_mut(); - if hash_code.is_none() { - *hash_code = Some(self.network.current_hash()); - } - hash_code.unwrap() - } - } -} - impl NodeNetworkInterface { /// Add DocumentNodePath input to the PathModifyNode protonode pub fn migrate_path_modify_node(&mut self) { diff --git a/editor/src/messages/portfolio/document/utility_types/network_interface/memo_network.rs b/editor/src/messages/portfolio/document/utility_types/network_interface/memo_network.rs new file mode 100644 index 0000000000..e6f7b126fd --- /dev/null +++ b/editor/src/messages/portfolio/document/utility_types/network_interface/memo_network.rs @@ -0,0 +1,57 @@ +use graph_craft::document::NodeNetwork; +use std::cell::Cell; +use std::hash::{Hash, Hasher}; + +#[derive(Debug, Default, Clone, PartialEq)] +pub struct MemoNetwork { + network: NodeNetwork, + hash_code: Cell>, +} + +impl<'de> serde::Deserialize<'de> for MemoNetwork { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + Ok(Self::new(NodeNetwork::deserialize(deserializer)?)) + } +} + +impl serde::Serialize for MemoNetwork { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + self.network.serialize(serializer) + } +} + +impl Hash for MemoNetwork { + fn hash(&self, state: &mut H) { + self.current_hash().hash(state); + } +} + +impl MemoNetwork { + pub fn network(&self) -> &NodeNetwork { + &self.network + } + + pub fn network_mut(&mut self) -> &mut NodeNetwork { + self.hash_code.set(None); + &mut self.network + } + + pub fn new(network: NodeNetwork) -> Self { + Self { network, hash_code: None.into() } + } + + pub fn current_hash(&self) -> u64 { + let mut hash_code = self.hash_code.get(); + if hash_code.is_none() { + hash_code = Some(self.network.current_hash()); + self.hash_code.set(hash_code); + } + hash_code.unwrap() + } +}