From 71a1903122cfa958aa24f912f71f853f2c077a96 Mon Sep 17 00:00:00 2001 From: bherbruck Date: Wed, 7 Oct 2026 22:25:45 +0000 Subject: [PATCH] Fix: sketch edit exit (double-click a sketch no longer traps you) Edit Sketch rolls the timeline back to just after the sketch and faces it; Finish Sketch, Esc (once nothing else is left to cancel) and editing another feature all leave it, restoring the marker and the previous view. Editing the open sketch again is a no-op and editing another sketch switches cleanly. The control channel's ui.click takes double: true. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AS5Tn3xieewUSg3uxrMVdL --- crates/ui-egui/src/browser.rs | 2 +- crates/ui-egui/src/control.rs | 7 ++- crates/ui-egui/src/lib.rs | 63 +++++++++++++++++++--- crates/ui-egui/src/sketch_edit_tests.rs | 71 +++++++++++++++++++++++++ crates/ui-egui/src/timeline.rs | 2 +- crates/ui-egui/src/toolbar.rs | 5 +- 6 files changed, 138 insertions(+), 12 deletions(-) create mode 100644 crates/ui-egui/src/sketch_edit_tests.rs diff --git a/crates/ui-egui/src/browser.rs b/crates/ui-egui/src/browser.rs index 29804c5d..ba45873a 100644 --- a/crates/ui-egui/src/browser.rs +++ b/crates/ui-egui/src/browser.rs @@ -145,7 +145,7 @@ pub fn browser(app: &mut SolveApp, ui: &mut egui::Ui) { row(ui, "sketch", &name, None, false) }; if r.double_clicked() { - let _ = app.run("SketchActivate", json!({"sketch": id})); + app.edit_sketch(id); } r.on_hover_text("Double-click to edit"); } diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index 460e0f0e..cbaee31d 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -150,8 +150,11 @@ pub fn handle(app: &mut SolveApp, ctx: &egui::Context, req: &ControlRequest) -> _ => egui::PointerButton::Primary, }; let modifiers = egui::Modifiers { shift: b("shift"), ctrl: b("ctrl"), command: b("ctrl"), ..Default::default() }; - app.synthetic.push(egui::Event::PointerButton { pos, button, pressed: true, modifiers }); - app.synthetic.push(egui::Event::PointerButton { pos, button, pressed: false, modifiers }); + // `double: true` sends two clicks in one frame, which egui reads as a double-click. + for _ in 0..if b("double") { 2 } else { 1 } { + app.synthetic.push(egui::Event::PointerButton { pos, button, pressed: true, modifiers }); + app.synthetic.push(egui::Event::PointerButton { pos, button, pressed: false, modifiers }); + } } ok(Value::Null) } diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 73a6dd8f..50bd5c5d 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -18,6 +18,8 @@ pub mod palette; pub mod preview; pub mod selection; pub mod sketch_dims; +#[cfg(test)] +mod sketch_edit_tests; pub mod theme; pub mod timeline; pub mod toolbar; @@ -85,6 +87,9 @@ pub struct SolveApp { pub now: f64, /// The view before the current sketch started (Finish Sketch returns to it). pub pre_sketch_cam: Option, + /// The history marker before Edit Sketch rolled the timeline to the sketch; Finish Sketch + /// restores it. + pub pre_sketch_marker: Option>, pub services: Services, pub viewport: viewport::ViewportState, pub tool: Option, @@ -112,6 +117,7 @@ impl SolveApp { cam_anim: None, now: 0.0, pre_sketch_cam: None, + pre_sketch_marker: None, services, viewport: viewport::ViewportState::default(), tool: None, @@ -178,12 +184,7 @@ impl SolveApp { return; } if id == "SketchStop" { - let r = self.run(id, json!({})); - if r.is_ok() - && let Some(c) = self.pre_sketch_cam.take() - { - self.animate_to(c); - } + self.finish_sketch(); return; } // Press Pull: edges get a fillet, faces and profiles an extrude. @@ -226,9 +227,12 @@ impl SolveApp { use solvecraft_engine::doc::FeatureKind; let Some(f) = self.session.doc.feature(id) else { return }; if matches!(f.kind, FeatureKind::Sketch { .. }) { - let _ = self.run("SketchActivate", json!({ "sketch": id })); + self.edit_sketch(id); return; } + if self.session.active_sketch.is_some() { + self.finish_sketch(); + } let Some(idx) = self.session.doc.feature_index(id) else { return }; let marker = self.session.doc.marker; self.tool = None; @@ -245,6 +249,51 @@ impl SolveApp { } } + /// Edit a sketch the way Fusion does: roll the timeline back to just after it (so later + /// features don't hide it), turn the camera to face it, and remember both so Finish Sketch + /// puts them back. Editing the sketch that is already open does nothing; editing another one + /// finishes the current sketch first, so edit sessions never nest. + pub fn edit_sketch(&mut self, id: u64) { + if self.session.active_sketch == Some(id) { + return; + } + if self.session.active_sketch.is_some() { + self.finish_sketch(); + } + self.tool = None; + self.dialog = None; + let Some(idx) = self.session.doc.feature_index(id) else { return }; + let marker = self.session.doc.marker; + let rolled = marker.is_none_or(|m| m > idx + 1) && self.run("timeline.rollTo", json!({ "position": idx + 1 })).is_ok(); + let before = self.cam; + if self.run("SketchActivate", json!({ "sketch": id })).is_err() { + if rolled { + self.restore_marker(marker); + } + return; + } + self.pre_sketch_marker = rolled.then_some(marker); + self.pre_sketch_cam = Some(before); + dialogs::look_at_sketch(self); + } + + /// Finish the active sketch: stop editing, restore the history marker Edit Sketch moved and + /// return to the view from before the sketch. + pub fn finish_sketch(&mut self) { + self.tool = None; + let _ = self.run("SketchStop", json!({})); + if let Some(marker) = self.pre_sketch_marker.take() { + self.restore_marker(marker); + } + if let Some(c) = self.pre_sketch_cam.take() { + self.animate_to(c); + } + } + + fn restore_marker(&mut self, marker: Option) { + let _ = self.run("timeline.rollTo", marker.map(|m| json!({ "position": m })).unwrap_or_else(|| json!({}))); + } + /// Everything shown as selected: the selection plus the open dialog's inputs. pub fn highlighted(&self) -> Vec { let mut v = self.session.selection.clone(); diff --git a/crates/ui-egui/src/sketch_edit_tests.rs b/crates/ui-egui/src/sketch_edit_tests.rs new file mode 100644 index 00000000..0a5cce6d --- /dev/null +++ b/crates/ui-egui/src/sketch_edit_tests.rs @@ -0,0 +1,71 @@ +//! Regression tests: editing a sketch from the Browser or timeline can always be left again. + +use crate::{Services, SolveApp}; +use solvecraft_engine::Session; +use solvecraft_engine::doc::FeatureKind; + +fn sample_app() -> SolveApp { + let mut s = Session::default(); + s.run_script(&solvecraft_engine::sample::script()).unwrap(); + SolveApp::new(s, Services::default()) +} + +fn first_sketch(app: &SolveApp) -> u64 { + app.session.doc.features.iter().find(|f| matches!(f.kind, FeatureKind::Sketch { .. })).map(|f| f.id).unwrap() +} + +#[test] +fn edit_sketch_rolls_back_and_finish_restores() { + let mut app = sample_app(); + let marker = app.session.doc.marker; + let id = first_sketch(&app); + app.edit_sketch(id); + assert_eq!(app.session.active_sketch, Some(id)); + let idx = app.session.doc.feature_index(id).unwrap(); + assert_eq!(app.session.doc.marker, Some(idx + 1), "later features are rolled back"); + app.finish_sketch(); + assert_eq!(app.session.active_sketch, None); + assert_eq!(app.session.doc.marker, marker, "marker restored"); +} + +#[test] +fn editing_twice_does_not_nest() { + let mut app = sample_app(); + let marker = app.session.doc.marker; + let id = first_sketch(&app); + app.edit_sketch(id); + app.edit_sketch(id); + app.edit_feature(id); + app.finish_sketch(); + assert_eq!(app.session.active_sketch, None); + assert_eq!(app.session.doc.marker, marker); +} + +#[test] +fn editing_another_sketch_switches_cleanly() { + let mut app = sample_app(); + let marker = app.session.doc.marker; + let ids: Vec = app.session.doc.features.iter().filter(|f| matches!(f.kind, FeatureKind::Sketch { .. })).map(|f| f.id).collect(); + app.edit_sketch(ids[0]); + app.edit_sketch(ids[1]); + assert_eq!(app.session.active_sketch, Some(ids[1])); + app.finish_sketch(); + assert_eq!(app.session.active_sketch, None); + assert_eq!(app.session.doc.marker, marker); +} + +#[test] +fn toolbar_finish_and_other_feature_edit_leave_the_sketch() { + let mut app = sample_app(); + let marker = app.session.doc.marker; + let id = first_sketch(&app); + app.edit_sketch(id); + app.start("SketchStop"); + assert_eq!(app.session.active_sketch, None); + assert_eq!(app.session.doc.marker, marker); + // Double-clicking a solid feature while sketching finishes the sketch first. + app.edit_sketch(id); + let extrude = app.session.doc.features.iter().find(|f| !matches!(f.kind, FeatureKind::Sketch { .. })).map(|f| f.id).unwrap(); + app.edit_feature(extrude); + assert_eq!(app.session.active_sketch, None); +} diff --git a/crates/ui-egui/src/timeline.rs b/crates/ui-egui/src/timeline.rs index 533b8b56..fa267618 100644 --- a/crates/ui-egui/src/timeline.rs +++ b/crates/ui-egui/src/timeline.rs @@ -199,7 +199,7 @@ pub fn timeline(app: &mut SolveApp, ui: &mut egui::Ui) { if let Some(sk) = sketch_of && ui.button("Edit Profile Sketch").clicked() { - let _ = app.run("SketchActivate", json!({ "sketch": sk })); + app.edit_sketch(sk); ui.close(); } if ui.button("Rename").clicked() { diff --git a/crates/ui-egui/src/toolbar.rs b/crates/ui-egui/src/toolbar.rs index 596203d7..d6b09929 100644 --- a/crates/ui-egui/src/toolbar.rs +++ b/crates/ui-egui/src/toolbar.rs @@ -378,8 +378,11 @@ pub fn shortcuts(app: &mut SolveApp, ctx: &egui::Context) { crate::tools::finish(app); } else if app.dialog.is_some() { crate::dialogs::cancel(app); - } else { + } else if !app.session.selection.is_empty() || app.session.active_sketch.is_none() { let _ = app.run("select.clear", json!({})); + } else { + // Nothing left to cancel: Esc finishes the sketch. + app.finish_sketch(); } } Key::S => app.ui.palette_open = true,