Keep annotations readable under reading colours (#1548) (#1557)

* fix(render): keep annotations readable under reading colours (#1548)

Reading colours remapped annotation paints as line art: the yellow
highlight fill is luminance-light, so it was mapped toward the dark
background, and Multiply then darkened the remapped-white words under
it into invisibility.

Two changes in hayro:

- The /Annots loop in interpret_page is bracketed by a new
  begin_annotations/end_annotations Device signal (patch 26), and the
  renderer's reading-colours mapping leaves annotation paints alone.
  Reading colours replace document colours, never the colours a
  comment was given — the Acrobat model.
- The page background is painted inside the page transparency group
  (patch 27), as the initial backdrop, so a Multiply highlight blends
  with the background too. With the background outside the group the
  band kept its own flat colour and the words vanished into it. White
  pages are unaffected: multiplying with white is the identity.

Regression test reading_colors_keep_highlight_annotations_readable
covers both annotation paths (/AP and the no-/AP /QuadPoints
fallback) under WHITE_ON_BLACK; parity entry a11y.replace-page-colors
updated with the behaviour and evidence.

Co-Authored-By: GLM <noreply@z.ai>

* contributors: credit @edurbs as Eduardo Soares

The contributor's own entry, chosen by them (real and display name).

Co-Authored-By: GLM <noreply@z.ai>

* #1557: record the vendored hayro patches in vendor/README.md

Main already uses hayro-interpret patches (26)-(28), so the annotation hooks become
hayro-interpret (29) and the renderer's parts hayro (17) and (18).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: GLM <noreply@z.ai>
Co-authored-by: Brandon Thomas <echelon@gmail.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

contributors/people.toml

@@ -25,6 +25,11 @@ real_name = "Brandon Thomas"
 display_name = "Brandon Thomas"
 sync_github_name = false
 
+[people.edurbs]
+real_name = "Eduardo Soares"
+display_name = "Eduardo Soares"
+sync_github_name = false
+
 [people.jcl-bit]
 real_name = "James Locke"
 display_name = "James Locke"

crates/render/src/raster.rs

@@ -5617,6 +5617,56 @@ trailer << /Root 1 0 R >>
         assert_eq!(pixel(&on, 150, 55), &[255, 255, 255, 255], "the image's white quadrant stays white");
     }
 
+    /// Reading colours replace *document* colours, never annotation colours (issue #1548): a
+    /// highlight keeps its `/C`, so on a dark background the content under it shows through the
+    /// Multiply blend in the highlight's colour instead of being darkened into invisibility.
+    /// Both annotation paths are covered: the `/AP` appearance stream and the no-`/AP`
+    /// `/QuadPoints` fallback.
+    #[test]
+    fn reading_colors_keep_highlight_annotations_readable() {
+        let content = "0 g 100 40 10 20 re f";
+        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 200 100] /Contents 4 0 R /Annots [5 0 R] >> endobj
+4 0 obj << /Length {} >> stream
+{content}
+endstream endobj
+5 0 obj << /Type /Annot /Subtype /Highlight /Rect [15 35 120 65] /QuadPoints [15 65 120 65 15 35 120 35] /C [1 1 0] {{annot}} >> endobj
+6 0 obj << /Type /XObject /Subtype /Form /BBox [15 35 120 65] /Length 24 >> stream
+0 0 1 rg 15 35 50 30 re f
+endstream endobj
+trailer << /Root 1 0 R >>
+%%EOF",
+            content.len()
+        );
+        let render = |annot: &str| {
+            let mut r = PageRenderer::new(
+                Arc::new(pdf.replace("{annot}", annot).into_bytes()),
+                RenderConfig { reading_colors: ReadingColors::WHITE_ON_BLACK, ..Default::default() },
+            );
+            let p = r.render(RenderRequest { page: 0, kind: RequestKind::Pixels, tile: None, scale: 1.0, tag: 0, ..Default::default() });
+            assert!(p.error.is_none(), "{annot}: {:?}", p.error);
+            p
+        };
+        let px = |p: &RenderedPage, x: u32, y: u32| p.rgba[((y * p.width + x) * 4) as usize..][..4].to_vec();
+
+        // No /AP: the page's black rectangle is remapped to white (document line art), the
+        // yellow fill is not, so Multiply leaves the content under the quad yellow — readable.
+        let p = render("");
+        assert_eq!(px(&p, 5, 5), vec![0, 0, 0, 255], "the page clears to the background");
+        assert_eq!(px(&p, 105, 50), vec![255, 255, 0, 255], "content under the quad shows through in the highlight's colour");
+        assert_eq!(px(&p, 30, 50), vec![0, 0, 0, 255], "the band itself multiplies into the dark background, as in Acrobat");
+        assert_eq!(px(&p, 150, 50), vec![0, 0, 0, 255], "outside the quad");
+
+        // With /AP the appearance's blue is an annotation colour too and stays blue instead of
+        // being remapped towards white by its luminance.
+        let p = render("/AP << /N 6 0 R >>");
+        assert_eq!(px(&p, 30, 50), vec![0, 0, 255, 255], "the appearance keeps its own colour");
+        assert_eq!(px(&p, 90, 50), vec![0, 0, 0, 255], "outside the appearance's own drawing");
+    }
+
     /// A print-preview request (`PRINT_TAG`) renders the document's true colours even though
     /// the document's config carries reading colours: a preview's job is fidelity to the
     /// printout, which is imposed from the uncoloured document. Covers both render paths —

