fix(viewcube): cache UCS picker names behind an epoch

The two per-frame ViewCube blocks rebuilt the named-UCS name list (N String clones plus a Vec) every frame; now Scene::cached_ucs_names memoises it behind ucs_epoch and the picker takes the Arc, so frames cost a refcount bump. Bumped on UCS SAVE, UCS DELETE, RENAME UCS, record-API ucss patch, file-open, and undo Full-swap. Tests: cached_ucs_names_returns_same_arc_until_bump pins the hit path; cached_ucs_names_invalidated_by_bump pins rebuild with new content. Full suite: 1976 passed / 0 failed / 25 ignored.
This commit is contained in:
Karim Jerbi committed 2026-10-06 20:10:23 +01:00
1 parent bd37a2f969
commit 8c2cfa2c8f
9 files changed
+85 -20

No files matched your search

+1
View File
@@ -648,6 +648,7 @@ impl OpenCADStudio {
self.tabs[i].scene.document = doc;
self.tabs[i].scene.bump_layout_epoch();
self.tabs[i].scene.bump_scale_epoch();
self.tabs[i].scene.bump_ucs_epoch();
self.tabs[i].scene.load_named_parameters_from_document();
self.tabs[i]
.scene
+2
View File
@@ -247,6 +247,7 @@ impl OpenCADStudio {
.document
.ucss
.add_or_replace(ucs.clone());
self.tabs[i].scene.bump_ucs_epoch();
self.tabs[i].active_ucs = Some(ucs);
active_changed = true;
self.tabs[i].dirty = true;
@@ -261,6 +262,7 @@ impl OpenCADStudio {
} else if let Some(removed) =
self.tabs[i].scene.document.ucss.remove(&name)
{
self.tabs[i].scene.bump_ucs_epoch();
let removed_handle = removed.handle;
for entity in self.tabs[i].scene.document.entities_mut() {
if let codec::EntityType::Viewport(viewport) = entity {
+1
View File
@@ -2997,6 +2997,7 @@ impl OpenCADStudio {
self.tabs[i].scene.bump_geometry_no_blocks();
}
if type_str == "UCS" {
self.tabs[i].scene.bump_ucs_epoch();
if let Some(active) = self.tabs[i].active_ucs.as_mut() {
if active.name.eq_ignore_ascii_case(&old_name) {
active.name = new_name.clone();
+1
View File
@@ -855,6 +855,7 @@ impl OpenCADStudio {
self.tabs[i].scene.invalidate_dependency_index();
self.tabs[i].scene.bump_layout_epoch();
self.tabs[i].scene.bump_scale_epoch();
self.tabs[i].scene.bump_ucs_epoch();
}
StructureSnapshot::Layers(entries) => {
let names: Vec<String> =
+6 -1
View File
@@ -1611,7 +1611,12 @@ impl OpenCADStudio {
"app_ids" => patch_table!(app_ids),
"views" => patch_table!(views),
"vports" => patch_table!(vports),
"ucss" => patch_table!(ucss),
"ucss" => {
patch_table!(ucss);
if changed {
self.tabs[i].scene.bump_ucs_epoch();
}
}
"vx_table" => patch_table!(vx_table),
_ => unreachable!(),
}
+1
View File
@@ -1860,6 +1860,7 @@ impl OpenCADStudio {
self.tabs[i].scene.document = doc;
self.tabs[i].scene.bump_layout_epoch();
self.tabs[i].scene.bump_scale_epoch();
self.tabs[i].scene.bump_ucs_epoch();
// Load parameters first so imported dimensional constraints
// can resolve their named driving values.
self.tabs[i].scene.load_named_parameters_from_document();
+6 -16
View File
@@ -1416,14 +1416,9 @@ bg={bg_ms:.1}ms n={view_count}"
.as_ref()
.map(|u| u.name.clone())
.unwrap_or_default();
let ucs_names: Vec<String> = tab
.scene
.document
.ucss
.iter()
.map(|u| u.name.clone())
.filter(|n| !n.is_empty())
.collect();
// Epoch-memoised (#34): Arc clone per frame, rebuilt only
// after a UCS-table bump.
let ucs_names = tab.scene.cached_ucs_names();
let picker = iced::widget::pin(iced::widget::opaque(viewcube_ucs_picker(
ucs_current,
ucs_names,
@@ -1456,14 +1451,9 @@ bg={bg_ms:.1}ms n={view_count}"
.as_ref()
.map(|u| u.name.clone())
.unwrap_or_default();
let ucs_names: Vec<String> = tab
.scene
.document
.ucss
.iter()
.map(|u| u.name.clone())
.filter(|n| !n.is_empty())
.collect();
// Epoch-memoised (#34): Arc clone per frame, rebuilt only
// after a UCS-table bump.
let ucs_names = tab.scene.cached_ucs_names();
let picker = iced::widget::pin(iced::widget::opaque(viewcube_ucs_picker(
ucs_current,
ucs_names,
+8 -3
View File
@@ -150,10 +150,15 @@ pub(super) fn viewcube_nav_controls<'a>(
/// Fixed width of the UCS picker, so it can be centred under the cube.
pub(super) const UCS_PICKER_W: f32 = 84.0;
/// The WCS / named-UCS selector shown under the cube.
pub(super) fn viewcube_ucs_picker<'a>(current: String, names: Vec<String>) -> Element<'a, Message> {
/// The WCS / named-UCS selector shown under the cube. `names` is the
/// epoch-memoised `Scene::cached_ucs_names` Arc (#34) — a refcount bump per
/// frame instead of N String clones.
pub(super) fn viewcube_ucs_picker<'a>(
current: String,
names: std::sync::Arc<[String]>,
) -> Element<'a, Message> {
let mut options = vec!["WCS".to_string()];
options.extend(names);
options.extend(names.iter().cloned());
let selected = if current.is_empty() {
"WCS".to_string()
} else {
+59
View File
@@ -2365,6 +2365,10 @@ pub struct Scene {
scale_epoch: u64,
scale_picker_cache:
RefCell<Option<(u64, f64, Option<bool>, String, std::sync::Arc<Vec<(String, f32, f64)>>)>>,
/// Bump on any named-UCS table mutation (UCS SAVE / DELETE / RENAME,
/// record-API patch, file-open, undo Full-swap).
ucs_epoch: u64,
ucs_names_cache: RefCell<Option<(u64, std::sync::Arc<[String]>)>>,
/// 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
@@ -2756,6 +2760,8 @@ impl Scene {
layout_names_cache: RefCell::new(None),
scale_epoch: 0,
scale_picker_cache: RefCell::new(None),
ucs_epoch: 0,
ucs_names_cache: RefCell::new(None),
dependency_index_cache: RefCell::new(None),
associative_hatch_source_cache: RefCell::new(None),
parametric_constraints: Vec::new(),
@@ -6578,6 +6584,32 @@ impl Scene {
list
}
pub fn bump_ucs_epoch(&mut self) {
self.ucs_epoch += 1;
}
/// Named-UCS names for the ViewCube picker, memoised behind `ucs_epoch`
/// (#34, mirrors `cached_layout_names`). Same empty-name filter the view
/// blocks used when building the list inline every frame.
pub fn cached_ucs_names(&self) -> std::sync::Arc<[String]> {
if let Some((epoch, names)) = self.ucs_names_cache.borrow().as_ref() {
if *epoch == self.ucs_epoch {
return std::sync::Arc::clone(names);
}
}
let names: std::sync::Arc<[String]> = self
.document
.ucss
.iter()
.map(|u| u.name.clone())
.filter(|n| !n.is_empty())
.collect::<Vec<_>>()
.into();
*self.ucs_names_cache.borrow_mut() =
Some((self.ucs_epoch, std::sync::Arc::clone(&names)));
names
}
/// Wire set for the Model layout, shared by every tile.
///
/// The model wire geometry is **camera-independent**, so it is tessellated
@@ -14253,6 +14285,33 @@ mod layout_cache_tests {
);
assert!(after.iter().any(|(n, _, _)| n == "1:50"));
}
#[test]
fn cached_ucs_names_returns_same_arc_until_bump() {
// RED: `Scene::cached_ucs_names` does not exist yet (#34).
let scene = Scene::new();
let first = scene.cached_ucs_names();
let second = scene.cached_ucs_names();
assert!(std::sync::Arc::ptr_eq(&first, &second));
}
#[test]
fn cached_ucs_names_invalidated_by_bump() {
// RED: `Scene::bump_ucs_epoch` does not exist yet (#34).
let mut scene = Scene::new();
let before = scene.cached_ucs_names();
scene
.document
.ucss
.add_or_replace(codec::tables::Ucs::new("TEST_CACHED_UCS"));
scene.bump_ucs_epoch();
let after = scene.cached_ucs_names();
assert!(
!std::sync::Arc::ptr_eq(&before, &after),
"ucs mutation + bump must replace the cached Arc"
);
assert!(after.iter().any(|n| n == "TEST_CACHED_UCS"));
}
}
#[cfg(test)]