Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for matrix transforms #18

Closed
naufraghi opened this issue Dec 22, 2020 · 7 comments · Fixed by #29
Closed

Add support for matrix transforms #18

naufraghi opened this issue Dec 22, 2020 · 7 comments · Fixed by #29

Comments

@naufraghi
Copy link
Contributor

Transformed paths are quite common, a tool like usvg can be used to move into a matrix transform most other flavors.

lyon_geom::euclid::Transform2D::row_major(...) can be used to transform the points.

Both lyon_geom and euclid changed a lot in the newer releases, the suggested method is the one re-exported by lyon_geom version 0.12 (now 0.16).

@dbrgn
Copy link
Owner

dbrgn commented Dec 22, 2020

I'd first have to dig into how SVG handles those transformations, but right now I don't actively use this project anymore so it has low priority. Pull requests would be welcome though!

@naufraghi
Copy link
Contributor Author

I have a rough matrix parser locally, I still need to add a few test but somehow it "works". I have a parent PR in issue #16 with some cosmetic changes (beside the tol parameter addition), do you have suggestions about that?

@dbrgn
Copy link
Owner

dbrgn commented Jul 17, 2022

@naufraghi how's the state of your matrix parser?

I started using svg2polylines for a new project and already ran into issues due to ignored transformations 🙂

@naufraghi
Copy link
Contributor Author

I completely forgot about it, I'll have a look if I can find something, but I think the code is stuck at the 2020 state.

@dbrgn
Copy link
Owner

dbrgn commented Jul 18, 2022

If you have some WIP code that might already be useful! 🙂

@naufraghi
Copy link
Contributor Author

I have something in a local working copy, it is based on some weird git state I cannot rember (subtree?), so I post here the patch that contains most of the changes.

0001-WIP-add-support-for-matrix-transform-in-SVG.patch
From e224384555bcf320ba9912300b70f7a314975b56 Mon Sep 17 00:00:00 2001
From: Matteo Bertini 
Date: Tue, 19 Jul 2022 09:48:23 +0200
Subject: [PATCH] WIP: add support for matrix transform in SVG

---
 svg2polylines/svg2polylines/src/lib.rs | 133 +++++++++++++++++++++----
 1 file changed, 116 insertions(+), 17 deletions(-)

diff --git svg2polylines/svg2polylines/src/lib.rs svg2polylines/svg2polylines/src/lib.rs
index 3df1782..928cb6e 100644
--- svg2polylines/svg2polylines/src/lib.rs
+++ svg2polylines/svg2polylines/src/lib.rs
@@ -23,10 +23,12 @@
 
 use std::convert;
 use std::mem;
+use std::ops::Index;
 use std::str;
 
 use log::trace;
 use lyon_geom::euclid::Point2D;
+use lyon_geom::euclid::Transform2D;
 use lyon_geom::{CubicBezierSegment, QuadraticBezierSegment};
 use quick_xml::events::attributes::Attribute;
 use quick_xml::events::Event;
@@ -44,10 +46,54 @@ pub struct CoordinatePair {
     pub y: f64,
 }
 
