diff --git a/crates/cos/src/document.rs b/crates/cos/src/document.rs index a0b1f56ee..f12f539da 100644 --- a/crates/cos/src/document.rs +++ b/crates/cos/src/document.rs @@ -337,6 +337,40 @@ impl PartialEq for SourceIdentity { impl Eq for SourceIdentity {} +/// What a document reads as: its source ([`SourceIdentity`]), its trailer and every object changed +/// since it was opened. A clone, or a snapshot that undo restores, has the same state, and every +/// change makes a different one. Changed objects are compared as allocations, which the state +/// keeps alive, so a later object can't take an address one of them had. +#[derive(Clone)] +pub struct ReadState { + source: SourceIdentity, + trailer: Dict, + overlay: BTreeMap, +} + +impl ReadState { + /// Whether `doc` reads as this state. + pub fn matches(&self, doc: &Document) -> bool { + self.source == doc.source_identity() + && self.trailer == doc.trailer + && self.overlay.len() == doc.overlay.len() + && self.overlay.iter().zip(&doc.overlay).all(|((a, x), (b, y))| { + a == b + && match (x, y) { + (Slot::Set(g, o), Slot::Set(h, p)) => g == h && Arc::ptr_eq(o, p), + (Slot::Freed(g), Slot::Freed(h)) => g == h, + _ => false, + } + }) + } +} + +impl std::fmt::Debug for ReadState { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("ReadState").field("changed_objects", &self.overlay.len()).finish_non_exhaustive() + } +} + #[derive(Clone)] pub struct Document { data: Arc>, @@ -801,6 +835,12 @@ impl Document { SourceIdentity(Arc::downgrade(&self.source_identity)) } + /// What the document reads as now (see [`ReadState`]): unlike [`Self::source_identity`], it + /// changes with every edit, so a value derived from the document can be cached against it. + pub fn read_state(&self) -> ReadState { + ReadState { source: self.source_identity(), trailer: self.trailer.clone(), overlay: self.overlay.clone() } + } + pub fn version(&self) -> &str { &self.version } @@ -1935,6 +1975,28 @@ const MAX_XREF_OBJECTS: usize = 8_388_607; mod tests { use super::*; + #[test] + fn a_read_state_matches_a_clone_and_no_changed_document() { + let mut doc = Document::new_empty(); + let a = doc.add(Object::Int(1)); + let state = doc.read_state(); + let snapshot = doc.clone(); + assert!(state.matches(&doc) && state.matches(&snapshot)); + // The same value set again is still a change: another allocation. + doc.set(a, Object::Int(1)); + assert!(!state.matches(&doc) && state.matches(&snapshot)); + let edited = doc.read_state(); + doc.free(a); + assert!(!edited.matches(&doc)); + let mut trailer_changed = snapshot.clone(); + trailer_changed.trailer_mut().set(b"Info".to_vec(), Object::Null); + assert!(!state.matches(&trailer_changed)); + // Another document's state never matches, even with the same changes. + let mut other = Document::new_empty(); + other.add(Object::Int(1)); + assert!(!other.read_state().matches(&snapshot)); + } + /// Build a classic-xref file from object bodies (object i+1 = bodies[i]). pub(crate) fn build(bodies: &[&str], trailer: &str) -> Vec { let mut out = b"%PDF-1.7\n".to_vec(); diff --git a/crates/cos/src/lib.rs b/crates/cos/src/lib.rs index 213ff341b..152daed2d 100644 --- a/crates/cos/src/lib.rs +++ b/crates/cos/src/lib.rs @@ -22,7 +22,7 @@ mod security; mod writer; pub use bytes::Bytes; -pub use document::{Document, Revision, SourceIdentity, XrefEntry}; +pub use document::{Document, ReadState, Revision, SourceIdentity, XrefEntry}; pub use object::{Dict, MAX_DECODED, Name, ObjRef, Object, PdfString, Stream}; pub use parser::{Lexer, is_whitespace, parse_indirect}; pub use pdfcraft_crypt::{ diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 5fefe9b4b..f86af3de0 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -316,15 +316,15 @@ struct Editor { text_blocks: TextBlocksCache, } -/// One page's text blocks ([`pdfcraft_edit::text_blocks`]) and the unmodified document state they -/// were read from. Opening a paragraph and applying its edit both need them, and grouping a long -/// page is not cheap. Only an unmodified document is cached: its identity changes on every edit, -/// undo and save (each reopens the working file), so a stale entry never matches. +/// One page's text blocks ([`pdfcraft_edit::text_blocks`]) and the document state they were read +/// from. Opening a paragraph and applying its edit both need them, and grouping a long page is not +/// cheap. The state ([`pdfcraft_cos::ReadState`]) changes with every edit and save and returns +/// with undo, so a stale entry never matches, also after the document has been edited. #[derive(Default)] struct TextBlocksCache(std::sync::Mutex>); /// The document state, the page (0-based) and its text blocks. -type CachedTextBlocks = (pdfcraft_cos::SourceIdentity, usize, Arc>); +type CachedTextBlocks = (pdfcraft_cos::ReadState, usize, Arc>); impl Clone for TextBlocksCache { fn clone(&self) -> Self { @@ -336,20 +336,16 @@ impl Clone for TextBlocksCache { impl Editor { /// The text blocks on `page` (0-based), from the cache when the document hasn't changed. fn text_blocks(&self, page: usize) -> Result>, EditError> { - let source = (!self.cos.is_modified()).then(|| self.cos.source_identity()); - if let Some(source) = &source - && let Ok(cache) = self.text_blocks.0.lock() - && let Some((s, p, blocks)) = cache.as_ref() - && s == source + if let Ok(cache) = self.text_blocks.0.lock() + && let Some((state, p, blocks)) = cache.as_ref() && *p == page + && state.matches(&self.cos) { return Ok(blocks.clone()); } let blocks = Arc::new(pdfcraft_edit::text_blocks(&self.cos, page)?); - if let Some(source) = source - && let Ok(mut cache) = self.text_blocks.0.lock() - { - *cache = Some((source, page, blocks.clone())); + if let Ok(mut cache) = self.text_blocks.0.lock() { + *cache = Some((self.cos.read_state(), page, blocks.clone())); } Ok(blocks) } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index ea410cafb..61c99bb70 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -5890,3 +5890,24 @@ fn a_phone_photo_used_as_a_custom_stamp_is_embedded_upright() { let rect = pdfcraft_annot::summaries(cos)[0].rect; assert!(((rect[2] - rect[0]) - 20.0).abs() < 0.01 && ((rect[3] - rect[1]) - 30.0).abs() < 0.01, "{rect:?}"); } + +/// Opening a paragraph and applying its edit read the page's text blocks; after an edit they are +/// still read once per document state, and undo and redo never get another state's blocks. +#[test] +fn text_blocks_stay_cached_after_an_edit_and_follow_undo_and_redo() { + let (mut session, id) = session_with(1); + let blocks = |s: &Session| s.get(id).unwrap().editor.as_ref().unwrap().text_blocks(0).unwrap(); + let before = blocks(&session); + assert_eq!(before[0].editor_text, "Page 1"); + let edit = Edit::EditTextBlock { page: 0, block: 0, text: "Edited".into(), style: pdfcraft_edit::BlockStyle::default() }; + session.apply(id, edit).unwrap(); + let edited = blocks(&session); + assert_eq!(edited[0].editor_text, "Edited"); + assert!(Arc::ptr_eq(&edited, &blocks(&session)), "the edited page is grouped once"); + session.undo(id).unwrap(); + assert_eq!(blocks(&session)[0].editor_text, "Page 1"); + session.redo(id).unwrap(); + let redone = blocks(&session); + assert_eq!(redone[0].editor_text, "Edited"); + assert!(Arc::ptr_eq(&redone, &blocks(&session))); +}