From 87ce9612e6f2225ab1d288fe06edc89caf052475 Mon Sep 17 00:00:00 2001 From: Trevor Brown Date: Sun, 8 Nov 2026 01:44:23 -0500 Subject: [PATCH] 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("