diff --git a/ROADMAP.md b/ROADMAP.md index b1283e775..ee1dc5823 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -172,6 +172,8 @@ Ranked; detail and the full milestone table in [docs/roadmap.md](docs/roadmap.md ## Progress log +- **2026-10-19 (M7, #3591):** A background added to a page that already has one is painted over the old one instead of going under it and staying hidden. Behind the content, backgrounds come first and then behind-content watermarks, each in the order added, so a watermark added after a background no longer disappears. Synthetic regressions in `crates/edit` check the order. Estimates unchanged. + - **2026-10-19 (M14, #3373):** The main menu and its long submenus scroll in short windows, keeping their commands reachable. Estimates unchanged. - **2026-10-19 (M3, #3387):** Save-prompt shortcuts yield to later consent and utility dialogs; a synthetic UI regression checks that an edited document stays open. Estimates unchanged. diff --git a/crates/edit/src/lib.rs b/crates/edit/src/lib.rs index c1062b410..0bd0ab569 100644 --- a/crates/edit/src/lib.rs +++ b/crates/edit/src/lib.rs @@ -505,8 +505,8 @@ fn isolation(doc: &Document, list: &[Object]) -> Isolation { Isolation::Clean { suffix: (dirty || comment_open || marked != 0).then_some(split), wrappers, marked } } -/// Put a mark's content on a page: behind (prepended) or on top (appended, after wrapping the -/// original content in q/Q). +/// Put a mark's content on a page: behind (with the behind-marks at the start) or on top +/// (appended, after wrapping the original content in q/Q). fn place(doc: &mut Document, page: &pdfcraft_model::Page, kind: MarkKind, content: Vec, behind: bool) -> Result<(), EditError> { place_tagged(doc, page, kind.tag(), content, behind) } @@ -518,6 +518,9 @@ fn place(doc: &mut Document, page: &pdfcraft_model::Page, kind: MarkKind, conten /// sequences are closed at the end too, so hidden optional content cannot hide the new mark. fn place_tagged(doc: &mut Document, page: &pdfcraft_model::Page, tag: &str, content: Vec, behind: bool) -> Result<(), EditError> { let mut list = contents(doc, page)?; + // The behind-marks at the start: backgrounds, then watermarks, each in the order added. + let behind_kinds = [MarkKind::Background.tag(), MarkKind::Watermark.tag()]; + let behind_marks = list.iter().take_while(|o| stream_tag(doc, o).is_some_and(|t| behind_kinds.contains(&t.as_str()))).count(); let wrap = if behind { None } else { @@ -526,16 +529,19 @@ fn place_tagged(doc: &mut Document, page: &pdfcraft_model::Page, tag: &str, cont // Content that can't be read precisely is wrapped whole, as before: after the // behind-marks at the start. A `q` it leaves open is closed at its end, where // closing it changes nothing already drawn. - Isolation::Unknown { unclosed, marked } => { - let behind_kinds = [MarkKind::Background.tag(), MarkKind::Watermark.tag()]; - let split = list.iter().position(|o| !stream_tag(doc, o).is_some_and(|t| behind_kinds.contains(&t.as_str()))).unwrap_or(list.len()); - (split < list.len()).then_some((split, unclosed, marked)) - } + Isolation::Unknown { unclosed, marked } => (behind_marks < list.len()).then_some((behind_marks, unclosed, marked)), } }; let mark = Object::Ref(doc.add(Object::Stream(Stream::flate(tagged(tag), &content)))); if behind { - list.insert(0, mark); + // A new background goes after the backgrounds already there and below any watermark; + // a new watermark after all of them. Each is painted over the older ones (#3591). + let at = if tag == MarkKind::Background.tag() { + list.iter().take(behind_marks).rposition(|o| stream_tag(doc, o).as_deref() == Some(tag)).map_or(0, |i| i + 1) + } else { + behind_marks + }; + list.insert(at, mark); } else { if let Some((split, unclosed, marked)) = wrap { let mut close_bytes = b"\nEMC".repeat(marked); diff --git a/crates/edit/src/tests.rs b/crates/edit/src/tests.rs index 879ebc6c6..bd506d3b9 100644 --- a/crates/edit/src/tests.rs +++ b/crates/edit/src/tests.rs @@ -168,6 +168,33 @@ fn watermarks_rotate_fade_and_can_go_behind() { assert_eq!(s.len(), 2, "no wrapper needed for content behind"); } +#[test] +fn a_second_background_is_drawn_over_the_first() { + // #3591: the new background went in front of the old one in /Contents, so the old colour + // was painted over it and stayed. + let mut doc = fixture(); + add_background(&mut doc, &[0], &Background { color: [0.0, 0.0, 1.0], opacity: 1.0, ..Background::default() }, false).unwrap(); + add_background(&mut doc, &[0], &Background { color: [0.0, 1.0, 0.0], opacity: 1.0, ..Background::default() }, false).unwrap(); + let s = streams(&doc, 0); + assert_eq!(s.len(), 3, "{s:?}"); + assert!(s[0].contains("0 0 1 rg") && s[1].contains("0 1 0 rg"), "the newer background is painted last: {s:?}"); + assert!(s[2].contains("(Hi) Tj"), "both stay behind the content: {s:?}"); +} + +#[test] +fn backgrounds_stay_below_watermarks_behind_the_content() { + // Behind the content, backgrounds come first and then watermarks, each in the order added, + // so a watermark added after a background shows and a later background doesn't cover it. + let mut doc = fixture(); + add_background(&mut doc, &[0], &Background { color: [0.0, 0.0, 1.0], opacity: 1.0, ..Background::default() }, false).unwrap(); + add_watermark(&mut doc, &[0], &Watermark { text: "DRAFT".into(), behind: true, ..Watermark::default() }, false).unwrap(); + add_background(&mut doc, &[0], &Background { color: [0.0, 1.0, 0.0], opacity: 1.0, ..Background::default() }, false).unwrap(); + let s = streams(&doc, 0); + assert_eq!(s.len(), 4, "{s:?}"); + assert!(s[0].contains("0 0 1 rg") && s[1].contains("0 1 0 rg"), "backgrounds first, oldest first: {s:?}"); + assert!(s[2].contains("/PCMark /Watermark") && s[3].contains("(Hi) Tj"), "then the watermark, then the content: {s:?}"); +} + #[test] fn watermark_offset_moves_the_centre_right_and_up() { let mut doc = fixture();