A rectangle is a traversal, not four corners visited

FredApps's note on #3327: five points that are all on the bounding box's
corners still need not trace the rectangle. Three sides and a step back
along the third --

    10 10 m 30 10 l 30 30 l 10 30 l 30 30 l S

-- visits every corner, so it was taken as the box and stroked with a
left side the path does not have.

path_as_rect now asks what a rectangle actually is: four distinct
points, each step from one to the next along a single axis, and the last
point back at the first (written as the fourth LineTo or as ClosePath).
That is a rectangle and nothing else is, so the corner test and its
off_corner flag are gone -- four points of an axis-aligned traversal are
necessarily the corners of their own bounding box. A degenerate path,
whose points are not distinct, now takes the general route as well,
where it painted a zero-width or zero-height rectangle before.

The vendored-patch table has the row this change was missing: without
it a re-vendor of hayro-interpret would drop the patch silently, which
is the other half of FredApps's review.

crates/render/src/raster.rs

@@ -4324,6 +4324,30 @@ trailer << /Root 1 0 R >>
         assert!(!(14..=26).any(|x| dark(x, 10)), "the side the path leaves open stays open");
     }
 
+    /// Five points all on the corners still need not trace the rectangle: three sides and a step
+    /// back along the third is such a path, and it was painted as the box -- drawing a fourth
+    /// side the path does not have (FredApps on #3327).
+    #[test]
+    fn a_path_that_visits_every_corner_out_of_order_is_not_a_rectangle() {
+        // Bottom, right, top, then back along the top. The left side is never drawn.
+        let content = b"0 0 0 RG 2 w 10 10 m 30 10 l 30 30 l 10 30 l 30 30 l S".to_vec();
+        let pdf = one_page_pdf(&content, "");
+        let (tx, rx) = std::sync::mpsc::channel();
+        std::thread::spawn(move || {
+            let mut r = PageRenderer::new(Arc::new(pdf), RenderConfig::default());
+            let _ = tx.send(r.render(RenderRequest { page: 0, kind: RequestKind::Pixels, tile: None, scale: 1.0, tag: 0, preview: false }));
+        });
+        let page = rx.recv_timeout(std::time::Duration::from_secs(60)).expect("the path must render");
+        assert!(page.error.is_none(), "{:?}", page.error);
+        let dark = |x: u32, y: u32| {
+            let offset = ((y * page.width + x) * 4) as usize;
+            page.rgba[offset] < 128
+        };
+        // Device y counts down the 40 pt page: the bottom side at y = 10 is row 30.
+        assert!((14..=26).any(|x| dark(x, 30)), "the sides the path does draw are drawn");
+        assert!(!(14..=26).any(|y| dark(10, y)), "the left side it never draws is not drawn");
+    }
+
     /// A rectangle still takes the rectangle route, written either way: `m l l l` and closed, or
     /// `m l l l l` back to its first corner.
     #[test]

vendor/hayro-interpret/src/context.rs

@@ -447,50 +447,60 @@ 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];
-
-    // PdfCraft patch: a point that is not a corner means this is not a rectangle. The length
-    // check above admits one MoveTo and four LineTo as well as the intended one MoveTo, three
-    // LineTo and one ClosePath, and covering the four corners says nothing about the points
-    // between them, so a five-point polyline whose ends reach every corner of its own bounding
-    // box was painted as that box (and a clip of that shape was discarded).
-    let mut off_corner = false;
-
-    let mut check_point = |p: Point| {
-        let hits = [
-            p.x.is_nearly_equal(min_x) && p.y.is_nearly_equal(min_y),
-            p.x.is_nearly_equal(min_x) && p.y.is_nearly_equal(max_y),
-            p.x.is_nearly_equal(max_x) && p.y.is_nearly_equal(min_y),
-            p.x.is_nearly_equal(max_x) && p.y.is_nearly_equal(max_y),
-        ];
-        off_corner |= !hits.iter().any(|hit| *hit);
-
-        for (corner, hit) in corners.iter_mut().zip(hits) {
-            *corner |= hit;
-        }
-    };
-
+    // PdfCraft patch: 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 !off_corner && corners[0] && corners[1] && corners[2] && corners[3] {
-        Some(bbox)
-    } else {
-        None
+    let same = |a: Point, b: Point| a.x.is_nearly_equal(b.x) && a.y.is_nearly_equal(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 {
+        if points.len() != 5 || !same(points[4], points[0]) {
+            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[(i + 1) % 4];
+        let along_x = p.y.is_nearly_equal(q.y) && !p.x.is_nearly_equal(q.x);
+        let along_y = p.x.is_nearly_equal(q.x) && !p.y.is_nearly_equal(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())
 }