Open drawings on a worker thread with an Opening... indicator (#694) (#797)

Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>

crates/engine/src/cmd/file.rs

@@ -228,18 +228,65 @@ fn file_name(path: &str) -> String {
     std::path::Path::new(path).file_name().map(|n| n.to_string_lossy().to_string()).unwrap_or_else(|| path.to_string())
 }
 
-pub(crate) fn open_bytes(s: &mut Session, bytes: &[u8], name: &str, path: Option<String>) -> Result<usize> {
+/// A file read and decoded but not yet added to a [`Session`]. Reading is the slow part of OPEN and
+/// needs no session, so a host can do it on a worker thread and hand the result to
+/// [`finish_open`] on the thread that owns the session.
+pub struct ReadFile {
+    drawing: Drawing,
+    binary_dxf: bool,
+    name: String,
+    path: Option<String>,
+}
+
+/// Read and decode `path` (the slow half of `OPEN {path}`). Needs no session; runs anywhere.
+pub fn read_file(path: &str) -> std::result::Result<ReadFile, String> {
+    #[cfg(not(target_arch = "wasm32"))]
+    {
+        let r = std::panic::catch_unwind(|| {
+            let bytes = std::fs::read(path).map_err(|e| bad("open", format!("{path}: {e}")))?;
+            read_bytes(&bytes, path, Some(path.to_string()))
+        });
+        match r {
+            Ok(r) => r.map_err(|e| e.to_string()),
+            Err(_) => Err(bad("open", format!("{path}: the file could not be read")).to_string()),
+        }
+    }
+    #[cfg(target_arch = "wasm32")]
+    {
+        let _ = path;
+        Err(bad("open", "paths are not available on the web; pass `data`").to_string())
+    }
+}
+
+/// Add a [`read_file`] result to the session as the active drawing: the fast half of OPEN.
+pub fn finish_open(s: &mut Session, f: ReadFile) -> Result<Value> {
+    let ReadFile { drawing, binary_dxf, name, path } = f;
+    let i = add_read(s, drawing, binary_dxf, &name, path);
+    super::blocks::xref::after_open(s, i);
+    Ok(json!({ "index": i, "entities": s.docs.get(i).map(|d| d.doc.entity_count()) }))
+}
+
+fn read_bytes(bytes: &[u8], name: &str, path: Option<String>) -> Result<ReadFile> {
     let hooks = io().ok_or_else(|| bad("open", "file formats are not available in this build"))?;
-    let d = (hooks.read)(bytes, name).map_err(|e| bad("open", e))?;
+    let drawing = (hooks.read)(bytes, name).map_err(|e| bad("open", e))?;
     // A file that isn't a drawing (an imported SVG) gives the drawing no file name: QSAVE then
     // asks where to save instead of exporting over the original.
     let path = path.filter(|p| is_drawing_file(p));
-    let i = s.open_drawing(d, &file_name(name), path);
+    Ok(ReadFile { drawing, binary_dxf: cadcraft_dxf::is_binary(bytes), name: name.to_string(), path })
+}
+
+fn add_read(s: &mut Session, drawing: Drawing, binary_dxf: bool, name: &str, path: Option<String>) -> usize {
+    let i = s.open_drawing(drawing, &file_name(name), path);
     if let Some(st) = s.docs.get_mut(i) {
         // QSAVE keeps a binary DXF binary.
-        st.binary_dxf = cadcraft_dxf::is_binary(bytes);
+        st.binary_dxf = binary_dxf;
     }
-    Ok(i)
+    i
+}
+
+pub(crate) fn open_bytes(s: &mut Session, bytes: &[u8], name: &str, path: Option<String>) -> Result<usize> {
+    let ReadFile { drawing, binary_dxf, name, path } = read_bytes(bytes, name, path)?;
+    Ok(add_read(s, drawing, binary_dxf, &name, path))
 }
 
 fn run_open(s: &mut Session, p: &Value) -> Result<Value> {

crates/engine/src/lib.rs

@@ -693,6 +693,20 @@ impl Session {
         }
     }
 
+    /// Add a file read off-thread with [`cmd::file::read_file`] as the active drawing (OPEN's fast
+    /// half). A panic while adding it is reported as an error, never propagated.
+    pub fn finish_open(&mut self, f: cmd::file::ReadFile) -> Result<Value> {
+        let n = self.docs.len();
+        match std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| cmd::file::finish_open(self, f))) {
+            Ok(r) => r,
+            Err(_) => {
+                self.docs.truncate(n);
+                self.active = self.active.min(self.docs.len().saturating_sub(1));
+                Err(EngineError::Internal("open".into(), "panic".into()))
+            }
+        }
+    }
+
     pub fn state(&self) -> Result<&DocState> {
         self.docs.get(self.active).ok_or(EngineError::NoDocument)
     }

