From 3c5dab25b724857e2a37b4c85ed93f7c4cbfa510 Mon Sep 17 00:00:00 2001 From: Michael Date: Thu, 5 Nov 2026 14:50:37 +0800 Subject: [PATCH] fix(edit): a clip is the union of its rectangles, and one of no area hides all (#3503 review) Second review of #5075, five findings, each with a case seen failing on the previous commit: - A path of several rectangles clipped to the box around them, so text in the gap between them was read though nothing of it shows. A clip is now the union of its parts, one convex region per rectangle; clips intersect part by part (a path of more than 16 keeps the box around them, which still holds them all). - A form's /BBox one point by 1e-10 fell under a fixed area threshold and was taken for no box. Whether a region has area is now its area against the square of its perimeter, the same at any scale. - At 1e12 points from the origin the shoelace products lost the area to rounding, and the box was dropped. The area is measured from the first corner. - Clips meeting only along an edge intersected to a region of no area that a run across the edge still showed through; an intersection with no area, or with an empty region on either side, is now empty. - A clip of no area was taken for no clip, and everything under it was read. It has no inside (ISO 32000-2 8.5.4), so it now hides all; only a clip whose corners or measure are not finite is left unread. Co-Authored-By: Claude Opus 5.5 --- crates/edit/src/tests.rs | 30 +++++++++- crates/edit/src/text.rs | 18 +++++- crates/edit/src/text/interp.rs | 103 ++++++++++++++++++++++++++------- crates/edit/src/text/lines.rs | 14 ++--- 4 files changed, 132 insertions(+), 33 deletions(-) diff --git a/crates/edit/src/tests.rs b/crates/edit/src/tests.rs index eb8cf31707..ade8e19528 100644 --- a/crates/edit/src/tests.rs +++ b/crates/edit/src/tests.rs @@ -5709,14 +5709,40 @@ fn a_malformed_form_bbox_clips_nothing() { } /// #5075 review: a clipping path of several rectangles clips to all of them, not to the last one -/// drawn. Text seen through the first rectangle was hidden. +/// drawn (text seen through the first rectangle was hidden), and not to the box around them +/// either (text in the gap between them was read, though nothing of it shows). #[test] fn a_clip_of_several_rectangles_keeps_text_in_each() { let content = "q 0 0 50 50 re 100 100 50 50 re W n BT /F1 12 Tf 5 5 Td (FIRST) Tj ET BT /F1 12 Tf 105 105 Td (SECOND) Tj ET \ - BT /F1 12 Tf 300 300 Td (OUTSIDE) Tj ET Q"; + BT /F1 12 Tf 60 60 Td (GAP) Tj ET BT /F1 12 Tf 300 300 Td (OUTSIDE) Tj ET Q"; assert_eq!(read_clipped(content, [("", ""), ("", "")]), ["FIRST", "SECOND"]); } +/// #5075 review: a form's `/BBox` clips however thin it is and wherever its matrix puts it. A box +/// one point by 1e-10 was taken for no box (its area fell under a fixed threshold), and one moved +/// 1e12 points away lost its area to rounding; either way the text outside it was read. +#[test] +fn a_thin_or_far_off_form_bbox_still_clips() { + let after = "BT /F1 12 Tf 5 300 Td (AFTER) Tj ET"; + let body = "BT /F1 12 Tf 5 5 Td (INSIDE) Tj ET BT /F1 12 Tf 300 300 Td (OUTSIDE) Tj ET"; + let sorted = |mut v: Vec| { + v.sort(); + v + }; + let thin = read_clipped(&format!("/Fa Do {after}"), [("/BBox [0 0 1 1e-10]", body), ("", "")]); + assert_eq!(thin, ["AFTER"], "a thin /BBox"); + let far = read_clipped(&format!("/Fa Do {after}"), [("/Matrix [1 0 0 1 1e12 1e12] /BBox [0 0 100 100]", body), ("", "")]); + assert_eq!(sorted(far), ["AFTER", "INSIDE"], "a /BBox 1e12 points away"); +} + +/// #5075 review: a clip of no area has no inside, so nothing drawn under it shows (ISO 32000-2 +/// §8.5.4). It was taken for no clip at all, and everything under it was read. +#[test] +fn a_clip_of_no_area_shows_nothing() { + let content = "q 0 0 100 0 re W n BT /F1 12 Tf 5 5 Td (HIDDEN) Tj ET Q BT /F1 12 Tf 5 300 Td (AFTER) Tj ET"; + assert_eq!(read_clipped(content, [("", ""), ("", "")]), ["AFTER"]); +} + /// #5075 review: what shows is the intersection of every clip in force. A long run that crosses a /// page clip and, apart from it, a form's `/BBox` overlaps each of them but shows in neither, as it /// does not when one form's `/BBox` lies apart from the one of the form it is drawn in. diff --git a/crates/edit/src/text.rs b/crates/edit/src/text.rs index e18459862d..7c8d844660 100644 --- a/crates/edit/src/text.rs +++ b/crates/edit/src/text.rs @@ -165,6 +165,13 @@ mod tests { // Squares apart share nothing, so nothing shows, not even a box across both. let none = intersect(&square(0.0, 0.0), &square(20.0, 0.0)); assert!(!shows([-100.0, -100.0, 100.0, 100.0], &none), "{none:?}"); + // Squares that meet only along an edge share no area either: a box across that edge + // does not show (#5075 review). + let edge = intersect(&square(0.0, 0.0), &square(10.0, 0.0)); + assert!(!shows([8.0, 2.0, 12.0, 4.0], &edge), "{edge:?}"); + // Nothing intersected with anything is nothing, whichever side it is on. + assert!(!shows([1.0, 1.0, 2.0, 2.0], &intersect(&square(0.0, 0.0), &Vec::new()))); + assert!(!shows([1.0, 1.0, 2.0, 2.0], &intersect(&Vec::new(), &square(0.0, 0.0)))); // A box touching an edge shares no area; one a hair inside does. assert!(!shows([10.0, 0.0, 12.0, 5.0], &square(0.0, 0.0))); assert!(shows([9.99, 0.0, 12.0, 5.0], &square(0.0, 0.0))); @@ -176,9 +183,16 @@ mod tests { // A corner repeated (an edge of no length) does not hide what the region covers. let doubled = vec![[0.0, 0.0], [10.0, 0.0], [10.0, 0.0], [10.0, 10.0], [0.0, 10.0]]; assert!(shows([4.0, 4.0, 6.0, 6.0], &doubled)); - // No area, or a corner that isn't finite: no region to clip with. - assert!(region(&Matrix::IDENTITY, [0.0, 0.0, 10.0, 0.0]).is_none()); + // A rectangle of no area is a region that covers nothing; a corner that isn't finite + // gives no region to clip with at all. + let flat = region(&Matrix::IDENTITY, [0.0, 0.0, 10.0, 0.0]).unwrap(); + assert!(!shows([-100.0, -100.0, 100.0, 100.0], &flat), "{flat:?}"); assert!(region(&Matrix::IDENTITY, [0.0, 0.0, f64::INFINITY, 10.0]).is_none()); + // However thin, a rectangle with area keeps it, and so does one far from the origin. + let thin = region(&Matrix::IDENTITY, [0.0, 0.0, 1.0, 1e-10]).unwrap(); + assert!(shows([0.25, 0.0, 0.75, 1.0], &thin) && !shows([0.25, 1.0, 0.75, 2.0], &thin)); + let far = region(&Matrix([1.0, 0.0, 0.0, 1.0, 1e12, 1e12]), [0.0, 0.0, 100.0, 100.0]).unwrap(); + assert!(shows([1e12 + 10.0, 1e12 + 10.0, 1e12 + 20.0, 1e12 + 20.0], &far) && !shows([1e12 + 200.0, 1e12, 1e12 + 300.0, 1e12 + 10.0], &far)); } /// A stream that sets clip after clip, each turned a little more, keeps its region bounded: diff --git a/crates/edit/src/text/interp.rs b/crates/edit/src/text/interp.rs index a5a1d8fcde..cd7edbcc01 100644 --- a/crates/edit/src/text/interp.rs +++ b/crates/edit/src/text/interp.rs @@ -222,13 +222,22 @@ pub(crate) fn union(a: [f64; 4], b: [f64; 4]) -> [f64; 4] { /// than three corners it covers nothing. pub(super) type Region = Vec<[f64; 2]>; +/// A clip, as the union of the convex regions it leaves visible: one for each rectangle of its +/// path. With no parts it leaves nothing visible. +pub(super) type Clip = Vec; + /// The most corners a clip region keeps. Each intersection with a four-cornered clip can add /// corners, so past this a region becomes its bounding box: still all it held, and a stream that /// sets clip after clip stays bounded. const MAX_REGION_CORNERS: usize = 32; +/// The most parts a clip keeps. Clips intersect part by part, so past this its parts become one, +/// the box around them: still all they held, and the work stays bounded. +const MAX_CLIP_PARTS: usize = 16; + /// `r` (`[x0, y0, x1, y1]`, either way round) under `m`, keeping its shape when `m` turns or -/// skews it; `None` when it has no area to clip with or a corner is not finite. +/// skews it. A rectangle of no area is a region that covers nothing, however thin a rectangle +/// with area is; `None` when a corner, or its measure, is not finite. pub(super) fn region(m: &Matrix, r: [f64; 4]) -> Option { let corners: Region = [(r[0], r[1]), (r[2], r[1]), (r[2], r[3]), (r[0], r[3])] .into_iter() @@ -237,18 +246,41 @@ pub(super) fn region(m: &Matrix, r: [f64; 4]) -> Option { [x, y] }) .collect(); - (corners.iter().flatten().all(|v| v.is_finite()) && twice_area(&corners).abs() > 1e-9).then_some(corners) + let finite = corners.iter().flatten().all(|v| v.is_finite()) && twice_area(&corners).is_finite() && perimeter(&corners).is_finite(); + finite.then(|| if encloses_area(&corners) { corners } else { Vec::new() }) } /// The polygon's signed area, doubled: positive when its corners run anticlockwise (y up). +/// Measured from its first corner, so that far from the origin it keeps its precision. fn twice_area(p: &[[f64; 2]]) -> f64 { - p.iter().zip(p.iter().cycle().skip(1)).map(|(a, b)| a[0] * b[1] - b[0] * a[1]).sum() + let Some(o) = p.first() else { return 0.0 }; + p.iter().zip(p.iter().cycle().skip(1)).map(|(a, b)| (a[0] - o[0]) * (b[1] - o[1]) - (b[0] - o[0]) * (a[1] - o[1])).sum() +} + +fn perimeter(p: &[[f64; 2]]) -> f64 { + p.iter().zip(p.iter().cycle().skip(1)).map(|(a, b)| (b[0] - a[0]).hypot(b[1] - a[1])).sum() +} + +/// Whether the polygon encloses any area: its area against the square of its perimeter, so the +/// answer is the same at any scale and a thin box still has one. +fn encloses_area(p: &[[f64; 2]]) -> bool { + let around = perimeter(p); + p.len() >= 3 && twice_area(p).abs() > 1e-12 * around * around +} + +/// The four corners of the box around `points`. +fn bounding<'a>(points: impl Iterator) -> Region { + let b = points.fold([f64::MAX, f64::MAX, f64::MIN, f64::MIN], |b, p| [b[0].min(p[0]), b[1].min(p[1]), b[2].max(p[0]), b[3].max(p[1])]); + vec![[b[0], b[1]], [b[2], b[1]], [b[2], b[3]], [b[0], b[3]]] } /// The part of `a` inside `b`, both convex (Sutherland-Hodgman: `a` cut by each edge of `b`). Clips -/// in force intersect, so text shows only where all of them let it through; regions that share -/// no area leave an empty one. +/// in force intersect, so text shows only where all of them let it through. Regions that share no +/// area (apart, or meeting only along an edge) leave an empty one, as does an empty `a` or `b`. pub(super) fn intersect(a: &Region, b: &Region) -> Region { + if a.len() < 3 || b.len() < 3 { + return Vec::new(); + } let turn = twice_area(b).signum(); let mut out = a.clone(); for (p, q) in b.iter().zip(b.iter().cycle().skip(1)) { @@ -267,13 +299,30 @@ pub(super) fn intersect(a: &Region, b: &Region) -> Region { } } } + if !encloses_area(&out) { + return Vec::new(); + } if out.len() > MAX_REGION_CORNERS { - let b = out.iter().fold([f64::MAX, f64::MAX, f64::MIN, f64::MIN], |b, p| [b[0].min(p[0]), b[1].min(p[1]), b[2].max(p[0]), b[3].max(p[1])]); - out = vec![[b[0], b[1]], [b[2], b[1]], [b[2], b[3]], [b[0], b[3]]]; + out = bounding(out.iter()); } out } +/// The clip that leaves visible only what both `a` and `b` do: each part of one intersected with +/// each part of the other. +pub(super) fn clip_within(a: &Clip, b: &Clip) -> Clip { + let mut out: Clip = a.iter().flat_map(|p| b.iter().map(move |q| intersect(p, q))).filter(|r| !r.is_empty()).collect(); + if out.len() > MAX_CLIP_PARTS { + out = vec![bounding(out.iter().flatten())]; + } + out +} + +/// Whether the box `r` shows through `clip`: through any of its parts. +pub(super) fn clip_shows(r: [f64; 4], clip: &Clip) -> bool { + clip.iter().any(|part| shows(r, part)) +} + /// Whether the box `r` (`[x0, y0, x1, y1]`) and `region` share some area, by the separating axis /// test: two convex shapes are apart exactly when their shadows on some axis, one of the box's or /// a normal of one of the region's edges, do not overlap. Touching is not sharing. @@ -297,11 +346,11 @@ pub(super) fn shows(r: [f64; 4], region: &Region) -> bool { #[derive(Clone)] pub(super) struct Ts { pub(super) ctm: Matrix, - /// The clip in force, if any, as the region it leaves visible: the intersection of every + /// The clip in force, if any, as what it leaves visible: the intersection of every /// rectangular content-stream clip (`re` `W`/`W*` `n`, ISO 32000-2 §8.5.4) and every enclosing /// form's `/BBox` ([`super::lines`]), each in the shape its matrix gives it. `q`/`Q` save and /// restore it along with the rest of `Ts`. - pub(super) clip: Option, + pub(super) clip: Option, /// The operators that set the current fill colour (`g`, `rg`, `k`, or `cs` + `sc`/`scn`). pub(super) fill: Vec, pub(super) font: Option<(Vec, Rc)>, @@ -926,29 +975,39 @@ pub(super) fn interpret( let mut soft_gap = 1usize; // A pending path of `x y w h re`s, and whether a `W`/`W*` has marked it as the path that `n` // turns into the new clip (ISO 32000-2 §8.5.4) -- the common "set a rectangular clip" idiom. - // The path is `Some(None)` once one of its rectangles can't be read. Several rectangles clip - // to the box around them all, which holds all of them, so no text they show is lost. Any - // other operator between `re` and `n` ends the idiom: the clip is not read then. - let mut pending_clip: Option> = None; + // The path is `Some(None)` once one of its rectangles can't be read, and then clips nothing. + // Each rectangle is a part of the clip; a path of more than `MAX_CLIP_PARTS` keeps the box + // around them, which still holds them all. Any other operator between `re` and `n` ends the + // idiom: the clip is not read then. + let mut pending_clip: Option>> = None; let mut clip_pending = false; for (i, op) in ops.iter().enumerate() { match op.op.as_slice() { b"re" => { let rect = op.nums::<4>().map(|[x, y, w, h]| [x.min(x + w), y.min(y + h), x.max(x + w), y.max(y + h)]); - pending_clip = Some(match (pending_clip, rect) { - (None, rect) => rect, - (Some(Some(path)), Some(rect)) => Some(union(path, rect)), - (Some(_), _) => None, + pending_clip = Some(match (pending_clip.take(), rect) { + (None, Some(rect)) => Some(vec![rect]), + (Some(Some(mut path)), Some(rect)) => { + if path.len() >= MAX_CLIP_PARTS { + path = vec![path.iter().fold(rect, |all, r| union(all, *r))]; + } else { + path.push(rect); + } + Some(path) + } + _ => None, }); } b"W" | b"W*" => clip_pending = pending_clip.is_some(), b"n" => { - if clip_pending && let Some(Some(r)) = pending_clip { - // A clip of no area (or past what a matrix can hold) is not read: it leaves - // the text as it was rather than hiding all of it. - if let Some(new) = region(&ts.ctm, r) { + if clip_pending && let Some(Some(path)) = pending_clip.take() { + // A rectangle past what a matrix can hold leaves the clip unread. One of no + // area is a part that shows nothing, so a clip of no area hides everything. + let parts: Option = path.iter().map(|r| region(&ts.ctm, *r)).collect(); + if let Some(parts) = parts { + let new: Clip = parts.into_iter().filter(|part| !part.is_empty()).collect(); ts.clip = Some(match &ts.clip { - Some(old) => intersect(old, &new), + Some(old) => clip_within(old, &new), None => new, }); } diff --git a/crates/edit/src/text/lines.rs b/crates/edit/src/text/lines.rs index 2b725fa761..5aafd3303d 100644 --- a/crates/edit/src/text/lines.rs +++ b/crates/edit/src/text/lines.rs @@ -1,7 +1,7 @@ use std::sync::Arc; use super::fallback::{same_font, same_paint}; -use super::interp::{Carry, Origin, Pen, Region, Shown, TextState, Ts, direction, interpret, intersect, region, shows, to_frame, union}; +use super::interp::{Carry, Clip, Origin, Pen, Shown, TextState, Ts, clip_shows, clip_within, direction, interpret, region, to_frame, union}; use super::{DecodeBudget, Dict, Document, EditError, HashMap, Matrix, Object, Op, Pieces, Stream, TextLine, parse, serialize_ops}; impl TextLine { @@ -450,8 +450,8 @@ impl FormBudget { } /// A form XObject a page draws: its object, content, resources, its matrix composed with the -/// CTM at the `Do`, and the region its `/BBox` leaves visible (page space, under that same matrix, -/// so a turned form keeps its turned box) if it has a valid one. +/// CTM at the `Do`, and the clip its `/BBox` makes (page space, under that same matrix, so a +/// turned form keeps its turned box; a box of no area hides everything) if it has a valid one. pub(crate) struct FormCall { pub(crate) obj: Option, pub(crate) data: Vec, @@ -460,7 +460,7 @@ pub(crate) struct FormCall { form: Arc, own: Option>, pub(crate) ctm: Matrix, - bbox: Option, + bbox: Option, } impl FormCall { @@ -546,7 +546,7 @@ pub(crate) fn form_call( }; budget.visits = budget.visits.saturating_add(1); let ctm = m.map_or(ctm, |m| Matrix(m).then(&ctm)); - Some(FormCall { obj, data, form, own, ctm, bbox: bbox.and_then(|b| region(&ctm, b)) }) + Some(FormCall { obj, data, form, own, ctm, bbox: bbox.and_then(|b| region(&ctm, b)).map(|r| if r.is_empty() { Vec::new() } else { vec![r] }) }) } /// The lines a page shows as it reads, including text drawn by form XObjects (which @@ -579,7 +579,7 @@ pub(super) fn reading_lines(doc: &Document, page: usize) -> Result<(Vec Result<(Vec Some(intersect(outer, own)), + (Some(outer), Some(own)) => Some(clip_within(outer, own)), (outer, own) => outer.clone().or_else(|| own.clone()), }; let mut carry =