diff --git a/README.md b/README.md index 7125a573..073fa482 100644 --- a/README.md +++ b/README.md @@ -150,6 +150,7 @@ deckcraft-cli commands format. # list commands deckcraft-cli run --sample --cmd 'slide.new={"layout":"titleOnly","title":"Hello"}' --save hello.deckcraft deckcraft-cli mcp # MCP server over stdio (headless) deckcraft-cli mcp --connect 7990 # drive the running app (deckcraft --control 7990) +deckcraft-cli mcp --automation-read-root DIR --automation-write-root DIR # confine the headless agent to folders ``` ## Roadmap diff --git a/apps/deckcraft-cli/src/automation_args.rs b/apps/deckcraft-cli/src/automation_args.rs new file mode 100644 index 00000000..8f0cabf8 --- /dev/null +++ b/apps/deckcraft-cli/src/automation_args.rs @@ -0,0 +1,90 @@ +//! Parse `--automation-read-root` / `--automation-write-root` before the server starts. A malformed +//! flag must not silently remove a root and leave the server unconfined. + +use std::path::PathBuf; + +pub struct AutomationArgs { + pub read_root: Option, + pub write_root: Option, + read_given: bool, + write_given: bool, +} + +impl AutomationArgs { + pub fn new(read_root: Option, write_root: Option) -> Self { + Self { read_root, write_root, read_given: false, write_given: false } + } + + /// Returns whether this argument is a root flag. A second occurrence of a flag is an error, + /// including when the two spellings (`--flag DIR`, `--flag=DIR`) are mixed. + pub fn consume(&mut self, arg: &str, args: &mut impl Iterator) -> Result { + let (flag, inline) = arg.split_once('=').map_or((arg, None), |(flag, value)| (flag, Some(value))); + let (root, given) = match flag { + "--automation-read-root" => (&mut self.read_root, &mut self.read_given), + "--automation-write-root" => (&mut self.write_root, &mut self.write_given), + _ => return Ok(false), + }; + if *given { + return Err(format!("{flag} given twice")); + } + let value = match inline { + Some(value) => value.to_owned(), + None => { + let value = args.next().ok_or_else(|| format!("{flag} needs a folder"))?; + if value.starts_with('-') { + return Err(format!("{flag} needs a folder; use {flag}= for a folder starting with '-'")); + } + value + } + }; + if value.is_empty() { + return Err(format!("{flag} needs a non-empty folder")); + } + *root = Some(value.into()); + *given = true; + Ok(true) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn roots_are_taken_once_and_other_arguments_are_left() { + let mut roots = AutomationArgs::new(None, None); + let mut args = ["a folder".to_owned(), "deck.pptx".to_owned()].into_iter(); + assert!(roots.consume("--automation-read-root", &mut args).unwrap()); + assert!(roots.consume("--automation-write-root=out", &mut args).unwrap()); + assert!(!roots.consume("--sample", &mut args).unwrap()); + assert_eq!(roots.read_root, Some("a folder".into())); + assert_eq!(roots.write_root, Some("out".into())); + assert_eq!(args.next().as_deref(), Some("deck.pptx")); + assert!(roots.consume("--automation-read-root=other", &mut args).is_err()); + assert_eq!(roots.read_root, Some("a folder".into())); + } + + #[test] + fn invalid_operands_leave_roots_unchanged() { + for flag in ["--automation-read-root", "--automation-write-root"] { + for operand in [None, Some(""), Some("--version"), Some("--connect"), Some("-folder")] { + let mut roots = AutomationArgs::new(Some("read".into()), Some("write".into())); + let mut args = operand.map(str::to_owned).into_iter(); + assert!(roots.consume(flag, &mut args).is_err(), "{flag} {operand:?}"); + assert_eq!(roots.read_root, Some("read".into())); + assert_eq!(roots.write_root, Some("write".into())); + } + let mut roots = AutomationArgs::new(None, None); + assert!(roots.consume(&format!("{flag}="), &mut std::iter::empty()).is_err()); + } + } + + #[test] + fn explicit_dash_folder_and_relative_dash_folder_are_valid() { + let mut roots = AutomationArgs::new(None, None); + assert!(roots.consume("--automation-read-root=-folder", &mut std::iter::empty()).unwrap()); + assert!(roots.consume("--automation-write-root", &mut ["./-folder".to_owned()].into_iter()).unwrap()); + assert_eq!(roots.read_root, Some("-folder".into())); + assert_eq!(roots.write_root, Some("./-folder".into())); + } +} diff --git a/apps/deckcraft-cli/src/help.rs b/apps/deckcraft-cli/src/help.rs index d6158f46..535ce848 100644 --- a/apps/deckcraft-cli/src/help.rs +++ b/apps/deckcraft-cli/src/help.rs @@ -126,11 +126,16 @@ and prints its answer as JSON. COMMAND [PARAMS] runs a command, as run --cmd doe }, Subcommand { name: "mcp", - usage: "mcp [--connect PORT] [--sample]", + usage: "mcp [--connect PORT] [--sample] [--automation-read-root DIR] [--automation-write-root DIR]", summary: "serve the commands to an agent over MCP on stdio", details: "Runs a Model Context Protocol server on stdin and stdout. It works on its own presentation without a window, or with --connect PORT (or host:port) on the DeckCraft app started with --control PORT. ---sample opens the sample presentation.", +--sample opens the sample presentation. + +--automation-read-root DIR and --automation-write-root DIR confine every file the agent's commands read +(open, pictures, audio, video) and write (save, export, PDF) to those folders, links followed; a root left +out grants none of its access. They run the in-process server, so they conflict with --connect (a running +app is not confined by them).", }, Subcommand { name: "bench", diff --git a/apps/deckcraft-cli/src/main.rs b/apps/deckcraft-cli/src/main.rs index 57106da8..5934973e 100644 --- a/apps/deckcraft-cli/src/main.rs +++ b/apps/deckcraft-cli/src/main.rs @@ -2,6 +2,8 @@ #![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)] #![forbid(unsafe_code)] +mod automation_args; + mod help; use std::process::ExitCode; @@ -584,13 +586,18 @@ fn app(args: &[String]) -> Result<(), String> { } fn mcp(args: &[String]) -> Result<(), String> { + use deckcraft_engine::file_access::{self, AutomationRoots}; use deckcraft_mcp::{Backend, Headless, Remote, Server, control_addr}; let mut connect: Option = None; let mut sample = false; - let mut it = args.iter(); + let mut roots_args = automation_args::AutomationArgs::new(None, None); + let mut it = args.iter().cloned(); while let Some(a) = it.next() { + if roots_args.consume(&a, &mut it)? { + continue; + } match a.as_str() { - "--connect" => connect = Some(it.next().cloned().ok_or("--connect needs a port or host:port")?), + "--connect" => connect = Some(it.next().ok_or("--connect needs a port or host:port")?), "--sample" => sample = true, "--help" | "-h" => { outln!("{}", help_page("mcp")); @@ -599,13 +606,27 @@ fn mcp(args: &[String]) -> Result<(), String> { other => return Err(format!("unknown mcp option `{other}`")), } } + let roots = AutomationRoots::new(roots_args.read_root.as_deref(), roots_args.write_root.as_deref())?; + if roots.is_some() && connect.is_some() { + return Err("--automation-read-root and --automation-write-root confine the in-process server; a running app is not confined by them, \ +so drop --connect (or use --connect without the roots)" + .into()); + } let backend: Box = match connect { Some(c) => Box::new(Remote::connect(&control_addr(&c)).map_err(|e| format!("can't reach the app at {c}: {e}"))?), None => { let mut h = Headless::new(); + // `--sample` is the operator's own choice, made before the roots come into force. if sample { deckcraft_engine::sample::open_sample(&mut h.session).map_err(|e| e.to_string())?; } + if let Some(roots) = roots { + // Every thread of this process, not only the server's: nothing reaches past the roots. + file_access::confine_process(roots.clone()); + let show = |root: Option<&std::path::Path>| root.map_or_else(|| "none".to_string(), |r| r.display().to_string()); + eprintln!("deckcraft-cli: files confined: read root {}, write root {}", show(roots.read_root()), show(roots.write_root())); + h = h.with_automation_roots(Some(roots)); + } Box::new(h) } }; diff --git a/apps/deckcraft-cli/tests/mcp_roots.rs b/apps/deckcraft-cli/tests/mcp_roots.rs new file mode 100644 index 00000000..e9e1f856 --- /dev/null +++ b/apps/deckcraft-cli/tests/mcp_roots.rs @@ -0,0 +1,118 @@ +//! `deckcraft-cli mcp` with the automation root flags, run as the real binary. +// Integration tests: unwrapping and panicking on failure is fine here, unlike in shipped code (AGENTS.md). +#![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] + +use std::io::Write; +use std::path::PathBuf; +use std::process::{Command, Output, Stdio}; + +use serde_json::Value; + +fn bin() -> Command { + Command::new(env!("CARGO_BIN_EXE_deckcraft-cli")) +} + +fn run(args: &[&str]) -> Output { + bin().args(args).output().unwrap() +} + +fn tmpdir(tag: &str) -> PathBuf { + let dir = std::env::temp_dir().join(format!("dc-cli-{tag}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + dir +} + +/// Send `calls` (tool name, arguments) to `deckcraft-cli mcp `; returns whether it exited +/// cleanly, its stderr, and the stdout lines after `initialize`, one per call. +fn mcp_session(args: &[&str], calls: &[(&str, Value)]) -> (bool, String, Vec) { + let mut child = bin().arg("mcp").args(args).stdin(Stdio::piped()).stdout(Stdio::piped()).stderr(Stdio::piped()).spawn().unwrap(); + { + let stdin = child.stdin.as_mut().unwrap(); + writeln!(stdin, r#"{{"jsonrpc":"2.0","id":1,"method":"initialize","params":{{"protocolVersion":"2025-06-18"}}}}"#).unwrap(); + for (i, (name, arguments)) in calls.iter().enumerate() { + let call = serde_json::json!({"jsonrpc": "2.0", "id": i + 2, "method": "tools/call", "params": {"name": name, "arguments": arguments}}); + writeln!(stdin, "{call}").unwrap(); + } + } + let out = child.wait_with_output().unwrap(); + let stdout = String::from_utf8(out.stdout).unwrap(); + let replies = stdout.lines().skip(1).map(|l| serde_json::from_str(l).unwrap()).collect(); + (out.status.success(), String::from_utf8_lossy(&out.stderr).into_owned(), replies) +} + +fn text(reply: &Value) -> &str { + reply["result"]["content"][0]["text"].as_str().unwrap() +} + +#[test] +fn mcp_automation_roots_confine_the_server() { + let base = tmpdir("roots"); + let (work, elsewhere) = (base.join("work"), base.join("elsewhere")); + std::fs::create_dir_all(&work).unwrap(); + std::fs::create_dir_all(&elsewhere).unwrap(); + std::fs::write(work.join("in.txt"), "Inside text").unwrap(); + std::fs::write(elsewhere.join("secret.txt"), "TOP SECRET").unwrap(); + let work_s = work.to_str().unwrap(); + let secret = elsewhere.join("secret.txt").to_str().unwrap().to_string(); + + // Both spellings of the flags; inside works, outside is refused with a readable message. + let read_flag = format!("--automation-read-root={work_s}"); + let calls = [ + ("open_presentation", serde_json::json!({"path": work.join("in.txt").to_str().unwrap()})), + ("open_presentation", serde_json::json!({"path": secret})), + ("save_presentation", serde_json::json!({"path": work.join("saved.deckcraft").to_str().unwrap()})), + ("save_presentation", serde_json::json!({"path": elsewhere.join("saved.deckcraft").to_str().unwrap()})), + ]; + let (success, stderr, replies) = mcp_session(&[&read_flag, "--automation-write-root", work_s], &calls); + assert!(success, "{stderr}"); + assert!(stderr.contains("files confined"), "{stderr}"); + assert_eq!(replies[0]["result"]["isError"], false, "{}", replies[0]); + assert_eq!(replies[1]["result"]["isError"], true); + assert!(text(&replies[1]).contains("outside the read root"), "{}", text(&replies[1])); + assert_eq!(replies[2]["result"]["isError"], false, "{}", replies[2]); + assert!(work.join("saved.deckcraft").exists()); + assert_eq!(replies[3]["result"]["isError"], true); + assert!(text(&replies[3]).contains("outside the write root"), "{}", text(&replies[3])); + assert!(!elsewhere.join("saved.deckcraft").exists()); + + // A root left out grants none of its access. + let save = [("new_presentation", serde_json::json!({})), calls[2].clone()]; + let (_, _, replies) = mcp_session(&[&read_flag], &save); + assert_eq!(replies[1]["result"]["isError"], true); + assert!(text(&replies[1]).contains("write authority is absent"), "{}", text(&replies[1])); + + // No flags: nothing is confined. + let (success, _, replies) = mcp_session(&[], &calls[1..2]); + assert!(success); + assert_eq!(replies[0]["result"]["isError"], false, "{}", replies[0]); + let _ = std::fs::remove_dir_all(base); +} + +#[test] +fn mcp_automation_root_flags_are_checked_up_front() { + let base = tmpdir("roots-args"); + let dir = base.to_str().unwrap(); + let missing = base.join("missing"); + let fails = |args: &[&str], needle: &str| { + let mut full = vec!["mcp"]; + full.extend_from_slice(args); + let o = run(&full); + assert!(!o.status.success(), "{args:?}"); + let err = String::from_utf8_lossy(&o.stderr).into_owned(); + assert!(err.contains(needle), "{args:?}: {err}"); + }; + // The flags confine the in-process server, so they cannot be combined with --connect. + fails(&["--connect", "7979", "--automation-read-root", dir], "--connect"); + fails(&["--automation-write-root", dir, "--connect", "7979"], "--connect"); + fails(&["--automation-read-root", dir, "--automation-read-root", dir], "given twice"); + fails(&["--automation-write-root", dir, "--automation-write-root", dir], "given twice"); + fails(&["--automation-read-root"], "needs a folder"); + fails(&["--automation-write-root", "--sample"], "needs a folder"); + fails(&["--automation-read-root", missing.to_str().unwrap()], "cannot open the automation read root"); + fails(&["--automation-write-root="], "needs a non-empty folder"); + // A file is not a folder. + std::fs::write(base.join("f.txt"), "x").unwrap(); + fails(&["--automation-write-root", base.join("f.txt").to_str().unwrap()], "cannot open the automation write root"); + let _ = std::fs::remove_dir_all(base); +} diff --git a/crates/anim/src/lib.rs b/crates/anim/src/lib.rs index 8522a774..cf8fb3ff 100644 --- a/crates/anim/src/lib.rs +++ b/crates/anim/src/lib.rs @@ -38,7 +38,7 @@ use serde::{Deserialize, Serialize}; pub use easing::ease; pub use interact::{Click, HitShape, Target}; pub use mesh::{MeshVertex, layer_mesh}; -pub use morph::{MorphFrame, MorphPath, MorphText, morph_frame, morph_pairs, morph_xfrm}; +pub use morph::{MorphFrame, MorphPath, MorphText, morph_crop, morph_frame, morph_pairs, morph_xfrm}; pub use path::{MotionPath, default_path}; pub use show::{LinkJump, ShowAction, ShowState, safe_url, show_order}; pub use timeline::{EffectTiming, Sequence, StepStamp, Timeline, TriggerPlay}; diff --git a/crates/anim/src/morph.rs b/crates/anim/src/morph.rs index a05f2d6c..a869ebdb 100644 --- a/crates/anim/src/morph.rs +++ b/crates/anim/src/morph.rs @@ -12,7 +12,9 @@ fn text_of(s: &Shape) -> String { /// Pairs of (shape on `a`, shape on `b`) that Morph animates from one to the other: first by /// `!!name` (forced match), then by exact name, then by the same kind and text, then by the same -/// kind and preset geometry. Each shape is used once; top-level shapes only (groups move whole). +/// kind and preset geometry if neither carries text. A text box with neither a name nor its text on +/// the other slide stays unpaired and fades in place, as in PowerPoint (#119). Each shape is used +/// once; top-level shapes only (groups move whole). pub fn morph_pairs(a: &Slide, b: &Slide) -> Vec<(ShapeId, ShapeId)> { let mut pairs: Vec<(ShapeId, ShapeId)> = Vec::new(); let mut used_a = vec![false; a.shapes.len()]; @@ -31,7 +33,8 @@ pub fn morph_pairs(a: &Slide, b: &Slide) -> Vec<(ShapeId, ShapeId)> { x.kind_name() == y.kind_name() && x.geom.preset_name().is_some() && x.geom.preset_name() == y.geom.preset_name() - && text_of(x).trim().is_empty() == text_of(y).trim().is_empty() + && text_of(x).trim().is_empty() + && text_of(y).trim().is_empty() }, ]; for pass in passes { @@ -173,9 +176,58 @@ fn reshaped(a: &Shape, b: &Shape) -> bool { a.geom != b.geom && outlined(a) && outlined(b) && Shape { geom: Geom::default(), ..content(a) } == Shape { geom: Geom::default(), ..content(b) } } +/// The content of a shape apart from what Morph interpolates, with a picture's crop left out. +fn uncropped(s: &Shape) -> Shape { + let mut c = content(s); + if let ShapeKind::Picture { fill } = &mut c.kind { + fill.crop = [0.0; 4]; + } + c +} + +fn crop_of(s: &Shape) -> [f64; 4] { + match &s.kind { + ShapeKind::Picture { fill } => fill.crop, + _ => [0.0; 4], + } +} + +/// Whether two pictures differ in their crop only (apart from box and fill), so the crop can morph. +fn recropped(a: &Shape, b: &Shape) -> bool { + matches!(a.kind, ShapeKind::Picture { .. }) && crop_of(a) != crop_of(b) && uncropped(a) == uncropped(b) +} + +/// The crop (left, top, right, bottom) of a picture morphing from box `xa` cropped by `ca` to box +/// `xb` cropped by `cb`, when its box is `x` at progress `t`: the whole image moves and scales +/// linearly between where it lies on either slide, and the crop is what of it `x` shows. Falls back +/// to blending the crops when a crop leaves nothing visible. +pub fn morph_crop(ca: [f64; 4], xa: Xfrm, cb: [f64; 4], xb: Xfrm, x: Xfrm, t: f64) -> [f64; 4] { + // Along one axis: (box start, box size, crop before, crop after) → the crops at `t`. + let axis = |(pa, sa, la, ha): (f64, f64, f64, f64), (pb, sb, lb, hb): (f64, f64, f64, f64), (p, s): (f64, f64)| { + let (va, vb) = (1.0 - la - ha, 1.0 - lb - hb); + if va <= 1e-6 || vb <= 1e-6 { + return (lerp(la, lb, t), lerp(ha, hb, t)); + } + // The whole image: its extent and start on either slide. + let (ea, eb) = (sa / va, sb / vb); + let e = lerp(ea, eb, t); + if !e.is_finite() || e.abs() <= 1e-9 { + return (lerp(la, lb, t), lerp(ha, hb, t)); + } + let o = lerp(pa - la * ea, pb - lb * eb, t); + let lo = (p - o) / e; + (lo, 1.0 - lo - s / e) + }; + let (l, r) = axis((xa.x, xa.w, ca[0], ca[2]), (xb.x, xb.w, cb[0], cb[2]), (x.x, x.w)); + let (tp, bt) = axis((xa.y, xa.h, ca[1], ca[3]), (xb.y, xb.h, cb[1], cb[3]), (x.y, x.h)); + let c = [l, tp, r, bt]; + if c.iter().all(|v| v.is_finite()) { c } else { cb } +} + /// Morph frame from slide `a` to slide `b` at progress `t` (0..1, eased here). Paired shapes /// ([`morph_pairs`]) move along [`morph_xfrm`]; when they differ only in solid fill colour the -/// colour blends, when only in geometry (and fill colour) the outline morphs ([`MorphFrame::paths`]), +/// colour blends, when pictures differ only in crop the crop morphs ([`morph_crop`]), when only +/// in geometry (and fill colour) the outline morphs ([`MorphFrame::paths`]), /// otherwise the new shape fades in under the fading old one on the same moving /// box; with the transition option `words` or `characters` their texts are listed in /// [`MorphFrame::text`] to morph word by word or character by character. Unpaired shapes of `a` fade out, unpaired shapes of `b` fade in, and shapes of `b` for @@ -223,8 +275,18 @@ pub fn morph_frame(pres: &Presentation, a: &Slide, b: &Slide, t: f64, hidden_b: (_, xb) => xb, }; let reshape = reshaped(sa, sb); - if reshape || content(sa) == content(sb) { + let recrop = recropped(sa, sb); + if reshape || recrop || content(sa) == content(sb) { let mut m = Shape { xfrm: x, ..sb.clone() }; + if recrop && let ShapeKind::Picture { fill } = &mut m.kind { + fill.crop = match (xa, xb, x) { + (Some(xa), Some(xb), Some(x)) => morph_crop(crop_of(sa), xa, crop_of(sb), xb, x, s), + _ => { + let (ca, cb) = (crop_of(sa), crop_of(sb)); + [lerp(ca[0], cb[0], s), lerp(ca[1], cb[1], s), lerp(ca[2], cb[2], s), lerp(ca[3], cb[3], s)] + } + }; + } if let (Some(fa), Some(fb)) = (solid_color(ca.as_ref(), sa), solid_color(cb.as_ref(), sb)) && fa != fb { diff --git a/crates/anim/src/tests.rs b/crates/anim/src/tests.rs index ab4d5146..ae68fcd6 100644 --- a/crates/anim/src/tests.rs +++ b/crates/anim/src/tests.rs @@ -529,6 +529,61 @@ fn morph_pairs_by_name_text_and_geometry() { assert!(morph_pairs(&Slide::default(), &b).is_empty()); } +#[test] +fn morph_pairs_leaves_unrelated_text_boxes_alone() { + // A text box that leaves and an unrelated one that arrives fade in place (#119). + let p = deckcraft_model::defaults::new_presentation(None); + let mut a = Slide::default(); + let mut b = Slide::default(); + let tb = |id, name, x, text| Shape { xfrm: Some(Xfrm::new(x, 100.0, 200.0, 100.0)), text: Some(TextBody::from_text(text)), ..shape(id, name) }; + a.shapes = vec![tb(1, "TextBox 1", 0.0, "Gone")]; + b.shapes = vec![tb(2, "TextBox 2", 400.0, "New")]; + assert!(morph_pairs(&a, &b).is_empty()); + let f = morph_frame(&p, &a, &b, 0.5, &|_| false); + let xs: Vec<(&str, f64)> = f.shapes.iter().map(|(s, _)| (s.name.as_str(), s.xfrm.map_or(f64::NAN, |x| x.x))).collect(); + assert_eq!(xs, [("TextBox 1", 0.0), ("TextBox 2", 400.0)]); + // Empty shapes of one kind and geometry still pair, and a named text box keeps its partner. + a.shapes.push(shape(3, "Rect A")); + b.shapes.push(shape(4, "Rect B")); + b.shapes[0].name = "TextBox 1".into(); + assert_eq!(morph_pairs(&a, &b), [(ShapeId(1), ShapeId(2)), (ShapeId(3), ShapeId(4))]); +} + +#[test] +fn morph_crop_moves_the_image_linearly() { + let bx = |x, w| Xfrm::new(x, 0.0, w, 100.0); + // Same box, the crop pans from the left half to the right half of a 200-wide image. + let c = morph_crop([0.0, 0.0, 0.5, 0.0], bx(0.0, 100.0), [0.5, 0.0, 0.0, 0.0], bx(0.0, 100.0), bx(0.0, 100.0), 0.5); + assert!(c.iter().zip([0.25, 0.0, 0.25, 0.0]).all(|(a, b)| (a - b).abs() < 1e-9), "{c:?}"); + // The box shrinks over an image that stays put: only the right crop grows. + let c = morph_crop([0.0; 4], bx(0.0, 100.0), [0.0, 0.0, 0.5, 0.0], bx(0.0, 50.0), bx(0.0, 75.0), 0.5); + assert!(c.iter().zip([0.0, 0.0, 0.25, 0.0]).all(|(a, b)| (a - b).abs() < 1e-9), "{c:?}"); + // Nothing visible on one side: the crops blend. + let c = morph_crop([0.5, 0.0, 0.5, 0.0], bx(0.0, 100.0), [0.0; 4], bx(0.0, 100.0), bx(0.0, 100.0), 0.5); + assert!(c.iter().all(|v| v.is_finite()) && (c[0] - 0.25).abs() < 1e-9, "{c:?}"); +} + +#[test] +fn morph_frame_morphs_a_picture_crop() { + // A picture recropped on the next slide stays one picture whose crop morphs (#118). + let p = deckcraft_model::defaults::new_presentation(None); + let pic = |id, crop| Shape { + kind: deckcraft_model::ShapeKind::Picture { fill: deckcraft_model::style::PictureFill { crop, ..Default::default() } }, + ..shape(id, "Picture 1") + }; + let mut a = Slide::default(); + let mut b = Slide::default(); + a.shapes = vec![pic(1, [0.0, 0.0, 0.5, 0.0])]; + b.shapes = vec![pic(2, [0.5, 0.0, 0.0, 0.0])]; + let f = morph_frame(&p, &a, &b, 0.5, &|_| false); + assert_eq!(f.shapes.len(), 1); + assert!(f.opacity.is_empty()); + let deckcraft_model::ShapeKind::Picture { fill } = &f.shapes[0].0.kind else { panic!("not a picture") }; + let want = 0.5 * f.mix; + assert!((fill.crop[0] - want).abs() < 1e-9 && (fill.crop[2] - (0.5 - want)).abs() < 1e-9, "{:?}", fill.crop); + assert_eq!(morph_frame(&p, &a, &b, 1.0, &|_| false).shapes[0].0.kind, b.shapes[0].kind); +} + #[test] fn morph_xfrm_interpolates() { let a = Xfrm { rot: 350.0, ..Xfrm::new(0.0, 0.0, 100.0, 100.0) }; diff --git a/crates/engine/src/cmd/design.rs b/crates/engine/src/cmd/design.rs index b79f94ca..e2051041 100644 --- a/crates/engine/src/cmd/design.rs +++ b/crates/engine/src/cmd/design.rs @@ -152,7 +152,7 @@ fn browse_theme(s: &mut Session, p: &Value) -> Result { #[cfg(not(target_arch = "wasm32"))] { let path = str_param(p, "path").ok_or_else(|| bad("design.browseTheme", "give `path` or `data`"))?; - std::fs::read(path).map_err(|e| crate::EngineError::Other(format!("{path}: {e}")))? + crate::io::read_file(path).map_err(crate::EngineError::Other)? } #[cfg(target_arch = "wasm32")] return Err(bad("design.browseTheme", "give `data`")); diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index 127ae31f..f96478c1 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -165,7 +165,7 @@ pub fn add_opened(s: &mut Session, name: &str, path: Option, p: Presenta #[cfg(not(target_arch = "wasm32"))] fn open(s: &mut Session, p: &Value) -> Result { let path = str_param(p, "path").ok_or_else(|| bad("file.open", "missing `path`"))?; - let bytes = std::fs::read(path).map_err(|e| EngineError::Other(format!("{path}: {e}")))?; + let bytes = crate::io::read_file(path).map_err(EngineError::Other)?; let (doc, damaged) = open_checked(path, &bytes)?; let native = deckcraft_format::sniff(&bytes) || deckcraft_pptx::sniff(&bytes); // A damaged file opens as an untitled repaired copy, so Save can't overwrite the original @@ -340,36 +340,8 @@ fn mark_saved(s: &mut Session, path: Option) -> Result<()> { Ok(()) } -#[cfg(not(target_arch = "wasm32"))] fn write_file(path: &str, bytes: &[u8]) -> Result<()> { - use std::io::Write; - use std::sync::atomic::{AtomicU64, Ordering}; - static COUNTER: AtomicU64 = AtomicU64::new(0); - let err = |e: std::io::Error| EngineError::Other(format!("{path}: {e}")); - // Write to a uniquely named temporary sibling, created exclusively (never following or reusing an - // existing file or symlink), then rename it over the destination so a failed save never truncates - // the old file and concurrent saves never share temporary bytes. - let mut attempts = 0; - let (tmp, mut file) = loop { - let tmp = format!("{path}.{}.{}.saving", std::process::id(), COUNTER.fetch_add(1, Ordering::Relaxed)); - match std::fs::OpenOptions::new().write(true).create_new(true).open(&tmp) { - Ok(f) => break (tmp, f), - Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists && attempts < 16 => attempts += 1, - Err(e) => return Err(err(e)), - } - }; - let written = file.write_all(bytes).and_then(|()| file.sync_all()); - drop(file); - let result = written.and_then(|()| std::fs::rename(&tmp, path)); - if let Err(e) = result { - let _ = std::fs::remove_file(&tmp); - return Err(err(e)); - } - Ok(()) -} -#[cfg(target_arch = "wasm32")] -fn write_file(_path: &str, _bytes: &[u8]) -> Result<()> { - Err(EngineError::Other("saving to a path is not available in the browser".into())) + crate::io::write_file(path, bytes).map_err(EngineError::Other) } fn save(s: &mut Session, p: &Value) -> Result { @@ -529,7 +501,7 @@ fn info(s: &mut Session, _p: &Value) -> Result { let st = s.doc()?; let doc = &st.doc; #[cfg(not(target_arch = "wasm32"))] - let bytes = st.path.as_ref().and_then(|p| std::fs::metadata(p).ok()).map(|m| m.len()); + let bytes = st.path.as_deref().and_then(crate::io::file_len); #[cfg(target_arch = "wasm32")] let bytes: Option = None; Ok(json!({ diff --git a/crates/engine/src/cmd/insert.rs b/crates/engine/src/cmd/insert.rs index ab9680b0..f2e54ed5 100644 --- a/crates/engine/src/cmd/insert.rs +++ b/crates/engine/src/cmd/insert.rs @@ -308,9 +308,8 @@ pub(crate) fn media_bytes(p: &Value, cmd: &str) -> Result<(String, Vec)> { let bytes = base64_decode(d).ok_or_else(|| bad(cmd, "invalid base64 `data`"))?; return Ok((str_param(p, "name").unwrap_or("media").to_string(), bytes)); } - #[cfg(not(target_arch = "wasm32"))] if let Some(path) = str_param(p, "path") { - let bytes = std::fs::read(path).map_err(|e| EngineError::Other(format!("{path}: {e}")))?; + let bytes = crate::io::read_file(path).map_err(EngineError::Other)?; let name = std::path::Path::new(path).file_name().map(|n| n.to_string_lossy().to_string()).unwrap_or_else(|| "media".into()); return Ok((name, bytes)); } diff --git a/crates/engine/src/cmd/review.rs b/crates/engine/src/cmd/review.rs index 4358a004..50eb4c5c 100644 --- a/crates/engine/src/cmd/review.rs +++ b/crates/engine/src/cmd/review.rs @@ -639,7 +639,7 @@ fn revised_bytes(p: &Value) -> Result<(String, Vec)> { } #[cfg(not(target_arch = "wasm32"))] if let Some(path) = str_param(p, "path") { - let bytes = std::fs::read(path).map_err(|e| crate::EngineError::Other(format!("{path}: {e}")))?; + let bytes = crate::io::read_file(path).map_err(crate::EngineError::Other)?; return Ok((path.to_string(), bytes)); } Err(bad("review.compare", "give `path` or `data`")) diff --git a/crates/engine/src/cmd/text.rs b/crates/engine/src/cmd/text.rs index 6e743618..8cb05ca3 100644 --- a/crates/engine/src/cmd/text.rs +++ b/crates/engine/src/cmd/text.rs @@ -5,7 +5,7 @@ use std::sync::Arc; use deckcraft_geom::{Affine, Point, Rect}; use deckcraft_model::edit::{self as ed, Pos}; use deckcraft_model::resolve::{self, Ctx}; -use deckcraft_model::text::{AutoFit, Hyperlink, Run, RunKind, RunProps, TextBody}; +use deckcraft_model::text::{Align, AutoFit, Hyperlink, Paragraph, Run, RunKind, RunProps, TextBody}; use deckcraft_model::{Presentation, Shape, ShapeId, ShapeKind}; use serde_json::{Value, json}; @@ -259,6 +259,22 @@ pub fn caret_line(st: &DocState, t: &TextSel) -> Option<(deckcraft_geom::Point, Some((m * deckcraft_geom::Point::new(cx, top), m * deckcraft_geom::Point::new(cx, bot))) } +/// The colour of text typed at the caret. The canvas draws the caret in it, so the caret shows +/// wherever the text does: light on a dark slide, dark on a light one. `None` when that text +/// has no fill to take a colour from. +pub fn caret_color(st: &DocState, t: &TextSel) -> Option { + if t.notes || t.cell.is_some() { + return None; + } + let sh = st.shape(t.shape)?; + let body = sh.text.as_ref()?; + let ctx = ctx_for(&st.doc, &st.selection)?; + let blank = Paragraph::default(); + let para = body.paragraphs.get(t.caret.0).unwrap_or(&blank); + let props = resolve::run(&ctx, sh, para, &ed::props_at(body, t.caret)); + Some(resolve::text_color(&ctx, &props)).filter(|c| c.a > 0) +} + fn pos_param(p: &Value, key: &str) -> Option { let a = p.get(key)?.as_array()?; Some((a.first()?.as_u64()? as usize, a.get(1)?.as_u64()? as usize)) @@ -330,6 +346,7 @@ fn typed(s: &Session, body: &TextBody, at: Pos, text: &str) -> String { } pub(crate) fn insert_text(s: &mut Session, text: &str, props: Option) -> Result { + let inherited = inherited_aligns_for_edit(s); let text = { let st = s.doc()?; match &st.selection.text { @@ -347,12 +364,53 @@ pub(crate) fn insert_text(s: &mut Session, text: &str, props: Option) let c = ed::insert(body, at, &text, props); t.anchor = c; t.caret = c; + for pi in at.0..=c.0 { + if let Some(para) = body.paragraphs.get_mut(pi) { + promote_rtl_if_needed(para, &inherited); + } + } c })?; autocorrect(s, text.chars().count() == 1)?; Ok(json!({"caret": [pos.0, pos.1]})) } +/// Per outline level (0..=8), whether shape `id` on the current slide already resolves an +/// alignment for its paragraphs (its own list style, the layout's or the master's). All `false` +/// when the shape can't be resolved (notes, a shape that is gone). +fn inherited_aligns(s: &Session, id: ShapeId) -> [bool; 9] { + let mut out = [false; 9]; + let Ok(st) = s.doc() else { return out }; + let Some(sh) = st.shape(id) else { return out }; + let Some(ctx) = ctx_for(&st.doc, &st.selection) else { return out }; + for (lvl, set) in out.iter_mut().enumerate() { + let para = deckcraft_model::text::Paragraph { level: lvl as u8, ..Default::default() }; + *set = resolve::para(&ctx, sh, ¶).align.is_some(); + } + out +} + +/// The aligns [`inherited_aligns`] gives for the text being edited (none for notes). +fn inherited_aligns_for_edit(s: &Session) -> [bool; 9] { + match s.doc().ok().and_then(|st| st.selection.text.clone()) { + Some(t) if !t.notes => inherited_aligns(s, t.shape), + _ => [false; 9], + } +} + +/// A paragraph with no direction of its own whose first strong character is right-to-left +/// (Arabic, Hebrew, Kurdish…) becomes right-to-left, and right-aligned when nothing (the +/// paragraph or what it inherits, `inherited`) sets its alignment. +fn promote_rtl_if_needed(para: &mut deckcraft_model::text::Paragraph, inherited: &[bool; 9]) { + if para.props.rtl.is_none() && deckcraft_model::bidi::first_strong_is_rtl(¶.text()) { + para.props.rtl = Some(true); + let inherits_align = inherited.get(usize::from(para.level)).copied().unwrap_or(false); + if para.props.align.is_none() && !inherits_align { + para.props.align = Some(Align::Right); + } + } +} + /// Typing right after a hyperlink doesn't extend it (as in PowerPoint): at the end of a linked /// run, the props to type with are the run's without the link. `None` elsewhere. fn unlinked_at_link_end(body: &TextBody, at: Pos) -> Option { @@ -623,12 +681,18 @@ fn autocorrect_options(s: &mut Session, p: &Value) -> Result { } pub(crate) fn paste_body(s: &mut Session, src: &TextBody) -> Result { + let inherited = inherited_aligns_for_edit(s); let pos = edit_text(s, |body, t| { let (a, b) = t.ordered(); let at = ed::delete(body, a, b); let c = ed::insert_body(body, at, src); t.anchor = c; t.caret = c; + for pi in at.0..=c.0 { + if let Some(para) = body.paragraphs.get_mut(pi) { + promote_rtl_if_needed(para, &inherited); + } + } c })?; Ok(json!({"caret": [pos.0, pos.1]})) @@ -1062,6 +1126,7 @@ fn set(s: &mut Session, p: &Value) -> Result { None => s.doc()?.selection.shapes.first().copied().ok_or_else(|| bad("text.set", "missing `id` (or select a shape)"))?, }; let cell = p.get("cell").and_then(Value::as_array).and_then(|a| Some((a.first()?.as_u64()? as usize, a.get(1)?.as_u64()? as usize))); + let inherited = inherited_aligns(s, id); edit_shapes(s, &json!({"id": id}), "text.set", |sh| { let target = match (&mut sh.kind, cell) { (ShapeKind::Table(t), Some((r, c))) => t.cell_mut(r, c).map(|x| &mut x.text), @@ -1080,6 +1145,7 @@ fn set(s: &mut Session, p: &Value) -> Result { para.level = *lv; para.props = pp.clone(); } + promote_rtl_if_needed(para, &inherited); for r in &mut para.runs { r.props = keep.clone(); } @@ -1289,6 +1355,30 @@ fn replace_font(s: &mut Session, p: &Value) -> Result { Ok(json!({"replaced": n})) } +#[cfg(test)] +mod rtl_promote_tests { + use deckcraft_model::text::{Align, Paragraph, Run}; + + use super::promote_rtl_if_needed; + + fn para(text: &str) -> Paragraph { + Paragraph { runs: vec![Run::new(text)], ..Default::default() } + } + + #[test] + fn right_aligns_only_when_no_alignment_is_set_or_inherited() { + let mut p = para("مرحبا"); + promote_rtl_if_needed(&mut p, &[false; 9]); + assert_eq!((p.props.rtl, p.props.align), (Some(true), Some(Align::Right))); + let mut p = para("مرحبا"); + promote_rtl_if_needed(&mut p, &[true; 9]); + assert_eq!((p.props.rtl, p.props.align), (Some(true), None)); + let mut p = para("Hello مرحبا"); + promote_rtl_if_needed(&mut p, &[false; 9]); + assert_eq!((p.props.rtl, p.props.align), (None, None)); + } +} + #[cfg(test)] mod replace_font_tests { use serde_json::json; diff --git a/crates/engine/src/file_access.rs b/crates/engine/src/file_access.rs new file mode 100644 index 00000000..bcda8492 --- /dev/null +++ b/crates/engine/src/file_access.rs @@ -0,0 +1,483 @@ +//! The folders automation may read and write. +//! +//! `deckcraft-cli mcp --automation-read-root --automation-write-root ` confines the +//! files an agent's commands reach. [`AutomationRoots`] holds the two folders. While they are in force, every path a command +//! reads goes through [`check_read`] and every path it writes through [`check_write`]: +//! +//! - The path is made absolute against the working directory, as the file system reads it. Its +//! links are followed: for a file that doesn't exist yet, the links of its nearest existing +//! folder, the rest of the path being plain names (`..` below a missing folder is refused, and so +//! is a link that leads nowhere). +//! - The result must lie inside the root, compared by whole components (`/work2` isn't inside +//! `/work`), without regard to case on Windows. +//! - Read and write access are separate: a root given alone grants only its own kind of access. +//! +//! Roots are in force on a thread inside [`confine`], and in the whole process after +//! [`confine_process`] (`deckcraft-cli mcp`), except inside [`unconfined`]. Without roots +//! nothing is checked. +//! +//! A path is checked, then opened: a process that can change the folders in between (swapping in +//! a link) can still win that race. The roots keep an agent to the files it was given; they don't +//! guard against other programs on the computer. + +use std::cell::RefCell; +use std::ffi::OsString; +use std::path::{Component, Path, PathBuf}; +use std::sync::{Arc, OnceLock}; + +/// Read access or write access. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum Access { + Read, + Write, +} + +/// The folders automation may read and write; a root left out grants no access of its kind. +#[derive(Debug)] +pub struct AutomationRoots { + read: Option, + write: Option, +} + +#[derive(Debug)] +struct Root { + /// The folder made absolute, links followed: what paths are compared with. + resolved: PathBuf, + /// The same without Windows' `\\?\` prefix, for people. + shown: PathBuf, +} + +impl Root { + /// The folder at `path`, which must exist. + fn open(path: &Path, access: Access) -> Result { + let kind = match access { + Access::Read => "read", + Access::Write => "write", + }; + let resolved = std::fs::canonicalize(path) + .and_then(|p| if p.is_dir() { Ok(p) } else { Err(std::io::ErrorKind::NotADirectory.into()) }) + .map_err(|e| format!("cannot open the automation {kind} root `{}`: {e}", path.display()))?; + Ok(Self { shown: plain(resolved.clone()), resolved }) + } +} + +impl AutomationRoots { + /// The roots `read` and `write`, which must be existing folders; `None` when neither is given + /// (nothing is confined then). + pub fn new(read: Option<&Path>, write: Option<&Path>) -> Result>, String> { + if read.is_none() && write.is_none() { + return Ok(None); + } + let read = read.map(|p| Root::open(p, Access::Read)).transpose()?; + let write = write.map(|p| Root::open(p, Access::Write)).transpose()?; + Ok(Some(Arc::new(Self { read, write }))) + } + + /// The read root, absolute with its links followed. + pub fn read_root(&self) -> Option<&Path> { + self.read.as_ref().map(|r| r.shown.as_path()) + } + + /// The write root, absolute with its links followed. + pub fn write_root(&self) -> Option<&Path> { + self.write.as_ref().map(|r| r.shown.as_path()) + } + + /// May the file or folder at `path` be read (or written)? `Err` says why not. + pub fn check(&self, path: &str, access: Access) -> Result<(), String> { + let root = match access { + Access::Read => self.read.as_ref(), + Access::Write => self.write.as_ref(), + }; + let Some(Root { resolved: folder, shown }) = root else { + return match access { + Access::Read => Err(format!("automation filesystem access is not granted: read authority is absent: {path}")), + Access::Write => Err(format!("automation filesystem access is not granted: write authority is absent: {path}")), + }; + }; + if inside(&resolve(path)?, folder) { + return Ok(()); + } + let root = shown.display(); + match access { + Access::Read => Err(format!("automation path rejected: outside the read root {root}: {path}")), + Access::Write => Err(format!("automation path rejected: outside the write root {root}: {path}")), + } + } +} + +/// Where the roots are in force on a thread ([`confine`], [`unconfined`]). +#[derive(Clone, Debug)] +enum Scope { + Confined(Arc), + Unconfined, +} + +thread_local! { + /// This thread's scope; `None`: the process's roots ([`confine_process`]). + static SCOPE: RefCell> = const { RefCell::new(None) }; +} + +/// The roots in force in every thread outside [`confine`] and [`unconfined`]. +static PROCESS: OnceLock> = OnceLock::new(); + +/// Put `roots` in force in the whole process, for good. `false` when roots were in force already +/// (they stay as they were). +pub fn confine_process(roots: Arc) -> bool { + PROCESS.set(roots).is_ok() +} + +/// Puts the scope it replaced back when dropped (also when `f` panics). +struct Restore(Option); + +impl Drop for Restore { + fn drop(&mut self) { + let previous = self.0.take(); + SCOPE.with(|s| *s.borrow_mut() = previous); + } +} + +fn with_scope(scope: Option, f: impl FnOnce() -> T) -> T { + let _restore = Restore(SCOPE.with(|s| s.replace(scope))); + f() +} + +/// Run `f` with `roots` in force on this thread (`None`: as things stand). +pub fn confine(roots: Option<&Arc>, f: impl FnOnce() -> T) -> T { + match roots { + Some(r) => with_scope(Some(Scope::Confined(r.clone())), f), + None => f(), + } +} + +/// Run `f` with no roots in force on this thread: what the person at the keyboard does, and the +/// app's own folders. +pub fn unconfined(f: impl FnOnce() -> T) -> T { + with_scope(Some(Scope::Unconfined), f) +} + +/// The roots in force on this thread, if any. +pub fn current() -> Option> { + match SCOPE.with(|s| s.borrow().clone()) { + Some(Scope::Confined(r)) => Some(r), + Some(Scope::Unconfined) => None, + None => PROCESS.get().cloned(), + } +} + +/// `path` without Windows' `\\?\` prefix (which `canonicalize` adds), for people to read. +fn plain(path: PathBuf) -> PathBuf { + if !cfg!(windows) { + return path; + } + let text = path.to_string_lossy(); + if let Some(rest) = text.strip_prefix(r"\\?\UNC\") { + return PathBuf::from(format!(r"\\{rest}")); + } + match text.strip_prefix(r"\\?\") { + Some(rest) => PathBuf::from(rest), + None => path, + } +} + +/// May `path` be read here? Always, without roots in force. +pub fn check_read(path: &str) -> Result<(), String> { + current().map_or(Ok(()), |r| r.check(path, Access::Read)) +} + +/// May `path` be written here? Always, without roots in force. +pub fn check_write(path: &str) -> Result<(), String> { + current().map_or(Ok(()), |r| r.check(path, Access::Write)) +} + +/// `path` made absolute with its links followed; for a path that doesn't exist (yet), its nearest +/// existing folder's links followed and the rest appended. +fn resolve(path: &str) -> Result { + if path.is_empty() { + return Err("automation path rejected: path is empty".into()); + } + // Checked as given: making `…\NUL` absolute gives the device path `\\.\NUL` on Windows. + if cfg!(windows) && Path::new(path).components().any(|c| matches!(c, Component::Normal(n) if is_device_name(&n.to_string_lossy()))) { + return Err(format!("automation path rejected: reserved device names are not allowed: {path}")); + } + let absolute = std::path::absolute(path).map_err(|e| format!("automation path rejected: cannot resolve {path}: {e}"))?; + // The names below the nearest existing folder, last first. + let mut missing: Vec = vec![]; + let mut at = absolute.as_path(); + loop { + match std::fs::canonicalize(at) { + Ok(mut found) => { + found.extend(missing.iter().rev()); + return Ok(found); + } + Err(e) => match std::fs::symlink_metadata(at) { + Ok(m) if m.file_type().is_symlink() => return Err(format!("automation path rejected: a link that leads nowhere: {path}")), + // There, yet its real path can't be had (no permission): nothing to compare. + Ok(_) => return Err(format!("automation path rejected: cannot resolve {path}: {e}")), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} + Err(e) => return Err(format!("automation path rejected: cannot resolve {path}: {e}")), + }, + } + let parent = at.parent(); + match (at.components().next_back(), parent) { + (Some(Component::Normal(name)), Some(parent)) => { + missing.push(name.to_os_string()); + at = parent; + } + (Some(Component::ParentDir), _) => { + return Err(format!("automation path rejected: parent traversal below a folder that doesn't exist: {path}")); + } + // Down to the root (or drive) and it doesn't exist either. + _ => { + let e = std::io::Error::from(std::io::ErrorKind::NotFound); + return Err(format!("automation path rejected: cannot resolve {path}: {e}")); + } + } + } +} + +/// Is `path` (resolved) `root` or inside it, component by component (without regard to case on +/// Windows, whose file systems ignore it)? +fn inside(path: &Path, root: &Path) -> bool { + let mut path = path.components(); + root.components().all(|r| path.next().is_some_and(|p| same_component(p, r))) +} + +fn same_component(a: Component, b: Component) -> bool { + if a == b { + return true; + } + if !cfg!(windows) { + return false; + } + match (a.as_os_str().to_str(), b.as_os_str().to_str()) { + (Some(a), Some(b)) => a.to_lowercase() == b.to_lowercase(), + _ => false, + } +} + +/// A Windows device name (`CON`, `NUL`, `COM1`… also with an extension), which names no file. +fn is_device_name(component: &str) -> bool { + let stem = component.split('.').next().unwrap_or(component).trim_end_matches(' ').to_ascii_uppercase(); + let numbered = |prefix: &str| stem.strip_prefix(prefix).is_some_and(|n| matches!(n, "1" | "2" | "3" | "4" | "5" | "6" | "7" | "8" | "9")); + matches!(stem.as_str(), "CON" | "PRN" | "AUX" | "NUL" | "CLOCK$" | "CONIN$" | "CONOUT$") || numbered("COM") || numbered("LPT") +} + +#[cfg(test)] +mod tests { + use super::*; + + /// A fresh folder holding `in` (read root), `out` (write root) and `outside`. + fn folders(name: &str) -> (PathBuf, Arc) { + let base = std::env::temp_dir().join(format!("dc-file-access-{name}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&base); + for d in ["in/sub", "out", "outside", "in2"] { + std::fs::create_dir_all(base.join(d)).unwrap(); + } + std::fs::write(base.join("in/a.deckcraft"), "x").unwrap(); + std::fs::write(base.join("outside/secret.deckcraft"), "x").unwrap(); + let roots = AutomationRoots::new(Some(&base.join("in")), Some(&base.join("out"))).unwrap().unwrap(); + (base, roots) + } + + fn s(p: PathBuf) -> String { + p.to_string_lossy().into_owned() + } + + #[test] + fn paths_inside_the_roots_pass_and_others_dont() { + let (base, roots) = folders("inside"); + assert_eq!(roots.check(&s(base.join("in/a.deckcraft")), Access::Read), Ok(())); + assert_eq!(roots.check(&s(base.join("in")), Access::Read), Ok(()), "the root itself"); + assert_eq!(roots.check(&s(base.join("in/sub/new.deckcraft")), Access::Read), Ok(()), "missing files are checked by their folder"); + assert_eq!(roots.check(&s(base.join("out/new/deeper/x.png")), Access::Write), Ok(()), "and by their nearest folder"); + let e = roots.check(&s(base.join("outside/secret.deckcraft")), Access::Read).unwrap_err(); + assert!(e.starts_with("automation path rejected: outside the read root"), "{e}"); + // Read and write access are separate. + let e = roots.check(&s(base.join("in/a.deckcraft")), Access::Write).unwrap_err(); + assert!(e.contains("outside the write root"), "{e}"); + assert!(roots.check(&s(base.join("out/x.png")), Access::Read).unwrap_err().contains("outside the read root")); + let _ = std::fs::remove_dir_all(base); + } + + #[test] + fn a_sibling_with_the_same_prefix_is_outside() { + let (base, roots) = folders("prefix"); + std::fs::write(base.join("in2/b.deckcraft"), "x").unwrap(); + assert!(roots.check(&s(base.join("in2/b.deckcraft")), Access::Read).is_err(), "in2 is not inside in"); + assert!(roots.check(&format!("{}2/c.deckcraft", s(base.join("in"))), Access::Read).is_err()); + let _ = std::fs::remove_dir_all(base); + } + + #[test] + fn parent_traversal_is_resolved_or_refused() { + let (base, roots) = folders("dotdot"); + let sep = std::path::MAIN_SEPARATOR; + let up = format!("{}{sep}..{sep}outside{sep}secret.deckcraft", s(base.join("in"))); + assert!(roots.check(&up, Access::Read).unwrap_err().contains("outside the read root"), "`..` out of the root"); + let back = format!("{}{sep}..{sep}a.deckcraft", s(base.join("in/sub"))); + assert_eq!(roots.check(&back, Access::Read), Ok(()), "`..` that stays inside"); + let missing = format!("{}{sep}nope{sep}..{sep}..{sep}outside{sep}x.deckcraft", s(base.join("out"))); + let e = roots.check(&missing, Access::Write).unwrap_err(); + // Windows resolves `..` by name first (so does opening the file there): then it's outside. + assert!(e.contains("parent traversal below a folder that doesn't exist") || e.contains("outside the write root"), "{e}"); + let _ = std::fs::remove_dir_all(base); + } + + #[test] + fn relative_paths_resolve_against_the_working_directory() { + let (base, roots) = folders("relative"); + // The tests' working directory is the crate's folder: not inside the roots. + assert!(roots.check("a.deckcraft", Access::Read).unwrap_err().contains("outside the read root")); + assert!(roots.check("", Access::Read).unwrap_err().contains("path is empty")); + let _ = std::fs::remove_dir_all(base); + } + + #[test] + fn a_missing_root_is_an_error_and_none_is_no_confinement() { + let (base, _) = folders("missing"); + let e = AutomationRoots::new(Some(&base.join("nope")), None).unwrap_err(); + assert!(e.starts_with("cannot open the automation read root"), "{e}"); + let e = AutomationRoots::new(None, Some(&base.join("in/a.deckcraft"))).unwrap_err(); + assert!( + e.starts_with("cannot open the automation write root") + && e.ends_with(&std::io::Error::from(std::io::ErrorKind::NotADirectory).to_string()), + "{e}" + ); + assert!(AutomationRoots::new(Some(Path::new("")), None).unwrap_err().starts_with("cannot open the automation read root ``")); + assert!(AutomationRoots::new(None, None).unwrap().is_none()); + // One root alone grants only its own access. + let read_only = AutomationRoots::new(Some(&base.join("in")), None).unwrap().unwrap(); + let e = read_only.check(&s(base.join("in/x.deckcraft")), Access::Write).unwrap_err(); + assert!(e.contains("write authority is absent"), "{e}"); + let _ = std::fs::remove_dir_all(base); + } + + #[test] + fn links_are_followed_before_comparing() { + let (base, roots) = folders("links"); + #[cfg(unix)] + let made = |target: &Path, link: &Path| std::os::unix::fs::symlink(target, link).is_ok(); + #[cfg(windows)] + let made = |target: &Path, link: &Path| { + let made = + if target.is_dir() { std::os::windows::fs::symlink_dir(target, link) } else { std::os::windows::fs::symlink_file(target, link) }; + made.is_ok() + }; + #[cfg(not(any(unix, windows)))] + let made = |_: &Path, _: &Path| false; + // Windows needs Developer Mode or elevation to make links: nothing to test without them. + if !made(&base.join("outside/secret.deckcraft"), &base.join("in/leak.deckcraft")) { + eprintln!("skipped: symbolic links can't be made here"); + let _ = std::fs::remove_dir_all(base); + return; + } + assert!(roots.check(&s(base.join("in/leak.deckcraft")), Access::Read).unwrap_err().contains("outside the read root"), "a file link out"); + assert!(made(&base.join("outside"), &base.join("out/away"))); + let e = roots.check(&s(base.join("out/away/new.png")), Access::Write).unwrap_err(); + assert!(e.contains("outside the write root"), "a folder link out: {e}"); + assert!(made(&base.join("outside/gone.deckcraft"), &base.join("out/dangling.png"))); + let e = roots.check(&s(base.join("out/dangling.png")), Access::Write).unwrap_err(); + assert!(e.contains("a link that leads nowhere"), "writing would create the file outside: {e}"); + // A link to a folder inside the root stays inside. + assert!(made(&base.join("in/sub"), &base.join("in/alias"))); + assert_eq!(roots.check(&s(base.join("in/alias/x.deckcraft")), Access::Read), Ok(())); + // A root given through a link is the folder it leads to. + assert!(made(&base.join("in"), &base.join("in-link"))); + let linked = AutomationRoots::new(Some(&base.join("in-link")), None).unwrap().unwrap(); + assert_eq!(linked.check(&s(base.join("in/a.deckcraft")), Access::Read), Ok(())); + let _ = std::fs::remove_dir_all(base); + } + + #[cfg(windows)] + #[test] + fn windows_ignores_case_and_refuses_devices() { + let (base, roots) = folders("case"); + let upper = s(base.join("IN/a.deckcraft")); + assert_eq!(roots.check(&upper, Access::Read), Ok(()), "{upper}"); + assert_eq!(roots.check(&s(base.join("In/Sub/New.deckcraft")), Access::Read), Ok(())); + let e = roots.check(&s(base.join("out/NUL")), Access::Write).unwrap_err(); + assert!(e.contains("reserved device names"), "{e}"); + assert!(roots.check(&s(base.join("out/com1.txt")), Access::Write).is_err()); + assert_eq!(roots.check(&s(base.join("out/console.txt")), Access::Write), Ok(())); + let _ = std::fs::remove_dir_all(base); + } + + #[test] + fn scopes_nest_and_stay_on_their_thread() { + let (base, roots) = folders("scopes"); + let outside = s(base.join("outside/secret.deckcraft")); + assert_eq!(check_read(&outside), Ok(()), "nothing is confined by default"); + confine(Some(&roots), || { + assert!(check_read(&outside).is_err()); + assert_eq!(unconfined(|| check_read(&outside)), Ok(())); + assert!(check_read(&outside).is_err(), "restored after unconfined"); + let o = outside.clone(); + let there = std::thread::spawn(move || check_read(&o).is_ok()).join().unwrap(); + assert!(there, "another thread is not confined by this thread's scope"); + }); + assert_eq!(check_read(&outside), Ok(()), "restored after confine"); + let r = std::panic::catch_unwind(|| confine(Some(&roots), || panic!("boom"))); + assert!(r.is_err()); + assert_eq!(check_read(&outside), Ok(()), "restored after a panic"); + assert_eq!(confine(None, || check_read(&outside)), Ok(())); + let _ = std::fs::remove_dir_all(base); + } + + #[test] + fn device_names() { + for d in ["CON", "nul", "Com1", "LPT9.txt", "aux.deckcraft", "NUL "] { + assert!(is_device_name(d), "{d}"); + } + for f in ["console", "com10", "COM0", "lpt", "a.deckcraft", "nul-x"] { + assert!(!is_device_name(f), "{f}"); + } + } + + /// Every file a command reads or writes by path goes through `io` (`read_file`, + /// `write_file`), which check the roots. Engine code that reaches for `std::fs` itself would + /// bypass them, so none may: every file under `src/` (commands, `lib.rs`, `guard.rs`...) + /// except `io.rs`, this module, `recovery.rs` (the recovery folder is set by the host + /// application, never by a command's path, and a headless session has none) and the test + /// files. + #[test] + fn commands_do_not_touch_the_file_system_themselves() { + fn walk(dir: &Path, files: &mut Vec) { + for entry in std::fs::read_dir(dir).unwrap() { + let path = entry.unwrap().path(); + if path.is_dir() { + walk(&path, files); + } else if path.extension().is_some_and(|e| e == "rs") { + files.push(path); + } + } + } + let src = Path::new(env!("CARGO_MANIFEST_DIR")).join("src"); + let mut files = vec![]; + walk(&src, &mut files); + assert!(files.iter().any(|f| f.ends_with("lib.rs")) && files.iter().any(|f| f.ends_with("cmd/file.rs")), "{files:?}"); + let mut offenders = vec![]; + for path in files { + let name = path.file_name().and_then(|n| n.to_str()).unwrap_or(""); + // `io` and this module are the checked gateway; `tests*.rs` are test-only modules. + // `recovery` and `spell` touch only the app's own folders (AutoRecover files, the + // system and user spelling dictionaries), never a path a command names. + if path.parent() == Some(src.as_path()) && matches!(name, "io.rs" | "file_access.rs" | "recovery.rs" | "spell.rs") + || name.starts_with("tests") + { + continue; + } + let text = std::fs::read_to_string(&path).unwrap(); + // Test modules (which build their own fixtures) come last in each file. + let code = text.split("#[cfg(test)]").next().unwrap_or(""); + for (n, line) in code.lines().enumerate() { + let line = line.trim_start(); + let touches = ["std::fs", "File::open", "File::create", "OpenOptions"].iter().any(|w| line.contains(w)); + if !line.starts_with("//") && touches { + offenders.push(format!("{}:{}: {line}", path.display(), n + 1)); + } + } + } + assert!(offenders.is_empty(), "use crate::io::{{read_file, write_file}}: {offenders:#?}"); + } +} diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs new file mode 100644 index 00000000..c19efc0f --- /dev/null +++ b/crates/engine/src/io.rs @@ -0,0 +1,69 @@ +//! Reading and writing the files a command names by path. +//! +//! Every such read and write goes through [`read_file`] and [`write_file`], so the automation +//! roots ([`crate::file_access`]) are checked in one place. A command module that called +//! `std::fs` itself would bypass them; a test in `file_access` keeps that from happening. + +/// The bytes of the file at `path`, inside the automation read root when there is one. +#[cfg(not(target_arch = "wasm32"))] +pub fn read_file(path: &str) -> Result, String> { + crate::file_access::check_read(path)?; + std::fs::read(path).map_err(|e| format!("{path}: {e}")) +} + +#[cfg(target_arch = "wasm32")] +pub fn read_file(path: &str) -> Result, String> { + Err(format!("{path}: files are opened through the browser on the web")) +} + +/// Write `bytes` to the file at `path`, inside the automation write root when there is one. +/// +/// The bytes go to a temporary sibling first and are renamed over `path`, so a failed save never +/// truncates the old file. The sibling is in the same folder, so it is inside the root whenever +/// `path` is. +#[cfg(not(target_arch = "wasm32"))] +pub fn write_file(path: &str, bytes: &[u8]) -> Result<(), String> { + use std::io::Write; + use std::sync::atomic::{AtomicU64, Ordering}; + static COUNTER: AtomicU64 = AtomicU64::new(0); + crate::file_access::check_write(path)?; + let err = |e: std::io::Error| format!("{path}: {e}"); + // A uniquely named temporary sibling, created exclusively (never following or reusing an + // existing file or symlink), is renamed over the destination: concurrent saves never share + // temporary bytes. + let mut attempts = 0; + let (tmp, mut file) = loop { + let tmp = format!("{path}.{}.{}.saving", std::process::id(), COUNTER.fetch_add(1, Ordering::Relaxed)); + match std::fs::OpenOptions::new().write(true).create_new(true).open(&tmp) { + Ok(f) => break (tmp, f), + Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists && attempts < 16 => attempts += 1, + Err(e) => return Err(err(e)), + } + }; + let written = file.write_all(bytes).and_then(|()| file.sync_all()); + drop(file); + let result = written.and_then(|()| std::fs::rename(&tmp, path)); + if let Err(e) = result { + let _ = std::fs::remove_file(&tmp); + return Err(err(e)); + } + Ok(()) +} + +#[cfg(target_arch = "wasm32")] +pub fn write_file(path: &str, _bytes: &[u8]) -> Result<(), String> { + Err(format!("{path}: saving to a path is not available in the browser")) +} + +/// The size in bytes of the file at `path`; `None` when it can't be read (or is outside the +/// automation read root, so File › Info doesn't reveal files the agent may not read). +#[cfg(not(target_arch = "wasm32"))] +pub fn file_len(path: &str) -> Option { + crate::file_access::check_read(path).ok()?; + std::fs::metadata(path).ok().map(|m| m.len()) +} + +#[cfg(target_arch = "wasm32")] +pub fn file_len(_path: &str) -> Option { + None +} diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 9a85a94c..340aed02 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -11,11 +11,13 @@ pub mod cmd; pub mod compare; pub mod connect; pub mod drawtable; +pub mod file_access; pub mod fonts; pub mod gif_export; pub mod guard; pub mod icons; pub mod inkshape; +pub mod io; pub mod keys; pub mod links; pub mod recovery; diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index bb93bb67..4bb0a6d9 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -867,6 +867,8 @@ fn right_to_left_paragraphs() { let mut s = session(); let title = s.doc().unwrap().current_slide().unwrap().shapes[0].id.0; s.execute("text.edit", &json!({"id": title})).unwrap(); + // Explicitly left-to-right first, so typing Arabic doesn't already turn it right-to-left (#169). + s.execute("format.rtl", &json!({"on": false})).unwrap(); s.execute("text.insert", &json!({"text": "سنة 2024 (Retrait) فقط"})).unwrap(); s.execute("format.rtl", &json!({})).unwrap(); let para = |s: &Session| s.doc().unwrap().current_slide().unwrap().shapes[0].text.as_ref().unwrap().paragraphs[0].props.clone(); @@ -887,6 +889,61 @@ fn right_to_left_paragraphs() { assert_eq!(para(&s).rtl, Some(true)); } +#[test] +fn typing_kurdish_or_arabic_auto_detects_rtl_and_advances_caret_smoothly() { + let mut s = session(); + let title = s.doc().unwrap().current_slide().unwrap().shapes[0].id.0; + s.execute("text.edit", &json!({"id": title})).unwrap(); + + let mut first_x = None; + let mut last_x = 0.0; + + // Type Kurdish sentence character-by-character: "من ناوم هێڤاره" + for ch in ["م", "ن", " ", "ن", "ا", "و", "م", " ", "ه", "ێ", "ڤ", "ا", "ر", "ه"] { + s.execute("text.insert", &json!({"text": ch})).unwrap(); + let st = s.doc().unwrap(); + let t = st.selection.text.as_ref().unwrap(); + let l = super::cmd::text::layout_for(st, t).unwrap(); + let (cx, ..) = l.caret(deckcraft_text::Pos { para: t.caret.0, ch: t.caret.1 }).unwrap(); + + if first_x.is_none() { + first_x = Some(cx); + } + last_x = cx; + + // The title is centred: the caret must NOT jump back to the left margin (~7.2 pt). + assert!(cx > 100.0, "caret should not jump to left margin: {cx}"); + } + + // Caret advances leftwards overall as text is appended + assert!(last_x < first_x.unwrap(), "caret should advance leftwards: last={last_x} first={:?}", first_x); + let para = s.doc().unwrap().current_slide().unwrap().shapes[0].text.as_ref().unwrap().paragraphs[0].props.clone(); + assert_eq!(para.rtl, Some(true), "auto-detected as RTL"); + assert_eq!(para.align, None, "the title keeps the alignment it inherits"); + + // A text box inherits the deck's default (left) alignment, which it keeps too. + s.execute("text.exit", &json!({})).ok(); + let id = s.execute("insert.textBox", &json!({"rect": [10, 10, 300, 100], "text": ""})).unwrap()["id"].as_u64().unwrap(); + s.execute("text.edit", &json!({"id": id})).unwrap(); + s.execute("text.insert", &json!({"text": "مرحبا"})).unwrap(); + let st = s.doc().unwrap(); + let sh = st.shape(deckcraft_model::ShapeId(id as u32)).unwrap(); + let para = sh.text.as_ref().unwrap().paragraphs[0].props.clone(); + assert_eq!(para.rtl, Some(true)); + assert_eq!(para.align, None, "the text box keeps the alignment it inherits"); +} + +#[test] +fn explicit_ltr_override_is_preserved_when_typing_rtl() { + let mut s = session(); + let title = s.doc().unwrap().current_slide().unwrap().shapes[0].id.0; + s.execute("text.edit", &json!({"id": title})).unwrap(); + s.execute("format.rtl", &json!({"on": false})).unwrap(); + s.execute("text.insert", &json!({"text": "مرحبا"})).unwrap(); + let para = s.doc().unwrap().current_slide().unwrap().shapes[0].text.as_ref().unwrap().paragraphs[0].props.clone(); + assert_eq!(para.rtl, Some(false), "user explicit LTR is preserved"); +} + #[test] fn header_footer_fixed_date_is_the_slides_own_text() { use deckcraft_model::text::RunKind; @@ -3117,6 +3174,37 @@ fn language_marks_text_and_no_proof_and_repeats_are_reported() { assert!(s.execute("format.language", &json!({"lang": "en US!"})).is_err()); } +#[test] +fn the_caret_takes_the_colour_of_the_text() { + let caret = |theme: &str| { + let mut s = session(); + s.execute("design.theme", &json!({"name": theme})).unwrap(); + let title = s.doc().unwrap().current_slide().unwrap().shapes[0].id.0; + s.execute("text.edit", &json!({"id": title})).unwrap(); + let st = s.doc().unwrap(); + let empty = cmd::text::caret_color(st, &st.selection.text.clone().unwrap()).unwrap(); + s.execute("text.insert", &json!({"text": "Typed"})).unwrap(); + let st = s.doc().unwrap(); + assert_eq!(cmd::text::caret_color(st, &st.selection.text.clone().unwrap()), Some(empty)); + empty + }; + // Dark text on the light theme, light text on the dark one: never black on a dark slide. + let (light, dark) = (caret("DeckCraft"), caret("Nocturne")); + assert!(light.r < 0x60 && light.g < 0x60 && light.b < 0x60, "{light:?}"); + assert!(dark.r > 0xC0 && dark.g > 0xC0 && dark.b > 0xC0, "{dark:?}"); + // Text set to red gets a red caret. + let mut s = session(); + let title = s.doc().unwrap().current_slide().unwrap().shapes[0].id.0; + s.execute("text.edit", &json!({"id": title})).unwrap(); + s.execute("text.insert", &json!({"text": "Red"})).unwrap(); + s.execute("text.selectAll", &json!({})).unwrap(); + s.execute("format.color", &json!({"color": "#FF0000"})).unwrap(); + s.execute("text.move", &json!({"to": "end"})).unwrap(); + let st = s.doc().unwrap(); + let red = cmd::text::caret_color(st, &st.selection.text.clone().unwrap()).unwrap(); + assert_eq!((red.r, red.g, red.b), (0xFF, 0, 0)); +} + /// Smooth Corners (#286): a rounded rectangle becomes a squircle outline that survives PPTX. #[test] fn smooth_corners_makes_a_squircle_outline() { diff --git a/crates/mcp/src/headless.rs b/crates/mcp/src/headless.rs index acd819e5..bd1faf09 100644 --- a/crates/mcp/src/headless.rs +++ b/crates/mcp/src/headless.rs @@ -1,6 +1,9 @@ //! An in-process engine session that answers the engine-level control-channel methods itself. use deckcraft_engine::{Mods, PointerEvent, PointerKind, Session, ToolKind}; +use std::sync::Arc; + +use deckcraft_engine::file_access::{self, AutomationRoots}; use serde_json::{Value, json}; use crate::backend::Backend; @@ -8,6 +11,7 @@ use crate::backend::Backend; /// Headless backend: a [`Session`] rendering slides on the CPU. pub struct Headless { pub session: Session, + roots: Option>, } impl Default for Headless { @@ -59,12 +63,19 @@ pub fn tool_kind(name: &str, preset: Option<&str>) -> Option { impl Headless { pub fn new() -> Self { - Self { session: Session::new() } + Self { session: Session::new(), roots: None } } /// A session with a fresh presentation. pub fn with_document() -> Self { - Self { session: Session::with_new() } + Self { session: Session::with_new(), roots: None } + } + + /// Confine every file the session's commands read and write to `roots` + /// (`deckcraft-cli mcp --automation-read-root … --automation-write-root …`). + pub fn with_automation_roots(mut self, roots: Option>) -> Self { + self.roots = roots; + self } fn exec(&mut self, id: &str, params: &Value) -> Result { @@ -111,7 +122,7 @@ impl Headless { ); let png = img.to_png_at(deckcraft_engine::render::dpi_for_scale(scale)); if let Some(path) = s(p, "path") { - std::fs::write(path, &png).map_err(|e| format!("{path}: {e}"))?; + deckcraft_engine::io::write_file(path, &png)?; return Ok(json!({"path": path, "width": img.width, "height": img.height})); } Ok(json!({"png": deckcraft_engine::cmd::base64_encode(&png), "width": img.width, "height": img.height, "slide": i})) @@ -120,6 +131,19 @@ impl Headless { impl Backend for Headless { fn call(&mut self, method: &str, params: Value) -> Result { + let roots = self.roots.clone(); + file_access::confine(roots.as_ref(), || self.answer(method, params)) + } + fn has_ui(&self) -> bool { + false + } + fn describe(&self) -> String { + "headless (in-process engine; no window)".into() + } +} + +impl Headless { + fn answer(&mut self, method: &str, params: Value) -> Result { let p = ¶ms; match method { "engine.execute" | "command" | "ui.menu.invoke" => { @@ -153,10 +177,4 @@ impl Backend for Headless { other => Err(format!("unknown method `{other}`")), } } - fn has_ui(&self) -> bool { - false - } - fn describe(&self) -> String { - "headless (in-process engine; no window)".into() - } } diff --git a/crates/mcp/tests/automation_roots.rs b/crates/mcp/tests/automation_roots.rs new file mode 100644 index 00000000..ed67fb3e --- /dev/null +++ b/crates/mcp/tests/automation_roots.rs @@ -0,0 +1,225 @@ +//! `deckcraft-cli mcp --automation-read-root --automation-write-root `: through the MCP +//! protocol, files open and load only from inside the read root and are saved and exported only +//! inside the write root. Paths that leave a root (`..`, another folder, a sibling with the same +//! prefix, a link) are refused, and nothing is written outside. +// Integration tests: unwrapping and panicking on failure is fine here, unlike in shipped code (AGENTS.md). +#![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] + +use std::path::{Path, PathBuf}; + +use deckcraft_engine::file_access::AutomationRoots; +use deckcraft_mcp::{Headless, Server}; +use serde_json::{Value, json}; + +/// A 1x1 PNG, so `insert_picture` has something real to read. +const PNG: &[u8] = &[ + 0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0x00, 0x00, 0x00, 0x0d, 0x49, 0x48, 0x44, 0x52, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, + 0x08, 0x06, 0x00, 0x00, 0x00, 0x1f, 0x15, 0xc4, 0x89, 0x00, 0x00, 0x00, 0x0d, 0x49, 0x44, 0x41, 0x54, 0x78, 0x9c, 0x63, 0xf8, 0xcf, 0xc0, 0xf0, + 0x1f, 0x00, 0x05, 0x00, 0x01, 0xff, 0x89, 0x99, 0x3d, 0x1d, 0x00, 0x00, 0x00, 0x00, 0x49, 0x45, 0x4e, 0x44, 0xae, 0x42, 0x60, 0x82, +]; + +/// A fresh folder holding `in` (the read root, with `a.txt` and `pic.png`), `out` (the write root), +/// `outside` (with `secret.txt` and `pic.png`) and `in2`, and a server confined to `in` and `out` +/// (either may be left out). The server starts with a new presentation. +fn confined(tag: &str, read: bool, write: bool) -> (Server, PathBuf) { + let base = std::env::temp_dir().join(format!("dc-mcp-roots-{tag}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&base); + for d in ["in", "out", "outside", "in2"] { + std::fs::create_dir_all(base.join(d)).unwrap(); + } + std::fs::write(base.join("in/a.txt"), "Inside text").unwrap(); + std::fs::write(base.join("in/pic.png"), PNG).unwrap(); + std::fs::write(base.join("outside/secret.txt"), "TOP SECRET").unwrap(); + std::fs::write(base.join("outside/pic.png"), PNG).unwrap(); + std::fs::write(base.join("in2/c.txt"), "Sibling").unwrap(); + let (in_dir, out_dir) = (base.join("in"), base.join("out")); + let roots = AutomationRoots::new(read.then_some(in_dir.as_path()), write.then_some(out_dir.as_path())).unwrap(); + let mut server = Server::new(Box::new(Headless::with_document().with_automation_roots(roots))); + ok(&mut server, "new_presentation", json!({})); + (server, base) +} + +fn s(p: &Path) -> String { + p.to_string_lossy().into_owned() +} + +/// `tools/call` -> (isError, the first text block). +fn call(server: &mut Server, name: &str, args: Value) -> (bool, String) { + let line = json!({"jsonrpc": "2.0", "id": 1, "method": "tools/call", "params": {"name": name, "arguments": args}}).to_string(); + let reply: Value = serde_json::from_str(&server.handle_line(&line).unwrap()).unwrap(); + let result = &reply["result"]; + let text = result["content"].as_array().unwrap().iter().find_map(|c| c["text"].as_str()).unwrap_or_default().to_string(); + (result["isError"] == true, text) +} + +fn ok(server: &mut Server, name: &str, args: Value) -> String { + let (error, text) = call(server, name, args.clone()); + assert!(!error, "{name} {args}: {text}"); + text +} + +fn refused(server: &mut Server, name: &str, args: Value, why: &str) { + let (error, text) = call(server, name, args.clone()); + assert!(error && text.contains(why), "{name} {args} should be refused ({why}): {text}"); +} + +fn run(server: &mut Server, command: &str, params: Value) -> (bool, String) { + call(server, "run_command", json!({"command": command, "params": params})) +} + +/// Everything the open presentation says, as one string. +fn deck_text(server: &mut Server) -> String { + format!("{} {}", ok(server, "inspect_document", json!({})), ok(server, "inspect_slide", json!({}))) +} + +#[test] +fn presentations_open_only_from_inside_the_read_root() { + let (mut server, base) = confined("open", true, true); + let sep = std::path::MAIN_SEPARATOR; + ok(&mut server, "open_presentation", json!({"path": s(&base.join("in/a.txt"))})); + assert!(deck_text(&mut server).contains("Inside text")); + + refused(&mut server, "open_presentation", json!({"path": s(&base.join("outside/secret.txt"))}), "outside the read root"); + refused( + &mut server, + "open_presentation", + json!({"path": format!("{}{sep}..{sep}outside{sep}secret.txt", s(&base.join("in")))}), + "outside the read root", + ); + refused(&mut server, "open_presentation", json!({"path": s(&base.join("in2/c.txt"))}), "outside the read root"); + // The write root isn't readable: the two kinds of access are separate. + std::fs::write(base.join("out/d.txt"), "d").unwrap(); + refused(&mut server, "open_presentation", json!({"path": s(&base.join("out/d.txt"))}), "outside the read root"); + // The same through the engine command and the control-channel method. + let (error, text) = run(&mut server, "file.open", json!({"path": s(&base.join("outside/secret.txt"))})); + assert!(error && text.contains("outside the read root"), "{text}"); + refused(&mut server, "call_app", json!({"method": "app.open", "params": {"path": s(&base.join("outside/secret.txt"))}}), "outside the read root"); + // Nothing from the refused files reached the deck. + let text = deck_text(&mut server); + assert!(!text.contains("TOP SECRET") && !text.contains("Sibling"), "{text}"); + assert!(text.contains("Inside text"), "the refused opens left the deck it had: {text}"); + let _ = std::fs::remove_dir_all(base); +} + +#[test] +fn pictures_media_and_other_path_reads_are_confined() { + let (mut server, base) = confined("media", true, true); + ok(&mut server, "insert_picture", json!({"path": s(&base.join("in/pic.png"))})); + let before = ok(&mut server, "inspect_slide", json!({})); + refused(&mut server, "insert_picture", json!({"path": s(&base.join("outside/pic.png"))}), "outside the read root"); + for command in ["insert.audio", "insert.video", "insert.picture", "picture.change"] { + let (error, text) = run(&mut server, command, json!({"path": s(&base.join("outside/pic.png"))})); + assert!(error && text.contains("outside the read root"), "{command}: {text}"); + } + assert_eq!(ok(&mut server, "inspect_slide", json!({})), before, "refused reads change nothing"); + + // Media inside the root gets as far as being read: a refusal there would name the root. + for command in ["insert.audio", "insert.video"] { + let (_, text) = run(&mut server, command, json!({"path": s(&base.join("in/pic.png"))})); + assert!(!text.contains("automation"), "{command}: {text}"); + } + let _ = std::fs::remove_dir_all(base); +} + +#[test] +fn every_writing_command_is_refused_outside_the_write_root() { + let (mut server, base) = confined("write", true, true); + let outside = |name: &str| s(&base.join("outside").join(name)); + let inside = |name: &str| s(&base.join("out").join(name)); + ok(&mut server, "add_slide", json!({"title": "Second"})); + + refused(&mut server, "save_presentation", json!({"path": outside("x.deckcraft")}), "outside the write root"); + refused(&mut server, "export", json!({"path": outside("x.pptx")}), "outside the write root"); + refused(&mut server, "export", json!({"path": outside("x.pdf")}), "outside the write root"); + refused(&mut server, "export", json!({"path": outside("x.png")}), "outside the write root"); + refused(&mut server, "export", json!({"path": outside("x.txt")}), "outside the write root"); + refused(&mut server, "export", json!({"path": outside("all.png"), "all": true}), "outside the write root"); + refused(&mut server, "call_app", json!({"method": "ui.render", "params": {"path": outside("r.png")}}), "outside the write root"); + refused(&mut server, "call_app", json!({"method": "app.save", "params": {"path": outside("y.deckcraft")}}), "outside the write root"); + refused(&mut server, "call_app", json!({"method": "app.export", "params": {"path": outside("y.pdf")}}), "outside the write root"); + for command in ["file.save", "file.saveAs", "file.export"] { + let (error, text) = run(&mut server, command, json!({"path": outside("z.deckcraft")})); + assert!(error && text.contains("outside the write root"), "{command}: {text}"); + } + // A sibling folder with the same prefix, and `..` out of the root. + refused(&mut server, "save_presentation", json!({"path": s(&base.join("out2/x.deckcraft"))}), "outside the write root"); + let sep = std::path::MAIN_SEPARATOR; + refused( + &mut server, + "save_presentation", + json!({"path": format!("{}{sep}..{sep}outside{sep}x.deckcraft", s(&base.join("out")))}), + "outside the write root", + ); + // The read root isn't writable. + refused(&mut server, "save_presentation", json!({"path": s(&base.join("in/x.deckcraft"))}), "outside the write root"); + + let leftovers: Vec<_> = std::fs::read_dir(base.join("outside")).unwrap().map(|e| e.unwrap().file_name()).collect(); + assert_eq!(leftovers.len(), 2, "nothing was written beside secret.txt and pic.png: {leftovers:?}"); + assert!(!base.join("out2").exists() && !base.join("in/x.deckcraft").exists()); + + // Inside the write root all of it works. + ok(&mut server, "save_presentation", json!({"path": inside("x.deckcraft")})); + ok(&mut server, "export", json!({"path": inside("x.pptx")})); + ok(&mut server, "export", json!({"path": inside("x.pdf")})); + ok(&mut server, "export", json!({"path": inside("all.png"), "all": true})); + ok(&mut server, "call_app", json!({"method": "ui.render", "params": {"path": inside("r.png")}})); + for f in ["x.deckcraft", "x.pptx", "x.pdf", "all1.png", "all2.png", "r.png"] { + assert!(base.join("out").join(f).exists(), "{f}"); + } + let temporaries: Vec<_> = std::fs::read_dir(base.join("out")) + .unwrap() + .filter_map(|e| e.ok()) + .filter(|e| e.file_name().to_string_lossy().ends_with(".saving")) + .collect(); + assert!(temporaries.is_empty(), "the temporary file is renamed away"); + let _ = std::fs::remove_dir_all(base); +} + +#[test] +fn one_root_grants_only_its_own_access() { + let (mut read_only, base) = confined("read-only", true, false); + ok(&mut read_only, "open_presentation", json!({"path": s(&base.join("in/a.txt"))})); + refused(&mut read_only, "save_presentation", json!({"path": s(&base.join("out/x.deckcraft"))}), "write authority is absent"); + refused(&mut read_only, "export", json!({"path": s(&base.join("out/x.pdf"))}), "write authority is absent"); + refused(&mut read_only, "call_app", json!({"method": "ui.render", "params": {"path": s(&base.join("out/r.png"))}}), "write authority is absent"); + // Calls that return bytes instead of writing a path keep working. + let (error, _) = call(&mut read_only, "render_slide", json!({"scale": 0.25})); + assert!(!error); + assert!(std::fs::read_dir(base.join("out")).unwrap().next().is_none()); + let _ = std::fs::remove_dir_all(base); + + let (mut write_only, base) = confined("write-only", false, true); + refused(&mut write_only, "open_presentation", json!({"path": s(&base.join("in/a.txt"))}), "read authority is absent"); + refused(&mut write_only, "insert_picture", json!({"path": s(&base.join("in/pic.png"))}), "read authority is absent"); + ok(&mut write_only, "save_presentation", json!({"path": s(&base.join("out/x.deckcraft"))})); + assert!(base.join("out/x.deckcraft").exists()); + let _ = std::fs::remove_dir_all(base); +} + +#[cfg(unix)] +#[test] +fn links_out_of_a_root_are_refused() { + let (mut server, base) = confined("links", true, true); + std::os::unix::fs::symlink(base.join("outside/secret.txt"), base.join("in/leak.txt")).unwrap(); + std::os::unix::fs::symlink(base.join("outside"), base.join("out/away")).unwrap(); + std::os::unix::fs::symlink(base.join("outside/gone.deckcraft"), base.join("out/dangling.deckcraft")).unwrap(); + refused(&mut server, "open_presentation", json!({"path": s(&base.join("in/leak.txt"))}), "outside the read root"); + refused(&mut server, "save_presentation", json!({"path": s(&base.join("out/away/x.deckcraft"))}), "outside the write root"); + refused(&mut server, "save_presentation", json!({"path": s(&base.join("out/dangling.deckcraft"))}), "a link that leads nowhere"); + assert!(!base.join("outside/x.deckcraft").exists() && !base.join("outside/gone.deckcraft").exists()); + assert!(!deck_text(&mut server).contains("TOP SECRET")); + let _ = std::fs::remove_dir_all(base); +} + +#[test] +fn without_roots_nothing_is_confined() { + let base = std::env::temp_dir().join(format!("dc-mcp-roots-none-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&base); + std::fs::create_dir_all(&base).unwrap(); + std::fs::write(base.join("a.txt"), "Anywhere").unwrap(); + let mut server = Server::new(Box::new(Headless::with_document().with_automation_roots(AutomationRoots::new(None, None).unwrap()))); + ok(&mut server, "open_presentation", json!({"path": s(&base.join("a.txt"))})); + ok(&mut server, "save_presentation", json!({"path": s(&base.join("b.deckcraft"))})); + assert!(base.join("b.deckcraft").exists()); + let _ = std::fs::remove_dir_all(base); +} diff --git a/crates/model/src/bidi.rs b/crates/model/src/bidi.rs new file mode 100644 index 00000000..0b113522 --- /dev/null +++ b/crates/model/src/bidi.rs @@ -0,0 +1,81 @@ +//! Bidirectional text helpers (UAX #9). +//! +//! Follows Unicode Bidirectional Algorithm (UAX #9 rules P2 and P3) for paragraph +//! base direction auto-detection. + +/// Does the first strong directional character in `text` belong to a right-to-left script? +/// +/// Follows UAX #9 rules P2 and P3: characters with strong bidirectional class AL or R +/// (Arabic, Hebrew, Syriac, Thaana…) give `true`; strong L characters (Latin, Greek, +/// Cyrillic, CJK…) give `false`. Neutral, weak, and whitespace characters are skipped. +pub fn first_strong_is_rtl(text: &str) -> bool { + for c in text.chars() { + if c == '\u{200F}' { + // Right-to-Left Mark (RLM) + return true; + } + if c == '\u{200E}' { + // Left-to-Right Mark (LRM) + return false; + } + if c.is_alphabetic() { + return is_rtl_script(c); + } + } + false +} + +/// Helper checking if a char belongs to standard RTL Unicode blocks. +pub fn is_rtl_script(c: char) -> bool { + matches!( + c, + '\u{0590}'..='\u{05FF}' // Hebrew + | '\u{0600}'..='\u{06FF}' // Arabic + | '\u{0700}'..='\u{074F}' // Syriac + | '\u{0750}'..='\u{077F}' // Arabic Supplement + | '\u{0780}'..='\u{07BF}' // Thaana + | '\u{0870}'..='\u{089F}' // Arabic Extended-B + | '\u{08A0}'..='\u{08FF}' // Arabic Extended-A + | '\u{FB1D}'..='\u{FB4F}' // Hebrew Presentation Forms + | '\u{FB50}'..='\u{FDFF}' // Arabic Presentation Forms-A + | '\u{FE70}'..='\u{FEFE}' // Arabic Presentation Forms-B + ) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn detects_arabic_and_kurdish() { + assert!(first_strong_is_rtl("مرحبا")); + assert!(first_strong_is_rtl("من ناوم هێڤاره")); + assert!(first_strong_is_rtl("سلام")); + } + + #[test] + fn detects_hebrew() { + assert!(first_strong_is_rtl("שלום")); + } + + #[test] + fn detects_latin_as_not_rtl() { + assert!(!first_strong_is_rtl("Hello")); + assert!(!first_strong_is_rtl("DeckCraft")); + } + + #[test] + fn skips_neutrals_numbers_and_whitespace() { + assert!(first_strong_is_rtl(" 123 مرحبا")); + assert!(first_strong_is_rtl("... (2024) سنة")); + assert!(!first_strong_is_rtl(" 123 Hello")); + assert!(!first_strong_is_rtl("")); + assert!(!first_strong_is_rtl(" 12345 ")); + } + + #[test] + fn honours_directional_marks() { + assert!(first_strong_is_rtl("\u{200F}Hello")); + assert!(!first_strong_is_rtl("\u{200E}مرحبا")); + } +} diff --git a/crates/model/src/lib.rs b/crates/model/src/lib.rs index d12680cf..c62a5adb 100644 --- a/crates/model/src/lib.rs +++ b/crates/model/src/lib.rs @@ -8,6 +8,7 @@ #![forbid(unsafe_code)] pub mod anim; +pub mod bidi; pub mod chart; pub mod defaults; pub mod edit; @@ -27,6 +28,7 @@ pub use deckcraft_geom::{Rect, Size, Xfrm}; use serde::{Deserialize, Serialize}; pub use anim::{AnimClass, AnimStart, Animation, Transition}; +pub use bidi::{first_strong_is_rtl, is_rtl_script}; pub use chart::Chart; pub use style::{ColorBase, ColorRef, Effects, Fill, Line, ShapeStyle}; pub use table::{Cell, Table}; @@ -315,6 +317,10 @@ pub enum ShapeKind { xml: String, preview: Option, label: String, + /// The parts its XML points to (SmartArt data, an OLE embedding…), so a save writes them + /// back: (relationship id used in `xml`, relationship type, the part as a media item). + #[serde(default, skip_serializing_if = "Vec::is_empty")] + parts: Vec<(String, String, MediaId)>, }, } diff --git a/crates/pptx/src/opc.rs b/crates/pptx/src/opc.rs index bd090438..ab1f8a6f 100644 --- a/crates/pptx/src/opc.rs +++ b/crates/pptx/src/opc.rs @@ -133,7 +133,7 @@ impl Package { let (Some(id), Some(ty), Some(target)) = (r.attr("Id"), r.attr("Type"), r.attr("Target")) else { continue }; let external = r.attr("TargetMode").is_some_and(|m| m.eq_ignore_ascii_case("external")); let target = if external { target.to_string() } else { resolve(&base, target) }; - out.items.push(Rel { id: id.to_string(), kind: ty.rsplit('/').next().unwrap_or(ty).to_string(), target, external }); + out.items.push(Rel { id: id.to_string(), kind: ty.rsplit('/').next().unwrap_or(ty).to_string(), ty: ty.to_string(), target, external }); } out } @@ -162,6 +162,8 @@ pub struct Rel { pub id: String, /// Last segment of the relationship type (`slide`, `image`, `media`…). pub kind: String, + /// The whole relationship type URI, to write it back as it was. + pub ty: String, /// Absolute part name for internal targets; the URL for external ones. pub target: String, pub external: bool, diff --git a/crates/pptx/src/read/chart.rs b/crates/pptx/src/read/chart.rs index 49df55d1..bc2b7062 100644 --- a/crates/pptx/src/read/chart.rs +++ b/crates/pptx/src/read/chart.rs @@ -195,9 +195,8 @@ fn chart_within(root: &El, mut budget: usize) -> Option { "surfaceChart" | "surface3DChart" => ChartType::Surface, _ => continue, }; - if let Some(v) = g.child_val("varyColors") { - c.vary_colors = v == "1" || v == "true"; - } + // A missing means "vary by point" (the schema default is true), as PowerPoint draws it. + c.vary_colors = g.child_val("varyColors").is_none_or(|v| v == "1" || v == "true"); // A bar group without c:gapWidth / c:overlap takes the schema's defaults (150 %, 0 %), // not the 219 % / -27 % PowerPoint writes into its new charts. let bar_group = g.is("barChart") || g.is("bar3DChart"); @@ -287,6 +286,10 @@ fn chart_within(root: &El, mut budget: usize) -> Option { c.series.push(ser); } } + // `autoTitleDeleted val="0"` without `c:title`: PowerPoint and LibreOffice draw the series name as the title of a single-series chart. + if ch.child("title").is_none() && ch.child("autoTitleDeleted").and_then(|a| a.bool("val")) == Some(false) && c.series.len() == 1 { + c.title = c.series.first().map(|s| s.name.clone()).filter(|n| !n.is_empty()); + } let first = *kinds.first()?; c.kind = if kinds.iter().any(|k| *k != first) { ChartType::Combo } else { first }; if c.kind != ChartType::Combo { @@ -608,4 +611,52 @@ mod tests { assert_eq!((t[1].kind, t[1].order), (TrendlineType::Poly, Some(6))); assert_eq!((t[2].kind, t[2].period), (TrendlineType::MovingAvg, Some(3))); } + + const NS_C: &str = "http://schemas.openxmlformats.org/drawingml/2006/chart"; + const NS_A: &str = "http://schemas.openxmlformats.org/drawingml/2006/main"; + + fn chart(auto_deleted: &str, title: &str, series: usize) -> Chart { + let val = r#"1"#; + let ser: String = (0..series).map(|i| format!(r#"S{i}{val}"#)).collect(); + let plot = format!(r#"{ser}"#); + let xml = format!(r#"{title}{auto_deleted}{plot}"#); + read_chart_xml(xml.as_bytes()).unwrap() + } + + #[test] + fn single_series_gets_automatic_title() { + assert_eq!(chart(r#""#, "", 1).title.as_deref(), Some("S0")); + } + + #[test] + fn no_automatic_title_when_deleted_or_multi_series() { + assert_eq!(chart(r#""#, "", 1).title, None); + assert_eq!(chart("", "", 1).title, None); + assert_eq!(chart(r#""#, "", 2).title, None); + } + + #[test] + fn explicit_title_wins() { + let t = "Mine"; + assert_eq!(chart(r#""#, t, 1).title.as_deref(), Some("Mine")); + } + + const NS_CHART: &str = "http://schemas.openxmlformats.org/drawingml/2006/chart"; + + fn vary_xml(vary: &str) -> String { + let ser = r#""#; + let bar = format!(r#"{vary}{ser}"#); + format!(r#"{bar}"#) + } + + #[test] + fn missing_vary_colors_means_vary_by_point() { + assert!(read_chart_xml(vary_xml("").as_bytes()).is_some_and(|c| c.vary_colors)); + } + + #[test] + fn explicit_vary_colors_is_respected() { + assert!(read_chart_xml(vary_xml(r#""#).as_bytes()).is_some_and(|c| !c.vary_colors)); + assert!(read_chart_xml(vary_xml(r#""#).as_bytes()).is_some_and(|c| c.vary_colors)); + } } diff --git a/crates/pptx/src/read/mod.rs b/crates/pptx/src/read/mod.rs index 5f2ad912..ac83dfc0 100644 --- a/crates/pptx/src/read/mod.rs +++ b/crates/pptx/src/read/mod.rs @@ -740,19 +740,33 @@ fn sane_len(v: f64, dflt: f64) -> f64 { /// Does the element (or a descendant) reference a relationship? pub fn has_rel_refs(el: &El) -> bool { - fn rec(e: &El, depth: usize) -> bool { + !rel_refs(el).is_empty() +} + +/// Is this attribute a relationship id (`r:id`, `r:embed`, `r:dm`…)? +pub fn is_rel_attr(k: &str, v: &str) -> bool { + !k.starts_with("xmlns") + && k.contains(':') + && matches!(k.rsplit(':').next(), Some("id" | "embed" | "link" | "dm" | "lo" | "qs" | "cs" | "pict" | "href")) + && !v.is_empty() +} + +/// Every relationship id `el` and its descendants use, sorted and without repeats. +pub fn rel_refs(el: &El) -> Vec { + fn rec(e: &El, out: &mut Vec, depth: usize) { if depth > crate::xml::MAX_DEPTH { - return false; + return; + } + out.extend(e.attrs.iter().filter(|(k, v)| is_rel_attr(k, v)).map(|(_, v)| v.clone())); + for c in e.elements() { + rec(c, out, depth + 1); } - let own = e.attrs.iter().any(|(k, v)| { - !k.starts_with("xmlns") - && k.contains(':') - && matches!(k.rsplit(':').next(), Some("id" | "embed" | "link" | "dm" | "lo" | "qs" | "cs" | "pict" | "href")) - && !v.is_empty() - }); - own || e.elements().any(|c| rec(c, depth + 1)) } - rec(el, 0) + let mut out = vec![]; + rec(el, &mut out, 0); + out.sort(); + out.dedup(); + out } pub fn color_map(cm: &El) -> Vec<(String, String)> { diff --git a/crates/pptx/src/read/shapes.rs b/crates/pptx/src/read/shapes.rs index 738f2f48..1e88e023 100644 --- a/crates/pptx/src/read/shapes.rs +++ b/crates/pptx/src/read/shapes.rs @@ -449,7 +449,22 @@ fn frame(imp: &mut Imp, part: &Part, ctx: &mut IdCtx, e: &El, depth: usize) -> S } else { uri.rsplit('/').next().unwrap_or("Object").to_string() }; - s.kind = ShapeKind::Opaque { xml: part.keep(e), preview, label }; + // Keep the parts it points to so a save writes it back whole (#203). A part with relationships + // of its own, or an external link, can't be carried: the save then falls back to the preview. + let parts = super::rel_refs(e) + .into_iter() + .map(|id| { + let rel = part.rels.get(&id).filter(|r| !r.external)?; + if !imp.pkg.rels(&rel.target).items.is_empty() { + return None; + } + let (kind, target) = (rel.ty.clone(), rel.target.clone()); + let media = if rel.kind == "diagramData" { diagram_data(imp, &target) } else { imp.media_for(&target) }; + Some((id, kind, media?)) + }) + .collect::>>() + .unwrap_or_default(); + s.kind = ShapeKind::Opaque { xml: part.keep(e), preview, label, parts }; s } @@ -490,6 +505,34 @@ fn vml_index(pkg: &crate::opc::Package, vml: &str) -> std::collections::HashMap< out } +/// A diagram's data part, without the link to its cached drawing: that link is a slide +/// relationship id, which a save renumbers. Only diagrams whose drawing is missing or empty get +/// here, and those are drawn from their data. +fn diagram_data(imp: &mut Imp, target: &str) -> Option { + fn strip(e: &mut El, depth: usize) { + if depth > crate::xml::MAX_DEPTH { + return; + } + e.children.retain(|n| !matches!(n, crate::xml::Node::El(c) if c.is("ext") && c.find("dataModelExt").is_some())); + for c in e.elements_mut() { + strip(c, depth + 1); + } + } + let mut doc = imp.pkg.xml(target)?; + strip(&mut doc.root, 0); + let xml = format!("\n{}", doc.root.to_xml()); + let id = deckcraft_model::MediaId(imp.alloc()); + let content_type = imp.pkg.content_type(target)?; + imp.media.push(deckcraft_model::MediaItem { + id, + name: crate::opc::file_of(target).to_string(), + content_type, + data: std::sync::Arc::new(xml.into_bytes()), + link: None, + }); + Some(id) +} + /// SmartArt: read the pre-drawn shapes of its drawing part as a group. fn smartart(imp: &mut Imp, part: &Part, gd: &El, frame: &Shape, depth: usize) -> Option { let rel_ids = gd.child("relIds")?; diff --git a/crates/pptx/src/write/shapes.rs b/crates/pptx/src/write/shapes.rs index 770ee775..fab67be4 100644 --- a/crates/pptx/src/write/shapes.rs +++ b/crates/pptx/src/write/shapes.rs @@ -437,7 +437,7 @@ pub fn shape(w: &mut W, x: &mut Exp, o: &mut Out, cx: &mut Ctx, s: &Shape, depth } w.close("p:grpSp"); } - ShapeKind::Opaque { xml, preview, label } => opaque(w, x, o, cx, s, xml, *preview, label), + ShapeKind::Opaque { xml, preview, label, parts } => opaque(w, x, o, cx, s, xml, *preview, label, parts), } } @@ -832,13 +832,32 @@ fn media(w: &mut W, x: &mut Exp, o: &mut Out, id: u32, s: &Shape, m: &deckcraft_ } #[allow(clippy::too_many_arguments)] -fn opaque(w: &mut W, x: &mut Exp, o: &mut Out, cx: &mut Ctx, s: &Shape, xml: &str, preview: Option, label: &str) { - // Kept XML that references no relationships can be written back as it was. +fn opaque( + w: &mut W, + x: &mut Exp, + o: &mut Out, + cx: &mut Ctx, + s: &Shape, + xml: &str, + preview: Option, + label: &str, + parts: &[(String, String, deckcraft_model::MediaId)], +) { + // Kept XML is written back as it was when every part it references was kept too, with those + // parts re-added under this slide's relationship ids. if let Ok(doc) = crate::xml::parse(xml.as_bytes()) - && !crate::read::has_rel_refs(&doc.root) && matches!(doc.root.local(), "graphicFrame" | "sp" | "grpSp" | "cxnSp") + && let Some(ids) = crate::read::rel_refs(&doc.root) + .iter() + .map(|r| { + let (_, kind, m) = parts.iter().find(|(id, ..)| id == r)?; + Some((r.clone(), x.media_rel(o, *m, kind)?)) + }) + .collect::>>() { let mut root = doc.root; + set_rel_ids(&mut root, &ids, 0); + ole_fallback(&mut root, x, o, preview); let id = cx.id_for(s); set_cnvpr_id(&mut root, id); w.raw(&root.to_xml()); @@ -888,6 +907,52 @@ fn shape_with_id(w: &mut W, x: &mut Exp, o: &mut Out, cx: &mut Ctx, s: &Shape, i } } +/// A legacy OLE object finds its preview in the slide's VML drawing through `spid`, and PowerPoint +/// won't open the slide without it. We don't write that drawing, so write PowerPoint's own +/// fallback form instead: no `spid`, and the preview as a `p:pic`. +fn ole_fallback(root: &mut crate::xml::El, x: &mut Exp, o: &mut Out, preview: Option) { + let xfrm = root.child("xfrm").map(|t| { + let at = |e: &str, a: &str| t.child(e).and_then(|c| c.attr(a)).unwrap_or("0").to_string(); + (at("off", "x"), at("off", "y"), at("ext", "cx"), at("ext", "cy")) + }); + let Some(ole) = root + .elements_mut() + .find(|e| e.is("graphic")) + .and_then(|g| g.elements_mut().find(|e| e.is("graphicData"))) + .and_then(|d| d.elements_mut().find(|e| e.is("oleObj"))) + else { + return; + }; + if ole.child("pic").is_some() || ole.attr("spid").is_none() { + return; + } + ole.attrs.retain(|(k, _)| k != "spid"); + let rel = preview.and_then(|m| x.media_rel(o, m, "image")).unwrap_or_else(|| x.placeholder_rel(o)); + let (ox, oy, cx, cy) = xfrm.unwrap_or_default(); + let pic = format!( + r#""# + ); + if let Ok(d) = crate::xml::parse(pic.as_bytes()) { + ole.children.push(crate::xml::Node::El(d.root)); + } +} + +fn set_rel_ids(e: &mut crate::xml::El, ids: &std::collections::HashMap, depth: usize) { + if depth > crate::xml::MAX_DEPTH { + return; + } + for (k, v) in &mut e.attrs { + if crate::read::is_rel_attr(k, v) + && let Some(new) = ids.get(v.as_str()) + { + v.clone_from(new); + } + } + for c in e.elements_mut() { + set_rel_ids(c, ids, depth + 1); + } +} + fn set_cnvpr_id(el: &mut crate::xml::El, id: u32) { fn rec(e: &mut crate::xml::El, id: u32, depth: usize) -> bool { if depth > 8 { diff --git a/crates/pptx/tests/corpus_media.rs b/crates/pptx/tests/corpus_media.rs index 26a9539d..c03f16d3 100644 --- a/crates/pptx/tests/corpus_media.rs +++ b/crates/pptx/tests/corpus_media.rs @@ -63,9 +63,9 @@ fn corpus() -> (Presentation, Vec, Vec, Vec) { let mut objects = new_slide(&mut p, "Objects"); let mut diagram = shape(&mut p, "Diagram", 40.0, 80.0, 288.0, 144.0); - diagram.kind = ShapeKind::Opaque { xml: SMARTART.into(), preview: None, label: "SmartArt".into() }; + diagram.kind = ShapeKind::Opaque { xml: SMARTART.into(), preview: None, label: "SmartArt".into(), parts: vec![] }; let mut embedded = shape(&mut p, "Embedded", 340.0, 80.0, 144.0, 72.0); - embedded.kind = ShapeKind::Opaque { xml: OLE.into(), preview: None, label: "DeckCraft.Note.1".into() }; + embedded.kind = ShapeKind::Opaque { xml: OLE.into(), preview: None, label: "DeckCraft.Note.1".into(), parts: vec![] }; let mut block_shape = shape(&mut p, "Block", 40.0, 240.0, 120.0, 90.0); block_shape.kind = ShapeKind::Model3d { media: block_id, poster: None }; let mut globe_shape = shape(&mut p, "Globe", 180.0, 240.0, 160.0, 120.0); diff --git a/crates/pptx/tests/malformed.rs b/crates/pptx/tests/malformed.rs index e76e3201..633d339e 100644 --- a/crates/pptx/tests/malformed.rs +++ b/crates/pptx/tests/malformed.rs @@ -251,7 +251,7 @@ fn export_of_odd_models_is_valid() { } s.shapes.push(Shape { id: ShapeId(7), - kind: ShapeKind::Opaque { xml: "Manager"#.to_vec(), + ); + let mut parts = vec![("rId2".to_string(), "http://schemas.openxmlformats.org/officeDocument/2006/relationships/diagramData".to_string(), data)]; + for (i, kind) in ["Layout", "QuickStyle", "Colors"].into_iter().enumerate() { + let xml = format!(r#""#).into_bytes(); + let m = p.add_media(&format!("{kind}1.xml"), &format!("application/vnd.openxmlformats-officedocument.drawingml.diagram{kind}+xml"), xml); + parts.push((format!("rId{}", i + 3), format!("http://schemas.openxmlformats.org/officeDocument/2006/relationships/diagram{kind}"), m)); + } + let xml = r#""#; + let mut s = new_slide(&mut p, "SmartArt"); + let mut frame = shape(&mut p, "Diagram 3", 0.0, 0.0, 72.0, 72.0); + frame.kind = ShapeKind::Opaque { xml: xml.into(), preview: None, label: "SmartArt".into(), parts }; + s.shapes.push(frame); + p.slides.push(Arc::new(s)); + + // Twice: the second save starts from what the first one wrote. + let q = round(&round(&p)); + let ShapeKind::Opaque { parts, .. } = &by_name(&q.slides[1], "Diagram 3").kind else { panic!("not kept as an object") }; + assert_eq!(parts.len(), 4, "data, layout, style and colours all kept"); + let data = q.media(parts.iter().find(|(_, k, _)| k.ends_with("/diagramData")).expect("data").2).expect("media"); + let data = String::from_utf8_lossy(&data.data); + assert!(data.contains("Manager"), "{data}"); + assert!(!data.contains("dataModelExt"), "the link to a drawing that isn't written is dropped: {data}"); +} + +/// #203: a legacy OLE object (preview in the VML drawing we don't write) is saved in PowerPoint's +/// fallback form, without `spid` and with a `p:pic` preview, or PowerPoint won't open the file. +#[test] +fn legacy_ole_object_is_written_in_the_fallback_form() { + let mut p = defaults::new_presentation(None); + let bin = p.add_media("oleObject1.bin", "application/vnd.openxmlformats-officedocument.oleObject", b"\xD0\xCF\x11\xE0not-a-real-ole".to_vec()); + let prev = p.add_media("image1.png", "image/png", png(4, 3, [10, 20, 30])); + let xml = r#""#; + let parts = vec![("rId4".to_string(), "http://schemas.openxmlformats.org/officeDocument/2006/relationships/oleObject".to_string(), bin)]; + let mut s = new_slide(&mut p, "OLE"); + let mut frame = shape(&mut p, "Object 3", 0.0, 0.0, 72.0, 72.0); + frame.kind = ShapeKind::Opaque { xml: xml.into(), preview: Some(prev), label: "Slide".into(), parts }; + s.shapes.push(frame); + p.slides.push(Arc::new(s)); + + let q = round(&round(&p)); + let ShapeKind::Opaque { xml, parts, preview, .. } = &by_name(&q.slides[1], "Object 3").kind else { panic!("not kept as an object") }; + assert!(xml.contains("") && !xml.contains("spid"), "{xml}"); + assert!(preview.is_some(), "the preview picture is kept"); + assert!(parts.iter().any(|(_, k, _)| k.ends_with("/oleObject")), "the embedding is kept: {parts:?}"); +} diff --git a/crates/render/src/chart.rs b/crates/render/src/chart.rs index c373eaf4..ac185780 100644 --- a/crates/render/src/chart.rs +++ b/crates/render/src/chart.rs @@ -44,6 +44,18 @@ fn point_fill(ctx: &Ctx, c: &Chart, i: usize, s: &deckcraft_model::chart::Series } } +/// Fill of one bar: a `` fill wins, and a single-series chart that varies colours by point uses one accent per point +/// (not waterfalls, which are never read from a ``). +fn bar_fill(ctx: &Ctx, c: &Chart, si: usize, pi: usize, s: &deckcraft_model::chart::Series) -> Rgba { + if s.point_fills.iter().any(|(k, _)| *k == pi) { + return point_fill(ctx, c, si, s, pi); + } + if c.vary_colors && c.series.len() == 1 && s.fill.is_none() && c.kind != ChartType::Waterfall { + return series_color(ctx, c, pi); + } + series_fill(ctx, c, si, Some(s)) +} + fn polyline(pts: &[Point]) -> BezPath { let mut p = BezPath::new(); for (k, q) in pts.iter().enumerate() { @@ -623,7 +635,7 @@ fn draw_inner(ctx: &mut RenderContext, rctx: &Ctx, c: &Chart, m: Affine, w: f64, }; series_color(rctx, c, slot) } else { - point_fill(rctx, c, *si, s, i) + bar_fill(rctx, c, *si, i, s) }; ctx.set_transform(m); ctx.set_paint(color(fill, 1.0)); diff --git a/crates/render/src/lib.rs b/crates/render/src/lib.rs index 1618a89d..926bb762 100644 --- a/crates/render/src/lib.rs +++ b/crates/render/src/lib.rs @@ -309,11 +309,21 @@ pub struct Renderer { /// The fill of the group being drawn, with the transform and box it is laid out in: children /// filled with the group's fill (`a:grpFill`) show it, as one fill across the whole group. group_fill: Option<(Fill, Affine, Rect)>, + /// Their aligned outlines, kept from frame to frame. + morphs: morph_path::Cache, } impl Default for Renderer { fn default() -> Self { - Renderer { resources: Resources::new(), paths: Vec::new(), bg: None, drawn: None, view: Affine::IDENTITY, group_fill: None } + Renderer { + resources: Resources::new(), + paths: Vec::new(), + bg: None, + drawn: None, + view: Affine::IDENTITY, + group_fill: None, + morphs: morph_path::Cache::default(), + } } } @@ -733,6 +743,7 @@ impl Renderer { } } self.paths.clear(); + self.morphs.sweep(); self.finish(ctx, w, h) } @@ -982,7 +993,7 @@ impl Renderer { depth: usize, ) { let (geo, fades) = match self.paths.iter().find(|p| p.id == s.id) { - Some(pm) => morph_path::geometry(pm, s, w, h), + Some(pm) => self.morphs.geometry(pm, s, w, h), None => (shape_geometry(s, w, h), Vec::new()), }; let fade = |i: usize| fades.get(i).copied().unwrap_or(deckcraft_geom::morph::Fade { fill: 1.0, stroke: 1.0 }); @@ -1410,7 +1421,10 @@ impl Renderer { ctx.set_paint(color(c.with_alpha(255), 1.0)); ctx.set_stroke(kurbo::Stroke::new(g.radius.max(0.0) * 0.95).with_join(kurbo::Join::Round)); ctx.stroke_path(&silhouette); - ctx.fill_path(&silhouette); + // A shape without a fill (an outline, a frame) glows along its line only (#225). + if fill_alpha.is_some() || content.is_some() { + ctx.fill_path(&silhouette); + } ctx.pop_layer(); } if let Some(rf) = &e.reflection { diff --git a/crates/render/src/morph_path.rs b/crates/render/src/morph_path.rs index 6579eb18..6947053f 100644 --- a/crates/render/src/morph_path.rs +++ b/crates/render/src/morph_path.rs @@ -29,12 +29,72 @@ fn ratio(now: f64, own: f64) -> f64 { if own.abs() > 1e-9 && r.is_finite() { r } else { 1.0 } } -/// The geometry of frame shape `s` at its current size `w` x `h`: both outlines are built at their -/// own sizes (so the alignment does not change while the box grows), then scaled into the box. -/// With a [`Fade`] per sub-path. -pub(crate) fn geometry(pm: &PathMorph, s: &Shape, w: f64, h: f64) -> (Geometry, Vec) { - let a = shape_geometry(&Shape { geom: pm.from.clone(), ..Shape::default() }, pm.from_box.w, pm.from_box.h); - let b = shape_geometry(s, pm.to_box.w, pm.to_box.h); - let scale = |x: Xfrm| Vec2::new(ratio(w, x.w), ratio(h, x.h)); - GeometryMorph::new(&a, &b).at(pm.t, scale(pm.from_box), scale(pm.to_box)) +/// What a [`GeometryMorph`] is built from: both outlines at their own sizes. None of it changes +/// while a transition plays, only `t` and the current box do. +#[derive(Debug)] +struct Source { + from: Geom, + from_wh: (f64, f64), + to: Geom, + to_wh: (f64, f64), +} + +impl Source { + fn of(pm: &PathMorph, s: &Shape) -> Self { + Source { from: pm.from.clone(), from_wh: (pm.from_box.w, pm.from_box.h), to: s.geom.clone(), to_wh: (pm.to_box.w, pm.to_box.h) } + } + + fn is(&self, pm: &PathMorph, s: &Shape) -> bool { + self.from == pm.from && self.from_wh == (pm.from_box.w, pm.from_box.h) && self.to == s.geom && self.to_wh == (pm.to_box.w, pm.to_box.h) + } + + fn build(&self) -> GeometryMorph { + let geo = |geom: &Geom, (w, h): (f64, f64)| shape_geometry(&Shape { geom: geom.clone(), ..Shape::default() }, w, h); + GeometryMorph::new(&geo(&self.from, self.from_wh), &geo(&self.to, self.to_wh)) + } +} + +/// The aligned outlines of the morphs being played, kept across frames: aligning is the expensive +/// part, blending them at `t` is cheap. +#[derive(Debug, Default)] +pub(crate) struct Cache { + /// With whether the current frame used it. + morphs: Vec<(Source, GeometryMorph, bool)>, +} + +impl Cache { + /// The geometry of frame shape `s` at its current size `w` x `h`: both outlines are built at + /// their own sizes (so the alignment does not change while the box grows), then scaled into the + /// box. With a [`Fade`] per sub-path. + pub(crate) fn geometry(&mut self, pm: &PathMorph, s: &Shape, w: f64, h: f64) -> (Geometry, Vec) { + let k = match self.morphs.iter().position(|(src, ..)| src.is(pm, s)) { + Some(k) => k, + None => { + let src = Source::of(pm, s); + let m = src.build(); + self.morphs.push((src, m, false)); + self.morphs.len() - 1 + } + }; + let scale = |x: Xfrm| Vec2::new(ratio(w, x.w), ratio(h, x.h)); + let Some((_, m, used)) = self.morphs.get_mut(k) else { + // Not reachable (k was just found or pushed); align afresh rather than panic. + return Source::of(pm, s).build().at(pm.t, scale(pm.from_box), scale(pm.to_box)); + }; + *used = true; + m.at(pm.t, scale(pm.from_box), scale(pm.to_box)) + } + + /// Ends a frame: forgets the morphs it did not use (their transition is over). + pub(crate) fn sweep(&mut self) { + self.morphs.retain(|(.., used)| *used); + for (.., used) in &mut self.morphs { + *used = false; + } + } + + #[cfg(test)] + pub(crate) fn len(&self) -> usize { + self.morphs.len() + } } diff --git a/crates/render/src/tests.rs b/crates/render/src/tests.rs index 2123cb44..1759855e 100644 --- a/crates/render/src/tests.rs +++ b/crates/render/src/tests.rs @@ -403,6 +403,34 @@ fn horizontal_bars_list_the_first_category_at_the_bottom_unless_reversed() { assert!(draw(true) < 180, "reversed: first category at the top: {}", draw(true)); } +#[test] +fn glow_of_an_unfilled_shape_follows_its_line() { + // A frame (no fill, thin line) with a strong glow: the glow lines its edge, the inside stays + // the slide's white, as PowerPoint draws it. + let glow = |fill: Option| { + let mut sh = Shape { + id: ShapeId(610), + xfrm: Some(Xfrm::new(100.0, 100.0, 400.0, 300.0)), + fill, + line: Some(deckcraft_model::style::Line::solid(ColorRef::rgb(deckcraft_color::Rgba::rgb(0, 176, 240)), 0.25)), + ..Default::default() + }; + sh.effects = Some(deckcraft_model::Effects { + glow: Some(deckcraft_model::style::Glow { color: ColorRef::rgb(deckcraft_color::Rgba::rgb(0, 0, 255)), radius: 8.0 }), + ..Default::default() + }); + render_slide(&deck_with(vec![sh]), 0, &RenderOpts { scale: 1.0, ..Default::default() }) + }; + let img = glow(Some(Fill::None)); + let inside = img.pixel(300, 250); + assert!(inside[0] > 245 && inside[1] > 245, "{inside:?}"); + let edge = img.pixel(300, 97); + assert!(edge[0] < 230, "{edge:?}"); + // A filled shape's glow is still the whole silhouette's (the fill covers its inside). + let img = glow(Some(Fill::solid(ColorRef::rgb(deckcraft_color::Rgba::rgb(255, 255, 255))))); + assert!(img.pixel(300, 94)[0] < 240); +} + #[test] fn placeholder_prompts_only_in_edit_view() { let p = Presentation::default(); @@ -715,6 +743,29 @@ fn morph_path_fades_a_fill_only_one_side_has() { assert!(half[0] > full[0] + 40 && half[0] < 215, "{half:?} vs {full:?}"); } +#[test] +fn morph_path_aligns_once_per_transition() { + let (from, to) = (Xfrm::new(0.0, 0.0, 200.0, 200.0), Xfrm::new(0.0, 0.0, 300.0, 150.0)); + let star = Shape { geom: Geom::preset("star5"), ..Default::default() }; + let pm = |t: f64| PathMorph { id: ShapeId(10), from: Geom::preset("ellipse"), from_box: from, to_box: to, t }; + let els = |(g, f): (deckcraft_geom::preset::Geometry, Vec<_>)| (g.paths.iter().map(|p| p.path.elements().to_vec()).collect::>(), f); + let mut cache = morph_path::Cache::default(); + for t in [0.0, 0.3, 0.7, 1.0] { + let got = els(cache.geometry(&pm(t), &star, 250.0, 175.0)); + // The same as aligning afresh. + assert_eq!(got, els(morph_path::Cache::default().geometry(&pm(t), &star, 250.0, 175.0))); + cache.sweep(); + } + assert_eq!(cache.len(), 1); + // Another target outline is another morph. + let _ = cache.geometry(&pm(0.5), &Shape { geom: Geom::preset("rect"), ..Default::default() }, 250.0, 175.0); + assert_eq!(cache.len(), 2); + // A frame without morphs ends the transition: the cache empties. + cache.sweep(); + cache.sweep(); + assert_eq!(cache.len(), 0); +} + #[test] fn text_transform_unmirrors_flips() { // Asymmetric text rect in a 100×50 box: flips move the rect, never mirror the text. @@ -1472,7 +1523,7 @@ fn an_object_deckcraft_doesnt_model_shows_its_preview_picture() { let sh = Shape { id: ShapeId(600), xfrm: Some(Xfrm::new(100.0, 100.0, 400.0, 200.0)), - kind: ShapeKind::Opaque { xml: String::new(), preview: with_preview.then_some(media), label: "Package".into() }, + kind: ShapeKind::Opaque { xml: String::new(), preview: with_preview.then_some(media), label: "Package".into(), parts: vec![] }, ..Default::default() }; std::sync::Arc::make_mut(&mut p.slides[0]).shapes = vec![sh]; @@ -2436,3 +2487,16 @@ fn render_an_emf_file_to_look_at() { let img = render_slide(&p, 0, &RenderOpts { scale: 1.0, ..Default::default() }); let _ = std::fs::write(out, img.to_png()); } + +#[test] +fn single_series_column_chart_varies_colours_by_point() { + let render = |vary: bool| { + let mut c = Chart::sample(ChartType::Column); + c.series.truncate(1); + c.vary_colors = vary; + let sh = + Shape { id: ShapeId(800), xfrm: Some(Xfrm::new(50.0, 50.0, 400.0, 300.0)), kind: ShapeKind::Chart(Box::new(c)), ..Default::default() }; + render_slide(&deck_with(vec![sh]), 0, &RenderOpts { scale: 1.0, ..Default::default() }).pixels + }; + assert_ne!(render(true), render(false)); +} diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index 648e6f8f..bc4e080e 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -186,7 +186,10 @@ pub fn show(app: &mut SlideApp, ui: &mut Ui) { let idx = d.selection.slide; let doc = d.doc.clone(); let frame = anim.as_ref().map(|(tl, t)| (tl, *t)); - app.textures.slide(ui.ctx(), &doc, idx, px, visible, true, crate::media::gif_key(&gifs), app.ui.grayscale, ×, frame) + // A placeholder's prompt makes way for the caret once it is being typed into. + let typing_into = d.selection.text.as_ref().filter(|t| !t.notes && t.cell.is_none()).map(|t| t.shape); + let no_prompt = typing_into.filter(|id| d.shape(*id).and_then(|sh| sh.text.as_ref()).is_none_or(|body| body.is_empty())); + app.textures.slide(ui.ctx(), &doc, idx, px, visible, true, crate::media::gif_key(&gifs), app.ui.grayscale, ×, no_prompt, frame) } Some((Target::Master { master }, d)) => master_tex(ui.ctx(), &d.doc, master, None, px, visible), Some((Target::Layout { master, layout }, d)) => master_tex(ui.ctx(), &d.doc, master, Some(layout), px, visible), @@ -906,7 +909,9 @@ fn overlays(app: &mut SlideApp, ui: &Ui, painter: &egui::Painter, xf: Xf, t: &To ui.ctx().request_repaint_after(std::time::Duration::from_millis(300)); // An empty placeholder shows the caret where typing begins too. if blink && let Some((top, bot)) = deckcraft_engine::cmd::text::caret_line(st, ts) { - painter.line_segment([xf.to_screen(top), xf.to_screen(bot)], Stroke::new(1.5, t.caret)); + // In the colour of the text being typed: a black caret is lost on a dark slide. + let color = deckcraft_engine::cmd::text::caret_color(st, ts).map_or(t.caret, theme::to_color32); + painter.line_segment([xf.to_screen(top), xf.to_screen(bot)], Stroke::new(1.5, color)); } } } diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 337bf502..9f13f8ec 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -498,6 +498,8 @@ pub struct SlideApp { pub sorter: sorter::State, /// The slide thumbnail pane was the last thing clicked, so ⌘A selects slides, not shapes. pub(crate) slides_focused: bool, + /// A V key press or a Paste event arrived since the last V release (see `keyboard`). + v_seen: bool, /// Which recent paths exist on disk, probed once per path per session (a dead network share /// must not stall every frame). Not persisted. recent_checked: HashMap, @@ -568,6 +570,7 @@ impl SlideApp { doc_window: None, sorter: sorter::State::default(), slides_focused: false, + v_seen: false, recent_checked: HashMap::new(), twisting: false, next_file_id: 1, @@ -758,10 +761,13 @@ impl SlideApp { let Some(doc) = self.session.active().map(|d| d.doc.clone()) else { return false }; let bytes = match format { "png" | "jpeg" => { - let scale = 2.0; - let img = deckcraft_render::render_slide(&doc, slide, &deckcraft_render::RenderOpts { scale, ..Default::default() }); - let dpi = deckcraft_render::dpi_for_scale(scale); - Ok(if format == "png" { img.to_png_at(dpi) } else { img.to_jpeg_at(92, Some(dpi)) }) + // Every slide for "all slides", one download each, named like the desktop export (#64). + let name = self.session.active().map(|d| d.title()).unwrap_or_else(|| "Presentation".into()); + let mut any = false; + for (file, bytes) in web_image_files(&doc, slide, all, &name, ext) { + any = self.download(&file, Ok(bytes)) || any; + } + return any; } "pdf" => deckcraft_engine::cmd::file::pdf_bytes(&doc, &extra), "gif" => deckcraft_engine::cmd::file::gif_bytes(&doc, &extra), @@ -1612,10 +1618,25 @@ impl SlideApp { /// Keyboard and clipboard for the slide (when no text field has focus). fn keyboard(&mut self, ctx: &egui::Context) { + let events = ctx.input(|i| i.events.clone()); + // Ctrl+V with only an image on the clipboard: egui-winit sends no Paste event and swallows the + // key press, so the release of a V press never seen is the paste. Tracked even while a text + // field has focus, so that a V typed there is not taken for one. + let mut image_paste = false; + for e in &events { + match e { + egui::Event::Paste(_) | egui::Event::Key { key: egui::Key::V, pressed: true, .. } => self.v_seen = true, + egui::Event::Key { key: egui::Key::V, pressed: false, modifiers, .. } => { + // Only a release with Command/Ctrl held is a paste: a bare V release is not. + let seen = std::mem::take(&mut self.v_seen); + image_paste = !seen && (modifiers.command || modifiers.ctrl); + } + _ => {} + } + } if ctx.memory(|m| m.focused().is_some()) || self.dialog.as_ref().is_some_and(|d| d.modal()) || self.palette.is_some() { return; } - let events = ctx.input(|i| i.events.clone()); if self.backstage.is_some() { // The File tab: its own keys (Esc goes back, arrows walk the side menu) and app // shortcuts (New, Open, Command Palette…); typing, pasting or deleting would edit the @@ -1638,6 +1659,13 @@ impl SlideApp { self.drain_requests(); return; } + let img = if image_paste { self.services.clipboard_image.as_mut().and_then(|f| f()) } else { None }; + if let Some(png) = img { + let data = deckcraft_engine::cmd::base64_encode(&png); + if let Err(e) = self.session.execute("insert.picture", &json!({"name": "Pasted image.png", "data": data})) { + self.set_status(e.to_string()); + } + } for e in events { match e { egui::Event::Text(t) => { @@ -1674,6 +1702,12 @@ impl SlideApp { } } egui::Event::Key { key, pressed: true, modifiers, .. } => { + if key == egui::Key::Tab { + // The slide has the keyboard, so Tab is ours (indent, tab character, next + // shape). Left to egui it would also focus the first ribbon widget, and + // every key after it would go there instead of the slide (#78). + ctx.memory_mut(|m| m.move_focus(egui::FocusDirection::None)); + } if key == egui::Key::Escape && self.anim_preview.is_some() && !modifiers.command && !modifiers.ctrl && !modifiers.alt { self.anim_preview = None; continue; @@ -1923,6 +1957,25 @@ pub fn now_ms() -> f64 { } } +/// Images for the browser download: the current slide, or every slide (named `1.png`, +/// `2.png`... like the native export) when `all`. +fn web_image_files(doc: &deckcraft_model::Presentation, current: usize, all: bool, name: &str, ext: &str) -> Vec<(String, Vec)> { + let slides: Vec = if all { (0..doc.slides.len()).collect() } else { vec![current] }; + let single = slides.len() == 1; + let scale = 2.0; + let dpi = deckcraft_render::dpi_for_scale(scale); + slides + .into_iter() + .enumerate() + .map(|(k, i)| { + let img = deckcraft_render::render_slide(doc, i, &deckcraft_render::RenderOpts { scale, ..Default::default() }); + let bytes = if ext == "png" { img.to_png_at(dpi) } else { img.to_jpeg_at(92, Some(dpi)) }; + let file = if single { format!("{name}.{ext}") } else { format!("{name}{}.{ext}", k + 1) }; + (file, bytes) + }) + .collect() +} + #[cfg(target_arch = "wasm32")] fn js_now() -> f64 { js_sys::Date::now() @@ -2007,6 +2060,23 @@ mod tests { assert!((shape_rot(harness.state()) - 0.0).abs() < 0.01, "one undo, got {}", shape_rot(harness.state())); } + /// The browser "PNG images (all slides)" export yields one image per slide, not just the selected one (#64). + #[test] + fn web_export_all_slides_gives_one_file_per_slide() { + let mut doc = deckcraft_model::Presentation::default(); + if let Some(first) = doc.slides.first().cloned() { + doc.slides.push(first.clone()); + doc.slides.push(first); + } + assert_eq!(doc.slides.len(), 3); + let all = web_image_files(&doc, 1, true, "Deck", "png"); + let names: Vec<&str> = all.iter().map(|(n, _)| n.as_str()).collect(); + assert_eq!(names, ["Deck1.png", "Deck2.png", "Deck3.png"]); + let one = web_image_files(&doc, 1, false, "Deck", "png"); + assert_eq!(one.len(), 1); + assert_eq!(one[0].0, "Deck.png"); + } + fn title_text(app: &SlideApp) -> String { let d = app.session.active().unwrap(); let sh = &d.doc.slides[0].shapes[0]; @@ -2281,6 +2351,88 @@ mod tests { assert_eq!(harness.state().dialog.as_ref().map(|d| d.id.as_str()), Some("export")); } + /// #53: Ctrl+V did nothing for an image-only clipboard: egui-winit sends no Paste event for it and + /// swallows the key press, so only the V release reaches the app. + #[test] + fn ctrl_v_pastes_an_image_when_no_paste_event_arrives() { + let session = Session::with_new(); + let png = { + let d = session.active().unwrap(); + deckcraft_engine::cmd::file::render_png(&d.doc, 0, 0.1, false).0 + }; + assert!(!png.is_empty()); + let services = Services { clipboard_image: Some(Box::new(move || Some(png.clone()))), ..Default::default() }; + let mut harness = egui_kittest::Harness::builder().with_size(egui::vec2(1200.0, 800.0)).build_ui_state( + |ui, app: &mut SlideApp| { + app.logic(ui.ctx()); + app.ui(ui); + }, + SlideApp::new(session, services), + ); + harness.run_steps(2); + let pictures = |app: &SlideApp| { + let d = app.session.active().unwrap(); + d.doc.slides[0].shapes.iter().filter(|s| matches!(s.kind, deckcraft_model::ShapeKind::Picture { .. })).count() + }; + let v = |pressed| egui::Event::Key { key: egui::Key::V, physical_key: None, pressed, repeat: false, modifiers: egui::Modifiers::COMMAND }; + // A text paste (Paste event on the press) does not also paste the image on release. + harness.event(egui::Event::Paste("hello".into())); + harness.run_steps(2); + harness.event(v(false)); + harness.run_steps(2); + assert_eq!(pictures(harness.state()), 0); + // A V release without Command/Ctrl is not a paste. + harness.event(egui::Event::Key { key: egui::Key::V, physical_key: None, pressed: false, repeat: false, modifiers: egui::Modifiers::NONE }); + harness.run_steps(2); + assert_eq!(pictures(harness.state()), 0); + // What egui-winit sends for Ctrl+V with only an image on the clipboard: the release alone. + harness.event(v(false)); + harness.run_steps(2); + assert_eq!(pictures(harness.state()), 1); + } + + /// #78: after Tab in a text box, typing and deleting no longer reached the text. + #[test] + fn typing_goes_on_after_tab_in_a_text_box() { + let mut harness = egui_kittest::Harness::builder().with_size(egui::vec2(1200.0, 800.0)).build_ui_state( + |ui, app: &mut SlideApp| { + app.logic(ui.ctx()); + app.ui(ui); + }, + { + let mut session = Session::with_new(); + session.prefs.autocorrect = false; + SlideApp::new(session, Services::default()) + }, + ); + harness.run_steps(2); + let title = harness.state().session.active().unwrap().doc.slides[0].shapes[0].id.0; + harness.state_mut().session.execute("text.edit", &json!({"id": title})).unwrap(); + harness.event(egui::Event::Text("ab".into())); + harness.run_steps(2); + harness.key_press(egui::Key::Tab); + harness.run_steps(2); + harness.event(egui::Event::Text("c".into())); + harness.run_steps(2); + assert_eq!(title_text(harness.state()), "ab\tc"); + harness.key_press(egui::Key::Backspace); + harness.run_steps(2); + assert_eq!(title_text(harness.state()), "ab\t"); + assert!(harness.state().session.active().unwrap().selection.text.is_some(), "still editing"); + // Tab at the start of a paragraph indents it, and typing goes on there too (#93). + harness.key_press(egui::Key::Enter); + harness.run_steps(2); + harness.key_press(egui::Key::Tab); + harness.run_steps(2); + harness.event(egui::Event::Text("d".into())); + harness.run_steps(2); + assert_eq!(title_text(harness.state()), "ab\t\nd"); + let level = |h: &egui_kittest::Harness| { + h.state().session.active().unwrap().doc.slides[0].shapes[0].text.as_ref().unwrap().paragraphs[1].level + }; + assert_eq!(level(&harness), 1); + } + #[test] fn home_needs_an_open_presentation() { let mut app = SlideApp::new(Session::new(), Services::default()); diff --git a/crates/ui-egui/src/textures.rs b/crates/ui-egui/src/textures.rs index dae67b44..9730f4d6 100644 --- a/crates/ui-egui/src/textures.rs +++ b/crates/ui-egui/src/textures.rs @@ -21,6 +21,7 @@ struct Key { extra: u64, /// The pasteboard region the slide's drawing calls for (canvas only). want: Region, + skip_text: Option, } struct Entry { @@ -247,7 +248,8 @@ impl Textures { /// The canvas texture for slide `index`, the slide being `size` pixels, and the region it /// covers: the slide, grown over the pasteboard to show what reaches off the slide within /// `visible` (the canvas, in pixels from the slide's top-left corner). `extra` changes with the - /// GIF frames. + /// GIF frames. `skip_text` leaves out one shape's text (the prompt of a placeholder being typed + /// into). #[allow(clippy::too_many_arguments)] pub fn slide( &mut self, @@ -260,6 +262,7 @@ impl Textures { extra: u64, grayscale: bool, gif_times: &[(ShapeId, f64)], + skip_text: Option, anim: Option<(&deckcraft_anim::Timeline, f64)>, ) -> Option<(TextureHandle, Region)> { let slide = p.slides.get(index)?.clone(); @@ -287,6 +290,7 @@ impl Textures { edit, extra: extra ^ grayscale as u64 ^ anim_key(anim), want, + skip_text, }; if let Some(e) = &self.canvas && e.key == key @@ -305,6 +309,7 @@ impl Textures { offset: region.offset(), size: Some(region.size()), state, + skip_text, ..Default::default() }; deckcraft_render::render_slide_drawn(p, index, &opts) @@ -362,6 +367,7 @@ impl Textures { edit: false, extra: 0, want: Region::slide(width_px, h), + skip_text: None, }; self.clock += 1; let clock = self.clock; diff --git a/docs/animation-parity.md b/docs/animation-parity.md index e27daccc..c8adf49b 100644 --- a/docs/animation-parity.md +++ b/docs/animation-parity.md @@ -43,7 +43,7 @@ perspective-correct subdivided meshes. | Duration, advance on click / after time, Apply To All | yes | yes | | Transition sounds | yes | yes: Transitions › Sound offers six built-in sounds (synthesised, no sound files shipped), Other Sound…, [Stop Previous Sound] and Loop Until Next Sound; they play in the show and round-trip | | Preview in the editor | yes | yes | -| Morph: objects move, resize, recolour | yes | yes | +| Morph: objects move, resize, recolour; unmatched text boxes fade in place; a picture's crop morphs | yes | yes (#119, #118) | | Morph: Words and Characters | yes | yes (#5) | | Morph: geometry (circle to star) | yes | yes (#27) | | Morph: `!!` name forcing, pictures, 3-D models, ink | yes | names partly; no 3-D or ink | @@ -105,4 +105,6 @@ In, Wipe, Zoom, Float In, Grow/Shrink, Pulse, Spin, motion lines): weighted, ~85 | 2026-10-13 | minor | Animations ▸ Preview and Animation Pane ▸ Play All play the current slide's animations on the editor canvas; preview from the selected effect is still open | | 2026-10-13 | major | Transitions measured frame by frame against PowerPoint's video export and retuned (mean diff 35.7 → 15.0); PowerPoint's XML defaults and option meanings; new options | | 2026-10-13 | minor | Triggers now play in the slide show (they were read and written but never started), effects of different triggers apply in the order they were clicked | +| 2026-10-11 | minor | Morph leaves unrelated text boxes unpaired: they fade in place (#119) | +| 2026-10-11 | minor | Morph animates a changed picture crop (#118) | | 2026-10-10 | major | Created: 48/48 transitions, 64 of ~190 animation effects, pane and option depth, estimates | diff --git a/docs/gaps.md b/docs/gaps.md index 5b329773..843154ac 100644 --- a/docs/gaps.md +++ b/docs/gaps.md @@ -225,11 +225,6 @@ Detail in [file-format-parity.md](file-format-parity.md). - **Impact:** copy-paste of screenshots is a core workflow. - **Estimate:** 2–4 h. **Beta blocker.** -### Tab inside a text box ends text entry (#78) -- **Evidence:** #78. -- **Impact:** tab stops and indented lists can't be typed. -- **Estimate:** 1–2 h. - ### Charts: data grid, elements and the newer chart types - **Evidence:** checklist rows "Edit chart data" (P0, partial), "Chart elements" (partial), "Other chart types" (partial); `chartex` parts (waterfall, funnel, treemap, sunburst, histogram, Pareto, @@ -519,6 +514,7 @@ Detail in [localization-parity.md](localization-parity.md). | 2026-10-12 | minor | IME composition in slide text and notes, and CJK interface text with real glyphs (#101): the IME gap is now Korean on the web and table-cell underlines | | 2026-10-12 | minor | EMF/WMF text, pattern and hatch brushes drawn; legacy OLE and ActiveX previews shown; the SVG gap keeps only EMF clip regions | | 2026-10-12 | minor | Text warps render (all 40 presets, adjusts round-trip); the warp gap is now the gallery, command and handles | +| 2026-10-11 | minor | Removed "Tab inside a text box ends text entry" (#78, fixed) | | 2026-10-11 | minor | Animated GIF export landed (#171); the entry now covers video export and animating transitions in the GIF | | 2026-10-10 | minor | Noted that no gap blocks alpha (core-workflow gate passes) | | 2026-10-10 | major | Created: every gap from the 2026-10-10 re-measure and the 16 open GitHub issues, ranked, with beta blockers marked | diff --git a/docs/mcp.md b/docs/mcp.md index 5f6c5756..b2f879b3 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -13,6 +13,58 @@ deck. } ``` +## Confining file access + +```sh +deckcraft-cli mcp --automation-read-root /work/project --automation-write-root /work/project/out +``` + +```json +{"mcpServers": {"deckcraft": {"command": "deckcraft-cli", + "args": ["mcp", "--automation-read-root", "/work/project", "--automation-write-root", "/work/project/out"]}}} +``` + +The two flags (VectorCraft, PhotoCraft and the other Craft apps take the same ones, so one client +config works across the suite) keep the agent to the folders you give it: + +- Every file a command reads must lie inside the **read root**: `open_presentation` and + `file.open`, and `insert_picture`, `insert.picture`, `picture.change`, `insert.audio` and + `insert.video` with a `path`. Pictures, audio and video are copied into the deck when they are + inserted, so playing them later reads nothing from disk. +- Every file a command writes must lie inside the **write root**: `save_presentation`, `export` + (every format, and every slide of an image export), `file.save`, `file.saveAs`, `file.export`, + and `render_slide` with a `path`, plus the `.saving` temporary file a save goes through. +- Read and write access are separate. Give the same folder twice for both; a root left out grants + none of its access (with only `--automation-read-root`, nothing can be written to disk). Without + either flag nothing changes. +- A path is made absolute against the server's working directory, then its links are followed: a + file (or folder) link that leads out of a root is outside it. For a file that doesn't exist yet, + its nearest existing folder's links are followed and the rest must be plain names, so `..` below a + folder that doesn't exist is refused, as is a link that leads nowhere (writing through it would + create its target). The result is compared with the root by whole folder names: `/work2/x` is not + inside `/work`. On Windows case is ignored, UNC paths (`\\server\share\...`) are resolved like + drive-letter ones, and device names (`CON`, `NUL`, `COM1`...) are refused. +- A refused path is an ordinary tool error the agent can read and correct, for example + `automation path rejected: outside the write root /work/project/out: /etc/x.pptx` or + `automation filesystem access is not granted: write authority is absent: /work/project/out/x.pptx`. +- The roots must be existing folders; a missing one stops the server with an error. Each flag is + given once (`--flag ` or `--flag=`). + +The flags confine the in-process server, so `--connect` together with them is an error: the server +would otherwise talk to an app that isn't confined. DeckCraft's desktop app has no equivalent flags +yet, so a running app (`deckcraft --control`) is not confined. `--sample` is the operator's own +choice and is opened before the roots apply. + +What the roots do not cover: commands that take no path still read and change the open deck, and +`inspect_document` and `inspect_slide` return it. The system font scan and the app's own recovery +folder are fixed paths, not agent input, and are not checked. Anything that reads or writes a path +goes through one place in the engine (`io::read_file` and `io::write_file`); a test keeps the rest of the +engine from touching the file system on its own. + +The checks hold for every thread of the server process. A path is checked, then opened: another +program that swaps a link in between can still win that race, so the roots keep an agent to the +files it was given rather than guarding against other software on the computer. + ## Tools | Tool | What it does | diff --git a/docs/ui-parity.md b/docs/ui-parity.md index 0bd84709..a169a84b 100644 --- a/docs/ui-parity.md +++ b/docs/ui-parity.md @@ -141,7 +141,7 @@ menu colours taken from the theme tokens (menu `chrome`, page `card`). | Edit points on freeforms | yes | **no** | | WordArt adjust handles | yellow handles on transformed text | yes, one per adjust when the text has a warp; dragging sets that guide value | | Crop handles, crop to shape, aspect ratio | yes | partial | -| Text caret, selection, double/triple click, drag-select | yes | yes; Tab in text boxes ends editing (#78) | +| Text caret, selection, double/triple click, drag-select | yes | yes | | Exact size/position, lock aspect | yes | yes — Size and Position dialog (height, width, rotation, scale %, lock aspect, position from the top-left), one undo step; ribbon spinners still resize one side at a time | | Drag the frame of a text box being edited to move it | yes | yes | | Table cells: caret and selection in the cell, click into another cell, drag row and column borders | yes | yes; no cell-range selection by dragging across cells | @@ -266,4 +266,5 @@ consistency is open (#73, #56). | 2026-10-12 | minor | Off-slide objects drawn and editable around the slide in Normal view (#270) | | 2026-10-12 | minor | Selection Pane matched to PowerPoint (#133): drag and drop, arrows, groups, eye, Lock | | 2026-10-12 | minor | Text editing keys: Ctrl+arrows, Ctrl+Backspace/Delete, Ctrl+Up/Down, Home/End on wrapped lines, Shift+F3, font size keys, as observed in PowerPoint for Windows (#268) | +| 2026-10-11 | minor | Tab in text boxes keeps editing (#78) | | 2026-10-10 | major | Created: shell, panes and dialogs, direct manipulation, shortcuts and visual fidelity against PowerPoint 16.113 |