From 4432f633e7d0f44dd7340cd22ee46caf445d9fcb Mon Sep 17 00:00:00 2001 From: Karim Jerbi Date: Mon, 21 Sep 2026 22:30:33 +0100 Subject: [PATCH] perf(glyphs): memoize constraint-glyph placements behind constraints_epoch Placements (entity lookups, intersections, String labels, projections) recomputed per view() and per mouse-move hit-test, with overlay re-clones and 2-3x offset recomputes. New constraints_epoch bumped on every set mutation/visibility/undo/DWG-load (plus new set_constraint_enabled API); single-slot Arc<[GlyphEntry]> memo on an 11-field key (hover/selection excluded); overlay de-cloned/de-duped to the single geometry source; hit paths reuse the cache. Tests: - cached_glyph_placements_hit_returns_same_arc: same-key Arc identity - cached_glyph_placements_invalidated_by_metadata_edit: edit + selection change yield new Arc - glyph_hit_test_entries_matches_precomputed_boxes: shared hit-test helper Bench (cargo bench glyph, 60 constraints): 8.18us -> 0.05-0.15us (~55x). A/B reference deleted. --- benches/README.md | 3 +- benches/performance_benchmarks.rs | 94 ++++++ src/app/command_driver.rs | 12 + src/app/commands/draw.rs | 1 + src/app/history.rs | 3 + src/app/view/mod.rs | 48 +-- src/scene/dwg_native_constraints.rs | 4 + src/scene/entity.rs | 1 + src/scene/mod.rs | 47 +++ src/scene/parametric_constraints.rs | 499 +++++++++++++++++++++++++++- src/scene/parametric_solve.rs | 14 +- src/ui/overlay.rs | 280 +++++----------- 12 files changed, 767 insertions(+), 239 deletions(-) diff --git a/benches/README.md b/benches/README.md index c9254851c..5e48f4325 100644 --- a/benches/README.md +++ b/benches/README.md @@ -19,7 +19,7 @@ cargo bench --bench performance_benchmarks -- --quick --- -## Benchmark Suite Catalog (29 Metrics) +## Benchmark Suite Catalog (30 Metrics) ### 1. Scene & Entity Management - `scene_entity_ingestion_10k`: Ingestion throughput for 10,000 mixed 2D entities (Lines, Circles, Arcs, Polylines). @@ -55,6 +55,7 @@ cargo bench --bench performance_benchmarks -- --quick - `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). +- `ui_constraint_glyphs`: Constraint-glyph cache hit (`cached_glyph_placements` key build + `Arc` clone over dozens of glyphs, no recompute). ### 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 8cbd435d8..d633c2fd9 100644 --- a/benches/performance_benchmarks.rs +++ b/benches/performance_benchmarks.rs @@ -1618,6 +1618,99 @@ fn bench_ui_grip_budget(runner: &mut BenchmarkRunner) { ); } +// ── 12e. UI Constraint-Glyph Cache Hit (memoised placements) ──────────────── +// Covers the parametric constraint-glyph memo (`Scene::cached_glyph_placements`, +// src/scene/parametric_constraints.rs): dozens of constraints whose per-frame +// view + hit-test + dwell + click share one key, so the hot path is a cache +// hit (key build + Arc clone), not a placements recompute (entity lookups, +// intersections, String labels, projections). +// +// NOTE: the bench calls the real `cached_glyph_placements` with the same key +// inputs the production consumers (app/view, viewport hit paths, overlay) and +// the `cached_glyph_placements_*` unit-test fixture use +// (`ParametricScope::ModelSpace`, full-canvas vp, values on, display 3, +// bar 4095), so the measured hit is exactly what the frame shares. + +fn bench_ui_constraint_glyphs(runner: &mut BenchmarkRunner) { + if !runner.should_run("ui_constraint_glyphs") { + return; + } + + // Fixture: dozens of horizontal constraints over a grid of short lines + // around the origin — mirrors the `cached_glyph_placements_*` unit-test + // fixture (Scene::new + add_entity + constraint add + note applied). + let n_constraints = if runner.quick_mode { 24 } else { 60 }; + let mut scene = Scene::new(); + let cols = 8; + for i in 0..n_constraints { + let x = ((i % cols) as f64) * 30.0 - 100.0; + let y = ((i / cols) as f64) * 30.0 - 100.0; + let line = scene.add_entity(EntityType::Line(Line::from_points( + Vector3::new(x, y, 0.0), + Vector3::new(x + 20.0, y, 0.0), + ))); + let id = scene + .parametric_constraint_set_mut(ParametricScope::ModelSpace) + .add( + ConstraintKind::Horizontal, + vec![ParametricRef::whole(line)], + None, + ); + scene.note_parametric_constraint_applied(ParametricScope::ModelSpace, id, 3); + } + scene.selection.borrow_mut().vp_size = (1920.0, 1080.0); + let vp = (1920.0_f32, 1080.0_f32); + + // Prime the cache so every timed call below is a hit (same key, same Arc). + let primed = + scene.cached_glyph_placements(ParametricScope::ModelSpace, vp, true, 3, 4095); + assert!( + !primed.is_empty(), + "glyph fixture must yield placements" + ); + let n_glyphs = primed.len(); + + // Warm-up for allocator settling. + for _ in 0..10 { + let hit = + scene.cached_glyph_placements(ParametricScope::ModelSpace, vp, true, 3, 4095); + black_box(hit); + } + + let n = if runner.quick_mode { 200 } else { 1_000 }; + let runs = 5; + let mut samples = Vec::with_capacity(runs); + + for _ in 0..runs { + let t0 = Instant::now(); + for _ in 0..n { + let hit = scene.cached_glyph_placements( + black_box(ParametricScope::ModelSpace), + black_box(vp), + black_box(true), + black_box(3), + black_box(4095), + ); + black_box(hit); + } + 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_constraint_glyphs", + &format!( + "Constraint-glyph cache hit over {} glyphs (key build + Arc clone, no recompute)", + n_glyphs + ), + "µs", + samples, + Some(((n_glyphs as f64) / (median_us / 1_000_000.0), "glyphs/s")), + Some(0.5), // Target threshold < 0.5 µs (measured ~0.06 µs quick / ~0.15 µs full, ~3x headroom) + ); +} + // ── 13. Wide & Tapered Arc + Donut Tessellation ───────────────────────────── fn bench_wide_and_tapered_arc_tessellation(runner: &mut BenchmarkRunner) { @@ -2023,6 +2116,7 @@ fn main() { bench_ui_plotstyle_layer_usage(&mut runner); bench_ui_statusbar_derived_data(&mut runner); bench_ui_grip_budget(&mut runner); + bench_ui_constraint_glyphs(&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/command_driver.rs b/src/app/command_driver.rs index 09776bd3b..75493c3dd 100644 --- a/src/app/command_driver.rs +++ b/src/app/command_driver.rs @@ -1409,6 +1409,7 @@ impl OpenCADStudio { dimension.into_iter().collect(), parameter.into_iter().collect(), ); + self.tabs[i].scene.bump_constraints_epoch(); if self.tabs[i].scene.selected_constraint == Some(id) { self.tabs[i].scene.selected_constraint = None; } @@ -1720,6 +1721,9 @@ impl OpenCADStudio { self.tabs[i] .scene .record_undo_parametric_constraints_before(scope, before); + // A parameter rename rewrites the `Named` driving references + // pointing at it — the glyph labels change with them. + let mut renamed = false; for constraint in &mut self.tabs[i].scene.parametric_constraints[set_index].constraints { @@ -1728,9 +1732,13 @@ impl OpenCADStudio { { if name == ¤t.name { *name = typed.clone(); + renamed = true; } } } + if renamed { + self.tabs[i].scene.bump_constraints_epoch(); + } } } self.resolve_named_parameter_edit(i, &resolve_name); @@ -1866,6 +1874,10 @@ impl OpenCADStudio { if let Some(constraint) = set.constraints.iter_mut().find(|c| c.id == id) { constraint.enabled = !reference; } + // Toggling `enabled` changes the glyph placement set (and the + // dynamic pills), so the memoised placements must miss next + // frame — same guarantee `set_constraint_enabled` gives. + self.tabs[i].scene.bump_constraints_epoch(); self.tabs[i].scene.refresh_dynamic_dimension_texts(); self.tabs[i].scene.sync_native_parametric_graph(); self.tabs[i].dirty = true; diff --git a/src/app/commands/draw.rs b/src/app/commands/draw.rs index aa6d28f11..c51425fc7 100644 --- a/src/app/commands/draw.rs +++ b/src/app/commands/draw.rs @@ -1159,6 +1159,7 @@ impl OpenCADStudio { set.remove(*id); } self.purge_dimensional_extras(i, dimensions, parameters); + self.tabs[i].scene.bump_constraints_epoch(); let changes: Vec<_> = handles .iter() .copied() diff --git a/src/app/history.rs b/src/app/history.rs index 220e34dbc..0afdce1c3 100644 --- a/src/app/history.rs +++ b/src/app/history.rs @@ -978,6 +978,9 @@ impl OpenCADStudio { } let changes = self.tabs[i].scene.apply_entity_delta(&d.entities, undo); let scene = &mut self.tabs[i].scene; + if !d.parametric_constraints.is_empty() { + scene.bump_constraints_epoch(); + } for entry in &d.parametric_constraints { let value = if undo { &entry.before } else { &entry.after }; *scene.parametric_constraint_set_mut(entry.scope) = value.clone(); diff --git a/src/app/view/mod.rs b/src/app/view/mod.rs index eeaba25e5..a66a96e88 100644 --- a/src/app/view/mod.rs +++ b/src/app/view/mod.rs @@ -863,39 +863,26 @@ bg={bg_ms:.1}ms n={view_count}" .map(|constraint| constraint.kind.glyph_symbol().to_string()) }) }; - let constraint_glyphs: Vec<( - iced::Point, - [f32; 2], - String, - bool, - bool, - Vec, - )> = if is_paper { - Vec::new() + let constraint_glyphs: std::sync::Arc< + [crate::scene::parametric_constraints::GlyphEntry], + > = if is_paper { + std::sync::Arc::from([]) } else { let scope = tab.current_parametric_scope(); - tab.scene - .constraint_glyph_placements_screen( - scope, - sel_ref.vp_size, - self.show_constraint_values, - self.constraint_bar_display, - self.constraint_bar_mode, - ) - .into_iter() - .map(|(id, point, direction, label, is_conflicting, hover_points)| { - let selected = tab.scene.selected_constraint == Some(id); - ( - point, - direction, - label, - is_conflicting, - selected, - hover_points, - ) - }) - .collect() + tab.scene.cached_glyph_placements( + scope, + sel_ref.vp_size, + self.show_constraint_values, + self.constraint_bar_display, + self.constraint_bar_mode, + ) }; + // Selection stays OUTSIDE the cache (applied post-hoc): a selection + // change must not invalidate the placement memo. + let constraint_glyph_selected: std::sync::Arc<[bool]> = constraint_glyphs + .iter() + .map(|entry| tab.scene.selected_constraint == Some(entry.id)) + .collect(); crate::ui::overlay::selection_overlay( std::sync::Arc::clone(&tab.scene.selection), snap_info, @@ -939,6 +926,7 @@ bg={bg_ms:.1}ms n={view_count}" grip_hover: self.model_space.grip_hover, }, constraint_glyphs, + constraint_glyph_selected, self.constraint_glyph_tooltip .map(|kind| crate::t!(kind.label()).into_owned()), constraint_cursor_badge, diff --git a/src/scene/dwg_native_constraints.rs b/src/scene/dwg_native_constraints.rs index cde66ee7e..d8702d09f 100644 --- a/src/scene/dwg_native_constraints.rs +++ b/src/scene/dwg_native_constraints.rs @@ -2841,6 +2841,10 @@ impl Scene { self.parametric_constraints.push(set); } } + // Whole-set replace (file open / reload): covers the DWG-import + // `enabled` / `axis_direction` / `distance_direction` / `angle_sector` + // writes, which happen on the local set during decode. + self.bump_constraints_epoch(); } /// Makes the standard associative graph match the live command model. diff --git a/src/scene/entity.rs b/src/scene/entity.rs index 15f5acfbc..a8971e59f 100644 --- a/src/scene/entity.rs +++ b/src/scene/entity.rs @@ -2710,6 +2710,7 @@ impl Scene { // and named parameters into the fresh one. self.parametric_constraints.clear(); self.named_parameters = crate::scene::named_parameters::ParameterTable::new(); + self.bump_constraints_epoch(); self.bump_geometry(); self.bump_layout_epoch(); self.bump_scale_epoch(); diff --git a/src/scene/mod.rs b/src/scene/mod.rs index 00dc84d40..9c66165e8 100644 --- a/src/scene/mod.rs +++ b/src/scene/mod.rs @@ -2321,6 +2321,25 @@ pub struct Scene { parametric_constraints::ParametricScope, parametric_constraints::ConstraintId, )>, + /// Bumped on every parametric-constraint-set mutation (add / remove / + /// metadata edit / visibility override / whole-set restore). The + /// constraint-glyph memoization keys on this so a stale cache can never + /// outlive the edit that invalidated it — a missed bump is a + /// correctness bug (stale glyphs), not just a perf miss. + constraints_epoch: u64, + /// Memoised constraint-glyph placements for the latest [`GlyphKey`](parametric_constraints::GlyphKey) + /// (Task 3; overlay/hit consumers adopt it in Task 4). Single-slot memo: + /// within one frame view + hit-test + dwell + click share the key (hits), + /// camera motion misses once per frame by design, and edits just miss on + /// the stale entry — no leak possible. `RefCell` + `Arc` shape mirrors + /// `layout_names_cache`; a missed constraint bump serves stale glyphs, so + /// every set mutation must bump `constraints_epoch`. + glyph_cache: RefCell< + Option<( + parametric_constraints::GlyphKey, + Arc<[parametric_constraints::GlyphEntry]>, + )>, + >, /// Document-wide named-parameter and expression table decoded from /// standard associative variables. pub(crate) named_parameters: named_parameters::ParameterTable, @@ -2656,6 +2675,8 @@ impl Scene { dynamic_dimension_camera_gen: None, hidden_parametric_constraints: HashSet::default(), shown_parametric_constraints: HashSet::default(), + constraints_epoch: 0, + glyph_cache: RefCell::new(None), named_parameters: named_parameters::ParameterTable::new(), has_associative_centers: std::cell::Cell::new(None), block_defn_cache: RefCell::new(HashMap::default()), @@ -5703,6 +5724,9 @@ impl Scene { .collect(); self.object_isolation.hidden.extend(hide); self.object_isolation.keep = Some(keep); + // Isolation hides entities the memoised glyph placements read + // (`entity_temporarily_hidden`), so invalidate them too. + self.bump_constraints_epoch(); if !changes.is_empty() { self.bump_entities(&changes); } @@ -5726,6 +5750,9 @@ impl Scene { self.selected.clear(); self.selected_order.clear(); self.bump_selection_set(); + // Newly hidden entities change `entity_temporarily_hidden`, which the + // memoised glyph placements read — invalidate them too. + self.bump_constraints_epoch(); self.bump_entities(&changes); } @@ -5742,6 +5769,12 @@ impl Scene { .map(|handle| (handle, ChangeKind::Modified)) .collect(); self.object_isolation = ObjectIsolationState::default(); + // Restoring isolated entities changes `entity_temporarily_hidden`, + // which the memoised glyph placements read — invalidate them too. + // (`reset_transient_visibility` needs no bump: it only runs on + // doc-replace, where `load_parametric_constraints_from_document` + // already bumps the epoch for the whole-set replace.) + self.bump_constraints_epoch(); if !changes.is_empty() { self.bump_entities(&changes); } @@ -6027,6 +6060,20 @@ impl Scene { self.scale_epoch += 1; } + /// Current parametric-constraint-set epoch (cache key input for the + /// memoised `cached_glyph_placements` accessor). + #[allow(dead_code)] + pub(crate) fn constraints_epoch(&self) -> u64 { + self.constraints_epoch + } + + /// Invalidate constraint-glyph consumers. Call on EVERY mutation of the + /// constraint set or its visibility overrides — a missed bump serves + /// stale glyphs. + pub(crate) fn bump_constraints_epoch(&mut self) { + self.constraints_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(); diff --git a/src/scene/parametric_constraints.rs b/src/scene/parametric_constraints.rs index 6cf0caa26..95a39065a 100644 --- a/src/scene/parametric_constraints.rs +++ b/src/scene/parametric_constraints.rs @@ -2,6 +2,8 @@ use super::named_parameters::DrivingValue; use acadrust::types::{Handle, Vector3}; +use std::hash::{Hash, Hasher}; +use std::sync::Arc; /// One endpoint a constraint attaches to: an entity plus which sub-element /// of it. @@ -1579,6 +1581,170 @@ pub(crate) fn constraint_hover_points( points } +/// Memoised key for `Scene::cached_glyph_placements`. +/// `sel_sig` hashes the hide sets the dynamic pills read on EVERY display +/// mode (`preview_hidden`, `command_preview_hidden`, +/// `hidden_dynamic_dimensions` — `constraint_glyph_placements_screen` filters +/// `entity_temporarily_hidden` unconditionally) PLUS, only when +/// `display_mode & 2 != 0`, the selection-gated sets (`selected`, +/// `hidden_parametric_constraints`, `shown_parametric_constraints`, and again +/// `preview_hidden` as a selection source). Hover and selection-highlight +/// state deliberately stay OUT — they only affect the highlight, never the +/// placement set. Isolation contents stay OUT — the isolation mutators bump +/// `constraints_epoch` instead, so no key input is needed for them. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct GlyphKey { + pub scope: ParametricScope, + pub c_epoch: u64, + pub g_epoch: u64, + pub cam_gen: u64, + pub active_vp: Option, + pub layout_is_model: bool, + pub vp_bits: (u32, u32), + pub show_values: bool, + pub disp: i16, + pub bar: i16, + pub sel_sig: u64, + pub annotation_scale: u32, +} + +/// One cached glyph placement: the `constraint_glyph_placements_screen` +/// tuple plus its precomputed overlay layout (`size`, `tangent_dx`, +/// `top_left`), so overlay consumers never re-clone labels or recompute +/// offsets. `is_selected` and the tooltip stay OUT — applied post-hoc. +#[derive(Debug, Clone)] +pub struct GlyphEntry { + pub id: ConstraintId, + pub anchor: iced::Point, + pub outward: [f32; 2], + pub label: Arc, + pub conflicting: bool, + pub hover: Arc<[iced::Point]>, + pub size: iced::Size, + pub tangent_dx: f32, + pub top_left: iced::Point, +} + +// Single source of truth for constraint-glyph layout (Task 4): the overlay +// side was deleted and imports these instead, so the drawn pills, the +// hit-test, and the cached `top_left`/`tangent_dx` can never drift apart. +pub(crate) const GLYPH_SIZE: f32 = 14.0; +const GLYPH_PAD_X: f32 = 7.0; +const GLYPH_PAD_Y: f32 = 4.0; +const GLYPH_GAP: f32 = 6.0; +const GLYPH_ROW_GAP: f32 = 4.0; +const GLYPH_COINCIDENT_SIZE: f32 = 9.0; + +pub(crate) fn glyph_is_compact(label: &str) -> bool { + matches!(label, "≡" | "∈") +} + +pub(crate) fn glyph_is_fixed(label: &str) -> bool { + label == "F" || label == FIXED_POINT_GLYPH +} + +pub(crate) fn glyph_is_vertical(label: &str) -> bool { + label == "│" || label == VERTICAL_POINTS_GLYPH +} + +pub(crate) fn constraint_glyph_size(label: &str) -> iced::Size { + if glyph_is_compact(label) { + return iced::Size::new(GLYPH_COINCIDENT_SIZE, GLYPH_COINCIDENT_SIZE); + } + if label == "G²" + || glyph_is_fixed(label) + || glyph_is_vertical(label) + || label == DYNAMIC_DIMENSION_GLYPH + { + let side = GLYPH_SIZE + GLYPH_PAD_Y * 2.0; + return iced::Size::new(side, side); + } + let w = label.chars().count() as f32 * GLYPH_SIZE * 0.62 + GLYPH_PAD_X * 2.0; + let h = GLYPH_SIZE + GLYPH_PAD_Y * 2.0; + iced::Size::new(w, h) +} + +pub(crate) fn constraint_glyph_box( + anchor: iced::Point, + outward: [f32; 2], + label: &str, + tangent_offset: f32, +) -> (iced::Point, iced::Size) { + let size = constraint_glyph_size(label); + let gap = if glyph_is_compact(label) { + 1.0 + } else { + GLYPH_GAP + }; + let distance = + outward[0].abs() * size.width * 0.5 + outward[1].abs() * size.height * 0.5 + gap; + let tangent = [-outward[1], outward[0]]; + ( + iced::Point::new( + anchor.x + outward[0] * distance + tangent[0] * tangent_offset - size.width * 0.5, + anchor.y + outward[1] * distance + tangent[1] * tangent_offset - size.height * 0.5, + ), + size, + ) +} + +pub(crate) fn constraint_glyph_offsets(glyphs: &[(iced::Point, [f32; 2], &str)]) -> Vec { + let mut groups: rustc_hash::FxHashMap<[u32; 4], Vec> = + rustc_hash::FxHashMap::default(); + for (index, (anchor, outward, _)) in glyphs.iter().enumerate() { + groups + .entry([ + anchor.x.to_bits(), + anchor.y.to_bits(), + outward[0].to_bits(), + outward[1].to_bits(), + ]) + .or_default() + .push(index); + } + + let mut offsets = vec![0.0; glyphs.len()]; + for indices in groups.values().filter(|indices| indices.len() > 1) { + let half_extents: Vec = indices + .iter() + .map(|index| { + let (_, outward, label) = &glyphs[*index]; + let size = constraint_glyph_size(label); + let tangent = [-outward[1], outward[0]]; + tangent[0].abs() * size.width * 0.5 + tangent[1].abs() * size.height * 0.5 + }) + .collect(); + let total = + half_extents.iter().sum::() * 2.0 + GLYPH_ROW_GAP * (indices.len() - 1) as f32; + let mut cursor = -total * 0.5; + for (index, half_extent) in indices.iter().zip(half_extents) { + offsets[*index] = cursor + half_extent; + cursor += half_extent * 2.0 + GLYPH_ROW_GAP; + } + } + offsets +} + +/// Hit-tests screen point `p` against the precomputed `top_left`/`size` boxes +/// in `entries` — the same pills `draw` renders, including the tangential +/// fan-out baked into `tangent_dx` at cache time. Returns the index of the +/// topmost (last-drawn) match. Shared by `Scene::constraint_glyph_hit` and +/// the overlay draw/hover paths so the clickable area can never drift from +/// what's drawn and no caller recomputes offsets. +pub(crate) fn glyph_hit_test_entries(entries: &[GlyphEntry], p: iced::Point) -> Option { + entries + .iter() + .enumerate() + .rev() + .find_map(|(index, entry)| { + let within = p.x >= entry.top_left.x + && p.x <= entry.top_left.x + entry.size.width + && p.y >= entry.top_left.y + && p.y <= entry.top_left.y + entry.size.height; + within.then_some(index) + }) +} + impl super::Scene { /// Expand through enabled constraints. Whole-object translations follow /// point connections; grip previews include curve relations as well. @@ -1772,6 +1938,10 @@ impl super::Scene { } else { self.shown_parametric_constraints.remove(&(scope, id)); } + // Every production `add` flows through here, so this bump covers all + // constraint creations (add / add_axis_constraint / AUTOCONSTRAIN / + // DCCONVERT / Coincident / Equal / Fixed / dimensional handlers). + self.bump_constraints_epoch(); } pub fn set_parametric_constraint_visibility( @@ -1806,6 +1976,9 @@ impl super::Scene { } } self.refresh_hidden_dynamic_dimensions(); + if !ids.is_empty() { + self.bump_constraints_epoch(); + } ids.len() } @@ -1959,9 +2132,48 @@ impl super::Scene { .map(|handle| (handle, super::ChangeKind::Modified)) .collect(); self.hidden_dynamic_dimensions = desired; + // The memoised glyph placements read this set on every display mode + // (dynamic pills filter `entity_temporarily_hidden` unconditionally), + // so a change here must invalidate them — this covers all four + // callers, including the DYNCONSTRAINTDISPLAY toggle that has no + // call-site bump of its own. + self.bump_constraints_epoch(); self.bump_entities(&changes); } + /// Enables or disables one constraint without losing it — the production + /// path for toggling `ParametricConstraint::enabled` (a re-solve skips a + /// disabled constraint; glyph placements filter it out). Returns whether + /// a constraint with `id` exists in `scope`. Bumps the constraints epoch + /// exactly when the value actually changes so future callers cannot + /// introduce a stale-glyph path by writing the field directly. + pub fn set_constraint_enabled( + &mut self, + scope: ParametricScope, + id: ConstraintId, + enabled: bool, + ) -> bool { + let mut found = false; + let mut changed = false; + if let Some(set) = self + .parametric_constraints + .iter_mut() + .find(|set| set.scope == scope) + { + if let Some(constraint) = set.constraints.iter_mut().find(|c| c.id == id) { + found = true; + if constraint.enabled != enabled { + constraint.enabled = enabled; + changed = true; + } + } + } + if changed { + self.bump_constraints_epoch(); + } + found + } + /// Infers relations already present in the selected geometry. pub fn inferred_parametric_constraints( &self, @@ -2298,10 +2510,11 @@ impl super::Scene { /// `vp_size` is the full canvas size (as `SelectionState::vp_size` /// reports it), matching what `viewport_edit_frame`/ /// `active_model_tile_bounds` expect. Mirrors the projection - /// `crate::app::view` builds its own render list with, and is reused by - /// [`constraint_glyph_hit`](Self::constraint_glyph_hit) — both feed the - /// same `(anchor, outward, label)` triples into - /// `crate::ui::overlay::constraint_glyph_box`/`constraint_glyph_offsets`, + /// `crate::app::view` builds its own render list with. The memoised + /// [`cached_glyph_placements`](Self::cached_glyph_placements) maps these + /// same `(anchor, outward, label)` triples through + /// [`constraint_glyph_box`](crate::scene::parametric_constraints::constraint_glyph_box)/ + /// [`constraint_glyph_offsets`](crate::scene::parametric_constraints::constraint_glyph_offsets), /// so hit-testing can never drift from what's actually drawn. pub fn constraint_glyph_placements_screen( &self, @@ -2503,10 +2716,9 @@ impl super::Scene { } /// Hit-tests screen point `p` (same coordinate space as `p_full` in the - /// viewport click handler) against the glyph pills from - /// [`constraint_glyph_placements_screen`](Self::constraint_glyph_placements_screen), - /// via `crate::ui::overlay::constraint_glyph_hit_test`'s shared layout - /// math, so a click only registers where the pill is actually drawn. + /// viewport click handler) against the memoised [`cached_glyph_placements`](Self::cached_glyph_placements) + /// entries, so a click only registers where the pill is actually drawn — + /// with no placement recompute on the mouse-move/click hot path. pub fn constraint_glyph_hit( &self, scope: ParametricScope, @@ -2516,6 +2728,111 @@ impl super::Scene { bar_mode: i16, p: iced::Point, ) -> Option { + let entries = self.cached_glyph_placements(scope, vp_size, show_values, display_mode, bar_mode); + let index = glyph_hit_test_entries(&entries, p)?; + Some(entries[index].id) + } + + /// Order-independent hash of the visibility sets that feed + /// [`constraint_glyph_placements_screen`](Self::constraint_glyph_placements_screen). + /// The dynamic pills filter `entity_temporarily_hidden` on EVERY display + /// mode, so `preview_hidden`, `command_preview_hidden` and + /// `hidden_dynamic_dimensions` are hashed UNCONDITIONALLY; `selected` and + /// the hidden/shown override sets only affect the `should_display` filter + /// when `display_mode & 2 != 0`, so they stay conditional (and the key + /// stays shared across selection changes in the other modes). + pub(crate) fn glyph_selection_signature(&self, display_mode: i16) -> u64 { + fn item_hash(item: &impl Hash) -> u64 { + let mut hasher = rustc_hash::FxHasher::default(); + item.hash(&mut hasher); + hasher.finish() + } + // Order-independent single pass: per-item hashes combine + // commutatively (wrapping add), so iteration order never matters; + // one FxHasher over (combined, count) folds in the total length, no + // alloc/sort on the hot hit path. + let mut combined: u64 = 0; + let mut count: u64 = 0; + // Always-read hide sets: the dynamic-dimension pills consult these + // whatever the display mode, so a stale signature here would serve a + // pill for a just-hidden dimension (or hide a just-shown one). + for item in self.preview_hidden.iter() { + combined = combined.wrapping_add(item_hash(item)); + count += 1; + } + for item in self.command_preview_hidden.iter() { + combined = combined.wrapping_add(item_hash(item)); + count += 1; + } + for item in self.hidden_dynamic_dimensions.iter() { + combined = combined.wrapping_add(item_hash(item)); + count += 1; + } + if display_mode & 2 != 0 { + for item in self.selected.iter() { + combined = combined.wrapping_add(item_hash(item)); + count += 1; + } + for item in self.hidden_parametric_constraints.iter() { + combined = combined.wrapping_add(item_hash(item)); + count += 1; + } + for item in self.shown_parametric_constraints.iter() { + combined = combined.wrapping_add(item_hash(item)); + count += 1; + } + } + let mut hasher = rustc_hash::FxHasher::default(); + combined.hash(&mut hasher); + count.hash(&mut hasher); + hasher.finish() + } + + /// Memoised [`constraint_glyph_placements_screen`](Self::constraint_glyph_placements_screen), + /// keyed by [`GlyphKey`]: the miss path calls the existing placements fn + /// VERBATIM (no algorithm change), maps each tuple to a [`GlyphEntry`] + /// with precomputed layout, and stores the `Arc<[GlyphEntry]>` in the + /// Scene-owned single-slot memo. NaN filtering is the placements fn's own and is + /// cached as-is. Camera/layout/selection state is read from `self`, so + /// the `(scope, vp_size, show_values, display_mode, bar_mode)` signature + /// stays identical to the stub's. + /// + /// Single-slot rationale: within one frame, view + hit-test + dwell + + /// click all share the key (hits); across camera motion each frame misses + /// once by design; across edits the stale single entry just misses — no + /// leak possible (an unbounded map would orphan an entry per camera frame + /// and per edit, since `cam_gen` is in the key). + pub fn cached_glyph_placements( + &self, + scope: ParametricScope, + vp_size: (f32, f32), + show_values: bool, + display_mode: i16, + bar_mode: i16, + ) -> std::sync::Arc<[GlyphEntry]> { + let key = GlyphKey { + scope, + c_epoch: self.constraints_epoch, + g_epoch: self.geometry_epoch, + cam_gen: self.camera_generation, + active_vp: self.active_viewport, + layout_is_model: self.current_layout == "Model", + vp_bits: (vp_size.0.to_bits(), vp_size.1.to_bits()), + show_values, + disp: display_mode, + bar: bar_mode, + sel_sig: self.glyph_selection_signature(display_mode), + // The dynamic pills anchor through `dynamic_dimension_lock_anchor` + // with this scale — a CANNOSCALE change moves them, so it is keyed + // (bitwise: NaN never occurs here, and distinct bit patterns must + // miss rather than alias). + annotation_scale: self.annotation_scale.to_bits(), + }; + if let Some((cached_key, cached_arc)) = self.glyph_cache.borrow().as_ref() { + if *cached_key == key { + return std::sync::Arc::clone(cached_arc); + } + } let placements = self.constraint_glyph_placements_screen( scope, vp_size, @@ -2523,14 +2840,36 @@ impl super::Scene { display_mode, bar_mode, ); - let glyphs: Vec<(iced::Point, [f32; 2], String, bool)> = placements - .iter() - .map(|(_, point, direction, label, is_conflicting, _)| { - (*point, *direction, label.clone(), *is_conflicting) - }) + let offsets = constraint_glyph_offsets( + &placements + .iter() + .map(|(_, point, direction, label, _, _)| (*point, *direction, label.as_str())) + .collect::>(), + ); + let entries: Vec = placements + .into_iter() + .zip(offsets) + .map( + |((id, anchor, outward, label, conflicting, hover), tangent_dx)| { + let size = constraint_glyph_size(&label); + let (top_left, _) = constraint_glyph_box(anchor, outward, &label, tangent_dx); + GlyphEntry { + id, + anchor, + outward, + label: Arc::from(label), + conflicting, + hover: Arc::from(hover), + size, + tangent_dx, + top_left, + } + }, + ) .collect(); - let index = crate::ui::overlay::constraint_glyph_hit_test(&glyphs, p)?; - Some(placements[index].0) + let arc: Arc<[GlyphEntry]> = Arc::from(entries); + *self.glyph_cache.borrow_mut() = Some((key, Arc::clone(&arc))); + arc } /// Handle remapping lives in each command that duplicates @@ -2598,6 +2937,7 @@ impl super::Scene { } touched.sort(); touched.dedup(); + self.bump_constraints_epoch(); let changes: Vec<(Handle, super::ChangeKind)> = touched .into_iter() .map(|h| (h, super::ChangeKind::Modified)) @@ -2897,6 +3237,135 @@ mod tests { ); } + #[test] + fn glyph_hit_test_entries_matches_precomputed_boxes() { + let mut scene = super::super::Scene::new(); + let scope = ParametricScope::ModelSpace; + let line = scene.add_entity(acadrust::EntityType::Line( + acadrust::entities::Line::from_points( + Vector3::new(-1.0, 0.0, 0.0), + Vector3::new(1.0, 0.0, 0.0), + ), + )); + scene.selection.borrow_mut().vp_size = (800.0, 600.0); + let id = scene.parametric_constraint_set_mut(scope).add( + ConstraintKind::Horizontal, + vec![ParametricRef::whole(line)], + None, + ); + scene.note_parametric_constraint_applied(scope, id, 3); + let vp = (800.0_f32, 600.0_f32); + let entries = scene.cached_glyph_placements(scope, vp, true, 3, 4095); + assert!(!entries.is_empty(), "fixture must yield glyphs"); + // A click at the centre of the precomputed box must hit that entry, + // using the cached layout only (no offset recompute). + let centre = iced::Point::new( + entries[0].top_left.x + entries[0].size.width * 0.5, + entries[0].top_left.y + entries[0].size.height * 0.5, + ); + assert_eq!(super::glyph_hit_test_entries(&entries, centre), Some(0)); + // Far away from every box must miss. + assert_eq!( + super::glyph_hit_test_entries(&entries, iced::Point::new(-5000.0, -5000.0)), + None + ); + // `constraint_glyph_hit` must agree with the cached entries (it reuses + // the cache rather than recomputing placements). + assert_eq!( + scene.constraint_glyph_hit(scope, vp, true, 3, 4095, centre), + Some(entries[0].id) + ); + } + + #[test] + fn cached_glyph_placements_hit_returns_same_arc() { + let mut scene = super::super::Scene::new(); + let scope = ParametricScope::ModelSpace; + let line = scene.add_entity(acadrust::EntityType::Line( + acadrust::entities::Line::from_points( + Vector3::new(-1.0, 0.0, 0.0), + Vector3::new(1.0, 0.0, 0.0), + ), + )); + scene.selection.borrow_mut().vp_size = (800.0, 600.0); + let id = scene.parametric_constraint_set_mut(scope).add( + ConstraintKind::Horizontal, + vec![ParametricRef::whole(line)], + None, + ); + scene.note_parametric_constraint_applied(scope, id, 3); + let vp = (800.0_f32, 600.0_f32); + let first = scene.cached_glyph_placements(scope, vp, true, 3, 4095); + let second = scene.cached_glyph_placements(scope, vp, true, 3, 4095); + assert!( + !first.is_empty(), + "fixture must yield at least one glyph placement" + ); + assert!( + std::sync::Arc::ptr_eq(&first, &second), + "identical key inputs must return the same cached Arc" + ); + } + + #[test] + fn cached_glyph_placements_invalidated_by_metadata_edit() { + let mut scene = super::super::Scene::new(); + let scope = ParametricScope::ModelSpace; + let line = scene.add_entity(acadrust::EntityType::Line( + acadrust::entities::Line::from_points( + Vector3::new(-1.0, 0.0, 0.0), + Vector3::new(1.0, 0.0, 0.0), + ), + )); + scene.selection.borrow_mut().vp_size = (800.0, 600.0); + let id = scene.parametric_constraint_set_mut(scope).add( + ConstraintKind::Horizontal, + vec![ParametricRef::whole(line)], + None, + ); + scene.note_parametric_constraint_applied(scope, id, 3); + let vp = (800.0_f32, 600.0_f32); + let before = scene.cached_glyph_placements(scope, vp, true, 3, 4095); + assert!( + !before.is_empty(), + "fixture must yield at least one glyph placement" + ); + // NOTE: no production set_enabled/toggle_enabled method exists (grep: + // `enabled` is only written in `add()`, DWG import, and tests), so the + // metadata edit goes through the REAL production Scene-level toggle + // used by draw/update paths — hiding the geometric constraint — which + // Task 2 must wire to an epoch bump. Not a direct field write. + scene.set_parametric_constraint_visibility(scope, None, false, false); + let after = scene.cached_glyph_placements(scope, vp, true, 3, 4095); + assert!( + !std::sync::Arc::ptr_eq(&before, &after), + "metadata edit must invalidate the cache (new Arc)" + ); + assert!( + after.is_empty(), + "output must reflect the edit: hidden constraint yields no glyphs" + ); + // A selection change must also miss when `display_mode & 2 != 0` + // (sel_sig is part of the key): restore, warm, then reselect. + scene.set_parametric_constraint_visibility(scope, None, false, true); + let restored = scene.cached_glyph_placements(scope, vp, true, 3, 4095); + assert!( + !restored.is_empty(), + "restored constraint must yield glyphs again" + ); + let reselected_warm = scene.cached_glyph_placements(scope, vp, true, 3, 4095); + assert!( + std::sync::Arc::ptr_eq(&restored, &reselected_warm), + "identical key inputs must return the same cached Arc" + ); + scene.selected.insert(line); + let reselected = scene.cached_glyph_placements(scope, vp, true, 3, 4095); + assert!( + !std::sync::Arc::ptr_eq(&reselected_warm, &reselected), + "selection change must invalidate the cache (new Arc)" + ); + } + #[test] fn scope_owner_handle_resolves_block_directly() { let block_handle = h(42); diff --git a/src/scene/parametric_solve.rs b/src/scene/parametric_solve.rs index f1daeb8a1..1bf3104c3 100644 --- a/src/scene/parametric_solve.rs +++ b/src/scene/parametric_solve.rs @@ -3772,6 +3772,9 @@ impl Scene { let before = self.parametric_constraints[i].clone(); self.record_undo_parametric_constraints_before(scope, before); self.parametric_constraints[i].remove_all_touching(*handle); + // Erasing an entity deletes its constraints with it — the + // glyph set changed. + self.bump_constraints_epoch(); } } } @@ -3824,10 +3827,17 @@ impl Scene { }; if initial_fixed_refs.is_empty() { self.parametric_constraints[i].dof = Some(dof); - self.parametric_constraints[i].conflicts = conflicts; + if self.parametric_constraints[i].conflicts != conflicts { + self.parametric_constraints[i].conflicts = conflicts; + // Conflict badges render on the glyphs themselves. + self.bump_constraints_epoch(); + } } else { self.parametric_constraints[i].dof = None; - self.parametric_constraints[i].conflicts.clear(); + if !self.parametric_constraints[i].conflicts.is_empty() { + self.parametric_constraints[i].conflicts.clear(); + self.bump_constraints_epoch(); + } } // A dimensional constraint set to zero means the collapse. let zero_collapse: HashSet = { diff --git a/src/ui/overlay.rs b/src/ui/overlay.rs index fbf49514c..aafd36b3d 100644 --- a/src/ui/overlay.rs +++ b/src/ui/overlay.rs @@ -1,7 +1,6 @@ //! Viewport overlay widgets. use std::cell::RefCell; -use std::collections::HashMap; use glam::{Mat4, Vec3}; use iced::mouse; @@ -22,51 +21,18 @@ pub const CROSSHAIR_ARM: f32 = 60.0; const DEFAULT_CURSOR_SIZE: i32 = 5; const DEFAULT_PICK_BOX: i32 = 3; const DEFAULT_PICK_APERTURE: f32 = 8.0; -const CONSTRAINT_GLYPH_SIZE: f32 = 14.0; -const CONSTRAINT_GLYPH_PAD_X: f32 = 7.0; -const CONSTRAINT_GLYPH_PAD_Y: f32 = 4.0; -const CONSTRAINT_GLYPH_GAP: f32 = 6.0; -const CONSTRAINT_GLYPH_ROW_GAP: f32 = 4.0; const CONSTRAINT_HOVER_MARKER_RADIUS: f32 = 7.0; -const COINCIDENT_GLYPH_SIZE: f32 = 9.0; -fn is_compact_coincident_glyph(label: &str) -> bool { - matches!(label, "≡" | "∈") -} - -/// Fixed's two padlock labels (`parametric_constraints::fixed_glyph_label`). -fn is_fixed_glyph(label: &str) -> bool { - label == "F" || label == crate::scene::parametric_constraints::FIXED_POINT_GLYPH -} +use crate::scene::parametric_constraints::{ + glyph_hit_test_entries, glyph_is_compact, glyph_is_fixed, glyph_is_vertical, + GlyphEntry, GLYPH_SIZE, +}; /// The lock mark a dynamic dimension carries (`DYNAMIC_DIMENSION_GLYPH`). fn is_dynamic_dimension_glyph(label: &str) -> bool { label == crate::scene::parametric_constraints::DYNAMIC_DIMENSION_GLYPH } -/// Vertical's two axis-mark labels (`parametric_constraints::vertical_glyph_label`). -fn is_vertical_glyph(label: &str) -> bool { - label == "│" || label == crate::scene::parametric_constraints::VERTICAL_POINTS_GLYPH -} - -fn constraint_glyph_size(label: &str) -> Size { - if is_compact_coincident_glyph(label) { - return Size::new(COINCIDENT_GLYPH_SIZE, COINCIDENT_GLYPH_SIZE); - } - if label == "G²" - || is_fixed_glyph(label) - || is_vertical_glyph(label) - || is_dynamic_dimension_glyph(label) - { - let side = CONSTRAINT_GLYPH_SIZE + CONSTRAINT_GLYPH_PAD_Y * 2.0; - return Size::new(side, side); - } - let w = label.chars().count() as f32 * CONSTRAINT_GLYPH_SIZE * 0.62 - + CONSTRAINT_GLYPH_PAD_X * 2.0; - let h = CONSTRAINT_GLYPH_SIZE + CONSTRAINT_GLYPH_PAD_Y * 2.0; - Size::new(w, h) -} - fn draw_smooth_constraint_glyph(frame: &mut canvas::Frame, center: Point, color: Color) { let curve = canvas::Path::new(|builder| { for step in 0..=16 { @@ -209,96 +175,6 @@ fn draw_vertical_constraint_glyph( } } -fn constraint_glyph_box( - anchor: Point, - outward: [f32; 2], - label: &str, - tangent_offset: f32, -) -> (Point, Size) { - let size = constraint_glyph_size(label); - let gap = if is_compact_coincident_glyph(label) { - 1.0 - } else { - CONSTRAINT_GLYPH_GAP - }; - let distance = outward[0].abs() * size.width * 0.5 - + outward[1].abs() * size.height * 0.5 - + gap; - let tangent = [-outward[1], outward[0]]; - ( - Point::new( - anchor.x + outward[0] * distance + tangent[0] * tangent_offset - - size.width * 0.5, - anchor.y + outward[1] * distance + tangent[1] * tangent_offset - - size.height * 0.5, - ), - size, - ) -} - -fn constraint_glyph_offsets(glyphs: &[(Point, [f32; 2], String, bool)]) -> Vec { - let mut groups: HashMap<[u32; 4], Vec> = HashMap::new(); - for (index, (anchor, outward, _, _)) in glyphs.iter().enumerate() { - groups - .entry([ - anchor.x.to_bits(), - anchor.y.to_bits(), - outward[0].to_bits(), - outward[1].to_bits(), - ]) - .or_default() - .push(index); - } - - let mut offsets = vec![0.0; glyphs.len()]; - for indices in groups.values().filter(|indices| indices.len() > 1) { - let half_extents: Vec = indices - .iter() - .map(|index| { - let (_, outward, label, _) = &glyphs[*index]; - let size = constraint_glyph_size(label); - let tangent = [-outward[1], outward[0]]; - tangent[0].abs() * size.width * 0.5 - + tangent[1].abs() * size.height * 0.5 - }) - .collect(); - let total = half_extents.iter().sum::() * 2.0 - + CONSTRAINT_GLYPH_ROW_GAP * (indices.len() - 1) as f32; - let mut cursor = -total * 0.5; - for (index, half_extent) in indices.iter().zip(half_extents) { - offsets[*index] = cursor + half_extent; - cursor += half_extent * 2.0 + CONSTRAINT_GLYPH_ROW_GAP; - } - } - offsets -} - -/// Hit-tests screen point `p` against the same glyph-pill layout `draw` -/// renders — reusing `constraint_glyph_offsets`/`constraint_glyph_box` so -/// the clickable area can never drift from what's actually drawn, including -/// the tangential fan-out applied when several glyphs share one anchor. -/// Returns the index into `glyphs` of the topmost (last-drawn) match. -/// `Scene::constraint_glyph_hit` maps the index back to a constraint id. -pub(crate) fn constraint_glyph_hit_test( - glyphs: &[(Point, [f32; 2], String, bool)], - p: Point, -) -> Option { - let offsets = constraint_glyph_offsets(glyphs); - glyphs - .iter() - .zip(offsets) - .enumerate() - .rev() - .find_map(|(index, ((anchor, outward, label, _), tangent_offset))| { - let (top_left, size) = constraint_glyph_box(*anchor, *outward, label, tangent_offset); - let within = p.x >= top_left.x - && p.x <= top_left.x + size.width - && p.y >= top_left.y - && p.y <= top_left.y + size.height; - within.then_some(index) - }) -} - /// Convert CURSORSIZE to a screen-space arm length while keeping the original /// 60 px cursor at the default value and the full-viewport result at 100. pub(crate) fn crosshair_arm_px(bounds: iced::Rectangle, value: i32) -> f32 { @@ -978,7 +854,11 @@ pub fn selection_overlay<'a>( crosshair_bg: [f32; 4], crosshair: CrosshairOptions, selection_visual: SelectionVisualOptions, - constraint_glyphs: Vec<(Point, [f32; 2], String, bool, bool, Vec)>, + constraint_glyphs: Arc<[GlyphEntry]>, + // Per-glyph selection flags, parallel to `constraint_glyphs`, applied + // post-hoc by the view layer — deliberately outside the memoised entries + // so selection changes never invalidate the placement cache. + constraint_glyph_selected: Arc<[bool]>, constraint_glyph_tooltip: Option, constraint_cursor_badge: Option, ) -> Element<'a, Message> { @@ -1005,6 +885,7 @@ pub fn selection_overlay<'a>( crosshair, selection_visual, constraint_glyphs, + constraint_glyph_selected, constraint_glyph_tooltip, constraint_cursor_badge, }) @@ -1066,10 +947,14 @@ struct SelectionCanvas { crosshair_bg: [f32; 4], crosshair: CrosshairOptions, selection_visual: SelectionVisualOptions, - /// Constraint glyph anchor, outward screen direction, label, conflict - /// state, whether the pill itself is the current click-to-select target, - /// and the points to mark while the pill is hovered. - constraint_glyphs: Vec<(Point, [f32; 2], String, bool, bool, Vec)>, + /// Memoised constraint-glyph entries (precomputed anchor/outward/label + + /// layout) shared with the hit-test and tooltip paths — draw reads + /// `top_left`/`size` directly and never recomputes offsets or clones + /// labels. + constraint_glyphs: Arc<[GlyphEntry]>, + /// Per-glyph selection flags, parallel to `constraint_glyphs`, applied + /// post-hoc by the view layer (outside the cache). + constraint_glyph_selected: Arc<[bool]>, /// Localized kind name made visible after the app-level hover dwell. constraint_glyph_tooltip: Option, /// Symbol shown beside the cursor while it targets an entity that already @@ -1257,14 +1142,7 @@ impl canvas::Program for SelectionCanvas { } } if let Some(pos) = cursor.position_in(bounds) { - let glyphs: Vec<_> = self - .constraint_glyphs - .iter() - .map(|(anchor, outward, label, conflict, _, _)| { - (*anchor, *outward, label.clone(), *conflict) - }) - .collect(); - if constraint_glyph_hit_test(&glyphs, pos).is_some() { + if glyph_hit_test_entries(&self.constraint_glyphs, pos).is_some() { return mouse::Interaction::Pointer; } } @@ -2066,22 +1944,13 @@ impl canvas::Program for SelectionCanvas { frame.stroke(&b1, stroke.clone()); frame.stroke(&b2, stroke); } - // Hit testing shares this layout math with the scene projection. + // Draw + hover + tooltip all read the memoised entries' precomputed + // `top_left`/`size` (one shared `hovered` lookup below) — no offset + // recompute, no label clones on this path. if !self.constraint_glyphs.is_empty() { - // `constraint_glyph_offsets` only needs the anchor/outward/label/ - // conflict quadruple it was written against; project away the - // trailing display fields rather than widen its signature. - let glyphs_for_offsets: Vec<(Point, [f32; 2], String, bool)> = self - .constraint_glyphs - .iter() - .map(|(anchor, outward, label, is_conflicting, _, _)| { - (*anchor, *outward, label.clone(), *is_conflicting) - }) - .collect(); - let offsets = constraint_glyph_offsets(&glyphs_for_offsets); let hovered = cursor .position_in(bounds) - .and_then(|point| constraint_glyph_hit_test(&glyphs_for_offsets, point)); + .and_then(|point| glyph_hit_test_entries(&self.constraint_glyphs, point)); let normal_bg = Color::from_rgb8(103, 109, 118); let normal_fg = Color::WHITE; // A redundant or conflicting constraint gets the danger palette @@ -2092,22 +1961,24 @@ impl canvas::Program for SelectionCanvas { let conflict_fg = theme.palette().danger.base.text; let selected_ring = theme.palette().primary.strong.color; let coincident_bg = Color::from_rgb8(35, 145, 230); - for ((anchor, outward, label, is_conflicting, is_selected, _), tangent_offset) in - self.constraint_glyphs.iter().zip(offsets.iter().copied()) + for (entry, is_selected) in self + .constraint_glyphs + .iter() + .zip(self.constraint_glyph_selected.iter()) { - if !anchor.x.is_finite() || !anchor.y.is_finite() { + let label: &str = &entry.label; + if !entry.anchor.x.is_finite() || !entry.anchor.y.is_finite() { continue; } - let compact_coincident = is_compact_coincident_glyph(label); - let (bg, fg) = if *is_conflicting { + let compact_coincident = glyph_is_compact(label); + let (bg, fg) = if entry.conflicting { (conflict_bg, conflict_fg) } else if compact_coincident { (coincident_bg, normal_fg) } else { (normal_bg, normal_fg) }; - let (top_left, size) = - constraint_glyph_box(*anchor, *outward, label, tangent_offset); + let (top_left, size) = (entry.top_left, entry.size); let pill = canvas::Path::rounded_rectangle( top_left, size, @@ -2141,7 +2012,7 @@ impl canvas::Program for SelectionCanvas { draw_concentric_constraint_glyph(&mut frame, glyph_center, fg); } else if is_dynamic_dimension_glyph(label) { draw_fixed_constraint_glyph(&mut frame, glyph_center, fg, bg, false); - } else if is_fixed_glyph(label) { + } else if glyph_is_fixed(label) { let point = label == crate::scene::parametric_constraints::FIXED_POINT_GLYPH; // The object-mode lock reads red, the point-mode one white. @@ -2151,7 +2022,7 @@ impl canvas::Program for SelectionCanvas { Color::from_rgb8(214, 76, 76) }; draw_fixed_constraint_glyph(&mut frame, glyph_center, lock, bg, point); - } else if is_vertical_glyph(label) { + } else if glyph_is_vertical(label) { draw_vertical_constraint_glyph( &mut frame, glyph_center, @@ -2166,10 +2037,10 @@ impl canvas::Program for SelectionCanvas { fg }; frame.fill_text(canvas::Text { - content: label.clone(), + content: label.to_string(), position: glyph_center, color, - size: iced::Pixels(CONSTRAINT_GLYPH_SIZE), + size: iced::Pixels(GLYPH_SIZE), align_x: iced::alignment::Horizontal::Center.into(), align_y: iced::alignment::Vertical::Center, shaping: iced::advanced::text::Shaping::Advanced, @@ -2181,7 +2052,7 @@ impl canvas::Program for SelectionCanvas { if let Some(index) = hovered { let red = Color::from_rgb(1.0, 0.0, 0.0); let stroke = canvas::Stroke::default().with_color(red).with_width(1.5); - for point in &self.constraint_glyphs[index].5 { + for point in self.constraint_glyphs[index].hover.iter() { let first = canvas::Path::line( Point::new( point.x - CONSTRAINT_HOVER_MARKER_RADIUS, @@ -2206,11 +2077,9 @@ impl canvas::Program for SelectionCanvas { frame.stroke(&second, stroke.clone()); } if let Some(label) = &self.constraint_glyph_tooltip { - let (glyph_top_left, glyph_size) = constraint_glyph_box( - self.constraint_glyphs[index].0, - self.constraint_glyphs[index].1, - &self.constraint_glyphs[index].2, - offsets[index], + let (glyph_top_left, glyph_size) = ( + self.constraint_glyphs[index].top_left, + self.constraint_glyphs[index].size, ); let width = (label.chars().count() as f32 * 7.0 + 14.0).max(54.0); let height = 24.0; @@ -2229,8 +2098,8 @@ impl canvas::Program for SelectionCanvas { .with_color(theme.palette().background.strong.text) .with_width(1.0), ); - frame.fill_text(canvas::Text { - content: label.clone(), + frame.fill_text(canvas::Text { + content: label.to_string(), position: Point::new(left + width * 0.5, top + height * 0.5), color: theme.palette().background.strong.text, size: iced::Pixels(12.0), @@ -3912,41 +3781,70 @@ mod clip_tests { #[cfg(test)] mod constraint_glyph_tests { use super::*; + use crate::scene::parametric_constraints::{ + constraint_glyph_box, constraint_glyph_offsets, + }; + use std::sync::Arc; + + /// Build cache-shaped entries the same way + /// `Scene::cached_glyph_placements` does: one offsets pass, then size + + /// box per glyph. Keeps this test on the single source of truth instead + /// of a local layout copy. + fn entries_for(glyphs: &[(Point, [f32; 2], &str)]) -> Arc<[GlyphEntry]> { + let offsets = constraint_glyph_offsets(glyphs); + Arc::from( + glyphs + .iter() + .zip(offsets) + .enumerate() + .map(|(i, ((anchor, outward, label), tangent_dx))| { + let size = + crate::scene::parametric_constraints::constraint_glyph_size(label); + let (top_left, _) = + constraint_glyph_box(*anchor, *outward, label, tangent_dx); + GlyphEntry { + id: i as u32, + anchor: *anchor, + outward: *outward, + label: Arc::from(*label), + conflicting: false, + hover: Arc::from([]), + size, + tangent_dx, + top_left, + } + }) + .collect::>(), + ) + } #[test] fn enlarged_glyph_box_stays_clear_of_its_geometry_anchor() { let anchor = Point::new(100.0, 80.0); let (right, size) = constraint_glyph_box(anchor, [1.0, 0.0], "⊥", 0.0); assert_eq!(size.height, 22.0); - assert!((right.x - anchor.x - CONSTRAINT_GLYPH_GAP).abs() < 1e-6); + assert!((right.x - anchor.x - 6.0).abs() < 1e-6); let (above, size) = constraint_glyph_box(anchor, [0.0, -1.0], "↔ 25.00", 0.0); - assert!((anchor.y - (above.y + size.height) - CONSTRAINT_GLYPH_GAP).abs() < 1e-6); + assert!((anchor.y - (above.y + size.height) - 6.0).abs() < 1e-6); } #[test] fn coincident_glyphs_are_arranged_side_by_side() { let anchor = Point::new(100.0, 80.0); - let glyphs = vec![ - (anchor, [0.0, -1.0], "—".to_string(), false), - (anchor, [0.0, -1.0], "∥".to_string(), false), - ]; - let offsets = constraint_glyph_offsets(&glyphs); - let (left, left_size) = - constraint_glyph_box(anchor, glyphs[0].1, &glyphs[0].2, offsets[0]); - let (right, right_size) = - constraint_glyph_box(anchor, glyphs[1].1, &glyphs[1].2, offsets[1]); + let entries = entries_for(&[ + (anchor, [0.0, -1.0], "—"), + (anchor, [0.0, -1.0], "∥"), + ]); + let (left, left_size) = (entries[0].top_left, entries[0].size); + let (right, right_size) = (entries[1].top_left, entries[1].size); assert!((left.y - right.y).abs() < 1e-6); - assert!( - (right.x - (left.x + left_size.width) - CONSTRAINT_GLYPH_ROW_GAP).abs() < 1e-4 - ); - assert!( - (anchor.y - (left.y + right_size.height) - CONSTRAINT_GLYPH_GAP).abs() < 1e-6 - ); + assert!((right.x - (left.x + left_size.width) - 4.0).abs() < 1e-4); + assert!((anchor.y - (left.y + right_size.height) - 6.0).abs() < 1e-6); let click = Point::new(left.x + left_size.width * 0.5, left.y + left_size.height * 0.5); - assert_eq!(constraint_glyph_hit_test(&glyphs, click), Some(0)); + assert_eq!(glyph_hit_test_entries(&entries, click), Some(0)); } }