From 4e8e17c7e63336d6d708c936a0f8c22dd76928cf Mon Sep 17 00:00:00 2001 From: Lombardi2003 Date: Wed, 21 Oct 2026 01:13:00 +0200 Subject: [PATCH] fix(edit): a header never takes over a font name the page already draws with MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From review of #3756: add_resources set /PCTimes (or /PCCour, /PCHelv) in the page's fonts whatever was there, so a page that already drew another font by that name would have its own text change face. Headers and footers now pick the usual name when it is free or already names the same standard face, otherwise the first free name_1, name_2…, and add only that face, never replacing an entry. An update finds its own name again. Co-Authored-By: Claude Opus 5.5 --- crates/edit/src/lib.rs | 48 ++++++++++++++++++++++++++++++++++------ crates/edit/src/tests.rs | 39 ++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 7 deletions(-) diff --git a/crates/edit/src/lib.rs b/crates/edit/src/lib.rs index 90d8f24cdd..a37ed4d1e0 100644 --- a/crates/edit/src/lib.rs +++ b/crates/edit/src/lib.rs @@ -308,19 +308,51 @@ fn contents(doc: &mut Document, page: &pdfcraft_model::Page) -> Result, content: Option<&[u8]>) -> Result<(), EditError> { - let mut res = page.dict.get(b"Resources").map(|r| doc.resolve(r)).and_then(|r| r.as_dict().cloned()).unwrap_or_default(); - let mut fonts = res.get(b"Font").map(|f| doc.resolve(f)).and_then(|f| f.as_dict().cloned()).unwrap_or_default(); + let mut faces: Vec<(Vec, &str)> = Vec::new(); for (name, base) in [(&b"PCHelv"[..], "Helvetica"), (b"PCTimes", "Times-Roman"), (b"PCCour", "Courier")] { // Helvetica always (marks use it); the others only when the content names them. if name != b"PCHelv" && !content.is_some_and(|c| c.windows(name.len() + 1).any(|w| w[0] == b'/' && &w[1..] == name)) { continue; } + faces.push((name.to_vec(), base)); + } + add_page_resources(doc, page, opacity, &faces) +} + +/// The fonts a page draws with: its own resources' or the ones it inherits. +fn page_fonts(doc: &Document, page: &pdfcraft_model::Page) -> Dict { + let res = page.dict.get(b"Resources").map(|r| doc.resolve(r)).and_then(|r| r.as_dict().cloned()).unwrap_or_default(); + res.get(b"Font").map(|f| doc.resolve(f)).and_then(|f| f.as_dict().cloned()).unwrap_or_default() +} + +/// A name in `fonts` for the standard face `base` that can't take over a font the page already +/// draws with: `preferred` when it is free or already names that face, otherwise the first such +/// `preferred_1`, `preferred_2`… (one of the first `fonts.len() + 1` always is). +fn free_font_name(doc: &Document, fonts: &Dict, preferred: &str, base: &str) -> String { + let usable = |name: &str| match fonts.get(name.as_bytes()) { + None => true, + Some(font) => doc.resolve(font).as_dict().is_some_and(|d| { + d.name(b"Subtype") == Some(b"Type1") && d.name(b"BaseFont") == Some(base.as_bytes()) && d.name(b"Encoding") == Some(b"WinAnsiEncoding") + }), + }; + if usable(preferred) { + return preferred.to_string(); + } + (1..=fonts.len().saturating_add(1)).map(|k| format!("{preferred}_{k}")).find(|name| usable(name)).unwrap_or_else(|| preferred.to_string()) +} + +/// Add the standard `faces` (resource name, base font) and an opacity state to the page's own +/// resources. +fn add_page_resources(doc: &mut Document, page: &pdfcraft_model::Page, opacity: Option, faces: &[(Vec, &str)]) -> Result<(), EditError> { + let mut res = page.dict.get(b"Resources").map(|r| doc.resolve(r)).and_then(|r| r.as_dict().cloned()).unwrap_or_default(); + let mut fonts = res.get(b"Font").map(|f| doc.resolve(f)).and_then(|f| f.as_dict().cloned()).unwrap_or_default(); + for (name, base) in faces { let mut font = Dict::new(); font.set(b"Type".to_vec(), Object::name("Font")); font.set(b"Subtype".to_vec(), Object::name("Type1")); font.set(b"BaseFont".to_vec(), Object::name(base)); font.set(b"Encoding".to_vec(), Object::name("WinAnsiEncoding")); - fonts.set(name.to_vec(), Object::Dict(font)); + fonts.set(name.clone(), Object::Dict(font)); } res.set(b"Font".to_vec(), Object::Dict(fonts)); if let Some(o) = opacity { @@ -656,12 +688,15 @@ pub fn add_header_footer(doc: &mut Document, pages: &[usize], hf: &HeaderFooter, let bates = bates0.checked_add(k as u64).ok_or_else(|| EditError::Invalid("the Bates numbers are too large".into()))?; let size = hf.font_size; let [top, bottom, left, right] = hf.margins; - // The page resource each face is added under (see `add_resources`). - let resource = match hf.font { + // The page resource the face is added under: the usual name, unless the page already + // draws another font by it, which must not change under the header. + let preferred = match hf.font { added::Family::Helvetica => "PCHelv", added::Family::Times => "PCTimes", added::Family::Courier => "PCCour", }; + let base = hf.font.base_font(false, false); + let resource = free_font_name(doc, &page_fonts(doc, page), preferred, base); let mut body: Vec = format!("BT\n/{resource} {} Tf\n{}\n", n(size), rgb(hf.color)).into_bytes(); let mut underlines = String::new(); for (slot, template) in hf.text.iter().enumerate() { @@ -694,8 +729,7 @@ pub fn add_header_footer(doc: &mut Document, pages: &[usize], hf: &HeaderFooter, let mut content = begin(MarkKind::HeaderFooter, "Header", page.view_matrix(doc)).into_bytes(); content.extend(body); content.extend_from_slice(END.as_bytes()); - // The content names the face, so Times or Courier is added beside Helvetica. - add_resources(doc, page, None, Some(&content))?; + add_page_resources(doc, page, None, &[(resource.into_bytes(), base)])?; let page = &page_list(doc)[i]; place(doc, page, MarkKind::HeaderFooter, content, false)?; } diff --git a/crates/edit/src/tests.rs b/crates/edit/src/tests.rs index 7e16f22ce0..a0f4272ecd 100644 --- a/crates/edit/src/tests.rs +++ b/crates/edit/src/tests.rs @@ -131,6 +131,45 @@ fn header_and_footer_are_drawn_in_the_chosen_standard_face() { assert!(mark.contains("/PCHelv 8 Tf") && !mark.contains("/PCTimes") && !mark.contains("/PCCour"), "{mark}"); } +#[test] +fn a_header_never_takes_over_a_font_name_the_page_already_draws_with() { + // From review of #3756: a page that already draws a different font as /PCTimes keeps it; the + // header's Times goes under a free name, and an update reuses that name, not a third one. + let mut doc = fixture(); + let page = pdfcraft_model::pages(&doc)[0].obj; + let mut fonts = Dict::new(); + let mut theirs = Dict::new(); + theirs.set(b"Type".to_vec(), Object::name("Font")); + theirs.set(b"Subtype".to_vec(), Object::name("Type1")); + theirs.set(b"BaseFont".to_vec(), Object::name("Helvetica-Bold")); + fonts.set(b"PCTimes".to_vec(), Object::Dict(theirs)); + let mut res = Dict::new(); + res.set(b"Font".to_vec(), Object::Dict(fonts)); + doc.update_dict(page, |d| d.set(b"Resources".to_vec(), Object::Dict(res))).unwrap(); + let hf = HeaderFooter { + text: ["Serif".into(), String::new(), String::new(), String::new(), String::new(), String::new()], + font: added::Family::Times, + ..HeaderFooter::default() + }; + let base_of = |doc: &Document, name: &[u8]| { + let fonts = page_fonts(doc, &pdfcraft_model::pages(doc)[0]); + fonts.get(name).map(|f| doc.resolve(f)).and_then(|f| f.as_dict().and_then(|d| d.name(b"BaseFont")).map(<[u8]>::to_vec)) + }; + add_header_footer(&mut doc, &[0], &hf, false, &cx()).unwrap(); + let doc = reopen(&doc); + assert_eq!(base_of(&doc, b"PCTimes").as_deref(), Some(&b"Helvetica-Bold"[..]), "the page's own font is untouched"); + assert_eq!(base_of(&doc, b"PCTimes_1").as_deref(), Some(&b"Times-Roman"[..])); + let mark = streams(&doc, 0).last().cloned().unwrap(); + assert!(mark.contains("/PCTimes_1 8 Tf"), "{mark}"); + // Updating the header finds its own name again. + let mut doc = doc; + add_header_footer(&mut doc, &[0], &hf, true, &cx()).unwrap(); + let doc = reopen(&doc); + assert!(streams(&doc, 0).last().unwrap().contains("/PCTimes_1 8 Tf")); + assert_eq!(base_of(&doc, b"PCTimes_2"), None, "no new name on update"); + assert_eq!(base_of(&doc, b"PCTimes").as_deref(), Some(&b"Helvetica-Bold"[..])); +} + #[test] fn header_start_number_that_would_wrap_is_refused_without_drawing() { // u32::MAX is a valid start_number on its own, but there is no room for a second page