parity/acrobat-features.toml

@@ -9119,8 +9119,8 @@ milestone = "M12"
 status = "shipped"
 commands = []
 tools = ["view_reading_colors"]
-evidence = ["crates/automation/tests/automation.rs::reading_colors_through_tools", "crates/render/src/raster.rs::reading_colors_map_solid_paints_and_leave_images", "crates/render/src/raster.rs::reading_colors_tiles_match_full_render", "crates/render/src/raster.rs::print_preview_requests_render_with_reading_colors_off", "crates/ui-egui/tests/reading_colors.rs::reading_colours_preference_round_trips", "crates/ui-egui/tests/reading_colors.rs::reading_colours_seed_on_open"]
-notes = "View only, off by default: Preferences ▸ Reading colours holds the app-wide preference, seeded into every document as it opens (including created ones), and view_reading_colors reads and sets it per document (Acrobat's Replace Document Colors). Solid text paints and, by default, line-art paints (including stencil-mask image paints) are remapped by luminance — each lerps between the preset's text and background colour by how dark or light the original paint is — while images, shadings and tiling patterns pass through untouched. Presets: black on white, white on black, yellow on black, green on black, or a custom #RRGGBB pair; only_black limits the mapping to near-black paints (a 10% luminance threshold — our choice; Acrobat's was not observed). Display only: the document is not changed and print and export are unaffected; print previews and view snapshots render with the document's true colours (a preview matches the uncoloured printout, and a snapshot is an export surface), so reading colours apply to the reading view only. Closes #897 (duplicate #488)."
+evidence = ["crates/automation/tests/automation.rs::reading_colors_through_tools", "crates/render/src/raster.rs::reading_colors_map_solid_paints_and_leave_images", "crates/render/src/raster.rs::reading_colors_tiles_match_full_render", "crates/render/src/raster.rs::print_preview_requests_render_with_reading_colors_off", "crates/render/src/raster.rs::reading_colors_keep_highlight_annotations_readable", "crates/ui-egui/tests/reading_colors.rs::reading_colours_preference_round_trips", "crates/ui-egui/tests/reading_colors.rs::reading_colours_seed_on_open"]
+notes = "View only, off by default: Preferences ▸ Reading colours holds the app-wide preference, seeded into every document as it opens (including created ones), and view_reading_colors reads and sets it per document (Acrobat's Replace Document Colors). Solid text paints and, by default, line-art paints (including stencil-mask image paints) are remapped by luminance — each lerps between the preset's text and background colour by how dark or light the original paint is — while images, shadings and tiling patterns pass through untouched. Annotations keep the colours they were given (both /AP appearances and the no-/AP Highlight fallback), so a Multiply highlight over a dark background shows the marked-up words in the highlight's colour instead of a flat bar that hides them (#1548). Presets: black on white, white on black, yellow on black, green on black, or a custom #RRGGBB pair; only_black limits the mapping to near-black paints (a 10% luminance threshold — our choice; Acrobat's was not observed). Display only: the document is not changed and print and export are unaffected; print previews and view snapshots render with the document's true colours (a preview matches the uncoloured printout, and a snapshot is an export surface), so reading colours apply to the reading view only. Closes #897 (duplicate #488)."
 
 [[feature]]
 id = "a11y.reflow-tagged"

vendor/hayro-interpret/src/device.rs

@@ -66,6 +66,13 @@ pub trait Device<'a> {
     fn actual_text(&mut self, _text: &[u8]) {}
     /// Called at the end of a marked content sequence (EMC).
     fn end_marked_content(&mut self) {}
+    /// PdfCraft patch (29): the page's annotations are about to be drawn (until
+    /// `end_annotations`), as page content finishes. Devices can use this to treat their
+    /// paints as annotation colours rather than document colours — the reading-colours
+    /// setting replaces document colours only, never the colours a comment was given.
+    fn begin_annotations(&mut self) {}
+    /// PdfCraft patch (29): the page's annotations are done; what follows is page content.
+    fn end_annotations(&mut self) {}
 }
 
 /// A device that discards all drawing operations.

vendor/hayro-interpret/src/interpret/mod.rs

@@ -175,6 +175,9 @@ pub fn interpret_page<'a>(
     if context.settings.render_annotations
         && let Some(annot_arr) = page.raw().get::<Array<'_>>(ANNOTS)
     {
+        // PdfCraft patch (29): bracket the annotations so devices can tell their paints from
+        // the document's own (the reading-colours setting recolours the document, not comments).
+        device.begin_annotations();
         for annot in annot_arr.iter::<Dict<'_>>() {
             let flags = annot.get::<u32>(F).unwrap_or(0);
 
@@ -275,6 +278,7 @@ pub fn interpret_page<'a>(
                 draw_highlight_without_appearance(&annot, context, device);
             }
         }
