From dcc05f310de89a6e39d920bcfcb2b11c1a47e684 Mon Sep 17 00:00:00 2001 From: Karim Jerbi Date: Sun, 20 Sep 2026 15:46:48 +0100 Subject: [PATCH] perf(statusbar): cache layout/scale derived data behind epochs (major) Status bar rescanned the whole object map twice per frame (layout_names + scale_picker_list), deep-cloned an already-cached Arc, and built two undo/redo label vecs consumed ~never. Cache layout names (epoch key) and scale picker (epoch + unit-factor + imperial-family + current-scale key) as Arc behind choke-point invalidation (layout/scale impl methods, undo/redo swaps, file-open, doc-reset); borrow across the widget boundary (owned-Arc params; E0515 forbids &'a slices over call-site temporaries); build undo/redo labels only when their dropdown is open. Tests: - cached_layout_names_returns_same_arc_until_bump: hit-path Arc identity - cached_layout_names_invalidated_by_add_layout: bump yields new Arc + names - cached_scale_picker_returns_same_arc_until_bump: hit-path Arc identity - cached_scale_picker_invalidated_by_add_scale: bump yields new Arc + scale - cached_scale_picker_invalidated_by_current_scale_change: key completeness Bench (cargo bench ui_statusbar_derived_data, 3k entities): cached hit below timer granularity vs ~1us uncached scans, PASS (50us threshold). --- benches/README.md | 3 +- benches/performance_benchmarks.rs | 67 ++++++++++++ src/app/automation.rs | 2 + src/app/history.rs | 27 +++++ src/app/update/file.rs | 2 + src/app/update/mod.rs | 2 +- src/app/update/viewport.rs | 2 +- src/app/view/mod.rs | 47 +++++++-- src/scene/entity.rs | 2 + src/scene/layout.rs | 12 +++ src/scene/mod.rs | 136 +++++++++++++++++++++++++ src/ui/popup/scale_popup.rs | 7 +- src/ui/popup/selection_filter_popup.rs | 8 +- src/ui/ribbon/mod.rs | 1 + src/ui/ribbon/widgets.rs | 4 +- src/ui/statusbar/mod.rs | 48 +++++---- 16 files changed, 327 insertions(+), 43 deletions(-) diff --git a/benches/README.md b/benches/README.md index a91fa0450..cb7aed311 100644 --- a/benches/README.md +++ b/benches/README.md @@ -19,7 +19,7 @@ cargo bench --bench performance_benchmarks -- --quick --- -## Benchmark Suite Catalog (26 Metrics) +## Benchmark Suite Catalog (27 Metrics) ### 1. Scene & Entity Management - `scene_entity_ingestion_10k`: Ingestion throughput for 10,000 mixed 2D entities (Lines, Circles, Arcs, Polylines). @@ -52,6 +52,7 @@ cargo bench --bench performance_benchmarks -- --quick - `ui_icon_handle_cached`: Themed SVG icon cache hit (FxHashMap lookup + Handle clone). - `ui_icon_handle_uncached`: Uncached SVG icon memory parse and Handle allocation. - `ui_plotstyle_layer_usage`: Plot Style 256-bucket ACI layer-usage table rebuild (200 layers). +- `ui_statusbar_derived_data`: Status-bar cached derived-data hit (layout names + scale picker `Arc` clones). ### 6. State Management & Draworder - `selection_state_deep_clone`: Deep cloning of `SelectionState` (with 64 polygon points). diff --git a/benches/performance_benchmarks.rs b/benches/performance_benchmarks.rs index 0374eaf68..c12b70741 100644 --- a/benches/performance_benchmarks.rs +++ b/benches/performance_benchmarks.rs @@ -1408,6 +1408,72 @@ fn bench_ui_plotstyle_layer_usage(runner: &mut BenchmarkRunner) { ); } +// ── 12c. UI Status Bar Derived-Data Cache Hit ───────────────────────────────── +// Covers the per-frame status-bar path (`cached_layout_names` + +// `cached_scale_picker_list`): layout names and the annotation-scale picker +// list are served as shared `Arc` clones instead of full doc scans. + +fn bench_ui_statusbar_derived_data(runner: &mut BenchmarkRunner) { + if !runner.should_run("ui_statusbar_derived_data") { + return; + } + + // Scene shaped like a working drawing: several thousand entities, + // multiple paper layouts, and a populated annotation-scale list. + let mut scene = Scene::new(); + for i in 0..3_000 { + let x = (i % 100) as f64 * 20.0; + let y = (i / 100) as f64 * 20.0; + let mut line = Line::new(); + line.start = Vector3::new(x, y, 0.0); + line.end = Vector3::new(x + 15.0, y + 15.0, 0.0); + scene.add_entity(EntityType::Line(line)); + } + for i in 0..8 { + let _ = scene.add_layout(&format!("SB_BENCH_{i}")); + } + for (i, (paper, drawing)) in [(1.0, 50.0), (1.0, 100.0), (0.5, 12.0), (1.0, 48.0)] + .iter() + .enumerate() + { + let _ = scene.add_scale(&format!("SB_SCALE_{i}"), *paper, *drawing); + } + + // Warm-up: populate both caches and settle the allocator. + for _ in 0..10 { + let layouts = scene.cached_layout_names(); + let scales = scene.cached_scale_picker_list(); + black_box(layouts); + black_box(scales); + } + + let n = if runner.quick_mode { 20 } else { 100 }; + let runs = 5; + let mut samples = Vec::with_capacity(runs); + + for _ in 0..runs { + let t0 = Instant::now(); + for _ in 0..n { + let layouts = scene.cached_layout_names(); + let scales = scene.cached_scale_picker_list(); + black_box(layouts); + black_box(scales); + } + let per_us = (t0.elapsed().as_micros() as f64) / (n as f64); + samples.push(per_us); + } + + let median_us = samples[samples.len() / 2]; + runner.record( + "ui_statusbar_derived_data", + "Status-bar cached derived data hit (layout names + scale picker Arc clones)", + "µs", + samples, + Some((1_000_000.0 / median_us, "hits/s")), + Some(50.0), // Target threshold < 50 µs + ); +} + // ── 13. Wide & Tapered Arc + Donut Tessellation ───────────────────────────── fn bench_wide_and_tapered_arc_tessellation(runner: &mut BenchmarkRunner) { @@ -1811,6 +1877,7 @@ fn main() { bench_ui_grid_geometry(&mut runner); bench_ui_icon_caching(&mut runner); bench_ui_plotstyle_layer_usage(&mut runner); + bench_ui_statusbar_derived_data(&mut runner); bench_wide_and_tapered_arc_tessellation(&mut runner); bench_zoom_extents_calculation(&mut runner); bench_batch_entity_mutation(&mut runner); diff --git a/src/app/automation.rs b/src/app/automation.rs index d9121c64e..48ec40066 100644 --- a/src/app/automation.rs +++ b/src/app/automation.rs @@ -317,6 +317,8 @@ impl OpenCADStudio { let i = self.active_tab; self.tabs[i].scene.clear(); self.tabs[i].scene.document = doc; + self.tabs[i].scene.bump_layout_epoch(); + self.tabs[i].scene.bump_scale_epoch(); self.tabs[i].scene.load_named_parameters_from_document(); self.tabs[i] .scene diff --git a/src/app/history.rs b/src/app/history.rs index 3b4ac1d89..220e34dbc 100644 --- a/src/app/history.rs +++ b/src/app/history.rs @@ -850,6 +850,8 @@ impl OpenCADStudio { .swap_structure(std::mem::replace(stored, acadrust::CadDocument::new())); *stored = inverse; self.tabs[i].scene.invalidate_dependency_index(); + self.tabs[i].scene.bump_layout_epoch(); + self.tabs[i].scene.bump_scale_epoch(); } StructureSnapshot::Layers(entries) => { let names: Vec = @@ -911,12 +913,31 @@ impl OpenCADStudio { } } StructureSnapshot::Objects(entries) => { + let mut layout_touched = false; + let mut scale_touched = false; for entry in entries { let value = if undo { entry.before.clone() } else { entry.after.clone() }; + // Either direction can add/remove a layout: undoing + // an insert removes it (`before` is `None`), redoing + // re-inserts it — so both sides must be inspected. + layout_touched |= matches!( + entry.before, + Some(acadrust::objects::ObjectType::Layout(_)) + ) || matches!( + entry.after, + Some(acadrust::objects::ObjectType::Layout(_)) + ); + scale_touched |= matches!( + entry.before, + Some(acadrust::objects::ObjectType::Scale(_)) + ) || matches!( + entry.after, + Some(acadrust::objects::ObjectType::Scale(_)) + ); if let Some(object) = value { self.tabs[i] .scene @@ -927,6 +948,12 @@ impl OpenCADStudio { self.tabs[i].scene.document.objects.remove(&entry.handle); } } + if layout_touched { + self.tabs[i].scene.bump_layout_epoch(); + } + if scale_touched { + self.tabs[i].scene.bump_scale_epoch(); + } } StructureSnapshot::Styles { before, diff --git a/src/app/update/file.rs b/src/app/update/file.rs index dd93a679d..ec707d1f6 100644 --- a/src/app/update/file.rs +++ b/src/app/update/file.rs @@ -1697,6 +1697,8 @@ impl OpenCADStudio { self.install_native_edit_guard(i, &path, opened_fingerprint); self.tabs[i].scene.material_base_dir = path.parent().map(std::path::Path::to_path_buf); self.tabs[i].scene.document = doc; + self.tabs[i].scene.bump_layout_epoch(); + self.tabs[i].scene.bump_scale_epoch(); // Load parameters first so imported dimensional constraints // can resolve their named driving values. self.tabs[i].scene.load_named_parameters_from_document(); diff --git a/src/app/update/mod.rs b/src/app/update/mod.rs index e93a218a8..7eded86aa 100644 --- a/src/app/update/mod.rs +++ b/src/app/update/mod.rs @@ -6657,7 +6657,7 @@ impl OpenCADStudio { .unwrap_or(1); let name = format!("Layout{n}"); self.push_undo_snapshot(i, "LAYOUT NEW"); - match self.tabs[i].scene.document.add_layout(&name) { + match self.tabs[i].scene.add_layout(&name) { Ok(_) => { self.tabs[i].dirty = true; self.layout_manager_selected = name.clone(); diff --git a/src/app/update/viewport.rs b/src/app/update/viewport.rs index 4e9d2006e..a94433ac7 100644 --- a/src/app/update/viewport.rs +++ b/src/app/update/viewport.rs @@ -6028,7 +6028,7 @@ properties={:.1}ms picked={}", idx += 1; }; self.push_undo_snapshot(i, "LAYOUT"); - match self.tabs[i].scene.document.add_layout(&new_name) { + match self.tabs[i].scene.add_layout(&new_name) { Ok(_) => { let layout_flags = i16::from( self.tabs[i] diff --git a/src/app/view/mod.rs b/src/app/view/mod.rs index dc6679743..eeaba25e5 100644 --- a/src/app/view/mod.rs +++ b/src/app/view/mod.rs @@ -15,6 +15,7 @@ use iced::widget::{ use iced::window; use iced::{keyboard, Background, Border, Color, Element, Fill, Length, Subscription, Task, Theme}; use iced_aw::ContextMenu; +use std::sync::Arc; mod controls; mod modal; @@ -2165,8 +2166,17 @@ bg={bg_ms:.1}ms n={view_count}" let last_coord = self.last_point.map(to_readout); let coords_mode = tab.scene.document.header.coords_mode; let picking = tab.active_cmd.is_some(); - let layout_names = tab.scene.layout_names(); - let block_tabs = tab + // Cached Arcs are moved (refcount bump only), never deep-cloned: + // `StatusBar::view` / `StatusMenuData` take ownership of the + // `Arc`s and borrow them internally. `current_layout` is the + // one true borrow — it anchors to `tab`, which outlives the + // call — because a call-site slice over an `Arc` temporary + // cannot outlive this block (E0515). + let layout_names = tab.scene.cached_layout_names(); + let scale_list = tab.scene.cached_scale_picker_list(); + let selection_types = tab.scene.entity_type_names_in_layout(); + let current_scale_name = tab.scene.displayed_annotation_scale_name(); + let block_tabs: Vec = tab .block_edits .iter() .map(|session| session.block_name.clone()) @@ -2185,13 +2195,13 @@ bg={bg_ms:.1}ms n={view_count}" ), ); let status_menu_data = crate::ui::statusbar::StatusMenuData { - layout_names: layout_names.clone(), + layout_names: Arc::clone(&layout_names), polar_custom_input: &self.polar_custom_input, scale_is_model: is_model, - current_scale_name: tab.scene.displayed_annotation_scale_name(), - scale_list: tab.scene.scale_picker_list(), + current_scale_name, + scale_list, has_selection: !tab.scene.selected.is_empty(), - selection_types: tab.scene.entity_type_names_in_layout().as_ref().clone(), + selection_types, selection_filter: &tab.scene.selection_filter, tooltip_hidden: self.status_menu_tooltip_hidden, }; @@ -2204,10 +2214,9 @@ bg={bg_ms:.1}ms n={view_count}" self.snapper.otrack_enabled, self.isometric_drafting, self.iso_plane, - layout_names.clone(), + layout_names, block_tabs, - layout_names.into_iter().skip(1).collect(), - tab.scene.current_layout.clone(), + &tab.scene.current_layout, active_block, tab.is_start, self.layout_rename_state.as_ref(), @@ -2266,11 +2275,27 @@ bg={bg_ms:.1}ms n={view_count}" .width(Fill) .height(Fill); + // History labels are built only when their dropdown is actually open; + // otherwise an empty slice short-circuits the overlay gate + // (`dropdown_overlay` returns `None` for empty labels anyway). + let open_dropdown = self.ribbon.open_dropdown.as_deref(); + let undo_labels: Vec = + if open_dropdown == Some(crate::ui::ribbon::UNDO_HISTORY_ID) { + history_dropdown_labels(&self.tabs[self.active_tab].history.undo_stack) + } else { + Vec::new() + }; + let redo_labels: Vec = + if open_dropdown == Some(crate::ui::ribbon::REDO_HISTORY_ID) { + history_dropdown_labels(&self.tabs[self.active_tab].history.redo_stack) + } else { + Vec::new() + }; let dropdown_layer: Element<'_, Message> = self .ribbon .dropdown_overlay( - &history_dropdown_labels(&self.tabs[self.active_tab].history.undo_stack), - &history_dropdown_labels(&self.tabs[self.active_tab].history.redo_stack), + &undo_labels, + &redo_labels, self.win_size, self.tabs[self.active_tab].is_start, &self.recent_colors, diff --git a/src/scene/entity.rs b/src/scene/entity.rs index 0fdc9a53d..15f5acfbc 100644 --- a/src/scene/entity.rs +++ b/src/scene/entity.rs @@ -2711,6 +2711,8 @@ impl Scene { self.parametric_constraints.clear(); self.named_parameters = crate::scene::named_parameters::ParameterTable::new(); self.bump_geometry(); + self.bump_layout_epoch(); + self.bump_scale_epoch(); } /// Reset to the drawing File → New produces. Every path that starts a diff --git a/src/scene/layout.rs b/src/scene/layout.rs index ac9750a04..2459fe360 100644 --- a/src/scene/layout.rs +++ b/src/scene/layout.rs @@ -22,12 +22,21 @@ impl Scene { self.model_pane_min_px.clone() } + /// Create a paper-space layout. Wraps `document.add_layout` so the + /// cached layout-name list is invalidated on insert. + pub fn add_layout(&mut self, name: &str) -> acadrust::Result { + let handle = self.document.add_layout(name)?; + self.bump_layout_epoch(); + Ok(handle) + } + /// Rename a paper-space layout. Updates the Layout object name in the document. pub fn rename_layout(&mut self, old_name: &str, new_name: &str) { for obj in self.document.objects.values_mut() { if let ObjectType::Layout(l) = obj { if l.name == old_name { l.name = new_name.to_string(); + self.bump_layout_epoch(); return; } } @@ -96,6 +105,7 @@ impl Scene { } self.bump_geometry(); + self.bump_layout_epoch(); true } @@ -123,6 +133,7 @@ impl Scene { } } } + self.bump_layout_epoch(); } } @@ -139,6 +150,7 @@ impl Scene { } } } + self.bump_layout_epoch(); } /// Rebuild the `pane_grid` layout from the current `model_tiles` rects diff --git a/src/scene/mod.rs b/src/scene/mod.rs index f44d948eb..00dc84d40 100644 --- a/src/scene/mod.rs +++ b/src/scene/mod.rs @@ -2282,6 +2282,13 @@ pub struct Scene { std::sync::Arc>, )>, >, + /// Bump on any Layout object insert/remove/rename/reorder. + layout_epoch: u64, + layout_names_cache: RefCell>)>>, + /// Bump on any annotation-scale list mutation. + scale_epoch: u64, + scale_picker_cache: + RefCell, String, std::sync::Arc>)>>, /// Reverse dependencies from layer/style/block definitions to the top-level /// entities whose resident wire runs actually change. Kept independent from /// `geometry_epoch`: a layer colour toggle can reuse the index, invalidate @@ -2636,6 +2643,10 @@ impl Scene { leaders_by_annotation_cache: RefCell::new(None), unindexable_cache: RefCell::new(None), layout_type_names_cache: RefCell::new(None), + layout_epoch: 0, + layout_names_cache: RefCell::new(None), + scale_epoch: 0, + scale_picker_cache: RefCell::new(None), dependency_index_cache: RefCell::new(None), associative_hatch_source_cache: RefCell::new(None), parametric_constraints: Vec::new(), @@ -4855,6 +4866,7 @@ impl Scene { .unwrap_or((1.0, 1.0 / factor)); self.add_scale(label, paper, drawing); } + self.bump_scale_epoch(); true } @@ -5333,6 +5345,7 @@ impl Scene { (drawing > 0.0).then_some((1.0, drawing)) })?; let (paper, drawing) = self.scale_paper_drawing(name).unwrap_or(fallback); + // `add_scale` already bumps `scale_epoch`; no extra bump here. self.add_scale(name, paper, drawing); self.scale_object_handle(name) } @@ -5375,6 +5388,7 @@ impl Scene { if let Some(ObjectType::Dictionary(sl)) = self.document.objects.get_mut(&scalelist_h) { sl.add_entry(name, sh); } + self.bump_scale_epoch(); true } @@ -5452,6 +5466,7 @@ impl Scene { } } self.invalidate_annotation_dependencies(); + self.bump_scale_epoch(); true } @@ -5484,6 +5499,7 @@ impl Scene { } } } + self.bump_scale_epoch(); true } @@ -5992,6 +6008,51 @@ impl Scene { names } + pub fn bump_layout_epoch(&mut self) { + self.layout_epoch += 1; + } + + pub fn cached_layout_names(&self) -> std::sync::Arc> { + if let Some((epoch, names)) = self.layout_names_cache.borrow().as_ref() { + if *epoch == self.layout_epoch { + return std::sync::Arc::clone(names); + } + } + let names = std::sync::Arc::new(self.layout_names()); + *self.layout_names_cache.borrow_mut() = Some((self.layout_epoch, std::sync::Arc::clone(&names))); + names + } + + pub fn bump_scale_epoch(&mut self) { + self.scale_epoch += 1; + } + + pub fn cached_scale_picker_list(&self) -> std::sync::Arc> { + let factor = self.annotation_scale_unit_factor(); + let imperial = self.prefers_imperial_scales(); + let current = self.document.header.current_annotation_scale.clone(); + if let Some((epoch, cached_factor, cached_imperial, cached_current, list)) = + self.scale_picker_cache.borrow().as_ref() + { + if *epoch == self.scale_epoch + && *cached_factor == factor + && *cached_imperial == imperial + && *cached_current == current + { + return std::sync::Arc::clone(list); + } + } + let list = std::sync::Arc::new(self.scale_picker_list()); + *self.scale_picker_cache.borrow_mut() = Some(( + self.scale_epoch, + factor, + imperial, + current, + std::sync::Arc::clone(&list), + )); + list + } + /// Wire set for the Model layout, shared by every tile. /// /// The model wire geometry is **camera-independent**, so it is tessellated @@ -13350,4 +13411,79 @@ mod layout_cache_tests { "Mixed polyline should have its 2 bulge arcs routed to analytical CircleGpu" ); } + + #[test] + fn cached_layout_names_returns_same_arc_until_bump() { + let mut scene = Scene::new(); + let first = scene.cached_layout_names(); + let second = scene.cached_layout_names(); + assert!(std::sync::Arc::ptr_eq(&first, &second)); + // Fresh `CadDocument::new()` ships a default "Layout1" paper layout. + assert_eq!(&first[..], &["Model".to_string(), "Layout1".to_string()]); + } + + #[test] + fn cached_layout_names_invalidated_by_add_layout() { + let mut scene = Scene::new(); + let before = scene.cached_layout_names(); + scene.add_layout("EXTRA").unwrap(); + let after = scene.cached_layout_names(); + assert!( + !std::sync::Arc::ptr_eq(&before, &after), + "add_layout must bump the epoch so the cached Arc is replaced" + ); + assert!(after.iter().any(|n| n == "EXTRA")); + } + + #[test] + fn cached_scale_picker_returns_same_arc_until_bump() { + let scene = Scene::new(); + let first = scene.cached_scale_picker_list(); + let second = scene.cached_scale_picker_list(); + assert!(std::sync::Arc::ptr_eq(&first, &second)); + } + + #[test] + fn cached_scale_picker_invalidated_by_add_scale() { + let mut scene = Scene::new(); + let before = scene.cached_scale_picker_list(); + assert!(scene.add_scale("TEST_CACHED_SCALE", 1.0, 100.0)); + let after = scene.cached_scale_picker_list(); + assert!( + !std::sync::Arc::ptr_eq(&before, &after), + "add_scale must bump the epoch so the cached Arc is replaced" + ); + assert!(after.iter().any(|(n, _, _)| n == "TEST_CACHED_SCALE")); + } + + #[test] + fn cached_scale_picker_invalidated_by_current_scale_change() { + let mut scene = Scene::new(); + // Force the metric family so the architectural entry is filtered out + // unless it is the kept-active current scale. + scene.document.header.insertion_units = 4; + assert!(scene.add_scale("1:50", 1.0, 50.0)); + assert!(scene.add_scale("1/2\" = 1'-0\"", 0.5, 12.0)); + // Current = opposite-family entry with no metric equivalent factor, so + // the keep-active path appends it to the visible list. + assert!(scene.set_annotation_scale_named("1/2\" = 1'-0\"").is_some()); + let before = scene.cached_scale_picker_list(); + assert!( + before.iter().any(|(n, _, _)| n == "1/2\" = 1'-0\""), + "warm cache must contain the active opposite-family entry" + ); + // Production setter: changes `current_annotation_scale` without + // bumping `scale_epoch` — the old (epoch, factor) key stayed hit here. + assert!(scene.set_annotation_scale_named("1:50").is_some()); + let after = scene.cached_scale_picker_list(); + assert!( + !std::sync::Arc::ptr_eq(&before, &after), + "current-scale change must miss the cache (new Arc)" + ); + assert!( + !after.iter().any(|(n, _, _)| n == "1/2\" = 1'-0\""), + "after switching to 1:50 the architectural entry must drop out" + ); + assert!(after.iter().any(|(n, _, _)| n == "1:50")); + } } diff --git a/src/ui/popup/scale_popup.rs b/src/ui/popup/scale_popup.rs index 19d7045b0..0e9777124 100644 --- a/src/ui/popup/scale_popup.rs +++ b/src/ui/popup/scale_popup.rs @@ -17,11 +17,12 @@ pub fn menu_entries( is_model: bool, current_scale_name: &str, viewport_scale: Option, - file_scales: Vec<(String, f32, f64)>, + file_scales: &[(String, f32, f64)], ) -> Vec> { let mut entries: Vec> = file_scales - .into_iter() + .iter() .map(|(label, _anno_scale, vp_scale)| { + let vp_scale = *vp_scale; let active = if is_model { label.eq_ignore_ascii_case(current_scale_name) } else { @@ -38,7 +39,7 @@ pub fn menu_entries( } else { Message::SetViewportScale(label.clone()) }; - Entry::close(scale_row(label, active, msg)) + Entry::close(scale_row(label.clone(), active, msg)) }) .collect(); diff --git a/src/ui/popup/selection_filter_popup.rs b/src/ui/popup/selection_filter_popup.rs index e3e4ed6bb..128851ade 100644 --- a/src/ui/popup/selection_filter_popup.rs +++ b/src/ui/popup/selection_filter_popup.rs @@ -14,7 +14,7 @@ use crate::ui::statusbar::status_menu::Entry; /// - `types`: entity-type names present in the current layout. /// - `excluded`: types currently filtered out (unchecked). pub fn menu_entries( - types: Vec, + types: &[String], excluded: &HashSet, ) -> Vec> { // "Select All / Clear All" header, mirroring the OSNAP popup: Select All @@ -51,10 +51,10 @@ pub fn menu_entries( vec![Entry::stay(empty_row())] } else { types - .into_iter() + .iter() .map(|name| { - let included = !excluded.contains(&name); - Entry::stay(type_row(name, included)) + let included = !excluded.contains(name); + Entry::stay(type_row(name.clone(), included)) }) .collect() }; diff --git a/src/ui/ribbon/mod.rs b/src/ui/ribbon/mod.rs index 7a71921f0..c1c2852f3 100644 --- a/src/ui/ribbon/mod.rs +++ b/src/ui/ribbon/mod.rs @@ -25,6 +25,7 @@ mod draw_panel; mod modify_panel; mod color_dropdown; use widgets::{StyleContext, *}; +pub(crate) use widgets::{REDO_HISTORY_ID, UNDO_HISTORY_ID}; mod collapse; use collapse::{CollapsePanels, Panel}; pub use collapse::CollapseMode; diff --git a/src/ui/ribbon/widgets.rs b/src/ui/ribbon/widgets.rs index 9ee5c2225..b1a977a39 100644 --- a/src/ui/ribbon/widgets.rs +++ b/src/ui/ribbon/widgets.rs @@ -301,8 +301,8 @@ pub(super) const TOP_HIST_GAP: f32 = 4.0; // ── Dropdown / combo ID constants ───────────────────────────────────────── -pub(super) const UNDO_HISTORY_ID: &str = "UNDO_HISTORY"; -pub(super) const REDO_HISTORY_ID: &str = "REDO_HISTORY"; +pub(crate) const UNDO_HISTORY_ID: &str = "UNDO_HISTORY"; +pub(crate) const REDO_HISTORY_ID: &str = "REDO_HISTORY"; pub(super) const LAYER_COMBO_ID: &str = "LAYER_COMBO"; /// Dropdown id for the tab-bar panel-density selector. pub(super) const COLLAPSE_MODE_ID: &str = "COLLAPSE_MODE"; diff --git a/src/ui/statusbar/mod.rs b/src/ui/statusbar/mod.rs index ea546a0cf..d82b6be5c 100644 --- a/src/ui/statusbar/mod.rs +++ b/src/ui/statusbar/mod.rs @@ -38,13 +38,16 @@ const ST_ANNO_AUTO_ADD: &[u8] = include_bytes!("../../../assets/icons/add_scale. const ST_VP_SCALE_SYNC: &[u8] = include_bytes!("../../../assets/icons/sync.svg"); pub struct StatusMenuData<'a> { - pub layout_names: Vec, + /// Cached layout names (shared `Arc`, moved in — never deep-cloned). + pub layout_names: Arc>, pub polar_custom_input: &'a str, pub scale_is_model: bool, pub current_scale_name: String, - pub scale_list: Vec<(String, f32, f64)>, + /// Cached scale picker list (shared `Arc`, moved in). + pub scale_list: Arc>, pub has_selection: bool, - pub selection_types: Vec, + /// Entity-type names in the current layout (shared `Arc`, moved in). + pub selection_types: Arc>, pub selection_filter: &'a rustc_hash::FxHashSet, pub tooltip_hidden: bool, } @@ -72,10 +75,9 @@ impl StatusBar { otrack: bool, isometric_drafting: bool, iso_plane: crate::app::settings::IsoPlane, - layouts: Vec, + layouts: Arc>, block_tabs: Vec, - reorderable_layouts: Vec, - current_layout: String, + current_layout: &'a str, active_block: Option, // Start/welcome view has no drawing to own layouts. is_start: bool, @@ -172,7 +174,7 @@ impl StatusBar { t!("Model and layout list"), tooltip_hidden, ), - statusbar_menu::layout_entries(&layout_names, ¤t_layout), + statusbar_menu::layout_entries(&layout_names, current_layout), 200.0, ) }; @@ -231,7 +233,7 @@ impl StatusBar { scale_is_model, ¤t_scale_name, viewport_scale, - scale_list, + &scale_list, ), if scale_is_model { 150.0 } else { 120.0 }, ) @@ -378,7 +380,7 @@ impl StatusBar { if vis(StatusPill::Space) { pills.push( tip( - space_mode_btn(¤t_layout, in_mspace), + space_mode_btn(current_layout, in_mspace), t!("PAPER: double-click viewport to enter MSPACE\nMODEL: click to switch to Model Space"), ) .into(), @@ -501,7 +503,7 @@ impl StatusBar { tooltip_hidden, ), crate::ui::popup::selection_filter_popup::menu_entries( - selection_types, + &selection_types, selection_filter, ), 180.0, @@ -571,16 +573,22 @@ impl StatusBar { let mut left: Vec> = Vec::new(); left.push(menu_btn); if show_layout_tabs { - let reorderable_layouts: Arc<[String]> = reorderable_layouts.into(); - for name in layouts { - let is_active = active_block.is_none() && name == current_layout; + // Paper layouts only: `layouts[0]` is always "Model". Derived once + // from the shared `Arc` and cloned (cheap refcount bump) per tab. + let reorderable_layouts: Arc<[String]> = + Arc::from(layouts.get(1..).unwrap_or(&[])); + // Shared once per frame: block tabs carry no reorder set, so reuse + // one empty Arc (refcount bump) instead of allocating per tab. + let empty_reorderable: Arc<[String]> = Arc::from([] as [String; 0]); + for name in layouts.iter() { + let is_active = active_block.is_none() && name.as_str() == current_layout; let renaming = rename_state - .filter(|(orig, _)| *orig == name) + .filter(|(orig, _)| orig.as_str() == name.as_str()) .map(|(_, edit)| edit.as_str()); - let switch_msg = Message::LayoutSwitch(name.clone()); + let switch_msg = Message::LayoutSwitch(name.to_owned()); left.push( space_tab( - name, + name.to_owned(), is_active, renaming, !is_start, @@ -591,16 +599,16 @@ impl StatusBar { .into(), ); } - for name in block_tabs { + for name in block_tabs.iter() { let is_active = active_block.as_deref() == Some(name.as_str()); - let switch_msg = Message::BlockEditSwitch(name.clone()); + let switch_msg = Message::BlockEditSwitch(name.to_owned()); left.push( space_tab( - name, + name.to_owned(), is_active, None, !is_start, - Arc::from(Vec::::new()), + empty_reorderable.clone(), switch_msg, "SB_BLOCK_TAB", )