M7: keep the text blocks cache after the document is edited (#2504)

The engine caches a page's text blocks for opening a paragraph and
applying its edit, but only while the document is unmodified: it was
keyed by the source identity, which edits keep. After the first edit the
cache never hit again, and the page was grouped again on every call (the
inline editor and the panel each call it after an apply).

pdfcraft_cos::Document::read_state() returns what the document reads as:
its source identity, its trailer and every changed object, compared as
allocations that the state keeps alive. A clone or a snapshot restored by
undo has the same state; any set, add, free or trailer change makes a
different one. The cache is keyed by it, so it hits after edits and never
returns another state's blocks.

On a page of 120 rows, text_blocks after an edit took 0.215 ms per call
and now 0.006 ms (release build, best of three; a throwaway timing test).

Tests: a read state matches a clone and not a document changed by set,
free or a trailer change, nor another document; after an edit the engine
groups the page once and undo and redo get their own blocks. The engine
test fails without this change.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Brandon Thomas <echelon@gmail.com>

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<u32, Slot>,
+}
+
+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<Vec<u8>>,
@@ -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<u8> {
         let mut out = b"%PDF-1.7\n".to_vec();

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::{

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<Option<CachedTextBlocks>>);
 
 /// The document state, the page (0-based) and its text blocks.
-type CachedTextBlocks = (pdfcraft_cos::SourceIdentity, usize, Arc<Vec<pdfcraft_edit::TextBlock>>);
+type CachedTextBlocks = (pdfcraft_cos::ReadState, usize, Arc<Vec<pdfcraft_edit::TextBlock>>);
 
 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<Arc<Vec<pdfcraft_edit::TextBlock>>, 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)
     }

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)));
+}