From a18bedddf694c4d0a60d99df66ecd4687e5ff5f5 Mon Sep 17 00:00:00 2001 From: Colin Faulkingham Date: Thu, 15 Oct 2026 11:47:37 -0500 Subject: [PATCH] Keep New Body and Offset Face previews moving during distance drags (#1238) New Body extrusions and Offset Face previews show successful intermediate distances from the same setup while a distance drag runs, then compute the latest one (tool previews keep streaming as before; errors still need exact inputs). The drag arrow and value box are painted from this frame's pointer, removing a frame of lag. Drag regression tests and a perf note added. --- ROADMAP.md | 7 ++ crates/ui-egui/src/canvas.rs | 26 ++-- crates/ui-egui/src/extrude_edit_tests.rs | 33 +++++ crates/ui-egui/src/preview.rs | 65 ++++++++-- crates/ui-egui/src/preview_drag_tests.rs | 149 +++++++++++++++++++++++ docs/perf.md | 37 ++++++ 6 files changed, 294 insertions(+), 23 deletions(-) create mode 100644 crates/ui-egui/src/preview_drag_tests.rs diff --git a/ROADMAP.md b/ROADMAP.md index f7562fb8..21a2cf07 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -66,6 +66,13 @@ pointer movement orbits without a mouse button, and vertical scrolling zooms. Ot retain their saved two-finger scroll preference; camera motion is available through `view.navigate` and modifier-aware `ui.move` automation. +## Interactive modelling + +- New Body extrusion and face-offset previews show completed intermediate geometry while a + drag continues, then catch up to the exact final value. Changes to the selected geometry, + operation or feature still discard obsolete results. Distance arrows and their value boxes + follow the current frame's input. + ## Physical material data - Physical material card validation (#923/#922) provides typed SI mechanical and diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index 7b04073e..22641c89 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -527,17 +527,12 @@ fn drag_arrow( ) -> Option { let t = Tokens::get(); let min = 40.0 * 2.0 * half_height / f64::from(proj.rect.height().max(1.0)); - let shown = if l.abs() < min { min * if l < 0.0 { -1.0 } else { 1.0 } } else { l }; - let ts = proj.to_screen(base + n * shown)?; - crate::scenario::publish_handle(if id == egui::Id::new("sc_manipulator") { "arrow" } else { "arrow2" }, ts); + let tip = |length: f64| { + let shown = if length.abs() < min { min * if length < 0.0 { -1.0 } else { 1.0 } } else { length }; + proj.to_screen(base + n * shown) + }; + let mut ts = tip(l)?; let r = ui.interact(egui::Rect::from_center_size(ts, vec2(20.0, 20.0)), id, egui::Sense::drag()); - let hot = r.hovered() || r.dragged(); - let col = if hot { t.accent } else { t.manipulator }; - arrow(painter, bs, ts, col); - if hot { - painter.circle_stroke(ts, 8.0, Stroke::new(1.5, col)); - ui.ctx().set_cursor_icon(egui::CursorIcon::Grab); - } if r.dragged() && let Some(p) = r.interact_pointer_pos() && let Some(v) = along(proj, p, base, n) @@ -549,6 +544,17 @@ fn drag_arrow( v = v.max(if free { 1e-3 } else { step }); } *value = format_len(v); + ts = tip(v)?; + } + // Paint the value just read from this frame's pointer, including the value box anchor. + // Using the old distance here made both trail the drag by another frame. + crate::scenario::publish_handle(if id == egui::Id::new("sc_manipulator") { "arrow" } else { "arrow2" }, ts); + let hot = r.hovered() || r.dragged(); + let col = if hot { t.accent } else { t.manipulator }; + arrow(painter, bs, ts, col); + if hot { + painter.circle_stroke(ts, 8.0, Stroke::new(1.5, col)); + ui.ctx().set_cursor_icon(egui::CursorIcon::Grab); } Some(ts) } diff --git a/crates/ui-egui/src/extrude_edit_tests.rs b/crates/ui-egui/src/extrude_edit_tests.rs index 39f4de18..5064292b 100644 --- a/crates/ui-egui/src/extrude_edit_tests.rs +++ b/crates/ui-egui/src/extrude_edit_tests.rs @@ -361,3 +361,36 @@ fn symmetric_through_all_handle_drag_switches_to_total_distance() { let bounds = h.app.session.world_state().body("Main").unwrap().mesh().bounds(); assert!((bounds.min.z + 60.0).abs() < 1e-7 && (bounds.max.z - 60.0).abs() < 1e-7); } + +/// Inspect each held frame, not just the settled end of the drag: the displayed arrow must +/// be at the distance the pointer just wrote, rather than the previous frame's distance. +#[test] +fn extrusion_arrow_tracks_the_current_drag_frame() { + let mut h = make(json!({"distance":10})); + open(&mut h); + let at = h.at(&json!({"handle":"arrow"})).unwrap(); + let at = egui::pos2(at[0] as f32, at[1] as f32); + h.app.synthetic_mods.ctrl = true; + h.app.synthetic.push(egui::Event::PointerMoved(at)); + h.frame(); + h.app.synthetic.push(egui::Event::PointerButton { + pos: at, + button: egui::PointerButton::Primary, + pressed: true, + modifiers: h.app.synthetic_mods, + }); + h.frame(); + for dy in [-20.0, -40.0, -60.0] { + h.app.synthetic.push(egui::Event::PointerMoved(at + egui::vec2(0.0, dy))); + h.frame(); + let d = h.app.dialog.as_ref().unwrap(); + let crate::dialogs::Kind::Extrude { distance, .. } = &d.kind else { panic!("extrude") }; + let distance = h.app.session.doc.eval(distance, solidcraft_engine::doc::expr::Kind::Length).unwrap(); + assert!(distance > 10.0); + let (base, normal) = crate::canvas::anchor(&h.app, d).unwrap(); + let world = base + normal.unwrap() * distance; + let expected = h.at(&json!({"world":[world.x, world.y, world.z]})).unwrap(); + let drawn = h.at(&json!({"handle":"arrow"})).unwrap(); + assert!((drawn[0] - expected[0]).hypot(drawn[1] - expected[1]) < 0.1, "arrow {drawn:?} trails current value {expected:?}"); + } +} diff --git a/crates/ui-egui/src/preview.rs b/crates/ui-egui/src/preview.rs index 56117241..2d6d28d3 100644 --- a/crates/ui-egui/src/preview.rs +++ b/crates/ui-egui/src/preview.rs @@ -71,8 +71,8 @@ pub struct PreviewState { running: Option<(u64, Receiver)>, /// The running job builds a tool alone (for a join, cut or intersect). running_tool: bool, - /// Inputs of an in-flight joint preview, excluding only its driven values. - running_motion_key: Option, + /// Inputs of a joint or distance preview, excluding only its driven values. + running_stream_key: Option, /// Bodies the preview stands in for (hidden in the model scene). pub replaced: Vec, /// The preview scene for the GPU and its key (changes whenever the scene does). @@ -102,7 +102,7 @@ impl PreviewState { fn clear(&mut self) { self.running = None; self.running_tool = false; - self.running_motion_key = None; + self.running_stream_key = None; self.shown_key = None; self.error = None; self.applied = None; @@ -152,7 +152,7 @@ impl PreviewState { /// Errors validate only the current inputs. Keep the last good geometry while /// computing new inputs, but never let an obsolete result block their Apply. - fn poll(&mut self, key: u64, motion_key: Option) { + fn poll(&mut self, key: u64, stream_key: Option) { if self.shown_key != Some(key) { self.sketches.clear(); self.error = None; @@ -167,11 +167,11 @@ impl PreviewState { if let Some(job) = job { self.running = None; let good = matches!(&job, Ok(b) if b.error.is_none()); - let intermediate = good && (self.running_tool || motion_key.is_some() && self.running_motion_key == motion_key); - self.running_motion_key = None; + let intermediate = good && (self.running_tool || stream_key.is_some() && self.running_stream_key == stream_key); + self.running_stream_key = None; self.running_tool = false; // A held slider, a dragged arrow or an animation may advance faster than the - // worker. Show completed poses from the same joint setup, and tools built for + // worker. Show completed joint/distance results from the same setup, and tools built for // inputs since changed, then compute the latest. Errors still require an exact // key. if k == key || intermediate { @@ -209,6 +209,41 @@ fn motion_key(cmds: &[(String, Value)], revision: u64, colors: Colors) -> Option Some(h.finish()) } +/// Tool previews already stream in `poll`. New-body extrusions and face offsets instead +/// build their full result; allow successful intermediate distances only within the same +/// setup. Preserve selections, feature identity, extent mode, revision and colours. The +/// exact job key still ensures the latest distance is computed and errors never go stale. +fn stream_key(cmds: &[(String, Value)], revision: u64, colors: Colors) -> Option { + if let Some(key) = motion_key(cmds, revision, colors) { + return Some(key); + } + if cmds.is_empty() || as_new_body(cmds).is_some() { + return None; + } + let mut setup = cmds.to_vec(); + for (command, params) in &mut setup { + let (command, params) = if command == "timeline.redefine" { + let command = params.get("command")?.as_str()?.to_string(); + (command, params.get_mut("params")?) + } else { + (command.clone(), params) + }; + let fields: &[&str] = match command.as_str() { + "solid.extrude" => &["distance", "distance2", "to_offset"], + "solid.offset_face" => &["distance"], + _ => return None, + }; + let params = params.as_object_mut()?; + for field in fields { + // Keep presence: changing extent mode must not reuse the old result. + if let Some(value) = params.get_mut(*field) { + *value = Value::Null; + } + } + } + Some(job_key(&setup, revision, colors)) +} + /// Keep the preview in step with the open dialog: start a computation when its inputs changed, /// take finished results. Called once per frame. pub fn update(app: &mut SolveApp, ctx: &egui::Context) { @@ -227,13 +262,13 @@ pub fn update(app: &mut SolveApp, ctx: &egui::Context) { }; let colors = Colors::from_tokens(&Tokens::get()); let key = job_key(&cmds, app.session.revision, colors); - let motion_key = motion_key(&cmds, app.session.revision, colors); + let stream_key = stream_key(&cmds, app.session.revision, colors); let tool = as_new_body(&cmds).is_some(); let pv = &mut app.preview; - pv.poll(key, motion_key); + pv.poll(key, stream_key); if pv.running.is_none() && pv.shown_key != Some(key) { pv.started = crate::now_ms(); - pv.running_motion_key = motion_key; + pv.running_stream_key = stream_key; let scratch = app.session.scratch(); spawn(&mut app.preview, key, scratch, cmds, colors, tool); } @@ -720,7 +755,7 @@ mod tests { for (running, wanted) in [(1, 2), (2, 3), (3, 4)] { let (tx, rx) = std::sync::mpsc::channel(); pv.running = Some((running, rx)); - pv.running_motion_key = Some(42); + pv.running_stream_key = Some(42); pv.poll(wanted, Some(42)); assert!(pv.running.is_some()); tx.send(Ok(pose())).unwrap(); @@ -733,7 +768,7 @@ mod tests { } let (tx, rx) = std::sync::mpsc::channel(); pv.running = Some((4, rx)); - pv.running_motion_key = Some(42); + pv.running_stream_key = Some(42); tx.send(Ok(pose())).unwrap(); pv.poll(4, Some(42)); assert_eq!(pv.shown_key, Some(4), "release catches up to the exact final pose"); @@ -748,7 +783,7 @@ mod tests { (Some(42), Err("old invalid value".into())), (Some(42), Ok(Built { error: Some("old tool preview error".into()), ..pose() })), ] { - let mut pv = PreviewState { running_motion_key: Some(42), ..PreviewState::default() }; + let mut pv = PreviewState { running_stream_key: Some(42), ..PreviewState::default() }; let (tx, rx) = std::sync::mpsc::channel(); pv.running = Some((1, rx)); tx.send(job).unwrap(); @@ -997,3 +1032,7 @@ mod tests { assert!(pv.error.is_none() && pv.shown_key == Some(1), "an old error is dropped"); } } + +#[cfg(test)] +#[path = "preview_drag_tests.rs"] +mod drag_tests; diff --git a/crates/ui-egui/src/preview_drag_tests.rs b/crates/ui-egui/src/preview_drag_tests.rs new file mode 100644 index 00000000..24f56f7b --- /dev/null +++ b/crates/ui-egui/src/preview_drag_tests.rs @@ -0,0 +1,149 @@ +//! Completed distances must reach the viewport before a continuously moving pointer stops. +use super::*; +use serde_json::json; + +fn colors() -> Colors { + Colors { body: [1, 1, 1, 255], added: [2, 2, 2, 255], removed: [3, 3, 3, 100], edge: [0, 0, 0, 255] } +} + +fn fixture() -> Session { + let mut s = Session::new(solidcraft_engine::doc::Document::new("Preview drag")); + s.execute("sketch.create", &json!({"plane":"XY", "name":"Profile"})).unwrap(); + s.execute("sketch.rectangle.two_point", &json!({"p0":[0,0], "p1":[10,10]})).unwrap(); + s.execute("sketch.finish", &json!({})).unwrap(); + s +} + +fn extrude(distance: i32) -> Vec<(String, Value)> { + vec![("solid.extrude".into(), json!({"sketch":"Profile", "distance":distance, "operation":"new"}))] +} + +fn check_drag(s: &Session, commands: impl Fn(i32) -> Vec<(String, Value)>, base_z: f64) { + let original = serde_json::to_value(&*s.doc).unwrap(); + let undo = s.undo.len(); + let mut pv = PreviewState::default(); + for (finished, wanted) in [(10, 20), (20, 30), (30, 30)] { + let old = commands(finished); + let new = commands(wanted); + let old_key = job_key(&old, s.revision, colors()); + let key = job_key(&new, s.revision, colors()); + let (tx, rx) = std::sync::mpsc::channel(); + pv.running = Some((old_key, rx)); + pv.running_tool = as_new_body(&old).is_some(); + pv.running_stream_key = stream_key(&old, s.revision, colors()); + let built = compute_tool(s, &old, colors()).unwrap(); + tx.send(Ok(built)).unwrap(); + pv.poll(key, stream_key(&new, s.revision, colors())); + assert!(pv.active && pv.error.is_none(), "completed geometry must show during the drag: {:?}", pv.error); + assert!((pv.bounds.max.z - base_z - f64::from(finished)).abs() < 1e-8); + assert_eq!(pv.shown_key, Some(old_key)); + assert_eq!(pv.shown_key == Some(key), finished == wanted, "only the final distance satisfies the latest request"); + assert!(pv.running.is_none()); + } + assert_eq!(serde_json::to_value(&*s.doc).unwrap(), original); + assert_eq!(s.undo.len(), undo); +} + +#[test] +fn new_body_extrude_keeps_moving_during_a_drag() { + check_drag(&fixture(), extrude, 0.0); +} + +#[test] +fn edited_new_body_extrude_keeps_moving_during_a_drag() { + let mut s = fixture(); + let feature = s.execute("solid.extrude", &extrude(5)[0].1).unwrap()["feature"].as_u64().unwrap(); + check_drag( + &s, + |distance| vec![("timeline.redefine".into(), json!({"feature":feature,"command":"solid.extrude","params":extrude(distance)[0].1}))], + 0.0, + ); +} + +#[test] +fn face_offset_keeps_moving_during_a_drag() { + let mut s = Session::new(solidcraft_engine::doc::Document::new("Face offset drag")); + s.execute("solid.box", &json!({"length":10,"width":10,"height":10})).unwrap(); + check_drag(&s, |distance| vec![("solid.offset_face".into(), json!({"faces":[[5,5,10]],"distance":distance}))], 10.0); +} + +#[test] +fn join_tool_keeps_moving_without_waiting_for_its_boolean() { + let mut s = fixture(); + s.execute("solid.box", &json!({"length":10,"width":10,"height":5})).unwrap(); + check_drag(&s, |distance| vec![("solid.extrude".into(), json!({"sketch":"Profile","distance":distance,"operation":"join"}))], 0.0); +} + +#[test] +fn distance_stream_keeps_feature_selection_and_extent_context() { + let old = extrude(10); + let key = stream_key(&old, 7, colors()); + assert!(key.is_some()); + assert_eq!(key, stream_key(&extrude(20), 7, colors())); + assert_ne!(key, stream_key(&old, 8, colors())); + let mut recolored = colors(); + recolored.added = [40, 50, 60, 255]; + assert_ne!(key, stream_key(&old, 7, recolored)); + for (field, value) in [ + ("sketch", json!("Other")), + ("profiles", json!([1])), + ("face", json!([0, 0, 5])), + ("operation", json!("cut")), + ("direction", json!("negative")), + ("distance2", json!(5)), + ("to", json!([0, 0, 50])), + ("taper", json!(5)), + ("start_offset", json!(3)), + ("participants", json!(["Other"])), + ] { + let mut changed = old.clone(); + changed[0].1[field] = value; + assert_ne!(key, stream_key(&changed, 7, colors()), "changed {field} cannot reuse the old result"); + } + for field in ["distance2", "to_offset"] { + let mut from = old.clone(); + from[0].1[field] = json!(5); + let mut to = from.clone(); + to[0].1[field] = json!(9); + assert_eq!(stream_key(&from, 7, colors()), stream_key(&to, 7, colors())); + } + let edit = + |feature, distance| vec![("timeline.redefine".into(), json!({"feature":feature,"command":"solid.extrude","params":extrude(distance)[0].1}))]; + assert_eq!(stream_key(&edit(2, 10), 7, colors()), stream_key(&edit(2, 20), 7, colors())); + assert_ne!(stream_key(&edit(2, 10), 7, colors()), stream_key(&edit(3, 20), 7, colors())); + assert_ne!(key, stream_key(&edit(2, 10), 7, colors())); + let offset = |distance| vec![("solid.offset_face".into(), json!({"faces":[[5,5,10]],"distance":distance}))]; + assert!(stream_key(&offset(1), 7, colors()).is_some()); + assert_eq!(stream_key(&offset(1), 7, colors()), stream_key(&offset(5), 7, colors())); + assert!(stream_key(&[], 7, colors()).is_none()); + assert!(stream_key(&[("solid.fillet".into(), json!({"radius":5}))], 7, colors()).is_none()); + let mut mixed = old; + mixed.push(("select.clear".into(), json!({}))); + assert!(stream_key(&mixed, 7, colors()).is_none()); +} + +#[test] +fn distance_results_reject_other_setups_and_obsolete_errors() { + let s = fixture(); + let old = extrude(10); + let current = extrude(20); + for (revision, failed) in [(s.revision + 1, false), (s.revision, true)] { + let mut built = compute_tool(&s, &old, colors()).unwrap(); + if failed { + built.error = Some("old distance failed".into()); + } + let (tx, rx) = std::sync::mpsc::channel(); + let mut pv = PreviewState { + running: Some((job_key(&old, s.revision, colors()), rx)), + running_stream_key: stream_key(&old, s.revision, colors()), + ..Default::default() + }; + tx.send(Ok(built)).unwrap(); + pv.poll(job_key(¤t, revision, colors()), stream_key(¤t, revision, colors())); + assert!(!pv.active && pv.error.is_none() && pv.shown_key.is_none()); + } + let mut join = current.clone(); + join[0].1["operation"] = json!("join"); + assert!(as_new_body(&join).is_some()); + assert!(stream_key(&join, s.revision, colors()).is_none(), "tool previews retain their upstream streaming path"); +} diff --git a/docs/perf.md b/docs/perf.md index 42b39092..ad0cd727 100644 --- a/docs/perf.md +++ b/docs/perf.md @@ -816,3 +816,40 @@ observed 47.6 ms check; no local PNG was created and the active document remaine empty. The screenshot shows the transferred file's picture while built-in sample pictures are still loading. This is one functional visual check, not a first-frame benchmark. The separate counting-reader regression proves source-blob deferral. + +## Extrude drag preview delivery + +On macOS with Metal, editing a 15 mm radius cylinder's 10 mm extrusion and dragging its +arrow upward by 150 screen points (120 input steps, Ctrl held) exposed preview starvation. +A completed worker result was discarded whenever the pointer had already changed the +requested distance. The arrow also drew the previous frame's distance. + +Main's tool-only preview already delivers intermediate Join/Cut/Intersect geometry. New Body +extrusions (including feature edits) and Offset Face use full-result previews and still discard +completed distances during a moving drag. Those paths now accept successful intermediate +distances from the same setup. Selections, operation, extent mode, feature identity, revision +and colours remain part of the setup identity. Errors still apply only to the exact requested +inputs, and the final distance is computed normally. Tool previews and background Apply retain +their existing paths. The arrow and value-box anchor use the current frame's pointer value. + +Release-build control-channel checks against upstream `4dc63810` with the arrow fix alone, +then with the distance fallback, used the same 150-point, 120-step drags. The cylinder used +the setup above; planar-face Press Pull (Join) and Offset Face used a 30 × 30 × 10 mm box. +The samples captured 25–27 distinct requested distances per drag: + +| Preview updates before reaching the final distance | Main + arrow fix | With distance fallback | +|---|---:|---:| +| Edited New Body extrusion | 0 | 24 | +| Planar-face Press Pull (Join) | 22 | 24 | +| Offset Face | 0 | 23 | + +Join already streams on main; the small count difference reflects sampling, not a change to +that path. Applying the fixed New Body extrusion produced the requested 19.585 mm height; +applying the 11.672 mm face offset produced a 21.672 mm tall box. + +These are sampled preview deliveries (`ui.inspect.preview.ms` changes), not every rendered +frame or a geometry-kernel speed benchmark. With main's preview code unchanged, the New Body, +edited New Body and Offset Face drag regressions fail while Join passes. With the fallback, +all six distance-preview regressions pass, covering intermediate geometry, exact final +convergence, unchanged document/undo, obsolete errors and setup changes. A separate regression +checks the arrow's position on each held input frame.