From b791068f81becdc87e5db2366401445d9a244893 Mon Sep 17 00:00:00 2001 From: n4n5 Date: Sat, 13 Dec 2025 14:01:53 -0700 Subject: [PATCH 1/3] 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 7514a1c265aba50e0d576669494686699e913783 Mon Sep 17 00:00:00 2001 From: n4n5 Date: Sun, 14 Dec 2025 11:56:55 -0700 Subject: [PATCH 2/3] Add display for Units (remove TODO) --- crates/usvg/src/tree/mod.rs | 9 +++++++++ crates/usvg/src/writer.rs | 9 +-------- 2 files changed, 10 insertions(+), 8 deletions(-) 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..4eebfbbbd 100644 --- a/crates/usvg/src/writer.rs +++ b/crates/usvg/src/writer.rs @@ -992,16 +992,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); } } From 93bfd5c12ba751cf9cf9633016d2bbfb4ffa7031 Mon Sep 17 00:00:00 2001 From: n4n5 Date: Sun, 14 Dec 2025 11:58:52 -0700 Subject: [PATCH 3/3] revert --- 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, 2 insertions(+), 19 deletions(-) delete mode 100644 crates/usvg/tests/files/path-simple-case-empty-group-expected.svg delete 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 4eebfbbbd..231f84467 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::{Options, XmlWriter}; +use xmlwriter::XmlWriter; use crate::parser::{AId, EId}; use crate::*; @@ -707,12 +707,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 { 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 deleted file mode 100644 index 3180288a7..000000000 --- a/crates/usvg/tests/files/path-simple-case-empty-group-expected.svg +++ /dev/null @@ -1,3 +0,0 @@ - - - diff --git a/crates/usvg/tests/files/path-simple-case-empty-group.svg b/crates/usvg/tests/files/path-simple-case-empty-group.svg deleted file mode 100644 index 4ba910e6e..000000000 --- a/crates/usvg/tests/files/path-simple-case-empty-group.svg +++ /dev/null @@ -1,4 +0,0 @@ - - - - diff --git a/crates/usvg/tests/write.rs b/crates/usvg/tests/write.rs index eb44322ba..14edfbb92 100644 --- a/crates/usvg/tests/write.rs +++ b/crates/usvg/tests/write.rs @@ -67,11 +67,6 @@ 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");