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

@@ -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<Arc<Clip>> {
-        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;

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]

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<Point> = 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();

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));
     }

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::<Vec<_>>());
     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");
+}