From 2d49204c50941019c7a1d1d37ac5b722d3224029 Mon Sep 17 00:00:00 2001 From: Karim Jerbi Date: Mon, 21 Sep 2026 07:08:36 +0100 Subject: [PATCH] perf(grips): cap total selection grips at 4096 GRIPOBJLIMIT gates object count, but one dense polyline emits ~2 grips/vertex past it (~200K grips through the per-frame project + draw path, ms-scale, no cache). Enforce a vertex budget once in refresh_selected_grips: stable vertex-first priority, entity order restored, handles truncated in lockstep. Below the cap selections are byte-identical. Tests: - refresh_selected_grips_caps_total_grip_count: cap + handles lockstep - refresh_selected_grips_prefers_vertex_grips_over_midpoints: vertices survive - grip_budget_boundary_lengths: empty/cap/cap+1 edge cases Bench (cargo bench grip, 200K-grip fixture): per-frame 6.8ms -> 0.13ms (45-52x); select-time build 2.36ms full. A/B reference deleted. --- benches/README.md | 4 +- benches/performance_benchmarks.rs | 145 ++++++++++++++++++++++++++ src/app/mod.rs | 7 ++ src/app/properties.rs | 164 +++++++++++++++++++++++++++++- src/app/settings.rs | 9 ++ 5 files changed, 327 insertions(+), 2 deletions(-) diff --git a/benches/README.md b/benches/README.md index cb7aed311..c9254851c 100644 --- a/benches/README.md +++ b/benches/README.md @@ -19,7 +19,7 @@ cargo bench --bench performance_benchmarks -- --quick --- -## Benchmark Suite Catalog (27 Metrics) +## Benchmark Suite Catalog (29 Metrics) ### 1. Scene & Entity Management - `scene_entity_ingestion_10k`: Ingestion throughput for 10,000 mixed 2D entities (Lines, Circles, Arcs, Polylines). @@ -53,6 +53,8 @@ cargo bench --bench performance_benchmarks -- --quick - `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). +- `ui_grip_budget`: Budgeted per-frame selection-grip projection (dense-polyline grips capped to `MAX_SELECTED_GRIPS`). +- `ui_grip_budget_build`: Selection-change `apply_grip_budget` cost over the full grip set (sort + FxHashSet + filter). ### 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 c12b70741..8cbd435d8 100644 --- a/benches/performance_benchmarks.rs +++ b/benches/performance_benchmarks.rs @@ -1474,6 +1474,150 @@ fn bench_ui_statusbar_derived_data(runner: &mut BenchmarkRunner) { ); } +// ── 12d. UI Grip Vertex Budget (budgeted build + per-frame projection) ────── +// Covers the selection-grip vertex budget (`MAX_SELECTED_GRIPS` in +// app/settings.rs): one dense 100K-vertex polyline emits ~2 grips/vertex +// (vertex + midpoint). The budget keeps entity order with vertex grips first +// and truncates the parallel handle vec in lockstep, so the per-frame +// `grips_to_screen` projection only ever sees the capped set. +// +// NOTE: the budget step below calls the real `apply_grip_budget` +// (app/properties.rs) with the real `MAX_SELECTED_GRIPS` (app/settings.rs), +// re-exported via `OpenCADStudio::app` so this external bench crate can reach +// them (both modules are otherwise private/`pub(crate)`). +// Projection is measured with the real `grips_to_screen` on the capped set, +// which bounds the per-frame cost by construction: uncapped it is O(vertices), +// budgeted it is O(cap). + +fn bench_ui_grip_budget(runner: &mut BenchmarkRunner) { + if !runner.should_run("ui_grip_budget") && !runner.should_run("ui_grip_budget_build") { + return; + } + + use OpenCADStudio::app::{apply_grip_budget, MAX_SELECTED_GRIPS}; + use OpenCADStudio::scene::model::object::{GripDef, GripShape}; + use OpenCADStudio::scene::pick::grip::grips_to_screen; + + // Fixture: one dense polyline's grips — 2 per vertex (vertex + midpoint), + // i.e. ~200K grips for a 100K-vertex polyline in full mode. Midpoint grips + // mirror the real LWPolyline producer (entities/lwpolyline.rs via + // entities/common.rs `rectangle_grip`): `Rectangle` shape with `dir: + // Some(chord)` so the bench exercises the second `camera.project` in + // `grips_to_screen` (scene/pick/grip.rs), exactly like production. + let n_vertices = if runner.quick_mode { 10_000 } else { 100_000 }; + let mut all_grips = Vec::with_capacity(2 * n_vertices); + for v in 0..n_vertices { + let base = (v % 1024) as f64; + all_grips.push(GripDef { + id: 2 * v, + world: glam::DVec3::new(base, base * 0.5, 0.0), + is_midpoint: false, + shape: GripShape::Square, + dir: None, + axis: None, + }); + all_grips.push(GripDef { + id: 2 * v + 1, + world: glam::DVec3::new(base + 0.5, base * 0.5 + 0.25, 0.0), + is_midpoint: true, + shape: GripShape::Rectangle, + // In-plane segment direction of the synthetic polyline + // (vertices run along (1, 0.5)), matching the chord `dir` the + // real producer passes to `rectangle_grip`. + dir: Some(glam::DVec3::new(1.0, 0.5, 0.0)), + axis: None, + }); + } + let all_handles: Vec = (0..all_grips.len() as u64) + .map(|k| acadrust::Handle::new(k + 1)) + .collect(); + let n_fixture_grips = all_grips.len(); + + // Budget once (selection-change path): the per-frame loop below only ever + // sees the capped set, exactly like view/mod.rs after refresh. Cloned so + // the full fixture stays available for the `ui_grip_budget_build` timing + // below, which measures this same selection-change cost per iteration. + let (capped_grips, _) = apply_grip_budget(all_grips.clone(), all_handles.clone()); + assert_eq!(capped_grips.len(), MAX_SELECTED_GRIPS.min(2 * n_vertices)); + + let cam = Camera::default(); + let bounds = Rectangle { + x: 0.0, + y: 0.0, + width: 1920.0, + height: 1080.0, + }; + + // Warm-up for allocator settling. + for _ in 0..10 { + let projected = grips_to_screen(black_box(&capped_grips), &cam, bounds); + black_box(projected); + } + + 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 projected = grips_to_screen(black_box(&capped_grips), &cam, bounds); + black_box(projected); + } + 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_grip_budget", + &format!( + "Budgeted per-frame grip projection ({}-grip polyline capped to {} grips)", + n_fixture_grips, + capped_grips.len() + ), + "µs", + samples, + Some(((capped_grips.len() as f64) / (median_us / 1_000_000.0), "grips/s")), + Some(500.0), // Target threshold < 500 µs (measured ~132 µs quick / ~147 µs full) + ); + + // Selection-change cost (previously unmeasured): `apply_grip_budget` sorts + // ~200K grips by `is_midpoint` (O(n log n)), builds an FxHashSet of the + // kept indices, and filters both parallel vecs — once per selection change, + // not per frame. Iteration inputs are pre-cloned before the timer so only + // the budget itself is measured, not the clone. + let n_build = if runner.quick_mode { 20 } else { 5 }; + let mut build_samples = Vec::with_capacity(runs); + for _ in 0..runs { + let mut build_inputs = Vec::with_capacity(n_build); + for _ in 0..n_build { + build_inputs.push((all_grips.clone(), all_handles.clone())); + } + let t0 = Instant::now(); + for (grips_in, handles_in) in build_inputs { + let grips_in = black_box(grips_in); + let handles_in = black_box(handles_in); + let (capped, _) = apply_grip_budget(grips_in, handles_in); + black_box(capped); + } + let per_us = (t0.elapsed().as_micros() as f64) / (n_build as f64); + build_samples.push(per_us); + } + let build_median_us = build_samples[build_samples.len() / 2]; + runner.record( + "ui_grip_budget_build", + &format!( + "Selection-change grip budget over {} grips (sort + FxHashSet + filter)", + n_fixture_grips + ), + "µs", + build_samples, + Some(((n_fixture_grips as f64) / (build_median_us / 1_000_000.0), "grips/s")), + Some(7_000.0), // Target < 7 ms (selection-change hitch budget; measured ~2.36 ms full / ~0.30 ms quick, ~3x headroom) + ); +} + // ── 13. Wide & Tapered Arc + Donut Tessellation ───────────────────────────── fn bench_wide_and_tapered_arc_tessellation(runner: &mut BenchmarkRunner) { @@ -1878,6 +2022,7 @@ fn main() { bench_ui_icon_caching(&mut runner); bench_ui_plotstyle_layer_usage(&mut runner); bench_ui_statusbar_derived_data(&mut runner); + bench_ui_grip_budget(&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/mod.rs b/src/app/mod.rs index 674622972..be4da2bdd 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -44,6 +44,13 @@ mod visibility; pub use style_ops::StyleKind; +/// Re-exported `pub` (not `pub(crate)`) so the `cargo bench` harness +/// (`benches/`, an external crate) can measure the real grip-budget helper as +/// `ui_grip_budget`. The `properties` / `settings` modules themselves stay +/// private / `pub(crate)`; only these two names are reachable externally. +pub use properties::apply_grip_budget; +pub use settings::MAX_SELECTED_GRIPS; + use document::DocumentTab; use crate::modules::ModuleEvent; diff --git a/src/app/properties.rs b/src/app/properties.rs index d1982697a..d95d8bfd1 100644 --- a/src/app/properties.rs +++ b/src/app/properties.rs @@ -1,7 +1,7 @@ use super::helpers::{entity_type_key, entity_type_label, title_case_word}; use super::{OpenCADStudio, VARIES_LABEL}; use crate::io::linetypes; -use crate::scene::model::object::PropValue; +use crate::scene::model::object::{GripDef, PropValue}; use crate::scene::view::dispatch; use crate::t; use crate::ui; @@ -13,6 +13,47 @@ use acadrust::{Entity, EntityType, Handle}; /// shows a count-only summary instead. Bulk edits still go through the ribbon. const MAX_PROP_AGGREGATE: usize = 2_000; +/// Cap the total number of selection grips at +/// [`crate::app::settings::MAX_SELECTED_GRIPS`], keeping entity order with +/// non-midpoint (vertex/stretch) grips first. Below the cap both vecs are +/// returned untouched, so existing selections are identical. The parallel +/// `handles` vec is truncated in lockstep with `grips`. +/// +/// `pub` (not private) so the `cargo bench` harness (external crate) measures +/// the real function as `ui_grip_budget`. +pub fn apply_grip_budget( + mut grips: Vec, + mut handles: Vec, +) -> (Vec, Vec) { + debug_assert_eq!( + grips.len(), + handles.len(), + "grips/handles are parallel vecs built in lockstep" + ); + if grips.len() > crate::app::settings::MAX_SELECTED_GRIPS { + // Vertex budget: one dense polyline can emit ~2 grips/vertex past the + // object-count gate above. Keep entity order; non-midpoint (stretch) + // grips first, midpoints fill the remainder. + let cap = crate::app::settings::MAX_SELECTED_GRIPS; + let mut idx: Vec = (0..grips.len()).collect(); + idx.sort_by_key(|&j| grips[j].is_midpoint); // false (vertex) first; stable + idx.truncate(cap); + idx.sort_unstable(); // restore entity order for determinism + let keep: rustc_hash::FxHashSet = idx.into_iter().collect(); + let mut kept_grips = Vec::with_capacity(cap); + let mut kept_handles = Vec::with_capacity(cap); + for (j, (grip, handle)) in grips.into_iter().zip(handles.into_iter()).enumerate() { + if keep.contains(&j) { + kept_grips.push(grip); + kept_handles.push(handle); + } + } + grips = kept_grips; + handles = kept_handles; + } + (grips, handles) +} + fn visual_style_properties_text(style: &acadrust::objects::VisualStyle) -> String { style .properties @@ -2798,6 +2839,7 @@ handles={handles_ms:.1} panel={:.1} ribbon={ribbon_ms:.1} tail={:.1} selected={} grips.push(grip); } } + let (grips, handles) = apply_grip_budget(grips, handles); (single_handle, grips, handles) }; self.tabs[i].selected_handle = new_handle; @@ -4497,6 +4539,126 @@ mod chprop_integration_tests { #[cfg(test)] mod grip_limit_tests { use super::*; + use crate::scene::model::object::GripShape; + + fn test_grip(id: usize, is_midpoint: bool) -> GripDef { + GripDef { + id, + world: glam::DVec3::ZERO, + is_midpoint, + shape: GripShape::Square, + dir: None, + axis: None, + } + } + + #[test] + fn refresh_selected_grips_caps_total_grip_count() { + let cap = crate::app::settings::MAX_SELECTED_GRIPS; + let n = cap + 500; + let grips: Vec = (0..n).map(|id| test_grip(id, false)).collect(); + let handles: Vec = (0..n as u64).map(|k| Handle::new(k + 1)).collect(); + let (grips, handles) = super::apply_grip_budget(grips, handles); + assert_eq!( + grips.len(), + cap, + "grips past the budget must be dropped, kept {}/{}", + grips.len(), + n, + ); + assert_eq!( + handles.len(), + grips.len(), + "handles must stay in lockstep with grips", + ); + for (grip, handle) in grips.iter().zip(handles.iter()) { + assert_eq!( + *handle, + Handle::new(grip.id as u64 + 1), + "handle at each index must still belong to its grip", + ); + } + } + + #[test] + fn refresh_selected_grips_prefers_vertex_grips_over_midpoints() { + let cap = crate::app::settings::MAX_SELECTED_GRIPS; + let n = cap + 500; + // Interleaved: even ids are midpoints, odd ids are vertices. + let grips: Vec = (0..n).map(|id| test_grip(id, id % 2 == 0)).collect(); + let handles: Vec = (0..n as u64).map(|k| Handle::new(k + 1)).collect(); + let (grips, handles) = super::apply_grip_budget(grips, handles); + assert_eq!(grips.len(), cap); + assert_eq!(handles.len(), grips.len()); + // Fewer vertices than the cap: every vertex grip survives, midpoints + // fill the remainder — no vertex is sacrificed for a midpoint. + let kept_vertices: Vec = grips + .iter() + .filter(|grip| !grip.is_midpoint) + .map(|grip| grip.id) + .collect(); + let all_vertices: Vec = (0..n).filter(|id| id % 2 == 1).collect(); + assert_eq!( + kept_vertices, all_vertices, + "no vertex grip may be sacrificed for a midpoint", + ); + let ids: Vec = grips.iter().map(|grip| grip.id).collect(); + assert!(ids.is_sorted(), "retained grips must keep entity order"); + for (grip, handle) in grips.iter().zip(handles.iter()) { + assert_eq!(*handle, Handle::new(grip.id as u64 + 1)); + } + + // Vertices alone past the cap: the tail is truncated deterministically. + let over = cap + 100; + let grips: Vec = (0..over).map(|id| test_grip(id, false)).collect(); + let handles: Vec = (0..over as u64).map(|k| Handle::new(k + 1)).collect(); + let (grips, handles) = super::apply_grip_budget(grips, handles); + assert_eq!(grips.len(), cap); + assert_eq!(handles.len(), grips.len()); + assert!( + grips.iter().zip(0..cap).all(|(grip, id)| grip.id == id), + "vertex overflow must keep the first `cap` grips in entity order", + ); + for (grip, handle) in grips.iter().zip(handles.iter()) { + assert_eq!(*handle, Handle::new(grip.id as u64 + 1)); + } + } + + #[test] + fn grip_budget_boundary_lengths() { + let cap = crate::app::settings::MAX_SELECTED_GRIPS; + + // Empty vec passthrough. + let (grips, handles) = super::apply_grip_budget(Vec::new(), Vec::new()); + assert!(grips.is_empty()); + assert!(handles.is_empty()); + + // len == cap passthrough: content untouched. + let grips: Vec = (0..cap).map(|id| test_grip(id, id % 2 == 0)).collect(); + let handles: Vec = (0..cap as u64).map(|k| Handle::new(k + 1)).collect(); + let (kept_grips, kept_handles) = super::apply_grip_budget(grips, handles); + assert_eq!(kept_grips.len(), cap); + assert_eq!(kept_handles.len(), cap); + assert!( + kept_grips.iter().zip(0..cap).all(|(grip, id)| grip.id == id), + "at exactly the cap every grip must survive in order", + ); + for (grip, handle) in kept_grips.iter().zip(kept_handles.iter()) { + assert_eq!(*handle, Handle::new(grip.id as u64 + 1)); + } + + // len == cap + 1 truncates to cap. + let over = cap + 1; + let grips: Vec = (0..over).map(|id| test_grip(id, false)).collect(); + let handles: Vec = (0..over as u64).map(|k| Handle::new(k + 1)).collect(); + let (kept_grips, kept_handles) = super::apply_grip_budget(grips, handles); + assert_eq!(kept_grips.len(), cap); + assert_eq!(kept_handles.len(), cap); + assert!( + kept_grips.iter().zip(0..cap).all(|(grip, id)| grip.id == id), + "one over the cap must drop exactly the tail grip", + ); + } #[test] fn a_selection_past_the_limit_gets_no_grips() { diff --git a/src/app/settings.rs b/src/app/settings.rs index 938189196..308f3754e 100644 --- a/src/app/settings.rs +++ b/src/app/settings.rs @@ -329,6 +329,15 @@ pub fn format_snap_angle(deg: f32) -> String { /// GRIPOBJLIMIT default: past this many selected objects, no grips are drawn. pub const DEFAULT_GRIP_OBJECT_LIMIT: i32 = 100; +/// GRIPVERTLIMIT default: cap on TOTAL selection grips across all selected +/// objects. `grip_object_limit` gates object count, but one dense polyline +/// can emit ~2 grips/vertex past it — this caps the vertex blowup. Mid-segment +/// grips are dropped first; vertex grips are kept. +/// +/// `pub` so the `cargo bench` harness (external crate) measures the real +/// constant alongside [`crate::app::apply_grip_budget`] as `ui_grip_budget`. +pub const MAX_SELECTED_GRIPS: usize = 4096; + /// The "settings" section of the consolidated config ([`crate::app::config`]). /// Field defaults mirror the app's in-code defaults so a missing key restores /// the value the app boots with.