From dc6e8a4253b89dfa82a6722760ecfb39f26f2605 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Fri, 9 Oct 2026 17:32:42 -0700 Subject: [PATCH] Object: Align, Move, Arrange and other commands that change nothing record no undo step MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These Object commands edited the document even when nothing changed, so the call recorded an undo step, emptied Redo and marked the document modified: - object.align, object.distributeSpacing and object.distribute (three or more objects) when no object moves. With one object selected, Object › Align aligns it to its own bounds, so it recorded a step every time. Calls on objects that were already aligned or evenly spaced also recorded a step, including objects that an earlier Align or Distribute left a rounding error away from the position the next call computes. - Move, Rotate, Scale, Shear, Transform and Nudge by an identity transform without a copy, Transform Again after one, Set Bounds with the current position and size, and a Selection tool drag that ends where it started. - Nudge by 0 with anchors direct-selected or with only ruler guides selected, and Transform with nothing selected or with ids that name no object, with or without a copy. - Bring to Front and Bring Forward on objects already in front, Send to Back and Send Backward on objects already at the back, and Send to Current Layer on objects already on top of the current layer. - Ungroup, Release Compound Path and Release Clipping Mask with none of their objects selected, and Expand Shape with no live shape selected. - Lock with every selected object already locked, Hide with every one already hidden, Unlock All with nothing locked and Show All with nothing hidden. - object.setProps with the values the objects already have, or with no object. - object.setLiveShape with the values the shape already has: a polygon's sides, angle, radius or equal sides, or a side length that gives exactly its current radius; an ellipse's pie, including Invert Pie on a whole ellipse; and a corner radius or kind that the corners already have. These commands now return without editing the document, and Align and Distribute count a move of 1e-9 pt or less on both axes as none. Such a call records no undo step, keeps the step to redo and leaves the document unmodified. In these cases Ungroup no longer resets the selection, and Release Compound Path, Unlock All and Show All no longer empty it. A Transform with a copy of no object keeps the selection. Lock and Hide still deselect the objects. A Nudge by 0 no longer expands a live shape whose anchors are direct-selected. The replies have the same form as before. An identity transform still sets what Transform Again repeats. The journal still records each call, so the Actions panel records it as before. When every value given to Set Bounds is the current one, it now applies the identity transform instead of the transform computed from those values, which can differ from the identity by a rounding error on a rotated object or with Use Preview Bounds. When a drag comes back to its start, the identity preview no longer edits the document. Session::preview now advances the revision when it undoes the previous preview, so views redraw that drag. The undo and redo row of docs/mcp.md now notes that many calls that change nothing record no undo step, so undo after one undoes the step before it. --- ROADMAP.md | 2 +- crates/engine/src/cmd/object.rs | 185 ++++++++++++-- crates/engine/src/lib.rs | 5 + crates/engine/src/tests.rs | 420 ++++++++++++++++++++++++++++++++ crates/ui-egui/src/shortcuts.rs | 9 +- docs/mcp.md | 2 +- 6 files changed, 599 insertions(+), 24 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 73b5e00f..490733ef 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -123,7 +123,7 @@ Ranked; detail in [docs/roadmap.md](docs/roadmap.md) and [docs/gaps.md](docs/gap | Date | What landed | |---|---| -| 2026-10-11 | Effect › Brush Strokes: Accented Edges, Angled Strokes, Crosshatch, Dark Strokes, Ink Outlines, Spatter, Sprayed Strokes, Sumi-e (G2). Command parameters from agents and files checked before anything changes (paint, place, cuts, dashes, crop, asset ids, anchors, paths, distort; #984–#999); command dialogs keep whole numbers whole, so Create Object Mosaic takes the tiles typed (#1000); stepper arrows take Shift (×10) and Ctrl/Cmd (÷10) like the arrow keys (#991). Exports and Rasterize supersample Art Optimized edges, so shapes meeting edge to edge leave no seam (#983). Character and paragraph styles take leading, kerning, mojikumi and the other attributes left out at their defaults (#1001). Drawing, typing and pasting with no layer shown and unlocked is refused instead of adding art to a hidden or locked layer (#1002). MCP `add_text` gives type on or in a path the colour asked for (#1003). MCP `screenshot` of a connected app renders the artboard asked for (#1004). `vectorcraft-cli run` ends quietly when its reader closes early (#1005). Select › All on Active Artboard selects on the artboard made active, not always the first (#1006). Reverse Gradient keeps off-centre midpoints, mirrored (#1007). A polar grid with no radial dividers draws none (#1008). Use Precise Cursors also covers the Pen's join pointer (#1009). SVGs with one absolute and one unitless root side import at their size, unstretched (#1010). DXF US survey inches, yards and miles open at their size and name (#1011). A file placed into a document with no other links follows its first change even while the app sits idle (#1031). PDF import keeps the style a font name says when the file doesn't embed the font (#1023). Shift selects ruler guides and art together, and Delete removes both (#1022). Open filled paths are hit by their fill on every side, the rotate cursor bows around the corner it is at, the flip icons match their flips, the Color Picker previews on the selection, and Properties › Transform reaches Scale Corners and Scale Strokes & Effects (#1036). Offset Path and the other booleans finish on outlines where the sweep used to loop until memory ran out (linesweeper 0.5, #1028). Floating panel groups stack into sets that move together, collapse to their tabs and dock as extra dock columns that scroll (#1020). Ruler guides live on layers: listed in the Layers panel, shown, hidden, locked, duplicated, merged and deleted with their layer, and moved between layers (#1021). A `.ai` whose editing data uses global process colours (`Xk`/`XK`) or pattern fills keeps its sublayers: the colours come in as global swatches at their tint, the patterns as pattern swatches (#1025). A `.ai` in PostScript form opens through its layers, read from the program itself, also when it names the prolog defining its operators without including it, as Rhino and other CAD apps write it (#1027). A fill or stroke chosen while the Type tool has characters selected colours just those characters (#1063). Scaled type shows its scaled font size, leading, baseline shift and horizontal scale in the Character panel, and a size typed there is the size you get (#1034). Character panel menu › Proportional Metrics sets Japanese and Chinese full-width glyphs on the font's proportional widths in horizontal type (#966). | +| 2026-10-11 | Effect › Brush Strokes: Accented Edges, Angled Strokes, Crosshatch, Dark Strokes, Ink Outlines, Spatter, Sprayed Strokes, Sumi-e (G2). Command parameters from agents and files checked before anything changes (paint, place, cuts, dashes, crop, asset ids, anchors, paths, distort; #984–#999); command dialogs keep whole numbers whole, so Create Object Mosaic takes the tiles typed (#1000); stepper arrows take Shift (×10) and Ctrl/Cmd (÷10) like the arrow keys (#991). Exports and Rasterize supersample Art Optimized edges, so shapes meeting edge to edge leave no seam (#983). Character and paragraph styles take leading, kerning, mojikumi and the other attributes left out at their defaults (#1001). Drawing, typing and pasting with no layer shown and unlocked is refused instead of adding art to a hidden or locked layer (#1002). MCP `add_text` gives type on or in a path the colour asked for (#1003). MCP `screenshot` of a connected app renders the artboard asked for (#1004). `vectorcraft-cli run` ends quietly when its reader closes early (#1005). Select › All on Active Artboard selects on the artboard made active, not always the first (#1006). Reverse Gradient keeps off-centre midpoints, mirrored (#1007). A polar grid with no radial dividers draws none (#1008). Use Precise Cursors also covers the Pen's join pointer (#1009). SVGs with one absolute and one unitless root side import at their size, unstretched (#1010). DXF US survey inches, yards and miles open at their size and name (#1011). A file placed into a document with no other links follows its first change even while the app sits idle (#1031). PDF import keeps the style a font name says when the file doesn't embed the font (#1023). Shift selects ruler guides and art together, and Delete removes both (#1022). Open filled paths are hit by their fill on every side, the rotate cursor bows around the corner it is at, the flip icons match their flips, the Color Picker previews on the selection, and Properties › Transform reaches Scale Corners and Scale Strokes & Effects (#1036). Offset Path and the other booleans finish on outlines where the sweep used to loop until memory ran out (linesweeper 0.5, #1028). Floating panel groups stack into sets that move together, collapse to their tabs and dock as extra dock columns that scroll (#1020). Ruler guides live on layers: listed in the Layers panel, shown, hidden, locked, duplicated, merged and deleted with their layer, and moved between layers (#1021). A `.ai` whose editing data uses global process colours (`Xk`/`XK`) or pattern fills keeps its sublayers: the colours come in as global swatches at their tint, the patterns as pattern swatches (#1025). A `.ai` in PostScript form opens through its layers, read from the program itself, also when it names the prolog defining its operators without including it, as Rhino and other CAD apps write it (#1027). A fill or stroke chosen while the Type tool has characters selected colours just those characters (#1063). Scaled type shows its scaled font size, leading, baseline shift and horizontal scale in the Character panel, and a size typed there is the size you get (#1034). Character panel menu › Proportional Metrics sets Japanese and Chinese full-width glyphs on the font's proportional widths in horizontal type (#966). Align, Distribute, Move, Arrange, Lock, Hide, Ungroup and other Object commands that would change nothing no longer record an undo step or mark the document changed (#1060). | | 2026-10-10 | Initial live Revolve (Effect › 3D and Materials, #846, #605), in its own `three-d` crate. Progress docs re-measured and restructured to the craftrules standard (this page, `docs/target-app-parity.md`, `gaps.md`, `ui-parity.md`, the format, hardware, localization, effects and type checklists). Same day: v0.8.0; Effect › Distort, Pixelate and Texture filters (13); German interface; Variables (data merge); Graph Type value axes and tick marks; restart when the first frame never reaches the screen (#964) | | 2026-10-09 | Radial Blur, Smart Blur, Unsharp Mask (gap 2 begins); 340 commits, the busiest day | | 2026-10-08 | v0.5.0–v0.7.0; community contributions: Pen and shape-tool modifiers, Layers, Artboards, Japanese composition, Hebrew/Arabic type, interface languages, saved selections | diff --git a/crates/engine/src/cmd/object.rs b/crates/engine/src/cmd/object.rs index 2e912780..832c0d58 100644 --- a/crates/engine/src/cmd/object.rs +++ b/crates/engine/src/cmd/object.rs @@ -251,17 +251,26 @@ pub(crate) fn apply_transform(s: &mut Session, label: &str, ids: Vec, xf let areas = bool_or(p, "typeAreas", false); let mut sc = if Scaling::factor(xf).is_some() { scaling(s, p) } else { Scaling::default() }; sc.patterns = transform_patterns(s, p, &ids)?; - let ids = s.edit(label, |d, sel| { - let targets = if copy { duplicate_in(d, sel, &ids, Affine::IDENTITY)? } else { ids.clone() }; - for id in &targets { - if let Some(n) = d.node_mut(*id) - && !(areas && resize_type_area(n, xf)) - { - n.transform(xf, sc); + // A transform of ids that name no object, or an identity transform without a copy, moves + // nothing: the document and its undo history stay as they are, and Transform Again repeats it. + // With a copy, the reply lists the copies, and there are none. + let d = &s.doc()?.doc; + let none = ids.iter().all(|id| d.node(*id).is_none()); + let ids = if none || (!copy && xf == Affine::IDENTITY) { + if copy { vec![] } else { ids } + } else { + s.edit(label, |d, sel| { + let targets = if copy { duplicate_in(d, sel, &ids, Affine::IDENTITY)? } else { ids.clone() }; + for id in &targets { + if let Some(n) = d.node_mut(*id) + && !(areas && resize_type_area(n, xf)) + { + n.transform(xf, sc); + } } - } - Ok(targets) - })?; + Ok(targets) + })? + }; let st = s.doc_mut()?; if st.interaction.is_none() { st.last_transform = Some((xf, copy)); @@ -300,10 +309,19 @@ fn nudge(s: &mut Session, p: &Value) -> Result { let dx = f64_or(p, "dx", 0.0) * k; let dy = f64_or(p, "dy", 0.0) * k; if !s.doc()?.selection.anchors.is_empty() { + // When the anchors move by 0, the document and its undo history stay as they are. + if dx == 0.0 && dy == 0.0 { + return ok(); + } return super::path::move_anchors(s, &json!({ "dx": dx, "dy": dy })); } if s.doc()?.selection.is_empty() { - return super::docmenu::guide_move(s, &json!({ "dx": dx, "dy": dy, "copy": bool_or(p, "copy", false) })); + let copy = bool_or(p, "copy", false); + // When guides move by 0 without a copy, the document and its undo history stay as they are. + if dx == 0.0 && dy == 0.0 && !copy { + return ok(); + } + return super::docmenu::guide_move(s, &json!({ "dx": dx, "dy": dy, "copy": copy })); } let ids = selected_roots(s)?; apply_transform(s, "Move", ids, Affine::translate((dx, dy)), p) @@ -386,7 +404,9 @@ fn arrange(s: &mut Session, how: Arrange) -> Result { Arrange::Backward => "Send Backward", Arrange::Back => "Send to Back", }; - s.edit(label, |d, _| { + // When nothing changes order, the document and its undo history stay as they are. + super::overprint::edit_counted(s, label, |d| { + let mut moved = 0; // Arrange within each parent independently (selections may span layers/groups). let mut by_parent: Vec<(Option, Vec)> = vec![]; for id in &ids { @@ -411,10 +431,11 @@ fn arrange(s: &mut Session, how: Arrange) -> Result { }; if to != idx { d.move_node(*id, par, to)?; + moved += 1; } } } - Ok(()) + Ok(moved) })?; ok() } @@ -424,6 +445,11 @@ fn send_to_current_layer(s: &mut Session, _: &Value) -> Result { // Ids are reused after undo: the remembered current layer must still be a layer. let st = s.doc()?; let layer = st.current_layer().or_else(|| st.doc.default_layer()).ok_or_else(|| EngineError::Other("no current layer".into()))?; + // When the selected objects are already the topmost in the current layer, in paint order, the + // document and its undo history stay as they are. + if st.doc.children(Some(layer)).is_some_and(|c| c.iter().rev().map(|n| n.id).take(ids.len()).eq(ids.iter().rev().copied())) { + return ok(); + } s.edit("Send to Current Layer", |d, _| { for id in &ids { d.move_node(*id, Some(layer), usize::MAX)?; @@ -457,6 +483,11 @@ pub(crate) fn group_nodes(d: &mut Document, ids: &[NodeId]) -> Result { fn ungroup(s: &mut Session, _: &Value) -> Result { let ids = selected_roots(s)?; + // With no group selected, the document, the selection and the undo history stay as they are. + let d = &s.doc()?.doc; + if !ids.iter().any(|id| d.node(*id).is_some_and(|n| matches!(n.kind, NodeKind::Group { .. }))) { + return ok(); + } s.edit("Ungroup", |d, sel| { let mut new_sel = vec![]; for id in &ids { @@ -492,6 +523,13 @@ fn ungroup(s: &mut Session, _: &Value) -> Result { fn lock(s: &mut Session, _: &Value) -> Result { let ids = selected_roots(s)?; + // With every selected object already locked, the document and its undo history stay as they + // are, and the objects are deselected. + let d = &s.doc()?.doc; + if ids.iter().all(|id| d.node(*id).is_none_or(|n| n.locked)) { + s.select(|_, sel| sel.clear())?; + return ok(); + } s.edit("Lock", |d, sel| { for id in &ids { if let Some(n) = d.node_mut(*id) { @@ -516,6 +554,10 @@ fn collect_flag(d: &Document, f: impl Fn(&Node) -> bool) -> Vec { fn unlock_all(s: &mut Session, _: &Value) -> Result { let ids = collect_flag(&s.doc()?.doc, |n| n.locked); + // With nothing locked, the document, the selection and the undo history stay as they are. + if ids.is_empty() { + return Ok(json!({ "count": 0 })); + } s.edit("Unlock All", |d, sel| { for id in &ids { if let Some(n) = d.node_mut(*id) { @@ -530,6 +572,13 @@ fn unlock_all(s: &mut Session, _: &Value) -> Result { fn hide(s: &mut Session, _: &Value) -> Result { let ids = selected_roots(s)?; + // With every selected object already hidden, the document and its undo history stay as they + // are, and the objects are deselected. + let d = &s.doc()?.doc; + if ids.iter().all(|id| d.node(*id).is_none_or(|n| !n.visible)) { + s.select(|_, sel| sel.clear())?; + return ok(); + } s.edit("Hide", |d, sel| { for id in &ids { if let Some(n) = d.node_mut(*id) { @@ -544,6 +593,10 @@ fn hide(s: &mut Session, _: &Value) -> Result { fn show_all(s: &mut Session, _: &Value) -> Result { let ids = collect_flag(&s.doc()?.doc, |n| !n.visible); + // With nothing hidden, the document, the selection and the undo history stay as they are. + if ids.is_empty() { + return Ok(json!({ "count": 0 })); + } s.edit("Show All", |d, sel| { for id in &ids { if let Some(n) = d.node_mut(*id) { @@ -602,6 +655,12 @@ fn compound_make(s: &mut Session, _: &Value) -> Result { fn compound_release(s: &mut Session, _: &Value) -> Result { let ids = selected_roots(s)?; + // With no compound path selected, the document, the selection and the undo history stay as + // they are. + let d = &s.doc()?.doc; + if !ids.iter().any(|id| d.node(*id).is_some_and(|n| matches!(n.kind, NodeKind::Compound { .. }))) { + return ok(); + } s.edit("Release Compound Path", |d, sel| { let mut out = vec![]; for id in &ids { @@ -678,6 +737,11 @@ fn clip_make(s: &mut Session, _: &Value) -> Result { fn clip_release(s: &mut Session, _: &Value) -> Result { let ids = selected_roots(s)?; + // With no clip group selected, the document and its undo history stay as they are. + let d = &s.doc()?.doc; + if !ids.iter().any(|id| d.node(*id).is_some_and(Node::clips)) { + return ok(); + } s.edit("Release Clipping Mask", |d, _| { for id in &ids { release_clip(d, *id); @@ -785,6 +849,27 @@ fn set_props(s: &mut Session, p: &Value) -> Result { ), None => None, }; + // When every object already has every value given, the document and its undo history stay as + // they are. + let flag = |k: &str| p.get(k).and_then(Value::as_bool); + let has_values = |n: &Node| { + str_param(p, "name").is_none_or(|v| n.name.as_deref() == Some(v).filter(|v| !v.is_empty())) + && flag("visible").is_none_or(|v| n.visible == v) + && flag("locked").is_none_or(|v| n.locked == v) + && opacity.is_none_or(|v| n.opacity == v) + && blend.is_none_or(|b| n.blend == b) + && flag("isolate").is_none_or(|v| n.isolate == v) + && knockout.is_none_or(|k| n.knockout == k) + && flag("knockoutShape").is_none_or(|v| n.knockout_shape == v) + && data + .iter() + .flatten() + .all(|(k, v)| n.attrs.as_ref().and_then(|a| a.data.iter().find(|(key, _)| key == k)).map(|(_, now)| now.as_str()) == v.as_deref()) + }; + let d = &s.doc()?.doc; + if ids.iter().all(|id| d.node(*id).is_some_and(has_values)) { + return ok(); + } s.edit("Object Properties", |d, _| { for id in &ids { let n = d.node_mut(*id).ok_or(EngineError::NoNode(*id))?; @@ -864,6 +949,11 @@ fn items_bounds(d: &Document, ids: &[NodeId], preview: bool) -> Vec<(NodeId, Rec ids.iter().filter_map(|id| Some((*id, d.bounds_of(&[*id], preview)?))).collect() } +/// Is every move in `moves` a rounding error (1e-9 pt or less on both axes)? +fn moves_nothing(moves: &[(NodeId, Vec2)]) -> bool { + moves.iter().all(|(_, dv)| dv.x.abs() <= 1e-9 && dv.y.abs() <= 1e-9) +} + /// What `object.align` aligns to: `to`, else the key object when there is one, else the /// selection. fn align_to<'a>(s: &Session, p: &'a Value) -> Option<&'a str> { @@ -915,6 +1005,10 @@ fn align(s: &mut Session, p: &Value) -> Result { }) .collect() }; + // When nothing moves, the document and its undo history stay as they are. + if moves_nothing(&moves) { + return ok(); + } s.edit("Align", |d, _| { for (id, dv) in &moves { if let Some(n) = d.node_mut(*id) { @@ -965,6 +1059,10 @@ fn distribute(s: &mut Session, p: &Value) -> Result { (*id, if horiz { Vec2::new(delta, 0.0) } else { Vec2::new(0.0, delta) }) }) .collect(); + // When nothing moves, the document and its undo history stay as they are. + if moves_nothing(&moves) { + return ok(); + } s.edit("Distribute", |d, _| { for (id, dv) in &moves { if let Some(n) = d.node_mut(*id) { @@ -1008,6 +1106,10 @@ fn distribute_spacing(s: &mut Session, p: &Value) -> Result { let fixed = spacing.and_then(|_| deltas.iter().find(|(id, _)| Some(*id) == key)).map_or(0.0, |k| k.1); let moves: Vec<(NodeId, Vec2)> = deltas.into_iter().map(|(id, d)| (id, if horiz { Vec2::new(d - fixed, 0.0) } else { Vec2::new(0.0, d - fixed) })).collect(); + // When nothing moves, the document and its undo history stay as they are. + if moves_nothing(&moves) { + return ok(); + } s.edit("Distribute Spacing", |d, _| { for (id, dv) in &moves { if let Some(n) = d.node_mut(*id) { @@ -1057,12 +1159,23 @@ fn set_bounds(s: &mut Session, p: &Value) -> Result { let new_rp = scale * gp + (rp - gp) * f(sx, sy); // x, y place the reference point on the page. let page = frame.to_doc() * rp; + // When every value given is the current one, the transform is the identity. Computing it can + // leave a rounding error in it on a rotated object or with Use Preview Bounds. + let current = |k: &str, now: f64| p.get(k).and_then(Value::as_f64).is_none_or(|x| x == now); + if current("x", page.x) && current("y", page.y) && current("width", v.width()) && current("height", v.height()) { + return apply_transform(s, "Transform", ids, Affine::IDENTITY, p); + } let to = frame.to_local(Point::new(f64_or(p, "x", page.x), f64_or(p, "y", page.y))); apply_transform(s, "Transform", ids, frame.conjugate(Affine::translate(to - new_rp) * scale), p) } fn expand_shape(s: &mut Session, _: &Value) -> Result { let ids = selected_roots(s)?; + // With no live shape selected, the document and its undo history stay as they are. + let d = &s.doc()?.doc; + if !ids.iter().any(|id| d.node(*id).is_some_and(|n| matches!(n.kind, NodeKind::Path { live: Some(_), .. }))) { + return ok(); + } s.edit("Expand Shape", |d, _| { for id in &ids { if let Some(NodeKind::Path { live, .. }) = d.node_mut(*id).map(|n| &mut n.kind) { @@ -1138,6 +1251,45 @@ fn set_live_shape(s: &mut Session, p: &Value) -> Result { // A polygon's Polygon Properties: its angle, radius or side length, Make Sides Equal. let (polygon_angle, polygon_radius, side) = (angle("polygonAngle")?, length("polygonRadius")?, length("sideLength")?); let equal = p.get("makeSidesEqual").and_then(Value::as_bool) == Some(true); + // An ellipse's pie becomes `pie_of(pie)`. An end of 0 is a full turn (Illustrator shows a whole + // ellipse as 0° to 360°). + let end_of = |a: f64| if a == 0.0 { 360.0 } else { a }; + let pie_of = |pie: (f64, f64)| { + let pie = (pie_start.unwrap_or(pie.0), pie_end.map_or(pie.1, end_of)); + if invert { (pie.1.rem_euclid(360.0), end_of(pie.0)) } else { pie } + }; + // When every value given is the current one, the document and its undo history stay as they + // are. + let st = s.doc()?; + let has_values = |id: &NodeId| { + let Some(NodeKind::Path { path, live, .. }) = st.doc.node(*id).map(|n| &n.kind) else { return true }; + match live { + Some(l @ LiveShape::Polygon { sides: now, .. }) + if !(sides.is_none_or(|n| n.clamp(3, 1000) == u64::from(*now)) + && (!equal || l.polygon_sides_equal()) + && polygon_angle.is_none_or(|a| l.polygon_angle() == Some(a)) + && side.and_then(|len| l.polygon_radius_for_side(len)).or(polygon_radius).is_none_or(|r| l.polygon_radius() == Some(r))) => + { + return false; + } + Some(LiveShape::Ellipse { pie, .. }) if pie_of(*pie) != *pie => return false, + _ => {} + } + if radius.is_none() && kind.is_none() { + return true; + } + let Some(c) = LiveCorners::new(path, live.as_ref()) else { return true }; + let picked = corners_of(*id).unwrap_or_else(|| c.picked(st.selection.anchors.get(id))); + // Setting corners folds a scale left in a rectangle's transform into its size, which + // changes it. + live.as_ref().is_none_or(|l| l.folded() == *l) + && picked + .iter() + .all(|k| *k < c.base.anchor_count() && radius.is_none_or(|r| c.radius(*k) == r.max(0.0)) && kind.is_none_or(|kd| c.kind(*k) == kd)) + }; + if ids.iter().all(has_values) { + return ok(); + } s.edit("Live Shape", |d, sel| { for id in &ids { let Some(NodeKind::Path { path, live, .. }) = d.node_mut(*id).map(|n| &mut n.kind) else { continue }; @@ -1163,12 +1315,7 @@ fn set_live_shape(s: &mut Session, p: &Value) -> Result { && (pie_start.is_some() || pie_end.is_some() || invert) { if let LiveShape::Ellipse { pie, .. } = l { - // An end of 0 is a full turn (Illustrator shows a whole ellipse as 0° to 360°). - let end_of = |a: f64| if a == 0.0 { 360.0 } else { a }; - *pie = (pie_start.unwrap_or(pie.0), pie_end.map_or(pie.1, end_of)); - if invert { - *pie = (pie.1.rem_euclid(360.0), end_of(pie.0)); - } + *pie = pie_of(*pie); } *path = l.to_path(); } diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 7b107a5f..e49a8c91 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -1324,6 +1324,11 @@ impl Session { { let st = self.doc_mut()?; let Some(it) = &st.interaction else { return Err(EngineError::Other("no interaction in progress".into())) }; + // Undoing the previous preview counts as a change, so views redraw even when the command + // below returns without an edit. + if !Arc::ptr_eq(&st.doc, &it.doc) || st.selection != it.selection { + st.revision += 1; + } st.doc = it.doc.clone(); st.selection = it.selection.clone(); } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 6aee69d7..4894c7e8 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -330,6 +330,426 @@ fn selection_tool_drag_is_one_undo_step() { assert_eq!(s.doc().unwrap().doc.node(a).unwrap().geometric_bounds().unwrap().x0, 100.0); } +/// How many steps there are to undo and to redo, and whether the document has unsaved changes. +fn history(s: &Session) -> (usize, usize, bool) { + let st = s.doc().unwrap(); + (st.history.undo.len(), st.history.redo.len(), st.is_dirty()) +} + +/// Select `ids` with a move of them left to redo, and mark the document saved. +fn select_with_a_step_to_redo(s: &mut Session, ids: &[NodeId]) { + s.execute("select.set", &json!({"ids": ids.iter().map(|i| i.0).collect::>()})).unwrap(); + s.execute("object.move", &json!({"dx": 5, "dy": 0})).unwrap(); + s.execute("edit.undo", &json!({})).unwrap(); + s.doc_mut().unwrap().mark_saved(); +} + +/// An Align that moves nothing (one object on the artboard's left edge, a pair already aligned, a +/// pair that an earlier Align left a rounding error apart) records no undo step: the step to redo +/// stays, the document stays unmodified, and the journal, which the Actions panel records from, +/// still has the command. +#[test] +fn an_align_that_moves_nothing_records_no_undo_step() { + let mut s = session(); + let x0 = |s: &Session, id| s.doc().unwrap().doc.node(id).unwrap().geometric_bounds().unwrap().x0; + let left = json!({"horizontal": "left"}); + let a = rect(&mut s, 0.0, 50.0, 40.0, 40.0); + select_with_a_step_to_redo(&mut s, &[a]); + let before = history(&s); + assert_eq!((before.1, before.2), (1, false), "a step to redo, and nothing unsaved"); + // Object › Align sends no `to`, and the Align panel aligns one object to the artboard. + for p in [left.clone(), json!({"horizontal": "left", "to": "artboard"})] { + s.execute("object.align", &p).unwrap(); + assert_eq!(history(&s), before, "one object and no key: {p}"); + assert_eq!(s.journal.last().unwrap(), &("object.align".to_string(), p)); + } + let b = rect(&mut s, 0.0, 200.0, 80.0, 20.0); + select_with_a_step_to_redo(&mut s, &[a, b]); + let before = history(&s); + s.execute("object.align", &left).unwrap(); + assert_eq!(history(&s), before, "a pair already aligned"); + s.execute("edit.redo", &json!({})).unwrap(); + assert_eq!((x0(&s, a), x0(&s, b)), (5.0, 5.0), "Redo brings back the move"); + // An Align that moves something is one undo step, and it clears Redo. + let c = rect(&mut s, 120.0, 300.0, 20.0, 20.0); + select_with_a_step_to_redo(&mut s, &[a, c]); + let undo = history(&s).0; + s.execute("object.align", &left).unwrap(); + assert_eq!(history(&s), (undo + 1, 0, true)); + assert_eq!(x0(&s, c), 5.0); + // Aligning a rectangle at 51.8 to one at 10.7 leaves it a rounding error off 10.7, and a + // second Align records no step. + let d = rect(&mut s, 10.7, 400.0, 20.0, 20.0); + let e = rect(&mut s, 51.8, 450.0, 20.0, 20.0); + s.execute("select.set", &json!({"ids": [d.0, e.0]})).unwrap(); + s.execute("object.align", &left).unwrap(); + assert!(x0(&s, e) != 10.7 && (x0(&s, e) - 10.7).abs() < 1e-12, "{}", x0(&s, e)); + s.doc_mut().unwrap().mark_saved(); + let before = history(&s); + s.execute("object.align", &left).unwrap(); + assert_eq!(history(&s), before, "a pair the first Align aligned"); +} + +/// Distribute and Distribute Spacing over objects already evenly spaced move nothing and record no +/// undo step either. +#[test] +fn a_distribute_that_moves_nothing_records_no_undo_step() { + let mut s = session(); + // The rectangles are 20 pt wide and 30 pt apart. + let ids = [0.0, 50.0, 100.0].map(|x| rect(&mut s, x, 0.0, 20.0, 20.0)); + select_with_a_step_to_redo(&mut s, &ids); + let before = history(&s); + let calls = [ + ("object.distribute", json!({"horizontal": "left"})), + ("object.distribute", json!({"horizontal": "center"})), + ("object.distribute", json!({"horizontal": "right"})), + ("object.distributeSpacing", json!({"axis": "horizontal"})), + ("object.distributeSpacing", json!({"axis": "horizontal", "spacing": 30})), + ]; + for (id, p) in calls { + s.execute(id, &p).unwrap(); + assert_eq!(history(&s), before, "{id} {p}"); + assert_eq!(s.journal.last().unwrap(), &(id.to_string(), p)); + } + // Distribute Spacing keeps the key object still and spaces the others from it. + s.execute("select.key", &json!({"id": ids[1].0})).unwrap(); + assert_eq!(s.doc().unwrap().selection.key, Some(ids[1])); + s.execute("object.distributeSpacing", &json!({"axis": "horizontal", "spacing": 30})).unwrap(); + assert_eq!(history(&s), before, "spaced 30 pt from the key object"); + // After the middle one moves 10 pt right, Distribute moves it back in one undo step. + s.execute("select.set", &json!({"ids": [ids[1].0]})).unwrap(); + s.execute("object.move", &json!({"dx": 10, "dy": 0})).unwrap(); + s.execute("select.set", &json!({"ids": ids.map(|i| i.0)})).unwrap(); + let undo = history(&s).0; + s.execute("object.distribute", &json!({"horizontal": "left"})).unwrap(); + assert_eq!(history(&s).0, undo + 1); + assert_eq!(s.doc().unwrap().doc.node(ids[1]).unwrap().geometric_bounds().unwrap().x0, 50.0); + // For rectangles 30.1 pt apart, the gap Distribute Spacing computes moves the third one by a + // rounding error. + let ids = [0.1, 50.2, 100.3].map(|x| rect(&mut s, x, 100.0, 20.0, 20.0)); + select_with_a_step_to_redo(&mut s, &ids); + let before = history(&s); + s.execute("object.distributeSpacing", &json!({"axis": "horizontal"})).unwrap(); + assert_eq!(history(&s), before, "30.1 pt apart"); + // Distributing these three leaves the middle one a rounding error off halfway between the + // others, and a second Distribute records no step. + let ids = [40.045, 433.482, 1935.587].map(|x| rect(&mut s, x, 200.0, 20.0, 20.0)); + s.execute("select.set", &json!({"ids": ids.map(|i| i.0)})).unwrap(); + s.execute("object.distribute", &json!({"horizontal": "left"})).unwrap(); + s.doc_mut().unwrap().mark_saved(); + let before = history(&s); + s.execute("object.distribute", &json!({"horizontal": "left"})).unwrap(); + assert_eq!(history(&s), before, "three objects the first Distribute spaced"); +} + +/// Move by 0 pt, Rotate and Shear by 0°, Scale to 100%, an identity Transform, Set Bounds with the +/// current position and size, a Nudge of 0 (of objects, anchors or ruler guides), a Transform with +/// no object to transform and a Selection tool drag that ends where it started record no undo +/// step. Transform Again then repeats the identity, and a Nudge of 0 of a live rectangle's anchor +/// leaves the rectangle a live shape. +#[test] +fn a_transform_that_moves_nothing_records_no_undo_step() { + let mut s = session(); + // The Nudges of a guide and of anchors below use this ruler guide and this plain path. + s.execute("guide.add", &json!({"vertical": true, "pos": 600})).unwrap(); + let made = s.execute("path.create", &json!({"anchors": [{"x": 300, "y": 300}, {"x": 400, "y": 300}]})).unwrap(); + let path = NodeId(made["id"].as_u64().unwrap()); + let a = rect(&mut s, 10.0, 20.0, 40.0, 30.0); + select_with_a_step_to_redo(&mut s, &[a]); + let before = history(&s); + let calls = [ + ("object.move", json!({"dx": 0, "dy": 0})), + ("object.rotate", json!({"angle": 0})), + ("object.scale", json!({"sx": 100})), + ("object.shear", json!({"angle": 0})), + ("object.transform", json!({"matrix": [1, 0, 0, 1, 0, 0]})), + ("object.nudge", json!({"dx": 0, "dy": 0})), + ("object.setBounds", json!({})), + ("object.transformAgain", json!({})), + ]; + for (id, p) in calls { + assert_eq!(s.execute(id, &p).unwrap(), json!({"ids": [a.0]}), "{id} {p}"); + assert_eq!(history(&s), before, "{id} {p}"); + assert_eq!(s.journal.last().unwrap(), &(id.to_string(), p)); + } + assert_eq!(s.doc().unwrap().last_transform, Some((Affine::IDENTITY, false))); + // A Nudge of 0 with an anchor of the rectangle direct-selected keeps it a live shape. + let is_live = |s: &Session| matches!(s.doc().unwrap().doc.node(a).unwrap().kind, NodeKind::Path { live: Some(_), .. }); + assert!(is_live(&s), "a rectangle is a live shape"); + s.execute("select.anchors", &json!({"id": a.0, "anchors": [[0, 0]]})).unwrap(); + s.execute("object.nudge", &json!({"dx": 0, "dy": 0})).unwrap(); + assert!(is_live(&s), "a Nudge of 0 keeps it a live shape"); + assert_eq!(history(&s), before, "an anchor of the live rectangle, then object.nudge"); + // A Nudge of 0 with anchors direct-selected or with only a ruler guide selected, and a + // Transform with nothing selected or with ids that name no object, with or without a copy, + // record no undo step either. + let calls = [ + ("select.anchors", json!({"id": path.0, "anchors": [[0, 0]]}), "object.nudge", json!({"dx": 0, "dy": 0})), + ("guide.select", json!({"indexes": [0]}), "object.nudge", json!({"dx": 0, "dy": 0})), + ("select.set", json!({"ids": []}), "object.transform", json!({"matrix": [1, 0, 0, 1, 5, 0]})), + ("select.set", json!({"ids": []}), "object.transform", json!({"matrix": [1, 0, 0, 1, 5, 0], "copy": true})), + ("select.set", json!({"ids": [a.0]}), "object.transform", json!({"matrix": [1, 0, 0, 1, 5, 0], "ids": [999999]})), + ("select.set", json!({"ids": [a.0]}), "object.transform", json!({"matrix": [1, 0, 0, 1, 5, 0], "ids": [999999], "copy": true})), + ]; + for (select, sp, id, p) in calls { + s.execute(select, &sp).unwrap(); + s.execute(id, &p).unwrap(); + assert_eq!(history(&s), before, "{select} {sp}, then {id} {p}"); + } + // A copy of no object lists no copies and keeps the selection. + let p = json!({"matrix": [1, 0, 0, 1, 5, 0], "ids": [999999], "copy": true}); + assert_eq!(s.execute("object.transform", &p).unwrap(), json!({"ids": []})); + assert_eq!(s.doc().unwrap().selection.objects, [a]); + // A Selection tool drag that ends where it started records no undo step either. Back at the + // start, the revision advances, so views redraw the object where it was. + let v = ViewInfo::default(); + s.select_tool("selection", v).unwrap(); + let x0 = |s: &Session| s.doc().unwrap().doc.node(a).unwrap().geometric_bounds().unwrap().x0; + let revision = |s: &Session| s.doc().unwrap().revision; + s.pointer(&PointerEvent::new(PointerKind::Down, 30.0, 35.0), v).unwrap(); + s.pointer(&PointerEvent::new(PointerKind::Drag, 60.0, 35.0), v).unwrap(); + assert_ne!(x0(&s), 10.0, "the preview moves it"); + let moved = revision(&s); + s.pointer(&PointerEvent::new(PointerKind::Drag, 30.0, 35.0), v).unwrap(); + assert_eq!(x0(&s), 10.0); + assert!(revision(&s) > moved, "views redraw it back at the start"); + s.pointer(&PointerEvent::new(PointerKind::Up, 30.0, 35.0), v).unwrap(); + assert_eq!(history(&s), before, "a drag back to the start"); + assert_eq!(s.journal.last().unwrap().0, "object.transform"); + // With a copy, a Move by 0 pt duplicates the object in one undo step. + let count = |s: &Session| s.doc().unwrap().doc.layers[0].children().unwrap().len(); + let n = count(&s); + s.execute("object.move", &json!({"dx": 0, "dy": 0, "copy": true})).unwrap(); + assert_eq!(history(&s), (before.0 + 1, 0, true)); + assert_eq!(count(&s), n + 1); +} + +/// Arrange commands that leave the stacking order as it is record no undo step: Bring to Front and +/// Bring Forward on the front objects, Send to Back and Send Backward on the back ones, and Send to +/// Current Layer on objects already on top of the current layer. +#[test] +fn an_arrange_that_changes_no_order_records_no_undo_step() { + let mut s = session(); + let ids = [0.0, 30.0, 60.0].map(|x| rect(&mut s, x, 0.0, 20.0, 20.0)); + let order = |s: &Session| s.doc().unwrap().doc.layers[0].children().unwrap().iter().map(|n| n.id).collect::>(); + let calls = [ + (vec![ids[2]], "object.arrange.bringToFront"), + (vec![ids[2]], "object.arrange.bringForward"), + (vec![ids[0]], "object.arrange.sendToBack"), + (vec![ids[0]], "object.arrange.sendBackward"), + (vec![ids[2]], "object.arrange.sendToCurrentLayer"), + (vec![ids[1], ids[2]], "object.arrange.bringToFront"), + (vec![ids[1], ids[2]], "object.arrange.bringForward"), + (vec![ids[0], ids[1]], "object.arrange.sendToBack"), + (vec![ids[0], ids[1]], "object.arrange.sendBackward"), + (vec![ids[1], ids[2]], "object.arrange.sendToCurrentLayer"), + ]; + for (sel, c) in calls { + select_with_a_step_to_redo(&mut s, &sel); + let before = history(&s); + s.execute(c, &json!({})).unwrap(); + assert_eq!(history(&s), before, "{c} {sel:?}"); + assert_eq!(order(&s), ids, "{c} {sel:?}"); + } + // Bring to Front on the back object is one undo step, and it clears Redo. + select_with_a_step_to_redo(&mut s, &[ids[0]]); + let undo = history(&s).0; + s.execute("object.arrange.bringToFront", &json!({})).unwrap(); + assert_eq!(history(&s), (undo + 1, 0, true)); + assert_eq!(order(&s), [ids[1], ids[2], ids[0]]); +} + +/// Ungroup, Release Compound Path and Release Clipping Mask with none of their objects selected, +/// and Expand Shape with no live shape selected, record no undo step and keep the selection and its +/// key object. +#[test] +fn ungroup_release_and_expand_shape_with_nothing_to_act_on_record_no_undo_step() { + let mut s = session(); + let a = rect(&mut s, 0.0, 0.0, 20.0, 20.0); + let b = rect(&mut s, 40.0, 0.0, 20.0, 20.0); + s.execute("select.set", &json!({"ids": [a.0, b.0]})).unwrap(); + // Expand Shape makes the two rectangles plain paths in one undo step. + let undo = history(&s).0; + s.execute("object.expandShape", &json!({})).unwrap(); + assert_eq!(history(&s).0, undo + 1); + select_with_a_step_to_redo(&mut s, &[a, b]); + s.execute("select.key", &json!({"id": a.0})).unwrap(); + let (before, selection) = (history(&s), s.doc().unwrap().selection.clone()); + assert_eq!(selection.key, Some(a)); + for c in ["object.ungroup", "object.compoundPath.release", "object.clippingMask.release", "object.expandShape"] { + s.execute(c, &json!({})).unwrap(); + assert_eq!(history(&s), before, "{c}"); + assert_eq!(s.doc().unwrap().selection, selection, "{c}"); + assert_eq!(s.journal.last().unwrap().0, c); + } + // Ungroup with a group selected is one undo step. + s.execute("object.group", &json!({})).unwrap(); + let undo = history(&s).0; + s.execute("object.ungroup", &json!({})).unwrap(); + assert_eq!(history(&s).0, undo + 1); +} + +/// Unlock All with nothing locked and Show All with nothing hidden record no undo step and keep the +/// selection. Lock with every selected object already locked and Hide with every one already +/// hidden record no undo step and deselect them. +#[test] +fn lock_hide_unlock_all_and_show_all_with_nothing_to_do_record_no_undo_step() { + let mut s = session(); + let a = rect(&mut s, 0.0, 0.0, 20.0, 20.0); + select_with_a_step_to_redo(&mut s, &[a]); + let before = history(&s); + for c in ["object.unlockAll", "object.showAll"] { + assert_eq!(s.execute(c, &json!({})).unwrap(), json!({"count": 0}), "{c}"); + assert_eq!(history(&s), before, "{c}"); + assert_eq!(s.doc().unwrap().selection.objects, [a], "{c}"); + } + // With the object locked or hidden, each is one undo step that selects it. + for (off, on) in [("object.lock", "object.unlockAll"), ("object.hide", "object.showAll")] { + s.execute(off, &json!({})).unwrap(); + let undo = history(&s).0; + assert_eq!(s.execute(on, &json!({})).unwrap(), json!({"count": 1}), "{on}"); + assert_eq!(history(&s).0, undo + 1, "{on}"); + assert_eq!(s.doc().unwrap().selection.objects, [a], "{on}"); + } + // Object Properties locks (then hides) a new rectangle `b`, and `select.set` selects it. + // Lock (then Hide) deselects it and records no undo step. With `a` selected too, it is one + // undo step. + for (c, props) in [("object.lock", json!({"locked": true})), ("object.hide", json!({"visible": false}))] { + let b = rect(&mut s, 40.0, 0.0, 20.0, 20.0); + s.execute("object.setProps", &props).unwrap(); + select_with_a_step_to_redo(&mut s, &[a]); + s.execute("select.set", &json!({"ids": [b.0]})).unwrap(); + let before = history(&s); + s.execute(c, &json!({})).unwrap(); + assert_eq!(history(&s), before, "{c}"); + assert!(s.doc().unwrap().selection.objects.is_empty(), "{c}"); + s.execute("select.set", &json!({"ids": [a.0, b.0]})).unwrap(); + s.execute(c, &json!({})).unwrap(); + assert_eq!(history(&s), (before.0 + 1, 0, true), "{c} with a"); + s.execute("edit.undo", &json!({})).unwrap(); + } +} + +/// Set Bounds with the current position and size records no undo step on a rotated object, with +/// and without Use Preview Bounds, and on an unrotated object with Use Preview Bounds. In these +/// cases computing the transform can leave a rounding error in it. +#[test] +fn set_bounds_with_the_current_position_and_size_records_no_undo_step() { + let mut s = session(); + let a = rect(&mut s, 10.0, 20.0, 40.0, 30.0); + s.execute("stroke.set", &json!({"weight": 5})).unwrap(); + s.execute("object.rotate", &json!({"angle": 30})).unwrap(); + let c = rect(&mut s, 0.1, 0.05, 10.1, 6.06); + s.execute("stroke.set", &json!({"weight": 0.3})).unwrap(); + select_with_a_step_to_redo(&mut s, &[a]); + let before = history(&s); + for (id, preview) in [(a, false), (a, true), (c, true)] { + s.execute("select.set", &json!({"ids": [id.0]})).unwrap(); + s.execute("prefs.set", &json!({"key": "usePreviewBounds", "value": preview})).unwrap(); + // The Transform panel's values: X and Y place the reference point, W and H are the sides. + let b = s.transform_box(&[id]).unwrap(); + let (rp, w, h) = (b.reference_point(4), b.rect.width(), b.rect.height()); + for p in [json!({}), json!({"x": rp.x, "y": rp.y, "width": w, "height": h}), json!({"width": w, "proportional": true})] { + assert_eq!(s.execute("object.setBounds", &p).unwrap(), json!({"ids": [id.0]}), "{p}"); + assert_eq!(history(&s), before, "{id:?}, Use Preview Bounds {preview}: {p}"); + assert_eq!(s.journal.last().unwrap().0, "object.setBounds"); + } + } + // A new width is one undo step, and it clears Redo. + s.execute("object.setBounds", &json!({"width": 50})).unwrap(); + assert_eq!(history(&s), (before.0 + 1, 0, true)); +} + +/// Object Properties with the values the objects already have records no undo step: every +/// property, data already set or already absent, and no object. A value that differs on one of +/// the objects is one undo step, and an id that names no object is still an error. +#[test] +fn object_properties_already_set_record_no_undo_step() { + let mut s = session(); + let a = rect(&mut s, 0.0, 0.0, 20.0, 20.0); + let b = rect(&mut s, 40.0, 0.0, 20.0, 20.0); + let set = json!({"ids": [a.0], "name": "A", "opacity": 50, "blend": "Multiply", "knockout": "on", "data": {"pivot": "1,2"}}); + s.execute("object.setProps", &set).unwrap(); + select_with_a_step_to_redo(&mut s, &[a, b]); + let before = history(&s); + let defaults = json!({ + "id": b.0, "name": "", "visible": true, "locked": false, "opacity": 100, "blend": "Normal", "isolate": false, + "knockout": "neutral", "knockoutShape": false, "data": {"pivot": null} + }); + let calls = + [set, defaults, json!({"id": a.0, "data": {"data-pivot": "1,2", "other": null}}), json!({"visible": true}), json!({}), json!({"ids": []})]; + for p in calls { + s.execute("object.setProps", &p).unwrap(); + assert_eq!(history(&s), before, "{p}"); + assert_eq!(s.journal.last().unwrap(), &("object.setProps".to_string(), p)); + } + // `a` already has 50% opacity, and `b` does not. + s.execute("object.setProps", &json!({"opacity": 50})).unwrap(); + assert_eq!(history(&s), (before.0 + 1, 0, true)); + assert!(s.execute("object.setProps", &json!({"ids": [999999], "visible": true})).is_err()); +} + +/// Live Shape Properties that a shape already has record no undo step: a polygon's sides, angle, +/// radius, side length and equal sides, an ellipse's pie, Invert Pie on a whole ellipse, and the +/// radius and kind its corners have (given corners, the Direct-Selected one, or all). A value that +/// differs is one undo step. Setting the radius of a rectangle that kept a scale in its transform +/// is one undo step too, because it folds the scale into the rectangle's size. +#[test] +fn live_shape_properties_already_set_record_no_undo_step() { + let mut s = session(); + let id = |v: Value| NodeId(v["id"].as_u64().unwrap()); + let r = rect(&mut s, 0.0, 0.0, 100.0, 60.0); + let polygon = id(s.execute("shape.polygon", &json!({"cx": 300, "cy": 100, "radius": 50, "sides": 6})).unwrap()); + let ellipse = id(s.execute("shape.ellipse", &json!({"x": 0, "y": 200, "width": 80, "height": 40})).unwrap()); + let star = id(s.execute("shape.star", &json!({"cx": 300, "cy": 300, "radius1": 60, "radius2": 30})).unwrap()); + // The rectangle's corners get 8 pt radii, its top-right corner gets 12 pt, and the polygon gets + // 30 pt sides. + for p in [json!({"id": r.0, "radius": 8}), json!({"id": r.0, "corners": [1], "radius": 12}), json!({"id": polygon.0, "sideLength": 30})] { + s.execute("object.setLiveShape", &p).unwrap(); + } + let NodeKind::Path { live: Some(l), .. } = &s.doc().unwrap().doc.node(polygon).unwrap().kind else { panic!("not a live polygon") }; + let (angle, radius) = (l.polygon_angle().unwrap(), l.polygon_radius().unwrap()); + select_with_a_step_to_redo(&mut s, &[r, polygon, ellipse, star]); + let before = history(&s); + let calls = [ + json!({"id": polygon.0, "sides": 6, "polygonAngle": angle, "polygonRadius": radius, "makeSidesEqual": true}), + json!({"id": polygon.0, "sideLength": 30, "kind": "round"}), + json!({"id": ellipse.0, "pieStart": 0, "pieEnd": 360}), + json!({"id": ellipse.0, "invertPie": true}), + json!({"id": r.0, "corners": [0, 2, 3], "radius": 8, "kind": "round"}), + json!({"items": [{"id": r.0, "corners": [1]}], "radius": 12}), + json!({"id": star.0, "radius": 0}), + ]; + for p in calls { + s.execute("object.setLiveShape", &p).unwrap(); + assert_eq!(history(&s), before, "{p}"); + assert_eq!(s.journal.last().unwrap(), &("object.setLiveShape".to_string(), p)); + } + // With an anchor of the top-left corner direct-selected, the radius applies to that corner. + s.execute("select.anchors", &json!({"id": r.0, "anchors": [[0, 0]]})).unwrap(); + s.execute("object.setLiveShape", &json!({"radius": 8})).unwrap(); + assert_eq!(history(&s), before, "the Direct-Selected corner"); + for p in [json!({"id": polygon.0, "sides": 7}), json!({"id": ellipse.0, "pieEnd": 90}), json!({"radius": 9})] { + let undo = history(&s).0; + s.execute("object.setLiveShape", &p).unwrap(); + assert_eq!(history(&s).0, undo + 1, "{p}"); + } + // The rectangle is now 50 × 60 pt with 10 pt corners, stretched to 100 × 60 pt by its + // transform, as files from before #291 saved it. Setting the radius its corners show makes the + // corners circles. + { + let d = std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().doc); + let NodeKind::Path { path, live: Some(live), .. } = &mut d.node_mut(r).unwrap().kind else { panic!("not a live rectangle") }; + let xf = Affine::scale_non_uniform(2.0, 1.0); + *live = vectorcraft_doc::LiveShape::Rectangle { w: 50.0, h: 60.0, radii: [10.0; 4], kinds: Default::default(), xf }; + *path = live.to_path(); + } + let shown = vectorcraft_doc::LiveCorners::of(s.doc().unwrap().doc.node(r).unwrap()).unwrap().radius(0); + let undo = history(&s).0; + s.execute("object.setLiveShape", &json!({"id": r.0, "radius": shown})).unwrap(); + assert_eq!(history(&s).0, undo + 1, "a stretched rectangle"); +} + #[test] fn rectangle_tool_draws() { let mut s = session(); diff --git a/crates/ui-egui/src/shortcuts.rs b/crates/ui-egui/src/shortcuts.rs index 3cd5716f..3908fa7a 100644 --- a/crates/ui-egui/src/shortcuts.rs +++ b/crates/ui-egui/src/shortcuts.rs @@ -624,7 +624,8 @@ mod tests { out.textures_delta.clear(); }; let kind = |app: &VectorcraftApp| app.ui.dialog.as_ref().map(|d| d.kind.clone()); - let steps = |app: &VectorcraftApp| app.session.active().unwrap().history.undo.len(); + // The undo steps and the journal entries (the dialog's OK runs its command, which the journal records). + let steps = |app: &VectorcraftApp| (app.session.active().unwrap().history.undo.len(), app.session.journal.len()); app.select_tool("selection"); frame(&mut app, vec![enter()]); assert_eq!(kind(&app), None, "nothing is selected"); @@ -645,9 +646,11 @@ mod tests { frame(&mut app, vec![]); assert_eq!(kind(&app).as_deref(), Some(dialog), "{tool}: and it stays open"); assert_eq!(steps(&app), before, "{tool}: the Enter that opened it didn't confirm it"); - // The next Enter is its OK. + // The next Enter is its OK. With its default values only Reflect changes the art, so only + // Reflect records an undo step. frame(&mut app, vec![enter()]); - assert_eq!((kind(&app), steps(&app)), (None, before + 1), "{tool}: Enter confirms"); + let after = (before.0 + usize::from(dialog == "reflect"), before.1 + 1); + assert_eq!((kind(&app), steps(&app)), (None, after), "{tool}: Enter confirms"); } app.select_tool("rectangle"); frame(&mut app, vec![enter()]); diff --git a/docs/mcp.md b/docs/mcp.md index d74f2083..92ac77be 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -284,7 +284,7 @@ objects' fills or strokes differ (`fillMixed` / `strokeMixed`, drawn as a "?" pr | `transform` | `{ids?, dx?, dy?, rotate?, scale?, scaleX?, scaleY?, reflect?, shear?, origin?, copy?}` | Runs move, rotate, scale, reflect, shear in that order. With `copy`, the first step duplicates. | | `create_graph` | `{type?, x, y, width, height, csv? \| series?, categories?, rows?}` | The nine Illustrator graph types: column, stacked column, bar, stacked bar, line, area, scatter, pie and radar. An empty CSV cell or a `null` in `rows` is a blank value; a number in straight quotes is a label. Edit later with `graph.setData` / `graph.setType` via `run_command`. | | `text_wrap` | `{ids?, offset?, invert?, release?}` | Area type below the objects (same layer) flows around them. | -| `undo` / `redo` | `{}` | | +| `undo` / `redo` | `{}` | Many calls that change nothing record no undo step, for example `object.align` with nothing to move, `object.move` by 0 without `copy` and `object.showAll` with nothing hidden. `undo` (`edit.undo`) after such a call undoes the step before it. `inspect_document` lists the undo steps as `history`. | With `mods.shift`, a marquee dragged with the Selection tool toggles the objects it reaches (the selected ones leave the selection, the others join it, the rest stays), and one dragged with Direct or Group Selection toggles the anchors