diff --git a/src/app/mod.rs b/src/app/mod.rs index a82dc50e..850220ff 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -679,7 +679,7 @@ pub(super) struct OpenCADStudio { /// keep their manifest listed but drop their ribbon tab and command /// dispatch. Persisted via [`settings::UserSettings::disabled_plugins`]. disabled_plugins: rustc_hash::FxHashSet, - /// `(tab id, selection signature)` last broadcast to V4 plugins, so + /// `(tab id, selection fingerprint)` last broadcast to V4 plugins, so /// `SelectionChangedV4` fires once per real change rather than per message. #[cfg(not(target_arch = "wasm32"))] last_plugin_selection: Option<(u64, u64)>, diff --git a/src/app/update/mod.rs b/src/app/update/mod.rs index 2a947376..24b59350 100644 --- a/src/app/update/mod.rs +++ b/src/app/update/mod.rs @@ -233,12 +233,6 @@ impl OpenCADStudio { /// Emit `SelectionChangedV4` to V4 plugins when the active tab's selection /// set actually changed since the last broadcast. - /// - /// Checked at the message boundary rather than at the mutation sites so - /// every path is covered: picking, window select, QSELECT, SELECTALL, - /// grip edits that drop the selection, and the automation `select` op. - /// The signature comparison keeps hover and repeated no-op selects from - /// producing spurious notifications. #[cfg(not(target_arch = "wasm32"))] pub(super) fn notify_plugins_selection_changed(&mut self) { if self.active_tab >= self.tabs.len() { @@ -246,8 +240,8 @@ impl OpenCADStudio { } let i = self.active_tab; let tab_id = self.tabs[i].id; - let sig = self.tabs[i].scene.selection_sig(); - let key = (tab_id, sig); + let fingerprint = self.tabs[i].scene.selection_fingerprint(); + let key = (tab_id, fingerprint); if self.last_plugin_selection == Some(key) { return; } diff --git a/src/scene/group_layer.rs b/src/scene/group_layer.rs index af957834..2c3aa78e 100644 --- a/src/scene/group_layer.rs +++ b/src/scene/group_layer.rs @@ -203,8 +203,11 @@ impl Scene { /// If any handle belongs to a selectable group, also select every member. pub fn expand_selection_for_groups(&mut self, handles: &[Handle]) { + let previous_len = self.selected.len(); self.selected .extend(self.handles_expanded_for_selectable_groups(handles)); - self.bump_selection(); + if self.selected.len() != previous_len { + self.bump_selection_set(); + } } } diff --git a/src/scene/mod.rs b/src/scene/mod.rs index cf14edc6..2347fb53 100644 --- a/src/scene/mod.rs +++ b/src/scene/mod.rs @@ -1486,6 +1486,9 @@ pub struct Scene { /// pick only refreshes the GPU xray overlay (cheap) instead of bumping /// `geometry_epoch` and re-tessellating the whole model. pub selection_generation: u64, + /// Cached fingerprint of `selected`, recomputed lazily after mutations. + selection_fingerprint_cache: u64, + selection_fingerprint_dirty: bool, /// Cached tessellation of all visible entity wires for the current layout. /// Keyed by `(geometry_epoch, camera_generation)` so a camera change /// invalidates the cull-dependent wire list as well as a geometry change. @@ -1861,6 +1864,8 @@ impl Scene { projection_bounds_epoch: std::cell::Cell::new(0), block_epoch: GEOMETRY_EPOCH.fetch_add(1, Ordering::Relaxed), selection_generation: 0, + selection_fingerprint_cache: 0, + selection_fingerprint_dirty: false, wire_cache: RefCell::new(None), interaction_index_cache: RefCell::new(Vec::new()), interaction_index_pending_key: std::cell::Cell::new(None), @@ -2610,6 +2615,11 @@ impl Scene { self.selection_generation = self.selection_generation.wrapping_add(1); } + pub(crate) fn bump_selection_set(&mut self) { + self.selection_fingerprint_dirty = true; + self.bump_selection(); + } + /// Milliseconds after the last camera change during which the view counts as /// "actively navigating" for interaction-LOD purposes. const NAV_SETTLE_MS: u128 = 130; @@ -4398,6 +4408,7 @@ impl Scene { .extend(self.selected.iter().copied()); self.selected.clear(); self.selected_order.clear(); + self.bump_selection_set(); self.bump_entities(&changes); } diff --git a/src/scene/selection.rs b/src/scene/selection.rs index 195e071e..aa951bdd 100644 --- a/src/scene/selection.rs +++ b/src/scene/selection.rs @@ -40,8 +40,11 @@ impl Scene { } pub fn select_entity(&mut self, handle: Handle, exclusive: bool) { let handles = self.handles_expanded_for_leader_annotations(&[handle]); + let mut changed = false; if exclusive { + changed = self.selected.len() != handles.len() + || handles.iter().any(|handle| !self.selected.contains(handle)); self.selected.clear(); self.selected_order.clear(); } @@ -49,30 +52,35 @@ impl Scene { for handle in handles { if self.selected.insert(handle) { self.selected_order.push(handle); + changed = true; } } - self.bump_selection(); + if changed { + self.bump_selection_set(); + } } pub fn deselect_all(&mut self) { + if self.selected.is_empty() { + return; + } self.selected.clear(); self.selected_order.clear(); - self.bump_selection(); + self.bump_selection_set(); } - /// Order-independent signature of the current selection set. - /// - /// `selection_generation` cannot be used for change detection because it - /// also bumps on hover. This signature combines a per-handle hash with XOR - /// so it is allocation-free, O(n), and insensitive to selection order. - pub(crate) fn selection_sig(&self) -> u64 { - let mut combined = 0u64; - for handle in &self.selected { - let mut hasher = DefaultHasher::new(); - handle.hash(&mut hasher); - combined ^= hasher.finish(); + pub(crate) fn selection_fingerprint(&mut self) -> u64 { + if self.selection_fingerprint_dirty { + let mut fingerprint = self.selected.len() as u64; + for handle in &self.selected { + let mut hasher = DefaultHasher::new(); + handle.hash(&mut hasher); + fingerprint ^= hasher.finish(); + } + self.selection_fingerprint_cache = fingerprint; + self.selection_fingerprint_dirty = false; } - combined + self.selection_fingerprint_cache } pub(crate) fn selected_handles_in_order(&self) -> Vec { @@ -121,7 +129,7 @@ impl Scene { order.extend(added); self.selected = selected; self.selected_order = order; - self.bump_selection(); + self.bump_selection_set(); } } @@ -136,7 +144,7 @@ impl Scene { } if changed { - self.bump_selection(); + self.bump_selection_set(); } } @@ -217,7 +225,7 @@ impl Scene { } } if added > 0 { - self.bump_selection(); + self.bump_selection_set(); } added } @@ -241,7 +249,9 @@ impl Scene { self.selected_order.push(h); } } - self.bump_selection(); + if self.selected != prev { + self.bump_selection_set(); + } self.selected.len() } @@ -727,7 +737,8 @@ impl Scene { let mut handle_set: HashSet = HashSet::default(); let mut erased: Vec<(Handle, ChangeKind)> = Vec::new(); - let mut highlight_changed = false; + let mut selection_changed = false; + let mut hover_changed = false; for &h in &erase_handles { // Objects on a locked layer can't be erased. @@ -742,11 +753,11 @@ impl Scene { self.delete_solid_history(h); self.remember_removed_cache_categories(h); self.document.remove_entity_arc(h); - highlight_changed |= self.selected.remove(&h); + selection_changed |= self.selected.remove(&h); self.selected_order.retain(|selected| *selected != h); if self.hover_highlight == Some(h) { self.hover_highlight = None; - highlight_changed = true; + hover_changed = true; } self.hatches.remove(&h); self.images.remove(&h); @@ -756,7 +767,9 @@ impl Scene { handle_set.insert(h); erased.push((h, ChangeKind::Removed)); } - if highlight_changed { + if selection_changed { + self.bump_selection_set(); + } else if hover_changed { self.bump_selection(); } // Capture exactly the group objects that this erase will rewrite, plus @@ -862,25 +875,23 @@ mod tests { use super::*; #[test] - fn selection_sig_is_order_independent() { - let mut scene_a = Scene::default(); - let mut scene_b = Scene::default(); - let h1 = Handle::new(1); - let h2 = Handle::new(2); - scene_a.select_entity(h1, false); - scene_a.select_entity(h2, false); - scene_b.select_entity(h2, false); - scene_b.select_entity(h1, false); - assert_eq!(scene_a.selection_sig(), scene_b.selection_sig()); - } - - #[test] - fn selection_sig_changes_when_selection_changes() { + fn selection_fingerprint_tracks_final_set_only() { let mut scene = Scene::default(); - let empty_sig = scene.selection_sig(); - scene.select_entity(Handle::new(1), false); - assert_ne!(scene.selection_sig(), empty_sig); + let first = Handle::new(1); + let second = Handle::new(2); + scene.select_entity(first, false); + scene.select_entity(second, false); + let fingerprint = scene.selection_fingerprint(); + assert!(!scene.selection_fingerprint_dirty); + scene.deselect_all(); - assert_eq!(scene.selection_sig(), empty_sig); + scene.select_entity(second, false); + scene.select_entity(first, false); + assert!(scene.selection_fingerprint_dirty); + assert_eq!(scene.selection_fingerprint(), fingerprint); + + scene.set_hover_highlight(Some(Handle::new(3))); + assert!(!scene.selection_fingerprint_dirty); + assert_eq!(scene.selection_fingerprint(), fingerprint); } }