From 576d8e228ce8efd100cb1a2849c064f13955119f Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Mon, 26 Oct 2026 22:46:33 +0100 Subject: [PATCH 01/18] Cap Sketch image members by compression ratio and total size Found while testing crates/format/src/sketch/mod.rs:241-249 (Archive::image, MAX_IMAGE at :69), called once per distinct imageRef from crates/format/src/rest/mod.rs:951 (Ctx::image_key) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Opus 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/format/src/sketch/mod.rs | 28 ++++++++++++++++++++++++-- crates/format/src/sketch/tree_tests.rs | 14 +++++++++++++ 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/crates/format/src/sketch/mod.rs b/crates/format/src/sketch/mod.rs index 985541b1..cd0a6830 100644 --- a/crates/format/src/sketch/mod.rs +++ b/crates/format/src/sketch/mod.rs @@ -67,6 +67,15 @@ 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 = 256 * 1024 * 1024; +/// Most bytes an image member may unpack to, per compressed byte (images are already compressed, +/// a zero-filled one packs a thousand times), with a floor for small members. +const MAX_IMAGE_RATIO: u64 = 100; +const MIN_IMAGE_ALLOWANCE: u64 = 16 * 1024 * 1024; +/// 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 +191,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 +245,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,7 +256,20 @@ 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 member = format!("images/{name}"); + let (size, packed) = { + let f = z.by_name(&member).ok()?; + (f.size(), f.compressed_size()) + }; + let allowed = MAX_IMAGE + .min(packed.saturating_mul(MAX_IMAGE_RATIO).max(MIN_IMAGE_ALLOWANCE)) + .min(MAX_IMAGE_TOTAL.saturating_sub(self.image_total.get())); + if size > allowed { + return None; + } + let data = crate::read_member(&mut z, &member, allowed).ok()?; + self.image_total.set(self.image_total.get() + data.len() as u64); + Some(data) } } 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); From 213cc75ff22e79aa434b57033157851a642e2805 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Tue, 27 Oct 2026 04:13:59 +0100 Subject: [PATCH 02/18] Fix looks_like_svg rejecting DOCTYPE with an internal subset Found while testing crates/svg/src/import/mod.rs:723-740 (looks_like_svg, the `')) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/svg/src/import.rs | 41 +++++++++++++++++++++++++++- crates/svg/src/import/shape_tests.rs | 11 ++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) 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(" ]>")); +} From aeaca0e1b9bdef028c8a99b2fa1d74b06fcdd6f0 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Tue, 27 Oct 2026 03:21:33 +0000 Subject: [PATCH 03/18] Reject cornerRadius/padding/strokeSides arrays with non-numeric entries Found while testing crates/engine/src/cmd/props.rs:165-175 (four, used at :506, :540 and :613) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/cmd/props.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/cmd/props.rs b/crates/engine/src/cmd/props.rs index 30a10393..957aeb32 100644 --- a/crates/engine/src/cmd/props.rs +++ b/crates/engine/src/cmd/props.rs @@ -165,7 +165,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]), @@ -1291,4 +1291,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])); + } } From 8ae76d9b2a8a822934b707c418afb32a7e2601c5 Mon Sep 17 00:00:00 2001 From: Mert Dede Date: Tue, 27 Oct 2026 07:57:53 +0300 Subject: [PATCH 04/18] M8: bound attached MCP replies to 512 MiB (#1113) The attached backend read each reply from the app's control port with read_line, which has no limit. It now reads up to 512 MiB before the newline (an app window capture is at most 256 MiB as base64) and stops at the first byte past that. An oversized or non-UTF-8 reply drops the connection and is reported like a lost reply; the next call reconnects. Co-Authored-By: Claude Opus 5.5 Co-Authored-By: Claude Fable 5.1 Co-Authored-By: GPT-6.1 Sol Co-Authored-By: GPT-6 Astra --- crates/mcp/src/backend.rs | 28 +++++++++--- crates/mcp/src/backend_tests.rs | 77 +++++++++++++++++++++++++++++++++ docs/control-protocol.md | 2 + docs/mcp.md | 4 ++ docs/roadmap.md | 3 +- 5 files changed, 108 insertions(+), 6 deletions(-) create mode 100644 crates/mcp/src/backend_tests.rs 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/docs/control-protocol.md b/docs/control-protocol.md index 448a9979..2f211950 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 05dfd1e6..cbf989cf 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 14d734ba..d9df3e02 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-26 · **Change:** minor (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-27 · **Change:** minor (the attached MCP server reads the app's replies with a 512 MiB bound (#1113); 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 @@ -75,6 +75,7 @@ blockers. | Date | Change | Summary | |---|---|---| +| 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-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 | From d3bde76bafd4f06b7afac68b17291961effb73a2 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Tue, 27 Oct 2026 06:04:47 +0000 Subject: [PATCH 05/18] Clamp Vector Drawable numbers to the f32 range so they stay finite Found while testing crates/vectordrawable/src/xml.rs:22 (num), reached via stroke_attrs at :189 Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/vectordrawable/src/tests/strokes.rs | 9 +++++++++ crates/vectordrawable/src/xml.rs | 3 +++ 2 files changed, 12 insertions(+) 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 } From f1264cf13e62fc44e84d929b397464955dd8e8da Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Tue, 27 Oct 2026 08:35:02 +0000 Subject: [PATCH 06/18] Reject '+' sign in NodeId::from_str Found while testing crates/doc/src/lib.rs:64-71 (impl FromStr for NodeId), reached through crates/engine/src/automation/params.rs:78 (id_of) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/doc/src/ids_tests.rs | 8 ++++++++ crates/doc/src/lib.rs | 4 ++++ 2 files changed, 12 insertions(+) diff --git a/crates/doc/src/ids_tests.rs b/crates/doc/src/ids_tests.rs index 4f489886..350d198c 100644 --- a/crates/doc/src/ids_tests.rs +++ b/crates/doc/src/ids_tests.rs @@ -110,3 +110,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 caf0c861..8307296d 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) } } From d21c9d4292e5223b5b52d6fe5b8d920c83546c3e Mon Sep 17 00:00:00 2001 From: Mert Dede Date: Tue, 27 Oct 2026 12:55:48 +0300 Subject: [PATCH 07/18] M8: keep undo entries after a failed batch undoes an earlier edit (#1410) Session::grouped saved only the undo stack's length, so a failed group could drop the entries it added but not bring back the ones an edit.undo inside it popped. It now keeps each document's undo entries on entry, as it already did for redo, and restores them when the group fails. Co-Authored-By: Claude Opus 5.5 Co-Authored-By: Claude Fable 5.1 Co-Authored-By: GPT-6.1 Sol Co-Authored-By: GPT-6 Astra --- crates/engine/src/batch_rollback_tests.rs | 199 ++++++++++++++++++++++ crates/engine/src/session/undo.rs | 6 +- docs/roadmap.md | 3 +- 3 files changed, 205 insertions(+), 3 deletions(-) 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/session/undo.rs b/crates/engine/src/session/undo.rs index a354ea27..abbce5bb 100644 --- a/crates/engine/src/session/undo.rs +++ b/crates/engine/src/session/undo.rs @@ -204,6 +204,8 @@ impl Session { self.ds()?; 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; @@ -233,13 +235,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/docs/roadmap.md b/docs/roadmap.md index 14d734ba..ed2ad933 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-26 · **Change:** minor (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-27 · **Change:** minor (a failed batch keeps the undo steps it undid (#1410); 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 @@ -75,6 +75,7 @@ blockers. | Date | Change | Summary | |---|---|---| +| 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 | From 3b78ca9558278261cee2370cc399e99237c2b217 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Tue, 27 Oct 2026 11:57:38 +0100 Subject: [PATCH 08/18] Treat a % suffix on node.set opacity as a percentage Found while testing crates/engine/src/cmd/props.rs:476-479 (the "opacity" arm), with crates/engine/src/automation/params.rs:14-20 (f64_of) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/cmd/props.rs | 3 ++- crates/engine/src/panels_behaviour_tests.rs | 11 +++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/cmd/props.rs b/crates/engine/src/cmd/props.rs index 30a10393..723dd717 100644 --- a/crates/engine/src/cmd/props.rs +++ b/crates/engine/src/cmd/props.rs @@ -475,7 +475,8 @@ pub fn apply(cmd: &str, doc: &mut Document, id: NodeId, props: &Map 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 percent = v.as_str().is_some_and(|s| s.trim().ends_with('%')); + let o = if percent || o > 1.0 { o / 100.0 } else { o }; doc.get_mut(id)?.opacity = o.clamp(0.0, 1.0) as f32; } "blend" | "blendMode" => { diff --git a/crates/engine/src/panels_behaviour_tests.rs b/crates/engine/src/panels_behaviour_tests.rs index 9870aa31..202fc4f2 100644 --- a/crates/engine/src/panels_behaviour_tests.rs +++ b/crates/engine/src/panels_behaviour_tests.rs @@ -475,3 +475,14 @@ 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}"); + } +} From 9d4a3c6d8674e3916c401c26da9b178c68ba81b6 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Tue, 27 Oct 2026 11:15:17 +0000 Subject: [PATCH 09/18] Reject colour arrays containing non-numeric entries in color_of Found while testing crates/engine/src/automation/params.rs:170 (color_of, Value::Array arm) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/automation/params.rs | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/automation/params.rs b/crates/engine/src/automation/params.rs index 35a74e0e..222348a4 100644 --- a/crates/engine/src/automation/params.rs +++ b/crates/engine/src/automation/params.rs @@ -167,7 +167,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 +278,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))); + } +} From d4471376caf73b5a35f9afb3ce11fa70fa63dc82 Mon Sep 17 00:00:00 2001 From: Zaxaerith Date: Tue, 27 Oct 2026 21:50:46 +0800 Subject: [PATCH 10/18] fix(engine): honor selected frame for file thumbnails (#1428) --- crates/engine/src/cmd/file/save.rs | 198 +++++++++++++++++++++++++++-- 1 file changed, 187 insertions(+), 11 deletions(-) diff --git a/crates/engine/src/cmd/file/save.rs b/crates/engine/src/cmd/file/save.rs index 593c4c21..e2716bc5 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 !matches!(n.kind, NodeKind::Frame) || 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. @@ -223,3 +242,160 @@ pub(super) fn save_as(s: &mut Session, p: &Value) -> Result { } } } + +#[cfg(test)] +mod thumbnail_tests { + 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"); + } + + #[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::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); + } +} From 2d02f289ec849d26c1ad4a461c3a3a956e70fcb6 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Wed, 28 Oct 2026 02:16:00 +0100 Subject: [PATCH 11/18] Keep session joins working when a document has out-of-range ids Fixes #1067 Co-Authored-By: Claude Opus 5.5 --- crates/collab/src/entry_ids_tests.rs | 31 ++++++++++++++++++++++++++++ crates/collab/src/server.rs | 16 +++++++++----- crates/doc/src/ids.rs | 5 +++-- crates/doc/src/ids_tests.rs | 4 ++++ 4 files changed, 49 insertions(+), 7 deletions(-) diff --git a/crates/collab/src/entry_ids_tests.rs b/crates/collab/src/entry_ids_tests.rs index db58bb2f..976d2cb6 100644 --- a/crates/collab/src/entry_ids_tests.rs +++ b/crates/collab/src/entry_ids_tests.rs @@ -92,6 +92,37 @@ 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 a space nobody was given yet. + for (k, space) in [c.id() + 1, c.id() + 1000, 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}"); + } +} + /// 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 52f112ad..54089af6 100644 --- a/crates/collab/src/server.rs +++ b/crates/collab/src/server.rs @@ -153,7 +153,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; @@ -481,9 +487,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 of one it hasn't given out yet + /// (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(); @@ -510,7 +516,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 >= self.next_client)) { 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..7c6a50e0 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); From 2199dcb2da7116fa1b7dacd6ace836929191abeb Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Wed, 28 Oct 2026 13:04:10 +0000 Subject: [PATCH 12/18] Reject non-numeric paddingHorizontal/paddingVertical in node.set Found while testing crates/engine/src/cmd/props.rs:623-634 (the "paddingHorizontal" and "paddingVertical" arms) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/cmd/layout.rs | 13 +++++++++++++ crates/engine/src/cmd/props.rs | 4 ++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/cmd/layout.rs b/crates/engine/src/cmd/layout.rs index 5a0a4000..22f50b0f 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/props.rs b/crates/engine/src/cmd/props.rs index 30a10393..8c09a9b6 100644 --- a/crates/engine/src/cmd/props.rs +++ b/crates/engine/src/cmd/props.rs @@ -621,13 +621,13 @@ pub fn apply(cmd: &str, doc: &mut Document, id: NodeId, props: &Map { - 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; From 877ac1fd5eed0f4cf7bfc97358f423bffe0e7604 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Wed, 28 Oct 2026 15:38:32 +0100 Subject: [PATCH 13/18] Reject non-page ids in page.duplicate Found while testing crates/engine/src/cmd/page.rs:96 (page.duplicate, `let src = id(p, "id").unwrap_or(s.page()?)`) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/cmd/page.rs | 2 +- crates/engine/src/page_target_tests.rs | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/cmd/page.rs b/crates/engine/src/cmd/page.rs index 08e65b0b..dab21122 100644 --- a/crates/engine/src/cmd/page.rs +++ b/crates/engine/src/cmd/page.rs @@ -93,7 +93,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/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"); +} From 71571cccad17c95d8b87ffe5c460493ff2d7b272 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Wed, 28 Oct 2026 22:34:52 +0100 Subject: [PATCH 14/18] Keep the right edge of right-aligned auto-width text as it grows Fixes #1477 Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/derived.rs | 14 ++ crates/engine/src/text_behaviour_tests.rs | 165 ++++++++++++++++++++++ 2 files changed, 179 insertions(+) diff --git a/crates/engine/src/derived.rs b/crates/engine/src/derived.rs index b6a001b8..362ef79f 100644 --- a/crates/engine/src/derived.rs +++ b/crates/engine/src/derived.rs @@ -54,7 +54,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()); @@ -139,6 +144,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/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"); +} From d3551f3c6c37350ff9509dbc46a893581a049bb4 Mon Sep 17 00:00:00 2001 From: bflatastic Date: Thu, 29 Oct 2026 09:26:09 +0700 Subject: [PATCH 15/18] #1432: accept components and instances as thumbnails; move the tests to thumbnail_tests.rs --- crates/engine/src/cmd/file.rs | 3 + crates/engine/src/cmd/file/save.rs | 159 +----------------- crates/engine/src/cmd/file/thumbnail_tests.rs | 155 +++++++++++++++++ 3 files changed, 159 insertions(+), 158 deletions(-) create mode 100644 crates/engine/src/cmd/file/thumbnail_tests.rs diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index 7c901293..576452c5 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -432,5 +432,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 e2716bc5..4b8939c7 100644 --- a/crates/engine/src/cmd/file/save.rs +++ b/crates/engine/src/cmd/file/save.rs @@ -20,7 +20,7 @@ pub fn thumbnail(doc: &Document) -> Option> { doc.ids() .find_map(|id| { let n = doc.node(id)?; - if !matches!(n.kind, NodeKind::Frame) || n.plugin_data.get("figcraft.thumbnail").map(String::as_str) != Some("true") { + 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. @@ -242,160 +242,3 @@ pub(super) fn save_as(s: &mut Session, p: &Value) -> Result { } } } - -#[cfg(test)] -mod thumbnail_tests { - 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"); - } - - #[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::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/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); +} From 08a36418a65bfa3ebad5a9d571ef6e5bf2bd2fa7 Mon Sep 17 00:00:00 2001 From: bflatastic Date: Thu, 29 Oct 2026 09:31:37 +0700 Subject: [PATCH 16/18] #1444: accept imported entries from unissued spaces; reserve only the top half (NO_SESSION_SPACE) --- crates/collab/src/entry_ids_tests.rs | 21 +++++++++++++++++++-- crates/collab/src/server.rs | 14 ++++++++++---- 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/crates/collab/src/entry_ids_tests.rs b/crates/collab/src/entry_ids_tests.rs index 976d2cb6..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}; @@ -113,8 +114,8 @@ fn an_id_in_the_top_space_does_not_block_joins() { 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 a space nobody was given yet. - for (k, space) in [c.id() + 1, c.id() + 1000, u32::MAX].into_iter().enumerate() { + // 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}") @@ -123,6 +124,22 @@ fn an_id_in_the_top_space_does_not_block_joins() { } } +#[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 c45b0ba6..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. @@ -506,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, or of one it hasn't given out yet - /// (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(); @@ -535,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) || *s >= self.next_client)) + && 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()))); } From 069449e33c3164026b8637ddd68a0f70c79f4090 Mon Sep 17 00:00:00 2001 From: bflatastic Date: Thu, 29 Oct 2026 09:18:57 +0700 Subject: [PATCH 17/18] #1425: share fraction_of for opacity, fill opacity, innerRadius and arcInner --- crates/engine/src/automation/params.rs | 7 +++++++ crates/engine/src/cmd/edit.rs | 4 ++-- crates/engine/src/cmd/edit/props.rs | 8 ++++---- crates/engine/src/panels_behaviour_tests.rs | 15 +++++++++++++++ 4 files changed, 28 insertions(+), 6 deletions(-) diff --git a/crates/engine/src/automation/params.rs b/crates/engine/src/automation/params.rs index 35a74e0e..f07e4d86 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) } 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 91855c2e..6d0deb29 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/panels_behaviour_tests.rs b/crates/engine/src/panels_behaviour_tests.rs index 202fc4f2..d97d2507 100644 --- a/crates/engine/src/panels_behaviour_tests.rs +++ b/crates/engine/src/panels_behaviour_tests.rs @@ -486,3 +486,18 @@ fn opacity_percent_strings_always_divide_by_100() { 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}"); +} From 23e16be7f5de8f2f295ed0f0d64ec32dc98b6317 Mon Sep 17 00:00:00 2001 From: bflatastic Date: Thu, 29 Oct 2026 09:36:10 +0700 Subject: [PATCH 18/18] #1398: share the member size check between JSON and images (capped_member) # Conflicts: # crates/format/src/sketch.rs --- crates/format/src/sketch.rs | 52 ++++++++++++++++++------------------- 1 file changed, 25 insertions(+), 27 deletions(-) diff --git a/crates/format/src/sketch.rs b/crates/format/src/sketch.rs index 21cb7d2b..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,10 +67,6 @@ 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 bytes an image member may unpack to, per compressed byte (images are already compressed, -/// a zero-filled one packs a thousand times), with a floor for small members. -const MAX_IMAGE_RATIO: u64 = 100; -const MIN_IMAGE_ALLOWANCE: u64 = 16 * 1024 * 1024; /// Most image bytes read from one file in all. #[cfg(not(target_pointer_width = "32"))] const MAX_IMAGE_TOTAL: u64 = 1024 * 1024 * 1024; @@ -256,37 +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(); - let member = format!("images/{name}"); - let (size, packed) = { - let f = z.by_name(&member).ok()?; - (f.size(), f.compressed_size()) - }; - let allowed = MAX_IMAGE - .min(packed.saturating_mul(MAX_IMAGE_RATIO).max(MIN_IMAGE_ALLOWANCE)) - .min(MAX_IMAGE_TOTAL.saturating_sub(self.image_total.get())); - if size > allowed { - return None; - } - let data = crate::read_member(&mut z, &member, allowed).ok()?; - self.image_total.set(self.image_total.get() + data.len() as u64); - Some(data) + 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}"))) }