diff --git a/ROADMAP.md b/ROADMAP.md index 129c21de..9604db66 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -2,7 +2,7 @@ **Stage: alpha** · next: beta, ~26 points and ~162–292 h away (one Opus 5.5 agent; ~33–67 h wall-clock with 5 agents) -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-28 · **Change:** minor (Arabic draft catalog; scaled instances keep their scale when their main is rebuilt (#922); .fig embedded images are copied once per blob, not once per hash (#1059); page ruler guides survive save and reopen (#1476); colour picker LCH and OKLCH models; non-ASCII layer names transliterate in code export (#1235); branches in live sessions and branch history entries, gap 17; plugin widgets take text input and a property menu (plugin API 4); boards: templates and tables; boards: a shared timer and voting sessions; boards: the dot grid, connector ends dragged to re-attach, curved connectors; Installed weights of an embedded font family (#1246); `.sketch` import, best effort (Sketch 48 and newer); CJK line breaking: JIS X 4051's line-start and line-end sets completed with the halfwidth small kana, the halfwidth and spacing (semi-)voiced sound marks, the iteration marks 〱–〵 and the halfwidth parentheses, with behaviour cases for each; Adobe Fonts found on macOS (#298); egui 0.37 hidden-capture deadline contract; Dev Mode measurements kept on the canvas and a read-only layers panel; boards: a board area row and progress entry; community fixes in #507, #510, #529 and #530: auto-save, a text box by dragging, batch step references, removing someone from a live session, Russian and Turkish catalogs reviewed, Czech and Indonesian drafts, the Japanese draft completed, zoom previews, the web clipboard, Alt-drag snapping and Cmd+D repeat, recovery copies, crash and rollback fixes, colour emoji, `.fig` import fixes; earlier changes are in the revision history below) · **Target:** Figma Design (desktop app 126.10.7) +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-28 · **Change:** minor (M4: zero-length round-capped dashes draw dots; dash expansion uses the existing safety budget; extracted from #1143; Arabic draft catalog; scaled instances keep their scale when their main is rebuilt (#922); .fig embedded images are copied once per blob, not once per hash (#1059); page ruler guides survive save and reopen (#1476); colour picker LCH and OKLCH models; non-ASCII layer names transliterate in code export (#1235); branches in live sessions and branch history entries, gap 17; plugin widgets take text input and a property menu (plugin API 4); boards: templates and tables; boards: a shared timer and voting sessions; boards: the dot grid, connector ends dragged to re-attach, curved connectors; Installed weights of an embedded font family (#1246); `.sketch` import, best effort (Sketch 48 and newer); CJK line breaking: JIS X 4051's line-start and line-end sets completed with the halfwidth small kana, the halfwidth and spacing (semi-)voiced sound marks, the iteration marks 〱–〵 and the halfwidth parentheses, with behaviour cases for each; Adobe Fonts found on macOS (#298); egui 0.37 hidden-capture deadline contract; Dev Mode measurements kept on the canvas and a read-only layers panel; boards: a board area row and progress entry; community fixes in #507, #510, #529 and #530: auto-save, a text box by dragging, batch step references, removing someone from a live session, Russian and Turkish catalogs reviewed, Czech and Indonesian drafts, the Japanese draft completed, zoom previews, the web clipboard, Alt-drag snapping and Cmd+D repeat, recovery copies, crash and rollback fixes, colour emoji, `.fig` import fixes; earlier changes are in the revision history below) · **Target:** Figma Design (desktop app 126.10.7) FigCraft is a clean-room, open-source, pure-Rust reimplementation of the Figma design workflow, native on macOS, Windows, Linux and FreeBSD and in the browser, fully drivable by agents through @@ -135,6 +135,7 @@ Ranked; detail and milestones in [`docs/roadmap.md`](docs/roadmap.md). - **2026-10-28** · M3, #922: an instance scaled with the Scale tool (or `transform.scale`) keeps its layers' scaled size and points when its main is rebuilt. The scaled layers' new size and place were read as what constraints give (an ancestor resized in the same edit), so no override was kept, and adding a layer to the main put a 14 px icon's 24 px triangle back. The Scale tool now records them as `size` and `position` overrides on layers inside instances; strokes already kept theirs. The report's own sequence (clearing effects on the component holding the icon) no longer reproduces on main. Regressions for a scaled instance, one scaled with the tool (with undo) and a scaled icon nested in a component and its instances. Numbers unchanged. - **2026-10-28** · M6, #1059: `.fig` import copies each image carried inside the message once per blob, shared by every hash that names it, instead of once per hash: a file pointing many hashes at one large blob no longer multiplies it in memory (100 hashes on a 250 MB blob were 25 GB). Regression test. Numbers unchanged. - **2026-10-28** · M1, #1476: page ruler guides survive save and reopen. A page kept its guides in the page kind while the layer's own `guides` field (frame guides, #70) took that key when a file was read, so a reopened page's guides vanished, and saving it wrote `guides` twice, which then refused to open. Every layer now keeps its guides in the one field, and a page with two `guides` lists opens with both. Regressions in the doc and format crates. Numbers unchanged. +- **2026-10-27** · M4: zero-length round-capped dashes draw dots; dash expansion uses the existing safety budget; extracted from #1143. Numbers unchanged. - **2026-10-26** · Non-ASCII layer names transliterate in code export (M6, #1235): `class_name` runs the name through `deunicode` before its ASCII filter, so 로그인 화면 → `rogeuin-hwamyeon`, кнопка → `knopka`, über → `uber` in the HTML/CSS/React/Leptos class names, component roots and `data-*` variant attributes (SwiftUI/Compose type names unchanged); a name left with nothing (symbols only) takes the node's kind (`.text`, `.frame`) instead of `layer-`, and the 40-character cut lands on a word boundary. New dependency: deunicode (pure Rust tables, zero deps, BSD-3-Clause, the `slug` crate's base). `html_codegen_tests`. Numbers unchanged. - **2026-10-25** · egui 0.37 capture lifecycle: screenshots with hidden or unknown visibility queue, request a hidden paint, and expire after 60 seconds if no capture arrives (headless-tested contract). Logic-only edits, saves and inspection continue while hidden. Native visible GUI and forced hidden-window paint, browser and Wayland behavior remain unqualified; parity percentages are unchanged. - **2026-10-24** · Open, Save and the folder dialogs start in the folder of the last file or folder picked in the run (#1290): the Linux portal does not remember it between dialogs as the macOS and Windows panels do. A folder deleted since falls back to the system's choice. No numbers change. @@ -355,6 +356,7 @@ Ranked; detail and milestones in [`docs/roadmap.md`](docs/roadmap.md). | 2026-10-28 | minor | M3, #922: instances scaled with the Scale tool keep their layers' scaled size and points when their main is rebuilt (`size`/`position` overrides recorded on scale). Numbers unchanged. | | 2026-10-28 | minor | M6, #1059: `.fig` import copies embedded image blobs once per blob, not once per hash naming them. Numbers unchanged. | | 2026-10-28 | minor | M1, #1476: page ruler guides survive save and reopen; files written with a page's `guides` key twice open again with both lists. Numbers unchanged. | +| 2026-10-27 | minor | M4: zero-length round-capped dashes draw dots; dash expansion uses the existing safety budget; extracted from #1143. Numbers unchanged. | | 2026-10-26 | minor | Non-ASCII layer names transliterate in code export (M6, #1235): `class_name` transliterates through the new `deunicode` dependency (pure Rust tables, zero deps, BSD-3-Clause), so Korean, Cyrillic, Latin-diacritic and CJK names become readable ASCII classes (`rogeuin-hwamyeon`, `knopka`, `uber`, `an-niu`) in HTML/CSS/React/Leptos output, component roots and `data-*` variant attributes; symbols-only names take the node's kind (`.text`) instead of `layer-`, and the 40-character cut lands on a word boundary. `html_codegen_tests` updated and extended. Numbers unchanged | | 2026-10-25 | minor | egui 0.37 capture lifecycle: screenshots with hidden or unknown visibility queue, request a hidden paint, and expire after 60 seconds if no capture arrives (headless-tested contract). Logic-only edits, saves and inspection continue while hidden. Native visible GUI and forced hidden-window paint, browser and Wayland behavior remain unqualified; parity percentages are unchanged. | | 2026-10-24 | minor | Server memory (#1134): when the last person leaves a file nobody edited, figcraft-server keeps the client ids it issued in the log instead of rewriting the snapshot (compaction still follows `compact_after`/`compact_mb` and edits), and a snapshot is encoded straight into its file buffer, without a second copy of the body; the file format is unchanged. Numbers unchanged | diff --git a/crates/collab/src/entry_ids_tests.rs b/crates/collab/src/entry_ids_tests.rs index db58bb2f..72a147ae 100644 --- a/crates/collab/src/entry_ids_tests.rs +++ b/crates/collab/src/entry_ids_tests.rs @@ -6,6 +6,7 @@ use figcraft_doc::{Collection, Comment, Document, Message, Mode, Style, StyleKin use crate::change::{Change, Entry, Json}; use crate::message::{ClientMsg, ServerMsg}; +use crate::server::NO_SESSION_SPACE; use crate::sim::Sim; use crate::{Client, Limits, NoRecompute, Outgoing, Server}; @@ -92,6 +93,53 @@ fn client_ids_skip_spaces_used_by_styles_and_comments() { assert_eq!(join(&mut server).id(), 10); } +#[test] +fn an_id_in_the_top_space_does_not_block_joins() { + let mut d = Document::new("a"); + d.styles.push(style(format!("{STYLE_ID_PREFIX}{}", u64::MAX), "Out of range")); + let c = comment(u64::from(u32::MAX) << 32, &d); + d.comments.push(c); + let mut server = Server::new(d, "", Limits::default()); + let a = join(&mut server); + assert!(a.id() > 0 && a.id() < u32::MAX, "{}", a.id()); + // After a reload the session still takes newcomers and keeps the document as it was. + let state = server.state().unwrap(); + let mut restored = Server::restore(state.clone(), "", Limits::default()).unwrap(); + let b = join(&mut restored); + assert!(b.id() > a.id() && b.id() < u32::MAX, "{}", b.id()); + assert_eq!(restored.document().styles.len(), 1); + assert_eq!(restored.document().comments.len(), 1); + // A state saved while the counter was stuck at the top recovers too. + let stuck = crate::SessionState { next_client: u32::MAX, ..state }; + let mut restored = Server::restore(stuck, "", Limits::default()).unwrap(); + let c = join(&mut restored); + assert!(c.id() > a.id() && c.id() < u32::MAX, "{}", c.id()); + // A new entry may not take an id from the reserved top half of the spaces. + for (k, space) in [NO_SESSION_SPACE, u32::MAX].into_iter().enumerate() { + let id = format!("{STYLE_ID_PREFIX}{}", (u64::from(space) << 32) | 1); + let ServerMsg::Reject { reason, .. } = send(&mut restored, c.id(), 1 + k as u64, vec![put_style(id.clone())]) else { + panic!("accepted {id}") + }; + assert!(reason.contains("id space"), "{reason}"); + } +} + +#[test] +fn styles_imported_from_another_session_are_accepted_and_joins_go_on() { + let mut server = Server::new(Document::new("a"), "", Limits::default()); + let a = join(&mut server); + // `file.import` copies styles with their ids, made by clients this session hasn't given out + // (up to the highest space a member may use). + for (k, space) in [a.id() + 1, a.id() + 1000, NO_SESSION_SPACE - 1].into_iter().enumerate() { + let id = format!("{STYLE_ID_PREFIX}{}", (u64::from(space) << 32) | 1); + assert!(matches!(send(&mut server, a.id(), 1 + k as u64, vec![put_style(id.clone())]), ServerMsg::Changes(_)), "refused {id}"); + } + // After a restart client ids start above them, and there are plenty left. + let mut restored = Server::restore(server.state().unwrap(), "", Limits::default()).unwrap(); + assert_eq!(join(&mut restored).id(), NO_SESSION_SPACE); + assert_eq!(join(&mut Server::new(restored.document().clone(), "", Limits::default())).id(), NO_SESSION_SPACE); +} + /// Each client makes `rounds` styles, collections (with an extra mode), variables and comments /// before seeing anyone else's. fn create_everything(doc: &mut Document, who: usize, round: usize) { diff --git a/crates/collab/src/server.rs b/crates/collab/src/server.rs index 9a04e54b..d8aab594 100644 --- a/crates/collab/src/server.rs +++ b/crates/collab/src/server.rs @@ -25,6 +25,12 @@ pub enum Outgoing { /// Why a viewer's batch is refused. pub const VIEW_ONLY: &str = "you can only view this file (your link is view-only)"; +/// A new style, variable, collection or comment may not take an id in a space at or above this +/// that isn't its creator's. Client ids start above every space in the document after a restart, +/// so no member can use them up: at least this many always remain. Lower spaces stay open, for +/// entries imported from other files' sessions. +pub const NO_SESSION_SPACE: u32 = 1 << 31; + /// What changed a server's lasting state, in order: what a store appends to its log between /// snapshots ([`Server::journaling`]) and replays over the last snapshot after a restart /// ([`Server::replay`]). Presence and who is connected aren't in it. @@ -158,7 +164,13 @@ impl Server { server.state = replica; server.session = state.session; server.seq = state.seq; - server.next_client = server.next_client.max(state.next_client); + // A counter stuck at the top (saved while an out-of-range id in the document blocked + // every join) carries on above the ids actually handed out. + let saved = match state.next_client { + u32::MAX => state.issued.keys().chain(state.log.iter().map(|a| &a.author)).max().map_or(0, |c| c.saturating_add(1)), + n => n, + }; + server.next_client = server.next_client.max(saved); server.issued = state.issued.into_iter().filter(|(id, _)| *id < server.next_client).collect(); server.deleted = state.deleted; server.removed = state.removed; @@ -500,9 +512,9 @@ impl Server { /// The document-level entries (styles, variables, collections, comments) a batch adds or /// removes, each with whether it is there afterwards. A new one is refused when its id lies in - /// the space of another client this session gave an id to (unless it was removed before: undo - /// puts entries back); ids from before the session (space 0, earlier sessions, other files) - /// are fine. + /// the space of another client this session gave an id to, or in a space at or above + /// [`NO_SESSION_SPACE`] (unless it was removed before: undo puts entries back); ids from + /// before the session (space 0, earlier sessions, other files) are fine. fn check_entries(&self, from: u32, changes: &[Change], after: &Document) -> Result, Error> { let before = self.state.document(); let mut touched: BTreeSet = BTreeSet::new(); @@ -529,7 +541,7 @@ impl Server { } if now && !self.removed.contains(&entry) - && let Some(space) = entry.id_space().filter(|s| *s != from && self.issued.contains_key(s)) + && let Some(space) = entry.id_space().filter(|s| *s != from && (self.issued.contains_key(s) || *s >= NO_SESSION_SPACE)) { return Err(Error::Invalid(format!("new {} isn't in client {from}'s id space (it is client {space}'s)", entry.describe()))); } diff --git a/crates/doc/src/ids.rs b/crates/doc/src/ids.rs index 024946bb..7e244fbb 100644 --- a/crates/doc/src/ids.rs +++ b/crates/doc/src/ids.rs @@ -128,7 +128,8 @@ impl Document { /// The highest client id space any id in the document lies in (layers, comments, styles, /// variables, collections, modes): a session hands out client ids above it, so ids made in - /// earlier sessions are never made again. + /// earlier sessions are never made again. Space `u32::MAX` doesn't count: numbers too big for + /// any client's space land there ([`space_of`]) and no session hands that client id out. pub fn highest_id_space(&self) -> u32 { let nodes = self.ids().map(|id| id.0); let comments = self.comments.iter().map(|c| c.id); @@ -136,6 +137,6 @@ impl Document { let vars = self.variables.variables.iter().filter_map(|v| id_number(&v.id, VARIABLE_ID_PREFIX)); let colls = self.variables.collections.iter().filter_map(|c| id_number(&c.id, COLLECTION_ID_PREFIX)); let modes = self.variables.collections.iter().flat_map(|c| c.modes.iter().filter_map(|m| mode_number(&m.id))); - nodes.chain(comments).chain(styles).chain(vars).chain(colls).chain(modes).map(space_of).max().unwrap_or(0) + nodes.chain(comments).chain(styles).chain(vars).chain(colls).chain(modes).map(space_of).filter(|s| *s != u32::MAX).max().unwrap_or(0) } } diff --git a/crates/doc/src/ids_tests.rs b/crates/doc/src/ids_tests.rs index 4f489886..a0091623 100644 --- a/crates/doc/src/ids_tests.rs +++ b/crates/doc/src/ids_tests.rs @@ -103,6 +103,10 @@ fn the_highest_space_counts_every_kind_of_id() { assert_eq!(d.highest_id_space(), 9); d.comments.push(comment(11u64 << 32, d.first_page().unwrap())); assert_eq!(d.highest_id_space(), 11); + // Numbers past every client's space (they saturate to `u32::MAX`) are no session's. + d.styles.push(style(&format!("S:{}", u64::MAX))); + d.comments.push(comment(u64::from(u32::MAX) << 32, d.first_page().unwrap())); + assert_eq!(d.highest_id_space(), 11); // Other forms are space 0. assert_eq!(id_space("lib:S:99999999999", "S:"), 0); assert_eq!(id_space("S:+5", "S:"), 0); @@ -110,3 +114,11 @@ fn the_highest_space_counts_every_kind_of_id() { assert_eq!(id_space("S:99999999999999999999999", "S:"), 0, "too big for a number"); assert_eq!(mode_number("1:0"), None); } + +#[test] +fn nodeid_rejects_plus_sign() { + assert!("+12".parse::().is_err()); + assert!("0:+12".parse::().is_err()); + assert_eq!("12".parse::(), Ok(crate::NodeId(12))); + assert_eq!("0:12".parse::(), Ok(crate::NodeId(12))); +} diff --git a/crates/doc/src/lib.rs b/crates/doc/src/lib.rs index 6f6af7a3..d19ef0c7 100644 --- a/crates/doc/src/lib.rs +++ b/crates/doc/src/lib.rs @@ -67,6 +67,10 @@ impl std::str::FromStr for NodeId { // Accept "12" and Figma-style "0:12". let t = s.trim(); let t = t.rsplit(':').next().unwrap_or(t); + if t.starts_with('+') { + // `u64::parse` accepts a leading '+'; ids are digits only. + return "-".parse::().map(NodeId); + } t.parse::().map(NodeId) } } diff --git a/crates/engine/src/automation/params.rs b/crates/engine/src/automation/params.rs index 35a74e0e..8a4cb84e 100644 --- a/crates/engine/src/automation/params.rs +++ b/crates/engine/src/automation/params.rs @@ -20,6 +20,13 @@ pub fn f64_of(v: &Value) -> Option { } } +/// A 0-1 fraction: a `%` suffix or a number above 1 is a percentage ("1%" is 0.01, 50 is 0.5). +pub fn fraction_of(v: &Value) -> Option { + let f = f64_of(v)?; + let percent = v.as_str().is_some_and(|s| s.trim().ends_with('%')); + Some(if percent || f > 1.0 { f / 100.0 } else { f }) +} + pub fn num(p: &Value, key: &str) -> Option { get(p, key).and_then(f64_of) } @@ -167,7 +174,7 @@ pub fn color_of(v: &Value) -> Option { Value::String(s) => Color::parse_css(s), Value::Object(_) => serde_json::from_value::(v.clone()).ok().map(Color::sanitized), Value::Array(a) => { - let c: Vec = a.iter().filter_map(|x| x.as_f64()).map(|f| f as f32).collect(); + let c = a.iter().map(|x| x.as_f64().map(|f| f as f32)).collect::>>()?; match c.as_slice() { [r, g, b] => Some(Color::new(*r, *g, *b, 1.0)), [r, g, b, a] => Some(Color::new(*r, *g, *b, *a)), @@ -278,3 +285,16 @@ pub fn parse(cmd: &str, p: &Value, key: &str) -> Some(v) => serde_json::from_value(v.clone()).map(Some).map_err(|e| bad(cmd, format!("`{key}`: {e}"))), } } + +#[cfg(test)] +mod color_of_tests { + use super::*; + use serde_json::json; + + #[test] + fn color_of_rejects_non_numeric_array_entries() { + assert_eq!(color_of(&json!([1, "x", 0, 0])), None); + assert_eq!(color_of(&json!([1, 0, 0, "x"])), None); + assert_eq!(color_of(&json!([1, 0, 0, 0.5])), Some(Color::new(1.0, 0.0, 0.0, 0.5))); + } +} diff --git a/crates/engine/src/batch_rollback_tests.rs b/crates/engine/src/batch_rollback_tests.rs index 298ca097..9d38ce11 100644 --- a/crates/engine/src/batch_rollback_tests.rs +++ b/crates/engine/src/batch_rollback_tests.rs @@ -59,6 +59,90 @@ fn failed_group_reopens_documents_closed_inside_it() { assert!(s.closed_in_group.is_empty()); } +#[test] +fn failed_group_restores_each_tabs_history_after_undo_close_and_open() { + let mut s = Session::new(); + let mut originals = Vec::new(); + for (index, name) in ["First", "Second"].into_iter().enumerate() { + if index > 0 { + s.execute("file.new", &json!({"name": name})).unwrap(); + } + let created = s + .execute( + "command.batch", + &json!({ + "label": format!("{name} shape"), + "commands": [{"command": "shape.rectangle", "params": { + "x": index * 20, "y": 0, "width": 10, "height": 10, "props": {"name": name} + }}] + }), + ) + .unwrap(); + let original = id(&created[0]); + s.execute( + "command.batch", + &json!({ + "label": format!("{name} name"), + "commands": [{"command": "node.set", "params": {"id": original, "props": {"name": format!("{name} renamed")}}}] + }), + ) + .unwrap(); + assert_eq!(s.execute("edit.undo", &json!({})).unwrap(), json!({"undone": true})); + assert_eq!(s.undo_depth(), (1, 1)); + originals.push(original); + } + let uids: Vec<_> = s.docs.iter().map(|ds| ds.uid).collect(); + let before_docs: Vec<_> = s.docs.iter().map(|ds| ds.doc.clone()).collect(); + let before_labels: Vec<_> = s + .docs + .iter() + .map(|ds| { + (ds.history.undo.iter().map(|h| h.label.clone()).collect::>(), ds.history.redo.iter().map(|h| h.label.clone()).collect::>()) + }) + .collect(); + assert_ne!(before_labels[0], before_labels[1]); + let active = s.active; + + let result = s.execute( + "command.batch", + &json!({ + "commands": [ + {"command": "edit.undo"}, + {"command": "doc.nextTab"}, + {"command": "edit.undo"}, + {"command": "file.close", "params": {"index": 0, "force": true}}, + {"command": "file.new", "params": {"name": "Temporary"}}, + {"command": "no.such.command"} + ] + }), + ); + + assert!(result.is_err()); + assert_eq!(s.docs.iter().map(|ds| ds.uid).collect::>(), uids); + assert_eq!(s.active, active); + assert!(s.closed_in_group.is_empty()); + for (index, ds) in s.docs.iter().enumerate() { + assert_eq!(ds.doc, before_docs[index]); + assert_eq!(ds.history.undo.iter().map(|h| h.label.clone()).collect::>(), before_labels[index].0); + assert_eq!(ds.history.redo.iter().map(|h| h.label.clone()).collect::>(), before_labels[index].1); + } + // Travel through each restored tab's real history, including the pre-existing redo step. + for _ in 0..2 { + let index = s.active; + let original = originals[index]; + assert_eq!(s.execute("edit.undo", &json!({})).unwrap(), json!({"undone": true})); + assert!(!s.doc().unwrap().contains(original)); + assert_eq!(s.execute("edit.redo", &json!({})).unwrap(), json!({"redone": true})); + assert_eq!(s.doc().unwrap(), before_docs[index].as_ref()); + assert_eq!(s.execute("edit.redo", &json!({})).unwrap(), json!({"redone": true})); + let name = &before_docs[index].node(original).unwrap().name; + assert_eq!(s.doc().unwrap().node(original).unwrap().name, format!("{name} renamed")); + assert_eq!(s.execute("edit.undo", &json!({})).unwrap(), json!({"undone": true})); + assert_eq!(s.doc().unwrap(), before_docs[index].as_ref()); + s.execute("doc.nextTab", &json!({})).unwrap(); + } +} + #[test] fn failed_plugin_run_restores_each_document_without_restoring_unrelated_state() { let mut s = Session::new(); @@ -87,6 +171,121 @@ fn failed_plugin_run_restores_each_document_without_restoring_unrelated_state() assert_eq!(s.message, before_message); } +#[test] +fn failed_group_keeps_the_undo_steps_it_undid() { + let mut s = Session::new(); + let original = id(&s.execute("shape.rectangle", &json!({"x": 0, "y": 0, "width": 10, "height": 10})).unwrap()); + let before_doc = s.doc().unwrap().clone(); + assert_eq!(s.undo_depth(), (1, 0)); + + let result = s.execute( + "command.batch", + &json!({ + "commands": [ + {"command": "edit.undo"}, + {"command": "no.such.command"} + ] + }), + ); + + assert!(result.is_err()); + assert_eq!(s.doc().unwrap(), &before_doc); + assert_eq!(s.undo_depth(), (1, 0), "the undo step survives the failed batch"); + assert_eq!(s.execute("edit.undo", &json!({})).unwrap(), json!({"undone": true})); + assert!(!s.doc().unwrap().contains(original)); + assert_eq!(s.execute("edit.redo", &json!({})).unwrap(), json!({"redone": true})); + assert!(s.doc().unwrap().contains(original)); +} + +#[test] +fn failed_group_keeps_history_after_undo_and_redo() { + let mut s = Session::new(); + let first = id(&s.execute("shape.rectangle", &json!({"x": 0, "y": 0, "width": 10, "height": 10})).unwrap()); + let second = id(&s.execute("shape.rectangle", &json!({"x": 20, "y": 0, "width": 10, "height": 10})).unwrap()); + assert!(s.undo().unwrap()); + let before_doc = s.doc().unwrap().clone(); + let before_labels = s.undo_labels(10); + assert_eq!(s.undo_depth(), (1, 1)); + + let result = s.execute( + "command.batch", + &json!({ + "commands": [ + {"command": "edit.undo"}, + {"command": "edit.redo"}, + {"command": "no.such.command"} + ] + }), + ); + + assert!(result.is_err()); + assert_eq!(s.doc().unwrap(), &before_doc); + assert_eq!(s.undo_depth(), (1, 1)); + assert_eq!(s.undo_labels(10), before_labels); + assert!(s.undo().unwrap()); + assert!(!s.doc().unwrap().contains(first)); + assert!(s.redo().unwrap()); + assert!(s.doc().unwrap().contains(first)); + assert!(!s.doc().unwrap().contains(second)); + assert!(s.redo().unwrap()); + assert!(s.doc().unwrap().contains(second)); +} + +#[test] +fn failed_outer_group_keeps_the_undo_steps_a_nested_batch_undid() { + let mut s = Session::new(); + let original = id(&s.execute("shape.rectangle", &json!({"x": 0, "y": 0, "width": 10, "height": 10})).unwrap()); + let before_doc = s.doc().unwrap().clone(); + + let result = s.execute( + "command.batch", + &json!({ + "commands": [ + {"command": "command.batch", "params": {"commands": [{"command": "edit.undo"}]}}, + {"command": "no.such.command"} + ] + }), + ); + + assert!(result.is_err()); + assert_eq!(s.doc().unwrap(), &before_doc); + assert_eq!(s.undo_depth(), (1, 0)); + assert!(s.undo().unwrap()); + assert!(!s.doc().unwrap().contains(original)); + assert!(s.redo().unwrap()); + assert!(s.doc().unwrap().contains(original)); +} + +#[test] +fn failed_group_restores_the_small_history_limit() { + let mut s = Session::new(); + s.ds_mut().unwrap().history.limit = 1; + let first = id(&s.execute("shape.rectangle", &json!({"x": 0, "y": 0, "width": 10, "height": 10})).unwrap()); + let before_doc = s.doc().unwrap().clone(); + + let result = s.execute( + "command.batch", + &json!({ + "commands": [ + {"command": "edit.undo"}, + {"command": "shape.rectangle", "params": {"x": 20, "y": 0, "width": 10, "height": 10}}, + {"command": "no.such.command"} + ] + }), + ); + + assert!(result.is_err()); + assert_eq!(s.doc().unwrap(), &before_doc); + assert_eq!(s.undo_depth(), (1, 0)); + assert_eq!(s.ds().unwrap().history.limit, 1); + let second = id(&s.execute("shape.rectangle", &json!({"x": 40, "y": 0, "width": 10, "height": 10})).unwrap()); + assert_eq!(s.undo_depth(), (1, 0), "new edits still respect the limit"); + assert!(s.undo().unwrap()); + assert!(s.doc().unwrap().contains(first)); + assert!(!s.doc().unwrap().contains(second)); + assert!(!s.undo().unwrap(), "the oldest step was dropped"); +} + #[test] fn failed_group_keeps_the_redo_steps() { let mut s = Session::new(); diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index 2d51b958..d24600c9 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -8,8 +8,8 @@ use serde_json::{Map, Value, json}; use super::{CommandSpec, ok}; use crate::params::{ - blend, boolean, color, get, id, id_of, num, num_or, opt_index, paint_of, parse, req_id, req_index, req_num, req_string, req_targets, string, - targets, + blend, boolean, color, fraction_of, get, id, id_of, num, num_or, opt_index, paint_of, parse, req_id, req_index, req_num, req_string, req_targets, + string, targets, }; use crate::{BatchStep, EngineError, Result, Session, bad, spec}; diff --git a/crates/engine/src/cmd/edit/props.rs b/crates/engine/src/cmd/edit/props.rs index 8156d072..69476a85 100644 --- a/crates/engine/src/cmd/edit/props.rs +++ b/crates/engine/src/cmd/edit/props.rs @@ -1,6 +1,6 @@ use super::{ - BTreeMap, Effect, EngineError, Map, Node, NodeId, NodeKind, Result, Session, Value, bad, blend, boolean, color, get, json, num, ok, opt_index, - paint_of, parse, req_id, req_index, req_string, req_targets, roots, string, targets, z_sorted, + BTreeMap, Effect, EngineError, Map, Node, NodeId, NodeKind, Result, Session, Value, bad, blend, boolean, color, fraction_of, get, json, num, ok, + opt_index, paint_of, parse, req_id, req_index, req_string, req_targets, roots, string, targets, z_sorted, }; pub(super) fn batch_rename(s: &mut Session, p: &Value) -> Result { @@ -312,8 +312,8 @@ pub(super) fn paint_set(s: &mut Session, p: &Value, fills: bool) -> Result 1.0 { o / 100.0 } else { o }).clamp(0.0, 1.0) as f32; + if let Some(o) = get(p, "opacity").and_then(fraction_of) { + slot.opacity = o.clamp(0.0, 1.0) as f32; slot.color_alpha = 1.0; } if let Some(v) = boolean(p, "visible") { diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index f0275618..ec7b2cb4 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -418,5 +418,8 @@ mod rest_import_tests; #[cfg(test)] mod staging_write_tests; +#[cfg(test)] +mod thumbnail_tests; + #[cfg(test)] mod zip_cap_tests; diff --git a/crates/engine/src/cmd/file/save.rs b/crates/engine/src/cmd/file/save.rs index 593c4c21..4b8939c7 100644 --- a/crates/engine/src/cmd/file/save.rs +++ b/crates/engine/src/cmd/file/save.rs @@ -3,19 +3,38 @@ use super::load::{ext, file_stem, write}; use super::open::remember; use super::*; -/// Thumbnail PNG of the first page's content (for file browsers / the home screen). +/// Thumbnail PNG of the chosen frame, or the first page's content (file browsers / home screen). pub fn thumbnail(doc: &Document) -> Option> { - let page = doc.first_page()?; - let b = doc.page_content_bounds(page)?; - let scale = (400.0 / b.width().max(b.height()).max(1.0)).min(2.0); - let view = figcraft_render::View { - origin: (b.x0, b.y0), - scale, - width: (b.width() * scale).ceil().max(1.0) as u32, - height: (b.height() * scale).ceil().max(1.0) as u32, + let render = |page, b: figcraft_geom::Rect, solo| { + let scale = (400.0 / b.width().max(b.height()).max(1.0)).min(2.0); + let view = figcraft_render::View { + origin: (b.x0, b.y0), + scale, + width: (b.width() * scale).ceil().max(1.0) as u32, + height: (b.height() * scale).ceil().max(1.0) as u32, + }; + let opts = figcraft_render::Options { solo, ..Default::default() }; + let pix = figcraft_render::render_view(doc, page, &view, &opts)?; + figcraft_render::encode_png(&pix).ok() }; - let pix = figcraft_render::render_view(doc, page, &view, &Default::default())?; - figcraft_render::encode_png(&pix).ok() + doc.ids() + .find_map(|id| { + let n = doc.node(id)?; + if !n.kind.is_frame_like() || n.plugin_data.get("figcraft.thumbnail").map(String::as_str) != Some("true") { + return None; + } + // Hidden library-store pages are not document pages. + let page = doc.page_of(id).filter(|p| doc.pages.contains(p))?; + let b = figcraft_render::export_bounds(doc, id)?; + if ![b.x0, b.y0, b.x1, b.y1, b.width(), b.height()].into_iter().all(f64::is_finite) || b.width() <= 0.0 || b.height() <= 0.0 { + return None; + } + render(page, b, Some(id)) + }) + .or_else(|| { + let page = doc.first_page()?; + render(page, doc.page_content_bounds(page)?, None) + }) } /// The `.figcraft` file for a document: content, thumbnail and version history. diff --git a/crates/engine/src/cmd/file/thumbnail_tests.rs b/crates/engine/src/cmd/file/thumbnail_tests.rs new file mode 100644 index 00000000..139fdc5a --- /dev/null +++ b/crates/engine/src/cmd/file/thumbnail_tests.rs @@ -0,0 +1,155 @@ +use super::*; +use figcraft_color::{Color, Paint}; +use figcraft_geom::Affine; + +const RED: Color = Color::from_rgb8_const(255, 0, 0); +const GREEN: Color = Color::from_rgb8_const(0, 255, 0); +const BLUE: Color = Color::from_rgb8_const(0, 0, 255); + +fn scene() -> (Session, NodeId, NodeId) { + let mut doc = Document::new("Thumbnail"); + let page = doc.first_page().unwrap(); + let cover = doc.create(page, NodeKind::Frame, "Cover", 0.0, 600.0, 100.0, 100.0).unwrap(); + let wide = doc.create(page, NodeKind::Frame, "Wide", 700.0, 600.0, 600.0, 100.0).unwrap(); + doc.get_mut(cover).unwrap().fills = vec![Paint::solid(RED)]; + doc.get_mut(wide).unwrap().fills = vec![Paint::solid(BLUE)]; + (Session::with_document(doc), cover, wide) +} + +fn choose(s: &mut Session, id: NodeId) { + s.execute("object.setThumbnail", &json!({"id": id})).unwrap(); +} + +fn picture(doc: &Document) -> image::RgbaImage { + image::load_from_memory(&thumbnail(doc).unwrap()).unwrap().to_rgba8() +} + +#[test] +fn chosen_frame_draws_its_children_without_siblings_or_document_changes() { + let (mut s, cover, _) = scene(); + let doc = s.doc_mut().unwrap(); + let page = doc.first_page().unwrap(); + let child = doc.create(cover, NodeKind::Rectangle, "Child", 20.0, 20.0, 20.0, 20.0).unwrap(); + doc.get_mut(child).unwrap().fills = vec![Paint::solid(GREEN)]; + // Cropping the whole page to Cover's bounds would still draw this overlapping sibling. + let sibling = doc.create(page, NodeKind::Rectangle, "Overlap", 0.0, 600.0, 100.0, 100.0).unwrap(); + doc.get_mut(sibling).unwrap().fills = vec![Paint::solid(BLUE)]; + choose(&mut s, cover); + let before = s.doc().unwrap().clone(); + let png = picture(s.doc().unwrap()); + assert_eq!(png.dimensions(), (200, 200)); + assert_eq!(png.get_pixel(10, 10).0, [255, 0, 0, 255]); + assert_eq!(png.get_pixel(60, 60).0, [0, 255, 0, 255]); + assert!(!png.pixels().any(|p| p.0 == [0, 0, 255, 255]), "neither blue sibling is drawn"); + assert_eq!(s.doc().unwrap(), &before); + s.doc_mut().unwrap().set_world_transform(cover, Affine::translate((4200.0, -3600.0))).unwrap(); + assert_eq!(picture(s.doc().unwrap()), png, "moving the frame changes neither size nor contents"); +} + +#[test] +fn no_thumbnail_choice_keeps_the_first_page_content_bounds() { + let (s, _, _) = scene(); + let png = picture(s.doc().unwrap()); + assert_eq!(png.dimensions(), (400, 31)); + assert_eq!(png.get_pixel(10, 15).0, [255, 0, 0, 255]); + assert_eq!(png.get_pixel(310, 15).0, [0, 0, 255, 255]); +} + +#[test] +fn invalid_flags_and_non_frames_keep_the_default_thumbnail() { + let (mut s, cover, wide) = scene(); + let default = thumbnail(s.doc().unwrap()).unwrap(); + for value in ["false", "TRUE", ""] { + s.doc_mut().unwrap().get_mut(cover).unwrap().plugin_data.insert("figcraft.thumbnail".into(), value.into()); + assert_eq!(thumbnail(s.doc().unwrap()).unwrap(), default, "flag {value:?}"); + } + choose(&mut s, wide); + s.doc_mut().unwrap().get_mut(wide).unwrap().kind = NodeKind::Rectangle; + assert_eq!(thumbnail(s.doc().unwrap()).unwrap(), default, "only frames can be thumbnails"); + s.doc_mut().unwrap().get_mut(wide).unwrap().kind = NodeKind::Component { description: String::new(), props: Default::default() }; + assert_ne!(thumbnail(s.doc().unwrap()).unwrap(), default, "a component counts as a frame"); +} + +#[test] +fn deleting_the_chosen_frame_falls_back_to_the_remaining_page_content() { + let (mut s, cover, _) = scene(); + choose(&mut s, cover); + s.doc_mut().unwrap().remove(cover).unwrap(); + let png = picture(s.doc().unwrap()); + assert_eq!(png.dimensions(), (400, 67)); + assert_eq!(png.get_pixel(200, 30).0, [0, 0, 255, 255]); +} + +#[test] +fn invalid_thumbnail_ranges_and_pages_fall_back() { + let (mut s, _, _) = scene(); + let default = thumbnail(s.doc().unwrap()).unwrap(); + let doc = s.doc_mut().unwrap(); + let page = doc.add_page("Invalid cover"); + let cover = doc.create(page, NodeKind::Frame, "Cover", 0.0, 0.0, 100.0, 100.0).unwrap(); + choose(&mut s, cover); + for (width, height) in [(0.0, 100.0), (100.0, 0.0), (f64::INFINITY, 100.0), (f64::NAN, 100.0)] { + let n = s.doc_mut().unwrap().get_mut(cover).unwrap(); + n.width = width; + n.height = height; + assert_eq!(thumbnail(s.doc().unwrap()).unwrap(), default, "invalid range {width} x {height}"); + } + let doc = s.doc_mut().unwrap(); + let n = doc.get_mut(cover).unwrap(); + n.width = 100.0; + n.height = 100.0; + n.parent = Some(NodeId(u64::MAX)); + assert_eq!(thumbnail(doc).unwrap(), default, "missing parent page"); + doc.get_mut(cover).unwrap().parent = Some(page); + doc.pages.retain(|p| *p != page); + assert_eq!(thumbnail(doc).unwrap(), default, "page outside the document's page list"); +} + +#[test] +fn a_nested_frame_on_another_page_uses_its_page_background() { + let (mut s, _, _) = scene(); + let doc = s.doc_mut().unwrap(); + let page = doc.add_page("Cover page"); + if let NodeKind::Page { background, .. } = &mut doc.get_mut(page).unwrap().kind { + *background = GREEN; + } + let section = doc.create(page, NodeKind::Section { contents_hidden: false }, "Section", 700.0, 900.0, 300.0, 200.0).unwrap(); + let cover = doc.create(section, NodeKind::Frame, "Cover", -50.0, 80.0, 80.0, 40.0).unwrap(); + doc.get_mut(cover).unwrap().fills.clear(); + let child = doc.create(cover, NodeKind::Rectangle, "Child", 10.0, 10.0, 20.0, 10.0).unwrap(); + doc.get_mut(child).unwrap().fills = vec![Paint::solid(RED)]; + choose(&mut s, cover); + let png = picture(s.doc().unwrap()); + assert_eq!(png.dimensions(), (160, 80)); + assert_eq!(png.get_pixel(10, 10).0, [0, 255, 0, 255]); + assert_eq!(png.get_pixel(30, 30).0, [255, 0, 0, 255]); +} + +#[test] +fn custom_thumbnails_keep_the_size_limit_and_aspect_ratio() { + let (mut s, cover, _) = scene(); + choose(&mut s, cover); + for (width, height, want) in [(100.0, 100.0, (200, 200)), (800.0, 200.0, (400, 100)), (200.0, 800.0, (100, 400))] { + let n = s.doc_mut().unwrap().get_mut(cover).unwrap(); + n.width = width; + n.height = height; + assert_eq!(picture(s.doc().unwrap()).dimensions(), want); + } +} + +#[test] +fn native_save_and_fig_export_embed_the_same_chosen_thumbnail() { + let (mut s, cover, _) = scene(); + choose(&mut s, cover); + let before = s.doc().unwrap().clone(); + let expected = picture(s.doc().unwrap()); + let native = file_bytes(s.ds().unwrap()).unwrap(); + let (fig, _) = super::export::fig_bytes(s.doc().unwrap()).unwrap(); + for bytes in [native, fig] { + let png = figcraft_format::thumbnail::thumbnail_png(std::io::Cursor::new(bytes)).unwrap(); + let actual = image::load_from_memory(&png).unwrap().to_rgba8(); + assert_eq!(actual.dimensions(), (200, 200)); + assert_eq!(actual, expected); + } + assert_eq!(s.doc().unwrap(), &before); +} diff --git a/crates/engine/src/cmd/layout.rs b/crates/engine/src/cmd/layout.rs index e22ca498..1d9ae325 100644 --- a/crates/engine/src/cmd/layout.rs +++ b/crates/engine/src/cmd/layout.rs @@ -181,6 +181,19 @@ mod tests { assert!(s.execute("node.set", &json!({"id": f, "props": {"paddingLeft": "wide"}})).is_err()); } + #[test] + fn junk_axis_padding_is_refused_and_keeps_padding() { + let mut s = Session::new(); + let (f, _, _) = al_frame(&mut s); + ex(&mut s, "node.set", json!({"id": f, "props": {"padding": 16}})); + for key in ["paddingHorizontal", "paddingVertical"] { + for junk in [json!("abc"), json!({"a": 1}), json!([1, 2])] { + assert!(s.execute("node.set", &json!({"id": f, "props": {key: junk}})).is_err(), "{key}"); + assert_eq!(node(&s, f).layout.as_ref().unwrap().padding, [16.0; 4], "{key}"); + } + } + } + #[test] fn baseline_alignment_lines_up_text() { let mut s = Session::new(); diff --git a/crates/engine/src/cmd/page.rs b/crates/engine/src/cmd/page.rs index fefd8c73..2835deff 100644 --- a/crates/engine/src/cmd/page.rs +++ b/crates/engine/src/cmd/page.rs @@ -94,7 +94,7 @@ pub fn specs() -> Vec { ok() }), spec!("page.duplicate", "Duplicate page", "{id?}", true, |s, p| { - let src = id(p, "id").unwrap_or(s.page()?); + let src = page_target(s, "page.duplicate", p)?; let doc = s.doc_mut()?; let name = format!("Copy of {}", doc.get(src)?.name); let np = doc.add_page(&name); diff --git a/crates/engine/src/cmd/props.rs b/crates/engine/src/cmd/props.rs index 469fcb01..c8aeef30 100644 --- a/crates/engine/src/cmd/props.rs +++ b/crates/engine/src/cmd/props.rs @@ -9,14 +9,14 @@ use figcraft_doc::{ use figcraft_geom::{Affine, Point}; use serde_json::{Map, Value, json}; -use crate::params::{bool_of, color_of, f64_of, get, paints_of}; +use crate::params::{bool_of, color_of, f64_of, fraction_of, get, paints_of}; use crate::{Result, bad}; /// The per-side stroke weight keys, in `StrokeProps::sides` order (top, right, bottom, left). pub const STROKE_SIDES: [&str; 4] = ["strokeTopWeight", "strokeRightWeight", "strokeBottomWeight", "strokeLeftWeight"]; /// Keys `node.set` understands (documented for agents). -pub const KEYS: &str = "name, visible, locked, opacity (0-1), blend, x, y (relative to the parent frame; groups don't count), width, height, rotation (degrees), \ +pub const KEYS: &str = "name, visible, locked, opacity (0-1 or \"50%\"), blend, x, y (relative to the parent frame; groups don't count), width, height, rotation (degrees), \ fills (paints or colours; gradients as {type: linearGradient|radialGradient|angularGradient|diamondGradient, stops: [\"#hex\", ...], from?, to?} too), fill (colour, or \"none\"/null: no fill), strokes, stroke (colour, or \"none\"/null: no stroke), strokeWeight, strokeAlign (inside|center|outside), strokeSides [t,r,b,l], strokeTopWeight / strokeRightWeight / strokeBottomWeight / strokeLeftWeight (one side; turns on individual strokes), \ strokeCap / startCap / endCap, strokeJoin, miterAngle (degrees: sharper joins bevel; or miterLimit as a ratio), dashes [..], widthProfile (preset uniform|wedge|taper|eye|rounded|pinch, [[t, scale], ...] or null), brush (preset id, {preset, spacing, sizeJitter, rotationJitter, scatter, seed} or null), cornerRadius (n or [tl,tr,br,bl]), cornerSmoothing (0-1), effects, \ clip, isMask, layout (null|horizontal|vertical|grid|{...}), gap, crossGap, padding (n or [t,r,b,l]), paddingTop/Right/Bottom/Left, \ @@ -169,7 +169,7 @@ pub(crate) fn letter_spacing(v: &Value) -> Option { fn four(v: &Value) -> Option<[f64; 4]> { match v { Value::Array(a) => { - let f: Vec = a.iter().filter_map(f64_of).collect(); + let f: Vec = a.iter().map(f64_of).collect::>()?; match f.as_slice() { [x] => Some([*x; 4]), [a, b] => Some([*a, *b, *a, *b]), @@ -487,8 +487,7 @@ fn apply_look(cmd: &str, doc: &mut Document, id: NodeId, k: &str, v: &Value) -> "visible" => doc.get_mut(id)?.visible = v.as_bool().ok_or_else(|| bad(cmd, "`visible` must be a boolean"))?, "locked" => doc.get_mut(id)?.locked = v.as_bool().ok_or_else(|| bad(cmd, "`locked` must be a boolean"))?, "opacity" => { - let o = f64_of(v).ok_or_else(|| bad(cmd, "`opacity` must be a number"))?; - let o = if o > 1.0 { o / 100.0 } else { o }; + let o = fraction_of(v).ok_or_else(|| bad(cmd, "`opacity` must be a number"))?; doc.get_mut(id)?.opacity = o.clamp(0.0, 1.0) as f32; } "blend" | "blendMode" => { @@ -650,13 +649,13 @@ fn apply_layout(cmd: &str, doc: &mut Document, id: NodeId, k: &str, v: &Value) - } } "paddingHorizontal" => { - let val = f64_of(v).unwrap_or(0.0).max(0.0); + let val = f64_of(v).ok_or_else(|| bad(cmd, format!("`{k}` must be a number")))?.max(0.0); let l = layout_mut(cmd, k, doc.get_mut(id)?)?; l.padding[1] = val; l.padding[3] = val; } "paddingVertical" => { - let val = f64_of(v).unwrap_or(0.0).max(0.0); + let val = f64_of(v).ok_or_else(|| bad(cmd, format!("`{k}` must be a number")))?.max(0.0); let l = layout_mut(cmd, k, doc.get_mut(id)?)?; l.padding[0] = val; l.padding[2] = val; @@ -888,20 +887,20 @@ fn apply_shape(cmd: &str, doc: &mut Document, id: NodeId, k: &str, v: &Value) -> } } "innerRadius" | "ratio" => { - let r = f64_of(v).unwrap_or(0.38); - let r = if r > 1.0 { r / 100.0 } else { r }; + let r = fraction_of(v).unwrap_or(0.38); match &mut doc.get_mut(id)?.kind { NodeKind::Star { inner_ratio, .. } => *inner_ratio = r.clamp(0.0, 1.0), _ => return Err(bad(cmd, "innerRadius applies to stars")), } } "arcStart" | "arcSweep" | "arcInner" => { - let val = f64_of(v).ok_or_else(|| bad(cmd, format!("`{k}` must be a number")))?; + let val = if k == "arcInner" { fraction_of(v) } else { f64_of(v) }; + let val = val.ok_or_else(|| bad(cmd, format!("`{k}` must be a number")))?; match &mut doc.get_mut(id)?.kind { NodeKind::Ellipse { start, sweep, inner } => match k { "arcStart" => *start = val.to_radians(), "arcSweep" => *sweep = val.clamp(-360.0, 360.0).to_radians(), - _ => *inner = (if val > 1.0 { val / 100.0 } else { val }).clamp(0.0, 0.99), + _ => *inner = val.clamp(0.0, 0.99), }, _ => return Err(bad(cmd, "arc applies to ellipses")), } @@ -1370,4 +1369,13 @@ mod tests { } assert!(matches!(line_height(&json!("150%")), Some(LineHeight::Percent(p)) if p == 150.0)); } + + #[test] + fn four_rejects_non_numeric_entries() { + assert_eq!(four(&json!([10, "abc"])), None); + assert_eq!(four(&json!([10, "abc", null, 20])), None); + assert_eq!(four(&json!([10, 20, 30])), None); + assert_eq!(four(&json!([1, 2, 3, 4])), Some([1.0, 2.0, 3.0, 4.0])); + assert_eq!(four(&json!([1, 2])), Some([1.0, 2.0, 1.0, 2.0])); + } } diff --git a/crates/engine/src/derived.rs b/crates/engine/src/derived.rs index c618269f..d4df70ac 100644 --- a/crates/engine/src/derived.rs +++ b/crates/engine/src/derived.rs @@ -73,7 +73,12 @@ pub fn normalize(doc: &mut Document, before: &Document) { if let Some((w, h)) = figcraft_text::auto_size(n) && ((w - n.width).abs() > 1e-6 || (h - n.height).abs() > 1e-6) { + let dw = w - n.width; + let anchor_right = keeps_right_edge(doc, before, n); figcraft_layout::set_size(doc, id, w, h); + if anchor_right && let Some(n) = doc.node_mut(id) { + n.transform *= figcraft_geom::kurbo::Affine::translate((-dw, 0.0)); + } } } touched.extend(doc.take_dirty()); @@ -158,6 +163,15 @@ pub fn normalize(doc: &mut Document, before: &Document) { cmd::comment::follow_anchors(doc, before); } +/// Whether auto-width text `n` keeps its right edge when its width changes: it was already +/// right-aligned auto-width text before the edit and no auto layout flow places it. +fn keeps_right_edge(doc: &Document, before: &Document, n: &Node) -> bool { + use figcraft_doc::{TextAlignH, TextAutoResize}; + let right_auto = |n: &Node| n.kind.text().is_some_and(|t| t.align_h == TextAlignH::Right && t.auto_resize == TextAutoResize::WidthAndHeight); + let in_flow = !n.child_layout.absolute && n.parent.and_then(|p| doc.node(p)).is_some_and(|p| p.layout.is_some() && p.kind.is_frame_like()); + right_auto(n) && before.node(n.id).is_some_and(right_auto) && !in_flow +} + /// Whether layer `id` changed (since `before`) in a way that can move or resize layers in an auto /// layout frame around it: new, resized, moved, shown or hidden, restructured, other text or /// layout settings. diff --git a/crates/engine/src/page_target_tests.rs b/crates/engine/src/page_target_tests.rs index fd88685d..348ffdf3 100644 --- a/crates/engine/src/page_target_tests.rs +++ b/crates/engine/src/page_target_tests.rs @@ -61,3 +61,13 @@ fn page_commands_still_work_on_pages() { assert!(!s.doc().unwrap().contains(second)); assert!(s.execute("page.delete", &json!({"id": first})).is_err(), "the last ordinary page stays"); } + +#[test] +fn page_duplicate_rejects_a_frame() { + let mut s = Session::new(); + let f = id_of(&ex(&mut s, "frame.create", json!({"x": 0, "y": 0, "width": 100, "height": 100}))); + let pages = s.doc().unwrap().pages.clone(); + let err = s.execute("page.duplicate", &json!({"id": f})).expect_err("duplicate refuses a layer"); + assert!(err.to_string().contains("is not a page"), "{err}"); + assert_eq!(s.doc().unwrap().pages, pages, "no page was added"); +} diff --git a/crates/engine/src/panels_behaviour_tests.rs b/crates/engine/src/panels_behaviour_tests.rs index 9870aa31..d97d2507 100644 --- a/crates/engine/src/panels_behaviour_tests.rs +++ b/crates/engine/src/panels_behaviour_tests.rs @@ -475,3 +475,29 @@ fn page_navigation_skips_dividers() { ex(&mut s, "view.nextPage", json!({})); assert_eq!(s.page().unwrap(), later, "Next page stops on the last page"); } + +#[test] +fn opacity_percent_strings_always_divide_by_100() { + let mut s = Session::new(); + let r = rect(&mut s, "R", 0.0, 0.0, None); + for (v, want) in [(json!("1%"), 0.01), (json!("0.5%"), 0.005), (json!("50%"), 0.5), (json!(1), 1.0), (json!(50), 0.5), (json!(0.25), 0.25)] { + ex(&mut s, "node.set", json!({"id": r, "props": {"opacity": v.clone()}})); + let o = s.doc().unwrap().node(r).unwrap().opacity; + assert!((o - want).abs() < 1e-6, "opacity {v} gave {o}, want {want}"); + } +} + +#[test] +fn fill_opacity_star_ratio_and_arc_ratio_read_a_percent_suffix() { + let mut s = Session::new(); + let r = rect(&mut s, "R", 0.0, 0.0, None); + ex(&mut s, "fill.set", json!({"ids": [r], "color": "#ff0000", "opacity": "1%"})); + let fill = s.doc().unwrap().node(r).unwrap().fills[0].opacity; + assert!((fill - 0.01).abs() < 1e-6, "fill opacity \"1%\" gave {fill}"); + let star = id(&ex(&mut s, "shape.star", json!({"x": 0, "y": 0, "width": 100, "height": 100, "props": {"innerRadius": "1%"}}))); + let figcraft_doc::NodeKind::Star { inner_ratio, .. } = s.doc().unwrap().node(star).unwrap().kind else { panic!("star") }; + assert!((inner_ratio - 0.01).abs() < 1e-9, "innerRadius \"1%\" gave {inner_ratio}"); + let arc = id(&ex(&mut s, "shape.ellipse", json!({"x": 0, "y": 0, "width": 100, "height": 100, "props": {"arcInner": "1%"}}))); + let figcraft_doc::NodeKind::Ellipse { inner, .. } = s.doc().unwrap().node(arc).unwrap().kind else { panic!("ellipse") }; + assert!((inner - 0.01).abs() < 1e-9, "arcInner \"1%\" gave {inner}"); +} diff --git a/crates/engine/src/session/undo.rs b/crates/engine/src/session/undo.rs index 3fca87b9..6e911c20 100644 --- a/crates/engine/src/session/undo.rs +++ b/crates/engine/src/session/undo.rs @@ -208,6 +208,8 @@ impl Session { let pending_mark = self.pending_events.len(); let before: Vec<_> = self.docs.iter().map(|ds| (ds.uid, ds.doc.clone(), ds.selection.clone(), ds.page, ds.history.undo.len(), ds.history.limit)).collect(); + // An undo inside pops the undo steps; a failed group has to bring them back. + let undo_before: Vec<_> = self.docs.iter().map(|ds| ds.history.undo.clone()).collect(); // An edit inside clears the redo steps; a failed group has to bring them back. let redo_before: Vec<_> = self.docs.iter().map(|ds| ds.history.redo.clone()).collect(); let active = self.active; @@ -239,13 +241,13 @@ impl Session { }; self.docs.extend(tab); } - for ((uid, doc, selection, page, undo_len, limit), redo) in before.iter().zip(redo_before) { + for (((uid, doc, selection, page, _, limit), undo), redo) in before.iter().zip(undo_before).zip(redo_before) { if let Some(ds) = self.docs.iter_mut().find(|ds| ds.uid == *uid) { ds.history.redo = redo; ds.doc = doc.clone(); ds.selection = selection.clone(); ds.page = *page; - ds.history.undo.truncate(*undo_len); + ds.history.undo = undo; ds.history.limit = *limit; } } diff --git a/crates/engine/src/text_behaviour_tests.rs b/crates/engine/src/text_behaviour_tests.rs index 8b33e303..eac41f51 100644 --- a/crates/engine/src/text_behaviour_tests.rs +++ b/crates/engine/src/text_behaviour_tests.rs @@ -370,3 +370,168 @@ fn max_lines_on_auto_height_text_hugs_that_many_lines() { s.execute("text.maxLines", &json!({"ids": [fixed], "lines": 2})).unwrap(); assert_eq!(resize_mode(&s, fixed), TextAutoResize::Truncate); } + +/// A synthetic monospaced TrueType font (outline-less glyphs, every printable ASCII character 500 of +/// 1000 units wide), loaded into the font database so text measures the same on every machine. +fn test_mono_font() -> &'static str { + const FAMILY: &str = "FigCraft Test Mono"; + static LOADED: std::sync::OnceLock<()> = std::sync::OnceLock::new(); + LOADED.get_or_init(|| { + let families = figcraft_text::load_font_bytes(synthetic_mono_font(FAMILY)); + assert_eq!(families, [FAMILY], "the synthetic font loads"); + }); + FAMILY +} + +fn synthetic_mono_font(family: &str) -> Vec { + let be16 = |v: &mut Vec, x: u16| v.extend_from_slice(&x.to_be_bytes()); + let glyphs: u16 = 1 + (0x7E - 0x20 + 1); + let mut head = Vec::new(); + for x in [0x0001_0000u32, 0x0001_0000, 0, 0x5F0F_3CF5] { + head.extend_from_slice(&x.to_be_bytes()); + } + be16(&mut head, 0); // flags + be16(&mut head, 1000); // unitsPerEm + head.extend_from_slice(&[0; 16]); // created, modified + for x in [0u16, (-200i16) as u16, 500, 800, 0, 8, 2, 0, 0] { + be16(&mut head, x); // bbox, macStyle, lowestRecPPEM, direction hint, short loca, glyph format + } + let mut hhea = 0x0001_0000u32.to_be_bytes().to_vec(); + for x in [800u16, (-200i16) as u16, 0, 500, 0, 0, 500, 1, 0, 0, 0, 0, 0, 0, 0, glyphs] { + be16(&mut hhea, x); + } + let mut maxp = 0x0001_0000u32.to_be_bytes().to_vec(); + be16(&mut maxp, glyphs); + for x in [0u16, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0] { + be16(&mut maxp, x); + } + let mut hmtx = Vec::new(); + for _ in 0..glyphs { + be16(&mut hmtx, 500); + be16(&mut hmtx, 0); + } + let loca = vec![0; 2 * (usize::from(glyphs) + 1)]; + let glyf = vec![0; 4]; + // cmap: one Windows Unicode BMP subtable, format 4, 0x20..=0x7E → glyphs 1.. plus the end segment. + let mut cmap = Vec::new(); + for x in [0u16, 1, 3, 1, 0, 12] { + be16(&mut cmap, x); + } + for x in [4u16, 32, 0, 4, 4, 1, 0, 0x7E, 0xFFFF, 0, 0x20, 0xFFFF, (1i16 - 0x20) as u16, 1, 0, 0] { + be16(&mut cmap, x); + } + let strings = [(1u16, family.to_string()), (2, "Regular".into()), (6, family.replace(' ', "") + "-Regular")]; + let mut name = Vec::new(); + be16(&mut name, 0); + be16(&mut name, strings.len() as u16); + be16(&mut name, 6 + 12 * strings.len() as u16); + let mut storage = Vec::new(); + for (id, s) in &strings { + let utf16: Vec = s.encode_utf16().flat_map(u16::to_be_bytes).collect(); + for x in [3u16, 1, 0x409, *id, utf16.len() as u16, storage.len() as u16] { + be16(&mut name, x); + } + storage.extend(utf16); + } + name.extend(storage); + let mut post = 0x0003_0000u32.to_be_bytes().to_vec(); + post.extend_from_slice(&[0; 28]); + let tables: [(&[u8; 4], Vec); 9] = [ + (b"cmap", cmap), + (b"glyf", glyf), + (b"head", head), + (b"hhea", hhea), + (b"hmtx", hmtx), + (b"loca", loca), + (b"maxp", maxp), + (b"name", name), + (b"post", post), + ]; + let mut font = 0x0001_0000u32.to_be_bytes().to_vec(); + for x in [tables.len() as u16, 128, 3, 16] { + be16(&mut font, x); + } + let mut offset = 12 + 16 * tables.len(); + for (tag, data) in &tables { + font.extend_from_slice(*tag); + font.extend_from_slice(&0u32.to_be_bytes()); + font.extend_from_slice(&(offset as u32).to_be_bytes()); + font.extend_from_slice(&(data.len() as u32).to_be_bytes()); + offset += data.len().next_multiple_of(4); + } + for (_, data) in &tables { + font.extend_from_slice(data); + font.resize(font.len().next_multiple_of(4), 0); + } + font +} + +/// The world position of a text's top-right corner (where right-aligned text is anchored). +fn top_right(s: &Session, t: NodeId) -> (f64, f64) { + let doc = s.doc().unwrap(); + let p = doc.world_transform(t) * figcraft_geom::kurbo::Point::new(doc.node(t).unwrap().width, 0.0); + (p.x, p.y) +} + +fn assert_near(a: (f64, f64), b: (f64, f64), why: &str) { + assert!((a.0 - b.0).abs() < 1e-6 && (a.1 - b.1).abs() < 1e-6, "{why}: {a:?} != {b:?}"); +} + +#[test] +fn right_aligned_auto_width_text_grows_and_shrinks_to_the_left() { + let font = test_mono_font(); + for (src, rotation) in [("ANCHOR TEST\nabc", 0.0), ("abc", 0.0), ("ANCHOR TEST\nabc", 90.0)] { + let mut s = Session::new(); + let t = text(&mut s, src, json!({"fontSize": 40, "fontFamily": font, "textAlign": "right", "textAutoResize": "widthAndHeight"})); + s.execute("node.set", &json!({"id": t, "props": {"x": 200, "y": 50, "rotation": rotation}})).unwrap(); + let anchor = top_right(&s, t); + let w0 = size(&s, t).0; + let widest = src.lines().map(|l| l.chars().count()).max().unwrap_or(0); + assert!((w0 - 20.0 * widest as f64).abs() < 1e-6, "{src:?}: the test font measures 20 px a character, not {w0}"); + begin(&mut s, t, Some(src.chars().count()), false).unwrap(); + for c in "defghijklmnopqrstuvwxyz".chars() { + crate::textedit::insert(&mut s, &c.to_string()).unwrap(); + assert_near(top_right(&s, t), anchor, &format!("{src:?} at {rotation}°: typing {c}")); + } + assert!(size(&s, t).0 > w0 + 1e-6, "{src:?} at {rotation}°: typing grows the text"); + for _ in 0..23 { + key(&mut s, "Backspace", false, false, false).unwrap(); + assert_near(top_right(&s, t), anchor, &format!("{src:?} at {rotation}°: deleting")); + } + assert!((size(&s, t).0 - w0).abs() < 1e-6, "back to the starting width"); + } +} + +#[test] +fn right_aligned_auto_width_growth_undoes_to_its_old_place() { + let font = test_mono_font(); + let mut s = Session::new(); + let t = text(&mut s, "abc", json!({"fontSize": 40, "fontFamily": font, "textAlign": "right", "textAutoResize": "widthAndHeight"})); + let before = (s.doc().unwrap().node(t).unwrap().transform, size(&s, t)); + let anchor = top_right(&s, t); + s.execute("node.set", &json!({"id": t, "props": {"characters": "abcdefghijklmnop"}})).unwrap(); + assert_near(top_right(&s, t), anchor, "set characters"); + let after = (s.doc().unwrap().node(t).unwrap().transform, size(&s, t)); + assert!(after.1.0 > before.1.0 + 1e-6, "the text grew"); + s.execute("edit.undo", &json!({})).unwrap(); + assert_eq!((s.doc().unwrap().node(t).unwrap().transform, size(&s, t)), before, "undo"); + s.execute("edit.redo", &json!({})).unwrap(); + assert_eq!((s.doc().unwrap().node(t).unwrap().transform, size(&s, t)), after, "redo"); +} + +#[test] +fn auto_layout_places_right_aligned_auto_width_text() { + let font = test_mono_font(); + let mut s = Session::new(); + let f = id(s.execute("frame.create", &json!({"x": 0, "y": 0, "width": 200, "height": 100})).unwrap()); + s.execute("node.set", &json!({"id": f, "props": {"layout": "horizontal", "padding": 10, "gap": 0}})).unwrap(); + let t = id(s + .execute( + "text.create", + &json!({"text": "abc", "x": 0, "y": 0, "parent": f, "props": {"fontSize": 40, "fontFamily": font, "textAlign": "right", "textAutoResize": "widthAndHeight"}}), + ) + .unwrap()); + s.execute("node.set", &json!({"id": t, "props": {"characters": "abcdefghijklmnop"}})).unwrap(); + let c = s.doc().unwrap().node(t).unwrap().transform.as_coeffs(); + assert_eq!((c[4], c[5]), (10.0, 10.0), "the auto layout keeps it at its padding"); +} diff --git a/crates/format/src/sketch.rs b/crates/format/src/sketch.rs index 8a35bb5e..8186ce26 100644 --- a/crates/format/src/sketch.rs +++ b/crates/format/src/sketch.rs @@ -56,10 +56,10 @@ pub const NEWEST_TESTED_VERSION: u64 = 139; pub const TOO_OLD: &str = "saved by a Sketch older than 48; open it in a newer Sketch and save it again"; /// Largest JSON member read (a real page of a few thousand layers takes 33 MB). const MAX_JSON_MEMBER: u64 = 512 * 1024 * 1024; -/// Most bytes JSON members may unpack to, per compressed byte (real files pack 5–20 times; a -/// zero-filled array a thousand times), with a floor for small members. -const MAX_JSON_RATIO: u64 = 100; -const MIN_JSON_ALLOWANCE: u64 = 16 * 1024 * 1024; +/// Most bytes a member may unpack to, per compressed byte (real JSON packs 5–20 times and images +/// barely; a zero-filled member a thousand times), with a floor for small members. +const MAX_RATIO: u64 = 100; +const MIN_ALLOWANCE: u64 = 16 * 1024 * 1024; /// Most JSON read from one file in all. #[cfg(not(target_pointer_width = "32"))] const MAX_JSON_TOTAL: u64 = 2 * 1024 * 1024 * 1024; @@ -67,6 +67,11 @@ const MAX_JSON_TOTAL: u64 = 2 * 1024 * 1024 * 1024; const MAX_JSON_TOTAL: u64 = 512 * 1024 * 1024; /// Largest image member read. const MAX_IMAGE: u64 = figcraft_doc::ImageData::MAX_BYTES; +/// Most image bytes read from one file in all. +#[cfg(not(target_pointer_width = "32"))] +const MAX_IMAGE_TOTAL: u64 = 1024 * 1024 * 1024; +#[cfg(target_pointer_width = "32")] +const MAX_IMAGE_TOTAL: u64 = 256 * 1024 * 1024; /// Most pages read from one file. const MAX_PAGES: usize = 1000; @@ -182,6 +187,8 @@ pub(crate) struct Archive<'b> { /// Problems met while reading (pages skipped). pub warnings: Vec, zip: zip::ZipArchive>, + /// Image bytes read so far. + image_total: std::cell::Cell, } impl<'b> Archive<'b> { @@ -234,7 +241,7 @@ impl<'b> Archive<'b> { warnings.push(format!("the file has more than {MAX_PAGES} pages; the rest were skipped")); } let version = meta.get("version").and_then(Value::as_u64).unwrap_or(0); - Ok(Archive { document, version, pages, warnings, zip: z }) + Ok(Archive { document, version, pages, warnings, zip: z, image_total: std::cell::Cell::new(0) }) } /// The bytes of the image a layer references (`images/`). @@ -245,24 +252,39 @@ impl<'b> Archive<'b> { } // A clone shares the parsed directory and the bytes; the resolver only has shared access. let mut z = self.zip.clone(); - crate::read_member(&mut z, &format!("images/{name}"), MAX_IMAGE).ok() + let mut total = self.image_total.get(); + let data = capped_member(&mut z, &format!("images/{name}"), MAX_IMAGE, &mut total, MAX_IMAGE_TOTAL).ok(); + self.image_total.set(total); + data } } -/// JSON member `name`, refused before unpacking when it would be larger than `limit`, than -/// [`MAX_JSON_RATIO`] times what it takes in the archive (past a floor), or than what is left of -/// the file's [`MAX_JSON_TOTAL`] (`total`: read so far). -fn json_member(z: &mut zip::ZipArchive>, name: &str, limit: u64, total: &mut u64) -> Result { +/// Member `name`, refused before unpacking when it would be larger than `limit`, than +/// [`MAX_RATIO`] times what it takes in the archive (past a floor), or than what is left of +/// `max_total` (`total`: read so far, counted up). +fn capped_member( + z: &mut zip::ZipArchive>, + name: &str, + limit: u64, + total: &mut u64, + max_total: u64, +) -> Result, FormatError> { let (size, packed) = { let f = z.by_name(name).map_err(|e| corrupt(format!("{name}: {e}")))?; (f.size(), f.compressed_size()) }; - let allowed = limit.min(packed.saturating_mul(MAX_JSON_RATIO).max(MIN_JSON_ALLOWANCE)).min(MAX_JSON_TOTAL.saturating_sub(*total)); + let allowed = limit.min(packed.saturating_mul(MAX_RATIO).max(MIN_ALLOWANCE)).min(max_total.saturating_sub(*total)); if size > allowed { return Err(corrupt(format!("{name} is too large"))); } let data = crate::read_member(z, name, allowed)?; *total += data.len() as u64; + Ok(data) +} + +/// JSON member `name`, read by [`capped_member`] within the file's [`MAX_JSON_TOTAL`]. +fn json_member(z: &mut zip::ZipArchive>, name: &str, limit: u64, total: &mut u64) -> Result { + let data = capped_member(z, name, limit, total, MAX_JSON_TOTAL)?; serde_json::from_slice(&data).map_err(|e| corrupt(format!("{name}: {e}"))) } diff --git a/crates/format/src/sketch/tree_tests.rs b/crates/format/src/sketch/tree_tests.rs index 5399a3df..738f6e40 100644 --- a/crates/format/src/sketch/tree_tests.rs +++ b/crates/format/src/sketch/tree_tests.rs @@ -158,6 +158,20 @@ fn oval_slice_and_bitmap() { assert_eq!(doc.images.len(), 1); } +#[test] +fn image_bombs_are_refused() { + // A valid 2×2 PNG followed by 40 MB of zeros packs to a few dozen kilobytes. + let mut png = Vec::new(); + image::RgbaImage::new(2, 2).write_to(&mut std::io::Cursor::new(&mut png), image::ImageFormat::Png).unwrap(); + png.resize(png.len() + 40 * 1024 * 1024, 0); + let mut bitmap = rect("Photo", 0.0, 0.0, 30.0, 20.0); + bitmap["_class"] = json!("bitmap"); + bitmap["image"] = json!({"_class": "MSJSONFileReference", "_ref_class": "MSImageData", "_ref": "images/bomb.png"}); + let (doc, _) = open_with(vec![artboard("A", "Board", 0.0, 0.0, 100.0, 100.0, vec![bitmap])], &[("images/bomb.png", &png)]); + let total: usize = doc.images.values().map(|i| i.bytes.len()).sum(); + assert!(total < 1024 * 1024, "{total} image bytes kept"); +} + #[test] fn unknown_class_is_a_warning() { let mut odd = rect("Odd", 0.0, 0.0, 10.0, 10.0); diff --git a/crates/mcp/src/backend.rs b/crates/mcp/src/backend.rs index f2021cc6..1c2ab141 100644 --- a/crates/mcp/src/backend.rs +++ b/crates/mcp/src/backend.rs @@ -1,7 +1,7 @@ //! Where MCP tool calls end up: a control-channel method call, answered either by an in-process //! engine session ([`crate::Headless`]) or by a running FigCraft app ([`Attached`]). -use std::io::{BufRead, BufReader, Write}; +use std::io::{BufRead, BufReader, Read, Write}; use std::net::{TcpStream, ToSocketAddrs}; use std::time::Duration; @@ -76,17 +76,23 @@ pub fn decode_data(v: &Value) -> Result, String> { base64::engine::general_purpose::STANDARD.decode(d.trim()).map_err(|e| format!("bad base64 from the app: {e}")) } +/// The longest reply line we read from the app (512 MiB). The app's window captures alone reach +/// 256 MiB as base64 (192 MiB PNGs); anything longer is refused without reading the rest. +const MAX_REPLY: usize = 512 * 1024 * 1024; + /// A running FigCraft app, reached through its loopback control port (`figcraft --control PORT`). pub struct Attached { addr: String, conn: Option<(BufReader, TcpStream)>, next_id: u64, + /// [`MAX_REPLY`]; smaller in tests. + max_reply: usize, } impl Attached { /// Connect to `addr` (e.g. `127.0.0.1:7979`), failing fast when nothing is listening. pub fn connect(addr: &str) -> std::io::Result { - let mut r = Self { addr: addr.to_string(), conn: None, next_id: 1 }; + let mut r = Self { addr: addr.to_string(), conn: None, next_id: 1, max_reply: MAX_REPLY }; r.reconnect()?; Ok(r) } @@ -133,17 +139,25 @@ impl Attached { } fn roundtrip(&mut self, line: &str) -> std::io::Result { + let max_reply = self.max_reply; let Some((reader, writer)) = self.conn.as_mut() else { return Err(std::io::Error::new(std::io::ErrorKind::NotConnected, "not connected to the app")); }; writer.write_all(line.as_bytes())?; writer.write_all(b"\n")?; writer.flush()?; - let mut reply = String::new(); - if reader.read_line(&mut reply)? == 0 { + let mut reply = Vec::new(); + if reader.take((max_reply + 1) as u64).read_until(b'\n', &mut reply)? == 0 { return Err(std::io::Error::new(std::io::ErrorKind::UnexpectedEof, "control channel closed")); } - Ok(reply) + if reply.last() == Some(&b'\n') { + reply.pop(); + } + if reply.len() > max_reply { + let mib = max_reply / (1024 * 1024); + return Err(std::io::Error::new(std::io::ErrorKind::InvalidData, format!("reply larger than {mib} MiB"))); + } + String::from_utf8(reply).map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e)) } } @@ -181,3 +195,7 @@ impl Backend for Attached { format!("attached to the FigCraft app at {}", self.addr) } } + +#[cfg(test)] +#[path = "backend_tests.rs"] +mod tests; diff --git a/crates/mcp/src/backend_tests.rs b/crates/mcp/src/backend_tests.rs new file mode 100644 index 00000000..ec4fc473 --- /dev/null +++ b/crates/mcp/src/backend_tests.rs @@ -0,0 +1,77 @@ +//! The attached backend reads the app's replies with a bound (#1113). + +use std::io::{BufRead, BufReader, Write}; +use std::net::TcpListener; +use std::sync::mpsc; +use std::time::Duration; + +use serde_json::json; + +use super::{Attached, Backend}; + +/// A fake app that takes one connection per reply, reads one request on it and answers with the +/// reply. It keeps the connections open until [`finish`] (so an unterminated reply never reaches +/// EOF), then returns how many requests it read. +fn fake_app(replies: Vec>) -> (u16, mpsc::Sender<()>, std::thread::JoinHandle) { + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + let port = listener.local_addr().unwrap().port(); + let (done, wait) = mpsc::channel::<()>(); + let app = std::thread::spawn(move || { + let mut open = Vec::new(); + for reply in &replies { + let (stream, _) = listener.accept().unwrap(); + let mut out = stream.try_clone().unwrap(); + let mut line = String::new(); + BufReader::new(stream).read_line(&mut line).unwrap(); + out.write_all(reply).unwrap(); + out.flush().unwrap(); + open.push(out); + } + wait.recv_timeout(Duration::from_secs(30)).ok(); + open.len() + }); + (port, done, app) +} + +/// Let the fake app close its connections, and check it read `requests` requests. +fn finish(done: mpsc::Sender<()>, app: std::thread::JoinHandle, requests: usize) { + drop(done); + assert_eq!(app.join().unwrap(), requests); +} + +fn ok_reply(text: &str) -> Vec { + format!("{}\n", json!({"id": 1, "ok": true, "result": text})).into_bytes() +} + +#[test] +fn an_oversized_reply_is_refused_and_the_next_call_reconnects() { + let (port, done, fake) = fake_app(vec![vec![b'x'; 200], ok_reply("fine")]); + let mut app = Attached::port(port).unwrap(); + app.max_reply = 64; + let err = app.call("engine.commands", json!({})).unwrap_err(); + assert!(err.contains("reply larger than"), "{err}"); + assert!(err.contains("may have completed"), "{err}"); + assert!(app.conn.is_none()); + assert_eq!(app.call("engine.commands", json!({})).unwrap(), json!("fine")); + finish(done, fake, 2); +} + +#[test] +fn a_reply_at_the_limit_is_read() { + let reply = ok_reply("at the limit"); + let (port, done, fake) = fake_app(vec![reply.clone()]); + let mut app = Attached::port(port).unwrap(); + app.max_reply = reply.len() - 1; // the newline doesn't count + assert_eq!(app.call("engine.commands", json!({})).unwrap(), json!("at the limit")); + finish(done, fake, 1); +} + +#[test] +fn a_reply_that_is_not_utf8_is_an_error() { + let (port, done, fake) = fake_app(vec![b"{\"ok\": true, \"result\": \"\xff\"}\n".to_vec()]); + let mut app = Attached::port(port).unwrap(); + let err = app.call("engine.commands", json!({})).unwrap_err(); + assert!(err.contains("failed"), "{err}"); + assert!(app.conn.is_none()); + finish(done, fake, 1); +} diff --git a/crates/pathops/src/dot_dash_tests.rs b/crates/pathops/src/dot_dash_tests.rs new file mode 100644 index 00000000..d964f3a1 --- /dev/null +++ b/crates/pathops/src/dot_dash_tests.rs @@ -0,0 +1,40 @@ +use super::*; + +#[test] +fn dense_round_dashes_fall_back_before_expansion() { + let mut line = BezPath::new(); + line.move_to((0.0, 0.0)); + line.line_to((10_000.0, 0.0)); + let dangerous = StrokeProps { + weight: 2.0, + dashes: vec![0.0, 0.02], + cap_start: StrokeCap::Round, + cap_end: StrokeCap::Round, + align: StrokeAlign::Center, + ..Default::default() + }; + let outlined = stroke_center(&line, 2.0, &dangerous); + let solid = stroke_center(&line, 2.0, &StrokeProps { dashes: vec![], ..dangerous }); + assert!((outlined.area().abs() - solid.area().abs()).abs() < 0.01); + assert!(outlined.elements().len() < 1000, "fallback must not emit hundreds of thousands of dots"); +} + +#[test] +fn zero_length_round_dashes_draw_dots() { + let mut line = BezPath::new(); + line.move_to((0.0, 0.0)); + line.line_to((100.0, 0.0)); + let round = StrokeProps { + weight: 2.0, + dashes: vec![0.0, 9.0], + cap_start: StrokeCap::Round, + cap_end: StrokeCap::Round, + align: StrokeAlign::Center, + ..Default::default() + }; + let dots = stroke_center(&line, 2.0, &round); + let area = dots.area().abs(); + assert!((area - 12.0 * std::f64::consts::PI).abs() < 1.5, "{area}"); + let butt = StrokeProps { cap_start: StrokeCap::None, cap_end: StrokeCap::None, ..round }; + assert!(stroke_center(&line, 2.0, &butt).area().abs() < 1e-6); +} diff --git a/crates/pathops/src/lib.rs b/crates/pathops/src/lib.rs index 03261fc6..23528e25 100644 --- a/crates/pathops/src/lib.rs +++ b/crates/pathops/src/lib.rs @@ -10,6 +10,8 @@ pub mod brush; #[cfg(test)] mod brush_tests; #[cfg(test)] +mod dot_dash_tests; +#[cfg(test)] mod ops_tests; mod points; #[cfg(test)] @@ -309,7 +311,9 @@ pub fn stroke_center(path: &BezPath, width: f64, props: &StrokeProps) -> BezPath .with_end_cap(kcap(props.cap_end)); // Each subpath restarts the pattern, so it can add up to one more pattern's worth of dashes. let subpaths = path.elements().iter().filter(|e| matches!(e, PathEl::MoveTo(_))).count() as f64; - if let Some(pat) = width::dash_pattern(props).filter(|p| width::dashes_fit(path.perimeter(1.0) + subpaths * p.iter().sum::(), p)) { + let round = [props.cap_start, props.cap_end].into_iter().any(|c| matches!(kcap(c), Cap::Round)); + let pattern = width::dash_pattern(props).map(|p| p.into_iter().map(|d| if round && d == 0.0 { 1e-3 } else { d }).collect::>()); + if let Some(pat) = pattern.filter(|p| width::dashes_fit(path.perimeter(1.0) + subpaths * p.iter().sum::(), p)) { style = style.with_dashes(0.0, pat); } kurbo::stroke(path.iter(), &style, &kurbo::StrokeOpts::default(), 0.05) diff --git a/crates/svg/src/import.rs b/crates/svg/src/import.rs index dc936260..ea9c321f 100644 --- a/crates/svg/src/import.rs +++ b/crates/svg/src/import.rs @@ -718,6 +718,45 @@ fn options(data: &[u8], limits: budget::Limits) -> usvg::Options<'static> { o } +/// Length of the `` declaration at the start of `s`, skipping `>` inside quotes and an +/// internal subset (`[ ... ]`). +fn doctype_end(s: &str) -> Option { + let mut quote = None; + let mut depth = 0u32; + let mut skip_to = 0; + for (i, c) in s.char_indices() { + if i < skip_to { + continue; + } + if quote.is_none() && depth > 0 { + // Comments and processing instructions in the subset may hold quotes and brackets. + let rest = &s[i..]; + let close = if rest.starts_with("") + } else if rest.starts_with("") + } else { + None + }; + if let Some(close) = close { + let len = rest.find(close)? + close.len(); + skip_to = i + len; + continue; + } + } + match (quote, c) { + (Some(q), _) if c == q => quote = None, + (Some(_), _) => {} + (None, '"' | '\'') => quote = Some(c), + (None, '[') => depth += 1, + (None, ']') => depth = depth.saturating_sub(1), + (None, '>') if depth == 0 => return Some(i + 1), + _ => {} + } + } + None +} + /// Whether `text` (from the clipboard, say) is an SVG document: ` bool { @@ -731,7 +770,7 @@ pub fn looks_like_svg(text: &str) -> bool { } else if s.starts_with("").map(|i| i + 3) } else if s.starts_with("').map(|i| i + 1) + doctype_end(s) } else { None }; diff --git a/crates/svg/src/import/shape_tests.rs b/crates/svg/src/import/shape_tests.rs index 5bad142f..71d0f508 100644 --- a/crates/svg/src/import/shape_tests.rs +++ b/crates/svg/src/import/shape_tests.rs @@ -107,3 +107,14 @@ fn layer_names_decode_id_escapes() { assert_eq!(layer_name("", "Group"), "Group"); assert_eq!(layer_name("é_x41_", "Group"), "éA"); } + +#[test] +fn doctype_with_internal_subset_is_still_svg() { + let s = "\n\n]>\n"; + assert!(super::looks_like_svg(s)); + let quoted = "y]\"> ]>"; + assert!(super::looks_like_svg(quoted)); + let commented = " --> ]>"; + assert!(super::looks_like_svg(commented)); + assert!(!super::looks_like_svg(" ]>")); +} diff --git a/crates/vectordrawable/src/tests/strokes.rs b/crates/vectordrawable/src/tests/strokes.rs index b29b2715..1d179824 100644 --- a/crates/vectordrawable/src/tests/strokes.rs +++ b/crates/vectordrawable/src/tests/strokes.rs @@ -42,6 +42,15 @@ fn a_miter_limit_other_than_four_is_written() { assert_eq!(paths_of(&out.xml)[0].get("strokeMiterLimit"), Some("10")); } +#[test] +fn a_huge_miter_limit_is_written_as_a_finite_number() { + let (mut d, r) = stroked(StrokeAlign::Center); + d.get_mut(r).unwrap().stroke.miter_limit = 1e306; + let out = export(&d, &[r], Compat::BestEffort); + let v = paths_of(&out.xml)[0].get("strokeMiterLimit").unwrap().to_string(); + assert!(v.parse::().unwrap().is_finite(), "{v}"); +} + #[test] fn a_uniform_scale_scales_the_stroke_width() { let (mut d, r) = stroked(StrokeAlign::Center); diff --git a/crates/vectordrawable/src/xml.rs b/crates/vectordrawable/src/xml.rs index 48d7dee2..62b6d5b6 100644 --- a/crates/vectordrawable/src/xml.rs +++ b/crates/vectordrawable/src/xml.rs @@ -19,6 +19,9 @@ pub(crate) fn num(v: f64) -> String { if !v.is_finite() { return "0".into(); } + // Android reads these as 32-bit floats, so clamp to that range; this also keeps the scaling + // below from overflowing to infinity. + let v = v.clamp(-f64::from(f32::MAX), f64::from(f32::MAX)); let r = (v * 1000.0).round() / 1000.0; let s = format!("{r}"); if s == "-0" { "0".into() } else { s } diff --git a/docs/control-protocol.md b/docs/control-protocol.md index b7d374ae..64534074 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -29,6 +29,8 @@ active connections**; additional connections are closed immediately. Socket read have a **60 s inactivity timeout**, so idle clients and clients that stop reading release their worker. This bounds blocked socket operations, not the total duration of a frame arriving slowly. Clients can reconnect after an idle timeout (the attached MCP backend already does). +Replies have no size limit on the app's side. The attached MCP backend reads them up to **512 MiB +before the newline** and drops the connection on a longer one (see [MCP server](mcp.md)). Both the socket queue and the UI queue hold at most **8 requests** each, including requests waiting for another window's frame. A full socket queue replies with `"control request queue is full"` diff --git a/docs/mcp.md b/docs/mcp.md index d460dbf5..ce2f1d07 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -23,6 +23,10 @@ Attached calls are never automatically replayed after a transport failure: the a applied the request before its reply was lost. The error reports that the operation may have completed; inspect the document before retrying an edit. The next explicit call reconnects. If that connection cannot be established, the error says the request was not sent. +A reply from the app is read up to **512 MiB before the newline** (the largest window capture is +256 MiB as base64). The backend stops reading a longer reply at the first byte past the limit and +treats it like a lost reply: the connection is closed, the operation may have completed, and the +next call reconnects. For larger exports, give `export` a `path` or lower the scale. A connection the app closed while it sat idle is noticed before the next request is written and replaced then, so a pause between calls is not an error. diff --git a/docs/roadmap.md b/docs/roadmap.md index 2ab450dc..060c7563 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -1,6 +1,6 @@ # FigCraft roadmap: milestones and what's next -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-28 · **Change:** minor (Arabic draft catalog; Android Vector Drawable export; a selected unfilled shape drags over a filled layer below it (#1370); text truncation fills the last line, survives resizing, and auto height takes Max lines (#1375, #1376, #1377); M11 widget fields and property menus; M12 Boards: timer and voting; Installed weights of an embedded font family (#1246); `.sketch` import, best effort; Adobe Fonts found on macOS (#298); M12 Boards; community fixes in #507, #510, #529 and #530: auto-save, a text box by dragging, batch step references, removing someone from a live session, Russian and Turkish catalogs reviewed, Czech and Indonesian drafts, the Japanese draft completed, zoom previews, the web clipboard, Alt-drag snapping and Cmd+D repeat, recovery copies, crash and rollback fixes, colour emoji, `.fig` import fixes; earlier changes are in the revision history below) · **Target:** Figma Design (desktop app 126.10.7) +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-28 · **Change:** minor (the attached MCP server reads the app's replies with a 512 MiB bound (#1113); a failed batch keeps the undo steps it undid (#1410); Arabic draft catalog; Android Vector Drawable export; a selected unfilled shape drags over a filled layer below it (#1370); text truncation fills the last line, survives resizing, and auto height takes Max lines (#1375, #1376, #1377); M11 widget fields and property menus; M12 Boards: timer and voting; Installed weights of an embedded font family (#1246); `.sketch` import, best effort; Adobe Fonts found on macOS (#298); M12 Boards; community fixes in #507, #510, #529 and #530: auto-save, a text box by dragging, batch step references, removing someone from a live session, Russian and Turkish catalogs reviewed, Czech and Indonesian drafts, the Japanese draft completed, zoom previews, the web clipboard, Alt-drag snapping and Cmd+D repeat, recovery copies, crash and rollback fixes, colour emoji, `.fig` import fixes; earlier changes are in the revision history below) · **Target:** Figma Design (desktop app 126.10.7) Forward-looking plan. Where FigCraft stands today: [`ROADMAP.md`](../ROADMAP.md) and [`target-app-parity.md`](target-app-parity.md); the full ranked list: [`gaps.md`](gaps.md). Hours @@ -76,6 +76,8 @@ blockers. | Date | Change | Summary | |---|---|---| | 2026-10-28 | minor | Arabic draft: 1,857 UI strings, 620 command labels, 31 menu wordings; count-neutral plurals and web font gate. Native review and panel mirroring remain open; grade and estimates unchanged. Evidence: `crates/ui-egui/locales/ar.toml`, i18n catalog tests and [translation notes](translating.md#arabic-draft) | +| 2026-10-27 | minor | Bounded attached MCP replies (M8, #1113): `figcraft-cli mcp --attach` read each reply from the app's control port with no size limit, so a process listening on that port could make it buffer until memory ran out. Replies are now read up to 512 MiB (an app window capture is at most 256 MiB as base64); reading stops at the first byte past that, the connection is closed and the call is reported like any other lost reply, and the next call reconnects. Regression tests in the MCP backend tests. Numbers unchanged | +| 2026-10-27 | minor | Undo kept after a failed batch (M8, #1410): a batch that undid an earlier edit and then failed restored the document but lost that edit's undo step; a failed group now restores the undo entries as well as the redo entries, so the earlier edit can still be undone and redone. Regression tests in the batch rollback suite. Numbers unchanged | | 2026-10-26 | minor | Dragging a selected unfilled shape (M1, #1370): a press inside a selected stroke-only shape over a filled layer stacked below it moved that layer; the drag-from-inside fallback of #910 now treats any layer painted below the selection like the frame or canvas behind it, so the selection drags, while a layer painted above it still takes the press. Regression test in the selection behaviour suite. Numbers unchanged | | 2026-10-26 | minor | Text truncation (M4, #1375, #1376, #1377): the last line shown takes as much of the next word as fits before the ellipsis ("Lorem consec…", not "Lorem…"); an explicit height, `transform.resize`, a resize handle or fixed sizing on a truncated text keeps it truncating instead of turning it fixed size; Max lines on auto-height text hugs up to that many lines and ends in an ellipsis (`text.maxLines`, `node.set {maxLines}`, Type settings, `.fig` export and import), and `node.get` / `get_node` report `maxLines`. Regression tests in the text and engine behaviour suites. Numbers unchanged | | 2026-10-24 | minor | Installed weights of an embedded font family (M7, #1246): release builds embed some families with only a few styles (Poppins Regular, Italic and Bold), and the first font scan dropped every installed face of such a family, so an installed Poppins SemiBold or Medium was never offered (a rescan did offer them). The first scan now keeps the embedded faces, which still win for the styles they cover, and offers the installed styles they lack beside them. Regression test in the font database tests. Numbers unchanged |