fix(edit): backdrop copies keep the stream dictionary and every slot

Review of #2815 (CodeRabbit):

- A content stream whose bytes live in an external file (`/F`) draws
  those, not its inline bytes, so it is never taken for a backdrop.
- The copy keeps the original stream dictionary, unknown entries
  included, less `Length` and `DL` (Flate re-encodes it); it had started
  from an empty dictionary.
- Every `/Contents` slot that lists the original stream gets the copy: a
  page listing the same stream twice painted the white fill again over
  the background.
- The Background dialog starts with replacing on each time it opens.
  Update restores backdrops before re-applying, so a replaced page shows
  no fill and no checkbox, and a choice left off from an earlier dialog
  hid the background again unseen.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
alehelandClaude Opus 5.5 committed 2026-10-18 16:24:14 +02:00
1 parent 911147126c
commit eca16e6bd5
6 files changed
+102 -11

No files matched your search

+23 -6
View File
@@ -80,7 +80,7 @@ pub(crate) fn replace(doc: &mut Document, page: &pdfcraft_model::Page) -> Result
let Some((Backdrop::Fill, Some(found))) = scan(doc, page) else { return Ok(false) };
let edited = found.pieces.splice([(found.op.clone(), b"n".to_vec())]);
let piece = found.pieces.piece_of(found.op.start);
let (Some(&(slot, original)), Some(bytes)) = (found.refs.get(piece), edited.get(piece)) else { return Ok(false) };
let (Some(&(_, original)), Some(bytes)) = (found.refs.get(piece), edited.get(piece)) else { return Ok(false) };
// Where the operator sits in that piece: the joined offset less the pieces before it.
let before: usize = (0..piece).filter_map(|i| edited.get(i)).map(|p| p.len() + 1).sum();
let at = found.op.start.saturating_sub(before);
@@ -89,14 +89,27 @@ pub(crate) fn replace(doc: &mut Document, page: &pdfcraft_model::Page) -> Result
note.set(b"Original".to_vec(), Object::Ref(original));
note.set(b"At".to_vec(), Object::Int(i64::try_from(at).unwrap_or(i64::MAX)));
note.set(b"Len".to_vec(), Object::Int(i64::try_from(len).unwrap_or(i64::MAX)));
let mut dict = Dict::new();
// The copy keeps the original's dictionary (unknown entries included) less what describes
// the old encoding; `scan` refused streams whose bytes live in an external file (`/F`).
let Object::Stream(source) = &*doc.get(original) else { return Ok(false) };
let mut dict = source.dict.clone();
for key in [&b"Length"[..], b"DL", b"PCBackdrop"] {
dict.remove(key);
}
dict.set(b"PCBackdrop".to_vec(), Object::Dict(note));
let copy = doc.add(Object::Stream(Stream::flate(dict, bytes)));
// Every slot that draws the original: a page may list the same stream twice, and a second
// copy of the fill would cover the background again.
let mut list = crate::contents(doc, page)?;
let Some(entry) = list.get_mut(slot) else { return Ok(false) };
*entry = Object::Ref(copy);
doc.update_dict(page.obj, |d| d.set(b"Contents".to_vec(), Object::Array(list)))?;
Ok(true)
let mut replaced = false;
for entry in list.iter_mut().filter(|o| matches!(o, Object::Ref(r) if *r == original)) {
*entry = Object::Ref(copy);
replaced = true;
}
if replaced {
doc.update_dict(page.obj, |d| d.set(b"Contents".to_vec(), Object::Array(list)))?;
}
Ok(replaced)
}
/// Put back the original content streams [`replace`] copied on `page`, where the copy is still
@@ -158,6 +171,10 @@ fn scan(doc: &Document, p: &pdfcraft_model::Page) -> Option<(Backdrop, Option<Fo
if s.dict.get(b"PCMark").is_some() {
continue;
}
// Bytes in an external file (`/F`) replace the stream's own: what it draws is unknown here.
if s.dict.get(b"F").is_some() {
return None;
}
let bytes = s.decoded().unwrap_or_default();
total = total.saturating_add(bytes.len()).saturating_add(1);
refs.push((slot, *r));
+52
View File
@@ -299,3 +299,55 @@ fn a_background_over_many_pages_replaces_each_backdrop_and_removing_restores_the
assert_eq!(refs(&after), refs(&before), "every page points at its original stream again");
assert_eq!(page_backdrops(&doc, &all), [Some(Backdrop::Fill), None, Some(Backdrop::Fill), None]);
}
#[test]
fn a_content_stream_whose_bytes_live_in_an_external_file_is_left_alone() {
let objs = vec![
"<< /Type /Catalog /Pages 2 0 R >>".to_string(),
"<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 300 200] >>".into(),
"<< /Type /Page /Parent 2 0 R /Contents [4 0 R] >>".into(),
stream("/F (content.bin)", &format!("1 g 0 0 300 200 re f {TEXT}")),
];
let mut doc = build(&objs);
assert_eq!(page_backdrop(&doc, 0).unwrap(), None, "the inline bytes are not what the page draws");
let before = streams(&doc);
add_background(&mut doc, &[0], &red(), false).unwrap();
assert_eq!(streams(&doc)[1..], before[..]);
}
#[test]
fn the_backdrop_copy_keeps_the_original_dictionary_less_its_old_encoding() {
let objs = vec![
"<< /Type /Catalog /Pages 2 0 R >>".to_string(),
"<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 300 200] >>".into(),
"<< /Type /Page /Parent 2 0 R /Contents [4 0 R] >>".into(),
stream("/Vendor (keep me) /DL 9999", &format!("1 g 0 0 300 200 re f {TEXT}")),
];
let mut doc = build(&objs);
add_background(&mut doc, &[0], &red(), false).unwrap();
let doc = reopen(&doc);
let p = &pdfcraft_model::pages(&doc)[0];
let Some(Object::Array(list)) = p.dict.get(b"Contents").map(|c| (*doc.resolve(c)).clone()) else { panic!("contents") };
let Object::Stream(copy) = &*doc.resolve(&list[1]) else { panic!("a stream") };
assert!(copy.dict.get(b"Vendor").is_some(), "unknown entries survive");
assert!(copy.dict.get(b"DL").is_none(), "the old decoded length no longer holds");
assert!(copy.dict.get(b"PCBackdrop").is_some());
assert_eq!(String::from_utf8_lossy(&copy.decoded().unwrap()), format!("1 g 0 0 300 200 re n {TEXT}"));
}
#[test]
fn a_stream_listed_twice_is_replaced_in_both_slots_and_restored_in_both() {
let mut doc = build(&[
"<< /Type /Catalog /Pages 2 0 R >>".to_string(),
"<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 300 200] >>".into(),
"<< /Type /Page /Parent 2 0 R /Contents [4 0 R 4 0 R] >>".into(),
stream("", &format!("q 1 g 0 0 300 200 re f Q {TEXT}")),
]);
let original = streams(&doc);
add_background(&mut doc, &[0], &red(), false).unwrap();
let s = streams(&doc);
assert_eq!(s.len(), 3, "{s:?}");
assert!(s[1..].iter().all(|c| c.contains("re n") && !c.contains("re f")), "no slot still paints the white fill: {s:?}");
remove_marks(&mut doc, &[0], MarkKind::Background).unwrap();
assert_eq!(streams(&doc), original);
}
+1
View File
@@ -371,6 +371,7 @@ impl PdfCraftApp {
K::Background
};
self.marks_draft.replace = id.ends_with(".update");
self.marks_draft.opened(kind);
self.dialog = Some(Dialog::Marks(kind));
}
"edit.bates" => {
+3
View File
@@ -2557,6 +2557,9 @@ impl PdfCraftApp {
}
"none" => None,
_ => Some(Dialog::About),
};
if let Some(Dialog::Marks(kind)) = self.dialog {
self.marks_draft.opened(kind);
}
}
("tools", _) => self.all_tools_expanded = value != "collapsed",
+12
View File
@@ -136,6 +136,18 @@ impl BackdropCheck {
}
}
impl MarksDraft {
/// The dialog for `kind` is opening. Other settings carry over from last time, but replacing
/// a page's own backdrop starts on: Update restores backdrops before it re-applies, so a
/// page whose backdrop was replaced shows none and the checkbox stays hidden, and a choice
/// left off from an earlier dialog would hide the background again unseen.
pub fn opened(&mut self, kind: MarkKind) {
if kind == MarkKind::Background {
self.bg.replace_backdrop = true;
}
}
}
/// "1 page" or "{n} pages", in a message.
fn pages_phrase(n: usize, one: &str, many: &str) -> String {
if n == 1 { one.to_string() } else { crate::i18n::fmt(many, &[("n", &n.to_string())]) }
+11 -5
View File
@@ -148,18 +148,24 @@ fn the_background_dialog_offers_to_replace_a_white_backdrop_and_warns_about_scan
assert_eq!(backdrop(&h, 0), Some(pdfcraft_engine::Backdrop::Fill));
assert!(h.state_mut().execute("edit.undo"));
h.run_steps(2);
// Ticked, it is replaced; the scan is reported, not touched.
// Reopened, the choice starts on again; ticked, the backdrop is replaced and the scan is
// reported, not touched.
h.state_mut().execute("edit.background");
h.run_steps(3);
h.get_by_label(replace).click();
h.run_steps(2);
assert!(h.state().marks_draft.bg.replace_backdrop);
h.get_by_label(replace);
assert!(h.state().marks_draft.bg.replace_backdrop, "on again when the dialog opens");
h.get_by_label("OK").click();
h.run_steps(3);
assert_eq!(backdrop(&h, 0), None);
assert_eq!(backdrop(&h, 1), Some(pdfcraft_engine::Backdrop::Image));
// Nothing left to replace: the offer is gone.
// Nothing left to replace: the offer is gone. A choice left off from an earlier dialog
// doesn't carry into Update, which restores the backdrop first and would otherwise keep it
// with no checkbox shown.
h.state_mut().marks_draft.bg.replace_backdrop = false;
h.state_mut().execute("edit.background.update");
h.run_steps(3);
assert!(h.query_by_label(replace).is_none());
h.get_by_label("OK").click();
h.run_steps(3);
assert_eq!(backdrop(&h, 0), None, "Update keeps the backdrop replaced");
}