Use viewport snap frame and accepted snaps for dimension measurement

Removes PR2's two seams now that PR1 (viewport snapping) is underneath.

* `Scene::viewport_frame` is PR1's camera-derived implementation. The
  viewport-record version is gone; PR1's returns `None` for oblique /
  perspective viewports, which is what we want — every entry point in
  `viewport_dim_seam` then declines rather than guessing a planar
  mapping. `viewport_dim_seam` is now just the dimension object-pick
  path, and its module doc says so.

* There is one accepted-snap store: PR1's `OpenCADStudio::accepted_snaps`.
  PR2's parallel `dim_accepted_snaps` (and its command-name ownership
  tracking, which PR1's clear-on-command-start/end already does) is
  deleted, along with the `record_dimension_accepted_snap` seam.
  `accepted_snap_for_point` reads PR1's list.

  Two things layer on top:
  - the explicit object pick is not a point step, so it still records its
    own snaps, now through `push_accepted_snap` into PR1's list, and now
    carrying the picked entity handle + block path as a `SnapSourceRef`
    so PR3 can associate it;
  - a typed coordinate carries no snap, so `measure_snap` re-attaches the
    content viewport under a snap that is free *and* source-less. That
    keeps `DIMLINEAR` with typed coordinates inside a 1:10 viewport
    measuring the model, without ever reinterpreting a real paper-sheet
    snap that happens to sit over a viewport.

* One association gate, not two. `dimension_association_allowed` is the
  single predicate, derived from the same `dimension_measure_space`
  classification that decides whether to compensate the measurement, so
  the two can never disagree. `infer_dimension_sources_guarded` delegates
  to it, and the commit path's "commit unassociated when measured through
  a viewport" condition is now that predicate rather than a second,
  independently-computed bool. It also covers the explicit-pick and
  explicit-source-list branches, which the old inference-only guard did
  not. Because the classification ignores the trailing dimension-line
  placement click, an ordinary paper dimension whose placement click lands
  inside a viewport rectangle keeps its association.

Co-Authored-By: Claude Fable 5.1 <[email protected]>
This commit is contained in:
Christopher OverbeckandClaude Fable 5.1 committed 2026-09-15 12:45:59 -05:00
1 parent 848f789018
commit 43dea41498
5 files changed
+170 -177

No files matched your search

