Menus: Delete Empty Artboards' enabled check does not walk the document every frame

The Artboards panel menu asks whether Delete Empty Artboards can run on
every frame while it is open, and each ask measured the document. With
an open path of 200,000 anchors across two artboards, that took 70.4 ms
a frame in a release build.

The answer is now kept per open document with the version of the
document it was measured on, and is measured again when the document
changes. A later frame takes under 0.001 ms. The first frame after an
edit measures once. A change of the selection or the view keeps the
answer.
This commit is contained in:
dotCooCoo
2026-11-08 11:22:40 +07:00
committed by bflatastic
parent 2432807197
commit 2f09d9998e
4 changed files with 168 additions and 1 deletions
+1 -1
View File
@@ -955,7 +955,7 @@ fn any_empty_artboards(d: &Document) -> bool {
fn has_empty_artboards(s: &Session) -> std::result::Result<(), String> {
let st = s.active().ok_or("no document open")?;
if any_empty_artboards(&st.doc) { Ok(()) } else { Err("no empty artboards to delete".into()) }
if st.any_empty_artboards(any_empty_artboards) { Ok(()) } else { Err("no empty artboards to delete".into()) }
}
fn artboard_delete_empty(s: &mut Session, _: &Value) -> Result<Value> {
+31
View File
@@ -102,6 +102,24 @@ pub struct DocState {
/// paths the document links to are looked at within them later too, when the Links panel
/// asks about a link outside any caller's scope.
pub opened_by: Option<Arc<file_access::AutomationRoots>>,
/// What [`DocState::any_empty_artboards`] last measured. Not saved.
empty_artboards: std::cell::RefCell<EmptyArtboards>,
}
/// Whether a document has an artboard for Delete Empty Artboards to delete, with the document that
/// was measured.
#[derive(Clone, Debug, Default)]
struct EmptyArtboards {
/// The document measured. While this reference lasts, its allocation holds no other document,
/// and `Arc::make_mut` moves a document it changes to a new allocation.
doc: std::sync::Weak<Document>,
any: bool,
}
#[cfg(test)]
thread_local! {
/// Tests: how many times [`DocState::any_empty_artboards`] measured a document on this thread.
pub(crate) static EMPTY_ARTBOARD_MEASURES: std::cell::Cell<usize> = const { std::cell::Cell::new(0) };
}
static NEXT_DOC_UID: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(1);
@@ -280,8 +298,21 @@ impl DocState {
import_losses: vec![],
last_perspective: None,
opened_by: file_access::current(),
empty_artboards: Default::default(),
}
}
/// `measure`'s answer for the document now. `measure` runs once for each version of the document,
/// so a menu that asks on every frame measures it once, and a change of the selection or the view
/// keeps the answer.
pub(crate) fn any_empty_artboards(&self, measure: impl FnOnce(&Document) -> bool) -> bool {
let mut last = self.empty_artboards.borrow_mut();
if !std::ptr::eq(last.doc.as_ptr(), Arc::as_ptr(&self.doc)) {
#[cfg(test)]
EMPTY_ARTBOARD_MEASURES.with(|c| c.set(c.get() + 1));
*last = EmptyArtboards { doc: Arc::downgrade(&self.doc), any: measure(&self.doc) };
}
last.any
}
/// Unsaved changes: the document differs from the saved one (selection changes and changes that
/// are not edits to save don't count, [`DocState::same_content`]).
pub fn is_dirty(&self) -> bool {
+135
View File
@@ -1706,6 +1706,141 @@ fn delete_empty_artboards_enabled_state_matches_the_command_on_varied_documents(
assert!(enabled_count > 5 && enabled_count < 55, "both answers come up: {enabled_count} of 60 enabled");
}
/// Delete Empty Artboards' enabled state, as a menu asks for it, and how many times asking measured
/// a document on this thread.
fn delete_empty_enabled_counted(s: &Session) -> (bool, usize) {
let spec = crate::cmd::command_specs().iter().find(|c| c.id == "artboard.deleteEmpty").unwrap();
let measures = || crate::doc_state::EMPTY_ARTBOARD_MEASURES.with(std::cell::Cell::get);
let before = measures();
let enabled = (spec.enabled)(s).is_ok();
(enabled, measures() - before)
}
/// A menu asks on every frame while it is open: the first frame measures the document, and the
/// others, a change of the selection among them, do not.
#[test]
fn delete_empty_artboards_check_measures_a_document_once() {
let mut s = row_of_artboards(2);
let art = box_at(&mut s, 10.0, 20.0);
assert_eq!(delete_empty_enabled_counted(&s), (true, 1), "the first frame");
for _ in 0..10 {
assert_eq!(delete_empty_enabled_counted(&s), (true, 0), "a later frame");
}
s.execute("select.set", &json!({"ids": [art.0]})).unwrap();
assert_eq!(delete_empty_enabled_counted(&s), (true, 0), "after a change of the selection");
s.execute("select.none", &json!({})).unwrap();
assert_eq!(delete_empty_enabled_counted(&s), (true, 0), "after another change of the selection");
}
/// The enabled state follows an edit, an undo, a redo, an added and a removed artboard, a locked
/// artboard, and each change measures the document once.
#[test]
fn delete_empty_artboards_check_follows_each_change_to_the_document() {
let mut s = row_of_artboards(2);
box_at(&mut s, 10.0, 20.0);
assert_eq!(delete_empty_enabled_counted(&s), (true, 1), "artboard 2 is empty");
let on_second = box_at(&mut s, 410.0, 20.0);
assert_eq!(delete_empty_enabled_counted(&s), (false, 1), "an edit puts art on artboard 2");
assert_eq!(delete_empty_enabled_counted(&s), (false, 0));
s.execute("edit.undo", &json!({})).unwrap();
assert_eq!(delete_empty_enabled_counted(&s), (true, 1), "Undo takes the art off artboard 2");
assert_eq!(delete_empty_enabled_counted(&s), (true, 0));
s.execute("edit.redo", &json!({})).unwrap();
assert_eq!(delete_empty_enabled_counted(&s), (false, 1), "Redo puts it back");
s.execute("select.set", &json!({"ids": [on_second.0]})).unwrap();
s.execute("object.move", &json!({"dx": 1000, "dy": 0})).unwrap();
assert_eq!(delete_empty_enabled_counted(&s), (true, 1), "an edit moves the art off artboard 2");
s.execute("edit.undo", &json!({})).unwrap();
assert_eq!(delete_empty_enabled_counted(&s), (false, 1));
s.execute("artboard.new", &json!({"x": 2000, "y": 0, "width": 200, "height": 150})).unwrap();
assert_eq!(delete_empty_enabled_counted(&s), (true, 1), "a new artboard is empty");
s.execute("artboard.setProps", &json!({"index": 2, "locked": true})).unwrap();
assert_eq!(delete_empty_enabled_counted(&s), (false, 1), "a locked artboard stays");
s.execute("artboard.setProps", &json!({"index": 2, "locked": false})).unwrap();
assert_eq!(delete_empty_enabled_counted(&s), (true, 1), "unlocked, it can go");
s.execute("artboard.delete", &json!({"index": 2})).unwrap();
assert_eq!(delete_empty_enabled_counted(&s), (false, 1), "with the empty artboard removed, none is left");
assert_eq!(delete_empty_enabled_counted(&s), (false, 0));
}
/// A change made to the document directly, with nothing else holding it and no new revision, is
/// measured again too.
#[test]
fn delete_empty_artboards_check_follows_a_change_in_place() {
let mut s = row_of_artboards(2);
box_at(&mut s, 10.0, 20.0);
assert_eq!(delete_empty_enabled_counted(&s), (true, 1));
let st = s.doc_mut().unwrap();
assert_eq!(Arc::strong_count(&st.doc), 1, "the undo history holds the document before the edit");
let revision = st.revision;
Arc::make_mut(&mut st.doc).artboards.truncate(1);
assert_eq!(st.revision, revision);
assert_eq!(delete_empty_enabled_counted(&s), (false, 1), "one artboard left");
}
/// Each open document has its own answer: switching documents gives the other's without measuring
/// again, and an edit to one leaves the other's alone.
#[test]
fn delete_empty_artboards_check_follows_a_document_switch() {
let mut s = row_of_artboards(2);
assert_eq!(delete_empty_enabled_counted(&s), (true, 1), "two empty artboards: the second can go");
s.execute("file.new", &json!({"width": 200, "height": 150})).unwrap();
assert_eq!(delete_empty_enabled_counted(&s), (false, 1), "the new document has one artboard");
assert!(s.set_active(0));
assert_eq!(delete_empty_enabled_counted(&s), (true, 0), "the first document's answer stands");
assert!(s.set_active(1));
assert_eq!(delete_empty_enabled_counted(&s), (false, 0), "the second document's answer stands");
s.execute("artboard.new", &json!({"x": 400, "y": 0, "width": 200, "height": 150})).unwrap();
assert_eq!(delete_empty_enabled_counted(&s), (true, 1), "the second document has an empty artboard now");
assert!(s.set_active(0));
assert_eq!(delete_empty_enabled_counted(&s), (true, 0), "the edit to the second document leaves the first's answer");
}
/// On documents made from a fixed seed, after each of a run of changes, the enabled state is what
/// measuring every artboard of the document gives.
#[test]
fn delete_empty_artboards_check_matches_measuring_after_varied_changes() {
let mut seed: u64 = 0x0de1_e7e0_e3b7;
let mut next = |n: u64| {
seed = seed.wrapping_mul(6364136223846793005).wrapping_add(1442695040888963407);
(seed >> 33) % n
};
let mut answers = [0usize; 2];
let mut measured = 0;
for _ in 0..30 {
let mut s = row_of_artboards(1 + next(3) as usize);
for _ in 0..12 {
let artboards = s.doc().unwrap().doc.artboards.len();
let index = next(artboards as u64);
// A change can be refused (nothing to undo, a locked artboard): the state still has to match.
let _ = match next(7) {
0 => s.execute("shape.rectangle", &json!({"x": next(1200) as f64 - 50.0, "y": 10, "width": 20, "height": 20})),
1 => s.execute("edit.undo", &json!({})),
2 => s.execute("edit.redo", &json!({})),
3 => s.execute("artboard.new", &json!({"x": 400 * (artboards + 1), "y": 0, "width": 200, "height": 150})),
4 => s.execute("artboard.delete", &json!({"index": index})),
5 => s.execute("artboard.setProps", &json!({"index": index, "locked": next(2) == 0})),
_ => s.execute("select.all", &json!({})),
};
let (enabled, count) = delete_empty_enabled_counted(&s);
measured += count;
answers[usize::from(enabled)] += 1;
let d = &s.doc().unwrap().doc;
assert_eq!(enabled, d.artboards.len() > 1 && d.artboards.iter().any(|a| !a.locked && !d.artboard_has_art(a.rect)));
assert_eq!(delete_empty_enabled_counted(&s), (enabled, 0), "a second frame");
}
}
assert!(answers[0] > 30 && answers[1] > 30, "both answers come up: {answers:?}");
assert!(measured < 360, "changes of the selection alone do not measure: {measured} of 360");
}
/// On paths and compound paths made from a fixed seed (curves, open and closed, with strokes of
/// every join, cap and alignment, miter limits, arrowheads, width profiles and dashes), an
/// artboard has art exactly when an object's visual bounds meet it, for artboards just inside,
+1
View File
@@ -559,6 +559,7 @@ since 2026-10-01.
- **Selected artboard deletion:** the Artboards panel menu, panel trash button and Properties use the same selected set; selecting all retains artboard 1. Pointer-event tests cover nonadjacent and single selections, the original first-board survivor, ordinary artwork, Undo/Redo and the single-board guard. Public reference: [Delete artboards](https://helpx.adobe.com/illustrator/desktop/create-manage-artboards/add-edit-artboards/delete-artboards.html).
- **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`.
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.