Merge pull request #88 from noahbald/feat/16-prefix-ids

feat(oxvg_optimiser): #16 prefix ids

Cargo.toml

@@ -33,7 +33,7 @@ cssparser = "0.34.0"
 derive-where = "1.2"
 itertools = "0.14"
 lazy_static = "1.5"
-lightningcss = { version = "1.0.0-alpha.63" }
+lightningcss = { version = "1.0.0-alpha.63", features = [ "visitor" ] }
 log = "0.4"
 markup5ever = "0.14"
 phf = { version = "0.11", features = ["macros"] }

crates/oxvg/src/args.rs

@@ -4,7 +4,7 @@ use std::{
 };
 
 use clap::{Parser, Subcommand};
-use oxvg_ast::serialize::Node;
+use oxvg_ast::{serialize::Node, visitor::Info};
 
 use crate::config::Config;
 
@@ -82,14 +82,19 @@ impl RunCommand for Optimise {
         use oxvg_ast::{implementations::markup5ever::Node5Ever, parse::Node};
 
         if self.paths.len() == 1 {
-            let file = std::fs::File::open(self.paths.first().unwrap())?;
+            let path = self.paths.first().unwrap();
+            let file = std::fs::File::open(path)?;
             let dom = Node5Ever::parse_file(&file)?;
             let jobs = config.optimisation.unwrap_or_default();
 
             let start_time = SystemTime::now().duration_since(UNIX_EPOCH)?;
             let prev_file_size = file.metadata()?.len();
 
-            jobs.run(&dom)?;
+            let info = Info {
+                path: Some(path.clone()),
+                multipass_count: 0,
+            };
+            jobs.run(&dom, &info)?;
             let mut stdout = StdoutCounter::new();
             dom.serialize_into(&mut stdout)?;
 

crates/oxvg_ast/src/element.rs

@@ -416,7 +416,7 @@ pub trait Element: Node + Features + Debug + std::hash::Hash + Eq + PartialEq {
         selector: &'a str,
     ) -> Result<
         crate::selectors::Select<Self>,
-        cssparser::ParseError<'_, selectors::parser::SelectorParseErrorKind<'_>>,
+        cssparser::ParseError<'a, selectors::parser::SelectorParseErrorKind<'a>>,
     > {
         crate::selectors::Select::new(self, selector)
     }

crates/oxvg_ast/src/implementations/markup5ever.rs

@@ -350,7 +350,7 @@ impl Debug for Attributes5Ever<'_> {
     }
 }
 
