Merge pull request #426 from mockydevs/fix/path-as-rect-open-polyline

pdf reader: a five-point open polyline is not a rectangle

crates/pdf/tests/interpretation.rs

@@ -118,3 +118,114 @@ fn caller_decode_policy_survives_open_fallback() {
     assert_eq!(low.decode_limits().max_decoded_stream(), 32);
     assert_eq!(normal.decode_limits(), craft_pdf::syntax::DecodeLimits::default());
 }
+
+/// A path reaches the device as a rectangle only when it is one. A five-point open polyline whose
+/// points reach every corner of its own bounding box (VectorCraft #2920, a zigzag in an `.ai`
+/// file) used to be drawn as that box.
+#[cfg(feature = "interpret")]
+mod rectangles {
+    use craft_pdf::interpret::{
+        BlendMode, ClipPath, Context, Device, GlyphDrawMode, Image, InterpreterCache, InterpreterSettings, Paint, PathDrawMode, SoftMask,
+        TransformExt, font::Glyph,
+    };
+    use kurbo::{Affine, BezPath, Rect};
+
+    #[derive(Default)]
+    struct Shapes {
+        rects: usize,
+        paths: Vec<BezPath>,
+        clips: usize,
+    }
+
+    impl Device<'_> for Shapes {
+        fn set_soft_mask(&mut self, _: Option<SoftMask<'_>>) {}
+        fn set_blend_mode(&mut self, _: BlendMode) {}
+        fn draw_path(&mut self, path: &BezPath, _: Affine, _: &Paint<'_>, _: &PathDrawMode) {
+            self.paths.push(path.clone());
+        }
+        fn draw_rect(&mut self, _: &Rect, _: Affine, _: &Paint<'_>, _: &PathDrawMode) {
+            self.rects += 1;
+        }
+        fn push_clip_path(&mut self, _: &ClipPath) {
+            self.clips += 1;
+        }
+        fn push_transparency_group(&mut self, _: f32, _: Option<SoftMask<'_>>, _: BlendMode) {}
+        fn draw_glyph(&mut self, _: &Glyph<'_>, _: Affine, _: Affine, _: &Paint<'_>, _: &GlyphDrawMode) {}
+        fn draw_image(&mut self, _: Image<'_, '_>, _: Affine) {}
+        fn pop_clip_path(&mut self) {}
+        fn pop_transparency_group(&mut self) {}
+    }
+
+    fn draw(content: &str) -> Shapes {
+        let bytes = format!(
+            "%PDF-1.7\n1 0 obj << /Type /Catalog /Pages 2 0 R >> endobj\n2 0 obj << /Type /Pages /Kids [3 0 R] /Count 1 >> endobj\n3 0 obj << /Type /Page /Parent 2 0 R /MediaBox [0 0 120 60] /Contents 4 0 R >> endobj\n4 0 obj << /Length {} >> stream\n{content}\nendstream endobj\ntrailer << /Root 1 0 R >>\n%%EOF",
+            content.len()
+        );
+        let pdf = craft_pdf::open_slice(bytes.as_bytes(), None).unwrap();
+        let page = &pdf.pages()[0];
+        let cache = InterpreterCache::new();
+        let mut context = Context::new(
+            page.initial_transform(true).to_kurbo(),
+            Rect::new(0.0, 0.0, 120.0, 60.0),
+            &cache,
+            page.xref(),
+            InterpreterSettings::default(),
+        );
+        let mut shapes = Shapes::default();
+        craft_pdf::interpret_page(page, &mut context, &mut shapes).unwrap();
+        shapes
+    }
+
+    /// The open zigzag of VectorCraft #2920: its ends and the points beside them land on all four
+    /// corners of its bounding box, and the peak at (60.571, 29.875) is on none.
+    const ZIGZAG: &str = "80 38 m 80 21.75 l 60.571 29.875 l 40 21.75 l 40 38 l";
+
+    #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test::wasm_bindgen_test)]
+    #[cfg_attr(not(target_arch = "wasm32"), test)]
+    fn an_open_five_point_polyline_is_stroked_as_drawn() {
+        let shapes = draw(&format!("0 0 0 RG 2 w {ZIGZAG} S"));
+        assert_eq!(shapes.rects, 0, "the zigzag was drawn as its bounding box");
+        assert_eq!(shapes.paths.len(), 1);
+        assert_eq!(shapes.paths[0].elements().len(), 5, "the five points arrive as written");
+    }
+
+    #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test::wasm_bindgen_test)]
+    #[cfg_attr(not(target_arch = "wasm32"), test)]
+    fn an_open_five_point_polyline_is_filled_as_drawn() {
+        let shapes = draw(&format!("0 0 0 rg {ZIGZAG} f"));
+        assert_eq!(shapes.rects, 0);
+        assert_eq!(shapes.paths.len(), 1);
+    }
+
+    #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test::wasm_bindgen_test)]
+    #[cfg_attr(not(target_arch = "wasm32"), test)]
+    fn a_closed_path_through_the_corners_out_of_order_is_not_a_rectangle() {
+        // A bow tie: every point is a corner of the bounding box, but two sides are diagonals.
+        let shapes = draw("0 0 0 RG 10 10 m 50 40 l 10 40 l 50 10 l h S");
+        assert_eq!(shapes.rects, 0);
+        assert_eq!(shapes.paths.len(), 1);
+    }
+
+    #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test::wasm_bindgen_test)]
+    #[cfg_attr(not(target_arch = "wasm32"), test)]
+    fn rectangles_still_take_the_rectangle_route() {
+        for content in [
+            "0 0 0 rg 10 10 40 30 re f",
+            "0 0 0 RG 10 10 40 30 re S",
+            "0 0 0 RG 10 10 m 50 10 l 50 40 l 10 40 l h S",
+            "0 0 0 rg 10 10 m 10 40 l 50 40 l 50 10 l 10 10 l f",
+        ] {
+            let shapes = draw(content);
+            assert_eq!((shapes.rects, shapes.paths.len()), (1, 0), "{content}");
+        }
+    }
+
+    #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test::wasm_bindgen_test)]
+    #[cfg_attr(not(target_arch = "wasm32"), test)]
+    fn a_polyline_clip_is_kept() {
+        // A clip that is a rectangle covering the page is dropped as a no-op; this one is not a
+        // rectangle, so it has to reach the device even though its bounding box covers the page.
+        let shapes = draw("0 0 m 120 0 l 60 30 l 0 60 l 120 60 l W n 0 0 0 rg 0 0 120 60 re f");
+        assert_eq!(shapes.clips, 1);
+    }
+}

vendor/README.craft-reader.md

@@ -301,6 +301,17 @@ and the digest that tells its profile from a document profile is still a hash of
 decoded profile bytes. PdfCraft's two raster tests are adapted in `tests/upstream_raster.rs`
 (`icc_profile_cache`).
 
+From [PdfCraft #3327](https://github.com/storytold/pdfcraft/pull/3327) (merged as
+b84121e978e13168caeaf86a63824111f16cdb99), hayro-interpret (35), marked `PdfCraft patch (35)` in
+the source: `path_as_rect` takes a five-element path as a rectangle only when its four points are
+distinct, each step from one to the next runs along a single axis, and the last returns to the
+first (with ClosePath, or a fourth line back to the exact start). Before, the length test also
+admitted one MoveTo and four LineTo, and the corner test only asked that every corner of the
+bounding box was visited, so a five-point open polyline whose ends reached the corners was drawn
+as that box (VectorCraft #2920: an open zigzag in an `.ai` file opened as a closed rectangle), and
+a clip of that shape was dropped. Tests: `context::path_as_rect_tests` and the `rectangles`
+module of `crates/pdf/tests/interpretation.rs`, which fails without the change.
+
 Local fixes on top of these ports, marked `Craft patch` or in the comments of the ported
 code: an inflated patch-mesh triangle is rebuilt with `Triangle::new`, so its stored dot
 products match its vertices (moving the points alone skewed barycentric coordinates by about

vendor/hayro-interpret/SOURCE.json

@@ -188,7 +188,7 @@
       "path": "src/context.rs",
       "source_sha256": "dc2be553958ee3f5e5e644a77a0301723bae9ca027a352c13b9caaf549cc0e56",
       "source_git_blob": "d6772baa15ebb95d3ba2d1f54ce5c250ad1e8137",
-      "sha256": "98ed66aab25f1e3b8b9dfd9b6837dc0d9519a9fd7f5429b695865879abc24f6d"
+      "sha256": "221174b2b65824e1489e8d0f61ac5c81b828caf7f5c7053e27c84a6657857f4c"
     },
     {
       "path": "src/convert.rs",

vendor/hayro-interpret/src/context.rs

@@ -521,37 +521,116 @@ pub(crate) fn path_as_rect(path: &BezPath) -> Option<Rect> {
         return None;
     }
 
-    let bbox = path.fast_bounding_box();
-    let (min_x, min_y, max_x, max_y) = (bbox.min_x(), bbox.min_y(), bbox.max_x(), bbox.max_y());
-    let mut corners = [false; 4];
-
-    let mut check_point = |p: Point| {
-        corners[0] |= p.x.is_nearly_equal(min_x) && p.y.is_nearly_equal(min_y);
-        corners[1] |= p.x.is_nearly_equal(min_x) && p.y.is_nearly_equal(max_y);
-        corners[2] |= p.x.is_nearly_equal(max_x) && p.y.is_nearly_equal(min_y);
-        corners[3] |= p.x.is_nearly_equal(max_x) && p.y.is_nearly_equal(max_y);
-    };
-
+    // PdfCraft patch (35), PdfCraft #3327: the points have to trace the rectangle, not merely
+    // reach its corners. The length check above admits one MoveTo and four LineTo as well as the
+    // intended one MoveTo, three LineTo and one ClosePath, and asking only that each corner of
+    // the bounding box was visited says nothing about the order or the steps between: a
+    // five-point polyline whose ends reached every corner of its own bounding box was painted as
+    // that box, and a clip of that shape was discarded. Four distinct points, each step along one
+    // axis, the last point back at the first: that is a rectangle and nothing else is.
+    let mut points: Vec<Point> = Vec::with_capacity(5);
+    let mut closed = false;
     for (idx, el) in path.elements().iter().enumerate() {
         match el {
             PathEl::MoveTo(p) => {
                 if idx != 0 {
                     return None;
                 }
-
-                check_point(*p);
+                points.push(*p);
             }
-            PathEl::LineTo(l) => check_point(*l),
+            PathEl::LineTo(p) => points.push(*p),
             PathEl::QuadTo(_, _) => return None,
             PathEl::CurveTo(_, _, _) => return None,
-            PathEl::ClosePath => {}
+            // A subpath that ends and starts again is more than one shape.
+            PathEl::ClosePath => {
+                if idx + 1 != path.elements().len() {
+                    return None;
+                }
+                closed = true;
+            }
         }
     }
 
-    if corners[0] && corners[1] && corners[2] && corners[3] {
-        Some(bbox)
-    } else {
-        None
+    // Exactly equal, not nearly: `is_nearly_equal` allows a 256th of a path unit, and the matrix
+    // is applied after this, so a large `cm` turns that into whole device units: a path that only
+    // nearly closes would be painted as the rectangle, with a side it does not have. A
+    // rectangle's coordinates are equal in the content stream, so nothing that is one loses the
+    // fast path by this; a path that misses takes the general route and is painted as written.
+    let same = |a: Point, b: Point| a.x == b.x && a.y == b.y;
+    // `m l l l l`: the fourth line goes back to the start, which `m l l l h` does with ClosePath.
+    if !closed {
+        match (points.first(), points.get(4)) {
+            (Some(first), Some(last)) if points.len() == 5 && same(*first, *last) => {}
+            _ => return None,
+        }
+        points.pop();
+    }
+    if points.len() != 4 {
+        return None;
+    }
+    // Distinct, so a degenerate path is not one, and each step along one axis only: the four
+    // points of an axis-aligned rectangle, in order round it.
+    for (i, p) in points.iter().enumerate() {
+        let q = points.get((i + 1) % 4)?;
+        let along_x = p.y == q.y && p.x != q.x;
+        let along_y = p.x == q.x && p.y != q.y;
+        if !(along_x || along_y) {
+            return None;
+        }
+        if points.iter().skip(i + 1).any(|o| same(*p, *o)) {
+            return None;
+        }
+    }
+
+    Some(path.fast_bounding_box())
+}
+
+/// PdfCraft patch (35) tests: only a path that traces an axis-aligned rectangle is one.
+#[cfg(test)]
+mod path_as_rect_tests {
+    use super::*;
+
+    fn path(points: &[(f64, f64)], close: bool) -> BezPath {
+        let mut path = BezPath::new();
+        for (i, p) in points.iter().enumerate() {
+            if i == 0 {
+                path.move_to(*p);
+            } else {
+                path.line_to(*p);
+            }
+        }
+        if close {
+            path.close_path();
+        }
+        path
+    }
+
+    #[test]
+    fn an_open_polyline_through_every_corner_is_not_a_rectangle() {
+        // VectorCraft #2920: the zigzag's ends reach all four corners; its peak reaches none.
+        let zigzag = [(80.0, 38.0), (80.0, 21.75), (60.571, 29.875), (40.0, 21.75), (40.0, 38.0)];
+        assert_eq!(path_as_rect(&path(&zigzag, false)), None);
+    }
+
+    #[test]
+    fn corners_visited_out_of_order_are_not_a_rectangle() {
+        let bow_tie = [(10.0, 10.0), (50.0, 40.0), (10.0, 40.0), (50.0, 10.0)];
+        assert_eq!(path_as_rect(&path(&bow_tie, true)), None);
+    }
+
+    #[test]
+    fn a_path_that_only_nearly_closes_is_not_a_rectangle() {
+        let open = [(10.0, 10.0), (50.0, 10.0), (50.0, 40.0), (10.0, 40.0), (10.0, 10.001)];
+        assert_eq!(path_as_rect(&path(&open, false)), None);
+    }
+
+    #[test]
+    fn closed_rectangles_are_rectangles() {
+        let expected = Some(Rect::new(10.0, 10.0, 50.0, 40.0));
+        let closed = [(10.0, 10.0), (50.0, 10.0), (50.0, 40.0), (10.0, 40.0)];
+        assert_eq!(path_as_rect(&path(&closed, true)), expected);
+        let back_to_start = [(10.0, 10.0), (10.0, 40.0), (50.0, 40.0), (50.0, 10.0), (10.0, 10.0)];
+        assert_eq!(path_as_rect(&path(&back_to_start, false)), expected);
     }
 }