From 9652d1d6b257effd6b06f69198a341d73c2e122c Mon Sep 17 00:00:00 2001 From: Gert Pellin Date: Tue, 13 Oct 2026 02:09:29 +0200 Subject: [PATCH] PDF/EPS import: a clip only counts as covering the page when it does Both readers skipped a clip of at most six straight segments whose bounding box contained the page. A diamond touching the page edges has the page as its bounding box, so its clip was dropped and everything it cuts away showed. geom::hit::polygon_covers_rect answers it properly (one convex polygon containing the page's corners); any other path is kept as a clip. Fixes #1542 --- crates/eps/src/import/graphics.rs | 6 +-- crates/eps/src/import/tests.rs | 6 +++ crates/geom/src/hit.rs | 67 +++++++++++++++++++++++++ crates/pdf/src/import.rs | 12 ++--- crates/pdf/src/tests_import_fidelity.rs | 13 +++++ 5 files changed, 94 insertions(+), 10 deletions(-) diff --git a/crates/eps/src/import/graphics.rs b/crates/eps/src/import/graphics.rs index abd7b7c9..00247fe7 100644 --- a/crates/eps/src/import/graphics.rs +++ b/crates/eps/src/import/graphics.rs @@ -366,9 +366,9 @@ impl Out { /// A new clip of `path` inside `clips`; `None` for a rectangle around the whole page, which /// clips nothing. fn clip(&mut self, path: BezPath, rule: FillRule) -> Option> { - let lines = path.elements().iter().all(|e| !matches!(e, PathEl::QuadTo(..) | PathEl::CurveTo(..))); - let around = path.bounding_box().inflate(0.5, 0.5); - if lines && path.elements().len() <= 6 && around.contains(self.frame.origin()) && around.contains(Point::new(self.frame.x1, self.frame.y1)) { + // Only a polygon that really covers the page: a diamond touching its edges has the page as + // its bounding box but cuts its corners away (#1542). + if path.elements().len() <= 6 && vectorcraft_geom::hit::polygon_covers_rect(&path, self.frame, 0.5) { return None; } self.next_clip += 1; diff --git a/crates/eps/src/import/tests.rs b/crates/eps/src/import/tests.rs index 7ce8fa9d..238fcfca 100644 --- a/crates/eps/src/import/tests.rs +++ b/crates/eps/src/import/tests.rs @@ -248,6 +248,12 @@ fn clips_become_clipping_groups() { // eoclip of a path. let o = objects(&read("10 10 moveto 30 10 lineto 30 30 lineto closepath eoclip newpath 0 0 50 50 rectfill").document); assert!(matches!(o[0].kind, NodeKind::Group { clip: true, .. })); + // #1542: a diamond touching the page edges has the page as its bounding box but still clips. + let o = objects(&read("50 0 moveto 100 50 lineto 50 100 lineto 0 50 lineto closepath clip newpath 0 0 100 100 rectfill").document); + assert!(matches!(o[0].kind, NodeKind::Group { clip: true, .. }), "{:?}", o[0].kind); + // A turned square around the whole page clips nothing. + let o = objects(&read("50 -60 moveto 160 50 lineto 50 160 lineto -60 50 lineto closepath clip newpath 0 0 100 100 rectfill").document); + assert!(matches!(o[0].kind, NodeKind::Path { .. }), "{:?}", o[0].kind); } #[test] diff --git a/crates/geom/src/hit.rs b/crates/geom/src/hit.rs index 8e8e11e5..98242d91 100644 --- a/crates/geom/src/hit.rs +++ b/crates/geom/src/hit.rs @@ -6,6 +6,47 @@ use kurbo::{BezPath, ParamCurveNearest, PathEl, Point, Rect, Shape}; use crate::path::{FillRule, PathData}; +/// Does the filled area of `path` cover all of `r` shrunk by `tol` on every side? Answered for one +/// convex polygon (a single subpath of straight segments), which covers the rect when it contains +/// its four corners; any other path gives `false`, so a caller treats it as not covering. +pub fn polygon_covers_rect(path: &BezPath, r: Rect, tol: f64) -> bool { + let mut pts: Vec = Vec::new(); + for (i, el) in path.elements().iter().enumerate() { + match *el { + PathEl::MoveTo(p) if i == 0 => pts.push(p), + PathEl::LineTo(p) if i > 0 => pts.push(p), + PathEl::ClosePath if i > 0 => {} + _ => return false, + } + } + if pts.len() > 1 && pts.first() == pts.last() { + pts.pop(); + } + if pts.len() < 3 { + return false; + } + // Convex: every turn goes the same way (straight runs allowed). + let n = pts.len(); + let mut sign = 0.0f64; + for i in 0..n { + let (a, b, c) = (pts[i], pts[(i + 1) % n], pts[(i + 2) % n]); + let cross = (b - a).cross(c - b); + if cross.abs() <= 1e-9 { + continue; + } + if sign == 0.0 { + sign = cross.signum(); + } else if cross.signum() != sign { + return false; + } + } + let inner = r.inflate(-tol, -tol); + sign != 0.0 + && [Point::new(inner.x0, inner.y0), Point::new(inner.x1, inner.y0), Point::new(inner.x1, inner.y1), Point::new(inner.x0, inner.y1)] + .into_iter() + .all(|c| fill_contains(path, FillRule::NonZero, c)) +} + /// Is `p` inside the filled area of `path` under `rule`? Open subpaths are implicitly closed (as when filled). pub fn fill_contains(path: &BezPath, rule: FillRule, p: Point) -> bool { let w = closed(path).winding(p); @@ -94,6 +135,32 @@ mod tests { use super::*; use crate::shapes; + #[test] + fn polygon_covers_rect_only_for_convex_polygons_around_it() { + let page = Rect::new(0.0, 0.0, 100.0, 100.0); + let poly = |pts: &[(f64, f64)]| { + let mut b = BezPath::new(); + b.move_to(pts[0]); + for &p in &pts[1..] { + b.line_to(p); + } + b.close_path(); + b + }; + assert!(polygon_covers_rect(&poly(&[(0.0, 0.0), (100.0, 0.0), (100.0, 100.0), (0.0, 100.0)]), page, 0.01)); + assert!(polygon_covers_rect(&poly(&[(50.0, -60.0), (160.0, 50.0), (50.0, 160.0), (-60.0, 50.0)]), page, 0.01), "turned, around it"); + // The page's bounding box, but its corners are cut away. + assert!(!polygon_covers_rect(&poly(&[(50.0, 0.0), (100.0, 50.0), (50.0, 100.0), (0.0, 50.0)]), page, 0.01)); + // Contains the corners but notches into the page: not convex. + assert!(!polygon_covers_rect(&poly(&[(-1.0, -1.0), (101.0, -1.0), (101.0, 101.0), (50.0, 40.0), (-1.0, 101.0)]), page, 0.01)); + // Curves and several subpaths aren't judged. + assert!(!polygon_covers_rect(&kurbo::Circle::new((50.0, 50.0), 200.0).to_path(0.1), page, 0.01)); + let mut two = poly(&[(0.0, 0.0), (100.0, 0.0), (100.0, 100.0), (0.0, 100.0)]); + two.extend(poly(&[(10.0, 10.0), (20.0, 10.0), (20.0, 20.0)])); + assert!(!polygon_covers_rect(&two, page, 0.01)); + assert!(!polygon_covers_rect(&BezPath::new(), page, 0.01)); + } + #[test] fn fill_and_stroke() { let p = shapes::rectangle(Rect::new(0.0, 0.0, 10.0, 10.0)).to_bezpath(); diff --git a/crates/pdf/src/import.rs b/crates/pdf/src/import.rs index bac31b72..4af87211 100644 --- a/crates/pdf/src/import.rs +++ b/crates/pdf/src/import.rs @@ -1769,14 +1769,12 @@ impl<'a> Device<'a> for Builder<'_> { (*isolate, *knockout) = (i, k); } } - let bb = clip_path.path.bounding_box(); - // Clips that contain the whole page do nothing visible; don't create groups for them. + // Clips that contain the whole page do nothing visible; don't create groups for them. Only + // a polygon that really covers it counts: a diamond touching the page edges has the page + // as its bounding box but cuts its corners away (#1542). let redundant = clip_path.path.elements().len() <= 6 - && bb.x0 <= self.page.x0 + 0.01 - && bb.y0 <= self.page.y0 + 0.01 - && bb.x1 >= self.page.x1 - 0.01 - && bb.y1 >= self.page.y1 - 0.01 - && self.page.area() > 0.0; + && self.page.area() > 0.0 + && vectorcraft_geom::hit::polygon_covers_rect(&clip_path.path, self.page, 0.01); let kind = if redundant { FrameKind::Skip } else { FrameKind::Clip(Box::new(self.clip_node(&clip_path.path, fill_rule(clip_path.fill)))) }; self.stack.push(Frame::new(kind)); } diff --git a/crates/pdf/src/tests_import_fidelity.rs b/crates/pdf/src/tests_import_fidelity.rs index 4a9a1acf..dcab2f19 100644 --- a/crates/pdf/src/tests_import_fidelity.rs +++ b/crates/pdf/src/tests_import_fidelity.rs @@ -596,3 +596,16 @@ fn glyphs_closed_up_by_half_an_em_stay_one_line() { assert_eq!(t.len(), 1, "one line: {:?}", t.iter().map(|t| t.plain_text()).collect::>()); assert_eq!(t[0].plain_text(), "Tight"); } + +/// #1542: a clip only counts as covering the page when it does; a diamond whose bounding box is +/// the page still clips, a page rectangle or a turned square around it doesn't make a group. +#[test] +fn clips_with_the_page_as_bounding_box_still_clip() { + let clipped = |clip: &str| { + let d = open(&one_page(&format!("q {clip} W n 1 0.5 0 rg 0 0 100 100 re f Q"), "", &[])); + all(&d).iter().any(|n| matches!(n.kind, NodeKind::Group { clip: true, .. })) + }; + assert!(clipped("50 0 m 100 50 l 50 100 l 0 50 l h"), "diamond"); + assert!(!clipped("0 0 100 100 re"), "the page"); + assert!(!clipped("50 -60 m 160 50 l 50 160 l -60 50 l h"), "a turned square around the page"); +}