From a747f2521ffeeadca3ca331957a073584ad3ab55 Mon Sep 17 00:00:00 2001 From: cyfung1031 <44498510+cyfung1031@users.noreply.github.com> Date: Fri, 9 Oct 2026 07:17:34 +0900 Subject: [PATCH] M2.6: W/W* clip after whichever path-painting operator ends the path (#362) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ISO 32000-2 §8.5.4: W and W* mark the current path to become the clip once the path-painting operator that ends it has painted it with the old clip, and n is only one such operator. The vendored interpreter applied the clip in n alone. After S, s, f, F, f*, B, B*, b or b* it was never applied, so everything painted afterwards ignored it, and it stayed pending until some later, unrelated n (even after Q) clipped content that should show. Issue #260's test file uses `W* S` on pages 3, 5 and 9: a shading meant for a circle flooded page 3, an image meant for a rounded rectangle and nested clips on 5 and 9 spilled out. Every painting operator now applies a pending clip after painting and clears it (apply_pending_clip, shared with n). The now unused Context::clip getter is removed. Test: raster::tests::clipping_path_applies_after_any_painting_operator. W and W* with each of n, S, s, f, F, f*, B, B*, b, b*: the later red fill stays inside the square, and a stroking operator's outer half still shows outside it (painted under the old clip). A clip used by S doesn't linger for a later n, inside or after q/Q. On main it fails at `W S`. MuPDF, Poppler and PDFium agree on every case. Page 3 of #260's file now differs from PDFium by 0.84/255 on average (was 164). Vendored patch (16). Co-authored-by: Claude Opus 5.5 Co-authored-by: Brandon Thomas --- crates/render/src/raster.rs | 51 ++++++++++++++++++++ vendor/README.md | 2 +- vendor/hayro-interpret/src/context.rs | 4 -- vendor/hayro-interpret/src/interpret/mod.rs | 12 +---- vendor/hayro-interpret/src/interpret/path.rs | 15 ++++++ 5 files changed, 69 insertions(+), 15 deletions(-) diff --git a/crates/render/src/raster.rs b/crates/render/src/raster.rs index 5cedcf8a1c..9f311e7626 100644 --- a/crates/render/src/raster.rs +++ b/crates/render/src/raster.rs @@ -1482,4 +1482,55 @@ trailer << /Root 1 0 R >> assert_eq!(px(25, 50), vec![255, 0, 0, 255], "inside the mask"); assert_eq!(px(75, 50), vec![255, 255, 255, 255], "outside the mask"); } + + /// ISO 32000-2 §8.5.4: `W` / `W*` clip with the current path once whichever path-painting + /// operator ends it has painted it, `S`, `f` or `B` as much as `n`. The vendored interpreter + /// applied the clip only on `n`: after `re W* S` everything later painted outside the clip, + /// and the forgotten clip stayed pending, so a later `re n` (even after `Q`) clipped + /// content that should show. MuPDF, Poppler and PDFium clip in every case below. + #[test] + fn clipping_path_applies_after_any_painting_operator() { + let render = |content: &str| { + let pdf = format!( + "%PDF-1.7 +1 0 obj << /Type /Catalog /Pages 2 0 R >> endobj +2 0 obj << /Type /Pages /Kids [3 0 R] /Count 1 >> endobj +3 0 obj << /Type /Page /Parent 2 0 R /MediaBox [0 0 100 100] /Contents 4 0 R >> endobj +4 0 obj << /Length {} >> stream +{content} +endstream endobj +trailer << /Root 1 0 R >> +%%EOF", + content.len() + ); + let mut r = PageRenderer::new(Arc::new(pdf.into_bytes()), RenderConfig::default()); + let p = r.render(RenderRequest { page: 0, kind: RequestKind::Pixels, tile: None, scale: 1.0, tag: 0 }); + assert!(p.error.is_none(), "{content}: {:?}", p.error); + p + }; + let px = |p: &RenderedPage, x: u32, y: u32| p.rgba[((y * p.width + x) * 4) as usize..][..4].to_vec(); + let red = vec![255, 0, 0, 255]; + // The clip is the square x 20..60, y 20..60 (device rows 40..80); then the page is filled red. + for op in ["n", "S", "s", "f", "F", "f*", "B", "B*", "b", "b*"] { + for clip in ["W", "W*"] { + let p = render(&format!("q 0 0 1 RG 4 w 0.8 g 20 20 40 40 re {clip} {op} 1 0 0 rg 0 0 100 100 re f Q")); + assert_eq!(px(&p, 40, 60), red, "{clip} {op}: inside the clip"); + assert_ne!(px(&p, 5, 5), red, "{clip} {op}: outside the clip"); + assert_ne!(px(&p, 95, 95), red, "{clip} {op}: outside the clip"); + // The operator that ends the path paints under the old clip: the outer half of + // the 4 pt stroke (x 18..20) shows outside the new one. + let stroked = !["n", "f", "F", "f*"].contains(&op); + let edge = if stroked { vec![0, 0, 255, 255] } else { vec![255, 255, 255, 255] }; + assert_eq!(px(&p, 18, 60), edge, "{clip} {op}: just outside the clip"); + } + } + // A clip is used once: it doesn't linger for a later `n`, inside or outside `q`/`Q`. + for content in [ + "q 0 0 1 RG 70 70 10 10 re W S Q q 10 10 20 20 re n 1 0 0 rg 0 0 100 100 re f Q", + "q 0 0 1 RG 0 0 100 100 re W S 10 10 20 20 re n 1 0 0 rg 0 0 100 100 re f Q", + ] { + let p = render(content); + assert!([(5, 5), (50, 50), (95, 95)].iter().all(|(x, y)| px(&p, *x, *y) == red), "{content}: no stray clip"); + } + } } diff --git a/vendor/README.md b/vendor/README.md index 862f574487..9225714ef5 100644 --- a/vendor/README.md +++ b/vendor/README.md @@ -7,7 +7,7 @@ the fix. Vendoring copyleft code is never allowed (plan/adr/0001). | Crate | Version | Licence | Patches | Test | |---|---|---|---|---| -| hayro-interpret | 0.7.0 | Apache-2.0 OR MIT | (1) select `/AP /N` appearance states via `/AS` (checkboxes, radios, stateful icons) and honour the NoView flag; (2) Type3 Unicode fallback to `/Encoding` glyph names, then printable ASCII; (3) `Type3Glyph::advance_width`; (4) `InterpreterSettings::ocg_overrides` for viewer layer toggles; (5) soft masks whose group lacks `/CS` were dropped (Chrome gradient text); (6) paint nesting cap `MAX_PAINT_NESTING` for tiling patterns and Type 3 glyphs (self-referencing content overflowed the stack); (7) CID width ranges in `/W` and `/W2` clamped to `MAX_CID` (a `0 4294967295 w` range hung); (8) `InterpreterSettings::hide_comments` skips markup annotations but keeps widgets and links (View ▸ Hide all comments); (9) `image_size_ok`: images and masks over 2^28 pixels are not decoded (a fuzzed `/W 4294967295` stencil mask allocated 4 GiB); (10) `MAX_NESTED_PAINTS`: nested paints (forms, Type 3 glyphs and tiling patterns inside one another) share a per-page budget of 50,000 (a glyph showing eight glyphs of its own font, sixteen deep, hung); (11) depends on skrifa 0.47 instead of 0.42 (upstream read-fonts fixed a Type 1 tokenizer that recursed through runs of numbers: a fuzzed font hung or overflowed the stack), with the one API change (`Charset::glyph_id` returns `Option`); (12) annotation appearances whose `/BBox` or `/Matrix` collapses to a line or a point are skipped (mapping them onto `/Rect` divided by zero and handed the device infinite or NaN matrices); (13) a Highlight annotation without a usable normal appearance is drawn from its `/QuadPoints`, `/C` and `/CA` with Multiply blending, as pdfcraft-annot draws highlights (it was not drawn at all); (14) simple-font `/Widths` (`read_widths`): entries only for codes 0–255 (one entry per code below `/FirstChar` was allocated: `4294967295` asked for 34 GB), an entry that isn't a number leaves its code missing instead of ending the array, and a `/LastChar` below `/FirstChar` gives no widths instead of discarding the font for a standard one; (15) the appearance box is mapped onto `/Rect` corner to corner: the translation now accounts for the scale (an appearance whose box didn't start at the origin and differed in size from `/Rect` was drawn shifted) | (1) `raster::tests::widget_appearance_states`; (2, 3) `xtask text-oracle` on corpus/pdfjs issue918; (4) `raster::tests::layer_override_hides_content`; (5) `raster::tests::soft_mask_without_group_colour_space_is_applied`; (6) `raster::tests::self_referencing_tiling_pattern_terminates`, `raster::tests::self_referencing_type3_glyph_terminates`; (7) `raster::tests::huge_cid_width_ranges_terminate`; (8) `raster::tests::hiding_comments_keeps_fields`; (9) `raster::tests::absurd_mask_and_fax_sizes_are_refused_before_decoding`; (10) `raster::tests::type3_glyphs_that_fan_out_into_themselves_finish`; (11) `raster::tests::type1_font_programs_with_long_runs_of_numbers_load_quickly`; (12) `raster::tests::degenerate_annotation_appearances_are_skipped`; (13) `raster::tests::highlights_without_an_appearance_are_drawn`; (14) `raster::tests::malformed_simple_font_widths_keep_later_widths_and_the_font`, `raster::tests::huge_simple_font_first_char_terminates`; (15) `raster::tests::annotation_appearance_box_maps_onto_rect` | +| hayro-interpret | 0.7.0 | Apache-2.0 OR MIT | (1) select `/AP /N` appearance states via `/AS` (checkboxes, radios, stateful icons) and honour the NoView flag; (2) Type3 Unicode fallback to `/Encoding` glyph names, then printable ASCII; (3) `Type3Glyph::advance_width`; (4) `InterpreterSettings::ocg_overrides` for viewer layer toggles; (5) soft masks whose group lacks `/CS` were dropped (Chrome gradient text); (6) paint nesting cap `MAX_PAINT_NESTING` for tiling patterns and Type 3 glyphs (self-referencing content overflowed the stack); (7) CID width ranges in `/W` and `/W2` clamped to `MAX_CID` (a `0 4294967295 w` range hung); (8) `InterpreterSettings::hide_comments` skips markup annotations but keeps widgets and links (View ▸ Hide all comments); (9) `image_size_ok`: images and masks over 2^28 pixels are not decoded (a fuzzed `/W 4294967295` stencil mask allocated 4 GiB); (10) `MAX_NESTED_PAINTS`: nested paints (forms, Type 3 glyphs and tiling patterns inside one another) share a per-page budget of 50,000 (a glyph showing eight glyphs of its own font, sixteen deep, hung); (11) depends on skrifa 0.47 instead of 0.42 (upstream read-fonts fixed a Type 1 tokenizer that recursed through runs of numbers: a fuzzed font hung or overflowed the stack), with the one API change (`Charset::glyph_id` returns `Option`); (12) annotation appearances whose `/BBox` or `/Matrix` collapses to a line or a point are skipped (mapping them onto `/Rect` divided by zero and handed the device infinite or NaN matrices); (13) a Highlight annotation without a usable normal appearance is drawn from its `/QuadPoints`, `/C` and `/CA` with Multiply blending, as pdfcraft-annot draws highlights (it was not drawn at all); (14) simple-font `/Widths` (`read_widths`): entries only for codes 0–255 (one entry per code below `/FirstChar` was allocated: `4294967295` asked for 34 GB), an entry that isn't a number leaves its code missing instead of ending the array, and a `/LastChar` below `/FirstChar` gives no widths instead of discarding the font for a standard one; (15) the appearance box is mapped onto `/Rect` corner to corner: the translation now accounts for the scale (an appearance whose box didn't start at the origin and differed in size from `/Rect` was drawn shifted); (16) a clip set by `W`/`W*` takes effect after whichever path-painting operator ends the path (`S`, `f`, `B`, … as well as `n`; ISO 32000-2 §8.5.4): it was applied only on `n`, so `re W* S` clipped nothing and the clip stayed pending for a later, unrelated `n` | (1) `raster::tests::widget_appearance_states`; (2, 3) `xtask text-oracle` on corpus/pdfjs issue918; (4) `raster::tests::layer_override_hides_content`; (5) `raster::tests::soft_mask_without_group_colour_space_is_applied`; (6) `raster::tests::self_referencing_tiling_pattern_terminates`, `raster::tests::self_referencing_type3_glyph_terminates`; (7) `raster::tests::huge_cid_width_ranges_terminate`; (8) `raster::tests::hiding_comments_keeps_fields`; (9) `raster::tests::absurd_mask_and_fax_sizes_are_refused_before_decoding`; (10) `raster::tests::type3_glyphs_that_fan_out_into_themselves_finish`; (11) `raster::tests::type1_font_programs_with_long_runs_of_numbers_load_quickly`; (12) `raster::tests::degenerate_annotation_appearances_are_skipped`; (13) `raster::tests::highlights_without_an_appearance_are_drawn`; (14) `raster::tests::malformed_simple_font_widths_keep_later_widths_and_the_font`, `raster::tests::huge_simple_font_first_char_terminates`; (15) `raster::tests::annotation_appearance_box_maps_onto_rect`; (16) `raster::tests::clipping_path_applies_after_any_painting_operator` | | hayro | 0.7.1 | Apache-2.0 OR MIT | (1) `RenderSettings::{x_offset, y_offset}` to render one tile of a large page; (2) images over `MAX_IMAGE_PIXELS` (2^28) are skipped (a fuzzed `/W 4294967295` hung in resampling); (3) `tiling_cell_scale` keeps a tiling pattern's cell pixmap within 3000 px a side whatever its `/XStep`/`/YStep` (a fuzzed step far beyond `/BBox` allocated 2 GB); (4) stroke widths clamped to 4 × (canvas width + height) device pixels (a fuzzed `/LW 9223372036854775807` allocated 10 GB in stroke expansion) | (1) `raster::tests::tiles_match_full_render`; (2) `raster::tests::absurd_image_dimensions_are_skipped`; (3) `raster::tests::tiling_cells_stay_small_whatever_the_step`; (4) `raster::tests::absurd_predictor_columns_and_line_widths_render` | | hayro-jbig2 | 0.3.0 | Apache-2.0 OR MIT | (1) at most 2^28 pixels per bitmap, page bitmap, halftone grid and grayscale image (each side could be 65535: 4.3 billion pixels decoded one by one); (2) at most 2^26 symbol pixels per symbol dictionary (a fuzzed dictionary decoded for minutes) | (1) `raster::tests::jbig2_regions_of_billions_of_pixels_are_refused`; (2) the nightly `cargo xtask fuzz` finding, which now finishes in about a second | diff --git a/vendor/hayro-interpret/src/context.rs b/vendor/hayro-interpret/src/context.rs index 5a2f2ab8ce..c5cea5cfe5 100644 --- a/vendor/hayro-interpret/src/context.rs +++ b/vendor/hayro-interpret/src/context.rs @@ -274,10 +274,6 @@ impl<'a> Context<'a> { &mut self.last_point } - pub(crate) fn clip(&self) -> &Option { - &self.clip - } - pub(crate) fn clip_mut(&mut self) -> &mut Option { &mut self.clip } diff --git a/vendor/hayro-interpret/src/interpret/mod.rs b/vendor/hayro-interpret/src/interpret/mod.rs index 5220120616..a96154119d 100644 --- a/vendor/hayro-interpret/src/interpret/mod.rs +++ b/vendor/hayro-interpret/src/interpret/mod.rs @@ -5,7 +5,7 @@ use crate::convert::{convert_line_cap, convert_line_join}; use crate::device::Device; use crate::font::{Font, FontData, FontQuery, StandardFont}; use crate::interpret::path::{ - close_path, fill_path, fill_path_impl, fill_stroke_path, stroke_path, + apply_pending_clip, close_path, fill_path, fill_path_impl, fill_stroke_path, stroke_path, }; use crate::interpret::state::{TextStateFont, handle_gs}; use crate::interpret::text::TextRenderingMode; @@ -479,15 +479,7 @@ pub fn interpret<'a>( stroke_path(context, device); } TypedInstruction::EndPath(_) => { - if let Some(clip) = *context.clip() - && !context.path().elements().is_empty() - { - let clip_path = context.get().ctm * context.path().clone(); - context.push_clip_path(clip_path, clip, device); - - *(context.clip_mut()) = None; - } - + apply_pending_clip(context, device); context.path_mut().truncate(0); } TypedInstruction::NonStrokeColor(c) => { diff --git a/vendor/hayro-interpret/src/interpret/path.rs b/vendor/hayro-interpret/src/interpret/path.rs index e1e7d96a5d..838ea099f1 100644 --- a/vendor/hayro-interpret/src/interpret/path.rs +++ b/vendor/hayro-interpret/src/interpret/path.rs @@ -12,12 +12,14 @@ pub(crate) fn fill_path<'a>( fill_rule: FillRule, ) { fill_path_impl(context, device, fill_rule, None); + apply_pending_clip(context, device); context.path_mut().truncate(0); } pub(crate) fn stroke_path<'a>(context: &mut Context<'a>, device: &mut impl Device<'a>) { stroke_path_impl(context, device, None); + apply_pending_clip(context, device); context.path_mut().truncate(0); } @@ -29,10 +31,23 @@ pub(crate) fn fill_stroke_path<'a>( ) { fill_path_impl(context, device, fill_rule, None); stroke_path_impl(context, device, None); + apply_pending_clip(context, device); context.path_mut().truncate(0); } +/// PdfCraft patch: a clip set by `W` / `W*` takes effect once the path-painting operator that +/// ends the path has painted it, whichever operator that is (ISO 32000-2 §8.5.4). Upstream +/// applied it only on `n`, and otherwise left it pending for some later, unrelated `n`. +pub(crate) fn apply_pending_clip<'a>(context: &mut Context<'a>, device: &mut impl Device<'a>) { + if let Some(rule) = context.clip_mut().take() + && !context.path().elements().is_empty() + { + let clip_path = context.get().ctm * context.path().clone(); + context.push_clip_path(clip_path, rule, device); + } +} + pub(crate) fn close_path(context: &mut Context<'_>) { // This is necessary to prevent artifacts (see for example issue 157), // but it does cause some weird lines (maybe conflation artifacts) in