crates/ui-egui/src/lib.rs

@@ -32,6 +32,7 @@ pub mod icons;
 pub mod layers;
 pub mod managers;
 pub mod menus;
+pub mod opening;
 pub mod options;
 pub mod palettes;
 pub mod parametric;
@@ -187,6 +188,8 @@ pub struct CadApp {
     pub spacemouse: spacemouse::State,
     pub cmd: cmdline::CmdLine,
     pub status: Option<(String, f64)>,
+    /// Drawings being read on worker threads ([`opening`]).
+    pub opening: opening::Opening,
     pub integrated_titlebar: bool,
     control_rx: Option<Receiver<ControlRequest>>,
     pending_shots: Vec<(u64, Option<String>, std::sync::mpsc::Sender<Value>, f64)>,
@@ -233,6 +236,7 @@ impl CadApp {
             spacemouse: spacemouse::State::default(),
             cmd: cmdline::CmdLine::default(),
             status: None,
+            opening: opening::Opening::default(),
             integrated_titlebar: false,
             control_rx: None,
             pending_shots: Vec::new(),
@@ -364,9 +368,34 @@ impl CadApp {
         self.status = Some((s.into(), now_ms()));
     }
 
+    /// Open the drawing at `path`. Natively the file is read on a worker thread while an
+    /// "Opening ..." card shows ([`opening`]); [`Self::finish_opening`] adds it when ready. On the
+    /// web it opens at once.
     pub fn open_path(&mut self, path: &str) {
-        let r = self.run("open", json!({ "path": path }));
-        self.opened(r);
+        #[cfg(not(target_arch = "wasm32"))]
+        {
+            self.opening.start(path);
+        }
+        #[cfg(target_arch = "wasm32")]
+        {
+            let r = self.run("open", json!({ "path": path }));
+            self.opened(r);
+        }
+    }
+
+    /// Add the files whose background read has finished; report the ones that failed as a failed
+    /// OPEN is reported. Called every frame.
+    pub fn finish_opening(&mut self) {
+        for (_, outcome) in self.opening.poll() {
+            let r = match outcome {
+                Ok(f) => self.session.finish_open(f).map_err(|e| e.to_string()),
+                Err(e) => Err(e),
+            };
+            if let Err(e) = &r {
+                self.session.echo(e.clone());
+            }
+            self.opened(r);
+        }
     }
 
     /// The largest file [`Self::open_bytes`] opens. A drawing opened from its contents (the
@@ -437,6 +466,8 @@ impl CadApp {
             self.shown_follows_system = follows_system;
         }
         self.drain_control(ctx);
+        self.finish_opening();
+        self.opening.show(ctx);
         if !self.synthetic.is_empty() {
             ctx.request_repaint();
         }

crates/ui-egui/src/opening.rs

@@ -0,0 +1,141 @@
+//! Opening drawings without freezing the window (#694). A file is read and decoded on a worker
+//! thread (the slow part of OPEN, which needs no session); the UI shows an "Opening ..." card with
+//! a spinner and the elapsed time and keeps painting. The finished drawing is added to the session
+//! on the UI thread. The web build has no threads and opens synchronously.
+
+use std::sync::mpsc::{Receiver, TryRecvError, channel};
+use std::time::{Duration, Instant};
+
+use cadcraft_engine::cmd::file::{ReadFile, read_file};
+
+use crate::theme::Tokens;
+
+type Outcome = Result<ReadFile, String>;
+
+struct Job {
+    path: String,
+    started: Instant,
+    rx: Receiver<Outcome>,
+}
+
+/// The files being read in the background, oldest first.
+#[derive(Default)]
+pub struct Opening {
+    jobs: Vec<Job>,
+}
+
+impl Opening {
+    /// Start reading `path` on a worker thread.
+    pub fn start(&mut self, path: &str) {
+        let (tx, rx) = channel();
+        let p = path.to_string();
+        let spawned = std::thread::Builder::new().name("cadcraft-open".into()).spawn(move || {
+            let _ = tx.send(read_file(&p));
+        });
+        if spawned.is_err() {
+            // No thread to read on: report it like any other failed open.
+            let (tx, rx2) = channel();
+            let _ = tx.send(Err(format!("open: {path}: could not start a reader thread")));
+            self.jobs.push(Job { path: path.to_string(), started: Instant::now(), rx: rx2 });
+            return;
+        }
+        self.jobs.push(Job { path: path.to_string(), started: Instant::now(), rx });
+    }
+
+    /// Whether any file is still being read.
+    pub fn busy(&self) -> bool {
+        !self.jobs.is_empty()
+    }
+
+    /// The finished reads, as (path, result). A reader that died without answering is an error.
+    pub fn poll(&mut self) -> Vec<(String, Outcome)> {
+        let mut done = Vec::new();
+        self.jobs.retain(|j| match j.rx.try_recv() {
+            Ok(r) => {
+                done.push((j.path.clone(), r));
+                false
+            }
+            Err(TryRecvError::Empty) => true,
+            Err(TryRecvError::Disconnected) => {
+                done.push((j.path.clone(), Err(format!("open: {}: the file could not be read", j.path))));
+                false
+            }
+        });
+        done
+    }
+
+    /// The text of the card: the file and how long it has taken, or how many files are queued.
+    fn label(&self) -> Option<String> {
+        let first = self.jobs.first()?;
+        let name = std::path::Path::new(&first.path).file_name().map_or_else(|| first.path.clone(), |n| n.to_string_lossy().to_string());
+        let secs = first.started.elapsed().as_secs_f32();
+        let more = self.jobs.len() - 1;
+        Some(if more == 0 { format!("Opening {name}... {secs:.0} s") } else { format!("Opening {name} and {more} more... {secs:.0} s") })
+    }
+
+    /// Paint the card in the middle of the window and keep frames coming while files load.
+    pub fn show(&self, ctx: &egui::Context) {
+        let Some(label) = self.label() else { return };
+        let t = Tokens::get();
+        egui::Area::new(egui::Id::new("cc_opening"))
+            .order(egui::Order::Foreground)
+            .anchor(egui::Align2::CENTER_CENTER, egui::Vec2::ZERO)
+            .interactable(false)
+            .show(ctx, |ui| {
+                egui::Frame::NONE
+                    .fill(t.panel)
+                    .stroke(egui::Stroke::new(1.0, t.accent))
+                    .corner_radius(6.0)
+                    .inner_margin(egui::Margin::symmetric(18, 12))
+                    .show(ui, |ui| {
+                        ui.horizontal(|ui| {
+                            ui.add(egui::Spinner::new().color(t.accent));
+                            ui.label(egui::RichText::new(label).color(t.text));
+                        });
+                    });
+            });
+        ctx.request_repaint_after(Duration::from_millis(50));
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use std::time::{Duration, Instant};
+
+    use cadcraft_engine::Session;
+
+    use crate::{CadApp, Services};
+
+    /// Opening starts in the background (the UI is not blocked), shows the card, and a file that
+    /// cannot be read is reported like a failed OPEN without adding a drawing.
+    #[test]
+    fn open_path_reads_in_the_background_and_reports_failure() {
+        let mut app = CadApp::new(Session::new(), Services { pick_open: None, pick_save: None });
+        let before = app.session.docs.len();
+        let missing = std::env::temp_dir().join(format!("cadcraft-missing-{}.dxf", std::process::id()));
+        let p = missing.to_string_lossy().to_string();
+        app.open_path(&p);
+        assert!(app.opening.busy() || app.status.is_some(), "the read starts in the background");
+        let t = Instant::now();
+        while app.opening.busy() && t.elapsed() < Duration::from_secs(10) {
+            app.finish_opening();
+            std::thread::sleep(Duration::from_millis(5));
+        }
+        app.finish_opening();
+        assert!(!app.opening.busy());
+        assert_eq!(app.session.docs.len(), before);
+        let (msg, _) = app.status.clone().unwrap_or_default();
+        assert!(msg.contains(&p), "status reports the file: {msg}");
+        assert!(app.session.log.iter().any(|l| l.contains(&p)));
+    }
+
+    #[test]
+    fn card_text_names_the_file_and_counts_queued_ones() {
+        let mut o = super::Opening::default();
+        assert!(o.label().is_none());
+        o.start("/nonexistent/dir/a.dxf");
+        o.start("/nonexistent/dir/b.dxf");
+        let l = o.label().unwrap_or_default();
+        assert!(l.starts_with("Opening a.dxf and 1 more"), "{l}");
+    }
+}