From d5640b5efbd945bbe66b026d0f13ccc28d7fff98 Mon Sep 17 00:00:00 2001 From: phanoob Date: Fri, 16 Oct 2026 12:26:10 +0200 Subject: [PATCH] Timeline marker, undo and redo compute off the window thread (#1675) Thanks @phanoob. --- crates/engine/src/lib.rs | 21 +++ crates/ui-egui/src/deferred.rs | 123 ++++++++++++++++++ .../ui-egui/src/history_off_thread_tests.rs | 100 ++++++++++++++ crates/ui-egui/src/lib.rs | 19 ++- crates/ui-egui/src/timeline.rs | 12 +- crates/ui-egui/src/timeline_left.rs | 8 +- crates/ui-egui/src/viewport.rs | 13 ++ 7 files changed, 285 insertions(+), 11 deletions(-) create mode 100644 crates/ui-egui/src/deferred.rs create mode 100644 crates/ui-egui/src/history_off_thread_tests.rs diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index c75ba999..d714352c 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -156,6 +156,16 @@ pub struct Snapshot { pub pending_moves: std::collections::BTreeMap, } +/// A copy of a session that evaluates commands ahead of it ([`Session::ahead`]). +pub struct Ahead(Session); + +impl Ahead { + /// Run `commands` (their results stay in the shared evaluation cache). + pub fn evaluate(self, commands: &[(String, Value)]) -> Result<()> { + self.0.preview_in_place(commands).map(|_| ()) + } +} + /// What some commands would make, evaluated on a scratch copy of a session /// ([`Session::preview`]). The session itself is not changed. #[derive(Clone, Debug)] @@ -887,6 +897,17 @@ impl Session { self.scratch().preview_in_place(commands).map(|(after, replies)| Preview { before: visible(self), after: visible(&after), replies }) } + /// Run `commands` on a [`Session::scratch`] copy that keeps the undo and redo stacks: what + /// they evaluate lands in the evaluation cache the copy shares with this session, so running + /// them here afterwards finds it there. For a front end that computes a command which may + /// take a while (undo, the history marker moved) off its window thread first. + pub fn ahead(&self) -> Ahead { + let mut copy = self.scratch(); + copy.undo.clone_from(&self.undo); + copy.redo.clone_from(&self.redo); + Ahead(copy) + } + fn preview_in_place(mut self, commands: &[(String, Value)]) -> Result<(Self, Vec)> { if commands.is_empty() { return Err(EngineError::Other("nothing to preview".into())); diff --git a/crates/ui-egui/src/deferred.rs b/crates/ui-egui/src/deferred.rs new file mode 100644 index 00000000..382e957e --- /dev/null +++ b/crates/ui-egui/src/deferred.rs @@ -0,0 +1,123 @@ +//! Commands that recompute the history, computed off the window thread as OK is (#1201): the +//! history marker dragged or clicked, a feature suppressed, reordered or deleted from the +//! timeline, undo and redo, the marker put back after a sketch. On a part with a slow feature +//! after the marker (a line of text cut into a block), each of these ran that feature's boolean +//! on the window thread, and the window froze until it was done. +//! +//! The command is evaluated on a copy of the session ([`solidcraft_engine::Session::ahead`]), +//! which leaves what it computes in the evaluation cache the session shares. Done within the +//! app's wait ([`crate::dialogs::APPLY_AT_ONCE`]), the command just runs; otherwise the window +//! goes on, saying it computes, and the command runs for real once the evaluation is in, finding +//! its features in the cache. Commands asked for meanwhile wait their turn; a marker moved again +//! replaces the move still waiting, so a drag computes where it ends rather than every slot it +//! crossed. + +use serde_json::Value; + +use crate::SolveApp; + +/// A command computing off the window thread. +pub struct Deferred { + done: std::sync::mpsc::Receiver<()>, + /// When it started (ms). + pub(crate) started: f64, + command: (String, Value), + /// The design's revision it was computed for: a design changed meanwhile computes it again. + revision: u64, +} + +impl Deferred { + fn finished(&self) -> bool { + !matches!(self.done.try_recv(), Err(std::sync::mpsc::TryRecvError::Empty)) + } +} + +/// Commands a later one of the same kind replaces while they wait: where the marker ends is all +/// that counts. +fn replaced_by_a_later_one(id: &str) -> bool { + id == "timeline.roll_to" +} + +impl SolveApp { + /// Run a command that may recompute much of the history, off the window thread when it takes + /// a while (see the module notes). `Ok(Null)` when it is left computing or waiting its turn: + /// its outcome then shows as any command's does (an error in the status line). + pub fn run_heavy(&mut self, id: &str, params: Value) -> Result { + if self.deferred.is_some() { + if replaced_by_a_later_one(id) && self.deferred_queue.last().is_some_and(|(q, _)| q == id) { + self.deferred_queue.pop(); + } + self.deferred_queue.push((id.to_string(), params)); + return Ok(Value::Null); + } + match self.begin_deferred(id, ¶ms) { + Some(job) => { + self.deferred = Some(job); + Ok(Value::Null) + } + None => self.run(id, params), + } + } + + /// Since when a command has been computing off the window thread (ms), if one is. + pub fn computing_since(&self) -> Option { + self.deferred.as_ref().map(|d| d.started) + } + + /// Each frame: a command whose evaluation is in runs, then the next one waiting starts. + pub(crate) fn poll_deferred(&mut self, ctx: &egui::Context) { + if let Some(job) = &self.deferred { + if !job.finished() { + ctx.request_repaint_after(std::time::Duration::from_millis(100)); + return; + } + if let Some(job) = self.deferred.take() { + let (id, params) = job.command; + let superseded = replaced_by_a_later_one(&id) && self.deferred_queue.first().is_some_and(|(q, _)| *q == id); + if job.revision != self.session.revision { + // The design changed meanwhile: computed again for the design as it is now. + let _ = self.run_heavy(&id, params); + } else if !superseded { + let _ = self.run(&id, params); + } + } + } + if self.deferred.is_none() && !self.deferred_queue.is_empty() { + let (id, params) = self.deferred_queue.remove(0); + let _ = self.run_heavy(&id, params); + } + } + + /// Start evaluating a command ahead. `None` when it should just run: not asked for + /// ([`SolveApp::apply_off_thread`]), a file operation in progress, no threads (the web), or + /// done within the wait. + #[cfg(not(target_arch = "wasm32"))] + fn begin_deferred(&self, id: &str, params: &Value) -> Option { + let wait = self.apply_off_thread?; + if self.file_load.busy() || self.file_picker.is_some() { + return None; + } + let ahead = self.session.ahead(); + let command = (id.to_string(), params.clone()); + let commands = vec![command.clone()]; + let (tx, rx) = std::sync::mpsc::channel(); + std::thread::Builder::new() + .name("history".into()) + .spawn(move || { + let _ = ahead.evaluate(&commands); + let _ = tx.send(()); + }) + .ok()?; + match rx.recv_timeout(wait) { + Err(std::sync::mpsc::RecvTimeoutError::Timeout) => { + Some(Deferred { done: rx, started: crate::now_ms(), command, revision: self.session.revision }) + } + _ => None, + } + } + + #[cfg(target_arch = "wasm32")] + fn begin_deferred(&self, _id: &str, _params: &Value) -> Option { + None + } +} diff --git a/crates/ui-egui/src/history_off_thread_tests.rs b/crates/ui-egui/src/history_off_thread_tests.rs new file mode 100644 index 00000000..99f98a77 --- /dev/null +++ b/crates/ui-egui/src/history_off_thread_tests.rs @@ -0,0 +1,100 @@ +//! Commands that recompute the history off the window thread (`deferred`): the marker moved, +//! undo, each computed beside the window and run once in; a marker moved again while one move +//! computes replaces the move still waiting. + +use std::time::Duration; + +use serde_json::json; + +use crate::scenario::Harness; + +/// A 60 × 20 × 10 block, a 10 × 10 × 5 boss on it, and a 4 mm hole through the boss, the +/// marker at the end. Every command computes off the window thread (no wait at all). +fn block_boss_hole() -> Harness { + let mut h = Harness::new(); + h.app.home.open = false; + for (id, p) in [ + ("solid.box", json!({"length": 60, "width": 20, "height": 10})), + ("solid.box", json!({"corner": [5, 5, 10], "length": 10, "width": 10, "height": 5, "operation": "join"})), + ("solid.cylinder", json!({"base": [10, 10, -1], "axis": [0, 0, 1], "radius": 2, "height": 20, "operation": "cut"})), + ] { + h.app.run(id, p).unwrap(); + } + h.app.apply_off_thread = Some(Duration::ZERO); + h +} + +/// Frames until nothing computes or waits (a few seconds at most). +fn until_settled(h: &mut Harness) { + for _ in 0..500 { + if h.app.deferred.is_none() && h.app.deferred_queue.is_empty() { + break; + } + h.frames(1); + std::thread::sleep(Duration::from_millis(10)); + } + h.frames(1); +} + +fn marker(h: &Harness) -> Option { + h.app.session.doc.marker +} + +#[test] +fn the_marker_moved_computes_off_the_window_thread_then_moves() { + let mut h = block_boss_hole(); + h.app.run_heavy("timeline.roll_to", json!({"position": 1})).unwrap(); + assert!(h.app.computing_since().is_some(), "computing"); + assert_eq!(marker(&h), None, "the design is as it was while it computes"); + until_settled(&mut h); + assert_eq!(marker(&h), Some(1)); + h.step(&json!({"expect": {"volume": 12000}})).unwrap(); + // And back to the end, the boss and the hole recomputed (or found in the cache). + h.app.run_heavy("timeline.roll_to", json!({})).unwrap(); + until_settled(&mut h); + assert_eq!(marker(&h), None); + let hole = std::f64::consts::PI * 4.0 * 15.0; + let v = h.app.run("inspect.measure", json!({})).unwrap()["total"]["volume_mm3"].as_f64().unwrap(); + assert!((v - (12500.0 - hole)).abs() < 1.0, "{v}"); +} + +#[test] +fn a_marker_moved_again_replaces_the_move_still_waiting() { + let mut h = block_boss_hole(); + h.app.run_heavy("timeline.roll_to", json!({"position": 1})).unwrap(); + assert!(h.app.computing_since().is_some()); + // A drag: two more slots while the first computes. Only the last one waits. + h.app.run_heavy("timeline.roll_to", json!({"position": 2})).unwrap(); + h.app.run_heavy("timeline.roll_to", json!({"position": 3})).unwrap(); + assert_eq!(h.app.deferred_queue.len(), 1, "{:?}", h.app.deferred_queue); + until_settled(&mut h); + assert_eq!(marker(&h), None, "where the drag ended: the end of the history"); +} + +#[test] +fn undo_computes_off_the_window_thread() { + let mut h = block_boss_hole(); + let features = h.app.session.doc.features.len(); + h.app.start("edit.undo"); + assert!(h.app.computing_since().is_some(), "undo computing"); + assert_eq!(h.app.session.doc.features.len(), features, "nothing undone yet"); + until_settled(&mut h); + assert_eq!(h.app.session.doc.features.len(), features - 1, "the hole undone"); + h.step(&json!({"expect": {"volume": 12500}})).unwrap(); + h.app.start("edit.redo"); + until_settled(&mut h); + assert_eq!(h.app.session.doc.features.len(), features, "and redone"); +} + +#[test] +fn a_design_changed_while_a_move_computes_is_computed_again() { + let mut h = block_boss_hole(); + h.app.run_heavy("timeline.roll_to", json!({"position": 2})).unwrap(); + assert!(h.app.computing_since().is_some()); + // Something else changes the design meanwhile (a command that just runs). + h.app.run("solid.box", json!({"corner": [40, 5, 10], "length": 5, "width": 5, "height": 5, "operation": "join"})).unwrap(); + let revision = h.app.session.revision; + until_settled(&mut h); + assert!(h.app.session.revision > revision, "the move ran after the change"); + assert_eq!(marker(&h), Some(2)); +} diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 8e5a2a43..60aec6bf 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -20,6 +20,7 @@ mod colour_vision_tests; pub mod context_menu; pub mod control; pub mod crash_ui; +pub mod deferred; pub mod delete; pub mod design_properties; pub mod dialogs; @@ -97,6 +98,8 @@ mod expression_complete; mod extrude_edit_tests; mod file_load; mod file_picker; +#[cfg(test)] +mod history_off_thread_tests; pub use file_picker::FilePickerFuture; #[cfg(test)] mod blend_arrow_tests; @@ -321,6 +324,10 @@ pub struct SolveApp { pub preview: preview::PreviewState, /// OK pressed on a feature that takes a while: its evaluation runs off the window thread. pub applying: Option, + /// A command recomputing the history (the marker moved, undo) computing off the window + /// thread, and those asked for meanwhile ([`deferred`]). + pub deferred: Option, + pub deferred_queue: Vec<(String, Value)>, /// How long OK waits for a feature before computing it off the window thread, saying so; /// `None`: it waits as long as it takes (headless use, tests). The desktop app sets it. pub apply_off_thread: Option, @@ -435,6 +442,8 @@ impl SolveApp { slicer_menu: dialogs_save_mesh::SlicerMenu::default(), preview: preview::PreviewState::default(), applying: None, + deferred: None, + deferred_queue: Vec::new(), apply_off_thread: None, last_command: None, esc_handled: false, @@ -843,6 +852,11 @@ impl SolveApp { self.dialog = Some(d); return; } + // Undo and redo can recompute much of the history: off the window thread when slow. + if matches!(id, "edit.undo" | "edit.redo") { + let _ = self.run_heavy(id, json!({})); + return; + } let _ = self.run(id, json!({})); } @@ -928,7 +942,9 @@ impl SolveApp { } fn restore_marker(&mut self, marker: Option) { - let _ = self.run("timeline.roll_to", marker.map(|m| json!({ "position": m })).unwrap_or_else(|| json!({}))); + // Rolling forward past the features after the sketch recomputes them (an edited sketch + // under a text cut): off the window thread when it takes a while. + let _ = self.run_heavy("timeline.roll_to", marker.map(|m| json!({ "position": m })).unwrap_or_else(|| json!({}))); } /// Everything shown as selected: the selection plus the open dialog's inputs. @@ -1320,6 +1336,7 @@ impl SolveApp { fn cad_ui(&mut self, ui: &mut egui::Ui, report_input: &mut Option) { scenario::clear_handles(); + self.poll_deferred(ui.ctx()); toolbar::app_bar(self, ui); issue_report::CadInput::block(self, ui, report_input); file_load::show(self, ui.ctx()); diff --git a/crates/ui-egui/src/timeline.rs b/crates/ui-egui/src/timeline.rs index 4e7fccd2..867f4d83 100644 --- a/crates/ui-egui/src/timeline.rs +++ b/crates/ui-egui/src/timeline.rs @@ -81,7 +81,7 @@ pub(crate) fn delete_features(app: &mut SolveApp, ids: Vec) { } } if with.is_empty() && fail.is_empty() { - let _ = app.run("timeline.delete", json!({ "features": ids })); + let _ = app.run_heavy("timeline.delete", json!({ "features": ids })); } else { app.dialog = Some(crate::dialogs::Dialog::confirm_delete_many(ids, with.into_iter().collect(), fail.into_iter().collect())); } @@ -192,7 +192,7 @@ pub fn timeline(app: &mut SolveApp, ui: &mut egui::Ui) { Some(k) if k < n => json!({"position": k}), _ => json!({}), }; - let _ = app.run("timeline.roll_to", pos); + let _ = app.run_heavy("timeline.roll_to", pos); } x += 24.0; } @@ -341,11 +341,11 @@ pub fn timeline(app: &mut SolveApp, ui: &mut egui::Ui) { ui.close(); } if ui.button(if *suppressed { "Unsuppress Features" } else { "Suppress Features" }).clicked() { - let _ = app.run("timeline.suppress", json!({"feature": id})); + let _ = app.run_heavy("timeline.suppress", json!({"feature": id})); ui.close(); } if ui.button(crate::i18n::tr_label("Roll History Marker Here")).clicked() { - let _ = app.run("timeline.roll_to", json!({ "feature": id })); + let _ = app.run_heavy("timeline.roll_to", json!({ "feature": id })); ui.close(); } if ui.button(crate::i18n::tr_label("Find in Browser")).clicked() { @@ -398,14 +398,14 @@ pub fn timeline(app: &mut SolveApp, ui: &mut egui::Ui) { let slot = slot_at(&xs, end, pp.x); match dg { Drag::Marker => { - let _ = app.run("timeline.roll_to", if slot >= n { json!({}) } else { json!({ "position": slot }) }); + let _ = app.run_heavy("timeline.roll_to", if slot >= n { json!({}) } else { json!({ "position": slot }) }); } Drag::Feature(id) => { let from = app.session.doc.feature_index(id).unwrap_or(0); // Dropping after itself means the slot shifts by one. let to = if slot > from { slot - 1 } else { slot }; if to != from { - let _ = app.run("timeline.reorder", json!({ "feature": id, "position": to })); + let _ = app.run_heavy("timeline.reorder", json!({ "feature": id, "position": to })); } } } diff --git a/crates/ui-egui/src/timeline_left.rs b/crates/ui-egui/src/timeline_left.rs index 51e1f09a..3f88b81a 100644 --- a/crates/ui-egui/src/timeline_left.rs +++ b/crates/ui-egui/src/timeline_left.rs @@ -117,7 +117,7 @@ fn timeline_transport(app: &mut SolveApp, ui: &mut egui::Ui) { Some(position) if position < n => json!({"position": position}), _ => json!({}), }; - let _ = app.run("timeline.roll_to", params); + let _ = app.run_heavy("timeline.roll_to", params); } } }); @@ -794,11 +794,11 @@ fn vertical_features(app: &mut SolveApp, ui: &mut egui::Ui) { ui.close(); } if ui.button(if *suppressed { "Unsuppress Feature" } else { "Suppress Feature" }).clicked() { - let _ = app.run("timeline.suppress", json!({"feature": id})); + let _ = app.run_heavy("timeline.suppress", json!({"feature": id})); ui.close(); } if ui.button("Roll Back Here").clicked() { - let _ = app.run("timeline.roll_to", json!({"feature": id})); + let _ = app.run_heavy("timeline.roll_to", json!({"feature": id})); ui.close(); } ui.separator(); @@ -875,7 +875,7 @@ fn vertical_features(app: &mut SolveApp, ui: &mut egui::Ui) { { match active { Drag::Marker => { - let _ = app.run("timeline.roll_to", if slot.position >= n { json!({}) } else { json!({"position": slot.position}) }); + let _ = app.run_heavy("timeline.roll_to", if slot.position >= n { json!({}) } else { json!({"position": slot.position}) }); } Drag::Items(items) => { let panel_left = ui.min_rect().left(); diff --git a/crates/ui-egui/src/viewport.rs b/crates/ui-egui/src/viewport.rs index 95eb6fda..e5d72eea 100644 --- a/crates/ui-egui/src/viewport.rs +++ b/crates/ui-egui/src/viewport.rs @@ -2591,6 +2591,19 @@ pub fn show(app: &mut SolveApp, ui: &mut egui::Ui) { crate::select_other::show(app, ui.ctx()); axis_view(app, ui, rect); nav_bar(app, ui, rect); + // A command recomputing the history off the window thread (the marker moved, undo): the + // window goes on, and says so above the status chip. + if let Some(since) = app.computing_since() { + let seconds = ((crate::now_ms() - since) / 1000.0).floor().max(0.0); + let text = crate::i18n::format("dialog.computing", &[("seconds", &format!("{seconds:.0}"))]); + let pos = pos2(rect.left() + 12.0, rect.bottom() - 76.0); + let galley = painter.layout_no_wrap(text, FontId::proportional(12.5), t.text); + let r = Rect::from_min_size(pos, galley.size() + vec2(34.0, 8.0)); + painter.rect_filled(r, 4.0, t.overlay); + egui::Spinner::new().size(12.0).paint_at(ui, Rect::from_center_size(pos2(r.left() + 13.0, r.center().y), vec2(12.0, 12.0))); + painter.galley(pos + vec2(26.0, 4.0), galley, t.text); + ui.ctx().request_repaint_after(std::time::Duration::from_millis(100)); + } // Status chip. if let Some((msg, at, err)) = app.status.clone() { if crate::now_ms() - at < 6000.0 {