diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index aa3a8b7a..a5cea382 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -19,6 +19,7 @@ env: jobs: gates: + timeout-minutes: 60 name: gates (${{ matrix.os }}) strategy: fail-fast: false diff --git a/ROADMAP.md b/ROADMAP.md index ce8c4a9f..8ea976e6 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -131,6 +131,9 @@ app's JSON control channel (`deckcraft --control PORT`). | Date | What landed | |---|---| +| 2026-10-15 | PDF export and rendering: `anchorCtr` centres the drawn text block — right-aligned text no longer ran past the box edge (#736) | +| 2026-10-14 | connected MCP rejects unrelated, malformed, incomplete and oversized replies without replay; buffered unsolicited bytes force a fresh connection before sending (#718) | +| 2026-10-14 | Empty bulleted and numbered paragraphs show gray markers while editing, including on first click and after Enter. Markers use the list style and stay out of slide shows and exports. | | 2026-10-13 | Interface font fallback continues to eligible installed faces when the first covering face is colour or exceeds the 48 MiB limit; document fallback order and the CJK/IME rendering assertions are retained (#101) | | 2026-10-13 | PDFW2: adopt the shared writer with native unwind recovery and explicit font-embedding failures; preserve app-owned export behavior and keep reader dependencies out of production | | 2026-10-13 | Eyedropper: Shape Fill, Shape Outline, Font Color and every other colour menu end with an Eyedropper that takes the colour drawn at a point of the slide (hover shows it, a click applies it, Esc cancels); `slide.colorAt` gives the same from scripts and MCP (#367) | @@ -223,6 +226,8 @@ app's JSON control channel (`deckcraft --control PORT`). | Date | Change | Summary | |---|---|---| +| 2026-10-14 | trivial | remote MCP checks reply IDs, envelopes and bounded newline framing (#718) | +| 2026-10-14 | minor | Gray list markers on empty paragraphs while editing; caret keeps the list indent before typing | | 2026-10-14 | minor | Community PR wave folded: headline breadth 84% over 191 features (P0 92%, P1 82%, P2 55%) and the feature-area breadths from `cargo xtask parity`; one row per area; Upcoming item 2 merged; per-PR count notes dropped from the progress log. Ready stays ~55%, stage alpha | | 2026-10-13 | trivial | Progress log: Eyedropper in the colour menus, slide.colorAt (#367) | | 2026-10-13 | trivial | Progress log: video seek frame-loss race and regression coverage | diff --git a/apps/deckcraft/src/control_server.rs b/apps/deckcraft/src/control_server.rs index 966657b0..0abac12a 100644 --- a/apps/deckcraft/src/control_server.rs +++ b/apps/deckcraft/src/control_server.rs @@ -151,10 +151,23 @@ fn serve(stream: TcpStream, tx: &SyncSender, ctx: &egui::Context let method = msg.get("method").and_then(Value::as_str).unwrap_or("").to_string(); let params = msg.get("params").cloned().unwrap_or(json!({})); let (req, rrx) = ControlRequest::new(method, params); + let state = Arc::clone(&req.state); let mut r = match tx.try_send(req) { Ok(()) => { ctx.request_repaint(); - rrx.recv_timeout(limits.reply_timeout).unwrap_or_else(|_| json!({"ok": false, "error": "timeout"})) + match rrx.recv_timeout(limits.reply_timeout) { + Ok(v) => v, + Err(_) => { + // Give up on the request. While it was still queued this makes + // sure it never runs late (#719); if the app had already taken + // it, its outcome is unknown rather than a definite failure. + if ControlRequest::abandon(&state) { + json!({"ok": false, "error": "timeout"}) + } else { + json!({"ok": false, "error": "timeout: the app had already started the request, so it may still run; its outcome is unknown"}) + } + } + } } Err(TrySendError::Full(_)) => json!({"ok": false, "error": "busy: too many queued requests"}), Err(TrySendError::Disconnected(_)) => break, @@ -296,4 +309,43 @@ mod tests { assert_eq!(a["error"], "timeout"); drop(rx); } + + /// A request that timed out while it was still queued must not run when the window resumes (#719). + #[test] + fn timed_out_request_does_not_run_when_the_window_resumes() { + let (addr, rx) = server(Limits { max_line: 512, ..small() }); + let (mut r, mut w) = client(addr); + writeln!(w, r#"{{"id":1,"method":"engine.execute","params":{{"command":"slide.new"}}}}"#).unwrap(); + let t = reply(&mut r).unwrap(); + assert_eq!(t["error"], "timeout"); + // The window resumes and drains the queue, exactly as `SlideApp::drain_control` does. + let mut app = deckcraft_ui_egui::SlideApp::new(deckcraft_engine::Session::with_new(), deckcraft_ui_egui::Services::default()); + let ctx = egui::Context::default(); + let before = app.session.active().unwrap().doc.slides.len(); + let req = rx.try_recv().expect("the timed-out request is still queued"); + if req.take() { + let _ = deckcraft_ui_egui::control::handle(&mut app, &ctx, &req); + } + assert_eq!(app.session.active().unwrap().doc.slides.len(), before, "a request that timed out must not run when the window resumes"); + drop(rx); + } + + /// A request the app had already started when the reply timed out reports an unknown outcome, + /// not a definite failure (#719). + #[test] + fn a_request_already_started_when_the_reply_times_out_reports_an_unknown_outcome() { + let (addr, rx) = server(small()); + let ui = std::thread::spawn(move || { + let req = rx.recv().expect("the request is queued"); + assert!(req.take(), "the app claims the request"); + // Still running it when the sender's wait expires. + std::thread::sleep(Duration::from_millis(450)); + let _ = req.reply.send(json!({"ok": true, "result": "ran"})); + }); + let (mut r, mut w) = client(addr); + writeln!(w, r#"{{"id":1,"method":"m"}}"#).unwrap(); + let v = reply(&mut r).unwrap(); + assert!(v["error"].as_str().unwrap().contains("outcome is unknown"), "{v}"); + ui.join().unwrap(); + } } diff --git a/crates/engine/src/cmd/format.rs b/crates/engine/src/cmd/format.rs index fd7a5232..36d54de9 100644 --- a/crates/engine/src/cmd/format.rs +++ b/crates/engine/src/cmd/format.rs @@ -1089,7 +1089,7 @@ fn columns(s: &mut Session, p: &Value) -> Result { } fn autofit(s: &mut Session, p: &Value) -> Result { let m = match str_param(p, "mode").unwrap_or("none") { - "shrink" => AutoFit::Shrink { font_scale: 1.0, line_reduction: 0.0 }, + "shrink" => AutoFit::Shrink { font_scale: Some(1.0), line_reduction: 0.0 }, "resize" | "shape" => AutoFit::Shape, _ => AutoFit::None, }; diff --git a/crates/engine/src/cmd/slide.rs b/crates/engine/src/cmd/slide.rs index 7b3384fb..4a82f2a7 100644 --- a/crates/engine/src/cmd/slide.rs +++ b/crates/engine/src/cmd/slide.rs @@ -122,7 +122,7 @@ fn new(s: &mut Session, p: &Value) -> Result { sh.text = Some(TextBody::from_text(t)); } if let Some(b) = str_param(p, "body") - && let Some(sh) = slide.shapes.iter_mut().find(|x| x.ph_type().is_some_and(|k| !k.is_title())) + && let Some(sh) = body_placeholder(&mut slide.shapes) { let mut body = TextBody::from_text(b); // Leading tabs set the outline level, as in text.set. @@ -158,6 +158,25 @@ fn new(s: &mut Session, p: &Value) -> Result { }) } +/// The placeholder a new slide's `body` goes into: the layout's main content placeholder. +/// Prefers `idx="1"` (the content slot PowerPoint fills), then a full-size text placeholder over +/// a `half`/`quarter` one, then a text placeholder over a picture/chart/table one; ties keep +/// shape order, so a layout with a single body placeholder behaves as before. +fn body_placeholder(shapes: &mut [deckcraft_model::Shape]) -> Option<&mut deckcraft_model::Shape> { + use deckcraft_model::PhType; + let rank = |sh: &deckcraft_model::Shape| { + let ph = sh.ph.as_ref()?; + if ph.kind.is_title() { + return None; + } + let text = matches!(ph.kind, PhType::Body | PhType::Obj | PhType::SubTitle); + let small = matches!(ph.size.as_deref(), Some("half" | "quarter")); + Some((ph.idx != 1, small, !text)) + }; + let best = shapes.iter().enumerate().filter_map(|(i, sh)| rank(sh).map(|r| (r, i))).min()?.1; + shapes.get_mut(best) +} + fn index_of(s: &Session, p: &Value) -> Result { let st = s.doc()?; let i = usize_param(p, "index").unwrap_or(st.selection.slide); diff --git a/crates/engine/src/cmd/smartart.rs b/crates/engine/src/cmd/smartart.rs index a483197c..c58d1830 100644 --- a/crates/engine/src/cmd/smartart.rs +++ b/crates/engine/src/cmd/smartart.rs @@ -541,7 +541,7 @@ fn shadow(blur: f64, dist: f64) -> Shadow { /// `title` false every line is a bullet (the bullet boxes of the bullet lists). fn text_shape(preset: &str, x: Xfrm, lines: &[&DiagramNode], title: bool) -> Shape { let mut body = TextBody::default(); - body.body.autofit = Some(AutoFit::Shrink { font_scale: 1.0, line_reduction: 0.0 }); + body.body.autofit = Some(AutoFit::Shrink { font_scale: Some(1.0), line_reduction: 0.0 }); let top = lines.first().map(|n| if title { n.level } else { n.level.saturating_sub(1) }).unwrap_or(0); for (k, n) in lines.iter().enumerate() { let mut para = Paragraph::new(&n.text); @@ -1913,7 +1913,7 @@ fn to_text(s: &mut Session, p: &Value) -> Result { body.paragraphs.push(Paragraph::new("")); } body.body.wrap = Some(true); - body.body.autofit = Some(AutoFit::Shrink { font_scale: 1.0, line_reduction: 0.0 }); + body.body.autofit = Some(AutoFit::Shrink { font_scale: Some(1.0), line_reduction: 0.0 }); let id = ShapeId(doc.alloc_id().ok_or_else(|| bad(CMD, NO_ID_SPACE))?); let tb = Shape { id, name: "Text".into(), xfrm: Some(area), text_box: true, text: Some(body), geom: Geom::preset("rect"), ..Default::default() }; diff --git a/crates/engine/src/cmd/text.rs b/crates/engine/src/cmd/text.rs index 9c6d88e1..a0408eff 100644 --- a/crates/engine/src/cmd/text.rs +++ b/crates/engine/src/cmd/text.rs @@ -119,9 +119,14 @@ pub(crate) fn edit_text(s: &mut Session, f: impl FnOnce(&mut TextBody, &mut T let mut t = t0.clone(); let body = body_mut(doc, sel, &t).ok_or_else(|| bad("text", "the text being edited is gone"))?; let r = f(body, &mut t); - // The saved shrink-on-overflow scale was for the old text: fit the new text afresh. + // The saved shrink-on-overflow scale was for the old text: fit the new text afresh. A + // body whose file carried no `fontScale` keeps none (100 %) here; `fit_shrink_scale` + // below fills in the computed scale when the new text overflows (#724). if let Some(AutoFit::Shrink { font_scale, line_reduction }) = &mut body.body.autofit { - (*font_scale, *line_reduction) = (1.0, 0.0); + if font_scale.is_some() { + *font_scale = Some(1.0); + } + *line_reduction = 0.0; } sel.text = Some(t); Ok(r) @@ -132,8 +137,11 @@ pub(crate) fn edit_text(s: &mut Session, f: impl FnOnce(&mut TextBody, &mut T Ok(r) } -/// Grow/shrink an auto-sizing text box (Resize shape to fit text) to its text. +/// Refit a shape after its text changed: remember the shrink-on-overflow scale the layout +/// computes (#724), then grow/shrink an auto-sizing text box (Resize shape to fit text) to its +/// text. pub fn fit_text_box(s: &mut Session, id: ShapeId) -> Result<()> { + fit_shrink_scale(s, id)?; let st = s.doc()?; let Some(sh) = st.shape(id).cloned() else { return Ok(()) }; let Some(body) = sh.text.clone() else { return Ok(()) }; @@ -171,6 +179,53 @@ pub fn fit_text_box(s: &mut Session, id: ShapeId) -> Result<()> { Ok(()) } +/// Store the shrink-on-overflow scale the layout computes for `id`, so a save keeps +/// `fontScale`/`lnSpcReduction` in step with what DeckCraft draws (#724). Runs after a text +/// change: a body with no meaningful saved scale — a file's bare `` (`None`) or +/// the reset an edit leaves (`1.0`, no reduction) — is fitted afresh; a scale the file saved is +/// left as it is, as PowerPoint draws it. +fn fit_shrink_scale(s: &mut Session, id: ShapeId) -> Result<()> { + let st = s.doc()?; + let Some(sh) = st.shape(id).cloned() else { return Ok(()) }; + let Some(body) = sh.text.clone() else { return Ok(()) }; + let Some(AutoFit::Shrink { font_scale, line_reduction }) = body.body.autofit else { return Ok(()) }; + if !(font_scale.is_none() || (font_scale == Some(1.0) && line_reduction == 0.0)) { + return Ok(()); + } + let doc = st.doc.clone(); + let sel = st.selection.clone(); + let Some(ctx) = ctx_for(&doc, &sel) else { return Ok(()) }; + let x = xfrm_of(&doc, &sel, &sh); + let tr = deckcraft_render::shape_geometry(&sh, x.w, x.h).text_rect; + // Lay out as if the scale had just been reset, so the fit is computed even for `None` (which + // is drawn at 100 % and only fitted again when its text changes). + let mut probe = body.clone(); + probe.body.autofit = Some(AutoFit::Shrink { font_scale: Some(1.0), line_reduction: 0.0 }); + let opts = deckcraft_text::Opts { rect: tr, fields: &deckcraft_text::NoFields, prompt_color: None, no_shrink: false }; + let l = deckcraft_text::layout(&ctx, &sh, &probe, &opts); + if l.font_scale >= 1.0 && l.line_reduction <= 0.0 { + return Ok(()); + } + // Change the body in place without an extra undo step (it is part of the edit). + let st = s.doc_mut()?; + let mut d = (*st.doc).clone(); + let stored = if let Some(list) = crate::shapes_mut(&mut d, &st.selection) + && let Some(target) = deckcraft_model::find_shape_mut(list, id) + && let Some(t) = target.text.as_mut() + && let Some(AutoFit::Shrink { font_scale, line_reduction }) = &mut t.body.autofit + { + (*font_scale, *line_reduction) = (Some(l.font_scale), l.line_reduction); + true + } else { + false + }; + if stored { + st.doc = Arc::new(d); + st.revision += 1; + } + Ok(()) +} + pub(crate) fn ctx_for<'a>(doc: &'a Presentation, sel: &Selection) -> Option> { match sel.target { Target::Slides => doc.slides.get(sel.slide).and_then(|s| Ctx::for_slide(doc, s)), diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 19da6b09..cc872e66 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -105,13 +105,45 @@ fn editing_text_refits_a_saved_shrink() { s.edit(|doc, sel| { let shapes = crate::shapes_mut(doc, sel).unwrap(); let sh = deckcraft_model::find_shape_mut(shapes, title).unwrap(); - sh.text.as_mut().unwrap().body.autofit = Some(AutoFit::Shrink { font_scale: 0.5, line_reduction: 0.2 }); + sh.text.as_mut().unwrap().body.autofit = Some(AutoFit::Shrink { font_scale: Some(0.5), line_reduction: 0.2 }); Ok(()) }) .unwrap(); s.execute("text.insert", &json!({"text": "!"})).unwrap(); let sh = s.doc().unwrap().shape(title).cloned().unwrap(); - assert_eq!(sh.text.unwrap().body.autofit, Some(AutoFit::Shrink { font_scale: 1.0, line_reduction: 0.0 })); + assert_eq!(sh.text.unwrap().body.autofit, Some(AutoFit::Shrink { font_scale: Some(1.0), line_reduction: 0.0 })); +} + +/// After an edit, the shrink-on-overflow scale the layout computes is stored, so the saved file +/// carries the scale DeckCraft draws instead of a bare normAutofit (#724). +#[test] +fn editing_shrink_text_stores_the_layout_scale() { + use deckcraft_model::text::AutoFit; + let mut s = session(); + let r = s.execute("shape.insert", &json!({"preset": "rect", "rect": [80.0, 80.0, 200.0, 40.0]})).unwrap(); + let id = deckcraft_model::ShapeId(r["id"].as_u64().unwrap() as u32); + s.execute("format.autofit", &json!({"mode": "shrink"})).unwrap(); + let text = (0..12).map(|i| format!("Line {i}")).collect::>().join("\n"); + s.execute("text.set", &json!({"id": id.0, "text": text})).unwrap(); + let sh = s.doc().unwrap().shape(id).cloned().unwrap(); + let Some(AutoFit::Shrink { font_scale, line_reduction }) = sh.text.unwrap().body.autofit else { panic!("shrink") }; + assert!(font_scale.is_some_and(|s| s < 1.0), "the computed fit is stored: {font_scale:?} / {line_reduction}"); + // The saved file must carry the scale DeckCraft draws, so PowerPoint opens it the same way. + let bytes = deckcraft_pptx::export(&s.doc().unwrap().doc).unwrap(); + let mut z = zip::ZipArchive::new(std::io::Cursor::new(bytes)).unwrap(); + let mut xml = String::new(); + std::io::Read::read_to_string(&mut z.by_name("ppt/slides/slide1.xml").unwrap(), &mut xml).unwrap(); + let scales: Vec = xml.split("fontScale=\"").skip(1).filter_map(|s| s.split('"').next()?.parse().ok()).collect(); + assert!(scales.iter().any(|v| *v < 100_000), "the fitted scale is saved: {scales:?}"); + // Undo restores the state before the edit, and redo brings the stored fit back. + s.execute("edit.undo", &json!({})).unwrap(); + let sh = s.doc().unwrap().shape(id).cloned().unwrap(); + let Some(AutoFit::Shrink { font_scale, .. }) = sh.text.unwrap().body.autofit else { panic!("shrink") }; + assert_eq!(font_scale, Some(1.0), "undo restores the scale the edit started from"); + s.execute("edit.redo", &json!({})).unwrap(); + let sh = s.doc().unwrap().shape(id).cloned().unwrap(); + let Some(AutoFit::Shrink { font_scale, .. }) = sh.text.unwrap().body.autofit else { panic!("shrink") }; + assert!(font_scale.is_some_and(|v| v < 1.0), "redo brings the stored fit back: {font_scale:?}"); } #[test] @@ -1425,6 +1457,41 @@ fn slide_ranges_parse_like_powerpoints_print_box() { assert_eq!(opts.slides, Some(vec![0, 1]), "indices past the end are dropped"); } +/// #725: a layout with a small body placeholder ahead of its content placeholder (as Keynote +/// exports "Title & Bullets") still gets the body in the content placeholder (`idx="1"`). +#[test] +fn new_slide_body_goes_to_the_content_placeholder() { + for before in [true, false] { + let mut s = session(); + let doc = std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().doc); + let id = deckcraft_model::ShapeId(doc.alloc_id().expect("id")); + let lay = std::sync::Arc::make_mut(&mut doc.masters[0]) + .layouts + .iter_mut() + .find(|l| l.kind == deckcraft_model::LayoutType::TitleAndContent) + .unwrap(); + let at = lay.shapes.iter().position(|x| x.ph.as_ref().is_some_and(|p| p.idx == 1)).unwrap(); + let mut small = lay.shapes[at].clone(); + small.id = id; + small.name = "Subtitle 21".into(); + let ph = small.ph.as_mut().unwrap(); + ph.kind = deckcraft_model::PhType::Body; + ph.idx = 21; + ph.size = Some("quarter".into()); + lay.shapes.insert(if before { at } else { at + 1 }, small); + + s.execute("slide.new", &json!({"layout": "titleAndContent", "title": "T", "body": "Line one\nLine two"})).unwrap(); + let doc = &s.doc().unwrap().doc; + let slide = &doc.slides[doc.slides.len() - 1]; + let text = |idx: u32| { + let sh = slide.shapes.iter().find(|x| x.ph.as_ref().is_some_and(|p| p.idx == idx)).unwrap(); + sh.text.as_ref().map(|t| t.text()).unwrap_or_default() + }; + assert_eq!(text(1), "Line one\nLine two", "before = {before}"); + assert_eq!(text(21), "", "before = {before}"); + } +} + #[test] fn cell_fill_walks_only_the_cells_a_huge_selection_covers() { // A selection may reach far past the table; filling it touches the real cells and nothing more. diff --git a/crates/fonts/src/fallback.rs b/crates/fonts/src/fallback.rs index 563e0337..de4b2b0a 100644 --- a/crates/fonts/src/fallback.rs +++ b/crates/fonts/src/fallback.rs @@ -188,7 +188,12 @@ impl FontDb { // A letter with marks the run's font lacks (stacked combining marks): a loaded plain // face that draws the whole cluster, as PowerPoint draws the marks, before the run's // font with missing-glyph boxes. - _ => self.read_faces().iter().find(|f| !f.is_color() && covers_cluster(f, cluster)).cloned().unwrap_or_else(first), + _ => { + // Searched before `first` runs: it may load the default family, which needs the + // faces' write lock, so the read lock must not be held across it. + let found = self.read_faces().iter().find(|f| !f.is_color() && covers_cluster(f, cluster)).cloned(); + found.unwrap_or_else(first) + } }; // A pictograph that defaults to text presentation (⚠ ♥ ⬆ ▶) whose first fallback is the // colour emoji font: PowerPoint draws it from a plain symbol font. @@ -277,6 +282,22 @@ mod tests { } } + #[cfg(not(target_arch = "wasm32"))] + #[test] + fn falling_back_to_the_default_face_does_not_deadlock() { + // An empty chain and a character no loaded face draws: the loaded faces are searched, + // then the default family is looked up, which loads it (taking the faces' write lock). + // Holding the search's read lock across that lookup hung the thread. + let (tx, rx) = std::sync::mpsc::channel(); + std::thread::spawn(move || { + let db = FontDb::with_font_dirs(Vec::new()); + db.set_system_fallback(false); + let _ = tx.send(db.cascade(&['\u{10FFFD}'], &[], "Regular").family.clone()); + }); + let family = rx.recv_timeout(std::time::Duration::from_secs(30)).expect("cascade did not return"); + assert!(!family.is_empty()); + } + #[test] fn ligature_carets_never_fail() { let face = FontDb::global().face(crate::DEFAULT_FAMILY, "Regular"); diff --git a/crates/format/src/lib.rs b/crates/format/src/lib.rs index 679dfc7f..f1d0dc83 100644 --- a/crates/format/src/lib.rs +++ b/crates/format/src/lib.rs @@ -690,6 +690,29 @@ mod tests { } } + /// A body whose file had no `fontScale` (``) keeps that state through the + /// native format: it is drawn at 100 % and only fitted when its text changes (#724). + #[test] + fn a_bare_normautofit_body_round_trips() { + use deckcraft_model::text::{AutoFit, TextBody}; + let mut p = Presentation::default(); + let mut body = TextBody::from_text("Overflowing"); + body.body.autofit = Some(AutoFit::Shrink { font_scale: None, line_reduction: 0.0 }); + let sh = deckcraft_model::Shape { text: Some(body), ..Default::default() }; + Arc::make_mut(&mut p.slides[0]).shapes.push(sh); + let back = load(&save(&p).unwrap()).unwrap_or_else(|e| panic!("{e}")); + let body = back.slides[0].shapes.last().unwrap().text.clone().unwrap(); + assert_eq!(body.body.autofit, Some(AutoFit::Shrink { font_scale: None, line_reduction: 0.0 })); + // A scale the file saved round-trips as itself. + let mut body = TextBody::from_text("Overflowing"); + body.body.autofit = Some(AutoFit::Shrink { font_scale: Some(0.62), line_reduction: 0.2 }); + let sh = deckcraft_model::Shape { text: Some(body), ..Default::default() }; + Arc::make_mut(&mut p.slides[0]).shapes.push(sh); + let back = load(&save(&p).unwrap()).unwrap_or_else(|e| panic!("{e}")); + let body = back.slides[0].shapes.last().unwrap().text.clone().unwrap(); + assert_eq!(body.body.autofit, Some(AutoFit::Shrink { font_scale: Some(0.62), line_reduction: 0.2 })); + } + /// Only `null`s where numbers belong are patched: other damage still fails. #[test] fn other_damage_is_still_reported() { diff --git a/crates/mcp/src/backend.rs b/crates/mcp/src/backend.rs index cc43b4ee..fb809c09 100644 --- a/crates/mcp/src/backend.rs +++ b/crates/mcp/src/backend.rs @@ -1,11 +1,15 @@ //! Where MCP tool calls end up: a control-channel method call. -use std::io::{BufRead, BufReader, Write}; +use std::io::{BufRead, BufReader, Read, Write}; use std::net::{TcpStream, ToSocketAddrs}; use std::time::Duration; use serde_json::{Value, json}; +// Includes the newline, matching the control server's request budget. In particular a peer +// must not be able to grow a reply indefinitely by withholding its newline. +const MAX_REPLY_BYTES: u64 = 16 * 1024 * 1024; + /// Something that answers control-channel methods (`engine.execute`, `document.inspect`, /// `ui.pointer`, `ui.render`, …). See `deckcraft_ui_egui::control` for the full list. pub trait Backend { @@ -58,7 +62,7 @@ impl Remote { /// Write one request line. A failure here means the app never got a complete line, so retrying is safe. /// A connection the app has already closed (it restarted since the last call) is replaced before writing. fn send(&mut self, line: &str) -> std::io::Result<()> { - if self.conn.as_ref().is_none_or(|(_, writer)| is_closed(writer)) { + if self.conn.as_ref().is_none_or(|(reader, writer)| !reader.buffer().is_empty() || is_closed(writer)) { self.reconnect()?; } let Some((_, writer)) = self.conn.as_mut() else { @@ -69,15 +73,34 @@ impl Remote { } /// Read the reply to a request that was already sent. - fn receive(&mut self) -> std::io::Result { + fn receive(&mut self, id: u64) -> std::io::Result> { let Some((reader, _)) = self.conn.as_mut() else { return Err(std::io::Error::new(std::io::ErrorKind::NotConnected, "not connected")); }; - let mut reply = String::new(); - if reader.read_line(&mut reply)? == 0 { - return Err(std::io::Error::new(std::io::ErrorKind::UnexpectedEof, "control channel closed")); + let mut reply = Vec::new(); + reader.take(MAX_REPLY_BYTES + 1).read_until(b'\n', &mut reply)?; + if reply.len() as u64 > MAX_REPLY_BYTES { + return Err(std::io::Error::new(std::io::ErrorKind::InvalidData, "control reply exceeds 16 MiB")); + } + if reply.last() != Some(&b'\n') { + return Err(std::io::Error::new(std::io::ErrorKind::UnexpectedEof, "control reply has no terminating newline")); + } + let invalid = |message| std::io::Error::new(std::io::ErrorKind::InvalidData, message); + let v: Value = serde_json::from_slice(&reply).map_err(|e| invalid(format!("bad reply from app: {e}")))?; + if v.get("id").and_then(Value::as_u64) != Some(id) { + return Err(invalid("control reply has an unexpected request id".to_string())); + } + match v.get("ok").and_then(Value::as_bool) { + Some(true) if v.get("error").is_none() => { + v.get("result").cloned().map(Ok).ok_or_else(|| invalid("successful control reply has no result".to_string())) + } + Some(false) if v.get("result").is_none() => v + .get("error") + .and_then(Value::as_str) + .map(|error| Err(error.to_string())) + .ok_or_else(|| invalid("failed control reply has no error string".to_string())), + _ => Err(invalid("control reply has an invalid envelope".to_string())), } - Ok(reply) } } @@ -107,19 +130,13 @@ impl Backend for Remote { format!("DeckCraft app at {} is not reachable: {e}", self.addr) })?; } - let reply = self.receive().map_err(|e| { + self.receive(id).map_err(|e| { self.conn = None; format!( "lost the reply from the DeckCraft app at {} ({e}); the command may or may not have run, check the document before retrying", self.addr ) - })?; - let v: Value = serde_json::from_str(reply.trim()).map_err(|e| format!("bad reply from app: {e}"))?; - if v.get("ok").and_then(Value::as_bool) == Some(true) { - Ok(v.get("result").cloned().unwrap_or(Value::Null)) - } else { - Err(v.get("error").and_then(Value::as_str).unwrap_or("unknown error").to_string()) - } + })? } fn has_ui(&self) -> bool { @@ -138,6 +155,138 @@ mod tests { use std::sync::Arc; use std::sync::atomic::{AtomicUsize, Ordering}; + fn reply_once(reply: Vec) -> (Remote, std::thread::JoinHandle<()>) { + let listener = TcpListener::bind("127.0.0.1:0").expect("bind"); + let addr = listener.local_addr().expect("addr").to_string(); + let peer = std::thread::spawn(move || { + let (mut stream, _) = listener.accept().expect("accept"); + stream.set_read_timeout(Some(Duration::from_secs(5))).expect("timeout"); + stream.set_write_timeout(Some(Duration::from_secs(5))).expect("write timeout"); + let mut line = String::new(); + BufReader::new(&stream).read_line(&mut line).expect("request"); + assert_eq!(serde_json::from_str::(&line).expect("JSON")["id"], 1); + // An oversized reply may be rejected before the peer finishes writing it. + let _ = stream.write_all(&reply); + }); + (Remote::connect(&addr).expect("connect"), peer) + } + + #[test] + fn wrong_reply_id_is_not_command_success() { + let (mut remote, peer) = reply_once(b"{\"id\":999,\"ok\":true,\"result\":\"unrelated\"}\n".to_vec()); + let result = remote.call("engine.execute", json!({"command":"slide.new"})); + peer.join().expect("peer"); + let error = result.expect_err("an unrelated reply cannot confirm a mutation"); + assert!(error.contains("may or may not have run"), "{error}"); + assert!(remote.conn.is_none(), "discard the invalid connection without replaying"); + } + + #[test] + fn matching_success_and_error_replies_keep_their_meaning() { + for (reply, expected) in [ + ("{\"id\":1,\"ok\":true,\"result\":null}\r\n", Ok(Value::Null)), + ("{\"id\":1,\"ok\":true,\"result\":{\"slides\":2}}\n", Ok(json!({"slides":2}))), + ("{\"id\":1,\"ok\":false,\"error\":\"disabled\"}\n", Err("disabled".into())), + ] { + let (mut remote, peer) = reply_once(reply.as_bytes().to_vec()); + assert_eq!(remote.call("engine.execute", json!({})), expected); + assert!(remote.conn.is_some(), "valid responses do not invalidate the connection"); + peer.join().expect("peer"); + } + } + + #[test] + fn malformed_reply_envelopes_leave_the_outcome_unknown() { + for reply in [ + "not JSON\n", + "[]\n", + "{\"ok\":true,\"result\":null}\n", + "{\"id\":\"1\",\"ok\":true,\"result\":null}\n", + "{\"id\":1,\"ok\":1,\"result\":null}\n", + "{\"id\":1,\"ok\":true}\n", + "{\"id\":1,\"ok\":false}\n", + "{\"id\":1,\"ok\":false,\"error\":42}\n", + "{\"id\":1,\"ok\":true,\"result\":null,\"error\":\"failed\"}\n", + "{\"id\":1,\"ok\":false,\"result\":null,\"error\":\"failed\"}\n", + ] { + let (mut remote, peer) = reply_once(reply.as_bytes().to_vec()); + let result = remote.call("engine.execute", json!({})); + peer.join().expect("peer"); + let error = result.expect_err(reply); + assert!(error.contains("may or may not have run"), "{reply}: {error}"); + assert!(remote.conn.is_none(), "{reply}: discard the invalid connection"); + } + } + + #[test] + fn eof_without_a_newline_is_not_a_complete_reply() { + let (mut remote, peer) = reply_once(b"{\"id\":1,\"ok\":true,\"result\":null}".to_vec()); + let result = remote.call("engine.execute", json!({})); + peer.join().expect("peer"); + assert!(result.is_err(), "a truncated frame cannot confirm a mutation"); + assert!(remote.conn.is_none()); + } + + #[test] + fn oversized_reply_is_rejected() { + let reply = format!("{{\"id\":1,\"ok\":true,\"result\":\"{}\"}}\n", "x".repeat(16 * 1024 * 1024)); + let (mut remote, peer) = reply_once(reply.into_bytes()); + let result = remote.call("engine.execute", json!({})); + let invalidated = remote.conn.is_none(); + // Close the socket before joining: the bounded reader need not drain the peer's reply. + remote.conn = None; + peer.join().expect("peer"); + assert!(result.is_err(), "a reply is limited to 16 MiB"); + let error = result.expect_err("checked above"); + assert!(error.contains("may or may not have run"), "{error}"); + assert!(invalidated, "discard the oversized reply's connection"); + } + + #[test] + fn reply_at_the_size_limit_is_accepted() { + let prefix = "{\"id\":1,\"ok\":true,\"result\":\""; + let suffix = "\"}\n"; + let payload_len = 16 * 1024 * 1024 - prefix.len() - suffix.len(); + let reply = format!("{prefix}{}{suffix}", "x".repeat(payload_len)); + let (mut remote, peer) = reply_once(reply.into_bytes()); + let result = remote.call("document.inspect", json!({})).expect("reply at the limit"); + assert_eq!(result.as_str().map(str::len), Some(payload_len)); + peer.join().expect("peer"); + } + + #[test] + fn buffered_unsolicited_reply_is_discarded_before_sending() { + let listener = TcpListener::bind("127.0.0.1:0").expect("bind"); + let addr = listener.local_addr().expect("addr").to_string(); + let peer = std::thread::spawn(move || { + let (mut first, _) = listener.accept().expect("first connection"); + first.set_read_timeout(Some(Duration::from_secs(5))).expect("timeout"); + let mut line = String::new(); + BufReader::new(&first).read_line(&mut line).expect("initial request"); + // Even a stale frame whose ID happens to match must not answer a new request. + first + .write_all(b"{\"id\":1,\"ok\":true,\"result\":\"first\"}\n{\"id\":2,\"ok\":true,\"result\":\"stale\"}\n") + .expect("two replies together"); + line.clear(); + let sent_on_stale = BufReader::new(&first).read_line(&mut line).expect("first request") != 0; + if !sent_on_stale { + let (mut fresh, _) = listener.accept().expect("fresh connection"); + fresh.set_read_timeout(Some(Duration::from_secs(5))).expect("timeout"); + BufReader::new(&fresh).read_line(&mut line).expect("fresh request"); + fresh.write_all(b"{\"id\":2,\"ok\":false,\"error\":\"rejected\"}\n").expect("actual reply"); + } + sent_on_stale + }); + let mut remote = Remote::connect(&addr).expect("connect"); + assert_eq!(remote.call("document.inspect", json!({})), Ok(json!("first"))); + // Force the stale frame into BufReader, where a socket-only peek cannot see it. + assert!(!remote.conn.as_mut().expect("connection").0.fill_buf().expect("buffer").is_empty()); + let result = remote.call("engine.execute", json!({"command":"slide.new"})); + let sent_on_stale = peer.join().expect("peer"); + assert_eq!(result, Err("rejected".into()), "only the fresh peer's response counts"); + assert!(!sent_on_stale, "replace a connection with buffered unsolicited bytes before sending"); + } + #[test] fn lost_reply_is_not_resent() { let listener = TcpListener::bind("127.0.0.1:0").expect("bind"); diff --git a/crates/media/src/lib.rs b/crates/media/src/lib.rs index 95150868..dc1a892b 100644 --- a/crates/media/src/lib.rs +++ b/crates/media/src/lib.rs @@ -92,9 +92,15 @@ impl Container { } else if at(0, &ASF_HEADER) { Container::Asf } else if at(0, b"ID3") { - // ID3v2 is followed by MP3 frames (or, rarely, ADTS). - let size = b.get(6..10).map(|s| s.iter().fold(0usize, |a, &x| (a << 7) | (x & 0x7f) as usize)).unwrap_or(0); - match Container::sniff(b.get(10 + size..).unwrap_or(&[])) { + // ID3v2 is followed by MP3 frames (or, rarely, ADTS). The chain of tags is walked + // iteratively — a crafted file can carry thousands of empty tags — and only the + // bytes after the last one are sniffed. + let mut rest = b; + while rest.starts_with(b"ID3") { + let size = rest.get(6..10).map(|s| s.iter().fold(0usize, |a, &x| (a << 7) | (x & 0x7f) as usize)).unwrap_or(0); + rest = rest.get(10 + size..).unwrap_or(&[]); + } + match Container::sniff(rest) { Container::Adts => Container::Adts, _ => Container::Mp3, } @@ -126,3 +132,20 @@ impl Container { /// Worker threads are available (not on wasm32). pub(crate) const THREADS: bool = !cfg!(target_arch = "wasm32"); + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn id3_tag_chains_do_not_recurse_the_stack() { + // 2 MB of empty ID3v2 tags (10 bytes each) chained ahead of the MP3 frames: walking + // the chain must not cost one stack frame per tag. + let mut b = Vec::with_capacity(200_000 * 10 + 2); + for _ in 0..200_000 { + b.extend_from_slice(&[b'I', b'D', b'3', 4, 0, 0, 0, 0, 0, 0]); + } + b.extend_from_slice(&[0xFF, 0xFB]); // an MP3 frame sync + assert_eq!(Container::sniff(&b), Container::Mp3); + } +} diff --git a/crates/media/src/probe.rs b/crates/media/src/probe.rs index cf5de96a..2453317a 100644 --- a/crates/media/src/probe.rs +++ b/crates/media/src/probe.rs @@ -198,8 +198,8 @@ fn probe_asf(b: &[u8]) -> Result { } let header_end = (u64le(16) as usize).min(b.len()); let mut info = MediaInfo { container: "asf", ..Default::default() }; - let mut pos = 30; - while pos + 24 <= header_end { + let mut pos: usize = 30; + while pos.checked_add(24).is_some_and(|end| end <= header_end) { let guid = &b[pos..pos + 16]; let size = u64le(pos + 16) as usize; if size < 24 { @@ -230,7 +230,24 @@ fn probe_asf(b: &[u8]) -> Result { info.video = Some(VideoInfo { codec: "WMV".into(), width: u32le(td), height: u32le(td + 4), decodable: false, ..Default::default() }); } } - pos += size; + // A hostile size must not wrap the walk (or walk off the end): an object that + // overruns the header ends it, which is also the correct ASF semantics. + let Some(next) = pos.checked_add(size).filter(|&p| p <= header_end) else { break }; + pos = next; } Ok(info) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn hostile_asf_object_sizes_end_the_walk() { + // The header object GUID followed by 48 × 0xFF: the walk reads usize::MAX back as + // every object size and must end instead of wrapping `pos` (and panicking). + let mut b = crate::ASF_HEADER.to_vec(); + b.extend_from_slice(&[0xFF; 48]); + assert!(matches!(probe_asf(&b), Ok(info) if info.container == "asf")); + } +} diff --git a/crates/model/src/defaults.rs b/crates/model/src/defaults.rs index 19adc7bb..af6948ea 100644 --- a/crates/model/src/defaults.rs +++ b/crates/model/src/defaults.rs @@ -203,13 +203,13 @@ pub fn build_master(size: Size, theme: Theme, first_id: u32) -> (Master, u32) { let title_box = |b: &mut B| { let (x, y, ww, hh) = r(60.0, 29.0, 840.0, 104.0); let mut s = b.shape("Title 1", PhType::Title, 0, x, y, ww, hh); - s.text = Some(body_with(Some(Anchor::Middle), Some(AutoFit::Shrink { font_scale: 1.0, line_reduction: 0.0 }))); + s.text = Some(body_with(Some(Anchor::Middle), Some(AutoFit::Shrink { font_scale: Some(1.0), line_reduction: 0.0 }))); s }; let body_box = |b: &mut B, name: &str, idx: u32, x: f64, y: f64, ww: f64, hh: f64| { let (x, y, ww, hh) = r(x, y, ww, hh); let mut s = b.shape(name, PhType::Body, idx, x, y, ww, hh); - s.text = Some(body_with(Some(Anchor::Top), Some(AutoFit::Shrink { font_scale: 1.0, line_reduction: 0.0 }))); + s.text = Some(body_with(Some(Anchor::Top), Some(AutoFit::Shrink { font_scale: Some(1.0), line_reduction: 0.0 }))); s }; @@ -263,7 +263,7 @@ pub fn build_master(size: Size, theme: Theme, first_id: u32) -> (Master, u32) { t.text = Some(TextBody { body: BodyProps { anchor: Some(Anchor::Bottom), - autofit: Some(AutoFit::Shrink { font_scale: 1.0, line_reduction: 0.0 }), + autofit: Some(AutoFit::Shrink { font_scale: Some(1.0), line_reduction: 0.0 }), ..Default::default() }, list_style: ls, @@ -288,7 +288,7 @@ pub fn build_master(size: Size, theme: Theme, first_id: u32) -> (Master, u32) { st.text = Some(TextBody { body: BodyProps { anchor: Some(Anchor::Top), - autofit: Some(AutoFit::Shrink { font_scale: 1.0, line_reduction: 0.0 }), + autofit: Some(AutoFit::Shrink { font_scale: Some(1.0), line_reduction: 0.0 }), ..Default::default() }, list_style: ls, diff --git a/crates/model/src/table.rs b/crates/model/src/table.rs index 1abd1177..0705d1a0 100644 --- a/crates/model/src/table.rs +++ b/crates/model/src/table.rs @@ -227,6 +227,29 @@ impl Table { if r1 >= self.rows.len() || c1 >= self.cols.len() || (r0 == r1 && c0 == c1) { return false; } + // Grow the rectangle until no existing merge sticks out of it. + let (mut r0, mut c0, mut r1, mut c1) = (r0, c0, r1, c1); + loop { + let before = (r0, c0, r1, c1); + for (r, row) in self.rows.iter().enumerate() { + for (c, cell) in row.cells.iter().enumerate().take(self.cols.len()) { + if cell.is_covered() { + continue; + } + let ar1 = r.saturating_add((cell.row_span.max(1) as usize).saturating_sub(1)).min(self.rows.len() - 1); + let ac1 = c.saturating_add((cell.grid_span.max(1) as usize).saturating_sub(1)).min(self.cols.len() - 1); + if r <= r1 && ar1 >= r0 && c <= c1 && ac1 >= c0 { + r0 = r0.min(r); + c0 = c0.min(c); + r1 = r1.max(ar1); + c1 = c1.max(ac1); + } + } + } + if (r0, c0, r1, c1) == before { + break; + } + } let mut text = vec![]; for r in r0..=r1 { for c in c0..=c1 { @@ -505,6 +528,20 @@ mod tests { t.cell(r, c).unwrap().text.text() } + #[test] + fn merging_over_an_existing_merge_takes_it_in() { + let mut t = Table::new(3, 3, 300.0, 30.0); + if let Some(c) = t.cell_mut(1, 2) { + c.text = TextBody::from_text("X"); + } + assert!(t.merge(1, 1, 1, 2)); + assert!(t.merge(0, 0, 1, 1)); + assert_topology(&t); + let a = t.cell(0, 0).unwrap(); + assert_eq!((a.row_span, a.grid_span), (2, 3)); + assert_eq!(anchor_text(&t, 0, 0), "X"); + } + #[test] fn row_and_column_edits_keep_merges_consistent() { for at in 0..=4 { diff --git a/crates/model/src/text.rs b/crates/model/src/text.rs index 1d862b15..b317d3d9 100644 --- a/crates/model/src/text.rs +++ b/crates/model/src/text.rs @@ -41,8 +41,14 @@ impl Anchor { pub enum AutoFit { /// Do not autofit. None, - /// Shrink text on overflow: font scale and line-spacing reduction (0–1). - Shrink { font_scale: f64, line_reduction: f64 }, + /// Shrink text on overflow: font scale and line-spacing reduction (0–1). `font_scale` is + /// `None` for a file's `` with no `fontScale`: the schema default 100 %, + /// which PowerPoint draws as it is and only fits again when the text changes (#724). + Shrink { + #[serde(skip_serializing_if = "Option::is_none")] + font_scale: Option, + line_reduction: f64, + }, /// Resize shape to fit text. Shape, } diff --git a/crates/pptx/src/read/dml.rs b/crates/pptx/src/read/dml.rs index 912a8547..c88bd1a9 100644 --- a/crates/pptx/src/read/dml.rs +++ b/crates/pptx/src/read/dml.rs @@ -338,7 +338,7 @@ pub fn body_pr(b: &El) -> BodyProps { match e.local() { "normAutofit" => { p.autofit = Some(AutoFit::Shrink { - font_scale: pct(e, "fontScale").unwrap_or(1.0).clamp(0.01, 1.0), + font_scale: pct(e, "fontScale").map(|v| v.clamp(0.01, 1.0)), line_reduction: pct(e, "lnSpcReduction").unwrap_or(0.0).clamp(0.0, 1.0), }) } @@ -645,8 +645,14 @@ mod tests { assert_eq!(b.wrap, Some(false)); assert_eq!(b.vert, Some(TextDir::Vertical270)); assert_eq!(b.columns, Some(2)); - assert_eq!(b.autofit, Some(AutoFit::Shrink { font_scale: 0.625, line_reduction: 0.2 })); + assert_eq!(b.autofit, Some(AutoFit::Shrink { font_scale: Some(0.625), line_reduction: 0.2 })); assert_eq!(b.space_first_last, None); + // A bare `` keeps no scale: the schema default 100 % is drawn as it is, + // not the same as a saved 100 % (`fontScale="100000"`) (#724). + let d = parse(br#""#).unwrap(); + assert_eq!(body_pr(&d.root).autofit, Some(AutoFit::Shrink { font_scale: None, line_reduction: 0.0 })); + let d = parse(br#""#).unwrap(); + assert_eq!(body_pr(&d.root).autofit, Some(AutoFit::Shrink { font_scale: Some(1.0), line_reduction: 0.0 })); let d = parse(br#""#).unwrap(); assert_eq!(body_pr(&d.root).space_first_last, Some(true)); // mongolianVert is its own direction (PowerPoint runs its lines left to right) and is diff --git a/crates/pptx/src/write/dml.rs b/crates/pptx/src/write/dml.rs index 371bad7e..09b298f8 100644 --- a/crates/pptx/src/write/dml.rs +++ b/crates/pptx/src/write/dml.rs @@ -489,9 +489,11 @@ pub fn body_pr(w: &mut W, b: &BodyProps) { Some(AutoFit::None) => w.empty0("a:noAutofit"), Some(AutoFit::Shape) => w.empty0("a:spAutoFit"), Some(AutoFit::Shrink { font_scale, line_reduction }) => { - let fs = pct(font_scale.clamp(0.01, 1.0)); + // A scale the model holds is written, 100 % included: reopening then keeps the same + // "draw the saved scale" state instead of becoming a bare `` (#724). + let fs = font_scale.map(|s| pct(s.clamp(0.01, 1.0))); let lr = pct(line_reduction.clamp(0.0, 1.0)); - w.empty("a:normAutofit", A::new().o("fontScale", (fs != 100_000).then_some(fs)).o("lnSpcReduction", (lr != 0).then_some(lr))); + w.empty("a:normAutofit", A::new().o("fontScale", fs).o("lnSpcReduction", (lr != 0).then_some(lr))); } None => {} } diff --git a/crates/pptx/tests/corpus_charts.rs b/crates/pptx/tests/corpus_charts.rs index ee3faffd..ff1e6424 100644 --- a/crates/pptx/tests/corpus_charts.rs +++ b/crates/pptx/tests/corpus_charts.rs @@ -109,7 +109,7 @@ fn corpus() -> Presentation { let mut columns = TextBody::from_text("Two columns\nof vertical text"); columns.body.columns = Some(3); columns.body.col_spacing = Some(18.0); - columns.body.autofit = Some(AutoFit::Shrink { font_scale: 0.85, line_reduction: 0.2 }); + columns.body.autofit = Some(AutoFit::Shrink { font_scale: Some(0.85), line_reduction: 0.2 }); columns.body.vert = Some(TextDir::Vertical270); let mut col = shape(&mut p, "Columns", 480.0, 40.0, 200.0, 160.0); col.text = Some(columns); @@ -176,7 +176,7 @@ fn chart_table_and_text_corpus_round_trips() { assert_eq!(plain(cols), "Two columns\nof vertical text"); assert_eq!(cols.body.columns, Some(3)); assert_eq!(cols.body.col_spacing, Some(18.0)); - assert_eq!(cols.body.autofit, Some(AutoFit::Shrink { font_scale: 0.85, line_reduction: 0.2 })); + assert_eq!(cols.body.autofit, Some(AutoFit::Shrink { font_scale: Some(0.85), line_reduction: 0.2 })); assert_eq!(cols.body.vert, Some(TextDir::Vertical270)); let warp = by_name(slide, "Warp").text.as_ref().expect("warp"); diff --git a/crates/pptx/tests/corpus_style.rs b/crates/pptx/tests/corpus_style.rs index 65c3631c..d552002f 100644 --- a/crates/pptx/tests/corpus_style.rs +++ b/crates/pptx/tests/corpus_style.rs @@ -79,7 +79,7 @@ fn style_deck() -> Presentation { let mut word = shape(&mut p, "Styled text", 40.0, 40.0, 400.0, 180.0); word.text = Some(TextBody { body: BodyProps { - autofit: Some(AutoFit::Shrink { font_scale: 0.8, line_reduction: 0.2 }), + autofit: Some(AutoFit::Shrink { font_scale: Some(0.8), line_reduction: 0.2 }), columns: Some(2), col_spacing: Some(12.0), vert: Some(TextDir::Vertical270), @@ -167,7 +167,7 @@ fn style_kinds_round_trip() { let twice = round(&once); let body = &by_name(&once, "Styled text").text.as_ref().expect("text").body; - assert_eq!(body.autofit, Some(AutoFit::Shrink { font_scale: 0.8, line_reduction: 0.2 })); + assert_eq!(body.autofit, Some(AutoFit::Shrink { font_scale: Some(0.8), line_reduction: 0.2 })); assert_eq!(body.columns, Some(2)); assert_eq!(body.col_spacing, Some(12.0)); assert_eq!(body.vert, Some(TextDir::Vertical270)); diff --git a/crates/render/src/lib.rs b/crates/render/src/lib.rs index 926bb762..93b0dc95 100644 --- a/crates/render/src/lib.rs +++ b/crates/render/src/lib.rs @@ -222,6 +222,9 @@ pub struct RenderOpts<'a> { pub skip: &'a [ShapeId], /// Hide the text of this shape (the UI draws it while editing). pub skip_text: Option, + /// Text being edited: replace its prompt with gray markers on empty list paragraphs. + /// Ignored outside editor rendering. + pub editing_text: Option, /// Animated GIF pictures and how far into their play they are (seconds); others show frame 1. pub gif_times: &'a [(ShapeId, f64)], /// Worker threads (0 = this thread; required when effects use filters). @@ -246,6 +249,7 @@ impl Default for RenderOpts<'_> { state: None, skip: &[], skip_text: None, + editing_text: None, gif_times: &[], threads: 0, offset: (0.0, 0.0), @@ -921,7 +925,7 @@ impl Renderer { } ShapeKind::Table(t) => { // Rows grow with their text: what the cells cover, borders included. - let cells = table::draw(ctx, f.pres, rctx, t, m, x.w, x.h, &f.fields); + let cells = table::draw(ctx, f.pres, rctx, t, m, x.w, x.h, &f.fields, f.opts.edit && f.opts.editing_text == Some(s.id)); self.track(m, cells.unwrap_or(Rect::new(0.0, 0.0, x.w, x.h)).inflate(2.0, 2.0)); } ShapeKind::Chart(c) => { @@ -1300,8 +1304,21 @@ impl Renderer { &text_geo }; let tr = geo.text_rect; - if !body.is_empty() { - let l = deckcraft_text::layout(rctx, s, body, &Opts { rect: tr, fields: &f.fields, prompt_color: None, no_shrink: false }); + let editing = f.opts.edit && f.opts.editing_text == Some(s.id); + if !body.is_empty() || editing { + // Imported placeholders can have no paragraph until the first keystroke. + let mut empty; + let body = if editing && body.paragraphs.is_empty() { + empty = body.clone(); + empty.paragraphs.push(Default::default()); + &empty + } else { + body + }; + let mut l = deckcraft_text::layout(rctx, s, body, &Opts { rect: tr, fields: &f.fields, prompt_color: None, no_shrink: false }); + if editing { + preview_empty_bullets(&mut l); + } let tm = text_transform(m, tr, l.rotation); for b in l.lines.iter().filter_map(line_bounds) { // Glyphs reach a little past their carets and line box (italics, swashes). @@ -2034,6 +2051,15 @@ pub fn draw_text(ctx: &mut RenderContext, rctx: &Ctx, s: &Shape, body: &TextBody draw_layout(ctx, &l, tr, m); } +/// Add transient list markers to an editor layout, using the document's own bullet glyphs. +fn preview_empty_bullets(l: &mut deckcraft_text::TextLayout) { + for mut run in l.empty_bullets.drain(..) { + run.color = Rgba::rgb(0xB3, 0xB3, 0xB3); + run.alpha = 1.0; + l.runs.push(run); + } +} + /// Where text laid out in text rect `tr` is drawn, for a shape drawn with `m` (shape-local → /// output): `m`, without mirroring, then the layout's `rotation` about the rect's centre. /// diff --git a/crates/render/src/paint.rs b/crates/render/src/paint.rs index 8011f1bb..ee572fd9 100644 --- a/crates/render/src/paint.rs +++ b/crates/render/src/paint.rs @@ -188,16 +188,6 @@ pub fn picture(ctx: &mut RenderContext, pres: &Presentation, pf: &PictureFill, b return; }; let px_scale = crate::scale_of(m); - let want = (bounds.width().max(bounds.height()) * px_scale).max(1.0); - let frame = gif_time.and_then(|t| images::gif_pixmap(&item.data, &pf.adjust, t)); - let Some(pm) = frame.or_else(|| images::decode_for(&item.data, want, &pf.adjust)) else { - missing(ctx, bounds); - return; - }; - let (iw, ih) = (pm.width() as f64, pm.height() as f64); - if iw < 1.0 || ih < 1.0 { - return; - } let dest = match &pf.mode { PictureMode::Stretch { fill_rect } => { let [l, t, r, b] = *fill_rect; @@ -211,6 +201,18 @@ pub fn picture(ctx: &mut RenderContext, pres: &Presentation, pf: &PictureFill, b } PictureMode::Tile { .. } => bounds, }; + // Decode for the whole image's size on screen, not the box's: a cropped picture shows only part + // of it, magnified (#762). + let want = (dest.width().abs().max(dest.height().abs()) * px_scale).max(1.0); + let frame = gif_time.and_then(|t| images::gif_pixmap(&item.data, &pf.adjust, t)); + let Some(pm) = frame.or_else(|| images::decode_for(&item.data, want, &pf.adjust)) else { + missing(ctx, bounds); + return; + }; + let (iw, ih) = (pm.width() as f64, pm.height() as f64); + if iw < 1.0 || ih < 1.0 { + return; + } let alpha = pf.alpha.unwrap_or(1.0).clamp(0.0, 1.0); let image = vello_cpu::Image { image: vello_cpu::ImageSource::Pixmap(pm.clone()), diff --git a/crates/render/src/table.rs b/crates/render/src/table.rs index 545d060e..498daf91 100644 --- a/crates/render/src/table.rs +++ b/crates/render/src/table.rs @@ -327,6 +327,7 @@ pub fn draw( _w: f64, _h: f64, fields: &dyn deckcraft_text::Fields, + editing: bool, ) -> Option { let _ = pres; let rects = cell_rects(t, &row_heights(rctx, t, fields)); @@ -418,14 +419,18 @@ pub fn draw( // Text. for (r, c, rect) in &rects { let Some(cell) = t.cell(*r, *c) else { continue }; - if cell.text.is_empty() || text_off_target(cell, *rect, m, (ctx.width(), ctx.height())) { + if (cell.text.is_empty() && !editing) || text_off_target(cell, *rect, m, (ctx.width(), ctx.height())) { continue; } let body = cell_body(rctx, t, *r, *c, cell); let tmp = Shape { text: Some(body.clone()), ..Default::default() }; #[cfg(test)] TEXT_CELLS_DRAWN.with(|n| n.set(n.get() + 1)); - crate::draw_text(ctx, rctx, &tmp, &body, *rect, m, fields, None); + let mut l = deckcraft_text::layout(rctx, &tmp, &body, &deckcraft_text::Opts { rect: *rect, fields, prompt_color: None, no_shrink: false }); + if editing { + crate::preview_empty_bullets(&mut l); + } + crate::draw_layout(ctx, &l, *rect, m); let _ = peniko::Fill::NonZero; } covered diff --git a/crates/render/src/tests.rs b/crates/render/src/tests.rs index c85a1e29..05d7c91a 100644 --- a/crates/render/src/tests.rs +++ b/crates/render/src/tests.rs @@ -2509,3 +2509,62 @@ fn single_series_column_chart_varies_colours_by_point() { }; assert_ne!(render(true), render(false)); } + +/// A 1024×1024 PNG of 2 px wide black and white vertical stripes. +fn fine_stripes_png() -> Vec { + let img = + image::RgbaImage::from_fn(1024, 1024, |x, _| if (x / 2) % 2 == 0 { image::Rgba([0, 0, 0, 255]) } else { image::Rgba([255, 255, 255, 255]) }); + let mut out = std::io::Cursor::new(Vec::new()); + image::DynamicImage::ImageRgba8(img).write_to(&mut out, image::ImageFormat::Png).expect("png"); + out.into_inner() +} + +#[test] +fn a_cropped_picture_is_decoded_at_the_resolution_of_the_whole_image() { + // The top-left sixteenth of the image in a 64 pt box: 64 image pixels over 64 output pixels. + // Decoding at the box's size instead of the whole image's blurs the stripes to grey (#762). + let mut p = Presentation::default(); + let media = p.add_media("stripes.png", "image/png", fine_stripes_png()).expect("media"); + let fill = deckcraft_model::style::PictureFill { media, crop: [0.0, 0.0, 15.0 / 16.0, 15.0 / 16.0], ..Default::default() }; + let sh = Shape { id: ShapeId(600), xfrm: Some(Xfrm::new(100.0, 100.0, 64.0, 64.0)), kind: ShapeKind::Picture { fill }, ..Default::default() }; + std::sync::Arc::make_mut(&mut p.slides[0]).shapes = vec![sh]; + let img = render_slide(&p, 0, &RenderOpts { scale: 1.0, ..Default::default() }); + let (mut sharp, mut total) = (0, 0); + for y in 110..154 { + for x in 110..154 { + let v = img.pixel(x, y)[0]; + total += 1; + if !(48..=208).contains(&v) { + sharp += 1; + } + } + } + assert!(sharp * 10 >= total * 9, "the stripes stay sharp: {sharp} of {total} pixels black or white"); +} + +#[test] +fn empty_bullet_is_gray_only_while_editing_and_never_in_exports() { + use deckcraft_model::text::{Bullet, Paragraph}; + let mut para = Paragraph::default(); + para.props.bullet = Some(Bullet::Char { char: "•".into() }); + para.props.margin_left = Some(24.0); + para.props.indent = Some(-24.0); + para.end_props.size = Some(32.0); + let sh = Shape { + id: ShapeId(901), + xfrm: Some(Xfrm::new(100.0, 100.0, 400.0, 120.0)), + text: Some(TextBody { paragraphs: vec![para], ..Default::default() }), + ..Default::default() + }; + let p = deck_with(vec![sh]); + let render = |edit, editing_text| render_slide(&p, 0, &RenderOpts { edit, editing_text, ..Default::default() }); + let plain = render(false, None); + assert_eq!(plain.pixels, render(true, None).pixels); + assert_eq!(plain.pixels, render(false, Some(ShapeId(901))).pixels, "export ignores editor markers"); + assert_eq!(plain.pixels, render(true, Some(ShapeId(902))).pixels, "other shapes stay empty"); + let editing = render(true, Some(ShapeId(901))); + let changed: Vec<_> = editing.pixels.as_chunks::<4>().0.iter().zip(plain.pixels.as_chunks::<4>().0).filter(|(a, b)| a != b).collect(); + assert!(!changed.is_empty(), "an empty bullet must be visible"); + assert!(changed.iter().any(|(px, _)| px[0] == 179 && px[1] == 179 && px[2] == 179)); + assert!(changed.iter().all(|(px, _)| px[0] >= 179 && px[0] == px[1] && px[1] == px[2])); +} diff --git a/crates/render/src/transition.rs b/crates/render/src/transition.rs index 05851448..bd6e3317 100644 --- a/crates/render/src/transition.rs +++ b/crates/render/src/transition.rs @@ -105,6 +105,7 @@ fn copy_opts<'a>(o: &RenderOpts<'a>) -> RenderOpts<'a> { state: o.state, skip: o.skip, skip_text: o.skip_text, + editing_text: o.editing_text, gif_times: o.gif_times, threads: o.threads, offset: o.offset, diff --git a/crates/text/src/lib.rs b/crates/text/src/lib.rs index 840b7834..9acc26c0 100644 --- a/crates/text/src/lib.rs +++ b/crates/text/src/lib.rs @@ -292,6 +292,9 @@ pub struct LineInfo { #[derive(Clone, Debug, Default)] pub struct TextLayout { pub runs: Vec, + /// Empty list markers for the editor only; ordinary rendering and export omit them. + /// Their space is still reserved so the caret starts where the first character will. + pub empty_bullets: Vec, pub decos: Vec, pub lines: Vec, /// Inner text box (insets applied), shape-local. @@ -739,56 +742,52 @@ fn shape_para(ctx: &Ctx, shape: &Shape, body: &TextBody, pi: usize, scale: f64, let lead_size = lead_style.size; let lead_face = lead_style.face.clone(); // Bullet. - let bullet = if para.is_empty() && opts.prompt_color.is_none() { - None - } else { - match &props.bullet { - Some(Bullet::Char { char }) => { - let mut rp = lead.clone(); - if let Some(f) = &props.bullet_font { - rp.font = Some(f.clone()); - } - rp.bold = Some(false); - rp.italic = Some(false); - if let Some(c) = &props.bullet_color { - rp.fill = Some(Fill::solid(c.clone())); - } - rp.size = Some(lead.size.unwrap_or(18.0) * props.bullet_size.unwrap_or(1.0)); - rp.underline = None; - rp.strike = None; - rp.highlight = None; - let mut st = make_style(ctx, &rp, scale, opts.prompt_color); - st.baseline = 0.0; - // Symbol fonts (Wingdings) aren't available: map common private-use bullets. - let ch = map_symbol_bullet(char, props.bullet_font.as_deref()); - bullet_face(db, &mut st, &ch); + let bullet = match &props.bullet { + Some(Bullet::Char { char }) => { + let mut rp = lead.clone(); + if let Some(f) = &props.bullet_font { + rp.font = Some(f.clone()); + } + rp.bold = Some(false); + rp.italic = Some(false); + if let Some(c) = &props.bullet_color { + rp.fill = Some(Fill::solid(c.clone())); + } + rp.size = Some(lead.size.unwrap_or(18.0) * props.bullet_size.unwrap_or(1.0)); + rp.underline = None; + rp.strike = None; + rp.highlight = None; + let mut st = make_style(ctx, &rp, scale, opts.prompt_color); + st.baseline = 0.0; + // Symbol fonts (Wingdings) aren't available: map common private-use bullets. + let ch = map_symbol_bullet(char, props.bullet_font.as_deref()); + bullet_face(db, &mut st, &ch); - // A symbol font (Wingdings, Symbol…) that is installed draws its own glyph, found - // at U+F000 + the character code as such fonts map them; otherwise common symbol - // bullets are mapped to Unicode look-alikes. - let ch = match symbol_font_char(&st.face, char, props.bullet_font.as_deref()) { - Some(c) => c.to_string(), - None => map_symbol_bullet(char, props.bullet_font.as_deref()), - }; - if !st.face.covers(ch.chars().next().unwrap_or('•')) { - st.face = db.face("Inter", "Regular"); - } - Some((ch, st)) + // A symbol font (Wingdings, Symbol…) that is installed draws its own glyph, found + // at U+F000 + the character code as such fonts map them; otherwise common symbol + // bullets are mapped to Unicode look-alikes. + let ch = match symbol_font_char(&st.face, char, props.bullet_font.as_deref()) { + Some(c) => c.to_string(), + None => map_symbol_bullet(char, props.bullet_font.as_deref()), + }; + if !st.face.covers(ch.chars().next().unwrap_or('•')) { + st.face = db.face("Inter", "Regular"); } - Some(Bullet::AutoNum { scheme, start_at }) => { - let mut rp = lead.clone(); - if let Some(c) = &props.bullet_color { - rp.fill = Some(Fill::solid(c.clone())); - } - rp.size = Some(lead.size.unwrap_or(18.0) * props.bullet_size.unwrap_or(1.0)); - rp.underline = None; - let mut st = make_style(ctx, &rp, scale, opts.prompt_color); - let label = autonum(scheme, start_at.saturating_sub(1).saturating_add(number.unwrap_or(1))); - bullet_face(db, &mut st, &label); - Some((label, st)) - } - _ => None, + Some((ch, st)) } + Some(Bullet::AutoNum { scheme, start_at }) => { + let mut rp = lead.clone(); + if let Some(c) = &props.bullet_color { + rp.fill = Some(Fill::solid(c.clone())); + } + rp.size = Some(lead.size.unwrap_or(18.0) * props.bullet_size.unwrap_or(1.0)); + rp.underline = None; + let mut st = make_style(ctx, &rp, scale, opts.prompt_color); + let label = autonum(scheme, start_at.saturating_sub(1).saturating_add(number.unwrap_or(1))); + bullet_face(db, &mut st, &label); + Some((label, st)) + } + _ => None, }; ParaShaped { cells, styles, faces, props, lead_size, lead_face, bullet, level: para.level, rtl } } @@ -1549,18 +1548,21 @@ pub fn layout(ctx: &Ctx, shape: &Shape, body: &TextBody, opts: &Opts) -> TextLay let autofit = bp.autofit.unwrap_or(AutoFit::None); // Shrink on overflow starts from the scale the file was saved with (`fontScale`, // `lnSpcReduction`), which is what PowerPoint draws; editing the text resets it. - let (scale0, reduce0) = match (autofit, opts.no_shrink) { + let (scale0, reduce0, no_saved_scale) = match (autofit, opts.no_shrink) { (AutoFit::Shrink { font_scale, line_reduction }, false) => ( - if font_scale.is_finite() { font_scale.clamp(0.1, 1.0) } else { 1.0 }, + font_scale.filter(|s| s.is_finite()).map(|s| s.clamp(0.1, 1.0)).unwrap_or(1.0), if line_reduction.is_finite() { line_reduction.clamp(0.0, 0.2) } else { 0.0 }, + font_scale.is_none(), ), - _ => (1.0, 0.0), + _ => (1.0, 0.0, false), }; let mut l = layout_scaled(ctx, shape, body, &bp, opts, scale0, reduce0); // A scale PowerPoint saved is drawn as it is: PowerPoint fits the text again only when it // changes (and editing resets the saved scale), not when a file opens, even where our // metrics would call it overflowing (trailing empty paragraphs, substituted fonts). - let saved = scale0 < 1.0 || reduce0 > 0.0; + // A body whose file has no `fontScale` at all (``) is the schema default + // 100 % and is drawn as it is too, not refitted on open (#724). + let saved = no_saved_scale || scale0 < 1.0 || reduce0 > 0.0; if let (AutoFit::Shrink { .. }, false) = (autofit, opts.no_shrink) && l.overflow && !saved @@ -1654,6 +1656,10 @@ fn layout_scaled(ctx: &Ctx, shape: &Shape, body: &TextBody, bp: &BodyProps, opts *deeper = 0; } numbering.get(lvl).copied() + } else if matches!(pp.bullet, Some(Bullet::AutoNum { .. })) { + // Preview the next number without consuming it: empty paragraphs do not advance + // numbering in the saved document or affect later populated paragraphs. + numbering.get(lvl).map(|n| n.saturating_add(1)) } else { if !para.is_empty() { for n in numbering.iter_mut().skip(lvl) { @@ -1908,9 +1914,20 @@ fn layout_scaled(ctx: &Ctx, shape: &Shape, body: &TextBody, bp: &BodyProps, opts // PowerPoint draws justified and distributed anchoring at the bottom. Anchor::Bottom | Anchor::Justified | Anchor::Distributed => inner.height() - used_h, }; - // anchorCtr: centre the block horizontally. - let dx = if bp.anchor_ctr.unwrap_or(false) { ((inner.width() - out.content_width) / 2.0).max(0.0) } else { 0.0 }; - let ox = inner.x0 + dx; + // anchorCtr: centre the drawn text block on the box. Lines sit where their alignment put + // them, so the origin is set so the block's centre lands on the box's centre — a left-edge + // offset alone pushed right-aligned lines out of the box (#736). Empty lines carry no ink + // and stay out of the block. Overflowing content centres symmetrically past both edges. + // Not applied to vertical text (there the offset would run along the wrong axis) or + // multi-column bodies (centring unimplemented for them). + let centre_block = bp.anchor_ctr.unwrap_or(false) && ncols == 1 && matches!(vert, TextDir::Horizontal); + let ox = if centre_block && !all_lines.is_empty() { + let ink = all_lines.iter().filter(|(li, ..)| li.end > li.start); + let (bmin, bmax) = ink.fold((f64::INFINITY, f64::NEG_INFINITY), |(a, b), (li, ..)| (a.min(li.extent.0), b.max(li.extent.1))); + if bmax >= bmin { inner.x0 + inner.width() / 2.0 - (bmin + bmax) / 2.0 } else { inner.x0 } + } else { + inner.x0 + }; let oy = inner.y0 + dy; // Emit runs, in visual order. for (line, place, _, bullet) in all_lines.iter_mut() { @@ -1930,7 +1947,7 @@ fn layout_scaled(ctx: &Ctx, shape: &Shape, body: &TextBody, bp: &BodyProps, opts && let Some((_, st)) = p.bullet.as_ref() { let (_, g) = measure_str(&st.face, st.size, &s); - out.runs.push(GlyphRun { + let run = GlyphRun { face: st.face.clone(), size: st.size, color: st.color, @@ -1946,7 +1963,12 @@ fn layout_scaled(ctx: &Ctx, shape: &Shape, body: &TextBody, bp: &BodyProps, opts gradient: None, effects: None, upright: false, - }); + }; + if body.paragraphs.get(line.para).is_some_and(|p| p.is_empty()) && opts.prompt_color.is_none() { + out.empty_bullets.push(run); + } else { + out.runs.push(run); + } } // Group visually consecutive cells by (style, face). let mut cur: Option = None; diff --git a/crates/text/src/tests.rs b/crates/text/src/tests.rs index 9eb20142..a9d936cd 100644 --- a/crates/text/src/tests.rs +++ b/crates/text/src/tests.rs @@ -186,26 +186,38 @@ fn saved_shrink_scale_is_drawn() { // A file saved with fontScale 62.5% and lnSpcReduction 20% draws at that scale even though // the text would fit at full size, as PowerPoint draws it. let (p, mut sh) = setup("Short", 600.0, 300.0); - sh.text.as_mut().unwrap().body.autofit = Some(AutoFit::Shrink { font_scale: 0.625, line_reduction: 0.2 }); + sh.text.as_mut().unwrap().body.autofit = Some(AutoFit::Shrink { font_scale: Some(0.625), line_reduction: 0.2 }); let l = lay(&p, &sh, 600.0, 300.0); assert_eq!((l.font_scale, l.line_reduction), (0.625, 0.2)); // Text our metrics find overflowing at the saved scale is still drawn at it, as PowerPoint // draws it until the text changes; with no saved scale it shrinks to fit. let text = (0..30).map(|i| format!("Line number {i}")).collect::>().join("\n"); let (p, mut sh) = setup(&text, 600.0, 200.0); - sh.text.as_mut().unwrap().body.autofit = Some(AutoFit::Shrink { font_scale: 0.9, line_reduction: 0.1 }); + sh.text.as_mut().unwrap().body.autofit = Some(AutoFit::Shrink { font_scale: Some(0.9), line_reduction: 0.1 }); let l = lay(&p, &sh, 600.0, 200.0); assert_eq!((l.font_scale, l.line_reduction), (0.9, 0.1)); - sh.text.as_mut().unwrap().body.autofit = Some(AutoFit::Shrink { font_scale: 1.0, line_reduction: 0.0 }); + sh.text.as_mut().unwrap().body.autofit = Some(AutoFit::Shrink { font_scale: Some(1.0), line_reduction: 0.0 }); let l = lay(&p, &sh, 600.0, 200.0); assert!(l.font_scale < 0.9, "{}", l.font_scale); // Nonsense saved values are ignored. let (p, mut sh) = setup("Short", 600.0, 300.0); - sh.text.as_mut().unwrap().body.autofit = Some(AutoFit::Shrink { font_scale: f64::NAN, line_reduction: 5.0 }); + sh.text.as_mut().unwrap().body.autofit = Some(AutoFit::Shrink { font_scale: Some(f64::NAN), line_reduction: 5.0 }); let l = lay(&p, &sh, 600.0, 300.0); assert_eq!((l.font_scale, l.line_reduction), (1.0, 0.2)); } +/// A body whose file has `` with no `fontScale` is the schema default 100 %: +/// PowerPoint draws it as it is, even overflowing. DeckCraft draws it the same (#724). +#[test] +fn bare_normautofit_draws_at_full_size() { + let text = (0..30).map(|i| format!("Line number {i}")).collect::>().join("\n"); + let (p, mut sh) = setup(&text, 600.0, 200.0); + sh.text.as_mut().unwrap().body.autofit = Some(AutoFit::Shrink { font_scale: None, line_reduction: 0.0 }); + let l = lay(&p, &sh, 600.0, 200.0); + assert_eq!(l.font_scale, 1.0, "a body with no saved fontScale draws at 100%"); + assert!(l.overflow, "it overflows, as PowerPoint draws it"); +} + #[test] fn exact_line_spacing_ends_at_the_last_descent() { // 12 pt text at exactly 26.59 pt per line (a table cell PowerPoint sizes to 24.3 pt of text): @@ -1333,3 +1345,125 @@ fn stacked_combining_marks_take_a_face_that_draws_them() { } let _ = f; } + +/// #736: `anchorCtr` centres the text block horizontally, whatever the paragraphs' own +/// alignment, and overflowing content centres symmetrically past both edges. +#[test] +fn anchorctr_centres_the_text_block() { + let text = "The shape is top - center - overriding paragraph right alignment."; + // The block's centre: (min left + max right) over the laid-out lines, in box coordinates. + let block_centre = |l: &TextLayout| -> f64 { + // Empty lines carry no ink and sit where their alignment left them: not part of the block. + let ink = l.lines.iter().filter(|li| li.end > li.start); + let (min, max) = ink.fold((f64::INFINITY, f64::NEG_INFINITY), |(a, b), li| (a.min(li.extent.0), b.max(li.extent.1))); + (min + max) / 2.0 + }; + let para = |align: Option, text: &str| Paragraph { + props: ParaProps { align, ..Default::default() }, + runs: vec![Run::new(text)], + ..Default::default() + }; + let anchored = + |paragraphs: Vec| TextBody { body: BodyProps { anchor_ctr: Some(true), ..Default::default() }, paragraphs, ..Default::default() }; + // One right-aligned line in a wide box: the block is centred, not pushed past the right edge. + let body_right = anchored(vec![para(Some(Align::Right), text)]); + let (p, mut sh) = setup("", 828.0, 104.0); + sh.text = Some(body_right); + let l = lay(&p, &sh, 828.0, 104.0); + assert!((block_centre(&l) - 414.0).abs() <= 1.0, "right-aligned: centre {}, want 414", block_centre(&l)); + // Left-aligned text centres the same way. + let body_left = anchored(vec![para(None, text)]); + let (p, mut sh) = setup("", 828.0, 104.0); + sh.text = Some(body_left); + let l = lay(&p, &sh, 828.0, 104.0); + assert!((block_centre(&l) - 414.0).abs() <= 1.0, "left-aligned: centre {}, want 414", block_centre(&l)); + // A trailing empty paragraph carries no ink and must not skew the block. + let body_empty = anchored(vec![para(Some(Align::Right), text), para(None, "")]); + let (p, mut sh) = setup("", 828.0, 104.0); + sh.text = Some(body_empty); + let l = lay(&p, &sh, 828.0, 104.0); + assert!((block_centre(&l) - 414.0).abs() <= 1.0, "trailing empty paragraph: centre {}, want 414", block_centre(&l)); + // Mixed alignments: the drawn block (not either alignment edge) is what centres. + let body_mixed = anchored(vec![para(Some(Align::Right), text), para(None, "short")]); + let (p, mut sh) = setup("", 828.0, 104.0); + sh.text = Some(body_mixed); + let l = lay(&p, &sh, 828.0, 104.0); + assert!((block_centre(&l) - 414.0).abs() <= 1.0, "mixed: centre {}, want 414", block_centre(&l)); + // Content wider than the box overflows symmetrically past both edges (no wrap). + let body_wide = TextBody { + body: BodyProps { anchor_ctr: Some(true), wrap: Some(false), ..Default::default() }, + paragraphs: vec![para(Some(Align::Right), text)], + ..Default::default() + }; + let (p, mut sh) = setup("", 300.0, 104.0); + sh.text = Some(body_wide); + let l = lay(&p, &sh, 300.0, 104.0); + assert!((block_centre(&l) - 150.0).abs() <= 2.0, "overflowing: centre {}, want 150", block_centre(&l)); + // Control: without anchorCtr a right-aligned line still ends at the box's right edge. + let body_control = TextBody { body: BodyProps::default(), paragraphs: vec![para(Some(Align::Right), text)], ..Default::default() }; + let (p, mut sh) = setup(text, 828.0, 104.0); + sh.text = Some(body_control); + let l = lay(&p, &sh, 828.0, 104.0); + assert!( + l.lines.iter().all(|li| li.extent.1 > 810.0), + "control: right-aligned at the box edge, got {:?}", + l.lines.iter().map(|li| li.extent).collect::>() + ); +} + +#[test] +fn empty_bullets_reserve_the_typed_indent_without_becoming_content() { + for rtl in [false, true] { + for level in [0, 1, 2] { + let (p, mut sh) = setup("First\n", 600.0, 300.0); + for para in &mut sh.text.as_mut().unwrap().paragraphs { + para.level = level; + para.props.rtl = Some(rtl); + para.props.align = Some(if rtl { Align::Right } else { Align::Left }); + } + let empty = lay(&p, &sh, 600.0, 300.0); + assert_eq!(empty.empty_bullets.len(), 1); + assert_eq!(empty.empty_bullets[0].para, 1); + assert!(empty.runs.iter().all(|r| r.para == 0), "empty bullet must not be exported"); + let caret = empty.caret(Pos { para: 1, ch: 0 }).unwrap(); + let bx = empty.empty_bullets[0].glyphs[0].1; + assert!(if rtl { bx >= caret.0 } else { bx < caret.0 }); + deckcraft_model::edit::insert(sh.text.as_mut().unwrap(), (1, 0), if rtl { "אבג" } else { "Next" }, None); + let typed = lay(&p, &sh, 600.0, 300.0); + assert!(typed.empty_bullets.is_empty()); + let after = typed.caret(Pos { para: 1, ch: 0 }).unwrap(); + assert!( + (caret.0 - after.0).abs() < 1e-6 && (caret.1 - after.1).abs() < 1e-6 && (caret.2 - after.2).abs() < 1e-6, + "typing must not move the line's start: {caret:?} vs {after:?}" + ); + } + } +} + +#[test] +fn empty_numbered_paragraph_previews_the_next_number_without_consuming_it() { + let (p, mut sh) = setup("One\n\nTwo", 600.0, 300.0); + for para in &mut sh.text.as_mut().unwrap().paragraphs { + para.props.bullet = Some(Bullet::AutoNum { scheme: "arabicPeriod".into(), start_at: 4 }); + } + let l = lay(&p, &sh, 600.0, 300.0); + let preview = &l.empty_bullets[0]; + let next = l.runs.iter().find(|r| r.para == 2 && r.cells.is_empty()).unwrap(); + let ids = |r: &GlyphRun| r.glyphs.iter().map(|g| g.0).collect::>(); + assert_eq!(preview.para, 1); + assert_eq!(ids(preview), ids(next), "both should use 5."); + let expected = measure_str(&preview.face, preview.size, "5.").1; + assert_eq!(ids(preview), expected.iter().map(|g| g.0).collect::>()); +} + +#[test] +fn empty_plain_paragraphs_and_soft_breaks_have_no_bullet_preview() { + let (p, mut sh) = setup("", 600.0, 300.0); + sh.text.as_mut().unwrap().paragraphs[0].props.bullet = Some(Bullet::None); + assert!(lay(&p, &sh, 600.0, 300.0).empty_bullets.is_empty()); + sh.text = Some(TextBody::from_text("First")); + deckcraft_model::edit::insert(sh.text.as_mut().unwrap(), (0, 5), "\u{b}", None); + let l = lay(&p, &sh, 600.0, 300.0); + assert_eq!(l.lines.len(), 2); + assert!(l.empty_bullets.is_empty(), "a soft line break is not a new list item"); +} diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index bc4e080e..ba66aad8 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -181,20 +181,18 @@ pub fn show(app: &mut SlideApp, ui: &mut Ui) { // One texture pixel in screen points, and the canvas in pixels from the slide's corner. let pixel = vec2(slide_rect.width() / px.0 as f32, slide_rect.height() / px.1 as f32); let visible = visible_region(avail, slide_rect, pixel); + let typing_into = st.and_then(|d| d.selection.text.as_ref()).filter(|t| !t.notes).map(|t| t.shape); let tex = match st.map(|d| (d.selection.target, d)) { Some((Target::Slides, d)) => { let idx = d.selection.slide; let doc = d.doc.clone(); let frame = anim.as_ref().map(|(tl, t)| (tl, *t)); - // 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) + app.textures.slide(ui.ctx(), &doc, idx, px, visible, true, crate::media::gif_key(&gifs), app.ui.grayscale, ×, typing_into, 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), + Some((Target::Master { master }, d)) => master_tex(ui.ctx(), &d.doc, master, None, px, visible, typing_into), + Some((Target::Layout { master, layout }, d)) => master_tex(ui.ctx(), &d.doc, master, Some(layout), px, visible, typing_into), Some((t @ (Target::Handout | Target::NotesMaster), d)) => { - deckcraft_engine::master_page(&d.doc, t).and_then(|p| master_tex(ui.ctx(), &p, 0, None, px, visible)) + deckcraft_engine::master_page(&d.doc, t).and_then(|p| master_tex(ui.ctx(), &p, 0, None, px, visible, typing_into)) } None => None, }; @@ -418,6 +416,7 @@ fn master_tex( layout: Option, px: (u32, u32), visible: deckcraft_render::Region, + editing_text: Option, ) -> Option<(egui::TextureHandle, deckcraft_render::Region)> { let scale = px.0 as f64 / doc.slide_size.width.max(1.0); let region = crate::textures::fit_texture( @@ -429,7 +428,7 @@ fn master_tex( doc, master, layout, - &deckcraft_render::RenderOpts { scale, edit: true, offset: region.offset(), size: Some(region.size()), ..Default::default() }, + &deckcraft_render::RenderOpts { scale, edit: true, editing_text, offset: region.offset(), size: Some(region.size()), ..Default::default() }, ); Some((crate::textures::upload_editor(ctx, "master-canvas", crate::textures::to_color_image(&img, false)), region)) } @@ -1476,6 +1475,61 @@ mod tests { } } + /// The first empty placeholder and the paragraph after Enter keep a list marker and indent. + #[test] + fn empty_bullet_preview_follows_click_type_enter_and_toggle() { + let mut h = harness(); + h.state_mut().run("slide.new", json!({"layout": "titleAndContent"})).unwrap(); + h.run_steps(3); + let st = h.state().session.active().unwrap(); + let sh = st + .shapes() + .iter() + .find(|s| s.ph_type() == Some(deckcraft_model::PhType::Body) || s.ph_type() == Some(deckcraft_model::PhType::Obj)) + .unwrap(); + let id = sh.id; + let x = deckcraft_engine::cmd::xfrm_of(&st.doc, &st.selection, sh); + let p = screen(h.state(), x.x + 40.0, x.y + 20.0); + click(&mut h, p); + h.run_steps(3); + let caret = |h: &Harness<'_, SlideApp>| { + let st = h.state().session.active().unwrap(); + let ts = st.selection.text.as_ref().expect("click starts typing"); + assert_eq!(ts.shape, id); + deckcraft_engine::cmd::text::caret_line(st, ts).unwrap().0 + }; + let first = caret(&h); + h.event(Event::Text("First point".into())); + h.run_steps(3); + h.key_press(egui::Key::Enter); + h.run_steps(3); + let second = caret(&h); + assert!((first.x - second.x).abs() < 0.01, "empty lines retain the bullet indent"); + assert!(second.y > first.y); + let st = h.state().session.active().unwrap(); + let l = deckcraft_engine::cmd::text::layout_for(st, st.selection.text.as_ref().unwrap()).unwrap(); + assert_eq!(l.empty_bullets.len(), 1); + assert_eq!(l.empty_bullets[0].para, 1); + let renders = h.state().textures.canvas_renders; + h.state_mut().run("text.exit", json!({})).unwrap(); + h.run_steps(3); + assert!(h.state().textures.canvas_renders > renders, "leaving text editing refreshes the marker texture"); + h.state_mut().run("text.edit", json!({"id": id, "at": [1, 0]})).unwrap(); + h.run_steps(3); + h.event(Event::Text("Second point".into())); + h.run_steps(3); + let st = h.state().session.active().unwrap(); + let ts = st.selection.text.as_ref().unwrap(); + assert_eq!(deckcraft_engine::cmd::text::body_of(st, ts).unwrap().text(), "First point\nSecond point"); + assert!(deckcraft_engine::cmd::text::layout_for(st, ts).unwrap().empty_bullets.is_empty()); + h.key_press(egui::Key::Enter); + h.run_steps(3); + h.state_mut().run("format.bullets", json!({})).unwrap(); + h.run_steps(3); + let st = h.state().session.active().unwrap(); + assert!(deckcraft_engine::cmd::text::layout_for(st, st.selection.text.as_ref().unwrap()).unwrap().empty_bullets.is_empty()); + } + /// Double-clicking a table cell edits it with the caret drawn in that cell, where clicked. #[test] fn double_click_in_a_table_cell_shows_the_caret_there() { diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index 8f474485..8292321c 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -28,12 +28,34 @@ pub struct ControlRequest { pub method: String, pub params: Value, pub reply: Sender, + /// Who has the request: [`PENDING`], [`TAKEN`] (the app runs it) or [`ABANDONED`] (the + /// sender gave up on it, so the app must not run it late: issue #719). + pub state: std::sync::Arc, } +/// Queued, not yet run. +pub const PENDING: u8 = 0; +/// The app has started it: it will answer. +pub const TAKEN: u8 = 1; +/// Its sender gave up waiting (the reply timeout expired): it is dropped unrun. +pub const ABANDONED: u8 = 2; + impl ControlRequest { pub fn new(method: impl Into, params: Value) -> (Self, std::sync::mpsc::Receiver) { let (tx, rx) = std::sync::mpsc::channel(); - (Self { method: method.into(), params, reply: tx }, rx) + (Self { method: method.into(), params, reply: tx, state: Default::default() }, rx) + } + + /// Claim the request for running; false when its sender already gave up on it. + pub fn take(&self) -> bool { + use std::sync::atomic::Ordering; + self.state.compare_exchange(PENDING, TAKEN, Ordering::SeqCst, Ordering::SeqCst).is_ok() || self.state.load(Ordering::SeqCst) == TAKEN + } + + /// Give up on the request; false when the app has already started it (it will answer). + pub fn abandon(state: &std::sync::atomic::AtomicU8) -> bool { + use std::sync::atomic::Ordering; + state.compare_exchange(PENDING, ABANDONED, Ordering::SeqCst, Ordering::SeqCst).is_ok() || state.load(Ordering::SeqCst) == ABANDONED } } @@ -402,3 +424,24 @@ pub fn save_screenshot(app: &mut SlideApp, image: &egui::ColorImage, path: Optio None => json!({"ok": false, "error": "no writer configured"}), } } + +#[cfg(test)] +mod tests { + #![allow(clippy::unwrap_used, clippy::expect_used)] + + use super::*; + + /// `take` and `abandon` are exclusive: whoever wins, the request runs at most once (#719). + #[test] + fn take_and_abandon_are_exclusive() { + let (req, _rx) = ControlRequest::new("m", json!({})); + assert!(req.take(), "a pending request is taken"); + assert!(!ControlRequest::abandon(&req.state), "a taken request is not abandoned"); + assert!(req.take(), "a taken request stays taken"); + + let (req, _rx) = ControlRequest::new("m", json!({})); + assert!(ControlRequest::abandon(&req.state), "a pending request is abandoned"); + assert!(!req.take(), "an abandoned request is never taken"); + assert!(ControlRequest::abandon(&req.state), "abandoning twice stays abandoned"); + } +} diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 18d112c8..576a2297 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -108,6 +108,9 @@ pub fn show(app: &mut SlideApp, ctx: &egui::Context) { let escape = ctx.input(|i| i.key_pressed(egui::Key::Escape)); if open && !close && !escape { app.dialog = Some(d); + } else if !close && d.id == "preferences" { + // Esc or the window's close button: the same as Cancel. + restore_preferences(app, &d); } else if app.dialog.is_none() && let Some(ret) = d.params.get("returnParams").filter(|v| v.is_object()).cloned() { @@ -137,6 +140,31 @@ fn buttons(ui: &mut Ui, ok_label: &str) -> (bool, bool) { (ok, cancel) } +/// What the Preferences dialog can change, as it was when the dialog opened. +fn preferences_snapshot(app: &SlideApp) -> Value { + json!({ + "dark": app.ui.brightness == theme::Brightness::Dark, + "uiScale": app.ui.ui_scale, + "prefs": serde_json::to_value(&app.session.prefs).unwrap_or(Value::Null), + }) +} + +/// Puts back the settings the Preferences dialog opened with (Cancel, Esc, the close button). +fn restore_preferences(app: &mut SlideApp, d: &Dialog) { + let Some(before) = d.params.get("before") else { return }; + if let Some(dark) = before.get("dark").and_then(Value::as_bool) + && dark != (app.ui.brightness == theme::Brightness::Dark) + { + let _ = app.run("view.dark", json!({ "on": dark })); + } + if let Some(scale) = before.get("uiScale").and_then(Value::as_f64) { + app.ui.ui_scale = scale as f32; + } + if let Some(prefs) = before.get("prefs").and_then(|p| serde_json::from_value(p.clone()).ok()) { + app.session.prefs = prefs; + } +} + fn field(ui: &mut Ui, d: &mut Dialog, label: &str, key: &str, default: &str) { let mut v = d.get(key, default); ui.horizontal(|ui| { @@ -2104,14 +2132,31 @@ fn body(app: &mut SlideApp, ui: &mut Ui, d: &mut Dialog) -> bool { } "gridOptions" => grid_options(app, ui, d), "preferences" => { + // The controls apply as they change, so the dialog remembers what it opened with and + // Cancel (or Esc, or the close button: see `show`) puts that back. + if d.params.get("before").is_none() { + d.params = json!({ "before": preferences_snapshot(app) }); + } ui.label(egui::RichText::new("General").font(theme::bold(13.0))); let mut dark = app.ui.brightness == theme::Brightness::Dark; if ui.checkbox(&mut dark, "Dark appearance").changed() { let _ = app.run("view.dark", json!({"on": dark})); } - let mut scale = app.ui.ui_scale; - if ui.add(egui::Slider::new(&mut scale, 0.75..=1.75).text("Interface size")).changed() { - app.ui.ui_scale = scale; + // While the pointer holds the slider its value only waits in the dialog: applying it + // would rescale the slider under the pointer and run it to an end (#767). It applies + // on release, and at once from the keyboard. + let pending = d.fields.get("uiScale").and_then(|v| v.parse::().ok()); + let mut scale = pending.unwrap_or(app.ui.ui_scale); + let r = ui.add(egui::Slider::new(&mut scale, 0.75..=1.75).text("Interface size")); + if r.is_pointer_button_down_on() { + if r.changed() || pending.is_some() { + d.fields.insert("uiScale".into(), scale.to_string()); + } + } else { + if r.changed() || pending.is_some() { + app.ui.ui_scale = scale; + } + d.fields.remove("uiScale"); } ui.label(egui::RichText::new("Editing").font(theme::bold(13.0))); ui.checkbox(&mut app.session.prefs.smart_guides, "Show smart guides"); @@ -2130,8 +2175,11 @@ fn body(app: &mut SlideApp, ui: &mut Ui, d: &mut Dialog) -> bool { ui.label("Your name (comments):"); ui.text_edit_singleline(&mut app.session.prefs.author); }); - let (ok, _) = buttons(ui, "Done"); - ok + let (ok, cancel) = buttons(ui, "Done"); + if cancel { + restore_preferences(app, d); + } + ok || cancel } "photoAlbum" => photo_album_dialog(app, ui, d), "themeColors" => theme_colors_dialog(app, ui, d), diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index f020f5f5..88674070 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -1879,6 +1879,10 @@ impl SlideApp { fn drain_control(&mut self, ctx: &egui::Context) { let Some(rx) = self.control_rx.take() else { return }; while let Ok(req) = rx.try_recv() { + // A request whose sender gave up waiting (reply timeout) must not run late (#719). + if !req.take() { + continue; + } let reply = req.reply.clone(); match control::handle(self, ctx, &req) { control::Outcome::Done(v) => { @@ -2573,6 +2577,71 @@ mod tests { assert!((harness.state().session.prefs.grid_spacing - 6.0).abs() < 1e-9); } + /// Preferences (#752): Cancel, Esc and the window's close button put back what the dialog + /// changed while it was open; Done keeps it. + #[test] + fn preferences_cancel_restores_the_settings_done_keeps_them() { + use egui_kittest::kittest::Queryable; + 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::with_new(), Services::default()), + ); + harness.run_steps(2); + let before = harness.state().session.prefs.clone(); + let dark_before = harness.state().ui.brightness; + let change = |harness: &mut egui_kittest::Harness<'static, SlideApp>| { + harness.state_mut().run("app.preferences", json!({})).unwrap(); + harness.run_steps(2); + for label in ["Dark appearance", "Show smart guides", "Snap objects to grid"] { + harness + .query_all_by_role_and_label(egui::accesskit::Role::CheckBox, label) + .next() + .unwrap_or_else(|| panic!("no {label} checkbox")) + .click_accesskit(); + harness.run_steps(2); + } + assert_ne!(harness.state().ui.brightness, dark_before, "the dialog previews the appearance"); + assert_ne!(harness.state().session.prefs.smart_guides, before.smart_guides); + }; + let unchanged = |harness: &egui_kittest::Harness<'static, SlideApp>, how: &str| { + let s = harness.state(); + assert!(s.dialog.is_none(), "{how} closes Preferences"); + assert_eq!(s.ui.brightness, dark_before, "{how} restores the appearance"); + assert_eq!(s.session.prefs.smart_guides, before.smart_guides, "{how} restores smart guides"); + assert_eq!(s.session.prefs.snap_to_grid, before.snap_to_grid, "{how} restores snap to grid"); + }; + + change(&mut harness); + harness + .query_all_by_role_and_label(egui::accesskit::Role::Button, "Cancel") + .next() + .unwrap_or_else(|| panic!("no Cancel button")) + .click_accesskit(); + harness.run_steps(2); + unchanged(&harness, "Cancel"); + + change(&mut harness); + harness.key_press(egui::Key::Escape); + harness.run_steps(2); + unchanged(&harness, "Esc"); + + change(&mut harness); + harness + .query_all_by_role_and_label(egui::accesskit::Role::Button, "Done") + .next() + .unwrap_or_else(|| panic!("no Done button")) + .click_accesskit(); + harness.run_steps(2); + let s = harness.state(); + assert!(s.dialog.is_none(), "Done closes Preferences"); + assert_ne!(s.ui.brightness, dark_before, "Done keeps the appearance"); + assert_ne!(s.session.prefs.smart_guides, before.smart_guides, "Done keeps smart guides"); + assert_ne!(s.session.prefs.snap_to_grid, before.snap_to_grid, "Done keeps snap to grid"); + } + fn canvas_harness() -> egui_kittest::Harness<'static, SlideApp> { let mut harness = egui_kittest::Harness::builder().with_size(egui::vec2(1200.0, 800.0)).build_ui_state( |ui, app: &mut SlideApp| { @@ -3253,4 +3322,38 @@ mod tests { flick(&mut harness, egui::vec2(-24.0, 0.0)); assert_eq!(harness.state().show.as_ref().map(|s| s.state.slide), Some(0)); } + + /// Preferences › Interface size (#767): dragging the slider leaves the interface size alone + /// until the pointer lets go, so the slider doesn't grow under the pointer mid-drag. + #[test] + fn interface_size_applies_when_the_slider_is_let_go() { + 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::with_new(), Services::default()), + ); + harness.run_steps(2); + harness.state_mut().run("app.preferences", json!({})).unwrap(); + harness.run_steps(4); + let before = harness.state().ui.ui_scale; + let rect = + harness.query_all_by_role_and_label(egui::accesskit::Role::Slider, "Interface size").next().expect("the Interface size slider").rect(); + let (from, to) = (rect.left_center() + egui::vec2(4.0, 0.0), rect.right_center() - egui::vec2(4.0, 0.0)); + harness.hover_at(from); + harness.run_steps(1); + harness.drag_at(from); + harness.run_steps(1); + for k in 1..=8 { + harness.hover_at(from.lerp(to, k as f32 / 8.0)); + harness.run_steps(1); + } + assert_eq!(harness.state().ui.ui_scale, before, "nothing rescales while the slider is held"); + harness.drop_at(to); + harness.run_steps(2); + let after = harness.state().ui.ui_scale; + assert!(after > before + 0.3, "letting go applies the size dragged to: {before} -> {after}"); + assert!(harness.state().dialog.is_some(), "Preferences stays open"); + } } diff --git a/crates/ui-egui/src/textures.rs b/crates/ui-egui/src/textures.rs index 9730f4d6..49df4144 100644 --- a/crates/ui-egui/src/textures.rs +++ b/crates/ui-egui/src/textures.rs @@ -21,7 +21,7 @@ struct Key { extra: u64, /// The pasteboard region the slide's drawing calls for (canvas only). want: Region, - skip_text: Option, + editing_text: Option, } struct Entry { @@ -248,8 +248,7 @@ 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. `skip_text` leaves out one shape's text (the prompt of a placeholder being typed - /// into). + /// GIF frames. `editing_text` replaces its placeholder prompt with gray empty list markers. #[allow(clippy::too_many_arguments)] pub fn slide( &mut self, @@ -262,7 +261,7 @@ impl Textures { extra: u64, grayscale: bool, gif_times: &[(ShapeId, f64)], - skip_text: Option, + editing_text: Option, anim: Option<(&deckcraft_anim::Timeline, f64)>, ) -> Option<(TextureHandle, Region)> { let slide = p.slides.get(index)?.clone(); @@ -290,7 +289,7 @@ impl Textures { edit, extra: extra ^ grayscale as u64 ^ anim_key(anim), want, - skip_text, + editing_text, }; if let Some(e) = &self.canvas && e.key == key @@ -309,7 +308,7 @@ impl Textures { offset: region.offset(), size: Some(region.size()), state, - skip_text, + editing_text, ..Default::default() }; deckcraft_render::render_slide_drawn(p, index, &opts) @@ -367,7 +366,7 @@ impl Textures { edit: false, extra: 0, want: Region::slide(width_px, h), - skip_text: None, + editing_text: None, }; self.clock += 1; let clock = self.clock; diff --git a/docs/control-protocol.md b/docs/control-protocol.md index b7b6f013..e715d1c7 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -3,10 +3,18 @@ Start the app with `--control `. The server listens on `127.0.0.1` only and speaks JSON lines: one request object per line, one reply per line. +The connected MCP client requires the reply's numeric `id` to match its request, `ok` to be a +boolean, and exactly one of `result` (success) or a string `error` (failure). It caps each reply at +16 MiB including its terminating newline, accepts CRLF, and rejects EOF inside a frame. Invalid +replies invalidate the connection and leave the command's outcome unknown; the client does not +replay it. Unsolicited bytes already buffered between calls force a reconnect before sending. + The server bounds what a client can hold: 16 connections at once (more get `{"ok":false,"error":"busy: …"}` and are closed), 16 MiB per request line (longer closes the connection), 64 queued requests (more are answered `busy`), 10 minutes idle and 30 seconds per blocked reply write before the socket is closed, and -60 seconds waiting for the UI (`"error":"timeout"`). +60 seconds waiting for the UI (`"error":"timeout"`). A request still queued when that wait expires is +abandoned: the UI skips it, so a retry cannot run it twice. If the UI had already started it, the reply +says the outcome is unknown instead of `timeout`. ```json {"id": 1, "method": "engine.execute", "params": {"command": "shape.insert", "params": {"preset": "roundRect", "rect": [100, 100, 240, 120], "text": "Hello"}}} diff --git a/docs/gaps.md b/docs/gaps.md index 4adfda40..4e564951 100644 --- a/docs/gaps.md +++ b/docs/gaps.md @@ -355,6 +355,7 @@ Detail in [localization-parity.md](localization-parity.md). | Date | Change | Summary | |---|---|---| +| 2026-10-14 | trivial | Fixed #718: remote MCP no longer accepts unrelated or buffered stale success; loopback regressions cover invalid envelopes, incomplete and oversized frames | | 2026-10-14 | minor | Community PR wave folded: one entry each for the soak tests (engine and UI), the PPTX corpus (seven owned corpus files), edit points / text warp / 3-D, proofing (Hunspell spelling, thesaurus, AutoCorrect), the smaller missing rows, dialogs (41 vs ~60; Print and Insert Object left) and pen/trackpad input; ranked rows 3, 4 and 10 de-duplicated (comment threads #70 and the pane timeline are done); the fixed command-palette entry (#79) removed | | 2026-10-13 | minor | EMF/WMF rectangle clip regions (`INTERSECTCLIPRECT`, `EXCLUDECLIPRECT`, `EXTSELECTCLIPRGN`) limit later drawing. EMF+ records remain | | 2026-10-13 | minor | Embedded fonts are read, written and drawn (TrueType, OpenType, Embedded OpenType). The gap is now MicroType Express subsets, which are kept but not drawn | diff --git a/docs/mcp.md b/docs/mcp.md index b2f879b3..a65fcda3 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -5,6 +5,18 @@ headless engine (no window); with `--connect PORT` it drives a running DeckCraft `--control PORT`, so an agent and a person can work on the same deck. `--sample` opens the sample deck. +Connected mode accepts only newline-terminated replies of at most 16 MiB (including the newline), +with the current request's numeric `id`, a boolean `ok`, and either `result` on success or a string +`error` on failure. CRLF is accepted. Invalid replies close the connection and report that the +command may or may not have run; they never cause a replay. Check the document before retrying a +mutation. Before sending another request, unsolicited bytes in the reader's buffer or on the +socket cause a fresh connection to be opened. Large image replies must fit this budget; render +to a file or use a smaller scale when necessary. + +Regression coverage for #718 lives in `crates/mcp/src/backend.rs`: synthetic loopback peers test +reply correlation, envelopes, framing, the size cap and buffered unsolicited bytes, alongside the +lost-reply and reconnect tests. + ```json { "mcpServers": { diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 9843d75e..94928d8c 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -1,6 +1,6 @@ # PowerPoint feature checklist -> **Generated:** 2026-10-14 (summary by `cargo xtask parity`; rows re-scored by the 2026-10-14 community PR wave, duplicate Thesaurus, AutoCorrect, Artistic effects and Corrections rows folded; Preview in editor re-scored 2026-10-13; earlier rows re-scored 2026-10-10 against PowerPoint for Mac 16.113.4; video/animated GIF row re-scored 2026-10-11, #171) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365) +> **Generated:** 2026-10-14 (bullets/numbering remain D after empty-line marker review; summary by `cargo xtask parity`; rows re-scored by the 2026-10-14 community PR wave, duplicate Thesaurus, AutoCorrect, Artistic effects and Corrections rows folded; Preview in editor re-scored 2026-10-13; earlier rows re-scored 2026-10-10 against PowerPoint for Mac 16.113.4; video/animated GIF row re-scored 2026-10-11, #171) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365) The summary between ` **Weighted breadth parity: 84%** over 191 features. @@ -145,7 +145,7 @@ Open P0 items: Contextual tabs (Shape Format, Picture Format, Table Design/Layou | Text | Change case, grow/shrink font, clear formatting | P1 | D | | Text | Text outline and text effects | P2 | P | | Text | Paragraph alignment, indents, spacing, line spacing | P0 | D | -| Text | Bullets and numbering | P0 | D | +| Text | Bullets and numbering (including gray empty-line markers while editing) | P0 | D | | Text | Picture bullets, custom bullet symbols | P2 | P | | Text | List levels (Tab / Shift+Tab) | P0 | D | | Text | Tab stops | P2 | D | @@ -228,7 +228,7 @@ Open P0 items: Contextual tabs (Shape Format, Picture Format, Table Design/Layou | Undo & clipboard | Unlimited undo/redo | P0 | D | | Undo & clipboard | Cut, copy, paste shapes and slides | P0 | D | | Undo & clipboard | Paste special (keep formatting, picture, text only) | P1 | D | -| Automation (✱) | Every command scriptable (CLI, control channel, MCP) | P0 | D | +| Automation (✱) | Every command scriptable (CLI, control channel, MCP; remote reply IDs, envelopes and framing validated, #718) | P0 | D | | Automation (✱) | Headless rendering and conversion | P0 | D | | Platforms (✱) | macOS, Windows, Linux builds | P0 | P | | Platforms (✱) | Web (WASM) build | P1 | D | diff --git a/docs/roadmap.md b/docs/roadmap.md index 1a6c3910..caf868de 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -115,6 +115,7 @@ plan above. | Date | Change | Summary | |---|---|---| +| 2026-10-14 | trivial | M13 reliability: remote MCP validates IDs and envelopes, caps replies and rejects incomplete frames; #718 | | 2026-10-14 | minor | Community PR wave folded: one Review gate row, one Current focus item 2, one beta row each for items 9 and 10 (timeline and timer controls are in), one after-beta paragraph, one M9 and M10 row, one corpus row in the 2026-10-07 history | | 2026-10-13 | trivial | Beta item 3: embedded fonts are in, except MicroType Express subsets | | 2026-10-13 | minor | M9: the WordArt Transform gallery applies a text warp; adjust handles stay open | diff --git a/docs/ui-parity.md b/docs/ui-parity.md index cc7801d7..cc97dae4 100644 --- a/docs/ui-parity.md +++ b/docs/ui-parity.md @@ -7,6 +7,13 @@ direct manipulation, keyboard and precision. Observations of PowerPoint are kept `plan/powerpoint/01-observed-ui.md` and `menu-tree.txt` (gitignored, clean-room rules in AGENTS.md §2). Dimension score: **~60% ready** (estimated), **30–50 h** to parity. +## Empty list paragraphs + +While editing slide text, empty bulleted and numbered paragraphs show gray markers at the list's +indent, including the first click into a content placeholder and the paragraph after Enter. +Typing gives the marker its normal text color. These temporary markers are hidden when editing +ends and are omitted from thumbnails, slide shows and exports. + ## Shell | Element | PowerPoint for Mac | DeckCraft | Gap | @@ -158,6 +165,7 @@ consistency is open (#73, #56). | Date | Change | Summary | |---|---|---| +| 2026-10-14 | minor | Gray list markers on empty paragraphs while editing; caret keeps the list indent before typing | | 2026-10-14 | minor | Community PR wave folded: one row each for Format Shape, Dialogs (41, listed from `dialogs.rs`), Animation Pane (timeline is in), window, Quick Access Toolbar, Format Background, rulers, image paste, context menus, exact size and edit points; estimates re-totalled (~26–46 h) | | 2026-10-13 | trivial | Eyedropper in the colour menus (#367) | | 2026-10-12 | minor | Title bar: an Export button next to Save opens the Export dialog, a quick-access shortcut beside File › Export | diff --git a/packaging/linux/ai.storyteller.deckcraft.metainfo.xml.in b/packaging/linux/ai.storyteller.deckcraft.metainfo.xml.in index 4e6f0ae5..bb475efd 100644 --- a/packaging/linux/ai.storyteller.deckcraft.metainfo.xml.in +++ b/packaging/linux/ai.storyteller.deckcraft.metainfo.xml.in @@ -37,6 +37,12 @@ slide show pptx + + + Slide editor with thumbnails, a process diagram and notes + https://raw.githubusercontent.com/storytold/deckcraft/main/docs/images/editor.png + + #ffc9a8 #b8471f