Collect all unique clipPaths, masks and filters during parsing.

CHANGELOG.md

@@ -17,6 +17,9 @@ This changelog also contains important changes in dependencies.
 - All `usvg::Tree` parsing methods require the `fontdb` argument now.
 - All `defs` children like gradients, patterns, clipPaths, masks and filters are guarantee
   to have a unique, non-empty ID.
+- `usvg::Tree::clip_paths`, `usvg::Tree::masks`, `usvg::Tree::filters` returns
+  a pre-collected slice of unique nodes now.
+  It's no longer a closure and you do not have to deduplicate nodes by yourself.
 
 ### Removed
 - `usvg::Tree::postprocess()` and `usvg::PostProcessingSteps`. No longer needed.

crates/usvg/src/parser/converter.rs

@@ -241,6 +241,9 @@ pub(crate) fn convert_doc(
         size,
         view_box,
         root: Group::empty(),
+        clip_paths: Vec::new(),
+        masks: Vec::new(),
+        filters: Vec::new(),
     };
 
     if !svg.is_visible_element(opt) {
@@ -270,14 +273,17 @@ pub(crate) fn convert_doc(
 
     convert_children(svg_doc.root(), &state, &mut cache, &mut tree.root);
 
-    tree.calculate_abs_transforms();
+    tree.root.collect_clip_paths(&mut tree.clip_paths);
+    tree.root.collect_masks(&mut tree.masks);
+    tree.root.collect_filters(&mut tree.filters);
+    tree.root.calculate_abs_transforms(Transform::identity());
 
     #[cfg(feature = "text")]
     {
         crate::text_to_paths::convert_text(&mut tree.root, fontdb, &mut cache);
     }
 
-    tree.calculate_bounding_boxes();
+    tree.root.calculate_bounding_boxes();
 
     if restore_viewbox {
         calculate_svg_bbox(&mut tree);

crates/usvg/src/parser/image.rs

@@ -263,8 +263,8 @@ pub(crate) fn load_sub_svg(data: &[u8], opt: &Options) -> Option<ImageKind> {
             return None;
         }
     };
-    tree.calculate_abs_transforms();
-    tree.calculate_bounding_boxes();
+    tree.root.calculate_abs_transforms(Transform::identity());
+    tree.root.calculate_bounding_boxes();
 
     Some(ImageKind::SVG(tree))
 }

crates/usvg/src/tree/mod.rs

@@ -1604,6 +1604,9 @@ pub struct Tree {
     pub(crate) size: Size,
     pub(crate) view_box: ViewBox,
     pub(crate) root: Group,
+    pub(crate) clip_paths: Vec<SharedClipPath>,
+    pub(crate) masks: Vec<SharedMask>,
+    pub(crate) filters: Vec<filter::SharedFilter>,
 }
 
 impl Tree {
@@ -1653,35 +1656,19 @@ impl Tree {
         loop_over_paint_servers(&self.root, &mut f)
     }
 
-    /// Calls a closure for each [`ClipPath`] in the tree.
-    ///
-    /// Doesn't guarantee to have unique clip paths. A caller must deduplicate them manually.
-    pub fn clip_paths<F: FnMut(SharedClipPath)>(&self, mut f: F) {
-        loop_over_clip_paths(&self.root, &mut f)
+    /// Returns a list of all unique [`ClipPath`]s in the tree.
+    pub fn clip_paths(&self) -> &[SharedClipPath] {
+        &self.clip_paths
     }
 
-    /// Calls a closure for each [`Mask`] in the tree.
-    ///
-    /// Doesn't guarantee to have unique masks. A caller must deduplicate them manually.
-    pub fn masks<F: FnMut(SharedMask)>(&self, mut f: F) {
-        loop_over_masks(&self.root, &mut f)
+    /// Returns a list of all unique [`Mask`]s in the tree.
+    pub fn masks(&self) -> &[SharedMask] {
+        &self.masks
     }
 
-    /// Calls a closure for each [`Filter`](filter::Filter) in the tree.
-    ///
-    /// Doesn't guarantee to have unique filters. A caller must deduplicate them manually.
-    pub fn filters<F: FnMut(filter::SharedFilter)>(&self, mut f: F) {
-        loop_over_filters(&self.root, &mut f)
-    }
-
-    /// Calculates absolute transforms for all nodes in the tree.
-    pub(crate) fn calculate_abs_transforms(&mut self) {
-        self.root.calculate_abs_transforms(Transform::identity());
-    }
-
-    /// Calculates bounding boxes for all nodes in the tree.
-    pub(crate) fn calculate_bounding_boxes(&mut self) {
-        self.root.calculate_bounding_boxes();
+    /// Returns a list of all unique [`Filter`](filter::Filter)s in the tree.
+    pub fn filters(&self) -> &[filter::SharedFilter] {
+        &self.filters
     }
 }
 
@@ -1774,65 +1761,73 @@ fn loop_over_paint_servers(parent: &Group, f: &mut dyn FnMut(&Paint)) {
     }
 }
 
-fn loop_over_clip_paths(parent: &Group, f: &mut dyn FnMut(SharedClipPath)) {
-    for node in &parent.children {
-        if let Node::Group(ref g) = node {
-            if let Some(ref clip) = g.clip_path {
-                f(clip.clone());
+impl Group {
+    pub(crate) fn collect_clip_paths(&self, clip_paths: &mut Vec<SharedClipPath>) {
+        for node in self.children() {
+            if let Node::Group(ref g) = node {
+                if let Some(ref clip) = g.clip_path {
+                    if !clip_paths.iter().any(|other| Rc::ptr_eq(&clip, other)) {
+                        clip_paths.push(clip.clone());
+                    }
 
-                if let Some(ref sub_clip) = clip.borrow().clip_path {
-                    f(sub_clip.clone());
+                    if let Some(ref sub_clip) = clip.borrow().clip_path {
+                        if !clip_paths.iter().any(|other| Rc::ptr_eq(&sub_clip, other)) {
+                            clip_paths.push(sub_clip.clone());
+                        }
+                    }
                 }
             }
-        }
 
-        node.subroots(|subroot| loop_over_clip_paths(subroot, f));
+            node.subroots(|subroot| subroot.collect_clip_paths(clip_paths));
 
-        if let Node::Group(ref g) = node {
-            loop_over_clip_paths(g, f);
+            if let Node::Group(ref g) = node {
+                g.collect_clip_paths(clip_paths);
+            }
         }
     }
-}
 
-fn loop_over_masks(parent: &Group, f: &mut dyn FnMut(SharedMask)) {
-    for node in &parent.children {
-        if let Node::Group(ref g) = node {
-            if let Some(ref mask) = g.mask {
-                f(mask.clone());
+    pub(crate) fn collect_masks(&self, masks: &mut Vec<SharedMask>) {
+        for node in self.children() {
+            if let Node::Group(ref g) = node {
+                if let Some(ref mask) = g.mask {
+                    if !masks.iter().any(|other| Rc::ptr_eq(&mask, other)) {
+                        masks.push(mask.clone());
+                    }
 
-                if let Some(ref sub_mask) = mask.borrow().mask {
-                    f(sub_mask.clone());
+                    if let Some(ref sub_mask) = mask.borrow().mask {
+                        if !masks.iter().any(|other| Rc::ptr_eq(&sub_mask, other)) {
+                            masks.push(sub_mask.clone());
+                        }
+                    }
                 }
             }
 
-            loop_over_masks(g, f);
-        }
-
-        node.subroots(|subroot| loop_over_masks(subroot, f));
+            node.subroots(|subroot| subroot.collect_masks(masks));
 
-        if let Node::Group(ref g) = node {
-            loop_over_masks(g, f);
+            if let Node::Group(ref g) = node {
+                g.collect_masks(masks);
+            }
         }
     }
-}
 
-fn loop_over_filters(parent: &Group, f: &mut dyn FnMut(filter::SharedFilter)) {
-    for node in &parent.children {
-        if let Node::Group(ref g) = node {
-            for filter in &g.filters {
-                f(filter.clone());
+    pub(crate) fn collect_filters(&self, filters: &mut Vec<filter::SharedFilter>) {
+        for node in self.children() {
+            if let Node::Group(ref g) = node {
+                for filter in g.filters() {
+                    if !filters.iter().any(|other| Rc::ptr_eq(&filter, other)) {
+                        filters.push(filter.clone());
+                    }
+                }
             }
-        }
 
-        node.subroots(|subroot| loop_over_filters(subroot, f));
+            node.subroots(|subroot| subroot.collect_filters(filters));
 
-        if let Node::Group(ref g) = node {
-            loop_over_filters(g, f);
+            if let Node::Group(ref g) = node {
+                g.collect_filters(filters);
+            }
         }
     }
-}
 
-impl Group {
     /// Calculates absolute transforms for all children of this group.
     pub(crate) fn calculate_abs_transforms(&mut self, transform: Transform) {
         for node in &mut self.children {

crates/usvg/src/writer.rs

@@ -4,7 +4,6 @@
 
 use std::fmt::Display;
 use std::io::Write;
-use std::rc::Rc;
 
 use svgtypes::{parse_font_families, FontFamily};
 use xmlwriter::XmlWriter;
@@ -96,15 +95,8 @@ pub(crate) fn convert(tree: &Tree, opt: &XmlOptions) -> String {
 }
 
 fn write_filters(tree: &Tree, opt: &XmlOptions, xml: &mut XmlWriter) {
-    let mut filters = Vec::new();
-    tree.filters(|filter| {
-        if !filters.iter().any(|other| Rc::ptr_eq(&filter, other)) {
-            filters.push(filter);
-        }
-    });
-
     let mut written_fe_image_nodes: Vec<String> = Vec::new();
-    for filter in filters {
+    for filter in tree.filters() {
         let filter = filter.borrow();
         for fe in &filter.primitives {
             if let filter::Kind::Image(ref img) = fe.kind {
@@ -536,13 +528,7 @@ fn write_defs(tree: &Tree, opt: &XmlOptions, xml: &mut XmlWriter) {
 
     write_filters(tree, opt, xml);
 
-    let mut clip_paths = Vec::new();
-    tree.clip_paths(|clip| {
-        if !clip_paths.iter().any(|other| Rc::ptr_eq(&clip, other)) {
-            clip_paths.push(clip);
-        }
-    });
-    for clip in clip_paths {
+    for clip in tree.clip_paths() {
         let clip = clip.borrow();
         xml.start_svg_element(EId::ClipPath);
         xml.write_id_attribute(clip.id(), opt);
@@ -558,13 +544,7 @@ fn write_defs(tree: &Tree, opt: &XmlOptions, xml: &mut XmlWriter) {
         xml.end_element();
     }
 
-    let mut masks = Vec::new();
-    tree.masks(|mask| {
-        if !masks.iter().any(|other| Rc::ptr_eq(&mask, other)) {
-            masks.push(mask);
-        }
-    });
-    for mask in masks {
+    for mask in tree.masks() {
         let mask = mask.borrow();
         xml.start_svg_element(EId::Mask);
         xml.write_id_attribute(mask.id(), opt);