From 758a31890dcc09de03dc9e11bc932a807aa2b090 Mon Sep 17 00:00:00 2001 From: Drew Dibble Date: Mon, 9 Nov 2026 14:18:05 -0700 Subject: [PATCH] Artboards: Fit to Artwork Bounds' kept answer follows the document's version and the plug-ins fit_has_art kept its answer for the document's uid and revision, which don't follow every change of what it measures: a document's tab coming to the front replaces it with its composite fonts synced, and a plug-in installed or removed changes what its effects draw, with no revision moving either time. The answer is now kept for that document Arc, held weakly as DocState::any_empty_artboards keeps its own, and the artboards asked about, until the plug-ins' revision moves. A selection or view change keeps it. The test replaces the document with art on an artboard, its revision as it was, and asks again: kept by revision, the answer was stale. --- crates/ui-egui/src/panels/artboards.rs | 28 +++++++++++++------- crates/ui-egui/src/panels/artboards/tests.rs | 12 ++++----- 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/crates/ui-egui/src/panels/artboards.rs b/crates/ui-egui/src/panels/artboards.rs index b167b1175..a950a24c7 100644 --- a/crates/ui-egui/src/panels/artboards.rs +++ b/crates/ui-egui/src/panels/artboards.rs @@ -313,23 +313,31 @@ pub struct MenuFacts { } /// Whether an unlocked artboard of `indices` has art that Fit to Artwork Bounds fits it to. The -/// answer is kept for the document's revision: the canvas menu asks on every frame it is open -/// without its [`MenuFacts`], and measuring builds the solids of 3D effects. One answer is kept, -/// the last asked: a document's uid is never given to another in the process, and its revision -/// only moves forward (a document read again, or a batch rolled back, moves past it), so a kept -/// answer is never another document's or another state's. +/// canvas menu asks on every frame it is open without its [`MenuFacts`], and measuring builds the +/// solids of 3D effects, so one answer is kept, the last asked: for that version of the document +/// (the same `Arc`, held weakly so its address isn't reused, as +/// `DocState::any_empty_artboards` keeps its answer) and those artboards, until a plug-in is +/// installed or removed, which can change what its effects draw. A change of the selection or the +/// view keeps it. pub(crate) fn fit_has_art(app: &VectorcraftApp, indices: &[usize]) -> bool { - /// The document's uid and revision, the artboards asked about, and the answer. - type Kept = Option<(u64, u64, Vec, bool)>; - thread_local!(static KEPT: std::cell::RefCell = const { std::cell::RefCell::new(None) }); + /// The document measured, the artboards asked about, the plug-ins' revision, and the answer. + struct Kept { + doc: std::sync::Weak, + indices: Vec, + plugins: u64, + any: bool, + } + thread_local!(static KEPT: std::cell::RefCell> = const { std::cell::RefCell::new(None) }); app.session.active().is_some_and(|st| { - if let Some(any) = KEPT.with_borrow(|k| k.as_ref().filter(|k| k.0 == st.uid && k.1 == st.revision && k.2 == indices).map(|k| k.3)) { + let plugins = crate::menus::plugin_revision(); + let same = |k: &Kept| std::ptr::eq(k.doc.as_ptr(), std::sync::Arc::as_ptr(&st.doc)) && k.indices == indices && k.plugins == plugins; + if let Some(any) = KEPT.with_borrow(|k| k.as_ref().filter(|k| same(k)).map(|k| k.any)) { return any; } // One renderer measures every artboard: each object once. let mut r = vectorcraft_render::Renderer::new(); let any = indices.iter().filter_map(|i| st.doc.artboards.get(*i)).any(|a| !a.locked && r.art_bounds_on(&st.doc, a.rect).is_some()); - KEPT.set(Some((st.uid, st.revision, indices.to_vec(), any))); + KEPT.set(Some(Kept { doc: std::sync::Arc::downgrade(&st.doc), indices: indices.to_vec(), plugins, any })); any }) } diff --git a/crates/ui-egui/src/panels/artboards/tests.rs b/crates/ui-egui/src/panels/artboards/tests.rs index e10fd8ab3..23aab1015 100644 --- a/crates/ui-egui/src/panels/artboards/tests.rs +++ b/crates/ui-egui/src/panels/artboards/tests.rs @@ -1656,13 +1656,14 @@ fn the_open_menu_reads_what_the_right_click_measured() { assert!(labels.contains(&"Delete Empty Artboards") && !labels.contains(&"Fit to Artwork Bounds"), "{labels:?}"); } -/// Whether Fit to Artwork Bounds has art to fit is measured once for a revision of the document: -/// the canvas menu asks on every frame it is open. +/// Whether Fit to Artwork Bounds has art to fit is kept for a version of the document, not its +/// revision: a document replaced with no revision moved (composite fonts synced as its tab comes +/// to the front) is measured again. #[test] -fn fit_to_art_is_measured_once_a_revision() { +fn fit_to_art_is_measured_for_each_version_of_the_document() { let mut app = menu_app(2); assert!(!super::fit_has_art(&app, &[0, 1])); - // Art on the first artboard, the revision left as it was: the answer kept stands. + // Art on the first artboard, the revision left as it was. let st = app.session.active_mut().unwrap(); let mut doc = (*st.doc).clone(); let l = doc.layers[0].id; @@ -1674,9 +1675,8 @@ fn fit_to_art_is_measured_once_a_revision() { ); doc.insert(Some(l), 0, art).unwrap(); st.doc = std::sync::Arc::new(doc); - assert!(!super::fit_has_art(&app, &[0, 1]), "kept for the revision"); - app.session.active_mut().unwrap().revision += 1; assert!(super::fit_has_art(&app, &[0, 1]), "measured again"); + assert!(!super::fit_has_art(&app, &[1]), "measured for the artboards asked about"); } /// `artboard.selectAll` chooses the Artboard tool first when another tool is in use, and fails