From e3d76471f6c3bcd8e19fba308c62ec0050ae1b64 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Fri, 13 Nov 2026 02:36:05 -0500 Subject: [PATCH] Fix vector import edge cases from review --- crates/dxf/src/paths.rs | 69 ++++++++++++++++++++++++++--------------- crates/eps/src/paths.rs | 49 +++++++++++++++++++++++------ crates/pdf/src/paths.rs | 44 +++++++++++++++++++++++--- crates/svg/src/write.rs | 11 +++++-- 4 files changed, 130 insertions(+), 43 deletions(-) diff --git a/crates/dxf/src/paths.rs b/crates/dxf/src/paths.rs index 1b57cec40..01e555797 100644 --- a/crates/dxf/src/paths.rs +++ b/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 { 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 { 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::().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]] +} diff --git a/crates/eps/src/paths.rs b/crates/eps/src/paths.rs index adbe294f3..ad1f23793 100644 --- a/crates/eps/src/paths.rs +++ b/crates/eps/src/paths.rs @@ -45,24 +45,46 @@ pub fn import(bytes: &[u8], limits: Limits) -> Result { 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, elements: &mut usize, limits: Limits) -> Result<(), Error> { +fn collect( + node: &vectorcraft_doc::Node, + paths: &mut Vec, + 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, 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(()) } diff --git a/crates/pdf/src/paths.rs b/crates/pdf/src/paths.rs index bb51e2a0f..9f7126013 100644 --- a/crates/pdf/src/paths.rs +++ b/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 { 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 { 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>) {} 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); diff --git a/crates/svg/src/write.rs b/crates/svg/src/write.rs index 664ce8f01..b73c8c1bf 100644 --- a/crates/svg/src/write.rs +++ b/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![