diff --git a/crates/engine/src/cmd.rs b/crates/engine/src/cmd.rs index 166436b0c8..94d8add68b 100644 --- a/crates/engine/src/cmd.rs +++ b/crates/engine/src/cmd.rs @@ -46,7 +46,7 @@ pub mod importmatches; pub(crate) mod inline; mod interactivity; mod intertwine; -mod layer; +pub(crate) mod layer; mod layerpanel; pub mod library; pub mod linkfiles; diff --git a/crates/engine/src/cmd/addons/cleanup/checks.rs b/crates/engine/src/cmd/addons/cleanup/checks.rs index 52b20cdddc..be32cf0a83 100644 --- a/crates/engine/src/cmd/addons/cleanup/checks.rs +++ b/crates/engine/src/cmd/addons/cleanup/checks.rs @@ -10,7 +10,7 @@ use vectorcraft_geom::{PathData, Point, Rect}; use vectorcraft_pathops::addons as po; use super::super::super::edit::outermost; -use super::super::super::layer::empty_artboards; +use super::super::super::layer::{empty_artboards, remove_artboards}; use crate::Result; /// Sine of the turn below which an anchor on a straight run is collinear. @@ -508,35 +508,6 @@ fn artboards(d: &mut Document, sel: &mut Selection, scope: &Scope) -> Result = HashSet::with_capacity(empty.len()); - let mut i = 0; - d.artboards.retain(|a| { - let keep = empty.binary_search(&i).is_err(); - i += 1; - if !keep { - gone.insert(a.id); - } - keep - }); - count_artboard_pass(); - for &id in &gone { - d.retire_artboard_id(id); - } - d.retain_guides(sel, |g| !g.artboard.is_some_and(|a| gone.contains(&a))); - count_artboard_pass(); + remove_artboards(d, sel, &empty)?; Ok(Outcome { count: empty.len(), ids: vec![], artboards: empty }) } - -#[cfg(test)] -thread_local! { - /// Tests: how many walks [`artboards`] made over the artboards and the ruler guides on this - /// thread. - pub(super) static ARTBOARD_PASSES: std::cell::Cell = const { std::cell::Cell::new(0) }; -} - -/// Tests: count one walk over the artboards or the ruler guides. -fn count_artboard_pass() { - #[cfg(test)] - ARTBOARD_PASSES.set(ARTBOARD_PASSES.get() + 1); -} diff --git a/crates/engine/src/cmd/addons/cleanup/tests.rs b/crates/engine/src/cmd/addons/cleanup/tests.rs index 2ccdce9dad..4d4ed6461b 100644 --- a/crates/engine/src/cmd/addons/cleanup/tests.rs +++ b/crates/engine/src/cmd/addons/cleanup/tests.rs @@ -502,7 +502,7 @@ fn removing_many_selected_objects_looks_at_each_root_once_a_pass() { #[test] fn deleting_many_empty_artboards_walks_the_artboards_and_guides_once() { - use super::checks::ARTBOARD_PASSES; + use crate::cmd::layer::ARTBOARD_PASSES; let mut s = session(); rect(&mut s, 200.0, 20.0, 80.0, 60.0); let (keep, gone) = s diff --git a/crates/engine/src/cmd/layer.rs b/crates/engine/src/cmd/layer.rs index f589ea62da..83f27cdd61 100644 --- a/crates/engine/src/cmd/layer.rs +++ b/crates/engine/src/cmd/layer.rs @@ -994,20 +994,53 @@ fn artboard_delete(s: &mut Session, p: &Value) -> Result { ok() } -/// Delete artboards `indices` (sorted) of `d` with their guides; at least one must be left. -fn remove_artboards(d: &mut Document, sel: &mut vectorcraft_doc::Selection, indices: &[usize]) -> Result<()> { +/// Delete artboards `indices` (sorted, each once) of `d` with their guides; at least one must be +/// left. The other artboards keep their order, no later artboard takes a deleted one's id, and +/// `sel` forgets the deleted guides. One walk over the artboards and one over the ruler guides, +/// however many go. Delete Artboard, Delete Empty Artboards, Cut Artboard and Artwork Cleanup's +/// Empty Artboards. +pub(crate) fn remove_artboards(d: &mut Document, sel: &mut vectorcraft_doc::Selection, indices: &[usize]) -> Result<()> { + if indices.iter().any(|i| *i >= d.artboards.len()) { + return Err(EngineError::Other("no such artboard".into())); + } if indices.len() >= d.artboards.len() { return Err(EngineError::Other("a document needs at least one artboard".into())); } - // The last first, so the others keep their places; their guides go with them. - for &i in indices.iter().rev() { - let id = d.artboards.remove(i).id; - d.retire_artboard_id(id); - d.retain_guides(sel, |g| g.artboard != Some(id)); + if indices.is_empty() { + return Ok(()); } + let mut gone: HashSet = HashSet::with_capacity(indices.len()); + let mut i = 0; + d.artboards.retain(|a| { + let keep = indices.binary_search(&i).is_err(); + i += 1; + if !keep { + gone.insert(a.id); + } + keep + }); + count_artboard_pass(); + for &id in &gone { + d.retire_artboard_id(id); + } + d.retain_guides(sel, |g| !g.artboard.is_some_and(|a| gone.contains(&a))); + count_artboard_pass(); Ok(()) } +#[cfg(test)] +thread_local! { + /// Tests: how many walks [`remove_artboards`] made over the artboards and the ruler guides on + /// this thread. + pub(crate) static ARTBOARD_PASSES: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + +/// Tests: count one walk over the artboards or the ruler guides. +fn count_artboard_pass() { + #[cfg(test)] + ARTBOARD_PASSES.set(ARTBOARD_PASSES.get() + 1); +} + /// The artboards Delete Empty Artboards deletes: those with no artwork /// ([`Document::artboard_has_art`]) that aren't locked, but artboard 1 when every artboard is one /// of them (a document keeps one, as Delete Artboards keeps artboard 1). diff --git a/crates/engine/src/cmd/panelcmds.rs b/crates/engine/src/cmd/panelcmds.rs index 5910435a75..6f4ba513f2 100644 --- a/crates/engine/src/cmd/panelcmds.rs +++ b/crates/engine/src/cmd/panelcmds.rs @@ -345,15 +345,7 @@ pub(crate) fn artboard_copy(s: &mut Session, p: &Value, cut: bool) -> Result= d.artboards.len()) { - return Err(EngineError::Other("no such artboard".into())); - } - // The last first, so the others keep their places; their guides go with them. - for &i in indices.iter().rev() { - let gone = d.artboards.remove(i).id; - d.retire_artboard_id(gone); - d.retain_guides(sel, |g| g.artboard != Some(gone)); - } + super::layer::remove_artboards(d, sel, &indices)?; d.remove_many(&art)?; sel.prune(d); Ok(()) diff --git a/crates/engine/src/tests/panelcmds.rs b/crates/engine/src/tests/panelcmds.rs index ac39de0646..8174cf36d1 100644 --- a/crates/engine/src/tests/panelcmds.rs +++ b/crates/engine/src/tests/panelcmds.rs @@ -1683,6 +1683,95 @@ fn delete_empty_artboards_deletes_those_with_no_art_in_one_step() { assert_eq!(s.doc().unwrap().doc.ruler_guides().len(), 3); } +/// `n` artboards in a row, each with a guide, and a guide on no artboard; every guide is selected, +/// and the key guide is the one on artboard `key`. +fn artboards_with_selected_guides(n: usize, key: usize) -> Session { + let mut s = row_of_artboards(n); + s.edit("selection fixture", |d, sel| { + d.add_ruler_guide(&vectorcraft_doc::Guide::new(false, 75.0)); + sel.guides = d.ruler_guides().iter().filter_map(|g| g.id).collect(); + let on_key = d.artboards[key].id; + sel.key_guide = d.ruler_guides().iter().find(|g| g.artboard == Some(on_key)).and_then(|g| g.id); + Ok(()) + }) + .unwrap(); + s +} + +/// A document's artboards, ruler guides, selected guides and key guide. +type BoardsAndGuides = (Vec, Vec, Vec, Option); + +/// The artboards, guides, selected guides and key guide after deleting artboards `indices` of the +/// open document one at a time, the last first. A clone shares the document's retired artboard id, +/// so the clone retires none. +fn deleted_one_at_a_time(s: &Session, indices: &[usize]) -> BoardsAndGuides { + let st = s.doc().unwrap(); + let (mut d, mut sel) = ((*st.doc).clone(), st.selection.clone()); + for &i in indices.iter().rev() { + let id = d.artboards.remove(i).id; + d.retain_guides(&mut sel, |g| g.artboard != Some(id)); + } + let guides = d.ruler_guides(); + (d.artboards, guides, sel.guides, sel.key_guide) +} + +/// The same as [`deleted_one_at_a_time`] for the open document as it is. +fn artboards_and_guides(s: &Session) -> BoardsAndGuides { + let st = s.doc().unwrap(); + (st.doc.artboards.clone(), st.doc.ruler_guides(), st.selection.guides.clone(), st.selection.key_guide) +} + +/// Delete Artboard with many artboards chosen walks the artboards once and the ruler guides once, +/// and leaves the same artboards, guides and selected guides as deleting them one at a time, in +/// one undo step. +#[test] +fn deleting_many_artboards_walks_the_artboards_and_guides_once() { + use crate::cmd::layer::ARTBOARD_PASSES; + let mut s = artboards_with_selected_guides(40, 7); + let indices: Vec = (0..40).filter(|i| i % 2 == 1 || *i == 38).collect(); + let expected = deleted_one_at_a_time(&s, &indices); + let steps = s.doc().unwrap().history.undo.len(); + ARTBOARD_PASSES.set(0); + s.execute("artboard.delete", &json!({ "indices": indices })).unwrap(); + assert_eq!(ARTBOARD_PASSES.get(), 2, "one walk over the artboards and one over the guides"); + let now = artboards_and_guides(&s); + assert_eq!(now, expected); + assert_eq!(now.0.len(), 19); + assert_eq!(now.1.len(), 20, "the kept artboards' guides and the guide on no artboard stay"); + assert_eq!((now.2.len(), now.3), (20, None), "the deleted guides leave the selection, the key guide with them"); + assert_eq!(s.doc().unwrap().history.undo.len(), steps + 1); + s.execute("edit.undo", &json!({})).unwrap(); + assert_eq!(s.doc().unwrap().doc.artboards.len(), 40); + assert_eq!(s.doc().unwrap().doc.ruler_guides().len(), 41); +} + +/// Delete Empty Artboards and Cut Artboard delete their artboards in the same single pass, with the +/// same result as deleting them one at a time. +#[test] +fn delete_empty_and_cut_artboards_walk_the_artboards_and_guides_once() { + use crate::cmd::layer::ARTBOARD_PASSES; + let mut s = artboards_with_selected_guides(30, 0); + box_at(&mut s, 4010.0, 20.0); + let empty: Vec = (0..30).filter(|i| *i != 10).collect(); + let expected = deleted_one_at_a_time(&s, &empty); + ARTBOARD_PASSES.set(0); + assert_eq!(s.execute("artboard.deleteEmpty", &json!({})).unwrap(), json!({ "deleted": empty })); + assert_eq!(ARTBOARD_PASSES.get(), 2); + assert_eq!(artboards_and_guides(&s), expected); + assert_eq!(artboard_names(&s), ["Artboard 11"]); + + let mut s = artboards_with_selected_guides(30, 4); + let cut: Vec = (2..28).collect(); + let expected = deleted_one_at_a_time(&s, &cut); + ARTBOARD_PASSES.set(0); + s.execute("artboard.cut", &json!({ "indices": cut, "art": false })).unwrap(); + assert_eq!(ARTBOARD_PASSES.get(), 2); + assert_eq!(artboards_and_guides(&s), expected); + assert_eq!(s.doc().unwrap().doc.artboards.len(), 4); + assert!(s.execute("artboard.cut", &json!({ "indices": [0, 1, 2, 3], "art": false })).is_err(), "a document keeps one artboard"); + assert_eq!(s.doc().unwrap().doc.artboards.len(), 4); +} + /// What counts as art: hidden and locked objects, an object straddling two artboards and one /// that only touches an artboard's edge; a group counts by its members, not by its box. #[test] diff --git a/docs/target-app-parity.md b/docs/target-app-parity.md index 9a9b0d06ed..e3a94f7544 100644 --- a/docs/target-app-parity.md +++ b/docs/target-app-parity.md @@ -655,6 +655,7 @@ since 2026-10-01. - **Delete Empty Artboards:** the Artboards panel menu item ([Delete artboards](https://helpx.adobe.com/illustrator/desktop/create-manage-artboards/add-edit-artboards/delete-artboards.html)) runs `artboard.deleteEmpty`: every unlocked artboard with no artwork goes with its guides in one undo step, artboard 1 stays when every artboard is empty and none is locked, and the panel makes an artboard that is left active; an artboard is empty when no object (hidden, locked and template art included; guides not) has visual bounds that meet or touch it, a group counting by its members and a clip group or clipping layer by its members inside the clip (our reading of "no artwork"; the pages give no finer rule). Engine tests in `tests/panelcmds.rs` (`delete_empty_artboards_*`) and the panel test `delete_empty_artboards_deletes_the_empty_ones_and_selects_one_left`. The menu item's enabled check measures nothing in a document with one artboard, and Save Gradient Preset's check reads the proxy's paint without measuring the object; both give the same answer as before. An artboard that no path's control points, grown by the farthest its strokes reach, can meet is found empty without measuring the path. With an open path of 200,000 anchors selected, the checks take under 0.01 ms instead of 21.9 ms and 17.2 ms in a release build; with a second, empty artboard, Delete Empty Artboards' check takes 0.85 ms instead of 42.1 ms. Tests `delete_empty_artboards_is_enabled_exactly_when_it_deletes_something`, `delete_empty_artboards_enabled_state_matches_the_command_on_varied_documents`, `empty_artboard_checks_match_the_visual_bounds_on_varied_strokes`, `the_empty_artboard_check_matches_measuring_every_artboard`, `the_save_check_matches_saving_with_and_without_a_gradient`, `the_unmeasured_proxy_paint_is_the_measured_one` and `the_unmeasured_proxy_paint_is_the_measured_one_on_varied_objects`. Delete Empty Artboards' enabled check measures each version of a document once, so the panel menu does not measure the document again on every frame while it is open. With an open path of 200,000 anchors across two artboards, the check takes under 0.001 ms on a later frame instead of 70.4 ms in a release build, and with 20,000 objects on two artboards it takes under 0.001 ms instead of 9.1 ms. Tests `delete_empty_artboards_check_measures_a_document_once`, `delete_empty_artboards_check_follows_each_change_to_the_document`, `delete_empty_artboards_check_follows_a_change_in_place`, `delete_empty_artboards_check_follows_a_document_switch` and `delete_empty_artboards_check_matches_measuring_after_varied_changes`. + Delete Artboards, Delete Empty Artboards and Cut Artboard remove all their artboards and those artboards' guides in one walk over the artboards and one over the ruler guides and layers, however many artboards go, instead of one walk over every guide and layer per artboard. The artboards, guides and selected guides left are the same as deleting the artboards one at a time. Tests `deleting_many_artboards_walks_the_artboards_and_guides_once` and `delete_empty_and_cut_artboards_walk_the_artboards_and_guides_once`. The Eyedropper, the Live Paint check for paint to sample, the Appearance panel's Characters chips and the colors the Libraries panel adds from the selection also read the proxy's paint without measuring the object; the Eyedropper measures it only to sample a gradient or a pattern, and the Libraries panel only for a gradient. The colors and paints are the same as before. With an open path of 200,000 anchors, the Live Paint check and the Eyedropper's sample for a point inside it take 0.004 ms together instead of 26.0 ms in a release build. Tests `samples_give_the_measured_proxy_paints_colors` and `colors_and_gradients_come_from_compound_paths_groups_and_colorized_objects`. - **Artboard history selection:** headless pointer/key tests in `panels/properties.rs` pass for panel New → Undo → Delete/Backspace/Properties, Duplicate → Undo → Delete, pruning removed indices and preserving valid multiselection. They assert board survivors, artwork and object selection, Move/Scale Artwork options, saved state and Undo/Redo. Full workspace CI passes. - **Artboard tab-switch follow-up:** changing the active document through its tab aligns the Artboard tool with the displayed board before Properties Delete; clicking the already-active tab preserves multiselection and Move/Scale Artwork options. Three headless `chrome::tests::artboard_document_tab_*` regressions cover actual tab A → B → A clicks followed by Properties Delete, surviving boards and guides, preserved artwork/symbols and Undo/Redo; controls cover same-tab multiselection, Move/Scale Artwork options and other tools. Historical focused validation on `dbd2497b` recorded one expected RED failure with two passing controls, then all three GREEN tests passed. All three also passed in historical full CI on `a0671777`. A full CI retry on the five-unit combined source at `a0671777` passed all seven stages with 6,613 test outcomes on 2026-10-14. An earlier full-CI run on the identical source panicked in the randomized `ai_mutated_save_never_panics` test; the cause remains unresolved. This five-fix batch does not claim to repair that failure.