Fix abysmal O(n^2) SVG import performance (#3938)

* Remove O(n^2) import by disabling bumping

* Add an import mode to avoid acyclic checks

* Rebuild the layer tree at the end, not after each step

* Incrementally update outward wires instead of repeatedly rebuilding them

* Add import->export direct connection guard

* Code review fixes

* Replace magic number offsets with consts

* Add consts for magic numbers

* Improve code structuring

Author: Keavon

editor/src/consts.rs

@@ -4,6 +4,10 @@ pub const EXPORTS_TO_TOP_EDGE_PIXEL_GAP: u32 = 72;
 pub const EXPORTS_TO_RIGHT_EDGE_PIXEL_GAP: u32 = 120;
 pub const IMPORTS_TO_TOP_EDGE_PIXEL_GAP: u32 = 72;
 pub const IMPORTS_TO_LEFT_EDGE_PIXEL_GAP: u32 = 120;
+/// Vertical grid distance between adjacent stack siblings, or between a parent layer and its first stack child.
+pub const STACK_VERTICAL_GAP: i32 = 3;
+/// Horizontal grid indentation of a child layer relative to its parent layer.
+pub const LAYER_INDENT_OFFSET: i32 = 8;
 
 // VIEWPORT
 pub const VIEWPORT_ZOOM_WHEEL_RATE: f64 = (1. / 600.) * 3.;

editor/src/messages/portfolio/document/graph_operation/graph_operation_message_handler.rs

@@ -1,5 +1,6 @@
 use super::transform_utils;
 use super::utility_types::ModifyInputsContext;
+use crate::consts::{LAYER_INDENT_OFFSET, STACK_VERTICAL_GAP};
 use crate::messages::portfolio::document::graph_operation::utility_types::TransformIn;
 use crate::messages::portfolio::document::utility_types::document_metadata::LayerNodeIdentifier;
 use crate::messages::portfolio::document::utility_types::network_interface::{InputConnector, NodeNetworkInterface, OutputConnector};
@@ -442,6 +443,12 @@ fn parse_hex_stop_color(hex: &str, opacity: f32) -> Option<Color> {
 	Some(Color::from_rgbaf32_unchecked(r, g, b, opacity))
 }
 
+/// Import a usvg node as the root of an SVG import operation.
+///
+/// The root layer uses the full `move_layer_to_stack` (with push/collision logic) to correctly
+/// interact with any existing layers in the parent stack. All descendant layers use a lightweight
+/// O(n) import path that skips collision detection and instead calculates positions directly from
+/// the known tree structure.
 fn import_usvg_node(
 	modify_inputs: &mut ModifyInputsContext,
 	node: &usvg::Node,
@@ -452,44 +459,192 @@ fn import_usvg_node(
 	graphite_gradient_stops: &HashMap<String, GradientStops>,
 ) {
 	let layer = modify_inputs.create_layer(id);
+
 	modify_inputs.network_interface.move_layer_to_stack(layer, parent, insert_index, &[]);
 	modify_inputs.layer_node = Some(layer);
 	if let Some(upstream_layer) = layer.next_sibling(modify_inputs.network_interface.document_metadata()) {
-		modify_inputs.network_interface.shift_node(&upstream_layer.to_node(), IVec2::new(0, 3), &[]);
+		modify_inputs.network_interface.shift_node(&upstream_layer.to_node(), IVec2::new(0, STACK_VERTICAL_GAP), &[]);
 	}
+
 	match node {
 		usvg::Node::Group(group) => {
+			// Collect child extents for O(n) position calculation
+			let mut child_extents_svg_order: Vec<u32> = Vec::new();
+			let mut group_extents_map: HashMap<LayerNodeIdentifier, Vec<u32>> = HashMap::new();
+
+			// Enable import mode: skips expensive is_acyclic checks and per-node cache invalidation
+			// during wiring since we're building a known tree structure where cycles are impossible
+			modify_inputs.import = true;
+
 			for child in group.children() {
-				import_usvg_node(modify_inputs, child, transform, NodeId::new(), layer, 0, graphite_gradient_stops);
+				let extent = import_usvg_node_inner(modify_inputs, child, transform, NodeId::new(), layer, 0, graphite_gradient_stops, &mut group_extents_map);
+				child_extents_svg_order.push(extent);
 			}
+
+			modify_inputs.import = false;
 			modify_inputs.layer_node = Some(layer);
+
+			// Rebuild the layer tree once now that all wiring is complete
+			modify_inputs.network_interface.load_structure();
+
+			// Set positions for all imported descendants in a single O(n) pass
+			let parent_pos = modify_inputs.network_interface.position(&layer.to_node(), &[]).unwrap_or(IVec2::ZERO);
+			set_import_child_positions(modify_inputs.network_interface, layer, parent_pos, &child_extents_svg_order, &group_extents_map);
+
+			// Invalidate caches once after all positions are set
+			modify_inputs.network_interface.unload_all_nodes_click_targets(&[]);
+			modify_inputs.network_interface.unload_all_nodes_bounding_box(&[]);
 		}
 		usvg::Node::Path(path) => {
-			let subpaths = convert_usvg_path(path);
-			let bounds = subpaths.iter().filter_map(|subpath| subpath.bounding_box()).reduce(Quad::combine_bounds).unwrap_or_default();
+			import_usvg_path(modify_inputs, node, path, transform, layer, graphite_gradient_stops);
+		}
+		usvg::Node::Image(_image) => {
+			warn!("Skip image");
+		}
+		usvg::Node::Text(text) => {
+			let font = Font::new(graphene_std::consts::DEFAULT_FONT_FAMILY.to_string(), graphene_std::consts::DEFAULT_FONT_STYLE.to_string());
+			modify_inputs.insert_text(text.chunks().iter().map(|chunk| chunk.text()).collect(), font, TypesettingConfig::default(), layer);
+			modify_inputs.fill_set(Fill::Solid(Color::BLACK));
+		}
+	}
+}
 
-			modify_inputs.insert_vector(subpaths, layer, true, path.fill().is_some(), path.stroke().is_some());
+/// Recursively import a usvg node as a descendant of the root import layer.
+/// Uses lightweight wiring (no push/collision) and returns the subtree extent for position calculation.
+///
+/// The subtree extent represents the additional vertical grid units that this node's descendants
+/// occupy below the node's position. This is used to calculate correct y_offsets between siblings.
+fn import_usvg_node_inner(
+	modify_inputs: &mut ModifyInputsContext,
+	node: &usvg::Node,
+	transform: DAffine2,
+	id: NodeId,
+	parent: LayerNodeIdentifier,
+	insert_index: usize,
+	graphite_gradient_stops: &HashMap<String, GradientStops>,
+	group_extents_map: &mut HashMap<LayerNodeIdentifier, Vec<u32>>,
+) -> u32 {
+	let layer = modify_inputs.create_layer(id);
+	modify_inputs.network_interface.move_layer_to_stack_for_import(layer, parent, insert_index, &[]);
+	modify_inputs.layer_node = Some(layer);
 
-			if let Some(transform_node_id) = modify_inputs.existing_network_node_id("Transform", true) {
-				transform_utils::update_transform(modify_inputs.network_interface, &transform_node_id, transform * usvg_transform(node.abs_transform()));
+	match node {
+		usvg::Node::Group(group) => {
+			let mut child_extents: Vec<u32> = Vec::new();
+			for child in group.children() {
+				let extent = import_usvg_node_inner(modify_inputs, child, transform, NodeId::new(), layer, 0, graphite_gradient_stops, group_extents_map);
+				child_extents.push(extent);
 			}
+			modify_inputs.layer_node = Some(layer);
 
-			if let Some(fill) = path.fill() {
-				let bounds_transform = DAffine2::from_scale_angle_translation(bounds[1] - bounds[0], 0., bounds[0]);
-				apply_usvg_fill(fill, modify_inputs, bounds_transform, graphite_gradient_stops);
-			}
-			if let Some(stroke) = path.stroke() {
-				apply_usvg_stroke(stroke, modify_inputs, transform * usvg_transform(node.abs_transform()));
-			}
+			let n = child_extents.len();
+			let total_extent = if n == 0 {
+				0
+			} else {
+				(2 * STACK_VERTICAL_GAP as u32) * n as u32 - STACK_VERTICAL_GAP as u32 + child_extents.iter().sum::<u32>()
+			};
+			group_extents_map.insert(layer, child_extents);
+			total_extent
+		}
+		usvg::Node::Path(path) => {
+			import_usvg_path(modify_inputs, node, path, transform, layer, graphite_gradient_stops);
+			0
 		}
 		usvg::Node::Image(_image) => {
-			warn!("Skip image")
+			warn!("Skip image");
+			0
 		}
 		usvg::Node::Text(text) => {
 			let font = Font::new(graphene_std::consts::DEFAULT_FONT_FAMILY.to_string(), graphene_std::consts::DEFAULT_FONT_STYLE.to_string());
 			modify_inputs.insert_text(text.chunks().iter().map(|chunk| chunk.text()).collect(), font, TypesettingConfig::default(), layer);
 			modify_inputs.fill_set(Fill::Solid(Color::BLACK));
+			0
+		}
+	}
+}
+
+/// Helper to apply path data (vector geometry, fill, stroke, transform) to a layer.
+fn import_usvg_path(
+	modify_inputs: &mut ModifyInputsContext,
+	node: &usvg::Node,
+	path: &usvg::Path,
+	transform: DAffine2,
+	layer: LayerNodeIdentifier,
+	graphite_gradient_stops: &HashMap<String, GradientStops>,
+) {
+	let subpaths = convert_usvg_path(path);
+	let bounds = subpaths.iter().filter_map(|subpath| subpath.bounding_box()).reduce(Quad::combine_bounds).unwrap_or_default();
+
+	modify_inputs.insert_vector(subpaths, layer, true, path.fill().is_some(), path.stroke().is_some());
+
+	if let Some(transform_node_id) = modify_inputs.existing_network_node_id("Transform", true) {
+		transform_utils::update_transform(modify_inputs.network_interface, &transform_node_id, transform * usvg_transform(node.abs_transform()));
+	}
+
+	if let Some(fill) = path.fill() {
+		let bounds_transform = DAffine2::from_scale_angle_translation(bounds[1] - bounds[0], 0., bounds[0]);
+		apply_usvg_fill(fill, modify_inputs, bounds_transform, graphite_gradient_stops);
+	}
+	if let Some(stroke) = path.stroke() {
+		apply_usvg_stroke(stroke, modify_inputs, transform * usvg_transform(node.abs_transform()));
+	}
+}
+
+/// Set correct positions for all imported layers in a single top-down O(n) pass.
+///
+/// For each group's child stack:
+/// - The top-of-stack child (last SVG child) gets an `Absolute` position at `(parent_x - LAYER_INDENT_OFFSET, parent_y + STACK_VERTICAL_GAP)`
+/// - All other children get `Stack(y_offset)` where `y_offset` accounts for the subtree extent of the sibling above them in the stack, ensuring no overlap.
+fn set_import_child_positions(
+	network_interface: &mut NodeNetworkInterface,
+	group: LayerNodeIdentifier,
+	group_pos: IVec2,
+	child_extents_svg_order: &[u32],
+	group_extents_map: &HashMap<LayerNodeIdentifier, Vec<u32>>,
+) {
+	use crate::messages::portfolio::document::utility_types::network_interface::LayerPosition;
+
+	let layer_children: Vec<_> = group.children(network_interface.document_metadata()).collect();
+	let n = child_extents_svg_order.len();
+
+	if n == 0 || layer_children.is_empty() {
+		return;
+	}
+
+	// Children in the layer tree are in stack order (top to bottom), which is the REVERSE of SVG order.
+	// SVG order:   [s_0,     s_1,     ..., s_{n-1}] with extents [e_0, e_1, ..., e_{n-1}]
+	// Stack order: [s_{n-1}, s_{n-2}, ..., s_0    ] (top to bottom)
+	//
+	// For stack child at index i:
+	//   - SVG index = n - 1 - i
+	//   - Previous stack sibling's SVG index = n - i
+	//   - y_offset = extent_of_previous_sibling + STACK_VERTICAL_GAP
+
+	let child_x = group_pos.x - LAYER_INDENT_OFFSET;
+	let mut current_y = group_pos.y + STACK_VERTICAL_GAP;
+
+	for (i, child_layer) in layer_children.iter().enumerate() {
+		let child_pos = IVec2::new(child_x, current_y);
+
+		if i == 0 {
+			// Top of stack — set to `Absolute` position
+			network_interface.set_layer_position_for_import(&child_layer.to_node(), LayerPosition::Absolute(child_pos), &[]);
+		} else {
+			// Below top — set `Stack` with `y_offset` based on previous sibling's subtree extent
+			let prev_sibling_svg_index = n - i;
+			let y_offset = child_extents_svg_order[prev_sibling_svg_index] + STACK_VERTICAL_GAP as u32;
+			network_interface.set_layer_position_for_import(&child_layer.to_node(), LayerPosition::Stack(y_offset), &[]);
 		}
+
+		// Recurse into group children to set their descendants' positions
+		if let Some(grandchild_extents) = group_extents_map.get(child_layer) {
+			set_import_child_positions(network_interface, *child_layer, child_pos, grandchild_extents, group_extents_map);
+		}
+
+		// Advance `current_y` for the next child: node height (STACK_VERTICAL_GAP) + gap (STACK_VERTICAL_GAP) + subtree extent
+		let child_svg_index = n - 1 - i;
+		let child_extent = child_extents_svg_order[child_svg_index];
+		current_y += 2 * STACK_VERTICAL_GAP + child_extent as i32;
 	}
 }
 

editor/src/messages/portfolio/document/graph_operation/utility_types.rs

@@ -33,6 +33,8 @@ pub struct ModifyInputsContext<'a> {
 	pub responses: &'a mut VecDeque<Message>,
 	// Cannot be LayerNodeIdentifier::ROOT_PARENT
 	pub layer_node: Option<LayerNodeIdentifier>,
+	/// When true, uses lightweight import paths that skip expensive checks during bulk import.
+	pub import: bool,
 }
 
 impl<'a> ModifyInputsContext<'a> {
@@ -42,6 +44,7 @@ impl<'a> ModifyInputsContext<'a> {
 			network_interface,
 			responses,
 			layer_node: None,
+			import: false,
 		}
 	}
 
@@ -150,7 +153,7 @@ impl<'a> ModifyInputsContext<'a> {
 
 		let boolean_id = NodeId::new();
 		self.network_interface.insert_node(boolean_id, boolean, &[]);
-		self.network_interface.move_node_to_chain_start(&boolean_id, layer, &[]);
+		self.network_interface.move_node_to_chain_start(&boolean_id, layer, &[], self.import);
 	}
 
 	pub fn insert_vector(&mut self, subpaths: Vec<Subpath<PointId>>, layer: LayerNodeIdentifier, include_transform: bool, include_fill: bool, include_stroke: bool) {
@@ -161,13 +164,13 @@ impl<'a> ModifyInputsContext<'a> {
 			.node_template_input_override([Some(NodeInput::value(TaggedValue::Vector(vector), false))]);
 		let shape_id = NodeId::new();
 		self.network_interface.insert_node(shape_id, shape, &[]);
-		self.network_interface.move_node_to_chain_start(&shape_id, layer, &[]);
+		self.network_interface.move_node_to_chain_start(&shape_id, layer, &[], self.import);
 
 		if include_transform {
 			let transform = resolve_network_node_type("Transform").expect("Transform node does not exist").default_node_template();
 			let transform_id = NodeId::new();
 			self.network_interface.insert_node(transform_id, transform, &[]);
-			self.network_interface.move_node_to_chain_start(&transform_id, layer, &[]);
+			self.network_interface.move_node_to_chain_start(&transform_id, layer, &[], self.import);
 		}
 
 		if include_stroke {
@@ -176,7 +179,7 @@ impl<'a> ModifyInputsContext<'a> {
 				.default_node_template();
 			let stroke_id = NodeId::new();
 			self.network_interface.insert_node(stroke_id, stroke, &[]);
-			self.network_interface.move_node_to_chain_start(&stroke_id, layer, &[]);
+			self.network_interface.move_node_to_chain_start(&stroke_id, layer, &[], self.import);
 		}
 
 		if include_fill {
@@ -185,7 +188,7 @@ impl<'a> ModifyInputsContext<'a> {
 				.default_node_template();
 			let fill_id = NodeId::new();
 			self.network_interface.insert_node(fill_id, fill, &[]);
-			self.network_interface.move_node_to_chain_start(&fill_id, layer, &[]);
+			self.network_interface.move_node_to_chain_start(&fill_id, layer, &[], self.import);
 		}
 	}
 
@@ -216,19 +219,19 @@ impl<'a> ModifyInputsContext<'a> {
 
 		let text_id = NodeId::new();
 		self.network_interface.insert_node(text_id, text, &[]);
-		self.network_interface.move_node_to_chain_start(&text_id, layer, &[]);
+		self.network_interface.move_node_to_chain_start(&text_id, layer, &[], self.import);
 
 		let transform_id = NodeId::new();
 		self.network_interface.insert_node(transform_id, transform, &[]);
-		self.network_interface.move_node_to_chain_start(&transform_id, layer, &[]);
+		self.network_interface.move_node_to_chain_start(&transform_id, layer, &[], self.import);
 
 		let stroke_id = NodeId::new();
 		self.network_interface.insert_node(stroke_id, stroke, &[]);
-		self.network_interface.move_node_to_chain_start(&stroke_id, layer, &[]);
+		self.network_interface.move_node_to_chain_start(&stroke_id, layer, &[], self.import);
 
 		let fill_id = NodeId::new();
 		self.network_interface.insert_node(fill_id, fill, &[]);
-		self.network_interface.move_node_to_chain_start(&fill_id, layer, &[]);
+		self.network_interface.move_node_to_chain_start(&fill_id, layer, &[], self.import);
 	}
 
 	pub fn insert_image_data(&mut self, image_frame: Table<Raster<CPU>>, layer: LayerNodeIdentifier) {
@@ -239,11 +242,11 @@ impl<'a> ModifyInputsContext<'a> {
 
 		let image_id = NodeId::new();
 		self.network_interface.insert_node(image_id, image, &[]);
-		self.network_interface.move_node_to_chain_start(&image_id, layer, &[]);
+		self.network_interface.move_node_to_chain_start(&image_id, layer, &[], self.import);
 
 		let transform_id = NodeId::new();
 		self.network_interface.insert_node(transform_id, transform, &[]);
-		self.network_interface.move_node_to_chain_start(&transform_id, layer, &[]);
+		self.network_interface.move_node_to_chain_start(&transform_id, layer, &[], self.import);
 	}
 
 	fn get_output_layer(&self) -> Option<LayerNodeIdentifier> {
@@ -328,12 +331,12 @@ impl<'a> ModifyInputsContext<'a> {
 				};
 				let node_id = NodeId::new();
 				self.network_interface.insert_node(node_id, flatten_path_definition.default_node_template(), &[]);
-				self.network_interface.move_node_to_chain_start(&node_id, output_layer, &[]);
+				self.network_interface.move_node_to_chain_start(&node_id, output_layer, &[], self.import);
 			}
 		}
 		let node_id = NodeId::new();
 		self.network_interface.insert_node(node_id, node_definition.default_node_template(), &[]);
-		self.network_interface.move_node_to_chain_start(&node_id, output_layer, &[]);
+		self.network_interface.move_node_to_chain_start(&node_id, output_layer, &[], self.import);
 		Some(node_id)
 	}
 

editor/src/messages/portfolio/document/node_graph/node_graph_message_handler.rs

@@ -717,7 +717,7 @@ impl<'a> MessageHandler<NodeGraphMessage, NodeGraphMessageContext<'a>> for NodeG
 				network_interface.move_layer_to_stack(layer, parent, insert_index, selection_network_path);
 			}
 			NodeGraphMessage::MoveNodeToChainStart { node_id, parent } => {
-				network_interface.move_node_to_chain_start(&node_id, parent, selection_network_path);
+				network_interface.move_node_to_chain_start(&node_id, parent, selection_network_path, false);
 			}
 			NodeGraphMessage::SetChainPosition { node_id } => {
 				network_interface.set_chain_position(&node_id, selection_network_path);