diff --git a/docs/viewport-dimension-association.svg b/docs/viewport-dimension-association.svg new file mode 100644 index 000000000..8a7a06428 --- /dev/null +++ b/docs/viewport-dimension-association.svg @@ -0,0 +1,19 @@ + + + + +The dimension follows its paper-space centerline + + +BeforeCenterline moved 1 paper unit right + + + + + + + +5040 +The model endpoint stays fixed. The dimension updates from 50 to 40. +Association schematic; viewport 1:10, dimension style unit factor = 1, DIMASSOC = 2. + 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 fe313cd1d..2190bdeb2 100644 --- a/docs/viewport-dimensions.md +++ b/docs/viewport-dimensions.md @@ -1,100 +1,74 @@ -# 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 -With `DIMASSOC=2`, acquired features attach a dimension to its viewport, each -block instance, and the source entity. Intersections retain both source chains. -Source edits and viewport changes update the definition points and recompute -compensation while preserving explicit user-positioned sheet text and dimension -style overrides. A successful update invalidates the saved picture block so the -new geometry and number are rendered and exported. Undo restores both the -reference data and the previous picture. +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. -Properties reports whether the dimension is associated, partially associated, -unresolved, broken, or nonassociative. A broken or unsupported required reference -preserves the entire dimension's last valid appearance and its original records. -Creating another dimension does not announce geometry changes to its sources. +## Associations -## Imported references and support limits +With `DIMASSOC=2`, acquired features retain their viewport, block-instance path, +source entity, and snap feature. Intersections retain both source chains. +Source edits and viewport changes update the dimension and its compensation, +preserving style overrides and user-positioned text on the sheet. -The resolver distinguishes the application's vertex/parameter convention from -imported edge references. Imported start/end and midpoint modes use their edge -marker and parameter together. The original regression drawing's first-edge -start/end pair now reproduces its `0.128` measurement. An imported polyline -endpoint beyond the first edge requires a usable stored point to validate the -marker; zero/sentinel data cannot establish that identity. Ambiguous imports -remain unresolved. Imported viewport scale is updated only when existing OCS -metadata or `ACAD_DIMASSOC_CALC_DIMLFAC` separates compensation from user scaling. +A sheet centerline stays on the sheet; a model endpoint follows its viewport. +Moving the viewport therefore changes the distance between them. Undo/redo and +DXF/DWG saves preserve the references. -Unsupported cases retain their saved data and appearance: +Properties shows association status. If a required reference is broken or cannot +be resolved, the entire dimension keeps its last valid geometry and reference +records. A successful refresh regenerates its displayed and exported geometry. -- Perspective/oblique viewport projections and malformed/missing reference chains. -- Ambiguous intersections within one batched block; they are not attached to a - guessed pair of children. Intersections between separate identified chains work. -- Array inserts without an addressable instance index, nesting beyond eight levels, - and radial measurements of nonuniformly transformed circular geometry. -- Imported quadrant references whose moved geometry no longer validates the stored - point; newly acquired circle/arc quadrants keep their source-local angle. -- Tangency with no verified solution, two dependent perpendicular/tangent references - requiring a coupled solve, unsupported snap modes, and imported measurement data that does not identify the old viewport compensation. +![Association schematic: moving the sheet centerline changes the dimension from 50 to 40.](viewport-dimension-association.svg) -The osnap codes and reference fields follow the -[Autodesk DIMASSOC reference](https://help.autodesk.com/cloudhelp/2024/ENU/AutoCAD-DXF/files/GUID-C0B96256-A911-4B4D-85E6-EB4AF2C91E27.htm). -The restricted imported-marker interpretation above is validated by fixtures; -it is not a claim that every exporter uses an interchangeable marker scheme. +## Limits -## Validation +Unsupported cases preserve saved dimensions: -`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. +- Perspective or oblique views; missing or malformed reference chains. +- Array instances without an addressable instance index, nesting beyond eight + levels, and radial measurements of nonuniformly scaled circles or arcs. +- Intersections without two identifiable source chains inside a batched block. +- Ambiguous imported endpoint, midpoint, or quadrant markers. A stored point can + validate a feature; it cannot establish an otherwise unknown identity. +- Tangency without a verified solution, or two perpendicular/tangent references + requiring a coupled solve. +- Imported viewport scaling without OCS scale metadata or + `ACAD_DIMASSOC_CALC_DIMLFAC` identifying the saved compensation. -Association tests also exercise regeneration, nearest and quadrant parameters, -angular arc/polyline object picks, spline tangency, -perpendicularity under nonuniform block scale, nested paths, intersection source -edits, preserved overrides, erased sources, copy detachment, saved picture -invalidation, and history. The checked-in DXF fixture is exercised directly. -The original private drawing can be checked separately: +## Tests -```powershell -$env:OPENCAD_VIEWPORT_REGRESSION_DXF = 'C:\path\to\115448.dxf' -cargo test --locked --offline --lib viewport_dimension_original -- --ignored --nocapture -``` - -Do not treat passing headless checks as completed desktop visual acceptance. +Run `cargo test --locked --lib viewport_dimension`. Tests cover the five commands, +mixed sheet/model input, styles, clipping, object picks, source/viewport edits, +snap features, nested paths, intersections, preserved overrides, undo/redo, and +DXF/DWG persistence. The synthetic association fixture is in +`tests/fixtures/dim_assoc/`. diff --git a/src/app/command_driver.rs b/src/app/command_driver.rs index cd78df641..8580c8eef 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)] @@ -2481,12 +2480,7 @@ impl OpenCADStudio { changes.dedup_by_key(|(handle, _)| handle.value()); self.tabs[i].scene.bump_entities(&changes); } else if association_mode == 2 { - // Measured through a layout viewport: a paper-space - // association would bind projected coordinates to - // geometry that is not on the sheet. Record the real - // chain instead — dimension -> viewport -> block - // path -> entity -> feature — from the AcceptedSnaps - // PR1 captured, in definition-point slot order. + // Preserve the acquired viewport and source paths. self.attach_viewport_dimension_association(i, handle); } } 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 a80b151c4..841b784e1 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, @@ -214,13 +211,13 @@ impl OpenCADStudio { if slots.is_empty() { return Vec::new(); } - let measuring = self.dimension_measuring_snaps(i); + let measuring = self.accepted_snaps(); slots .iter() .enumerate() .map(|(index, slot)| { let snap = measuring.get(index)?.clone(); - let frame = snap.frame?; + let frame = snap.frame; let slot = DVec3::new(slot.x, slot.y, slot.z); if snap .paper_point @@ -232,7 +229,7 @@ impl OpenCADStudio { } Some(AcceptedSnap { paper_point: slot, - model_point: frame.paper_to_model(slot), + model_point: frame.map_or(slot, |frame| frame.paper_to_model(slot)), ..snap }) }) 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/properties.rs b/src/app/properties.rs index 6a6081a37..e816f8e1f 100644 --- a/src/app/properties.rs +++ b/src/app/properties.rs @@ -1277,13 +1277,22 @@ impl OpenCADStudio { &self.tabs[i].scene.document, d.base().common.handle, ); - let statuses = self.tabs[i].scene.dimension_association_status(d.base().common.handle); - let slots = self.tabs[i].scene.dimension_association_slot_points(d.base().common.handle).len(); + let statuses = self.tabs[i] + .scene + .dimension_association_status(d.base().common.handle); + let slots = self.tabs[i] + .scene + .dimension_association_slot_points(d.base().common.handle) + .len(); let status = if statuses.is_empty() { t!("Nonassociative") - } else if statuses.iter().any(|(_, status)| matches!(status, crate::scene::ReferenceStatus::Broken(_))) { + } else if statuses.iter().any(|(_, status)| { + matches!(status, crate::scene::ReferenceStatus::Broken(_)) + }) { t!("Broken reference") - } else if statuses.iter().any(|(_, status)| matches!(status, crate::scene::ReferenceStatus::Unresolved)) { + } else if statuses.iter().any(|(_, status)| { + matches!(status, crate::scene::ReferenceStatus::Unresolved) + }) { t!("Unresolved reference") } else if statuses.len() < slots { t!("Partially associated") @@ -1293,7 +1302,9 @@ impl OpenCADStudio { general.props.push(crate::scene::model::object::Property { label: t!("Association status").into_owned(), field: "association_status", - value: crate::scene::model::object::PropValue::ReadOnly(status.into_owned()), + value: crate::scene::model::object::PropValue::ReadOnly( + status.into_owned(), + ), }); general.props.push(crate::scene::model::object::Property { label: t!("Associative").into_owned(), 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 4a638f6e3..927fd22e1 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), @@ -177,7 +131,7 @@ impl OpenCADStudio { ) else { return false; }; - let distance = crate::scene::viewport_dim_seam::feature_pick_distance( + let distance = crate::scene::viewport_dimension_pick::feature_pick_distance( &entity, accepted.model_point, Some(source.snap_type), @@ -207,33 +161,15 @@ impl OpenCADStudio { } } - /// 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, @@ -316,10 +252,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 44d1aff54..de448dbba 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()); } @@ -623,8 +622,8 @@ fn viewport_dimension_spline_tangency_is_parallel_and_validated() { } #[test] -#[ignore = "Set OPENCAD_VIEWPORT_REGRESSION_DXF to the original local regression drawing"] -fn viewport_dimension_original_drawing_keeps_unchanged_measurements() { +#[ignore = "Set OPENCAD_VIEWPORT_REGRESSION_DXF to an external regression fixture"] +fn viewport_dimension_external_fixture_keeps_unchanged_measurements() { use crate::scene::ChangeKind; let path = std::env::var("OPENCAD_VIEWPORT_REGRESSION_DXF").expect("regression drawing path"); let doc = acadrust::DxfReader::from_file(std::path::Path::new(&path)) @@ -820,7 +819,9 @@ fn viewport_dimension_nested_block_path_survives_edit_and_copy() { let root = scene.add_entity(EntityType::Insert(root)); scene.set_current_layout("Dimensions".into()); let transform = - crate::scene::dimension_assoc_chain::block_transform(&scene.document, &[root, nested]); + crate::scene::dimension_assoc_chain::walk_chain(&scene.document, &[root, nested, leaf]) + .unwrap() + .transform; let model = |x| { let v = transform.apply(Vector3::new(x, 0.0, 0.0)); DVec3::new(v.x, v.y, v.z) @@ -1095,3 +1096,249 @@ fn viewport_dimension_quadrant_keeps_its_feature_after_translation() { .bump_entities(&[(source, ChangeKind::Modified)]); assert!((displayed(&app.tabs[i].scene.document) - 400.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); + } + } +} + +#[test] +fn viewport_dimension_paper_centerline_tracks_each_owning_space() { + use crate::scene::{ChangeKind, ReferenceStatus}; + for (paper_first, kind) in [(true, SnapType::Midpoint), (false, SnapType::Perpendicular)] { + let (mut app, model, paper, frame) = + start_centerline_dimension("DIMALIGNED", paper_first, kind, 1.0); + let i = app.active_tab; + let _ = app.feed_command(StepInput::Point(DVec3::new(60.0, 80.0, 0.0))); + let dim = dimension(&app.tabs[i].scene.document).base().common.handle; + assert!((displayed(&app.tabs[i].scene.document) - 50.0).abs() < 1e-4); + let assoc = app.tabs[i].scene.dimension_association(dim).unwrap(); + assert!(assoc.trans_space); + assert!(assoc + .references + .iter() + .flatten() + .any(|r| r.xrefs == vec![paper])); + assert!(assoc + .references + .iter() + .flatten() + .any(|r| r.xrefs == vec![frame.viewport, model])); + assert!(app.tabs[i] + .scene + .dimension_association_status(dim) + .iter() + .all(|(_, s)| *s == ReferenceStatus::Resolved)); + let pending = app.begin_undo(i, "Move paper centerline", 1, true).unwrap(); + let before = app.tabs[i].scene.document.get_entity_arc(paper); + app.tabs[i].scene.record_undo_before(paper, before); + let EntityType::Line(line) = app.tabs[i].scene.document.get_entity_mut(paper).unwrap() + else { + panic!() + }; + line.start.x = 56.0; + line.end.x = 56.0; + app.tabs[i] + .scene + .bump_entities(&[(paper, ChangeKind::Modified)]); + app.commit_undo_delta(i, pending); + assert!((displayed(&app.tabs[i].scene.document) - 40.0).abs() < 1e-4); + app.undo_steps(1); + assert!((displayed(&app.tabs[i].scene.document) - 50.0).abs() < 1e-4); + app.redo_steps(1); + let EntityType::Viewport(vp) = app.tabs[i] + .scene + .document + .get_entity_mut(frame.viewport) + .unwrap() + else { + panic!() + }; + vp.view_height = 500.0; + vp.custom_scale = 0.2; + app.tabs[i].scene.notify_viewport_changed(frame.viewport); + // Model endpoint now appears at paper x=70; paper centerline stays x=56. + assert!((displayed(&app.tabs[i].scene.document) - 70.0).abs() < 1e-4); + let EntityType::Viewport(vp) = app.tabs[i] + .scene + .document + .get_entity_mut(frame.viewport) + .unwrap() + else { + panic!() + }; + vp.view_target.x = 20.0; + app.tabs[i].scene.notify_viewport_changed(frame.viewport); + assert!((displayed(&app.tabs[i].scene.document) - 50.0).abs() < 1e-4); + let EntityType::Line(line) = app.tabs[i].scene.document.get_entity_mut(model).unwrap() + else { + panic!() + }; + line.end.x = 160.0; + app.tabs[i] + .scene + .bump_entities(&[(model, ChangeKind::Modified)]); + assert!((displayed(&app.tabs[i].scene.document) - 110.0).abs() < 1e-4); + for ext in ["dxf", "dwg"] { + let doc = &app.tabs[i].scene.document; + let bytes = crate::io::save_to_bytes(doc, ext, doc.version).unwrap(); + let mut scene = Scene::new(); + scene.document = crate::io::load_bytes(&format!("centerline.{ext}"), bytes).unwrap(); + scene.notify_viewport_changed(frame.viewport); + assert!((displayed(&scene.document) - 110.0).abs() < 1e-4, "{ext}"); + assert!(scene + .dimension_association_status(dim) + .iter() + .all(|(_, s)| *s == ReferenceStatus::Resolved)); + } + let before = app.tabs[i].scene.document.get_entity(dim).unwrap().clone(); + let model_owner = app.tabs[i] + .scene + .document + .get_entity(model) + .unwrap() + .common() + .owner_handle; + app.tabs[i] + .scene + .document + .get_entity_mut(paper) + .unwrap() + .common_mut() + .owner_handle = model_owner; + app.tabs[i].scene.notify_viewport_changed(frame.viewport); + assert_eq!(app.tabs[i].scene.document.get_entity(dim).unwrap(), &before); + assert!(app.tabs[i] + .scene + .dimension_association_status(dim) + .iter() + .any(|(_, s)| *s == ReferenceStatus::Unresolved)); + } +} + +#[test] +fn viewport_dimension_invalid_paths_leave_no_partial_association() { + use crate::scene::viewport_ref::SnapSourceRef; + use acadrust::entities::{DimensionAligned, Insert}; + use acadrust::tables::BlockRecord; + + let (mut app, model, frame) = fixture(); + let scene = &mut app.tabs[app.active_tab].scene; + let mut block = BlockRecord::new("Collapsed"); + block.handle = scene.document.allocate_handle(); + let owner = block.handle; + scene.document.block_records.add(block).unwrap(); + let mut leaf = Line::from_points(Vector3::ZERO, Vector3::UNIT_X); + leaf.common.owner_handle = owner; + let leaf = scene.document.add_entity(EntityType::Line(leaf)).unwrap(); + scene.set_current_layout("Model".into()); + let mut insert = Insert::new("Collapsed", Vector3::ZERO); + insert.set_x_scale(0.0); + insert.set_y_scale(0.0); + let root = scene.add_entity(EntityType::Insert(insert)); + scene.set_current_layout("Dimensions".into()); + let paper = scene.add_entity(EntityType::Line(Line::from_points( + Vector3::new(55.0, 50.0, 0.0), + Vector3::new(55.0, 60.0, 0.0), + ))); + for (source, path) in [(model, vec![Handle::new(u64::MAX)]), (leaf, vec![root])] { + let dim = scene.add_entity(EntityType::Dimension(Dimension::Aligned( + DimensionAligned::new(Vector3::new(50.0, 50.0, 0.0), Vector3::new(55.0, 50.0, 0.0)), + ))); + let invalid = AcceptedSnap { + paper_point: DVec3::new(50.0, 50.0, 0.0), + model_point: DVec3::ZERO, + viewport: Some(frame.viewport), + frame: Some(frame), + source: Some(SnapSourceRef { + source: DimensionAssociationSource::inferred(source), + block_path: path, + snap_type: SnapType::Endpoint, + intersection: None, + }), + }; + let mut valid = AcceptedSnap::free(DVec3::new(55.0, 50.0, 0.0)); + valid.source = Some(SnapSourceRef { + source: DimensionAssociationSource::inferred(paper), + block_path: Vec::new(), + snap_type: SnapType::Endpoint, + intersection: None, + }); + scene.attach_viewport_dimension_association(dim, &[Some(invalid), Some(valid)]); + let association = scene.dimension_association(dim).unwrap(); + assert!(!association.trans_space); + assert!(association.references[0].is_empty()); + assert_eq!(association.references[1][0].xrefs, vec![paper]); + assert_eq!(scene.dimension_association_sources(dim), vec![paper]); + assert_eq!( + scene.dimension_association_status(dim), + vec![(1, crate::scene::ReferenceStatus::Resolved)] + ); + } +} 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/dimension_assoc.rs b/src/scene/dimension_assoc.rs index f8ccb3cb9..3eb93accc 100644 --- a/src/scene/dimension_assoc.rs +++ b/src/scene/dimension_assoc.rs @@ -370,15 +370,7 @@ fn source_marker(entity: &EntityType, point: Vector3) -> Option { // ── Chain-aware reference resolution ────────────────────────────────────── -/// How a resolved feature point travels from the entity it belongs to up into -/// the space the dimension is drawn in. -/// -/// `transform` is the block-instance path (entity-local -> model). -/// `frame` is the layout viewport the chain looks through, when it has one; -/// with a frame the dimension lives on the sheet and every point it holds is -/// a *paper* point, while the value it displays is a *model* length. Keeping -/// both directions in one place is what stops the viewport compensation from -/// being applied twice (see `viewport_ref::measurement_rule`). +/// Transform an entity-local feature through its block path and optional viewport. #[derive(Clone, Copy, Debug)] pub(crate) struct ChainMap { pub transform: Transform, @@ -528,21 +520,10 @@ fn hint_error(entity: &EntityType, point: Vector3) -> f64 { f64::INFINITY } -/// The stored osnap point, brought into the entity's own coordinates. -/// -/// Which space the file wrote it in is not recoverable from the record: our -/// own pre-PR3 writer stored a model/WCS point, AutoCAD's trans-space records -/// hold the point in the space the dimension is drawn in, and both appear in -/// the same drawing. Rather than guess, both readings are mapped down to the -/// entity and the one that actually lands on the geometry wins. The point is -/// only ever a *hint* — it disambiguates candidates, it is never the answer. -fn local_hint( - scene: &Scene, - map: &ChainMap, - entity: &EntityType, - stored: Vector3, -) -> Option { - let _ = scene; +/// Interpret a stored point as a source-space or dimension-space hint. +/// Validate both interpretations against the geometry before using the hint +/// to disambiguate a feature; the stored point is never the resolved result. +fn local_hint(map: &ChainMap, entity: &EntityType, stored: Vector3) -> Option { let inverse = chain::invert(&map.transform)?; let mut candidates = vec![inverse.apply(stored)]; if map.frame.is_some() { @@ -610,7 +591,7 @@ pub fn resolve_reference_chain( .max(reference.osnap_point.z.abs()) < 1e40 { - local_hint(scene, &map, entity, reference.osnap_point) + local_hint(&map, entity, reference.osnap_point) } else { None }; @@ -636,9 +617,8 @@ pub fn resolve_reference_chain( }) } -/// The pre-PR3 marker/parameter resolution, unchanged in behaviour but now -/// evaluated in the *entity's own* coordinates rather than assumed to be in -/// model space. Lifting through the block path is the caller's job. +/// Resolve a marker/parameter in entity-local coordinates. +/// The caller applies the block and viewport transforms. fn resolve_local_by_marker( entity: &EntityType, reference: &AssocDimensionReference, @@ -1224,8 +1204,7 @@ impl Scene { } /// Persist the association for a dimension whose points were picked - /// through [`AcceptedSnap`]s — the entry point PR2 calls once it has - /// created a paper-space dimension that measures model geometry. + /// through [`AcceptedSnap`]s, preserving each source and its owning space. /// /// `snaps` is positional: one slot per definition point, in the same order /// [`dimension_reference_points`] returns them (linear/aligned: first, @@ -1245,13 +1224,7 @@ impl Scene { else { return; }; - // A radial dimension has ONE reference, and it is an - // `AcDbOsnapPointRef` of osnap type 10 carrying the *radial* marker - // (which circle / which polyline arc segment) and the angle around it, - // not a vertex marker. `source_reference` would return the generic - // curve-parameter marker `-2`, which `radial_source_for_marker` cannot - // find, so radial goes down its own path — the same one - // `attach_dimension_association_sources` uses for model space. + // Radial references identify a circle/arc and angle, rather than a vertex. let radial_dimension = matches!( dimension_entity, Dimension::Radius(_) | Dimension::Diameter(_) | Dimension::LargeRadial(_) @@ -1278,18 +1251,17 @@ impl Scene { let mut xrefs = Vec::new(); if let Some(viewport) = snap.viewport { xrefs.push(viewport); - trans_space = true; } xrefs.extend(source.block_path.iter().copied()); xrefs.push(entity_handle); - reactor_targets.extend(xrefs.iter().copied()); - + let Ok(walked) = chain::walk_chain(&self.document, &xrefs) else { + continue; + }; + let Some(inverse) = chain::invert(&walked.transform) else { + continue; + }; + let local = inverse.apply(dvec3(snap.model_point)); let osnap_type = chain::osnap_type_for(source.snap_type); - let local = Self::point_in_entity_space( - &self.document, - &source.block_path, - dvec3(snap.model_point), - ); let marker_parameter = source .source .marker @@ -1314,6 +1286,8 @@ impl Scene { } reactor_targets.extend(intersection_objects.iter().copied()); } + trans_space |= snap.viewport.is_some(); + reactor_targets.extend(xrefs.iter().copied()); associativity |= 1 << index; references[index].push(AssocDimensionReference { class_name: "AcDbOsnapPointRef".to_string(), @@ -1323,8 +1297,8 @@ impl Scene { main_subent_type: 1, main_gs_marker: marker, osnap_distance: parameter, - // Stored in MODEL space. `local_hint` reads either convention, - // so an imported paper-space point still resolves. + // Store the source's owning-space point: model coordinates for + // viewport features, paper coordinates for direct sheet features. osnap_point: dvec3(snap.model_point), ..AssocDimensionReference::default() }); @@ -1351,12 +1325,7 @@ impl Scene { dimension: Handle, snaps: &[Option], ) { - let Some(snap) = snaps - .iter() - .flatten() - .find(|snap| snap.source.is_some()) - .cloned() - else { + let Some(snap) = snaps.iter().flatten().find(|snap| snap.source.is_some()) else { return; }; let source = snap.source.as_ref().expect("filtered above"); @@ -1364,14 +1333,17 @@ impl Scene { let Some(entity) = self.document.get_entity(entity_handle) else { return; }; - // The acquired point in the source entity's own coordinates, so the - // radial geometry and the angle are both computed where the entity - // lives. - let local = Self::point_in_entity_space( - &self.document, - &source.block_path, - dvec3(snap.model_point), - ); + let mut xrefs = Vec::new(); + xrefs.extend(snap.viewport); + xrefs.extend(source.block_path.iter().copied()); + xrefs.push(entity_handle); + let Ok(walked) = chain::walk_chain(&self.document, &xrefs) else { + return; + }; + let Some(inverse) = chain::invert(&walked.transform) else { + return; + }; + let local = inverse.apply(dvec3(snap.model_point)); let radial = match source.source.marker { Some(marker) => radial_source_for_marker(entity, marker), None => radial_source_at(entity, local), @@ -1382,14 +1354,7 @@ impl Scene { } else { radial.angle_at(dpoint(local)) }; - let mut xrefs = Vec::new(); - let mut trans_space = false; - if let Some(viewport) = snap.viewport { - xrefs.push(viewport); - trans_space = true; - } - xrefs.extend(source.block_path.iter().copied()); - xrefs.push(entity_handle); + let trans_space = snap.viewport.is_some(); let reactor_targets = xrefs.clone(); let reference = AssocDimensionReference { class_name: "AcDbOsnapPointRef".to_string(), @@ -1431,28 +1396,64 @@ impl Scene { } } - /// A model-space point brought down into the coordinates of the entity at - /// the end of `block_path`. Identity for top-level geometry. - fn point_in_entity_space( - document: &acadrust::CadDocument, - block_path: &[Handle], - point: Vector3, - ) -> Vector3 { - if block_path.is_empty() { - return point; + /// Select one measurement viewport while allowing direct references to + /// geometry owned by the dimension's paper layout. A model-space chain + /// without its viewport cannot be interpreted as a paper-space anchor. + fn association_measurement_frame( + &self, + association: &AssocDimensionAssociation, + ) -> Result, ()> { + let mut viewport = None; + for reference in association.references.iter().flatten() { + for handles in [&reference.xrefs, &reference.intersection_objects] { + let Some(root) = handles.first() else { + continue; + }; + if matches!( + self.document.get_entity(*root), + Some(EntityType::Viewport(_)) + ) { + if viewport.is_some_and(|previous| previous != *root) { + return Err(()); + } + viewport = Some(*root); + } + } } - chain::invert(&chain::block_transform(document, block_path)) - .map(|inverse| inverse.apply(point)) - .unwrap_or(point) + let Some(viewport) = viewport else { + return if association.trans_space { + Err(()) + } else { + Ok(None) + }; + }; + let owner = self + .document + .get_entity(association.dimension) + .ok_or(())? + .common() + .owner_handle; + for reference in association.references.iter().flatten() { + for handles in [&reference.xrefs, &reference.intersection_objects] { + let Some(root) = handles.first() else { + continue; + }; + if self + .document + .get_entity(*root) + .ok_or(())? + .common() + .owner_handle + != owner + { + return Err(()); + } + } + } + self.viewport_frame(viewport).map(Some).ok_or(()) } - /// Per-slot health of a dimension's association, for the UI and for tests. - /// - /// Nothing here is persisted: brokenness is *derived* from whether the - /// chain still resolves. That is what makes undo restore a link for free — - /// the erased source comes back and the same query starts answering - /// [`ReferenceStatus::Resolved`] again, with the reference record itself - /// never having been rewritten. + /// Resolve each reference's current status without changing its saved data. pub fn dimension_association_status(&self, dimension: Handle) -> Vec<(usize, ReferenceStatus)> { let Some(association) = self.dimension_association(dimension) else { return Vec::new(); @@ -1485,8 +1486,15 @@ impl Scene { .is_some() }); let measurement_supported = measurement_supported - && association.references.iter().flatten() - .filter(|r| chain::needs_from_point(r.osnap_type)).count() <= 1; + && association + .references + .iter() + .flatten() + .filter(|r| chain::needs_from_point(r.osnap_type)) + .count() + <= 1; + let measurement_supported = + measurement_supported && self.association_measurement_frame(association).is_ok(); let points = self.dimension_association_slot_points(dimension); association .references @@ -1519,7 +1527,7 @@ impl Scene { /// /// Layer visibility and viewport clipping are deliberately not consulted: /// hiding a layer or clipping a viewport changes what a *new* snap may - /// acquire (PR1's concern), never what an existing association points at. + /// acquire, never what an existing association points at. pub fn dimension_association_sources(&self, dimension: Handle) -> Vec { let Some(association) = self.dimension_association(dimension) else { return Vec::new(); @@ -1553,15 +1561,8 @@ impl Scene { }) } - /// Duplicate dimension associations across a COPY/MIRROR/ARRAY handle map. - /// - /// A copied dimension only keeps an association when *every* handle in - /// every chain was copied too — the viewport, each INSERT on the path and - /// the source entity. Then the whole chain is remapped onto the copies, so - /// the copy measures the copied geometry. Copying a dimension on its own - /// leaves it with no association at all, which is exactly what the model - /// space path did before this: an all-or-nothing rule, never a copy that - /// silently drives off the original's geometry. + /// Preserve an association only when the dimension and every source-chain + /// handle were copied. Remap the complete chain to the new entities. pub(crate) fn copy_dimension_associations( &mut self, handle_map: &rustc_hash::FxHashMap, @@ -1591,7 +1592,7 @@ impl Scene { if !complete { return None; } - let remap = |handles: &Vec| -> Vec { + let remap = |handles: &[Handle]| -> Vec { handles .iter() .map(|handle| handle_map.get(handle).copied().unwrap_or(*handle)) @@ -1640,15 +1641,7 @@ impl Scene { } } - /// Report a layout viewport as changed so the dimensions drawn through it - /// are regenerated. - /// - /// A viewport move / pan / zoom / twist changes where model geometry lands - /// on the sheet without touching that geometry, so nothing in the ordinary - /// entity-change path would notice. Every reference that looks through a - /// viewport carries that viewport's handle as `xrefs[0]`, so announcing the - /// viewport itself is all that is needed — the existing dependency lookup - /// in `refresh_associative_dimensions` finds the associations from there. + /// Refresh dimensions when their viewport moves, pans, zooms, or twists. pub fn notify_viewport_changed(&mut self, viewport: Handle) { if matches!( self.document.get_entity(viewport), @@ -1750,17 +1743,25 @@ impl Scene { .chain(reference.intersection_objects.iter()) .any(|handle| changed.contains(handle)) }) - .then_some(association.clone()) + .then(|| association.clone()) }) .collect(); let mut refreshed = Vec::new(); for association in associations { + let Ok(frame) = self.association_measurement_frame(&association) else { + continue; + }; // Dependent snaps need a resolved independent definition point. // Two such features require a coupled solve; old points are not // evidence of a solution after the geometry changes. - if association.references.iter().flatten() - .filter(|r| chain::needs_from_point(r.osnap_type)).count() > 1 + if association + .references + .iter() + .flatten() + .filter(|r| chain::needs_from_point(r.osnap_type)) + .count() + > 1 { continue; } @@ -1806,27 +1807,20 @@ impl Scene { { continue; } - let viewports: std::collections::HashSet<_> = - resolved.iter().flatten().map(|r| r.viewport).collect(); - if viewports.len() > 1 { - continue; - } - // The map of the first live reference governs how the whole - // dimension is placed and measured — all of a dimension's points - // are in one space. - let map = resolved - .iter() - .flatten() - .next() - .and_then(|value| value.viewport) - .and_then(|handle| self.viewport_frame(handle)) + // Every point is already in paper space: direct sheet references + // stay there, and model references are projected individually. + // Only the measurement factor is shared by the whole dimension. + let map = frame .map(|frame| ChainMap { transform: Transform::identity(), frame: Some(frame), }) .unwrap_or_else(ChainMap::identity); - let model_points: [Option; 4] = - std::array::from_fn(|index| resolved[index].map(|value| value.model)); + let model_points: [Option; 4] = std::array::from_fn(|index| { + resolved[index] + .filter(|value| value.viewport.is_some()) + .map(|value| value.model) + }); let resolved: [Option; 4] = std::array::from_fn(|index| resolved[index].map(|value| value.space)); @@ -2117,18 +2111,7 @@ impl Scene { Dimension::Linear(linear) => linear_measurement(linear), _ => dimension.measurement(), }; - // `actual_measurement` stays RAW, in the dimension's own space — - // paper, for a dimension drawn on the sheet through a viewport. - // The viewport compensation is carried by the negative DIMLFAC - // override and applied once, by the formatter. See - // `viewport_ref::measurement_rule`: substituting the model - // distance here (or multiplying by `MeasurementScale::model_factor`) - // would make a refreshed dimension read a different number from an - // identical freshly-created one. - // - // The similarity property of a viewport frame is what makes this - // exact: paper_span * (1 / scale) is the model span, for a rotated - // linear projection just as much as for a free distance. + // Store the raw sheet measurement; the formatter applies DIMLFAC once. debug_assert!( map.frame.is_none() || model_points @@ -2176,12 +2159,7 @@ impl Scene { } } -/// How far the midpoint of a dimension's measured points moved. -/// -/// Used to carry the sheet annotation — the dimension line's standoff and a -/// user-dragged text position — along with the geometry. Slots that did not -/// resolve keep their old point, so a one-ended edit shifts the annotation by -/// half the movement, which is what keeps the dimension line centred. +/// Average displacement of the dimension's resolved definition points. fn centroid_shift(before: [Option; 2], after: [Option; 2]) -> Vector3 { let mut delta = Vector3::new(0.0, 0.0, 0.0); let mut count = 0.0; @@ -2194,8 +2172,6 @@ fn centroid_shift(before: [Option; 2], after: [Option; 2]) -> if count == 0.0 { return Vector3::new(0.0, 0.0, 0.0); } - // Averaged over the slots that actually moved, not over both, so a - // dimension with one broken reference still tracks the live one. Vector3::new(delta.x / count, delta.y / count, delta.z / count) } diff --git a/src/scene/dimension_assoc_chain.rs b/src/scene/dimension_assoc_chain.rs index 2bcf31342..6cd56aa7c 100644 --- a/src/scene/dimension_assoc_chain.rs +++ b/src/scene/dimension_assoc_chain.rs @@ -1,38 +1,8 @@ -//! Full reference chains for associative dimensions. -//! -//! An `AssocDimensionReference` addresses a *feature* — an osnap point on a -//! piece of geometry — through a chain of handles: -//! -//! ```text -//! dimension -> [viewport] -> [insert ...] -> entity -> feature -//! ``` -//! -//! `AssocDimensionReference::xrefs` is that chain, outermost first. AutoCAD -//! calls the same list `mainObjectIds`; `intersection_objects` -//! (`intersectObjectIds`) is the second chain an INTERSECTION / APPARENT -//! INTERSECTION osnap needs. -//! -//! The pre-PR3 resolver only ever read `xrefs[0]`, which is correct for a -//! model-space dimension on top-level geometry and wrong for everything else: -//! a paper-space dimension measuring through a layout viewport stores the -//! VIEWPORT as `xrefs[0]`, so the resolver stopped at a viewport entity, found -//! no curve on it and gave up. Nine of the ten association objects in the -//! 115448 regression drawing are of that shape. -//! -//! This module owns: -//! * [`walk_chain`] — turn `xrefs` into a viewport, a block-instance path -//! with its accumulated transform, and the innermost entity. -//! * [`feature_point`] — evaluate the osnap feature on that entity, in the -//! entity's own coordinates, then lift it through the block path into -//! model space. -//! -//! Mapping model -> paper through the viewport frame is deliberately *not* -//! done here; the caller owns it, because only the caller knows whether the -//! association is `trans_space` and which [`ViewportFrame`] is current. -//! -//! All arithmetic is `f64`. -//! -//! [`ViewportFrame`]: super::viewport_ref::ViewportFrame +//! Resolve a dimension feature through `[viewport?, insert..., entity]`. +//! `xrefs` stores the primary chain; `intersection_objects` stores the second +//! intersection source. Features are evaluated in entity-local coordinates and +//! transformed through their block instances. The caller projects viewport +//! references onto the sheet. All arithmetic uses `f64`. use acadrust::objects::AssocDimensionReference; use acadrust::types::{Handle, Matrix4, Transform, Vector3}; @@ -45,12 +15,8 @@ use crate::entities::curve::entity_curve; /// `AcDb::OsnapMode`, the value stored in /// [`AssocDimensionReference::osnap_type`]. /// -/// These are the AutoCAD ObjectARX codes, not our own [`crate::snap::SnapType`] -/// discriminants. The pre-PR3 writer already emitted `END` for generic points -/// and `NEAR` for a point taken at a parameter on a circle, so those two keep -/// meaning exactly what they used to. -#[allow(dead_code)] // The full code table is kept so a reader can see what a - // stored `osnap_type` means, including the modes we pass through untouched. +/// These are ObjectARX codes, independent of [`crate::snap::SnapType`]. +#[allow(dead_code)] // Include codes preserved from unsupported imported modes. pub(crate) mod osnap { pub const NONE: u8 = 0; pub const END: u8 = 1; @@ -72,7 +38,7 @@ pub(crate) mod osnap { /// /// Modes with no ObjectARX equivalent (grid, object pick) map to /// [`osnap::NONE`]; a reference carrying `NONE` resolves by marker/parameter -/// alone, exactly like the pre-PR3 records. +/// alone. pub(crate) fn osnap_type_for(snap: crate::snap::SnapType) -> u8 { use crate::snap::SnapType as S; match snap { @@ -166,7 +132,11 @@ pub(crate) fn walk_chain( return Err(ChainError::Invalid); }; // Arrays need an instance index that this reference format does not carry. - if insert.row_count > 1 || insert.column_count > 1 || block_path.contains(&handle) { + if insert.row_count > 1 + || insert.column_count > 1 + || block_path.len() >= 8 + || block_path.contains(&handle) + { return Err(ChainError::Invalid); } let block = document @@ -182,64 +152,23 @@ pub(crate) fn walk_chain( Err(ChainError::Empty) } -/// Accumulated entity-local -> model transform for an ordered INSERT path. -pub(crate) fn block_transform(document: &CadDocument, block_path: &[Handle]) -> Transform { - let mut transform = Transform::identity(); - for handle in block_path { - if let Some(EntityType::Insert(insert)) = document.get_entity(*handle) { - transform = transform.compose(&crate::scene::render_graph::insert_transform( - document, insert, - )); - } - } - transform -} - -/// Affine inverse of `transform`. -/// -/// `acadrust::types::Matrix4` has no inverse of its own, and a block path only -/// ever produces affine matrices (translate / rotate / scale), so this inverts -/// the 3x3 linear part by cofactors and back-substitutes the translation. A -/// degenerate (zero-scale) insert returns `None` rather than exploding. +/// Invert an affine block transform, rejecting singular or nonfinite matrices. pub(crate) fn invert(transform: &Transform) -> Option { let m = &transform.matrix.m; - let a = [ - [m[0][0], m[0][1], m[0][2]], - [m[1][0], m[1][1], m[1][2]], - [m[2][0], m[2][1], m[2][2]], - ]; - let det = a[0][0] * (a[1][1] * a[2][2] - a[1][2] * a[2][1]) - - a[0][1] * (a[1][0] * a[2][2] - a[1][2] * a[2][0]) - + a[0][2] * (a[1][0] * a[2][1] - a[1][1] * a[2][0]); - if !det.is_finite() || det.abs() < 1e-18 { + let matrix = glam::DMat4::from_cols_array_2d(&std::array::from_fn(|column| { + std::array::from_fn(|row| m[row][column]) + })); + if !matrix.is_finite() || matrix.determinant().abs() < 1e-18 { return None; } - let inv_det = 1.0 / det; - let mut inverse = [[0.0f64; 3]; 3]; - for row in 0..3 { - for col in 0..3 { - // Cofactor of (col, row) — transposed, which is the adjugate. - let (r0, r1) = ((col + 1) % 3, (col + 2) % 3); - let (c0, c1) = ((row + 1) % 3, (row + 2) % 3); - inverse[row][col] = (a[r0][c0] * a[r1][c1] - a[r0][c1] * a[r1][c0]) * inv_det; - } + let inverse = matrix.inverse(); + if !inverse.is_finite() { + return None; } - let t = [m[0][3], m[1][3], m[2][3]]; - let translation = [ - -(inverse[0][0] * t[0] + inverse[0][1] * t[1] + inverse[0][2] * t[2]), - -(inverse[1][0] * t[0] + inverse[1][1] * t[1] + inverse[1][2] * t[2]), - -(inverse[2][0] * t[0] + inverse[2][1] * t[1] + inverse[2][2] * t[2]), - ]; - Some(Transform { - matrix: Matrix4 { - m: [ - [inverse[0][0], inverse[0][1], inverse[0][2], translation[0]], - [inverse[1][0], inverse[1][1], inverse[1][2], translation[1]], - [inverse[2][0], inverse[2][1], inverse[2][2], translation[2]], - [0.0, 0.0, 0.0, 1.0], - ], - }, - }) + let columns = inverse.to_cols_array_2d(); + Some(Transform::from_matrix(Matrix4 { + m: std::array::from_fn(|row| std::array::from_fn(|column| columns[column][row])), + })) } // ── Feature evaluation ──────────────────────────────────────────────────── @@ -482,9 +411,7 @@ fn nearest_point( hint: Option, ) -> Option { let planar = entity_curve(entity)?; - // Circles and arcs carry an *angle* in `osnap_distance` — that is the - // convention the pre-PR3 writer established and imported files share it. - // Every other curve carries the kernel's normalised 0..1 parameter. + // Circles and arcs store an angle; other curves use a normalized parameter. match &planar.curve { KernelCurve::Circle(_) | KernelCurve::Arc(_) => match entity { EntityType::Circle(circle) => Some(circle.point_at_angle_wcs(reference.osnap_distance)), @@ -506,14 +433,8 @@ fn nearest_point( // ── Tangent ─────────────────────────────────────────────────────────────── -/// Tangent point on `entity` for the touch line through `context.from`. -/// -/// Circles and arcs have a closed form. A spline does not, so the stored -/// parameter (`osnap_distance`) seeds a Newton refinement of the tangency -/// condition `(C(u) - P) · C'(u) = 0`; with no usable parameter the seed is -/// the point on the spline nearest the stored osnap point. That is what makes -/// the 115448 drawing's tangent-to-spline reference survive an edit to the -/// spline instead of silently freezing at its imported location. +/// Tangent point for the line through `context.from`. +/// Spline tangency solves `(C(u) - P) x C'(u) = 0` and validates the residual. fn tangent_point( entity: &EntityType, reference: &AssocDimensionReference, diff --git a/src/scene/mod.rs b/src/scene/mod.rs index 431160c3b..3ac3b5d0d 100644 --- a/src/scene/mod.rs +++ b/src/scene/mod.rs @@ -43,7 +43,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 a4928e9a0..3f804f11a 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 84% rename from src/scene/viewport_dim_seam.rs rename to src/scene/viewport_dimension_pick.rs index efb62e1b5..c5d87e7d2 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(), @@ -188,19 +158,14 @@ 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, - kind, - )?; - Some((found.0, found.1)) - } + EntityType::Insert(insert) => self.descend_block_instance( + insert, + Transform::identity(), + model_point, + 0, + viewport, + kind, + ), other => { feature_pick_distance(other, model_point, kind)?; Some((other.clone(), Vec::new())) @@ -212,21 +177,17 @@ 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, kind: 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; } @@ -234,10 +195,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; }; @@ -249,13 +209,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, kind, @@ -300,8 +258,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 2f861d862..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 @@ -318,39 +263,6 @@ impl MeasurementScale { } } -/// # Where the viewport compensation lives, once -/// -/// A dimension that sits on a paper layout but measures model geometry seen -/// through a viewport has **paper** definition points and displays a **model** -/// number. Exactly one rule governs that, everywhere: -/// -/// > `Dimension::actual_measurement` (DXF group 42) is always the **raw** -/// > geometric measurement in the dimension's *own* space — paper, for a -/// > viewport dimension. The viewport compensation lives **only** in the -/// > magnitude of the negative DIMLFAC override -/// > ([`MeasurementScale::viewport_dimlfac_override`]), and is applied -/// > **once**, at format time, by the linear formatter. -/// -/// So for a 1:10 viewport and a user DIMLFAC of 1: a 100-unit model line is -/// 10 paper units, `actual_measurement` is `10`, the override is `-10`, -/// [`MeasurementScale::user_lfac_for_space`] resolves that to `10` for a -/// paper-space dimension, and the text reads `100`. -/// -/// The consequence for association refresh -/// (`Scene::refresh_associative_dimensions`) is that after re-placing a -/// viewport dimension's definition points on the sheet it must write the -/// **paper** measurement back — it must *not* substitute the model distance, -/// and it must *not* multiply by [`MeasurementScale::model_factor`]. Doing -/// either makes a refreshed dimension read a different number from an -/// identical freshly-created one (`user_lfac` too many times). This is the -/// rule that makes `attach_viewport_dimension_association` + refresh agree -/// with creation by construction, and it is why `actual_measurement` stays -/// raw: that is also what the DXF format specifies and what export writes. -/// -/// Angular dimensions are exempt on both counts — a similarity preserves -/// angles, so there is no compensation and no override. -pub mod measurement_rule {} - #[cfg(test)] mod tests { use super::*; @@ -385,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 e8f5b3c7d..ea71b6675 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,7 +90,7 @@ 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. @@ -102,7 +102,7 @@ pub struct SnapResult { /// the instance path needs the document and the accepted model point, so /// it happens once at accept time, in /// `OpenCADStudio::resolve_source_block_path`, which fills - /// [`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, @@ -996,10 +996,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, }) } @@ -1234,7 +1234,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; @@ -1261,10 +1262,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, }); } }; @@ -1318,7 +1319,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 @@ -1338,7 +1341,9 @@ impl Snapper { try_pt( wp_f64(wire, wire.points.len() - 1), SnapType::Endpoint, - wire_source(wire), None); + wire_source(wire), + None, + ); } } } else { @@ -1361,7 +1366,9 @@ impl Snapper { try_pt( wp_f64(wire, wire.points.len() - 1), SnapType::Endpoint, - src, None); + src, + None, + ); } } } @@ -1412,7 +1419,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() { @@ -1436,7 +1445,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 { @@ -1465,7 +1476,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, + ); } } } @@ -1563,7 +1576,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)); @@ -1600,7 +1613,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; } @@ -1646,7 +1661,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), + ); } } } @@ -1772,7 +1792,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; } @@ -1812,7 +1834,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), + ); } } } @@ -2033,10 +2057,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, }); } } @@ -2206,19 +2230,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)); @@ -2235,6 +2253,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 { diff --git a/tests/fixtures/dim_assoc/README.md b/tests/fixtures/dim_assoc/README.md index a006911b7..4a01c475d 100644 --- a/tests/fixtures/dim_assoc/README.md +++ b/tests/fixtures/dim_assoc/README.md @@ -1,16 +1,10 @@ # Viewport association fixture -`viewport_associations.dxf` is a synthetic paper-space fixture for line, circle, -arc, polyline, spline, block-instance, and intersection references through one -1:2 viewport. `src/app/viewport_dimension_tests.rs` loads it and exercises actual -association refresh and an edit to the second intersection source. +`viewport_associations.dxf` contains line, circle, arc, polyline, spline, +block-instance, and intersection references through a 1:2 viewport. +`src/app/viewport_dimension_tests.rs` tests refresh and intersection source edits. -Each dimension carries `DIMLFAC=-2` and `ACAD_DIMASSOC_CALC_DIMLFAC=-2`. These are -necessary for its paper definition points and saved model measurements to agree; -the original fixture omitted both factors. The spline tangent also uses the -actual touch point `(90, 283.125)` at parameter `0.75`, measured from `(0, 300)`. -The original endpoint `(120, 300)` was not tangent. Tests verify the cross product -with the spline derivative and preservation after an unsupported reference edit. - -The larger original `115448.dxf` is private and is not included. An opt-in test -uses `OPENCAD_VIEWPORT_REGRESSION_DXF` to read a local copy without modifying it. +Each dimension stores `DIMLFAC=-2` and `ACAD_DIMASSOC_CALC_DIMLFAC=-2` so its paper +definition points measure in model units. The spline tangent from `(0, 300)` +touches `(90, 283.125)` at parameter `0.75`; tests verify its tangency and preserve +the dimension if an edited reference cannot resolve.