From 27f20f97cb5331f06f092dcf2392b38466474446 Mon Sep 17 00:00:00 2001 From: Jonas Wrede Date: Tue, 13 Oct 2026 11:22:52 +0200 Subject: [PATCH] feat: view.apply restores a named view or camera; MCP screenshot takes a named view or camera (#528) (#595) New view.apply command restores a named view, a standard view or any camera (view state, not an undo step); the desktop viewport follows it and the browser's Restore uses it. The MCP screenshot tool takes a named view or a camera. Un-ignores the #528 regression test. Fixes #528. Co-authored-by: jonaswre --- crates/engine/src/cmd/browser.rs | 64 +++++++++++++++++++ crates/engine/src/lib.rs | 6 ++ crates/mcp/src/backend.rs | 25 ++++++-- crates/mcp/src/tests.rs | 13 +++- crates/mcp/src/tools.rs | 29 +++++++-- crates/ui-egui/src/browser.rs | 6 +- crates/ui-egui/src/control.rs | 8 ++- crates/ui-egui/src/lib.rs | 16 +++++ .../scenarios/view_apply_named_view.json | 10 +++ docs/features.md | 3 +- docs/mcp.md | 2 +- 11 files changed, 159 insertions(+), 23 deletions(-) create mode 100644 crates/ui-egui/tests/scenarios/view_apply_named_view.json diff --git a/crates/engine/src/cmd/browser.rs b/crates/engine/src/cmd/browser.rs index d7b7a1b6..b27c8718 100644 --- a/crates/engine/src/cmd/browser.rs +++ b/crates/engine/src/cmd/browser.rs @@ -33,6 +33,9 @@ pub static COMMANDS: &[CommandSpec] = &[ CommandSpec::new("view.rename", "Rename Named View", view_rename).params("view: name, name"), CommandSpec::new("view.delete", "Delete Named View", view_delete).params("view: name"), CommandSpec::new("view.list", "List Named Views", view_list).noundo().doc_only(), + CommandSpec::new("view.apply", "Restore Named View", view_apply).icon("camera").noundo().params( + "view?: front|back|top|bottom|left|right|iso|home (frames the model) | a named view's name; or camera?: {target: [x,y,z], yaw, pitch, distance, fov?} (radians, mm) → camera. The viewport (and the MCP screenshot) moves there. View state: not saved, not an undo step", + ), CommandSpec::new("browser.order", "Order Browser Items", order).params("folder, component?, order: [keys] (the folder's items in display order)"), CommandSpec::new("browser.visibility", "Show/Hide", visibility).icon("eye").noundo().params( "visible: bool; items?: [sketch id or name | body name | construction name | O|X|Y|Z|XY|XZ|YZ]; \ @@ -375,6 +378,38 @@ fn view_save(s: &mut Session, p: &Value) -> Result { Ok(json!({ "view": name })) } +fn view_apply(s: &mut Session, p: &Value) -> Result { + let cmd = "view.apply"; + let cam = match (str_(p, "view"), p.get("camera")) { + (Some(_), Some(_)) => return Err(bad(cmd, "give `view` or `camera`, not both")), + (None, None) => return Err(bad(cmd, "`view` (a name) or `camera` is required")), + (None, Some(c)) => { + let v: solidcraft_doc::NamedView = serde_json::from_value(c.clone()).map_err(|e| bad(cmd, format!("bad camera: {e}")))?; + camera_of(&v) + } + // Standard view names come first, as in the browser's Named Views folder. + (Some(name), None) => match crate::render::StandardView::parse(name) { + Some(sv) => { + let mut c = crate::view::home_camera(s); + c.set_view(sv); + c.fit(&crate::view::bounds(s)); + c + } + None => s.doc.named_views.iter().find(|v| v.name == name).map(camera_of).ok_or_else(|| bad(cmd, format!("no named view `{name}`")))?, + }, + }; + if !cam.is_valid() || !cam.fov.is_finite() || cam.fov < 0.0 { + return Err(bad(cmd, "the camera must be finite with a positive distance")); + } + s.camera = Some(cam); + Ok(json!({ "camera": cam })) +} + +/// The orbit camera of a named view (Z up, no roll). +fn camera_of(v: &solidcraft_doc::NamedView) -> crate::render::Camera { + crate::render::Camera { target: v.target, yaw: v.yaw, pitch: v.pitch, distance: v.distance, fov: v.fov, ..Default::default() } +} + fn view_index(s: &Session, p: &Value, cmd: &str) -> Result { let k = str_(p, "view").ok_or_else(|| bad(cmd, "`view` must be a view name"))?; s.doc.named_views.iter().position(|v| v.name == k).ok_or_else(|| bad(cmd, format!("no named view `{k}`"))) @@ -724,6 +759,35 @@ mod tests { assert_eq!(s.doc.param("w").map(|p| p.expr.as_str()), Some("2 in")); } + #[test] + fn applying_a_named_view_or_camera_moves_the_view() { + let mut s = Session::default(); + run(&mut s, "solid.box", json!({"length": 40, "width": 20, "height": 10})); + run(&mut s, "view.save", json!({"name": "Photo", "camera": {"target": [1, 2, 3], "yaw": 0.5, "pitch": -0.6, "distance": 150}})); + let undo = s.undo.len(); + assert!(s.camera.is_none()); + let r = run(&mut s, "view.apply", json!({"view": "Photo"})); + let c = s.camera.expect("a camera for the view"); + assert_eq!((c.target, c.yaw, c.pitch, c.distance), (solidcraft_geom::Vec3::new(1.0, 2.0, 3.0), 0.5, -0.6, 150.0)); + assert_eq!(r["camera"]["yaw"], 0.5); + // A standard view looks from its direction and frames the model. + run(&mut s, "view.apply", json!({"view": "top"})); + let c = s.camera.expect("a camera for the top view"); + assert!((c.pitch - std::f64::consts::FRAC_PI_2).abs() < 1e-3 && (c.target.x - 20.0).abs() < 1e-6, "{c:?}"); + run(&mut s, "view.apply", json!({"camera": {"target": [0, 0, 0], "yaw": 1, "pitch": 0.2, "distance": 80, "fov": 0.5}})); + assert_eq!(s.camera.map(|c| (c.yaw, c.distance, c.fov)), Some((1.0, 80.0, 0.5))); + assert_eq!(s.undo.len(), undo, "view state, not an undo step"); + for p in [ + json!({}), + json!({"view": "Nope"}), + json!({"view": "Photo", "camera": {"target": [0, 0, 0], "yaw": 0, "pitch": 0, "distance": 1}}), + json!({"camera": {"target": [0, 0, 0], "yaw": 0, "pitch": 0, "distance": 0}}), + json!({"camera": "x"}), + ] { + assert!(s.execute("view.apply", &p).is_err(), "{p}"); + } + } + #[test] fn named_views_are_saved_renamed_and_deleted() { let mut s = Session::default(); diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index e778ef10..087277a4 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -221,6 +221,10 @@ pub struct Session { /// What is hidden or shown in the view (`browser.visibility`): saved with the design, not an /// undo step. A design brings its saved visibility when it opens (`Session::new`). pub visibility: Visibility, + /// The camera `view.apply` asked the view to use (view state, not saved). The desktop app + /// moves its viewport there and takes it; headless renders keep using it until a standard + /// view is chosen. + pub camera: Option, /// The visibility the design file holds (what `is_dirty` compares with). saved_visibility: Visibility, /// States to go back to (`edit.checkpoint`, `edit.restore_checkpoint`): how batches of @@ -316,6 +320,7 @@ impl Session { next_checkpoint: 1, saved_visibility: visibility.clone(), visibility, + camera: None, #[cfg(test)] evaluations: 0, crash_journal: false, @@ -670,6 +675,7 @@ impl Session { checkpoints: Vec::new(), next_checkpoint: 1, visibility: self.visibility.clone(), + camera: self.camera, saved_visibility: self.saved_visibility.clone(), #[cfg(test)] evaluations: 0, diff --git a/crates/mcp/src/backend.rs b/crates/mcp/src/backend.rs index 01309059..acb245e6 100644 --- a/crates/mcp/src/backend.rs +++ b/crates/mcp/src/backend.rs @@ -144,16 +144,29 @@ impl Backend for Headless { "document.inspect" => s.execute("document.inspect", &p).map_err(|e| e.to_string()), "ui.view" => { let v = str_p("view").unwrap_or("home"); - self.view = match v { - "fit" => self.view, - _ => StandardView::parse(v).ok_or_else(|| format!("unknown view `{v}`"))?, - }; + match v { + // Fit keeps the orientation, also of a camera `view.apply` chose. + "fit" => { + let b = solidcraft_engine::view::bounds(s); + if let Some(c) = s.camera.as_mut() { + c.fit(&b); + } + } + _ => { + self.view = StandardView::parse(v).ok_or_else(|| format!("unknown view `{v}`"))?; + s.camera = None; + } + } Ok(json!({"view": self.view})) } "ui.render" => { let (w, h) = (dim(&p, "width", 1280.0), dim(&p, "height", 800.0)); - let mut cam = solidcraft_engine::view::home_camera(s); - cam.set_view(self.view); + // The camera `view.apply` chose, else the standard view framing the model. + let cam = s.camera.unwrap_or_else(|| { + let mut cam = solidcraft_engine::view::home_camera(s); + cam.set_view(self.view); + cam + }); let scene = solidcraft_engine::view::scene(s, &cam); let png = render_png(&scene, &cam, w, h).ok_or("render failed")?; let path = str_p("path").ok_or("missing `path`")?; diff --git a/crates/mcp/src/tests.rs b/crates/mcp/src/tests.rs index d1a3a86d..2f8cae85 100644 --- a/crates/mcp/src/tests.rs +++ b/crates/mcp/src/tests.rs @@ -313,7 +313,11 @@ fn unknown_tools_and_bad_arguments_are_clean_errors() { assert_eq!(raw_tool(&mut s, "execute", json!({}))["error"]["code"], -32602); assert_eq!(raw_tool(&mut s, "execute", json!({"command": 5}))["error"]["code"], -32602); assert_eq!(raw_tool(&mut s, "measure", json!({"bogus": 1}))["error"]["code"], -32602); - assert_eq!(raw_tool(&mut s, "screenshot", json!({"view": "sideways"}))["error"]["code"], -32602); + assert_eq!(raw_tool(&mut s, "screenshot", json!({"source": "sideways"}))["error"]["code"], -32602); + // `view` also takes saved named views, so an unknown name is a tool error naming it. + let r = raw_tool(&mut s, "screenshot", json!({"view": "sideways"})); + assert_eq!(r["result"]["isError"], true, "{r}"); + assert!(r["result"]["content"][0]["text"].as_str().unwrap().contains("sideways"), "{r}"); assert_eq!(call(&mut s, 1, "tools/call", json!({"arguments": {}}))["error"]["code"], -32602); // Engine errors are tool errors the model can read. let r = raw_tool(&mut s, "execute", json!({"command": "solid.extrude", "params": {"distance": 10}})); @@ -439,12 +443,11 @@ fn legacy_design_files_and_resource_uris() { /// model from that view after each change. The render must use the saved camera, not a /// standard view. #[test] -#[ignore = "fails until #528 is fixed"] fn screenshot_renders_a_saved_named_view() { let mut s = server(); tool(&mut s, "execute", json!({"command": "solid.box", "params": {"length": 40, "width": 20, "height": 10}})); let camera = json!({"target": [20, 10, 5], "yaw": 0.5, "pitch": -0.6, "distance": 150}); - tool(&mut s, "execute", json!({"command": "view.save", "params": {"name": "Photo", "camera": camera}})); + tool(&mut s, "execute", json!({"command": "view.save", "params": {"name": "Photo", "camera": camera.clone()}})); let shot = |s: &mut Server, view: &str| { let img = tool(s, "screenshot", json!({"view": view, "width": 96, "height": 64})); img["data"].as_str().unwrap_or_default().to_string() @@ -453,6 +456,10 @@ fn screenshot_renders_a_saved_named_view() { let photo = shot(&mut s, "Photo"); assert!(photo.starts_with("iVBORw0KGgo"), "not a PNG"); assert_ne!(photo, iso, "the named view rendered like the standard iso view"); + // The same camera given directly renders the same picture; a standard view turns back. + let direct = tool(&mut s, "screenshot", json!({"camera": camera, "width": 96, "height": 64})); + assert_eq!(direct["data"].as_str().unwrap_or_default(), photo); + assert_eq!(shot(&mut s, "iso"), iso); } /// #530: in a design with many bodies every feature result lists all of them, so a batch reply diff --git a/crates/mcp/src/tools.rs b/crates/mcp/src/tools.rs index 3a98e2dd..37f9c643 100644 --- a/crates/mcp/src/tools.rs +++ b/crates/mcp/src/tools.rs @@ -155,10 +155,13 @@ pub fn tool_definitions() -> Vec { "Render the design and return it as a PNG image. Headless: a CPU render of the model, fitted to the view. \ Connected to the app: a screenshot of the window (source \"window\", the default) or a render of the model \ with the app's camera (source \"model\"). Both show an active section (inspect.section) with its cut face \ - capped. `view` first turns the camera to a standard view.", + capped. `view` first turns the camera to a standard view or a saved named view (view.save), `camera` to any camera (as view.apply).", obj( json!({ - "view": {"type": "string", "enum": VIEWS, "description": "Standard view (default: keep the current one; headless starts at iso)"}, + "view": string( + "Standard view (iso, home, front, back, top, bottom, left, right, fit) or the name of a saved named view (default: keep the current one; headless starts at iso)" + ), + "camera": {"type": "object", "description": "Look from this camera instead: {target: [x,y,z], yaw, pitch, distance, fov?} (radians, mm; as view.save and view.apply)"}, "path": string("Also keep the PNG at this path"), "width": {"type": "integer", "description": "Render width in pixels (model renders; default 960)"}, "height": {"type": "integer", "description": "Render height in pixels (model renders; default 600)"}, @@ -513,12 +516,24 @@ fn screenshot(b: &mut dyn Backend, a: &Map) -> Value { } fn capture(b: &mut dyn Backend, a: &Map) -> Result<(Vec, Value), String> { - if let Some(v) = str_arg(a, "view") { - b.call("ui.view", json!({"view": v}))?; - // Turning to a standard view also frames the model (like a axis view click). - if v != "fit" && v != "home" { - b.call("ui.view", json!({"view": "fit"}))?; + let apply = |b: &mut dyn Backend, params: Value| b.call("engine.execute", json!({"command": "view.apply", "params": params})); + match (str_arg(a, "view"), a.get("camera").filter(|c| !c.is_null())) { + (Some(_), Some(_)) => return Err("screenshot: give `view` or `camera`, not both".into()), + (None, Some(c)) => { + apply(b, json!({"camera": c}))?; } + (Some(v), None) if VIEWS.contains(&v) => { + b.call("ui.view", json!({"view": v}))?; + // Turning to a standard view also frames the model (like a axis view click). + if v != "fit" && v != "home" { + b.call("ui.view", json!({"view": "fit"}))?; + } + } + // A saved named view. + (Some(v), None) => { + apply(b, json!({"view": v}))?; + } + (None, None) => {} } let keep = str_arg(a, "path").filter(|p| !p.trim().is_empty()).map(str::to_string); let path = keep.clone().unwrap_or_else(|| { diff --git a/crates/ui-egui/src/browser.rs b/crates/ui-egui/src/browser.rs index 64b1b360..77169d11 100644 --- a/crates/ui-egui/src/browser.rs +++ b/crates/ui-egui/src/browser.rs @@ -862,10 +862,8 @@ pub fn restore_view(app: &mut SolveApp, name: &str) { app.animate_view(v); return; } - let Some(v) = app.session.doc.named_views.iter().find(|v| v.name == name) else { return }; - let j = json!({ "target": v.target, "yaw": v.yaw, "pitch": v.pitch, "distance": v.distance, "fov": v.fov }); - if let Ok(cam) = serde_json::from_value::(j) { - app.animate_to(cam); + if app.run("view.apply", json!({ "view": name })).is_ok() { + app.follow_view_request(true); } } diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index 3ae4a7f7..b4475b8c 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -98,6 +98,8 @@ pub fn handle(app: &mut SolveApp, ctx: &egui::Context, req: &ControlRequest) -> if r.is_ok() && app.session.active_sketch.is_none() && app.session.world_state().bodies.len() == 1 && app.session.undo.len() <= 2 { app.fit_view(); } + // After the first-body fit, so a camera the command asked for (`view.apply`) wins. + app.follow_view_request(false); wrap(r) } "ui.agent_cursor" => { @@ -116,7 +118,11 @@ pub fn handle(app: &mut SolveApp, ctx: &egui::Context, req: &ControlRequest) -> } ok(json!(app.ui.agent_cursor)) } - "engine.script" => wrap(app.session.run_script(p).map(|v| json!(v)).map_err(|e| e.to_string())), + "engine.script" => { + let r = app.session.run_script(p).map(|v| json!(v)).map_err(|e| e.to_string()); + app.follow_view_request(false); + wrap(r) + } "engine.commands" => wrap(app.session.execute("engine.commands", &json!({})).map_err(|e| e.to_string())), "document.inspect" => wrap(app.session.execute("document.inspect", p).map_err(|e| e.to_string())), "ui.inspect" => ok(json!({ diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 4b89ee56..e3d9d3d6 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -884,6 +884,21 @@ impl SolveApp { self.pending_fit = request; } + /// Move the viewport to the camera a command asked for (`view.apply`): animated like a view + /// change, or at once (control requests, so a screenshot right after shows it). + pub fn follow_view_request(&mut self, animate: bool) { + let Some(mut to) = self.session.camera.take() else { return }; + to.up = self.cam.up; + if animate { + self.animate_to(to); + } else if to.is_valid() { + self.cancel_view_animation(); + self.viewport.orbit_snap = None; + self.viewport.orbit_free = None; + self.cam = to; + } + } + /// Stop any view animation where it is (the user took over the camera). pub fn cancel_view_animation(&mut self) { self.pending_fit = None; @@ -930,6 +945,7 @@ impl SolveApp { } self.step_view_animation(ctx); self.drain_control(ctx); + self.follow_view_request(true); agent_cursor::step(self, ctx); if !self.synthetic.is_empty() { ctx.request_repaint(); diff --git a/crates/ui-egui/tests/scenarios/view_apply_named_view.json b/crates/ui-egui/tests/scenarios/view_apply_named_view.json new file mode 100644 index 00000000..d95b6f8f --- /dev/null +++ b/crates/ui-egui/tests/scenarios/view_apply_named_view.json @@ -0,0 +1,10 @@ +[ + {"call": "engine.execute", "params": {"command": "solid.box", "params": {"length": 40, "width": 20, "height": 10}}}, + {"call": "engine.execute", "params": {"command": "solid.cylinder", "params": {"radius": 4, "height": 12, "base": [10, 10, 10]}}}, + {"call": "engine.execute", "params": {"command": "view.save", "params": {"name": "Photo", "camera": {"target": [20, 10, 8], "yaw": 2.3, "pitch": 0.35, "distance": 120}}}}, + {"call": "ui.view", "params": {"view": "top"}}, + {"shot": "view_apply_before_top"}, + {"call": "engine.execute", "params": {"command": "view.apply", "params": {"view": "Photo"}}}, + {"expect": {"camera_matches": "Photo"}}, + {"shot": "view_apply_after_photo"} +] diff --git a/docs/features.md b/docs/features.md index 8b1efd64..f4424318 100644 --- a/docs/features.md +++ b/docs/features.md @@ -490,7 +490,7 @@ SolidCraft's commands by area, generated by `cargo xtask features` from the comm |---|---|---| | Delete | `selection.delete` | – | -## `view.*` (4) +## `view.*` (5) | Command | Id | Toolbar | |---|---|---| @@ -498,3 +498,4 @@ SolidCraft's commands by area, generated by `cargo xtask features` from the comm | Rename Named View | `view.rename` | – | | Delete Named View | `view.delete` | – | | List Named Views | `view.list` | – | +| Restore Named View | `view.apply` | – | diff --git a/docs/mcp.md b/docs/mcp.md index 716dc6df..f2673117 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -56,7 +56,7 @@ Claude Desktop (`claude_desktop_config.json`), bridged to a running app: | `measure` | `body?` | Volume (mm³), area (mm²), centre of mass, bbox and topology counts per body, with totals. | | `body_topology` | `body` | Edges (index, midpoint, length, ends) and faces (index, area, centroid, normal). An edge midpoint is the `[x,y,z]` that picks that edge for fillet and chamfer. | | `set_parameter` | `name`, `value` (number or expression), `unit?`, `comment?` | Create or change a user parameter and recompute; reports `recomputed` (features rebuilt), `restored` (features whose earlier result for the same inputs was reused, e.g. a value set back) and features that now fail. | -| `screenshot` | `view?` (iso, top, front, back, bottom, left, right, home, fit), `path?`, `width?`, `height?`, `source?` (window, model) | A PNG image content block. Headless: CPU render fitted to the model. Bridged: the app window (default) or a model render with the app's camera. | +| `screenshot` | `view?` (iso, top, front, back, bottom, left, right, home, fit, or a named view saved with `view.save`), `camera?` (`{target, yaw, pitch, distance, fov?}`, as `view.apply`), `path?`, `width?`, `height?`, `source?` (window, model) | A PNG image content block. Headless: CPU render fitted to the model, or from the named view or camera. Bridged: the app window (default) or a model render with the app's camera; a named view or camera moves the app's viewport there first (`execute` `view.apply` does the same without a screenshot). | | `export` | `path`, `format?` (step, 3mf, stl, stla, obj), `bodies?` | STEP, 3MF or mesh export. | | `undo`, `redo` | — | Undo / redo one change. | | `crash_reports` | `id?`, `limit?` | The crash reports on this computer, newest first; with `id`, the whole report (backtrace, command, recent commands, `design_path`/`replay_path`). See [crash-reports.md](crash-reports.md). |