From 8016b79ddf26b50a2142f76ef5095de4435e3d52 Mon Sep 17 00:00:00 2001 From: Hakan Seven Date: Sun, 9 Aug 2026 23:55:06 +0300 Subject: [PATCH] fix(snap): align extension tracking Require OTRACK crossings to include the active Extension ray and resolve Shift constraints consistently across preview, grips, and clicks.\n\nRefs #716 --- src/app/update/viewport.rs | 373 ++++++++++++++++++------------------- src/snap.rs | 93 ++++----- 2 files changed, 230 insertions(+), 236 deletions(-) diff --git a/src/app/update/viewport.rs b/src/app/update/viewport.rs index 49e55f302..e97172ddb 100644 --- a/src/app/update/viewport.rs +++ b/src/app/update/viewport.rs @@ -157,6 +157,68 @@ impl OpenCADStudio { Some((base, target)) } + fn active_axis_lock( + &mut self, + tab: usize, + cursor: glam::DVec3, + base: glam::DVec3, + allowed: bool, + ) -> Option { + if self.shift_down && allowed { + if self.axis_lock_dir.is_none() { + let ucs = self.tabs[tab].ucs_xform(); + self.axis_lock_dir = axis_lock_capture( + cursor, + base, + self.polar_mode, + self.polar_increment_deg, + &ucs, + self.isometric_drafting, + self.iso_plane, + self.snap_angle_deg, + ); + } + } else { + self.axis_lock_dir = None; + } + self.axis_lock_dir + } + + fn active_otrack_hit( + &self, + tab: usize, + cursor: glam::DVec3, + snap: Option, + base: Option, + drafting: bool, + view_rot: glam::Mat4, + eye: glam::DVec3, + bounds: iced::Rectangle, + ) -> Option { + if snap.is_some_and(|hit| hit.snap_type != crate::snap::SnapType::Extension) { + return None; + } + let required_crossing_ray = match snap { + Some(extension) => Some((extension.extension_origin?, extension.extension_dir?)), + None => None, + }; + let step = (self.polar_mode && drafting).then_some(self.polar_increment_deg); + let (_, (ucs_x, ucs_y, _)) = self.drafting_grid_basis(tab); + let hit = self.snapper.otrack_snap( + cursor, + view_rot, + eye, + bounds, + step, + base, + required_crossing_ray, + self.ortho_mode && drafting, + ucs_x.as_dvec3(), + ucs_y.as_dvec3(), + )?; + Some(hit) + } + /// Write PDSIZE from the dialog buffer with the current relative/absolute /// sign. A relative size is stored negative; absolute positive. Switching to /// absolute with an empty/zero size seeds a positive value from the current @@ -1094,47 +1156,36 @@ impl OpenCADStudio { self.tabs[i].snap_result = snap_hit; - // Grip edits use the same tracking-point acquisition and OTRACK - // resolution as normal interactive point commands. - let otrack_hit = { - self.snapper.update_otrack_dwell( + self.snapper.update_otrack_dwell( + snap_hit, + &snap_candidates, + view_rot, + eye, + bounds, + Instant::now(), + ); + let axis_lock = self.active_axis_lock(i, raw, base, true); + let otrack_hit = if axis_lock.is_none() { + self.active_otrack_hit( + i, + raw, snap_hit, - &snap_candidates, + Some(base), + true, view_rot, eye, bounds, - Instant::now(), - ); - - if snap_hit.is_none() { - let polar_step = if self.polar_mode { - Some(self.polar_increment_deg) - } else { - None - }; - - let (_, (ucs_x, ucs_y, _)) = self.drafting_grid_basis(i); - - self.snapper.otrack_snap( - raw, - view_rot, - eye, - bounds, - polar_step, - Some(grip.origin_world), - self.ortho_mode, - ucs_x.as_dvec3(), - ucs_y.as_dvec3(), - ) - } else { - None - } + ) + } else { + None }; self.otrack_active = otrack_hit.map(|hit| (hit.base, hit.dir)); self.otrack_kind = otrack_hit.map(|hit| hit.kind); - let mut snapped = if let Some(hit) = otrack_hit { + let mut snapped = if let Some(dir) = axis_lock { + axis_lock_apply(snap_hit.map(|hit| hit.world).unwrap_or(raw), base, dir) + } else if let Some(hit) = otrack_hit { hit.aligned } else { snap_hit.map(|s| s.world).unwrap_or(raw) @@ -1144,29 +1195,8 @@ impl OpenCADStudio { s.screen.y += tile_b.y; } - // Hard axis lock (#312) works for grip drags too: Shift - // pins the drag to the nearest polar/ortho ray from the - // grip's origin, osnap hits included. - if self.shift_down { - if self.axis_lock_dir.is_none() { - let ucs_xf = self.tabs[i].ucs_xform(); - self.axis_lock_dir = axis_lock_capture( - raw, - grip.origin_world, - self.polar_mode, - self.polar_increment_deg, - &ucs_xf, - self.isometric_drafting, - self.iso_plane, - self.snap_angle_deg, - ); - } - } else { - self.axis_lock_dir = None; - } - if let Some(dir) = self.axis_lock_dir { - snapped = axis_lock_apply(snapped, grip.origin_world, dir); - } else if otrack_hit.is_none() + if axis_lock.is_none() + && otrack_hit.is_none() && !snap_hit.is_some_and(|s| s.snap_type != crate::snap::SnapType::Grid) { let base = grip.origin_world; @@ -1537,56 +1567,51 @@ impl OpenCADStudio { ) }; - // Object Snap Tracking: update dwell, then align the cursor - // to a tracking ray (and store the alignment so a typed - // distance can place a point along it — issue #69). - let otrack_hit = { - self.snapper.update_otrack_dwell( + let wants_point = self.tabs[i].active_cmd.as_ref().is_some_and(|command| { + !command.needs_entity_pick() + && !command.needs_tangent_pick() + && !command.is_selection_gathering() + }); + let axis_lock = if let Some(base) = self.last_point { + self.active_axis_lock( + i, + cursor_world, + base, + wants_point && !is_window_corner, + ) + } else { + self.axis_lock_dir = None; + None + }; + self.snapper.update_otrack_dwell( + self.tabs[i].snap_result, + &snap_candidates, + view_rot, + eye, + bounds, + Instant::now(), + ); + self.snapper.update_parallel( + cursor_world.as_vec3(), + &snap_candidates, + view_rot, + eye, + bounds, + Instant::now(), + ); + let otrack_hit = if axis_lock.is_none() { + self.active_otrack_hit( + i, + cursor_world, self.tabs[i].snap_result, - &snap_candidates, + self.last_point, + !is_window_corner, view_rot, eye, bounds, - Instant::now(), - ); - // Parallel snap: acquire the reference line under the - // cursor (independent of OTRACK). (#277) - self.snapper.update_parallel( - cursor_world.as_vec3(), - &snap_candidates, - view_rot, - eye, - bounds, - Instant::now(), - ); - if self.tabs[i].snap_result.is_none() - || self.tabs[i] - .snap_result - .is_some_and(|s| s.snap_type == crate::snap::SnapType::Extension) - { - // A window corner is a free point, so neither the - // Polar increment nor the Ortho lock may pull the - // OTRACK alignment onto an axis (#291). - let step = if self.polar_mode && !is_window_corner { - Some(self.polar_increment_deg) - } else { - None - }; - let (_, (ucs_x, ucs_y, _)) = self.drafting_grid_basis(i); - self.snapper.otrack_snap( - cursor_world, - view_rot, - eye, - bounds, - step, - self.last_point, - self.ortho_mode && !is_window_corner, - ucs_x.as_dvec3(), - ucs_y.as_dvec3(), - ) - } else { - None - } + ) + } else { + None }; self.otrack_active = otrack_hit.map(|h| (h.base, h.dir)); self.otrack_kind = otrack_hit.map(|h| h.kind); @@ -1595,7 +1620,10 @@ impl OpenCADStudio { // the point onto the line through last_point parallel to the // acquired reference, and drive the alignment guide off it. // (#277) - if self.tabs[i].snap_result.is_none() && otrack_hit.is_none() { + if axis_lock.is_none() + && self.tabs[i].snap_result.is_none() + && otrack_hit.is_none() + { if let Some(par) = self.snapper.parallel_snap( cursor_world.as_vec3(), self.last_point.map(|p| p.as_vec3()), @@ -1613,9 +1641,15 @@ impl OpenCADStudio { } let effective = { - let mut pt: glam::DVec3 = if let Some(h) = otrack_hit { - // Tracking alignment wins over the free cursor; - // ortho/polar don't re-constrain it. + let mut pt: glam::DVec3 = if let (Some(dir), Some(base)) = + (axis_lock, self.last_point) + { + let point = self.tabs[i] + .snap_result + .map(|snap| snap.world) + .unwrap_or(cursor_world); + axis_lock_apply(point, base, dir) + } else if let Some(h) = otrack_hit { h.aligned } else { // Snap runs in model space (viewport camera or the @@ -1624,78 +1658,36 @@ impl OpenCADStudio { .snap_result .map(|s| s.world) .unwrap_or(cursor_world); - // Hard axis lock (#312): Shift during a rubber-band - // pick captures the nearest polar/ortho direction - // and pins the pick to that ray — even an osnap hit - // contributes only its along-axis component. - // Only while an active command is asking for a - // POINT — Shift has other meanings during entity - // picks (TRIM/EXTEND swap) and selection. - let wants_point = self.tabs[i].active_cmd.as_ref().is_some_and(|c| { - !c.needs_entity_pick() - && !c.needs_tangent_pick() - && !c.is_selection_gathering() - }); - if let Some(base) = self.last_point { - if self.shift_down && wants_point && !is_window_corner { - if self.axis_lock_dir.is_none() { - let ucs_xf = self.tabs[i].ucs_xform(); - self.axis_lock_dir = axis_lock_capture( - cursor_world, + let osnap_locked = self.tabs[i] + .snap_result + .is_some_and(|s| s.snap_type != crate::snap::SnapType::Grid); + if !osnap_locked && !is_window_corner { + if let Some(base) = self.last_point { + let ucs_xf = self.tabs[i].ucs_xform(); + if self.ortho_mode { + pt = drafting_constrain( + pt, base, - self.polar_mode, - self.polar_increment_deg, &ucs_xf, self.isometric_drafting, self.iso_plane, self.snap_angle_deg, ); - } - } else if !self.shift_down { - self.axis_lock_dir = None; - } - } else if !self.shift_down { - self.axis_lock_dir = None; - } - if let (Some(dir), Some(base)) = (self.axis_lock_dir, self.last_point) { - pt = axis_lock_apply(pt, base, dir); - pt - } else { - // Object snap wins over ortho/polar: a snapped point - // is taken as-is so it isn't pulled onto the ortho - // axis. Grid snap is positional, not an object snap, - // so it still combines with ortho/polar. (#132) - let osnap_locked = self.tabs[i] - .snap_result - .is_some_and(|s| s.snap_type != crate::snap::SnapType::Grid); - if !osnap_locked && !is_window_corner { - if let Some(base) = self.last_point { - let ucs_xf = self.tabs[i].ucs_xform(); - if self.ortho_mode { - pt = drafting_constrain( - pt, - base, - &ucs_xf, - self.isometric_drafting, - self.iso_plane, - self.snap_angle_deg, - ); - } else if self.polar_mode { - pt = polar_constrain_near( - pt, - base, - self.polar_increment_deg, - view_rot, - eye, - bounds, - self.snapper.osnap_radius_px, - &ucs_xf, - ); - } + } else if self.polar_mode { + pt = polar_constrain_near( + pt, + base, + self.polar_increment_deg, + view_rot, + eye, + bounds, + self.snapper.osnap_radius_px, + &ucs_xf, + ); } } - pt } + pt }; // Clamp to world XY only when no UCS is active; with a // UCS the point already lies on the UCS XY plane. @@ -1798,6 +1790,7 @@ impl OpenCADStudio { extension_base: None, extension_base2: None, extension_origin: None, + extension_dir: None, }); if let Some(cmd) = self.tabs[i].active_cmd.as_mut() { cmd.set_acquisition_hint(Some(pick.label)); @@ -2889,39 +2882,33 @@ impl OpenCADStudio { if self.tabs[i].active_ucs.is_none() { pt.z = 0.0; } - let mut world_pt_locked = false; - // Hard axis lock (#312): the held Shift lock projects - // the committed point — osnap hits included — onto the - // locked ray, matching the preview exactly. - if let (Some(dir), Some(base)) = (self.axis_lock_dir, self.last_point) { + let axis_lock = if let Some(base) = self.last_point { + self.active_axis_lock( + i, + raw, + base, + !needs_entity_click && !needs_tan && !is_window_corner, + ) + } else { + self.axis_lock_dir = None; + None + }; + if let (Some(dir), Some(base)) = (axis_lock, self.last_point) { pt = axis_lock_apply(pt, base, dir); if self.tabs[i].active_ucs.is_none() { pt.z = 0.0; } - world_pt_locked = true; } - // OTRACK alignment wins over ortho/polar; otherwise apply - // ortho/polar relative to the last point. - let otrack = if !world_pt_locked - && (snap_hit.is_none() - || snap_hit.is_some_and(|s| s.snap_type == crate::snap::SnapType::Extension)) - { - let step = if self.polar_mode && !is_window_corner { - Some(self.polar_increment_deg) - } else { - None - }; - let (_, (ucs_x, ucs_y, _)) = self.drafting_grid_basis(i); - self.snapper.otrack_snap( + let otrack = if axis_lock.is_none() { + self.active_otrack_hit( + i, raw, + snap_hit, + self.last_point, + !is_window_corner, view_rot, eye, bounds, - step, - self.last_point, - self.ortho_mode && !is_window_corner, - ucs_x.as_dvec3(), - ucs_y.as_dvec3(), ) } else { None @@ -2931,7 +2918,7 @@ impl OpenCADStudio { if self.tabs[i].active_ucs.is_none() { pt.z = 0.0; } - } else if !world_pt_locked + } else if axis_lock.is_none() && !is_window_corner && !snap_hit.is_some_and(|s| s.snap_type != crate::snap::SnapType::Grid) { diff --git a/src/snap.rs b/src/snap.rs index 35b5a735f..f6a010281 100644 --- a/src/snap.rs +++ b/src/snap.rs @@ -80,12 +80,10 @@ pub struct SnapResult { /// overlay draws a dashed guide from each base to the crossing so both /// contributing extensions stay visible. `None` otherwise. (#247, #259) pub extension_base2: Option, - /// World-space twin of `extension_base`: the acquired endpoint the guide is - /// drawn from. Typed-distance entry measures along this ray, so taking it - /// from the same decision that drew the guide keeps the two from - /// disagreeing — the guide showing while typing does nothing, or the - /// distance running off along a ray the user cannot see. + /// Acquired endpoint used by the Extension guide and typed distance. pub extension_origin: Option, + /// Source direction paired with `extension_origin`. + pub extension_dir: Option, } /// Object-snap-tracking alignment: the cursor projected onto a ray from an @@ -280,9 +278,7 @@ impl Snapper { self.enabled.clear(); } - /// Update dwell tracking and possibly acquire a new OST point. - /// Should be called on every ViewportMove when snap is active. - /// `snap_world` is the current snap result world point (if any). + /// Update dwell tracking from the current snap result. pub fn update_otrack_dwell( &mut self, snap: Option, @@ -292,20 +288,14 @@ impl Snapper { bounds: iced::Rectangle, now: Instant, ) { - // Temporary tracking points are acquired when OTRACK is on, OR when the - // Extension object snap is on: Extension tracks a segment's line only - // from an endpoint the user has acquired, independently of OTRACK's - // on/off state (#262). + // Extension uses acquired points even when OTRACK is off. (#262) if !self.tracking_active() { self.last_snap_world = None; self.dwell_since = None; self.dwell_acquired = false; return; } - // Only stable geometric snap points may be acquired automatically for - // OTRACK. Continuous/contextual snaps such as Nearest, Perpendicular, - // Tangent and Extension can move along geometry and would otherwise - // create arbitrary tracking points while the cursor is hovering. (#716) + // Acquire only stable geometric snap points for OTRACK. (#716) let snap_world = snap.and_then(|hit| { matches!( hit.snap_type, @@ -459,21 +449,7 @@ impl Snapper { } } - /// Project the cursor onto a tracking ray emanating from one of the - /// acquired tracking points, in the XY plane. Without `polar_step_deg` the - /// rays are horizontal / vertical (0° / 90°); with it, every polar - /// increment is a candidate so the user can track along POLAR angles. Each - /// acquired corner also contributes a ray along its own edge directions, so - /// pulling the cursor along a segment's extension locks to that line (#219). - /// - /// When the cursor sits near the crossing of two active vectors from - /// different origins the intersection point wins, so the cursor locks onto - /// the exact crossing rather than a free point along one vector: - /// * two OTRACK vectors from different tracking points (#112), and - /// * a POLAR vector from `last_point` crossing an OTRACK vector (#111). - /// - /// Returns the aligned point, the unit ray direction (pointing toward the - /// cursor side, used for typed-distance entry), and the originating point. + /// Project the cursor onto acquired tracking rays and eligible crossings. pub fn otrack_snap( &self, cursor_world: DVec3, @@ -482,6 +458,7 @@ impl Snapper { bounds: iced::Rectangle, polar_step_deg: Option, last_point: Option, + required_crossing_ray: Option<(DVec3, DVec3)>, // Ortho on: the axis from `last_point` is a hard lock. Only crossings of // an acquired ray with that axis lock; single tracking rays are // suppressed so the cursor can't leave the ortho axis. (#218) @@ -640,6 +617,12 @@ impl Snapper { if rays[i].group == rays[j].group { continue; } + if required_crossing_ray.is_some_and(|(origin, dir)| { + !((rays[i].origin == origin && rays[i].dir == dir) + || (rays[j].origin == origin && rays[j].dir == dir)) + }) { + continue; + } // Under an ortho lock only crossings that involve the ortho axis // are valid — every other crossing lies off it. (#218) if ortho_lock && rays[i].group != ORTHO_GROUP && rays[j].group != ORTHO_GROUP { @@ -650,6 +633,13 @@ impl Snapper { else { continue; }; + if required_crossing_ray.is_some_and(|(origin, dir)| { + let len2 = dir.x * dir.x + dir.y * dir.y; + let t = ((x.x - origin.x) * dir.x + (x.y - origin.y) * dir.y) / len2; + t < 0.05 + }) { + continue; + } let sd = screen_dist(x); if sd < r && best_x.as_ref().map_or(true, |(bd, _)| sd < *bd) { // Report an acquired tracking ray (not an auxiliary @@ -676,6 +666,9 @@ impl Snapper { if let Some((_, h)) = best_x { return Some(h); } + if required_crossing_ray.is_some() { + return None; + } // With Ortho on and a base point, the axis is a hard lock: no single // tracking ray may pull the cursor off it. Only crossings with the @@ -821,6 +814,7 @@ impl Snapper { extension_base: None, extension_base2: None, extension_origin: None, + extension_dir: None, }) } @@ -952,6 +946,7 @@ impl Snapper { extension_base: None, extension_base2: None, extension_origin: None, + extension_dir: None, }); best_rank = snap_tier(SnapType::Grid); best_sub = snap_priority(SnapType::Grid); @@ -1051,6 +1046,7 @@ impl Snapper { extension_base: None, extension_base2: None, extension_origin: None, + extension_dir: None, }); } }; @@ -1204,11 +1200,7 @@ impl Snapper { } // ── Perpendicular — foot of perpendicular from the drawing base ── - // A perpendicular snap requires a real drawing base. Without one - // (for example while picking the first point of LINE) there is no - // meaningful perpendicular direction, so do not offer a candidate. - // Falling back to the cursor here would make Perpendicular behave - // like Nearest. (#716) + // Perpendicular requires a drawing base; cursor fallback acts like Nearest. (#716) if self.is_on(SnapType::Perpendicular) { if let Some(q) = self.from_point.map(|v| v.as_dvec3()) { if let Some(segments) = &local_segments { @@ -1663,6 +1655,7 @@ impl Snapper { extension_base: None, extension_base2: None, extension_origin: None, + extension_dir: None, }); } } @@ -1712,6 +1705,7 @@ impl Snapper { extension_base: None, extension_base2: None, extension_origin: None, + extension_dir: None, }); } }; @@ -1769,7 +1763,7 @@ impl Snapper { // intersection yields none, so its guides simply don't draw. (#238, #247, #259) if let Some(b) = best.as_mut() { if matches!(b.snap_type, SnapType::Extension | SnapType::Intersection) { - let (b1, b2, origin) = extension_bases_screen( + let (b1, b2, ray) = extension_bases_screen( b.world, &self.tracking_points, &self.tracking_dirs, @@ -1779,7 +1773,8 @@ impl Snapper { ); b.extension_base = b1; b.extension_base2 = b2; - b.extension_origin = origin; + b.extension_origin = ray.map(|(origin, _)| origin); + b.extension_dir = ray.map(|(_, dir)| dir); } } @@ -2277,12 +2272,16 @@ fn extension_bases_screen( view_rot: Mat4, eye: glam::DVec3, bounds: Rectangle, -) -> (Option, Option, Option) { +) -> ( + Option, + Option, + Option<(glam::DVec3, glam::DVec3)>, +) { let snapped_screen = world_to_screen(snapped, view_rot, eye, bounds); // Collect qualifying endpoints, off-ray distance measured in screen space so // the tolerance stays scale-independent at UTM coordinates (a world² test // would reject the crossing base once coordinates reach ~1e7). - let mut found: Vec<(f32, glam::DVec3, Point)> = Vec::new(); + let mut found: Vec<(f32, glam::DVec3, glam::DVec3, Point)> = Vec::new(); for (&origin, dirs) in tracking_points.iter().zip(tracking_dirs) { for &dir in dirs { let len2 = dir.x * dir.x + dir.y * dir.y; @@ -2297,7 +2296,7 @@ fn extension_bases_screen( let off = dist2(world_to_screen(on, view_rot, eye, bounds), snapped_screen); if off <= 4.0 { let base = world_to_screen(origin, view_rot, eye, bounds); - found.push((off, origin, base)); + found.push((off, origin, dir, base)); } } } @@ -2306,7 +2305,8 @@ fn extension_bases_screen( found.sort_by(|x, y| x.0.partial_cmp(&y.0).unwrap_or(std::cmp::Ordering::Equal)); let mut bases: [Option; 2] = [None, None]; let mut origins: Vec = Vec::new(); - for (_, origin, base) in found { + let mut directions: Vec = Vec::new(); + for (_, origin, dir, base) in found { if origins .iter() .any(|o| (*o - origin).length_squared() < 1e-12) @@ -2314,6 +2314,7 @@ fn extension_bases_screen( continue; } origins.push(origin); + directions.push(dir); if bases[0].is_none() { bases[0] = Some(base); } else { @@ -2323,7 +2324,11 @@ fn extension_bases_screen( } // `origins` is sorted nearest-fit first, so its head is the endpoint the // primary guide is drawn from — the one a typed distance measures along. - (bases[0], bases[1], origins.first().copied()) + ( + bases[0], + bases[1], + origins.first().copied().zip(directions.first().copied()), + ) } // ── Projection helpers ──────────────────────────────────────────────────── @@ -2567,6 +2572,7 @@ mod ext_tests { bounds, None, Some(base), + None, false, DVec3::X, DVec3::Y, @@ -2595,6 +2601,7 @@ mod ext_tests { bounds, None, None, + None, false, DVec3::X, DVec3::Y,