From 50d9ad00e607ad892b6a09dcc757cc289bcc6129 Mon Sep 17 00:00:00 2001 From: Gert Pellin Date: Wed, 14 Oct 2026 19:06:35 +0200 Subject: [PATCH 01/16] Slide: a new slide's body goes into the layout's content placeholder slide.new put the body into the first non-title placeholder in shape order, so a layout with a small body placeholder ahead of its content placeholder (as Keynote exports "Title & Bullets") got the body in the small box and left the content placeholder empty. The body now goes to the main content placeholder: idx 1 first, then a full-size placeholder over a half/quarter one, then a text placeholder over a picture/chart/table one, keeping shape order on ties. Fixes #725 --- crates/engine/src/cmd/slide.rs | 21 +++++++++++++++++++- crates/engine/src/tests.rs | 35 ++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/cmd/slide.rs b/crates/engine/src/cmd/slide.rs index 3d8ad344..42b9d0b7 100644 --- a/crates/engine/src/cmd/slide.rs +++ b/crates/engine/src/cmd/slide.rs @@ -106,7 +106,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) { sh.text = Some(TextBody::from_text(b)); } @@ -137,6 +137,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/tests.rs b/crates/engine/src/tests.rs index e83f72ef..6dcb4a40 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -835,3 +835,38 @@ fn slide_ranges_parse_like_powerpoints_print_box() { let opts = cmd::file::pdf_options(&json!({"slides": [0, 7, 1]}), 2).unwrap(); 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()); + 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}"); + } +} From 8c3404fc87cdb056c963528887248c24cc94a0b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=9C=D0=B0=D0=B3=D0=B4=D0=B0=20=D0=9C=2E=D0=93=2E?= Date: Wed, 14 Oct 2026 20:35:43 +0200 Subject: [PATCH 02/16] Fonts: falling back to the default face no longer deadlocks cascade's last fallback searched the loaded faces and, finding none that draws the cluster, called `first` while the search's read guard was still alive (a temporary in the match arm lives to the end of the `let`). `first` looks up the default family, which loads it through add_fonts, and add_fonts takes the faces' write lock even when there is nothing to add: the thread waited on itself, and every other thread then waited on the loading lock it held. It shows when a run's chain is empty and nothing loaded draws the character before the default family has been looked up, so it depends on which test (or document) gets there first: deckcraft-text's tests hung in 4 of 10 parallel runs here (all threads parked in system_fallback / try_family, one in add_fonts' write), and in 0 of 15 with this change. The search now finishes before `first` runs. New test falling_back_to_the_default_face_does_not_deadlock reaches that path on a fresh database; it times out on main and passes with the fix. --- crates/fonts/src/fallback.rs | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) 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"); From 16bef64ea154f6a71580f38387c76762ce6b0c41 Mon Sep 17 00:00:00 2001 From: IanHollow <72767437+IanHollow@users.noreply.github.com> Date: Wed, 14 Oct 2026 13:13:44 -0700 Subject: [PATCH 03/16] Add an editor screenshot to AppStream metadata Assisted-by: OpenAI Codex (GPT-6) Signed-off-by: IanHollow <72767437+IanHollow@users.noreply.github.com> --- packaging/linux/ai.storyteller.deckcraft.metainfo.xml.in | 6 ++++++ 1 file changed, 6 insertions(+) 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 From f4ce764d1c9d267b4ce9c8d9c2bf2b3d386ad604 Mon Sep 17 00:00:00 2001 From: Mex <44814932+MMexicann@users.noreply.github.com> Date: Wed, 14 Oct 2026 22:24:15 +0200 Subject: [PATCH 04/16] M13.1: validate remote MCP replies (#718) Reject unrelated, malformed, incomplete and oversized replies without replaying commands. Reconnect before sending when unsolicited reply bytes are buffered. Contribution on behalf of @MMexicann. --- ROADMAP.md | 4 +- crates/mcp/src/backend.rs | 179 ++++++++++++++++++++++++++++++++++---- docs/control-protocol.md | 6 ++ docs/gaps.md | 3 +- docs/mcp.md | 12 +++ docs/parity-checklist.md | 2 +- docs/roadmap.md | 3 +- 7 files changed, 190 insertions(+), 19 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 9303bd70..2733cab3 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -2,7 +2,7 @@ **Stage: alpha** · next: beta, ~20 points and ~120–210 h away -> **Last reviewed:** 2026-10-14 · **Last updated:** 2026-10-14 · **Change:** major (community PR wave 2026-10-14: ~60 landed items in the progress log, among them the Font, Paragraph, Photo Album, theme colour and font and Date and Time dialogs, presenter and show controls, the SmartArt Text Pane and PPTX corpus slices; breadth 84% after `cargo xtask parity`, ready stays ~55%, stage stays alpha; duplicated rows and headers folded; previous: minor, hours to ~95% per audience added) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365) +> **Last reviewed:** 2026-10-14 · **Last updated:** 2026-10-14 · **Change:** trivial (M13.1: connected MCP validates bounded reply frames, #718; parity estimates unchanged) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365) DeckCraft aims at full PowerPoint parity, and to be better: faster, open (a documented zip+JSON format plus PPTX), scriptable by agents (CLI, JSON control channel, MCP), and available everywhere @@ -131,6 +131,7 @@ app's JSON control channel (`deckcraft --control PORT`). | Date | What landed | |---|---| +| 2026-10-14 | M13.1: connected MCP rejects unrelated, malformed, incomplete and oversized replies without replay; buffered unsolicited bytes force a fresh connection before sending (#718) | | 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) | | 2026-10-13 | Slide show: Honeycomb's hexagons cover the whole slide again — the bottom quarter stayed black until the endpoint (#420) | | 2026-10-13 | Copy and Cut from the ribbon, the slide's right-click menu and the menus put the text on the system clipboard, as Ctrl+C and Ctrl+X do (#431) | @@ -220,6 +221,7 @@ app's JSON control channel (`deckcraft --control PORT`). | Date | Change | Summary | |---|---|---| +| 2026-10-14 | trivial | M13.1: remote MCP checks reply IDs, envelopes and bounded newline framing (#718) | | 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/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/docs/control-protocol.md b/docs/control-protocol.md index b7b6f013..f4436ff8 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -3,6 +3,12 @@ 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 diff --git a/docs/gaps.md b/docs/gaps.md index 4adfda40..2cba9043 100644 --- a/docs/gaps.md +++ b/docs/gaps.md @@ -1,6 +1,6 @@ # Where DeckCraft falls short of PowerPoint -> **Last reviewed:** 2026-10-14 · **Last updated:** 2026-10-14 · **Change:** major (community PR wave 2026-10-14: soak tests, PPTX corpus, comment threads (#70), embedded fonts, SmartArt Text Pane, Animation Pane timeline, Hunspell spelling, thesaurus, AutoCorrect, 14 new dialogs, pen pressure and trackpad gestures; duplicated entries and headers folded; previous: minor, 3-D bevel, camera and face lighting rendered) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365) +> **Last reviewed:** 2026-10-14 · **Last updated:** 2026-10-14 · **Change:** trivial (connected MCP reply validation, #718; existing gaps and estimates unchanged) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365) Every known shortfall, one entry each, ranked by how much it stops someone who presents with PowerPoint from switching. This is the work list: pick from the top. Numbers are summarized in @@ -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..2cd18abf 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -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 8294137d..7dc0021c 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -1,6 +1,6 @@ # DeckCraft roadmap: milestones and what's next -> **Last reviewed:** 2026-10-14 · **Last updated:** 2026-10-14 · **Change:** major (community PR wave 2026-10-14: PPTX corpus, presenter and show controls, review navigation, starter decks, Grid Options and New Window, WordArt gallery and handles, pen pressure; beta items 9 and 10 narrowed; duplicated rows and headers folded; previous: minor, core-workflow alpha gate added) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365) +> **Last reviewed:** 2026-10-14 · **Last updated:** 2026-10-14 · **Change:** trivial (M13 connected MCP reply validation, #718; beta priorities unchanged) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365) Forward-looking: where DeckCraft is going and in what order. The one-page summary is [ROADMAP.md](../ROADMAP.md); every shortfall is in [gaps.md](gaps.md); numbers are in @@ -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 | From 2568c55afac04d13d748019925b16889f56450a9 Mon Sep 17 00:00:00 2001 From: midasmatterhorn <248507632+midasmatterhorn@users.noreply.github.com> Date: Thu, 15 Oct 2026 00:37:58 +0300 Subject: [PATCH 05/16] control: a request that timed out while queued is abandoned, never run late (#719) --- apps/deckcraft/src/control_server.rs | 54 +++++++++++++++++++++++++++- crates/ui-egui/src/control.rs | 45 ++++++++++++++++++++++- crates/ui-egui/src/lib.rs | 4 +++ docs/control-protocol.md | 4 ++- 4 files changed, 104 insertions(+), 3 deletions(-) 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/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/lib.rs b/crates/ui-egui/src/lib.rs index f020f5f5..e2864b9f 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) => { diff --git a/docs/control-protocol.md b/docs/control-protocol.md index b7b6f013..0bd20302 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -6,7 +6,9 @@ lines: one request object per line, one reply per line. 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"}}} From 82ee5dec25225b572dc72045686778c0f94af196 Mon Sep 17 00:00:00 2001 From: midasmatterhorn <248507632+midasmatterhorn@users.noreply.github.com> Date: Wed, 14 Oct 2026 23:05:12 +0300 Subject: [PATCH 06/16] media: walk ID3 tag chains iteratively so crafted files cannot overflow the stack --- crates/media/src/lib.rs | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) 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); + } +} From a82f69199379eaf6dc854be180a56f5bf1feb33d Mon Sep 17 00:00:00 2001 From: midasmatterhorn <248507632+midasmatterhorn@users.noreply.github.com> Date: Thu, 15 Oct 2026 00:56:41 +0300 Subject: [PATCH 07/16] text: a bare normAutofit draws at 100% and an edit stores the fitted scale (#724) --- crates/engine/src/cmd/format.rs | 2 +- crates/engine/src/cmd/smartart.rs | 4 +- crates/engine/src/cmd/text.rs | 61 ++++++++++++++++++++++++++++-- crates/engine/src/tests.rs | 36 +++++++++++++++++- crates/format/src/lib.rs | 23 +++++++++++ crates/model/src/defaults.rs | 8 ++-- crates/model/src/text.rs | 10 ++++- crates/pptx/src/read/dml.rs | 10 ++++- crates/pptx/src/write/dml.rs | 6 ++- crates/pptx/tests/corpus_charts.rs | 4 +- crates/pptx/tests/corpus_style.rs | 4 +- crates/text/src/lib.rs | 11 ++++-- crates/text/src/tests.rs | 20 ++++++++-- 13 files changed, 169 insertions(+), 30 deletions(-) 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/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..1ce0725d 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] 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/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/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/text/src/lib.rs b/crates/text/src/lib.rs index 840b7834..38f0a7b4 100644 --- a/crates/text/src/lib.rs +++ b/crates/text/src/lib.rs @@ -1549,18 +1549,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 diff --git a/crates/text/src/tests.rs b/crates/text/src/tests.rs index d647a3e7..9d40d79b 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): From b69613bf8868976a8a2987b99840540a5b45a53e Mon Sep 17 00:00:00 2001 From: Gert Pellin Date: Wed, 14 Oct 2026 23:58:43 +0200 Subject: [PATCH 08/16] Preferences: Cancel, Esc and the close button undo the dialog's changes (#752) The Preferences dialog applies its controls as they change (appearance, interface size, the editing checkboxes, the comment author), and its Cancel result was discarded, so Cancel did nothing and the window stayed open. The dialog now remembers the settings it opened with. Cancel, Esc and the window's close button put them back and close it; Done keeps the changes. --- crates/ui-egui/src/dialogs.rs | 40 +++++++++++++++++++-- crates/ui-egui/src/lib.rs | 65 +++++++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+), 2 deletions(-) diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 18d112c8..abe8c87e 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,6 +2132,11 @@ 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() { @@ -2130,8 +2163,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..9d4a681b 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -2573,6 +2573,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| { From 40e05abd45584c13731c6b1dffa8489f6fd35627 Mon Sep 17 00:00:00 2001 From: Sazid Al Bayazid Date: Thu, 15 Oct 2026 04:41:37 +0600 Subject: [PATCH 09/16] Text: anchorCtr centres the drawn text block - right-aligned text stayed glued to the box edge (#736) anchorCtr's centreing offset was computed for a block that starts at the box's left edge and then added to lines that alignment had already placed within the box. For a right-aligned line the two shifts stacked: the line ended up starting at the box's centre and ran past the right edge - on the tdf165341 fixture the sentence was cut off at "...paragraph rig". The clamp at zero made it worse for content wider than the box: all the overflow went to the right only. The origin is now set so the drawn text block's own centre (min/max of the laid-out line extents) lands on the box's centre: every alignment stays inside the box when it fits, and overflow centres symmetrically past both edges, as PowerPoint draws it. Empty lines carry no ink and stay out of the block; vertical text and multi-column bodies are unchanged (centring is unimplemented for them, as before). Tests: right/left-aligned, trailing empty paragraph, mixed alignment, symmetric overflow, and a no-anchorCtr control - the block's centre equals the box's centre in every case. Reported with a public LibreOffice test fixture (tdf165341.pptx); verified by rendering the slide before (text cut at the right edge) and after (whole sentence centred). --- ROADMAP.md | 1 + crates/text/src/lib.rs | 17 +++++++++-- crates/text/src/tests.rs | 65 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 80 insertions(+), 3 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 9d1662a6..37dd2a42 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -131,6 +131,7 @@ 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-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) | | 2026-10-13 | Slide show: Honeycomb's hexagons cover the whole slide again — the bottom quarter stayed black until the endpoint (#420) | | 2026-10-13 | Copy and Cut from the ribbon, the slide's right-click menu and the menus put the text on the system clipboard, as Ctrl+C and Ctrl+X do (#431) | diff --git a/crates/text/src/lib.rs b/crates/text/src/lib.rs index 840b7834..d028a845 100644 --- a/crates/text/src/lib.rs +++ b/crates/text/src/lib.rs @@ -1908,9 +1908,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() { diff --git a/crates/text/src/tests.rs b/crates/text/src/tests.rs index d647a3e7..f9dbd0a1 100644 --- a/crates/text/src/tests.rs +++ b/crates/text/src/tests.rs @@ -1330,3 +1330,68 @@ 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::>() + ); +} From 57c03d36fc83486fde669a3dc94d6b7d868ccc0c Mon Sep 17 00:00:00 2001 From: Gert Pellin Date: Thu, 15 Oct 2026 01:28:10 +0200 Subject: [PATCH 10/16] Render: decode a cropped picture for the whole image's size, not its box's (#762) A picture was decoded at the size of its box on screen. A cropped picture shows only part of the image, magnified, so it was decoded at a fraction of the resolution it needs and stretched back up: Merge Shapes > Intersect, which crops the picture to the new smaller box, left it visibly blurred. Size the decode from the rectangle the whole image occupies instead. --- crates/render/src/paint.rs | 22 ++++++++++++---------- crates/render/src/tests.rs | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 10 deletions(-) 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/tests.rs b/crates/render/src/tests.rs index c85a1e29..b1e58f93 100644 --- a/crates/render/src/tests.rs +++ b/crates/render/src/tests.rs @@ -2509,3 +2509,35 @@ 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"); +} From 8b08718b9afee8439e951a654903e62ac0fb4f7c Mon Sep 17 00:00:00 2001 From: Gert Pellin Date: Thu, 15 Oct 2026 01:49:41 +0200 Subject: [PATCH 11/16] Preferences: Interface size applies when the slider is let go (#767) Dragging the Interface size slider set the interface scale on every frame, which grew or shrank the slider under the pointer and ran it to one of its ends. Keep the value pending in the dialog while the pointer holds the slider and apply it on release; keyboard changes still apply at once. --- crates/ui-egui/src/dialogs.rs | 18 +++++++++++++++--- crates/ui-egui/src/lib.rs | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 3 deletions(-) diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 18d112c8..453c4184 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -2109,9 +2109,21 @@ fn body(app: &mut SlideApp, ui: &mut Ui, d: &mut Dialog) -> bool { 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"); diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index f020f5f5..f46b51c9 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -3253,4 +3253,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"); + } } From 8d3e4a630f3657732fddcb9a4f20001e37c5b847 Mon Sep 17 00:00:00 2001 From: midasmatterhorn <248507632+midasmatterhorn@users.noreply.github.com> Date: Thu, 15 Oct 2026 03:07:22 +0300 Subject: [PATCH 12/16] fix(media): prevent overflow while probing ASF headers Use checked arithmetic for ASF header-object bounds and cursor advances so hostile object sizes cannot panic. Keep a synthetic regression for the malformed header. Addresses #750 (ASF object-size overflow). --- crates/media/src/probe.rs | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) 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")); + } +} From 51de1ad2cf7741c0b866e747c390c548b4e128ef Mon Sep 17 00:00:00 2001 From: hajdukste <50714682+hajdukste@users.noreply.github.com> Date: Wed, 14 Oct 2026 17:58:53 -0700 Subject: [PATCH 13/16] Text (#93): show gray markers on empty list paragraphs --- ROADMAP.md | 4 +- crates/render/src/lib.rs | 32 +++++++++- crates/render/src/table.rs | 9 ++- crates/render/src/tests.rs | 27 ++++++++ crates/render/src/transition.rs | 1 + crates/text/src/lib.rs | 106 +++++++++++++++++--------------- crates/text/src/tests.rs | 57 +++++++++++++++++ crates/ui-egui/src/canvas.rs | 84 ++++++++++++++++++++++--- crates/ui-egui/src/textures.rs | 13 ++-- docs/parity-checklist.md | 4 +- docs/ui-parity.md | 10 ++- 11 files changed, 274 insertions(+), 73 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 9d1662a6..d8955b57 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -2,7 +2,7 @@ **Stage: alpha** · next: beta, ~20 points and ~120–210 h away -> **Last reviewed:** 2026-10-14 · **Last updated:** 2026-10-14 · **Change:** major (community PR wave 2026-10-14: ~60 landed items in the progress log, among them the Font, Paragraph, Photo Album, theme colour and font and Date and Time dialogs, presenter and show controls, the SmartArt Text Pane and PPTX corpus slices; breadth 84% after `cargo xtask parity`, ready stays ~55%, stage stays alpha; duplicated rows and headers folded; previous: minor, hours to ~95% per audience added) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365) +> **Last reviewed:** 2026-10-14 · **Last updated:** 2026-10-14 · **Change:** minor (Gray list markers on empty paragraphs while editing; caret keeps the list indent before typing) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365) DeckCraft aims at full PowerPoint parity, and to be better: faster, open (a documented zip+JSON format plus PPTX), scriptable by agents (CLI, JSON control channel, MCP), and available everywhere @@ -131,6 +131,7 @@ app's JSON control channel (`deckcraft --control PORT`). | Date | What landed | |---|---| +| 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 | 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) | | 2026-10-13 | Slide show: Honeycomb's hexagons cover the whole slide again — the bottom quarter stayed black until the endpoint (#420) | | 2026-10-13 | Copy and Cut from the ribbon, the slide's right-click menu and the menus put the text on the system clipboard, as Ctrl+C and Ctrl+X do (#431) | @@ -221,6 +222,7 @@ app's JSON control channel (`deckcraft --control PORT`). | 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: 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/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/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..c0c56791 100644 --- a/crates/render/src/tests.rs +++ b/crates/render/src/tests.rs @@ -2509,3 +2509,30 @@ fn single_series_column_chart_varies_colours_by_point() { }; assert_ne!(render(true), render(false)); } + +#[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.chunks_exact(4).zip(plain.pixels.chunks_exact(4)).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..e74f336f 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 } } @@ -1654,6 +1653,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) { @@ -1930,7 +1933,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 +1949,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 d647a3e7..d2ddef1d 100644 --- a/crates/text/src/tests.rs +++ b/crates/text/src/tests.rs @@ -1330,3 +1330,60 @@ fn stacked_combining_marks_take_a_face_that_draws_them() { } let _ = f; } + +#[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..baf48a4f 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,75 @@ 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); + // Optional local screenshots use the actual canvas renderer, including its cache. + let preview = |h: &mut Harness<'_, SlideApp>, name: &str| { + if let Ok(dir) = std::env::var("BULLET_PREVIEW_DIR") { + // Capture the visible phase of the caret, with the pointer clear of the text. + let time = h.ctx.input(|i| i.time); + h.input_mut().time = Some((time * 1.6).floor() / 1.6 + 0.65); + h.event(Event::PointerMoved(pos2(25.0, 25.0))); + h.step(); + h.render().unwrap().save(std::path::Path::new(&dir).join(name)).unwrap(); + } + }; + preview(&mut h, "bullet-empty.png"); + 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); + preview(&mut h, "bullet-after-enter.png"); + 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"); + preview(&mut h, "bullet-after-exit.png"); + 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/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/parity-checklist.md b/docs/parity-checklist.md index 9843d75e..6e9db9f4 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 | diff --git a/docs/ui-parity.md b/docs/ui-parity.md index cc7801d7..815b0ef5 100644 --- a/docs/ui-parity.md +++ b/docs/ui-parity.md @@ -1,12 +1,19 @@ # UI parity with PowerPoint -> **Last reviewed:** 2026-10-14 · **Last updated:** 2026-10-14 · **Change:** major (community PR wave 2026-10-14: 14 new dialogs (41 in all), Eyedropper, Export button, New Window, Grid Options, WordArt gallery and handles, Slide Sorter sections, Quick Access Toolbar, Format Background and Format Shape depth, rich-text clipboard; duplicated rows and headers folded; previous: minor, File tab / Backstage, #76) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365) +> **Last reviewed:** 2026-10-14 · **Last updated:** 2026-10-14 · **Change:** minor (Gray list markers on empty paragraphs while editing; caret keeps the list indent before typing) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365) How DeckCraft looks and feels next to PowerPoint: the shell, the ribbon, menus, panes, dialogs, direct manipulation, keyboard and precision. Observations of PowerPoint are kept locally in `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 | From 0222a90e5d3cdab164ad8a7aecc739a1e09baeac Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Thu, 15 Oct 2026 01:07:48 +0000 Subject: [PATCH 14/16] Grow table merges to include merged areas they overlap Found while testing crates/model/src/table.rs:224 (Table::merge) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 Co-Authored-By: Claude Opus 5.5 --- crates/model/src/table.rs | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) 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 { From a15d93acdd5fdf3739c616ee40d4b51177fae8ef Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Wed, 14 Oct 2026 22:00:48 -0400 Subject: [PATCH 15/16] Fix up #747 and #772 from review: docs keep their status headers (log rows stay), no milestone renaming; the bullet test no longer saves screenshots; clippy's as_chunks in the bullet render test Co-Authored-By: Claude Opus 5.5 --- ROADMAP.md | 7 +++---- crates/render/src/tests.rs | 2 +- crates/ui-egui/src/canvas.rs | 14 -------------- docs/gaps.md | 2 +- docs/roadmap.md | 2 +- docs/ui-parity.md | 2 +- 6 files changed, 7 insertions(+), 22 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 7e857b80..44b9c4f1 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -2,8 +2,7 @@ **Stage: alpha** · next: beta, ~20 points and ~120–210 h away -> **Last reviewed:** 2026-10-14 · **Last updated:** 2026-10-14 · **Change:** trivial (M13.1: connected MCP validates bounded reply frames, #718; parity estimates unchanged) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365) -> **Last reviewed:** 2026-10-14 · **Last updated:** 2026-10-14 · **Change:** minor (Gray list markers on empty paragraphs while editing; caret keeps the list indent before typing) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365) +> **Last reviewed:** 2026-10-14 · **Last updated:** 2026-10-14 · **Change:** major (community PR wave 2026-10-14: ~60 landed items in the progress log, among them the Font, Paragraph, Photo Album, theme colour and font and Date and Time dialogs, presenter and show controls, the SmartArt Text Pane and PPTX corpus slices; breadth 84% after `cargo xtask parity`, ready stays ~55%, stage stays alpha; duplicated rows and headers folded; previous: minor, hours to ~95% per audience added) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365) DeckCraft aims at full PowerPoint parity, and to be better: faster, open (a documented zip+JSON format plus PPTX), scriptable by agents (CLI, JSON control channel, MCP), and available everywhere @@ -133,7 +132,7 @@ 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 | M13.1: connected MCP rejects unrelated, malformed, incomplete and oversized replies without replay; buffered unsolicited bytes force a fresh connection before sending (#718) | +| 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 | 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) | | 2026-10-13 | Slide show: Honeycomb's hexagons cover the whole slide again — the bottom quarter stayed black until the endpoint (#420) | @@ -225,7 +224,7 @@ app's JSON control channel (`deckcraft --control PORT`). | Date | Change | Summary | |---|---|---| -| 2026-10-14 | trivial | M13.1: remote MCP checks reply IDs, envelopes and bounded newline framing (#718) | +| 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) | diff --git a/crates/render/src/tests.rs b/crates/render/src/tests.rs index cfc106b0..05d7c91a 100644 --- a/crates/render/src/tests.rs +++ b/crates/render/src/tests.rs @@ -2563,7 +2563,7 @@ fn empty_bullet_is_gray_only_while_editing_and_never_in_exports() { 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.chunks_exact(4).zip(plain.pixels.chunks_exact(4)).filter(|(a, b)| a != b).collect(); + 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/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index baf48a4f..ba66aad8 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -1499,18 +1499,6 @@ mod tests { deckcraft_engine::cmd::text::caret_line(st, ts).unwrap().0 }; let first = caret(&h); - // Optional local screenshots use the actual canvas renderer, including its cache. - let preview = |h: &mut Harness<'_, SlideApp>, name: &str| { - if let Ok(dir) = std::env::var("BULLET_PREVIEW_DIR") { - // Capture the visible phase of the caret, with the pointer clear of the text. - let time = h.ctx.input(|i| i.time); - h.input_mut().time = Some((time * 1.6).floor() / 1.6 + 0.65); - h.event(Event::PointerMoved(pos2(25.0, 25.0))); - h.step(); - h.render().unwrap().save(std::path::Path::new(&dir).join(name)).unwrap(); - } - }; - preview(&mut h, "bullet-empty.png"); h.event(Event::Text("First point".into())); h.run_steps(3); h.key_press(egui::Key::Enter); @@ -1522,12 +1510,10 @@ mod tests { 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); - preview(&mut h, "bullet-after-enter.png"); 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"); - preview(&mut h, "bullet-after-exit.png"); h.state_mut().run("text.edit", json!({"id": id, "at": [1, 0]})).unwrap(); h.run_steps(3); h.event(Event::Text("Second point".into())); diff --git a/docs/gaps.md b/docs/gaps.md index 2cba9043..4e564951 100644 --- a/docs/gaps.md +++ b/docs/gaps.md @@ -1,6 +1,6 @@ # Where DeckCraft falls short of PowerPoint -> **Last reviewed:** 2026-10-14 · **Last updated:** 2026-10-14 · **Change:** trivial (connected MCP reply validation, #718; existing gaps and estimates unchanged) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365) +> **Last reviewed:** 2026-10-14 · **Last updated:** 2026-10-14 · **Change:** major (community PR wave 2026-10-14: soak tests, PPTX corpus, comment threads (#70), embedded fonts, SmartArt Text Pane, Animation Pane timeline, Hunspell spelling, thesaurus, AutoCorrect, 14 new dialogs, pen pressure and trackpad gestures; duplicated entries and headers folded; previous: minor, 3-D bevel, camera and face lighting rendered) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365) Every known shortfall, one entry each, ranked by how much it stops someone who presents with PowerPoint from switching. This is the work list: pick from the top. Numbers are summarized in diff --git a/docs/roadmap.md b/docs/roadmap.md index 7dc0021c..4ff4c455 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -1,6 +1,6 @@ # DeckCraft roadmap: milestones and what's next -> **Last reviewed:** 2026-10-14 · **Last updated:** 2026-10-14 · **Change:** trivial (M13 connected MCP reply validation, #718; beta priorities unchanged) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365) +> **Last reviewed:** 2026-10-14 · **Last updated:** 2026-10-14 · **Change:** major (community PR wave 2026-10-14: PPTX corpus, presenter and show controls, review navigation, starter decks, Grid Options and New Window, WordArt gallery and handles, pen pressure; beta items 9 and 10 narrowed; duplicated rows and headers folded; previous: minor, core-workflow alpha gate added) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365) Forward-looking: where DeckCraft is going and in what order. The one-page summary is [ROADMAP.md](../ROADMAP.md); every shortfall is in [gaps.md](gaps.md); numbers are in diff --git a/docs/ui-parity.md b/docs/ui-parity.md index 815b0ef5..cc97dae4 100644 --- a/docs/ui-parity.md +++ b/docs/ui-parity.md @@ -1,6 +1,6 @@ # UI parity with PowerPoint -> **Last reviewed:** 2026-10-14 · **Last updated:** 2026-10-14 · **Change:** minor (Gray list markers on empty paragraphs while editing; caret keeps the list indent before typing) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365) +> **Last reviewed:** 2026-10-14 · **Last updated:** 2026-10-14 · **Change:** major (community PR wave 2026-10-14: 14 new dialogs (41 in all), Eyedropper, Export button, New Window, Grid Options, WordArt gallery and handles, Slide Sorter sections, Quick Access Toolbar, Format Background and Format Shape depth, rich-text clipboard; duplicated rows and headers folded; previous: minor, File tab / Backstage, #76) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365) How DeckCraft looks and feels next to PowerPoint: the shell, the ribbon, menus, panes, dialogs, direct manipulation, keyboard and precision. Observations of PowerPoint are kept locally in From 6b03a3f29c5c882ce6c76745b68fd39ea884e131 Mon Sep 17 00:00:00 2001 From: skylartaylor <26588250+skylartaylor@users.noreply.github.com> Date: Thu, 15 Oct 2026 04:27:05 +0200 Subject: [PATCH 16/16] ci: add comfortable runtime limits to unbounded jobs --- .github/workflows/ci.yml | 1 + 1 file changed, 1 insertion(+) 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