diff --git a/ROADMAP.md b/ROADMAP.md index 40fabb8f3a..f19c158c42 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -41,6 +41,7 @@ > **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-11 · **Change:** minor (architectural paper controls from #599; localized shortcut labels, #496; readiness table with hours per audience; full number as weighted sum 49%; earlier: second look: ready 40% → 47%, mainstream 55% added; core-workflow gate applied → alpha) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) > **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-11 · **Change:** minor (closed issues synced: #774, #756 and the #799–#821 batch; earlier: localized shortcut labels, #496; readiness table with hours per audience; full number as weighted sum 49%; earlier: second look: ready 40% → 47%, mainstream 55% added; core-workflow gate applied → alpha) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) > **Last reviewed:** 2026-10-18 - **Last updated:** 2026-10-18 - **Change:** minor (M6 #2892: Push-button normal labels are editable per widget through Options and form_set_props, independently of field names; rendering, undo/redo and incremental/full save/reopen are covered. Estimates unchanged) +> **Last reviewed:** 2026-10-18 - **Last updated:** 2026-10-18 - **Change:** minor (M7 #3187: Added-image automation normalizes extreme signed rotation angles before combining them with the current orientation; integer bounds, history, pixels, save/reopen and MCP have regressions. Estimates unchanged) PdfCraft's one-page summary: how close it is to Adobe Acrobat Pro's offline feature set, and what comes next. Adobe's cloud services (Document Cloud, Acrobat Sign requests, shared review, the AI @@ -176,6 +177,7 @@ Ranked; detail and the full milestone table in [docs/roadmap.md](docs/roadmap.md - **2026-10-20 (M6, calculation order page):** Prepare a form ▸ Calculation order, a page of its own, lists the calculated fields in the order they calculate with their formulas and values, moved with Up/Down arrows (as Acrobat's Set Field Calculation Order), Alt+↑/↓ or by dragging. A card says what is wrong with the order and holds Fix order; a loop has a card of its own that opens one of its formulas. On the page, each calculated field shows its step, and arrows show what the field in focus reads and what reads it. Estimates unchanged. - **2026-10-20 (M6, calculation order):** The order a form's fields calculate in (`/CO`) is read, checked, fixed and set headlessly with `form_calculation_order`: it says which fields read one calculated later (they show stale values), which read each other in a loop, and which the PDF's order leaves out; Fix order moves only what must move, in one undo step, and the form recalculates. Editing a formula no longer moves its field to the end of the order, and a field the order lists twice calculates once. Estimates unchanged. - **2026-10-19 (M6, renames keep formulas):** Renaming a field renames it in every calculation that reads it, in the same undo step: `AFSimple_Calculate` field lists, simplified field notation (the comment and the JavaScript after it) and `getField` calls naming it in one string literal; the rest of each script is kept as written. A sum used to drop the renamed field, notation read it as 0 and a script kept a stale value. Field Properties says how many formulas changed and `form_set_props` lists them as `formulas_updated`. Names built at run time and other events' scripts are not rewritten. Estimates unchanged. +- **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, in the order added, and then behind-content watermarks, so a watermark added after a background no longer disappears. Synthetic regressions in `crates/edit` check the order. Estimates unchanged. - **2026-10-16 (M7, #2516):** A paragraph number from `text_paragraphs` keeps naming the paragraph it listed after edits regroup the page: `text_edit`, `text_edit_layout` and `page_render`'s `hide_text_block` find it again by its text and box, and refuse one that was merged into a neighbour (the edited paragraph included) instead of rewriting the paragraph that took its number (`stale_paragraphs` lists the numbers gone). The inline editor's switch-paragraph click holds the same key. No stable identity across save and reopen yet (#2508). @@ -192,6 +194,7 @@ Ranked; detail and the full milestone table in [docs/roadmap.md](docs/roadmap.md - **2026-10-18 (M6, #2892):** Push-button normal labels are editable per widget through Options and form_set_props, independently of field names; rendering, undo/redo and incremental/full save/reopen are covered. Estimates unchanged. - **2026-10-18 (M7, #3212):** Headers, Bates numbers, watermarks and added text stay visible after unfinished marked content, with original content preserved. +- **2026-10-18 (M7, #3187):** Added-image automation normalizes extreme signed rotation angles before combining them with the current orientation; integer bounds, history, pixels, save/reopen and MCP have regressions. Estimates unchanged. - **2026-10-15 (M7, Edit text caret):** In a text region that mixes sizes (a 24 pt title over 12 pt lines), the inline editor's caret on a small row was as tall as the region's first size and reached into the row below: egui draws the caret at least one row of the text field's font tall, and that font had the paragraph's first size. The field's font is now half the region's smallest size, so each row's own height decides. Kittest regression; checked in the app on a real document. Estimates unchanged. - **2026-10-16 (M3, #2781):** The Pages panel follows the current page, as Acrobat's page thumbnails do: going to a page whose thumbnail is out of sight (scrolling, Page Down, Go to page, a bookmark) scrolls the panel just enough to show it. A thumbnail already in view, or clicked, leaves the panel where it is, and a page change while the user scrolls the panel or drags thumbnails does not move it. Estimates unchanged. - **2026-10-16 (M12, #1645):** CLI info, text and render use the engine's dynamic XFA layout rather than placeholder pages. Synthetic two-page and encrypted-form regressions cover counts, text, pixels, copy permissions and unchanged input bytes. Estimates unchanged. diff --git a/crates/automation/README.md b/crates/automation/README.md index 669ea61463..87111b2eaf 100644 --- a/crates/automation/README.md +++ b/crates/automation/README.md @@ -15,6 +15,12 @@ tools() -> Vec // name, title, descrip mcp::McpServer::new(a).serve(stdin, stdout)? // newline-delimited JSON-RPC 2.0 ``` +`content_update.rotate` rotates an added image counter-clockwise by an integer multiple of +90 degrees. Large positive and negative signed 64-bit angles are normalized before combining +them with the image's current orientation. Full turns retain its appearance, rectangle, crop +and flips. Angles that are not multiples of 90 are refused without changing the document or +its undo/redo history. + `Content` is `Json(Value)` or `Png { data, width, height }`. Errors are `UnknownTool`, `InvalidArgs` (the call didn't match the schema) or `Failed` (a readable message for the agent). ## Conventions diff --git a/crates/automation/src/content.rs b/crates/automation/src/content.rs index faefc5fa7c..aa8615f29e 100644 --- a/crates/automation/src/content.rs +++ b/crates/automation/src/content.rs @@ -171,7 +171,8 @@ impl Automation { if deg.rem_euclid(90) != 0 { return Err(bad("rotate must be a multiple of 90")); } - i.rotation = ((i64::from(i.rotation) * 90 + deg).rem_euclid(360) / 90) as u8; + // Reduce untrusted degrees before adding the current quarter turns. + i.rotation = (i64::from(i.rotation) + deg.rem_euclid(360) / 90).rem_euclid(4) as u8; } if a.opt_bool("flip_h")? == Some(true) { i.flip_h = !i.flip_h; diff --git a/crates/automation/src/lib.rs b/crates/automation/src/lib.rs index c48c25e903..775955fc12 100644 --- a/crates/automation/src/lib.rs +++ b/crates/automation/src/lib.rs @@ -1290,6 +1290,14 @@ impl Automation { if let Some(s) = a.opt_num("font_size")? { hf.font_size = s; } + hf.font = match a.opt_str("font")? { + None | Some("Helvetica") => pdfcraft_engine::FontFamily::Helvetica, + Some("Times") => pdfcraft_engine::FontFamily::Times, + Some("Courier") => pdfcraft_engine::FontFamily::Courier, + Some(other) => { + return Err(ToolError::InvalidArgs(format!("font must be \"Helvetica\", \"Times\" or \"Courier\", not {other:?}"))); + } + }; hf.color = color("color", hf.color)?; if let Some(m) = a.get("margins").and_then(Value::as_array) { let m: Vec = m.iter().filter_map(Value::as_f64).collect(); diff --git a/crates/automation/src/tools.rs b/crates/automation/src/tools.rs index d41ab46d53..50efc37af7 100644 --- a/crates/automation/src/tools.rs +++ b/crates/automation/src/tools.rs @@ -875,7 +875,7 @@ pub fn tools() -> Vec { t( "doc_header_footer", "Add header and footer", - "Add a header and/or footer to pages. Text boxes: header_left/center/right, footer_left/center/right. Tokens: <<1>> page number, <> page count, <<1 of n>>, <>, <<1/n>>, dates <> <> <> (and more), Bates <>. replace: true swaps out existing ones (Update). Undoable.", + "Add a header and/or footer to pages. Text boxes: header_left/center/right, footer_left/center/right. Tokens: <<1>> page number, <> page count, <<1 of n>>, <>, <<1/n>>, dates <> <> <> (and more), Bates <>. font: Helvetica (default), Times or Courier. replace: true swaps out existing ones (Update). Undoable.", ) .cmd("edit.header_footer") .with(schema( @@ -884,6 +884,7 @@ pub fn tools() -> Vec { "pages": pages("to mark (default: all)"), "header_left": { "type": "string" }, "header_center": { "type": "string" }, "header_right": { "type": "string" }, "footer_left": { "type": "string" }, "footer_center": { "type": "string" }, "footer_right": { "type": "string" }, + "font": { "type": "string", "enum": ["Helvetica", "Times", "Courier"] }, "font_size": { "type": "number", "exclusiveMinimum": 0 }, "color": { "type": "string", "description": "#RRGGBB or a colour name." }, "margins": { "type": "array", "items": { "type": "number", "minimum": 0 }, "minItems": 4, "maxItems": 4, "description": "Top, bottom, left, right in points (default 36, 36, 72, 72)." }, diff --git a/crates/automation/tests/automation.rs b/crates/automation/tests/automation.rs index f5298c562d..12e525aa94 100644 --- a/crates/automation/tests/automation.rs +++ b/crates/automation/tests/automation.rs @@ -2239,6 +2239,20 @@ fn headers_watermarks_and_backgrounds_through_tools() { assert!(matches!(a.call("doc_remove_marks", &json!({ "doc": doc, "kind": "watermark" })), Err(ToolError::Failed(_)))); assert!(matches!(a.call("doc_header_footer", &json!({ "doc": doc })), Err(ToolError::Failed(_))), "no text"); ok(&mut a, "doc_save", json!({ "doc": doc, "path": "marked.pdf" })); + // A standard face other than Helvetica (#3754), kept through save and reopen; one it doesn't + // offer is refused with the faces it does. + let mono = ok(&mut a, "doc_open", json!({ "path": "b.pdf" }))["doc"].as_u64().unwrap(); + ok(&mut a, "doc_header_footer", json!({ "doc": mono, "header_left": "MONO", "font": "Courier" })); + assert!(page_text(&mut a, mono)[0].contains("MONO")); + ok(&mut a, "doc_save", json!({ "doc": mono, "path": "courier.pdf" })); + let saved = pdfcraft_cos::Document::open(std::sync::Arc::new(std::fs::read(dir.join("courier.pdf")).unwrap())).unwrap(); + let page = &pdfcraft_model::pages(&saved)[0]; + let res = saved.resolve(page.dict.get(b"Resources").unwrap()); + let fonts = saved.resolve(res.as_dict().unwrap().get(b"Font").unwrap()).as_dict().cloned().unwrap(); + let face = saved.resolve(fonts.get(b"PCCour").expect("Courier is added")).as_dict().cloned().unwrap(); + assert_eq!(face.name(b"BaseFont"), Some(&b"Courier"[..])); + let refused = a.call("doc_header_footer", &json!({ "doc": mono, "header_left": "X", "font": "Arial" })); + assert!(matches!(refused, Err(ToolError::InvalidArgs(e)) if e.contains("Courier"))); } #[test] @@ -3729,6 +3743,103 @@ fn content_index_out_of_range_is_an_error_not_a_panic() { assert_eq!(ok(&mut a, "content_list", json!({ "doc": doc }))["count"], 1); } +fn added_rotation_image(dir: &Path) { + let mut png = Vec::new(); + let mut enc = png::Encoder::new(&mut png, 2, 1); + enc.set_color(png::ColorType::Rgb); + enc.write_header().unwrap().write_image_data(&[255, 0, 0, 0, 0, 255]).unwrap(); + std::fs::write(dir.join("rotation.png"), png).unwrap(); +} + +#[test] +fn large_added_image_rotations_keep_orientation_and_undo() { + let dir = workdir("large-added-rotation"); + added_rotation_image(&dir); + let mut a = auto(&dir); + let image = |a: &Automation, doc| { + let d = a.session().get(pdfcraft_engine::DocId(doc)).unwrap(); + let pdfcraft_engine::AddedContent::Image(image) = &d.added()[0].content else { panic!("added image") }; + image.clone() + }; + let largest = i64::MAX / 90 * 90; + let smallest = i64::MIN / 90 * 90; + for initial in [1, 0, 2, 3] { + let doc = ok(&mut a, "doc_create", json!({ "from": "blank", "width": 200, "height": 300 }))["doc"].as_u64().unwrap(); + ok(&mut a, "page_add_image", json!({ "doc": doc, "page": 1, "path": "rotation.png", "rect": [20, 20, 120, 70] })); + ok( + &mut a, + "content_update", + json!({ "doc": doc, "page": 1, "index": 1, "rotate": initial * 90, + "flip_h": true, "flip_v": true, "crop": [0.1, 0.05, 0.2, 0.1] }), + ); + let before = image(&a, doc); + for degrees in [largest, largest - 90, largest - 180, smallest, smallest + 90, smallest + 180, -450, -90, 0, 90, 450] { + ok(&mut a, "content_update", json!({ "doc": doc, "page": 1, "index": 1, "rotate": degrees })); + let expected = pdfcraft_engine::AddedImage { rotation: (initial + (degrees / 90).rem_euclid(4)).rem_euclid(4) as u8, ..before.clone() }; + assert_eq!(image(&a, doc), expected, "initial quarter turn {initial}, degrees {degrees}"); + ok(&mut a, "edit_undo", json!({ "doc": doc })); + assert_eq!(image(&a, doc), before, "Undo restores the image and its properties"); + ok(&mut a, "edit_redo", json!({ "doc": doc })); + assert_eq!(image(&a, doc), expected, "Redo restores the normalized orientation"); + ok(&mut a, "edit_undo", json!({ "doc": doc })); + } + ok(&mut a, "doc_save", json!({ "doc": doc, "path": format!("initial-{initial}.pdf") })); + let state = |a: &Automation| { + let d = a.session().get(pdfcraft_engine::DocId(doc)).unwrap(); + (d.bytes.clone(), d.dirty, d.edit_generation(), d.can_undo().map(str::to_owned), d.can_redo().map(str::to_owned)) + }; + let saved = state(&a); + assert!(!saved.1 && saved.4.is_some(), "a clean document with a redo step"); + for degrees in [i64::MIN, i64::MAX, -1, 1, 45] { + assert!(matches!( + a.call("content_update", &json!({ "doc": doc, "page": 1, "index": 1, "rotate": degrees })), + Err(ToolError::InvalidArgs(_)) + )); + assert_eq!(state(&a), saved, "invalid degrees {degrees} leave bytes and history alone"); + } + } +} + +#[cfg(feature = "mcp")] +#[test] +fn large_added_image_rotations_round_trip_through_mcp() { + let dir = workdir("large-added-rotation-mcp"); + added_rotation_image(&dir); + let mut a = auto(&dir); + let doc = ok(&mut a, "doc_create", json!({ "from": "blank", "width": 200, "height": 300 }))["doc"].as_u64().unwrap(); + ok(&mut a, "page_add_image", json!({ "doc": doc, "page": 1, "path": "rotation.png", "rect": [20, 20, 120, 70] })); + ok( + &mut a, + "content_update", + json!({ "doc": doc, "page": 1, "index": 1, "rotate": 90, + "flip_h": true, "flip_v": true, "crop": [0.1, 0.05, 0.2, 0.1] }), + ); + let mut server = McpServer::new(a); + let render = json!({ "name": "page_render", "arguments": { "doc": doc, "page": 1, "dpi": 72 } }); + let before = rpc(&mut server, 1, "tools/call", render.clone()); + let update = rpc( + &mut server, + 2, + "tools/call", + json!({ "name": "content_update", "arguments": { + "doc": doc, "page": 1, "index": 1, "rotate": i64::MAX / 360 * 360 + } }), + ); + assert_eq!(update["result"]["isError"], false, "{update}"); + let after = rpc(&mut server, 3, "tools/call", render.clone()); + assert_eq!(after["result"]["content"], before["result"]["content"], "full turns retain the rendered orientation"); + let saved = rpc(&mut server, 4, "tools/call", json!({ "name": "doc_save", "arguments": { "doc": doc, "path": "rotated.pdf" } })); + assert_eq!(saved["result"]["isError"], false, "{saved}"); + let mut reopened = auto(&dir); + let doc = ok(&mut reopened, "doc_open", json!({ "path": "rotated.pdf" }))["doc"].as_u64().unwrap(); + let d = reopened.session().get(pdfcraft_engine::DocId(doc)).unwrap(); + let pdfcraft_engine::AddedContent::Image(image) = &d.added()[0].content else { panic!("saved image") }; + assert_eq!(image.rotation, 1); + let png = reopened.call("page_render", &json!({ "doc": doc, "page": 1, "dpi": 72 })).unwrap(); + let Content::Png { data, .. } = &png[0] else { panic!("rendered PNG") }; + assert_eq!(after["result"]["content"][0]["data"], base64::Engine::encode(&base64::engine::general_purpose::STANDARD, data)); +} + /// Text styling on an added image is refused (#1331), as image transforms are on text, and the /// refusal leaves the content, the dirty flag and the undo and redo steps as they were. #[test] diff --git a/crates/edit/src/images.rs b/crates/edit/src/images.rs index 3b3d62f230..75bb12f47d 100644 --- a/crates/edit/src/images.rs +++ b/crates/edit/src/images.rs @@ -165,12 +165,19 @@ impl ImageState { } } b"l" => { - if let Some([x, y]) = op.nums::<2>() { + // A segment before any m (or re) has no subpath to extend: BezPath's line_to and + // friends panic on an empty path by contract, so the segment is skipped, as the + // renderer, MuPDF and pdftotext do (#3581). + if let Some([x, y]) = op.nums::<2>() + && self.path.elements().last().is_some() + { self.path.line_to(self.point(x, y)); } } b"c" => { - if let Some([x1, y1, x2, y2, x3, y3]) = op.nums::<6>() { + if let Some([x1, y1, x2, y2, x3, y3]) = op.nums::<6>() + && self.path.elements().last().is_some() + { self.path.curve_to(self.point(x1, y1), self.point(x2, y2), self.point(x3, y3)); } } @@ -182,12 +189,19 @@ impl ImageState { } } b"y" => { - if let Some([x1, y1, x2, y2]) = op.nums::<4>() { + if let Some([x1, y1, x2, y2]) = op.nums::<4>() + && self.path.elements().last().is_some() + { let end = self.point(x2, y2); self.path.curve_to(self.point(x1, y1), end, end); } } - b"h" => self.path.close_path(), + b"h" => { + // ClosePath with no subpath panics by the same contract (#3581). + if self.path.elements().last().is_some() { + self.path.close_path(); + } + } b"re" => { if let Some([x, y, w, h]) = op.nums::<4>() { self.path.move_to(self.point(x, y)); diff --git a/crates/edit/src/lib.rs b/crates/edit/src/lib.rs index 83ad404877..e2d69a97bc 100644 --- a/crates/edit/src/lib.rs +++ b/crates/edit/src/lib.rs @@ -2,8 +2,9 @@ //! Watermark, Background and Bates numbering (execution plan M7.6). //! //! **How marks are stored.** Each mark is one content stream appended to (or, for content behind -//! the page, prepended to) the page's `/Contents`, drawn in display space (after `/Rotate`) and -//! wrapped in a marked-content artifact (§14.8.2.2) so readers and screen readers skip it: +//! the page, inserted after the backgrounds at the start of) the page's `/Contents`, drawn in +//! display space (after `/Rotate`) and wrapped in a marked-content artifact (§14.8.2.2) so +//! readers and screen readers skip it: //! `/Artifact <> BDC … EMC`. The //! `/PCMark` entry (also in the stream dictionary) names the kind of mark, which is how Update and //! Remove find them again without decompressing page content. When @@ -11,7 +12,8 @@ //! so its graphics state cannot reach the mark. The closing stream also ends known unfinished //! marked-content sequences before the new mark. //! -//! Text uses standard Helvetica (WinAnsi) added to the page resources as `/PCHelv`. +//! Text uses standard Helvetica (WinAnsi) added to the page resources as `/PCHelv`; headers and +//! footers may use standard Times (`/PCTimes`) or Courier (`/PCCour`) instead (#3754). #![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)] @@ -58,6 +60,8 @@ pub struct HeaderFooter { /// `<>`, `<>`, and Bates numbers `<>` /// (digits, start, prefix, suffix). pub text: [String; 6], + /// The standard face the text is drawn in (#3754): Helvetica (the default), Times or Courier. + pub font: added::Family, pub font_size: f64, pub color: Rgb, pub underline: bool, @@ -70,7 +74,15 @@ pub struct HeaderFooter { impl Default for HeaderFooter { /// Acrobat's defaults: 8 pt black, margins 0.5 in top and bottom, 1 in left and right. fn default() -> Self { - Self { text: Default::default(), font_size: 8.0, color: [0.0; 3], underline: false, margins: [36.0, 36.0, 72.0, 72.0], start_number: 1 } + Self { + text: Default::default(), + font: added::Family::Helvetica, + font_size: 8.0, + color: [0.0; 3], + underline: false, + margins: [36.0, 36.0, 72.0, 72.0], + start_number: 1, + } } } @@ -297,19 +309,57 @@ 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 exactly the dictionary +/// [`add_page_resources`] writes for that face, otherwise the first such `preferred_1`, +/// `preferred_2`… (one of the first `fonts.len() + 1` always is). A font with anything more +/// (`/ToUnicode`, `/Widths`…) is someone else's, even in the same face. +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.len() == 4 + && d.name(b"Type") == Some(b"Font") + && 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 { @@ -505,8 +555,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 +568,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 in the order added, then watermarks. + 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 +579,17 @@ 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); + // Right after the backgrounds already there, so a new background is painted over the + // older ones (#3591) and a watermark over all of them. Not after the watermarks: on a + // page with no content, one on top is at the start too and can't be told apart, so a + // newer watermark behind goes under the older ones. + let at = list.iter().take(behind_marks).rposition(|o| stream_tag(doc, o).as_deref() == Some(MarkKind::Background.tag())).map_or(0, |i| i + 1); + list.insert(at, mark); } else { if let Some((split, unclosed, marked)) = wrap { let mut close_bytes = b"\nEMC".repeat(marked); @@ -645,7 +699,16 @@ 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; - let mut body: Vec = format!("BT\n/PCHelv {} Tf\n{}\n", n(size), rgb(hf.color)).into_bytes(); + // 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() { if template.trim().is_empty() { @@ -655,7 +718,8 @@ pub fn add_header_footer(doc: &mut Document, pages: &[usize], hf: &HeaderFooter, let header = slot < 3; let line_h = size * 1.2; for (li, line) in lines.iter().enumerate() { - let tw = helvetica_width(line, size); + // Centred and right-aligned text is placed by its width in the chosen face. + let tw = hf.font.width(line, size, false, false); let x = match slot % 3 { 0 => left, 1 => (w - tw) / 2.0, @@ -676,7 +740,10 @@ 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()); - add_resources(doc, page, None, None)?; + // A name already there is that very font (see `free_font_name`): left as it is. + if !page_fonts(doc, page).contains(resource.as_bytes()) { + 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 e9b743c5c2..1333a3057c 100644 --- a/crates/edit/src/tests.rs +++ b/crates/edit/src/tests.rs @@ -118,6 +118,99 @@ fn header_and_footer_are_drawn_in_display_space_and_wrap_the_original_content() assert_eq!(marks_present(&doc), [MarkKind::HeaderFooter]); } +#[test] +fn header_and_footer_are_drawn_in_the_chosen_standard_face() { + // #3754: Times or Courier instead of Helvetica. The content names the face, the page's fonts + // gain it, and right-aligned text is placed by that face's widths (not Helvetica's). + let text = "Wide words"; + for (font, resource, base) in [(added::Family::Times, "PCTimes", "Times-Roman"), (added::Family::Courier, "PCCour", "Courier")] { + let mut doc = fixture(); + let hf = HeaderFooter { + text: [String::new(), String::new(), text.into(), String::new(), String::new(), String::new()], + font, + ..HeaderFooter::default() + }; + add_header_footer(&mut doc, &[0], &hf, false, &cx()).unwrap(); + let doc = reopen(&doc); + let mark = streams(&doc, 0).last().cloned().unwrap(); + assert!(mark.contains(&format!("/{resource} 8 Tf")), "{base}: {mark}"); + let x = 600.0 - 72.0 - font.width(text, 8.0, false, false); + assert!((x - (600.0 - 72.0 - pdfcraft_fonts::helvetica_width(text, 8.0))).abs() > 1.0, "{base}: the faces differ in width"); + assert!(mark.contains(&format!("1 0 0 1 {} ", n(x))), "{base}: right-aligned by its own widths: {mark}"); + let page = &pdfcraft_model::pages(&doc)[0]; + let res = doc.resolve(page.dict.get(b"Resources").unwrap()); + let fonts = doc.resolve(res.as_dict().unwrap().get(b"Font").unwrap()).as_dict().cloned().unwrap(); + let face = doc.resolve(fonts.get(resource.as_bytes()).unwrap()).as_dict().cloned().unwrap(); + assert_eq!(face.name(b"BaseFont"), Some(base.as_bytes()), "{base}"); + } + // The default is still Helvetica, and only Helvetica is added. + let mut doc = fixture(); + let hf = + HeaderFooter { text: [text.into(), String::new(), String::new(), String::new(), String::new(), String::new()], ..HeaderFooter::default() }; + add_header_footer(&mut doc, &[0], &hf, false, &cx()).unwrap(); + let mark = streams(&reopen(&doc), 0).last().cloned().unwrap(); + 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"[..])); + // The same face with more to it (here a /ToUnicode map) is the page's own too: kept whole. + let mut doc = fixture(); + let page = pdfcraft_model::pages(&doc)[0].obj; + let map = doc.add(Object::Stream(pdfcraft_cos::Stream::flate(Dict::new(), b"/CIDInit /ProcSet findresource begin"))); + let mut theirs = Dict::new(); + for (k, v) in [("Type", "Font"), ("Subtype", "Type1"), ("BaseFont", "Times-Roman"), ("Encoding", "WinAnsiEncoding")] { + theirs.set(k.as_bytes().to_vec(), Object::name(v)); + } + theirs.set(b"ToUnicode".to_vec(), Object::Ref(map)); + let mut fonts = Dict::new(); + 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(); + add_header_footer(&mut doc, &[0], &hf, false, &cx()).unwrap(); + let doc = reopen(&doc); + let fonts = page_fonts(&doc, &pdfcraft_model::pages(&doc)[0]); + let kept = doc.resolve(fonts.get(b"PCTimes").unwrap()).as_dict().cloned().unwrap(); + assert!(kept.contains(b"ToUnicode"), "the page's map is kept"); + assert!(streams(&doc, 0).last().unwrap().contains("/PCTimes_1 8 Tf")); +} + #[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 @@ -189,6 +282,45 @@ 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, in the order added, and then watermarks, 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 a_watermark_behind_goes_under_one_on_top_of_a_page_without_content() { + // With no content of its own, the page's watermark on top is its first stream, which looks + // like a behind-mark; a watermark added behind later must still be painted under it. + let mut doc = fixture(); + add_watermark(&mut doc, &[2], &Watermark { text: "TOP".into(), ..Watermark::default() }, false).unwrap(); + add_watermark(&mut doc, &[2], &Watermark { text: "BEHIND".into(), behind: true, ..Watermark::default() }, false).unwrap(); + let s = streams(&doc, 2); + assert_eq!(s.len(), 2, "{s:?}"); + assert!(s[0].contains("(BEHIND) Tj") && s[1].contains("(TOP) Tj"), "the watermark behind is painted first: {s:?}"); +} + #[test] fn watermark_offset_moves_the_centre_right_and_up() { let mut doc = fixture(); @@ -2111,6 +2243,25 @@ fn image_page_with(content: &str) -> Document { Document::open(Arc::new(out)).unwrap() } +/// A path segment operator before any `m` (or `re`) has no subpath to extend: the image scan used +/// to call `BezPath`'s `line_to`/`curve_to`/`close_path` on an empty path, which panics by contract, +/// taking `object_list`, `page_images` and the Office export's image walk down with it (#3581). +/// The segment is now skipped, as the renderer, MuPDF and pdftotext do, and the page's images are +/// still listed. +#[test] +fn image_scan_skips_a_path_segment_before_any_m() { + for bad in ["1 1 l S", "1 1 2 2 3 3 c S", "1 1 2 2 4 4 y S", "1 1 2 2 3 3 v S", "h S"] { + let content = format!("q {bad} Q 100 100 200 100 cm /Im0 Do"); + let doc = image_page_with(&content); + assert_eq!(images::page_images(&doc, 0).unwrap().len(), 1, "{bad}: the image is still listed"); + } + // A path started with m (or re) still extends and closes as before. + let doc = image_page_with("q 0 0 m 1 1 l h S Q 100 100 200 100 cm /Im0 Do"); + assert_eq!(images::page_images(&doc, 0).unwrap().len(), 1); + let doc = image_page_with("q 10 10 20 20 re W n Q 100 100 200 100 cm /Im0 Do"); + assert_eq!(images::page_images(&doc, 0).unwrap().len(), 1); +} + /// The box of the first `re` on the page in user space, after the `cm`s before it. fn first_rect_in_user_space(doc: &Document) -> [f64; 4] { let mut ctm = pdfcraft_content::Matrix::IDENTITY; @@ -2364,6 +2515,43 @@ fn paragraphs_take_new_formatting() { } } +#[test] +fn a_one_row_title_keeps_its_centre_when_it_grows_and_when_it_wraps() { + // A centred one-row title (its centre is at x ≈ 306 on the 600 pt page) above a body + // left-aligned at x = 72. Growing it must not drag it to the right. + let src = "BT /F1 18 Tf 1 0 0 1 252.477 700 Tm (Centered title) Tj ET\n\ + BT /F1 12 Tf 1 0 0 1 72 640 Tm (Body text below the heading starts here and runs) Tj ET\n\ + BT /F1 12 Tf 1 0 0 1 72 625.6 Tm (on for a second row of ordinary words.) Tj ET"; + let mut doc = text_page(src); + let before = text::text_blocks(&doc, 0).unwrap()[0].clone(); + let centre = (before.rect[0] + before.rect[2]) / 2.0; + text::rewrite_block(&mut doc, 0, 0, Some("Centered title with a few more words"), &text::BlockStyle::default()).unwrap(); + let doc = reopen(&doc); + let blocks = text::text_blocks(&doc, 0).unwrap(); + let lines = text::text_lines(&doc, 0).unwrap(); + assert_eq!(blocks[0].text, "Centered title with a few more words"); + let r = lines[blocks[0].lines[0]].rect; + assert!(((r[0] + r[2]) / 2.0 - centre).abs() < 1.0, "the grown title stays centred on {centre}, its centre is {}", (r[0] + r[2]) / 2.0); + // Long enough to have to wrap: every row stays centred on the same centre too. + let mut doc = text_page(src); + text::rewrite_block( + &mut doc, + 0, + 0, + Some("Centered title with many more words added so that the single row has to become much longer"), + &text::BlockStyle::default(), + ) + .unwrap(); + let doc = reopen(&doc); + let blocks = text::text_blocks(&doc, 0).unwrap(); + let lines = text::text_lines(&doc, 0).unwrap(); + assert!(blocks[0].lines.len() > 1, "the long title wraps into rows: {:?}", blocks[0].text); + for i in &blocks[0].lines { + let r = lines[*i].rect; + assert!(((r[0] + r[2]) / 2.0 - centre).abs() < 2.0, "row {:?} stays centred on {centre}", lines[*i].text); + } +} + #[test] fn a_new_paragraph_colour_does_not_spill_into_the_text_after_it() { let mut doc = text_page("BT /F1 10 Tf 72 700 Td (First paragraph) Tj 0 -40 Td (Second paragraph) Tj ET"); @@ -5345,6 +5533,23 @@ fn flattening_comments_keeps_annotations_that_are_not_comments() { assert_eq!(left(&reopen(&doc)), without_widget, "flattening fields touches no annotation that isn't a widget"); } +/// A footnote marker (`Ts` text rise, a smaller `Tf` size) does not split the paragraph around it +/// into separate editable zones (#3252): on `main` this read as three blocks — the text up to the +/// marker, the marker alone, and the text after it — because the row-joining baseline tolerance +/// was scaled by the smaller of the two runs' sizes, far tighter than a typical superscript's rise. +#[test] +fn a_superscript_footnote_marker_stays_in_its_paragraphs_row() { + let doc = text_page( + "BT /F1 12 Tf 72 700 Td (and A.) Tj 0 -14 Td (Edward Siecienski) Tj 4 Ts /F1 7.2 Tf (3) Tj 0 Ts /F1 12 Tf (. More words continue the paragraph right after the marker.) Tj ET", + ); + let lines = text::text_lines(&doc, 0).unwrap(); + assert_eq!(lines.len(), 4, "the marker is its own line, raised and smaller: {lines:?}"); + let blocks = text::text_blocks(&doc, 0).unwrap(); + assert_eq!(blocks.len(), 1, "one paragraph, not split at the marker: {blocks:?}"); + assert_eq!(blocks[0].rows.len(), 2, "two visual rows, the second carrying the marker"); + assert_eq!(blocks[0].text, "and A. Edward Siecienski3. More words continue the paragraph right after the marker.", "{:?}", blocks[0]); +} + /// A page whose text is a clipping mask (`mode` Tr) with a fill painted through it. fn clipping_text_page(mode: i64) -> Document { text_page(&format!("q\nBT {mode} Tr /F1 48 Tf 40 60 Td (MASK) Tj ET\n1 0 0 rg 0 0 300 150 re f\nQ\n")) @@ -5703,3 +5908,40 @@ fn vertical_text_drawn_one_glyph_per_tj_is_one_line() { } assert_eq!(text::text_blocks(&doc, 0).unwrap().len(), 2); } + +/// Two genuinely stacked rows of ordinary body text must not join into one row just because the +/// first is narrow enough to put the second's start within the #3252 tolerance band: here the +/// second row starts 1.4pt to the left of where the first ends (so the two overlap a little, +/// as compressed text or a tight column can) and 5pt below its baseline — inside the widened 50% +/// tolerance but outside the original 20% one. They stay two rows (and two paragraphs). +#[test] +fn stacked_rows_that_merely_overlap_do_not_join_into_one_row() { + let doc = text_page("BT /F1 12 Tf 72 700 Td (A compressed line of body text here) Tj 190 -5 Td (Second stacked line of body text) Tj ET"); + let lines = text::text_lines(&doc, 0).unwrap(); + assert_eq!(lines.len(), 2); + let gap = lines[1].rect[0] - lines[0].rect[2]; + assert!((-2.4..0.0).contains(&gap), "the rows overlap a little, within the old tolerance band: {gap}"); + let baseline_shift = lines[0].rect[3] - lines[1].rect[3]; + assert!((2.4..6.0).contains(&baseline_shift), "between the old and new vertical tolerance: {baseline_shift}"); + let blocks = text::text_blocks(&doc, 0).unwrap(); + assert_eq!(blocks.len(), 2, "two separate rows stay two paragraphs, not one: {blocks:?}"); +} + +/// Two equal-size rows with no horizontal overlap must not join into one row either (#3252): +/// the wider tolerance is for a run distinctly smaller or larger than its neighbour (a +/// super/subscript), not for two ordinary body-text rows that happen to sit close enough +/// vertically — here a 5pt baseline shift, inside the widened 50% tolerance but outside the +/// original 20% one, with the second row starting just past where the first ends. +#[test] +fn equal_size_rows_need_the_tighter_tolerance_even_without_overlap() { + let doc = text_page("BT /F1 12 Tf 72 700 Td (Short line one) Tj 75 -5 Td (Second line two here) Tj ET"); + let lines = text::text_lines(&doc, 0).unwrap(); + assert_eq!(lines.len(), 2); + assert_eq!(lines[0].size, lines[1].size, "same size: no super/subscript here"); + let gap = lines[1].rect[0] - lines[0].rect[2]; + assert!(gap >= 0.0, "no horizontal overlap: {gap}"); + let baseline_shift = lines[0].rect[3] - lines[1].rect[3]; + assert!((2.4..6.0).contains(&baseline_shift), "between the old and new vertical tolerance: {baseline_shift}"); + let blocks = text::text_blocks(&doc, 0).unwrap(); + assert_eq!(blocks.len(), 2, "two separate rows stay two paragraphs, not one: {blocks:?}"); +} diff --git a/crates/edit/src/text.rs b/crates/edit/src/text.rs index 13b828db67..eea1ef3dd3 100644 --- a/crates/edit/src/text.rs +++ b/crates/edit/src/text.rs @@ -24,7 +24,7 @@ use crate::EditError; mod regions; #[path = "text_session.rs"] mod session; -use regions::{group_blocks, rewrite_styled_block}; +use regions::{group_blocks, is_centred, rewrite_styled_block}; pub use session::TextSession; /// One line of existing text. @@ -2558,6 +2558,11 @@ fn rewrite_block_from( } else { members.iter().map(|l| l.frame_rect()).reduce(|a, f| [a[0].min(f[0]), a[1].min(f[1]), a[2].max(f[2]), a[3].max(f[3])]).unwrap_or(b.rect) }; + // A centred title is laid out in the box it already sits in, so it keeps its centre as it + // grows or wraps, rather than being grown to the right up to the page's margin (#2581). + // Turned text is left alone: its centre lives along its own baseline, not in page space. + let crop = p.crop(doc); + let centred = upright && !rtl && style.width.is_none() && style.align.is_none() && is_centred(&b, &lines, (crop[0] + crop[2]) / 2.0, first.size); // A paragraph keeps its width (or takes the one asked for); a single line grows to the right // (to the left when right-to-left), up to the page's margin. let width = match style.width { @@ -2566,14 +2571,15 @@ fn rewrite_block_from( Some(_) => return Err(EditError::Invalid("the paragraph's width must be a number".into())), // Turned text: the page's margins are along another axis. A single line grows (below). None if !upright => (frame[2] - frame[0]).max(size * k), - None if anchored_right => (b.rect[2] - b.rect[0]).max(size * k).max(b.rect[2] + dx - (p.crop(doc)[0] + 36.0)), - None if members.len() == 1 => (b.rect[2] - b.rect[0]).max(size * k).max(p.crop(doc)[2] - 36.0 - (b.rect[0] + dx)), + None if anchored_right => (b.rect[2] - b.rect[0]).max(size * k).max(b.rect[2] + dx - (crop[0] + 36.0)), + // A centred line is not stretched to the margin: its own box is the frame it is centred in. + None if members.len() == 1 && !centred => (b.rect[2] - b.rect[0]).max(size * k).max(crop[2] - 36.0 - (b.rect[0] + dx)), None => (b.rect[2] - b.rect[0]).max(size * k), }; if !width.is_finite() { return Err(EditError::Invalid("the paragraph's width must be a number".into())); } - let align = style.align.or(rtl.then_some(crate::added::Align::Right)); + let align = if centred { Some(crate::added::Align::Center) } else { style.align.or(rtl.then_some(crate::added::Align::Right)) }; // Where a right-aligned line ends, from the paragraph's left edge (user space). let right_edge = if anchored_right { (b.rect[2] - b.rect[0]).max(0.0) } else { width }; // The move and the width are checked here, before the fallback fonts are written: a refused diff --git a/crates/edit/src/text_regions.rs b/crates/edit/src/text_regions.rs index a0a59ffb94..a5fdca2935 100644 --- a/crates/edit/src/text_regions.rs +++ b/crates/edit/src/text_regions.rs @@ -28,6 +28,43 @@ fn frame_row_rect(row: &[usize], lines: &[TextLine]) -> Option<[f64; 4]> { row.iter().filter_map(|i| lines.get(*i)).map(TextLine::frame_rect).reduce(union) } +/// The left edge the rest of the page's text sits on, ignoring the rows of `block`. A single row +/// that does not share it is not merely an indented or full-width left-aligned line, so its own +/// centre is the only remaining explanation for where it sits. +fn neighbour_left(lines: &[TextLine], block: &TextBlock) -> Option { + let mut edges: HashMap = HashMap::new(); + for (i, line) in lines.iter().enumerate() { + if block.lines.contains(&i) { + continue; + } + // Quarter-point buckets: equal edges found through different runs still agree. + *edges.entry((line.rect[0] * 4.0).round() as i64).or_insert(0) += 1; + } + let (best, _) = edges.into_iter().max_by_key(|(_, count)| *count)?; + Some(best as f64 / 4.0) +} + +/// Whether a block reads as centred, and so must keep its centre when it grows or wraps. +/// +/// Several ragged rows whose left edges disagree while their centres agree are centred. A one row +/// block cannot say that: all of its left edges are the same, so the ragged test never fires and a +/// centred title would be taken for left-aligned and grow to the right (#2581). Fall back to the +/// page instead — a lone row sitting on the page's own centre, and not on the edge the rest of the +/// text uses, is a centred title. A full-width left-aligned line still lines up with that edge, so +/// it is not mistaken for one. +pub(super) fn is_centred(block: &TextBlock, lines: &[TextLine], page_centre: f64, size: f64) -> bool { + let centre = (block.rect[0] + block.rect[2]) / 2.0; + let lefts: Vec<_> = block.rows.iter().filter_map(|r| row_rect(r, lines).map(|r| r[0])).collect(); + if block.rows.len() > 1 { + let left_span = lefts.iter().copied().fold(f64::NEG_INFINITY, f64::max) - lefts.iter().copied().fold(f64::INFINITY, f64::min); + return left_span > size * 0.5 + && block.rows.iter().all(|row| row_rect(row, lines).is_some_and(|r| ((r[0] + r[2]) / 2.0 - centre).abs() < size)); + } + lefts + .first() + .is_some_and(|left| (centre - page_centre).abs() < size && neighbour_left(lines, block).is_none_or(|edge| (*left - edge).abs() > size * 0.5)) +} + pub(super) fn group_blocks(lines: &[TextLine]) -> Vec { group_layout(lines, true) } @@ -108,7 +145,18 @@ fn group_layout(lines: &[TextLine], mixed: bool) -> Vec { scanned && (mixed || same_style(prev, line)) && same_plane(prev, line) - && (prev.frame_origin().1 - line.frame_origin().1).abs() < prev.size.min(line.size) * 0.2 + // A superscript or subscript run (a footnote marker, say) sits on the same visual + // row as the text around it, raised or lowered by `Ts` from the body baseline + // (#3252). That shift is typically up to half the body size, not the marker's own + // smaller size — but the wider tolerance only applies between a run and a + // distinctly smaller or larger one, with no horizontal overlap (`gap >= 0.0`): + // two equal-size runs can land within it too, at a baseline shift tight columns + // or compressed text (`Tz`) can produce between genuinely separate rows, and the + // original 20% tolerance is what keeps those apart. + && ((prev.frame_origin().1 - line.frame_origin().1).abs() < prev.size.min(line.size) * 0.2 + || (gap >= 0.0 + && prev.size.min(line.size) < prev.size.max(line.size) * 0.9 + && (prev.frame_origin().1 - line.frame_origin().1).abs() < prev.size.max(line.size) * 0.5)) && gap >= -prev.size.min(line.size) * 0.2 && gap < prev.size.max(line.size) * 1.5 // Separate same-style text objects that merely touch or overlap, with no space @@ -739,12 +787,8 @@ pub(super) fn rewrite_styled_block( .ok_or_else(|| EditError::Invalid("the paragraph is drawn in a space it cannot be edited in".into()))?; let (tx, ty) = back.apply(dx, dy); block_ops.push(Op::new("cm", vec![n(1.0), n(0.0), n(0.0), n(1.0), n(tx), n(ty)])); - let centre = (block.rect[0] + block.rect[2]) / 2.0; - let lefts: Vec<_> = block.rows.iter().filter_map(|r| row_rect(r, lines).map(|r| r[0])).collect(); - let left_span = lefts.iter().copied().fold(f64::NEG_INFINITY, f64::max) - lefts.iter().copied().fold(f64::INFINITY, f64::min); - let centered = block.rows.len() > 1 - && left_span > first.size * 0.5 - && block.rows.iter().all(|row| row_rect(row, lines).is_some_and(|r| ((r[0] + r[2]) / 2.0 - centre).abs() < first.size)); + let crop = p.crop(doc); + let centered = is_centred(block, lines, (crop[0] + crop[2]) / 2.0, first.size); let source_leading = block .rows .windows(2) diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 77e0c21474..1bd9c9c3f1 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -173,6 +173,23 @@ use std::sync::{Arc, OnceLock}; #[cfg(test)] thread_local! { static SECONDARY_SCAN_COUNTS: std::cell::Cell<(usize, usize)> = const { std::cell::Cell::new((0, 0)) }; + static REFRESH_PANICS: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + +#[cfg(test)] +fn panic_on_refresh(count: usize) { + REFRESH_PANICS.with(|n| n.set(count)); +} + +#[cfg(test)] +fn check_refresh_panic() { + REFRESH_PANICS.with(|n| { + let rem = n.get(); + if rem > 0 { + n.set(rem - 1); + panic!("injected refresh panic"); + } + }); } #[cfg(test)] @@ -3860,14 +3877,28 @@ impl Session { pub fn undo(&mut self, id: DocId) -> Result { let doc = self.doc_mut(id)?; - doc.edit_notices.clear(); let editor = doc.editor.as_mut().ok_or(EditError::NothingToUndo)?; let (label, prev, keys, scope) = editor.undo.pop().ok_or(EditError::NothingToUndo)?; let current = std::mem::replace(&mut editor.cos, prev); let current_keys = std::mem::replace(&mut editor.keys, keys); editor.redo.push((label.clone(), current, current_keys, scope)); Self::adopt_keys(doc); - Self::refresh_scoped(doc, scope).inspect_err(|_| Self::show_released(doc))?; + let refreshed = guard(|| Self::refresh_scoped(doc, scope)).unwrap_or_else(|m| Err(EditError::Reopen(m))); + if let Err(e) = refreshed { + if let Some(ed) = doc.editor.as_mut() + && let Some((label, current, current_keys, scope)) = ed.redo.pop() + { + let prev = std::mem::replace(&mut ed.cos, current); + let keys = std::mem::replace(&mut ed.keys, current_keys); + ed.undo.push((label, prev, keys, scope)); + } + Self::adopt_keys(doc); + if !matches!(guard(|| Self::refresh(doc)), Ok(Ok(()))) { + Self::show_released(doc); + } + return Err(e); + } + doc.edit_notices.clear(); Self::sync_dirty(doc); doc.generation += 1; Ok(label) @@ -3875,14 +3906,28 @@ impl Session { pub fn redo(&mut self, id: DocId) -> Result { let doc = self.doc_mut(id)?; - doc.edit_notices.clear(); let editor = doc.editor.as_mut().ok_or(EditError::NothingToRedo)?; let (label, next, keys, scope) = editor.redo.pop().ok_or(EditError::NothingToRedo)?; let current = std::mem::replace(&mut editor.cos, next); let current_keys = std::mem::replace(&mut editor.keys, keys); editor.undo.push((label.clone(), current, current_keys, scope)); Self::adopt_keys(doc); - Self::refresh_scoped(doc, scope).inspect_err(|_| Self::show_released(doc))?; + let refreshed = guard(|| Self::refresh_scoped(doc, scope)).unwrap_or_else(|m| Err(EditError::Reopen(m))); + if let Err(e) = refreshed { + if let Some(ed) = doc.editor.as_mut() + && let Some((label, current, current_keys, scope)) = ed.undo.pop() + { + let next = std::mem::replace(&mut ed.cos, current); + let keys = std::mem::replace(&mut ed.keys, current_keys); + ed.redo.push((label, next, keys, scope)); + } + Self::adopt_keys(doc); + if !matches!(guard(|| Self::refresh(doc)), Ok(Ok(()))) { + Self::show_released(doc); + } + return Err(e); + } + doc.edit_notices.clear(); Self::sync_dirty(doc); doc.generation += 1; Ok(label) @@ -3944,6 +3989,8 @@ impl Session { doc.info.producer = info("Producer"); doc.info.custom = pdfcraft_organize::custom_info(&editor.cos); doc.info.file_size = bytes.len(); + #[cfg(test)] + check_refresh_panic(); if !doc.signatures.is_empty() { let signatures = signatures_of(&editor.cos, &bytes, &doc.trust, &doc.sig_cache); doc.signatures = Arc::new(signatures); @@ -4057,6 +4104,8 @@ impl Session { doc.renderer = RenderPool::new_inline(Arc::new(Vec::new()), doc.config.clone()); doc.display = original.clone(); doc.bytes = original; + #[cfg(test)] + check_refresh_panic(); } /// After a refresh failed: draw the display bytes it left (the original, once released), diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 080dc32dd3..808e8aaa8e 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -6147,3 +6147,110 @@ fn undo_never_makes_an_unsaved_document_clean() { s.undo(new).unwrap(); assert!(!s.get(new).unwrap().dirty, "once saved, undo back to the saved state is clean"); } + +#[test] +fn undo_and_redo_roll_back_on_a_refresh_panic_or_error() { + let (mut s, id) = session_with(1); + s.apply(id, Edit::RotatePages { pages: vec![0], degrees: 90 }).unwrap(); + let (bytes_90, gen_90) = { + let d = s.get(id).unwrap(); + (d.bytes.clone(), d.generation) + }; + + // 1. A panic during undo's refresh catches the unwind and rolls back cos, undo/redo, info, and working bytes. + panic_on_refresh(1); + let err = s.undo(id).unwrap_err(); + assert_eq!(err, EditError::Reopen("injected refresh panic".into())); + { + let d = s.get(id).unwrap(); + let ed = d.editor.as_ref().unwrap(); + assert_eq!(d.info.pages[0].rotation, 90); + assert_eq!(d.bytes, bytes_90); + assert_eq!(d.generation, gen_90); + assert!(d.dirty); + assert!(ed.cos.is_modified()); + assert_eq!(ed.undo.len(), 1); + assert!(ed.redo.is_empty()); + } + let _ = shown(&s, id, 0); + + // 2. If the rollback refresh also panics, `show_released` restores the renderer so the page still draws. + panic_on_refresh(2); + assert_eq!(s.undo(id), Err(EditError::Reopen("injected refresh panic".into()))); + assert_eq!(s.get(id).unwrap().editor.as_ref().unwrap().undo.len(), 1); + let _ = shown(&s, id, 0); + + // Undo now succeeds cleanly. + assert_eq!(s.undo(id).unwrap().as_str(), "Rotate page"); + let (bytes_0, gen_0) = { + let d = s.get(id).unwrap(); + assert_eq!(d.info.pages[0].rotation, 0); + assert!(!d.dirty); + (d.bytes.clone(), d.generation) + }; + + // 3. A panic during redo's refresh catches the unwind and rolls back to the undone state. + panic_on_refresh(1); + let err = s.redo(id).unwrap_err(); + assert_eq!(err, EditError::Reopen("injected refresh panic".into())); + { + let d = s.get(id).unwrap(); + let ed = d.editor.as_ref().unwrap(); + assert_eq!(d.info.pages[0].rotation, 0); + assert_eq!(d.bytes, bytes_0); + assert_eq!(d.generation, gen_0); + assert!(!d.dirty); + assert!(!ed.cos.is_modified()); + assert!(ed.undo.is_empty()); + assert_eq!(ed.redo.len(), 1); + } + let _ = shown(&s, id, 0); + + // 4. If redo's rollback refresh also panics, `show_released` restores the renderer. + panic_on_refresh(2); + assert_eq!(s.redo(id), Err(EditError::Reopen("injected refresh panic".into()))); + assert_eq!(s.get(id).unwrap().editor.as_ref().unwrap().redo.len(), 1); + let _ = shown(&s, id, 0); + + // Redo now succeeds cleanly. + assert_eq!(s.redo(id).unwrap().as_str(), "Rotate page"); + assert_eq!(s.get(id).unwrap().info.pages[0].rotation, 90); + + // 5. Metadata scope: a panic after `doc.info.title` was written rolls back `info.title` and keeps `edit_notices`. + s.apply(id, Edit::SetInfo { key: "Title".into(), value: "Draft".into() }).unwrap(); + let notice = TextNotice { field: None, page: 0, ch: '☺', redrawn_elsewhere: false }; + s.doc_mut(id).unwrap().edit_notices.push(notice.clone()); + panic_on_refresh(1); + assert_eq!(s.undo(id), Err(EditError::Reopen("injected refresh panic".into()))); + assert_eq!(s.get(id).unwrap().info.title.as_deref(), Some("Draft")); + assert_eq!(s.get(id).unwrap().edit_notices, [notice]); + assert_eq!(s.undo(id).unwrap().as_str(), "Change Title"); + assert_eq!(s.get(id).unwrap().info.title, None); + assert!(s.get(id).unwrap().edit_notices.is_empty()); + + panic_on_refresh(1); + assert_eq!(s.redo(id), Err(EditError::Reopen("injected refresh panic".into()))); + assert_eq!(s.get(id).unwrap().info.title, None); + assert_eq!(s.redo(id).unwrap().as_str(), "Change Title"); + assert_eq!(s.get(id).unwrap().info.title.as_deref(), Some("Draft")); + + // 6. Protection keys: undo/redo rollback restores `editor.keys` and `doc.password` before re-inspecting. + let protect = Protection { open_password: Some("u".into()), permissions_password: Some("o".into()), ..Protection::default() }; + s.apply(id, Edit::Protect(protect)).unwrap(); + assert!(s.get(id).unwrap().info.encrypted); + panic_on_refresh(1); + assert_eq!(s.undo(id), Err(EditError::Reopen("injected refresh panic".into()))); + assert!(s.get(id).unwrap().info.encrypted); + assert_eq!(s.get(id).unwrap().password.as_deref(), Some("u")); + assert_eq!(s.undo(id).unwrap().as_str(), "Protect with password"); + assert!(!s.get(id).unwrap().info.encrypted); + assert_eq!(s.get(id).unwrap().password, None); + + panic_on_refresh(1); + assert_eq!(s.redo(id), Err(EditError::Reopen("injected refresh panic".into()))); + assert!(!s.get(id).unwrap().info.encrypted); + assert_eq!(s.get(id).unwrap().password, None); + assert_eq!(s.redo(id).unwrap().as_str(), "Protect with password"); + assert!(s.get(id).unwrap().info.encrypted); + assert_eq!(s.get(id).unwrap().password.as_deref(), Some("u")); +} diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index 54d2d268a4..7e85cb8808 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -1054,6 +1054,12 @@ impl DocView { self.pages.get(&page).map(|p| p.tex.id()) } + /// Whether `page`'s whole-page raster (a tiled page's backdrop too) is still the one from + /// before its last change, whatever its tiles are. + pub(crate) fn page_raster_out_of_date(&self, page: usize) -> bool { + self.pages.get(&page).is_some_and(|p| p.tag == STALE_TAG) + } + /// Whether `page` is still drawn, somewhere, as it was before its last change: its whole-page /// raster (a tiled page's backdrop too) or any of its tiles waits for a replacement. pub(crate) fn page_out_of_date(&self, page: usize) -> bool { @@ -6517,4 +6523,120 @@ trailer << /Root 1 0 R >> assert_eq!(v.find.as_ref().unwrap().matches, vec![(1, 5..9)]); assert!(v.goto.is_none()); } + + /// A page whose picture moved can get its whole-page raster back before its last stale tile. + /// From then on the raster has the picture where it went: it is drawn from there, not from + /// where it was (which the raster now shows empty), and another drag starts from there too. + #[test] + fn a_moved_picture_is_drawn_from_the_fresh_raster_while_a_tile_is_still_stale() { + let content = b"q 60 0 0 60 40 260 cm /Im0 Do Q"; + let objs: Vec> = vec![ + b"<< /Type /Catalog /Pages 2 0 R >>".to_vec(), + b"<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 300 400] >>".to_vec(), + b"<< /Type /Page /Parent 2 0 R /Contents 4 0 R /Resources << /XObject << /Im0 5 0 R >> >> >>".to_vec(), + [format!("<< /Length {} >>\nstream\n", content.len()).as_bytes(), content.as_slice(), b"\nendstream"].concat(), + [ + b"<< /Type /XObject /Subtype /Image /Width 1 /Height 1 /BitsPerComponent 8 /ColorSpace /DeviceRGB /Length 3 >>\nstream\n".as_slice(), + &[200, 30, 30], + b"\nendstream", + ] + .concat(), + ]; + let mut bytes = b"%PDF-1.7\n".to_vec(); + let mut offsets = Vec::new(); + for (i, o) in objs.iter().enumerate() { + offsets.push(bytes.len()); + bytes.extend_from_slice(format!("{} 0 obj\n", i + 1).as_bytes()); + bytes.extend_from_slice(o); + bytes.extend_from_slice(b"\nendobj\n"); + } + let xref = bytes.len(); + bytes.extend_from_slice(format!("xref\n0 {}\n0000000000 65535 f \n", objs.len() + 1).as_bytes()); + for o in offsets { + bytes.extend_from_slice(format!("{o:010} 00000 n \n").as_bytes()); + } + bytes.extend_from_slice(format!("trailer\n<< /Size {} /Root 1 0 R >>\nstartxref\n{xref}\n%%EOF\n", objs.len() + 1).as_bytes()); + let mut h = egui_kittest::Harness::builder().with_size(vec2(1400.0, 1100.0)).build_eframe(move |_cc| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.open_bytes("picture.pdf", None, bytes.clone()).unwrap(); + app.set_option("zoom", "100").unwrap(); + app.quick_tool = QuickTool::EditText; + app + }); + let settle = |h: &mut egui_kittest::Harness<'static, PdfCraftApp>| { + for _ in 0..400 { + h.step(); + if !h.state().render_pending() { + return; + } + std::thread::sleep(std::time::Duration::from_millis(5)); + } + }; + h.run_steps(4); + settle(&mut h); + let page = h.state().views[0].page_screen_rect(0).unwrap(); + let at = |x: f32, y: f32| egui::pos2(page.left() + x / 300.0 * page.width(), page.top() + (400.0 - y) / 400.0 * page.height()); + let red = |pixels: &image::RgbaImage, r: Rect| { + let (w, hh) = (pixels.width(), pixels.height()); + (r.top().max(0.0) as u32..(r.bottom() as u32).min(hh)) + .flat_map(|y| (r.left().max(0.0) as u32..(r.right() as u32).min(w)).map(move |x| (x, y))) + .filter(|(x, y)| matches!(pixels.get_pixel(*x, *y).0, [r, g, b, _] if r > 150 && g < 90 && b < 90)) + .count() + }; + let (old, new) = (Rect::from_two_pos(at(42.0, 262.0), at(98.0, 318.0)), Rect::from_two_pos(at(182.0, 82.0), at(238.0, 138.0))); + // Select the picture and drag it, without overlap, from (40, 260) to (180, 80). + let drag = |h: &mut egui_kittest::Harness<'static, PdfCraftApp>, from: Pos2, to: Pos2, release: bool| { + h.hover_at(from); + h.run_steps(1); + h.drag_at(from); + h.run_steps(1); + for k in 1..=4 { + h.hover_at(from + (to - from) * (k as f32 / 4.0)); + h.run_steps(1); + } + if release { + h.drop_at(to); + h.step(); + } + }; + let (grab, put) = (at(70.0, 290.0), at(210.0, 110.0)); + h.hover_at(grab); + h.run_steps(1); + h.drag_at(grab); + h.run_steps(1); + h.drop_at(grab); + h.run_steps(3); + assert!(h.state().views[0].image_selection.is_some(), "the picture is selected"); + h.state_mut().views[0].hold_rasters = true; + drag(&mut h, grab, put, true); + // One tile stays stale, as a tile result arriving after the whole-page raster would; then + // the whole-page raster is let through. + let tile = h.ctx.load_texture("stale-tile", egui::ColorImage::new([1, 1], vec![Color32::TRANSPARENT]), TextureOptions::LINEAR); + h.state_mut().views[0].tiles.insert((0, 1, 0, 0), tile); + h.state_mut().views[0].stale_tiles.insert((0, 1, 0, 0)); + h.state_mut().views[0].hold_rasters = false; + for _ in 0..400 { + h.step(); + if !h.state().views[0].page_raster_out_of_date(0) { + break; + } + std::thread::sleep(std::time::Duration::from_millis(5)); + } + assert!(!h.state().views[0].page_raster_out_of_date(0), "the whole-page raster is fresh"); + assert!(h.state().views[0].page_out_of_date(0), "a tile is still stale"); + h.run_steps(2); + let pixels = h.render().unwrap(); + assert!(red(&pixels, new) > 100, "the picture shows where it went"); + assert_eq!(red(&pixels, old), 0, "and not where it was"); + // Another drag in that interval takes its pixels from where the raster has them. + let further = at(210.0, 200.0); + drag(&mut h, put, further, false); + let pixels = h.render().unwrap(); + let moving = Rect::from_two_pos(at(182.0, 172.0), at(238.0, 228.0)); + assert!(red(&pixels, moving) > 100, "the dragged picture is drawn from the fresh raster"); + assert_eq!(red(&pixels, old), 0, "nothing shows where it first was"); + h.state_mut().views[0].stale_tiles.clear(); + h.state_mut().views[0].tiles.clear(); + } } diff --git a/crates/ui-egui/src/edit_text_ui.rs b/crates/ui-egui/src/edit_text_ui.rs index b9b675cc37..5e0e401dce 100644 --- a/crates/ui-egui/src/edit_text_ui.rs +++ b/crates/ui-egui/src/edit_text_ui.rs @@ -462,6 +462,9 @@ pub struct ImageSelection { pub crop: Option, /// The drag is turning the picture by its rotate handle (the `drag` start is then unused). turning: bool, + /// A move or resize applied on release: the frame it had and the one it got, in the page's + /// 0..1 coordinates, while the page raster still has the picture where it was. + settled: Option<(Rect, Rect)>, } /// Cropping a page image: the handles trim instead of resizing, the part that will go is @@ -637,6 +640,28 @@ impl ImageSelection { } } +/// The page raster's picture of the box `from`, stretched into `to`, over blanks at `blank` (where +/// the screen may still show the picture). +fn stretched_picture(painter: &egui::Painter, xf: &PageXform, tex: egui::TextureId, blank: &[Rect], from: Rect, to: Rect) { + let clipped = painter.with_clip_rect(painter.clip_rect().intersect(xf.rect)); + for r in blank { + clipped.rect_filled(*r, CornerRadius::ZERO, Color32::WHITE); + } + let mut mesh = egui::Mesh::with_texture(tex); + for (a, b) in [ + (from.left_top(), to.left_top()), + (from.right_top(), to.right_top()), + (from.right_bottom(), to.right_bottom()), + (from.left_bottom(), to.left_bottom()), + ] { + let (u, v) = xf.screen_to_norm(a); + mesh.vertices.push(egui::epaint::Vertex { pos: b, uv: Pos2::new(u, v), color: Color32::WHITE }); + } + mesh.add_triangle(0, 1, 2); + mesh.add_triangle(0, 2, 3); + clipped.add(egui::Shape::mesh(mesh)); +} + /// Once a selected corner or the rotate knob is grabbed, text under it cannot take over the gesture. The same /// applies after the pointer leaves the image/page while dragging. pub(crate) fn image_owns_pointer(ui: &egui::Ui, xf: &PageXform, page: usize, info: &DocInfo, view: &DocView) -> bool { @@ -797,14 +822,17 @@ pub(crate) fn image_input( // hovered boxes, so a ~1000-image CAD page stays interactive in Edit text & images. // Fully clipped placements (empty visible frames) get no outline. let faint_all = boxes.len() <= MAX_OUTLINES; + // The picture being dragged has its frame at the pointer; its outline where it was would + // trail behind until the release. + let moving = selected.filter(|_| dragging); if faint_all { - for b in boxes.iter().filter(|b| b.is_positive()) { + for (_, b) in boxes.iter().enumerate().filter(|(i, b)| b.is_positive() && moving != Some(*i)) { painter.rect_stroke(*b, CornerRadius::ZERO, Stroke::new(0.75, ACCENT.gamma_multiply(0.35)), egui::StrokeKind::Outside); } } let hovered_box = hover_pos.and_then(|p| boxes.iter().enumerate().rposition(|(i, b)| b.is_positive() && b.contains(p) && image_contains(i, p))); if !faint_all { - for i in selected.into_iter().chain(hovered_box.filter(|h| Some(*h) != selected)) { + for i in selected.filter(|_| moving.is_none()).into_iter().chain(hovered_box.filter(|h| Some(*h) != selected)) { if let Some(b) = boxes.get(i).filter(|b| b.is_positive()) { painter.rect_stroke(*b, CornerRadius::ZERO, Stroke::new(0.75, ACCENT.gamma_multiply(0.35)), egui::StrokeKind::Outside); } @@ -839,6 +867,26 @@ pub(crate) fn image_input( ); } }; + // Until the page is rendered again its raster has an applied move or resize's picture + // where it was: it stays blanked there and drawn where it went. + // The whole-page raster and its tiles are replaced separately: the old frame stays blanked + // while anything still shows the picture there, but its pixels come from the old frame + // only while the whole-page raster itself is the old one. Once that is fresh it has the + // picture where it went, and that is where they come from. + let (out_of_date, raster_old, raster) = (view.page_out_of_date(page), view.page_raster_out_of_date(page), view.page_raster(page)); + let screen = |r: Rect| Rect::from_two_pos(xf.norm_to_screen(r.min.x, r.min.y), xf.norm_to_screen(r.max.x, r.max.y)); + // Where the screen may still show the picture from before an applied move, while the page + // isn't drawn again yet. Another drag keeps it blanked too. + let left_at = view.image_selection.as_ref().and_then(|s| s.settled).filter(|_| out_of_date).map(|(from, _)| from); + if let Some(s) = view.image_selection.as_mut().filter(|s| s.drag.is_none()) { + match (s.settled, raster) { + (Some((from, to)), Some(tex)) if out_of_date => { + let source = if raster_old { from } else { to }; + stretched_picture(painter, xf, tex, &[screen(from)], screen(source), screen(to)); + } + _ => s.settled = None, + } + } if view.image_selection.as_ref().is_some_and(|s| s.drag.is_none()) { painter.rect_stroke(b, CornerRadius::ZERO, Stroke::new(1.5, ACCENT), egui::StrokeKind::Outside); handles(b); @@ -943,21 +991,10 @@ pub(crate) fn image_input( if preview != b && let Some(tex) = view.page_raster(page) { - let clipped = painter.with_clip_rect(painter.clip_rect().intersect(xf.rect)); - clipped.rect_filled(b, CornerRadius::ZERO, Color32::WHITE); - let mut mesh = egui::Mesh::with_texture(tex); - for (from, to) in [ - (b.left_top(), preview.left_top()), - (b.right_top(), preview.right_top()), - (b.right_bottom(), preview.right_bottom()), - (b.left_bottom(), preview.left_bottom()), - ] { - let (u, v) = xf.screen_to_norm(from); - mesh.vertices.push(egui::epaint::Vertex { pos: to, uv: Pos2::new(u, v), color: Color32::WHITE }); - } - mesh.add_triangle(0, 1, 2); - mesh.add_triangle(0, 2, 3); - clipped.add(egui::Shape::mesh(mesh)); + // Its pixels: where the whole-page raster has it. + let source = left_at.filter(|_| raster_old).map_or(b, screen); + let blank: Vec = left_at.map(screen).into_iter().chain(std::iter::once(b)).collect(); + stretched_picture(painter, xf, tex, &blank, source, preview); } painter.rect_filled(visible, CornerRadius::ZERO, ACCENT.gamma_multiply(0.04)); painter.rect_stroke(visible, CornerRadius::ZERO, Stroke::new(1.5, ACCENT), egui::StrokeKind::Outside); @@ -966,6 +1003,11 @@ pub(crate) fn image_input( if resp.drag_stopped() || !ui.input(|i| i.pointer.primary_down()) { if let Some(s) = view.image_selection.as_mut() { s.drag = None; + let norm = |r: Rect| { + let ((u0, v0), (u1, v1)) = (xf.screen_to_norm(r.min), xf.screen_to_norm(r.max)); + Rect::from_two_pos(Pos2::new(u0, v0), Pos2::new(u1, v1)) + }; + s.settled = (preview != b).then(|| (left_at.unwrap_or_else(|| norm(b)), norm(preview))); } if preview != b { view.pending_edit = @@ -1021,7 +1063,7 @@ pub(crate) fn image_input( _ => egui::CursorIcon::PointingHand, }); if resp.clicked() || resp.secondary_clicked() { - view.image_selection = Some(ImageSelection { page, index: hit, drag: None, crop: None, turning: false }); + view.image_selection = Some(ImageSelection { page, index: hit, drag: None, crop: None, turning: false, settled: None }); } if resp.secondary_clicked() { ui.ctx().data_mut(|d| d.insert_temp(menu_key, true)); diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 2829ec89fb..bfc9e00d65 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -2166,11 +2166,10 @@ impl PdfCraftApp { } } - /// Select the workspace and its matching tool panel, just like the mode bar. Edit turns on - /// Edit text & images at once, as Acrobat does: every paragraph and image on the page gets its - /// box and is editable with one click. Leaving Edit puts the editing tools down again. + /// Select the workspace and its matching tool panel, just like the mode bar. The tool goes + /// with the workspace as [`Self::enter_mode`] sets it. pub(crate) fn select_mode(&mut self, mode: Mode) { - self.mode = mode; + self.enter_mode(mode); self.reveal_tools(); if let Some(view) = self.active.and_then(|i| self.views.get_mut(i)) { view.organize = false; @@ -2181,6 +2180,15 @@ impl PdfCraftApp { Mode::Sign => LeftPanel::Tool("fill_sign"), _ => LeftPanel::AllTools, }; + } + + /// Changes the workspace and the tool that goes with it. The mode bar (through + /// [`Self::select_mode`]) and the Read mode command both come here, so they leave Edit alike. + /// Edit turns on Edit text & images at once, as Acrobat does: every paragraph and image on the + /// page gets its box and is editable with one click. Leaving Edit puts the editing tools down + /// again and applies a paragraph still open in the editor. + fn enter_mode(&mut self, mode: Mode) { + self.mode = mode; if mode == Mode::Edit { self.quick_tool = QuickTool::EditText; } else if matches!(self.quick_tool, QuickTool::EditText | QuickTool::AddText) { @@ -2214,7 +2222,7 @@ impl PdfCraftApp { if select_tools { self.select_mode(mode); } else { - self.mode = mode; + self.enter_mode(mode); } if mode == Mode::Read && !was_read { self.right_before_read = self.right.take(); diff --git a/crates/ui-egui/src/marks_ui.rs b/crates/ui-egui/src/marks_ui.rs index d91a69f937..bc84aa1588 100644 --- a/crates/ui-egui/src/marks_ui.rs +++ b/crates/ui-egui/src/marks_ui.rs @@ -3,7 +3,7 @@ //! preview of the current page. "Update" opens the same dialog in replace mode. use egui::{Align, Color32, CornerRadius, Layout, Rect, Stroke, pos2, vec2}; -use pdfcraft_engine::{Background, Edit, HeaderFooter, MarkKind, Watermark}; +use pdfcraft_engine::{Background, Edit, FontFamily, HeaderFooter, MarkKind, Watermark}; use crate::theme::{self, Tokens}; use crate::{PdfCraftApp, widgets}; @@ -160,7 +160,13 @@ pub(crate) fn body(ui: &mut egui::Ui, app: &mut PdfCraftApp, t: &Tokens, kind: M match kind { MarkKind::HeaderFooter => { ui.horizontal(|ui| { - ui.label(tl!("Font: Helvetica")); + // The standard faces a header can be drawn in (#3754). + ui.label(tl!("Font:")); + egui::ComboBox::from_id_salt("hf-font").selected_text(d.hf.font.label()).width(100.0).show_ui(ui, |ui| { + for f in [FontFamily::Helvetica, FontFamily::Times, FontFamily::Courier] { + ui.selectable_value(&mut d.hf.font, f, f.label()); + } + }); ui.label(tl!("Size")); ui.add(egui::DragValue::new(&mut d.hf.font_size).range(4.0..=72.0).speed(0.5)); ui.add(widgets::checkbox(&mut d.hf.underline, tl!("Underline"))); @@ -322,7 +328,10 @@ pub(crate) fn body(ui: &mut egui::Ui, app: &mut PdfCraftApp, t: &Tokens, kind: M 1 => (r.center().x, egui::Align2::CENTER_CENTER), _ => (r.right() - hf.margins[3] as f32 * s, egui::Align2::RIGHT_CENTER), }; - p.text(pos2(x, y), align, shown, egui::FontId::proportional((hf.font_size as f32 * s).max(5.0)), rgb32(hf.color)); + // Courier previews in the monospaced UI face, the others proportional. + let size = (hf.font_size as f32 * s).max(5.0); + let font = if hf.font == FontFamily::Courier { egui::FontId::monospace(size) } else { egui::FontId::proportional(size) }; + p.text(pos2(x, y), align, shown, font, rgb32(hf.color)); } } MarkKind::Watermark if file_name.is_some() => { diff --git a/crates/ui-egui/tests/editing.rs b/crates/ui-egui/tests/editing.rs index 1d38c6058b..03e358c3b5 100644 --- a/crates/ui-egui/tests/editing.rs +++ b/crates/ui-egui/tests/editing.rs @@ -3081,6 +3081,40 @@ fn editing_existing_text_in_place() { assert_eq!(texts_of(s, 0), ["Chapter One"], "the page shows it"); } +/// The Read mode command (View ▸ Read mode, its shortcut, the palette) leaves Edit as the Edit tab +/// does when another tab is chosen (#2241): Edit text & images is put down, and a paragraph still +/// open in the editor is applied instead of staying open, editable, over the page in Read mode. +#[test] +fn the_read_mode_command_applies_an_open_paragraph_and_puts_edit_text_down() { + use pdfcraft_ui_egui::{Mode, QuickTool}; + let mut h = harness(1, |_| {}); + h.get_by_label("Edit").click(); + h.run_steps(3); + assert_eq!((h.state().mode, h.state().quick_tool), (Mode::Edit, QuickTool::EditText)); + // Open the line "Page 1" (at (20, 150) on a 200 × 300 page, 24 pt) and retype it. + let r = h.state().views[0].page_screen_rect(0).expect("on screen"); + let at = egui::pos2(r.left() + 40.0 / 200.0 * r.width(), r.top() + (300.0 - 158.0) / 300.0 * r.height()); + h.hover_at(at); + h.run_steps(1); + h.drag_at(at); + h.run_steps(1); + h.drop_at(at); + h.run_steps(3); + assert_eq!(h.state().views[0].line_editor.as_ref().expect("the line opens for editing").text, "Page 1"); + h.state_mut().views[0].line_editor.as_mut().unwrap().text = "Chapter One".into(); + h.run_steps(1); + assert!(h.state_mut().execute("view.read_mode")); + h.run_steps(3); + let s = h.state(); + assert_eq!(s.mode, Mode::Read); + assert_eq!(s.quick_tool, QuickTool::Select, "Edit text & images is put down"); + assert!(s.views[0].line_editor.is_none(), "no paragraph editor is left open in Read mode"); + let doc = s.session.get(s.views[0].id).unwrap(); + assert_eq!(doc.can_undo(), Some("Edit text"), "the open paragraph's edit is applied"); + assert_eq!(doc.text_lines(0)[0].text, "Chapter One"); + assert_eq!(texts_of(s, 0), ["Chapter One"], "the page shows it"); +} + #[test] fn text_tool_clicking_source_text_edits_instead_of_adding_a_copy() { let mut h = harness(1, |_| {}); @@ -4873,6 +4907,16 @@ fn image_pixels_follow_the_frame_while_a_corner_is_dragged() { assert!(h.state_mut().execute("edit.edit_text")); h.state_mut().views[0].set_zoom(1.2); h.run_steps(4); + // The dragged pixels are the page raster's: wait for it, as a slow runner may not have it yet + // (the drag then drew no picture at all). + for _ in 0..400 { + h.run_steps(1); + if !h.state().render_pending() { + break; + } + std::thread::sleep(std::time::Duration::from_millis(5)); + } + assert!(!h.state().render_pending(), "the page raster arrives"); let page = h.state().views[0].page_screen_rect(0).unwrap(); let at = |x: f32, y: f32| egui::pos2(page.left() + x / 200.0 * page.width(), page.top() + (300.0 - y) / 300.0 * page.height()); let p = at(95.0, 195.0); @@ -4943,6 +4987,110 @@ fn image_pixels_follow_the_frame_while_a_corner_is_dragged() { } } +/// Red pixels in `r`: the test picture. +fn red(pixels: &image::RgbaImage, r: egui::Rect) -> usize { + let (x0, y0, x1, y1) = (r.left().max(0.0) as u32, r.top().max(0.0) as u32, r.right() as u32, r.bottom() as u32); + (y0..y1.min(pixels.height())) + .flat_map(|y| (x0..x1.min(pixels.width())).map(move |x| (x, y))) + .filter(|(x, y)| matches!(pixels.get_pixel(*x, *y).0, [r, g, b, _] if r > 150 && g < 90 && b < 90)) + .count() +} + +/// A resized picture is applied when the corner is let go, but the page raster still has it at +/// its old size until the page is rendered again. Until then it stays drawn at its new size and +/// blanked at its old one; the old one used to show again for those frames. +#[test] +fn a_resized_image_keeps_its_new_size_until_the_page_is_redrawn() { + use pdfcraft_cos::{Dict, Document, Object, Stream, write_incremental}; + let mut doc = Document::open(std::sync::Arc::new(fixture(1))).unwrap(); + let mut image = Dict::new(); + image.set(b"Type".to_vec(), Object::name("XObject")); + image.set(b"Subtype".to_vec(), Object::name("Image")); + image.set(b"Width".to_vec(), Object::Int(1)); + image.set(b"Height".to_vec(), Object::Int(1)); + image.set(b"BitsPerComponent".to_vec(), Object::Int(8)); + image.set(b"ColorSpace".to_vec(), Object::name("DeviceRGB")); + let image = doc.add(Object::Stream(Stream::flate(image, &[200, 30, 30]))); + let page = pdfcraft_model::pages(&doc).remove(0); + let contents = doc.add(Object::Stream(Stream::flate(Dict::new(), b"q 80 0 0 80 60 40 cm /Im0 Do Q"))); + let mut resources = doc.resolve(page.dict.get(b"Resources").unwrap()).as_dict().unwrap().clone(); + let mut xo = Dict::new(); + xo.set(b"Im0".to_vec(), Object::Ref(image)); + resources.set(b"XObject".to_vec(), Object::Dict(xo)); + doc.update_dict(page.obj, |d| { + d.set(b"Resources".to_vec(), Object::Dict(resources)); + d.set(b"Contents".to_vec(), Object::Ref(contents)); + }) + .unwrap(); + let bytes = write_incremental(&doc, &Default::default()).unwrap(); + let mut h = Harness::builder().with_size(egui::vec2(1400.0, 1100.0)).build_eframe(move |_cc| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.open_bytes("picture.pdf", None, bytes).unwrap(); + app + }); + h.run_steps(4); + assert!(h.state_mut().execute("edit.edit_text")); + h.state_mut().views[0].set_zoom(1.2); + h.run_steps(4); + support::wait_until(&mut h, "the page raster", |app| !app.render_pending()); + let page = h.state().views[0].page_screen_rect(0).unwrap(); + let at = |x: f32, y: f32| egui::pos2(page.left() + x / 200.0 * page.width(), page.top() + (300.0 - y) / 300.0 * page.height()); + click_at(&mut h, at(100.0, 80.0)); + assert!(h.state().views[0].image_selection.is_some(), "the picture is selected"); + support::wait_until(&mut h, "the page raster", |app| !app.render_pending()); + // From 60..140 × 40..120 to half its size, keeping its lower left corner. + let (grip, to) = (at(140.0, 120.0), at(100.0, 80.0)); + let kept = egui::Rect::from_two_pos(at(62.0, 42.0), at(98.0, 78.0)); + let gone = egui::Rect::from_two_pos(at(104.0, 84.0), at(138.0, 118.0)); + h.hover_at(grip); + h.run_steps(1); + h.drag_at(grip); + h.run_steps(1); + for k in 1..=4 { + h.hover_at(grip + (to - grip) * (k as f32 / 4.0)); + h.run_steps(1); + } + // While dragged, no outline stays where the picture was. + let old = egui::Rect::from_two_pos(at(60.0, 40.0), at(140.0, 120.0)); + let trailing = h.output().shapes.iter().any(|shape| { + matches!(&shape.shape, egui::Shape::Rect(r) if r.stroke.width == 0.75 && (r.rect.min - old.min).length() < 2.0 && (r.rect.max - old.max).length() < 2.0) + }); + assert!(!trailing, "the dragged picture's outline trails behind it"); + h.state_mut().views[0].hold_rasters = true; + h.drop_at(to); + for frame in 0..4 { + h.step(); + let pixels = h.render().unwrap(); + assert!(red(&pixels, kept) > 100, "frame {frame} after the release: the picture is not at its new size"); + assert_eq!(red(&pixels, gone), 0, "frame {frame} after the release: the picture shows at its old size"); + } + // Resized again before the page is drawn: the raster still has the first size, so that is + // what stays blanked while the corner moves. + let (grip, to) = (to, at(80.0, 60.0)); + let again = egui::Rect::from_two_pos(at(62.0, 42.0), at(78.0, 58.0)); + h.hover_at(grip); + h.run_steps(1); + h.drag_at(grip); + h.run_steps(1); + for k in 1..=4 { + h.hover_at(grip + (to - grip) * (k as f32 / 4.0)); + h.run_steps(1); + } + let pixels = h.render().unwrap(); + assert!(red(&pixels, again) > 20, "the second drag draws the picture at its new size"); + assert_eq!(red(&pixels, gone), 0, "the second drag shows the picture at its first size"); + h.drop_at(to); + h.run_steps(2); + let pixels = h.render().unwrap(); + assert_eq!(red(&pixels, gone), 0, "after the second release"); + h.state_mut().views[0].hold_rasters = false; + support::wait_until(&mut h, "the page drawn with the resized picture", |app| !app.render_pending()); + h.run_steps(2); + let pixels = h.render().unwrap(); + assert!(red(&pixels, again) > 20 && red(&pixels, gone) == 0, "the page raster has the picture at its new size"); +} + /// Exercise actual pointer gestures on each cropped placement, without capturing private pixels. #[test] #[ignore = "requires an explicitly supplied local PDF with cropped artwork"] diff --git a/crates/ui-egui/tests/ui_group/marks.rs b/crates/ui-egui/tests/ui_group/marks.rs index 1f1515ad60..e0d86c4ab3 100644 --- a/crates/ui-egui/tests/ui_group/marks.rs +++ b/crates/ui-egui/tests/ui_group/marks.rs @@ -58,6 +58,33 @@ fn header_and_footer_dialog_inserts_tokens_and_applies_to_a_subset() { assert!(marks(&h).is_empty()); } +#[test] +fn header_and_footer_dialog_draws_in_the_chosen_face() { + // #3754: the dialog offers Helvetica, Times and Courier (it only said "Font: Helvetica"), + // and the face chosen is the one the page's header is drawn in. + let mut h = harness(); + assert!(h.state_mut().execute("edit.header_footer")); + h.run_steps(3); + h.get_by_label("Font:"); + h.get_by_value("Helvetica").click(); + h.run_steps(2); + h.get_by_label("Times").click(); + h.run_steps(2); + assert_eq!(h.state().marks_draft.hf.font, pdfcraft_engine::FontFamily::Times); + h.state_mut().marks_draft.hf.text[0] = "Serif".into(); + h.run_steps(2); + h.get_by_label("OK").click(); + h.run_steps(4); + assert_eq!(marks(&h), [MarkKind::HeaderFooter]); + let s = h.state(); + let doc = pdfcraft_cos::Document::open(s.session.get(s.views[0].id).unwrap().bytes.clone()).unwrap(); + let page = &pdfcraft_model::pages(&doc)[0]; + let res = doc.resolve(page.dict.get(b"Resources").unwrap()); + let fonts = doc.resolve(res.as_dict().unwrap().get(b"Font").unwrap()).as_dict().cloned().unwrap(); + let face = doc.resolve(fonts.get(b"PCTimes").expect("Times is added")).as_dict().cloned().unwrap(); + assert_eq!(face.name(b"BaseFont"), Some(&b"Times-Roman"[..])); +} + #[test] fn watermark_and_background_dialogs() { let mut h = harness(); diff --git a/docs/roadmap.md b/docs/roadmap.md index 201cc1d6c3..07a50c8488 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -48,6 +48,7 @@ > **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-11 · **Change:** minor (Traditional Chinese accessibility semantics and Taiwan form/table terminology; estimates unchanged) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) > **Last reviewed:** 2026-10-12 · **Last updated:** 2026-10-12 · **Change:** patch (M1 #1337: exhausted xref hints are repaired; allocation avoids overflow; estimates unchanged) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) > **Last reviewed:** 2026-10-18 - **Last updated:** 2026-10-18 - **Change:** minor (M6 #2892: Push-button normal labels are editable per widget through Options and form_set_props, independently of field names; rendering, undo/redo and incremental/full save/reopen are covered. Estimates unchanged) +> **Last reviewed:** 2026-10-18 - **Last updated:** 2026-10-18 - **Change:** minor (M7 #3187: Added-image automation normalizes extreme signed rotation angles before combining them with the current orientation; integer bounds, history, pixels, save/reopen and MCP have regressions. Estimates unchanged) Forward-looking: the milestones, the current focus and what comes next. The parity numbers behind it are in [target-app-parity.md](target-app-parity.md), the ranked work list in [gaps.md](gaps.md), @@ -139,7 +140,7 @@ total matches the parity estimate. | M6 | Forms + JavaScript | 160–320 | 50% | 40–75 | Done: filling all field types with regenerated appearances; Prepare a form (every field type, multi-select/shared properties, move/resize/delete, Field Properties General/Appearance/Position/Options/Format/Validate/Calculate); Acrobat's AF format/keystroke/validate/calculate functions and simplified field notation run natively in Acrobat's event order; tab order; push-button actions; flatten; agent tools. Form data exchange (FDF, XFDF, XML, CSV, text) done; XFDF values retain text across XML comments and processing instructions. Plain AcroForm XML imports retain all direct text around comments and processing instructions, including CDATA and character references.
Done since: JavaScript engine (boa, sandboxed) running custom keystroke/validate/calculate/format and button scripts, document JavaScripts, console. Missing: Actions tab, wider object model (annotations, layers, dialogs), XFA
Done since: Multi-select choice fields write ascending `/I` option indices while preserving `/V` selection order; synthetic forms/engine regressions cover reverse selection and save/reopen. CSV export preserves carriage returns in names and values. | | M6 | Forms + JavaScript | 160–320 | 50% | 40–75 | Done: filling all field types with regenerated appearances; Prepare a form (every field type, multi-select/shared properties, move/resize/delete, Field Properties General/Appearance/Position/Options/Format/Validate/Calculate); Acrobat's AF format/keystroke/validate/calculate functions and simplified field notation run natively in Acrobat's event order; tab order; push-button actions; flatten; agent tools. Form data exchange (FDF, XFDF, XML, CSV, text) done; XFDF values retain text across XML comments and processing instructions. XML/XFDF exports preserve tabs and line breaks in names and comment metadata. Refreshed against upstream `99941daa` (2026-10-15).
Done since: JavaScript engine (boa, sandboxed) running custom keystroke/validate/calculate/format and button scripts, document JavaScripts, console. Missing: Actions tab, wider object model (annotations, layers, dialogs), XFA
Done since: Multi-select choice fields write ascending `/I` option indices while preserving `/V` selection order; synthetic forms/engine regressions cover reverse selection and save/reopen. CSV export preserves carriage returns in names and values. | | M6 | Forms + JavaScript | 160–320 | 50% | 40–75 | Done: filling all field types with regenerated appearances; Prepare a form (every field type, multi-select/shared properties, move/resize/delete, Field Properties General/Appearance/Position/Options/Format/Validate/Calculate); Acrobat's AF format/keystroke/validate/calculate functions and simplified field notation run natively in Acrobat's event order; tab order; push-button actions; flatten; agent tools. Form data exchange (FDF, XFDF, XML, CSV, text) done; XFDF values retain text across XML comments and processing instructions. XML/XFDF exports preserve tabs and line breaks in names and comment metadata. Plain AcroForm XML imports retain all direct text around comments and processing instructions, including CDATA and character references. CSV export and spreadsheet merging quote tabs, and import distinguishes quoted tabs from TSV delimiters without changing field identities.
Done since: JavaScript engine (boa, sandboxed) running custom keystroke/validate/calculate/format and button scripts, document JavaScripts, console. Missing: Actions tab, wider object model (annotations, layers, dialogs), XFA
Done since: Multi-select choice fields write ascending `/I` option indices while preserving `/V` selection order; synthetic forms/engine regressions cover reverse selection and save/reopen. CSV export preserves carriage returns in names and values. | -| M7 | Content editing (text, images, header/footer, watermark) | 250–500 | 20% | 100–180 | Done: header & footer, watermarks, backgrounds, Bates; added text and images that stay editable (move, resize, format, rotate, flip, crop, replace); links (Link tool, Link Properties, create from URLs, remove all). Done since: Edit text preserves Arabic/Hebrew `/ActualText` typing order, with font-independent regressions; single-glyph RTL ligature spans (including lam-alef) join their surrounding word. Done since (#1116): editable text regions grouped across compatible words and subset fonts, one rich-text editor over a background with the source text removed, embedded sfnt faces reused as editor faces, and visible (clipped) bounds for cropped-image selection and resize. Exclusive crops now resize across nested states and content streams, with shared clips fixed.
Done since: #594 ordinary paragraph drags preserve overlapping source text; UI/engine regressions, native save/reopen, independent-reader checks and full CI passed in the Linux test environment. #1200 candidate: local GoToR links open in desktop tabs and through root-confined automation; appearance-only edits preserve existing actions. Named destinations remain unapplied and `/NewWindow` is a tab hint. Missing: editing existing text and images in place (the longest pole), image/PDF watermarks | +| M7 | Content editing (text, images, header/footer, watermark) | 250–500 | 20% | 100–180 | Done: header & footer, watermarks, backgrounds, Bates; added text and images that stay editable (move, resize, format, rotate, flip, crop, replace); links (Link tool, Link Properties, create from URLs, remove all). Done since: Edit text preserves Arabic/Hebrew `/ActualText` typing order, with font-independent regressions; single-glyph RTL ligature spans (including lam-alef) join their surrounding word. Done since (#1116): editable text regions grouped across compatible words and subset fonts, one rich-text editor over a background with the source text removed, embedded sfnt faces reused as editor faces, and visible (clipped) bounds for cropped-image selection and resize. Exclusive crops now resize across nested states and content streams, with shared clips fixed.
Done since: #594 ordinary paragraph drags preserve overlapping source text; UI/engine regressions, native save/reopen, independent-reader checks and full CI passed in the Linux test environment. #1200 candidate: local GoToR links open in desktop tabs and through root-confined automation; appearance-only edits preserve existing actions. Named destinations remain unapplied and `/NewWindow` is a tab hint. Done since: #3187: Added-image automation normalizes extreme signed rotation angles before combining them with the current orientation; integer bounds, history, pixels, save/reopen and MCP have regressions. Missing: editing existing text and images in place (the longest pole), image/PDF watermarks | | M7 | Content editing (text, images, header/footer, watermark) | 250–500 | 20% | 100–180 | Done: header & footer, watermarks, backgrounds, Bates; added text and images that stay editable (move, resize, format, rotate, flip, crop, replace); links (Link tool, Link Properties, create from URLs, remove all). Done since: Edit text preserves Arabic/Hebrew `/ActualText` typing order, with font-independent regressions. Done since (#1116): editable text regions grouped across compatible words and subset fonts, one rich-text editor over a background with the source text removed, embedded sfnt faces reused as editor faces, and visible (clipped) bounds for cropped-image selection and resize. Exclusive crops now resize across nested states and content streams, with shared clips fixed.
Done since: #594 ordinary paragraph drags preserve overlapping source text; UI/engine regressions, native save/reopen, independent-reader checks and full CI passed in the Linux test environment. #1200 candidate: local GoToR links open in desktop tabs and through root-confined automation; appearance-only edits preserve existing actions. Named destinations remain unapplied and `/NewWindow` is a tab hint. #1316: replacing a line or paragraph keeps a neighbouring line whose glyph box overlaps when the baselines differ; a second copy on the same baseline is still replaced. Missing: editing existing text and images in place (the longest pole), image/PDF watermarks | | M7 | Content editing (text, images, header/footer, watermark) | 250–500 | 20% | 100–180 | Done: header & footer, watermarks, backgrounds, Bates; added text and images that stay editable (move, resize, format, rotate, flip, crop, replace); links (Link tool, Link Properties, create from URLs, remove all). #3212: Headers, Bates numbers, watermarks and added text stay visible after unfinished marked content, without revealing hidden content. Tracked editable text distinguishes repeated glyph spacing from word gaps, with synthetic paragraph/object regressions. Missing: editing existing text and images in place (the longest pole), watermark print/screen visibility and repeat | | M6 | Forms + JavaScript | 160–320 | 50% | 40–75 | Done: filling all field types with regenerated appearances; Prepare a form (every field type, multi-select/shared properties, move/resize/delete, Field Properties General/Appearance/Position/Options/Format/Validate/Calculate); Acrobat's AF format/keystroke/validate/calculate functions and simplified field notation run natively in Acrobat's event order; tab order; push-button actions; flatten; agent tools. Form data exchange (FDF, XFDF, XML, CSV, text) done; XFDF values retain text across XML comments and processing instructions.
Done since: JavaScript engine (boa, sandboxed) running custom keystroke/validate/calculate/format and button scripts, document JavaScripts, console. Missing: Actions tab, wider object model (annotations, layers, dialogs), XFA
Done since: Existing simple field-font encodings and Differences are respected when generating appearances; an unencodable value uses a separate Helvetica appearance font without changing the source font. Synthetic reopen regressions cover punctuation, subset fallback and multiline text. Exact font metrics/layout and broader Unicode fallback remain incomplete. | diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 954f836877..b999d67779 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -1962,8 +1962,8 @@ milestone = "M7" status = "shipped" commands = ["edit.edit_text", "edit.text"] tools = ["text_edit", "content_update"] -evidence = ["crates/edit/src/tests.rs::justify_underline_and_spacing", "crates/edit/src/tests.rs::paragraphs_take_new_formatting", "crates/ui-egui/tests/editing.rs::the_format_panel_restyles_the_paragraph_being_edited"] -notes = "left, centre, right and justify (word spacing per line, last line left) for existing paragraphs and added text" +evidence = ["crates/edit/src/tests.rs::justify_underline_and_spacing", "crates/edit/src/tests.rs::paragraphs_take_new_formatting", "crates/edit/src/tests.rs::a_one_row_title_keeps_its_centre_when_it_grows_and_when_it_wraps", "crates/ui-egui/tests/editing.rs::the_format_panel_restyles_the_paragraph_being_edited"] +notes = "left, centre, right and justify (word spacing per line, last line left) for existing paragraphs and added text. A centred paragraph is found either from its ragged rows or, for a one row title, from its centre sitting on the page's own centre away from the edge the rest of the text uses, so growing or wrapping it keeps that centre (#2581)" [[feature]] id = "edit.line-paragraph-spacing" @@ -2117,9 +2117,9 @@ tier = "P0" milestone = "M7" status = "shipped" commands = ["edit.edit_text"] -tools = ["image_edit"] -evidence = ["crates/edit/src/tests.rs::page_images_move_turn_replace_and_delete", "crates/ui-egui/tests/editing.rs::editing_existing_images_on_the_page", "crates/automation/tests/automation.rs::editing_page_images_through_tools"] -notes = "existing raster images and grouped Form artwork: rotate either way about the centre, flip horizontally or vertically (a q/cm/Q wrapper); Forms stay intact as groups; added images too" +tools = ["image_edit", "content_update"] +evidence = ["crates/edit/src/tests.rs::page_images_move_turn_replace_and_delete", "crates/ui-egui/tests/editing.rs::editing_existing_images_on_the_page", "crates/automation/tests/automation.rs::editing_page_images_through_tools", "crates/automation/tests/automation.rs::large_added_image_rotations_keep_orientation_and_undo", "crates/automation/tests/automation.rs::large_added_image_rotations_round_trip_through_mcp"] +notes = "Existing raster images and grouped Form artwork: rotate either way about the centre, flip horizontally or vertically (a q/cm/Q wrapper); Forms stay intact as groups; added images too. Added-image content_update rotation accepts positive or negative integer multiples of 90 degrees; reducing the angle before addition prevents overflow at signed 64-bit limits and preserves the current orientation through full turns, Undo/Redo and save/reopen. Added-image rotations remain limited to 90-degree steps." [[feature]] id = "edit.image-move-resize" @@ -2401,7 +2401,7 @@ status = "shipped" commands = ["edit.header_footer"] tools = ["doc_header_footer"] evidence = ["crates/ui-egui/tests/ui_group/marks.rs::header_and_footer_dialog_inserts_tokens_and_applies_to_a_subset", "crates/edit/src/tests.rs::header_and_footer_are_drawn_in_display_space_and_wrap_the_original_content", "crates/engine/src/tests.rs::headers_footers_watermarks_and_backgrounds_show_update_and_remove"] -notes = "Helvetica only (no font choice yet); drawn in display space on rotated pages; artifact-tagged" +notes = "Standard faces only: Helvetica, Times or Courier (#3754; no bold, italic or embedded fonts yet); drawn in display space on rotated pages; artifact-tagged" [[feature]] id = "edit.header-footer-update-remove"