From c65f32a0782c7bf28fe53c9389f760c75fb8eecd Mon Sep 17 00:00:00 2001 From: Maksim Popov <51298369+MaksPV@users.noreply.github.com> Date: Mon, 12 Oct 2026 00:28:30 +0300 Subject: [PATCH] Content edits on heavy documents: narrow refresh, cached appearances, per-page invalidation (#1122) * Content edits on heavy documents: narrow refresh, cached appearances, per-page invalidation Every content edit (drag, resize, retype) on a large document ran a full synchronous refresh on the UI thread: a whole-file re-inspection plus a redraw of every missing annotation appearance, then dropped all page rasters. On a 72 MB / 202-page CAD file with 27k appearance-less annotations each commit cost ~1.5 s before anything re-rendered. Now ~0.3 s. - New Scope::Content for edits that touch only page content, resources and added content/marks (move/resize/retype/images/added items/marks): the page tree, annotations, fields, links, outline and fonts are unchanged, so refresh_content rebuilds working bytes, display bytes and renderer and validates with a light check_pages (parse plus page count, keeping the roll-back guarantee) instead of a full inspect. added/marks are re-read only when the edit can change them; undo/redo re-read both. - AppearanceCache in pdfcraft-annot: built display appearances are reused while no annotation edit runs (content edits leave every annotation dictionary alone); cleared on comment, structural and full refreshes. set_appearance is split into build/apply halves so a hit skips the draw. - check_pages in pdfcraft-render: light post-edit validation. - Viewer: single-page content edits invalidate only their page (page_changed); other pages keep tiles, text and thumbnails. - XFA dataset sync now also runs for content edits. Co-Authored-By: Muse Spark * Fonts added by a content edit show in Properties and doc_info (read again on demand); form edits clear cached appearances too * M14: DATE_LANGUAGES is a static (21 entries trip clippy's large_const_arrays); rustfmt the Greek entry Co-Authored-By: Claude Opus 5.5 * Preferences scroll in a short window; tests scroll to controls below the fold (#487's roomier rows); the highlighter test waits for its mark under load Co-Authored-By: Claude Opus 5.5 * Select tool: a multi-click selects only on the frame egui reports it, so a cleared selection (or the highlighter's marked word) isn't selected again The highlighter test now clicks at a human pace, with frames in between, which is what exposed it. Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Muse Spark Co-authored-by: Brandon Thomas Co-authored-by: Claude Opus 5.5 --- crates/annot/src/lib.rs | 92 ++++++++++++- crates/annot/src/tests.rs | 23 ++++ crates/automation/src/lib.rs | 6 +- crates/engine/src/dates.rs | 32 ++++- crates/engine/src/lib.rs | 132 +++++++++++++++++-- crates/engine/src/tests.rs | 100 ++++++++++++++ crates/render/src/inspect.rs | 26 ++++ crates/render/src/lib.rs | 2 +- crates/ui-egui/src/canvas.rs | 5 +- crates/ui-egui/src/dialogs.rs | 8 +- crates/ui-egui/src/editing.rs | 38 +++++- crates/ui-egui/src/i18n/mod.rs | 4 +- crates/ui-egui/tests/date_format.rs | 3 + crates/ui-egui/tests/javascript.rs | 3 + crates/ui-egui/tests/ui.rs | 2 + crates/ui-egui/tests/ui_group/bulk_fields.rs | 3 + crates/ui-egui/tests/ui_group/identity.rs | 4 +- crates/ui-egui/tests/ui_group/text_select.rs | 20 +++ crates/ui-egui/tests/view.rs | 2 + 19 files changed, 477 insertions(+), 28 deletions(-) diff --git a/crates/annot/src/lib.rs b/crates/annot/src/lib.rs index b59f727186..00e8776bae 100644 --- a/crates/annot/src/lib.rs +++ b/crates/annot/src/lib.rs @@ -14,7 +14,7 @@ #![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)] -use pdfcraft_cos::{Dict, Document, ObjRef, Object, PdfString}; +use pdfcraft_cos::{Dict, Document, ObjRef, Object, PdfString, Stream}; pub mod appearance; pub mod links; @@ -1255,7 +1255,14 @@ pub fn orient_image_stamp(doc: &mut Document, page: usize, index: usize, rotatio pub fn set_appearance(doc: &mut Document, r: ObjRef) -> Result<(), AnnotError> { let d = annot_dict(doc, r); let subtype = String::from_utf8_lossy(d.name(b"Subtype").unwrap_or_default()).into_owned(); - let Some(mut stream) = appearance::build(&d) else { return Err(AnnotError::Unsupported(subtype)) }; + let Some(stream) = build_appearance(doc, &d) else { return Err(AnnotError::Unsupported(subtype)) }; + apply_appearance(doc, r, &d, stream) +} + +/// Draw the appearance for an annotation dictionary: the expensive half of [`set_appearance`]. +/// Pure from the dictionary and the objects it points at, so [`AppearanceCache`] can reuse it. +fn build_appearance(doc: &Document, d: &Dict) -> Option { + let mut stream = appearance::build(d)?; // Image stamp restyling must retain the placement's page-axis correction. if matches!(d.get(b"PCPictureImage"), Some(Object::Bool(true))) { let matrix = d.get(b"AP").and_then(|ap| { @@ -1268,6 +1275,11 @@ pub fn set_appearance(doc: &mut Document, r: ObjRef) -> Result<(), AnnotError> { stream.dict.set(b"Matrix".to_vec(), matrix); } } + Some(stream) +} + +/// Store a built appearance on the annotation: the cheap half of [`set_appearance`]. +fn apply_appearance(doc: &mut Document, r: ObjRef, d: &Dict, stream: Stream) -> Result<(), AnnotError> { let ap = doc.add(Object::Stream(stream)); // A copy (a shared /AP is left alone) that keeps unknown entries; the old down and rollover // appearances would show the previous look on press or hover, so they go with the old /N. @@ -1282,12 +1294,75 @@ pub fn set_appearance(doc: &mut Document, r: ObjRef) -> Result<(), AnnotError> { Ok(()) } +/// Display appearances built for annotations without any, reused while the annotation +/// dictionaries they were drawn from are unchanged. +/// +/// A content edit (moving a paragraph, retyping text, …) leaves every annotation dictionary +/// alone, but the display copy is rebuilt from the edited document on every refresh: without +/// this cache each refresh redrew every appearance (27k on one CAD file, ~0.7 s per edit +/// commit). Entries are keyed by annotation reference; object numbers are never reused within +/// a document lineage, so an entry stays valid until an annotation edit runs. Callers clear +/// the cache on any refresh that may change annotations (comments, structural and full +/// refreshes) and keep it for content-only ones. +#[derive(Clone, Debug, Default)] +pub struct AppearanceCache { + entries: std::collections::HashMap, + bytes: usize, + hits: u64, + builds: u64, +} + +impl AppearanceCache { + /// Cached appearance data restarts past this instead of growing without bound. + const MAX_BYTES: usize = 64 << 20; + + /// Forget every entry (an annotation edit may have changed the dictionaries). + pub fn clear(&mut self) { + *self = Self::default(); + } + + fn get(&mut self, r: ObjRef) -> Option { + let stream = self.entries.get(&r)?.clone(); + self.hits += 1; + Some(stream) + } + + fn insert(&mut self, r: ObjRef, stream: &Stream) { + self.builds += 1; + if self.entries.len() >= 1_000_000 { + self.clear(); + } + if self.bytes.saturating_add(stream.raw.len()) <= Self::MAX_BYTES { + self.bytes += stream.raw.len(); + self.entries.insert(r, stream.clone()); + } + // Else over budget: keep serving the entries already cached instead of growing. + } + + /// Cache hits so far (reused appearances); for tests. + pub fn hits(&self) -> u64 { + self.hits + } + + /// Appearances built so far; for tests. + pub fn builds(&self) -> u64 { + self.builds + } +} + /// A copy of `doc` in which every annotation without a normal appearance (`/AP /N`) has the one /// [`appearance::build`] draws from its dictionary, for displaying the document (`None` when no /// annotation needs one). Many files carry comments without appearances (FreeText, Ink, notes, /// stamps, …) that viewers draw from the dictionary. `doc` is not changed: saving writes the file /// as it was. Links, form fields and pop-ups keep their own handling. pub fn with_missing_appearances(doc: &Document) -> Option { + with_missing_appearances_cached(doc, &mut AppearanceCache::default()) +} + +/// A copy of `doc` with [`AppearanceCache`] reuse across refreshes: annotations whose +/// dictionary is unchanged since the cached build keep their stored appearance instead of +/// redrawing it. See [`AppearanceCache`] for when entries stay valid. +pub fn with_missing_appearances_cached(doc: &Document, cache: &mut AppearanceCache) -> Option { let mut copy: Option = None; for page in page_refs(doc).ok()? { for r in annots(doc, page).iter().filter_map(Object::as_ref) { @@ -1296,11 +1371,18 @@ pub fn with_missing_appearances(doc: &Document) -> Option { continue; } let has_normal = d.get(b"AP").is_some_and(|ap| doc.resolve(ap).as_dict().is_some_and(|ap| ap.contains(b"N"))); - if has_normal || appearance::build(&d).is_none() { + if has_normal { continue; } - // Fails only when `r` isn't a dictionary, which `build` above has just ruled out. - set_appearance(copy.get_or_insert_with(|| doc.clone()), r).ok(); + if let Some(stream) = cache.get(r) { + // Fails only when `r` isn't a dictionary, which the cached build ruled out. + apply_appearance(copy.get_or_insert_with(|| doc.clone()), r, &d, stream).ok(); + continue; + } + let Some(stream) = build_appearance(doc, &d) else { continue }; + cache.insert(r, &stream); + // Fails only when `r` isn't a dictionary, which the build just ruled out. + apply_appearance(copy.get_or_insert_with(|| doc.clone()), r, &d, stream).ok(); } } copy diff --git a/crates/annot/src/tests.rs b/crates/annot/src/tests.rs index d0e29e06e8..589a6abbd4 100644 --- a/crates/annot/src/tests.rs +++ b/crates/annot/src/tests.rs @@ -1158,3 +1158,26 @@ fn drawings_resize_by_scaling_their_strokes() { // Too small to hold the stroke margin. assert!(matches!(set_rect(&mut doc, 0, i, [100.0, 400.0, 101.0, 401.0], &meta("")), Err(AnnotError::Invalid(_)))); } + +/// Content edits rebuild the display copy on every commit but leave annotations alone: the +/// cached appearance is drawn once and reused after that, byte-identical. +#[test] +fn cached_appearances_are_reused_while_annotations_are_unchanged() { + let mut doc = fixture(); + let idx = add_annotation(&mut doc, &new(0, Shape::Note { at: [100.0, 700.0], icon: NoteIcon::Comment }), &meta("cache")).expect("adds"); + // Files carry comments without appearances; strip the created one to simulate that. + let page = page_refs(&doc).unwrap()[0]; + let r = annots(&doc, page)[idx].as_ref().expect("indirect"); + doc.update_dict(r, |d| { + d.remove(b"AP"); + }) + .expect("strips"); + let mut cache = AppearanceCache::default(); + let first = with_missing_appearances_cached(&doc, &mut cache).expect("builds"); + assert_eq!((cache.builds(), cache.hits()), (1, 0), "only the note needs an appearance"); + let second = with_missing_appearances_cached(&doc, &mut cache).expect("reuses"); + assert_eq!((cache.builds(), cache.hits()), (1, 1), "nothing is redrawn"); + let [a, b] = [&first, &second].map(|copy| ap_content(copy, &list(copy, 0)[idx])); + assert!(!a.is_empty()); + assert_eq!(a, b, "the reused appearance is byte-identical"); +} diff --git a/crates/automation/src/lib.rs b/crates/automation/src/lib.rs index c2e43a3a55..71602908b1 100644 --- a/crates/automation/src/lib.rs +++ b/crates/automation/src/lib.rs @@ -165,7 +165,11 @@ impl Automation { let out = match name { "doc_open" => self.doc_open(&a)?, "doc_list" => json!({ "documents": self.session.docs().iter().map(summary).collect::>() }), - "doc_info" => info(self.doc(&a)?), + "doc_info" => { + let id = self.doc(&a)?.id; + self.session.refresh_fonts(id); + info(self.doc(&a)?) + } "doc_close" => self.doc_close(&a)?, "doc_save" => self.doc_save(&a)?, "doc_set_info" => { diff --git a/crates/engine/src/dates.rs b/crates/engine/src/dates.rs index 195202a193..38577e17c4 100644 --- a/crates/engine/src/dates.rs +++ b/crates/engine/src/dates.rs @@ -56,7 +56,7 @@ const AR_DAYS: [&str; 7] = ["الأحد", "الاثنين", "الثلاثاء", /// The languages month and weekday names can be written in (Preferences ▸ Date format ▸ Language): /// the interface languages, in the same order and with the same names. -pub const DATE_LANGUAGES: [DateLanguage; 21] = [ +pub static DATE_LANGUAGES: [DateLanguage; 21] = [ DateLanguage { code: "en", name: "English", @@ -297,8 +297,34 @@ pub const DATE_LANGUAGES: [DateLanguage; 21] = [ DateLanguage { code: "el", name: "Ελληνικά", - months: ["Ιανουάριος", "Φεβρουάριος", "Μάρτιος", "Απρίλιος", "Μάιος", "Ιούνιος", "Ιούλιος", "Αύγουστος", "Σεπτέμβριος", "Οκτώβριος", "Νοέμβριος", "Δεκέμβριος"], - months_with_day: Some(["Ιανουαρίου", "Φεβρουαρίου", "Μαρτίου", "Απριλίου", "Μαΐου", "Ιουνίου", "Ιουλίου", "Αυγούστου", "Σεπτεμβρίου", "Οκτωβρίου", "Νοεμβρίου", "Δεκεμβρίου"]), + months: [ + "Ιανουάριος", + "Φεβρουάριος", + "Μάρτιος", + "Απρίλιος", + "Μάιος", + "Ιούνιος", + "Ιούλιος", + "Αύγουστος", + "Σεπτέμβριος", + "Οκτώβριος", + "Νοέμβριος", + "Δεκέμβριος", + ], + months_with_day: Some([ + "Ιανουαρίου", + "Φεβρουαρίου", + "Μαρτίου", + "Απριλίου", + "Μαΐου", + "Ιουνίου", + "Ιουλίου", + "Αυγούστου", + "Σεπτεμβρίου", + "Οκτωβρίου", + "Νοεμβρίου", + "Δεκεμβρίου", + ]), months_short: ["Ιαν", "Φεβ", "Μαρ", "Απρ", "Μαΐ", "Ιουν", "Ιουλ", "Αυγ", "Σεπ", "Οκτ", "Νοε", "Δεκ"], days: ["Κυριακή", "Δευτέρα", "Τρίτη", "Τετάρτη", "Πέμπτη", "Παρασκευή", "Σάββατο"], days_short: ["Κυρ", "Δευ", "Τρί", "Τετ", "Πέμ", "Παρ", "Σάβ"], diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index cb602e134e..a0ac6ed312 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -108,8 +108,8 @@ pub fn typed_signature_graphic(text: &str) -> Option { pub use pdfcraft_annot::appearance as annot_text; pub use pdfcraft_annot::links::{Highlight as LinkHighlight, LinkAction, LinkItem, LinkStyle}; pub use pdfcraft_annot::{ - AttachIcon, FillMark, LineEnding, Markup, NOTE_SIZE, NewAnnotation, NoteIcon, OverlayFont, OverlayLook, Props as CommentProps, ReviewState, Rgb, - Shape, StampGroup, StampKind, Style, rect_quad, + AppearanceCache, AttachIcon, FillMark, LineEnding, Markup, NOTE_SIZE, NewAnnotation, NoteIcon, OverlayFont, OverlayLook, Props as CommentProps, + ReviewState, Rgb, Shape, StampGroup, StampKind, Style, rect_quad, }; pub use pdfcraft_forms::detect; pub use pdfcraft_optimize as optimize; @@ -166,6 +166,10 @@ enum Scope { Comments, /// Only form field values (and their widget appearances). Form, + /// Only page content streams, resources and added content/marks: the page tree, + /// annotations, form fields, links, outline and fonts are unchanged, so the full + /// re-inspection is skipped (seconds on very large files). + Content, } /// Edits during which field scripts run. @@ -199,6 +203,21 @@ fn scope_of(edit: &Edit) -> Scope { | Edit::FillAnnotation { .. } | Edit::SetAnnotationInfo { .. } => Scope::Comments, Edit::SetFieldValue { .. } | Edit::ResetForm { .. } | Edit::SetFieldImage { .. } | Edit::ApplyScriptChanges { .. } => Scope::Form, + // Page content, resources and added content/marks only: the page tree, annotations, + // fields, links and document metadata are untouched (see `refresh_content`). + Edit::MoveObjects { .. } + | Edit::EditTextLine { .. } + | Edit::EditTextBlock { .. } + | Edit::EditPageImage { .. } + | Edit::AddText { .. } + | Edit::AddImage { .. } + | Edit::UpdateContent { .. } + | Edit::DeleteContent { .. } + | Edit::ReplaceImage { .. } + | Edit::AddHeaderFooter { .. } + | Edit::AddWatermark { .. } + | Edit::AddBackground { .. } + | Edit::RemoveMarks { .. } => Scope::Content, Edit::Batch { edits, .. } => { let mut scopes = edits.iter().map(scope_of); let first = scopes.next().unwrap_or(Scope::Full); @@ -218,6 +237,10 @@ struct Editor { undo: Vec, redo: Vec, keys: Keys, + /// Display appearances built for annotations without any, reused while no annotation edit + /// runs (content edits leave every annotation dictionary alone). Cleared on any refresh + /// that may change annotations; see [`AppearanceCache`]. + appearances: AppearanceCache, } pub struct Document { @@ -262,6 +285,9 @@ pub struct Document { pub xfa: Option, /// The parsed template of a laid-out XFA form, for its scripts. xfa_template: Option>, + /// A content-only refresh keeps `info` from before the edit; an edit may have added or + /// embedded fonts, so the font list is read again on demand ([`Session::refresh_fonts`]). + fonts_stale: bool, /// XFA forms: what was approximated, rewritten or could not be written to the XFA data /// (also in `info.warnings`, kept there when the document is re-read). pub xfa_warnings: Vec, @@ -592,8 +618,10 @@ fn is_placeholder_title(title: &str) -> bool { /// What is displayed: the working file, plus (in memory only, never saved) the appearances /// PdfCraft draws for comments that have none ([`pdfcraft_annot::with_missing_appearances`]). /// The working file is unchanged, and so is everything that reads it (saving, signatures, …). -fn display_bytes(editor: Option<&Editor>, bytes: &Arc>) -> Arc> { - let Some(display) = editor.and_then(|e| pdfcraft_annot::with_missing_appearances(&e.cos)) else { return bytes.clone() }; +fn display_bytes(editor: Option<&mut Editor>, bytes: &Arc>) -> Arc> { + let Some(display) = editor.and_then(|e| pdfcraft_annot::with_missing_appearances_cached(&e.cos, &mut e.appearances)) else { + return bytes.clone(); + }; // Writing fails only as saving the document would; the page then shows as it always did. write_incremental(&display, &SaveOptions::default()).map(Arc::new).unwrap_or_else(|_| bytes.clone()) } @@ -2657,12 +2685,12 @@ impl Session { let (mut editor, read_only_reason) = match cos { Ok(Ok(cos)) => { let keys = Keys { render: render_password.clone(), reopen: password.map(str::to_owned) }; - (Some(Editor { cos, undo: Vec::new(), redo: Vec::new(), keys }), None) + (Some(Editor { cos, undo: Vec::new(), redo: Vec::new(), keys, appearances: AppearanceCache::default() }), None) } Ok(Err(e)) => (None, Some(e.to_string())), Err(_) => (None, Some("the document structure could not be read for editing".into())), }; - let display = display_bytes(editor.as_ref(), &bytes); + let display = display_bytes(editor.as_mut(), &bytes); let renderer = RenderPool::new(display.clone(), render_threads(), config.clone()); let (form, adopted) = editor .as_ref() @@ -2720,6 +2748,7 @@ impl Session { js_output: Default::default(), xfa, xfa_template: None, + fonts_stale: false, xfa_warnings: Vec::new(), }); Ok(id) @@ -2764,7 +2793,7 @@ impl Session { // every calculation (and a reset recalculates). They read the data, so it is brought up // to date first. if let Some(tpl) = cx.xfa.clone() { - if is_xfa && matches!(scope_of(&edit), Scope::Form | Scope::Full) { + if is_xfa && matches!(scope_of(&edit), Scope::Form | Scope::Full | Scope::Content) { let datasets = &mut cx.xfa_datasets; let notes = guard(|| xfa_sync_datasets(&mut next, Some(&tpl), datasets)) .unwrap_or_else(|m| Err(format!("writing the XFA data failed unexpectedly ({m})"))) @@ -2797,7 +2826,7 @@ impl Session { } // XFA forms keep their values in the datasets packet too, for Adobe's viewers. let mut xfa_notes = Vec::new(); - if is_xfa && matches!(scope_of(&edit), Scope::Form | Scope::Full) { + if is_xfa && matches!(scope_of(&edit), Scope::Form | Scope::Full | Scope::Content) { let (tpl, datasets) = (cx.xfa.as_deref(), &mut cx.xfa_datasets); xfa_notes = guard(|| xfa_sync_datasets(&mut next, tpl, datasets)) .unwrap_or_else(|m| Err(format!("writing the XFA data failed unexpectedly ({m})"))) @@ -2819,7 +2848,8 @@ impl Session { } editor.redo.clear(); Self::adopt_keys(doc); - let refreshed = guard(|| Self::refresh_scoped(doc, scope)).unwrap_or_else(|m| Err(EditError::Reopen(m))); + let refreshed = guard(|| if scope == Scope::Content { Self::refresh_content(doc, Some(&edit)) } else { Self::refresh_scoped(doc, scope) }) + .unwrap_or_else(|m| Err(EditError::Reopen(m))); if let Err(e) = refreshed { // Roll back: the edit produced something we cannot display. if let Some(ed) = doc.editor.as_mut() @@ -2892,7 +2922,15 @@ impl Session { if scope == Scope::Full || doc.info.encrypted != doc.editor.as_ref().is_some_and(|e| e.cos.output_handler().is_some()) { return Self::refresh(doc); } - let Some(editor) = doc.editor.as_ref() else { return Ok(()) }; + if scope == Scope::Content { + return Self::refresh_content(doc, None); + } + let Some(editor) = doc.editor.as_mut() else { return Ok(()) }; + // Comment edits, and form edits (a script may change annotation properties), may change + // annotation dictionaries, invalidating cached appearances. + if matches!(scope, Scope::Comments | Scope::Form) { + editor.appearances.clear(); + } let bytes = if editor.cos.is_modified() { Arc::new(write_incremental(&editor.cos, &SaveOptions::default()).map_err(|e| EditError::Write(e.to_string()))?) } else { @@ -2943,6 +2981,8 @@ impl Session { } // Handled above; kept as a full refresh so the match stays total. Scope::Full => return Self::refresh(doc), + // Handled above (content-only refresh); the arm keeps the match total. + Scope::Content => return Self::refresh_content(doc, None), } doc.info.file_size = bytes.len(); doc.form = form; @@ -2955,9 +2995,68 @@ impl Session { Ok(()) } + /// Which derived lists a content-only edit can change: added content (`None`, from + /// undo/redo, re-reads both: always correct, rarely paid). + fn content_refresh_lists(edit: Option<&Edit>) -> (bool, bool) { + let touches_added = edit.is_none_or(|e| { + matches!( + e, + Edit::AddText { .. } | Edit::AddImage { .. } | Edit::UpdateContent { .. } | Edit::DeleteContent { .. } | Edit::ReplaceImage { .. } + ) || matches!(e, Edit::MoveObjects { objects, .. } if objects.iter().any(|o| o.kind == ObjectKind::Added)) + }); + let touches_marks = edit.is_none_or(|e| { + matches!(e, Edit::AddHeaderFooter { .. } | Edit::AddWatermark { .. } | Edit::AddBackground { .. } | Edit::RemoveMarks { .. }) + }); + (touches_added, touches_marks) + } + + /// Rebuild working bytes, display bytes and renderer after a content-only edit: page + /// content streams, resources and added content/marks. The page tree, annotations, form + /// fields, links, outline and fonts are unchanged, so the full re-inspection (seconds on + /// very large files) is skipped. `edit` selects which derived lists are re-read (`None`, + /// from undo/redo, re-reads both: always correct, rarely paid). The cached display + /// appearances are kept: content edits leave every annotation dictionary alone. + fn refresh_content(doc: &mut Document, edit: Option<&Edit>) -> Result<(), EditError> { + let Some(editor) = doc.editor.as_mut() else { return Ok(()) }; + // A script may have laid an XFA form out again (rows added, subforms shown). + if doc.xfa.is_some() + && let Some(report) = pdfcraft_xfa::existing_layout(&editor.cos) + { + doc.xfa = Some(report); + } + let bytes = if editor.cos.is_modified() { + Arc::new(write_incremental(&editor.cos, &SaveOptions::default()).map_err(|e| EditError::Write(e.to_string()))?) + } else { + editor.cos.bytes().clone() + }; + // The edit must still leave a file the viewer can open; anything else rolls back above. + pdfcraft_render::check_pages(bytes.clone(), doc.password.as_deref(), doc.info.pages.len()).map_err(|e| EditError::Reopen(e.to_string()))?; + let (touches_added, touches_marks) = Self::content_refresh_lists(edit); + if touches_added { + doc.added = pdfcraft_edit::list_added(&editor.cos); + } + if touches_marks { + doc.marks = pdfcraft_edit::marks_present(&editor.cos); + } + doc.info.file_size = bytes.len(); + if !doc.signatures.is_empty() { + doc.signatures = signatures_of(&editor.cos, &bytes, &doc.trust, &doc.sig_cache); + } + doc.display = display_bytes(Some(editor), &bytes); + doc.renderer = RenderPool::new(doc.display.clone(), render_threads(), doc.config.clone()); + doc.bytes = bytes; + doc.fonts_stale = true; + Ok(()) + } + /// Rebuild working bytes, inspection and renderer from the current edit state. fn refresh(doc: &mut Document) -> Result<(), EditError> { - let Some(editor) = doc.editor.as_ref() else { return Ok(()) }; + // A full refresh may follow structural or annotation changes: cached appearances + // may be stale, so they are rebuilt below. + if let Some(editor) = doc.editor.as_mut() { + editor.appearances.clear(); + } + let Some(editor) = doc.editor.as_mut() else { return Ok(()) }; // A script may have laid an XFA form out again (rows added, subforms shown). if doc.xfa.is_some() && let Some(report) = pdfcraft_xfa::existing_layout(&editor.cos) @@ -2979,6 +3078,7 @@ impl Session { } note_warnings(&mut info.warnings, &doc.xfa_warnings); doc.info = info; + doc.fonts_stale = false; let mut form = pdfcraft_forms::fields(&editor.cos); xfa::mark_script_buttons(&editor.cos, &mut form); doc.form = Arc::new(form); @@ -3655,6 +3755,16 @@ impl Session { self.docs.iter().find(|d| d.id == id) } + /// Read document `id`'s font list again if content edits since the last full inspection may + /// have changed it (Document Properties ▸ Fonts, `doc_info`). A no-op otherwise. + pub fn refresh_fonts(&mut self, id: DocId) { + let Some(doc) = self.docs.iter_mut().find(|d| d.id == id && d.fonts_stale) else { return }; + if let Ok(info) = pdfcraft_render::inspect(doc.bytes.clone(), doc.password.as_deref()) { + doc.info.fonts = info.fonts; + } + doc.fonts_stale = false; + } + pub fn docs(&self) -> &[Document] { &self.docs } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 68cdc0c852..838f9af279 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -3361,3 +3361,103 @@ fn enhance_camera_page_refuses_bad_corners_and_pages() { assert!(matches!(s.enhance_camera_page(DocId(999), 0, ok), Err(EditError::NoDocument))); assert_eq!(s.get(id).unwrap().can_undo(), None, "nothing changed"); } + +/// A content-only edit must not disturb the inspection results: pages, annotations, fields, +/// links and marks are unchanged, only the working bytes, the added-content list and the +/// display copy move. +#[test] +fn content_edit_keeps_pages_annotations_fields_and_links() { + let (mut session, id) = session_with(2); + let before = session.get(id).unwrap(); + let pages = format!("{:?}", before.info.pages); + let annotations = format!("{:?}", before.info.annotations); + let links = format!("{:?}", before.links); + let form = before.form.clone(); + let marks = before.marks.clone(); + let bytes = before.bytes.clone(); + let text = AddedText { + rect: [20.0, 20.0, 200.0, 60.0], + text: "hello".into(), + family: FontFamily::Helvetica, + bold: false, + italic: false, + size: 12.0, + color: [0.0, 0.0, 0.0], + align: TextAlign::Left, + }; + session.apply(id, Edit::AddText { page: 1, text }).unwrap(); + let after = session.get(id).unwrap(); + assert_eq!(format!("{:?}", after.info.pages), pages, "page geometry is untouched"); + assert_eq!(format!("{:?}", after.info.annotations), annotations); + assert_eq!(format!("{:?}", after.links), links); + assert_eq!(after.form, form, "no field was touched"); + assert_eq!(after.marks, marks, "no mark was touched"); + assert!(after.added.iter().any(|a| a.page == 1), "the added text is listed"); + assert_ne!(after.bytes, bytes, "the working file carries the edit"); + assert_ne!(after.display, bytes, "the display copy carries the edit too"); + // The display bytes still open and render the edited page. + let mut renderer = pdfcraft_render::PageRenderer::new(after.display.clone(), after.config.clone()); + let rendered = renderer.render(pdfcraft_render::RenderRequest { page: 1, scale: 1.0, ..Default::default() }); + assert!(rendered.error.is_none(), "{:?}", rendered.error); + // Undo goes through the same narrow refresh without the edit at hand. + session.undo(id).unwrap(); + let undone = session.get(id).unwrap(); + assert!(undone.added.is_empty(), "undo drops the added text"); + assert_eq!(format!("{:?}", undone.info.pages), pages); +} + +/// `refresh_content` re-reads only the derived lists the edit can change (undo/redo, with no +/// edit at hand, re-read both). +#[test] +fn content_refresh_lists_match_the_storages_an_edit_touches() { + let target = |kind| ObjectTarget { kind, index: 0 }; + let added = [ + Edit::AddText { + page: 0, + text: AddedText { + rect: [0.0, 0.0, 10.0, 10.0], + text: String::new(), + family: FontFamily::Helvetica, + bold: false, + italic: false, + size: 12.0, + color: [0.0, 0.0, 0.0], + align: TextAlign::Left, + }, + }, + Edit::MoveObjects { page: 0, objects: vec![target(crate::ObjectKind::Added)], offset: [1.0, 0.0] }, + ]; + for edit in &added { + assert_eq!(Session::content_refresh_lists(Some(edit)), (true, false), "{}", edit.label()); + } + let plain = [ + Edit::MoveObjects { page: 0, objects: vec![target(crate::ObjectKind::Text)], offset: [1.0, 0.0] }, + Edit::EditTextBlock { page: 0, block: 0, text: String::new(), style: pdfcraft_edit::BlockStyle::default() }, + ]; + for edit in &plain { + assert_eq!(Session::content_refresh_lists(Some(edit)), (false, false), "{}", edit.label()); + } + let marks = [Edit::AddWatermark { pages: vec![0], settings: Watermark::default(), replace: false, file: None }]; + for edit in &marks { + assert_eq!(Session::content_refresh_lists(Some(edit)), (false, true), "{}", edit.label()); + } + assert_eq!(Session::content_refresh_lists(None), (true, true), "undo/redo re-read both"); +} + +/// A content edit can add a font (Add text in Times); the narrow refresh leaves the font list +/// as it was, and `refresh_fonts` reads it again before Properties ▸ Fonts or `doc_info` show it. +#[test] +fn fonts_added_by_a_content_edit_show_after_refresh_fonts() { + let (mut session, id) = session_with(1); + let has_times = |s: &Session| s.get(id).unwrap().info.fonts.iter().any(|f| f.name.contains("Times")); + assert!(!has_times(&session)); + let text = AddedText { text: "serif".into(), rect: [20.0, 20.0, 200.0, 60.0], family: FontFamily::Times, ..Default::default() }; + session.apply(id, Edit::AddText { page: 0, text }).unwrap(); + assert!(!has_times(&session), "the narrow refresh doesn't re-inspect"); + session.refresh_fonts(id); + assert!(has_times(&session), "{:?}", session.get(id).unwrap().info.fonts); + // Nothing changed since: a second call is a no-op. + let fonts = format!("{:?}", session.get(id).unwrap().info.fonts); + session.refresh_fonts(id); + assert_eq!(format!("{:?}", session.get(id).unwrap().info.fonts), fonts); +} diff --git a/crates/render/src/inspect.rs b/crates/render/src/inspect.rs index d22d63c5c0..438932f993 100644 --- a/crates/render/src/inspect.rs +++ b/crates/render/src/inspect.rs @@ -346,6 +346,32 @@ pub struct FontInfo { pub encoding: Option, } +/// Check that `bytes` still open as the same document: the parser reads them and the page +/// count matches. Content-only edits skip the full [`inspect`] (seconds on very large files) +/// but must still catch an edit that produced something the viewer cannot display, so the +/// caller can roll it back. Never panics: parser crashes are caught and reported. +pub fn check_pages(bytes: Arc>, password: Option<&str>, expected: usize) -> Result<(), OpenError> { + let pdf = catch_unwind(AssertUnwindSafe(|| Pdf::new_with_password(bytes, password.unwrap_or("")))) + .map_err(|p| OpenError::Invalid(format!("the parser crashed: {}", crate::raster::panic_message(&p))))? + .map_err(|e| match e { + LoadPdfError::Decryption(DecryptionError::PasswordProtected) => { + if password.is_some() { + OpenError::WrongPassword + } else { + OpenError::NeedsPassword + } + } + LoadPdfError::Decryption(other) => OpenError::Unsupported(format!("encryption not supported: {other:?}")), + LoadPdfError::Invalid => OpenError::Invalid("no readable page tree or cross-reference data was found".into()), + })?; + let pages = catch_unwind(AssertUnwindSafe(|| pdf.pages().len())) + .map_err(|p| OpenError::Invalid(format!("the page tree could not be read: {}", crate::raster::panic_message(&p))))?; + if pages != expected { + return Err(OpenError::Invalid(format!("the edit changed the page count from {expected} to {pages}"))); + } + Ok(()) +} + /// Inspect a document. Fails only if the renderer itself cannot open the file (or needs a /// password). Never panics: parser crashes are caught and reported. pub fn inspect(bytes: Arc>, password: Option<&str>) -> Result { diff --git a/crates/render/src/lib.rs b/crates/render/src/lib.rs index 23d4036097..832582d601 100644 --- a/crates/render/src/lib.rs +++ b/crates/render/src/lib.rs @@ -16,7 +16,7 @@ pub mod text; pub use inspect::{ Annotation, Attachment, AttachmentSource, DestView, DocInfo, Field, FieldKind, FontInfo, Layer, LayerOp, Link, LinkTarget, OutlineItem, PageInfo, - Xfa, attachment_data, inspect, pretty_date, + Xfa, attachment_data, check_pages, inspect, pretty_date, }; pub use pixels::Pixels; pub use raster::{ diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index c26b108daa..c5b939bff6 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -2684,7 +2684,10 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { view.clicks = release_clicks; } let clicks = if view.clicks > 2 && tool != QuickTool::Select { 1 } else { view.clicks }; - if clicks >= 2 && over_text { + // Only on the frame egui reports the multi-click: `view.clicks` outlives it, and + // acting on every frame re-selected the word as soon as a selection was cleared + // (a highlight made by the double-click left its word selected). + if release_clicks > 0 && clicks >= 2 && over_text { if view.selection.is_none() { let span = text.nearest(vx, vy).and_then(|a| match clicks { 2 => text.word_at(a), diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 812b6c2100..bab565fd76 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -25,6 +25,10 @@ fn dialogs(app: &mut PdfCraftApp, ctx: &egui::Context) { app.view_draft = None; return; }; + // Properties ▸ Fonts shows the fonts as they are now, after any content edits. + if let (Dialog::Properties(PropsTab::Fonts), Some((_, id))) = (dialog, app.active_ids()) { + app.session.refresh_fonts(id); + } // Seed the editable Description fields from the document when the dialog opens. if let (Dialog::Properties(_), Some((_, id))) = (dialog, app.active_ids()) && app.props_draft.as_ref().is_none_or(|(d, _)| *d != id) @@ -897,7 +901,9 @@ fn dialogs(app: &mut PdfCraftApp, ctx: &egui::Context) { return; } Dialog::Preferences => { - close = crate::js_ui::preferences_body(ui, app, &t); + // The settings outgrow a short window: scroll them rather than run off the screen. + let max = (ui.ctx().content_rect().height() - 120.0).max(200.0); + close = egui::ScrollArea::vertical().max_height(max).show(ui, |ui| crate::js_ui::preferences_body(ui, app, &t)).inner; return; } Dialog::RecognizeText => { diff --git a/crates/ui-egui/src/editing.rs b/crates/ui-egui/src/editing.rs index 81fb1f2a72..bd57ea4cf4 100644 --- a/crates/ui-egui/src/editing.rs +++ b/crates/ui-egui/src/editing.rs @@ -43,8 +43,8 @@ impl PdfCraftApp { let info = &doc.info; let view = &mut self.views[i]; view.signature_drag.committed(&edit, doc.edit_generation()); - if let Some(page) = comment_page(&edit) { - // Comment edits change one page: keep every other raster. + if let Some(page) = comment_page(&edit).or_else(|| content_page(&edit)) { + // Comment and content edits change one page: keep every other raster. view.page_changed(page); } else if let Some(form_before) = form_before && doc.info.pages.len() == page_count_before @@ -833,7 +833,23 @@ impl PdfCraftApp { } } -/// The page a comment edit changes (`None` for other edits). +/// The single page a content edit changes (`None` for other edits). Like comment edits, +/// these leave every other page's bytes, geometry and text alone, so their rasters stay valid. +fn content_page(edit: &Edit) -> Option { + match edit { + Edit::MoveObjects { page, .. } + | Edit::EditTextLine { page, .. } + | Edit::EditTextBlock { page, .. } + | Edit::EditPageImage { page, .. } + | Edit::AddText { page, .. } + | Edit::AddImage { page, .. } + | Edit::UpdateContent { page, .. } + | Edit::DeleteContent { page, .. } + | Edit::ReplaceImage { page, .. } => Some(*page), + _ => None, + } +} + fn comment_page(edit: &Edit) -> Option { match edit { Edit::AddAnnotation(a) => Some(a.page), @@ -1366,3 +1382,19 @@ mod revert_draft_tests { assert_eq!(app.toast.as_ref().unwrap().0, pdfcraft_engine::EditError::NoDocument.to_string()); } } + +#[cfg(test)] +mod content_page_tests { + use super::content_page; + use pdfcraft_engine::{Edit, ObjectKind, ObjectTarget}; + + /// Single-page content edits keep every other page's rasters; anything else still + /// invalidates the whole document. + #[test] + fn single_page_content_edits_map_to_their_page() { + let moved = Edit::MoveObjects { page: 3, objects: vec![ObjectTarget { kind: ObjectKind::Text, index: 0 }], offset: [1.0, 0.0] }; + assert_eq!(content_page(&moved), Some(3)); + assert_eq!(content_page(&Edit::RotatePages { pages: vec![0], degrees: 90 }), None); + assert_eq!(content_page(&Edit::SetInfo { key: "Title".into(), value: "x".into() }), None); + } +} diff --git a/crates/ui-egui/src/i18n/mod.rs b/crates/ui-egui/src/i18n/mod.rs index 4c399246f8..ba6be0f838 100644 --- a/crates/ui-egui/src/i18n/mod.rs +++ b/crates/ui-egui/src/i18n/mod.rs @@ -1434,7 +1434,9 @@ mod tests { assert!(errors.is_empty(), "Turkish catalog errors: {errors:?}"); for e in &entries { assert!( - e.source != e.translation || KEEP_AS_IS.contains(&e.source.as_str()) || (e.context == "key" && KEY_CAP_NAMES.contains(&e.source.as_str())), + e.source != e.translation + || KEEP_AS_IS.contains(&e.source.as_str()) + || (e.context == "key" && KEY_CAP_NAMES.contains(&e.source.as_str())), "{:?} is untranslated", e.source ); diff --git a/crates/ui-egui/tests/date_format.rs b/crates/ui-egui/tests/date_format.rs index 0fbe9760e5..0606e021de 100644 --- a/crates/ui-egui/tests/date_format.rs +++ b/crates/ui-egui/tests/date_format.rs @@ -19,6 +19,9 @@ fn shows(h: &Harness<'static, PdfCraftApp>, text: &str) -> bool { } fn retype(h: &mut Harness<'static, PdfCraftApp>, text: &str) { + // The preferences scroll; bring the field into view before clicking it. + h.get_by_role_and_label(Role::TextInput, "Pattern").scroll_to_me(); + h.run_steps(4); h.get_by_role_and_label(Role::TextInput, "Pattern").click(); h.run_steps(1); h.key_press_modifiers(Modifiers::COMMAND, Key::A); diff --git a/crates/ui-egui/tests/javascript.rs b/crates/ui-egui/tests/javascript.rs index a5c5a6aff9..2d525104bf 100644 --- a/crates/ui-egui/tests/javascript.rs +++ b/crates/ui-egui/tests/javascript.rs @@ -94,6 +94,9 @@ fn document_scripts_and_preferences() { assert_eq!(h.state().session.get(id).unwrap().document_scripts()[0].0, "init"); h.state_mut().dialog = Some(Dialog::Preferences); h.run_steps(2); + // Below the fold in a 900-point window: the preferences scroll to it. + h.get_by_label("Enable Acrobat JavaScript").scroll_to_me(); + h.run_steps(4); h.get_by_label("Enable Acrobat JavaScript").click(); h.run_steps(2); assert!(!h.state().session.javascript()); diff --git a/crates/ui-egui/tests/ui.rs b/crates/ui-egui/tests/ui.rs index ded8c0ca94..b4c62bd3e5 100644 --- a/crates/ui-egui/tests/ui.rs +++ b/crates/ui-egui/tests/ui.rs @@ -800,6 +800,8 @@ fn preferences_selects_default_workspace_for_next_open() { use pdfcraft_ui_egui::Mode; let mut h = harness(|app| app.set_option("dialog", "preferences").unwrap()); h.get_by_label("Default workspace mode"); + h.get_by_role_and_label(Role::RadioButton, "Read").scroll_to_me(); + h.run_steps(4); h.get_by_role_and_label(Role::RadioButton, "Read").click(); h.run_steps(2); assert_eq!(h.state().default_mode, Mode::Read); diff --git a/crates/ui-egui/tests/ui_group/bulk_fields.rs b/crates/ui-egui/tests/ui_group/bulk_fields.rs index b2c75c2624..1ddf07b1ad 100644 --- a/crates/ui-egui/tests/ui_group/bulk_fields.rs +++ b/crates/ui-egui/tests/ui_group/bulk_fields.rs @@ -89,6 +89,9 @@ fn shared_properties_preserve_mixed_values_and_undo_the_whole_change() { h.state_mut().bulk_field_props.as_mut().unwrap().required.value = true; h.get_by_label("Appearance").click(); h.run_steps(4); + // Below the fold of the dialog's scroll area with the roomier rows (#487). + h.get_by_label("Line Thickness:").scroll_to_me(); + h.run_steps(4); h.get_by_label("Line Thickness:").click(); h.run_steps(4); h.state_mut().bulk_field_props.as_mut().unwrap().width.value = 4.0; diff --git a/crates/ui-egui/tests/ui_group/identity.rs b/crates/ui-egui/tests/ui_group/identity.rs index 9a4e3c38f0..52581fe6a5 100644 --- a/crates/ui-egui/tests/ui_group/identity.rs +++ b/crates/ui-egui/tests/ui_group/identity.rs @@ -29,7 +29,9 @@ fn the_identity_name_signs_new_comments_and_is_remembered() { }); h.run_steps(4); h.get_by_label_contains("Name on new comments"); - // Replace the name in the field. + // Replace the name in the field (the preferences scroll to it in a 900-point window). + h.get_by_role_and_label(Role::TextInput, "Name on new comments").scroll_to_me(); + h.run_steps(4); h.get_by_role_and_label(Role::TextInput, "Name on new comments").click(); h.run_steps(1); h.key_press_modifiers(Modifiers::COMMAND, Key::A); diff --git a/crates/ui-egui/tests/ui_group/text_select.rs b/crates/ui-egui/tests/ui_group/text_select.rs index fc8d19a816..d6f970de80 100644 --- a/crates/ui-egui/tests/ui_group/text_select.rs +++ b/crates/ui-egui/tests/ui_group/text_select.rs @@ -77,8 +77,28 @@ fn quick_clicks_with_the_highlighter_mark_only_the_word() { let (mut h, at) = harness(|app| assert!(app.execute("comment.highlight"))); for _ in 0..4 { click(&mut h, at); + // The double-click's highlight re-renders the page; give its text layer up to 8 frames + // to come back (still well inside egui's 0.3 s multi-click window at 1/60 s a frame), so + // a loaded machine doesn't make the later clicks land on a page with no text yet. + for _ in 0..8 { + if !h.state().render_pending() { + break; + } + std::thread::sleep(std::time::Duration::from_millis(20)); + h.run_steps(1); + } } h.run_steps(3); + // Under load the mark can land a few frames later; wait for it (egui's clock is fixed per + // frame, so waiting can't turn the clicks into a different run). + for _ in 0..200 { + let app = h.state(); + if app.session.get(app.views[0].id).is_some_and(|d| !d.info.annotations.is_empty()) && !app.render_pending() { + break; + } + std::thread::sleep(std::time::Duration::from_millis(5)); + h.run_steps(1); + } let app = h.state(); let doc = app.session.get(app.views[0].id).unwrap(); let marks: Vec<(&str, usize)> = doc.info.annotations.iter().map(|a| (a.subtype.as_str(), a.quads.len())).collect(); diff --git a/crates/ui-egui/tests/view.rs b/crates/ui-egui/tests/view.rs index e1963d331c..e6890f9b18 100644 --- a/crates/ui-egui/tests/view.rs +++ b/crates/ui-egui/tests/view.rs @@ -376,6 +376,8 @@ fn preferences_set_the_default_zoom() { let mut h = harness(&[]); assert!(h.state_mut().execute("app.preferences")); h.run_steps(3); + h.get_by_label("Zoom to page level").scroll_to_me(); + h.run_steps(4); h.get_by_label("Zoom to page level").click(); h.run_steps(2); assert!(h.state_mut().execute("create.blank"));