From b791068f81becdc87e5db2366401445d9a244893 Mon Sep 17 00:00:00 2001 From: n4n5 Date: Sat, 13 Dec 2025 14:01:53 -0700 Subject: [PATCH 1/5] poc: don't write empty nodes --- crates/usvg/src/writer.rs | 9 +++++++-- .../files/path-simple-case-empty-group-expected.svg | 3 +++ crates/usvg/tests/files/path-simple-case-empty-group.svg | 4 ++++ crates/usvg/tests/write.rs | 5 +++++ 4 files changed, 19 insertions(+), 2 deletions(-) create mode 100644 crates/usvg/tests/files/path-simple-case-empty-group-expected.svg create mode 100644 crates/usvg/tests/files/path-simple-case-empty-group.svg diff --git a/crates/usvg/src/writer.rs b/crates/usvg/src/writer.rs index 96444043d..ea5c7d7d6 100644 --- a/crates/usvg/src/writer.rs +++ b/crates/usvg/src/writer.rs @@ -5,7 +5,7 @@ use std::fmt::Display; use std::io::Write; use svgtypes::{parse_font_families, FontFamily}; -use xmlwriter::XmlWriter; +use xmlwriter::{Options, XmlWriter}; use crate::parser::{AId, EId}; use crate::*; @@ -707,7 +707,12 @@ fn write_element(node: &Node, is_clip_path: bool, opt: &WriteOptions, xml: &mut xml.end_element(); } Node::Group(ref g) => { - write_group_element(g, is_clip_path, opt, xml); + let mut fake_xml = XmlWriter::new(Options::default()); + write_group_element(g, is_clip_path, opt, &mut fake_xml); + let fake_xml_str = fake_xml.end_document(); + if fake_xml_str != "\n" { + write_group_element(g, is_clip_path, opt, xml); + } } Node::Text(ref text) => { if opt.preserve_text { diff --git a/crates/usvg/tests/files/path-simple-case-empty-group-expected.svg b/crates/usvg/tests/files/path-simple-case-empty-group-expected.svg new file mode 100644 index 000000000..3180288a7 --- /dev/null +++ b/crates/usvg/tests/files/path-simple-case-empty-group-expected.svg @@ -0,0 +1,3 @@ + + + diff --git a/crates/usvg/tests/files/path-simple-case-empty-group.svg b/crates/usvg/tests/files/path-simple-case-empty-group.svg new file mode 100644 index 000000000..4ba910e6e --- /dev/null +++ b/crates/usvg/tests/files/path-simple-case-empty-group.svg @@ -0,0 +1,4 @@ + + + + diff --git a/crates/usvg/tests/write.rs b/crates/usvg/tests/write.rs index 14edfbb92..eb44322ba 100644 --- a/crates/usvg/tests/write.rs +++ b/crates/usvg/tests/write.rs @@ -67,6 +67,11 @@ fn path_simple_case() { resave("path-simple-case"); } +#[test] +fn path_simple_case_empty_group() { + resave("path-simple-case-empty-group"); +} + #[test] fn ellipse_simple_case() { resave("ellipse-simple-case"); From 4dd570287cbff5e7145f49361d6dee0b82aa6f0c Mon Sep 17 00:00:00 2001 From: n4n5 Date: Sun, 14 Dec 2025 11:52:43 -0700 Subject: [PATCH 2/5] feat: change to LazyWriter --- crates/usvg/src/tree/mod.rs | 9 ++ crates/usvg/src/writer.rs | 112 +++++++++++++----- .../path-simple-case-empty-path-expected.svg | 3 + .../files/path-simple-case-empty-path.svg | 4 + crates/usvg/tests/write.rs | 6 + 5 files changed, 107 insertions(+), 27 deletions(-) create mode 100644 crates/usvg/tests/files/path-simple-case-empty-path-expected.svg create mode 100644 crates/usvg/tests/files/path-simple-case-empty-path.svg diff --git a/crates/usvg/src/tree/mod.rs b/crates/usvg/src/tree/mod.rs index f2f9b6e34..70916da30 100644 --- a/crates/usvg/src/tree/mod.rs +++ b/crates/usvg/src/tree/mod.rs @@ -73,6 +73,15 @@ pub(crate) enum Units { // `Units` cannot have a default value, because it changes depending on an element. +impl std::fmt::Display for Units { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Units::UserSpaceOnUse => write!(f, "userSpaceOnUse"), + Units::ObjectBoundingBox => write!(f, "objectBoundingBox"), + } + } +} + /// A visibility property. /// /// `visibility` attribute in the SVG. diff --git a/crates/usvg/src/writer.rs b/crates/usvg/src/writer.rs index ea5c7d7d6..d2926ab62 100644 --- a/crates/usvg/src/writer.rs +++ b/crates/usvg/src/writer.rs @@ -1,11 +1,11 @@ // Copyright 2023 the Resvg Authors // SPDX-License-Identifier: Apache-2.0 OR MIT -use std::fmt::Display; +use std::fmt::{self, Display}; use std::io::Write; use svgtypes::{parse_font_families, FontFamily}; -use xmlwriter::{Options, XmlWriter}; +use xmlwriter::XmlWriter; use crate::parser::{AId, EId}; use crate::*; @@ -138,6 +138,74 @@ impl Default for WriteOptions { } } +pub struct LazyXmlWriter<'a> { + xml: &'a mut xmlwriter::XmlWriter, + current_node: Option, + force_write_node: bool, +} + +impl<'a> LazyXmlWriter<'a> { + pub fn new(xml: &'a mut xmlwriter::XmlWriter, node: EId) -> Self { + Self { + xml, + current_node: Some(node), + force_write_node: false, + } + } + + fn force_write(&mut self) -> &mut xmlwriter::XmlWriter { + self.init_node(); + self.force_write_node = true; + self.xml + } + + fn init_node(&mut self) { + if let Some(node) = self.current_node { + self.xml.start_svg_element(node); + self.current_node = None; + } + } + + pub fn write_id_attribute(&mut self, id: &str, opt: &WriteOptions) { + self.init_node(); + self.xml.write_id_attribute(id, opt); + } + + pub fn write_func_iri(&mut self, aid: AId, id: &str, opt: &WriteOptions) { + self.init_node(); + self.xml.write_func_iri(aid, id, opt); + } + + fn write_svg_attribute(&mut self, id: AId, value: &V) { + self.init_node(); + self.xml.write_svg_attribute(id, value); + } + + fn write_transform(&mut self, id: AId, ts: Transform, opt: &WriteOptions) { + if !ts.is_default() { + self.init_node(); + self.xml.write_transform(id, ts, opt); + } + } + + pub fn write_attribute_fmt(&mut self, name: &str, fmt: fmt::Arguments) { + self.init_node(); + self.xml.write_attribute_fmt(name, fmt); + } + + pub fn end_element(&mut self) { + if self.force_write_node { + // simulate that node was written + self.current_node = None; + } + if let Some(_node) = self.current_node { + // Node was never written. + return; + } + self.xml.end_element(); + } +} + pub(crate) fn convert(tree: &Tree, opt: &WriteOptions) -> String { let mut xml = XmlWriter::new(xmlwriter::Options { use_single_quote: opt.use_single_quote, @@ -707,12 +775,7 @@ fn write_element(node: &Node, is_clip_path: bool, opt: &WriteOptions, xml: &mut xml.end_element(); } Node::Group(ref g) => { - let mut fake_xml = XmlWriter::new(Options::default()); - write_group_element(g, is_clip_path, opt, &mut fake_xml); - let fake_xml_str = fake_xml.end_document(); - if fake_xml_str != "\n" { - write_group_element(g, is_clip_path, opt, xml); - } + write_group_element(g, is_clip_path, opt, xml); } Node::Text(ref text) => { if opt.preserve_text { @@ -871,17 +934,17 @@ fn write_group_element(g: &Group, is_clip_path: bool, opt: &WriteOptions, xml: & return; } - xml.start_svg_element(EId::G); + let mut lazy_writer = LazyXmlWriter::new(xml, EId::G); if !g.id.is_empty() { - xml.write_id_attribute(&g.id, opt); + lazy_writer.write_id_attribute(&g.id, opt); }; if let Some(ref clip) = g.clip_path { - xml.write_func_iri(AId::ClipPath, clip.id(), opt); + lazy_writer.write_func_iri(AId::ClipPath, clip.id(), opt); } if let Some(ref mask) = g.mask { - xml.write_func_iri(AId::Mask, mask.id(), opt); + lazy_writer.write_func_iri(AId::Mask, mask.id(), opt); } if !g.filters.is_empty() { @@ -891,14 +954,14 @@ fn write_group_element(g: &Group, is_clip_path: bool, opt: &WriteOptions, xml: & .iter() .map(|filter| format!("url(#{}{})", prefix, filter.id())) .collect(); - xml.write_svg_attribute(AId::Filter, &ids.join(" ")); + lazy_writer.write_svg_attribute(AId::Filter, &ids.join(" ")); } if g.opacity != Opacity::ONE { - xml.write_svg_attribute(AId::Opacity, &g.opacity.get()); + lazy_writer.write_svg_attribute(AId::Opacity, &g.opacity.get()); } - xml.write_transform(AId::Transform, g.transform, opt); + lazy_writer.write_transform(AId::Transform, g.transform, opt); if g.blend_mode != BlendMode::Normal || g.isolate { let blend_mode = match g.blend_mode { @@ -923,15 +986,17 @@ fn write_group_element(g: &Group, is_clip_path: bool, opt: &WriteOptions, xml: & // For reasons unknown, `mix-blend-mode` and `isolation` must be written // as `style` attribute. let isolation = if g.isolate { "isolate" } else { "auto" }; - xml.write_attribute_fmt( + lazy_writer.write_attribute_fmt( AId::Style.to_str(), format_args!("mix-blend-mode:{};isolation:{}", blend_mode, isolation), ); } - write_elements(g, false, opt, xml); - - xml.end_element(); + if !g.children.is_empty() { + let xml = lazy_writer.force_write(); + write_elements(g, false, opt, xml); + } + lazy_writer.end_element(); } trait XmlWriterExt { @@ -992,16 +1057,9 @@ impl XmlWriterExt for XmlWriter { }); } - // TODO: simplify fn write_units(&mut self, id: AId, units: Units, def: Units) { if units != def { - self.write_attribute( - id.to_str(), - match units { - Units::UserSpaceOnUse => "userSpaceOnUse", - Units::ObjectBoundingBox => "objectBoundingBox", - }, - ); + self.write_attribute(id.to_str(), &units); } } diff --git a/crates/usvg/tests/files/path-simple-case-empty-path-expected.svg b/crates/usvg/tests/files/path-simple-case-empty-path-expected.svg new file mode 100644 index 000000000..3180288a7 --- /dev/null +++ b/crates/usvg/tests/files/path-simple-case-empty-path-expected.svg @@ -0,0 +1,3 @@ + + + diff --git a/crates/usvg/tests/files/path-simple-case-empty-path.svg b/crates/usvg/tests/files/path-simple-case-empty-path.svg new file mode 100644 index 000000000..95c4d31d4 --- /dev/null +++ b/crates/usvg/tests/files/path-simple-case-empty-path.svg @@ -0,0 +1,4 @@ + + + + diff --git a/crates/usvg/tests/write.rs b/crates/usvg/tests/write.rs index eb44322ba..42364fb38 100644 --- a/crates/usvg/tests/write.rs +++ b/crates/usvg/tests/write.rs @@ -72,6 +72,12 @@ fn path_simple_case_empty_group() { resave("path-simple-case-empty-group"); } +#[test] +fn path_simple_case_empty_path() { + // this tests will succeed because the parser removes empty paths (not the writer) + resave("path-simple-case-empty-path"); +} + #[test] fn ellipse_simple_case() { resave("ellipse-simple-case"); From 074a5deba0844b3b3ce078a9ee35bc716534e7ba Mon Sep 17 00:00:00 2001 From: n4n5 Date: Sun, 14 Dec 2025 11:54:57 -0700 Subject: [PATCH 3/5] typo --- crates/usvg/tests/write.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/usvg/tests/write.rs b/crates/usvg/tests/write.rs index 42364fb38..576e7bc85 100644 --- a/crates/usvg/tests/write.rs +++ b/crates/usvg/tests/write.rs @@ -74,7 +74,7 @@ fn path_simple_case_empty_group() { #[test] fn path_simple_case_empty_path() { - // this tests will succeed because the parser removes empty paths (not the writer) + // this test will succeed because the parser removes empty paths (not the writer) resave("path-simple-case-empty-path"); } From b94f400ac19873f42c55a60583078b8e8ebe772d Mon Sep 17 00:00:00 2001 From: n4n5 Date: Sun, 14 Dec 2025 12:00:16 -0700 Subject: [PATCH 4/5] split MR to https://github.com/linebender/resvg/pull/986 --- crates/usvg/src/tree/mod.rs | 9 --------- crates/usvg/src/writer.rs | 8 +++++++- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/crates/usvg/src/tree/mod.rs b/crates/usvg/src/tree/mod.rs index 70916da30..f2f9b6e34 100644 --- a/crates/usvg/src/tree/mod.rs +++ b/crates/usvg/src/tree/mod.rs @@ -73,15 +73,6 @@ pub(crate) enum Units { // `Units` cannot have a default value, because it changes depending on an element. -impl std::fmt::Display for Units { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - Units::UserSpaceOnUse => write!(f, "userSpaceOnUse"), - Units::ObjectBoundingBox => write!(f, "objectBoundingBox"), - } - } -} - /// A visibility property. /// /// `visibility` attribute in the SVG. diff --git a/crates/usvg/src/writer.rs b/crates/usvg/src/writer.rs index d2926ab62..8a321ad94 100644 --- a/crates/usvg/src/writer.rs +++ b/crates/usvg/src/writer.rs @@ -1059,7 +1059,13 @@ impl XmlWriterExt for XmlWriter { fn write_units(&mut self, id: AId, units: Units, def: Units) { if units != def { - self.write_attribute(id.to_str(), &units); + self.write_attribute( + id.to_str(), + match units { + Units::UserSpaceOnUse => "userSpaceOnUse", + Units::ObjectBoundingBox => "objectBoundingBox", + }, + ); } } From 4c257bc41a6f57f61ad73bbd934e177a94ac7e2c Mon Sep 17 00:00:00 2001 From: n4n5 Date: Sun, 14 Dec 2025 12:01:40 -0700 Subject: [PATCH 5/5] revert fix --- crates/usvg/src/writer.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/usvg/src/writer.rs b/crates/usvg/src/writer.rs index 8a321ad94..e61ebdb16 100644 --- a/crates/usvg/src/writer.rs +++ b/crates/usvg/src/writer.rs @@ -1,7 +1,7 @@ // Copyright 2023 the Resvg Authors // SPDX-License-Identifier: Apache-2.0 OR MIT -use std::fmt::{self, Display}; +use std::fmt::{Arguments, Display}; use std::io::Write; use svgtypes::{parse_font_families, FontFamily}; @@ -188,7 +188,7 @@ impl<'a> LazyXmlWriter<'a> { } } - pub fn write_attribute_fmt(&mut self, name: &str, fmt: fmt::Arguments) { + pub fn write_attribute_fmt(&mut self, name: &str, fmt: Arguments) { self.init_node(); self.xml.write_attribute_fmt(name, fmt); } @@ -1057,6 +1057,7 @@ impl XmlWriterExt for XmlWriter { }); } + // TODO: simplify fn write_units(&mut self, id: AId, units: Units, def: Units) { if units != def { self.write_attribute(