Fix vector import edge cases from review

crates/dxf/src/paths.rs

@@ -14,6 +14,8 @@ pub enum Error {
     Parse(#[from] crate::DxfError),
     #[error("DXF contains more than {MAX_PATHS} paths")]
     TooManyPaths,
+    #[error("DXF curve exceeds the subdivision limit")]
+    TooManySegments,
     #[error("DXF contains a non-finite path coordinate")]
     NonFinite,
 }
@@ -41,6 +43,9 @@ pub fn import(bytes: &[u8]) -> Result<Imported, Error> {
         if !entity.common.visible || record.paper_space() {
             continue;
         }
+        if excessive_sweep(&entity.kind) {
+            return Err(Error::TooManySegments);
+        }
         let converted = entity_paths(&entity.kind, entity.common.extrusion);
         if converted.iter().flat_map(|path| path.elements()).any(|element| {
             let finite = |p: Point| p.x.is_finite() && p.y.is_finite();
@@ -54,17 +59,7 @@ pub fn import(bytes: &[u8]) -> Result<Imported, Error> {
             return Err(Error::NonFinite);
         }
         if converted.is_empty() {
-            if matches!(
-                entity.kind,
-                crate::model::EntityKind::Other
-                    | crate::model::EntityKind::Text(_)
-                    | crate::model::EntityKind::MText(_)
-                    | crate::model::EntityKind::Insert(_)
-                    | crate::model::EntityKind::Dimension(_)
-                    | crate::model::EntityKind::Hatch(_)
-            ) {
-                unsupported = unsupported.saturating_add(1);
-            }
+            unsupported = unsupported.saturating_add(1);
             continue;
         }
         if paths.len().saturating_add(converted.len()) > MAX_PATHS {
@@ -117,27 +112,44 @@ fn entity_paths(kind: &crate::model::EntityKind, normal: crate::model::Point) ->
     match kind {
         E::Line { start, end } => vec![line(*start, *end)],
         E::Circle { center, radius } if radius.is_finite() && *radius > 0.0 => {
-            vec![circle(ocs(normal, *center), *radius)]
+            vec![map_ocs_path(circle(Point::new(center[0], center[1]), *radius), normal, center[2])]
         }
         E::Arc { center, radius, start_angle, end_angle }
             if radius.is_finite() && *radius > 0.0 && start_angle.is_finite() && end_angle.is_finite() =>
         {
-            vec![arc(ocs(normal, *center), *radius, start_angle.to_radians(), end_angle.to_radians())]
+            vec![map_ocs_path(arc(Point::new(center[0], center[1]), *radius, start_angle.to_radians(), end_angle.to_radians()), normal, center[2])]
+        }
+        E::LwPolyline(poly) => {
+            vec![map_ocs_path(polyline(poly.vertices.iter().map(|v| (Point::new(v.x, v.y), v.bulge)), poly.closed), normal, poly.elevation)]
         }
-        E::LwPolyline(poly) => vec![polyline(poly.vertices.iter().map(|v| (ocs(normal, [v.x, v.y, poly.elevation]), v.bulge)), poly.closed)],
         E::Polyline(poly) if !poly.is_mesh() && !poly.is_polyface() => {
-            vec![polyline(poly.vertices.iter().filter(|v| !v.is_face()).map(|v| (ocs(normal, v.location), v.bulge)), poly.closed())]
+            let is_3d = poly.flags & 8 != 0;
+            let z = if is_3d { 0.0 } else { poly.elevation };
+            let path =
+                polyline(poly.vertices.iter().filter(|v| !v.is_face()).map(|v| (Point::new(v.location[0], v.location[1]), v.bulge)), poly.closed());
+            vec![if is_3d { path } else { map_ocs_path(path, normal, z) }]
         }
         E::Ellipse { center, major, ratio, start, end } if ratio.is_finite() && *ratio > 0.0 => {
-            vec![ellipse(*center, *major, *ratio, *start, *end)]
+            vec![ellipse(*center, *major, normal, *ratio, *start, *end)]
         }
         _ => Vec::new(),
     }
 }
 
-fn ocs(normal: crate::model::Point, point: crate::model::Point) -> Point {
-    let p = crate::model::ocs_to_wcs(normal, point);
-    Point::new(p[0], p[1])
+fn map_ocs_path(path: BezPath, normal: crate::model::Point, z: f64) -> BezPath {
+    let [x, y, n] = crate::model::ocs_axes(normal);
+    let transform = kurbo::Affine::new([x[0], y[0], x[1], y[1], n[0] * z, n[1] * z]);
+    transform * path
+}
+
+fn excessive_sweep(kind: &crate::model::EntityKind) -> bool {
+    let cap = 4096.0 * std::f64::consts::FRAC_PI_2;
+    let over = |sweep: f64| !sweep.is_finite() || sweep.abs() > cap;
+    match kind {
+        crate::model::EntityKind::Arc { start_angle, end_angle, .. } => over((end_angle - start_angle).to_radians()),
+        crate::model::EntityKind::Ellipse { start, end, .. } => over(end - start),
+        _ => false,
+    }
 }
 
 fn line(start: crate::model::Point, end: crate::model::Point) -> BezPath {
@@ -220,11 +232,14 @@ fn arc_segments(path: &mut BezPath, center: Point, start: f64, sweep: f64, radiu
     }
 }
 
-fn ellipse(center: crate::model::Point, major: crate::model::Point, ratio: f64, start: f64, end: f64) -> BezPath {
-    let (cx, cy) = (center[0], center[1]);
-    let (ax, ay) = (major[0], major[1]);
-    let (bx, by) = (-ay * ratio, ax * ratio);
-    let point = |t: f64| Point::new(cx + ax * t.cos() + bx * t.sin(), cy + ay * t.cos() + by * t.sin());
+fn ellipse(center: crate::model::Point, major: crate::model::Point, normal: crate::model::Point, ratio: f64, start: f64, end: f64) -> BezPath {
+    let minor = cross(normal, major);
+    let minor_len = minor.iter().map(|v| v * v).sum::<f64>().sqrt();
+    if !(minor_len.is_finite() && minor_len > 0.0) {
+        return BezPath::new();
+    }
+    let bx = [minor[0] / minor_len * ratio, minor[1] / minor_len * ratio, minor[2] / minor_len * ratio];
+    let point = |t: f64| Point::new(center[0] + major[0] * t.cos() + bx[0] * t.sin(), center[1] + major[1] * t.cos() + bx[1] * t.sin());
     let mut path = BezPath::new();
     path.move_to(point(start));
     let sweep = if end >= start { end - start } else { end - start + std::f64::consts::TAU };
@@ -237,10 +252,14 @@ fn ellipse(center: crate::model::Point, major: crate::model::Point, ratio: f64,
         let a = start + step * i as f64;
         let b = a + step;
         let k = 4.0 / 3.0 * (step / 4.0).tan();
-        let derivative = |t: f64| Point::new(-ax * t.sin() + bx * t.cos(), -ay * t.sin() + by * t.cos());
+        let derivative = |t: f64| Point::new(-major[0] * t.sin() + bx[0] * t.cos(), -major[1] * t.sin() + bx[1] * t.cos());
         let p1 = point(a) + derivative(a).to_vec2() * k;
         let p2 = point(b) - derivative(b).to_vec2() * k;
         path.curve_to(p1, p2, point(b));
     }
     path
 }
+
+fn cross(a: crate::model::Point, b: crate::model::Point) -> crate::model::Point {
+    [a[1] * b[2] - a[2] * b[1], a[2] * b[0] - a[0] * b[2], a[0] * b[1] - a[1] * b[0]]
+}

crates/eps/src/paths.rs

@@ -45,24 +45,46 @@ pub fn import(bytes: &[u8], limits: Limits) -> Result<Imported, Error> {
         return Err(Error::InputLimit { limit: limits.max_input_bytes });
     }
     let source = vectorcraft_eps::import(bytes).map_err(Error::Parse)?;
-    let page_bounds = source
-        .document
-        .artboards
-        .first()
-        .map_or(Rect::new(0.0, 0.0, 612.0, 792.0), |board| Rect::new(board.rect.x0, board.rect.y0, board.rect.x1, board.rect.y1));
+    let section = crate::sections(bytes).map_err(|e| Error::Parse(e.to_string()))?;
+    let source_box = crate::Comments::read(section.postscript).page_box().unwrap_or([0.0, 0.0, 612.0, 792.0]);
+    let [llx, _lly, urx, ury] = source_box;
+    let page_bounds = Rect::new(llx, lly, urx, ury);
     let mut paths = Vec::new();
     let mut total_elements = 0usize;
+    let mut clipped = false;
+    let mut omitted = false;
     for layer in &source.document.layers {
-        collect(layer, &mut paths, &mut total_elements, limits)?;
+        collect(layer, &mut paths, &mut total_elements, limits, &mut clipped, &mut omitted)?;
     }
-    Ok(Imported { paths, page_bounds, warnings: source.warnings, preview_fallback: source.preview })
+    let inverse_page = kurbo::Affine::new([1.0, 0.0, 0.0, -1.0, llx, ury]);
+    for path in &mut paths {
+        *path = inverse_page * path.clone();
+    }
+    let mut warnings = source.warnings;
+    if clipped {
+        warnings.push("EPS clipping groups are returned as unclipped artwork".into());
+    }
+    if omitted {
+        warnings.push("visible EPS text or image content is not returned as paths".into());
+    }
+    Ok(Imported { paths, page_bounds, warnings, preview_fallback: source.preview })
 }
 
-fn collect(node: &vectorcraft_doc::Node, paths: &mut Vec<BezPath>, elements: &mut usize, limits: Limits) -> Result<(), Error> {
+fn collect(
+    node: &vectorcraft_doc::Node,
+    paths: &mut Vec<BezPath>,
+    elements: &mut usize,
+    limits: Limits,
+    clipped: &mut bool,
+    omitted: &mut bool,
+) -> Result<(), Error> {
     if !node.visible {
         return Ok(());
     }
     if let Some(data) = node.path_data() {
+        if matches!(node.kind, vectorcraft_doc::NodeKind::Path { clipping: true, .. }) {
+            return Ok(());
+        }
         let path = data.to_bezpath();
         if !path.elements().is_empty() {
             *elements = elements.saturating_add(path.elements().len());
@@ -72,9 +94,16 @@ fn collect(node: &vectorcraft_doc::Node, paths: &mut Vec<BezPath>, elements: &mu
             paths.push(path);
         }
     } else if let Some(children) = node.children() {
-        for child in children {
-            collect(child, paths, elements, limits)?;
+        let is_clip = matches!(node.kind, vectorcraft_doc::NodeKind::Group { clip: true, .. } | vectorcraft_doc::NodeKind::Layer { clip: true, .. });
+        *clipped |= is_clip;
+        for (index, child) in children.iter().enumerate() {
+            if is_clip && index == 0 {
+                continue;
+            }
+            collect(child, paths, elements, limits, clipped, omitted)?;
         }
+    } else {
+        *omitted = true;
     }
     Ok(())
 }

crates/pdf/src/paths.rs

@@ -8,6 +8,10 @@ use hayro_interpret::{
     BlendMode, ClipPath, Device, GlyphDrawMode, Image, InterpreterCache, InterpreterSettings, Paint, PathDrawMode, RectExt, SoftMask,
 };
 use kurbo::{Affine, BezPath, Rect};
+use std::sync::{
+    Arc,
+    atomic::{AtomicBool, Ordering},
+};
 
 /// Limits for vector path extraction. They bound the output model, not interpreter peak memory.
 #[derive(Debug, Clone, Copy)]
@@ -61,14 +65,31 @@ pub fn import(bytes: &[u8], limits: Limits) -> Result<Imported, Error> {
         return Err(Error::Pages { limit: limits.max_pages });
     }
     let cache = InterpreterCache::new();
+    let truncated = Arc::new(AtomicBool::new(false));
+    let truncation_flag = truncated.clone();
+    let settings = InterpreterSettings {
+        warning_sink: Arc::new(move |warning| {
+            if matches!(warning, hayro_interpret::InterpreterWarning::ContentTruncated) {
+                truncation_flag.store(true, Ordering::Relaxed);
+            }
+        }),
+        ..InterpreterSettings::default()
+    };
     let mut pages = Vec::with_capacity(pdf.pages().len());
     let mut total_paths = 0usize;
     let mut total_elements = 0usize;
     for page in pdf.pages().iter() {
         let crop_box = page.intersected_crop_box().to_kurbo();
-        let mut context = hayro_interpret::Context::new(Affine::IDENTITY, crop_box, &cache, pdf.xref(), InterpreterSettings::default());
-        let mut device = Collector::default();
+        let mut context = hayro_interpret::Context::new(Affine::IDENTITY, crop_box, &cache, pdf.xref(), settings.clone());
+        let mut device = Collector {
+            max_paths: limits.max_paths.saturating_sub(total_paths),
+            max_elements: limits.max_path_elements.saturating_sub(total_elements),
+            ..Collector::default()
+        };
         crate::read::interpret_page(page, &mut context, &mut device)?;
+        if device.exceeded {
+            return Err(Error::OutputLimit);
+        }
         total_paths = total_paths.saturating_add(device.paths.len());
         total_elements = device.paths.iter().fold(total_elements, |n, path| n.saturating_add(path.elements().len()));
         if total_paths > limits.max_paths || total_elements > limits.max_path_elements {
@@ -92,6 +113,9 @@ pub fn import(bytes: &[u8], limits: Limits) -> Result<Imported, Error> {
     if pages.iter().any(|page| page.clips_seen > 0) {
         warnings.push("clip paths are not applied to extracted geometry".into());
     }
+    if truncated.load(Ordering::Relaxed) {
+        warnings.push("PDF content was truncated by the interpreter".into());
+    }
     Ok(Imported { pages, warnings })
 }
 
@@ -101,16 +125,26 @@ struct Collector {
     omitted_text: usize,
     omitted_images: usize,
     clips_seen: usize,
+    max_paths: usize,
+    max_elements: usize,
+    elements: usize,
+    exceeded: bool,
 }
 
 impl Device<'_> for Collector {
     fn set_soft_mask(&mut self, _: Option<SoftMask<'_>>) {}
     fn set_blend_mode(&mut self, _: BlendMode) {}
     fn draw_path(&mut self, path: &BezPath, transform: Affine, _: &Paint<'_>, _: &PathDrawMode) {
-        let path = transform * path.clone();
-        if !path.is_empty() {
-            self.paths.push(path);
+        if path.is_empty() {
+            return;
         }
+        let count = path.elements().len();
+        if self.paths.len() >= self.max_paths || self.elements.saturating_add(count) > self.max_elements {
+            self.exceeded = true;
+            return;
+        }
+        self.elements = self.elements.saturating_add(count);
+        self.paths.push(transform * path.clone());
     }
     fn push_clip_path(&mut self, _: &ClipPath) {
         self.clips_seen = self.clips_seen.saturating_add(1);

crates/svg/src/write.rs

@@ -107,7 +107,11 @@ impl Scene {
 }
 
 fn write_attributes<'a>(mut tag: craft_xml::Tag<'a>, attributes: &'a [Attribute]) -> craft_xml::Tag<'a> {
-    for attribute in attributes {
+    // SVG consumers disagree about duplicate attributes; make the last supplied value win.
+    for (index, attribute) in attributes.iter().enumerate() {
+        if attributes[index + 1..].iter().any(|later| later.name == attribute.name) {
+            continue;
+        }
         tag = match &attribute.value {
             Value::Text(value) => tag.attr(&attribute.name, value),
             Value::Number(value) => tag.attr(&attribute.name, num(*value, 3)),
@@ -136,8 +140,9 @@ fn write_elements(writer: &mut SvgWriter, elements: &[Element], depth: usize) {
                 writer.close();
             }
             Element::Path { attributes, path } => {
-                let attributes = attributes.clone();
-                write_attributes(writer.tag("path").attr("d", path_data(path)), &attributes).empty();
+                let mut attrs = vec![Attribute::text("d", path_data(path))];
+                attrs.extend(attributes.iter().cloned());
+                write_attributes(writer.tag("path"), &attrs).empty();
             }
             Element::Rect { attributes, x, y, width, height } => {
                 let mut attrs = vec![