diff --git a/crates/mcp/src/tests.rs b/crates/mcp/src/tests.rs index bdb3c11b..dfa086c2 100644 --- a/crates/mcp/src/tests.rs +++ b/crates/mcp/src/tests.rs @@ -831,6 +831,59 @@ fn bridged_screenshots_with_grid_or_axes_are_model_renders() { let _ = std::fs::remove_dir_all(&dir); } +/// #1677: a hidden window gives no picture. With no `source`, a model render comes back and says +/// it is one (`fallback`); asked for the window, the tool says why there is none, at once. +#[test] +fn a_hidden_window_screenshot_says_so() { + struct Hidden { + calls: Vec, + } + impl crate::backend::Backend for Hidden { + fn call(&mut self, method: &str, params: Value) -> Result { + self.calls.push(method.to_string()); + match method { + "ui.screenshot" => Err("no frame was presented (window hidden?); use ui.render".into()), + "ui.render" => { + let path = params["path"].as_str().ok_or("missing `path`")?; + std::fs::write(path, b"\x89PNG").map_err(|e| e.to_string())?; + Ok(json!({"path": path, "width": 64, "height": 48})) + } + other => Err(format!("unknown method `{other}`")), + } + } + fn has_ui(&self) -> bool { + true + } + fn describe(&self) -> String { + "app".into() + } + } + let dir = std::env::temp_dir().join(format!("solidcraft-mcp-hidden-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let path = dir.join("shot.png").to_string_lossy().to_string(); + let shoot = |extra: Value| { + let mut args = json!({"path": path, "width": 64, "height": 48}); + args.as_object_mut().unwrap().extend(extra.as_object().unwrap().clone()); + let mut app = Hidden { calls: Vec::new() }; + let r = crate::tools::call_tool(&mut app, "screenshot", &args); + (app.calls, r) + }; + // The default: the window first, then a model render that says why. + let (calls, r) = shoot(json!({})); + assert!(r["isError"] != json!(true), "{r}"); + assert_eq!(calls, ["ui.screenshot", "ui.render"]); + let info: Value = serde_json::from_str(r["content"][1]["text"].as_str().unwrap()).unwrap(); + assert_eq!(info["source"], "model", "{info}"); + assert!(info["fallback"].as_str().is_some_and(|f| f.contains("window hidden")), "{info}"); + // Asked for the window: an error naming the cause and the way out, and no render. + let (calls, r) = shoot(json!({"source": "window"})); + assert_eq!(r["isError"], json!(true), "{r}"); + let text = r["content"][0]["text"].as_str().unwrap(); + assert!(text.contains("window hidden") && text.contains("source \"model\""), "{text}"); + assert_eq!(calls, ["ui.screenshot"]); + let _ = std::fs::remove_dir_all(&dir); +} + /// #530: in a design with many bodies every feature result lists all of them, so a batch reply /// is mostly the same body list once per step. A compact batch reply names only what each step /// added or changed. diff --git a/crates/mcp/src/tools.rs b/crates/mcp/src/tools.rs index 561ecc99..138361d0 100644 --- a/crates/mcp/src/tools.rs +++ b/crates/mcp/src/tools.rs @@ -639,11 +639,21 @@ fn capture(b: &mut dyn Backend, a: &Map) -> Result<(Vec, Valu } b.call("ui.render", params) }; + let asked_window = str_arg(a, "source") == Some("window"); + let mut fallback = None; let (info, source) = if window { match b.call("ui.screenshot", json!({"path": path})) { Ok(v) => (v, "window"), - // No presented frame (minimised or hidden window): fall back to a model render. - Err(_) => (render(b)?, "model"), + // Asked for the window: say why there is none, so the caller can show it or ask for + // a model render (#1677). + Err(e) if asked_window => { + return Err(format!("screenshot: the window gave no picture ({e}); show the window, or use source \"model\"")); + } + // No presented frame (minimised or hidden window): a model render, and the reply says so. + Err(e) => { + fallback = Some(e); + (render(b)?, "model") + } } } else { (render(b)?, "model") @@ -659,6 +669,9 @@ fn capture(b: &mut dyn Backend, a: &Map) -> Result<(Vec, Valu }; let bytes = png.len(); let mut reply = json!({"source": source, "path": keep, "width": w, "height": h, "bytes": bytes}); + if let Some(why) = fallback { + reply["fallback"] = json!(format!("the window gave no picture ({why}); this is a model render")); + } // What drew a model render (gpu, gpu-software or cpu), and the warning a CPU render carries. for k in ["renderer", "warning"] { if let Some(v) = info.get(k).filter(|v| !v.is_null()) { diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 60aec6bf..54d735f0 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -1445,6 +1445,11 @@ impl SolveApp { control::Outcome::Done(v) => { let _ = reply.send(v); } + // A window that is minimised or covered (another macOS Space) presents no frame: + // say so at once rather than after the deadline (#1677). + control::Outcome::Screenshot { .. } if window_hidden(ctx) => { + let _ = reply.send(json!({"ok": false, "hidden": true, "error": "no frame was presented (window hidden?); use ui.render"})); + } control::Outcome::Screenshot { path } => { self.shot_token += 1; let token = self.shot_token; @@ -1498,8 +1503,10 @@ impl SolveApp { } } let now = now_ms(); + // Hidden while waiting: no frame is coming. + let hidden = window_hidden(ctx); self.pending_shots.retain(|(_, _, reply, deadline)| { - if now < *deadline { + if now < *deadline && !hidden { return true; } let _ = reply.send(json!({"ok": false, "error": "no frame was presented (window hidden?); use ui.render"})); @@ -1508,6 +1515,12 @@ impl SolveApp { } } +/// The window is minimised or fully covered, so it presents no frames (#1677). Unknown counts as +/// shown: the screenshot deadline still ends a wait for a frame that never comes. +fn window_hidden(ctx: &egui::Context) -> bool { + ctx.input(|i| i.viewport().visible()) == Some(false) +} + /// A command's name as people see it (its label), for the status bar. pub fn command_label(id: &str) -> &str { solidcraft_engine::find_command(id).map_or(id, |c| c.label) diff --git a/docs/control-protocol.md b/docs/control-protocol.md index ebddf4c3..019701a4 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -144,7 +144,7 @@ labels; `prefs:measure_axis_distances` identifies the checkbox. | `ui.scroll` | `x`, `y`, `delta?` | wheel zoom at the cursor | | `ui.key` | `key` (egui key name, `Enter`, `Escape`…), `cmd?`, `shift?` | | | `ui.text` | `text` | typed text | -| `ui.screenshot` | `path?` | PNG of the window (needs a presented frame) | +| `ui.screenshot` | `path?` | PNG of the window (needs a presented frame: a minimised or fully covered window answers at once with `hidden: true` and an error; use `ui.render`) | | `ui.render` | `path`, `width?`, `height?`, `grid?`, `axes?` | Render of the model with the current camera (no window needed), drawn on the app's GPU device with the viewport's pipelines (the reply's `renderer`: `gpu`, or `gpu-software` on a software adapter), in the window's theme (light or dark; `solidcraft-cli snapshot` keeps the light colours); cut by the section the viewport shows (`inspect.section`, an open Section Analysis dialog's preview, a sketch's Slice), with the cut face capped. The ground grid and the origin's axes are drawn as the window shows them (`ui.set` `showGrid` and `showOrigin`, `hiddenOrigin`); `grid` and `axes` (booleans) say otherwise for this render | | `ui.confirm` | `accept?`: bool | answers the Delete confirmation (shown when a delete takes other features with it or makes some fail); without `accept`, what it lists | | `ui.documents` | `action?`: `new`, `switch`, `close`, `quit`; `index?`; `force?` (close without asking); `answer?`: `save`, `dont_save`, `cancel` (answers "save changes?") | the open designs (tabs): names, unsaved flags, the active one, a pending "save changes?" prompt, whether a quit is asking | diff --git a/docs/mcp.md b/docs/mcp.md index 4085cd84..3f44efa3 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -149,7 +149,7 @@ They cover the workflow, the command shapes, how to verify with `measure`, and t | `measure` | `body?` or `items?` | Volume (mm³), area (mm²), centre of mass, bbox and topology counts per body, with totals. With `items` (one or two selections as `execute` takes them: `face`, `edge`, `vertex`, `sketch_curve`, `profile`), the picked geometry instead: see below. | | `body_topology` | `body` | Edges (index, midpoint, length, ends) and faces (index, name, area, centroid, normal, `point`, and for a cylindrical face its `cylinder`: `origin`, `axis`, `radius`). An edge midpoint is the `[x,y,z]` that picks that edge for fillet and chamfer. A face's `point` lies on it and picks it (its centroid may not: that of a face with a hole is in the hole); a face's `name` picks it for the commands that take faces (`face.delete`, `solid.shell`, `solid.draft`, `solid.offset_face`, `solid.split_face`, `solid.replace_face`, and `face` of `sheet.convert`). | | `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, or a named view saved with `view.save`), `camera?` (`{target, yaw, pitch, distance, fov?}`, as `view.apply`), `path?`, `width?`, `height?`, `source?` (window, model), `grid?`, `axes?` | A PNG image content block. Headless: a render with the viewport's pipelines on a headless GPU device (see `solidcraft-cli gpu`; with no adapter, the CPU rasterizer, without analysis colours), fitted to the model, or from the named view or camera; the reply's `renderer` says which (`gpu`, `gpu-software`, `cpu`, with a `warning` for `cpu`). 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). `grid: false` and `axes: false` leave the ground grid (it lies on Z 0 and cuts through a part that straddles it) and the origin's axes out of a model render; axes hidden with `browser.visibility` stay out whatever `axes` says. Bridged, a screenshot that gives `grid` or `axes` is a model render, which follows the app's Grid and Origin switches for what it does not give; `source: "window"` with either is an error. | +| `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), `grid?`, `axes?` | A PNG image content block. Headless: a render with the viewport's pipelines on a headless GPU device (see `solidcraft-cli gpu`; with no adapter, the CPU rasterizer, without analysis colours), fitted to the model, or from the named view or camera; the reply's `renderer` says which (`gpu`, `gpu-software`, `cpu`, with a `warning` for `cpu`). 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). `grid: false` and `axes: false` leave the ground grid (it lies on Z 0 and cuts through a part that straddles it) and the origin's axes out of a model render; axes hidden with `browser.visibility` stay out whatever `axes` says. Bridged, a screenshot that gives `grid` or `axes` is a model render, which follows the app's Grid and Origin switches for what it does not give; `source: "window"` with either is an error. A window that is minimised or on another macOS Space presents no frame: with no `source` the reply is a model render and says so (`fallback`); `source: "window"` is an error saying why, at once. Agents that don't need the app's chrome can ask for `source: "model"`. | | `export` | `path`, `format?` (step, 3mf, 3mf-tagged, stl, stla, obj, urdf), `bodies?`, `mesh_prefix?`, `effort?`, `velocity?` | STEP, 3MF or mesh export; URDF writes the whole assembly with its meshes next to it (`docs/urdf.md`). | | `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). |