+12 -23
View File
@@ -454,17 +454,6 @@ impl OpenCADStudio {
}
}
if matches!(&input, StepInput::Point(_)) { self.refresh_command_point_pick_context(i); }
// PR2: remember which viewport (if any) each dimension point was
// picked through, before the command turns it into geometry.
if let StepInput::Point(p) = &input {
if self.tabs[i]
.active_cmd
.as_ref()
.is_some_and(|command| command.measures_through_viewports())
{
self.record_dimension_accepted_snap(i, *p);
}
}
let ctrl = self.ctrl_down;
let shift = self.shift_down;
let result: Option<CmdResult> = {
@@ -2117,15 +2106,17 @@ impl OpenCADStudio {
acadrust::entities::Dimension::Ordinate(_)
)
);
// PR2: a dimension placed on the sheet but measuring model
// geometry through a viewport carries the compensation as a
// negative DIMLFAC override. It is deliberately *not*
// associated: its definition points are projected paper
// coordinates, so inferring a paper-space association would
// bind it to geometry that is not there. PR3 records the real
// dimension -> viewport -> entity chain.
let measured_through_viewport =
self.apply_viewport_dimension_measurement(i, &mut entity);
// A dimension placed on the sheet but measuring model geometry
// through a viewport carries the compensation as a negative
// DIMLFAC override.
self.apply_viewport_dimension_measurement(i, &mut entity);
// ...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`.
let association_allowed = self.dimension_association_allowed(i);
let inherited_dimension = if preserve_base_style {
match &entity {
acadrust::EntityType::Dimension(dimension) => Some((
@@ -2192,7 +2183,7 @@ impl OpenCADStudio {
entity,
preserve_base_style,
) {
if association_mode == 2 && !measured_through_viewport {
if association_mode == 2 && association_allowed {
let mut changes = vec![
(handle, crate::scene::ChangeKind::Modified),
];
@@ -2236,7 +2227,6 @@ impl OpenCADStudio {
self.tabs[i].dirty = true;
self.tabs[i].scene.clear_preview_wire();
self.tabs[i].snap_result = None;
self.clear_dimension_accepted_snaps();
if let Some(pd) = pending {
self.commit_undo_delta(i, pd);
}
@@ -2254,7 +2244,6 @@ impl OpenCADStudio {
}
}
CmdResult::CommitDimensionsAndExit(dimensions) => {
self.clear_dimension_accepted_snaps();
let association_mode = self.tabs[i]
.scene
.document
+105 -84
View File
@@ -5,29 +5,42 @@
//! *report* the model measurement. Because a viewport frame is a similarity
//! (uniform scale plus in-plane twist), the model measurement of any linear,
//! aligned or radial span is exactly the raw paper measurement times the
//! viewport compensation `1 / scale` — projecting the model span onto the
//! viewport compensation `1 / scale` -- projecting the model span onto the
//! rotated paper axis and compensating is the same number. So no geometry is
//! rewritten: the compensation is persisted once, as a negative DIMLFAC
//! override on the dimension (see
//! [`MeasurementScale::viewport_dimlfac_override`]), and applied once, by the
//! linear formatter.
//!
//! Angular dimensions are never compensated — a similarity preserves angles.
//! Angular dimensions are never compensated -- a similarity preserves angles.
//!
//! ## PR1 seam
//! [`OpenCADStudio::record_dimension_accepted_snap`] is the one function PR1
//! replaces. PR1 records a real [`AcceptedSnap`] per accepted point (with the
//! snapped source and the frame in effect at snap time); this branch derives
//! the same information from the cursor's paper position and the content
//! viewport under it. Everything downstream consumes `AcceptedSnap` only.
//! ## Where the snaps come from
//! The per-point [`AcceptedSnap`] list is PR1's
//! ([`OpenCADStudio::accepted_snaps`]): the click handler records the real
//! snap for every interactive pick, and `apply_step_input` records a free
//! snap for typed / dynamic-input / headless points. There is exactly one
//! source of truth. Two things are layered on top here:
//!
//! * the explicit object-pick path ([`Scene::dimension_pick_through_viewport`])
//! is not a point step at all, so it records its own snaps
//! ([`OpenCADStudio::record_dimension_viewport_pick`]);
//! * a *typed* coordinate carries no snap, so PR1 records it as a free point
//! with no viewport. [`OpenCADStudio::measure_snap`] re-attaches the
//! content viewport under such a point, which is what makes
//! `DIMLINEAR` + typed coordinates inside a 1:10 viewport measure the model.
//! It is deliberately limited to snaps that are free *and* source-less, so a
//! genuine paper-sheet snap that happens to sit over a viewport is never
//! reinterpreted.
use acadrust::entities::Dimension;
use acadrust::types::Handle;
use acadrust::xdata::XDataValue;
use acadrust::EntityType;
use glam::DVec3;
use crate::command::DimensionAssociationSource;
use crate::entities::dim_override;
use crate::scene::viewport_ref::{AcceptedSnap, MeasurementScale, ViewportFrame};
use crate::scene::viewport_ref::{AcceptedSnap, MeasurementScale, SnapSourceRef, ViewportFrame};
use super::OpenCADStudio;
@@ -46,101 +59,106 @@ pub(crate) enum DimensionMeasureSpace {
}
impl OpenCADStudio {
// ───────────────────────── PR1 SEAM: BEGIN ──────────────────────────
/// Record the snap accepted for one collected point of the active
/// command.
/// Record the accepted snaps produced by the explicit object-pick path (a
/// dimension "select object" pick resolved through a content viewport).
///
/// PR1 replaces the body with the real snap result (which knows the
/// snapped entity, the block path and the frame at snap time). Until then
/// the viewport is resolved from the paper position of the accepted point,
/// which is correct for every point that lands inside a content viewport
/// and yields a free snap everywhere else.
pub(crate) fn record_dimension_accepted_snap(&mut self, i: usize, point: DVec3) {
self.sync_dimension_snap_owner(i);
let snap = match self.tabs[i].scene.viewport_frame_at_paper_point(point) {
Some(frame) => AcceptedSnap {
paper_point: point,
model_point: frame.paper_to_model(point),
viewport: Some(frame.viewport),
frame: Some(frame),
source: None,
},
None => AcceptedSnap::free(point),
};
self.dim_accepted_snaps.push(snap);
}
// ────────────────────────── PR1 SEAM: END ───────────────────────────
/// Record an accepted snap produced by the explicit object-pick path (a
/// pick resolved through a content viewport). Independent of PR1.
/// An object pick is not a point step, so PR1's click handler never sees
/// it. One pick supplies *both* extension origins, so it is recorded
/// twice: that keeps the list index-parallel with the definition points
/// the command derives from the entity, and lets the "every measuring
/// point came through one viewport" classification see a complete set.
pub(crate) fn record_dimension_viewport_pick(
&mut self,
i: usize,
frame: ViewportFrame,
paper_point: DVec3,
model_point: DVec3,
entity: Handle,
block_path: Vec<Handle>,
) {
self.sync_dimension_snap_owner(i);
// An object pick supplies both extension origins at once; record it
// twice so the "every measuring point came through one viewport"
// test sees a complete set.
for _ in 0..2 {
self.dim_accepted_snaps.push(AcceptedSnap {
self.push_accepted_snap(AcceptedSnap {
paper_point,
model_point,
viewport: Some(frame.viewport),
frame: Some(frame),
source: None,
source: Some(SnapSourceRef {
source: DimensionAssociationSource::inferred(entity),
block_path: block_path.clone(),
// An object pick acquires the whole curve, not one
// feature; the per-slot feature point is derived from the
// committed dimension's own definition points.
snap_type: crate::snap::SnapType::Nearest,
}),
});
}
}
/// The accepted snap for the `idx`-th collected point of the active
/// command, if one was recorded. The contracted accessor PR1 and PR3 read;
/// PR2 itself classifies the whole set at commit time.
/// command, with the typed-coordinate viewport fallback applied.
///
/// Reads PR1's list -- there is no second store.
#[allow(dead_code)]
pub(crate) fn accepted_snap_for_point(&self, idx: usize) -> Option<&AcceptedSnap> {
self.dim_accepted_snaps.get(idx)
pub(crate) fn accepted_snap_for_point(&self, i: usize, idx: usize) -> Option<AcceptedSnap> {
self.accepted_snaps().get(idx).map(|snap| self.measure_snap(i, snap))
}
/// Drop recorded snaps when the active command changes, so one command
/// never sees another's points.
fn sync_dimension_snap_owner(&mut self, i: usize) {
let name = self.tabs[i]
.active_cmd
.as_ref()
.map(|command| command.name().to_string());
if self.dim_accepted_snaps_owner != name {
self.dim_accepted_snaps.clear();
self.dim_accepted_snaps_owner = name;
/// One accepted snap as *measurement* sees it.
///
/// Identical to the recorded snap except that a point which carries no
/// snap at all (a typed coordinate, a dynamic-input point, a headless
/// script point) and lands inside a content viewport is re-attached to
/// that viewport, so typing `100,50` inside a 1:10 viewport measures the
/// model just like clicking there would. A snap that landed on real
/// paper-sheet geometry keeps `viewport: None` and is never reinterpreted.
pub(crate) fn measure_snap(&self, i: usize, snap: &AcceptedSnap) -> AcceptedSnap {
if snap.frame.is_some() || snap.source.is_some() {
return snap.clone();
}
let scene = &self.tabs[i].scene;
if scene.current_layout == "Model" || scene.active_viewport.is_some() {
return snap.clone();
}
match scene.viewport_frame_at_paper_point(snap.paper_point) {
Some(frame) => AcceptedSnap {
paper_point: snap.paper_point,
model_point: frame.paper_to_model(snap.paper_point),
viewport: Some(frame.viewport),
frame: Some(frame),
source: None,
},
None => snap.clone(),
}
}
pub(crate) fn clear_dimension_accepted_snaps(&mut self) {
self.dim_accepted_snaps.clear();
self.dim_accepted_snaps_owner = None;
/// The accepted snaps that say what the pending dimension *measures*, in
/// collection order.
///
/// The final collected point of every dimension command in scope is the
/// dimension-line / text placement click, which says nothing about what is
/// being measured -- it routinely lands on bare sheet even for a viewport
/// measurement, and inside a viewport rectangle even for a plain paper
/// one. It is dropped here, which is also why association inference is
/// guarded on this set rather than on every recorded snap.
pub(crate) fn dimension_measuring_snaps(&self, i: usize) -> Vec<AcceptedSnap> {
let all = self.accepted_snaps();
let measuring = match all.len() {
0 | 1 => all,
n => &all[..n - 1],
};
measuring.iter().map(|snap| self.measure_snap(i, snap)).collect()
}
/// Classify how the pending dimension should be measured from the snaps
/// recorded for it.
fn dimension_measure_space(&self, i: usize) -> DimensionMeasureSpace {
pub(crate) fn dimension_measure_space(&self, i: usize) -> DimensionMeasureSpace {
let scene = &self.tabs[i].scene;
// Only a dimension being created *on the sheet* can measure through a
// viewport. Inside MSPACE the command already works in model space.
if scene.current_layout == "Model" || scene.active_viewport.is_some() {
return DimensionMeasureSpace::Direct;
}
// The final collected point of every dimension command in scope is the
// dimension-line / text placement click, which says nothing about what
// is being measured — it routinely lands on bare sheet even for a
// viewport measurement, and inside a viewport rectangle even for a
// plain paper one. Only the measuring points are classified.
let measuring = match self.dim_accepted_snaps.len() {
0 | 1 => self.dim_accepted_snaps.as_slice(),
n => &self.dim_accepted_snaps[..n - 1],
};
let measuring = self.dimension_measuring_snaps(i);
let measuring = measuring.as_slice();
if measuring.is_empty() {
return DimensionMeasureSpace::Direct;
}
@@ -167,24 +185,23 @@ impl OpenCADStudio {
/// Apply viewport compensation to a dimension about to be committed.
///
/// Returns `true` when the dimension measures through a viewport, which
/// tells the caller not to infer a paper-space association from the
/// projected definition points (PR3 creates the real viewport
/// associations).
/// Whether the dimension may also take a paper-space association is a
/// separate question with the same answer source:
/// [`OpenCADStudio::dimension_association_allowed`].
pub(crate) fn apply_viewport_dimension_measurement(
&mut self,
i: usize,
entity: &mut EntityType,
) -> bool {
) {
let EntityType::Dimension(dimension) = entity else {
return false;
return;
};
let is_angular = matches!(
dimension,
Dimension::Angular2Ln(_) | Dimension::Angular3Pt(_)
);
let frame = match self.dimension_measure_space(i) {
DimensionMeasureSpace::Direct => return false,
DimensionMeasureSpace::Direct => return,
DimensionMeasureSpace::Mixed => {
self.command_line.push_warning(
crate::t!(
@@ -192,16 +209,15 @@ impl OpenCADStudio {
)
.as_ref(),
);
return false;
return;
}
DimensionMeasureSpace::Viewport(frame) => frame,
};
// A similarity preserves angles, so an angular dimension reads the
// model angle already. It still measured through a viewport, so the
// caller must skip association inference.
// model angle already and needs no override.
if is_angular {
return true;
return;
}
let compensation = frame.paper_to_model_length_factor();
@@ -225,7 +241,6 @@ impl OpenCADStudio {
Some(XDataValue::Real(value)),
);
}
true
}
/// Explicit "select object" pick for a dimension command, resolved through
@@ -251,7 +266,13 @@ impl OpenCADStudio {
let pick = self.tabs[i]
.scene
.dimension_pick_through_viewport(paper, aperture_paper)?;
self.record_dimension_viewport_pick(i, pick.frame, pick.paper_point, pick.model_point);
self.record_dimension_viewport_pick(
pick.frame,
pick.paper_point,
pick.model_point,
pick.entity_handle,
pick.block_path.clone(),
);
let command = self.tabs[i].active_cmd.as_mut()?;
command.inject_picked_entity(pick.paper_entity);
Some(command.on_entity_pick(pick.entity_handle, pick.paper_point))
+1 -9
View File
@@ -9,7 +9,7 @@ pub(crate) mod config;
pub use automation::{export_headless, serve};
mod command_driver;
pub(crate) mod commands;
mod dim_viewport;
pub(crate) mod dim_viewport;
mod document;
mod drafting_settings;
pub(crate) mod expr_eval;
@@ -509,12 +509,6 @@ pub(super) struct OpenCADStudio {
cursor_size: i32,
/// Selection-box size setting (PICKBOX, 0..=50).
pick_box: i32,
/// PR2: accepted snaps for the points the active dimension command has
/// collected, in collection order. Transient command state — the committed
/// result is an ordinary dimension entity, so undo/redo needs nothing here.
pub(crate) dim_accepted_snaps: Vec<crate::scene::viewport_ref::AcceptedSnap>,
/// Name of the command `dim_accepted_snaps` belongs to.
pub(crate) dim_accepted_snaps_owner: Option<String>,
/// Use REFEDIT rather than BEDIT when double-clicking an attribute-free block.
double_click_block_refedit: bool,
/// Open ATTEDIT when double-clicking a block with attributes.
@@ -3641,8 +3635,6 @@ impl OpenCADStudio {
zoom_factor: 60,
cursor_size: 5,
pick_box: 3,
dim_accepted_snaps: Vec::new(),
dim_accepted_snaps_owner: None,
double_click_block_refedit: false,
double_click_block_attedit: true,
grip_object_limit: settings::DEFAULT_GRIP_OBJECT_LIMIT,
+37 -20
View File
@@ -46,43 +46,51 @@ impl OpenCADStudio {
self.accepted_snaps.last()
}
/// `true` when any point the active command collected came through a
/// layout viewport. Dimension association inference (which compares a
/// dimension's own definition points against geometry in the *current*
/// space) must not run in that case: the definition points are paper
/// coordinates that correspond to no paper-sheet geometry, and the model
/// geometry they really describe lives at completely different
/// coordinates. PR3 replaces this guard with real viewport-aware
/// associations built from `AcceptedSnap::source`.
pub(crate) fn accepted_snaps_through_viewport(&self) -> bool {
self.accepted_snaps.iter().any(AcceptedSnap::through_viewport)
}
/// `Scene::infer_dimension_sources`, suppressed when the dimension's points
/// were picked through a layout viewport (PR1).
/// `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.
/// 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.
///
/// Deliberately minimal: PR3 replaces this with a real viewport-aware
/// association built from `AcceptedSnap::source`.
/// 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.
pub(crate) fn infer_dimension_sources_guarded(
&self,
tab: usize,
dimension: Handle,
) -> Vec<Option<Handle>> {
if self.accepted_snaps_through_viewport() {
if !self.dimension_association_allowed(tab) {
return Vec::new();
}
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.
pub(crate) fn dimension_association_allowed(&self, tab: usize) -> bool {
matches!(
self.dimension_measure_space(tab),
crate::app::dim_viewport::DimensionMeasureSpace::Direct
)
}
/// Forget the accepted snaps of a finished / abandoned command.
pub(crate) fn clear_accepted_snaps(&mut self) {
self.accepted_snaps.clear();
@@ -107,6 +115,15 @@ impl OpenCADStudio {
}
None => AcceptedSnap::free(committed),
};
self.push_accepted_snap(accepted);
}
/// 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.
pub(crate) fn push_accepted_snap(&mut self, accepted: AcceptedSnap) {
if self.accepted_snaps.len() >= MAX_ACCEPTED_SNAPS {
self.accepted_snaps.remove(0);
}
+15 -41
View File
@@ -1,19 +1,20 @@
//! PR2 seams onto the paper-space viewport world.
//! Object-pick resolution for paper-space dimensions taken through a layout
//! viewport.
//!
//! Everything in this file exists so that viewport-aware dimension
//! measurement works *without* PR1 (viewport snapping) in the tree. Each seam
//! is small and self-contained; the merge notes say which ones PR1 replaces.
//! 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.
//!
//! ## Seams
//! * [`Scene::viewport_frame`] — **replaced by PR1.** PR1 owns construction of
//! [`ViewportFrame`] from the live viewport camera. This implementation
//! derives the frame straight from the DXF viewport record, which is correct
//! for plan (non-oblique) content viewports.
//! * [`Scene::content_viewport_at_paper_point`] /
//! [`Scene::viewport_frame_at_paper_point`] — PR1 supersedes these with its
//! snap-time viewport resolution; harmless to keep.
//! * [`Scene::dimension_pick_through_viewport`] — PR2 owns this permanently.
//! It is the explicit-object-pick path and does not depend on PR1.
//! 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.
use super::*;
@@ -43,33 +44,6 @@ pub struct ViewportDimPick {
}
impl Scene {
// ───────────────────────── PR1 SEAM: BEGIN ──────────────────────────
// Replace the body of `viewport_frame` with PR1's implementation on
// merge; the signature is the shared contract and must not change.
/// Planar paper↔model mapping for one layout viewport.
///
/// Derived from the viewport record rather than the render camera:
/// `paper_center = vp.center`, `model_target = vp.view_target`,
/// `scale = vp_effective_scale(...)`, `twist = vp.twist_angle`.
pub fn viewport_frame(&self, viewport: Handle) -> Option<ViewportFrame> {
let Some(EntityType::Viewport(vp)) = self.document.get_entity(viewport) else {
return None;
};
let scale = vp_effective_scale(vp.custom_scale, vp.view_height, vp.height);
if !scale.is_finite() || scale.abs() < 1e-12 {
return None;
}
Some(ViewportFrame {
viewport,
paper_center: DVec2::new(vp.center.x, vp.center.y),
model_target: DVec2::new(vp.view_target.x, vp.view_target.y),
scale,
twist: vp.twist_angle,
locked: vp.status.locked,
})
}
// ────────────────────────── PR1 SEAM: END ───────────────────────────
/// The smallest *on* content viewport of the current layout whose paper
/// rectangle contains `paper`. `None` on the Model tab, while editing