From 67a278b053453193d2e961ed97ffb8bbcc2b44cb Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Tue, 13 Oct 2026 17:37:59 -0700 Subject: [PATCH] Commands: ids leave out an object listed with a group that holds it, as the selection does Commands that read the selection through edit::selected_roots leave out an object whose group is also selected (edit::roots_of). Fourteen places that read `ids` kept an object listed together with its group. Given ids that list an object and a group that holds it, Make Repeat failed with "no such object", New Symbol failed or stored the object twice in the symbol, and object.transform moved it twice (20 pt for a 10 pt move). These places now pass explicit ids through edit::outermost, which drops an id whose ancestor is also listed, with the filter that roots_of runs on the selection (now edit::without_inner). Nothing else is dropped or replaced: unlike roots_of, it keeps layers and compound-path members. A list without nesting gives the same result as before. Commands that take both ids and the selection as listed, such as paint.setFill without item, are unchanged. Docs: docs/mcp.md, docs/target-app-parity.md. --- crates/engine/src/cmd/appearance.rs | 12 +-- crates/engine/src/cmd/brushsym.rs | 6 +- crates/engine/src/cmd/buildcmds.rs | 4 +- crates/engine/src/cmd/distortcmds.rs | 7 +- crates/engine/src/cmd/docinfo.rs | 3 +- crates/engine/src/cmd/edit.rs | 28 ++++-- crates/engine/src/cmd/gradient.rs | 4 +- crates/engine/src/cmd/mod.rs | 5 +- crates/engine/src/cmd/object.rs | 4 +- crates/engine/src/cmd/opacitymask.rs | 2 +- crates/engine/src/cmd/patterncmds.rs | 4 +- crates/engine/src/cmd/slices.rs | 2 +- crates/engine/src/cmd/xform.rs | 5 +- crates/engine/src/tests_brushsym.rs | 32 +++++++ .../engine/src/tests_ids_without_selection.rs | 86 +++++++++++++++++++ crates/engine/src/tests_large_edits.rs | 12 +-- crates/engine/src/tests_pattern.rs | 29 +++++++ docs/mcp.md | 3 +- docs/target-app-parity.md | 1 + 19 files changed, 208 insertions(+), 41 deletions(-) diff --git a/crates/engine/src/cmd/appearance.rs b/crates/engine/src/cmd/appearance.rs index 71f23b1c6..7c9f6337c 100644 --- a/crates/engine/src/cmd/appearance.rs +++ b/crates/engine/src/cmd/appearance.rs @@ -22,7 +22,7 @@ use vectorcraft_color::{BlendMode, Paint}; use vectorcraft_doc::{Appearance, AppearanceItem, CharStyle, FillLayer, Node, NodeKind, ParaStyle, StrokeLayer}; use vectorcraft_geom::Rect; -use super::edit::selected_roots; +use super::edit::{outermost, selected_roots}; use super::opacitymask::percent; use super::paint::paint_from; use super::*; @@ -294,15 +294,15 @@ pub(crate) fn subject_roots(s: &Session) -> Result> { } } -/// Objects whose own appearance stack a command edits: `ids`/`id` (layers too), else the -/// [`subject_roots`]; with `target: "contents"` the painted objects inside the groups and layers -/// among them ([`leaf_targets`]). +/// Objects whose own appearance stack a command edits: `ids`/`id` (layers too; an id inside +/// another given one is dropped), else the [`subject_roots`]; with `target: "contents"` the painted +/// objects inside the groups and layers among them ([`leaf_targets`]). pub(crate) fn appearance_targets(s: &Session, p: &Value) -> Result> { let roots = if p.get("ids").is_none() && p.get("id").is_none() { subject_roots(s)? } else { let d = &s.doc()?.doc; - targets(s, p)?.into_iter().filter(|id| d.node(*id).is_some()).collect() + outermost(d, targets(s, p)?.into_iter().filter(|id| d.node(*id).is_some()).collect()) }; if targets_contents(p)? { leaf_targets(s, &roots) } else { Ok(roots) } } @@ -745,7 +745,7 @@ fn copy_from(s: &mut Session, p: &Value) -> Result { let (reverse, append) = (bool_or(p, "reverse", false), bool_or(p, "append", false)); let attrs = s.prefs.eyedropper.attrs(p).map_err(|e| bad(C, e))?; let selected = match ids_param(p, "ids") { - Some(v) => v, + Some(v) => outermost(&s.doc()?.doc, v), None => selected_roots(s)?, }; // Reversed, the first selected object's attributes go onto the clicked object. diff --git a/crates/engine/src/cmd/brushsym.rs b/crates/engine/src/cmd/brushsym.rs index a1d3e09b6..d72b3414e 100644 --- a/crates/engine/src/cmd/brushsym.rs +++ b/crates/engine/src/cmd/brushsym.rs @@ -14,7 +14,7 @@ use vectorcraft_color::{Color, Paint}; use vectorcraft_doc::{AppearanceItem, Document, FillLayer, Node, NodeId, NodeKind, Symbol, SymbolRegistration}; use vectorcraft_geom::{Affine, Point, Vec2}; -use super::edit::selected_roots; +use super::edit::{outermost, selected_roots}; use super::pathops::{remove_each, separate}; use super::*; use crate::EngineError; @@ -198,11 +198,11 @@ fn ids_json(ids: &[NodeId]) -> Value { fn roots(s: &Session, p: &Value) -> Result> { match ids_param(p, "ids").or_else(|| id_param(p, "id").map(|i| vec![i])) { Some(mut v) => { - // Explicit ids: only existing, non-layer objects. + // Explicit ids: only existing, non-layer objects, none inside another of them. let d = &s.doc()?.doc; v.retain(|id| d.node(*id).is_some_and(|n| !n.is_layer())); v.dedup(); - Ok(v) + Ok(outermost(d, v)) } None => selected_roots(s), } diff --git a/crates/engine/src/cmd/buildcmds.rs b/crates/engine/src/cmd/buildcmds.rs index 7eeacbbf2..b3b232b34 100644 --- a/crates/engine/src/cmd/buildcmds.rs +++ b/crates/engine/src/cmd/buildcmds.rs @@ -24,7 +24,7 @@ use vectorcraft_tools::builder::{ }; use vectorcraft_trace as tr; -use super::edit::selected_roots; +use super::edit::{outermost, selected_roots}; use super::paint::paint_from; use super::pathops::{Beside, shape_node}; use super::*; @@ -192,7 +192,7 @@ fn root_ids(s: &Session, p: &Value) -> Result> { if ids.is_empty() { return Err(EngineError::Other("select objects, not layers".into())); } - Ok(sorted_roots(d, &ids)) + Ok(outermost(d, sorted_roots(d, &ids))) } fn points_param(p: &Value, key: &str) -> Option> { diff --git a/crates/engine/src/cmd/distortcmds.rs b/crates/engine/src/cmd/distortcmds.rs index 669c15d43..689f1dbb5 100644 --- a/crates/engine/src/cmd/distortcmds.rs +++ b/crates/engine/src/cmd/distortcmds.rs @@ -13,7 +13,7 @@ use vectorcraft_tools::distort::liquify::{Dabber, LiquifyParams, PathStroke, Sam use vectorcraft_tools::distort::perspective::{self as persp, PerspectiveGrid, Plane}; use vectorcraft_tools::distort::{PinSet, arap, collect_points, mesh_for, project_node, stroke_owner, warp_from_rest, warp_node_with}; -use super::edit::{duplicate_in, selected_roots}; +use super::edit::{duplicate_in, outermost, selected_roots}; use super::*; use crate::EngineError; @@ -706,7 +706,8 @@ fn puppet_warp(s: &mut Session, p: &Value) -> Result { if pins.iter().chain(&moved).any(|q| !q.x.is_finite() || !q.y.is_finite()) { return Err(bad(C, "pins must be finite")); } - let ids = if rest { puppet_ids(p, || Ok(s.doc()?.selection.objects.clone()))? } else { puppet_ids(p, || selected_roots(s))? }; + let ids = + if rest { puppet_ids(p, || Ok(s.doc()?.selection.objects.clone()))? } else { outermost(&s.doc()?.doc, puppet_ids(p, || selected_roots(s))?) }; let expand = f64_or(p, "expand", 3.0).clamp(0.0, 1000.0); let angles: Vec> = match p.get("angles") { None | Some(Value::Null) => vec![None; pins.len()], @@ -846,7 +847,7 @@ fn plane_param(g: &PerspectiveGrid, p: &Value, cmd: &str) -> Result { fn roots(s: &Session, p: &Value) -> Result> { match ids_param(p, "ids").or_else(|| id_param(p, "id").map(|i| vec![i])) { - Some(v) => Ok(v), + Some(v) => Ok(outermost(&s.doc()?.doc, v)), None => selected_roots(s), } } diff --git a/crates/engine/src/cmd/docinfo.rs b/crates/engine/src/cmd/docinfo.rs index 8f55e845e..65fc0d2f0 100644 --- a/crates/engine/src/cmd/docinfo.rs +++ b/crates/engine/src/cmd/docinfo.rs @@ -372,7 +372,8 @@ fn snap_xf(b: Rect, half: bool) -> Affine { } fn pixel_perfect(s: &mut Session, p: &Value) -> Result { - let ids: Vec = if p.get("ids").is_some() { targets(s, p)? } else { super::edit::selected_roots(s)? }; + let ids: Vec = + if p.get("ids").is_some() { super::edit::outermost(&s.doc()?.doc, targets(s, p)?) } else { super::edit::selected_roots(s)? }; let n = ids.len(); s.edit("Make Pixel Perfect", |d, _| { for id in &ids { diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index 92e64de6a..89946fb09 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -342,17 +342,31 @@ pub(crate) fn roots_of(doc: &Document, ids: Vec) -> Vec { }) .collect(); sel.dedup_by_key(|(id, _)| *id); - let listed: std::collections::HashSet = sel.iter().map(|(id, _)| *id).collect(); - sel.iter() - .filter(|(_, path)| { - let anc = path.as_deref().and_then(|p| doc.ancestry_at(p)).unwrap_or_default(); - !anc[..anc.len().saturating_sub(1)].iter().any(|a| listed.contains(a)) - }) + without_inner(doc, sel) .filter(|(_, path)| path.as_deref().and_then(|p| doc.node_at(p)).is_some_and(|n| !n.is_layer())) - .map(|(id, _)| *id) + .map(|(id, _)| id) .collect() } +/// `ids` in the order given, less those whose ancestor is also listed: an object listed with a +/// group that holds it is dropped, as [`roots_of`] drops it from a selection. Nothing else is +/// dropped or replaced: unlike `roots_of`, it keeps layers, ids that aren't in the document and +/// compound-path members. +pub(crate) fn outermost(doc: &Document, ids: Vec) -> Vec { + let paths = doc.index_paths(ids.iter().copied()); + let placed = ids.into_iter().map(|id| (id, paths.get(&id).cloned())).collect(); + without_inner(doc, placed).map(|(id, _)| id).collect() +} + +/// The entries of `placed` (ids with their index paths) whose id has no ancestor among its ids. +fn without_inner(doc: &Document, placed: Vec<(NodeId, Option>)>) -> impl Iterator>)> + '_ { + let listed: std::collections::HashSet = placed.iter().map(|(id, _)| *id).collect(); + placed.into_iter().filter(move |(_, path)| { + let anc = path.as_deref().and_then(|p| doc.ancestry_at(p)).unwrap_or_default(); + !anc[..anc.len().saturating_sub(1)].iter().any(|a| listed.contains(a)) + }) +} + fn copy(s: &mut Session, _: &Value) -> Result { // The Artboard tool copies its artboard (and the art on it). if s.tool_id() == "artboard" { diff --git a/crates/engine/src/cmd/gradient.rs b/crates/engine/src/cmd/gradient.rs index be1f4da62..becbbf13b 100644 --- a/crates/engine/src/cmd/gradient.rs +++ b/crates/engine/src/cmd/gradient.rs @@ -8,7 +8,7 @@ use vectorcraft_geom::{Affine, Point, Rect}; use vectorcraft_tools::params::color_json; use super::appearance::{ItemTarget, edit_items, edits_stroke, item_target}; -use super::edit::selected_roots; +use super::edit::{outermost, selected_roots}; use super::paint::swatch_solid; use super::*; use crate::EngineError; @@ -512,7 +512,7 @@ fn set_gradient_geom(s: &mut Session, p: &Value) -> Result { let stroke = edits_stroke(s, p, item, !s.fill_active)?; let item = item.of_kind(s, !stroke); let targets = match item { - ItemTarget::Top => leaf_targets(s, &ids_param(p, "ids").map_or_else(|| selected_roots(s), Ok)?)?, + ItemTarget::Top => leaf_targets(s, &ids_param(p, "ids").map_or_else(|| selected_roots(s), |ids| Ok(outermost(&s.doc()?.doc, ids)))?)?, _ => item.targets(s, p)?, }; if targets.is_empty() { diff --git a/crates/engine/src/cmd/mod.rs b/crates/engine/src/cmd/mod.rs index 490548e5c..f4a534953 100644 --- a/crates/engine/src/cmd/mod.rs +++ b/crates/engine/src/cmd/mod.rs @@ -340,9 +340,10 @@ pub(crate) fn checked_ids_param(s: &Session, p: &Value, key: &str, command: &str values.iter().map(|value| checked_id(s, value, command)).collect() } -/// The objects a command acts on: `ids` when given (every one checked), else the selected objects. +/// The objects a command acts on: `ids` when given (every one checked, and those inside another +/// given one dropped), else the selected objects. pub(crate) fn checked_ids_or_selection(s: &Session, p: &Value, command: &str) -> Result> { - if p.get("ids").is_some() { checked_ids_param(s, p, "ids", command) } else { edit::selected_roots(s) } + if p.get("ids").is_some() { Ok(edit::outermost(&s.doc()?.doc, checked_ids_param(s, p, "ids", command)?)) } else { edit::selected_roots(s) } } pub(crate) fn checked_id(s: &Session, value: &Value, command: &str) -> Result { diff --git a/crates/engine/src/cmd/object.rs b/crates/engine/src/cmd/object.rs index 3e33c9a33..8620ce988 100644 --- a/crates/engine/src/cmd/object.rs +++ b/crates/engine/src/cmd/object.rs @@ -11,7 +11,7 @@ use vectorcraft_doc::{Appearance, Document, Knockout, LiveCorners, LiveShape, No use vectorcraft_geom::shapes::CornerKind; use vectorcraft_geom::{Affine, FillRule, PathData, Point, Rect, Vec2}; -use super::edit::{duplicate_in, selected_roots}; +use super::edit::{duplicate_in, outermost, selected_roots}; use super::opacitymask::percent; use super::*; use crate::{EngineError, LastTransform}; @@ -492,7 +492,7 @@ fn resize_type_area(n: &mut Node, xf: Affine) -> bool { fn transform(s: &mut Session, p: &Value) -> Result { let m = matrix_param(p, "matrix").ok_or_else(|| bad("object.transform", "missing matrix [a,b,c,d,e,f]"))?; let (ids, anchors) = match ids_param(p, "ids") { - Some(v) => (v, false), + Some(v) => (outermost(&s.doc()?.doc, v), false), None => (selected_roots(s)?, bool_or(p, "anchors", false)), }; apply_transform(s, "Transform", ids, m, p, anchors) diff --git a/crates/engine/src/cmd/opacitymask.rs b/crates/engine/src/cmd/opacitymask.rs index bca66fc73..de1bf95ab 100644 --- a/crates/engine/src/cmd/opacitymask.rs +++ b/crates/engine/src/cmd/opacitymask.rs @@ -173,7 +173,7 @@ fn make(s: &mut Session, p: &Value) -> Result { if let Some(id) = ids.iter().find(|id| d.node(**id).is_none()) { return Err(EngineError::NoNode(*id)); } - Selection { objects: ids, ..Default::default() }.in_paint_order(d) + super::edit::outermost(d, Selection { objects: ids, ..Default::default() }.in_paint_order(d)) } None => super::appearance::subject_roots(s)?, }; diff --git a/crates/engine/src/cmd/patterncmds.rs b/crates/engine/src/cmd/patterncmds.rs index 319dae72d..c390475c7 100644 --- a/crates/engine/src/cmd/patterncmds.rs +++ b/crates/engine/src/cmd/patterncmds.rs @@ -17,7 +17,7 @@ use vectorcraft_doc::pattern::{self, Overlap, PATTERN_EDIT_LAYER, PatternDef, Pa use vectorcraft_doc::{Document, Node, NodeId, NodeKind, Selection}; use vectorcraft_geom::Rect; -use super::edit::selected_roots; +use super::edit::{outermost, selected_roots}; use super::pathops::Beside; use super::*; @@ -203,7 +203,7 @@ fn roots(s: &Session, p: &Value) -> Result> { let st = s.doc()?; let mut sel = Selection::default(); sel.set(ids.iter().copied().filter(|id| st.doc.node(*id).is_some_and(|n| !n.is_layer()))); - Ok(sel.in_paint_order(&st.doc)) + Ok(outermost(&st.doc, sel.in_paint_order(&st.doc))) } None => selected_roots(s), } diff --git a/crates/engine/src/cmd/slices.rs b/crates/engine/src/cmd/slices.rs index 8668ddd94..2a814a55e 100644 --- a/crates/engine/src/cmd/slices.rs +++ b/crates/engine/src/cmd/slices.rs @@ -280,7 +280,7 @@ pub(crate) fn options_json(o: &SliceOptions) -> Value { fn make(s: &mut Session, p: &Value) -> Result { let ids = match ids_param(p, "ids") { - Some(ids) => ids, + Some(ids) => super::edit::outermost(&s.doc()?.doc, ids), None => super::edit::selected_roots(s)?, }; let d = &s.doc()?.doc; diff --git a/crates/engine/src/cmd/xform.rs b/crates/engine/src/cmd/xform.rs index e937c1d0f..f5a05c55e 100644 --- a/crates/engine/src/cmd/xform.rs +++ b/crates/engine/src/cmd/xform.rs @@ -7,7 +7,7 @@ use vectorcraft_color::Paint; use vectorcraft_doc::{Node, NodeKind, Scaling}; use vectorcraft_geom::{Affine, Point, Rect, Vec2}; -use super::edit::selected_roots; +use super::edit::{outermost, selected_roots}; use super::*; use crate::EngineError; @@ -140,7 +140,8 @@ fn distort(s: &mut Session, p: &Value) -> Result { .filter(|a| a.len() == 4) .and_then(|a| a.iter().map(|c| finite_numbers(c).map(|[x, y]| Point::new(x, y))).collect()); let Some(&[tl, tr, br, bl]) = corners.as_deref() else { return Err(bad(C, "corners must be 4 [x, y] points (TL, TR, BR, BL)")) }; - let ids = if p.get("ids").is_some_and(|v| !v.is_null()) { checked_ids_param(s, p, "ids", C)? } else { selected_roots(s)? }; + let ids = + if p.get("ids").is_some_and(|v| !v.is_null()) { outermost(&s.doc()?.doc, checked_ids_param(s, p, "ids", C)?) } else { selected_roots(s)? }; if ids.is_empty() { return Err(bad(C, "nothing to distort")); } diff --git a/crates/engine/src/tests_brushsym.rs b/crates/engine/src/tests_brushsym.rs index b6f3efdb1..690e1ca28 100644 --- a/crates/engine/src/tests_brushsym.rs +++ b/crates/engine/src/tests_brushsym.rs @@ -417,6 +417,38 @@ fn new_symbol_of_an_object_and_the_group_that_holds_it_keeps_order_and_ids() { assert!(*s.doc().unwrap().doc == *before, "Undo gives back the document"); } +/// New Symbol given `ids` that list an object and a group that holds it (directly, or inside +/// another group) makes the symbol of the outer group, whose art holds the object once, as it does +/// with those objects selected. +#[test] +fn new_symbol_of_an_object_and_a_group_that_holds_it_has_the_object_once() { + let nested = || { + let mut s = session(); + let (a, b) = (rect(&mut s, 0.0, 0.0, 10.0, 10.0), rect(&mut s, 100.0, 0.0, 10.0, 10.0)); + select(&mut s, &[a, b]); + let inner = NodeId(s.execute("object.group", &json!({})).unwrap()["id"].as_u64().unwrap()); + let c = rect(&mut s, 200.0, 0.0, 10.0, 10.0); + select(&mut s, &[inner, c]); + let outer = NodeId(s.execute("object.group", &json!({})).unwrap()["id"].as_u64().unwrap()); + s.execute("select.none", &json!({})).unwrap(); + (s, [a, inner, outer]) + }; + for (picks, paths) in [(&[0, 1][..], 2), (&[1, 0, 2], 3)] { + let (mut given, ids) = nested(); + let list: Vec = picks.iter().map(|i| ids[*i]).collect(); + given.execute("symbol.new", &json!({"name": "Pair", "ids": list.iter().map(|i| i.0).collect::>()})).unwrap(); + let d = &given.doc().unwrap().doc; + let mut n = 0; + d.symbols[0].art.walk(&mut |c| n += usize::from(matches!(c.kind, NodeKind::Path { .. }))); + assert_eq!(n, paths, "ids {list:?}"); + let (mut selected, _) = nested(); + select(&mut selected, &list); + selected.execute("symbol.new", &json!({"name": "Pair"})).unwrap(); + let e = &selected.doc().unwrap().doc; + assert!(d.symbols == e.symbols && d.layers == e.layers, "ids {list:?}"); + } +} + #[test] fn place_and_break_link_round_trip_geometry() { let mut s = session(); diff --git a/crates/engine/src/tests_ids_without_selection.rs b/crates/engine/src/tests_ids_without_selection.rs index cf007fb5c..053365088 100644 --- a/crates/engine/src/tests_ids_without_selection.rs +++ b/crates/engine/src/tests_ids_without_selection.rs @@ -82,3 +82,89 @@ fn ids_do_what_the_selection_does_in_every_command_that_takes_them() { } assert!(checked.iter().filter(|c| c.1).count() >= 20, "{checked:?}"); } + +/// Rectangles `a` and `b` grouped in `inner`, `inner` and a third rectangle grouped in `outer`, and +/// a rectangle `d` apart, with nothing selected → `[a, inner, outer, d]`. The document has no +/// creation date. +fn doc_with_nested_groups() -> (Session, [u64; 4]) { + let mut s = Session::new(); + s.execute("file.new", &json!({"width": 200, "height": 200, "created": null})).unwrap(); + let rect = + |s: &mut Session, x: u32| s.execute("shape.rectangle", &json!({"x": x, "y": 10, "width": 20, "height": 20})).unwrap()["id"].as_u64().unwrap(); + let group = |s: &mut Session, ids: [u64; 2]| { + s.execute("select.set", &json!({ "ids": ids })).unwrap(); + s.execute("object.group", &json!({})).unwrap()["id"].as_u64().unwrap() + }; + let (a, b) = (rect(&mut s, 10), rect(&mut s, 40)); + let inner = group(&mut s, [a, b]); + let c = rect(&mut s, 70); + let outer = group(&mut s, [inner, c]); + let d = rect(&mut s, 120); + s.execute("select.none", &json!({})).unwrap(); + (s, [a, inner, outer, d]) +} + +/// The params besides `ids` that `cmd` needs to change these objects (`d`: the rectangle apart). +fn params_for(cmd: &str, d: u64) -> serde_json::Value { + match cmd { + "object.transform" => json!({"matrix": [1, 0, 0, 1, 10, 0]}), + "object.distort" => json!({"corners": [[0, 0], [150, 0], [150, 60], [0, 60]]}), + "object.puppetWarp" => json!({"pins": [[10, 10], [100, 10]], "moved": [[10, 10], [100, 20]]}), + "appearance.copyFrom" => json!({ "source": d }), + _ => json!({}), + } +} + +/// `cmd` run with the objects at `picks` of [`doc_with_nested_groups`] as `ids`, and with them +/// selected: whether it succeeded, when both give the same result or the same error and the same +/// document; else what each gave. +fn ids_and_selection(cmd: &str, picks: &[usize]) -> std::result::Result { + let (mut given, ids) = doc_with_nested_groups(); + let (mut selected, _) = doc_with_nested_groups(); + let list: Vec = picks.iter().map(|i| ids[*i]).collect(); + selected.execute("select.set", &json!({ "ids": list })).unwrap(); + let params = params_for(cmd, ids[3]); + let mut with_ids = params.clone(); + with_ids["ids"] = json!(list); + let (a, b) = (given.execute(cmd, &with_ids), selected.execute(cmd, ¶ms)); + let same_result = match (&a, &b) { + (Ok(x), Ok(y)) => x == y, + (Err(x), Err(y)) => x.to_string() == y.to_string(), + _ => false, + }; + let same_doc = match (given.doc(), selected.doc()) { + (Ok(g), Ok(s)) => g.doc == s.doc, + (g, s) => g.is_ok() == s.is_ok(), + }; + if same_result && same_doc { + return Ok(a.is_ok()); + } + let docs = if same_doc { "" } else { "; the documents differ" }; + Err(format!("{cmd} with ids {list:?}: {:.160}; with them selected: {:.160}{docs}", format!("{a:?}"), format!("{b:?}"))) +} + +/// Every command whose `ids` stand in for the selection: given an object together with a group that +/// holds it (directly, or inside another group), it does what it does with those objects selected +/// (the same result or the same error, and the same document). `edit::roots_of` drops such an +/// object from the selection and keeps the group. +#[test] +fn ids_of_an_object_and_a_group_that_holds_it_do_what_that_selection_does() { + let (mut failures, mut succeeded) = (vec![], 0); + for spec in command_specs() { + // `outer` and `d` are separate objects. Commands that need `ids`, or whose `ids` are Layers + // panel rows, images or paths only, differ already there and are left out. + if !cmd::params::takes_key(spec.params, "ids") || ids_and_selection(spec.id, &[2, 3]).is_err() { + continue; + } + // Each list holds an object and a group around it: `a` and `inner`; `inner`, `a` and `outer`; + // `a`, `outer` and `d`. + for picks in [&[0, 1][..], &[1, 0, 2], &[0, 2, 3]] { + match ids_and_selection(spec.id, picks) { + Ok(ok) => succeeded += usize::from(ok), + Err(e) => failures.push(e), + } + } + } + assert!(failures.is_empty(), "{} differ:\n{}", failures.len(), failures.join("\n")); + assert!(succeeded >= 150, "only {succeeded} runs succeeded"); +} diff --git a/crates/engine/src/tests_large_edits.rs b/crates/engine/src/tests_large_edits.rs index a402c1a23..0347ebdf3 100644 --- a/crates/engine/src/tests_large_edits.rs +++ b/crates/engine/src/tests_large_edits.rs @@ -630,9 +630,9 @@ fn path_commands_across_layers_and_inside_a_group_keep_order_and_ids() { undo_gives_back(&mut s, &before); } -/// `object.transform` copies ids listed twice, and an object listed before a group around it. -/// Each copy goes directly above its original, a later copy of the same object goes below an -/// earlier one, and the copy of the group holds a copy of the copy made inside the group before it. +/// `object.transform` copies ids listed twice, and copies an object listed before a group around it +/// only with the group, as it does with the two selected. Each copy goes directly above its +/// original, and a later copy of the same object goes below an earlier one. #[test] fn copies_of_ids_listed_twice_or_inside_another_listed_id_keep_their_order_and_ids() { let (mut s, [a, b, c, d, e, f, g, h, group]) = two_layers(); @@ -640,9 +640,9 @@ fn copies_of_ids_listed_twice_or_inside_another_listed_id_keep_their_order_and_i let n = before.peek_next_id(); let r = s.execute("object.transform", &json!({"matrix": [1, 0, 0, 1, 5, 0], "copy": true, "ids": [e, group]})).unwrap(); - assert_eq!(r["ids"], json!([n, n + 1])); - let copied = json!([n + 1, n + 2, n + 3, n + 4, n + 5]); - assert_eq!(layers(&s), [json!([a, b, c, [group, d, e, n, f], copied]), json!([g, h])]); + assert_eq!(r["ids"], json!([n])); + let copied = json!([n, n + 1, n + 2, n + 3]); + assert_eq!(layers(&s), [json!([a, b, c, [group, d, e, f], copied]), json!([g, h])]); undo_gives_back(&mut s, &before); let r = s.execute("object.transform", &json!({"matrix": [1, 0, 0, 1, 5, 0], "copy": true, "ids": [a, a]})).unwrap(); diff --git a/crates/engine/src/tests_pattern.rs b/crates/engine/src/tests_pattern.rs index 0b36ea3bd..5b5789d95 100644 --- a/crates/engine/src/tests_pattern.rs +++ b/crates/engine/src/tests_pattern.rs @@ -354,6 +354,35 @@ fn radial_repeat_wraps_selection() { assert_eq!(n.kind_label(), "Radial Repeat"); } +/// Repeat given `ids` that list an object and a group that holds it (directly, or inside another +/// group) repeats the outer group, which holds the object once, as it does with those objects +/// selected. +#[test] +fn repeat_of_an_object_and_a_group_that_holds_it_repeats_the_group() { + let nested = || { + let mut s = session(); + let (a, b) = (rect(&mut s, 195.0, 95.0, 10.0, 10.0), rect(&mut s, 215.0, 95.0, 10.0, 10.0)); + sel(&mut s, &[a, b]); + let inner = id_of(&s.execute("object.group", &json!({})).unwrap()); + let c = rect(&mut s, 235.0, 95.0, 10.0, 10.0); + sel(&mut s, &[inner, c]); + let outer = id_of(&s.execute("object.group", &json!({})).unwrap()); + s.execute("select.none", &json!({})).unwrap(); + (s, [a, inner, outer]) + }; + for (picks, group) in [(&[0, 1][..], 1), (&[1, 0, 2], 2)] { + let (mut given, ids) = nested(); + let list: Vec = picks.iter().map(|i| ids[*i]).collect(); + let v = given.execute("object.repeat.radial", &json!({ "ids": list.iter().map(|i| i.0).collect::>() })).unwrap(); + let NodeKind::Repeat(spec) = node(&given, id_of(&v)).kind else { panic!("{v}") }; + assert_eq!(spec.source.iter().map(|n| n.id).collect::>(), [ids[group]]); + let (mut selected, _) = nested(); + sel(&mut selected, &list); + selected.execute("object.repeat.radial", &json!({})).unwrap(); + assert!(doc(&given).layers == doc(&selected).layers, "ids {list:?}"); + } +} + #[test] fn radial_instance_transforms_rotate_around_center() { let mut s = session(); diff --git a/docs/mcp.md b/docs/mcp.md index 741591a8f..39410f8ff 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -274,7 +274,8 @@ currently describes them in prose, so MCP does not guess schemas or silently rem Coordinates are points in document space: y points down, the origin is the first artboard's top-left, and a new document is 612 × 792 (US Letter). New objects become the selection. Most commands act on the selection or on -explicit `ids`. +explicit `ids`. A command that leaves out an object selected inside a selected group also leaves it out when `ids` +list it together with a group that holds it. Paint values (`fill`, `stroke`) accept `"#rrggbb"`, `"none"`, `[r,g,b]` (0..1), `{"c","m","y","k"}`, `{"gray"}`, or a full `paint.setFill` params object (`{"gradient": …}`, `{"swatch": "name"}`). diff --git a/docs/target-app-parity.md b/docs/target-app-parity.md index d01a526b6..1eae3e6f3 100644 --- a/docs/target-app-parity.md +++ b/docs/target-app-parity.md @@ -464,6 +464,7 @@ Moved from the ROADMAP's "Shipped so far" on 2026-10-10. Update it when a featur Anchor Align moves the selected points to their bounds or the artboard (#1180). Repeating it, or moving by at most 1e-9 pt on both axes as whole-object Align does, leaves the document, Undo, Redo and saved state unchanged; actual moves still take one undo step. `crates/engine/src/tests.rs` covers repeated alignment, rounding-only alignment, unchanged handles, saved state, command recording and Undo/Redo. A symbol instance is measured by its symbol's art, as its transform places it, for clicks, the marquee, the bounding box, the Transform panel, Align and Distribute (the art's strokes too with Use Preview Bounds), Fit to Artwork Bounds, exports of the art, thumbnails and canvas culling; one whose symbol is missing is measured as the 20 × 20 square around the origin. Clear deletes individual selected compound-path members without deleting their siblings, and explicit object ids work without a selection; Cut still removes the same objects it copies (#786). + A command that leaves out an object selected inside a selected group also leaves it out when `ids` list it together with a group that holds it. Make Repeat and Live Paint no longer fail on such a list, New Symbol no longer stores the object twice, and `object.transform` no longer moves it twice. - **Compound paths:** Make keeps the backmost object's appearance and each contour; selecting a member in Direct Selection reads and changes the compound's fill rule in Attributes. Direction remains a per-member edit. Rendered holes, unchanged contour geometry, no-op history and Undo/Redo are covered by `engine/tests/compound_attributes.rs` (#1197). Behaviour reference: [public compound-path documentation](https://helpx.adobe.com/illustrator/desktop/manage-objects/reshape-transform-objects/create-compound-paths.html). Make Compound Path on a group (such as outlined type) keeps the paint of its backmost path instead of drawing nothing, and compound paths joined into one keep their holes (#1537). - **Saved selections:** Select ▸ Save Selection… asks for a name and keeps the selection in the document (saved with the file, undoable, up to 25); each one is listed at the bottom of the Select menu and a click selects its objects again; Edit Selection… lists them to rename or delete, applied together on OK. A file's saved selections are capped and checked against its objects when it opens. Not yet: Update Selection and the Layers panel's saved-selection menu. - **Drawing modes:** in Draw Inside, a double-click with a selection tool outside the object being drawn into sets the drawing mode to Draw Normal, as Shift+D does.