From 23121f8c13091a63f2ca36505904f813a418038c Mon Sep 17 00:00:00 2001 From: theaniketgiri Date: Fri, 26 Dec 2025 01:07:40 +0530 Subject: [PATCH] feat: Set exported SVG tag ID to its source layer name Implements #2879: When exporting artwork as SVG, group elements ( tags) now include an 'id' attribute set to the layer's display name. Changes: - Add node_names HashMap to RenderParams and RenderConfig to pass layer names from editor to rendering system - Add sanitize_svg_id() to convert layer names to valid SVG IDs: - Replaces spaces and colons with underscores - Strips invalid XML Name characters - Ensures ID starts with letter or underscore - Add make_unique_svg_id() to handle duplicate names with _2, _3 suffixes - Modify Table::render_svg() to add id attribute during export - Collect layer names recursively including nested layers/groups - Skip default 'Untitled Layer' and 'Untitled Node' names - Add comprehensive unit tests for ID sanitization and deduplication Example: A layer named 'my:square' exports as Duplicate names get suffixes: 'Layer', 'Layer_2', 'Layer_3' Closes #2879 --- editor/src/node_graph_executor.rs | 23 +++ editor/src/node_graph_executor/runtime.rs | 7 +- .../libraries/application-io/src/lib.rs | 6 +- .../libraries/rendering/src/renderer.rs | 177 +++++++++++++++++- node-graph/nodes/gstd/src/render_node.rs | 1 + 5 files changed, 209 insertions(+), 5 deletions(-) diff --git a/editor/src/node_graph_executor.rs b/editor/src/node_graph_executor.rs index fb9b2eb37d..8a3acb9ce4 100644 --- a/editor/src/node_graph_executor.rs +++ b/editor/src/node_graph_executor.rs @@ -154,6 +154,7 @@ impl NodeGraphExecutor { render_mode: document.render_mode, hide_artboards: false, for_export: false, + node_names: std::collections::HashMap::new(), }; // Execute the node graph @@ -200,6 +201,27 @@ impl NodeGraphExecutor { let resolution = (bounds[1] - bounds[0]).as_uvec2(); let transform = DAffine2::from_translation(bounds[0]).inverse(); + // Build node_names mapping for SVG export (layer NodeId -> display name) + // This includes all nodes recursively (nested layers/groups) + let node_names = if export_format == graphene_std::application_io::ExportFormat::Svg { + document + .network_interface + .document_network() + .recursive_nodes() + .filter_map(|(node_id, _, network_path)| { + let name = document.network_interface.display_name(node_id, &network_path); + // Skip default "Untitled Layer" and "Untitled Node" names + if name.starts_with("Untitled") { + None + } else { + Some((*node_id, name)) + } + }) + .collect() + } else { + std::collections::HashMap::new() + }; + let render_config = RenderConfig { viewport: Footprint { resolution, @@ -212,6 +234,7 @@ impl NodeGraphExecutor { render_mode: document.render_mode, hide_artboards: export_config.transparent_background, for_export: true, + node_names, }; export_config.size = resolution.as_dvec2(); diff --git a/editor/src/node_graph_executor/runtime.rs b/editor/src/node_graph_executor/runtime.rs index 66022cc047..2cf79cdf0b 100644 --- a/editor/src/node_graph_executor/runtime.rs +++ b/editor/src/node_graph_executor/runtime.rs @@ -231,6 +231,9 @@ impl NodeRuntime { } } + // Clone for_export flag before moving render_config + let for_export = render_config.for_export; + let result = self.execute_network(render_config).await; let mut responses = VecDeque::new(); // TODO: Only process monitor nodes if the graph has changed, not when only the Footprint changes @@ -244,7 +247,7 @@ impl NodeRuntime { Ok(TaggedValue::RenderOutput(RenderOutput { data: RenderOutputType::Texture(image_texture), metadata, - })) if render_config.for_export => { + })) if for_export => { let executor = self .editor_api .application_io @@ -269,7 +272,7 @@ impl NodeRuntime { Ok(TaggedValue::RenderOutput(RenderOutput { data: RenderOutputType::Texture(image_texture), metadata, - })) if !render_config.for_export => { + })) if !for_export => { // On WASM, for viewport rendering, blit the texture to a surface and return a CanvasFrame let app_io = self.editor_api.application_io.as_ref().unwrap(); let executor = app_io.gpu_executor().expect("GPU executor should be available when we receive a texture"); diff --git a/node-graph/libraries/application-io/src/lib.rs b/node-graph/libraries/application-io/src/lib.rs index c023204e1e..9868187fb7 100644 --- a/node-graph/libraries/application-io/src/lib.rs +++ b/node-graph/libraries/application-io/src/lib.rs @@ -233,7 +233,7 @@ pub struct TimingInformation { pub animation_time: Duration, } -#[derive(Debug, Default, Clone, Copy, PartialEq, DynAny, serde::Serialize, serde::Deserialize)] +#[derive(Debug, Default, Clone, PartialEq, DynAny, serde::Serialize, serde::Deserialize)] pub struct RenderConfig { pub viewport: Footprint, pub scale: f64, @@ -243,6 +243,10 @@ pub struct RenderConfig { pub render_mode: RenderMode, pub hide_artboards: bool, pub for_export: bool, + /// Mapping from NodeId to layer display names for SVG export. + /// When `for_export` is true, these names are used to set `id` attributes on SVG elements. + #[serde(default)] + pub node_names: std::collections::HashMap, } struct Logger; diff --git a/node-graph/libraries/rendering/src/renderer.rs b/node-graph/libraries/rendering/src/renderer.rs index 79bf087423..38d531bc84 100644 --- a/node-graph/libraries/rendering/src/renderer.rs +++ b/node-graph/libraries/rendering/src/renderer.rs @@ -182,6 +182,9 @@ pub struct RenderParams { pub alignment_parent_transform: Option, pub aligned_strokes: bool, pub override_paint_order: bool, + /// Mapping from NodeId to layer display names, used during SVG export to set element IDs. + /// This field is excluded from the Hash implementation since it doesn't affect render output. + pub node_names: HashMap, } impl Hash for RenderParams { @@ -203,12 +206,20 @@ impl Hash for RenderParams { impl RenderParams { pub fn for_clipper(&self) -> Self { - Self { for_mask: true, ..*self } + Self { + for_mask: true, + node_names: self.node_names.clone(), + ..*self + } } pub fn for_alignment(&self, transform: DAffine2) -> Self { let alignment_parent_transform = Some(transform); - Self { alignment_parent_transform, ..*self } + Self { + alignment_parent_transform, + node_names: self.node_names.clone(), + ..*self + } } pub fn to_canvas(&self) -> bool { @@ -229,6 +240,64 @@ pub fn format_transform_matrix(transform: DAffine2) -> String { }) + ")" } +/// Sanitizes a layer name to be a valid SVG ID. +/// +/// SVG ID values must be valid XML Name tokens. This function: +/// - Replaces spaces with underscores +/// - Removes characters that are invalid in XML Names +/// - Ensures the ID starts with a letter or underscore +/// - Returns None if the result would be empty +fn sanitize_svg_id(name: &str) -> Option { + if name.is_empty() { + return None; + } + + let mut result = String::with_capacity(name.len()); + + for c in name.chars() { + let is_start = result.is_empty(); + + match c { + // Spaces become underscores (valid anywhere after we have a valid start) + ' ' if !is_start => result.push('_'), + // Colons replaced with underscores for CSS compatibility + ':' if !is_start => result.push('_'), + // Valid start characters: letters and underscore + 'A'..='Z' | 'a'..='z' | '_' => result.push(c), + // Valid continuation characters only: digits, hyphen, period + '0'..='9' | '-' | '.' if !is_start => result.push(c), + // Skip all other characters (including spaces/colons before valid start) + _ => {} + } + } + + if result.is_empty() { + None + } else { + Some(result) + } +} + +/// Generates a unique SVG ID by appending a numeric suffix if the ID already exists. +/// +/// For example, if "Layer" already exists, this returns "Layer_2", then "Layer_3", etc. +fn make_unique_svg_id(base_id: &str, used_ids: &mut HashSet) -> String { + if !used_ids.contains(base_id) { + used_ids.insert(base_id.to_string()); + return base_id.to_string(); + } + + let mut counter = 2; + loop { + let candidate = format!("{}_{}", base_id, counter); + if !used_ids.contains(&candidate) { + used_ids.insert(candidate.clone()); + return candidate; + } + counter += 1; + } +} + fn max_scale(transform: DAffine2) -> f64 { let sx = transform.x_axis.length_squared(); let sy = transform.y_axis.length_squared(); @@ -526,11 +595,28 @@ impl Render for Table { fn render_svg(&self, render: &mut SvgRender, render_params: &RenderParams) { let mut iter = self.iter().peekable(); let mut mask_state = None; + // Track used IDs to ensure uniqueness when exporting with layer names + let mut used_ids = HashSet::new(); while let Some(row) = iter.next() { + // Look up layer name for export ID if available + let layer_id = if render_params.for_export { + row.source_node_id + .and_then(|node_id| render_params.node_names.get(&node_id)) + .and_then(|name| sanitize_svg_id(name)) + .map(|base_id| make_unique_svg_id(&base_id, &mut used_ids)) + } else { + None + }; + render.parent_tag( "g", |attributes| { + // Add layer name as ID attribute for export + if let Some(ref id) = layer_id { + attributes.push("id", id.clone()); + } + let matrix = format_transform_matrix(*row.transform); if !matrix.is_empty() { attributes.push("transform", matrix); @@ -1651,3 +1737,90 @@ impl SvgRenderAttrs<'_> { self.0.svg.push(value.into()); } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_sanitize_svg_id_basic() { + assert_eq!(sanitize_svg_id("Hello"), Some("Hello".to_string())); + assert_eq!(sanitize_svg_id("Layer1"), Some("Layer1".to_string())); + assert_eq!(sanitize_svg_id("my_layer"), Some("my_layer".to_string())); + } + + #[test] + fn test_sanitize_svg_id_spaces() { + assert_eq!(sanitize_svg_id("My Layer"), Some("My_Layer".to_string())); + // Leading spaces are skipped until we find a valid start character + assert_eq!(sanitize_svg_id(" spaces "), Some("spaces__".to_string())); + } + + #[test] + fn test_sanitize_svg_id_colons() { + // Colons are replaced with underscores for CSS compatibility + assert_eq!(sanitize_svg_id("my:layer"), Some("my_layer".to_string())); + assert_eq!(sanitize_svg_id("a:b:c"), Some("a_b_c".to_string())); + } + + #[test] + fn test_sanitize_svg_id_invalid_start() { + // IDs can't start with digits + assert_eq!(sanitize_svg_id("123layer"), Some("layer".to_string())); + // IDs can start with underscore + assert_eq!(sanitize_svg_id("_layer"), Some("_layer".to_string())); + } + + #[test] + fn test_sanitize_svg_id_special_chars() { + // Most special characters are stripped + assert_eq!(sanitize_svg_id("hello@world!"), Some("helloworld".to_string())); + assert_eq!(sanitize_svg_id("test#123"), Some("test123".to_string())); + } + + #[test] + fn test_sanitize_svg_id_empty() { + assert_eq!(sanitize_svg_id(""), None); + assert_eq!(sanitize_svg_id("@#$%"), None); + assert_eq!(sanitize_svg_id("123"), None); // All digits, no valid start + } + + #[test] + fn test_sanitize_svg_id_hyphens_dots() { + // Hyphens and dots are valid after the first character + assert_eq!(sanitize_svg_id("layer-1"), Some("layer-1".to_string())); + assert_eq!(sanitize_svg_id("layer.name"), Some("layer.name".to_string())); + // But not at the start + assert_eq!(sanitize_svg_id("-layer"), Some("layer".to_string())); + assert_eq!(sanitize_svg_id(".layer"), Some("layer".to_string())); + } + + #[test] + fn test_make_unique_svg_id_no_conflict() { + let mut used_ids = HashSet::new(); + assert_eq!(make_unique_svg_id("layer", &mut used_ids), "layer"); + assert!(used_ids.contains("layer")); + } + + #[test] + fn test_make_unique_svg_id_with_conflicts() { + let mut used_ids = HashSet::new(); + used_ids.insert("layer".to_string()); + + assert_eq!(make_unique_svg_id("layer", &mut used_ids), "layer_2"); + assert!(used_ids.contains("layer_2")); + + assert_eq!(make_unique_svg_id("layer", &mut used_ids), "layer_3"); + assert!(used_ids.contains("layer_3")); + } + + #[test] + fn test_make_unique_svg_id_multiple_names() { + let mut used_ids = HashSet::new(); + + assert_eq!(make_unique_svg_id("circle", &mut used_ids), "circle"); + assert_eq!(make_unique_svg_id("square", &mut used_ids), "square"); + assert_eq!(make_unique_svg_id("circle", &mut used_ids), "circle_2"); + assert_eq!(make_unique_svg_id("square", &mut used_ids), "square_2"); + } +} diff --git a/node-graph/nodes/gstd/src/render_node.rs b/node-graph/nodes/gstd/src/render_node.rs index 462ee85985..af07cf9e28 100644 --- a/node-graph/nodes/gstd/src/render_node.rs +++ b/node-graph/nodes/gstd/src/render_node.rs @@ -106,6 +106,7 @@ async fn create_context<'a: 'n>( render_output_type, footprint: Footprint::default(), scale: render_config.scale, + node_names: render_config.node_names, ..Default::default() };