From 853e8301260da7816d2c9bad1cb9c15c5e17f9ab Mon Sep 17 00:00:00 2001 From: Hakan Seven Date: Sun, 30 Aug 2026 22:26:31 +0300 Subject: [PATCH] fix(grips): prevent phantom snapping Keep hidden pre-drag geometry out of snap candidates and avoid empty scene invalidations. Require stable OTRACK dwell observations and cache planar-solid visibility during live previews. Refs #930 --- src/app/command_driver.rs | 1 - src/app/mod.rs | 8 ------ src/app/update/viewport.rs | 33 ++++-------------------- src/scene/pipeline/mod.rs | 3 +++ src/scene/view/render.rs | 20 +++++++++++++-- src/snap.rs | 52 ++++++++++++++++++++++---------------- 6 files changed, 56 insertions(+), 61 deletions(-) diff --git a/src/app/command_driver.rs b/src/app/command_driver.rs index 40eab62e1..4bffb222f 100644 --- a/src/app/command_driver.rs +++ b/src/app/command_driver.rs @@ -232,7 +232,6 @@ impl OpenCADStudio { self.tabs[i].dirty = dirty_before; } - self.grip_snap_wires.clear(); self.grip_text_verts.clear(); self.grip_text_slide = false; self.tabs[i].scene.clear_preview_wire(); diff --git a/src/app/mod.rs b/src/app/mod.rs index 8cf5f8920..91034b4e9 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -583,13 +583,6 @@ pub(super) struct OpenCADStudio { )>, /// Document dirty state before the live grip mutation began. grip_dirty_before: Option, - /// Frozen wire geometry of the entities being grip-edited. - /// - /// The live entities are hidden from the resident hit-test set while their - /// grip preview is active. Keep their pre-drag geometry here so OSNAP, - /// Extension and OTRACK can still reference the entity's own vertices and - /// segments without snapping against the geometry being deformed. - grip_snap_wires: Vec, /// Drag-start snapshot of the dragged entity's SDF glyph quads. A whole- /// entity text move slides these each frame (translating the already-shaped /// glyphs) instead of re-tessellating the run every cursor move (issue #316). @@ -3274,7 +3267,6 @@ impl OpenCADStudio { grip_originals: Vec::new(), grip_history_originals: Vec::new(), grip_dirty_before: None, - grip_snap_wires: Vec::new(), grip_text_verts: Vec::new(), grip_text_slide: false, qselect: None, diff --git a/src/app/update/viewport.rs b/src/app/update/viewport.rs index 34eddd246..55164a91b 100644 --- a/src/app/update/viewport.rs +++ b/src/app/update/viewport.rs @@ -1266,17 +1266,6 @@ impl OpenCADStudio { // the re-wrap is exact). let snap = self.tabs[i].scene.wire_models_for(&edited_handles); - // Keep the entity's original geometry available for self-OSNAP, - // Extension and OTRACK while the live grip preview is being deformed. - self.grip_snap_wires = snap.clone(); - // The user explicitly picked this grip, so treat its original position as an - // acquired tracking point immediately. This lets Extension/OTRACK use the - // original edge direction on the very first movement instead of requiring the - // cursor to return to the old grip position and dwell there first. - self.snapper.acquire_grip_tracking_point( - grip.origin_world, - &self.grip_snap_wires, - ); self.grip_text_verts = snap .iter() .flat_map(|w| w.text_verts.iter().copied()) @@ -1321,19 +1310,6 @@ impl OpenCADStudio { bounds, self.snapper.osnap_radius_px, ); - // `snap_candidates` contains only the spatially-local wires from the rest of - // the drawing. Add the frozen pre-drag geometry of the edited entity so its - // own vertices and segments remain valid snap references. - // - // This intentionally uses a plain Vec: Snapper will use its normal - // unindexed fallback over this already-small local candidate set rather than - // cloning/scanning the whole drawing. - let mut grip_snap_candidates: Vec<_> = - snap_candidates.iter().cloned().collect(); - - grip_snap_candidates.extend( - self.grip_snap_wires.iter().cloned() - ); // The engaged grip is the rubber-band origin. Perpendicular // snapping must drop its foot from this point, including when a // hot-grip set is moved by the same drag vector. @@ -1347,7 +1323,7 @@ impl OpenCADStudio { let snap_hit = self.snapper.snap( raw, p, - &grip_snap_candidates, + &snap_candidates, view_rot, eye, bounds, @@ -1360,7 +1336,7 @@ impl OpenCADStudio { self.snapper.update_otrack_dwell( snap_hit, - &grip_snap_candidates, + &snap_candidates, view_rot, eye, bounds, @@ -1579,7 +1555,9 @@ impl OpenCADStudio { .filter(|handle| self.tabs[i].scene.meshes.contains_key(handle)) .map(|handle| (handle, crate::scene::ChangeKind::Modified)) .collect(); - self.tabs[i].scene.bump_entities(&mesh_changes); + if !mesh_changes.is_empty() { + self.tabs[i].scene.bump_entities(&mesh_changes); + } self.tabs[i].scene.set_preview_hatches(&edited_handles); self.tabs[i].dirty = true; if let Some(active) = self.tabs[i].active_grip.as_mut() { @@ -3032,7 +3010,6 @@ impl OpenCADStudio { ); self.tabs[i].dirty = true; } - self.grip_snap_wires.clear(); self.grip_text_verts = Vec::new(); self.grip_text_slide = false; for &handle in &handles { diff --git a/src/scene/pipeline/mod.rs b/src/scene/pipeline/mod.rs index b6013f888..62cb5cedc 100644 --- a/src/scene/pipeline/mod.rs +++ b/src/scene/pipeline/mod.rs @@ -314,6 +314,8 @@ pub struct Pipeline { pub cached_selection: (u64, u64), /// Face upload key; keeps the resident wire Arc uniquely owned by Scene. pub cached_face3d_key: (u64, bool, bool, u64), + /// Cached planar-solid visibility for `(wire_content_id, view direction)`. + pub cached_solid_visibility: (u64, [u32; 3], u64), /// Handle → indices into the resident wire set, built once per wire upload /// (when `cached_wire_id` changes). Lets the selection/hover xray overlay /// gather just the highlighted entity's wires (`O(highlighted)`) instead of @@ -2320,6 +2322,7 @@ impl Pipeline { cached_wire_id: u64::MAX, cached_selection: (u64::MAX, u64::MAX), cached_face3d_key: (u64::MAX, false, false, u64::MAX), + cached_solid_visibility: (u64::MAX, [u32::MAX; 3], u64::MAX), wire_handle_index: std::sync::Arc::new(rustc_hash::FxHashMap::default()), render_sig: u64::MAX, skip_geometry: false, diff --git a/src/scene/view/render.rs b/src/scene/view/render.rs index 0a4d074cd..c2d8d2f2b 100644 --- a/src/scene/view/render.rs +++ b/src/scene/view/render.rs @@ -381,6 +381,8 @@ impl shader::Primitive for Primitive { inner.cached_selection = (u64::MAX, u64::MAX); inner.cached_mesh_content_id = u64::MAX; inner.cached_face3d_key = (u64::MAX, false, false, u64::MAX); + inner.cached_solid_visibility = + (u64::MAX, [u32::MAX; 3], u64::MAX); inner.cached_hatch_source = None; inner.cached_preview_hatch_source = None; inner.cached_wipeout_source = None; @@ -487,11 +489,25 @@ impl shader::Primitive for Primitive { // the Wireframe overlay style. let face3d_fill_active = fill_mode && !vp.view_wireframe; let solid_fill_active = fill_mode && vp.show_2d_solid_fills; - let solid_visibility_key = + let view_dir_key = [ + vp.view_dir.x.to_bits(), + vp.view_dir.y.to_bits(), + vp.view_dir.z.to_bits(), + ]; + let solid_visibility_key = if !solid_fill_active { + 0 + } else if inner.cached_solid_visibility.0 == vp.wire_content_id + && inner.cached_solid_visibility.1 == view_dir_key + { + inner.cached_solid_visibility.2 + } else { crate::scene::pipeline::face3d_gpu::planar_solid_visibility_key( &vp_wires, vp.view_dir, - ); + ) + }; + inner.cached_solid_visibility = + (vp.wire_content_id, view_dir_key, solid_visibility_key); let fill_changed = inner.cached_fill_mode != fill_mode; let hatch_changed = inner .cached_hatch_source diff --git a/src/snap.rs b/src/snap.rs index 2077e5af1..2da917e6d 100644 --- a/src/snap.rs +++ b/src/snap.rs @@ -142,6 +142,8 @@ pub struct Snapper { pub dwell_since: Option, /// Whether the current dwell already acquired/removed a point (fire once). pub dwell_acquired: bool, + /// Stable observations of the current dwell candidate. + dwell_observations: u8, /// The point the in-progress command is drawing *from* (the rubber-band /// origin), if any. Perpendicular snap drops its foot from here so the new /// segment is genuinely perpendicular to the target — without it, perp @@ -188,6 +190,7 @@ impl Default for Snapper { last_snap_world: None, dwell_since: None, dwell_acquired: false, + dwell_observations: 0, from_point: None, parallel_ref: None, parallel_dwell: None, @@ -293,6 +296,7 @@ impl Snapper { self.last_snap_world = None; self.dwell_since = None; self.dwell_acquired = false; + self.dwell_observations = 0; return; } // Acquire only stable geometric snap points for OTRACK. (#716) @@ -322,15 +326,23 @@ impl Snapper { // does not create accidental tracking points. const DWELL_MS: u128 = 250; const DWELL_PX: f32 = 8.0; + const MIN_OBSERVATIONS: u8 = 3; match snap_world { None => { // Leaving all geometry: capture the point we were dwelling on if // it qualified, before the reset loses it. - self.acquire_on_leave(now, DWELL_MS, wires, endpoints_only); + self.acquire_on_leave( + now, + DWELL_MS, + MIN_OBSERVATIONS, + wires, + endpoints_only, + ); self.last_snap_world = None; self.dwell_since = None; self.dwell_acquired = false; + self.dwell_observations = 0; } Some(p) => { // Convert to screen to measure pixel distance. @@ -344,10 +356,14 @@ impl Snapper { false }; if is_same { + self.dwell_observations = self.dwell_observations.saturating_add(1); let elapsed = self .dwell_since .map_or(0, |t| now.duration_since(t).as_millis()); - if !self.dwell_acquired && elapsed >= DWELL_MS { + if !self.dwell_acquired + && self.dwell_observations >= MIN_OBSERVATIONS + && elapsed >= DWELL_MS + { self.dwell_acquired = true; // Dwelling over an already-acquired point removes it; // otherwise acquire it. @@ -374,32 +390,21 @@ impl Snapper { // Moved to a different snap point: capture the previous one // first if it was dwelt on long enough, so a pause-then-drag // gesture reliably acquires it even without in-place events. - self.acquire_on_leave(now, DWELL_MS, wires, endpoints_only); + self.acquire_on_leave( + now, + DWELL_MS, + MIN_OBSERVATIONS, + wires, + endpoints_only, + ); self.last_snap_world = Some(p); self.dwell_since = Some(now); self.dwell_acquired = false; + self.dwell_observations = 1; } } } } - /// Immediately acquire an explicit grip origin as a tracking point. - /// - /// Unlike normal cursor acquisition, a grip was deliberately selected by the - /// user, so it should not require OTRACK dwell before Extension/OTRACK can use it. - pub fn acquire_grip_tracking_point( - &mut self, - p: DVec3, - wires: &W, - ) { - if !self.tracking_active() { - return; - } - - // With OTRACK disabled, Extension acquisition must remain endpoint-only. - let endpoints_only = !self.otrack_enabled; - - self.acquire_tracking_point(p, wires, endpoints_only); - } /// Add `p` as a tracking point (capturing its corner edge directions) unless /// it is already tracked; drops the oldest when the 4-point cap is reached. /// Edge directions are scanned once here, at acquisition, so OTRACK can align @@ -449,10 +454,11 @@ impl Snapper { &mut self, now: Instant, dwell_ms: u128, + min_observations: u8, wires: &W, endpoints_only: bool, ) { - if self.dwell_acquired { + if self.dwell_acquired || self.dwell_observations < min_observations { return; // already handled by the in-place branch } let Some(prev) = self.last_snap_world else { @@ -732,6 +738,7 @@ impl Snapper { self.last_snap_world = None; self.dwell_since = None; self.dwell_acquired = false; + self.dwell_observations = 0; } /// Parallel snap acquisition. When the Parallel object snap is on, hovering @@ -862,6 +869,7 @@ impl Snapper { last_snap_world: None, dwell_since: None, dwell_acquired: false, + dwell_observations: 0, from_point: None, parallel_ref: None, parallel_dwell: None,