collective
Browse / github.com/storytold/designcraft / patch
Recovery: the timed save is written on its own thread, without recompressing images
Crash recovery freezes the UI every interval with large documents (recompresses images on the UI thread) **What happens:** With a large unsaved document, the UI freezes for a moment roughly every 30 seconds while the CPU is mostly idle. Clicks and edits made during the freeze are lost or arrive late. **Why:** The crash-recovery timer in `DesignApp::logic` runs `file.recovery.save` on the UI thread. It packs every unsaved document into a `.designcraft` zip (`designcraft_format::save`), writes it and `fsync`s it, and the UI waits. Assets that aren't PNG, JPEG, GIF or WebP (a placed TIFF or PSD, for example) are deflated again on every checkpoint, single-threaded. A 125 MB TIFF takes seconds each t
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- PiMinusMax (imported from pull request #1750, open)
- Base
ed8aabd1dc5a88652a076631c2d46886ca60c031- Size
- +275 −28 in 5 files
- Review
- pending
- Shared
- 2026-10-28
- Id
7c99fe3eaf659d08fc7e2906
How it went
No other operator has reported using it yet.
Diff
raw · ROADMAP.md apps/designcraft/src/main.rs crates/engine/src/recovery.rs crates/format/src/lib.rs crates/ui-egui/src/lib.rs
@@ -380,6 +380,7 @@ list) · [roadmap.md](docs/roadmap.md) (milestones, plan) · [parity-checklist.m Newest first. +- **2026-10-24** · Crash recovery no longer stalls the UI: the timed save packs and writes the unsaved documents on its own thread (the frame that starts it takes a snapshot of each and doesn't wait), and records the checkpoints when it is done; a document saved or closed meanwhile leaves none behind. Recovery checkpoints store their images as they are instead of compressing a large TIFF or PSD again every time (it took seconds). Quitting waits for a save in progress. No readiness percentage change. - **2026-10-20** · Edit › Keyboard Shortcuts is craft-keymap's shared dialog: commands grouped by menu with Show and Filter, recording that asks before taking a key from another command, undo and redo, named shortcut sets (edits of Default become Custom; an imported set becomes a set of its own), an on-screen US keyboard, InDesign Show Set files reviewed before they apply, JSON export and an HTML summary. ui.json keeps the `shortcuts` map earlier versions read, for the active set. No readiness percentage change. - **2026-10-19** · Import InDesign Set… runs on craft-keymap's importer: a set's Window menu panel items (Layers F7, Pages F12, Swatches F5) and Tools items set the panel and tool shortcuts, all of an item's Default keys apply, and the report adds numeric keypad keys read as main keys and likely commands for unmatched items (`window.importShortcuts`). On InDesign's Windows defaults it matches 366 of 1,629 items (was 233) and changes 23 commands. No readiness percentage change. - **2026-10-19** · Keyboard shortcuts run on the shared craft-keymap crate. Panels take shortcuts, with InDesign's defaults where the key is free (Swatches F5, Color F6, Layers F7, Info F8, Stroke F10, Paragraph Styles F11, Character Styles Shift+F11, Pages F12, Align Shift+F7); tool keys are editable in Edit › Keyboard Shortcuts and `window.setShortcut`, and tools sharing a key take turns (B). Saved shortcuts load unchanged. No readiness percentage change.@@ -122,6 +122,8 @@ fn discard_marked_text() { /// Orderly OS termination can bypass egui's cancelable close event. Finish only a complete, /// current quit plan; otherwise preserve every live document, including edits after the timer. fn preserve_documents_on_exit(app: &mut DesignApp) -> Result<(), String> {+ // A timed recovery save still being written finishes first.+ app.finish_recovery(); app.finish_confirmed_quit()?; if app.session.documents().is_empty() { return Ok(());@@ -23,9 +23,12 @@ #[cfg(not(target_arch = "wasm32"))] use std::io::Write; use std::path::{Path, PathBuf};+use std::sync::Arc; use serde_json::{Value, json}; +use designcraft_doc::Document;+ use crate::{DocState, EngineError, Result, Session}; /// The platform's per-user recovery folder.@@ -537,6 +540,39 @@ pub fn save(s: &Session, dir: &Path) -> Result<usize> { /// Command-path checkpointing also records explicitly selected directories, without configuring /// automatic recovery. The immutable public helper remains available for fixtures and callers. pub(crate) fn save_tracked(s: &mut Session, dir: &Path) -> Result<usize> {+ let pending = begin_tracked(s, dir)?;+ let written = pending.write();+ finish_tracked(s, written)+}++/// A recovery save whose slow part, packing and writing the documents, can run on another thread+/// ([`PendingSave::write`]) while the user goes on working: a document with large images took+/// seconds, and the UI waited. [`begin_tracked`] makes it on the session's thread,+/// [`finish_tracked`] records what was written there.+pub struct PendingSave {+ dir: PathBuf,+ access: Access,+ entries: Vec<PendingEntry>,+}++struct PendingEntry {+ uid: u64,+ /// The document as it was when the save began (a cheap shared copy).+ doc: Arc<Document>,+ metadata: Value,+ replace: bool,+}++/// What a [`PendingSave`] wrote.+pub struct WrittenSave {+ dir: PathBuf,+ written: Vec<u64>,+ error: Option<EngineError>,+}++/// Begin a recovery save of the unsaved documents into `dir`: the checks, and the clean+/// documents' checkpoints dropped, here; the writing in [`PendingSave::write`].+pub fn begin_tracked(s: &mut Session, dir: &Path) -> Result<PendingSave> { // Reject the complete batch before writing rather than silently losing old ownership records. for document in &s.docs { if document.is_dirty() && !has_checkpoint(document, dir) && document.recovery_entries.len() >= MAX_RECOVERY_ENTRIES {@@ -546,8 +582,8 @@ pub(crate) fn save_tracked(s: &mut Session, dir: &Path) -> Result<usize> { } } let access = prepare_save(s, dir)?;- let mut n = 0; let preview = s.new_document_preview();+ let mut entries = Vec::new(); for index in 0..s.docs.len() { // New Document's preview is never written, so it has no checkpoint to discard either. if s.docs.get(index).is_some_and(|document| preview == Some(document.uid)) {@@ -557,29 +593,76 @@ pub(crate) fn save_tracked(s: &mut Session, dir: &Path) -> Result<usize> { discard_document(s, index, Some(dir)); continue; }- let Some(document) = s.docs.get_mut(index) else { continue };- let (doc, meta) = files(dir, document.uid);- let bytes = designcraft_format::save(document.preview_stash.as_deref().unwrap_or(document.doc.as_ref()))- .map_err(|e| EngineError::Other(e.to_string()))?;- // An entry this run did not write is never replaced, even one made since it checked.- let replace = access.owns(document.uid);- publish_recovery_file(&doc, &bytes, replace)?;- access.record(document.uid);- // The primary is useful even if optional metadata fails next. Track it immediately.- if !has_checkpoint(document, dir) {- document.recovery_entries.push((dir.to_path_buf(), document.uid));+ let Some(document) = s.docs.get(index) else { continue };+ entries.push(PendingEntry {+ uid: document.uid,+ doc: document.preview_stash.clone().unwrap_or_else(|| document.doc.clone()),+ metadata: json!({"path": document.path, "title": document.title(), "saved": designcraft_doc::vars::now()}),+ // An entry this run did not write is never replaced, even one made since it checked.+ replace: access.owns(document.uid),+ });+ }+ Ok(PendingSave { dir: dir.to_path_buf(), access, entries })+}++impl PendingSave {+ /// Whether there is nothing to write.+ pub fn is_empty(&self) -> bool {+ self.entries.is_empty()+ }++ /// Pack and write every document (any thread). Assets are stored, not compressed: a+ /// checkpoint is written every few minutes and only read after a crash.+ pub fn write(self) -> WrittenSave {+ let mut written = Vec::new();+ let mut error = None;+ for e in &self.entries {+ let (doc, meta) = files(&self.dir, e.uid);+ let result = designcraft_format::save_uncompressed_assets(&e.doc)+ .map_err(|err| EngineError::Other(err.to_string()))+ .and_then(|bytes| publish_recovery_file(&doc, &bytes, e.replace));+ if let Err(err) = result {+ error = Some(err);+ break;+ }+ self.access.record(e.uid);+ // The primary is useful even if optional metadata fails next. Track it immediately.+ written.push(e.uid);+ if let Err(err) = publish_recovery_file(&meta, e.metadata.to_string().as_bytes(), e.replace) {+ error = Some(err);+ break;+ } }- let metadata = json!({"path": document.path, "title": document.title(), "saved": designcraft_doc::vars::now()});- publish_recovery_file(&meta, metadata.to_string().as_bytes(), replace)?;- n += 1;+ WrittenSave { dir: self.dir, written, error }+ }+}++/// Record what a [`PendingSave`] wrote: each document's checkpoint, or, for a document saved or+/// closed while it was being written, the checkpoint removed again. Returns how many were+/// written, or the error that stopped the writing.+pub fn finish_tracked(s: &mut Session, w: WrittenSave) -> Result<usize> {+ for uid in &w.written {+ match s.docs.iter_mut().find(|d| d.uid == *uid) {+ Some(document) if document.is_dirty() => {+ if !has_checkpoint(document, &w.dir) {+ document.recovery_entries.push((w.dir.clone(), *uid));+ }+ }+ // Saved or closed meanwhile: a checkpoint would bring back a document that is safe.+ _ => discard(&w.dir, *uid),+ }+ }+ match w.error {+ Some(e) => Err(e),+ None => Ok(w.written.len()), }- Ok(n) } /// Publish a document's recovery entry before its optional metadata. fn write_entry(access: &Access, dir: &Path, d: &DocState) -> Result<()> { let (doc, meta) = files(dir, d.uid);- let bytes = designcraft_format::save(d.preview_stash.as_deref().unwrap_or(d.doc.as_ref())).map_err(|e| EngineError::Other(e.to_string()))?;+ let bytes = designcraft_format::save_uncompressed_assets(d.preview_stash.as_deref().unwrap_or(d.doc.as_ref()))+ .map_err(|e| EngineError::Other(e.to_string()))?; let replace = access.owns(d.uid); publish_recovery_file(&doc, &bytes, replace)?; access.record(d.uid);@@ -920,6 +1003,32 @@ mod tests { assert_eq!(list(dir.path()).len(), 1); } + /// The background save writes what the documents held when it began, and a document saved+ /// or closed while it was being written leaves no checkpoint behind.+ #[test]+ fn a_save_begun_here_and_written_elsewhere_records_its_checkpoints() {+ let dir = tempfile::tempdir().unwrap();+ let mut s = unsaved("written on another thread");+ let pending = begin_tracked(&mut s, dir.path()).unwrap();+ assert!(!pending.is_empty());+ // Edits after the begin don't reach this checkpoint (the next one has them).+ s.execute("frame.create", &json!({"rect": [0, 0, 10, 10]})).unwrap();+ let written = std::thread::spawn(move || pending.write()).join().unwrap();+ assert_eq!(finish_tracked(&mut s, written).unwrap(), 1);+ assert_eq!(list(dir.path()).len(), 1);+ let mut reopened = Session::new();+ assert_eq!(open(&mut reopened, dir.path()).unwrap().len(), 1);+ assert_eq!(texts(&reopened), ["written on another thread"]);+ // Closed while a save was being written: its checkpoint goes again.+ let dir = tempfile::tempdir().unwrap();+ let mut s = unsaved("closed meanwhile");+ let pending = begin_tracked(&mut s, dir.path()).unwrap();+ let written = pending.write();+ s.close_document(0);+ assert_eq!(finish_tracked(&mut s, written).unwrap(), 1);+ assert!(list(dir.path()).is_empty(), "no checkpoint of a closed document");+ }+ #[test] fn opening_recovery_again_does_not_duplicate_live_documents() { let dir = tempfile::tempdir().unwrap();@@ -63,7 +63,14 @@ pub fn save(doc: &Document) -> Result<Vec<u8>, FormatError> { /// [`save`], plus `preview` (PNG bytes) as `preview.png`. pub fn save_with_preview(doc: &Document, preview: Option<&[u8]>) -> Result<Vec<u8>, FormatError> {- save_with_large_entries_from(doc, preview, LARGE_ENTRY)+ save_with_large_entries_from(doc, preview, LARGE_ENTRY, false)+}++/// [`save`] with every asset stored as it is, none compressed: for crash-recovery checkpoints,+/// written every few minutes while the user works, where a large TIFF or PSD took seconds to+/// deflate. Any reader opens it like any other package.+pub fn save_uncompressed_assets(doc: &Document) -> Result<Vec<u8>, FormatError> {+ save_with_large_entries_from(doc, None, LARGE_ENTRY, true) } /// Asset entries this size or larger are written with zip64 sizes. Zip rejects an entry over@@ -85,7 +92,7 @@ fn required_version(doc: &Document) -> u32 { if doc.assets.values().any(|a| designcraft_images::is_eps(&a.data)) { 3 } else { 2 } } -fn save_with_large_entries_from(doc: &Document, preview: Option<&[u8]>, large_entry: usize) -> Result<Vec<u8>, FormatError> {+fn save_with_large_entries_from(doc: &Document, preview: Option<&[u8]>, large_entry: usize, store_assets: bool) -> Result<Vec<u8>, FormatError> { // JSON writes infinity and NaN as null, which no load accepts: the file would not open. if let Some(id) = doc.non_finite_item() { return Err(FormatError::Unsaveable(format!("object {} has a position or size that is not a finite number", id.0)));@@ -117,7 +124,8 @@ fn save_with_large_entries_from(doc: &Document, preview: Option<&[u8]>, large_en continue; } // Already-compressed images are stored as-is.- let opts = if matches!(a.mime.as_str(), "image/png" | "image/jpeg" | "image/gif" | "image/webp") { stored } else { deflate };+ let opts =+ if store_assets || matches!(a.mime.as_str(), "image/png" | "image/jpeg" | "image/gif" | "image/webp") { stored } else { deflate }; let opts = opts.large_file(a.data.len() >= large_entry); z.start_file(format!("assets/{}.{}", id.0, ext_for(&a.mime)), opts).map_err(io)?; z.write_all(&a.data).map_err(|e| FormatError::Io(e.to_string()))?;@@ -736,12 +744,31 @@ mod tests { let aid = png_asset(&mut d, &[5; 64]); place(&mut d, aid, 0.0); let normal = save(&d).unwrap();- assert_eq!(normal, save_with_large_entries_from(&d, None, 65).unwrap(), "an asset under the threshold is unchanged");- let large = save_with_large_entries_from(&d, None, 64).unwrap();+ assert_eq!(normal, save_with_large_entries_from(&d, None, 65, false).unwrap(), "an asset under the threshold is unchanged");+ let large = save_with_large_entries_from(&d, None, 64, false).unwrap(); assert!(large.len() > normal.len(), "the asset entry carries zip64 sizes"); assert_eq!(*load(&large).unwrap().assets[&aid].data, vec![5; 64]); } + /// Recovery checkpoints store every asset as it is (a TIFF is deflated in a normal save) and+ /// open like any other package.+ #[test]+ fn uncompressed_asset_saves_store_every_asset_and_open_again() {+ let mut d = Document::new(&NewDocument::default());+ let aid = png_asset(&mut d, &[7; 4096]);+ Arc::make_mut(d.assets.get_mut(&aid).unwrap()).mime = "image/tiff".into();+ place(&mut d, aid, 0.0);+ let method = |bytes: &[u8]| {+ let mut z = zip::ZipArchive::new(Cursor::new(bytes.to_vec())).unwrap();+ let name = (0..z.len()).map(|i| z.by_index(i).unwrap().name().to_string()).find(|n| n.starts_with("assets/")).unwrap();+ z.by_name(&name).unwrap().compression()+ };+ assert_eq!(method(&save(&d).unwrap()), zip::CompressionMethod::Deflated);+ let fast = save_uncompressed_assets(&d).unwrap();+ assert_eq!(method(&fast), zip::CompressionMethod::Stored);+ assert_eq!(*load(&fast).unwrap().assets[&aid].data, vec![7; 4096]);+ }+ #[test] fn saves_hold_each_used_file_once() { let mut d = Document::new(&NewDocument::default());@@ -815,6 +815,9 @@ pub struct DesignApp { last_time: f64, /// When recovery data was last written (seconds on the monotonic clock; see [`now_ms`]). pub last_recovery: f64,+ /// The recovery save being written on its own thread (see `ui_frame_end`).+ #[cfg(not(target_arch = "wasm32"))]+ recovery_job: Option<std::thread::JoinHandle<designcraft_engine::recovery::WrittenSave>>, /// The egui context (set by the first UI pass): copied text goes to the system clipboard through it. egui_ctx: Option<egui::Context>, /// A control-channel request is being handled: commands it runs never read or write the@@ -942,6 +945,8 @@ impl DesignApp { shortcut_answers: Default::default(), last_time: now, last_recovery: now,+ #[cfg(not(target_arch = "wasm32"))]+ recovery_job: None, egui_ctx: None, in_control: false, object_clip_text: None,@@ -1066,6 +1071,12 @@ impl DesignApp { /// THE entry point for every action (menus, shortcuts, palette, panels, control channel). pub fn run(&mut self, id: &str, params: Value) -> Result<Value, String> {+ // Recovery data saved or opened by command (control channel, a script) while the timed+ // save is being written: that one is recorded first, so an older snapshot can't land last.+ #[cfg(not(target_arch = "wasm32"))]+ if id.starts_with("file.recovery.") {+ self.finish_recovery();+ } if let Some(r) = menus::run_ui(self, id, ¶ms) { return r; }@@ -1527,14 +1538,41 @@ impl DesignApp { /// uses the monotonic clock, so a wall-clock/NTP jump cannot postpone or advance it. fn ui_frame_end(&mut self, ctx: &egui::Context) { let now = now_ms() / 1000.0;- // Crash recovery: unsaved documents are written to the recovery folder every 30 s.+ // Crash recovery: unsaved documents are written to the recovery folder on a timer. The+ // packing and writing run on their own thread: with large images they took seconds, and+ // the UI stood still (clicks were lost) every time. #[cfg(not(target_arch = "wasm32"))]- if self.session.recovery_dir.is_some() && now - self.last_recovery > (self.session.prefs.recovery_minutes * 60.0).max(5.0) {- self.last_recovery = now;- if self.session.documents().iter().any(|d| d.is_dirty())- && let Err(e) = self.session.execute("file.recovery.save", &json!({}))+ {+ if self.recovery_job.as_ref().is_some_and(std::thread::JoinHandle::is_finished) {+ self.finish_recovery();+ }+ if self.recovery_job.is_none()+ && let Some(dir) = self.session.recovery_dir.clone()+ && now - self.last_recovery > (self.session.prefs.recovery_minutes * 60.0).max(5.0) {- self.status(crate::i18n::tr(&self.ui.language, "Couldn't write recovery data: {error}").replace("{error}", &e.to_string()));+ self.last_recovery = now;+ if self.session.documents().iter().any(|d| d.is_dirty()) {+ let started =+ designcraft_engine::recovery::begin_tracked(&mut self.session, &dir).map_err(|e| e.to_string()).and_then(|pending| {+ if pending.is_empty() {+ return Ok(None);+ }+ std::thread::Builder::new()+ .name("designcraft-recovery".into())+ .spawn(move || pending.write())+ .map(Some)+ .map_err(|e| e.to_string())+ });+ match started {+ Ok(job) => self.recovery_job = job,+ Err(e) => self.recovery_failed(&e),+ }+ }+ }+ // An idle app draws no frame on its own: ask for one, so a finished save is recorded+ // (and a document saved meanwhile loses its checkpoint) without waiting for input.+ if self.recovery_job.is_some() {+ ctx.request_repaint_after(std::time::Duration::from_millis(250)); } } let dt = now - self.last_time;@@ -1646,6 +1684,10 @@ impl DesignApp { /// terminate before egui receives its queued Close; an unfinished/stale plan leaves every /// document live so the host can preserve its recovery data instead. pub fn finish_confirmed_quit(&mut self) -> Result<(), String> {+ // A recovery save still being written finishes first: closing a document discards its+ // checkpoint, which the writer must not publish again afterwards.+ #[cfg(not(target_arch = "wasm32"))]+ self.finish_recovery(); let approved = self.quitting.as_ref().is_some_and(|q| q.pending.is_none()) && self.ui.dialog.is_none() && self.next_unapproved_document().is_none(); if !approved {@@ -2086,6 +2128,25 @@ impl DesignApp { } } + /// Wait for the recovery save being written, if any, and record it (a quit preserves the+ /// documents only after it, so the two never write the same files at once).+ #[cfg(not(target_arch = "wasm32"))]+ pub fn finish_recovery(&mut self) {+ let Some(job) = self.recovery_job.take() else { return };+ let result = match job.join() {+ Ok(written) => designcraft_engine::recovery::finish_tracked(&mut self.session, written).map_err(|e| e.to_string()),+ Err(_) => Err("the recovery writer stopped".to_string()),+ };+ if let Err(e) = result {+ self.recovery_failed(&e);+ }+ }++ #[cfg(not(target_arch = "wasm32"))]+ fn recovery_failed(&mut self, error: &str) {+ self.status(crate::i18n::tr(&self.ui.language, "Couldn't write recovery data: {error}").replace("{error}", error));+ }+ /// Lay out the whole window. pub fn ui(&mut self, ui: &mut egui::Ui) { #[cfg(test)]@@ -2750,6 +2811,53 @@ mod tests { app } + /// The timed recovery save is written on its own thread: the frame that starts it doesn't+ /// wait for it, and a later frame records it.+ #[cfg(not(target_arch = "wasm32"))]+ #[test]+ fn the_timed_recovery_save_is_written_on_its_own_thread() {+ let ctx = egui::Context::default();+ let mut app = app_with_unsaved_document();+ let directory = tempfile::tempdir().unwrap();+ app.session.recovery_dir = Some(directory.path().to_path_buf());+ app.last_recovery = -1e9;+ let mut out = ctx.run_ui(egui::RawInput::default(), |ui| app.ui_frame_end(ui.ctx()));+ out.textures_delta.clear();+ assert!(app.recovery_job.is_some(), "the save runs on a thread");+ // An idle app asks for a frame while the save runs, so it is recorded without input.+ let repaint = out.viewport_output.get(&egui::ViewportId::ROOT).map(|v| v.repaint_delay);+ assert!(repaint.is_some_and(|d| d <= std::time::Duration::from_millis(250)), "{repaint:?}");+ app.finish_recovery();+ assert!(app.recovery_job.is_none());+ assert_eq!(designcraft_engine::recovery::list(directory.path()).len(), 1, "the checkpoint is written and recorded");+ // Recorded as this document's: closing it removes the checkpoint again.+ app.session.close_document(0);+ assert!(designcraft_engine::recovery::list(directory.path()).is_empty());+ }++ /// A recovery save by command while the timed one is being written waits for it, so the+ /// command's newer snapshot is the one left on disk.+ #[cfg(not(target_arch = "wasm32"))]+ #[test]+ fn a_recovery_save_by_command_waits_for_the_timed_one() {+ let ctx = egui::Context::default();+ let mut app = app_with_unsaved_document();+ let directory = tempfile::tempdir().unwrap();+ app.session.recovery_dir = Some(directory.path().to_path_buf());+ app.last_recovery = -1e9;+ let mut out = ctx.run_ui(egui::RawInput::default(), |ui| app.ui_frame_end(ui.ctx()));+ out.textures_delta.clear();+ assert!(app.recovery_job.is_some());+ app.run("frame.create", json!({"rect": [10, 10, 50, 50]})).unwrap();+ app.run("file.recovery.save", json!({})).unwrap();+ assert!(app.recovery_job.is_none(), "the timed save was recorded first");+ let saved = designcraft_engine::recovery::list(directory.path());+ assert_eq!(saved.len(), 1);+ let bytes = std::fs::read(directory.path().join(format!("{}.designcraft", saved[0].0))).unwrap();+ let doc = designcraft_engine::cmd::from_bytes(&bytes).unwrap();+ assert_eq!(doc.spreads[0].items.len(), app.session.active().unwrap().doc.spreads[0].items.len(), "the newer snapshot is on disk");+ }+ /// File ▸ Close (and its shortcut) closed a document with unsaved changes without asking, and /// closing discards the document's recovery data too. #[test]