From bd7bc46d8c3e16f01ddabb1d2ef6a33eadf3d41b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Fri, 9 Oct 2026 23:26:40 -0400 Subject: [PATCH] ElyPDF: failed reads stay unread until an edit; a save stales a signature read (P-08) --- apps/ely-pdf/src/annotate.rs | 45 +++++++++++---- apps/ely-pdf/src/attach.rs | 9 ++- apps/ely-pdf/src/content.rs | 11 +++- apps/ely-pdf/src/edits.rs | 3 + apps/ely-pdf/src/sign.rs | 17 ++++-- apps/ely-pdf/src/tests.rs | 1 + apps/ely-pdf/src/tests/reads.rs | 98 +++++++++++++++++++++++++++++++++ apps/ely-pdf/src/view.rs | 17 ++++++ tasks/BUGS.md | 2 +- 9 files changed, 180 insertions(+), 23 deletions(-) create mode 100644 apps/ely-pdf/src/tests/reads.rs diff --git a/apps/ely-pdf/src/annotate.rs b/apps/ely-pdf/src/annotate.rs index c33dd93..ddc5f20 100644 --- a/apps/ely-pdf/src/annotate.rs +++ b/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 = pages.into_iter().filter(|page| !self.notes.contains_key(page)).take(BATCH).collect(); + let wanted: Vec = (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::>>() + let read = (wanted.into_iter()) + .map(|page| (page, worker.marks(doc, page).and_then(|marks| Ok((marks, worker.links(doc, page)?))))) + .collect::>(); + Ok(read) }, - move |view, read, cx| { + move |view, read: std::io::Result>, 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) { - 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(); }, diff --git a/apps/ely-pdf/src/attach.rs b/apps/ely-pdf/src/attach.rs index 92e86cd..3309f15 100644 --- a/apps/ely-pdf/src/attach.rs +++ b/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) { - 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(); }, diff --git a/apps/ely-pdf/src/content.rs b/apps/ely-pdf/src/content.rs index a718080..cb2bae3 100644 --- a/apps/ely-pdf/src/content.rs +++ b/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(); }, diff --git a/apps/ely-pdf/src/edits.rs b/apps/ely-pdf/src/edits.rs index 93cdc58..721f771 100644 --- a/apps/ely-pdf/src/edits.rs +++ b/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.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); } diff --git a/apps/ely-pdf/src/sign.rs b/apps/ely-pdf/src/sign.rs index 6f73596..2f8a4c4 100644 --- a/apps/ely-pdf/src/sign.rs +++ b/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) { - 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>, 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(); }, diff --git a/apps/ely-pdf/src/tests.rs b/apps/ely-pdf/src/tests.rs index b969cdc..40ba2c9 100644 --- a/apps/ely-pdf/src/tests.rs +++ b/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; diff --git a/apps/ely-pdf/src/tests/reads.rs b/apps/ely-pdf/src/tests/reads.rs new file mode 100644 index 0000000..cb909d7 --- /dev/null +++ b/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 { + 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"); +} diff --git a/apps/ely-pdf/src/view.rs b/apps/ely-pdf/src/view.rs index 7a7cf97..8f9e212 100644 --- a/apps/ely-pdf/src/view.rs +++ b/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>, 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, + /// Reads that failed; not asked again until an edit. + pub unread: HashSet, /// Old tiles shown under their page until covered. pub backdrop: Vec, pub text: HashMap>>, @@ -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>, pub thumbs_failed: HashSet, @@ -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(), diff --git a/tasks/BUGS.md b/tasks/BUGS.md index 3f36727..15a6bfd 100644 --- a/tasks/BUGS.md +++ b/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 的步数不再同步