-impl<'a> ClassList for ClassList5Ever<'a> {
+impl ClassList for ClassList5Ever<'_> {
     type Attribute = Attribute;
 
     fn length(&self) -> usize {
@@ -402,7 +402,7 @@ impl<'a> ClassList for ClassList5Ever<'a> {
         };
 
         let attr = self.attr().expect("had token");
-        let mut new_value = attr.value.subtendril(0, start as u32);
+        let mut new_value = attr.value.subtendril(0, start);
         new_value.push_tendril(&attr.value.subtendril(end, attr.value.len() as u32 - end));
         drop(attr);
         if new_value.trim().is_empty() {
@@ -619,6 +619,10 @@ impl Node for Node5Ever {
             .collect()
     }
 
+    fn child_node_count(&self) -> usize {
+        self.0.children.borrow().len()
+    }
+
     #[allow(refining_impl_trait)]
     fn element(&self) -> Option<Element5Ever> {
         match self.node_type() {
@@ -775,6 +779,18 @@ impl Node for Node5Ever {
         }
     }
 
+    fn set_text_content(&mut self, content: Self::Atom) {
+        match &self.0.data {
+            NodeData::Text { contents } => *contents.borrow_mut() = content,
+            NodeData::Element { .. } => {
+                let text = self.text(content);
+                self.empty();
+                self.append_child(text);
+            }
+            _ => {}
+        }
+    }
+
     fn text(&self, content: Self::Atom) -> Self {
         Node5Ever(Rc::new(rcdom::Node {
             parent: Cell::new(None),
@@ -1169,7 +1185,7 @@ impl Element for Element5Ever {
     fn get_attribute_node<'a>(
         &'a self,
         attr_name: &<<Self::Attributes<'a> as Attributes<'a>>::Attribute as Attr>::Name,
-    ) -> Option<Ref<<Self::Attributes<'a> as Attributes<'a>>::Attribute>> {
+    ) -> Option<Ref<'a, <Self::Attributes<'a> as Attributes<'a>>::Attribute>> {
         self.attributes().get_named_item(attr_name)
     }
 
@@ -1185,7 +1201,7 @@ impl Element for Element5Ever {
     fn get_attribute_node_local<'a>(
         &'a self,
         attr_name: &<<<Self::Attributes<'a> as Attributes<'a>>::Attribute as Attr>::Name as Name>::LocalName,
-    ) -> Option<Ref<<Self::Attributes<'a> as Attributes<'a>>::Attribute>> {
+    ) -> Option<Ref<'a, <Self::Attributes<'a> as Attributes<'a>>::Attribute>> {
         self.attributes().get_named_item_local(attr_name)
     }
 
@@ -1220,7 +1236,7 @@ impl Element for Element5Ever {
         &'a self,
         namespace: &<<<Self::Attributes<'a> as Attributes<'a>>::Attribute as Attr>::Name as Name>::Namespace,
         name: &<<<Self::Attributes<'a> as Attributes<'a>>::Attribute as Attr>::Name as Name>::LocalName,
-    ) -> Option<Ref<Self::Atom>> {
+    ) -> Option<Ref<'a, Self::Atom>> {
         self.get_attribute_node_ns(namespace, name)
             .map(|a| Ref::map(a, |a| &a.value))
     }
@@ -1303,6 +1319,10 @@ impl Node for Element5Ever {
         self.node.child_nodes().into_iter()
     }
 
+    fn child_node_count(&self) -> usize {
+        self.node.child_node_count()
+    }
+
     fn child_nodes(&self) -> Vec<Self::Child> {
         self.node.child_nodes()
     }
@@ -1395,6 +1415,10 @@ impl Node for Element5Ever {
         self.node.text_content()
     }
 
+    fn set_text_content(&mut self, content: Self::Atom) {
+        self.node.set_text_content(content);
+    }
+
     fn text(&self, content: Self::Atom) -> Self::Child {
         self.node.text(content)
     }
@@ -1716,11 +1740,11 @@ impl selectors::Element for Element5Ever {
     }
 
     fn is_empty(&self) -> bool {
-        self.all_children(|child| match child.node_type() {
-            node::Type::Element => false,
-            node::Type::Text => child.node_value().is_some(),
-            _ => true,
-        })
+        !self.has_child_nodes()
+            || self.all_children(|child| match &child.0.data {
+                NodeData::Text { contents } => contents.borrow().trim().is_empty(),
+                _ => false,
+            })
     }
 
     fn is_root(&self) -> bool {

crates/oxvg_ast/src/node.rs

@@ -86,9 +86,11 @@ pub trait Node: Clone + Debug + 'static + Features {
     /// [MDN | childNodes](https://developer.mozilla.org/en-US/docs/Web/API/Node/childNodes)
     fn child_nodes(&self) -> Vec<Self::Child>;
 
+    fn child_node_count(&self) -> usize;
+
     /// Returns whether the node's list of children is empty or not
     fn has_child_nodes(&self) -> bool {
-        self.any_child(|_| true)
+        self.child_node_count() > 0
     }
 
     /// Upcasts self as an element
@@ -182,11 +184,7 @@ pub trait Node: Clone + Debug + 'static + Features {
     /// [MDN | textContent](https://developer.mozilla.org/en-US/docs/Web/API/Node/textContent)
     fn text_content(&self) -> Option<String>;
 
-    fn set_text_content(&mut self, content: Self::Atom) {
-        let text = self.text(content);
-        self.empty();
-        self.append_child(text);
-    }
+    fn set_text_content(&mut self, content: Self::Atom);
 
     /// Creates a text node with the given content
     fn text(&self, content: Self::Atom) -> Self::Child;

crates/oxvg_ast/src/style.rs

@@ -585,7 +585,7 @@ impl<'i> Parse<'i> for FontVariant<'i> {
     }
 }
 
-impl<'i> ToCss for FontVariant<'i> {
+impl ToCss for FontVariant<'_> {
     fn to_css<W>(&self, dest: &mut Printer<W>) -> Result<(), PrinterError>
     where
         W: std::fmt::Write,
@@ -983,7 +983,7 @@ impl SVGTransform {
             }
         }
 
-        log::debug!(r#"converted to transform: {:?}"#, shortest);
+        log::debug!("converted to transform: {:?}", shortest);
         shortest
     }
 
@@ -1745,7 +1745,7 @@ impl<'i> ComputedStyles<'i> {
         Some((value.mode(), string))
     }
 
-    fn get_important(&'i self, id: &Id<'i>) -> Option<&Style<'i>> {
+    fn get_important(&'i self, id: &Id<'i>) -> Option<&'i Style<'i>> {
         match id {
             Id::CSS(id) => {
                 if let Some(value) = self.inline_important.get(id) {
@@ -1760,7 +1760,7 @@ impl<'i> ComputedStyles<'i> {
         }
     }
 
-    fn get_unimportant(&'i self, id: &Id<'i>) -> Option<&Style<'i>> {
+    fn get_unimportant(&'i self, id: &Id<'i>) -> Option<&'i Style<'i>> {
         match id {
             Id::CSS(id) => {
                 if let Some(value) = self.inline.get(id) {
@@ -1784,7 +1784,7 @@ impl<'i> ComputedStyles<'i> {
         self.inherited.get(id)
     }
 
-    pub fn get_static<'a>(&'i self, id: &'a Id<'a>) -> Option<&Static>
+    pub fn get_static<'a>(&'i self, id: &'a Id<'a>) -> Option<&'i Static<'i>>
     where
         'a: 'i,
     {
@@ -1794,7 +1794,7 @@ impl<'i> ComputedStyles<'i> {
         }
     }
 
-    pub fn computed(&'i self) -> HashMap<Id, &Style> {
+    pub fn computed(&'i self) -> HashMap<Id<'i>, &'i Style<'i>> {
         let mut result = HashMap::new();
         let map = |s: &'i (u32, Style<'i>)| &s.1;
         let mut insert = |s: &'i Style<'i>| {

crates/oxvg_ast/src/visitor.rs

@@ -9,27 +9,36 @@ use crate::{
     style::{self, ComputedStyles, ElementData},
 };
 
+#[derive(Debug, Default, Clone)]
+pub struct Info {
+    pub path: Option<std::path::PathBuf>,
+    pub multipass_count: usize,
+}
+
 #[derive(Debug)]
 pub struct Context<'i, 'o, E: Element> {
     pub computed_styles: crate::style::ComputedStyles<'i>,
     pub stylesheet: Option<lightningcss::stylesheet::StyleSheet<'i, 'o>>,
     pub element_styles: &'i HashMap<E, ElementData<E>>,
     pub root: E,
     pub flags: ContextFlags,
+    pub info: &'i Info,
 }
 
 impl<'i, 'o, E: Element> Context<'i, 'o, E> {
     pub fn new(
         root: E,
         flags: ContextFlags,
         element_styles: &'i HashMap<E, ElementData<E>>,
+        info: &'i Info,
     ) -> Self {
         Self {
             computed_styles: crate::style::ComputedStyles::default(),
             stylesheet: None,
             element_styles,
             root,
             flags,
+            info,
         }
     }
 }
@@ -88,7 +97,7 @@ pub trait Visitor<E: Element> {
     ///
     /// # Errors
     /// Whether the visitor fails
-    fn document(&mut self, document: &mut E) -> Result<(), Self::Error> {
+    fn document(&mut self, document: &mut E, context: &Context<E>) -> Result<(), Self::Error> {
         Ok(())
     }
 
@@ -164,7 +173,7 @@ pub trait Visitor<E: Element> {
     ///
     /// # Errors
     /// If any of the visitor's methods fail
-    fn start(&mut self, root: &mut E) -> Result<PrepareOutcome, Self::Error> {
+    fn start(&mut self, root: &mut E, info: &Info) -> Result<PrepareOutcome, Self::Error> {
         let element_styles = &mut HashMap::new();
         let mut flags = ContextFlags::empty();
         let prepare_outcome = self.prepare(root, &mut flags);
@@ -181,11 +190,14 @@ pub trait Visitor<E: Element> {
             )
             .ok();
             *element_styles = ElementData::new(root);
-            let mut context = Context::new(root.clone(), flags, element_styles);
+            let mut context = Context::new(root.clone(), flags, element_styles, info);
             context.stylesheet = stylesheet;
             self.visit(root, &mut context)?;
         } else {
-            self.visit(root, &mut Context::new(root.clone(), flags, element_styles))?;
+            self.visit(
+                root,
+                &mut Context::new(root.clone(), flags, element_styles, info),
+            )?;
         };
         Ok(prepare_outcome)
     }
@@ -201,7 +213,7 @@ pub trait Visitor<E: Element> {
     ) -> Result<(), Self::Error> {
         match element.node_type() {
             node::Type::Document => {
-                self.document(element)?;
+                self.document(element, context)?;
                 self.visit_children(element, context)?;
                 self.exit_document(element, context)
             }

crates/oxvg_optimiser/benches/default_jobs.rs

@@ -4,6 +4,7 @@ use criterion::{black_box, criterion_group, criterion_main, BenchmarkId, Criteri
 use oxvg_ast::{
     implementations::markup5ever::{Element5Ever, Node5Ever},
     parse::Node,
+    visitor::Info,
 };
 use oxvg_optimiser::Jobs;
 
@@ -33,7 +34,7 @@ pub fn criterion_benchmark(c: &mut Criterion) {
                         let dom = Node5Ever::parse(svg).unwrap();
                         let jobs = Jobs::<Element5Ever>::default();
                         let start = Instant::now();
-                        let _ = black_box(jobs.run(&dom));
+                        let _ = black_box(jobs.run(&dom, &Info::default()));
                         result += start.elapsed();
                     }
                     result

crates/oxvg_optimiser/benches/path.rs

@@ -5,7 +5,7 @@ use oxvg_ast::{
     element::Element,
     implementations::markup5ever::{Element5Ever, Node5Ever},
     parse::Node,
-    visitor::Visitor,
+    visitor::{Info, Visitor},
 };
 use oxvg_optimiser::ConvertPathData;
 
@@ -36,7 +36,7 @@ pub fn criterion_benchmark(c: &mut Criterion) {
                         let mut dom = Element5Ever::from_parent(dom).unwrap();
                         let mut job = ConvertPathData::default();
                         let start = Instant::now();
-                        let _ = black_box(job.start(&mut dom));
+                        let _ = black_box(job.start(&mut dom, &Info::default()));
                         result += start.elapsed();
                     }
                     result