+        device.end_annotations();
     }
 }
 

vendor/hayro/src/lib.rs

@@ -254,6 +254,16 @@ pub fn render_into<'a>(
         interpreter_settings.preview,
     );
 
+    // PdfCraft patch (18): the page renders as one transparency group with the background
+    // painted *inside* it, as the initial backdrop. A group isolates what it wraps from what
+    // was painted before it, so with the background outside, a Multiply highlight blended
+    // only with the group's transparent backdrop and stayed its own colour — over a dark
+    // reading-colours background the whole quad became one flat bar and the words under it
+    // vanished. Inside the group the highlight multiplies with the background too: the words
+    // keep the highlight's colour and the band between them takes the background (white
+    // backgrounds are unaffected — multiplying with white is the identity).
+    device.push_transparency_group(1.0, None, BlendMode::Normal);
+
     device.ctx.set_paint(render_settings.bg_color);
     device
         .ctx
@@ -265,12 +275,10 @@ pub fn render_into<'a>(
         fill: FillRule::NonZero,
     });
 
-    device.push_transparency_group(1.0, None, BlendMode::Normal);
     interpret_page(page, &mut state, &mut device);
 
-    device.pop_transparency_group();
-
     device.pop_clip_path();
+    device.pop_transparency_group();
 
     let mut resources = vello_cpu::Resources::default();
     device.ctx.render_to_buffer(

vendor/hayro/src/renderer.rs

@@ -432,6 +432,9 @@ pub(crate) struct Renderer {
     /// PdfCraft patch: when set, solid text and line-art paint colours are remapped
     /// while rendering (the "reading colours" view setting).
     color_replacement: Option<ColorReplacement>,
+    /// PdfCraft patch (17): while the page's annotations are being drawn, paints keep their
+    /// colours — reading colours replace document colours, never annotation colours.
+    in_annotations: bool,
     pub(crate) inside_pattern: bool,
     /// Soft masks drawn during this render. PdfCraft note: it belongs to one `Renderer` (one
     /// render, every nested renderer starting empty) and so to one `preview` setting: a mask
@@ -466,6 +469,7 @@ impl Renderer {
         Self {
             ctx: RenderContext::new_with(width, height, settings),
             color_replacement,
+            in_annotations: false,
             inside_pattern: false,
             soft_mask_cache: HashMap::default(),
             outline_cache: cache.outline_cache.clone(),
@@ -573,6 +577,7 @@ impl Renderer {
                 // PdfCraft patch: this renders an image's alpha mask, which must keep its
                 // original values, so it is never remapped by the reading-colours setting.
                 color_replacement: None,
+                in_annotations: false,
                 inside_pattern: false,
                 soft_mask_cache: HashMap::default(),
                 outline_cache: self.outline_cache.clone(),
@@ -1023,6 +1028,11 @@ impl Renderer {
         let Some(replacement) = self.color_replacement else {
             return paint.clone();
         };
+        // PdfCraft patch (17): annotations keep the colours they were given; reading colours
+        // replace document colours only (issue #1548).
+        if self.in_annotations {
+            return paint.clone();
+        }
         match paint {
             Paint::Color(c) => {
                 let mapped = replacement.map(c.to_rgba().to_rgba8(), is_text);
@@ -1154,6 +1164,7 @@ impl Renderer {
                             // PdfCraft patch: a pattern cell is painted as one artwork, so its
                             // content is never remapped by the reading-colours setting.
                             color_replacement: None,
+                            in_annotations: false,
                             cur_mask: None,
                             inside_pattern: true,
                             soft_mask_cache: HashMap::default(),
@@ -1469,6 +1480,7 @@ impl<'a> Device<'a> for Renderer {
                                         // coverage values, so its content is never remapped by
                                         // the reading-colours setting.
                                         color_replacement: None,
+                                        in_annotations: false,
                                         inside_pattern: false,
                                         soft_mask_cache: HashMap::default(),
                                         outline_cache: self.outline_cache.clone(),
@@ -1613,6 +1625,16 @@ impl<'a> Device<'a> for Renderer {
         }
     }
 
+    /// PdfCraft patch (17): the reading-colours mapping is suspended for annotations.
+    fn begin_annotations(&mut self) {
+        self.in_annotations = true;
+    }
+
+    /// PdfCraft patch (17): back to document content; reading colours apply again.
+    fn end_annotations(&mut self) {
+        self.in_annotations = false;
+    }
+
     fn draw_rect(
         &mut self,
         rect: &Rect,
@@ -1754,6 +1776,7 @@ fn draw_soft_mask(mask: &SoftMask<'_>, settings: RenderSettings, width: u16, hei
         // PdfCraft patch: a soft mask must keep its original luminance values, so its
         // content is never remapped by the reading-colours setting.
         color_replacement: None,
+        in_annotations: false,
         inside_pattern: false,
         cur_mask: None,
         soft_mask_cache: HashMap::default(),