From 386c6c1d3eb923459111e9df0fb75c231a5c3722 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Sat, 7 Nov 2026 13:45:31 +0100 Subject: [PATCH 01/23] Reject negative and fractional index in image.savePreset Found while testing crates/engine/src/cmd/image.rs:239 (save_preset, `index = num(p, "index").filter(..).map(|k| k as usize)`) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/cmd/image.rs | 7 +++++-- crates/engine/src/paint_tests.rs | 10 ++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/cmd/image.rs b/crates/engine/src/cmd/image.rs index 865e4d19a..37472b2d4 100644 --- a/crates/engine/src/cmd/image.rs +++ b/crates/engine/src/cmd/image.rs @@ -5,7 +5,7 @@ use figcraft_doc::{Document, ImagePreset, Node}; use serde_json::{Value, json}; use super::{CommandSpec, ok}; -use crate::params::{f64_of, get, num, req_index, req_string, req_targets, string, targets}; +use crate::params::{f64_of, get, req_index, req_string, req_targets, string, targets}; use crate::{EngineError, Result, Session, bad, spec}; pub fn specs() -> Vec { @@ -236,7 +236,10 @@ fn save_preset(s: &mut Session, p: &Value) -> Result { Some(v) => with_values(cmd, ImageFilters::NONE, v)?, None => { let stroke = strokes(cmd, p)?; - let index = num(p, "index").filter(|k| k.is_finite() && *k >= 0.0).map(|k| k as usize); + let index = match get(p, "index") { + None | Some(Value::Null) => None, + Some(_) => Some(req_index(cmd, p, "index")?), + }; let doc = s.doc()?; targets(s, p) .iter() diff --git a/crates/engine/src/paint_tests.rs b/crates/engine/src/paint_tests.rs index 1e97b05ca..00ada8d79 100644 --- a/crates/engine/src/paint_tests.rs +++ b/crates/engine/src/paint_tests.rs @@ -135,6 +135,16 @@ fn image_adjust_refuses_what_it_cannot_do() { assert_eq!(s.execute("image.adjust", &json!({"ids": [plain, r], "index": 1, "preset": "vivid"})).unwrap()["changed"], 1); } +#[test] +fn save_preset_rejects_bad_index() { + let mut s = Session::new(); + let r = image_rect(&mut s); + assert!(s.execute("image.savePreset", &json!({"ids": [r], "name": "B", "index": 0})).is_err()); + assert!(s.execute("image.savePreset", &json!({"ids": [r], "name": "B", "index": -1})).is_err(), "negative index accepted"); + assert!(s.execute("image.savePreset", &json!({"ids": [r], "name": "B", "index": 1.5})).is_err(), "fractional index accepted"); + assert!(s.execute("image.savePreset", &json!({"ids": [r], "name": "B", "index": 1})).is_ok()); +} + #[test] fn saved_image_presets() { let mut s = Session::new(); From 34c44e6063e4eb70f801bf3b8bffc6d6398faf6c Mon Sep 17 00:00:00 2001 From: Trevor Brown Date: Sat, 7 Nov 2026 22:15:22 -0500 Subject: [PATCH 02/23] Text: a font loaded with text.loadFont wins over the built-in and installed faces of the same family and style (#2092) FigCraft chose the first face added for a family and style, so the embedded Inter 4.x kept Regular, SemiBold and Bold after Inter 3.19 was loaded and only Medium refitted. A loaded font is the counterpart of a font uploaded to the organization, which Figma's help center ranks first ("Manage conflicting fonts"). The test builds its faces from any embedded or installed font (skipped when there is none) and checks a loaded face beats a built-in one, the latest load beats earlier ones, and other styles resolve as before. Co-Authored-By: Claude Sonnet 5.5 Model-Effort: Opus 5.5 drove; Opus 5.5 briefed; Sonnet 5.5 wrote at medium --- crates/text/src/fontdb.rs | 29 +++++++++++++++++++++--- crates/text/src/fontdb_tests.rs | 39 +++++++++++++++++++++++++++++++++ docs/file-format-parity.md | 2 ++ 3 files changed, 67 insertions(+), 3 deletions(-) diff --git a/crates/text/src/fontdb.rs b/crates/text/src/fontdb.rs index 3c82cd261..5159a68d1 100644 --- a/crates/text/src/fontdb.rs +++ b/crates/text/src/fontdb.rs @@ -92,6 +92,10 @@ pub struct Face { pub italic: bool, pub bytes: FontBytes, pub index: u32, + /// 0 for a built-in or installed face; else the order it was loaded in with `text.loadFont` + /// ([`FontDb::add_loaded`]). A loaded face wins over any other of its family and style, the + /// latest load over earlier ones (#2092). + pub loaded: u32, pub upem: f64, /// Ascent and descent in font units, both positive. pub ascent: f64, @@ -207,6 +211,7 @@ impl Face { family, style, index, + loaded: 0, upem, ascent, descent, @@ -620,6 +625,8 @@ pub struct FontDb { loaded_files: std::collections::BTreeSet<(std::path::PathBuf, u32)>, /// Font files added as bytes (hash and length → families), so loading one again adds nothing. loaded_bytes: std::collections::HashMap<(u64, usize), Vec>, + /// Loads so far ([`Face::loaded`]). + load_seq: u32, system_scanned: bool, /// Browser fonts text asked for and not yet taken by the browser layer. wanted: Vec, @@ -716,8 +723,20 @@ impl FontDb { face } + /// [`add`](Self::add) a face loaded explicitly (`text.loadFont`): it wins over the other faces + /// of its family and style, and over those loaded before it. + pub fn add_loaded(&mut self, mut face: Face) -> Arc { + self.load_seq += 1; + face.loaded = self.load_seq; + self.add(face) + } + /// Add font file bytes (all faces of a collection). Returns the faces added. pub fn add_bytes(&mut self, bytes: Vec) -> Vec> { + self.add_bytes_with(bytes, false) + } + + fn add_bytes_with(&mut self, bytes: Vec, loaded: bool) -> Vec> { let shared = Arc::new(bytes); let count = match FileRef::new(shared.as_slice()) { // A collection's header may name any number of faces, each a few bytes pointing at @@ -727,7 +746,9 @@ impl FontDb { Ok(FileRef::Font(_)) => 1, Err(_) => 0, }; - (0..count).filter_map(|i| Face::from_bytes(FontBytes::Shared(shared.clone()), i, None, None)).map(|f| self.add(f)).collect() + (0..count).filter_map(|i| Face::from_bytes(FontBytes::Shared(shared.clone()), i, None, None)) + .map(|f| if loaded { self.add_loaded(f) } else { self.add(f) }) + .collect() } /// The family and style of the face whose PostScript name is `ps` (ignoring case), loaded or @@ -1035,7 +1056,9 @@ impl FontDb { /// The loaded face of `family` nearest `style`. fn best(&self, family: &str, style: &str) -> Option> { let (_, idx) = self.families.get(&self.key(family))?; - let faces: Vec<&Arc> = idx.iter().filter_map(|i| self.faces.get(*i)).collect(); + let mut faces: Vec<&Arc> = idx.iter().filter_map(|i| self.faces.get(*i)).collect(); + // A loaded font comes first (the latest load first): of faces as near, the first wins. + faces.sort_by_key(|f| std::cmp::Reverse(f.loaded)); let want = norm_style(style); if let Some(f) = faces.iter().find(|f| norm_style(&f.style) == want).or_else(|| faces.iter().find(|f| has_style(f, style))) { return Some((*f).clone()); @@ -1595,7 +1618,7 @@ pub fn load_font_bytes(bytes: Vec) -> Vec { if let Some(families) = db.loaded_bytes.get(&key) { return families.clone(); } - let faces = db.add_bytes(bytes); + let faces = db.add_bytes_with(bytes, true); let mut families: Vec = faces.iter().map(|f| f.family.clone()).collect(); families.dedup(); if !families.is_empty() { diff --git a/crates/text/src/fontdb_tests.rs b/crates/text/src/fontdb_tests.rs index 38e04c790..a3dd15396 100644 --- a/crates/text/src/fontdb_tests.rs +++ b/crates/text/src/fontdb_tests.rs @@ -382,6 +382,45 @@ fn font_bytes_load_into_the_db() { } } +/// #2092: a font loaded with `text.loadFont` wins over the built-in and installed faces of its family +/// and style, the latest load over earlier ones; other styles resolve as before. +#[test] +fn a_loaded_face_wins_over_the_built_in_one_of_its_family_and_style() { + // Any embedded or installed font stands in for the faces (none without craft-fonts on CI). + let stand_in = CRAFT_FONTS.first().map(|f| f.bytes.to_vec()).or_else(|| { + let mut installed = BTreeMap::new(); + for dir in system_font_dirs() { + scan_dir(&dir, 0, &mut installed); + } + installed.values().flat_map(|(_, v)| v).find_map(|p| std::fs::read(&p.path).ok()) + }); + let Some(bytes) = stand_in.map(Arc::new) else { + eprintln!("skipping: no font to stand in"); + return; + }; + let face = |style: &str, name: &str| { + let mut f = Face::from_bytes(FontBytes::Shared(bytes.clone()), 0, Some("Brand Sans"), Some(style)).unwrap(); + f.instances.clear(); + f.postscript = Some(name.into()); + f + }; + let name = |db: &FontDb, style: &str| db.best("Brand Sans", style).and_then(|f| f.postscript.clone()); + let mut db = FontDb::default(); + db.add(face("Regular", "Built-in-Regular")); + db.add(face("Bold", "Built-in-Bold")); + assert_eq!(name(&db, "Regular").as_deref(), Some("Built-in-Regular")); + db.add_loaded(face("Regular", "Loaded-Regular")); + assert_eq!(name(&db, "Regular").as_deref(), Some("Loaded-Regular")); + // Bold wasn't loaded: the built-in one stays. Nor does the loaded Regular stand in for it. + assert_eq!(name(&db, "Bold").as_deref(), Some("Built-in-Bold")); + // The latest load wins. + db.add_loaded(face("Regular", "Loaded-Regular-2")); + assert_eq!(name(&db, "Regular").as_deref(), Some("Loaded-Regular-2")); + // A face added after a load, not loaded, doesn't displace it. + db.add(face("Regular", "Installed-Regular")); + assert_eq!(name(&db, "Regular").as_deref(), Some("Loaded-Regular-2")); +} + /// Apple's system font answers to the names designs and CSS use for it (SF Pro, SF UI Text, /// system-ui, …), unless a font of that name is installed. #[test] diff --git a/docs/file-format-parity.md b/docs/file-format-parity.md index bce90b008..0aaf249ca 100644 --- a/docs/file-format-parity.md +++ b/docs/file-format-parity.md @@ -181,6 +181,7 @@ source paint coordinates. | | Figma | FigCraft | |---|---|---| | Local fonts | yes (font helper / desktop app) | yes (system scan) | +| Conflicting fonts | an uploaded organization font comes first, then fonts installed on the device, then Figma's web fonts (help center, "Manage conflicting fonts") | a font loaded with `text.loadFont` wins over the built-in and installed faces of the same family and style, the latest load over earlier ones (#2092); installed versus built-in order is unchanged, and browser local fonts count as installed | | Bundled/shared fonts | Google Fonts in the cloud | optional `CRAFT_FONTS_DIR` build input (storytold/craft-fonts) | | Variable fonts | yes | yes (axes, named instances, per-range axis values) | | Missing-font replacement on open | yes | listed, not replaced: in the desktop app the Missing fonts window opens after a file that uses them opens and from Text ▸ Missing fonts (`text.missingFonts`; `figcraft-cli info` prints `missing font` lines), and their text draws in a fallback font. A family the font store has (Google Fonts, Noto) downloads from the window once the user allows downloading fonts (`font.download`, #754): pinned, checksum-verified files into the Craft apps' shared font folder, after which the text draws in it. A family is found however a file names it: by the legacy name of one of its weights ("Family Medium") or by the typographic name of a family installed one family per weight, and spelt with other case, spaces or hyphens (#1292). No substitute picker | @@ -189,6 +190,7 @@ source paint coordinates. | Date | Change | Summary | |---|---|---| +| 2026-11-07 | patch | Fonts, #2092: a font loaded with `text.loadFont` is the counterpart of a font uploaded to the organization, which Figma's help center ("Manage conflicting fonts") ranks first, ahead of installed fonts and the web fonts. FigCraft chose the first face added for a family and style, so the built-in Inter 4.x kept Regular, SemiBold and Bold after Inter 3.19 was loaded and only Medium refitted. A loaded face now wins over the other faces of its family and style, the latest load over earlier ones; style matching and fallbacks are unchanged, and `refitted` lists the text whose size changes. Installed versus built-in order is not changed here. Test: `fontdb::tests::a_loaded_face_wins_over_the_built_in_one_of_its_family_and_style`. Numbers unchanged. | | 2026-11-03 | patch | M6, #1909: `.jam` import: a connector end whose magnet is `AUTO` (FigJam picks the side when it draws) or `CENTER` (the end aims at the layer's middle) left the side its stale `relativePosition` named, the left one in the reporter's file, so the connectors looped behind their shape or crossed it. It now leaves the side facing the other end, as a board connector drawn here does (`Side::Auto`), for straight, elbowed and curved connectors alike; a named magnet (`TOP`, `RIGHT`, `BOTTOM`, `LEFT`) is unchanged, and an end without a magnet still takes the edge its point lies on. The Plugin API lists the magnets (`ConnectorEndpoint.magnet`: "Which side of the node it attaches to"; NONE, AUTO, TOP, LEFT, BOTTOM, RIGHT, CENTER; "Straight connectors may use only CENTER or NONE magnets"). Test: `connectors_with_an_automatic_side_leave_the_side_facing_the_target`. Numbers unchanged. | | 2026-11-04 | minor | JPG export: `quality` (1 to 100, default 90) on `file.export`, `export.add` / `export.update` and MCP `export`, kept in the export setting; an sRGB file's JPGs embed the sRGB ICC profile, as Display P3 files' JPGs already embedded theirs, since an untagged JPG is only assumed to be sRGB (#2004). | | 2026-11-04 | minor | `.fig` and `.jam` import, #1910: an elbowed connector saved with its path keeps it. FigJam stores `connectorControlPoints`, one per run between the first and the last, each a point on its run (in the connector's parent's coordinates) with the run's axis; the first run leaves the start's side and the last arrives at the end's. They become the `ConnectorPath::Runs` a board connector reshaped by hand has (`figcraft_doc::route_along`), so a connector drawn around a layer in FigJam stays around it instead of being routed through it. Points that don't make such a path (off both axes, two runs in a row the same way, a first or last run sideways to its side or going back into its object, more runs than a hand could make) leave the connector to the router as before, and only connectors routed here count in the "follow FigCraft's own routing" note. A curved connector's `connectorBezierControlPoints` are still not read. Tests: `tail_tests::an_elbow_connector_saved_with_its_path_keeps_it`, `tail_tests::an_elbow_whose_saved_path_doesnt_fit_its_sides_is_routed_here`. | From a375ac06893ed41af6916508c18bb5896e1e527b Mon Sep 17 00:00:00 2001 From: Trevor Brown Date: Sat, 7 Nov 2026 22:15:41 -0500 Subject: [PATCH 03/23] Format (#2092) Co-Authored-By: Claude Sonnet 5.5 --- crates/text/src/fontdb.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/text/src/fontdb.rs b/crates/text/src/fontdb.rs index 5159a68d1..65df051e7 100644 --- a/crates/text/src/fontdb.rs +++ b/crates/text/src/fontdb.rs @@ -746,7 +746,8 @@ impl FontDb { Ok(FileRef::Font(_)) => 1, Err(_) => 0, }; - (0..count).filter_map(|i| Face::from_bytes(FontBytes::Shared(shared.clone()), i, None, None)) + (0..count) + .filter_map(|i| Face::from_bytes(FontBytes::Shared(shared.clone()), i, None, None)) .map(|f| if loaded { self.add_loaded(f) } else { self.add(f) }) .collect() } From 0fec344f3cc1cc439808aed4781390f9ca76123c Mon Sep 17 00:00:00 2001 From: Trevor Brown Date: Sat, 7 Nov 2026 23:38:58 -0500 Subject: [PATCH 04/23] M8: copy, cut and paste in vector edit mode act on the selected points and segments (#1875) Measured in Figma: in vector edit mode Copy takes only the selected segments (a lone point, the segments touching it) and a paste outside makes a new vector in place; Cut removes them, splitting the path in the same layer and staying in edit mode; Paste while editing adds the clipboard's segments or a copied layer's outline to the vector's network in place, selecting them. FigCraft copied the whole layer, Cut left nothing to paste, and Paste while editing did nothing. 16 new engine and UI tests cover copy, cut, paste, undo and junk parameters; the whole engine suite passes bar the known font test. Co-Authored-By: Claude Sonnet 5.5 Model-Effort: Opus 5.5 drove; Opus 5.5 briefed; Sonnet 5.5 wrote at high --- crates/engine/src/canvas/vectoredit.rs | 2 +- crates/engine/src/cmd/edit.rs | 21 +- crates/engine/src/cmd/edit/clipboard.rs | 62 +++- .../engine/src/cmd/edit/vector_clipboard.rs | 274 +++++++++++++++ .../src/cmd/edit/vector_clipboard_tests.rs | 324 ++++++++++++++++++ crates/ui-egui/src/menus.rs | 10 +- crates/ui-egui/src/shortcuts.rs | 13 +- crates/ui-egui/src/vector_keys_tests.rs | 12 + docs/ui-parity.md | 5 +- 9 files changed, 701 insertions(+), 22 deletions(-) create mode 100644 crates/engine/src/cmd/edit/vector_clipboard.rs create mode 100644 crates/engine/src/cmd/edit/vector_clipboard_tests.rs diff --git a/crates/engine/src/canvas/vectoredit.rs b/crates/engine/src/canvas/vectoredit.rs index 4f265e0f5..749e246c0 100644 --- a/crates/engine/src/canvas/vectoredit.rs +++ b/crates/engine/src/canvas/vectoredit.rs @@ -638,7 +638,7 @@ fn index(cmd: &str, p: &Value, key: &str) -> Result { } /// Delete the points and segments of `parts` (`heal` keeps paths through deleted points closed). -fn delete_selection(s: &mut Session, parts: Vec, heal: bool) -> Result<()> { +pub(crate) fn delete_selection(s: &mut Session, parts: Vec, heal: bool) -> Result<()> { let doc = s.doc_mut()?; for EditPart { id, vertices: verts, segments: segs } in parts { edit(doc, id, |net| { diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index efcb0fc52..19a7d7011 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -17,6 +17,9 @@ mod clipboard; mod props; pub(crate) use props::{edit_region_fills, fill_regions}; mod select; +mod vector_clipboard; +#[cfg(test)] +mod vector_clipboard_tests; pub(crate) use clipboard::apply_copied_props; pub use clipboard::{CLIP_CLOSE, CLIP_OPEN, copy_to_clipboard, design_clipboard_buffer, paste_into}; @@ -117,12 +120,24 @@ pub fn specs() -> Vec { true, duplicate ), - spec!("edit.copy", "Copy", "{ids?}", false, copy), - spec!("edit.cut", "Cut", "{ids?}", true, cut), + spec!( + "edit.copy", + "Copy", + "{ids?} in vector edit mode with points or segments selected (and no ids): just those, the segments touching a selected point, as a new vector with the edited layer's look", + false, + copy + ), + spec!( + "edit.cut", + "Cut", + "{ids?} in vector edit mode with points or segments selected (and no ids): copies them as Copy does, then removes them as Delete does; the path splits in the same layer and edit mode stays on", + true, + cut + ), spec!( "edit.paste", "Paste", - "{parent?, x?, y? (page coordinates of the pasted top-left)} a copied main component pastes as an instance of it within its file (cut ones, and pastes into other files, keep the component)", + "{parent?, x?, y? (page coordinates of the pasted top-left)} in vector edit mode (and no parent): the copied vectors' segments and shapes' outlines join the edited vector's network in place, selected, and the layers that have no outline (text, frames, groups) are skipped (`skipped`); a copied main component pastes as an instance of it within its file (cut ones, and pastes into other files, keep the component)", true, paste ), diff --git a/crates/engine/src/cmd/edit/clipboard.rs b/crates/engine/src/cmd/edit/clipboard.rs index 150a3b577..da5089419 100644 --- a/crates/engine/src/cmd/edit/clipboard.rs +++ b/crates/engine/src/cmd/edit/clipboard.rs @@ -1,6 +1,6 @@ use super::{ - Affine, BTreeMap, BTreeSet, Document, EngineError, ImageData, Node, NodeId, NodeKind, Result, Session, Value, bad, delete, id, json, num, ok, - req_targets, roots, string, targets, z_sorted, + Affine, BTreeMap, BTreeSet, Document, EngineError, ImageData, Node, NodeId, NodeKind, Result, Session, Value, bad, delete, get, id, json, num, + ok, req_targets, roots, string, targets, vector_clipboard, z_sorted, }; /// The copied form of the selection. @@ -12,6 +12,18 @@ pub fn copy_to_clipboard(s: &mut Session, cmd: &str, ids: &[NodeId]) -> Result<( return Err(EngineError::Disabled(cmd.into(), "pages can't be copied; use page.duplicate".into())); } let (nodes, images, bounds) = copied(&ds.doc, &layers); + store_clipboard(s, cmd, nodes, images, bounds) +} + +/// Make `nodes` (the copied layers, each root's transform in page space) the clipboard's layers. +pub(super) fn store_clipboard( + s: &mut Session, + cmd: &str, + nodes: Vec<(NodeId, Vec)>, + images: BTreeMap, + bounds: Option, +) -> Result<()> { + let ds = s.ds()?; let (source, source_path, uid) = (ds.doc.clone(), ds.path.clone(), ds.uid); s.clipboard.source = Some(source); s.clipboard.source_path = source_path; @@ -43,21 +55,26 @@ fn copied(doc: &Document, ids: &[NodeId]) -> Copied { if let Some(r) = sub.first_mut() { r.transform = world; } - for n in &sub { - for p in n.fills.iter().chain(n.strokes.iter()) { - if let figcraft_color::PaintKind::Image(img) = &p.kind - && let Some(d) = doc.images.get(&img.image) - { - images.insert(img.image.clone(), d.clone()); - } - } - } + collect_images(doc, &sub, &mut images); nodes.push((*i, sub)); } let bounds = doc.bounds_of(&ids); (nodes, images, bounds) } +/// The images the paints of `nodes` use, taken from `doc` into `images`. +pub(super) fn collect_images(doc: &Document, nodes: &[Node], images: &mut BTreeMap) { + for n in nodes { + for p in n.fills.iter().chain(n.strokes.iter()) { + if let figcraft_color::PaintKind::Image(img) = &p.kind + && let Some(d) = doc.images.get(&img.image) + { + images.insert(img.image.clone(), d.clone()); + } + } + } +} + /// The markers around the copied layers in the clipboard's HTML, spelt as that format spells them. pub const CLIP_OPEN: &str = "(figma)"; @@ -167,13 +184,32 @@ fn layers_missing_images(doc: &figcraft_doc::Document, ids: &[NodeId]) -> Vec bool { + get(p, "ids").is_some() || get(p, "id").is_some() +} + pub(super) fn copy(s: &mut Session, p: &Value) -> Result { + // In vector edit mode with points or segments selected, Copy takes just those (#1875). + if !names_layers(p) + && let Some(sel) = vector_clipboard::edit_selection(s) + && !sel.parts.is_empty() + { + return vector_clipboard::copy_selection(s, "edit.copy", sel); + } let ids = req_targets("edit.copy", s, p)?; copy_to_clipboard(s, "edit.copy", &ids)?; Ok(json!({"copied": s.clipboard.nodes.len()})) } pub(super) fn cut(s: &mut Session, p: &Value) -> Result { + // In vector edit mode with points or segments selected, Cut takes just those and stays in edit mode (#1875). + if !names_layers(p) + && let Some(sel) = vector_clipboard::edit_selection(s) + && sel.any + { + return vector_clipboard::cut_selection(s, sel); + } let ids = req_targets("edit.cut", s, p)?; copy_to_clipboard(s, "edit.cut", &ids)?; let doc = s.doc()?; @@ -238,6 +274,10 @@ pub fn paste_into(s: &mut Session, parent: NodeId, at: Option<(f64, f64)>) -> Re } pub(super) fn paste(s: &mut Session, p: &Value) -> Result { + // While editing a vector, the copied layers join its network (#1875); `parent` asks for layers. + if id(p, "parent").is_none() && s.ds()?.vector_edit.is_some() { + return vector_clipboard::paste_into_vector(s, p); + } let page = s.page()?; // Figma pastes into the selected frame (if one is selected), at the same position when it fits, // else in the frame's centre; otherwise in place on the page (or the viewport centre). diff --git a/crates/engine/src/cmd/edit/vector_clipboard.rs b/crates/engine/src/cmd/edit/vector_clipboard.rs new file mode 100644 index 000000000..d9efea269 --- /dev/null +++ b/crates/engine/src/cmd/edit/vector_clipboard.rs @@ -0,0 +1,274 @@ +//! Copy, cut and paste in vector edit mode (#1875). +//! +//! With points or segments selected, Copy and Cut take just those: a temporary vector holding the +//! selected segments (and the segments touching a selected point) goes on the ordinary layer +//! clipboard, kept at its place on the page, so a paste outside edit mode is a plain layer paste +//! in place. Cut then removes the selection the way Delete does and stays in edit mode. Paste +//! while editing adds the clipboard's layers to the edited vector's network, in place: a vector +//! with its segments, a shape or boolean with its outline. + +use figcraft_doc::{NodeKind, Region, VectorNetwork}; + +use super::clipboard::{collect_images, store_clipboard}; +use super::{Affine, BTreeMap, BTreeSet, Document, EngineError, Node, NodeId, Result, Session, Value, json, num}; +use crate::canvas::vectoredit::{self, EditPart}; + +/// The most segments a vector network takes through a paste. +const MAX_SEGMENTS: usize = 500_000; + +/// What the edited layers have selected. +pub(super) struct EditSelection { + /// Whether any edited layer has a point or a segment selected. + pub any: bool, + /// Each edited layer with something to copy, and the sub-network that is. + pub parts: Vec<(NodeId, VectorNetwork)>, +} + +/// The selection of the vector being edited (and the other layers edited with it), or `None` when +/// not in vector edit mode. +pub(super) fn edit_selection(s: &Session) -> Option { + let ds = s.ds().ok()?; + let ve = ds.vector_edit.as_ref()?; + let mut sel = EditSelection { any: false, parts: Vec::new() }; + for part in ve.parts() { + if part.vertices.is_empty() && part.segments.is_empty() { + continue; + } + sel.any = true; + let Some(net) = vectoredit::network(&ds.doc, part.id) else { continue }; + let active = part.id == ve.id; + // Points selected only through a selected region don't pull in the segments touching + // them: the region's own segments are the selection. + let vertices: Vec = part.vertices.iter().copied().filter(|v| !active || !ve.region_vertices.contains(v)).collect(); + let mut segments = part.segments.clone(); + if active { + segments.extend(ve.regions.iter().filter_map(|r| net.regions.get(*r)).flat_map(|r| r.loops.iter().flatten().copied())); + } + if let Some(sub) = sub_network(net, &vertices, &segments) { + sel.parts.push((part.id, sub)); + } + } + Some(sel) +} + +/// The part of `net` that is the segments `segments` and those touching a point of `vertices`, +/// with the points they join; `None` when there are none (a lone point with no segment draws +/// nothing). The regions whose every segment is kept stay regions. +fn sub_network(net: &VectorNetwork, vertices: &[usize], segments: &[usize]) -> Option { + let picked: BTreeSet = vertices.iter().copied().collect(); + let mut keep: BTreeSet = segments.iter().copied().filter(|i| *i < net.segments.len()).collect(); + for (i, sg) in net.segments.iter().enumerate() { + if picked.contains(&sg.a) || picked.contains(&sg.b) { + keep.insert(i); + } + } + let used: BTreeSet = keep.iter().filter_map(|i| net.segments.get(*i)).flat_map(|sg| [sg.a, sg.b]).collect(); + let vmap: BTreeMap = used.iter().enumerate().map(|(new, old)| (*old, new)).collect(); + let smap: BTreeMap = keep.iter().enumerate().map(|(new, old)| (*old, new)).collect(); + let mut out = VectorNetwork { + vertices: used.iter().filter_map(|v| net.vertices.get(*v).cloned()).collect(), + segments: keep + .iter() + .filter_map(|i| net.segments.get(*i)) + .filter_map(|sg| { + let mut sg = *sg; + sg.a = *vmap.get(&sg.a)?; + sg.b = *vmap.get(&sg.b)?; + Some(sg) + }) + .collect(), + regions: Vec::new(), + }; + if out.segments.is_empty() || out.segments.len() != keep.len() || out.vertices.len() != used.len() { + return None; + } + for r in &net.regions { + let loops: Option>> = r.loops.iter().map(|l| l.iter().map(|i| smap.get(i).copied()).collect::>>()).collect(); + if let Some(loops) = loops.filter(|l| !l.is_empty()) { + out.regions.push(Region { loops, ..r.clone() }); + } + } + Some(out) +} + +/// The clipboard's layer for edited vector `id` holding `sub`: it keeps the layer's look (fills, +/// strokes, effects, opacity, blend) and its place on the page. A variable width profile runs +/// along the whole path, so it stays only when every segment is copied. +fn clip_node(doc: &Document, id: NodeId, mut sub: VectorNetwork) -> Option { + let mut n = doc.node(id)?.clone(); + let whole = vectoredit::network(doc, id).is_some_and(|all| all.segments.len() == sub.segments.len()); + let b = sub.bounds().filter(|b| b.x0.is_finite() && b.y0.is_finite() && b.width().is_finite() && b.height().is_finite())?; + sub.translate(-b.x0, -b.y0); + n.transform = doc.world_transform(id) * Affine::translate((b.x0, b.y0)); + n.width = b.width(); + n.height = b.height(); + n.kind = NodeKind::Vector { network: sub }; + n.parent = None; + n.children.clear(); + n.name = "Vector".into(); + if !whole { + n.stroke.width_profile = None; + } + Some(n) +} + +/// Put the selection's sub-networks on the layer clipboard. Returns how many segments went. +fn copy_parts(s: &mut Session, cmd: &str, sel: &EditSelection) -> Result { + let doc = s.doc()?; + let mut nodes = Vec::new(); + let mut images = BTreeMap::new(); + let mut bounds: Option = None; + let mut segments = 0; + for (id, sub) in &sel.parts { + segments += sub.segments.len(); + let Some(n) = clip_node(doc, *id, sub.clone()) else { continue }; + let r = figcraft_geom::transform_rect_bbox(n.transform, n.local_rect()); + bounds = Some(bounds.map_or(r, |b| b.union(r))); + collect_images(doc, std::slice::from_ref(&n), &mut images); + nodes.push((*id, vec![n])); + } + if nodes.is_empty() { + return Err(EngineError::Disabled(cmd.into(), "the selection has nothing to copy".into())); + } + store_clipboard(s, cmd, nodes, images, bounds)?; + Ok(segments) +} + +/// Copy the selected segments (see [`edit_selection`]). +pub(super) fn copy_selection(s: &mut Session, cmd: &str, sel: EditSelection) -> Result { + let segments = copy_parts(s, cmd, &sel)?; + Ok(json!({"copied": s.clipboard.nodes.len(), "segments": segments})) +} + +/// Cut: copy the selected segments, then remove the selection as Delete does (the path splits in +/// the same layer and edit mode stays on). A selection that has no segment to copy (lone points +/// with nothing attached) is removed and the clipboard is left as it was. +pub(super) fn cut_selection(s: &mut Session, sel: EditSelection) -> Result { + let segments = if sel.parts.is_empty() { 0 } else { copy_parts(s, "edit.cut", &sel)? }; + let parts: Vec = vectoredit::editing(s)?.parts(); + vectoredit::delete_selection(s, parts, false)?; + Ok(json!({"cut": sel.parts.len(), "segments": segments})) +} + +/// The network of the outline of copied layer `n` in its own coordinates: a vector's segments, a +/// shape's or boolean's outline. `None` for the layers with no outline to add (text, images, +/// frames, groups, instances, …). `doc` is the file the layer was copied from. +fn outline_network(doc: &Document, n: &Node) -> Option { + let net = match &n.kind { + NodeKind::Vector { network } => network.clone(), + k if k.is_shape() || matches!(k, NodeKind::BooleanOp { .. }) => { + let (path, rule) = figcraft_render::node_outline(doc, n)?; + let mut net = VectorNetwork::from_path(&path); + if matches!(n.kind, NodeKind::Line) { + net.regions.clear(); + } + if rule == figcraft_render::FillRule::EvenOdd { + for r in &mut net.regions { + r.winding = figcraft_doc::WindingRule::EvenOdd; + } + } + net + } + _ => return None, + }; + (!net.segments.is_empty()).then_some(net) +} + +/// `net` moved by `m` (points, handles, the regions' paint frames and the points' radii). +fn placed(mut net: VectorNetwork, m: Affine) -> Option { + let c = m.as_coeffs(); + if !c.iter().all(|x| x.is_finite()) { + return None; + } + let linear = Affine::new([c[0], c[1], c[2], c[3], 0.0, 0.0]); + let scale = m.determinant().abs().sqrt(); + for v in &mut net.vertices { + let p = m * v.point(); + (v.x, v.y) = (p.x, p.y); + v.radius *= scale; + } + for sg in &mut net.segments { + let (ta, tb) = (linear * figcraft_geom::Point::new(sg.ta.0, sg.ta.1), linear * figcraft_geom::Point::new(sg.tb.0, sg.tb.1)); + sg.ta = (ta.x, ta.y); + sg.tb = (tb.x, tb.y); + } + for r in &mut net.regions { + if let Some(frame) = r.paint_space.as_mut() { + frame.transform = m * frame.transform; + } + } + let finite = net.vertices.iter().all(|v| v.x.is_finite() && v.y.is_finite()) + && net.segments.iter().all(|sg| [sg.ta.0, sg.ta.1, sg.tb.0, sg.tb.1].iter().all(|x| x.is_finite())); + finite.then_some(net) +} + +/// Paste while editing a vector: the clipboard's layers join its network in place (with `x`, `y`: +/// the copied top-left moved there), their points selected. Layers with no outline are skipped +/// (named in `skipped`); when none has one, nothing is pasted. +pub(super) fn paste_into_vector(s: &mut Session, p: &Value) -> Result { + const CMD: &str = "edit.paste"; + let clip = s.clipboard.clone(); + if clip.nodes.is_empty() { + return Err(EngineError::Disabled(CMD.into(), "the clipboard is empty".into())); + } + let target = vectoredit::editing(s)?.id; + let shift = match (num(p, "x"), num(p, "y"), clip.bounds) { + (Some(x), Some(y), Some(b)) => Affine::translate((x - b.x0, y - b.y0)), + _ => Affine::IDENTITY, + }; + let live = s.doc()?; + let source: &Document = clip.source.as_deref().unwrap_or(live); + let to_local = live.world_transform(target).inverse() * shift; + let mut pieces = Vec::new(); + let mut skipped: Vec = Vec::new(); + for (_, nodes) in &clip.nodes { + let Some(root) = nodes.first() else { continue }; + match outline_network(source, root).and_then(|net| placed(net, to_local * root.transform)) { + Some(net) => pieces.push(net), + None => skipped.push(root.name.clone()), + } + } + if pieces.is_empty() { + return Err(EngineError::Disabled( + CMD.into(), + "none of the copied layers has an outline to add to the vector (text, frames and groups have none)".into(), + )); + } + let have = vectoredit::network(live, target).map_or(0, |n| n.segments.len()); + if have.saturating_add(pieces.iter().map(|n| n.segments.len()).sum::()) > MAX_SEGMENTS { + return Err(EngineError::Disabled(CMD.into(), format!("a vector takes at most {MAX_SEGMENTS} segments"))); + } + let mut pasted: Vec = Vec::new(); + vectoredit::edit(s.doc_mut()?, target, |net| { + for piece in pieces { + let (vo, so) = (net.vertices.len(), net.segments.len()); + pasted.extend(vo..vo + piece.vertices.len()); + net.vertices.extend(piece.vertices); + net.segments.extend(piece.segments.into_iter().map(|mut sg| { + sg.a += vo; + sg.b += vo; + sg + })); + net.regions.extend(piece.regions.into_iter().map(|mut r| { + for i in r.loops.iter_mut().flatten() { + *i += so; + } + r + })); + } + })?; + let selected = pasted.clone(); + vectoredit::set_state(s, |ve| { + ve.clear_selection(); + ve.clear_regions(); + ve.vertices = selected; + ve.anchor = None; + ve.pen_out = None; + ve.width_point = None; + })?; + let mut out = json!({"ids": [], "edited": target, "vertices": pasted.len()}); + if !skipped.is_empty() { + out["skipped"] = json!(skipped); + } + Ok(out) +} diff --git a/crates/engine/src/cmd/edit/vector_clipboard_tests.rs b/crates/engine/src/cmd/edit/vector_clipboard_tests.rs new file mode 100644 index 000000000..012a110fe --- /dev/null +++ b/crates/engine/src/cmd/edit/vector_clipboard_tests.rs @@ -0,0 +1,324 @@ +//! Copy, cut and paste in vector edit mode (#1875), measured in Figma: Copy takes the selected +//! segments (a lone point: the segments touching it) and a paste outside is a new vector in place; +//! Cut removes them, splitting the path in the same layer; Paste while editing adds the clipboard's +//! segments or a copied layer's outline to the vector's network in place, selected. + +use serde_json::{Value, json}; + +use crate::{EngineError, Session, params}; +use figcraft_doc::{NodeId, NodeKind, VectorNetwork}; + +fn ex(s: &mut Session, c: &str, p: Value) -> Value { + s.execute(c, &p).unwrap_or_else(|e| panic!("{c}: {e}")) +} + +fn id_of(v: &Value) -> NodeId { + params::id_of(&v["id"]).unwrap() +} + +/// Open path A-B-C-D (three segments) with a look of its own, in edit mode. +fn path(s: &mut Session) -> NodeId { + let r = ex(s, "vector.create", json!({"points": [[100, 100], [200, 150], [300, 100], [400, 200]], "closed": false})); + let id = id_of(&r); + ex(s, "node.set", json!({"id": id, "props": {"strokeWeight": 7, "opacity": 0.5}})); + ex(s, "vector.edit.begin", json!({"id": id})); + id +} + +fn net(s: &Session, id: NodeId) -> VectorNetwork { + crate::vectoredit::network(s.doc().unwrap(), id).unwrap().clone() +} + +fn layers(s: &Session) -> Vec { + s.doc().unwrap().node(s.page().unwrap()).unwrap().children.clone() +} + +/// The network's points on the page, rounded, in vertex order. +fn page_points(s: &Session, id: NodeId) -> Vec<(i64, i64)> { + let w = s.doc().unwrap().world_transform(id); + net(s, id).vertices.iter().map(|v| w * v.point()).map(|p| (p.x.round() as i64, p.y.round() as i64)).collect() +} + +/// The number of separate paths a network is made of. +fn subpaths(n: &VectorNetwork) -> usize { + let mut group: Vec = (0..n.vertices.len()).collect(); + fn root(g: &[usize], mut i: usize) -> usize { + while g[i] != i { + i = g[i]; + } + i + } + for sg in &n.segments { + let (a, b) = (root(&group, sg.a), root(&group, sg.b)); + group[a] = b; + } + (0..n.vertices.len()).filter(|i| root(&group, *i) == *i && n.segments.iter().any(|sg| root(&group, sg.a) == *i)).count() +} + +fn select(s: &mut Session, vertices: &[usize], segments: &[usize]) { + ex(s, "vector.edit.select", json!({"vertices": vertices, "segments": segments})); +} + +fn leave(s: &mut Session) { + ex(s, "vector.edit.end", json!({})); + assert!(s.ds().unwrap().vector_edit.is_none()); +} + +fn world_rect(s: &Session, id: NodeId) -> (f64, f64, f64, f64) { + let b = s.doc().unwrap().world_bounds(id).unwrap(); + (b.x0, b.y0, b.width(), b.height()) +} + +#[test] +fn a_copied_segment_pastes_outside_as_a_new_vector_in_place() { + let mut s = Session::new(); + let id = path(&mut s); + select(&mut s, &[], &[1]); + let r = ex(&mut s, "edit.copy", json!({})); + assert_eq!(r["segments"], 1); + assert_eq!(net(&s, id).segments.len(), 3, "Copy changes nothing"); + leave(&mut s); + ex(&mut s, "edit.paste", json!({})); + let all = layers(&s); + assert_eq!(all.len(), 2); + let new = all[1]; + let n = net(&s, new); + assert_eq!((n.vertices.len(), n.segments.len()), (2, 1)); + assert_eq!(page_points(&s, new), vec![(200, 150), (300, 100)], "at the segment's own page position"); + assert_eq!(world_rect(&s, new), (200.0, 100.0, 100.0, 50.0)); + // The layer's look comes along (the width profile aside). + let node = s.doc().unwrap().node(new).unwrap(); + assert_eq!((node.stroke.weight, node.opacity), (7.0, 0.5)); + assert_eq!(s.selection(), vec![new]); +} + +#[test] +fn a_copied_point_takes_the_segments_touching_it() { + let mut s = Session::new(); + path(&mut s); + select(&mut s, &[1], &[]); + ex(&mut s, "edit.copy", json!({})); + leave(&mut s); + ex(&mut s, "edit.paste", json!({})); + let new = *layers(&s).last().unwrap(); + let n = net(&s, new); + assert_eq!((n.vertices.len(), n.segments.len()), (3, 2), "A-B and B-C"); + assert_eq!(page_points(&s, new), vec![(100, 100), (200, 150), (300, 100)]); +} + +#[test] +fn nothing_selected_still_copies_the_whole_layer() { + let mut s = Session::new(); + path(&mut s); + ex(&mut s, "edit.copy", json!({})); + leave(&mut s); + ex(&mut s, "edit.paste", json!({})); + let new = *layers(&s).last().unwrap(); + assert_eq!(net(&s, new).segments.len(), 3); +} + +#[test] +fn cut_removes_the_segment_in_the_same_layer_and_stays_in_edit_mode() { + let mut s = Session::new(); + let id = path(&mut s); + select(&mut s, &[], &[1]); + ex(&mut s, "edit.cut", json!({})); + assert_eq!(layers(&s), vec![id], "no new layer"); + let n = net(&s, id); + assert_eq!(n.segments.len(), 2); + assert_eq!(subpaths(&n), 2, "the path split in two"); + let ve = s.ds().unwrap().vector_edit.clone().expect("edit mode stays on"); + assert!(ve.segments.is_empty() && ve.vertices.is_empty()); + leave(&mut s); + ex(&mut s, "edit.paste", json!({})); + let all = layers(&s); + assert_eq!(all.len(), 2); + let new = all[1]; + assert_eq!(net(&s, new).segments.len(), 1); + assert_eq!(page_points(&s, new), vec![(200, 150), (300, 100)]); +} + +#[test] +fn cut_is_one_undo_step() { + let mut s = Session::new(); + let id = path(&mut s); + select(&mut s, &[], &[1]); + ex(&mut s, "edit.cut", json!({})); + assert_eq!(net(&s, id).segments.len(), 2); + s.undo().unwrap(); + assert_eq!(net(&s, id).segments.len(), 3); + assert_eq!(page_points(&s, id), vec![(100, 100), (200, 150), (300, 100), (400, 200)]); +} + +#[test] +fn a_cut_point_takes_its_segments_with_it() { + let mut s = Session::new(); + let id = path(&mut s); + select(&mut s, &[1], &[]); + ex(&mut s, "edit.cut", json!({})); + assert_eq!(net(&s, id).segments.len(), 1, "only C-D is left"); + leave(&mut s); + ex(&mut s, "edit.paste", json!({})); + let new = *layers(&s).last().unwrap(); + assert_eq!(net(&s, new).segments.len(), 2, "A-B and B-C"); +} + +#[test] +fn pasting_segments_while_editing_adds_them_to_the_same_network_in_place() { + let mut s = Session::new(); + let id = path(&mut s); + select(&mut s, &[], &[1]); + ex(&mut s, "edit.copy", json!({})); + let r = ex(&mut s, "edit.paste", json!({})); + assert_eq!(r["vertices"], 2); + assert_eq!(layers(&s), vec![id], "no new layer"); + let n = net(&s, id); + assert_eq!((n.vertices.len(), n.segments.len()), (6, 4)); + assert_eq!(page_points(&s, id)[4..], [(200, 150), (300, 100)], "on top of the original"); + let ve = s.ds().unwrap().vector_edit.clone().unwrap(); + assert_eq!(ve.vertices, vec![4, 5], "the pasted points are selected"); + assert!(ve.segments.is_empty()); + // One undo reverts it. + s.undo().unwrap(); + assert_eq!(net(&s, id).segments.len(), 3); + assert_eq!(net(&s, id).vertices.len(), 4); +} + +#[test] +fn pasting_at_a_point_moves_the_copied_top_left_there() { + let mut s = Session::new(); + let id = path(&mut s); + select(&mut s, &[], &[1]); + ex(&mut s, "edit.copy", json!({})); + ex(&mut s, "edit.paste", json!({"x": 0, "y": 0})); + assert_eq!(page_points(&s, id)[4..], [(0, 50), (100, 0)]); +} + +#[test] +fn pasting_a_rectangle_while_editing_adds_its_outline_in_the_vectors_coordinates() { + let mut s = Session::new(); + let rect = id_of(&ex(&mut s, "shape.rectangle", json!({"x": 500, "y": 300, "width": 60, "height": 40}))); + ex(&mut s, "select.set", json!({"ids": [rect]})); + ex(&mut s, "edit.copy", json!({})); + let id = path(&mut s); + let before = world_rect(&s, id); + ex(&mut s, "edit.paste", json!({})); + assert_eq!(layers(&s), vec![rect, id], "no new layer"); + let n = net(&s, id); + assert_eq!((n.vertices.len(), n.segments.len()), (8, 7), "four corners, four sides, joined to the path"); + let mut corners: Vec<(i64, i64)> = page_points(&s, id)[4..].to_vec(); + corners.sort_unstable(); + assert_eq!(corners, vec![(500, 300), (500, 340), (560, 300), (560, 340)]); + let ve = s.ds().unwrap().vector_edit.clone().unwrap(); + assert_eq!(ve.vertices, vec![4, 5, 6, 7]); + let after = world_rect(&s, id); + assert!(after.2 > before.2 && after.3 > before.3, "the vector's bounds grow: {before:?} to {after:?}"); + assert_eq!((after.0, after.1), (before.0, before.1)); + assert_eq!(after.0 + after.2, 560.0); + s.undo().unwrap(); + assert_eq!(net(&s, id).vertices.len(), 4); + assert_eq!(world_rect(&s, id), before); +} + +#[test] +fn a_rotated_vector_takes_pasted_points_in_its_own_coordinates() { + let mut s = Session::new(); + let id = path(&mut s); + ex(&mut s, "node.set", json!({"id": id, "props": {"rotation": 90}})); + select(&mut s, &[], &[1]); + ex(&mut s, "edit.copy", json!({})); + let before = page_points(&s, id); + assert_ne!(before[0], (100, 100), "the vector is rotated"); + ex(&mut s, "edit.paste", json!({})); + let after = page_points(&s, id); + assert_eq!(after[4..], before[1..3]); +} + +#[test] +fn layers_with_no_outline_are_skipped_and_named() { + let mut s = Session::new(); + let rect = id_of(&ex(&mut s, "shape.rectangle", json!({"x": 500, "y": 300, "width": 60, "height": 40}))); + let text = id_of(&ex(&mut s, "text.create", json!({"text": "hello", "x": 500, "y": 400}))); + let name = s.doc().unwrap().node(text).unwrap().name.clone(); + // Only text: nothing to add, and nothing changes. + ex(&mut s, "select.set", json!({"ids": [text]})); + ex(&mut s, "edit.copy", json!({})); + let id = path(&mut s); + let err = s.execute("edit.paste", &json!({})).unwrap_err(); + assert!(matches!(err, EngineError::Disabled(..)), "{err}"); + assert_eq!(net(&s, id).vertices.len(), 4); + // The rectangle joins, the text is named as skipped. + leave(&mut s); + ex(&mut s, "edit.copy", json!({"ids": [rect, text]})); + ex(&mut s, "vector.edit.begin", json!({"id": id})); + let r = ex(&mut s, "edit.paste", json!({})); + assert_eq!(r["skipped"], json!([name])); + assert_eq!(net(&s, id).vertices.len(), 8); +} + +#[test] +fn a_parent_still_pastes_layers_while_editing() { + let mut s = Session::new(); + let id = path(&mut s); + select(&mut s, &[], &[1]); + ex(&mut s, "edit.copy", json!({})); + let page = s.page().unwrap(); + ex(&mut s, "edit.paste", json!({"parent": page})); + assert_eq!(layers(&s).len(), 2); + assert_eq!(net(&s, id).segments.len(), 3); +} + +#[test] +fn an_empty_clipboard_refuses_to_paste_while_editing() { + let mut s = Session::new(); + path(&mut s); + assert!(matches!(s.execute("edit.paste", &json!({})), Err(EngineError::Disabled(..)))); +} + +#[test] +fn a_selected_face_copies_its_own_outline_with_the_fill() { + let mut s = Session::new(); + let r = ex(&mut s, "shape.rectangle", json!({"x": 100, "y": 100, "width": 80, "height": 60})); + let id = id_of(&r); + ex(&mut s, "vector.edit.begin", json!({"id": id})); + select(&mut s, &[], &[0, 1, 2, 3]); + ex(&mut s, "edit.copy", json!({})); + leave(&mut s); + ex(&mut s, "edit.paste", json!({})); + let new = *layers(&s).last().unwrap(); + let n = net(&s, new); + assert_eq!((n.segments.len(), n.regions.len()), (4, 1), "the closed shape stays filled"); + assert!(matches!(s.doc().unwrap().node(new).unwrap().kind, NodeKind::Vector { .. })); +} + +#[test] +fn copy_cut_and_paste_survive_junk_in_vector_edit_mode() { + let junk = [ + json!({}), + json!(null), + json!([1, 2]), + json!({"ids": ["x", -1, 99999]}), + json!({"id": "nope", "x": "NaN", "y": 1e308}), + json!({"parent": "nope", "x": -1e308, "y": "x"}), + json!({"x": 1e308, "y": 1e308}), + ]; + for cmd in ["edit.copy", "edit.cut", "edit.paste", "edit.pasteOver", "edit.pasteReplace"] { + for j in &junk { + let mut s = Session::new(); + path(&mut s); + select(&mut s, &[1], &[0]); + ex(&mut s, "text.create", json!({"text": "t"})); + if let Err(EngineError::Internal { cmd, msg }) = s.execute(cmd, j) { + panic!("{cmd} panicked on {j}: {msg}"); + } + // And after something was copied from the edit selection. + let mut s = Session::new(); + path(&mut s); + select(&mut s, &[2], &[]); + let _ = s.execute("edit.copy", &json!({})); + if let Err(EngineError::Internal { cmd, msg }) = s.execute(cmd, j) { + panic!("{cmd} panicked on {j}: {msg}"); + } + } + } +} diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index 98fd881d1..8ef67ef66 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -197,7 +197,15 @@ pub fn invoke(app: &mut FigApp, id: &str) { let ids = app.session.selection(); // For other apps: text layers' text, other layers as SVG; taken before a cut removes them. let doc = app.session.doc().ok(); - let mirror = doc.and_then(|d| text_layers_text(d, &ids).or_else(|| figcraft_svg_export(d, &ids))); + // Points or segments selected in vector edit mode copy just those: the whole vector's + // SVG would misdescribe them. + let partial = app + .session + .ds() + .ok() + .and_then(|d| d.vector_edit.as_ref()) + .is_some_and(|ve| ve.parts().iter().any(|p| !p.vertices.is_empty() || !p.segments.is_empty())); + let mirror = doc.filter(|_| !partial).and_then(|d| text_layers_text(d, &ids).or_else(|| figcraft_svg_export(d, &ids))); app.run(id, json!({})); // Mirror to the system clipboard (text or SVG, else a marker) and remember it, so a // later paste knows whether the clipboard still holds our layers. Control-channel diff --git a/crates/ui-egui/src/shortcuts.rs b/crates/ui-egui/src/shortcuts.rs index 26bdb60d2..48b61c39c 100644 --- a/crates/ui-egui/src/shortcuts.rs +++ b/crates/ui-egui/src/shortcuts.rs @@ -178,7 +178,7 @@ pub(crate) fn handle_input( let props = ctx.input(|i| i.modifiers.alt); crate::menus::invoke(app, if props { "edit.copyProperties" } else { "edit.copy" }); } - Event::Cut => cut(app), + Event::Cut => cut(app, ctx.input(|i| i.modifiers)), Event::Paste(text) if !paste.drop_text => crate::menus::paste(app, Some(text), None), Event::Key { key, physical_key, pressed: true, modifiers, repeat } if key_event(app, *key, *physical_key, *modifiers, *repeat, now, ctx) => @@ -192,13 +192,18 @@ pub(crate) fn handle_input( /// Cut: usually the layers. Windows sends Shift+Delete (remove the points and heal the path) /// as a cut command and never as a key, so in vector edit mode the selection is points: they are -/// removed, a heal joining the path through them. -fn cut(app: &mut FigApp) { +/// removed, a heal joining the path through them. Ctrl/Cmd+X is the real Cut: with points or +/// segments selected the engine copies them and removes them, staying in edit mode (#1875). +fn cut(app: &mut FigApp, modifiers: egui::Modifiers) { let Some(ve) = app.session.ds().ok().and_then(|d| d.vector_edit.clone()) else { crate::menus::invoke(app, "edit.cut"); return; }; - if ve.parts().iter().any(|p| !p.vertices.is_empty()) { + if modifiers.command { + if ve.parts().iter().any(|p| !p.vertices.is_empty() || !p.segments.is_empty()) { + crate::menus::invoke(app, "edit.cut"); + } + } else if ve.parts().iter().any(|p| !p.vertices.is_empty()) { app.run("vector.deleteHeal", json!({})); } else if ve.parts().iter().any(|p| !p.segments.is_empty()) { app.run("vector.edit.delete", json!({})); diff --git a/crates/ui-egui/src/vector_keys_tests.rs b/crates/ui-egui/src/vector_keys_tests.rs index dce51717d..9b3d0cb04 100644 --- a/crates/ui-egui/src/vector_keys_tests.rs +++ b/crates/ui-egui/src/vector_keys_tests.rs @@ -49,6 +49,18 @@ fn a_cut_command_in_edit_mode_takes_the_selected_points_not_the_layer() { assert_eq!(net(&u, path).vertices.len(), 2, "it takes the selected point"); } +#[test] +fn ctrl_x_on_selected_segments_puts_them_on_the_clipboard_and_keeps_the_layer() { + let mut u = Ui::new(Session::new()); + let path = abc(&mut u); + u.app.session.execute("vector.edit.select", &json!({"segments": [0]})).unwrap(); + u.frame_with(vec![Event::Cut], Modifiers::COMMAND); + assert!(u.app.session.doc().unwrap().node(path).is_some(), "Ctrl+X keeps the layer"); + assert_eq!(net(&u, path).segments.len(), 1, "the segment is gone"); + assert_eq!(u.app.session.clipboard.nodes.len(), 1, "and on the clipboard"); + assert!(u.app.session.ds().unwrap().vector_edit.is_some(), "edit mode stays on"); +} + #[test] fn a_cut_command_in_edit_mode_with_nothing_selected_keeps_the_layer() { let mut u = Ui::new(Session::new()); diff --git a/docs/ui-parity.md b/docs/ui-parity.md index aa0ef25bd..98816af28 100644 --- a/docs/ui-parity.md +++ b/docs/ui-parity.md @@ -1,6 +1,6 @@ # UI and interaction parity -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-11-04 · **Change:** minor (the left rail's and tab strip's icons are larger, 20 pt (#553); pick select: Tab while the keyboard selection box shows (or the chord again) puts a pink outline on a layer and the keys walk the layer tree from it, as Accessibility at Figma says (#1836); scrubbing a field faster or slower by the pointer's height, with a notice of the speed, as the help center's Scrub fields says (#1837); component rows in the Layers panel at AA contrast in the theme's component tint, with the rows inside a selected component in a tone of its row (#1903); Cmd/Ctrl-click on a padding field sets all four sides with the value typed, a documented auto layout shortcut; a fill per region of a vector: with regions selected in edit mode, the Fill section and the fill commands edit those regions' own fills, as Edit vector layers says (#1875); the Fill section of a text whose characters were coloured differently while editing shows mixed content, characters that all share fills the layer doesn't have show those, and a layer-level fill edit reaches every character (#1856); a number field scrubs when dragged with Alt held, as the help center's scrub section says, not only by its label; reuse visible layer selection roots (M8); ; with a shape or frame tool chosen and nothing selected, the arrows show a placement crosshair in the middle of the view and move it, and Enter places there, as Accessibility at Figma says (#1839); text direction per paragraph: Text ▸ Text direction forces a paragraph left to right or right to left, as Add right-to-left text says (#1830), with `text.direction`, `node.set textDirection`, the Type settings control once a right-to-left script is in the text, inspect, the CSS `direction` and the `.fig` lines' `sourceDirectionality`; the boolean operation menu in the Design panel leads each entry with an original icon drawn in code, a filled preview of the result (#2003); a press on the open gradient's handles or stops on the canvas keeps the colour picker open through the drag, as its own control (#1852); a click with the Bend tool on a sharp point curves it, its handles along the line between its neighbours, as Edit vector layers says (#1875); wrap style: Auto, Balance and Pretty on a text layer decide where its lines break, as Explore text properties says (#1827), with the Type settings control, `text.wrapStyle`, `node.set wrapStyle`, inspect and the CSS `text-wrap`; Backspace or Delete at the start of a list item keeps the item's indentation level, as the lists article says; max lines on auto-width text keep it auto width, hugging that many lines, as the text properties article says; superscript and subscript are synthesised where the font has no glyphs for them, from the standard glyphs shrunk and shifted by the font's OS/2 metrics, and a run mixing available and unavailable glyphs is synthesised whole, as Explore text properties says (#1828); Alt+Enter while editing text opens the link at the caret, as the accessibility page documents; Minimize, by its shortcut or the button by the file name, collapses the navigation bar and both sidebars as the help center says, with the right sidebar back while something is selected (#1835); the interface size keys of the help center's accessibility page: Cmd/Ctrl+Shift+Alt with =, - and 0 step the interface through the Preferences sizes and back to the system's size; Cmd/Ctrl+Alt+arrows rotate the selection a degree at a time, 15 with Shift, as the help center's accessibility page says; they resized before; the Layers panel answers Ctrl+1 as well as Alt+1 on Windows and Linux, the two keys the help center gives it; the colour picker's Image tab shows a placeholder and Upload from computer, an asset preview, and a file dropped on it replaces the image, keeping the fill mode, crop and position (#1682); the keyboard's Menu key opens the context menu, as Shift+F10 does; the eyedropper with Cmd/Ctrl+Shift makes a colour style or variable of what it picks; sidebar number and text fields keep their text vertically centered when focused, including typography spacing; with the pen out, Enter or a double-click ends the path and edits the vector with the Move tool instead of drawing on (#1430); the Actions palette offers only commands that run from it, and no second row reading like a menu item: Place image opens the file dialog (#1574); Preferences ▸ Scroll wheel zoom: a mouse wheel zooms the canvas about the pointer and Cmd/Ctrl+wheel scrolls (#1390); boolean operations refuse input past their caps, and a boolean group past them draws without its result (#1104); the selection and hover outlines of rounded rectangles and frames follow their corners (#1506); colour picker review fixes for text undo, fitted sidebars, moving controls and gradient variables; Corner smoothing: an original icon and the shared number-field drag gesture, preserving fractional baselines; guides snap to layers when dragged; Crop image: Object ▸ Crop image, the right-click menu and the image fill row enter crop mode (`image.crop`, #1464); the Design panel's sections on one grid — 16 / 8 margins, 88 + 8 + 88 + 8 + 24 columns, 24 pt rows, 4 pt between a row's buttons (#858); the toolbar on one 32 pt grid, without its Resources button (#1210); colour picker LCH and OKLCH models; an XML (Android Vector Drawable) export format; Context menus: the Menu key reaches egui 0.37 but is unbound (row done → partial); the Text on a path tool; board tool keys pinned; community fixes: the text box by dragging, the font picker keys, the pen path on leaving Design, a still canvas in vector edit mode, a scrolling pages list, colour emoji, readable new text in frames, system theme on first run; the Small caps sample in Type settings drawn from capitals; recent files as cards; vector edit mode: hovering, hatching, selecting and dragging filled regions; home screen: Templates tab, first-run templates and Getting started tips; number fields: scrub rates, arrow steps, arithmetic with brackets, relative edits of mixed values; outline mode's hidden layers and object bounds; Hide when publishing; cursors per tool and modifier; panel widths; dark pages for files made in the dark theme; drag threshold and double-clicks into text in groups; rotation origin on Alt+R and placing with Enter: no documented shortcut missing; keyboard shortcut parity suite and bindings; cursor chat and spotlight in live sessions; layers and assets panels behaviour suite and fixes; vector multi-edit and the vector edit tail; canvas navigation, guides and grids behaviour suite and fixes; shapes and vector behaviour suite and fixes; paint and effects behaviour suite and fixes; frames, groups, sections and masks behaviour suite and fixes; layers panel keyboard: F2 rename, Shift+F10 context menu, F8 box selection; selection and transform behaviour suite and fixes; Display P3: gamut warning and soft-proof (true P3 blocked upstream); several windows; boolean hit testing on the result, group drags, frame outlines and slices; outlines for the selection, mask outlines; arc fields, live boolean editing, shape builder on strokes, one-sided widths; fields select their text on focus; Typography spacing icons support scrubbing (#373); CSS colour names in hex fields; a rename selects the whole name, so typing replaces it; Escape or Enter after a pen path stays on the same vector (#760); IME composition kept on through idle frames of a text edit, candidates at the caret (#747); Selection colors for mixed selections and smart-selection spacing (#753); submenus with a hover delay and a safe triangle; the Actions palette's tabs, ranked search, highlight and recents; menu items that ask for a value; editor fixes from the community batches) · **Target:** Figma Design UI3 (desktop app 126.10.7) +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-11-04 · **Change:** minor (copy, cut and paste in vector edit mode act on the selected points and segments (#1875); the left rail's and tab strip's icons are larger, 20 pt (#553); pick select: Tab while the keyboard selection box shows (or the chord again) puts a pink outline on a layer and the keys walk the layer tree from it, as Accessibility at Figma says (#1836); scrubbing a field faster or slower by the pointer's height, with a notice of the speed, as the help center's Scrub fields says (#1837); component rows in the Layers panel at AA contrast in the theme's component tint, with the rows inside a selected component in a tone of its row (#1903); Cmd/Ctrl-click on a padding field sets all four sides with the value typed, a documented auto layout shortcut; a fill per region of a vector: with regions selected in edit mode, the Fill section and the fill commands edit those regions' own fills, as Edit vector layers says (#1875); the Fill section of a text whose characters were coloured differently while editing shows mixed content, characters that all share fills the layer doesn't have show those, and a layer-level fill edit reaches every character (#1856); a number field scrubs when dragged with Alt held, as the help center's scrub section says, not only by its label; reuse visible layer selection roots (M8); ; with a shape or frame tool chosen and nothing selected, the arrows show a placement crosshair in the middle of the view and move it, and Enter places there, as Accessibility at Figma says (#1839); text direction per paragraph: Text ▸ Text direction forces a paragraph left to right or right to left, as Add right-to-left text says (#1830), with `text.direction`, `node.set textDirection`, the Type settings control once a right-to-left script is in the text, inspect, the CSS `direction` and the `.fig` lines' `sourceDirectionality`; the boolean operation menu in the Design panel leads each entry with an original icon drawn in code, a filled preview of the result (#2003); a press on the open gradient's handles or stops on the canvas keeps the colour picker open through the drag, as its own control (#1852); a click with the Bend tool on a sharp point curves it, its handles along the line between its neighbours, as Edit vector layers says (#1875); wrap style: Auto, Balance and Pretty on a text layer decide where its lines break, as Explore text properties says (#1827), with the Type settings control, `text.wrapStyle`, `node.set wrapStyle`, inspect and the CSS `text-wrap`; Backspace or Delete at the start of a list item keeps the item's indentation level, as the lists article says; max lines on auto-width text keep it auto width, hugging that many lines, as the text properties article says; superscript and subscript are synthesised where the font has no glyphs for them, from the standard glyphs shrunk and shifted by the font's OS/2 metrics, and a run mixing available and unavailable glyphs is synthesised whole, as Explore text properties says (#1828); Alt+Enter while editing text opens the link at the caret, as the accessibility page documents; Minimize, by its shortcut or the button by the file name, collapses the navigation bar and both sidebars as the help center says, with the right sidebar back while something is selected (#1835); the interface size keys of the help center's accessibility page: Cmd/Ctrl+Shift+Alt with =, - and 0 step the interface through the Preferences sizes and back to the system's size; Cmd/Ctrl+Alt+arrows rotate the selection a degree at a time, 15 with Shift, as the help center's accessibility page says; they resized before; the Layers panel answers Ctrl+1 as well as Alt+1 on Windows and Linux, the two keys the help center gives it; the colour picker's Image tab shows a placeholder and Upload from computer, an asset preview, and a file dropped on it replaces the image, keeping the fill mode, crop and position (#1682); the keyboard's Menu key opens the context menu, as Shift+F10 does; the eyedropper with Cmd/Ctrl+Shift makes a colour style or variable of what it picks; sidebar number and text fields keep their text vertically centered when focused, including typography spacing; with the pen out, Enter or a double-click ends the path and edits the vector with the Move tool instead of drawing on (#1430); the Actions palette offers only commands that run from it, and no second row reading like a menu item: Place image opens the file dialog (#1574); Preferences ▸ Scroll wheel zoom: a mouse wheel zooms the canvas about the pointer and Cmd/Ctrl+wheel scrolls (#1390); boolean operations refuse input past their caps, and a boolean group past them draws without its result (#1104); the selection and hover outlines of rounded rectangles and frames follow their corners (#1506); colour picker review fixes for text undo, fitted sidebars, moving controls and gradient variables; Corner smoothing: an original icon and the shared number-field drag gesture, preserving fractional baselines; guides snap to layers when dragged; Crop image: Object ▸ Crop image, the right-click menu and the image fill row enter crop mode (`image.crop`, #1464); the Design panel's sections on one grid — 16 / 8 margins, 88 + 8 + 88 + 8 + 24 columns, 24 pt rows, 4 pt between a row's buttons (#858); the toolbar on one 32 pt grid, without its Resources button (#1210); colour picker LCH and OKLCH models; an XML (Android Vector Drawable) export format; Context menus: the Menu key reaches egui 0.37 but is unbound (row done → partial); the Text on a path tool; board tool keys pinned; community fixes: the text box by dragging, the font picker keys, the pen path on leaving Design, a still canvas in vector edit mode, a scrolling pages list, colour emoji, readable new text in frames, system theme on first run; the Small caps sample in Type settings drawn from capitals; recent files as cards; vector edit mode: hovering, hatching, selecting and dragging filled regions; home screen: Templates tab, first-run templates and Getting started tips; number fields: scrub rates, arrow steps, arithmetic with brackets, relative edits of mixed values; outline mode's hidden layers and object bounds; Hide when publishing; cursors per tool and modifier; panel widths; dark pages for files made in the dark theme; drag threshold and double-clicks into text in groups; rotation origin on Alt+R and placing with Enter: no documented shortcut missing; keyboard shortcut parity suite and bindings; cursor chat and spotlight in live sessions; layers and assets panels behaviour suite and fixes; vector multi-edit and the vector edit tail; canvas navigation, guides and grids behaviour suite and fixes; shapes and vector behaviour suite and fixes; paint and effects behaviour suite and fixes; frames, groups, sections and masks behaviour suite and fixes; layers panel keyboard: F2 rename, Shift+F10 context menu, F8 box selection; selection and transform behaviour suite and fixes; Display P3: gamut warning and soft-proof (true P3 blocked upstream); several windows; boolean hit testing on the result, group drags, frame outlines and slices; outlines for the selection, mask outlines; arc fields, live boolean editing, shape builder on strokes, one-sided widths; fields select their text on focus; Typography spacing icons support scrubbing (#373); CSS colour names in hex fields; a rename selects the whole name, so typing replaces it; Escape or Enter after a pen path stays on the same vector (#760); IME composition kept on through idle frames of a text edit, candidates at the caret (#747); Selection colors for mixed selections and smart-selection spacing (#753); submenus with a hover delay and a safe triangle; the Actions palette's tabs, ranked search, highlight and recents; menu items that ask for a value; editor fixes from the community batches) · **Target:** Figma Design UI3 (desktop app 126.10.7) Fine canvas control is what Figma users feel first: handles, snapping, nudging, modifier keys, drag behaviour, numeric precision. This checklist covers the tools, gestures and panels. Status: @@ -92,7 +92,7 @@ interaction feel not yet compared. | Rectangle, ellipse (arc handles and Design panel arc fields), polygon, star, line, arrow; Shift/Alt | done | Alt draws lines from the centre too (`shapes_behaviour_tests`) | | Corner radius handles (inside each corner of a rectangle or frame, a polygon's or star's top point; Alt: one corner), star ratio handle; Count and Ratio fields | done | handles hide under 48 px on screen; no count handle on the canvas (the Count field instead); corner smoothing rounds rectangles and frames only | | Pen: vector networks, branch, close, bend; pencil | done | pen pressure on Windows and from browser styluses only; outside edit mode the Pen closes on its first point, connects to its other points and continues a selected open path from an end point (`vector_multi_behaviour_tests`); Escape ends the path and keeps its layer in edit mode with the pen, so the next path joins the same vector and another Escape leaves edit mode (#760, `pen_end_tests`); Enter or a double-click ends the path (at the double-click's point, added once) and edits the vector with the Move tool, a double-click with the idle pen edits the layer under it, and in edit mode the pen's Enter or double-click takes the Move tool (#1430, `pen_vector_edit_tests`); switching from Design to Prototype or Inspect ends the path and returns to Move (#308) | -| Vector edit mode: bend, paint bucket, lasso, cut, eraser, transform box, point snapping, mirroring | done | a double-click a few pixels off a thin stroke enters edit mode (#646); entering edit mode and picking a region keep the canvas still (#322); a point dropped on another point merges with it, and dropping one end of an open path on the other closes it (a 2-point line's end just moves, #401); Flatten keeps open paths open (#377); paint bucket drags across areas; cut drags divide a shape into layers; delete and heal follows the curve; rounded points are drawn; Shift-click deselects a point at a corner of the box (`vector_behaviour_tests`); several vectors edited at once, Shift-click handles to drag them together, a cap and join per point, rounded points beside curves (`vector_multi_behaviour_tests`); filled regions with the Move tool: the area under the pointer is hatched ("/" lines in the selection colour, clipped by the area's own fill rule, holes left out, gone 3.5 px from a point or 2 px from a segment at any zoom), the line spacing follows octave folding (fitted to measurements at several zoom levels: 8 px at every power-of-two zoom, growing with the zoom and stepping back at √2 · 2ⁿ) and stays fixed to the shape when panning; a click selects the area and its points (moved with it but drawn unselected), Shift adds or removes an area, a press on a selected area drags the whole selection, no hover hatch during drags, a click on an empty spot (a hole) clears it and stays in edit mode; `vector.edit.select {regions}` (`regions_tests`, `vector_regions_tests`); a Bend click on a sharp point between two segments curves it (handles along the line between its neighbours, a third of the way to each, `ops::auto_smooth`), on a curved one sharpens it, a drag sets the handles by hand (#1875); with regions selected, the Fill section (its rows and the colour picker) and `fill.set`, `fill.add`, `fill.remove`, `fill.move` and `fill.paste` edit those regions' own fills, the layer's untouched, or the regions named by `regions: [index, ...]`; Copy fill takes the region's paint, the colour picker and its canvas gradient and crop handles edit it (an image from Upload from computer included; a file dropped on the picker goes to the canvas instead), what belongs to the layer (its fill style, a variable bound to its fill, the image crop button) stays off a region's rows, and `fill.pattern` sets the layer's fill (#1875). Not yet: areas of other layers edited at once | +| Vector edit mode: bend, paint bucket, lasso, cut, eraser, transform box, point snapping, mirroring | done | a double-click a few pixels off a thin stroke enters edit mode (#646); entering edit mode and picking a region keep the canvas still (#322); a point dropped on another point merges with it, and dropping one end of an open path on the other closes it (a 2-point line's end just moves, #401); Flatten keeps open paths open (#377); paint bucket drags across areas; cut drags divide a shape into layers; delete and heal follows the curve; rounded points are drawn; Shift-click deselects a point at a corner of the box (`vector_behaviour_tests`); several vectors edited at once, Shift-click handles to drag them together, a cap and join per point, rounded points beside curves (`vector_multi_behaviour_tests`); filled regions with the Move tool: the area under the pointer is hatched ("/" lines in the selection colour, clipped by the area's own fill rule, holes left out, gone 3.5 px from a point or 2 px from a segment at any zoom), the line spacing follows octave folding (fitted to measurements at several zoom levels: 8 px at every power-of-two zoom, growing with the zoom and stepping back at √2 · 2ⁿ) and stays fixed to the shape when panning; a click selects the area and its points (moved with it but drawn unselected), Shift adds or removes an area, a press on a selected area drags the whole selection, no hover hatch during drags, a click on an empty spot (a hole) clears it and stays in edit mode; `vector.edit.select {regions}` (`regions_tests`, `vector_regions_tests`); a Bend click on a sharp point between two segments curves it (handles along the line between its neighbours, a third of the way to each, `ops::auto_smooth`), on a curved one sharpens it, a drag sets the handles by hand (#1875); with regions selected, the Fill section (its rows and the colour picker) and `fill.set`, `fill.add`, `fill.remove`, `fill.move` and `fill.paste` edit those regions' own fills, the layer's untouched, or the regions named by `regions: [index, ...]`; Copy fill takes the region's paint, the colour picker and its canvas gradient and crop handles edit it (an image from Upload from computer included; a file dropped on the picker goes to the canvas instead), what belongs to the layer (its fill style, a variable bound to its fill, the image crop button) stays off a region's rows, and `fill.pattern` sets the layer's fill (#1875); Copy and Cut with points or segments selected take just those (a lone point: the segments touching it) as a vector with the layer's look, at their own place, so a paste outside edit mode is a new layer in place; Cut removes them as Delete does, the path splitting in the same layer with edit mode staying on; Paste while editing adds the clipboard's segments, or a copied shape's or boolean's outline, to the edited network in place with the new points selected, skipping layers with no outline (text, frames, groups; named in `skipped`) and refusing when none has one, as measured in Figma (`vector_clipboard_tests`, #1875). Not yet: areas of other layers edited at once | | Shape builder (Shift+M) on fills, strokes and open paths; variable width (Shift+W), one side with Alt | done | | | Boolean groups: clicked where the result is, double-click or Cmd/Ctrl-click into operands, live results while dragging or editing points, per-operand operations | done | a group past the path operations' caps (250 000 segments, 2 million possible crossings, 4 million segments swept over a fold) draws without its result, with a warning, and Flatten refuses it (#1104); the Design panel's Boolean groups menu leads Union, Subtract, Intersect and Exclude with their own icons, two offset squares with the result filled at reduced alpha (#2003) | | Text: point, auto height, fixed; caret, words, lines, IME | done | Backspace or Delete at the start of a list item takes its marker and keeps its level, the paragraph starting where the item's text did ("delete the counter, but keep the same level of indentation", Create bulleted and numbered lists; `text_tail_tests::list_keys_at_list_items`, text crate `behaviour_tests`); max lines on auto-width text keep it auto width, hugging up to that many lines and growing with its longest line ("Text resizing is set to auto height or auto width", Explore text properties; `text_behaviour_tests::max_lines_on_auto_width_text_keeps_auto_width_and_hugs_that_many_lines`); Alt+Enter while editing opens the link at the caret, web and mail links only (`text.openLink`; "press ⌥Option Return/Alt Enter to activate a focused link", Accessibility at Figma; `text_link_tests`); the font picker previews a family on the canvas while hovered (no undo step; Escape restores, #653) and Up/Down step through it (#352); a selection highlight after a line break starts at that line's start and an empty last line shows its mark (#665); carets step over whole emoji and accented letters; colour emoji draw from their colour tables (#300); IME composition replayed in tests as macOS sends it, not yet checked on a Mac (#747); CJK words by script run (no dictionary); vertical trim in Type settings; no spell check | @@ -187,6 +187,7 @@ in the table. `crates/ui-egui/src/shortcut_keys_tests.rs` drives the keys a look | Date | Change | Summary | |---|---|---| +| 2026-11-07 | minor | Vector edit mode, #1875, measured in Figma (web, real keys): Copy with segments selected takes only those (with just a point selected, the segments touching it) and a paste outside edit mode is a new vector layer in place; Cut removes them, the path splitting into subpaths in the same layer with edit mode staying on, and a paste outside gives a new layer with the cut segments; Paste while editing a vector adds the clipboard's segments, or a copied layer's outline, to the same network in place, selecting the pasted points, with no new layer and the bounds growing. FigCraft copied the whole layer, Cut left nothing to paste and Paste did nothing. `edit.copy`, `edit.cut` and `edit.paste` act so with no `ids`/`parent` given (Ctrl+X is the real Cut again; Shift+Delete still heals); the copy carries the layer's look (fills, strokes, effects, opacity, blend; the width profile only when every segment is copied); layers with no outline are skipped. Tests: `vector_clipboard_tests` | | 2026-11-02 | patch | Shortcuts: a Cmd/Ctrl-click on a padding field (the horizontal or vertical pair, or a side with Individual padding open) makes the value typed go to all four sides, in one undo step (Guide to auto layout, Keyboard shortcut guide); a plain click still edits that axis or side alone. A documented gesture row: 134 documented, 133 bound. Test: `padding_individual_tests` | | 2026-11-01 | patch | Number fields: dragging the field itself with Alt held scrubs it ("You can also hover the input field itself and hold down the modifier key: Mac: ⌥ Option Windows: Alt", Adjust alignment, rotation, position, and dimensions), with the resize cursor and the label's rates (Alt is the enabler there, so it isn't the fine rate); a click with Alt doesn't open the editor. Found by a documentation audit of the ui-parity rows. Test: `field_tests::alt_held_over_a_field_scrubs_it`. Counts unchanged | | 2026-11-02 | patch | M6, #1839: with a shape or frame tool chosen and nothing selected, the arrows show a placement crosshair in the middle of the view and move it by the nudge amounts (`tool.crosshair`), Enter places the object there (`tool.place`), Escape or a tool change takes it away; the canvas draws the crosshair and `ui.inspect` reports it (`tool.crosshair`), the Keyboard shortcuts panel lists the gesture ("Use the arrow keys to activate the crosshair cursor...", Accessibility at Figma). The board Shapes tool places on it too (its 160 px shape centred, as the tool centres a press), and the view pans by the overshoot when the crosshair is nudged past its edge. Tests: `place_crosshair_tests`, `shortcut_keys_tests::arrows_show_and_move_the_placement_crosshair_with_a_shape_tool`. | From ad701088a27b69be2cb22c5d2f51055d56bed966 Mon Sep 17 00:00:00 2001 From: Trevor Brown Date: Sun, 8 Nov 2026 00:38:49 -0500 Subject: [PATCH 05/23] Vector edit: Cut deletes exactly what it copies for a selected region (#2105 review) Co-Authored-By: Claude Sonnet 5.5 Model-Effort: Opus 5.5 drove; Opus 5.5 briefed; Sonnet 5.5 wrote at medium --- .../engine/src/cmd/edit/vector_clipboard.rs | 11 +++++--- .../src/cmd/edit/vector_clipboard_tests.rs | 25 +++++++++++++++++++ docs/ui-parity.md | 4 +-- 3 files changed, 34 insertions(+), 6 deletions(-) diff --git a/crates/engine/src/cmd/edit/vector_clipboard.rs b/crates/engine/src/cmd/edit/vector_clipboard.rs index d9efea269..1a600f119 100644 --- a/crates/engine/src/cmd/edit/vector_clipboard.rs +++ b/crates/engine/src/cmd/edit/vector_clipboard.rs @@ -22,6 +22,8 @@ pub(super) struct EditSelection { pub any: bool, /// Each edited layer with something to copy, and the sub-network that is. pub parts: Vec<(NodeId, VectorNetwork)>, + /// What Cut removes, per edited layer: the same points and segments the copy was built from. + pub delete: Vec, } /// The selection of the vector being edited (and the other layers edited with it), or `None` when @@ -29,7 +31,7 @@ pub(super) struct EditSelection { pub(super) fn edit_selection(s: &Session) -> Option { let ds = s.ds().ok()?; let ve = ds.vector_edit.as_ref()?; - let mut sel = EditSelection { any: false, parts: Vec::new() }; + let mut sel = EditSelection { any: false, parts: Vec::new(), delete: Vec::new() }; for part in ve.parts() { if part.vertices.is_empty() && part.segments.is_empty() { continue; @@ -44,6 +46,7 @@ pub(super) fn edit_selection(s: &Session) -> Option { if active { segments.extend(ve.regions.iter().filter_map(|r| net.regions.get(*r)).flat_map(|r| r.loops.iter().flatten().copied())); } + sel.delete.push(EditPart { id: part.id, vertices: vertices.clone(), segments: segments.clone() }); if let Some(sub) = sub_network(net, &vertices, &segments) { sel.parts.push((part.id, sub)); } @@ -144,10 +147,10 @@ pub(super) fn copy_selection(s: &mut Session, cmd: &str, sel: EditSelection) -> /// the same layer and edit mode stays on). A selection that has no segment to copy (lone points /// with nothing attached) is removed and the clipboard is left as it was. pub(super) fn cut_selection(s: &mut Session, sel: EditSelection) -> Result { + let cut = sel.parts.len(); let segments = if sel.parts.is_empty() { 0 } else { copy_parts(s, "edit.cut", &sel)? }; - let parts: Vec = vectoredit::editing(s)?.parts(); - vectoredit::delete_selection(s, parts, false)?; - Ok(json!({"cut": sel.parts.len(), "segments": segments})) + vectoredit::delete_selection(s, sel.delete, false)?; + Ok(json!({"cut": cut, "segments": segments})) } /// The network of the outline of copied layer `n` in its own coordinates: a vector's segments, a diff --git a/crates/engine/src/cmd/edit/vector_clipboard_tests.rs b/crates/engine/src/cmd/edit/vector_clipboard_tests.rs index 012a110fe..4490a21f2 100644 --- a/crates/engine/src/cmd/edit/vector_clipboard_tests.rs +++ b/crates/engine/src/cmd/edit/vector_clipboard_tests.rs @@ -322,3 +322,28 @@ fn copy_cut_and_paste_survive_junk_in_vector_edit_mode() { } } } + +#[test] +fn cutting_a_selected_region_deletes_exactly_the_segments_it_copies_and_keeps_a_tail() { + let mut s = Session::new(); + let r = ex(&mut s, "vector.create", json!({"points": [[100, 100], [200, 100], [150, 200]], "closed": true})); + let id = id_of(&r); + // A tail: one open segment from the first corner to a new point. + crate::vectoredit::edit(s.doc_mut().unwrap(), id, |n| { + n.vertices.push(figcraft_doc::Vertex::new(20.0, 20.0)); + let tail = n.vertices.len() - 1; + crate::vectoredit::ops::connect(n, 0, tail, (0.0, 0.0), (0.0, 0.0)); + }) + .unwrap(); + assert_eq!((net(&s, id).segments.len(), net(&s, id).regions.len()), (4, 1)); + ex(&mut s, "vector.edit.begin", json!({"id": id})); + ex(&mut s, "vector.edit.select", json!({"regions": [0]})); + ex(&mut s, "edit.cut", json!({})); + let n = net(&s, id); + assert_eq!(n.segments.len(), 1, "only the tail is left"); + assert!(n.regions.is_empty()); + leave(&mut s); + ex(&mut s, "edit.paste", json!({})); + let new = *layers(&s).last().unwrap(); + assert_eq!(net(&s, new).segments.len(), 3, "the copy holds the region's three segments, not the tail"); +} diff --git a/docs/ui-parity.md b/docs/ui-parity.md index 98816af28..930f41cb6 100644 --- a/docs/ui-parity.md +++ b/docs/ui-parity.md @@ -1,6 +1,6 @@ # UI and interaction parity -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-11-04 · **Change:** minor (copy, cut and paste in vector edit mode act on the selected points and segments (#1875); the left rail's and tab strip's icons are larger, 20 pt (#553); pick select: Tab while the keyboard selection box shows (or the chord again) puts a pink outline on a layer and the keys walk the layer tree from it, as Accessibility at Figma says (#1836); scrubbing a field faster or slower by the pointer's height, with a notice of the speed, as the help center's Scrub fields says (#1837); component rows in the Layers panel at AA contrast in the theme's component tint, with the rows inside a selected component in a tone of its row (#1903); Cmd/Ctrl-click on a padding field sets all four sides with the value typed, a documented auto layout shortcut; a fill per region of a vector: with regions selected in edit mode, the Fill section and the fill commands edit those regions' own fills, as Edit vector layers says (#1875); the Fill section of a text whose characters were coloured differently while editing shows mixed content, characters that all share fills the layer doesn't have show those, and a layer-level fill edit reaches every character (#1856); a number field scrubs when dragged with Alt held, as the help center's scrub section says, not only by its label; reuse visible layer selection roots (M8); ; with a shape or frame tool chosen and nothing selected, the arrows show a placement crosshair in the middle of the view and move it, and Enter places there, as Accessibility at Figma says (#1839); text direction per paragraph: Text ▸ Text direction forces a paragraph left to right or right to left, as Add right-to-left text says (#1830), with `text.direction`, `node.set textDirection`, the Type settings control once a right-to-left script is in the text, inspect, the CSS `direction` and the `.fig` lines' `sourceDirectionality`; the boolean operation menu in the Design panel leads each entry with an original icon drawn in code, a filled preview of the result (#2003); a press on the open gradient's handles or stops on the canvas keeps the colour picker open through the drag, as its own control (#1852); a click with the Bend tool on a sharp point curves it, its handles along the line between its neighbours, as Edit vector layers says (#1875); wrap style: Auto, Balance and Pretty on a text layer decide where its lines break, as Explore text properties says (#1827), with the Type settings control, `text.wrapStyle`, `node.set wrapStyle`, inspect and the CSS `text-wrap`; Backspace or Delete at the start of a list item keeps the item's indentation level, as the lists article says; max lines on auto-width text keep it auto width, hugging that many lines, as the text properties article says; superscript and subscript are synthesised where the font has no glyphs for them, from the standard glyphs shrunk and shifted by the font's OS/2 metrics, and a run mixing available and unavailable glyphs is synthesised whole, as Explore text properties says (#1828); Alt+Enter while editing text opens the link at the caret, as the accessibility page documents; Minimize, by its shortcut or the button by the file name, collapses the navigation bar and both sidebars as the help center says, with the right sidebar back while something is selected (#1835); the interface size keys of the help center's accessibility page: Cmd/Ctrl+Shift+Alt with =, - and 0 step the interface through the Preferences sizes and back to the system's size; Cmd/Ctrl+Alt+arrows rotate the selection a degree at a time, 15 with Shift, as the help center's accessibility page says; they resized before; the Layers panel answers Ctrl+1 as well as Alt+1 on Windows and Linux, the two keys the help center gives it; the colour picker's Image tab shows a placeholder and Upload from computer, an asset preview, and a file dropped on it replaces the image, keeping the fill mode, crop and position (#1682); the keyboard's Menu key opens the context menu, as Shift+F10 does; the eyedropper with Cmd/Ctrl+Shift makes a colour style or variable of what it picks; sidebar number and text fields keep their text vertically centered when focused, including typography spacing; with the pen out, Enter or a double-click ends the path and edits the vector with the Move tool instead of drawing on (#1430); the Actions palette offers only commands that run from it, and no second row reading like a menu item: Place image opens the file dialog (#1574); Preferences ▸ Scroll wheel zoom: a mouse wheel zooms the canvas about the pointer and Cmd/Ctrl+wheel scrolls (#1390); boolean operations refuse input past their caps, and a boolean group past them draws without its result (#1104); the selection and hover outlines of rounded rectangles and frames follow their corners (#1506); colour picker review fixes for text undo, fitted sidebars, moving controls and gradient variables; Corner smoothing: an original icon and the shared number-field drag gesture, preserving fractional baselines; guides snap to layers when dragged; Crop image: Object ▸ Crop image, the right-click menu and the image fill row enter crop mode (`image.crop`, #1464); the Design panel's sections on one grid — 16 / 8 margins, 88 + 8 + 88 + 8 + 24 columns, 24 pt rows, 4 pt between a row's buttons (#858); the toolbar on one 32 pt grid, without its Resources button (#1210); colour picker LCH and OKLCH models; an XML (Android Vector Drawable) export format; Context menus: the Menu key reaches egui 0.37 but is unbound (row done → partial); the Text on a path tool; board tool keys pinned; community fixes: the text box by dragging, the font picker keys, the pen path on leaving Design, a still canvas in vector edit mode, a scrolling pages list, colour emoji, readable new text in frames, system theme on first run; the Small caps sample in Type settings drawn from capitals; recent files as cards; vector edit mode: hovering, hatching, selecting and dragging filled regions; home screen: Templates tab, first-run templates and Getting started tips; number fields: scrub rates, arrow steps, arithmetic with brackets, relative edits of mixed values; outline mode's hidden layers and object bounds; Hide when publishing; cursors per tool and modifier; panel widths; dark pages for files made in the dark theme; drag threshold and double-clicks into text in groups; rotation origin on Alt+R and placing with Enter: no documented shortcut missing; keyboard shortcut parity suite and bindings; cursor chat and spotlight in live sessions; layers and assets panels behaviour suite and fixes; vector multi-edit and the vector edit tail; canvas navigation, guides and grids behaviour suite and fixes; shapes and vector behaviour suite and fixes; paint and effects behaviour suite and fixes; frames, groups, sections and masks behaviour suite and fixes; layers panel keyboard: F2 rename, Shift+F10 context menu, F8 box selection; selection and transform behaviour suite and fixes; Display P3: gamut warning and soft-proof (true P3 blocked upstream); several windows; boolean hit testing on the result, group drags, frame outlines and slices; outlines for the selection, mask outlines; arc fields, live boolean editing, shape builder on strokes, one-sided widths; fields select their text on focus; Typography spacing icons support scrubbing (#373); CSS colour names in hex fields; a rename selects the whole name, so typing replaces it; Escape or Enter after a pen path stays on the same vector (#760); IME composition kept on through idle frames of a text edit, candidates at the caret (#747); Selection colors for mixed selections and smart-selection spacing (#753); submenus with a hover delay and a safe triangle; the Actions palette's tabs, ranked search, highlight and recents; menu items that ask for a value; editor fixes from the community batches) · **Target:** Figma Design UI3 (desktop app 126.10.7) +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-11-08 · **Change:** minor (copy, cut and paste in vector edit mode act on the selected points and segments (#1875); the left rail's and tab strip's icons are larger, 20 pt (#553); pick select: Tab while the keyboard selection box shows (or the chord again) puts a pink outline on a layer and the keys walk the layer tree from it, as Accessibility at Figma says (#1836); scrubbing a field faster or slower by the pointer's height, with a notice of the speed, as the help center's Scrub fields says (#1837); component rows in the Layers panel at AA contrast in the theme's component tint, with the rows inside a selected component in a tone of its row (#1903); Cmd/Ctrl-click on a padding field sets all four sides with the value typed, a documented auto layout shortcut; a fill per region of a vector: with regions selected in edit mode, the Fill section and the fill commands edit those regions' own fills, as Edit vector layers says (#1875); the Fill section of a text whose characters were coloured differently while editing shows mixed content, characters that all share fills the layer doesn't have show those, and a layer-level fill edit reaches every character (#1856); a number field scrubs when dragged with Alt held, as the help center's scrub section says, not only by its label; reuse visible layer selection roots (M8); ; with a shape or frame tool chosen and nothing selected, the arrows show a placement crosshair in the middle of the view and move it, and Enter places there, as Accessibility at Figma says (#1839); text direction per paragraph: Text ▸ Text direction forces a paragraph left to right or right to left, as Add right-to-left text says (#1830), with `text.direction`, `node.set textDirection`, the Type settings control once a right-to-left script is in the text, inspect, the CSS `direction` and the `.fig` lines' `sourceDirectionality`; the boolean operation menu in the Design panel leads each entry with an original icon drawn in code, a filled preview of the result (#2003); a press on the open gradient's handles or stops on the canvas keeps the colour picker open through the drag, as its own control (#1852); a click with the Bend tool on a sharp point curves it, its handles along the line between its neighbours, as Edit vector layers says (#1875); wrap style: Auto, Balance and Pretty on a text layer decide where its lines break, as Explore text properties says (#1827), with the Type settings control, `text.wrapStyle`, `node.set wrapStyle`, inspect and the CSS `text-wrap`; Backspace or Delete at the start of a list item keeps the item's indentation level, as the lists article says; max lines on auto-width text keep it auto width, hugging that many lines, as the text properties article says; superscript and subscript are synthesised where the font has no glyphs for them, from the standard glyphs shrunk and shifted by the font's OS/2 metrics, and a run mixing available and unavailable glyphs is synthesised whole, as Explore text properties says (#1828); Alt+Enter while editing text opens the link at the caret, as the accessibility page documents; Minimize, by its shortcut or the button by the file name, collapses the navigation bar and both sidebars as the help center says, with the right sidebar back while something is selected (#1835); the interface size keys of the help center's accessibility page: Cmd/Ctrl+Shift+Alt with =, - and 0 step the interface through the Preferences sizes and back to the system's size; Cmd/Ctrl+Alt+arrows rotate the selection a degree at a time, 15 with Shift, as the help center's accessibility page says; they resized before; the Layers panel answers Ctrl+1 as well as Alt+1 on Windows and Linux, the two keys the help center gives it; the colour picker's Image tab shows a placeholder and Upload from computer, an asset preview, and a file dropped on it replaces the image, keeping the fill mode, crop and position (#1682); the keyboard's Menu key opens the context menu, as Shift+F10 does; the eyedropper with Cmd/Ctrl+Shift makes a colour style or variable of what it picks; sidebar number and text fields keep their text vertically centered when focused, including typography spacing; with the pen out, Enter or a double-click ends the path and edits the vector with the Move tool instead of drawing on (#1430); the Actions palette offers only commands that run from it, and no second row reading like a menu item: Place image opens the file dialog (#1574); Preferences ▸ Scroll wheel zoom: a mouse wheel zooms the canvas about the pointer and Cmd/Ctrl+wheel scrolls (#1390); boolean operations refuse input past their caps, and a boolean group past them draws without its result (#1104); the selection and hover outlines of rounded rectangles and frames follow their corners (#1506); colour picker review fixes for text undo, fitted sidebars, moving controls and gradient variables; Corner smoothing: an original icon and the shared number-field drag gesture, preserving fractional baselines; guides snap to layers when dragged; Crop image: Object ▸ Crop image, the right-click menu and the image fill row enter crop mode (`image.crop`, #1464); the Design panel's sections on one grid — 16 / 8 margins, 88 + 8 + 88 + 8 + 24 columns, 24 pt rows, 4 pt between a row's buttons (#858); the toolbar on one 32 pt grid, without its Resources button (#1210); colour picker LCH and OKLCH models; an XML (Android Vector Drawable) export format; Context menus: the Menu key reaches egui 0.37 but is unbound (row done → partial); the Text on a path tool; board tool keys pinned; community fixes: the text box by dragging, the font picker keys, the pen path on leaving Design, a still canvas in vector edit mode, a scrolling pages list, colour emoji, readable new text in frames, system theme on first run; the Small caps sample in Type settings drawn from capitals; recent files as cards; vector edit mode: hovering, hatching, selecting and dragging filled regions; home screen: Templates tab, first-run templates and Getting started tips; number fields: scrub rates, arrow steps, arithmetic with brackets, relative edits of mixed values; outline mode's hidden layers and object bounds; Hide when publishing; cursors per tool and modifier; panel widths; dark pages for files made in the dark theme; drag threshold and double-clicks into text in groups; rotation origin on Alt+R and placing with Enter: no documented shortcut missing; keyboard shortcut parity suite and bindings; cursor chat and spotlight in live sessions; layers and assets panels behaviour suite and fixes; vector multi-edit and the vector edit tail; canvas navigation, guides and grids behaviour suite and fixes; shapes and vector behaviour suite and fixes; paint and effects behaviour suite and fixes; frames, groups, sections and masks behaviour suite and fixes; layers panel keyboard: F2 rename, Shift+F10 context menu, F8 box selection; selection and transform behaviour suite and fixes; Display P3: gamut warning and soft-proof (true P3 blocked upstream); several windows; boolean hit testing on the result, group drags, frame outlines and slices; outlines for the selection, mask outlines; arc fields, live boolean editing, shape builder on strokes, one-sided widths; fields select their text on focus; Typography spacing icons support scrubbing (#373); CSS colour names in hex fields; a rename selects the whole name, so typing replaces it; Escape or Enter after a pen path stays on the same vector (#760); IME composition kept on through idle frames of a text edit, candidates at the caret (#747); Selection colors for mixed selections and smart-selection spacing (#753); submenus with a hover delay and a safe triangle; the Actions palette's tabs, ranked search, highlight and recents; menu items that ask for a value; editor fixes from the community batches) · **Target:** Figma Design UI3 (desktop app 126.10.7) Fine canvas control is what Figma users feel first: handles, snapping, nudging, modifier keys, drag behaviour, numeric precision. This checklist covers the tools, gestures and panels. Status: @@ -187,7 +187,7 @@ in the table. `crates/ui-egui/src/shortcut_keys_tests.rs` drives the keys a look | Date | Change | Summary | |---|---|---| -| 2026-11-07 | minor | Vector edit mode, #1875, measured in Figma (web, real keys): Copy with segments selected takes only those (with just a point selected, the segments touching it) and a paste outside edit mode is a new vector layer in place; Cut removes them, the path splitting into subpaths in the same layer with edit mode staying on, and a paste outside gives a new layer with the cut segments; Paste while editing a vector adds the clipboard's segments, or a copied layer's outline, to the same network in place, selecting the pasted points, with no new layer and the bounds growing. FigCraft copied the whole layer, Cut left nothing to paste and Paste did nothing. `edit.copy`, `edit.cut` and `edit.paste` act so with no `ids`/`parent` given (Ctrl+X is the real Cut again; Shift+Delete still heals); the copy carries the layer's look (fills, strokes, effects, opacity, blend; the width profile only when every segment is copied); layers with no outline are skipped. Tests: `vector_clipboard_tests` | +| 2026-11-08 | minor | Vector edit mode, #1875, measured in Figma (web, real keys): Copy with segments selected takes only those (with just a point selected, the segments touching it) and a paste outside edit mode is a new vector layer in place; Cut removes them, the path splitting into subpaths in the same layer with edit mode staying on, and a paste outside gives a new layer with the cut segments; Paste while editing a vector adds the clipboard's segments, or a copied layer's outline, to the same network in place, selecting the pasted points, with no new layer and the bounds growing. FigCraft copied the whole layer, Cut left nothing to paste and Paste did nothing. `edit.copy`, `edit.cut` and `edit.paste` act so with no `ids`/`parent` given (Ctrl+X is the real Cut again; Shift+Delete still heals); the copy carries the layer's look (fills, strokes, effects, opacity, blend; the width profile only when every segment is copied); layers with no outline are skipped. Tests: `vector_clipboard_tests` | | 2026-11-02 | patch | Shortcuts: a Cmd/Ctrl-click on a padding field (the horizontal or vertical pair, or a side with Individual padding open) makes the value typed go to all four sides, in one undo step (Guide to auto layout, Keyboard shortcut guide); a plain click still edits that axis or side alone. A documented gesture row: 134 documented, 133 bound. Test: `padding_individual_tests` | | 2026-11-01 | patch | Number fields: dragging the field itself with Alt held scrubs it ("You can also hover the input field itself and hold down the modifier key: Mac: ⌥ Option Windows: Alt", Adjust alignment, rotation, position, and dimensions), with the resize cursor and the label's rates (Alt is the enabler there, so it isn't the fine rate); a click with Alt doesn't open the editor. Found by a documentation audit of the ui-parity rows. Test: `field_tests::alt_held_over_a_field_scrubs_it`. Counts unchanged | | 2026-11-02 | patch | M6, #1839: with a shape or frame tool chosen and nothing selected, the arrows show a placement crosshair in the middle of the view and move it by the nudge amounts (`tool.crosshair`), Enter places the object there (`tool.place`), Escape or a tool change takes it away; the canvas draws the crosshair and `ui.inspect` reports it (`tool.crosshair`), the Keyboard shortcuts panel lists the gesture ("Use the arrow keys to activate the crosshair cursor...", Accessibility at Figma). The board Shapes tool places on it too (its 160 px shape centred, as the tool centres a press), and the view pans by the overshoot when the crosshair is nudged past its edge. Tests: `place_crosshair_tests`, `shortcut_keys_tests::arrows_show_and_move_the_placement_crosshair_with_a_shape_tool`. | From 87ce9612e6f2225ab1d288fe06edc89caf052475 Mon Sep 17 00:00:00 2001 From: Trevor Brown Date: Sun, 8 Nov 2026 01:44:23 -0500 Subject: [PATCH 06/23] Vector edit clipboard: Cut and Copy with only lone points are refused; one x/y refused; the system clipboard gets the partial copy; specs match (#2105 review) Cut with nothing to copy (only points that touch no segment) is Disabled and changes nothing, as Copy now is (Copy used to fall through to the whole layer); paste borrows the clipboard instead of cloning it; a partial copy or cut mirrors its SVG to the system clipboard; x without y (or y without x) is refused; the junk test asserts refusals change nothing and accepted results are finite; the edit.copy, edit.cut and edit.paste specs state the behaviour and the mode-specific paste response. Co-Authored-By: Claude Sonnet 5.5 Model-Effort: Opus 5.5 drove; Opus 5.5 briefed; Sonnet 5.5 wrote at medium --- crates/engine/src/cmd/edit.rs | 7 +- crates/engine/src/cmd/edit/clipboard.rs | 4 +- .../engine/src/cmd/edit/vector_clipboard.rs | 40 +++++- .../src/cmd/edit/vector_clipboard_tests.rs | 121 ++++++++++++++++-- crates/ui-egui/src/menus.rs | 7 +- 5 files changed, 152 insertions(+), 27 deletions(-) diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index 19a7d7011..c03721af4 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -18,6 +18,7 @@ mod props; pub(crate) use props::{edit_region_fills, fill_regions}; mod select; mod vector_clipboard; +pub use vector_clipboard::clipboard_svg; #[cfg(test)] mod vector_clipboard_tests; @@ -123,21 +124,21 @@ pub fn specs() -> Vec { spec!( "edit.copy", "Copy", - "{ids?} in vector edit mode with points or segments selected (and no ids): just those, the segments touching a selected point, as a new vector with the edited layer's look", + "{ids?} in vector edit mode with points or segments selected (and no ids): just those, the segments touching a selected point, as a temporary vector on the clipboard (one per edited layer) with the edited layer's look, kept at its place on the page; points that touch no segment have nothing to copy, so Copy with only those is refused (Disabled). Returns {copied, segments}", false, copy ), spec!( "edit.cut", "Cut", - "{ids?} in vector edit mode with points or segments selected (and no ids): copies them as Copy does, then removes them as Delete does; the path splits in the same layer and edit mode stays on", + "{ids?} in vector edit mode with points or segments selected (and no ids): copies them as Copy does, then removes them as Delete does; the path splits in the same layer and edit mode stays on. Returns {cut (layers), segments}. With only lone points (nothing to copy) it is refused (Disabled) and changes nothing, as Copy is; Delete removes lone points", true, cut ), spec!( "edit.paste", "Paste", - "{parent?, x?, y? (page coordinates of the pasted top-left)} in vector edit mode (and no parent): the copied vectors' segments and shapes' outlines join the edited vector's network in place, selected, and the layers that have no outline (text, frames, groups) are skipped (`skipped`); a copied main component pastes as an instance of it within its file (cut ones, and pastes into other files, keep the component)", + "{parent?, x?, y? (page coordinates of the pasted top-left; give both or neither, one alone is refused)} in vector edit mode (and no parent): the copied vectors' segments and the outlines of shapes, booleans and lines join the edited vector's network (at x/y when given, else in place), their points selected, as one undo step; layers with no outline (text, images, instances, frames, groups) are skipped and named in `skipped` (present only when some were), and when none has an outline the paste is refused (Disabled), as it is past 500,000 segments in one vector. Returns {ids: [] (always empty: no layer is created), edited (the vector's id), vertices (points added), skipped?}, unlike a paste outside edit mode, which returns the new layers' `ids`; a copied main component pastes as an instance of it within its file (cut ones, and pastes into other files, keep the component)", true, paste ), diff --git a/crates/engine/src/cmd/edit/clipboard.rs b/crates/engine/src/cmd/edit/clipboard.rs index da5089419..a04bfcb5c 100644 --- a/crates/engine/src/cmd/edit/clipboard.rs +++ b/crates/engine/src/cmd/edit/clipboard.rs @@ -193,8 +193,9 @@ pub(super) fn copy(s: &mut Session, p: &Value) -> Result { // In vector edit mode with points or segments selected, Copy takes just those (#1875). if !names_layers(p) && let Some(sel) = vector_clipboard::edit_selection(s) - && !sel.parts.is_empty() + && sel.any { + // Only lone points selected: nothing to copy, and not the whole layer either. return vector_clipboard::copy_selection(s, "edit.copy", sel); } let ids = req_targets("edit.copy", s, p)?; @@ -208,6 +209,7 @@ pub(super) fn cut(s: &mut Session, p: &Value) -> Result { && let Some(sel) = vector_clipboard::edit_selection(s) && sel.any { + // Lone points with nothing to copy: `cut_selection` refuses, as Copy does. return vector_clipboard::cut_selection(s, sel); } let ids = req_targets("edit.cut", s, p)?; diff --git a/crates/engine/src/cmd/edit/vector_clipboard.rs b/crates/engine/src/cmd/edit/vector_clipboard.rs index 1a600f119..72c60bf02 100644 --- a/crates/engine/src/cmd/edit/vector_clipboard.rs +++ b/crates/engine/src/cmd/edit/vector_clipboard.rs @@ -7,6 +7,7 @@ //! while editing adds the clipboard's layers to the edited vector's network, in place: a vector //! with its segments, a shape or boolean with its outline. +use crate::bad; use figcraft_doc::{NodeKind, Region, VectorNetwork}; use super::clipboard::{collect_images, store_clipboard}; @@ -144,15 +145,37 @@ pub(super) fn copy_selection(s: &mut Session, cmd: &str, sel: EditSelection) -> } /// Cut: copy the selected segments, then remove the selection as Delete does (the path splits in -/// the same layer and edit mode stays on). A selection that has no segment to copy (lone points -/// with nothing attached) is removed and the clipboard is left as it was. +/// the same layer and edit mode stays on). Cut is Copy plus Delete, so it is refused exactly where +/// Copy is: a selection with nothing to copy (only lone points that touch no segment) is +/// `Disabled` and changes nothing, clipboard included. Delete removes such points. pub(super) fn cut_selection(s: &mut Session, sel: EditSelection) -> Result { + if sel.parts.is_empty() { + return Err(EngineError::Disabled("edit.cut".into(), "the selection has nothing to copy (Delete removes lone points)".into())); + } let cut = sel.parts.len(); - let segments = if sel.parts.is_empty() { 0 } else { copy_parts(s, "edit.cut", &sel)? }; + let segments = copy_parts(s, "edit.cut", &sel)?; vectoredit::delete_selection(s, sel.delete, false)?; Ok(json!({"cut": cut, "segments": segments})) } +/// The SVG of the layers a partial vector-edit copy or cut put on the clipboard, for the system +/// clipboard: they are temporary layers that live only in the clipboard, so they are placed in a +/// scratch document to export. `None` when the clipboard holds none or the export fails. +pub fn clipboard_svg(s: &Session) -> Option { + let mut scratch = Document::new("Clipboard"); + let page = scratch.current_page?; + for (k, v) in &s.clipboard.images { + scratch.images.insert(k.clone(), v.clone()); + } + let mut ids = Vec::new(); + for (root, nodes) in &s.clipboard.nodes { + let map: BTreeMap = nodes.iter().map(|n| (n.id, scratch.alloc_id())).collect(); + ids.push(scratch.paste_nodes(nodes, &map, *root, page, None).ok()?); + } + let bytes = crate::cmd::file::export_bytes(&scratch, &ids, figcraft_doc::ExportFormat::Svg, 1.0).ok()?; + String::from_utf8(bytes).ok() +} + /// The network of the outline of copied layer `n` in its own coordinates: a vector's segments, a /// shape's or boolean's outline. `None` for the layers with no outline to add (text, images, /// frames, groups, instances, …). `doc` is the file the layer was copied from. @@ -210,16 +233,18 @@ fn placed(mut net: VectorNetwork, m: Affine) -> Option { /// (named in `skipped`); when none has one, nothing is pasted. pub(super) fn paste_into_vector(s: &mut Session, p: &Value) -> Result { const CMD: &str = "edit.paste"; - let clip = s.clipboard.clone(); - if clip.nodes.is_empty() { + if s.clipboard.nodes.is_empty() { return Err(EngineError::Disabled(CMD.into(), "the clipboard is empty".into())); } let target = vectoredit::editing(s)?.id; - let shift = match (num(p, "x"), num(p, "y"), clip.bounds) { + let shift = match (num(p, "x"), num(p, "y"), s.clipboard.bounds) { (Some(x), Some(y), Some(b)) => Affine::translate((x - b.x0, y - b.y0)), - _ => Affine::IDENTITY, + (None, None, _) | (Some(_), Some(_), None) => Affine::IDENTITY, + _ => return Err(bad(CMD, "give both x and y, or neither")), }; let live = s.doc()?; + // Borrow the clipboard (it holds the source document and the images) to build the pieces. + let clip = &s.clipboard; let source: &Document = clip.source.as_deref().unwrap_or(live); let to_local = live.world_transform(target).inverse() * shift; let mut pieces = Vec::new(); @@ -269,6 +294,7 @@ pub(super) fn paste_into_vector(s: &mut Session, p: &Value) -> Result { ve.pen_out = None; ve.width_point = None; })?; + // `ids` stays empty: no layer is created, the pasted outlines joined layer `edited`. let mut out = json!({"ids": [], "edited": target, "vertices": pasted.len()}); if !skipped.is_empty() { out["skipped"] = json!(skipped); diff --git a/crates/engine/src/cmd/edit/vector_clipboard_tests.rs b/crates/engine/src/cmd/edit/vector_clipboard_tests.rs index 4490a21f2..e737c11d6 100644 --- a/crates/engine/src/cmd/edit/vector_clipboard_tests.rs +++ b/crates/engine/src/cmd/edit/vector_clipboard_tests.rs @@ -291,6 +291,19 @@ fn a_selected_face_copies_its_own_outline_with_the_fill() { assert!(matches!(s.doc().unwrap().node(new).unwrap().kind, NodeKind::Vector { .. })); } +/// Everything junk must not change when a command is refused: the vector, the layers, the edit +/// selection and what the clipboard holds. +fn state(s: &Session, id: NodeId) -> String { + let ve = s.ds().unwrap().vector_edit.clone(); + let clip: Vec = s.clipboard.nodes.iter().flat_map(|(_, n)| n).map(|n| format!("{:?}", n.kind)).collect(); + format!("{:?}|{:?}|{:?}|{:?}|{}", net(s, id), layers(s), ve.map(|v| (v.vertices, v.segments)), clip, s.clipboard.layers_generation) +} + +fn finite(n: &VectorNetwork) -> bool { + n.vertices.iter().all(|v| v.x.is_finite() && v.y.is_finite()) + && n.segments.iter().all(|sg| [sg.ta.0, sg.ta.1, sg.tb.0, sg.tb.1].iter().all(|x| x.is_finite())) +} + #[test] fn copy_cut_and_paste_survive_junk_in_vector_edit_mode() { let junk = [ @@ -301,28 +314,108 @@ fn copy_cut_and_paste_survive_junk_in_vector_edit_mode() { json!({"id": "nope", "x": "NaN", "y": 1e308}), json!({"parent": "nope", "x": -1e308, "y": "x"}), json!({"x": 1e308, "y": 1e308}), + json!({"x": 1e308}), ]; for cmd in ["edit.copy", "edit.cut", "edit.paste", "edit.pasteOver", "edit.pasteReplace"] { for j in &junk { - let mut s = Session::new(); - path(&mut s); - select(&mut s, &[1], &[0]); - ex(&mut s, "text.create", json!({"text": "t"})); - if let Err(EngineError::Internal { cmd, msg }) = s.execute(cmd, j) { - panic!("{cmd} panicked on {j}: {msg}"); - } - // And after something was copied from the edit selection. - let mut s = Session::new(); - path(&mut s); - select(&mut s, &[2], &[]); - let _ = s.execute("edit.copy", &json!({})); - if let Err(EngineError::Internal { cmd, msg }) = s.execute(cmd, j) { - panic!("{cmd} panicked on {j}: {msg}"); + // Both a selection with a text layer next to it, and after a copy from the selection. + for copied in [false, true] { + let mut s = Session::new(); + let id = path(&mut s); + if copied { + select(&mut s, &[2], &[]); + let _ = s.execute("edit.copy", &json!({})); + } else { + select(&mut s, &[1], &[0]); + ex(&mut s, "text.create", json!({"text": "t"})); + ex(&mut s, "vector.edit.begin", json!({"id": id})); + select(&mut s, &[1], &[0]); + } + let before = state(&s, id); + match s.execute(cmd, j) { + Err(EngineError::Internal { cmd, msg }) => panic!("{cmd} panicked on {j}: {msg}"), + Err(_) => { + if cmd != "edit.pasteOver" && cmd != "edit.pasteReplace" { + assert_eq!(state(&s, id), before, "{cmd} {j} was refused but changed something"); + } + } + // Accepted: whatever it made has finite coordinates. + Ok(_) => { + if let Some(n) = crate::vectoredit::network(s.doc().unwrap(), id) { + assert!(finite(n), "{cmd} {j} left non-finite coordinates"); + } + } + } } } } } +#[test] +fn a_huge_paste_position_is_refused_or_stays_finite() { + let mut s = Session::new(); + let id = path(&mut s); + select(&mut s, &[], &[0]); + ex(&mut s, "edit.copy", json!({})); + let before = state(&s, id); + match s.execute("edit.paste", &json!({"x": 1e308, "y": 1e308})) { + Err(e) => { + assert!(!matches!(e, EngineError::Internal { .. }), "{e}"); + assert_eq!(state(&s, id), before, "a refused paste changed something"); + } + // It is accepted here: 1e308 plus a few hundred is still finite, so the points are too. + Ok(_) => assert!(finite(&net(&s, id)), "the points are finite"), + } +} + +#[test] +fn cut_with_only_lone_points_is_refused_like_copy_and_changes_nothing() { + let mut s = Session::new(); + let id = path(&mut s); + // A point no segment touches. + crate::vectoredit::edit(s.doc_mut().unwrap(), id, |n| n.vertices.push(figcraft_doc::Vertex::new(50.0, 50.0))).unwrap(); + // Something on the clipboard first, then only the lone point selected. + select(&mut s, &[0], &[]); + ex(&mut s, "edit.copy", json!({})); + select(&mut s, &[4], &[]); + let before = state(&s, id); + for cmd in ["edit.copy", "edit.cut"] { + let err = s.execute(cmd, &json!({})).expect_err(cmd); + assert!(matches!(err, EngineError::Disabled(..)), "{cmd}: {err}"); + assert_eq!(state(&s, id), before, "{cmd} changed something"); + } + // Delete is the way to remove it. + ex(&mut s, "vector.edit.delete", json!({})); + assert_eq!(net(&s, id).vertices.len(), 4); +} + +#[test] +fn only_one_of_x_and_y_is_refused_and_pastes_nothing() { + let mut s = Session::new(); + let id = path(&mut s); + select(&mut s, &[], &[0]); + ex(&mut s, "edit.copy", json!({})); + let before = state(&s, id); + for p in [json!({"x": 10}), json!({"y": 10})] { + let err = s.execute("edit.paste", &p).expect_err("one coordinate"); + assert!(err.to_string().contains("give both x and y, or neither"), "{err}"); + assert_eq!(state(&s, id), before); + } + ex(&mut s, "edit.paste", json!({"x": 10, "y": 10})); +} + +#[test] +fn a_partial_copy_has_an_svg_for_the_system_clipboard() { + let mut s = Session::new(); + let r = ex(&mut s, "shape.rectangle", json!({"x": 100, "y": 100, "width": 80, "height": 60})); + assert!(crate::cmd::edit::clipboard_svg(&s).is_none(), "nothing copied yet"); + ex(&mut s, "vector.edit.begin", json!({"id": id_of(&r)})); + select(&mut s, &[], &[0, 1, 2, 3]); + ex(&mut s, "edit.copy", json!({})); + let svg = crate::cmd::edit::clipboard_svg(&s).expect("svg"); + assert!(svg.contains(" Date: Sun, 8 Nov 2026 13:53:24 -0300 Subject: [PATCH 07/23] M6: a vector point without a corner radius of its own takes the layer's on .fig import (#2114) Figma rounds every point whose style doesn't set a corner radius with the layer's cornerRadius. The import read only the point's style, so those vectors came in sharp. Points now fall back to the layer's radius, at the scale of the layer that carries it (#2091), and the network cache keys on it. The export writes an explicit 0 for a sharp point on a rounded vector so it stays sharp. --- contributors/people.toml | 3 + crates/format/src/fig/export/shape.rs | 4 +- crates/format/src/fig/export_tests.rs | 19 ++++++ crates/format/src/fig/map.rs | 5 +- crates/format/src/fig/map/layer.rs | 7 ++- crates/format/src/fig/tail_tests.rs | 90 +++++++++++++++++++++++++++ 6 files changed, 123 insertions(+), 5 deletions(-) diff --git a/contributors/people.toml b/contributors/people.toml index c6924df5f..34f849ed7 100644 --- a/contributors/people.toml +++ b/contributors/people.toml @@ -115,3 +115,6 @@ display_name = "Hamza0101" [people.ashokram-si] real_name = "Ashok Ram" + +[people.GiovanniBot] +sync_github_name = true diff --git a/crates/format/src/fig/export/shape.rs b/crates/format/src/fig/export/shape.rs index ca5846cde..bfc2b0224 100644 --- a/crates/format/src/fig/export/shape.rs +++ b/crates/format/src/fig/export/shape.rs @@ -207,7 +207,9 @@ impl<'d> Exporter<'d> { .enumerate() .map(|(i, v)| { let mut st: Fields = Vec::new(); - if v.radius > 0.0 { + // A point without a radius of its own is drawn with the layer's, so a sharp + // point on a rounded layer says so. + if v.radius > 0.0 || n.corners.radii[0] > 0.0 { st.push(("cornerRadius", fl(v.radius))); } if let Some(m) = v.mirror { diff --git a/crates/format/src/fig/export_tests.rs b/crates/format/src/fig/export_tests.rs index 9651a1025..4fd533545 100644 --- a/crates/format/src/fig/export_tests.rs +++ b/crates/format/src/fig/export_tests.rs @@ -955,6 +955,25 @@ fn closed_vector_geometry_is_written_with_its_corner_smoothing() { assert_eq!(radii, vec![20.0, 0.0, 10.0, 0.0], "each point keeps its own radius"); } +#[test] +fn sharp_points_on_a_rounded_vector_stay_sharp() { + // A point without a radius takes the layer's when read, so a sharp point on a vector with a + // layer radius is written with its own zero. + let mut doc = Document::new("sharp point"); + let page = doc.first_page().unwrap(); + let mut network = VectorNetwork::from_points(&[(0.0, 0.0), (100.0, 0.0), (100.0, 100.0), (0.0, 100.0)].map(|(x, y)| Point::new(x, y)), true); + for (v, r) in network.vertices.iter_mut().zip([8.0, 0.0, 8.0, 0.0]) { + v.radius = r; + } + let id = doc.create(page, NodeKind::Vector { network }, "V", 0.0, 0.0, 100.0, 100.0).unwrap(); + doc.node_mut(id).unwrap().corners.radii = [8.0; 4]; + let (back, _) = round_trip(&doc, "sharp point"); + let v = back.nodes().find(|n| matches!(n.kind, NodeKind::Vector { .. })).unwrap(); + let NodeKind::Vector { network } = &v.kind else { panic!("vector") }; + let radii: Vec = network.vertices.iter().map(|x| x.radius).collect(); + assert_eq!(radii, vec![8.0, 0.0, 8.0, 0.0]); +} + #[test] fn the_written_schema_parses_and_round_trips() { let schema = Schema::parse(export::SCHEMA).unwrap(); diff --git a/crates/format/src/fig/map.rs b/crates/format/src/fig/map.rs index fd1b47575..aa0db5cf4 100644 --- a/crates/format/src/fig/map.rs +++ b/crates/format/src/fig/map.rs @@ -356,8 +356,9 @@ struct Mapper<'a> { /// holding it) name it by `vectorNetworkRef`. Kept out of the JSON, where a large file's /// networks took gigabytes. networks: Vec>, - /// Index in `networks` by its blob, position scale, style table and point-radius scale. - network_ids: HashMap<(usize, u64, u64, usize, u64), usize>, + /// Index in `networks` by its blob, position scale, style table, point-radius scale and the + /// layer's corner radius (which points without one of their own take). + network_ids: HashMap<(usize, u64, u64, usize, u64, u64), usize>, /// Region paint elements (paints and gradient stops) read so far: one budget for the whole /// file, since layers with region paints don't share their network. region_paint_elements: usize, diff --git a/crates/format/src/fig/map/layer.rs b/crates/format/src/fig/map/layer.rs index e3de96368..a69bb7d46 100644 --- a/crates/format/src/fig/map/layer.rs +++ b/crates/format/src/fig/map/layer.rs @@ -668,6 +668,8 @@ impl<'a> Mapper<'a> { size.zip(norm).filter(|(s, n)| s.is_finite() && *n > 1e-9).map_or(1.0, |(s, n)| s / n) }; let (sx, sy) = (scale("x"), scale("y")); + // A point whose style doesn't set a radius takes the layer's, as the editor draws it. + let layer_radius = v.len("cornerRadius"); let vertices: Vec = net .vertices .iter() @@ -676,7 +678,7 @@ impl<'a> Mapper<'a> { let st = style(p.style); // Point radii are lengths in this row's space, unlike the normalized positions. // An override/derived row already carries the instance's scale. - if let Some(r) = st.and_then(|s| num(s, "cornerRadius")).and_then(|r| scaled(r, radius_scale)).filter(|r| *r > 0.0) { + if let Some(r) = st.and_then(|s| num(s, "cornerRadius")).and_then(|r| scaled(r, radius_scale)).or(layer_radius).filter(|r| *r > 0.0) { out["radius"] = json!(r); } match st.and_then(|s| s.get("handleMirroring")).and_then(Kv::str) { @@ -793,7 +795,8 @@ impl<'a> Mapper<'a> { return true; } // A resized instance and a K-scaled one can have the same box but different radii. - let key = (blob_id, sx.to_bits(), sy.to_bits(), std::ptr::from_ref(vd) as usize, radius_scale.to_bits()); + let key = + (blob_id, sx.to_bits(), sy.to_bits(), std::ptr::from_ref(vd) as usize, radius_scale.to_bits(), layer_radius.unwrap_or(0.0).to_bits()); let id = match self.network_ids.get(&key) { Some(id) => *id, None => { diff --git a/crates/format/src/fig/tail_tests.rs b/crates/format/src/fig/tail_tests.rs index 176c95c9b..663d6002b 100644 --- a/crates/format/src/fig/tail_tests.rs +++ b/crates/format/src/fig/tail_tests.rs @@ -521,6 +521,96 @@ fn network_of<'d>(doc: &'d figcraft_doc::Document, name: &str) -> &'d figcraft_d } } +#[test] +fn vector_points_without_a_radius_of_their_own_take_the_layers() { + // A vector rounded from the layer's corner radius: the editor rounds every point whose + // style doesn't set a radius (no style, or one without `cornerRadius`) with the layer's, + // while a point's own radius, zero included, wins. + let mut message = tail_message(); + let Kv::Array(nodes) = field_mut(&mut message, "nodeChanges").unwrap() else { panic!() }; + let wave = nodes.iter_mut().find(|node| node.get("name").and_then(Kv::str) == Some("Wave")).unwrap(); + let Kv::Object(fields) = wave else { panic!() }; + fields.push(("cornerRadius", f(6.0))); + let vector_data = field_mut(wave, "vectorData").unwrap(); + let Kv::Array(styles) = field_mut(vector_data, "styleOverrideTable").unwrap() else { panic!() }; + let Kv::Object(sharp) = styles.get_mut(2).unwrap() else { panic!() }; + sharp.push(("cornerRadius", f(0.0))); + let (doc, _) = import(&canvas(&schema(), &message), "Rounded").unwrap(); + let radii: Vec = network_of(&doc, "Wave").vertices.iter().map(|v| v.radius).collect(); + assert_eq!(radii, [6.0, 4.0, 6.0, 0.0], "no style, own radius, style without one, own zero"); + // Without a layer radius, the points without one of their own stay sharp. + let (doc, _) = import(&tail_canvas(), "Tail").unwrap(); + let radii: Vec = network_of(&doc, "Wave").vertices.iter().map(|v| v.radius).collect(); + assert_eq!(radii, [0.0, 4.0, 0.0, 0.0]); +} + +#[test] +fn an_instance_that_rounds_a_vector_keeps_its_own_points() { + // An instance's copy shares its main's vector data; overriding the layer's radius must not + // reuse the main's network, nor hand the instance's to the main. + let mut message = tail_message(); + let Kv::Array(nodes) = field_mut(&mut message, "nodeChanges").unwrap() else { panic!() }; + let wave = nodes.iter_mut().find(|node| node.get("name").and_then(Kv::str) == Some("Wave")).unwrap(); + *field_mut(wave, "parentIndex").unwrap() = o(vec![("guid", g(50)), ("position", s("!"))]); + *field_mut(wave, "transform").unwrap() = at(0.0, 0.0); + let rounded = o(vec![("guidPath", o(vec![("guids", arr(vec![g(30)]))])), ("cornerRadius", f(6.0))]); + nodes.push(node(50, Some((1, "~")), "SYMBOL", "Mark", vec![("size", xy(200.0, 100.0)), ("transform", at(700.0, 0.0))])); + nodes.push(node( + 51, + Some((1, "~!")), + "INSTANCE", + "Rounded mark", + vec![ + ("size", xy(200.0, 100.0)), + ("transform", at(700.0, 400.0)), + ("symbolData", o(vec![("symbolID", g(50)), ("symbolOverrides", arr(vec![rounded]))])), + ], + )); + let (doc, _) = import(&canvas(&schema(), &message), "Instance").unwrap(); + let radii_under = |parent: &str| -> Vec { + let p = named(&doc, parent); + let wave = p.children.iter().map(|c| doc.node(*c).unwrap()).find(|n| n.name == "Wave").unwrap(); + let NodeKind::Vector { network } = &wave.kind else { panic!() }; + network.vertices.iter().map(|v| v.radius).collect() + }; + assert_eq!(radii_under("Mark"), [0.0, 4.0, 0.0, 0.0]); + assert_eq!(radii_under("Rounded mark"), [6.0, 4.0, 6.0, 6.0]); +} + +#[test] +fn a_scaled_instance_scales_its_vectors_point_radii() { + // Point radii are lengths: an instance scaled with the Scale tool draws its main's, from a + // point's own style or from the layer, times the factor (#2091). + let mut message = tail_message(); + let Kv::Array(nodes) = field_mut(&mut message, "nodeChanges").unwrap() else { panic!() }; + let wave = nodes.iter_mut().find(|node| node.get("name").and_then(Kv::str) == Some("Wave")).unwrap(); + *field_mut(wave, "parentIndex").unwrap() = o(vec![("guid", g(50)), ("position", s("!"))]); + *field_mut(wave, "transform").unwrap() = at(0.0, 0.0); + let Kv::Object(fields) = wave else { panic!() }; + fields.push(("cornerRadius", f(3.0))); + nodes.push(node(50, Some((1, "~")), "SYMBOL", "Mark", vec![("size", xy(200.0, 100.0)), ("transform", at(700.0, 0.0))])); + nodes.push(node( + 51, + Some((1, "~!")), + "INSTANCE", + "Big mark", + vec![ + ("size", xy(400.0, 200.0)), + ("transform", at(700.0, 400.0)), + ("symbolData", o(vec![("symbolID", g(50)), ("uniformScaleFactor", f(2.0))])), + ], + )); + let (doc, _) = import(&canvas(&schema(), &message), "Scaled").unwrap(); + let radii_under = |parent: &str| -> Vec { + let p = named(&doc, parent); + let wave = p.children.iter().map(|c| doc.node(*c).unwrap()).find(|n| n.name == "Wave").unwrap(); + let NodeKind::Vector { network } = &wave.kind else { panic!() }; + network.vertices.iter().map(|v| v.radius).collect() + }; + assert_eq!(radii_under("Mark"), [3.0, 4.0, 3.0, 3.0]); + assert_eq!(radii_under("Big mark"), [6.0, 8.0, 6.0, 6.0]); +} + #[test] fn vectors_import_as_editable_networks() { let (doc, warnings) = import(&tail_canvas(), "Tail").unwrap(); From a6d9b74df9b1d52e1c15ecc6f5e43a9694ac55ce Mon Sep 17 00:00:00 2001 From: impulsado <72570835+impulsado@users.noreply.github.com> Date: Sun, 8 Nov 2026 18:23:08 +0100 Subject: [PATCH 08/23] M3: resolve inherited slot properties by their defining component --- crates/engine/src/assets/components/sync.rs | 23 +++- crates/engine/src/slot_tests.rs | 119 ++++++++++++++++++++ 2 files changed, 137 insertions(+), 5 deletions(-) diff --git a/crates/engine/src/assets/components/sync.rs b/crates/engine/src/assets/components/sync.rs index 7b34c33c2..17836b111 100644 --- a/crates/engine/src/assets/components/sync.rs +++ b/crates/engine/src/assets/components/sync.rs @@ -34,7 +34,7 @@ pub(super) fn sync_instance_depth(doc: &mut Document, inst: NodeId, depth: usize let nid = doc.alloc_id(); map.insert(s, nid); } - let Copies { nodes: new_nodes, nested, mut remeasure } = copy_layers(doc, inst, &src, &map, &old, &edited, eff_props); + let Copies { nodes: new_nodes, nested, mut remeasure } = copy_layers(doc, inst, component, &src, &map, &old, &edited, eff_props); for n in new_nodes { let (id, parent) = (n.id, n.parent); if insert_linked(doc, n).is_err() @@ -132,6 +132,7 @@ struct Copies { fn copy_layers( doc: &Document, inst: NodeId, + component: NodeId, src: &[NodeId], map: &BTreeMap, old: &BTreeMap>, @@ -139,7 +140,7 @@ fn copy_layers( props: BTreeMap, ) -> Copies { let mut enclosing: BTreeMap> = BTreeMap::from([(inst, vec![inst])]); - let mut inst_props: BTreeMap> = BTreeMap::from([(inst, props)]); + let mut inst_props: BTreeMap)> = BTreeMap::from([(inst, (component, props))]); let mut out = Copies { nodes: Vec::new(), nested: Vec::new(), remeasure: Vec::new() }; for s in src { let Some(orig) = doc.node(*s) else { continue }; @@ -169,7 +170,7 @@ fn copy_layers( let enc = enclosing.get(&parent).cloned().unwrap_or_default(); apply_enclosing_props(doc, &mut n, orig, &enc, &inst_props); if let NodeKind::Instance { component: c, props } = &n.kind { - inst_props.insert(nid, props.clone()); + inst_props.insert(nid, (*c, props.clone())); let mut e = enc; e.push(nid); enclosing.insert(nid, e); @@ -253,14 +254,26 @@ fn rebuild_nested(doc: &mut Document, nested: Vec, old: &BTreeMap>) { +fn apply_enclosing_props( + doc: &Document, + n: &mut Node, + orig: &Node, + enc: &[NodeId], + inst_props: &BTreeMap)>, +) { let mut chain: Vec<&Node> = vec![orig]; while chain.len() < enc.len() { let Some(next) = chain.last().and_then(|c| c.source).and_then(|x| doc.node(x)) else { break }; chain.push(next); } for (j, level) in chain.iter().enumerate().rev() { - let Some(props) = enc.get(j).and_then(|e| inst_props.get(e)) else { continue }; + if level.prop_refs.is_empty() { + continue; + } + let Some(owner) = owning_component(doc, level.id) else { continue }; + // Instance-owned slot content skips a source hop through the slot's component. Match + // the binding's defining component, not the position in the enclosing instance chain. + let Some((_, props)) = enc.iter().rev().filter_map(|e| inst_props.get(e)).find(|(component, _)| *component == owner) else { continue }; for (field, prop) in &level.prop_refs { let g = field_group(field); if n.overrides.contains(g) || (j > 0 && orig.overrides.contains(g)) { diff --git a/crates/engine/src/slot_tests.rs b/crates/engine/src/slot_tests.rs index 01a5db08e..a3f9c868a 100644 --- a/crates/engine/src/slot_tests.rs +++ b/crates/engine/src/slot_tests.rs @@ -376,6 +376,125 @@ fn slots_of_nested_instances_fill_from_the_outer_main_and_its_instances() { assert_eq!(fill(&s, *kids(&s, nslot).first().unwrap()), "CCCCCC", "content no longer follows the Card main"); } +/// Wrap a component instance in another main, keeping the nested instance addressable. +fn slot_text_wrapper(s: &mut Session, component: NodeId, name: &str) -> (NodeId, NodeId) { + let frame = idv(&ex(s, "frame.create", json!({"width": 300, "height": 200, "name": name}))); + let nested = idv(&ex(s, "instance.create", json!({"component": component, "parent": frame}))); + (idv(&ex(s, "component.create", json!({"ids": [frame]}))), nested) +} + +/// Content.Label binds Text; Composition holds Base, whose Body uses default or own Content. +/// Returns (Content main, Label, Base main, Composition main, nested Content instance). +fn slot_text_composition(s: &mut Session, own: bool) -> (NodeId, NodeId, NodeId, NodeId, NodeId) { + let frame = idv(&ex(s, "frame.create", json!({"width": 100, "height": 40, "name": "Content"}))); + let label = idv(&ex(s, "text.create", json!({"text": "Default", "parent": frame, "props": {"name": "Label"}}))); + let content = idv(&ex(s, "component.create", json!({"ids": [frame]}))); + ex(s, "component.addProperty", json!({"id": content, "name": "Text", "type": "text", "default": "Default", "layer": label})); + let frame = idv(&ex(s, "frame.create", json!({"width": 200, "height": 100, "name": "Base"}))); + let body = idv(&ex(s, "frame.create", json!({"width": 180, "height": 80, "name": "Body", "parent": frame}))); + let default = (!own).then(|| idv(&ex(s, "instance.create", json!({"component": content, "parent": body})))); + let base = idv(&ex(s, "component.create", json!({"ids": [frame]}))); + ex(s, "component.makeSlot", json!({"id": body})); + let (composition, nested_base) = slot_text_wrapper(s, base, "Composition"); + let nested_content = if let Some(default) = default { + copy_of(s, nested_base, default) + } else { + idv(&ex(s, "instance.slotInsert", json!({"id": nested_base, "slot": "Body", "component": content}))) + }; + (content, label, base, composition, nested_content) +} + +fn assert_slot_text(s: &mut Session, instance: NodeId, expected: &str) { + let label = kids(s, instance)[0]; + let characters = node(s, label).kind.text().unwrap().characters.clone(); + let html = ex(s, "inspect.code", json!({"id": instance, "language": "html"}))["code"].as_str().unwrap().to_string(); + assert_eq!(characters, expected, "slot Label characters; HTML: {html}"); + assert!(html.contains(&format!(">{expected}

")), "slot Label HTML: {html}"); +} + +fn assert_local_slot_text(s: &mut Session, instance: NodeId, expected: &str) { + assert!(matches!(&node(s, instance).kind, NodeKind::Instance { props, .. } if props.get("Text") == Some(&json!(expected)))); + let properties = props_of(s, instance); + assert_eq!(properties.iter().find(|p| p["name"] == "Text").unwrap()["value"], expected); +} + +#[test] +fn slot_text_properties_stay_local_in_default_and_own_content() { + for own in [false, true] { + let mut s = Session::new(); + let (content, label, _base, composition, main_content) = slot_text_composition(&mut s, own); + let a = idv(&ex(&mut s, "instance.create", json!({"component": composition, "x": 400}))); + let b = idv(&ex(&mut s, "instance.create", json!({"component": composition, "x": 800}))); + let ac = copy_of(&s, a, main_content); + let bc = copy_of(&s, b, main_content); + ex(&mut s, "instance.setProperties", json!({"ids": [ac], "properties": {"Text": "Custom"}})); + assert_local_slot_text(&mut s, ac, "Custom"); + assert_slot_text(&mut s, ac, "Custom"); + assert_slot_text(&mut s, bc, "Default"); + assert_slot_text(&mut s, main_content, "Default"); + assert_eq!(node(&s, label).kind.text().unwrap().characters, "Default"); + // Main property edits propagate without replacing another instance's local value. + ex(&mut s, "instance.setProperties", json!({"ids": [main_content], "properties": {"Text": "Main value"}})); + assert_slot_text(&mut s, ac, "Custom"); + assert_slot_text(&mut s, bc, "Main value"); + assert_slot_text(&mut s, main_content, "Main value"); + ex(&mut s, "component.editProperty", json!({"id": content, "name": "Text", "default": "New default"})); + assert_slot_text(&mut s, ac, "Custom"); + assert_slot_text(&mut s, bc, "Main value"); + healthy(&s); + } +} + +#[test] +fn slot_text_properties_do_not_read_an_unbound_base_property() { + let mut s = Session::new(); + let (_content, _label, base, composition, main_content) = slot_text_composition(&mut s, true); + ex(&mut s, "component.addProperty", json!({"id": base, "name": "Text", "type": "text", "default": "Wrong owner"})); + let i = idv(&ex(&mut s, "instance.create", json!({"component": composition, "x": 400}))); + let nested = copy_of(&s, i, main_content); + assert_slot_text(&mut s, nested, "Default"); + ex(&mut s, "instance.setProperties", json!({"ids": [nested], "properties": {"Text": "Custom"}})); + assert_local_slot_text(&mut s, nested, "Custom"); + assert_slot_text(&mut s, nested, "Custom"); + assert_slot_text(&mut s, main_content, "Default"); + healthy(&s); +} + +#[test] +fn slot_text_properties_survive_inherited_own_content_and_persistence() { + let mut s = Session::new(); + let (_content, _label, _base, composition, main_content) = slot_text_composition(&mut s, true); + let (outer, inherited) = slot_text_wrapper(&mut s, composition, "Outer composition"); + let inherited_content = copy_of(&s, inherited, main_content); + let a = idv(&ex(&mut s, "instance.create", json!({"component": outer, "x": 400}))); + let b = idv(&ex(&mut s, "instance.create", json!({"component": outer, "x": 800}))); + let ac = copy_of(&s, a, inherited_content); + let bc = copy_of(&s, b, inherited_content); + ex(&mut s, "instance.setProperties", json!({"ids": [ac], "properties": {"Text": "Custom"}})); + assert_local_slot_text(&mut s, ac, "Custom"); + let bytes = figcraft_format::save_bytes(s.doc().unwrap(), None).unwrap(); + let (back, _) = figcraft_format::load_bytes(&bytes).unwrap(); + let mut s = Session::with_document(back); + assert_local_slot_text(&mut s, ac, "Custom"); + assert_slot_text(&mut s, ac, "Custom"); + assert_slot_text(&mut s, bc, "Default"); + assert_slot_text(&mut s, inherited_content, "Default"); + assert_slot_text(&mut s, main_content, "Default"); + // Explicit characters override the bound property, even after a main update. + let alabel = kids(&s, ac)[0]; + ex(&mut s, "text.set", json!({"id": alabel, "characters": "Explicit"})); + ex(&mut s, "instance.setProperties", json!({"ids": [ac], "properties": {"Text": "Later"}})); + ex(&mut s, "instance.setProperties", json!({"ids": [main_content], "properties": {"Text": "Main value"}})); + assert_local_slot_text(&mut s, ac, "Later"); + assert_slot_text(&mut s, ac, "Explicit"); + assert_slot_text(&mut s, bc, "Main value"); + ex(&mut s, "component.resetOverride", json!({"id": alabel, "group": "characters"})); + assert_slot_text(&mut s, ac, "Later"); + s.undo().unwrap(); + assert_slot_text(&mut s, ac, "Explicit"); + healthy(&s); +} + /// Random slot edits, main edits and resets keep every instance mirroring its main. #[test] fn random_slot_edits_keep_instances_consistent() { From 0aba3f7632457150cfe18c96510022e1566151c6 Mon Sep 17 00:00:00 2001 From: ljkx <177671075+ljkx@users.noreply.github.com> Date: Mon, 9 Nov 2026 06:15:26 +0100 Subject: [PATCH 09/23] M4: a Pencil stroke stays where it was drawn, without spikes off it (#1741) kurbo's simplify_bezpath, which smooths Pencil samples and runs Simplify path, fitted two common shapes with a curve that ran far off the path: a stroke that goes out and back to the very point it left (kurbo keeps a reversal as a smooth join) and nearly collinear samples (kurbo measures its error across the path, not along it). figcraft_geom::simplify_path cuts the path where it turns back by more than a right angle, simplifies each run with kurbo, and keeps a run as it was when its fit strays more than twice the accuracy from it. The Pencil and vector.simplify use it. --- crates/engine/src/brush_tests.rs | 46 +++++++ crates/engine/src/canvas/tools/pen.rs | 5 +- crates/engine/src/cmd/structure.rs | 8 +- crates/engine/src/vector_behaviour_tests.rs | 21 +++ crates/geom/src/lib.rs | 2 + crates/geom/src/simplify.rs | 140 ++++++++++++++++++++ crates/geom/src/tests.rs | 117 ++++++++++++++++ 7 files changed, 333 insertions(+), 6 deletions(-) create mode 100644 crates/geom/src/simplify.rs diff --git a/crates/engine/src/brush_tests.rs b/crates/engine/src/brush_tests.rs index 35dba6a7f..50317496d 100644 --- a/crates/engine/src/brush_tests.rs +++ b/crates/engine/src/brush_tests.rs @@ -149,3 +149,49 @@ fn the_pencil_draws_white_on_a_dark_background() { assert_eq!(stroke(&mut s, 0.0, 600.0).0, want, "on {bg}"); } } + +/// A Pencil stroke stays where it was drawn: the fitted curve shot far off the stroke, a spike +/// (#1741, its second point), where the stroke went out and back to the very point it left and +/// where samples ran on nearly straight. Pointer pixels become fractional page points, so going +/// back to a pixel lands on exactly the same point. Both sample runs are ones kurbo's fit took +/// hundreds of units off. +#[test] +fn the_pencil_keeps_its_stroke_where_it_was_drawn() { + let mut s = Session::new(); + s.execute("tool.set", &json!({"tool": "pencil"})).unwrap(); + let (p, q) = ((86.043_565_719_086_97, -173.744_402_349_100_94), (87.851_850_739_335_35, -179.202_846_445_954_52)); + let back = [(80.0, -160.0), p, q, p, (95.0, -185.0)]; + let straight = [ + (204.0, -198.0), + (209.598_690_794_015_5, -204.909_328_119_739_88), + (211.277_914_263_870_97, -207.190_974_020_753_4), + (214.246_506_828_720_84, -211.228_827_590_623_24), + (218.0, -214.0), + ]; + for (what, samples) in [("out and back", &back[..]), ("nearly straight", &straight[..])] { + let (first, last) = (samples[0], samples[samples.len() - 1]); + s.pointer(&PointerEvent::at(PointerKind::Down, first.0, first.1, 1.0)).unwrap(); + for (x, y) in &samples[1..] { + s.pointer(&PointerEvent::at(PointerKind::Move, *x, *y, 1.0)).unwrap(); + } + s.pointer(&PointerEvent::at(PointerKind::Up, last.0, last.1, 1.0)).unwrap(); + let doc = s.doc().unwrap(); + let n = doc.nodes().filter(|n| matches!(n.kind, NodeKind::Vector { .. })).max_by_key(|n| n.id).unwrap(); + let NodeKind::Vector { network } = &n.kind else { panic!("{what}: not a vector") }; + let drawn = doc.world_transform(n.id) * network.stroke_path(); + let samples_box = figcraft_geom::points_bounds(samples.iter().map(|(x, y)| Point::new(*x, *y))).unwrap(); + let (path_box, node_box) = (figcraft_geom::Shape::bounding_box(&drawn), doc.world_bounds(n.id).unwrap()); + assert!( + samples_box.inflate(1.5, 1.5).contains_rect(path_box), + "{what}: the stroke {path_box:?} goes past the samples {samples_box:?}: {}", + drawn.to_svg() + ); + assert!(samples_box.inflate(1.5, 1.5).contains_rect(node_box), "{what}: the layer {node_box:?}, the samples {samples_box:?}"); + } + // The turn is a point of the stroke, where it was drawn. + let doc = s.doc().unwrap(); + let turned = doc.nodes().find(|n| n.name == "Vector" && doc.world_bounds(n.id).is_some_and(|b| b.y1 < -150.0 && b.x1 < 100.0)).unwrap(); + let NodeKind::Vector { network } = &turned.kind else { panic!("not a vector") }; + let xf = doc.world_transform(turned.id); + assert!(network.vertices.iter().any(|v| (xf * Point::new(v.x, v.y) - Point::new(q.0, q.1)).hypot() < 1e-6), "no point at the turn"); +} diff --git a/crates/engine/src/canvas/tools/pen.rs b/crates/engine/src/canvas/tools/pen.rs index 4b98ccee1..3f8b47e59 100644 --- a/crates/engine/src/canvas/tools/pen.rs +++ b/crates/engine/src/canvas/tools/pen.rs @@ -313,7 +313,8 @@ impl Session { if points.len() < 2 { return Ok(()); } - // Smooth: fit a curve through the samples. + // Smooth: fit a curve through the samples, kept from shooting past them where the stroke + // turns back or runs nearly straight (#1741). let mut poly = figcraft_geom::BezPath::new(); for (k, p) in points.iter().enumerate() { if k == 0 { @@ -322,7 +323,7 @@ impl Session { poly.line_to(*p); } } - let fitted = figcraft_geom::kurbo::simplify::simplify_bezpath(poly.iter(), 0.75, &Default::default()); + let fitted = figcraft_geom::simplify_path(&poly, 0.75); let d = fitted.to_svg(); if matches!(self.tool.tool, ToolId::Marker | ToolId::Highlighter) { return self.ink_finish(&d); diff --git a/crates/engine/src/cmd/structure.rs b/crates/engine/src/cmd/structure.rs index 3339faa38..34cc91fbb 100644 --- a/crates/engine/src/cmd/structure.rs +++ b/crates/engine/src/cmd/structure.rs @@ -799,7 +799,7 @@ fn simplify(s: &mut Session, p: &Value) -> Result { WindingRule::NonZero }; let path = drawn.fill_path(); - let path = figcraft_geom::kurbo::simplify::simplify_bezpath(path.iter(), tol, &Default::default()); + let path = figcraft_geom::simplify_path(&path, tol); let (path, output_winding) = match figcraft_pathops::simplify_with_rule(&path, winding == WindingRule::EvenOdd) { Ok(path) => (path, WindingRule::EvenOdd), Err(_) => (path, winding), @@ -809,7 +809,7 @@ fn simplify(s: &mut Session, p: &Value) -> Result { } else { for (index, region) in source.regions.iter().enumerate() { let Some(path) = drawn.region_path(index) else { continue }; - let path = figcraft_geom::kurbo::simplify::simplify_bezpath(path.iter(), tol, &Default::default()); + let path = figcraft_geom::simplify_path(&path, tol); let path = figcraft_pathops::simplify_with_rule(&path, region.winding == WindingRule::EvenOdd).unwrap_or(path); append_region_path(&mut net, &path, region, region.winding); } @@ -826,7 +826,7 @@ fn simplify(s: &mut Session, p: &Value) -> Result { open.regions.clear(); if !open.segments.is_empty() { let path = open.stroke_path(); - let simplified = figcraft_geom::kurbo::simplify::simplify_bezpath(path.iter(), tol, &Default::default()); + let simplified = figcraft_geom::simplify_path(&path, tol); let mut loose = VectorNetwork::from_path(&simplified); loose.regions.clear(); append_network(&mut net, loose); @@ -834,7 +834,7 @@ fn simplify(s: &mut Session, p: &Value) -> Result { } if source.regions.is_empty() { let path = source.stroke_path(); - let simplified = figcraft_geom::kurbo::simplify::simplify_bezpath(path.iter(), tol, &Default::default()); + let simplified = figcraft_geom::simplify_path(&path, tol); net = VectorNetwork::from_path(&simplified); net.regions.clear(); } diff --git a/crates/engine/src/vector_behaviour_tests.rs b/crates/engine/src/vector_behaviour_tests.rs index c990ad348..4666c3028 100644 --- a/crates/engine/src/vector_behaviour_tests.rs +++ b/crates/engine/src/vector_behaviour_tests.rs @@ -326,6 +326,27 @@ fn ex_to_vector(doc: &mut figcraft_doc::Document, id: NodeId) { doc.get_mut(id).unwrap().kind = NodeKind::Vector { network }; } +/// Simplify path keeps nearly collinear points near where they were: kurbo's fit, which Simplify +/// path and the Pencil share, ran a handle hundreds of units on along the line (#1741). +#[test] +fn simplify_keeps_nearly_collinear_points_near_them() { + let mut s = Session::new(); + let id = id_of(&ex(&mut s, "vector.create", json!({"path": "M0 0 L10 10"}))["id"]).unwrap(); + // The points exactly as sampled, at the page origin (`vector.create` would move them). + let d = "M209.5986907940155 -204.90932811973988 L211.27791426387097 -207.1909740207534 L214.24650682872084 -211.22882759062324"; + let before = BezPath::from_svg(d).unwrap(); + { + let n = s.doc_mut().unwrap().get_mut(id).unwrap(); + n.transform = figcraft_geom::Affine::IDENTITY; + n.kind = NodeKind::Vector { network: VectorNetwork::from_path(&before) }; + } + assert_eq!(net(&s, id).stroke_path().elements(), before.elements(), "the samples as they were"); + ex(&mut s, "vector.simplify", json!({"ids": [id], "tolerance": 0.75})); + let after = s.doc().unwrap().world_transform(id) * net(&s, id).stroke_path(); + let (b0, b1) = (before.bounding_box(), after.bounding_box()); + assert!(b0.inflate(1.5, 1.5).contains_rect(b1), "{b1:?} past {b0:?}: {}", after.to_svg()); +} + fn ex(s: &mut Session, cmd: &str, p: Value) -> Value { s.execute(cmd, &p).unwrap_or_else(|e| panic!("{cmd}: {e}")) } diff --git a/crates/geom/src/lib.rs b/crates/geom/src/lib.rs index a775a4170..191b30419 100644 --- a/crates/geom/src/lib.rs +++ b/crates/geom/src/lib.rs @@ -12,7 +12,9 @@ use kurbo::ParamCurve; use std::f64::consts::{FRAC_PI_2, PI, TAU}; pub mod measure; +mod simplify; mod transform; +pub use simplify::simplify_path; pub use transform::*; /// Corner radii in clockwise order from the top-left: `[tl, tr, br, bl]`. diff --git a/crates/geom/src/simplify.rs b/crates/geom/src/simplify.rs new file mode 100644 index 000000000..e768f303a --- /dev/null +++ b/crates/geom/src/simplify.rs @@ -0,0 +1,140 @@ +//! Path simplification that keeps to its input where kurbo's fit would shoot past it. + +use kurbo::{BezPath, ParamCurve, ParamCurveNearest, PathEl, PathSeg, Point, Vec2}; + +/// How far, as a multiple of the accuracy asked for, a fitted run may stray from the run it +/// replaces before the run is kept as it was. kurbo's own fits stay within about the accuracy; its +/// failures stray by tens to hundreds of units, so twice the accuracy tells them apart without +/// undoing fits that are merely a little over. +const STRAY: f64 = 2.0; + +/// `path` simplified to within `accuracy` by kurbo's [`simplify_bezpath`], kept from straying +/// far from `path` where kurbo's fit does. +/// +/// Two inputs made kurbo's fit shoot far past the path it simplified (#1741, a Pencil stroke with +/// a spike off it), and both are common in pointer samples: +/// - A join where the path turns straight back (samples going down and back up the same line): +/// kurbo keeps two segments in one smooth fit when their tangents are parallel whichever way +/// they point (`|cross| <= |dot| * angle_thresh`), so it fits a smooth curve through the cusp. +/// The path is cut where it turns back by more than a right angle, each run between the cuts is +/// simplified on its own, and the runs are joined again, so the turn stays a sharp corner. Any +/// other join that isn't straight within kurbo's threshold is a corner to kurbo already, so the +/// cut changes nothing there. +/// - A run of nearly collinear segments: kurbo measures its error across the path, not along it, so +/// a fitted cubic whose handle runs hundreds of units on along the line passes its check. Each +/// fitted run is measured against the run it replaces, and one that strays more than +/// twice `accuracy` from it is kept as it was instead. +/// +/// Subpaths and closes are kept, and a closing segment stays implicit, as kurbo leaves it. +/// +/// [`simplify_bezpath`]: kurbo::simplify::simplify_bezpath +pub fn simplify_path(path: &BezPath, accuracy: f64) -> BezPath { + let mut out = BezPath::new(); + let mut run = BezPath::new(); + // Where the current subpath started, where the pen is, and the last segment's end tangent. + let mut start = Point::ZERO; + let mut at = Point::ZERO; + let mut last_tan: Option = None; + // Whether the next run that draws anything has to start the subpath with its move. + let mut fresh = true; + for el in path.elements() { + let seg = match *el { + PathEl::MoveTo(p) => { + flush(&mut out, &mut run, accuracy, &mut fresh); + (start, at, last_tan, fresh) = (p, p, None, true); + run.move_to(p); + continue; + } + PathEl::ClosePath => { + flush(&mut out, &mut run, accuracy, &mut fresh); + if !fresh { + out.close_path(); + } + // A segment after a close starts a new subpath at the closed one's start. + (at, last_tan, fresh) = (start, None, true); + run.move_to(start); + continue; + } + PathEl::LineTo(p) => PathSeg::Line(kurbo::Line::new(at, p)), + PathEl::QuadTo(p1, p2) => PathSeg::Quad(kurbo::QuadBez::new(at, p1, p2)), + PathEl::CurveTo(p1, p2, p3) => PathSeg::Cubic(kurbo::CubicBez::new(at, p1, p2, p3)), + }; + if run.elements().is_empty() { + run.move_to(at); + } + let Some((t0, t1)) = tangents(seg) else { continue }; + if last_tan.is_some_and(|last| last.dot(t0) < 0.0) { + flush(&mut out, &mut run, accuracy, &mut fresh); + run.move_to(at); + } + run.push(*el); + at = seg.end(); + last_tan = Some(t1); + } + flush(&mut out, &mut run, accuracy, &mut fresh); + out +} + +/// Simplify `run` (one move and the segments after it) with kurbo and append it to `out`; the +/// run's move is kept only when it starts a subpath (`fresh`), else the run carries on from the +/// last point drawn. `run` is left empty. +fn flush(out: &mut BezPath, run: &mut BezPath, accuracy: f64, fresh: &mut bool) { + if run.elements().len() > 1 { + let fitted = kurbo::simplify::simplify_bezpath(run.iter(), accuracy, &Default::default()); + let fitted = if keeps_to(&fitted, run, STRAY * accuracy) { fitted } else { std::mem::take(run) }; + for el in fitted.elements() { + match el { + PathEl::MoveTo(_) if !*fresh => {} + _ => { + out.push(*el); + *fresh = false; + } + } + } + } + *run = BezPath::new(); +} + +/// Whether every point of `fitted` lies within `tol` of `source`: one subpath each, from the same +/// start to the same end, `fitted` being kurbo's simplification of `source`. +/// +/// kurbo ends each fitted segment on the source, so the two are walked together: for each fitted +/// segment, the source segments from where it starts to the first one its end lies on are the only +/// ones its sample points are measured against. The work is linear in the two lengths. A fitted end +/// found on no source segment counts as straying. +fn keeps_to(fitted: &BezPath, source: &BezPath, tol: f64) -> bool { + /// Parts each fitted segment is cut into: the points between them are the ones measured. + const SAMPLES: u32 = 16; + let src: Vec = source.segments().collect(); + // kurbo puts each end on the source by evaluating it: off by rounding at most. + let on2 = (tol * 1e-3).powi(2); + let (tol2, mut cur) = (tol * tol, 0); + for seg in fitted.segments() { + let Some(last) = (cur..src.len()).find(|&i| src.get(i).is_some_and(|s| s.nearest(seg.end(), 1e-9).distance_sq <= on2)) else { + return false; + }; + let near = src.get(cur..=last).unwrap_or_default(); + for i in 1..SAMPLES { + let p = seg.eval(f64::from(i) / f64::from(SAMPLES)); + if !near.iter().any(|s| s.nearest(p, 1e-6).distance_sq <= tol2) { + return false; + } + } + cur = last; + } + true +} + +/// The directions `seg` leaves its start and reaches its end, skipping control points that sit on +/// an end; `None` for a segment of no length, which kurbo skips too. +fn tangents(seg: PathSeg) -> Option<(Vec2, Vec2)> { + let pts: Vec = match seg { + PathSeg::Line(l) => vec![l.p0, l.p1], + PathSeg::Quad(q) => vec![q.p0, q.p1, q.p2], + PathSeg::Cubic(c) => vec![c.p0, c.p1, c.p2, c.p3], + }; + let (first, last) = (*pts.first()?, *pts.last()?); + let t0 = pts.iter().map(|p| *p - first).find(|d| d.hypot2() > 0.0)?; + let t1 = pts.iter().rev().map(|p| last - *p).find(|d| d.hypot2() > 0.0)?; + Some((t0, t1)) +} diff --git a/crates/geom/src/tests.rs b/crates/geom/src/tests.rs index 459b2b101..10e09318c 100644 --- a/crates/geom/src/tests.rs +++ b/crates/geom/src/tests.rs @@ -366,3 +366,120 @@ fn svg_path_refuses_numbers_past_the_limit_before_parsing() { assert!(number_past("M0 0L-2.5.5 1e-3-4e3z", 1e3)); assert!(number_past("M.5.5e10", 1e3)); } + +/// A path through `pts` with straight lines (pencil samples). +fn polyline(pts: &[(f64, f64)]) -> BezPath { + let mut p = BezPath::new(); + for (k, (x, y)) in pts.iter().enumerate() { + if k == 0 { p.move_to((*x, *y)) } else { p.line_to((*x, *y)) } + } + p +} + +/// How far `out` strays from `input`: the largest distance from a point sampled along `out` to the +/// nearest segment of `input` (closing segments included). +fn deviation(out: &BezPath, input: &BezPath) -> f64 { + use kurbo::{ParamCurve, ParamCurveNearest}; + let segs: Vec = input.segments().collect(); + out.segments() + .flat_map(|seg| (0..=32).map(move |i| seg.eval(f64::from(i) / 32.0))) + .map(|p| segs.iter().map(|s| s.nearest(p, 1e-6).distance_sq).fold(f64::INFINITY, f64::min).sqrt()) + .fold(0.0, f64::max) +} + +/// Pencil strokes kurbo's fit shot far past keep to their samples (#1741): a stroke that goes out +/// and back to the very point it left, nearly collinear samples, and strokes that turn back along +/// their own line. The first two are from fuzzing pencil-like strokes, where kurbo's curve went +/// about 390 and 160 units off; at whole-pixel coordinates the same shapes happened to fit well. +#[test] +fn simplifying_keeps_to_strokes_kurbo_shot_past() { + use kurbo::ParamCurve; + let cases: [(&str, &[(f64, f64)]); 5] = [ + ( + "out and back to the same point", + &[ + (86.043_565_719_086_97, -173.744_402_349_100_94), + (87.851_850_739_335_35, -179.202_846_445_954_52), + (86.043_565_719_086_97, -173.744_402_349_100_94), + ], + ), + ( + "nearly collinear", + &[ + (209.598_690_794_015_5, -204.909_328_119_739_88), + (211.277_914_263_870_97, -207.190_974_020_753_4), + (214.246_506_828_720_84, -211.228_827_590_623_24), + ], + ), + ( + "a dip straight down and back up", + &[(0.0, 0.0), (10.0, 0.0), (20.0, 2.0), (30.0, 6.0), (30.0, 12.0), (30.0, 6.0), (40.0, 2.0), (50.0, 0.0)], + ), + ("a slip back at the end", &[(0.0, 0.0), (10.0, 3.0), (20.0, 8.0), (30.0, 15.0), (30.0, 25.0), (30.0, 22.0)]), + ( + "a near reversal in a curve", + &[(0.0, 0.0), (8.0, 2.0), (16.0, 6.0), (24.0, 12.0), (32.0, 20.0), (24.0, 12.000_01), (16.0, 6.0), (8.0, 2.0)], + ), + ]; + for (what, pts) in cases { + let input = polyline(pts); + let out = simplify_path(&input, 0.75); + let d = deviation(&out, &input); + assert!(d <= 1.5, "{what}: strays {d} from the samples: {}", out.to_svg()); + let (inb, outb) = (input.bounding_box(), out.bounding_box()); + assert!(inb.inflate(1.5, 1.5).contains_rect(outb), "{what}: {outb:?} past {inb:?}"); + // One subpath, starting and ending where the stroke does. + let moves = out.elements().iter().filter(|e| matches!(e, kurbo::PathEl::MoveTo(_))).count(); + assert_eq!(moves, 1, "{what}: {}", out.to_svg()); + let (first, last) = (pts[0], pts[pts.len() - 1]); + assert_eq!(out.segments().next().map(|s| s.start()), Some(Point::new(first.0, first.1)), "{what}"); + assert_eq!(out.segments().last().map(|s| s.end()), Some(Point::new(last.0, last.1)), "{what}"); + } +} + +/// Where a path never turns back and kurbo's fit holds, the result is kurbo's own, element for +/// element: smooth runs, corners sharper than a right angle, closed subpaths and several subpaths. +#[test] +fn simplifying_is_kurbos_where_its_fit_holds() { + let wave: Vec<(f64, f64)> = (0..=60).map(|i| (f64::from(i) * 4.0, (f64::from(i) / 6.0).sin() * 30.0)).collect(); + let zigzag: Vec<(f64, f64)> = (0..=8).map(|i| (f64::from(i) * 10.0, if i % 2 == 0 { 0.0 } else { 40.0 })).collect(); + let mut closed = polyline(&wave[..20]); + closed.close_path(); + let mut several = polyline(&wave); + several.extend(polyline(&zigzag).elements().iter().map(|e| match *e { + kurbo::PathEl::MoveTo(p) => kurbo::PathEl::MoveTo(p + Vec2::new(0.0, 100.0)), + kurbo::PathEl::LineTo(p) => kurbo::PathEl::LineTo(p + Vec2::new(0.0, 100.0)), + e => e, + })); + let curves = BezPath::from_svg("M0 0 C10 -20 30 -20 40 0 C50 20 70 20 80 0 Q100 -30 120 0 L140 10 Z M200 0 L240 0").unwrap(); + for (what, input) in + [("a wave", polyline(&wave)), ("a zigzag", polyline(&zigzag)), ("closed", closed), ("two subpaths", several), ("curves", curves)] + { + let ours = simplify_path(&input, 0.75); + let theirs = kurbo::simplify::simplify_bezpath(input.iter(), 0.75, &Default::default()); + assert_eq!(ours.elements(), theirs.elements(), "{what}"); + } +} + +/// A closed path with a reversal in it stays one closed subpath, and each subpath of several keeps +/// its own start. +#[test] +fn simplifying_keeps_closes_and_subpaths_across_a_reversal() { + let mut input = polyline(&[(0.0, 0.0), (40.0, 0.0), (40.0, 30.0), (40.0, 20.0), (0.0, 30.0)]); + input.close_path(); + input.extend(polyline(&[(100.0, 0.0), (100.0, 20.0), (100.0, 10.0)]).elements().iter().copied()); + let out = simplify_path(&input, 0.75); + let els = out.elements(); + let moves: Vec = els.iter().filter_map(|e| if let kurbo::PathEl::MoveTo(p) = e { Some(*p) } else { None }).collect(); + assert_eq!(moves, [Point::new(0.0, 0.0), Point::new(100.0, 0.0)], "{}", out.to_svg()); + assert_eq!(els.iter().filter(|e| matches!(e, kurbo::PathEl::ClosePath)).count(), 1, "{}", out.to_svg()); + let close = els.iter().position(|e| matches!(e, kurbo::PathEl::ClosePath)).unwrap(); + assert!(matches!(els[close + 1], kurbo::PathEl::MoveTo(_)), "the first subpath closes before the second starts"); + assert!(deviation(&out, &input) <= 1.5, "{}", out.to_svg()); + // Nothing to draw: nothing out, not a stray move or close. + let mut empty = BezPath::new(); + empty.move_to((5.0, 5.0)); + empty.line_to((5.0, 5.0)); + empty.close_path(); + assert!(simplify_path(&empty, 0.75).elements().is_empty()); +} From 1ee3087bc97fc1476a4026ab53fef32c851cbc05 Mon Sep 17 00:00:00 2001 From: Trevor Brown Date: Mon, 9 Nov 2026 08:46:12 -0500 Subject: [PATCH 10/23] M12: a sticky's text keeps its size and the note grows taller (#2157) FigJam's sticky text is 18 (Small) and never changes as the note fills; the note keeps its width and grows taller (measured in the web app, 2026-11-09). A new sticky's text is STICKY_TEXT_SIZE (18) and the normalize pass no longer resets a sticky's text size by its length, so saved stickies keep the size they have. No size menu yet. Co-Authored-By: Claude Sonnet 5.5 Model-Effort: Opus 5.5 drove and briefed; Sonnet 5.5 measured in Figma and wrote at medium --- crates/doc/src/board.rs | 4 +- crates/engine/src/board_tests.rs | 71 ++++++++++++++++++++--- crates/engine/src/boards/board.rs | 2 +- crates/engine/src/boards/board/derived.rs | 19 +----- crates/engine/src/boards/board/items.rs | 17 ++---- docs/boards-parity.md | 5 +- 6 files changed, 76 insertions(+), 42 deletions(-) diff --git a/crates/doc/src/board.rs b/crates/doc/src/board.rs index 89413b2e9..c0fcd7c68 100644 --- a/crates/doc/src/board.rs +++ b/crates/doc/src/board.rs @@ -37,8 +37,8 @@ impl FileKind { #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] #[serde(tag = "type", rename_all = "camelCase")] pub enum BoardItem { - /// A sticky note: a frame whose first text layer is the note (its size shrinks as the note - /// grows) and, below it, who wrote it. + /// A sticky note: a frame whose first text layer is the note (its size stays fixed, the note + /// grows taller) and, below it, who wrote it. Sticky { #[serde(default, skip_serializing_if = "String::is_empty")] author: String, diff --git a/crates/engine/src/board_tests.rs b/crates/engine/src/board_tests.rs index 9e99bd1c9..1b0ac120c 100644 --- a/crates/engine/src/board_tests.rs +++ b/crates/engine/src/board_tests.rs @@ -7,7 +7,7 @@ use figcraft_doc::{ use figcraft_geom::Point; use serde_json::{Value, json}; -use crate::board::{route, sticky_font_size}; +use crate::board::{STICKY_TEXT_SIZE, route}; use crate::collab_live_tests::Room; use crate::tools::{PointerEvent, PointerKind, ToolId}; use crate::{EngineError, Session}; @@ -77,7 +77,7 @@ fn board_commands_refuse_on_design_files_and_prototyping_refuses_on_boards() { } #[test] -fn a_sticky_is_a_square_frame_whose_note_shrinks_as_it_grows() { +fn a_sticky_is_a_square_frame_whose_note_grows_taller_as_it_fills() { let mut s = board(); s.prefs.user_name = "Ada".into(); let r = s.execute("board.sticky", &json!({"x": 10, "y": 20, "text": "Hi", "color": "blue"})).unwrap(); @@ -88,19 +88,74 @@ fn a_sticky_is_a_square_frame_whose_note_shrinks_as_it_grows() { assert_eq!(n.fills[0].solid_color(), board_color("blue")); assert!(n.layout.is_some() && !n.effects.is_empty()); assert_eq!(n.children.len(), 2, "the note and its author"); - assert_eq!(node(&s, note).kind.text().unwrap().style.font_size, 24.0); + assert_eq!(node(&s, note).kind.text().unwrap().style.font_size, 18.0); // A click on the note picks the sticky whole. let doc = s.doc().unwrap(); assert_eq!(figcraft_doc::geometry::select_target(doc, note, &[], false), st); - // A long note takes a smaller size; a very long one grows the sticky past its square. + // A long note keeps the size; a very long one grows the sticky past its square. let long = "word ".repeat(120); s.execute("node.set", &json!({"id": note, "props": {"characters": long}})).unwrap(); - assert_eq!(node(&s, note).kind.text().unwrap().style.font_size, sticky_font_size(&long)); - assert!(sticky_font_size(&long) < 16.0); + assert_eq!(node(&s, note).kind.text().unwrap().style.font_size, STICKY_TEXT_SIZE); assert!(node(&s, st).width == 200.0 && node(&s, st).height >= 200.0); - // Undo puts the short note and its size back. + // Undo puts the short note back. s.execute("edit.undo", &json!({})).unwrap(); - assert_eq!(node(&s, note).kind.text().unwrap().style.font_size, 24.0); + assert_eq!(node(&s, note).kind.text().unwrap().style.font_size, 18.0); +} + +#[test] +fn a_new_sticky_starts_at_18() { + let mut s = board(); + for (cmd, p) in [("board.sticky", json!({"x": 0, "y": 0, "text": "Hi"})), ("board.sticky", json!({"x": 300, "y": 0}))] { + let note = id(&s.execute(cmd, &p).unwrap(), "text"); + assert_eq!(node(&s, note).kind.text().unwrap().style.font_size, 18.0); + } + let next = id(&s.execute("board.nextSticky", &json!({})).unwrap(), "text"); + assert_eq!(node(&s, next).kind.text().unwrap().style.font_size, 18.0); + assert_eq!(STICKY_TEXT_SIZE, 18.0); +} + +#[test] +fn a_sticky_keeps_its_text_size_as_text_is_added() { + let mut s = board(); + let r = s.execute("board.sticky", &json!({"x": 0, "y": 0, "text": "one"})).unwrap(); + let (st, note) = (id(&r, "id"), id(&r, "text")); + let (height, text_height) = (node(&s, st).height, node(&s, note).height); + // Typing in the note: each step is a real edit that runs the normalize pass. + crate::board::edit_text(&mut s, st).unwrap(); + for _ in 0..9 { + crate::textedit::key(&mut s, "Enter", false, false, false).unwrap(); + crate::textedit::insert(&mut s, "another line").unwrap(); + } + let ten = node(&s, note).kind.text().unwrap().clone(); + assert_eq!(ten.characters.matches('\n').count(), 9, "ten lines"); + assert_eq!(ten.style.font_size, 18.0); + // Growth needs text measured: a build without the craft-fonts input measures nothing (the same + // reason `a_double_click_types_in_the_cell...` fails there), so it is only checked where it is. + let measured = node(&s, note).height > text_height; + assert!(!measured || node(&s, st).height > height, "ten lines make the note taller"); + assert_eq!(node(&s, st).child_layout.sizing_v, figcraft_doc::Sizing::Hug, "the note hugs its text"); + let (w, h10) = (node(&s, st).width, node(&s, st).height); + crate::textedit::insert(&mut s, &"a long paragraph ".repeat(15)).unwrap(); + crate::textedit::end(&mut s).unwrap(); + let t = node(&s, note).kind.text().unwrap().clone(); + assert!(t.characters.chars().count() >= 250); + assert_eq!(t.style.font_size, 18.0); + assert!(t.runs.iter().all(|r| r.style.font_size == 18.0)); + assert_eq!(node(&s, st).width, w, "the note keeps its width"); + assert!(!measured || node(&s, st).height > h10, "and grows taller"); +} + +#[test] +fn a_sticky_saved_with_another_text_size_keeps_it() { + let mut s = board(); + let r = s.execute("board.sticky", &json!({"x": 0, "y": 0, "text": "old"})).unwrap(); + let (st, note) = (id(&r, "id"), id(&r, "text")); + s.execute("node.set", &json!({"id": note, "props": {"fontSize": 11}})).unwrap(); + assert_eq!(node(&s, note).kind.text().unwrap().style.font_size, 11.0); + s.execute("node.set", &json!({"id": note, "props": {"characters": "old and now much longer than it was"}})).unwrap(); + assert_eq!(node(&s, note).kind.text().unwrap().style.font_size, 11.0, "an edit"); + s.execute("node.set", &json!({"id": st, "props": {"x": 40}})).unwrap(); + assert_eq!(node(&s, note).kind.text().unwrap().style.font_size, 11.0, "a move"); } #[test] diff --git a/crates/engine/src/boards/board.rs b/crates/engine/src/boards/board.rs index 1a66eed22..158e679d4 100644 --- a/crates/engine/src/boards/board.rs +++ b/crates/engine/src/boards/board.rs @@ -68,8 +68,8 @@ pub(crate) use edit::{copied_items, copy_map, rejoin}; use edit::{label_cmd, set_color, set_style}; pub(crate) use items::add_text; pub use items::{CONNECTED_GAP, connected_place}; +pub use items::{STICKY_TEXT_SIZE, edit_text, owner_of_text, text_of}; use items::{add_connected, ink_cmd, next_sticky, shape_cmd, stamp_cmd, sticky_cmd}; -pub use items::{edit_text, owner_of_text, sticky_font_size, text_of}; pub(crate) use reshape::path_handle_at; pub use reshape::selected_connector_handles; diff --git a/crates/engine/src/boards/board/derived.rs b/crates/engine/src/boards/board/derived.rs index 9fae17206..37a2f9ba9 100644 --- a/crates/engine/src/boards/board/derived.rs +++ b/crates/engine/src/boards/board/derived.rs @@ -1,7 +1,6 @@ use figcraft_doc::RouteEnd; -use super::edit::set_font_size; -use super::items::{owner_of_text, shape_padding, sticky_font_size, text_of}; +use super::items::shape_padding; use super::{ Affine, BTreeSet, BezPath, BoardItem, Connector, Document, End, NodeId, NodeKind, PathEl, Point, Rect, Routing, Shape, VectorNetwork, is_board, }; @@ -9,26 +8,14 @@ use super::{ // --------------------------------------------------------------------------------------------- // Derived data (the normalize pass). -/// Before text is measured: a sticky's note takes the size its length calls for, and a resized -/// shape keeps its text inside its outline. +/// Before text is measured: a resized shape keeps its text inside its outline. A sticky's text +/// keeps whatever size it has (FigJam's size is the note's own setting). pub(crate) fn settle_text(doc: &mut Document, before: &Document, touched: &BTreeSet) { if !is_board(doc) { return; } for &id in touched { let Some(n) = doc.node(id) else { continue }; - if let Some(owner) = - owner_of_text(doc, id).or(Some(id)).filter(|o| doc.node(*o).is_some_and(|k| matches!(k.board, Some(BoardItem::Sticky { .. })))) - && let Some(t) = text_of(doc, owner) - && let Some(NodeKind::Text(td)) = doc.node(t).map(|k| &k.kind) - { - let size = sticky_font_size(&td.characters); - let off = td.style.font_size != size || td.runs.iter().any(|r| r.style.font_size != size); - if off && let Some(NodeKind::Text(td)) = doc.node_mut(t).map(|k| &mut k.kind) { - set_font_size(td, size); - } - continue; - } if let Some(BoardItem::Shape { shape }) = n.board && before.node(id).is_none_or(|b| (b.width, b.height) != (n.width, n.height)) { diff --git a/crates/engine/src/boards/board/items.rs b/crates/engine/src/boards/board/items.rs index b409eeb90..115ce6d3b 100644 --- a/crates/engine/src/boards/board/items.rs +++ b/crates/engine/src/boards/board/items.rs @@ -57,18 +57,9 @@ pub(crate) fn add_text(doc: &mut Document, parent: NodeId, text: &str, size: f64 Ok(id) } -/// The text size a sticky's note gets: smaller the more it holds (by characters and lines, so -/// every copy of the file picks the same size whatever fonts it has). -pub fn sticky_font_size(text: &str) -> f64 { - let weight = text.chars().count() + 24 * text.matches('\n').count(); - match weight { - 0..=24 => 24.0, - 25..=64 => 20.0, - 65..=140 => 16.0, - 141..=280 => 13.0, - _ => 11.0, - } -} +/// The text size of a new sticky's note, FigJam's "Small" default (measured in the web app, +/// 2026-11-09): it never changes as the note fills, the note grows taller instead. +pub const STICKY_TEXT_SIZE: f64 = 18.0; /// A sticky note's layers: the frame, the note and (when known) its author. fn make_sticky(doc: &mut Document, parent: NodeId, at: (f64, f64), text: &str, color: Color, author: &str) -> Result<(NodeId, NodeId)> { @@ -89,7 +80,7 @@ fn make_sticky(doc: &mut Document, parent: NodeId, at: (f64, f64), text: &str, c n.board = Some(BoardItem::Sticky { author: author.to_string() }); })?; let inner = STICKY - 2.0 * STICKY_PAD; - let note = add_text(doc, id, text, sticky_font_size(text), INK, TextAlignH::Left, inner)?; + let note = add_text(doc, id, text, STICKY_TEXT_SIZE, INK, TextAlignH::Left, inner)?; if !author.is_empty() { let a = add_text(doc, id, author, 10.0, AUTHOR_INK, TextAlignH::Left, inner)?; if let Some(n) = doc.node_mut(a) { diff --git a/docs/boards-parity.md b/docs/boards-parity.md index 16456492c..2cc099f08 100644 --- a/docs/boards-parity.md +++ b/docs/boards-parity.md @@ -1,6 +1,6 @@ # Boards parity -> **Last reviewed:** 2026-10-24 · **Last updated:** 2026-11-04 · **Change:** minor (a `.jam` elbowed connector saved with its path keeps it on import instead of being routed again (#1910); a "+" handle dragged carries the preview: the next shape is made where it is let go, or a connector joins the object it is let go on (#1919); a connector attached to a side starts 4 pt outside the border, so its stroke never lies on the fill (#1916); a connector end moved with Cmd/Ctrl held goes on any part of an object, and stays there as the object moves or resizes; a connector end dropped along a side of a shape attaches to that side (#1708); a board connector's path can be reshaped by dragging its path handles (#1710); boards have 16 more shapes (arrows, polygons, star, plus and flowchart symbols), in pickers that lay them out in a grid; a board keeps earlier votes' results, listed newest first in Results and deleted one at a time; a connector's bar sets its start and end points, dashes, weight and any colour (#1596); cells copied from a spreadsheet paste onto a board as a table; Shift+Cmd/Ctrl+Enter on a selected board shape adds the next shape already joined to it, and goes on typing (#1597); "+" handles beside a selected sticky or shape make the next one, joined by a connector (#1597); a FigJam `.jam` file opens as a board (#1342); board grid settings: dots, lines, graph paper or none, spacing, size and contrast; the connector tool shows the connector it will make while dragging, ends snapping to attach points (#1387); templates and tables; facilitation: a shared timer and voting sessions; the dot grid, dragging a connector's end to re-attach it, curved connectors, elbows that go around what they join) · **Target:** the incumbent's whiteboard product (behaviour from its public help centre) +> **Last reviewed:** 2026-10-24 · **Last updated:** 2026-11-09 · **Change:** minor (a sticky's text stays 18 and the note grows taller instead of the text shrinking, as measured in FigJam's web app (#2157); a `.jam` elbowed connector saved with its path keeps it on import instead of being routed again (#1910); a "+" handle dragged carries the preview: the next shape is made where it is let go, or a connector joins the object it is let go on (#1919); a connector attached to a side starts 4 pt outside the border, so its stroke never lies on the fill (#1916); a connector end moved with Cmd/Ctrl held goes on any part of an object, and stays there as the object moves or resizes; a connector end dropped along a side of a shape attaches to that side (#1708); a board connector's path can be reshaped by dragging its path handles (#1710); boards have 16 more shapes (arrows, polygons, star, plus and flowchart symbols), in pickers that lay them out in a grid; a board keeps earlier votes' results, listed newest first in Results and deleted one at a time; a connector's bar sets its start and end points, dashes, weight and any colour (#1596); cells copied from a spreadsheet paste onto a board as a table; Shift+Cmd/Ctrl+Enter on a selected board shape adds the next shape already joined to it, and goes on typing (#1597); "+" handles beside a selected sticky or shape make the next one, joined by a connector (#1597); a FigJam `.jam` file opens as a board (#1342); board grid settings: dots, lines, graph paper or none, spacing, size and contrast; the connector tool shows the connector it will make while dragging, ends snapping to attach points (#1387); templates and tables; facilitation: a shared timer and voting sessions; the dot grid, dragging a connector's end to re-attach it, curved connectors, elbows that go around what they join) · **Target:** the incumbent's whiteboard product (behaviour from its public help centre) **Breadth ~47%, ready ~29% (estimated, erring low).** A board is a FigCraft file of its own kind (`file.newBoard`) with a dot grid on the page, sticky notes, shapes with text, connectors that @@ -30,7 +30,7 @@ all end with the same geometry. | Board toolbar at the bottom | done | Move/Hand, Marker/Highlighter/Eraser, Sticky (colour flyout), Shapes (flyout), Connector, Text, Section, Stamp (flyout), Comment, Actions, then the templates and the Design panel's buttons — on the same 32 pt grid as a design file's (no Resources button, as there) | | Design panel hidden by default; a contextual bar above the selection | done | colour, shape, text size (S/M/L/XL); for connectors straight/elbow/curved, the start and end points (none, round, square, line arrow, triangle arrow, reversed triangle, circle, diamond), solid or dashed, the weight, and any colour in the colour picker beside the board colours (#1596); each control says what it is on hover; the Design panel opens with its button or View ▸ Design panel | | Prototyping and Developer Mode refuse on boards | done | `proto.*`, `dev.*`, `view.prototype`, `view.devMode` say why | -| Sticky notes | done | a square frame (200 wide, growing taller through auto layout), 8 soft colours, a soft shadow, the note's text size shrinking as it grows (24 → 11, by characters and lines), the author's name at the bottom; a click places one and starts typing; Enter or a double-click edits; Shift+Cmd/Ctrl+Enter adds the next one beside it | +| Sticky notes | done | a square frame (200 wide, growing taller through auto layout), 8 soft colours, a soft shadow, the note's text 18 by default and fixed (FigJam's "Small"): the note grows taller as it fills and never shrinks its text, measured in FigJam's web app, 2026-11-09; stickies saved with another size keep it, and there is no size menu yet (#2157), the author's name at the bottom; a click places one and starts typing; Enter or a double-click edits; Shift+Cmd/Ctrl+Enter adds the next one beside it | | Sticky options: hide the author, change size presets, sticky stacks | partial | `board.sticky {author: false}`; no size presets or stacks | | Shapes with text | done | rectangle, rounded rectangle, ellipse, diamond, triangle, speech bubble, chevron; text centred and wrapping inside the outline; shape type and text size from the contextual bar | | Shape auto-fit to its text, more shapes (cylinder, cross, star…), shape libraries | missing | | @@ -72,6 +72,7 @@ all end with the same geometry. | Date | Change | Summary | |---|---|---| +| 2026-11-09 | patch | M12, #2157: a sticky's text is 18 and stays 18 (measured in FigJam's web app: ten lines, then a long paragraph, never changed it; the note kept its width and grew taller). The normalize pass no longer resets a sticky's text to a size by its length (24 down to 11 before), so stickies in saved files keep their size. Size presets (Small to Huge) are still missing. Tests: `a_new_sticky_starts_at_18`, `a_sticky_keeps_its_text_size_as_text_is_added`, `a_sticky_saved_with_another_text_size_keeps_it` | | 2026-11-03 | minor | M12, #1919: as the quick create help says ("Click to select and create the next shape in that direction, or drag the preview to any location on your board"; "A new shape is created with a connector and its text field active"), a "+" beside the selected sticky or shape can be dragged: the preview follows the pointer and, let go on empty board, the next one is made there, centred on the pointer, and joined by a connector leaving by the handle's side and arriving by the side facing it (`board.addConnected` gains `at: [x, y]`, a point of the page placed in the parent's coordinates); let go on another sticky or shape, inside a section or frame too (`board::sticky_or_shape_at`), a connector joins the two instead (`board.connector`), that object outlined while the pointer is over it; let go off the canvas or over the toolbar, nothing, and the bar above the selection stays away meanwhile, as for any drag. The click is as before. A shape picker on drop and choosing the connector type are not done (the help doesn't describe them). Tests: `board_connect_tests` (the drag ones), engine `board_connected_tests::the_next_one_goes_where_it_is_dropped`. Numbers unchanged. | | 2026-11-04 | minor | M12, #1916: a connector end attached to a side starts 4 pt outside the border (`board_route::END_GAP`), as FigJam's do (the issue's screenshots: the line starts a few pixels outside, with a round cap), so the stroke never lies on the fill; elbows leaving one side share their first run along its normal, curves leave tangent to it and straight ones fan out from the same point; the `.jam` importer's routes, re-attached ends, path handles and labels follow the route's points; an end placed by hand with Cmd/Ctrl held keeps its exact point. Tests: `board_route_tests` (three). Numbers unchanged. | | 2026-11-04 | patch | M6, #1910: a `.jam` elbowed connector saved with its path (FigJam's `connectorControlPoints`) keeps it on import: the points become the `ConnectorPath::Runs` a connector reshaped by hand has and the line is walked along them, so a connector drawn around a layer stays around it; one whose points don't make a path from its ends' sides is routed as before. Still a vector, not a native board object. Details in [`file-format-parity.md`](file-format-parity.md). | From ff3e4e6010b905bf7dc09c507a2078426efa9d37 Mon Sep 17 00:00:00 2001 From: Trevor Brown Date: Mon, 9 Nov 2026 09:38:56 -0500 Subject: [PATCH 11/23] M12: sticky growth checked with a measured font; the normalize note drops sticky size (#2164 review) The growth test scans the system fonts, as the other text-measuring engine tests do (#333), and asserts the note grows taller unconditionally instead of only when text happened to measure. docs/boards-parity.md no longer lists a sticky's text size among what the normalize pass derives. Co-Authored-By: Claude Opus 5.5 Model-Effort: Opus 5.5 checked the findings and wrote --- crates/engine/src/board_tests.rs | 11 ++++++----- docs/boards-parity.md | 4 ++-- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/crates/engine/src/board_tests.rs b/crates/engine/src/board_tests.rs index 1b0ac120c..eb00647ac 100644 --- a/crates/engine/src/board_tests.rs +++ b/crates/engine/src/board_tests.rs @@ -116,6 +116,9 @@ fn a_new_sticky_starts_at_18() { #[test] fn a_sticky_keeps_its_text_size_as_text_is_added() { + // The note's growth needs text measured: without craft-fonts that is with a system font, + // scanned here rather than by whichever test happens to run first (#333). + figcraft_text::scan_system_fonts(); let mut s = board(); let r = s.execute("board.sticky", &json!({"x": 0, "y": 0, "text": "one"})).unwrap(); let (st, note) = (id(&r, "id"), id(&r, "text")); @@ -129,10 +132,8 @@ fn a_sticky_keeps_its_text_size_as_text_is_added() { let ten = node(&s, note).kind.text().unwrap().clone(); assert_eq!(ten.characters.matches('\n').count(), 9, "ten lines"); assert_eq!(ten.style.font_size, 18.0); - // Growth needs text measured: a build without the craft-fonts input measures nothing (the same - // reason `a_double_click_types_in_the_cell...` fails there), so it is only checked where it is. - let measured = node(&s, note).height > text_height; - assert!(!measured || node(&s, st).height > height, "ten lines make the note taller"); + assert!(node(&s, note).height > text_height, "ten lines make the text taller"); + assert!(node(&s, st).height > height, "and the note"); assert_eq!(node(&s, st).child_layout.sizing_v, figcraft_doc::Sizing::Hug, "the note hugs its text"); let (w, h10) = (node(&s, st).width, node(&s, st).height); crate::textedit::insert(&mut s, &"a long paragraph ".repeat(15)).unwrap(); @@ -142,7 +143,7 @@ fn a_sticky_keeps_its_text_size_as_text_is_added() { assert_eq!(t.style.font_size, 18.0); assert!(t.runs.iter().all(|r| r.style.font_size == 18.0)); assert_eq!(node(&s, st).width, w, "the note keeps its width"); - assert!(!measured || node(&s, st).height > h10, "and grows taller"); + assert!(node(&s, st).height > h10, "and grows taller"); } #[test] diff --git a/docs/boards-parity.md b/docs/boards-parity.md index 2cc099f08..f874daad5 100644 --- a/docs/boards-parity.md +++ b/docs/boards-parity.md @@ -17,8 +17,8 @@ design product). The work item is gap 24 in [`gaps.md`](gaps.md). Board objects are ordinary layers with a small `board` field on the node (`figcraft_doc::BoardItem`), so rendering, hit testing, undo, saving, SVG/PNG/PDF export, copy and paste and live sessions work on them unchanged. The file kind (`Document.kind`) is saved in `.figcraft` and reaches people who -join a live session in their snapshot. What follows from the metadata (a sticky's text size, a -shape's text room, connector routes and their labels) is recomputed in the engine's normalize pass, +join a live session in their snapshot. What follows from the metadata (a shape's text +room, connector routes and their labels) is recomputed in the engine's normalize pass, deterministically from the document, so local edits, undo and edits from others in a live session all end with the same geometry. From 61389cdd5fc03fe76bea0c80f8bed3d0606c8da1 Mon Sep 17 00:00:00 2001 From: ljkx <177671075+ljkx@users.noreply.github.com> Date: Mon, 9 Nov 2026 21:51:37 +0100 Subject: [PATCH 12/23] M4: test the simplified stroke by how far it strays, at more accuracies and inputs (#1741) Review follow-ups, tests only. - The Pencil test and the `vector.simplify` test measure how far the result strays from the samples (at most 1.5), not only its box. - The Pencil test also draws with the Marker and the Highlighter, each stroke in a fresh session, and takes the layer that stroke made. - The `vector.simplify` test runs on an open stroke, a filled shape, a region with its own fill, and a stroke beside a filled shape. - The geom tests run at accuracies 0.1, 0.75 and 8.0, on quadratic and cubic inputs as well as lines, and check both ways round how far the result and the input are apart. - A path that turns straight back is checked to be kurbo's fit of each side on its own, with a point at the turn. - Empty paths, lone moves and segments after a close are covered. --- crates/engine/src/brush_tests.rs | 72 +++++--- crates/engine/src/vector_behaviour_tests.rs | 63 +++++-- crates/geom/src/tests.rs | 191 ++++++++++++++++++-- 3 files changed, 267 insertions(+), 59 deletions(-) diff --git a/crates/engine/src/brush_tests.rs b/crates/engine/src/brush_tests.rs index 50317496d..065e5eec9 100644 --- a/crates/engine/src/brush_tests.rs +++ b/crates/engine/src/brush_tests.rs @@ -8,6 +8,7 @@ use crate::params::id_of; use crate::tools::{PointerEvent, PointerKind, pressure_profile}; use figcraft_doc::{NodeId, NodeKind}; use figcraft_geom::Point; +use figcraft_geom::kurbo::PathEl; fn id(v: Value) -> NodeId { id_of(&v["id"]).unwrap() @@ -155,10 +156,12 @@ fn the_pencil_draws_white_on_a_dark_background() { /// where samples ran on nearly straight. Pointer pixels become fractional page points, so going /// back to a pixel lands on exactly the same point. Both sample runs are ones kurbo's fit took /// hundreds of units off. +/// +/// Each stroke is drawn in a fresh session, with the Pencil and with the Marker and Highlighter +/// of a board, whose ink comes from the same fit. Every point along the stroke drawn must lie +/// within 1.5 (twice the fit's accuracy) of the polyline through the samples. #[test] fn the_pencil_keeps_its_stroke_where_it_was_drawn() { - let mut s = Session::new(); - s.execute("tool.set", &json!({"tool": "pencil"})).unwrap(); let (p, q) = ((86.043_565_719_086_97, -173.744_402_349_100_94), (87.851_850_739_335_35, -179.202_846_445_954_52)); let back = [(80.0, -160.0), p, q, p, (95.0, -185.0)]; let straight = [ @@ -168,30 +171,47 @@ fn the_pencil_keeps_its_stroke_where_it_was_drawn() { (214.246_506_828_720_84, -211.228_827_590_623_24), (218.0, -214.0), ]; - for (what, samples) in [("out and back", &back[..]), ("nearly straight", &straight[..])] { - let (first, last) = (samples[0], samples[samples.len() - 1]); - s.pointer(&PointerEvent::at(PointerKind::Down, first.0, first.1, 1.0)).unwrap(); - for (x, y) in &samples[1..] { - s.pointer(&PointerEvent::at(PointerKind::Move, *x, *y, 1.0)).unwrap(); + for tool in ["pencil", "marker", "highlighter"] { + for (what, samples) in [("out and back", &back[..]), ("nearly straight", &straight[..])] { + let what = format!("{tool}, {what}"); + let mut s = if tool == "pencil" { Session::new() } else { crate::board_tests::board() }; + s.execute("tool.set", &json!({"tool": tool})).unwrap(); + let before: Vec = s.doc().unwrap().nodes().map(|n| n.id).collect(); + let (first, last) = (samples[0], samples[samples.len() - 1]); + s.pointer(&PointerEvent::at(PointerKind::Down, first.0, first.1, 1.0)).unwrap(); + for (x, y) in &samples[1..] { + s.pointer(&PointerEvent::at(PointerKind::Move, *x, *y, 1.0)).unwrap(); + } + s.pointer(&PointerEvent::at(PointerKind::Up, last.0, last.1, 1.0)).unwrap(); + // The layer the stroke made, and only it. + let doc = s.doc().unwrap(); + let made: Vec<&figcraft_doc::Node> = + doc.nodes().filter(|n| !before.contains(&n.id) && matches!(n.kind, NodeKind::Vector { .. })).collect(); + let [n] = made[..] else { panic!("{what}: {} vectors made", made.len()) }; + let NodeKind::Vector { network } = &n.kind else { panic!("{what}: not a vector") }; + let drawn = doc.world_transform(n.id) * network.stroke_path(); + let polyline = figcraft_geom::BezPath::from_vec( + samples + .iter() + .enumerate() + .map(|(k, (x, y))| if k == 0 { PathEl::MoveTo(Point::new(*x, *y)) } else { PathEl::LineTo(Point::new(*x, *y)) }) + .collect(), + ); + let d = crate::vector_behaviour_tests::strays(&drawn, &polyline); + assert!(d <= 1.5, "{what}: the stroke strays {d} from the samples: {}", drawn.to_svg()); + let samples_box = figcraft_geom::points_bounds(samples.iter().map(|(x, y)| Point::new(*x, *y))).unwrap(); + let node_box = doc.world_bounds(n.id).unwrap(); + if tool == "pencil" { + assert!(samples_box.inflate(1.5, 1.5).contains_rect(node_box), "{what}: the layer {node_box:?}, the samples {samples_box:?}"); + } + // The turn is a point of the stroke, where it was drawn. + if samples == back { + let xf = doc.world_transform(n.id); + assert!( + network.vertices.iter().any(|v| (xf * Point::new(v.x, v.y) - Point::new(q.0, q.1)).hypot() < 1e-6), + "{what}: no point at the turn" + ); + } } - s.pointer(&PointerEvent::at(PointerKind::Up, last.0, last.1, 1.0)).unwrap(); - let doc = s.doc().unwrap(); - let n = doc.nodes().filter(|n| matches!(n.kind, NodeKind::Vector { .. })).max_by_key(|n| n.id).unwrap(); - let NodeKind::Vector { network } = &n.kind else { panic!("{what}: not a vector") }; - let drawn = doc.world_transform(n.id) * network.stroke_path(); - let samples_box = figcraft_geom::points_bounds(samples.iter().map(|(x, y)| Point::new(*x, *y))).unwrap(); - let (path_box, node_box) = (figcraft_geom::Shape::bounding_box(&drawn), doc.world_bounds(n.id).unwrap()); - assert!( - samples_box.inflate(1.5, 1.5).contains_rect(path_box), - "{what}: the stroke {path_box:?} goes past the samples {samples_box:?}: {}", - drawn.to_svg() - ); - assert!(samples_box.inflate(1.5, 1.5).contains_rect(node_box), "{what}: the layer {node_box:?}, the samples {samples_box:?}"); } - // The turn is a point of the stroke, where it was drawn. - let doc = s.doc().unwrap(); - let turned = doc.nodes().find(|n| n.name == "Vector" && doc.world_bounds(n.id).is_some_and(|b| b.y1 < -150.0 && b.x1 < 100.0)).unwrap(); - let NodeKind::Vector { network } = &turned.kind else { panic!("not a vector") }; - let xf = doc.world_transform(turned.id); - assert!(network.vertices.iter().any(|v| (xf * Point::new(v.x, v.y) - Point::new(q.0, q.1)).hypot() < 1e-6), "no point at the turn"); } diff --git a/crates/engine/src/vector_behaviour_tests.rs b/crates/engine/src/vector_behaviour_tests.rs index 4666c3028..d1d6907ee 100644 --- a/crates/engine/src/vector_behaviour_tests.rs +++ b/crates/engine/src/vector_behaviour_tests.rs @@ -326,25 +326,56 @@ fn ex_to_vector(doc: &mut figcraft_doc::Document, id: NodeId) { doc.get_mut(id).unwrap().kind = NodeKind::Vector { network }; } -/// Simplify path keeps nearly collinear points near where they were: kurbo's fit, which Simplify -/// path and the Pencil share, ran a handle hundreds of units on along the line (#1741). +/// How far `path` strays from `from`: the largest distance from a point sampled along `path` to +/// the nearest segment of `from`. +pub(crate) fn strays(path: &BezPath, from: &BezPath) -> f64 { + use figcraft_geom::kurbo::ParamCurveNearest; + let segs: Vec<_> = from.segments().collect(); + path.segments() + .flat_map(|seg| (0..=32).map(move |i| seg.eval(f64::from(i) / 32.0))) + .map(|p| segs.iter().map(|s| s.nearest(p, 1e-6).distance_sq).fold(f64::INFINITY, f64::min).sqrt()) + .fold(0.0, f64::max) +} + +/// Simplify path keeps nearly collinear points, and a stroke that goes out and back, near where +/// they were: kurbo's fit, which Simplify path and the Pencil share, ran a handle hundreds of units +/// on along the line (#1741). That holds for each way the command simplifies a layer: an open +/// stroke, a filled shape, a region with its own fill, and a stroke beside a filled shape. #[test] fn simplify_keeps_nearly_collinear_points_near_them() { - let mut s = Session::new(); - let id = id_of(&ex(&mut s, "vector.create", json!({"path": "M0 0 L10 10"}))["id"]).unwrap(); - // The points exactly as sampled, at the page origin (`vector.create` would move them). - let d = "M209.5986907940155 -204.90932811973988 L211.27791426387097 -207.1909740207534 L214.24650682872084 -211.22882759062324"; - let before = BezPath::from_svg(d).unwrap(); - { - let n = s.doc_mut().unwrap().get_mut(id).unwrap(); - n.transform = figcraft_geom::Affine::IDENTITY; - n.kind = NodeKind::Vector { network: VectorNetwork::from_path(&before) }; + // The points exactly as sampled. + let straight = "M209.5986907940155 -204.90932811973988 L211.27791426387097 -207.1909740207534 L214.24650682872084 -211.22882759062324"; + let back = + "M80 -160 L86.04356571908697 -173.74440234910094 L87.85185073933535 -179.20284644595452 L86.04356571908697 -173.74440234910094 L95 -185"; + let shape = format!("{straight} L260 -150 Z"); + let cases = [ + ("an open stroke", straight.to_string(), false), + ("a filled shape", shape.clone(), false), + ("a region with its own fill", shape.clone(), true), + ("a stroke beside a filled shape", format!("{shape} {back}"), false), + ]; + for (what, d, own_fill) in cases { + let mut s = Session::new(); + let id = id_of(&ex(&mut s, "vector.create", json!({"path": "M0 0 L10 10"}))["id"]).unwrap(); + // At the page origin, as given (`vector.create` would move the points). + let mut network = VectorNetwork::from_path(&BezPath::from_svg(&d).unwrap()); + if own_fill { + network.regions[0].fills = Some(vec![figcraft_color::Paint::solid(figcraft_color::Color::from_hex("#FF0000").unwrap())]); + } + { + let n = s.doc_mut().unwrap().get_mut(id).unwrap(); + n.transform = figcraft_geom::Affine::IDENTITY; + n.kind = NodeKind::Vector { network }; + } + let before = net(&s, id); + ex(&mut s, "vector.simplify", json!({"ids": [id], "tolerance": 0.75})); + let after = s.doc().unwrap().world_transform(id) * net(&s, id).stroke_path(); + let gone = strays(&after, &before.stroke_path()); + assert!(gone <= 1.5, "{what}: strays {gone}: {}", after.to_svg()); + let (b0, b1) = (before.stroke_path().bounding_box(), after.bounding_box()); + assert!(b0.inflate(1.5, 1.5).contains_rect(b1), "{what}: {b1:?} past {b0:?}: {}", after.to_svg()); + assert_eq!(net(&s, id).regions.len(), before.regions.len(), "{what}: the fill is kept"); } - assert_eq!(net(&s, id).stroke_path().elements(), before.elements(), "the samples as they were"); - ex(&mut s, "vector.simplify", json!({"ids": [id], "tolerance": 0.75})); - let after = s.doc().unwrap().world_transform(id) * net(&s, id).stroke_path(); - let (b0, b1) = (before.bounding_box(), after.bounding_box()); - assert!(b0.inflate(1.5, 1.5).contains_rect(b1), "{b1:?} past {b0:?}: {}", after.to_svg()); } fn ex(s: &mut Session, cmd: &str, p: Value) -> Value { diff --git a/crates/geom/src/tests.rs b/crates/geom/src/tests.rs index 10e09318c..786bf1935 100644 --- a/crates/geom/src/tests.rs +++ b/crates/geom/src/tests.rs @@ -393,7 +393,6 @@ fn deviation(out: &BezPath, input: &BezPath) -> f64 { /// about 390 and 160 units off; at whole-pixel coordinates the same shapes happened to fit well. #[test] fn simplifying_keeps_to_strokes_kurbo_shot_past() { - use kurbo::ParamCurve; let cases: [(&str, &[(f64, f64)]); 5] = [ ( "out and back to the same point", @@ -422,23 +421,127 @@ fn simplifying_keeps_to_strokes_kurbo_shot_past() { ), ]; for (what, pts) in cases { - let input = polyline(pts); - let out = simplify_path(&input, 0.75); - let d = deviation(&out, &input); - assert!(d <= 1.5, "{what}: strays {d} from the samples: {}", out.to_svg()); + keeps_to_one_stroke(what, &polyline(pts)); + } +} + +/// Accuracies the simplification is checked at: fine, the Pencil's, and coarse. +const ACCURACIES: [f64; 3] = [0.1, 0.75, 8.0]; + +/// Check that `input` (one open subpath), simplified at each of [`ACCURACIES`], strays no more +/// than twice the accuracy from it and leaves no part of it further off than that, stays in its +/// box grown by as much, and is still one subpath from its start to its end. +fn keeps_to_one_stroke(what: &str, input: &BezPath) { + for accuracy in ACCURACIES { + let out = simplify_path(input, accuracy); + let (svg, limit) = (out.to_svg(), 2.0 * accuracy); + let d = deviation(&out, input); + assert!(d <= limit, "{what} at {accuracy}: strays {d} from the input: {svg}"); + let left = deviation(input, &out); + assert!(left <= limit, "{what} at {accuracy}: leaves the input {left} off: {svg}"); let (inb, outb) = (input.bounding_box(), out.bounding_box()); - assert!(inb.inflate(1.5, 1.5).contains_rect(outb), "{what}: {outb:?} past {inb:?}"); - // One subpath, starting and ending where the stroke does. + assert!(inb.inflate(limit, limit).contains_rect(outb), "{what} at {accuracy}: {outb:?} past {inb:?}"); let moves = out.elements().iter().filter(|e| matches!(e, kurbo::PathEl::MoveTo(_))).count(); - assert_eq!(moves, 1, "{what}: {}", out.to_svg()); - let (first, last) = (pts[0], pts[pts.len() - 1]); - assert_eq!(out.segments().next().map(|s| s.start()), Some(Point::new(first.0, first.1)), "{what}"); - assert_eq!(out.segments().last().map(|s| s.end()), Some(Point::new(last.0, last.1)), "{what}"); + assert_eq!(moves, 1, "{what} at {accuracy}: {svg}"); + // From its start to its end: kurbo evaluates a fitted end on the input, so it is off by + // rounding at most. + let ends = |p: &BezPath| p.segments().next().map(|s| s.start()).zip(p.segments().last().map(|s| s.end())); + let ((start, end), (start_in, end_in)) = (ends(&out).unwrap(), ends(input).unwrap()); + assert!((start - start_in).hypot() < 1e-9 && (end - end_in).hypot() < 1e-9, "{what} at {accuracy}: {svg}"); + } +} + +/// Strokes made of curves keep to them as polylines do (#1741): a quadratic or a cubic that goes +/// out and comes straight back, one whose control points sit on its ends (so its direction is +/// read from the control point after them), and nearly collinear curves. +#[test] +fn simplifying_keeps_to_curves_that_turn_back_or_run_straight() { + let (p, q, r) = ( + (209.598_690_794_015_5, -204.909_328_119_739_88), + (211.277_914_263_870_97, -207.190_974_020_753_4), + (214.246_506_828_720_84, -211.228_827_590_623_24), + ); + // The nearly collinear samples, each line as a cubic with its handles a third of the way along. + let third = |a: (f64, f64), b: (f64, f64), t: f64| (a.0 + (b.0 - a.0) * t, a.1 + (b.1 - a.1) * t); + let mut straight_cubics = BezPath::new(); + straight_cubics.move_to(p); + for (a, b) in [(p, q), (q, r)] { + straight_cubics.curve_to(third(a, b, 1.0 / 3.0), third(a, b, 2.0 / 3.0), b); + } + let mut straight_quads = BezPath::new(); + straight_quads.move_to(p); + straight_quads.quad_to(third(p, q, 0.5), q); + straight_quads.quad_to(third(q, r, 0.5), r); + let cases = [ + ("a quadratic out and back", BezPath::from_svg("M0 0 Q15 10 30 0 Q15 10 0 0 Q-15 10 -30 0").unwrap()), + ("a cubic out and back", BezPath::from_svg("M0 0 C10 8 20 8 30 0 C20 8 10 8 0 0 C-10 8 -20 8 -30 0").unwrap()), + ("handles on the ends", BezPath::from_svg("M0 0 C0 0 30 4 30 4 C30 4 0 0 0 0 L-30 -4").unwrap()), + ("a cubic dip down and back up", BezPath::from_svg("M0 0 L10 0 C15 0 20 5 20 12 C20 5 25 0 30 0 L40 0").unwrap()), + ("nearly collinear cubics", straight_cubics), + ("nearly collinear quadratics", straight_quads), + ]; + for (what, input) in cases { + keeps_to_one_stroke(what, &input); + } +} + +/// `sides` simplified by kurbo one at a time and joined into one subpath. +fn fitted_apart(sides: &[&BezPath], accuracy: f64) -> BezPath { + let mut out = BezPath::new(); + for side in sides { + let fit = kurbo::simplify::simplify_bezpath(side.iter(), accuracy, &Default::default()); + let skip = usize::from(!out.elements().is_empty()); + out.extend(fit.elements().iter().copied().skip(skip)); + } + out +} + +/// `curve` cut into `n` cubics, drawn on from where `path` is: forwards, or back from its end. +fn in_pieces(path: &mut BezPath, curve: kurbo::CubicBez, n: u32, back: bool) { + use kurbo::ParamCurve; + for i in 0..n { + let i = if back { n - 1 - i } else { i }; + let c = curve.subsegment(f64::from(i) / f64::from(n)..f64::from(i + 1) / f64::from(n)); + if back { path.curve_to(c.p2, c.p1, c.p0) } else { path.curve_to(c.p1, c.p2, c.p3) } + } +} + +/// Where a path turns straight back it is cut, so the result is kurbo's fit of each side on its +/// own, with a point of the path at the turn. Fitted as one, kurbo rounds the turn off: its curve +/// passes the stray check, but falls short of the turn or takes more segments. A cubic whose +/// control points sit on its ends turns back as a line does. +#[test] +fn simplifying_fits_each_side_of_a_turn_back_on_its_own() { + use kurbo::ParamCurve; + let svg = |d: &str| BezPath::from_svg(d).unwrap(); + let s = kurbo::CubicBez::new((0.0, 0.0), (40.0, -60.0), (80.0, 60.0), (120.0, 0.0)); + let (mut s_out, mut s_back) = (BezPath::new(), BezPath::new()); + s_out.move_to(s.p0); + in_pieces(&mut s_out, s, 8, false); + s_back.move_to(s.p3); + in_pieces(&mut s_back, s, 8, true); + let cases = [ + ("lines", svg("M0 0 L10 0 L20 0 L30 0"), svg("M30 0 L20 0 L10 0")), + ("handles on the ends", svg("M0 0 L10 0 L20 0 C20 0 30 0 30 0"), svg("M30 0 C30 0 20 0 20 0 L10 0")), + ("a curve in pieces", s_out, s_back), + ]; + for (what, out_side, back_side) in cases { + let turn = out_side.segments().last().map(|seg| seg.end()).unwrap(); + let mut input = out_side.clone(); + input.extend(back_side.elements().iter().copied().skip(1)); + for accuracy in ACCURACIES { + let ours = simplify_path(&input, accuracy); + let apart = fitted_apart(&[&out_side, &back_side], accuracy); + assert_eq!(ours.elements(), apart.elements(), "{what} at {accuracy}"); + assert!(ours.segments().any(|seg| (seg.end() - turn).hypot() < 1e-9), "{what} at {accuracy}: {}", ours.to_svg()); + } } } /// Where a path never turns back and kurbo's fit holds, the result is kurbo's own, element for /// element: smooth runs, corners sharper than a right angle, closed subpaths and several subpaths. +/// A fit counts as holding up to twice the accuracy asked for, whatever that is: the large wave's +/// is further off at the coarse accuracy than the Pencil's limit of 1.5. #[test] fn simplifying_is_kurbos_where_its_fit_holds() { let wave: Vec<(f64, f64)> = (0..=60).map(|i| (f64::from(i) * 4.0, (f64::from(i) / 6.0).sin() * 30.0)).collect(); @@ -452,12 +555,30 @@ fn simplifying_is_kurbos_where_its_fit_holds() { e => e, })); let curves = BezPath::from_svg("M0 0 C10 -20 30 -20 40 0 C50 20 70 20 80 0 Q100 -30 120 0 L140 10 Z M200 0 L240 0").unwrap(); - for (what, input) in - [("a wave", polyline(&wave)), ("a zigzag", polyline(&zigzag)), ("closed", closed), ("two subpaths", several), ("curves", curves)] - { - let ours = simplify_path(&input, 0.75); - let theirs = kurbo::simplify::simplify_bezpath(input.iter(), 0.75, &Default::default()); - assert_eq!(ours.elements(), theirs.elements(), "{what}"); + // A sine 300 high in 40 smooth cubics. + let sine = |x: f64| ((x / 120.0).sin() * 300.0, (x / 120.0).cos() * 2.5); + let mut large = BezPath::new(); + large.move_to((0.0, 0.0)); + for i in 0..40 { + let (x0, x1) = (f64::from(i) * 40.0, f64::from(i + 1) * 40.0); + let ((y0, d0), (y1, d1)) = (sine(x0), sine(x1)); + large.curve_to((x0 + 40.0 / 3.0, y0 + d0 * 40.0 / 3.0), (x1 - 40.0 / 3.0, y1 - d1 * 40.0 / 3.0), (x1, y1)); + } + let coarse = simplify_path(&large, 8.0); + assert!(coarse.elements().len() < 10 && (1.5..=8.0).contains(&deviation(&coarse, &large)), "{}", deviation(&coarse, &large)); + for (what, input) in [ + ("a wave", polyline(&wave)), + ("a zigzag", polyline(&zigzag)), + ("closed", closed), + ("two subpaths", several), + ("curves", curves), + ("a large wave", large), + ] { + for accuracy in ACCURACIES { + let ours = simplify_path(&input, accuracy); + let theirs = kurbo::simplify::simplify_bezpath(input.iter(), accuracy, &Default::default()); + assert_eq!(ours.elements(), theirs.elements(), "{what} at {accuracy}"); + } } } @@ -482,4 +603,40 @@ fn simplifying_keeps_closes_and_subpaths_across_a_reversal() { empty.line_to((5.0, 5.0)); empty.close_path(); assert!(simplify_path(&empty, 0.75).elements().is_empty()); + for d in ["", "M5 5", "M5 5 M6 6", "M5 5 Z"] { + let input = if d.is_empty() { BezPath::new() } else { BezPath::from_svg(d).unwrap() }; + assert!(simplify_path(&input, 0.75).elements().is_empty(), "{d:?}"); + } +} + +/// The subpaths simplified keep their starts: a move with nothing drawn from it between two +/// subpaths leaves no trace, and a segment drawn on from a close starts a subpath where the closed +/// one started, as it does in the path given. +#[test] +fn simplifying_keeps_where_each_subpath_starts() { + let starts = + |p: &BezPath| -> Vec { p.elements().iter().filter_map(|e| if let kurbo::PathEl::MoveTo(p) = e { Some(*p) } else { None }).collect() }; + let between = BezPath::from_svg("M0 0 L10 0 L20 5 M50 50 M100 0 L110 0 L120 5").unwrap(); + let out = simplify_path(&between, 0.75); + assert_eq!(starts(&out), [Point::new(0.0, 0.0), Point::new(100.0, 0.0)], "{}", out.to_svg()); + assert!(deviation(&out, &between) <= 1.5, "{}", out.to_svg()); + // After the close, the pen is back at (0, 0): the next segments start there, also across a + // reversal. + use kurbo::PathEl::{ClosePath, LineTo, MoveTo}; + let on = BezPath::from_vec(vec![ + MoveTo(Point::new(0.0, 0.0)), + LineTo(Point::new(10.0, 0.0)), + LineTo(Point::new(10.0, 10.0)), + ClosePath, + LineTo(Point::new(20.0, 20.0)), + LineTo(Point::new(30.0, 20.0)), + LineTo(Point::new(20.0, 20.0)), + ]); + let out = simplify_path(&on, 0.75); + let els = out.elements(); + let close = els.iter().position(|e| matches!(e, kurbo::PathEl::ClosePath)).unwrap(); + assert_eq!(els.get(close + 1), Some(&kurbo::PathEl::MoveTo(Point::new(0.0, 0.0))), "{}", out.to_svg()); + assert_eq!(starts(&out), [Point::new(0.0, 0.0), Point::new(0.0, 0.0)], "{}", out.to_svg()); + assert_eq!(out.segments().last().map(|s| kurbo::ParamCurve::end(&s)), Some(Point::new(20.0, 20.0))); + assert!(deviation(&out, &on) <= 1.5, "{}", out.to_svg()); } From a66946d13e6d22eb8fcfcc9ea0100f2f59db349a Mon Sep 17 00:00:00 2001 From: impulsado <72570835+impulsado@users.noreply.github.com> Date: Tue, 10 Nov 2026 09:34:17 +0100 Subject: [PATCH 13/23] M3: strengthen inherited slot property regression coverage Cover inherited Boolean bindings and independent same-component slot content. Strengthen text, HTML and stored-property assertions, split default and own-content cases, and document ownership-based binding resolution. --- crates/engine/src/assets/components/sync.rs | 6 +- crates/engine/src/slot_tests.rs | 196 +++++++++++++++++--- 2 files changed, 172 insertions(+), 30 deletions(-) diff --git a/crates/engine/src/assets/components/sync.rs b/crates/engine/src/assets/components/sync.rs index 17836b111..513f4f7c8 100644 --- a/crates/engine/src/assets/components/sync.rs +++ b/crates/engine/src/assets/components/sync.rs @@ -165,8 +165,8 @@ fn copy_layers( // No longer a slot in the main: it mirrors the main again. n.overrides.remove(SLOT); } - // Component properties: the main layer's bindings use this instance's values; layers of - // nested instances use the nested instance's values (one level per source hop). + // Bindings use the enclosing instance of their defining component, even when slot + // content has fewer source hops than enclosing instances. let enc = enclosing.get(&parent).cloned().unwrap_or_default(); apply_enclosing_props(doc, &mut n, orig, &enc, &inst_props); if let NodeKind::Instance { component: c, props } = &n.kind { @@ -254,6 +254,8 @@ fn rebuild_nested(doc: &mut Document, nested: Vec, old: &BTreeMap (NodeId, NodeId, NodeId, } fn assert_slot_text(s: &mut Session, instance: NodeId, expected: &str) { - let label = kids(s, instance)[0]; - let characters = node(s, label).kind.text().unwrap().characters.clone(); + let children = kids(s, instance); + assert_eq!(children.len(), 1, "Content {instance} must have exactly one Label"); + let label = children[0]; + assert_eq!(node(s, label).name, "Label"); + let characters = node(s, label).kind.text().expect("Content's only child must be text").characters.clone(); let html = ex(s, "inspect.code", json!({"id": instance, "language": "html"}))["code"].as_str().unwrap().to_string(); assert_eq!(characters, expected, "slot Label characters; HTML: {html}"); + assert_eq!(html.matches("

").count(), 1, "slot Label HTML must contain exactly one paragraph: {html}"); assert!(html.contains(&format!(">{expected}

")), "slot Label HTML: {html}"); } fn assert_local_slot_text(s: &mut Session, instance: NodeId, expected: &str) { - assert!(matches!(&node(s, instance).kind, NodeKind::Instance { props, .. } if props.get("Text") == Some(&json!(expected)))); + assert!( + matches!(&node(s, instance).kind, NodeKind::Instance { props, .. } if props.get("Text") == Some(&json!(expected))), + "instance {instance} must store its own Text value {expected:?}: {:?}", + node(s, instance).kind + ); let properties = props_of(s, instance); - assert_eq!(properties.iter().find(|p| p["name"] == "Text").unwrap()["value"], expected); + let text = properties.iter().find(|p| p["name"] == "Text").unwrap_or_else(|| panic!("instance {instance} has no Text property: {properties:?}")); + assert_eq!(text["value"], expected, "instance {instance} Text property"); +} + +/// Exercise the same ownership contract for default slot content and content supplied by the +/// Composition main; separate tests identify which route regresses. +fn check_slot_text_properties_stay_local(own: bool) { + let mut s = Session::new(); + let (content, label, _base, composition, main_content) = slot_text_composition(&mut s, own); + let a = idv(&ex(&mut s, "instance.create", json!({"component": composition, "x": 400}))); + let b = idv(&ex(&mut s, "instance.create", json!({"component": composition, "x": 800}))); + let ac = copy_of(&s, a, main_content); + let bc = copy_of(&s, b, main_content); + ex(&mut s, "instance.setProperties", json!({"ids": [ac], "properties": {"Text": "Custom"}})); + assert_local_slot_text(&mut s, ac, "Custom"); + assert_slot_text(&mut s, ac, "Custom"); + assert_local_slot_text(&mut s, bc, "Default"); + assert_slot_text(&mut s, bc, "Default"); + assert_local_slot_text(&mut s, main_content, "Default"); + assert_slot_text(&mut s, main_content, "Default"); + assert_eq!(node(&s, label).kind.text().unwrap().characters, "Default"); + // Main property edits propagate without replacing another instance's local value. + ex(&mut s, "instance.setProperties", json!({"ids": [main_content], "properties": {"Text": "Main value"}})); + assert_slot_text(&mut s, ac, "Custom"); + assert_slot_text(&mut s, bc, "Main value"); + assert_slot_text(&mut s, main_content, "Main value"); + ex(&mut s, "component.editProperty", json!({"id": content, "name": "Text", "default": "New default"})); + assert_slot_text(&mut s, ac, "Custom"); + assert_slot_text(&mut s, bc, "Main value"); + assert_local_slot_text(&mut s, main_content, "Main value"); + assert_slot_text(&mut s, main_content, "Main value"); + healthy(&s); } #[test] -fn slot_text_properties_stay_local_in_default_and_own_content() { - for own in [false, true] { - let mut s = Session::new(); - let (content, label, _base, composition, main_content) = slot_text_composition(&mut s, own); - let a = idv(&ex(&mut s, "instance.create", json!({"component": composition, "x": 400}))); - let b = idv(&ex(&mut s, "instance.create", json!({"component": composition, "x": 800}))); - let ac = copy_of(&s, a, main_content); - let bc = copy_of(&s, b, main_content); - ex(&mut s, "instance.setProperties", json!({"ids": [ac], "properties": {"Text": "Custom"}})); - assert_local_slot_text(&mut s, ac, "Custom"); - assert_slot_text(&mut s, ac, "Custom"); - assert_slot_text(&mut s, bc, "Default"); - assert_slot_text(&mut s, main_content, "Default"); - assert_eq!(node(&s, label).kind.text().unwrap().characters, "Default"); - // Main property edits propagate without replacing another instance's local value. - ex(&mut s, "instance.setProperties", json!({"ids": [main_content], "properties": {"Text": "Main value"}})); - assert_slot_text(&mut s, ac, "Custom"); - assert_slot_text(&mut s, bc, "Main value"); - assert_slot_text(&mut s, main_content, "Main value"); - ex(&mut s, "component.editProperty", json!({"id": content, "name": "Text", "default": "New default"})); - assert_slot_text(&mut s, ac, "Custom"); - assert_slot_text(&mut s, bc, "Main value"); - healthy(&s); - } +fn slot_text_properties_stay_local_in_default_content() { + check_slot_text_properties_stay_local(false); +} + +#[test] +fn slot_text_properties_stay_local_in_own_content() { + check_slot_text_properties_stay_local(true); } #[test] @@ -451,11 +474,15 @@ fn slot_text_properties_do_not_read_an_unbound_base_property() { let (_content, _label, base, composition, main_content) = slot_text_composition(&mut s, true); ex(&mut s, "component.addProperty", json!({"id": base, "name": "Text", "type": "text", "default": "Wrong owner"})); let i = idv(&ex(&mut s, "instance.create", json!({"component": composition, "x": 400}))); + let nested_base = kids(&s, i)[0]; + assert_local_slot_text(&mut s, nested_base, "Wrong owner"); let nested = copy_of(&s, i, main_content); + assert_local_slot_text(&mut s, nested, "Default"); assert_slot_text(&mut s, nested, "Default"); ex(&mut s, "instance.setProperties", json!({"ids": [nested], "properties": {"Text": "Custom"}})); assert_local_slot_text(&mut s, nested, "Custom"); assert_slot_text(&mut s, nested, "Custom"); + assert_local_slot_text(&mut s, nested_base, "Wrong owner"); assert_slot_text(&mut s, main_content, "Default"); healthy(&s); } @@ -472,6 +499,9 @@ fn slot_text_properties_survive_inherited_own_content_and_persistence() { let bc = copy_of(&s, b, inherited_content); ex(&mut s, "instance.setProperties", json!({"ids": [ac], "properties": {"Text": "Custom"}})); assert_local_slot_text(&mut s, ac, "Custom"); + assert_local_slot_text(&mut s, bc, "Default"); + assert_local_slot_text(&mut s, inherited_content, "Default"); + assert_local_slot_text(&mut s, main_content, "Default"); let bytes = figcraft_format::save_bytes(s.doc().unwrap(), None).unwrap(); let (back, _) = figcraft_format::load_bytes(&bytes).unwrap(); let mut s = Session::with_document(back); @@ -495,6 +525,116 @@ fn slot_text_properties_survive_inherited_own_content_and_persistence() { healthy(&s); } +/// Check both the stored property and the Label it drives, not just one representation. +fn assert_slot_visibility(s: &mut Session, instance: NodeId, expected: bool) { + assert!( + matches!(&node(s, instance).kind, NodeKind::Instance { props, .. } if props.get("Visible") == Some(&json!(expected))), + "instance {instance} must store Visible={expected}: {:?}", + node(s, instance).kind + ); + let properties = props_of(s, instance); + let visible = + properties.iter().find(|p| p["name"] == "Visible").unwrap_or_else(|| panic!("instance {instance} has no Visible property: {properties:?}")); + assert_eq!(visible["value"], expected, "instance {instance} Visible property"); + let children = kids(s, instance); + assert_eq!(children.len(), 1, "Content {instance} must have exactly one Label"); + assert_eq!(node(s, children[0]).name, "Label"); + assert_eq!(node(s, children[0]).visible, expected, "instance {instance} Label visibility"); +} + +#[test] +fn slot_boolean_properties_stay_local_in_inherited_own_content() { + let mut s = Session::new(); + let (content, label, base, composition, main_content) = slot_text_composition(&mut s, true); + ex(&mut s, "component.addProperty", json!({"id": content, "name": "Visible", "type": "boolean", "default": true, "layer": label})); + ex(&mut s, "component.addProperty", json!({"id": base, "name": "Visible", "type": "boolean", "default": false})); + let (outer, inherited) = slot_text_wrapper(&mut s, composition, "Outer composition"); + let inherited_base = kids(&s, inherited)[0]; + let inherited_content = copy_of(&s, inherited, main_content); + let a = idv(&ex(&mut s, "instance.create", json!({"component": outer, "x": 400}))); + let b = idv(&ex(&mut s, "instance.create", json!({"component": outer, "x": 800}))); + let ac = copy_of(&s, a, inherited_content); + let bc = copy_of(&s, b, inherited_content); + let abase = copy_of(&s, a, inherited_base); + assert!(matches!(&node(&s, abase).kind, NodeKind::Instance { props, .. } if props.get("Visible") == Some(&json!(false)))); + assert_slot_visibility(&mut s, ac, true); + assert_slot_visibility(&mut s, bc, true); + ex(&mut s, "instance.setProperties", json!({"ids": [ac], "properties": {"Visible": false}})); + assert_slot_visibility(&mut s, ac, false); + assert_slot_visibility(&mut s, bc, true); + assert_slot_visibility(&mut s, inherited_content, true); + assert_slot_visibility(&mut s, main_content, true); + // A main update must not replace either instance's local value with the Base's homonym. + ex(&mut s, "instance.setProperties", json!({"ids": [main_content], "properties": {"Visible": false}})); + assert_slot_visibility(&mut s, ac, false); + assert_slot_visibility(&mut s, bc, false); + ex(&mut s, "instance.setProperties", json!({"ids": [bc], "properties": {"Visible": true}})); + ex(&mut s, "node.set", json!({"id": content, "props": {"fill": "#abcdef"}})); + assert_slot_visibility(&mut s, ac, false); + assert_slot_visibility(&mut s, bc, true); + assert_slot_visibility(&mut s, main_content, false); + healthy(&s); + let bytes = figcraft_format::save_bytes(s.doc().unwrap(), None).unwrap(); + let (back, _) = figcraft_format::load_bytes(&bytes).unwrap(); + let mut s = Session::with_document(back); + assert_slot_visibility(&mut s, ac, false); + assert_slot_visibility(&mut s, bc, true); + assert_slot_visibility(&mut s, inherited_content, false); + assert_slot_visibility(&mut s, main_content, false); + ex(&mut s, "instance.setProperties", json!({"ids": [main_content], "properties": {"Visible": true}})); + assert_slot_visibility(&mut s, ac, false); + assert_slot_visibility(&mut s, bc, true); + assert_slot_visibility(&mut s, main_content, true); + healthy(&s); +} + +#[test] +fn same_component_in_own_slot_keeps_independent_text_properties() { + let mut s = Session::new(); + let frame = idv(&ex(&mut s, "frame.create", json!({"width": 200, "height": 100, "name": "Card"}))); + let label = idv(&ex(&mut s, "text.create", json!({"text": "Default", "parent": frame, "props": {"name": "Label"}}))); + let body = idv(&ex(&mut s, "frame.create", json!({"width": 180, "height": 60, "y": 40, "name": "Body", "parent": frame}))); + let card = idv(&ex(&mut s, "component.create", json!({"ids": [frame]}))); + ex(&mut s, "component.addProperty", json!({"id": card, "name": "Text", "type": "text", "default": "Default", "layer": label})); + ex(&mut s, "component.makeSlot", json!({"id": body})); + // Only page instance content holds another Card: the main never contains itself. + let outer = idv(&ex(&mut s, "instance.create", json!({"component": card, "x": 400}))); + let inner = idv(&ex(&mut s, "instance.slotInsert", json!({"id": outer, "slot": "Body", "component": card}))); + let check = |s: &mut Session, outer_text: &str, inner_text: &str| { + assert_local_slot_text(s, outer, outer_text); + assert_local_slot_text(s, inner, inner_text); + assert_eq!(node(s, copy_of(s, outer, label)).kind.text().unwrap().characters, outer_text); + assert_eq!(node(s, copy_of(s, inner, label)).kind.text().unwrap().characters, inner_text); + let outer_body = copy_of(s, outer, body); + assert!(slot_edited(s.doc().unwrap(), outer_body)); + assert_eq!(kids(s, outer_body), vec![inner]); + assert!(kids(s, copy_of(s, inner, body)).is_empty()); + assert!(kids(s, body).is_empty(), "the main's slot must remain empty"); + healthy(s); + }; + ex(&mut s, "instance.setProperties", json!({"ids": [outer], "properties": {"Text": "Outer"}})); + check(&mut s, "Outer", "Default"); + ex(&mut s, "instance.setProperties", json!({"ids": [inner], "properties": {"Text": "Inner"}})); + check(&mut s, "Outer", "Inner"); + ex(&mut s, "text.set", json!({"id": label, "characters": "New default"})); + ex(&mut s, "node.set", json!({"id": card, "props": {"fill": "#abcdef"}})); + check(&mut s, "Outer", "Inner"); + assert_eq!(fill(&s, outer), "ABCDEF"); + assert_eq!(fill(&s, inner), "ABCDEF"); + ex(&mut s, "instance.setProperties", json!({"ids": [outer], "properties": {"Text": "Outer updated"}})); + check(&mut s, "Outer updated", "Inner"); + ex(&mut s, "instance.setProperties", json!({"ids": [inner], "properties": {"Text": "Inner updated"}})); + check(&mut s, "Outer updated", "Inner updated"); + let bytes = figcraft_format::save_bytes(s.doc().unwrap(), None).unwrap(); + let (back, _) = figcraft_format::load_bytes(&bytes).unwrap(); + let mut s = Session::with_document(back); + check(&mut s, "Outer updated", "Inner updated"); + ex(&mut s, "node.set", json!({"id": card, "props": {"fill": "#123456"}})); + check(&mut s, "Outer updated", "Inner updated"); + assert_eq!(fill(&s, outer), "123456"); + assert_eq!(fill(&s, inner), "123456"); +} + /// Random slot edits, main edits and resets keep every instance mirroring its main. #[test] fn random_slot_edits_keep_instances_consistent() { From 4d3fe2aa1ac041df7fa4d785beb8429d66678ed8 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 11 Nov 2026 15:59:59 +0000 Subject: [PATCH 14/23] svg import: cap attributes per element, map span offsets to characters once --- crates/svg/src/import.rs | 35 ++++++++++++++++++++++---- crates/svg/src/import/budget.rs | 2 +- crates/svg/src/import/hostile_tests.rs | 12 +++++++++ crates/svg/src/import/text.rs | 12 ++++++--- crates/svg/src/import/text_tests.rs | 8 ++++++ 5 files changed, 60 insertions(+), 9 deletions(-) diff --git a/crates/svg/src/import.rs b/crates/svg/src/import.rs index 1ac8a78bf..f77b10bfa 100644 --- a/crates/svg/src/import.rs +++ b/crates/svg/src/import.rs @@ -704,13 +704,28 @@ fn simple_hash(b: &[u8]) -> String { /// Deepest element nesting the parser is given: deeper documents would exhaust the stack. const MAX_NESTING: usize = 512; +/// The most attributes one element may have. The XML parser checks each attribute against the +/// earlier ones, so a huge count is quadratic work; real elements have a few dozen at most. +const MAX_ATTRIBUTES: usize = 4096; + /// How deep elements nest in `data`: a scan of its tags (comments, CDATA, declarations and quoted /// attribute values skipped) that stops once past `limit`. pub(crate) fn nesting(data: &[u8], limit: usize) -> usize { + scan(data, limit).0 +} + +/// Whether an element of `data` has more than [`MAX_ATTRIBUTES`] attributes (counted as quoted +/// values, before the parser sees them). +pub(crate) fn too_wide(data: &[u8]) -> bool { + scan(data, usize::MAX).1 > MAX_ATTRIBUTES +} + +/// [`nesting`] and the most attribute values on one element; stops once either is past its limit. +fn scan(data: &[u8], limit: usize) -> (usize, usize) { let find = |from: usize, pat: &[u8]| -> usize { data.get(from..).and_then(|r| r.windows(pat.len()).position(|w| w == pat)).map_or(data.len(), |p| from + p + pat.len()) }; - let (mut depth, mut max, mut i) = (0usize, 0usize, 0usize); + let (mut depth, mut max, mut i, mut widest) = (0usize, 0usize, 0usize, 0usize); while let Some(off) = data.get(i..).and_then(|r| r.iter().position(|b| *b == b'<')) { i += off; let rest = data.get(i..).unwrap_or(&[]); @@ -725,12 +740,15 @@ pub(crate) fn nesting(data: &[u8], limit: usize) -> usize { i = find(i + 2, b">"); } else { // A start tag: to its `>` outside quotes; `/>` closes it. - let (mut quote, mut j, mut closed) = (None, i + 1, false); + let (mut quote, mut j, mut closed, mut attrs) = (None, i + 1, false, 0usize); while let Some(&c) = data.get(j) { match quote { Some(q) if c == q => quote = None, Some(_) => {} - None if c == b'"' || c == b'\'' => quote = Some(c), + None if c == b'"' || c == b'\'' => { + quote = Some(c); + attrs += 1; + } None if c == b'>' => { closed = data.get(j.wrapping_sub(1)) == Some(&b'/'); break; @@ -739,17 +757,21 @@ pub(crate) fn nesting(data: &[u8], limit: usize) -> usize { } j += 1; } + widest = widest.max(attrs); + if widest > MAX_ATTRIBUTES { + return (max, widest); + } if !closed { depth += 1; max = max.max(depth); if max > limit { - return max; + return (max, widest); } } i = j + 1; } } - max + (max, widest) } /// Whether `data` nests elements more than [`MAX_NESTING`] deep: the byte scan first, then the @@ -883,6 +905,9 @@ fn import_within( ) -> Result { // SVGZ is unpacked here, within a budget, so every check below sees the real document. let data = budget::document(data, &limits).map_err(ImportError)?; + if too_wide(&data) { + return Err(ImportError(format!("an element has more than {MAX_ATTRIBUTES} attributes"))); + } if too_deep(&data) { return Err(ImportError(format!("elements nested more than {MAX_NESTING} deep"))); } diff --git a/crates/svg/src/import/budget.rs b/crates/svg/src/import/budget.rs index 7de080af4..c50266dc0 100644 --- a/crates/svg/src/import/budget.rs +++ b/crates/svg/src/import/budget.rs @@ -87,7 +87,7 @@ fn embedded(used: &Used, limits: &Limits, data: &[u8], opts: &Options) -> Option } }; used.bytes.fetch_add(plain.len(), Ordering::Relaxed); - if super::too_deep(&plain) { + if super::too_wide(&plain) || super::too_deep(&plain) { return None; } used.depth.fetch_add(1, Ordering::Relaxed); diff --git a/crates/svg/src/import/hostile_tests.rs b/crates/svg/src/import/hostile_tests.rs index 0af1297d0..16731407c 100644 --- a/crates/svg/src/import/hostile_tests.rs +++ b/crates/svg/src/import/hostile_tests.rs @@ -181,3 +181,15 @@ fn entity_expansion_cannot_bypass_the_nesting_limit() { let err = import_svg(&mut d, data.as_bytes(), p, 0.0, 0.0, "x").err().map(|e| e.to_string()).unwrap_or_default(); assert!(err.contains("nested more than 512 deep"), "{err:?}"); } + +#[test] +fn an_element_with_thousands_of_attributes_is_refused_before_parsing() { + use super::too_wide; + let many = |n: usize| (0..n).map(|i| format!(" a{i}=\"1\"")).collect::(); + assert!(!too_wide(format!("", many(4096)).as_bytes())); + assert!(too_wide(format!("", many(4097)).as_bytes())); + assert!(!too_wide(format!("{}", "".repeat(10_000)).as_bytes()), "per element, not per file"); + let mut d = Document::new("t"); + let p = d.first_page().unwrap(); + assert!(import_svg(&mut d, &svg(&format!("", many(100_000))), p, 0.0, 0.0, "x").is_err()); +} diff --git a/crates/svg/src/import/text.rs b/crates/svg/src/import/text.rs index cee54bbd8..7ed8d018f 100644 --- a/crates/svg/src/import/text.rs +++ b/crates/svg/src/import/text.rs @@ -195,7 +195,7 @@ pub(crate) fn text(cx: &mut Ctx, t: &usvg::Text, parent: NodeId, base: Affine, d let x = chunk.x().map_or(pen_x, f64::from) + d(t.dx()); let y = chunk.y().map_or(pen_y, f64::from) + d(t.dy()); char_at += count; - let ci = |b: usize| text.get(..b).map_or(count, |p| p.chars().count()); + let ci = char_index(text); let spans: Vec<_> = chunk.spans().iter().map(|sp| (ci(sp.start()), ci(sp.end()), sp, span_style(sp, s, fonts))).collect(); if spans.is_empty() { continue; @@ -229,6 +229,13 @@ pub(crate) fn text(cx: &mut Ctx, t: &usvg::Text, parent: NodeId, base: Affine, d } } +/// The characters of `text` before a byte offset (all of them for an offset that is past the end +/// or inside a character), by a search in a table built once: many spans don't rescan the prefix. +fn char_index(text: &str) -> impl Fn(usize) -> usize + '_ { + let starts: Vec = text.char_indices().map(|(i, _)| i).collect(); + move |b| if text.is_char_boundary(b) { starts.partition_point(|s| *s < b) } else { starts.len() } +} + /// The text of `lines` (one per line, stacked at their pitch; `s`: their scale): characters, the /// first span's style and a run per span, fills still to come. fn text_data(lines: &[Line], s: f64) -> TextData { @@ -262,8 +269,7 @@ fn on_path(cx: &mut Ctx, t: &usvg::Text, world: Affine, parent: NodeId) -> bool let Some(len) = figcraft_geom::measure::polylines(&path).and_then(|l| l.first().map(|l| l.length())).filter(|l| *l > 1e-9) else { return false }; let fonts = cx.fonts.get_or_insert_with(Fonts::load); let text = chunk.text(); - let count = text.chars().count(); - let ci = |b: usize| text.get(..b).map_or(count, |p| p.chars().count()); + let ci = char_index(text); let spans: Vec<_> = chunk.spans().iter().map(|sp| (ci(sp.start()), ci(sp.end()), sp, span_style(sp, 1.0, fonts))).collect(); let line = Line { x: 0.0, y: 0.0, anchor: chunk.anchor(), text: text.to_string(), spans }; let mut td = text_data(std::slice::from_ref(&line), 1.0); diff --git a/crates/svg/src/import/text_tests.rs b/crates/svg/src/import/text_tests.rs index f267b2947..94c6ae5b2 100644 --- a/crates/svg/src/import/text_tests.rs +++ b/crates/svg/src/import/text_tests.rs @@ -30,3 +30,11 @@ fn css_system_font_names_keep_their_name_where_the_system_font_is() { assert_eq!(family(&named(&["-apple-system", "BlinkMacSystemFont", "system-ui", "sans-serif"]), &elsewhere), "Inter"); assert_eq!(family(&named(&["-apple-system"]), &elsewhere), "-apple-system", "nothing installed: the first name it gives"); } + +#[test] +fn char_index_counts_the_characters_before_a_byte_offset() { + let t = "aé😀b"; + let ci = super::char_index(t); + assert_eq!([ci(0), ci(1), ci(3), ci(7), ci(8)], [0, 1, 2, 3, 4]); + assert_eq!([ci(2), ci(4), ci(99)], [4, 4, 4], "inside a character or past the end: all of them"); +} From 1935c3eac22b9c4dc62e278ef56fd0f036001c5e Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 11 Nov 2026 16:03:10 +0000 Subject: [PATCH 15/23] layout: bound the snap lines a frame's grids generate --- crates/layout/src/depth_tests.rs | 12 ++++++++++++ crates/layout/src/lib.rs | 7 +++++++ 2 files changed, 19 insertions(+) diff --git a/crates/layout/src/depth_tests.rs b/crates/layout/src/depth_tests.rs index f8b597a34..b533c7e42 100644 --- a/crates/layout/src/depth_tests.rs +++ b/crates/layout/src/depth_tests.rs @@ -93,3 +93,15 @@ fn deeply_nested_fill_frames_lay_out_without_running_out_of_stack() { assert_eq!(width(&d, chain[DEEP - 1]), 10.0, "past the nesting the layout follows, layers stay"); }); } + +#[test] +fn snap_lines_are_bounded_across_a_frames_grids() { + use figcraft_doc::{Document, GridKind, LayoutGrid, NodeKind}; + let mut d = Document::new("t"); + let p = d.first_page().unwrap(); + let f = d.create(p, NodeKind::Frame, "F", 0.0, 0.0, 4000.0, 4000.0).unwrap(); + d.get_mut(f).unwrap().grids = vec![LayoutGrid { kind: GridKind::Grid, size: 1.0, visible: true, ..LayoutGrid::default() }; 1000]; + let (xs, ys) = crate::grid_snap_lines(&d, f); + assert!(xs.len() + ys.len() < 250_000, "{}", xs.len() + ys.len()); + assert!(xs.len() > 4000 && ys.len() > 4000, "the first grids still give their lines"); +} diff --git a/crates/layout/src/lib.rs b/crates/layout/src/lib.rs index 880c6dad7..04abbc39a 100644 --- a/crates/layout/src/lib.rs +++ b/crates/layout/src/lib.rs @@ -1159,6 +1159,9 @@ pub fn grid_snap_lines(doc: &Document, frame: NodeId) -> (Vec, Vec) { grid_snap_lines_in(doc, frame, doc.world_transform(frame)) } +/// The most snap coordinates a frame's grids give together (a few dozen grids' worth of lines). +const MAX_SNAP_LINES: usize = 200_000; + /// [`grid_snap_lines`] mapped from the frame's coordinates by `xf` (the frame's world transform /// for page lines). pub fn grid_snap_lines_in(doc: &Document, frame: NodeId, xf: Affine) -> (Vec, Vec) { @@ -1166,6 +1169,10 @@ pub fn grid_snap_lines_in(doc: &Document, frame: NodeId, xf: Affine) -> (Vec= MAX_SNAP_LINES { + break; + } for r in grid_lines(g, f.width, f.height) { let a = xf * Point::new(r.x0, r.y0); let b = xf * Point::new(r.x1, r.y1); From 9aaf7114e09268b8893d3ba01a8790e94f674577 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 11 Nov 2026 16:37:35 +0000 Subject: [PATCH 16/23] widgets: bound the decoded pixels of a widget's images, read from their headers --- crates/engine/src/extensions/widgets.rs | 48 +++++++++++++++++++++++-- 1 file changed, 45 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/extensions/widgets.rs b/crates/engine/src/extensions/widgets.rs index 13c6088c4..efdd21414 100644 --- a/crates/engine/src/extensions/widgets.rs +++ b/crates/engine/src/extensions/widgets.rs @@ -41,6 +41,8 @@ const FIELD: &str = "figcraft.widget.field"; const HOVER: &str = "figcraft.widget.hover"; /// Images in a widget are at most this many pixels on a side. const MAX_IMAGE_SIDE: u32 = 4096; +/// All images of one widget together are at most this many pixels once decoded (four of the largest). +const MAX_IMAGES_PIXELS: u64 = 4 * 4096 * 4096; /// Whether `n` is a widget's own layer. pub fn is_widget(n: &Node) -> bool { @@ -259,8 +261,9 @@ fn build(s: &mut Session, plugin: &str, w: NodeId, r: &WidgetRender) -> Result<( _ => n.plugin_data.remove(MENU), }; set_hover(n, r.root.hover.as_ref()); + let mut pixels = 0; for c in &r.root.children { - add(doc, w, c, cmd)?; + add(doc, w, c, cmd, &mut pixels)?; } // An SVG brings as many layers as it describes. if doc.subtree(w).len() > MAX_NODES { @@ -271,7 +274,7 @@ fn build(s: &mut Session, plugin: &str, w: NodeId, r: &WidgetRender) -> Result<( } /// Add layer `d` (and what it holds) inside `parent`, locked. -fn add(doc: &mut Document, parent: NodeId, d: &WidgetNode, cmd: &str) -> Result<()> { +fn add(doc: &mut Document, parent: NodeId, d: &WidgetNode, cmd: &str, pixels: &mut u64) -> Result<()> { let (kind, name) = match d.kind { WidgetKind::Frame => (NodeKind::Frame, "Frame"), WidgetKind::Text => (NodeKind::Text(TextData { characters: d.text.clone(), ..TextData::default() }), "Text"), @@ -292,6 +295,16 @@ fn add(doc: &mut Document, parent: NodeId, d: &WidgetNode, cmd: &str) -> Result< if bytes.len() > MAX_IMAGE_BYTES { return Err(bad(cmd, format!("an image is larger than {} KB", MAX_IMAGE_BYTES / 1024))); } + // From the header, before anything is decoded: a small file can hold billions of pixels. + if let Some((w, h)) = figcraft_render::images::dimensions(&bytes) { + if w > MAX_IMAGE_SIDE || h > MAX_IMAGE_SIDE { + return Err(bad(cmd, format!("an image is larger than {MAX_IMAGE_SIDE} pixels on a side"))); + } + *pixels += u64::from(w) * u64::from(h); + if *pixels > MAX_IMAGES_PIXELS { + return Err(bad(cmd, format!("the images are larger than {MAX_IMAGES_PIXELS} pixels together"))); + } + } let (hash, iw, ih) = crate::cmd::create::add_readable_image(doc, bytes).map_err(|e| bad(cmd, e))?; if iw > MAX_IMAGE_SIDE || ih > MAX_IMAGE_SIDE { return Err(bad(cmd, format!("an image is larger than {MAX_IMAGE_SIDE} pixels on a side"))); @@ -302,7 +315,7 @@ fn add(doc: &mut Document, parent: NodeId, d: &WidgetNode, cmd: &str) -> Result< } doc.insert(n, parent, None)?; crate::cmd::props::apply(cmd, doc, id, &props(d, false))?; - d.children.iter().try_for_each(|c| add(doc, id, c, cmd)) + d.children.iter().try_for_each(|c| add(doc, id, c, cmd, pixels)) } /// Add SVG layer `d` inside `parent`: a frame of the picture's own size holding its layers, locked @@ -604,3 +617,32 @@ pub fn specs() -> Vec { spec!("widget.list", "List widgets", "{} → {widgets: [{id, plugin, name, page, state, targets, fields, menu}]}", false, list), ] } + +#[cfg(test)] +mod pixel_budget_tests { + use super::*; + use base64::Engine as _; + + /// A `side` x `side` blank PNG (a few KB however many pixels). + fn png(side: u32) -> String { + let pix = figcraft_render::Pixmap::new(side, side).unwrap(); + base64::engine::general_purpose::STANDARD.encode(figcraft_render::encode_png(&pix).unwrap()) + } + + #[test] + fn images_of_a_widget_share_a_decoded_pixel_budget() { + let mut doc = Document::new("t"); + let page = doc.first_page().unwrap(); + let data = png(4096); + let image = |i: usize| serde_json::from_value::(json!({"type": "image", "name": format!("I{i}"), "data": data})).unwrap(); + let mut pixels = 0; + for i in 0..4 { + add(&mut doc, page, &image(i), "widget.insert", &mut pixels).unwrap(); + } + let err = add(&mut doc, page, &image(4), "widget.insert", &mut pixels).unwrap_err().to_string(); + assert!(err.contains("together"), "{err}"); + // A header past the side limit is refused before it is decoded. + let wide = serde_json::from_value::(json!({"type": "image", "data": png(4097)})).unwrap(); + assert!(add(&mut doc, page, &wide, "widget.insert", &mut 0).unwrap_err().to_string().contains("pixels on a side")); + } +} From a40c76ab68a1d9856a6a79d26e01603643da885d Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 11 Nov 2026 16:48:03 +0000 Subject: [PATCH 17/23] collab-net: cut a host's peer names and colours where the guest keeps them --- crates/collab-net/src/client_cap_tests.rs | 14 ++++++++++++++ crates/collab-net/src/join.rs | 21 +++++++++++++++++---- 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/crates/collab-net/src/client_cap_tests.rs b/crates/collab-net/src/client_cap_tests.rs index 920d53483..73f181774 100644 --- a/crates/collab-net/src/client_cap_tests.rs +++ b/crates/collab-net/src/client_cap_tests.rs @@ -59,3 +59,17 @@ fn joins_past_the_cap_are_left_out_until_someone_leaves() { send(joined(over)); assert!(matches!(inbox.drain().as_slice(), [Event::PeerLeft(7), Event::PeerJoined(p)] if p.client_id == over)); } + +#[test] +fn a_hosts_long_names_and_colours_are_cut_where_they_are_kept() { + let inbox = inbox(); + let cfg = GuestConfig::new(Invite::new("127.0.0.1:47900", "0123456789abcdef", "AbCdEfGhIjKlMnOpQrStUv"), "Ada".into()); + let resume = Mutex::new(None); + let mut seen = Seen::default(); + let long = "x".repeat(100_000); + take(ServerMsg::PeerJoined(Peer { client_id: 1, name: long.clone(), color: long }), &cfg, &inbox, &mut seen, &resume, &|| {}); + match inbox.drain().as_slice() { + [Event::PeerJoined(p)] => assert!(p.name.chars().count() <= 256 && p.color.chars().count() <= 32), + other => panic!("{other:?}"), + } +} diff --git a/crates/collab-net/src/join.rs b/crates/collab-net/src/join.rs index 8aacff711..5982ed6e8 100644 --- a/crates/collab-net/src/join.rs +++ b/crates/collab-net/src/join.rs @@ -226,6 +226,19 @@ pub(crate) struct Seen { peers: Vec, } +/// `p` with its name and colour cut to what the UI shows: the host is untrusted, and the roster is +/// copied whenever someone's identity is updated. +fn bounded(mut p: Peer) -> Peer { + p.name = p.name.chars().take(MAX_PEER_NAME).collect(); + p.color = p.color.chars().take(MAX_COLOR).collect(); + p +} + +/// The most characters of a colour (`#rrggbb` and a few spellings more) kept for someone. +const MAX_COLOR: usize = 32; +/// The most characters of a name kept for someone. +const MAX_PEER_NAME: usize = 256; + /// What taking in one message from the host means for the connection. pub(crate) enum Taken { Continue, @@ -251,7 +264,7 @@ pub(crate) fn take(mut m: ServerMsg, cfg: &GuestConfig, inbox: &Inbox, seen: &mu *r = Some(Resume { session, client_id, seq, secret }); } // The sync client opens the document (and fills in its name and pages). - let peers: Vec = peers.iter().take(MAX_PEERS).cloned().collect(); + let peers: Vec = peers.iter().take(MAX_PEERS).cloned().map(bounded).collect(); let inv = &cfg.invite; let info = SessionInfo { role: Role::Guest, @@ -274,14 +287,14 @@ pub(crate) fn take(mut m: ServerMsg, cfg: &GuestConfig, inbox: &Inbox, seen: &mu inbox.push(Event::Sync(m)); return Taken::Welcomed; } - ServerMsg::PeerJoined(p) => match seen.info.as_mut() { + ServerMsg::PeerJoined(p) => match (seen.info.as_mut(), bounded(p)) { // The host tells you how you are shown. - Some(info) if p.client_id == info.me => { + (Some(info), p) if p.client_id == info.me => { info.name = p.name; info.color = p.color; inbox.push(Event::Welcome { info: info.clone(), peers: seen.peers.clone() }); } - _ => { + (_, p) => { match seen.peers.iter().position(|q| q.client_id == p.client_id) { Some(i) => { seen.peers.remove(i); From 2f8dd85f4284c9b1ba0007d401d76200551c209e Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 11 Nov 2026 17:43:55 +0000 Subject: [PATCH 18/23] load: share one layer allowance across Penpot raw SVGs, sum font-run coverage once --- crates/engine/src/cmd/file/load.rs | 111 +++++++++++++++++++++++------ 1 file changed, 91 insertions(+), 20 deletions(-) diff --git a/crates/engine/src/cmd/file/load.rs b/crates/engine/src/cmd/file/load.rs index 3e917d5dd..77dd68198 100644 --- a/crates/engine/src/cmd/file/load.rs +++ b/crates/engine/src/cmd/file/load.rs @@ -399,11 +399,19 @@ pub(crate) fn load_fig(bytes: &[u8], name: &str) -> Option) { + expand_penpot_raw_svgs_within(doc, notes, MAX_PENPOT_SVG_LAYERS); +} + +/// [`expand_penpot_raw_svgs`] with at most `room` layers made in all. +fn expand_penpot_raw_svgs_within(doc: &mut Document, notes: &mut Vec, mut room: usize) { let layers = doc .nodes() .filter_map(|node| { @@ -413,12 +421,25 @@ fn expand_penpot_raw_svgs(doc: &mut Document, notes: &mut Vec) { }) .collect::>(); let mut ok_count = 0usize; + // The SVG importer caps its own layers per file; `room` is the cap over all of a file's placeholders. for (id, name, width, height, xml, empty) in layers { + if room == 0 { + if let Some(n) = doc.node_mut(id) { + n.plugin_data.insert("penpot.svgRawStatus".into(), "not-expanded".into()); + } + if notes.len() < 100 { + notes.push(format!("Penpot SVG '{name}': not converted, the file's SVG layers are over the limit; the original markup is preserved")); + } + continue; + } if !empty { notes.push(format!("Penpot SVG '{name}': has separate child layers; the original tree was preserved to prevent duplicate rendering")); continue; } - match figcraft_svg::import_svg_with_notes(doc, xml.as_bytes(), id, 0.0, 0.0, &name) { + let before = doc.node_count(); + let imported = figcraft_svg::import_svg_with_notes(doc, xml.as_bytes(), id, 0.0, 0.0, &name); + room = room.saturating_sub(doc.node_count().saturating_sub(before)); + match imported { Ok(imported) => { // An SVG can be syntactically valid yet contain no supported drawable // elements. Do not claim success or leave an empty extra frame. @@ -470,6 +491,28 @@ fn expand_penpot_raw_svgs(doc: &mut Document, notes: &mut Vec) { } } +/// Whether some of the first `len` characters are in none of `runs`. Where runs start and stop +/// covering is summed once, so runs overlapping any number of times cost no more than their count. +fn has_uncovered(len: usize, runs: &[figcraft_doc::StyleRun]) -> bool { + let mut change = vec![0i64; len + 1]; + for run in runs { + let (start, end) = (run.start.min(len), run.end.min(len)); + if start < end { + if let Some(c) = change.get_mut(start) { + *c += 1; + } + if let Some(c) = change.get_mut(end) { + *c -= 1; + } + } + } + let mut open = 0i64; + change.iter().take(len).any(|c| { + open += c; + open <= 0 + }) +} + /// Non-destructive font diagnostic for the action palette, CLI and control API. /// Includes styled runs, not just the default typeface of each text layer. pub(super) fn font_report(doc: &Document) -> Value { @@ -485,21 +528,15 @@ fn font_report_with_families(doc: &Document, installed: &[String]) -> Value { if text.characters.is_empty() { continue; } - let mut covered = vec![false; text.characters.chars().count().min(100_000)]; + let len = text.characters.chars().count().min(100_000); let mut used = BTreeSet::new(); for run in &text.runs { - let start = run.start.min(covered.len()); - let end = run.end.min(covered.len()); - if start < end { - if !run.style.font_family.trim().is_empty() { - used.insert(run.style.font_family.clone()); - } - for entry in covered.iter_mut().take(end).skip(start) { - *entry = true; - } + if run.start.min(len) < run.end.min(len) && !run.style.font_family.trim().is_empty() { + used.insert(run.style.font_family.clone()); } } - if covered.iter().any(|entry| !entry) { + let uncovered = has_uncovered(len, &text.runs); + if uncovered { used.insert(text.style.font_family.clone()); } for family in used { @@ -599,19 +636,14 @@ fn missing_penpot_font_warnings(doc: &Document, families: &[String]) -> Vec"#; + let ids: Vec = (0..2) + .map(|i| { + let id = doc.create(page, NodeKind::Frame, &format!("S{i}"), 0.0, 0.0, 10.0, 10.0).unwrap(); + let layer = doc.node_mut(id).unwrap(); + layer.plugin_data.insert("penpot.svgRawXml".into(), svg.into()); + layer.plugin_data.insert("penpot.svgRawStatus".into(), "pending".into()); + id + }) + .collect(); + let mut warnings = Vec::new(); + expand_penpot_raw_svgs_within(&mut doc, &mut warnings, 1); + let status = |i: usize| doc.node(ids[i]).unwrap().plugin_data.get("penpot.svgRawStatus").cloned(); + assert_eq!(status(0).as_deref(), Some("expanded"), "the first uses the allowance"); + assert_eq!(status(1).as_deref(), Some("not-expanded")); + assert!(doc.node(ids[1]).unwrap().children.is_empty()); + assert!(warnings.iter().any(|w| w.contains("over the limit")), "{warnings:?}"); + } + + #[test] + fn many_overlapping_style_runs_are_summed_once() { + use figcraft_doc::StyleRun; + let mut doc = Document::new("Runs"); + let page = doc.first_page().unwrap(); + let mut t = TextData::new("x".repeat(100_000)); + t.style.font_family = "Unused Base".into(); + let mut run = t.style.clone(); + run.font_family = "Run Font".into(); + t.runs = (0..50_000).map(|_| StyleRun { start: 0, end: 100_000, style: run.clone() }).collect(); + doc.create(page, NodeKind::Text(t), "Long", 0.0, 0.0, 100.0, 30.0).unwrap(); + let report = font_report_with_families(&doc, &[]); + assert_eq!(report["fonts"].as_array().map(Vec::len), Some(1), "{report}"); + assert_eq!(report["fonts"][0]["family"], "Run Font"); + } + #[test] fn expands_inline_svg_into_editable_shapes_without_discarding_source() { let mut doc = Document::new("SVG fixture"); From a8e34e75a8e15b27e5654f417081f0b546d5b76b Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 11 Nov 2026 18:38:59 +0000 Subject: [PATCH 19/23] components: work out each main's layer paths once while linking imported instances --- crates/engine/src/assets/components.rs | 2 +- crates/engine/src/assets/components/rebase.rs | 22 +++++++++++++++++-- crates/engine/src/cmd/file/load.rs | 4 +++- 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/assets/components.rs b/crates/engine/src/assets/components.rs index 7ddb22427..cb273fc51 100644 --- a/crates/engine/src/assets/components.rs +++ b/crates/engine/src/assets/components.rs @@ -60,7 +60,7 @@ pub use patch::sync_after_edit; pub(crate) use patch::{FULL_REBUILD, PATCHED}; #[cfg(test)] use rebase::RebaseSources; -pub use rebase::rebase_instance; +pub use rebase::{MainPaths, rebase_instance, rebase_instance_with}; pub use sync::keep_text_overrides; pub use variants::{default_variant, exposed_copies, find_variant, mirror_issues, variant_conflicts, variant_name, variant_options, variant_props}; diff --git a/crates/engine/src/assets/components/rebase.rs b/crates/engine/src/assets/components/rebase.rs index ea9cb6f3e..aa782cb20 100644 --- a/crates/engine/src/assets/components/rebase.rs +++ b/crates/engine/src/assets/components/rebase.rs @@ -165,10 +165,28 @@ impl<'a> RebaseSources<'a> { /// values (after detaching an outer instance, restoring a main, or copying a nested instance out): /// the look is kept exactly. Values a component property sets aren't overrides. pub fn rebase_instance(doc: &mut Document, inst: NodeId) { + rebase_instance_with(doc, inst, &mut MainPaths::new()); +} + +/// The layers of mains by path, kept across several [`rebase_instance_with`] calls. Rebasing only +/// writes instances' `source` and `overrides`, never a main's layers or names, so a main's paths +/// stay right for as long as the caller changes nothing else. +pub type MainPaths = BTreeMap>>; + +/// [`rebase_instance`], taking each main's path map from `mains` (and adding it there). +pub fn rebase_instance_with(doc: &mut Document, inst: NodeId, mains: &mut MainPaths) { let Some(NodeKind::Instance { component, .. }) = doc.node(inst).map(|n| n.kind.clone()) else { return }; let mine = path_map(doc, inst, true); - let theirs: BTreeMap = - if doc.contains(component) { path_map(doc, component, false).into_iter().map(|(k, v)| (v, k)).collect() } else { BTreeMap::new() }; + let theirs = mains + .entry(component) + .or_insert_with(|| { + std::rc::Rc::new(if doc.contains(component) { + path_map(doc, component, false).into_iter().map(|(k, v)| (v, k)).collect() + } else { + BTreeMap::new() + }) + }) + .clone(); let value_groups = |a: &Node, b: &Node| -> Vec { diff_groups(a, b).into_iter().filter(|g| !matches!(*g, "size" | "position")).map(str::to_string).collect() }; diff --git a/crates/engine/src/cmd/file/load.rs b/crates/engine/src/cmd/file/load.rs index 3e917d5dd..d03921653 100644 --- a/crates/engine/src/cmd/file/load.rs +++ b/crates/engine/src/cmd/file/load.rs @@ -652,8 +652,10 @@ fn link_instances(doc: &mut Document) { let mut insts: Vec<(usize, NodeId)> = doc.nodes().filter(|n| matches!(n.kind, NodeKind::Instance { .. })).map(|n| (doc.ancestors(n.id).len(), n.id)).collect(); insts.sort_by_key(|a| std::cmp::Reverse(a.0)); + // Rebasing leaves the mains alone, so each main's layer paths are worked out once. + let mut mains = crate::components::MainPaths::new(); for (_, i) in insts { - crate::components::rebase_instance(doc, i); + crate::components::rebase_instance_with(doc, i, &mut mains); } } From 70ed6c1636b1560f8f17d8471f1c2661d06130b7 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 11 Nov 2026 18:53:53 +0000 Subject: [PATCH 20/23] svg import: budget attribute work over the whole file and DTD, map span offsets in one pass --- crates/svg/src/import.rs | 42 ++++++++++++++++++++------ crates/svg/src/import/hostile_tests.rs | 12 ++++++++ crates/svg/src/import/text.rs | 33 +++++++++++++++----- crates/svg/src/import/text_tests.rs | 2 +- 4 files changed, 71 insertions(+), 18 deletions(-) diff --git a/crates/svg/src/import.rs b/crates/svg/src/import.rs index f77b10bfa..fbdb9b2c1 100644 --- a/crates/svg/src/import.rs +++ b/crates/svg/src/import.rs @@ -714,18 +714,26 @@ pub(crate) fn nesting(data: &[u8], limit: usize) -> usize { scan(data, limit).0 } -/// Whether an element of `data` has more than [`MAX_ATTRIBUTES`] attributes (counted as quoted -/// values, before the parser sees them). +/// The most the parser's duplicate-attribute checks may cost over a whole document, as the sum of +/// each element's attribute count squared (a real file with 100,000 elements of 20 attributes is +/// 40 million). +const MAX_ATTRIBUTE_WORK: usize = 1 << 28; + +/// Whether an element of `data` has more than [`MAX_ATTRIBUTES`] attributes, or all of them cost +/// more than [`MAX_ATTRIBUTE_WORK`] (counted as quoted values, before the parser sees them; the +/// declarations of a DTD count as one element, as entities can hold elements). pub(crate) fn too_wide(data: &[u8]) -> bool { - scan(data, usize::MAX).1 > MAX_ATTRIBUTES + let (_, widest, work) = scan(data, usize::MAX); + widest > MAX_ATTRIBUTES || work > MAX_ATTRIBUTE_WORK } -/// [`nesting`] and the most attribute values on one element; stops once either is past its limit. -fn scan(data: &[u8], limit: usize) -> (usize, usize) { +/// [`nesting`], the most attribute values on one element and the attribute work (see +/// [`MAX_ATTRIBUTE_WORK`]); stops once any is past its limit. +fn scan(data: &[u8], limit: usize) -> (usize, usize, usize) { let find = |from: usize, pat: &[u8]| -> usize { data.get(from..).and_then(|r| r.windows(pat.len()).position(|w| w == pat)).map_or(data.len(), |p| from + p + pat.len()) }; - let (mut depth, mut max, mut i, mut widest) = (0usize, 0usize, 0usize, 0usize); + let (mut depth, mut max, mut i, mut widest, mut work) = (0usize, 0usize, 0usize, 0usize, 0usize); while let Some(off) = data.get(i..).and_then(|r| r.iter().position(|b| *b == b'<')) { i += off; let rest = data.get(i..).unwrap_or(&[]); @@ -733,6 +741,19 @@ fn scan(data: &[u8], limit: usize) -> (usize, usize) { i = find(i + 4, b"-->"); } else if rest.starts_with(b""); + } else if rest.starts_with(b"` closing its subset (the first `>` when it has none). + let end = match (rest.iter().position(|b| *b == b'['), rest.iter().position(|b| *b == b'>')) { + (Some(o), Some(g)) if o < g => find(i, b"]>"), + _ => find(i, b">"), + }; + let quotes = data.get(i..end).unwrap_or(&[]).iter().filter(|b| matches!(**b, b'"' | b'\'')).count(); + widest = widest.max(quotes / 2); + work = work.saturating_add((quotes / 2).saturating_mul(quotes / 2)); + if widest > MAX_ATTRIBUTES || work > MAX_ATTRIBUTE_WORK { + return (max, widest, work); + } + i = end; } else if rest.starts_with(b""); } else if rest.starts_with(b" (usize, usize) { j += 1; } widest = widest.max(attrs); - if widest > MAX_ATTRIBUTES { - return (max, widest); + work = work.saturating_add(attrs * attrs); + if widest > MAX_ATTRIBUTES || work > MAX_ATTRIBUTE_WORK { + return (max, widest, work); } if !closed { depth += 1; max = max.max(depth); if max > limit { - return (max, widest); + return (max, widest, work); } } i = j + 1; } } - (max, widest) + (max, widest, work) } /// Whether `data` nests elements more than [`MAX_NESTING`] deep: the byte scan first, then the diff --git a/crates/svg/src/import/hostile_tests.rs b/crates/svg/src/import/hostile_tests.rs index 16731407c..daeaa085a 100644 --- a/crates/svg/src/import/hostile_tests.rs +++ b/crates/svg/src/import/hostile_tests.rs @@ -193,3 +193,15 @@ fn an_element_with_thousands_of_attributes_is_refused_before_parsing() { let p = d.first_page().unwrap(); assert!(import_svg(&mut d, &svg(&format!("", many(100_000))), p, 0.0, 0.0, "x").is_err()); } + +#[test] +fn attribute_work_is_bounded_over_the_whole_document_and_its_dtd() { + use super::too_wide; + let many = |n: usize| (0..n).map(|i| format!(" a{i}=\"1\"")).collect::(); + let tag = format!("", many(4000)); + assert!(!too_wide(format!("{}", tag.repeat(10)).as_bytes())); + assert!(too_wide(format!("{}", tag.repeat(40)).as_bytes()), "each element is within the limit, all together are not"); + let dtd = format!("'>]>&e;", many(5000)); + assert!(too_wide(dtd.as_bytes()), "entities can hold elements"); + assert!(!too_wide(b"]>")); +} diff --git a/crates/svg/src/import/text.rs b/crates/svg/src/import/text.rs index 7ed8d018f..6f557fb13 100644 --- a/crates/svg/src/import/text.rs +++ b/crates/svg/src/import/text.rs @@ -195,7 +195,7 @@ pub(crate) fn text(cx: &mut Ctx, t: &usvg::Text, parent: NodeId, base: Affine, d let x = chunk.x().map_or(pen_x, f64::from) + d(t.dx()); let y = chunk.y().map_or(pen_y, f64::from) + d(t.dy()); char_at += count; - let ci = char_index(text); + let ci = char_index(text, chunk.spans().iter().flat_map(|sp| [sp.start(), sp.end()])); let spans: Vec<_> = chunk.spans().iter().map(|sp| (ci(sp.start()), ci(sp.end()), sp, span_style(sp, s, fonts))).collect(); if spans.is_empty() { continue; @@ -229,11 +229,30 @@ pub(crate) fn text(cx: &mut Ctx, t: &usvg::Text, parent: NodeId, base: Affine, d } } -/// The characters of `text` before a byte offset (all of them for an offset that is past the end -/// or inside a character), by a search in a table built once: many spans don't rescan the prefix. -fn char_index(text: &str) -> impl Fn(usize) -> usize + '_ { - let starts: Vec = text.char_indices().map(|(i, _)| i).collect(); - move |b| if text.is_char_boundary(b) { starts.partition_point(|s| *s < b) } else { starts.len() } +/// The characters of `text` before each of the byte `offsets` (all of them for an offset that is +/// past the end or inside a character): one pass over the text, whatever the number of spans. +fn char_index(text: &str, offsets: impl IntoIterator) -> impl Fn(usize) -> usize { + let mut want: Vec = offsets.into_iter().collect(); + want.sort_unstable(); + want.dedup(); + let mut found: Vec<(usize, usize)> = Vec::with_capacity(want.len()); + let mut wanted = want.into_iter().peekable(); + let mut count = 0; + for (i, _) in text.char_indices() { + while let Some(&o) = wanted.peek() { + match o.cmp(&i) { + std::cmp::Ordering::Equal => found.push((o, count)), + std::cmp::Ordering::Greater => break, + std::cmp::Ordering::Less => {} + } + wanted.next(); + } + count += 1; + } + if wanted.any(|o| o == text.len()) { + found.push((text.len(), count)); + } + move |b| found.binary_search_by_key(&b, |f| f.0).ok().and_then(|k| found.get(k)).map_or(count, |f| f.1) } /// The text of `lines` (one per line, stacked at their pitch; `s`: their scale): characters, the @@ -269,7 +288,7 @@ fn on_path(cx: &mut Ctx, t: &usvg::Text, world: Affine, parent: NodeId) -> bool let Some(len) = figcraft_geom::measure::polylines(&path).and_then(|l| l.first().map(|l| l.length())).filter(|l| *l > 1e-9) else { return false }; let fonts = cx.fonts.get_or_insert_with(Fonts::load); let text = chunk.text(); - let ci = char_index(text); + let ci = char_index(text, chunk.spans().iter().flat_map(|sp| [sp.start(), sp.end()])); let spans: Vec<_> = chunk.spans().iter().map(|sp| (ci(sp.start()), ci(sp.end()), sp, span_style(sp, 1.0, fonts))).collect(); let line = Line { x: 0.0, y: 0.0, anchor: chunk.anchor(), text: text.to_string(), spans }; let mut td = text_data(std::slice::from_ref(&line), 1.0); diff --git a/crates/svg/src/import/text_tests.rs b/crates/svg/src/import/text_tests.rs index 94c6ae5b2..c43bd7f9e 100644 --- a/crates/svg/src/import/text_tests.rs +++ b/crates/svg/src/import/text_tests.rs @@ -34,7 +34,7 @@ fn css_system_font_names_keep_their_name_where_the_system_font_is() { #[test] fn char_index_counts_the_characters_before_a_byte_offset() { let t = "aé😀b"; - let ci = super::char_index(t); + let ci = super::char_index(t, [0, 1, 3, 7, 8, 2, 4, 99]); assert_eq!([ci(0), ci(1), ci(3), ci(7), ci(8)], [0, 1, 2, 3, 4]); assert_eq!([ci(2), ci(4), ci(99)], [4, 4, 4], "inside a character or past the end: all of them"); } From 9d48625995fc63c99e0164e424f78bcea3ac8a47 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 11 Nov 2026 18:57:25 +0000 Subject: [PATCH 21/23] layout: leave out a grid that would pass the snap budget --- crates/layout/src/depth_tests.rs | 2 +- crates/layout/src/lib.rs | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/crates/layout/src/depth_tests.rs b/crates/layout/src/depth_tests.rs index b533c7e42..20e022817 100644 --- a/crates/layout/src/depth_tests.rs +++ b/crates/layout/src/depth_tests.rs @@ -102,6 +102,6 @@ fn snap_lines_are_bounded_across_a_frames_grids() { let f = d.create(p, NodeKind::Frame, "F", 0.0, 0.0, 4000.0, 4000.0).unwrap(); d.get_mut(f).unwrap().grids = vec![LayoutGrid { kind: GridKind::Grid, size: 1.0, visible: true, ..LayoutGrid::default() }; 1000]; let (xs, ys) = crate::grid_snap_lines(&d, f); - assert!(xs.len() + ys.len() < 250_000, "{}", xs.len() + ys.len()); + assert!(xs.len() + ys.len() <= 200_000, "{}", xs.len() + ys.len()); assert!(xs.len() > 4000 && ys.len() > 4000, "the first grids still give their lines"); } diff --git a/crates/layout/src/lib.rs b/crates/layout/src/lib.rs index 04abbc39a..b3d8e27f0 100644 --- a/crates/layout/src/lib.rs +++ b/crates/layout/src/lib.rs @@ -1169,11 +1169,13 @@ pub fn grid_snap_lines_in(doc: &Document, frame: NodeId, xf: Affine) -> (Vec= MAX_SNAP_LINES { + // A frame-wide budget: each grid adds up to ~17,000 lines, and a file can hold any number of grids; + // a grid that would pass it is left out whole. + let lines = grid_lines(g, f.width, f.height); + if xs.len() + ys.len() + 2 * lines.len() > MAX_SNAP_LINES { break; } - for r in grid_lines(g, f.width, f.height) { + for r in lines { let a = xf * Point::new(r.x0, r.y0); let b = xf * Point::new(r.x1, r.y1); match g.kind { From 2cfd2a3531e7df2dae4d9821b1b733756b659eb0 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 11 Nov 2026 19:16:24 +0000 Subject: [PATCH 22/23] collab-net: hand the sync client the bounded roster; assert the kept prefixes --- crates/collab-net/src/client_cap_tests.rs | 26 +++++++++++++++++++++-- crates/collab-net/src/join.rs | 4 ++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/crates/collab-net/src/client_cap_tests.rs b/crates/collab-net/src/client_cap_tests.rs index 73f181774..5af7c7a2f 100644 --- a/crates/collab-net/src/client_cap_tests.rs +++ b/crates/collab-net/src/client_cap_tests.rs @@ -4,7 +4,7 @@ use std::sync::{Arc, Mutex}; -use figcraft_collab::message::{Peer, Presence, ServerMsg}; +use figcraft_collab::message::{Access, CatchUp, Peer, Presence, Processed, ServerMsg}; use figcraft_engine::live::{Event, MAX_PEERS}; use crate::guest::GuestConfig; @@ -69,7 +69,29 @@ fn a_hosts_long_names_and_colours_are_cut_where_they_are_kept() { let long = "x".repeat(100_000); take(ServerMsg::PeerJoined(Peer { client_id: 1, name: long.clone(), color: long }), &cfg, &inbox, &mut seen, &resume, &|| {}); match inbox.drain().as_slice() { - [Event::PeerJoined(p)] => assert!(p.name.chars().count() <= 256 && p.color.chars().count() <= 32), + [Event::PeerJoined(p)] => assert_eq!((p.name.as_str(), p.color.as_str()), ("x".repeat(256).as_str(), "x".repeat(32).as_str())), other => panic!("{other:?}"), } } + +#[test] +fn the_welcome_handed_to_the_sync_client_carries_the_bounded_roster() { + let inbox = inbox(); + let cfg = GuestConfig::new(Invite::new("127.0.0.1:47900", "0123456789abcdef", "AbCdEfGhIjKlMnOpQrStUv"), "Ada".into()); + let resume = Mutex::new(None); + let long = "y".repeat(100_000); + let welcome = ServerMsg::Welcome { + session: 1, + client_id: 9, + seq: 0, + catch_up: CatchUp::Snapshot(Vec::new()), + processed: Processed::default(), + peers: vec![Peer { client_id: 1, name: long.clone(), color: long }], + access: Access::default(), + secret: 0, + }; + take(welcome, &cfg, &inbox, &mut Seen::default(), &resume, &|| {}); + let events = inbox.drain(); + let Some(Event::Sync(ServerMsg::Welcome { peers, .. })) = events.iter().find(|e| matches!(e, Event::Sync(_))) else { panic!("{events:?}") }; + assert_eq!(peers.first().map(|p| (p.name.chars().count(), p.color.chars().count())), Some((256, 32))); +} diff --git a/crates/collab-net/src/join.rs b/crates/collab-net/src/join.rs index 5982ed6e8..b5de889f9 100644 --- a/crates/collab-net/src/join.rs +++ b/crates/collab-net/src/join.rs @@ -281,6 +281,10 @@ pub(crate) fn take(mut m: ServerMsg, cfg: &GuestConfig, inbox: &Inbox, seen: &mu view: None, }; seen.peers = peers.clone(); + // The sync client keeps the roster of the welcome it is handed: the bounded one. + if let ServerMsg::Welcome { peers: sent, .. } = &mut m { + sent.clone_from(&peers); + } seen.info = Some(info.clone()); welcomed(); inbox.push(Event::Welcome { info, peers }); From cf77dfdabc040b98e2958429e40ad495dfd15659 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 11 Nov 2026 21:31:26 +0000 Subject: [PATCH 23/23] load: an SVG that doesn't fit the allowance is taken out again; charge after cleanup --- crates/engine/src/cmd/file/load.rs | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/cmd/file/load.rs b/crates/engine/src/cmd/file/load.rs index 77dd68198..da03ca1d1 100644 --- a/crates/engine/src/cmd/file/load.rs +++ b/crates/engine/src/cmd/file/load.rs @@ -438,7 +438,6 @@ fn expand_penpot_raw_svgs_within(doc: &mut Document, notes: &mut Vec, mu } let before = doc.node_count(); let imported = figcraft_svg::import_svg_with_notes(doc, xml.as_bytes(), id, 0.0, 0.0, &name); - room = room.saturating_sub(doc.node_count().saturating_sub(before)); match imported { Ok(imported) => { // An SVG can be syntactically valid yet contain no supported drawable @@ -453,6 +452,21 @@ fn expand_penpot_raw_svgs_within(doc: &mut Document, notes: &mut Vec, mu } continue; } + // Charged once the empty ones are gone; one that doesn't fit is taken out again. + let added = doc.node_count().saturating_sub(before); + if added > room { + let _ = doc.remove(imported.frame); + if let Some(n) = doc.node_mut(id) { + n.plugin_data.insert("penpot.svgRawStatus".into(), "not-expanded".into()); + } + if notes.len() < 100 { + notes.push(format!( + "Penpot SVG '{name}': not converted, the file's SVG layers are over the limit; the original markup is preserved" + )); + } + continue; + } + room -= added; let size = doc.node(imported.frame).map(|n| (n.width, n.height)); if let Some((svg_w, svg_h)) = size && svg_w.is_finite() @@ -837,7 +851,9 @@ mod penpot_svg_v10_tests { }) .collect(); let mut warnings = Vec::new(); - expand_penpot_raw_svgs_within(&mut doc, &mut warnings, 1); + let nodes = doc.node_count(); + expand_penpot_raw_svgs_within(&mut doc, &mut warnings, 4); + assert!(doc.node_count() - nodes <= 4, "never more layers than the allowance"); let status = |i: usize| doc.node(ids[i]).unwrap().plugin_data.get("penpot.svgRawStatus").cloned(); assert_eq!(status(0).as_deref(), Some("expanded"), "the first uses the allowance"); assert_eq!(status(1).as_deref(), Some("not-expanded"));