ElyPDF: failed reads stay unread until an edit; a save stales a signature read (P-08)

apps/ely-pdf/src/annotate.rs

@@ -9,7 +9,7 @@ use gpui::{AnyElement, App, Context, IntoElement, Pixels, Point, Styled as _, ca
 use crate::content::{Aim, Tool};
 use crate::layout::Placed;
 use crate::pages::{rect_on, to_view};
-use crate::view::PdfView;
+use crate::view::{PdfView, Unread};
 
 /// How far a click may move, in window pixels.
 pub(crate) const CLICK: f32 = 3.0;
@@ -77,31 +77,49 @@ impl PdfView {
         if self.reading_notes {
             return;
         }
-        let wanted: Vec<usize> = pages.into_iter().filter(|page| !self.notes.contains_key(page)).take(BATCH).collect();
+        let wanted: Vec<usize> = (pages.into_iter())
+            .filter(|page| !self.notes.contains_key(page) && !self.unread.contains(&Unread::Notes(*page)))
+            .take(BATCH)
+            .collect();
         if wanted.is_empty() {
             return;
         }
+        let asked = wanted.clone();
         self.reading_notes = true;
         let (doc, version) = (self.doc, self.version);
         self.off(
+            // Page by page: one bad page spares the rest.
             move |worker| {
-                (wanted.into_iter())
-                    .map(|page| Ok((page, worker.marks(doc, page)?, worker.links(doc, page)?)))
-                    .collect::<std::io::Result<Vec<_>>>()
+                let read = (wanted.into_iter())
+                    .map(|page| (page, worker.marks(doc, page).and_then(|marks| Ok((marks, worker.links(doc, page)?)))))
+                    .collect::<Vec<_>>();
+                Ok(read)
             },
-            move |view, read, cx| {
+            move |view, read: std::io::Result<Vec<_>>, cx| {
                 view.reading_notes = false;
                 if view.version != version {
                     return cx.notify();
                 }
-                match read {
-                    Ok(read) => {
-                        for (page, marks, links) in read {
+                // No worker at all: every page asked failed.
+                let read = read.unwrap_or_else(|error| {
+                    let why = error.to_string();
+                    asked.into_iter().map(|page| (page, Err(std::io::Error::other(why.clone())))).collect()
+                });
+                let mut first = None;
+                for (page, page_read) in read {
+                    match page_read {
+                        Ok((marks, links)) => {
                             view.notes.insert(page, marks);
                             view.links.insert(page, links);
                         }
+                        Err(error) => {
+                            view.unread.insert(Unread::Notes(page));
+                            first.get_or_insert(error);
+                        }
                     }
-                    Err(error) => view.fail("reading annotations", &error, cx),
+                }
+                if let Some(error) = first {
+                    view.fail("reading annotations", &error, cx);
                 }
                 cx.notify();
             },
@@ -111,7 +129,7 @@ impl PdfView {
 
     /// Reads every field name in the form, once an edit.
     pub fn ask_names(&mut self, cx: &mut Context<Self>) {
-        if self.form_names.is_some() || self.reading_names {
+        if self.form_names.is_some() || self.reading_names || self.unread.contains(&Unread::Names) {
             return;
         }
         self.reading_names = true;
@@ -125,7 +143,10 @@ impl PdfView {
                 }
                 match read {
                     Ok(names) => view.form_names = Some(names),
-                    Err(error) => view.fail("reading field names", &error, cx),
+                    Err(error) => {
+                        view.unread.insert(Unread::Names);
+                        view.fail("reading field names", &error, cx)
+                    }
                 }
                 cx.notify();
             },

apps/ely-pdf/src/attach.rs

@@ -15,7 +15,7 @@ use gpui::{
 };
 
 use crate::document::{RemoveAttachment, SaveAttachment};
-use crate::view::PdfView;
+use crate::view::{PdfView, Unread};
 use crate::viewer::Viewer;
 
 /// A size as people read it.
@@ -30,7 +30,7 @@ pub fn size(bytes: u64) -> String {
 impl PdfView {
     /// Reads the attachments once an edit.
     pub fn ask_files(&mut self, cx: &mut Context<Self>) {
-        if self.files.is_some() || self.reading_files {
+        if self.files.is_some() || self.reading_files || self.unread.contains(&Unread::Files) {
             return;
         }
         self.reading_files = true;
@@ -44,7 +44,10 @@ impl PdfView {
                 }
                 match read {
                     Ok(files) => view.files = Some(files),
-                    Err(error) => view.fail("reading attachments", &error, cx),
+                    Err(error) => {
+                        view.unread.insert(Unread::Files);
+                        view.fail("reading attachments", &error, cx)
+                    }
                 }
                 cx.notify();
             },

apps/ely-pdf/src/content.rs

@@ -8,7 +8,7 @@ use gpui::{AnyElement, App, Context, IntoElement, Pixels, Point, Styled as _, di
 
 use crate::layout::Placed;
 use crate::pages::rect_on;
-use crate::view::PdfView;
+use crate::view::{PdfView, Unread};
 
 /// What a press on a page does.
 #[derive(Clone, Copy, Debug, Default, PartialEq, Eq)]
@@ -139,7 +139,9 @@ impl PdfView {
         if !self.tool.edits() || self.reading_layer {
             return;
         }
-        let Some(page) = pages.iter().copied().find(|page| !self.layers.contains_key(page)) else {
+        let Some(page) = (pages.iter().copied())
+            .find(|page| !self.layers.contains_key(page) && !self.unread.contains(&Unread::Layer(*page)))
+        else {
             return;
         };
         self.reading_layer = true;
@@ -156,7 +158,10 @@ impl PdfView {
                     Ok(layer) => {
                         view.layers.insert(page, layer);
                     }
-                    Err(error) => view.fail("reading what the page holds", &error, cx),
+                    Err(error) => {
+                        view.unread.insert(Unread::Layer(page));
+                        view.fail("reading what the page holds", &error, cx)
+                    }
                 }
                 cx.notify();
             },

apps/ely-pdf/src/edits.rs

@@ -149,6 +149,7 @@ impl PdfView {
         }
         self.marks = fresh.marks;
         self.failed.clear();
+        self.unread.clear();
         self.text.clear();
         self.asking_text.clear();
         self.thumbs_failed.clear();
@@ -172,8 +173,10 @@ impl PdfView {
     /// The file now holds history `mark`.
     pub fn saved(&mut self, mark: &str, cx: &mut Context<Self>) {
         self.saved = mark.to_string();
+        self.saves += 1;
         // The file changed; read it again.
         self.signatures = None;
+        self.unread.retain(|unread| !matches!(unread, crate::view::Unread::Signatures(_)));
         cx.emit(Reported);
     }
 

apps/ely-pdf/src/sign.rs

@@ -17,7 +17,7 @@ use ely_platform::trust::{self, Trust};
 use gpui::{AnyElement, App, AppContext as _, Context, Entity, InteractiveElement as _, IntoElement, ParentElement as _, StatefulInteractiveElement as _, Styled as _, TaskExt as _, Window, div};
 
 use crate::redact::beside;
-use crate::view::PdfView;
+use crate::view::{PdfView, Unread};
 use crate::viewer::Viewer;
 
 /// A signature field and the system's word on its certificate.
@@ -47,11 +47,13 @@ pub struct Signer {
 impl PdfView {
     /// Reads and checks the signatures of the file at `path`.
     pub fn ask_signatures(&mut self, path: PathBuf, cx: &mut Context<Self>) {
-        if self.signatures.as_ref().is_some_and(|(read, _)| *read == path) || self.reading_signatures {
+        let failed = self.unread.contains(&Unread::Signatures(path.clone()));
+        if self.signatures.as_ref().is_some_and(|(read, _)| *read == path) || self.reading_signatures || failed {
             return;
         }
         self.reading_signatures = true;
-        let (doc, asked) = (self.doc, path.clone());
+        // A save in between makes this read stale: dropped.
+        let (doc, asked, saves) = (self.doc, path.clone(), self.saves);
         self.off(
             move |worker| {
                 let fields = worker.signatures(doc, &path)?;
@@ -64,9 +66,16 @@ impl PdfView {
             },
             move |view, read: io::Result<Vec<Checked>>, cx| {
                 view.reading_signatures = false;
+                if view.saves != saves {
+                    log::info!("pdf: a signature read from before a save is dropped");
+                    return cx.notify();
+                }
                 match read {
                     Ok(read) => view.signatures = Some((asked, read)),
-                    Err(error) => view.fail("reading signatures", &error, cx),
+                    Err(error) => {
+                        view.unread.insert(Unread::Signatures(asked));
+                        view.fail("reading signatures", &error, cx)
+                    }
                 }
                 cx.notify();
             },

apps/ely-pdf/src/tests.rs

@@ -18,6 +18,7 @@ mod notes;
 mod organize;
 mod output;
 mod protect;
+mod reads;
 mod removal;
 mod sign;
 mod structure;

apps/ely-pdf/src/tests/reads.rs

@@ -0,0 +1,98 @@
+//! Reads that fail are kept, not asked each frame (P-08).
+
+use ely_pdf_engine::sample::{Extras, pdf_with};
+use gpui::TestAppContext;
+
+use super::{settle, shown, windowed};
+
+/// Two pages; the first's square has an unreadable rectangle.
+fn bad_then_good() -> Vec<u8> {
+    let nines = "9".repeat(50);
+    let objects = [
+        "<< /Type /Catalog /Pages 2 0 R >>".to_string(),
+        "<< /Type /Pages /Kids [3 0 R 4 0 R] /Count 2 >>".to_string(),
+        "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] /Annots [5 0 R] >>".to_string(),
+        "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] /Annots [6 0 R] >>".to_string(),
+        format!("<< /Type /Annot /Subtype /Square /Rect [0 0 {nines} 100] >>"),
+        "<< /Type /Annot /Subtype /Text /Rect [100 500 120 520] /Contents (fine) >>".to_string(),
+    ];
+    let mut pdf = b"%PDF-1.7\n".to_vec();
+    let mut offsets = Vec::new();
+    for (i, body) in objects.iter().enumerate() {
+        offsets.push(pdf.len());
+        pdf.extend(format!("{} 0 obj\n{body}\nendobj\n", i + 1).bytes());
+    }
+    let xref = pdf.len();
+    pdf.extend(format!("xref\n0 {}\n0000000000 65535 f \n", objects.len() + 1).bytes());
+    for offset in offsets {
+        pdf.extend(format!("{offset:010} 00000 n \n").bytes());
+    }
+    pdf.extend(format!("trailer\n<< /Size {} /Root 1 0 R >>\nstartxref\n{xref}\n%%EOF\n", objects.len() + 1).bytes());
+    pdf
+}
+
+#[test]
+fn one_bad_page_spares_the_rest_of_its_batch() {
+    let dir = tempfile::tempdir().expect("a temp dir");
+    let mut cx = TestAppContext::single();
+    let path = dir.path().join("two.pdf");
+    std::fs::write(&path, bad_then_good()).expect("writes");
+    let (viewer, cx) = windowed(&mut cx, dir.path(), &[path], true);
+    let view = shown(cx, &viewer);
+    settle(cx, |app| !view.read(app).reading_notes);
+    cx.update(|_, app| view.update(app, |view, cx| {
+        view.notes.clear();
+        view.unread.clear();
+        view.ask_notes([0, 1], cx)
+    }));
+    settle(cx, |app| !view.read(app).reading_notes);
+    let (good, bad) = cx.update(|_, app| {
+        let view = view.read(app);
+        (view.notes.contains_key(&1), view.unread.contains(&crate::view::Unread::Notes(0)))
+    });
+    assert!(bad, "the bad page is kept as failed");
+    assert!(good, "the good page is read all the same");
+    assert!(!cx.update(|_, app| view.read(app).unread.contains(&crate::view::Unread::Notes(1))));
+}
+
+#[test]
+fn a_signature_read_from_before_a_save_counts_for_nothing() {
+    let dir = tempfile::tempdir().expect("a temp dir");
+    let mut cx = TestAppContext::single();
+    let path = dir.path().join("one.pdf");
+    let page: (f32, f32, &[&str]) = (612.0, 792.0, &["Hello"]);
+    std::fs::write(&path, pdf_with(&[page], &[], &Extras::default())).expect("writes");
+    let (viewer, cx) = windowed(&mut cx, dir.path(), &[path], true);
+    let view = shown(cx, &viewer);
+    let gone = dir.path().join("gone.pdf");
+    // The read fails, but a save lands before its answer.
+    cx.update(|_, app| view.update(app, |view, cx| {
+        view.ask_signatures(gone.clone(), cx);
+        let same = view.saved.clone();
+        view.saved(&same, cx);
+    }));
+    settle(cx, |app| !view.read(app).reading_signatures);
+    assert!(cx.update(|_, app| view.read(app).unread.is_empty()), "a stale failure blocks nothing");
+    cx.update(|_, app| view.update(app, |view, cx| view.ask_signatures(gone.clone(), cx)));
+    assert!(cx.update(|_, app| view.read(app).reading_signatures), "asked again after the save");
+}
+
+#[test]
+fn a_failed_read_is_not_asked_again() {
+    let dir = tempfile::tempdir().expect("a temp dir");
+    let mut cx = TestAppContext::single();
+    let path = dir.path().join("one.pdf");
+    let page: (f32, f32, &[&str]) = (612.0, 792.0, &["Hello"]);
+    std::fs::write(&path, pdf_with(&[page], &[], &Extras::default())).expect("writes");
+    let (viewer, cx) = windowed(&mut cx, dir.path(), &[path], true);
+    let view = shown(cx, &viewer);
+    assert!(cx.update(|_, app| view.read(app).form_names.is_none()), "not read yet");
+    // A dead worker fails every read.
+    let pid = cx.update(|_, app| viewer.read(app).core.worker.lock().expect("the lock").get().expect("a worker").pid());
+    std::process::Command::new("kill").args(["-9", &pid.to_string()]).status().expect("kills");
+    cx.update(|_, app| view.update(app, |view, cx| view.ask_names(cx)));
+    settle(cx, |app| !view.read(app).reading_names);
+    assert!(cx.update(|_, app| view.read(app).unread.contains(&crate::view::Unread::Names)), "the failure is kept");
+    cx.update(|_, app| view.update(app, |view, cx| view.ask_names(cx)));
+    assert!(!cx.update(|_, app| view.read(app).reading_names), "not asked again");
+}

apps/ely-pdf/src/view.rs

@@ -23,6 +23,17 @@ pub struct Failed(pub String);
 /// The document changed what it shows.
 pub struct Reported;
 
+/// A page or document read that failed once.
+#[derive(Clone, Debug, PartialEq, Eq, Hash)]
+pub enum Unread {
+    Notes(usize),
+    Layer(usize),
+    Names,
+    Files,
+    /// The signatures of the file at this path.
+    Signatures(std::path::PathBuf),
+}
+
 pub struct PdfView {
     worker: Arc<Mutex<Mupdf>>,
     pub doc: Doc,
@@ -41,6 +52,8 @@ pub struct PdfView {
     pub painted: f64,
     /// Tiles the worker could not draw; not asked again.
     pub failed: HashSet<TileKey>,
+    /// Reads that failed; not asked again until an edit.
+    pub unread: HashSet<Unread>,
     /// Old tiles shown under their page until covered.
     pub backdrop: Vec<crate::backdrop::Old>,
     pub text: HashMap<usize, Arc<Vec<Char>>>,
@@ -70,6 +83,8 @@ pub struct PdfView {
     /// History marks: now, and at the last load or save.
     pub mark: String,
     pub saved: String,
+    /// Saves so far: a read from before one is stale.
+    pub saves: u64,
     /// Page pictures for the side panel, by page and darkness.
     pub thumbs: HashMap<(usize, bool), Arc<RenderImage>>,
     pub thumbs_failed: HashSet<usize>,
@@ -173,6 +188,7 @@ impl PdfView {
             retired: Default::default(),
             painted: 0.0,
             failed: HashSet::new(),
+            unread: HashSet::new(),
             backdrop: Vec::new(),
             text: HashMap::new(),
             asking_text: HashSet::new(),
@@ -190,6 +206,7 @@ impl PdfView {
             editing: false,
             moving: false,
             saved: loaded.mark.clone(),
+            saves: 0,
             mark: loaded.mark,
             thumbs: HashMap::new(),
             thumbs_failed: HashSet::new(),

tasks/BUGS.md

@@ -40,7 +40,7 @@
 - [x] P-05 连续快画或快点被拒：“文档在此间已改动，未执行” — 按坐标的编辑排队、跑在出队时的状态上，只在页面变动时拒绝；R2 PASS
 - [x] P-06 默认字体“原字体”在 Type 1/Type 3 字体的文件上失败，只给引擎原话 — 原字体写不了新字（非 CID 编码、页面内直接字体、不在本页）时面板提示换字体；R2 PASS
 - [x] P-07 含连字的文字块永远不可编辑 — 字形按 stext 的写法展开（多字字形、连字码点），整页、单块与删除同一判法；R3 PASS
-- [ ] P-08 页面读取失败后每帧重试、每帧弹提示
+- [x] P-08 页面读取失败后每帧重试、每帧弹提示 — 失败的读取按页或种类记下，编辑后重读；签名读取以保存计数判过期
 - [ ] P-09 识别文字的写入被拒后仍弹“成功”
 - [ ] P-10 预览回复被丢弃时“应用”静默无事
 - [ ] P-11 撤销或重做在 MuPDF 侧失败后 worker 的步数不再同步