From 55f76ab767cd7740f99e28b0ca83d0aaecd944e9 Mon Sep 17 00:00:00 2001 From: cyfung1031 <44498510+cyfung1031@users.noreply.github.com> Date: Tue, 13 Oct 2026 13:04:23 +0900 Subject: [PATCH] Desktop: prepare large PDFs off the UI thread (#1493) * #307: split document open into prepare and commit * #307: move desktop open preparation off the frame thread --- crates/engine/src/lib.rs | 113 ++++++--- crates/engine/src/tests.rs | 14 ++ crates/ui-egui/src/control.rs | 11 +- crates/ui-egui/src/lib.rs | 314 +++++++++++++++++++++--- crates/ui-egui/src/open_worker.rs | 208 ++++++++++++++++ crates/ui-egui/src/open_worker_tests.rs | 90 +++++++ crates/ui-egui/src/recovery.rs | 19 ++ 7 files changed, 706 insertions(+), 63 deletions(-) create mode 100644 crates/ui-egui/src/open_worker.rs create mode 100644 crates/ui-egui/src/open_worker_tests.rs diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 4a71f7855c..76bdc381b6 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -2609,6 +2609,34 @@ pub struct Session { date_language: Option, } +/// Immutable session state needed while a document is prepared away from its owning thread. +#[derive(Clone)] +pub struct OpenContext { + now: Option, + js_off: bool, + trust: Arc, +} + +/// A fully prepared document that has not yet been registered in a session. +pub struct PreparedDocument { + document: Document, +} + +impl OpenContext { + /// Prepare a document without mutating the session that supplied this context. + pub fn prepare_open( + &self, + name: impl Into, + path: Option, + bytes: Arc>, + password: Option<&str>, + ) -> Result { + let name = name.into(); + guard(|| Session::prepare_open_unguarded(self, name, path, bytes, password)) + .unwrap_or_else(|m| Err(OpenError::Invalid(format!("reading it failed unexpectedly ({m})")))) + } +} + /// Lay a dynamic XFA form out (pages and fields) and give its widgets appearances. fn xfa_layout(doc: &mut pdfcraft_cos::Document) -> Result { let mut report = pdfcraft_xfa::render_into(doc).map_err(|e| e.to_string())?; @@ -2901,13 +2929,45 @@ impl Session { self.open(name, None, bytes, password.as_deref()).map_err(|e| e.to_string()) } - pub fn open(&mut self, name: impl Into, path: Option, bytes: Arc>, password: Option<&str>) -> Result { - let name = name.into(); - guard(|| self.open_unguarded(name, path, bytes, password)) - .unwrap_or_else(|m| Err(OpenError::Invalid(format!("reading it failed unexpectedly ({m})")))) + /// Snapshot the session state that affects open-time scripts, dates and signature trust. + pub fn open_context(&self) -> OpenContext { + OpenContext { now: self.now(), js_off: self.js_off, trust: self.trust.clone() } } - fn open_unguarded(&mut self, name: String, path: Option, bytes: Arc>, password: Option<&str>) -> Result { + /// Prepare a document without adding it to this session. + pub fn prepare_open( + &self, + name: impl Into, + path: Option, + bytes: Arc>, + password: Option<&str>, + ) -> Result { + self.open_context().prepare_open(name, path, bytes, password) + } + + /// Register a prepared document. This is the only open step that mutates the session. + pub fn commit_open(&mut self, prepared: PreparedDocument) -> DocId { + self.next_id += 1; + let id = DocId(self.next_id); + let mut document = prepared.document; + document.id = id; + self.docs.push(document); + id + } + + /// Synchronous convenience API used by the CLI and automation. + pub fn open(&mut self, name: impl Into, path: Option, bytes: Arc>, password: Option<&str>) -> Result { + let prepared = self.prepare_open(name, path, bytes, password)?; + Ok(self.commit_open(prepared)) + } + + fn prepare_open_unguarded( + context: &OpenContext, + name: String, + path: Option, + bytes: Arc>, + password: Option<&str>, + ) -> Result { let cos = std::panic::catch_unwind(|| pdfcraft_cos::Document::open_with_password(bytes.clone(), password)); // The renderer authenticates on its own. It cannot use the owner password of R2–R4 // files, so give it the user password that owner authentication recovers. @@ -2926,7 +2986,7 @@ impl Session { // A dynamic XFA form is a shell around an XML template; lay the template out into real // pages and fields so the rest of the engine works on it. The original bytes stay: the // laid-out form is one appended revision. - let opts = SaveOptions { mod_date: self.now().map(pdfcraft_cos::pdf_date), ..SaveOptions::default() }; + let opts = SaveOptions { mod_date: context.now.map(pdfcraft_cos::pdf_date), ..SaveOptions::default() }; // Write `work` as one more revision and reopen it: what the document then is. let rebase = |work: &pdfcraft_cos::Document| -> Result<(Arc>, DocInfo, pdfcraft_cos::Document), String> { let new_bytes = write_incremental(work, &opts).map(Arc::new).map_err(|e| e.to_string())?; @@ -2995,7 +3055,7 @@ impl Session { _ => None, }; let (bytes, info, cos) = match (&xfa, cos) { - (Some(_), Ok(Ok(cos))) if !self.js_off => match xfa_template.clone() { + (Some(_), Ok(Ok(cos))) if !context.js_off => match xfa_template.clone() { Some(tpl) => { let mut work = cos.clone(); let pages = info.pages.len(); @@ -3038,24 +3098,22 @@ impl Session { }; // XFA notes survive the document being re-read after edits. let xfa_warnings: Vec = if info.xfa.is_some() { info.warnings.clone() } else { Vec::new() }; - let id = self.push_document(name, path, bytes, info, cos, render_password, password, xfa)?; - if let Some(d) = self.docs.iter_mut().find(|d| d.id == id) { - // A script that ran away on open turns the form's scripts off for this document. - d.xfa_template = if ran_away { None } else { xfa_template }; - d.js_output.append(script_output); - if ran_away { - d.xfa_scripts_off = true; - d.js_output.errors.push(XFA_SCRIPTS_OFF.into()); - } - note_warnings(&mut d.xfa_warnings, &xfa_warnings); + let mut document = Self::build_document(&context.trust, name, path, bytes, info, cos, render_password, password, xfa); + // A script that ran away on open turns the form's scripts off for this document. + document.xfa_template = if ran_away { None } else { xfa_template }; + document.js_output.append(script_output); + if ran_away { + document.xfa_scripts_off = true; + document.js_output.errors.push(XFA_SCRIPTS_OFF.into()); } - Ok(id) + note_warnings(&mut document.xfa_warnings, &xfa_warnings); + Ok(PreparedDocument { document }) } /// The last step of opening: build the document record and register it. #[allow(clippy::too_many_arguments)] - fn push_document( - &mut self, + fn build_document( + trust: &Arc, name: String, path: Option, bytes: Arc>, @@ -3064,7 +3122,7 @@ impl Session { render_password: Option, password: Option<&str>, xfa: Option, - ) -> Result { + ) -> Document { let config = RenderConfig { password: render_password.as_deref().map(Arc::from), ..Default::default() }; let (mut editor, read_only_reason) = match cos { Ok(Ok(cos)) => { @@ -3114,11 +3172,9 @@ impl Session { .unwrap_or_default(); let links = editor.as_ref().map(|e| pdfcraft_annot::links::list(&e.cos)).unwrap_or_default(); let sig_cache = Arc::new(pdfcraft_sign::DigestCache::default()); - let signatures = editor.as_ref().map(|e| signatures_of(&e.cos, &bytes, &self.trust, &sig_cache)).unwrap_or_default(); - self.next_id += 1; - let id = DocId(self.next_id); - self.docs.push(Document { - id, + let signatures = editor.as_ref().map(|e| signatures_of(&e.cos, &bytes, trust, &sig_cache)).unwrap_or_default(); + Document { + id: DocId(0), name, path, bytes, @@ -3135,7 +3191,7 @@ impl Session { added, links, signatures, - trust: self.trust.clone(), + trust: trust.clone(), sig_cache, editor, config, @@ -3146,8 +3202,7 @@ impl Session { fonts_stale: false, xfa_warnings: Vec::new(), edit_notices: Vec::new(), - }); - Ok(id) + } } fn doc_mut(&mut self, id: DocId) -> Result<&mut Document, EditError> { diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 0ec09c2f2c..ff98c2df53 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -36,6 +36,20 @@ fn session_with(n: usize) -> (Session, DocId) { (s, id) } +#[test] +fn prepared_open_does_not_mutate_the_session_until_commit() { + fn assert_send() {} + assert_send::(); + + let mut session = Session::new(); + let prepared = session.prepare_open("prepared.pdf", None, Arc::new(fixture(3)), None).expect("prepares"); + assert!(session.get(DocId(1)).is_none()); + + let id = session.commit_open(prepared); + assert_eq!(id, DocId(1)); + assert_eq!(session.get(id).expect("committed document").info.pages.len(), 3); +} + #[test] fn custom_image_stamp_keeps_displayed_orientation_on_rotated_pages() { let colours = [[240, 20, 20, 255], [20, 180, 20, 255], [20, 20, 240, 255], [230, 180, 20, 255]]; diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index 65a12e1997..d83553105e 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -36,7 +36,8 @@ //! for this session without changing the saved preference. `ui.state.default_mode` reports it. //! `theme=light|dark|system` changes the saved theme preference; `ui.state.theme_preference` //! reports the choice and `ui.state.theme` reports the resolved light/dark colours. -//! - `ui.open {path}`: open a file. +//! - `ui.open {path}`: accept a file open; native opens report `pending: true` until they +//! complete and are listed in `ui.state.pending_opens`. //! - `ui.screenshot {region?}`: PNG of the window (base64), optionally cropped to a rect. use std::collections::{HashMap, VecDeque}; @@ -642,9 +643,15 @@ impl Host for crate::PdfCraftApp { Some(json!({ "tab": i, "doc": v.id.0, "name": d.name, "path": d.path, "pages": d.info.pages.len(), "dirty": d.dirty, "active": self.active == Some(i) })) }) .collect(); + #[cfg(not(target_arch = "wasm32"))] + let pending_opens: Vec = + self.pending_opens.iter().map(|pending| json!({ "token": pending.token, "name": pending.name, "path": pending.path })).collect(); + #[cfg(target_arch = "wasm32")] + let pending_opens: Vec = Vec::new(); json!({ "docking": self.docking, "documents": docs, + "pending_opens": pending_opens, "active": active.map(|v| json!({ "doc": v.id.0, "page": v.current + 1, @@ -743,6 +750,8 @@ impl Host for crate::PdfCraftApp { self.open_path(path); if self.views.len() > before || self.password_prompt.is_some() { Ok(json!({ "opened": path, "password_prompt": self.password_prompt.is_some() })) + } else if self.pending_opens.iter().any(|pending| pending.path.as_deref() == Some(path)) { + Ok(json!({ "accepted": path, "pending": true })) } else { Err(self.toast.as_ref().map(|t| t.0.clone()).unwrap_or_else(|| format!("couldn't open {path}"))) } diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 2d4f5b0993..3418fcdf8f 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -117,6 +117,8 @@ pub(crate) fn date_text_for_pdf(session: &Session) -> Result { /// The longest author name kept (Preferences ▸ Identity, restored settings). pub(crate) const MAX_AUTHOR_CHARS: usize = 200; +#[cfg(not(target_arch = "wasm32"))] +mod open_worker; pub mod portable; mod protect; mod recovery; @@ -130,6 +132,9 @@ pub mod updates; mod wheel_pager; mod widgets; +#[cfg(all(test, not(target_arch = "wasm32")))] +mod open_worker_tests; + use pdfcraft_engine::{DocId, Session}; pub use canvas::{DocView, RasterMemory}; @@ -392,6 +397,8 @@ pub struct PasswordPrompt { pub name: String, pub path: Option, pub bytes: std::sync::Arc>, + #[cfg(not(target_arch = "wasm32"))] + stamp: Option, pub input: String, pub error: Option, /// A late startup file must stay in the background even after it is unlocked. @@ -406,6 +413,32 @@ pub struct PasswordPrompt { after_open: Option, } +#[cfg(not(target_arch = "wasm32"))] +struct PendingOpen { + token: u64, + receiver: std::sync::mpsc::Receiver, + name: String, + path: Option, + stamp: Option, + activate: bool, + after_open: Option, + recovered: Option, + then: Option<&'static str>, +} + +#[cfg(not(target_arch = "wasm32"))] +struct OpenRequest { + name: String, + path: Option, + stamp: Option, + input: open_worker::Input, + password: Option, + activate: bool, + after_open: Option, + recovered: Option, + then: Option<&'static str>, +} + /// Asks for the permissions password of an open document whose security restricts it, which /// lifts the restrictions (#995). Acrobat asks for it when such a document's security is changed. pub struct UnlockPrompt { @@ -524,6 +557,15 @@ pub struct PdfCraftApp { /// Open documents' files, watched for changes on disk. #[cfg(not(target_arch = "wasm32"))] pub(crate) watch: file_watch::FileWatch, + /// One native worker prepares opened documents without owning app or session state. + #[cfg(not(target_arch = "wasm32"))] + open_worker: Option, + #[cfg(not(target_arch = "wasm32"))] + pending_opens: Vec, + #[cfg(not(target_arch = "wasm32"))] + next_open_token: u64, + #[cfg(all(test, not(target_arch = "wasm32")))] + open_prepare_gate: Option>, /// Folders pinned to Home, and what they held when last listed. pub pinned: folders_ui::PinnedFolders, pub toast: Option<(String, f64)>, @@ -844,6 +886,14 @@ impl PdfCraftApp { reload_changed_files: true, #[cfg(not(target_arch = "wasm32"))] watch: Default::default(), + #[cfg(not(target_arch = "wasm32"))] + open_worker: None, + #[cfg(not(target_arch = "wasm32"))] + pending_opens: Vec::new(), + #[cfg(not(target_arch = "wasm32"))] + next_open_token: 0, + #[cfg(all(test, not(target_arch = "wasm32")))] + open_prepare_gate: None, pinned: Default::default(), toast: None, integrated_titlebar: false, @@ -1032,6 +1082,119 @@ impl PdfCraftApp { self.try_open(name, path, std::sync::Arc::new(bytes), None, activate) } + #[cfg(not(target_arch = "wasm32"))] + fn schedule_open(&mut self, request: OpenRequest) -> Result { + if self.open_worker.is_none() { + self.open_worker = Some( + open_worker::OpenWorker::new( + #[cfg(test)] + self.open_prepare_gate.clone(), + ) + .map_err(|e| format!("couldn't start the document-open worker: {e}"))?, + ); + } + let token = self.next_open_token.wrapping_add(1); + let worker = self.open_worker.as_ref().ok_or("the document-open worker is unavailable")?; + if let Some(ctx) = &self.ctx { + worker.set_context(ctx); + } + let receiver = + worker.submit(token, self.session.open_context(), request.name.clone(), request.path.clone(), request.password, request.input)?; + self.next_open_token = token; + self.pending_opens.push(PendingOpen { + token, + receiver, + name: request.name, + path: request.path, + stamp: request.stamp, + activate: request.activate, + after_open: request.after_open, + recovered: request.recovered, + then: request.then, + }); + Ok(token) + } + + #[cfg(not(target_arch = "wasm32"))] + fn poll_pending_opens(&mut self) { + let mut ready = Vec::new(); + let mut waiting = Vec::new(); + let pending_opens = std::mem::take(&mut self.pending_opens); + for pending in pending_opens { + match pending.receiver.try_recv() { + Ok(result) if result.token == pending.token => ready.push((pending, result.kind)), + Ok(_) => waiting.push(pending), + Err(std::sync::mpsc::TryRecvError::Empty) => waiting.push(pending), + Err(std::sync::mpsc::TryRecvError::Disconnected) => { + self.notify_fmt("Couldn't open {name}: the background worker stopped", &[("name", &pending.name)]); + } + } + } + self.pending_opens = waiting; + for (pending, kind) in ready { + self.finish_pending_open(pending, kind); + } + } + + #[cfg(not(target_arch = "wasm32"))] + fn finish_pending_open(&mut self, pending: PendingOpen, kind: open_worker::ResultKind) { + match kind { + open_worker::ResultKind::ReadFailed(reason) => { + self.notify_fmt("Couldn't read {name}: {e}", &[("name", &pending.name), ("e", &reason)]); + } + open_worker::ResultKind::RemoteFailed(reason) => { + self.notify_fmt("Couldn't follow linked PDF: {reason}", &[("reason", &reason)]); + } + open_worker::ResultKind::RecoveryFailed(reason) => { + self.notify_fmt("Couldn't recover {name}: {e}", &[("name", &pending.name), ("e", &reason)]); + } + open_worker::ResultKind::Failed(reason) => { + self.notify_fmt("Couldn't open {name}: {reason}", &[("name", &pending.name), ("reason", &reason)]); + } + open_worker::ResultKind::Conversion(bytes) => { + if let Some(Err(error)) = self.open_converted(&pending.name, &bytes) { + self.notify_fmt("Couldn't open {name}: {e}", &[("name", &pending.name), ("e", &error)]); + } + } + open_worker::ResultKind::Prepared { bytes, result } => match *result { + Ok(prepared) => match self.finish_open(&pending.name, pending.path.clone(), bytes.len(), prepared, pending.activate) { + Ok(id) => { + if pending.path.is_some() { + self.watch.known_as(id, pending.stamp); + } + if let Some(destination) = pending.after_open { + self.apply_remote_destination(destination); + } + if let Some(meta) = &pending.recovered { + self.finish_recovery(meta); + } + if let Some(command) = pending.then { + self.execute(command); + } + } + Err(error) => self.notify_fmt("Couldn't open {name}: {e}", &[("name", &pending.name), ("e", &error)]), + }, + Err(error @ (pdfcraft_render::OpenError::NeedsPassword | pdfcraft_render::OpenError::WrongPassword)) => { + let password_error = + matches!(error, pdfcraft_render::OpenError::WrongPassword).then(|| "Incorrect password. Try again.".to_string()); + self.password_prompt = Some(PasswordPrompt { + name: pending.name, + path: pending.path, + bytes, + stamp: pending.stamp, + input: String::new(), + error: password_error, + activate: pending.activate, + recovered: pending.recovered, + then: pending.then, + after_open: pending.after_open, + }); + } + Err(error) => self.notify_fmt("Couldn't open {name}: {e}", &[("name", &pending.name), ("e", &error.to_string())]), + }, + } + } + fn try_open( &mut self, name: &str, @@ -1041,15 +1204,18 @@ impl PdfCraftApp { activate: bool, ) -> Result<(), String> { use pdfcraft_render::OpenError; - let size = bytes.len(); - let id = match self.session.open(name, path.clone(), bytes.clone(), password) { - Ok(id) => id, + let prepared = match self.session.prepare_open(name, path.clone(), bytes.clone(), password) { + Ok(prepared) => prepared, Err(e @ (OpenError::NeedsPassword | OpenError::WrongPassword)) => { let error = matches!(e, OpenError::WrongPassword).then(|| "Incorrect password. Try again.".to_string()); + #[cfg(not(target_arch = "wasm32"))] + let stamp = path.as_deref().and_then(file_watch::Stamp::at); self.password_prompt = Some(PasswordPrompt { name: name.to_string(), path, bytes, + #[cfg(not(target_arch = "wasm32"))] + stamp, input: String::new(), error, activate, @@ -1061,6 +1227,18 @@ impl PdfCraftApp { } Err(e) => return Err(e.to_string()), }; + self.finish_open(name, path, bytes.len(), prepared, activate).map(|_| ()) + } + + fn finish_open( + &mut self, + name: &str, + path: Option, + size: usize, + prepared: pdfcraft_engine::PreparedDocument, + activate: bool, + ) -> Result { + let id = self.session.commit_open(prepared); self.password_prompt = None; // The reading-colours preference is app-wide: a newly opened document starts with it. // Idempotent: the engine returns `false` when the document has the setting already. @@ -1115,7 +1293,7 @@ impl PdfCraftApp { let out = self.session.take_js_output(id); self.handle_js(id, out); } - Ok(()) + Ok(id) } #[cfg(not(target_arch = "wasm32"))] @@ -1128,8 +1306,23 @@ impl PdfCraftApp { let name = f.path().file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_else(|| "dropped.pdf".into()); match f.bytes() { Ok(bytes) => { - if let Err(e) = self.open_bytes(&name, None, bytes) { - self.notify_fmt("Couldn't open {name}: {e}", &[("name", &name), ("e", &e.to_string())]); + if self.ctx.is_some() && !self.run_inline { + let result = self.schedule_open(OpenRequest { + name: name.clone(), + path: None, + stamp: None, + input: open_worker::Input::Bytes(std::sync::Arc::new(bytes)), + password: None, + activate: true, + after_open: None, + recovered: None, + then: None, + }); + if let Err(error) = result { + self.notify_fmt("Couldn't open {name}: {e}", &[("name", &name), ("e", &error)]); + } + } else if let Err(error) = self.open_bytes(&name, None, bytes) { + self.notify_fmt("Couldn't open {name}: {e}", &[("name", &name), ("e", &error)]); } } Err(e) => self.notify_fmt("Couldn't read {name}: {e}", &[("name", &name), ("e", &e.to_string())]), @@ -1260,11 +1453,43 @@ impl PdfCraftApp { } return; }; + #[cfg(not(target_arch = "wasm32"))] + if self.ctx.is_some() && !self.run_inline { + let result = self.schedule_open(OpenRequest { + name: p.name.clone(), + path: p.path.clone(), + stamp: p.stamp, + input: open_worker::Input::Bytes(p.bytes.clone()), + password: Some(pw), + activate: p.activate, + after_open: p.after_open.clone(), + recovered: p.recovered.clone(), + then: p.then, + }); + if let Err(error) = result { + let mut prompt = p; + prompt.error = Some(error); + self.password_prompt = Some(prompt); + } + return; + } + #[cfg(not(target_arch = "wasm32"))] + let (path_for_watch, stamp_for_watch) = (p.path.clone(), p.stamp); let after_open = p.after_open.clone(); match self.try_open(&p.name, p.path, p.bytes, Some(&pw), p.activate) { Err(e) => self.notify_fmt("Couldn't open {name}: {e}", &[("name", &p.name), ("e", &e.to_string())]), // A recovered encrypted document is open once its prompt is gone. Ok(()) if self.password_prompt.is_none() => { + #[cfg(not(target_arch = "wasm32"))] + if let Some(path) = path_for_watch.as_deref() + && let Some(id) = self + .views + .iter() + .rev() + .find_map(|view| self.session.get(view.id).and_then(|doc| (doc.path.as_deref() == Some(path)).then_some(view.id))) + { + self.watch.known_as(id, stamp_for_watch); + } if let Some(destination) = after_open { self.apply_remote_destination(destination); } @@ -1338,24 +1563,20 @@ impl PdfCraftApp { let path = path.to_string_lossy().into_owned(); let name = std::path::Path::new(&path).file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_else(|| path.clone()); let stamp = file_watch::Stamp::at(&path); - let tabs = self.views.len(); self.startup_superseded = true; - let bytes = match pdfcraft_engine::read_remote_document(std::path::Path::new(&path)) { - Ok(bytes) => bytes, - Err(e) => { - self.notify_fmt("Couldn't follow linked PDF: {reason}", &[("reason", &e.to_string())]); - return; - } - }; - if let Err(e) = self.try_open(&name, Some(path), std::sync::Arc::new(bytes), None, true) { - self.notify_fmt("Couldn't open {name}: {e}", &[("name", &name), ("e", &e.to_string())]); - } else if let Some(prompt) = self.password_prompt.as_mut() { - prompt.after_open = Some(destination); - } else if self.views.len() > tabs { - if let Some(view) = self.views.last() { - self.watch.known_as(view.id, stamp); - } - self.apply_remote_destination(destination); + let result = self.schedule_open(OpenRequest { + name: name.clone(), + path: Some(path.clone()), + stamp, + input: open_worker::Input::RemotePath(std::path::PathBuf::from(path)), + password: None, + activate: true, + after_open: Some(destination), + recovered: None, + then: None, + }); + if let Err(error) = result { + self.notify_fmt("Couldn't follow linked PDF: {reason}", &[("reason", &error)]); } } @@ -1386,18 +1607,38 @@ impl PdfCraftApp { let name = std::path::Path::new(path).file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_else(|| path.to_string()); // Taken before reading: a build that rewrites the file meanwhile still counts as a change. let stamp = file_watch::Stamp::at(path); - let tabs = self.views.len(); - match std::fs::read(path) { - Ok(bytes) => { - if let Err(e) = self.open_bytes(&name, Some(path.to_string()), bytes) { - self.notify_fmt("Couldn't open {name}: {e}", &[("name", &name), ("e", &e.to_string())]); - } else if self.views.len() > tabs - && let Some(doc) = self.views.last().map(|v| v.id) - { - self.watch.known_as(doc, stamp); + // Before the first frame there is no interaction to keep responsive; this also preserves + // deterministic inline startup and test flows. Once live, reading and preparation use + // the bounded worker and commit on the UI thread. + if self.ctx.is_none() || self.run_inline { + match std::fs::read(path) { + Ok(bytes) => { + let before = self.views.len(); + if let Err(error) = self.open_bytes(&name, Some(path.to_string()), bytes) { + self.notify_fmt("Couldn't open {name}: {e}", &[("name", &name), ("e", &error)]); + } else if self.views.len() > before + && let Some(doc) = self.views.last().map(|view| view.id) + { + self.watch.known_as(doc, stamp); + } } + Err(error) => self.notify_fmt("Couldn't read {name}: {e}", &[("name", &name), ("e", &error.to_string())]), } - Err(e) => self.notify_fmt("Couldn't read {name}: {e}", &[("name", &name), ("e", &e.to_string())]), + return; + } + self.startup_superseded = true; + if let Err(error) = self.schedule_open(OpenRequest { + name: name.clone(), + path: Some(path.to_string()), + stamp, + input: open_worker::Input::Path(std::path::PathBuf::from(path)), + password: None, + activate: true, + after_open: None, + recovered: None, + then: None, + }) { + self.notify_fmt("Couldn't open {name}: {e}", &[("name", &name), ("e", &error)]); } } @@ -2395,6 +2636,13 @@ impl eframe::App for PdfCraftApp { // Notices raised outside `ui` (opened files, OS events, the control channel) translate too. let lang = i18n::Lang::from_pref(&self.language); i18n::set_current(lang); + #[cfg(not(target_arch = "wasm32"))] + { + if let Some(worker) = &self.open_worker { + worker.set_context(ctx); + } + self.poll_pending_opens(); + } // Chinese wants its own faces before the Japanese ones (one baseline per line). let cjk = theme::CjkPreference::for_language(lang.code()); if !self.styled { diff --git a/crates/ui-egui/src/open_worker.rs b/crates/ui-egui/src/open_worker.rs new file mode 100644 index 0000000000..04bd1fefe5 --- /dev/null +++ b/crates/ui-egui/src/open_worker.rs @@ -0,0 +1,208 @@ +//! Native document reads and preparation run away from the frame thread. + +use std::collections::VecDeque; +use std::path::PathBuf; +use std::sync::mpsc::{self, Receiver, Sender}; +use std::sync::{Arc, Condvar, Mutex, PoisonError}; + +use crate::RecoveryStore; +use pdfcraft_engine::{OpenContext, PreparedDocument, SourceKind}; +use pdfcraft_render::OpenError; + +const MAX_QUEUED_OPENS: usize = 16; + +pub(crate) enum Input { + Path(PathBuf), + RemotePath(PathBuf), + Recovery(RecoveryStore, String), + Bytes(Arc>), +} + +pub(crate) enum ResultKind { + Conversion(Arc>), + Prepared { bytes: Arc>, result: Box> }, + ReadFailed(String), + RemoteFailed(String), + RecoveryFailed(String), + Failed(String), +} + +pub(crate) struct ResultMessage { + pub token: u64, + pub kind: ResultKind, +} + +struct Job { + token: u64, + context: OpenContext, + name: String, + path: Option, + password: Option, + input: Input, + reply: Sender, + #[cfg(test)] + gate: Option>, +} + +struct Queue { + jobs: VecDeque, + shutdown: bool, + repaint: Option, +} + +pub(crate) struct OpenWorker { + queue: Arc<(Mutex, Condvar)>, + #[cfg(test)] + gate: Option>, +} + +impl OpenWorker { + pub fn new(#[cfg(test)] gate: Option>) -> std::io::Result { + let queue = Arc::new((Mutex::new(Queue { jobs: VecDeque::new(), shutdown: false, repaint: None }), Condvar::new())); + let worker_queue = queue.clone(); + std::thread::Builder::new().name("pdfcraft-open".into()).spawn(move || run(worker_queue))?; + Ok(Self { + queue, + #[cfg(test)] + gate, + }) + } + + pub fn set_context(&self, ctx: &egui::Context) { + let (lock, _) = &*self.queue; + let mut queue = lock.lock().unwrap_or_else(PoisonError::into_inner); + queue.repaint = Some(ctx.clone()); + } + + pub fn submit( + &self, + token: u64, + context: OpenContext, + name: String, + path: Option, + password: Option, + input: Input, + ) -> Result, String> { + let (reply, receiver) = mpsc::channel(); + let job = Job { + token, + context, + name, + path, + password, + input, + reply, + #[cfg(test)] + gate: self.gate.clone(), + }; + let (lock, ready) = &*self.queue; + let mut queue = lock.lock().unwrap_or_else(PoisonError::into_inner); + if queue.shutdown { + return Err("the document-open worker has stopped".into()); + } + if queue.jobs.len() >= MAX_QUEUED_OPENS { + return Err("too many documents are waiting to open".into()); + } + queue.jobs.push_back(job); + ready.notify_one(); + Ok(receiver) + } +} + +impl Drop for OpenWorker { + fn drop(&mut self) { + let (lock, ready) = &*self.queue; + let mut queue = lock.lock().unwrap_or_else(PoisonError::into_inner); + queue.shutdown = true; + queue.jobs.clear(); + queue.repaint = None; + ready.notify_one(); + } +} + +fn run(queue: Arc<(Mutex, Condvar)>) { + loop { + let job = { + let (lock, ready) = &*queue; + let mut state = lock.lock().unwrap_or_else(PoisonError::into_inner); + while state.jobs.is_empty() && !state.shutdown { + state = ready.wait(state).unwrap_or_else(PoisonError::into_inner); + } + if state.shutdown { + return; + } + state.jobs.pop_front() + }; + let Some(job) = job else { continue }; + #[cfg(test)] + if let Some(gate) = &job.gate { + gate.pause_before_prepare(); + } + let token = job.token; + let reply = job.reply.clone(); + #[cfg(test)] + let gate = job.gate.clone(); + let kind = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| prepare(job))) + .unwrap_or_else(|_| ResultKind::Failed("opening failed unexpectedly in the background worker".into())); + let _ = reply.send(ResultMessage { token, kind }); + let repaint = { + let (lock, _) = &*queue; + lock.lock().unwrap_or_else(PoisonError::into_inner).repaint.clone() + }; + if let Some(ctx) = repaint { + ctx.request_repaint(); + } + #[cfg(test)] + if let Some(gate) = &gate { + let _ = gate.finished.send(()); + } + } +} + +fn prepare(job: Job) -> ResultKind { + let bytes = match &job.input { + Input::Path(path) => match std::fs::read(path) { + Ok(bytes) => Arc::new(bytes), + Err(error) => return ResultKind::ReadFailed(error.to_string()), + }, + Input::RemotePath(path) => match pdfcraft_engine::read_remote_document(path) { + Ok(bytes) => Arc::new(bytes), + Err(error) => return ResultKind::RemoteFailed(error.to_string()), + }, + Input::Recovery(store, key) => match store.read(key) { + Ok(bytes) => Arc::new(bytes), + Err(error) => return ResultKind::RecoveryFailed(error.to_string()), + }, + Input::Bytes(bytes) => bytes.clone(), + }; + if matches!(pdfcraft_engine::source_kind(&job.name, &bytes), Some(SourceKind::Image | SourceKind::Text)) { + return ResultKind::Conversion(bytes); + } + let result = job.context.prepare_open(job.name, job.path, bytes.clone(), job.password.as_deref()); + ResultKind::Prepared { bytes, result: Box::new(result) } +} + +#[cfg(test)] +pub(crate) struct PrepareGate { + pub started: Sender<()>, + release: Mutex>, + pub finished: Sender<()>, +} + +#[cfg(test)] +impl PrepareGate { + pub fn pause_before_prepare(&self) { + let _ = self.started.send(()); + if let Ok(release) = self.release.lock() { + let _ = release.recv(); + } + } +} + +#[cfg(test)] +pub(crate) fn prepare_gate() -> (Arc, Receiver<()>, Sender<()>, Receiver<()>) { + let (started, started_rx) = mpsc::channel(); + let (release_tx, release) = mpsc::channel(); + let (finished, finished_rx) = mpsc::channel(); + (Arc::new(PrepareGate { started, release: Mutex::new(release), finished }), started_rx, release_tx, finished_rx) +} diff --git a/crates/ui-egui/src/open_worker_tests.rs b/crates/ui-egui/src/open_worker_tests.rs new file mode 100644 index 0000000000..329e6ece15 --- /dev/null +++ b/crates/ui-egui/src/open_worker_tests.rs @@ -0,0 +1,90 @@ +use std::sync::{Arc, Mutex}; +use std::time::Duration; + +use egui_kittest::Harness; +use serde_json::{Value, json}; + +use crate::{PdfCraftApp, control::ControlClient, open_worker}; + +fn fixture(pages: usize) -> Vec { + let mut objects = vec!["<< /Type /Catalog /Pages 2 0 R >>".to_string()]; + let kids = (0..pages).map(|page| format!("{} 0 R", 4 + 2 * page)).collect::>().join(" "); + objects.push(format!("<< /Type /Pages /Kids [{kids}] /Count {pages} /MediaBox [0 0 200 300] >>")); + objects.push("<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica >>".into()); + for page in 0..pages { + objects.push(format!("<< /Type /Page /Parent 2 0 R /Contents {} 0 R /Resources << /Font << /F1 3 0 R >> >> >>", 5 + 2 * page)); + let body = format!("BT /F1 24 Tf 20 150 Td (Page {}) Tj ET", page + 1); + objects.push(format!("<< /Length {} >>\nstream\n{body}\nendstream", body.len())); + } + let mut bytes = b"%PDF-1.7\n".to_vec(); + let mut offsets = Vec::new(); + for (index, object) in objects.iter().enumerate() { + offsets.push(bytes.len()); + bytes.extend_from_slice(format!("{} 0 obj\n{object}\nendobj\n", index + 1).as_bytes()); + } + let xref = bytes.len(); + bytes.extend_from_slice(format!("xref\n0 {}\n0000000000 65535 f \n", objects.len() + 1).as_bytes()); + for offset in offsets { + bytes.extend_from_slice(format!("{offset:010} 00000 n \n").as_bytes()); + } + bytes.extend_from_slice(format!("trailer\n<< /Size {} /Root 1 0 R >>\nstartxref\n{xref}\n%%EOF\n", objects.len() + 1).as_bytes()); + bytes +} + +#[allow(clippy::expect_used, clippy::panic, clippy::unwrap_used)] +fn harness() -> (Harness<'static, PdfCraftApp>, ControlClient) { + let slot: Arc>> = Arc::default(); + let saved = slot.clone(); + let mut harness = Harness::builder().with_size(egui::vec2(1000.0, 700.0)).build_eframe(move |creation| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").expect("language preference"); + *saved.lock().expect("control-client slot") = Some(app.attach_control(&creation.egui_ctx)); + app.open_bytes("existing.pdf", None, fixture(3)).expect("existing document"); + app + }); + harness.run_steps(3); + (harness, slot.lock().expect("control-client slot").take().expect("control client")) +} + +#[allow(clippy::expect_used, clippy::panic, clippy::unwrap_used)] +fn request(harness: &mut Harness<'static, PdfCraftApp>, client: &ControlClient, method: &str, params: Value) -> Value { + let reply = client.send(method, params); + for _ in 0..5 { + harness.step(); + if let Ok(result) = reply.try_recv() { + return result.expect("control request succeeded"); + } + } + panic!("{method} was not answered during the held prepare phase"); +} + +#[test] +#[allow(clippy::expect_used, clippy::panic, clippy::unwrap_used)] +fn open_prepare_keeps_control_and_existing_tabs_live_until_atomic_commit() { + let (mut harness, client) = harness(); + let (gate, started, release, finished) = open_worker::prepare_gate(); + harness.state_mut().open_prepare_gate = Some(gate); + + let path = std::env::temp_dir().join(format!("pdfcraft-open-worker-{}.pdf", std::process::id())); + std::fs::write(&path, fixture(2)).expect("write test PDF"); + let open_reply = request(&mut harness, &client, "ui.open", json!({ "path": path.to_string_lossy() })); + assert_eq!(open_reply["pending"], true); + started.recv_timeout(Duration::from_secs(5)).expect("worker reached prepare barrier"); + + let state = request(&mut harness, &client, "ui.state", json!({})); + assert_eq!(state["documents"].as_array().map(Vec::len), Some(1)); + assert_eq!(state["documents"][0]["name"], "existing.pdf"); + assert_eq!(state["pending_opens"].as_array().map(Vec::len), Some(1)); + request(&mut harness, &client, "ui.command", json!({ "id": "page.insert_blank" })); + assert_eq!(harness.state().session.get(harness.state().views[0].id).expect("existing tab").info.pages.len(), 4); + assert_eq!(harness.state().views.len(), 1, "prepared state is not visible in the session"); + + release.send(()).expect("release prepare barrier"); + finished.recv_timeout(Duration::from_secs(5)).expect("worker finished preparation"); + harness.step(); + let app = harness.state(); + assert_eq!(app.views.len(), 2, "one prepared document commits exactly once"); + assert_eq!(app.session.get(app.views[1].id).expect("new tab").info.pages.len(), 2); + assert_eq!(app.active, Some(1)); + let _ = std::fs::remove_file(path); +} diff --git a/crates/ui-egui/src/recovery.rs b/crates/ui-egui/src/recovery.rs index 7a8b8d464e..134f54ecd7 100644 --- a/crates/ui-egui/src/recovery.rs +++ b/crates/ui-egui/src/recovery.rs @@ -199,6 +199,25 @@ impl PdfCraftApp { let Some(store) = self.recovery.clone() else { return }; for key in keys { let Some(meta) = self.recoverable.iter().find(|m| &m.key == key).cloned() else { continue }; + #[cfg(not(target_arch = "wasm32"))] + if self.ctx.is_some() && !self.run_inline { + self.startup_superseded = true; + let result = self.schedule_open(crate::OpenRequest { + name: meta.name.clone(), + path: None, + stamp: None, + input: crate::open_worker::Input::Recovery(store.clone(), key.clone()), + password: None, + activate: true, + after_open: None, + recovered: Some(meta.clone()), + then: None, + }); + if let Err(error) = result { + self.notify_fmt("Couldn't recover {name}: {e}", &[("name", &meta.name), ("e", &error)]); + } + continue; + } let bytes = match store.read(key) { Ok(b) => b, Err(e) => {