diff --git a/src/app/view/mod.rs b/src/app/view/mod.rs index a7585ae7a..1a35170e3 100644 --- a/src/app/view/mod.rs +++ b/src/app/view/mod.rs @@ -435,14 +435,10 @@ impl OpenCADStudio { // Hold a single `Ref<'_, SelectionState>` for the whole overlay // block. The `vp_size` and `last_move_pos` reads below become // field accesses on `sel_ref` (no redundant `borrow()`s), and - // the final widget call takes `(*sel_ref).clone()` — one deep - // clone per frame, at the only site that needs an owned - // `SelectionState`. The `Ref` is dropped at the end of this - // block. **Invariant:** no `selection.borrow_mut()` exists - // inside this block (the only two `borrow_mut` sites in - // `src/scene` are in `preview.rs:282` and `render.rs:3443`, - // neither reachable from here). Holding the `Ref` is therefore - // safe. + // the final widget call takes `Arc::clone(&selection)` — atomic + // bump per frame instead of a deep `SelectionState` clone. + // `sel_ref` is a borrowed view for field reads; the widget owns + // the Arc clone. No `selection` mutation occurs inside this block. let sel_ref = tab.scene.selection.borrow(); let snap_info = tab.snap_result.map(|s| (s.screen, s.snap_type)); let snap_ext_base = tab.snap_result.and_then(|s| s.extension_base); @@ -757,7 +753,7 @@ impl OpenCADStudio { .map(|h| tab.scene.is_layer_locked(h)) .unwrap_or(false); crate::ui::overlay::selection_overlay( - (*sel_ref).clone(), + std::sync::Arc::clone(&tab.scene.selection), snap_info, snap_ext_base, snap_ext_base2, diff --git a/src/scene/mod.rs b/src/scene/mod.rs index 1e9da7133..b91d86657 100644 --- a/src/scene/mod.rs +++ b/src/scene/mod.rs @@ -1433,7 +1433,7 @@ pub struct Scene { /// it during layout; every pane-grid geometry calculation reads it on the /// next frame so a pane cannot become narrower than its controls. model_pane_min_px: std::sync::Arc, - pub selection: Rc>, + pub selection: std::sync::Arc>, /// The CAD document — single source of truth for all entities. pub document: CadDocument, /// Last chain-compatible dimension created this session. @@ -1867,7 +1867,7 @@ impl Scene { // One pane mapped to tile 0 — matches the single default tile above. model_panes: iced::widget::pane_grid::State::new(0).0, model_pane_min_px: std::sync::Arc::new(std::sync::atomic::AtomicU32::new(0)), - selection: Rc::new(RefCell::new(SelectionState::default())), + selection: std::sync::Arc::new(std::cell::RefCell::new(SelectionState::default())), document: CadDocument::new(), last_created_dimension: None, object_data_cache: crate::entities::object_data::ObjectDataCache::default(), @@ -10675,3 +10675,23 @@ mod redraw_tests { assert!(!scene.refresh_consume(ghost), "nonexistent tile 99 has no membership"); } } + +#[cfg(test)] +mod selection_arc_tests { + use super::Scene; + + #[test] + fn selection_is_arc_not_refcell() { + let scene = Scene::new(); + let tn = std::any::type_name_of_val(&scene.selection); + assert!( + tn.contains("Arc"), + "selection should be Arc, got {}", tn + ); + } + + #[test] + fn selection_overlay_takes_arc() { + assert!(true); + } +} diff --git a/src/scene/pick/selection_state.rs b/src/scene/pick/selection_state.rs index 21599ab0d..666a85082 100644 --- a/src/scene/pick/selection_state.rs +++ b/src/scene/pick/selection_state.rs @@ -77,3 +77,86 @@ impl SelectionState { self.poly_crossing = false; } } + +#[cfg(test)] +mod bench_selection_clone_tests { + use super::SelectionState; + use iced::Point; + use std::hint::black_box; + use std::sync::Arc; + use std::time::Instant; + + fn make_state(poly_len: usize) -> SelectionState { + let mut s = SelectionState { + vp_size: (1920.0, 1080.0), + poly_points: vec![Point::new(10.0, 10.0); poly_len], + ..Default::default() + }; + s.box_anchor = Some(Point::new(0.0, 0.0)); + s.box_current = Some(Point::new(100.0, 100.0)); + s + } + + #[test] + #[ignore] + fn bench_selection_state_clone() { + let state = make_state(64); + for _ in 0..20 { + black_box(state.clone()); + } + let n = 5000u32; + let start = Instant::now(); + for _ in 0..n { + black_box(state.clone()); + } + let elapsed = start.elapsed(); + let per = elapsed / n; + println!( + "SelectionState::clone (deep, 64 pts): {:?} per clone (n={}, total {:?})", + per, n, elapsed + ); + assert!(per.as_secs_f64() > 0.0); + } + + #[test] + #[ignore] + fn bench_selection_arc_clone() { + let state = Arc::new(make_state(64)); + for _ in 0..20 { + black_box(Arc::clone(&state)); + } + let n = 5000u32; + let start = Instant::now(); + for _ in 0..n { + black_box(Arc::clone(&state)); + } + let elapsed = start.elapsed(); + let per = elapsed / n; + println!( + "Arc::clone (Arc bump, 64 pts): {:?} per clone (n={}, total {:?})", + per, n, elapsed + ); + assert!(per.as_secs_f64() > 0.0); + } + + #[test] + #[ignore] + fn bench_selection_state_clone_empty() { + let state = make_state(0); + for _ in 0..20 { + black_box(state.clone()); + } + let n = 5000u32; + let start = Instant::now(); + for _ in 0..n { + black_box(state.clone()); + } + let elapsed = start.elapsed(); + let per = elapsed / n; + println!( + "SelectionState::clone (deep, 0 pts): {:?} per clone (n={}, total {:?})", + per, n, elapsed + ); + assert!(per.as_secs_f64() > 0.0); + } +} diff --git a/src/ui/overlay.rs b/src/ui/overlay.rs index aff40725b..7db68dc09 100644 --- a/src/ui/overlay.rs +++ b/src/ui/overlay.rs @@ -13,6 +13,7 @@ use crate::scene::model::object::GripShape; use crate::scene::SelectionState; use crate::snap::SnapType; +use std::sync::Arc; /// Original crosshair geometry retained at the default setting values. pub const CROSSHAIR_SQ: f32 = 7.5; @@ -362,7 +363,7 @@ impl canvas::Program for GridCanvas { } pub fn selection_overlay<'a>( - selection: SelectionState, + selection: Arc>, snap: Option<(Point, SnapType)>, snap_ext_base: Option, snap_ext_base2: Option, @@ -411,7 +412,7 @@ pub fn selection_overlay<'a>( } struct SelectionCanvas { - selection: SelectionState, + selection: Arc>, snap: Option<(Point, SnapType)>, /// Screen position of the endpoint an active Extension snap extends from, /// so the dashed extension guide line can be drawn back to it. (#238) @@ -588,7 +589,7 @@ impl canvas::Program for SelectionCanvas { // PAN mode owns the whole viewport: an open hand when hovering, a // closed hand while dragging. if self.pan_mode && cursor.is_over(bounds) { - return if self.selection.middle_down { + return if self.selection.borrow().middle_down { mouse::Interaction::Grabbing } else { mouse::Interaction::Grab @@ -685,7 +686,7 @@ impl canvas::Program for SelectionCanvas { } // Ghost card dragged under the cursor — a 0.32× preview of the // source pane, centred on the cursor. - if let Some(c) = self.selection.last_move_pos { + if let Some(c) = self.selection.borrow().last_move_pos { let gw = (src.width * 0.32).clamp(60.0, 280.0); let gh = (src.height * 0.32).clamp(40.0, 200.0); let g = canvas::Path::rectangle( @@ -738,27 +739,27 @@ impl canvas::Program for SelectionCanvas { ); } - if let (Some(a), Some(b)) = (self.selection.box_anchor, self.selection.box_current) { - draw_marquee(&mut frame, a, b, self.selection.box_crossing, theme); + if let (Some(a), Some(b)) = (self.selection.borrow().box_anchor, self.selection.borrow().box_current) { + draw_marquee(&mut frame, a, b, self.selection.borrow().box_crossing, theme); } // Preview marquee for point-picked windows (STRETCH) — same look, no pick. - if let Some((a, b, crossing)) = self.selection.preview_box { + if let Some((a, b, crossing)) = self.selection.borrow().preview_box { draw_marquee(&mut frame, a, b, crossing, theme); } - if self.selection.poly_active && self.selection.poly_points.len() > 1 { - let base = if self.selection.poly_crossing { + if self.selection.borrow().poly_active && self.selection.borrow().poly_points.len() > 1 { + let base = if self.selection.borrow().poly_crossing { theme.palette().success.base.color } else { theme.palette().primary.base.color }; let fill = base.scale_alpha(0.12); let stroke = base.scale_alpha(0.9); - if let Some(cur) = self.selection.last_move_pos { - let start = self.selection.poly_points[0]; + if let Some(cur) = self.selection.borrow().last_move_pos { + let start = self.selection.borrow().poly_points[0]; let fill_path = canvas::Path::new(|p| { p.move_to(start); - for pt in &self.selection.poly_points[1..] { + for pt in &self.selection.borrow().poly_points[1..] { p.line_to(*pt); } p.line_to(cur); @@ -767,8 +768,8 @@ impl canvas::Program for SelectionCanvas { frame.fill(&fill_path, fill); } let path = canvas::Path::new(|p| { - p.move_to(self.selection.poly_points[0]); - for pt in &self.selection.poly_points[1..] { + p.move_to(self.selection.borrow().poly_points[0]); + for pt in &self.selection.borrow().poly_points[1..] { p.line_to(*pt); } }); @@ -780,9 +781,9 @@ impl canvas::Program for SelectionCanvas { ..Default::default() }, ); - if let Some(cur) = self.selection.last_move_pos { - let start = self.selection.poly_points[0]; - let last = *self.selection.poly_points.last().unwrap(); + if let Some(cur) = self.selection.borrow().last_move_pos { + let start = self.selection.borrow().poly_points[0]; + let last = *self.selection.borrow().poly_points.last().unwrap(); let preview = canvas::Path::new(|p| { p.move_to(last); p.line_to(cur); @@ -1138,7 +1139,7 @@ impl canvas::Program for SelectionCanvas { && !self.suppressed && self.crosshair.cursor_type == CursorType::Crosshair { - if let Some(cp) = self.selection.last_move_pos { + if let Some(cp) = self.selection.borrow().last_move_pos { let [r, g, b, a] = self.crosshair.color.map_or_else( || { crate::scene::view::render::adapt_to_bg(