+#[allow(clippy::many_single_char_names)]
+fn parse_tranform(transform: &str) -> Result, String> {
+    use std::convert::TryInto;
+    let transform = transform.trim();
+    if !transform.starts_with("matrix(") {
+        return Err(format!(
+            "Only 'matrix' transform supported in {:?}",
+            transform
+        ));
+    }
+    if !transform.ends_with(')') {
+        return Err(format!("Missing closing parenthesis in {:?}", transform));
+    }
+    let matrix = transform
+        .strip_prefix("matrix(")
+        .expect("checked before")
+        .strip_suffix(")")
+        .expect("checked to be there");
+    let coefs: Result, _> = matrix
+        .replace(",", " ") // strip extra commas
+        .split_whitespace()
+        .map(str::parse::)
+        .collect();
+    if coefs.is_err() {
+        return Err(format!("Invalid matrix coefficients in {:?}", transform));
+    }
+    let coefs = coefs.expect("checked to be valid");
+    if coefs.len() != 6 {
+        return Err(format!(
+            "Invalid number of matrix coefficients in {:?}",
+            transform
+        ));
+    }
+    let [a, b, c, d, e, f]: [f64; 6] = coefs.as_slice().try_into().expect("checked to be 6");
+    Ok(Transform2D::row_major(a, b, c, d, e, f))
+}
+
 impl CoordinatePair {
+    #[must_use]
     pub fn new(x: f64, y: f64) -> Self {
         Self { x, y }
     }
+    pub fn transform(&mut self, t: Transform2D) {
+        let p = Point2D::new(self.x, self.y);
+        let pt = t.transform_point(&p);
+        self.x = pt.x;
+        self.y = pt.y;
+    }
 }
 
 impl convert::From<(f64, f64)> for CoordinatePair {
@@ -57,7 +103,37 @@ impl convert::From<(f64, f64)> for CoordinatePair {
 }
 
 /// A polyline is a vector of `CoordinatePair` instances.
-pub type Polyline = Vec;
+#[derive(Debug, PartialEq)]
+pub struct Polyline(Vec);
+
+impl Polyline {
+    fn new() -> Self {
+        Polyline(vec![])
+    }
+    fn transform(mut self, matrix: Transform2D) -> Self {
+        for p in &mut self.0 {
+            p.transform(matrix);
+        }
+        self
+    }
+    fn push(&mut self, val: CoordinatePair) {
+        self.0.push(val)
+    }
+    fn len(&self) -> usize {
+        self.0.len()
+    }
+    fn last(&self) -> Option<&CoordinatePair> {
+        self.0.last()
+    }
+}
+
+impl Index for Polyline {
+    type Output = CoordinatePair;
+
+    fn index(&self, id: usize) -> &Self::Output {
+        &self.0[id]
+    }
+}
 
 #[derive(Debug, PartialEq)]
 struct CurrentLine {
@@ -78,12 +154,12 @@ impl CurrentLine {
         }
     }
 
-    /// Add a CoordinatePair to the internal polyline.
+    /// Add a `CoordinatePair` to the internal polyline.
     fn add_absolute(&mut self, pair: CoordinatePair) {
         self.line.push(pair);
     }
 
-    /// Add a relative CoordinatePair to the internal polyline.
+    /// Add a relative `CoordinatePair` to the internal polyline.
     fn add_relative(&mut self, pair: CoordinatePair) {
         if let Some(last) = self.line.last() {
             let cp = CoordinatePair::new(last.x + pair.x, last.y + pair.y);
@@ -95,7 +171,7 @@ impl CurrentLine {
         }
     }
 
-    /// Add a CoordinatePair to the internal polyline.
+    /// Add a `CoordinatePair` to the internal polyline.
     fn add(&mut self, abs: bool, pair: CoordinatePair) {
         if abs {
             self.add_absolute(pair);
@@ -104,7 +180,7 @@ impl CurrentLine {
         }
     }
 
-    /// A polyline is only valid if it has more than 1 CoordinatePair.
+    /// A polyline is only valid if it has more than 1 `CoordinatePair`.
     fn is_valid(&self) -> bool {
         self.line.len() > 1
     }
@@ -127,13 +203,13 @@ impl CurrentLine {
     /// Close the line by adding the first entry to the end.
     fn close(&mut self) -> Result<(), String> {
         if self.line.len() < 2 {
-            Err("Lines with less than 2 coordinate pairs cannot be closed.".into())
+            trace!("Lines with less than 2 coordinate pairs cannot be closed.");
         } else {
             let first = self.line[0];
             self.line.push(first);
             self.prev_end = Some(first);
-            Ok(())
         }
+        Ok(())
     }
 
     /// Replace the internal polyline with a new instance and return the
@@ -147,7 +223,7 @@ impl CurrentLine {
 }
 
 /// Parse an SVG string, return vector of path expressions.
-fn parse_xml(svg: &str) -> Result, String> {
+fn parse_xml(svg: &str) -> Result)>, String> {
     trace!("parse_xml");
 
     let mut reader = quick_xml::Reader::from_str(svg);
@@ -160,6 +236,11 @@ fn parse_xml(svg: &str) -> Result, String> {
             Ok(Event::Start(ref e)) | Ok(Event::Empty(ref e)) => {
                 trace!("parse_xml: Matched start of {:?}", e.name());
                 match e.name() {
+                    // TODO: add support for `transform` attribute
+                    // 
+                    // Will support only the `matrix()` transform, one can use `usvg` to obtain a
+                    // simplified SVG (https://github.com/RazrFalcon/resvg/blob/master/docs/usvg_spec.adoc#transform-type)
                     b"path" => {
                         trace!("parse_xml: Found path attribute");
                         let path_expr: Option = e
@@ -174,8 +255,20 @@ fn parse_xml(svg: &str) -> Result, String> {
                                     None
                                 }
                             });
+                        let transform_expr: Option = e
+                            .attributes()
+                            .filter_map(Result::ok)
+                            .find_map(|attr: Attribute| {
+                                if attr.key == b"transform" {
+                                    attr.unescaped_value()
+                                        .ok()
+                                        .and_then(|v| str::from_utf8(&v).map(str::to_string).ok())
+                                } else {
+                                    None
+                                }
+                            });
                         if let Some(expr) = path_expr {
-                            paths.push(expr);
+                            paths.push((expr, transform_expr));
                         }
                     }
                     _ => {}
@@ -403,8 +496,14 @@ pub fn parse(svg: &str, tol: f64) -> Result, String> {
     let mut polylines: Vec = Vec::new();
 
     // Process path expressions
-    for expr in path_exprs {
-        polylines.extend(parse_path(&expr, tol)?);
+    for (path_expr, tr_expr) in path_exprs {
+        let path = parse_path(&path_expr, tol)?;
+        if let Some(m) = tr_expr {
+            let m = parse_tranform(&m)?;
+            polylines.extend(path.into_iter().map(|poly| poly.transform(m)));
+        } else {
+            polylines.extend(path);
+        }
     }
 
     trace!("parse: This results in {} polylines", polylines.len());
@@ -785,7 +884,7 @@ mod tests {
         let result = parse_xml(&input).unwrap();
         assert_eq!(
             result,
-            vec!["M 10,100 40,70 h 10 m -20,40 10,-20".to_string()]
+            vec![("M 10,100 40,70 h 10 m -20,40 10,-20".to_string(), None)]
         );
     }
 
@@ -803,8 +902,8 @@ mod tests {
         assert_eq!(
             result,
             vec![
-                "M 10,100 40,70 h 10 m -20,40 10,-20".to_string(),
-                "M 20,30".to_string(),
+                ("M 10,100 40,70 h 10 m -20,40 10,-20".to_string(), None),
+                ("M 20,30".to_string(), None),
             ]
         );
     }
@@ -820,7 +919,7 @@ mod tests {
             
         "#;
         let result = parse_xml(&input).unwrap();
-        assert_eq!(result, vec!["M 20,30".to_string()]);
+        assert_eq!(result, vec![("M 20,30".to_string(), None)]);
     }
 
     #[test]
@@ -854,7 +953,7 @@ mod tests {
         assert_eq!(result.len(), 1);
         assert_eq!(result[0].len(), 11);
         assert_eq!(
-            result[0],
+            result[0].0,
             vec![
                 CoordinatePair {
                     x: 0.10650371,
@@ -921,7 +1020,7 @@ mod tests {
         assert_eq!(result.len(), 1);
         assert_eq!(result[0].len(), 31);
         assert_eq!(
-            result[0],
+            result[0].0,
             vec![
                 CoordinatePair { x: 10.0, y: 80.0 },
                 CoordinatePair {
-- 
2.35.1

@dbrgn
Copy link
Owner

dbrgn commented Aug 15, 2022

The hint about usvg was great btw, I think if I use this as a preprocessing step I can solve a huge number of potential edge cases in one go 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants