From 6de4ed5a062bea92552b87b9a2c131d8f0f137f4 Mon Sep 17 00:00:00 2001 From: Christopher Overbeck Date: Tue, 15 Sep 2026 15:56:33 -0500 Subject: [PATCH] Support paper centerline dimensions through viewports --- docs/viewport-dimension-creation.svg | 22 ++++ docs/viewport-dimensions.md | 82 +++++++------- src/app/command_driver.rs | 41 ++++--- src/app/commands/mod.rs | 2 +- src/app/dim_viewport.rs | 49 ++++----- src/app/mod.rs | 17 +-- src/app/update/viewport.rs | 56 ++++++---- src/app/update/viewport_snap.rs | 95 +++------------- src/app/viewport_dimension_tests.rs | 100 ++++++++++++++--- src/command.rs | 16 +-- src/entities/dim_override.rs | 12 +-- src/entities/dimension.rs | 16 +-- src/modules/annotate/aligned_dim.rs | 5 +- src/modules/annotate/angular_dim.rs | 28 +++-- src/modules/annotate/diameter_dim.rs | 5 +- src/modules/annotate/linear_dim.rs | 5 +- src/modules/annotate/radius_dim.rs | 5 +- src/scene/mod.rs | 2 +- src/scene/mspace.rs | 65 ++++++----- src/scene/project.rs | 5 +- ...dim_seam.rs => viewport_dimension_pick.rs} | 78 +++----------- src/scene/viewport_ref.rs | 95 ++-------------- src/snap.rs | 102 +++++++++++------- 23 files changed, 421 insertions(+), 482 deletions(-) create mode 100644 docs/viewport-dimension-creation.svg rename src/scene/{viewport_dim_seam.rs => viewport_dimension_pick.rs} (82%) diff --git a/docs/viewport-dimension-creation.svg b/docs/viewport-dimension-creation.svg new file mode 100644 index 000000000..efd1464c1 --- /dev/null +++ b/docs/viewport-dimension-creation.svg @@ -0,0 +1,22 @@ + + + + +Dimension from a paper centerline to a model endpoint + +Viewport 1:10 + + + +Model geometry + + + +50 +Brown dashed line: paper-space centerline +5 paper units between the picked points +5 × 10 = 50 model units +The viewport supplies the length scale. +Either pick order gives the same value. +Measurement schematic; dimension style unit factor = 1. + diff --git a/docs/viewport-dimensions.md b/docs/viewport-dimensions.md index 119b2361a..2af0b7aa4 100644 --- a/docs/viewport-dimensions.md +++ b/docs/viewport-dimensions.md @@ -1,50 +1,50 @@ -# Dimensions through paper-space viewports +# Dimensions through layout viewports -Paper-space point commands can snap to displayed model geometry through a planar, -orthographic layout viewport. The acquired result retains the original model -coordinate, its paper coordinate, the viewport frame, and feature identity. -Ordinary selection continues to operate on the active drawing space. +In paper space, snap to model geometry displayed through a planar, orthographic +viewport. `DIMLINEAR`, `DIMALIGNED`, `DIMANGULAR`, `DIMRADIUS`, and `DIMDIAMETER` +also support their usual object-pick modes through these viewports. -`DIMLINEAR`, `DIMALIGNED`, `DIMANGULAR`, `DIMRADIUS`, and `DIMDIAMETER` use this -acquisition context. The supported object-pick modes also query viewport geometry. -Snapping and object picking share viewport draw order, visibility, and clipping -rules. Object picking uses the resident interaction index and applies block -instance transforms. Nonuniformly transformed circles and circular arcs are -declined as radial object picks because their displayed geometry is elliptical. -Array inserts without an addressable instance index are also declined; resolved -block geometry must remain within the original pick aperture. +## Measurement -## Input and measurement rules +- Model geometry supplies the viewport scale. A 100-unit model length displayed + at 1:10 measures 100, although it occupies 10 paper units. +- A paper-space centerline or free sheet point can be combined with model + geometry through one viewport, in either pick order. The paper-space span is + measured using that viewport's scale. +- Sheet-only dimensions measure paper units, even inside a viewport rectangle. +- Model picks from a second viewport are rejected without advancing the command. + Dimension-line and text placement do not change the measurement scale. +- The current dimension style's unit conversion is preserved. Angular dimensions + do not use length compensation. `DIMASSOC=0` explodes the compensated dimension. -- Free and typed sheet points measure paper units, including points inside a - viewport rectangle. -- Measuring points must all belong to the same space and, when applicable, the - same viewport. An incompatible pick leaves the command at its current step. -- Dimension-line and text placement do not change the measurement's space. -- Paper geometry takes priority over viewport geometry at an object pick. -- The final dimension style is applied before computing measurement scaling. - Length dimensions keep paper-space definition points and a negative `DIMLFAC` - override containing the effective user factor multiplied by `1 / viewport scale`. - Angular measurements do not use this length factor. -- `DIMASSOC=0` uses the same compensated value before exploding the annotation. +Snapping and object picking respect visibility, clipping, and viewport draw order. +Paper geometry has priority in object picks. Ordinary selection uses the active +drawing space. Object picks reuse the resident interaction index and resolve +nested block transforms. -## Persistent measurement data +![Measurement schematic: a five-unit sheet span measures 50 through a 1:10 viewport.](viewport-dimension-creation.svg) -`OCS_VIEWPORT_MEASUREMENT` XData stores three values: version `1` (integer16), -the positive user factor (real), and viewport compensation (real). This keeps the -user's unit conversion separate from viewport scale. The ordinary `DIMLFAC` -override remains sufficient for displaying the saved measurement in other CAD -readers. The data survives DXF/DWG saves and undo/redo. +## Saved scale -The dependent association change uses this metadata when viewport scale changes. -This creation change alone creates nonassociative viewport dimensions and leaves -the drawing's `DIMASSOC` setting unchanged. Direct model/paper associations retain -their existing behavior. +Length dimensions store paper-space definition points and a negative `DIMLFAC` +override: user unit factor multiplied by `1 / viewport scale`. +`OCS_VIEWPORT_MEASUREMENT` XData stores version `1` (integer16), the positive user +factor (real), and viewport compensation (real). Keeping the factors separate +allows compensation to change without losing the user's unit conversion. +Both the override and XData survive DXF/DWG saves and undo/redo. -## Validation +Viewport dimensions are nonassociative: later source or viewport edits do not +update them. The drawing's `DIMASSOC` setting and direct model/paper associations +keep their existing behavior. -`src/app/viewport_dimension_tests.rs` exercises snap-engine acquisition, command -commits for all five commands, object picks, input retries, active positive and -negative style factors, exploded dimensions, undo/redo, DXF/DWG persistence, -clipping, overlapping viewports, pan, scale, and twist. These are headless tests; -they do not establish visual acceptance in the desktop UI. +## Limits + +Perspective and oblique views are unsupported. Object picks decline array +instances without an addressable instance index, nesting beyond eight levels, +and radial measurements of nonuniformly scaled circles or arcs. + +## Tests + +Run `cargo test --locked --lib viewport_dimension`. Tests cover the five commands, +mixed sheet/model input, styles, clipped/overlapping viewports, object picks, +input retries, exploded dimensions, undo/redo, and DXF/DWG persistence. diff --git a/src/app/command_driver.rs b/src/app/command_driver.rs index 6cc436e5f..7aa1f47fb 100644 --- a/src/app/command_driver.rs +++ b/src/app/command_driver.rs @@ -98,7 +98,7 @@ impl OpenCADStudio { self.tabs[i].snap_result = None; self.last_point = None; // Points collected in the space being left are meaningless in the new - // one (PR1). + // one. self.clear_accepted_snaps(); self.pending_click_snap = None; self.snapper.from_point = None; @@ -386,7 +386,7 @@ impl OpenCADStudio { } // Typed / dynamic-input / headless points carry no snap, but the // accepted-snap list must stay index-parallel with the points the - // command collects (PR1). Interactive picks record themselves in + // command collects. Interactive picks record themselves in // the click handler and never reach here. if !self.record_accepted_snap(i, None, None, *point) { return Task::none(); @@ -404,7 +404,9 @@ impl OpenCADStudio { self.push_ucs_to_cmd(i); } if let StepInput::EntityPick(handle, point) = &input { - if !self.dimension_acquisition_allowed(i, None) { return Task::none(); } + if !self.dimension_acquisition_allowed(i, None) { + return Task::none(); + } let solid_pick = matches!( self.tabs[i].scene.document.get_entity(*handle), Some( @@ -477,7 +479,11 @@ impl OpenCADStudio { } let ctrl = self.ctrl_down; let shift = self.shift_down; - let picked_handle = if let StepInput::EntityPick(h, _) = &input { Some(*h) } else { None }; + let picked_handle = if let StepInput::EntityPick(h, _) = &input { + Some(*h) + } else { + None + }; let result: Option = { let Some(cmd) = self.tabs[i].active_cmd.as_mut() else { return Task::none(); @@ -2366,17 +2372,15 @@ impl OpenCADStudio { // through a viewport carries the compensation as a negative // DIMLFAC override. if !preserve_base_style { - crate::scene::creation_style::apply_current_creation_styles(&self.tabs[i].scene.document, &mut entity); + crate::scene::creation_style::apply_current_creation_styles( + &self.tabs[i].scene.document, + &mut entity, + ); } if !self.apply_viewport_dimension_measurement(i, &mut entity) { return Task::none(); } - // ...and is deliberately *not* associated in paper space: its - // definition points are projected paper coordinates, so a - // paper-space association would bind it to geometry that is - // not there. PR3 records the real dimension -> viewport -> - // entity chain instead. One gate, shared with - // `infer_dimension_sources_guarded`. + // Projected points cannot use direct paper-space source inference. let association_allowed = self.dimension_association_allowed(i); let inherited_dimension = if preserve_base_style { match &entity { @@ -2438,20 +2442,15 @@ impl OpenCADStudio { } else { let delta_safe = self.delta_add_safe(i, &entity); let pending = self.begin_undo(i, label, 1, delta_safe); - if let Some(handle) = self.commit_entity_handle_with_dimension_policy( - entity, - preserve_base_style, - ) { + if let Some(handle) = + self.commit_entity_handle_with_dimension_policy(entity, preserve_base_style) + { if association_mode == 2 && association_allowed { - let mut changes = vec![ - (handle, crate::scene::ChangeKind::Modified), - ]; + let mut changes = vec![(handle, crate::scene::ChangeKind::Modified)]; match association { crate::command::DimensionAssociationInput::Infer(source) => { let sources: Vec<_> = source.map_or_else( - || { - self.infer_dimension_sources_guarded(i, handle) - }, + || self.infer_dimension_sources_guarded(i, handle), |source| { if single_source_dimension { vec![Some(source)] diff --git a/src/app/commands/mod.rs b/src/app/commands/mod.rs index a8cf35bc5..7befcd464 100644 --- a/src/app/commands/mod.rs +++ b/src/app/commands/mod.rs @@ -121,7 +121,7 @@ impl OpenCADStudio { // is not constrained by ortho/polar relative to a previous command's endpoint. self.last_point = None; // A new command collects its own points, so the previous command's - // accepted snaps must not leak into it (PR1). + // accepted snaps must not leak into it. self.clear_accepted_snaps(); // Starting a command restarts the right-click cycle, so its first // right-click acts as Enter rather than opening the context menu. diff --git a/src/app/dim_viewport.rs b/src/app/dim_viewport.rs index 577237e57..e7760d0b8 100644 --- a/src/app/dim_viewport.rs +++ b/src/app/dim_viewport.rs @@ -1,5 +1,5 @@ -//! Paper-space dimension acquisition and measurement. Only acquired geometry -//! carries viewport compensation; free and typed sheet coordinates stay on the sheet. +//! Paper-space dimension acquisition and measurement. Acquiring model geometry +//! supplies a viewport scale; sheet-only dimensions retain paper units. use super::OpenCADStudio; use crate::command::DimensionAssociationSource; @@ -14,7 +14,7 @@ use glam::DVec3; pub(crate) enum DimensionMeasureSpace { Direct, Viewport(ViewportFrame), - Mixed, + ConflictingViewports, } impl OpenCADStudio { @@ -26,33 +26,23 @@ impl OpenCADStudio { selection.left_dragging = false; } - #[allow(dead_code)] // Consumed by the dependent association change. - pub(crate) fn dimension_measuring_snaps(&self, _i: usize) -> Vec { - self.accepted_snaps().to_vec() - } - pub(crate) fn dimension_measure_space(&self, i: usize) -> DimensionMeasureSpace { let scene = &self.tabs[i].scene; if scene.current_layout == "Model" || scene.active_viewport.is_some() { return DimensionMeasureSpace::Direct; } let mut frame: Option = None; - let mut direct = false; - for snap in self.accepted_snaps() { - match snap.frame { - Some(f) => match frame { - None => frame = Some(f), - Some(previous) if previous.viewport == f.viewport => {} - Some(_) => return DimensionMeasureSpace::Mixed, - }, - None => direct = true, + for f in self.accepted_snaps().iter().filter_map(|snap| snap.frame) { + match frame { + None => frame = Some(f), + Some(previous) if previous.viewport == f.viewport => {} + Some(_) => return DimensionMeasureSpace::ConflictingViewports, } } - match (frame, direct) { - (None, _) => DimensionMeasureSpace::Direct, - (Some(f), false) => DimensionMeasureSpace::Viewport(f), - _ => DimensionMeasureSpace::Mixed, - } + frame.map_or( + DimensionMeasureSpace::Direct, + DimensionMeasureSpace::Viewport, + ) } /// Reject an incompatible measuring input before it advances the command. @@ -68,8 +58,15 @@ impl OpenCADStudio { { return true; } - if self.accepted_snaps().iter().any(|s| s.viewport != viewport) { - self.command_line.push_error(crate::t!("Pick geometry from the same viewport, or use paper-space points for the whole dimension.").as_ref()); + // Sheet geometry and free sheet points may accompany one viewport. + // A second viewport would make the measurement scale ambiguous. + if viewport.is_some_and(|viewport| { + self.accepted_snaps() + .iter() + .filter_map(|s| s.viewport) + .any(|previous| previous != viewport) + }) { + self.command_line.push_error(crate::t!("Model geometry must come from one viewport. Paper-space points can be included.").as_ref()); return false; } true @@ -134,8 +131,8 @@ impl OpenCADStudio { ); let frame = match self.dimension_measure_space(i) { DimensionMeasureSpace::Direct => return true, - DimensionMeasureSpace::Mixed => { - self.command_line.push_error(crate::t!("Pick geometry from the same viewport, or use paper-space points for the whole dimension.").as_ref()); + DimensionMeasureSpace::ConflictingViewports => { + self.command_line.push_error(crate::t!("Model geometry must come from one viewport. Paper-space points can be included.").as_ref()); return false; } DimensionMeasureSpace::Viewport(f) => f, diff --git a/src/app/mod.rs b/src/app/mod.rs index 48ef49c54..56e44bb8a 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -717,21 +717,12 @@ pub(super) struct OpenCADStudio { show_layout_tabs: bool, /// Last point committed by a drawing command — used as ortho/polar base. last_point: Option, - /// Viewport frame the live snap marker is currently looking through (PR1). - /// Set by the cursor-move handler when the displayed snap came from a - /// layout viewport, cleared otherwise. Display/bookkeeping only — the point - /// itself is already projected onto the sheet. + /// Viewport used by the current snap; the displayed point is in paper space. pub(crate) vp_snap_frame: Option, - /// Snaps accepted by the active command's point steps, parallel to the - /// points it collected (PR1). A command that does not care keeps using the - /// plain paper point; dimension commands (PR2) read the model point, - /// viewport and frame from here, and PR3 reads `source`. Cleared whenever a - /// command starts or ends. + /// Acquired coordinates and source identities in command-step order. + /// Cleared when the command starts or ends. accepted_snaps: Vec, - /// Snap resolved by the click currently being handled, with the viewport - /// frame it came through (PR1). Set inside the click handler's point - /// resolution so the point-accept path further down can record the - /// [`crate::scene::viewport_ref::AcceptedSnap`] without recomputing. + /// Click result retained until the command accepts its point. pending_click_snap: Option<( crate::snap::SnapResult, Option, diff --git a/src/app/update/viewport.rs b/src/app/update/viewport.rs index f404c0a83..acbb65bdf 100644 --- a/src/app/update/viewport.rs +++ b/src/app/update/viewport.rs @@ -2059,24 +2059,24 @@ impl OpenCADStudio { ) }; - // Paper-space snapping THROUGH layout viewports (PR1). `edit_cam` + // Paper-space snapping through layout viewports. `edit_cam` // is None here, so `cursor_world` is the paper point and the sheet // fills the canvas (pane-local == canvas pixels). The viewport hit // arrives already projected onto the sheet, so everything // downstream keeps working in paper coordinates — LINE still draws // at the projected paper point. self.vp_snap_frame = None; - if edit_cam.is_none() && !needs_entity && !is_gathering && !needs_structure + if edit_cam.is_none() + && !needs_entity + && !is_gathering + && !needs_structure && !needs_tan { if let Some((vp_hit, frame)) = self.paper_viewport_snap(i, p_full, vp_size, cursor_world) { - let merged = crate::snap::merge_snap( - self.tabs[i].snap_result, - Some(vp_hit), - p_full, - ); + let merged = + crate::snap::merge_snap(self.tabs[i].snap_result, Some(vp_hit), p_full); if merged.is_some_and(|hit| hit.viewport.is_some()) { self.vp_snap_frame = Some(frame); } @@ -3701,7 +3701,7 @@ impl OpenCADStudio { } } } - // Paper-space snapping through layout viewports (PR1). Mirrors + // Paper-space snapping through layout viewports. Mirrors // the cursor-move pass: the click recomputes snapping from // scratch, so the viewport query has to run here too. The hit // comes back already projected onto the sheet. @@ -3710,8 +3710,7 @@ impl OpenCADStudio { if let Some((vp_hit, frame)) = self.paper_viewport_snap(i, p_full, (vw, vh), raw) { - let merged = - crate::snap::merge_snap(snap_hit, Some(vp_hit), p_full); + let merged = crate::snap::merge_snap(snap_hit, Some(vp_hit), p_full); if merged.is_some_and(|hit| hit.viewport.is_some()) { click_frame = Some(frame); } @@ -4046,7 +4045,10 @@ impl OpenCADStudio { } } - if !self.dimension_acquisition_allowed(i, None) { self.finish_command_click(i); return Task::none(); } + if !self.dimension_acquisition_allowed(i, None) { + self.finish_command_click(i); + return Task::none(); + } let shift = self.shift_down; let result = self.tabs[i].active_cmd.as_mut().map(|c| { // Shift-swap state for TRIM/EXTEND (#336). @@ -4126,11 +4128,21 @@ impl OpenCADStudio { } result } else if let Some(result) = { - let dimension = self.tabs[i].active_cmd.as_ref().is_some_and(|c| c.measures_through_viewports()); + let dimension = self.tabs[i] + .active_cmd + .as_ref() + .is_some_and(|c| c.measures_through_viewports()); if dimension { - let per_px = self.tabs[i].scene.camera.borrow().ortho_size() as f64 * 2.0 / (bounds.height as f64).max(1.0); - self.try_dimension_viewport_entity_pick(i, pick_wcs, crate::ui::overlay::pick_box_aperture_px(self.pick_box) as f64 * per_px) - } else { None } + let per_px = self.tabs[i].scene.camera.borrow().ortho_size() as f64 * 2.0 + / (bounds.height as f64).max(1.0); + self.try_dimension_viewport_entity_pick( + i, + pick_wcs, + crate::ui::overlay::pick_box_aperture_px(self.pick_box) as f64 * per_px, + ) + } else { + None + } } { Some(result) } else if self.tabs[i] @@ -4139,7 +4151,12 @@ impl OpenCADStudio { .is_some_and(|command| command.entity_pick_accepts_points()) { let pending = self.pending_click_snap.take(); - if !self.record_accepted_snap(i, pending.map(|(hit, _)| hit), pending.and_then(|(_, frame)| frame), pick_wcs) { + if !self.record_accepted_snap( + i, + pending.map(|(hit, _)| hit), + pending.and_then(|(_, frame)| frame), + pick_wcs, + ) { self.finish_command_click(i); return Task::none(); } @@ -4212,7 +4229,7 @@ impl OpenCADStudio { } } // Record what this point step accepted, alongside the point - // itself (PR1): paper point, model point, viewport, frame and + // itself: paper point, model point, viewport, frame and // geometry identity. Commands that don't care keep consuming // `world_pt` exactly as before. let pending = self.pending_click_snap.take(); @@ -4221,7 +4238,10 @@ impl OpenCADStudio { pending.map(|(hit, _)| hit), pending.and_then(|(_, frame)| frame), world_pt, - ) { self.finish_command_click(i); return Task::none(); } + ) { + self.finish_command_click(i); + return Task::none(); + } self.last_point = Some(world_pt); // The one-shot snap override is spent by this pick — // restore the running osnap configuration (#337). diff --git a/src/app/update/viewport_snap.rs b/src/app/update/viewport_snap.rs index 35c112b85..a2a2e28c8 100644 --- a/src/app/update/viewport_snap.rs +++ b/src/app/update/viewport_snap.rs @@ -1,24 +1,7 @@ -//! Paper-space snapping THROUGH layout viewports (PR1). -//! -//! In a paper layout with no active viewport, `Scene::hit_test_wires` returns -//! only the paper sheet's own entities — viewport contents are deliberately not -//! interactive, and that must stay true for *selection*. Snapping, however, -//! should see the model geometry a viewport displays, so a paper-space LINE / -//! DIMENSION can pick a real model endpoint and land on the paper pixel where -//! it is drawn. -//! -//! This module adds that as a SEPARATE query. It never changes -//! `hit_test_wires`. For each displayed content viewport under the cursor -//! (top-most first) it runs the EXISTING snap engine against that viewport's -//! resident model wires, using the viewport's own camera and screen rectangle -//! (`Scene::viewport_edit_frame_for` — the same adapter MSPACE editing uses), -//! then maps the accepted model point back onto the sheet with -//! [`ViewportFrame::model_to_paper`]. -//! -//! Performance: no projected geometry is rebuilt per pointer event. The wire -//! set comes from `model_wires_for_viewport_arc`, which is the renderer's own -//! resident, camera-independent `Arc`, and the cursor-local broad phase reuses -//! the shared interaction index built over it. +//! Snap to displayed model geometry while a paper-space command is active. +//! Viewport queries use the renderer's resident wires and interaction index, +//! then project the accepted model point onto the sheet. Ordinary selection +//! continues to use paper-space hit testing. use iced::Point; @@ -34,37 +17,14 @@ const MAX_ACCEPTED_SNAPS: usize = 64; impl OpenCADStudio { /// Snaps accepted by the active command's point steps, oldest first, in - /// step order. Consumed by the dimension commands in PR2/PR3. - #[allow(dead_code)] + /// step order. pub(crate) fn accepted_snaps(&self) -> &[AcceptedSnap] { &self.accepted_snaps } - /// The most recently accepted snap, if any. - #[allow(dead_code)] - pub(crate) fn last_accepted_snap(&self) -> Option<&AcceptedSnap> { - self.accepted_snaps.last() - } - - /// `Scene::infer_dimension_sources`, suppressed when the dimension being - /// created measures through a layout viewport. - /// - /// Inference works by looking for geometry near the dimension's own - /// definition points, in whatever space it is handed. A paper-space - /// dimension measured through a viewport has PAPER definition points that - /// happen to sit over projected model geometry, so unguarded inference - /// would either associate it with unrelated paper-sheet geometry at those - /// coordinates, or match model geometry using paper coordinates. Neither - /// is correct, and a wrong association is worse than none. Returning an - /// empty source list makes `attach_dimension_association` a no-op. - /// - /// The classification is - /// [`OpenCADStudio::dimension_measure_space`] — the *same* one that - /// decides whether to compensate the measurement, so the two can never - /// disagree. In particular it ignores the trailing dimension-line - /// placement click, which routinely lands inside a viewport rectangle for - /// an ordinary paper-space dimension and must not suppress its - /// association. + /// Infer direct sources only when the measurement uses the active space. + /// Projected model points must retain their acquired source identities; + /// proximity on the sheet could bind them to unrelated paper geometry. pub(crate) fn infer_dimension_sources_guarded( &self, tab: usize, @@ -76,14 +36,8 @@ impl OpenCADStudio { self.tabs[tab].scene.infer_dimension_sources(dimension) } - /// The single gate on creating a paper-space association for the - /// dimension the active command is committing. - /// - /// `false` exactly when the dimension measures through (or partly through) - /// a layout viewport, whatever supplied its sources — inference, - /// an explicit object pick, or an explicit source list. Everything in the - /// commit path that used to decide this for itself now asks here, so the - /// measurement rule and the association rule cannot drift apart. + /// Whether the dimension can use ordinary active-space associations. + /// Placement clicks are excluded by the measurement classification. pub(crate) fn dimension_association_allowed(&self, tab: usize) -> bool { matches!( self.dimension_measure_space(tab), @@ -136,33 +90,15 @@ impl OpenCADStudio { true } - /// Append an already-built accepted snap, keeping the retention cap. - /// - /// Used by paths that are not point steps (the dimension object pick - /// resolved through a viewport) and therefore never reach the click - /// handler. + /// Retain a bounded history for point and object picks. pub(crate) fn push_accepted_snap(&mut self, accepted: AcceptedSnap) { if self.accepted_snaps.len() >= MAX_ACCEPTED_SNAPS { self.accepted_snaps.remove(0); } self.accepted_snaps.push(accepted); } - /// Snap the paper-space cursor to model geometry seen through a layout - /// viewport. - /// - /// * `i` — tab index. - /// * `cursor_canvas` — cursor in canvas pixels. - /// * `canvas` — canvas size in pixels. - /// * `cursor_paper` — the paper-space point under the cursor. - /// - /// Returns the hit expressed in PAPER coordinates (`world` projected onto - /// the sheet, `screen` in canvas pixels) together with the frame it came - /// through, so the caller can merge it with the ordinary paper-sheet snap - /// and later recover the model point. - /// - /// `None` in the Model layout, while a viewport is active (MSPACE already - /// snaps in model space), when snapping is off, or when the cursor is not - /// over a displayed viewport. + /// Query visible viewport geometry with a canvas-pixel cursor and sheet point. + /// Returns paper coordinates and canvas pixels, plus the source viewport frame. pub(in crate::app) fn paper_viewport_snap( &mut self, i: usize, @@ -245,10 +181,7 @@ impl OpenCADStudio { let Some(hit) = hit.filter(|h| h.snap_type != SnapType::Grid) else { continue; }; - // The wire set is the whole model, not a clipped copy, so the only - // way a feature outside the visible part could be offered is via - // the aperture reaching past the clip. Reject those: a snap must be - // a real feature the viewport actually displays. + // The snap aperture can reach outside the visible clip boundary. let paper = frame.model_to_paper(hit.world); if !self.tabs[i] .scene diff --git a/src/app/viewport_dimension_tests.rs b/src/app/viewport_dimension_tests.rs index 3b47db064..281005f03 100644 --- a/src/app/viewport_dimension_tests.rs +++ b/src/app/viewport_dimension_tests.rs @@ -149,24 +149,13 @@ fn viewport_dimension_creation_preserves_active_units_and_roundtrips() { } #[test] -fn viewport_dimension_mixed_and_degenerate_picks_allow_retry() { +fn viewport_dimension_conflicting_viewports_and_degenerate_picks_allow_retry() { let (mut app, line, frame) = fixture(); let i = app.active_tab; let _ = app.dispatch_command("DIMALIGNED"); point(&mut app, frame, line, DVec3::ZERO); point(&mut app, frame, line, DVec3::ZERO); assert_eq!(app.accepted_snaps().len(), 1); - let _ = app.feed_command(StepInput::Point(DVec3::new(60.0, 50.0, 0.0))); - assert_eq!(app.accepted_snaps().len(), 1); - assert_eq!( - app.tabs[i] - .active_cmd - .as_ref() - .unwrap() - .dimension_acquired_points() - .len(), - 1 - ); let other = ViewportFrame { viewport: Handle::new(0xFFFF), ..frame @@ -318,7 +307,12 @@ fn viewport_dimension_eligibility_is_shared_and_uses_clipping() { let top = scene.add_entity(overlay); let paper = DVec3::new(55.0, 50.0, 0.0); assert_eq!( - scene.viewport_frame_at_paper_point(paper).unwrap().viewport, + scene + .viewport_frames_at_paper_point(paper) + .into_iter() + .next() + .unwrap() + .viewport, top ); assert_eq!( @@ -340,7 +334,12 @@ fn viewport_dimension_eligibility_is_shared_and_uses_clipping() { vp.clip_boundary_handle = clip; } assert_eq!( - scene.viewport_frame_at_paper_point(paper).unwrap().viewport, + scene + .viewport_frames_at_paper_point(paper) + .into_iter() + .next() + .unwrap() + .viewport, first.viewport ); assert_eq!( @@ -354,7 +353,7 @@ fn viewport_dimension_eligibility_is_shared_and_uses_clipping() { if let Some(EntityType::Viewport(vp)) = scene.document.get_entity_mut(first.viewport) { vp.status.is_on = false; } - assert!(scene.viewport_frame_at_paper_point(paper).is_none()); + assert!(scene.viewport_frames_at_paper_point(paper).is_empty()); assert!(scene.dimension_pick_through_viewport(paper, 0.2).is_none()); } @@ -421,3 +420,74 @@ fn viewport_dimension_angular_object_picks_measure_arc_features() { assert!((displayed(&app.tabs[i].scene.document) - 90.0).abs() < 1e-5); } } + +fn start_centerline_dimension( + command: &str, + paper_first: bool, + paper_kind: SnapType, + factor: f64, +) -> (OpenCADStudio, Handle, Handle, ViewportFrame) { + let (mut app, model, frame) = fixture(); + let i = app.active_tab; + let paper = app.tabs[i] + .scene + .add_entity(EntityType::Line(Line::from_points( + Vector3::new(55.0, 40.0, 0.0), + Vector3::new(55.0, 60.0, 0.0), + ))); + let mut style = acadrust::tables::DimStyle::new("CenterlineUnits"); + style.dimlfac = factor; + app.tabs[i].scene.document.dim_styles.add(style).unwrap(); + app.tabs[i].scene.document.header.current_dimstyle_name = "CenterlineUnits".into(); + let _ = app.dispatch_command(command); + let mut paper_hit = hit(frame, paper, DVec3::new(55.0, 50.0, 0.0)); + paper_hit.world = DVec3::new(55.0, 50.0, 0.0); + paper_hit.model_point = None; + paper_hit.viewport = None; + paper_hit.snap_type = paper_kind; + let mut picks = [ + (paper_hit, None), + (hit(frame, model, DVec3::X * 100.0), Some(frame)), + ]; + if !paper_first { + picks.reverse(); + } + for (snap, context) in picks { + assert!(app.record_accepted_snap(i, Some(snap), context, snap.world)); + let result = app.tabs[i] + .active_cmd + .as_mut() + .unwrap() + .on_point(snap.world); + app.sync_dimension_snaps(i); + let _ = app.apply_cmd_result(result); + } + (app, model, paper, frame) +} + +#[test] +fn viewport_dimension_paper_centerline_uses_viewport_units_in_either_order() { + for command in ["DIMLINEAR", "DIMALIGNED"] { + for paper_first in [true, false] { + let (mut app, _, paper, _) = + start_centerline_dimension(command, paper_first, SnapType::Midpoint, 25.4); + let paper_snap = app + .accepted_snaps() + .iter() + .find(|s| s.viewport.is_none()) + .unwrap(); + assert_eq!(paper_snap.source.as_ref().unwrap().source.handle, paper); + assert_eq!(paper_snap.paper_point, paper_snap.model_point); + let _ = app.feed_command(StepInput::Point(DVec3::new(60.0, 80.0, 0.0))); + let doc = &app.tabs[app.active_tab].scene.document; + assert!( + (displayed(doc) - 1270.0).abs() < 1e-3, + "{command}: {}", + displayed(doc) + ); + app.undo_steps(1); + app.redo_steps(1); + assert!((displayed(&app.tabs[app.active_tab].scene.document) - 1270.0).abs() < 1e-3); + } + } +} diff --git a/src/command.rs b/src/command.rs index a248db896..8368c9f8a 100644 --- a/src/command.rs +++ b/src/command.rs @@ -2245,23 +2245,23 @@ pub trait CadCommand: Send { CmdResult::Cancel } - /// Returns `true` when the command needs entity picking (hit-test) instead of point picking. - /// PR2: this command creates a dimension whose measurement must be - /// compensated when its points are picked through a paper-space viewport, - /// and whose explicit object pick may land on model geometry displayed - /// inside one. See `crate::app::dim_viewport`. + /// Supports acquiring dimension geometry through a paper-space viewport. fn measures_through_viewports(&self) -> bool { false } /// Definition points acquired so far, in the command's working space. /// Used to retain references only for accepted measuring inputs. - fn dimension_acquired_points(&self) -> Vec { Vec::new() } + fn dimension_acquired_points(&self) -> Vec { + Vec::new() + } /// The next point places annotation rather than acquiring geometry. - fn dimension_placement_pending(&self) -> bool { false } - + fn dimension_placement_pending(&self) -> bool { + false + } + /// Needs entity hit-testing instead of point input. fn needs_entity_pick(&self) -> bool { false } diff --git a/src/entities/dim_override.rs b/src/entities/dim_override.rs index dc9813c28..d9688ea5b 100644 --- a/src/entities/dim_override.rs +++ b/src/entities/dim_override.rs @@ -255,12 +255,9 @@ pub fn set_on_entity(entity: &mut EntityType, code: i16, value: Option) -> f64 { let raw = dimension.measurement(); - if matches!(dimension, Dimension::Angular2Ln(_) | Dimension::Angular3Pt(_)) { + if matches!( + dimension, + Dimension::Angular2Ln(_) | Dimension::Angular3Pt(_) + ) { return raw; } let lfac = style.map(|s| s.dimlfac).unwrap_or(1.0); diff --git a/src/modules/annotate/aligned_dim.rs b/src/modules/annotate/aligned_dim.rs index 9cde3e7f2..2e2f3f133 100644 --- a/src/modules/annotate/aligned_dim.rs +++ b/src/modules/annotate/aligned_dim.rs @@ -161,7 +161,7 @@ impl CadCommand for AlignedDimensionCommand { CmdResult::Cancel } - /// PR2: points and object picks may come through a paper-space viewport; + /// Points and object picks may come through a paper-space viewport; /// the committed dimension then reports the model measurement. fn measures_through_viewports(&self) -> bool { true @@ -169,7 +169,8 @@ impl CadCommand for AlignedDimensionCommand { fn dimension_acquired_points(&self) -> Vec { match self.step { - Step::First => vec![], Step::Second(p) => vec![p], + Step::First => vec![], + Step::Second(p) => vec![p], Step::DimLine { p1, p2 } => vec![p1, p2], } } diff --git a/src/modules/annotate/angular_dim.rs b/src/modules/annotate/angular_dim.rs index e9bdb0aea..66fb49596 100644 --- a/src/modules/annotate/angular_dim.rs +++ b/src/modules/annotate/angular_dim.rs @@ -482,7 +482,7 @@ impl CadCommand for AngularDimensionCommand { CmdResult::Cancel } - /// PR2: points and object picks may come through a paper-space viewport; + /// Points and object picks may come through a paper-space viewport; /// the committed dimension then reports the model measurement. fn measures_through_viewports(&self) -> bool { true @@ -490,11 +490,27 @@ impl CadCommand for AngularDimensionCommand { fn dimension_acquired_points(&self) -> Vec { match self.step { - Step::Vertex => vec![], Step::FirstRay(p) => vec![p], - Step::SecondRay { vertex, first } | Step::CircleSecondRay { vertex, first, .. } => vec![vertex, first], - Step::SecondLine { first_start, first_end, .. } => vec![first_start, first_end], - Step::ArcPoint3 { vertex, first, second } => vec![vertex, first, second], - Step::ArcPoint2 { first_start, first_end, second_start, second_end } => vec![first_start, first_end, second_start, second_end], + Step::Vertex => vec![], + Step::FirstRay(p) => vec![p], + Step::SecondRay { vertex, first } | Step::CircleSecondRay { vertex, first, .. } => { + vec![vertex, first] + } + Step::SecondLine { + first_start, + first_end, + .. + } => vec![first_start, first_end], + Step::ArcPoint3 { + vertex, + first, + second, + } => vec![vertex, first, second], + Step::ArcPoint2 { + first_start, + first_end, + second_start, + second_end, + } => vec![first_start, first_end, second_start, second_end], } } diff --git a/src/modules/annotate/diameter_dim.rs b/src/modules/annotate/diameter_dim.rs index b128d4dde..b41fa2839 100644 --- a/src/modules/annotate/diameter_dim.rs +++ b/src/modules/annotate/diameter_dim.rs @@ -132,7 +132,7 @@ impl CadCommand for DiameterDimensionCommand { CmdResult::Cancel } - /// PR2: points and object picks may come through a paper-space viewport; + /// Points and object picks may come through a paper-space viewport; /// the committed dimension then reports the model measurement. fn measures_through_viewports(&self) -> bool { true @@ -140,7 +140,8 @@ impl CadCommand for DiameterDimensionCommand { fn dimension_acquired_points(&self) -> Vec { match self.step { - Step::SelectObject => vec![], Step::DimLine(source) => vec![dvec(source.point_at_angle(source.start_angle))], + Step::SelectObject => vec![], + Step::DimLine(source) => vec![dvec(source.point_at_angle(source.start_angle))], } } diff --git a/src/modules/annotate/linear_dim.rs b/src/modules/annotate/linear_dim.rs index e8f55c32f..7e92b0c6b 100644 --- a/src/modules/annotate/linear_dim.rs +++ b/src/modules/annotate/linear_dim.rs @@ -217,7 +217,7 @@ impl CadCommand for LinearDimensionCommand { CmdResult::Cancel } - /// PR2: points and object picks may come through a paper-space viewport; + /// Points and object picks may come through a paper-space viewport; /// the committed dimension then reports the model measurement. fn measures_through_viewports(&self) -> bool { true @@ -225,7 +225,8 @@ impl CadCommand for LinearDimensionCommand { fn dimension_acquired_points(&self) -> Vec { match self.step { - Step::FirstPoint => vec![], Step::SecondPoint(p) => vec![p], + Step::FirstPoint => vec![], + Step::SecondPoint(p) => vec![p], Step::DimensionLine { first, second } => vec![first, second], } } diff --git a/src/modules/annotate/radius_dim.rs b/src/modules/annotate/radius_dim.rs index 214933a8e..898ba52ca 100644 --- a/src/modules/annotate/radius_dim.rs +++ b/src/modules/annotate/radius_dim.rs @@ -130,7 +130,7 @@ impl CadCommand for RadiusDimensionCommand { CmdResult::Cancel } - /// PR2: points and object picks may come through a paper-space viewport; + /// Points and object picks may come through a paper-space viewport; /// the committed dimension then reports the model measurement. fn measures_through_viewports(&self) -> bool { true @@ -138,7 +138,8 @@ impl CadCommand for RadiusDimensionCommand { fn dimension_acquired_points(&self) -> Vec { match self.step { - Step::SelectObject => vec![], Step::DimLine(source) => vec![dvec(source.point_at_angle(source.start_angle))], + Step::SelectObject => vec![], + Step::DimLine(source) => vec![dvec(source.point_at_angle(source.start_angle))], } } diff --git a/src/scene/mod.rs b/src/scene/mod.rs index 4137628bf..9622be181 100644 --- a/src/scene/mod.rs +++ b/src/scene/mod.rs @@ -41,7 +41,7 @@ mod limits; mod modify; mod mspace; pub mod viewport_ref; -pub mod viewport_dim_seam; +pub mod viewport_dimension_pick; pub mod named_parameters; mod page_setup; mod paper; diff --git a/src/scene/mspace.rs b/src/scene/mspace.rs index 11f516329..3cd822f1c 100644 --- a/src/scene/mspace.rs +++ b/src/scene/mspace.rs @@ -83,14 +83,7 @@ impl Scene { self.viewport_edit_frame_for(self.active_viewport?, canvas_px) } - /// [`viewport_edit_frame`] for an explicitly-named viewport, rather than - /// only the active (MSPACE) one. - /// - /// Paper-space snapping through viewports (PR1) needs the same camera + - /// screen rectangle pair for a viewport the user has NOT entered, so the - /// existing snap engine can run against that viewport's model wires on the - /// pixels the GPU actually drew them on. `viewport_edit_frame` is now a - /// thin wrapper over this. + /// Camera and canvas rectangle for snapping through an inactive viewport. pub fn viewport_edit_frame_for( &self, vp_handle: Handle, @@ -101,15 +94,12 @@ impl Scene { Some((cam, full)) } - /// Planar paper <-> model mapping for a layout viewport, derived from the - /// *same* camera the renderer displays its contents with - /// ([`camera_for_viewport`]) so a snap computed through the frame lands on - /// the pixels the GPU drew. - /// - /// Returns `None` for a viewport whose view is not an in-plane orthographic - /// top view (oblique / isometric / perspective viewports are unsupported by - /// the planar frame — callers fall back to no viewport snapping there). - pub fn viewport_frame(&self, vp_handle: Handle) -> Option { + /// Planar paper/model mapping derived from the viewport's display camera. + /// Oblique and perspective views have no supported planar mapping. + pub fn viewport_frame( + &self, + vp_handle: Handle, + ) -> Option { use crate::scene::viewport_ref::ViewportFrame; let (paper_center, vp_height, locked) = match self.document.get_entity(vp_handle) { @@ -120,7 +110,7 @@ impl Scene { ), _ => return None, }; - if vp_height.abs() < 1e-9 { + if !paper_center.is_finite() || !vp_height.is_finite() || vp_height < 1e-9 { return None; } let cam = self.camera_for_viewport(vp_handle)?; @@ -133,7 +123,7 @@ impl Scene { // an oblique 3-D view: unsupported. let right = (cam.rotation * glam::Vec3::X).as_dvec3(); let up = (cam.rotation * glam::Vec3::Y).as_dvec3(); - if right.z.abs() > 1e-6 || up.z.abs() > 1e-6 { + if !right.is_finite() || !up.is_finite() || right.z.abs() > 1e-6 || up.z.abs() > 1e-6 { return None; } let normal = right.cross(up); @@ -142,7 +132,7 @@ impl Scene { } // Model units visible across the viewport's height -> paper per model. let model_height = cam.ortho_size() as f64 * 2.0; - if model_height.abs() < 1e-12 { + if !model_height.is_finite() || model_height < 1e-12 { return None; } let scale = vp_height / model_height; @@ -162,23 +152,28 @@ impl Scene { /// Visible, supported content viewports at a sheet point, topmost first. /// Shared by snapping and explicit dimension object picking. - pub fn viewport_frames_at_paper_point(&self, paper: glam::DVec3) -> Vec { - if self.current_layout == "Model" || self.active_viewport.is_some() { return Vec::new(); } - self.layout_content_viewports().iter().rev().filter_map(|&handle| { - if !self.viewport_displays_content(handle) || !self.viewport_displays_paper_point(handle, paper.truncate()) { - return None; - } - self.viewport_frame(handle) - }).collect() + pub fn viewport_frames_at_paper_point( + &self, + paper: glam::DVec3, + ) -> Vec { + if self.current_layout == "Model" || self.active_viewport.is_some() { + return Vec::new(); + } + self.layout_content_viewports() + .iter() + .rev() + .filter_map(|&handle| { + if !self.viewport_displays_content(handle) + || !self.viewport_displays_paper_point(handle, paper.truncate()) + { + return None; + } + self.viewport_frame(handle) + }) + .collect() } - /// Does `paper` lie inside the viewport's displayed area? - /// - /// Always bounded by the viewport's paper rectangle; a viewport with a - /// `clip_boundary_handle` (non-rectangular / clipped viewport) additionally - /// has to contain the point inside that boundary polygon. Used so a - /// paper-space snap never reports a model feature the viewport does not - /// actually show. + /// Test the viewport rectangle and optional nonrectangular clip boundary. pub fn viewport_displays_paper_point(&self, vp_handle: Handle, paper: glam::DVec2) -> bool { let Some(EntityType::Viewport(vp)) = self.document.get_entity(vp_handle) else { return false; diff --git a/src/scene/project.rs b/src/scene/project.rs index f20c10e38..4d67327ac 100644 --- a/src/scene/project.rs +++ b/src/scene/project.rs @@ -674,10 +674,7 @@ pub(crate) fn clip_boundary_polygon_for_document( z: f32, ) -> Vec<[f32; 3]> { use std::f64::consts::TAU; - let Some(entity) = document - .entities() - .find(|e| e.common().handle == handle) - else { + let Some(entity) = document.get_entity(handle) else { return vec![]; }; // Circles and ellipses tessellate directly — their `to_render` returns a diff --git a/src/scene/viewport_dim_seam.rs b/src/scene/viewport_dimension_pick.rs similarity index 82% rename from src/scene/viewport_dim_seam.rs rename to src/scene/viewport_dimension_pick.rs index 5f4ca3ce4..e14cffe29 100644 --- a/src/scene/viewport_dim_seam.rs +++ b/src/scene/viewport_dimension_pick.rs @@ -1,20 +1,5 @@ -//! Object-pick resolution for paper-space dimensions taken through a layout -//! viewport. -//! -//! Paper-space hit testing only sees the sheet's own entities, so a dimension -//! command's explicit "select object" pick that lands inside a content -//! viewport finds nothing. [`Scene::dimension_pick_through_viewport`] maps the -//! click into model space through the viewport's -//! [`crate::scene::viewport_ref::ViewportFrame`], hit-tests the viewport's -//! resident model wires (descending into block instances and baking the -//! instance transform), and hands back a copy of the picked entity already -//! projected onto the sheet. -//! -//! The frame itself is built by [`Scene::viewport_frame`] (see -//! `crate::scene::mspace`), which derives it from the *live viewport camera* -//! so it always agrees with the pixels the renderer drew. It returns `None` -//! for oblique / perspective viewports, and every entry point here then -//! declines the pick rather than guessing a planar mapping. +//! Pick dimension geometry through a layout viewport. Resolve block instances +//! in model space, then project the picked entity onto the paper sheet. use super::*; @@ -27,7 +12,7 @@ use glam::DVec3; /// A model entity resolved by clicking inside a paper-space content viewport. #[derive(Clone, Debug)] -pub struct ViewportDimPick { +pub struct ViewportDimensionPick { /// The viewport the click looked through. pub frame: ViewportFrame, /// Handle of the innermost model entity that owns the picked geometry. @@ -46,18 +31,6 @@ pub struct ViewportDimPick { } impl Scene { - pub fn content_viewport_at_paper_point(&self, paper: DVec3) -> Option { - self.viewport_frames_at_paper_point(paper) - .first() - .map(|f| f.viewport) - } - - pub fn viewport_frame_at_paper_point(&self, paper: DVec3) -> Option { - self.viewport_frames_at_paper_point(paper) - .into_iter() - .next() - } - /// Resolve an explicit dimension object pick made on the sheet but landing /// inside a content viewport, against the *model* geometry that viewport /// displays. @@ -67,14 +40,13 @@ impl Scene { &self, paper: DVec3, aperture_paper: f64, - ) -> Option { + ) -> Option { for frame in self.viewport_frames_at_paper_point(paper) { let model_point = frame.paper_to_model(paper); let aperture_model = (aperture_paper.max(0.0) * frame.paper_to_model_length_factor()).max(1e-9); - let Some(top) = - self.nearest_model_wire_handle(frame.viewport, model_point, aperture_model) + let Some(top) = self.nearest_model_wire_handle(&frame, model_point, aperture_model) else { continue; }; @@ -93,7 +65,7 @@ impl Scene { &EntityTransform::Affine(viewport_model_to_paper_transform(&frame)), ); let entity_handle = paper_entity.as_entity().handle(); - return Some(ViewportDimPick { + return Some(ViewportDimensionPick { frame, entity_handle: if entity_handle.is_valid() { entity_handle @@ -114,10 +86,11 @@ impl Scene { /// draws, so this matches what the user sees. fn nearest_model_wire_handle( &self, - viewport: Handle, + frame: &ViewportFrame, model_point: DVec3, aperture: f64, ) -> Option { + let viewport = frame.viewport; let camera = self.camera_for_viewport(viewport)?; let bounds = iced::Rectangle { x: 0.0, @@ -150,9 +123,6 @@ impl Scene { .unwrap_or(DVec3::ZERO); let distance = wire_polyline_nearest(wire, offset, model_point); if let Some((distance, nearest)) = distance { - let Some(frame) = self.viewport_frame(viewport) else { - continue; - }; if !self.viewport_displays_paper_point( viewport, frame.model_to_paper(nearest).truncate(), @@ -178,17 +148,8 @@ impl Scene { ) -> Option<(EntityType, Vec)> { let entity = self.document.get_entity(handle)?; match entity { - EntityType::Insert(_) => { - let mut path = Vec::new(); - let found = self.descend_block_instance( - entity.clone(), - Transform::identity(), - model_point, - &mut path, - 0, - viewport, - )?; - Some((found.0, found.1)) + EntityType::Insert(insert) => { + self.descend_block_instance(insert, Transform::identity(), model_point, 0, viewport) } other => { planar_pick_distance(other, model_point)?; @@ -201,20 +162,16 @@ impl Scene { /// Returns the entity already transformed into WCS plus the INSERT path. fn descend_block_instance( &self, - insert_entity: EntityType, + insert: &acadrust::entities::Insert, outer: Transform, model_point: DVec3, - path: &mut Vec, depth: usize, viewport: Option, ) -> Option<(EntityType, Vec)> { const MAX_DEPTH: usize = 8; - if depth > MAX_DEPTH { + if depth >= MAX_DEPTH { return None; } - let EntityType::Insert(insert) = &insert_entity else { - return None; - }; if insert.row_count > 1 || insert.column_count > 1 { return None; } @@ -222,10 +179,9 @@ impl Scene { let local = crate::scene::render_graph::insert_transform(&self.document, insert); let combined = local.then(&outer); let block = self.document.block_records.get(&insert.block_name)?; - let children: Vec = block.entity_handles.clone(); let mut best: Option<(f64, EntityType, Vec)> = None; - for child in children { + for &child in &block.entity_handles { let Some(child_entity) = self.document.get_entity(child) else { continue; }; @@ -237,13 +193,11 @@ impl Scene { if common.invisible || layer.is_some_and(|l| l.flags.off || l.flags.frozen) || frozen { continue; } - if matches!(child_entity, EntityType::Insert(_)) { - let mut nested = Vec::new(); + if let EntityType::Insert(child_insert) = child_entity { if let Some((entity, mut nested_path)) = self.descend_block_instance( - child_entity.clone(), + child_insert, combined, model_point, - &mut nested, depth + 1, viewport, ) { @@ -286,8 +240,6 @@ impl Scene { } } let (_, entity, found_path) = best?; - path.clear(); - path.extend(found_path.iter().copied()); Some((entity, found_path)) } } diff --git a/src/scene/viewport_ref.rs b/src/scene/viewport_ref.rs index 3ce7dfae9..327764b6d 100644 --- a/src/scene/viewport_ref.rs +++ b/src/scene/viewport_ref.rs @@ -187,17 +187,8 @@ impl AcceptedSnap { } } -/// The two independent factors that turn a raw measured distance into the -/// number a linear/aligned/radial dimension displays. -/// -/// * `user_lfac` is the drawing/style's DIMLFAC as the user intends it -/// (always positive here; the sign convention of DXF DIMLFAC is resolved by -/// [`MeasurementScale::from_dimlfac`]). -/// * `viewport_compensation` undoes the viewport's paper-per-model scale for -/// a dimension placed in paper space but measuring model geometry. It is -/// `1.0` for model-space dimensions and for paper-only dimensions. -/// -/// Angular dimensions never use either factor. +/// Length dimensions store raw distances in their owning space. The displayed +/// value uses `user_lfac * viewport_compensation`; angular dimensions are exempt. #[derive(Clone, Copy, Debug, PartialEq)] pub struct MeasurementScale { pub user_lfac: f64, @@ -252,59 +243,13 @@ impl MeasurementScale { common.extended_data = data; } - pub const IDENTITY: Self = Self { - user_lfac: 1.0, - viewport_compensation: 1.0, - }; - - /// Resolve the DXF DIMLFAC convention: - /// * `dimlfac > 0`: applies to every dimension. - /// * `dimlfac < 0`: `|dimlfac|` applies only to dimensions created in - /// paper space that measure model geometry through a viewport; model - /// space dimensions use `1.0`. - /// * `dimlfac == 0`: treated as `1.0`. - /// - /// `viewport` supplies the compensation when the dimension measures - /// through a viewport; pass `None` for model-space or paper-only - /// dimensions. - pub fn from_dimlfac(dimlfac: f64, viewport: Option<&ViewportFrame>) -> Self { - let through_viewport = viewport.is_some(); - let user_lfac = if dimlfac == 0.0 { - 1.0 - } else if dimlfac > 0.0 { - dimlfac - } else if through_viewport { - -dimlfac - } else { - 1.0 - }; - let viewport_compensation = viewport - .map(ViewportFrame::paper_to_model_length_factor) - .unwrap_or(1.0); - Self { - user_lfac, - viewport_compensation, - } - } - /// Combined factor applied to a distance measured on the paper sheet. pub fn paper_factor(&self) -> f64 { self.user_lfac * self.viewport_compensation } - /// Factor applied to a distance already measured in model space (the - /// viewport compensation must not be applied again). - pub fn model_factor(&self) -> f64 { - self.user_lfac - } - - /// The DXF DIMLFAC sign convention, resolved without a [`ViewportFrame`]. - /// - /// Rendering an *existing* dimension only knows whether the entity lives - /// in a paper-space layout; the viewport compensation is already baked - /// into the persisted magnitude. This is the same rule - /// as [`MeasurementScale::from_dimlfac`] with `viewport.is_some()` - /// replaced by `paper_space`. + /// Positive DIMLFAC applies everywhere; negative DIMLFAC applies only on + /// paper. Zero means one. Saved viewport compensation is already in DIMLFAC. pub fn user_lfac_for_space(dimlfac: f64, paper_space: bool) -> f64 { if dimlfac == 0.0 { 1.0 @@ -352,34 +297,10 @@ mod tests { } #[test] - fn negative_dimlfac_only_applies_through_viewports() { - let f = frame(0.5, 0.0); - let through = MeasurementScale::from_dimlfac(-2.0, Some(&f)); - assert_eq!(through.user_lfac, 2.0); - assert!((through.viewport_compensation - 2.0).abs() < 1e-12); - assert!((through.paper_factor() - 4.0).abs() < 1e-12); - assert!((through.model_factor() - 2.0).abs() < 1e-12); - - let model = MeasurementScale::from_dimlfac(-2.0, None); - assert_eq!(model, MeasurementScale::IDENTITY); - - let positive = MeasurementScale::from_dimlfac(3.0, None); - assert_eq!(positive.user_lfac, 3.0); - assert_eq!(positive.viewport_compensation, 1.0); - } - - #[test] - fn user_lfac_for_space_matches_from_dimlfac() { - let f = frame(0.5, 0.0); - for lfac in [-2.0, 0.0, 1.0, 3.5] { - assert_eq!( - MeasurementScale::user_lfac_for_space(lfac, true), - MeasurementScale::from_dimlfac(lfac, Some(&f)).user_lfac - ); - assert_eq!( - MeasurementScale::user_lfac_for_space(lfac, false), - MeasurementScale::from_dimlfac(lfac, None).user_lfac - ); + fn dimlfac_sign_depends_on_dimension_space() { + for (factor, paper, model) in [(-2.0, 2.0, 1.0), (0.0, 1.0, 1.0), (3.5, 3.5, 3.5)] { + assert_eq!(MeasurementScale::user_lfac_for_space(factor, true), paper); + assert_eq!(MeasurementScale::user_lfac_for_space(factor, false), model); } } } diff --git a/src/snap.rs b/src/snap.rs index 75bcf5f20..fa837110b 100644 --- a/src/snap.rs +++ b/src/snap.rs @@ -9,9 +9,9 @@ use iced::time::Instant; use iced::{Point, Rectangle}; use cadkernel::geom2d::Curve; +use acadrust::types::Handle; use crate::command::{DimensionAssociationSource, TangentObject}; -use acadrust::types::Handle; use crate::scene::model::wire_model::{SnapHint, TangentGeom, WireModel}; use crate::scene::pick::interaction_index::WireSource; const DEFAULT_OSNAP_RADIUS_PX: f32 = 15.0; @@ -90,12 +90,12 @@ pub struct SnapResult { /// Layout viewport this snap was seen *through*, when the snap ran against /// model geometry displayed by a paper-space viewport. `None` for ordinary /// model-space / paper-sheet snaps. Filled by the paper-space viewport snap - /// query, never by the engine itself. (PR1) + /// query, never by the engine itself. pub viewport: Option, /// Owning entity of the geometry the snap landed on, when the engine could /// attribute the feature to a wire. Block sub-entities report the top-level /// INSERT handle; the block path is resolved later by - /// [`crate::scene::viewport_ref::SnapSourceRef::block_path`]. (PR1) + /// [`crate::scene::viewport_ref::SnapSourceRef::block_path`]. pub source: Option, /// Second real object at an intersection, when both objects are known. pub secondary_source: Option, @@ -986,10 +986,10 @@ impl Snapper { extension_base2: None, extension_origin: None, extension_dir: None, - viewport: None, - source: None, - secondary_source: None, - model_point: None, + viewport: None, + source: None, + secondary_source: None, + model_point: None, }) } @@ -1224,7 +1224,8 @@ impl Snapper { let mut try_pt = |world: glam::DVec3, snap_type: SnapType, - src: Option, secondary: Option| { + src: Option, + secondary: Option| { let screen = world_to_screen(world, view_rot, eye, bounds); if !in_bounds(screen) { return; @@ -1251,10 +1252,10 @@ impl Snapper { extension_base2: None, extension_origin: None, extension_dir: None, - viewport: None, - source: src, - secondary_source: secondary, - model_point: None, + viewport: None, + source: src, + secondary_source: secondary, + model_point: None, }); } }; @@ -1308,7 +1309,9 @@ impl Snapper { try_pt( DVec3::from_array(point), SnapType::Endpoint, - wire_source(wire), None); + wire_source(wire), + None, + ); } // Tessellated open curves have no key-vertex set. Their only // endpoints are first/last, so testing candidate wires remains @@ -1328,7 +1331,9 @@ impl Snapper { try_pt( wp_f64(wire, wire.points.len() - 1), SnapType::Endpoint, - wire_source(wire), None); + wire_source(wire), + None, + ); } } } else { @@ -1351,7 +1356,9 @@ impl Snapper { try_pt( wp_f64(wire, wire.points.len() - 1), SnapType::Endpoint, - src, None); + src, + None, + ); } } } @@ -1402,7 +1409,9 @@ impl Snapper { try_pt( nearest_on_segment(cursor_world, seg.a, seg.b), SnapType::Nearest, - wires.source_wire(seg.wire).and_then(wire_source), None); + wires.source_wire(seg.wire).and_then(wire_source), + None, + ); } } else { for wire in in_range_wires.iter() { @@ -1426,7 +1435,9 @@ impl Snapper { try_pt( foot, SnapType::Perpendicular, - wires.source_wire(seg.wire).and_then(wire_source), None); + wires.source_wire(seg.wire).and_then(wire_source), + None, + ); } } } else { @@ -1455,7 +1466,9 @@ impl Snapper { try_pt( point, SnapType::Intersection, - wires.source_wire(segment.wire).and_then(wire_source), None); + wires.source_wire(segment.wire).and_then(wire_source), + None, + ); } } } @@ -1553,7 +1566,7 @@ impl Snapper { for &wire_j in &local_wires[idx + 1..] { if let Some(points) = exact_curve_intersections(wire_i, wire_j) { // Two wires meet here; attribute the feature to the - // first. PR3 resolves the true pair. + // first. Association resolution requires both source paths. let src = wire_source(wire_i); for point in points { try_pt(point, SnapType::Intersection, src, wire_source(wire_j)); @@ -1590,7 +1603,9 @@ impl Snapper { try_pt( pt, SnapType::Intersection, - wires.source_wire(a.wire).and_then(wire_source), wires.source_wire(b.wire).and_then(wire_source)); + wires.source_wire(a.wire).and_then(wire_source), + wires.source_wire(b.wire).and_then(wire_source), + ); if exact_cursor { break 'intersection_sweep; } @@ -1636,7 +1651,12 @@ impl Snapper { continue; } if let Some(pt) = seg_intersect_3d(a0, a1, b0, b1) { - try_pt(pt, SnapType::Intersection, wire_source(wire_i), wire_source(wire_j)); + try_pt( + pt, + SnapType::Intersection, + wire_source(wire_i), + wire_source(wire_j), + ); } } } @@ -1762,7 +1782,9 @@ impl Snapper { try_pt( segment_a.a + ta as f64 * (segment_a.b - segment_a.a), SnapType::ApparentIntersection, - wires.source_wire(segment_a.wire).and_then(wire_source), wires.source_wire(segment_b.wire).and_then(wire_source)); + wires.source_wire(segment_a.wire).and_then(wire_source), + wires.source_wire(segment_b.wire).and_then(wire_source), + ); if dist2(apparent_screen, cursor_screen) <= f32::EPSILON { break 'apparent_sweep; } @@ -1802,7 +1824,9 @@ impl Snapper { try_pt( wa0 + ta as f64 * (wa1 - wa0), SnapType::ApparentIntersection, - wire_source(wire_i), wire_source(wire_j)); + wire_source(wire_i), + wire_source(wire_j), + ); } } } @@ -2023,10 +2047,10 @@ impl Snapper { extension_base2: None, extension_origin: None, extension_dir: None, - viewport: None, - source: wire_source(wire), - secondary_source: None, - model_point: None, + viewport: None, + source: wire_source(wire), + secondary_source: None, + model_point: None, }); } } @@ -2196,19 +2220,13 @@ fn snap_tier(t: SnapType) -> u8 { } } -/// Candidate ordering: tier first, then cursor distance, and only when two -/// candidates are effectively equidistant (coincident features) the classic -/// sub-priority — so an Endpoint still beats an Intersection sitting on the -/// exact same point. Distances are screen-px²; 4.0 ≈ a 2 px coincidence band. -/// Choose between two independently-computed snap results using the SAME -/// priority-then-distance rule the engine applies internally -/// ([`snap_better`]): a higher-priority snap inside the aperture wins even -/// when a lower-priority one is nearer, and distance only breaks ties. -/// -/// Used to merge the ordinary paper-sheet snap with the viewport snap taken -/// through a layout viewport (PR1). Both `screen` fields must be expressed in -/// the same pixel frame. -pub fn merge_snap(a: Option, b: Option, cursor: Point) -> Option { +/// Merge paper and viewport snaps using the engine's ordering. +/// Both screen positions must use canvas pixels. +pub fn merge_snap( + a: Option, + b: Option, + cursor: Point, +) -> Option { match (a, b) { (Some(a), Some(b)) => { let (at, asub) = (snap_tier(a.snap_type), snap_priority(a.snap_type)); @@ -2225,6 +2243,10 @@ pub fn merge_snap(a: Option, b: Option, cursor: Point) - } } +/// Candidate ordering: tier first, then cursor distance, and only when two +/// candidates are effectively equidistant (coincident features) the classic +/// sub-priority — so an Endpoint still beats an Intersection sitting on the +/// exact same point. Distances are screen-px²; 4.0 ≈ a 2 px coincidence band. fn snap_better(tier: u8, d2: f32, sub: u8, best: (u8, f32, u8)) -> bool { let (bt, bd2, bsub) = best; if tier != bt {