Record viewport associations for new dimensions

Removes PR3's `viewport_frame` seam in `viewport_ref.rs`; the frame is
PR1's camera-derived one everywhere.

The five dimension commands (DIMLINEAR / DIMALIGNED / DIMRADIUS /
DIMDIAMETER / DIMANGULAR) now record a real association when they
measure through a viewport. The commit path's one association gate
(`dimension_association_allowed`, added when PR2 was stacked) gains an
else-branch: where PR2 committed the dimension unassociated and PR1's
inference guard returned an empty source list, `Scene::attach_viewport_
dimension_association` now stores the whole chain — dimension ->
viewport -> block path -> entity -> feature — from the AcceptedSnaps PR1
captured, in definition-point slot order.

* `Scene::dimension_association_slot_points` names that order, including
  the radial dimensions `dimension_reference_points` does not cover.
* `OpenCADStudio::viewport_dimension_snaps` builds the slot array. A snap
  whose paper point is no longer the definition point it fills has its
  model point recomputed from that definition point through the frame:
  the object pick records one click for both extension origins, and a
  rotated DIMLINEAR projects its origins onto the measuring axis.
* Radial dimensions take their own path inside
  `attach_viewport_dimension_association`. Their single reference is an
  osnap type 10 carrying the radial marker (which circle / which polyline
  arc segment) and the angle around it; `source_reference` would hand
  back the generic curve-parameter marker -2, which
  `radial_source_for_marker` cannot find.

One measurement rule, documented in `viewport_ref::measurement_rule`:
`actual_measurement` is always the RAW measurement in the dimension's own
space (paper, for a viewport dimension), and the viewport compensation
lives only in the magnitude of the negative DIMLFAC override, applied
once by the formatter. PR2's persistence already followed it; PR3's
refresh did not — it substituted the model distance times
`model_factor()`, which made a refreshed dimension read `user_lfac` times
too much versus an identical freshly-created one. Refresh now writes the
paper measurement (a debug assert pins resolved paper points to the model
points through the frame), and `axis_distance` goes with it.

Block sub-entity snaps: verified that `block_cache` stamps the top-level
INSERT's handle onto every expanded and clipped sub-wire, so
`wire_source` can only ever report the INSERT — nested inserts included.
`OpenCADStudio::resolve_source_block_path` now descends from that INSERT
at accept time (using the object-pick path's existing
`resolve_measurable_entity`, so arbitrary depth, not just one level) and
fills `SnapSourceRef.block_path` with the real inner entity. Where the
descent cannot identify one — a non-planar-pickable type such as text,
hatch, spline or solid, or nesting past eight levels — the source stays
the INSERT with an empty path, which is coarser but never wrong. Both the
limitation and where it bites are documented on
`Scene::resolve_block_snap_source` and `SnapResult::source`.

Co-Authored-By: Claude Fable 5.1 <[email protected]>
This commit is contained in:
Christopher OverbeckandClaude Fable 5.1 committed 2026-09-15 12:52:00 -05:00
1 parent f2c9e90c34
commit be7ba3d68e
8 files changed
+336 -95

No files matched your search

+9 -1
View File
@@ -376,7 +376,7 @@ impl OpenCADStudio {
// accepted-snap list must stay index-parallel with the points the
// command collects (PR1). Interactive picks record themselves in
// the click handler and never reach here.
self.record_accepted_snap(None, None, *point);
self.record_accepted_snap(i, None, None, *point);
}
if default_start {
let StepInput::Point(point) = &input else {
@@ -2220,6 +2220,14 @@ impl OpenCADStudio {
changes.sort_by_key(|(handle, _)| handle.value());
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.
self.attach_viewport_dimension_association(i, handle);
}
}
pending
+77
View File
@@ -183,6 +183,83 @@ impl OpenCADStudio {
}
}
/// The slot array [`Scene::attach_viewport_dimension_association`] wants,
/// built from the snaps PR1 recorded for the command that just committed
/// `dimension`.
///
/// Slot `k` describes the feature at
/// `Scene::dimension_association_slot_points()[k]`, so the measuring snaps
/// are taken in collection order and truncated / padded to that length.
///
/// A snap whose paper point is not (still) the definition point it would
/// fill has its `model_point` recomputed from that definition point
/// through the frame. Two cases need it:
///
/// * the explicit object pick, which records one click twice — once per
/// extension origin — so neither recorded point is the endpoint the
/// command derived from the entity;
/// * a point the command moved after the snap (a rotated DIMLINEAR
/// projects its origins onto the measuring axis).
///
/// The source identity is kept in both cases: it is the entity that was
/// acquired, and the feature marker is re-derived from the corrected
/// point.
pub(crate) fn viewport_dimension_snaps(
&self,
i: usize,
dimension: Handle,
) -> Vec<Option<AcceptedSnap>> {
let slots = self.tabs[i].scene.dimension_association_slot_points(dimension);
if slots.is_empty() {
return Vec::new();
}
let measuring = self.dimension_measuring_snaps(i);
slots
.iter()
.enumerate()
.map(|(index, slot)| {
let snap = measuring.get(index)?.clone();
let frame = snap.frame?;
let slot = DVec3::new(slot.x, slot.y, slot.z);
if snap.paper_point.truncate().distance_squared(slot.truncate()) <= 1e-12 {
return Some(snap);
}
Some(AcceptedSnap {
paper_point: slot,
model_point: frame.paper_to_model(slot),
..snap
})
})
.collect()
}
/// Record the viewport association for a dimension that has just been
/// committed while measuring through a layout viewport.
///
/// Announces the whole chain — the viewport, every INSERT on the block
/// path and the source entity — so the dependency index picks the new
/// association up immediately.
pub(crate) fn attach_viewport_dimension_association(&mut self, i: usize, dimension: Handle) {
let snaps = self.viewport_dimension_snaps(i, dimension);
if snaps.iter().flatten().all(|snap| snap.source.is_none()) {
return;
}
self.tabs[i]
.scene
.attach_viewport_dimension_association(dimension, &snaps);
let mut changes = vec![(dimension, crate::scene::ChangeKind::Modified)];
changes.extend(
self.tabs[i]
.scene
.dimension_association_sources(dimension)
.into_iter()
.map(|handle| (handle, crate::scene::ChangeKind::Modified)),
);
changes.sort_by_key(|(handle, _)| handle.value());
changes.dedup_by_key(|(handle, _)| handle.value());
self.tabs[i].scene.bump_entities(&changes);
}
/// Apply viewport compensation to a dimension about to be committed.
///
/// Whether the dimension may also take a paper-space association is a
+1
View File
@@ -4173,6 +4173,7 @@ impl OpenCADStudio {
// `world_pt` exactly as before.
let pending = self.pending_click_snap.take();
self.record_accepted_snap(
i,
pending.map(|(hit, _)| hit),
pending.and_then(|(_, frame)| frame),
world_pt,
+35 -1
View File
@@ -102,11 +102,12 @@ impl OpenCADStudio {
/// after ortho / polar / axis-lock / dynamic-input resolution.
pub(crate) fn record_accepted_snap(
&mut self,
tab: usize,
snap: Option<SnapResult>,
frame: Option<ViewportFrame>,
committed: glam::DVec3,
) {
let accepted = match snap {
let mut accepted = match snap {
// A viewport snap keeps its frame only when the hit really came
// through it, so a paper-sheet snap never claims a model point.
Some(hit) => {
@@ -115,9 +116,42 @@ impl OpenCADStudio {
}
None => AcceptedSnap::free(committed),
};
self.resolve_source_block_path(tab, &mut accepted);
self.push_accepted_snap(accepted);
}
/// Replace a snap source that is a block INSTANCE with the entity inside
/// it that actually owns the snapped feature, plus the INSERT path.
///
/// The snap engine can only report what the wire carries, and the
/// renderer stamps every wire of an expanded block with the *top-level
/// INSERT's* handle — nested inserts included. `SnapSourceRef` promises
/// the innermost entity in `source.handle` and the instance chain in
/// `block_path`, so the descent happens here, once, at accept time (not
/// per pointer move).
///
/// When the descent cannot identify the inner entity the source stays the
/// INSERT with an empty path. See
/// [`crate::scene::Scene::resolve_block_snap_source`] for exactly when
/// that happens; the resulting association is coarser (it tracks the whole
/// block instance) but never wrong.
fn resolve_source_block_path(&self, tab: usize, accepted: &mut AcceptedSnap) {
let Some(source) = accepted.source.as_mut() else {
return;
};
if !source.block_path.is_empty() {
return;
}
if let Some((entity, path)) = self.tabs[tab]
.scene
.resolve_block_snap_source(source.source.handle, accepted.model_point)
{
source.source =
crate::command::DimensionAssociationSource::inferred(entity);
source.block_path = path;
}
}
/// Append an already-built accepted snap, keeping the retention cap.
///
/// Used by paths that are not point steps (the dimension object pick
+137 -55
View File
@@ -17,7 +17,7 @@ use crate::command::DimensionAssociationSource;
use super::dimension_assoc_chain::{
self as chain, ChainError, FeatureContext, ReferenceChain,
};
use super::viewport_ref::{AcceptedSnap, MeasurementScale, ViewportFrame};
use super::viewport_ref::{AcceptedSnap, ViewportFrame};
use super::{ChangeKind, Scene};
pub(crate) const POLYLINE_ARC_CENTER_MARKER: i32 = -4;
@@ -372,7 +372,7 @@ fn source_marker(entity: &EntityType, point: Vector3) -> Option<i32> {
/// 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 [`MeasurementScale`]).
/// being applied twice (see `viewport_ref::measurement_rule`).
#[derive(Clone, Copy, Debug)]
pub(crate) struct ChainMap {
pub transform: Transform,
@@ -1175,9 +1175,25 @@ impl Scene {
dimension: Handle,
snaps: &[Option<AcceptedSnap>],
) {
let Some(EntityType::Dimension(_)) = self.document.get_entity(dimension) else {
let Some(EntityType::Dimension(dimension_entity)) = self.document.get_entity(dimension)
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.
let radial_dimension = matches!(
dimension_entity,
Dimension::Radius(_) | Dimension::Diameter(_) | Dimension::LargeRadial(_)
);
if radial_dimension {
self.attach_viewport_radial_association(dimension, snaps);
return;
}
let mut references: [Vec<AssocDimensionReference>; 4] =
std::array::from_fn(|_| Vec::new());
let mut associativity = 0;
@@ -1244,6 +1260,96 @@ impl Scene {
);
}
/// The radial half of [`Scene::attach_viewport_dimension_association`].
///
/// The first snap that carries a source wins: a radius / diameter
/// dimension measures one circle or one polyline arc segment, and the
/// command only ever acquires one.
fn attach_viewport_radial_association(
&mut self,
dimension: Handle,
snaps: &[Option<AcceptedSnap>],
) {
let Some(snap) = snaps
.iter()
.flatten()
.find(|snap| snap.source.is_some())
.cloned()
else {
return;
};
let source = snap.source.as_ref().expect("filtered above");
let entity_handle = source.source.handle;
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 radial = match source.source.marker {
Some(marker) => radial_source_for_marker(entity, marker),
None => radial_source_at(entity, local),
};
let Some(radial) = radial else { return };
let angle = if source.source.marker.is_some() && source.source.parameter.is_finite() {
source.source.parameter
} 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 reactor_targets = xrefs.clone();
let reference = AssocDimensionReference {
class_name: "AcDbOsnapPointRef".to_string(),
osnap_type: 10,
xrefs,
main_subent_type: 1,
main_gs_marker: radial.marker,
osnap_distance: angle,
osnap_point: dvec3(snap.model_point),
..AssocDimensionReference::default()
};
self.store_dimension_association_full(
dimension,
[vec![reference], Vec::new(), Vec::new(), Vec::new()],
1,
trans_space,
reactor_targets,
);
}
/// The definition points an association fills, slot by slot, in the same
/// order [`Scene::attach_viewport_dimension_association`] consumes its
/// `snaps` argument.
///
/// Callers use it to size and sanity-check the slot array they build from
/// the accepted snaps: slot `k` of that array must describe the feature
/// sitting at `dimension_association_slot_points()[k]`.
pub(crate) fn dimension_association_slot_points(&self, dimension: Handle) -> Vec<Vector3> {
let Some(EntityType::Dimension(entity)) = self.document.get_entity(dimension) else {
return Vec::new();
};
match entity {
// A radial dimension's single reference describes the point where
// the leader meets the circle.
Dimension::Radius(radius) => vec![radius.definition_point],
Dimension::Diameter(diameter) => vec![diameter.angle_vertex],
Dimension::LargeRadial(radial) => vec![radial.chord_point],
other => dimension_reference_points(other),
}
}
/// 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(
@@ -1638,22 +1744,6 @@ impl Scene {
let before = self.document.get_entity_arc(association.dimension);
self.record_undo_before(association.dimension, before);
}
// The user's DIMLFAC override travels with the dimension (PR2
// persists it when it creates a viewport dimension); split it from
// the viewport compensation so the compensation is applied exactly
// once, on the model length, and never again on the paper one.
let scale = MeasurementScale::from_dimlfac(
self.document
.get_entity(association.dimension)
.and_then(|entity| {
crate::entities::dim_override::real(
&entity.common().extended_data,
crate::entities::dim_override::DIMLFAC,
)
})
.unwrap_or(0.0),
map.frame.as_ref(),
);
let Some(EntityType::Dimension(dimension)) =
self.document.get_entity_mut(association.dimension)
else {
@@ -1897,32 +1987,34 @@ impl Scene {
Dimension::Linear(linear) => linear_measurement(linear),
_ => dimension.measurement(),
};
// A dimension drawn on the sheet through a viewport has just been
// rebuilt from *paper* points, so measuring those gives the paper
// length. The number it shows is the model one: take it from the
// model points the same references resolved to, and apply only the
// user's DIMLFAC on top (`model_factor`) — the viewport
// compensation is already inherent in having measured in model
// space, and applying it again is the double-scaling this whole
// three-PR effort exists to stop.
let measurement = match (map.frame.as_ref(), &*dimension) {
(Some(_), Dimension::Linear(_) | Dimension::Aligned(_)) => {
match (model_points[0], model_points[1]) {
(Some(first), Some(second)) => {
let raw = match &*dimension {
// Rotated linear measures only along its axis.
Dimension::Linear(linear) => {
axis_distance(first, second, linear.rotation)
}
_ => first.distance(&second),
};
raw * scale.model_factor()
}
_ => measurement,
}
}
_ => 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.
debug_assert!(
map.frame.is_none()
|| model_points
.iter()
.zip(resolved.iter())
.all(|(model, space)| match (model, space, map.frame.as_ref()) {
(Some(model), Some(space), Some(frame)) => {
let projected =
frame.model_to_paper(glam::DVec3::new(model.x, model.y, model.z));
(projected.x - space.x).abs() < 1e-6
&& (projected.y - space.y).abs() < 1e-6
}
_ => true,
}),
"resolved paper points must be the model points through the frame",
);
dimension.base_mut().actual_measurement = measurement;
refreshed.push((association.dimension, ChangeKind::Modified));
}
@@ -1930,16 +2022,6 @@ impl Scene {
}
}
/// Distance between two points measured along the direction `rotation`.
///
/// The rotated-linear rule, evaluated in whatever space the two points are in
/// — the same formula [`linear_measurement`] applies on the sheet, reused for
/// the model points a trans-space dimension measures.
fn axis_distance(first: Vector3, second: Vector3, rotation: f64) -> f64 {
let axis = [rotation.cos(), rotation.sin()];
((second.x - first.x) * axis[0] + (second.y - first.y) * axis[1]).abs()
}
/// How far the midpoint of a dimension's measured points moved.
///
/// Used to carry the sheet annotation — the dimension line's standoff and a
+33
View File
@@ -110,6 +110,39 @@ impl Scene {
})
}
/// Resolve a snapped INSERT handle into the innermost entity that owns the
/// geometry, plus the INSERT path down to it.
///
/// The renderer names every wire of an expanded block with the **top-level
/// INSERT's** handle (see `block_cache`), including wires belonging to
/// nested inserts, so a snap inside a block reports the INSERT and nothing
/// finer. This walks the block definition from that INSERT looking for the
/// nearest measurable entity to `model_point`, which recovers the real
/// source handle and the instance path for the association chain.
///
/// Returns `None` — and the caller then keeps the INSERT itself as the
/// source, which still gives a correct, if coarse, association — when:
/// * `top` is not an INSERT (the common case: `Some((top, vec![]))`);
/// * the geometry is a type [`planar_pick_distance`] does not handle
/// (text, hatch, spline, solid, 3-D geometry);
/// * the nesting is deeper than eight levels.
///
/// Depth is not limited to one level: [`Scene::descend_block_instance`]
/// recurses, and the returned path lists every INSERT from the outermost
/// down to the one that directly contains the entity.
pub fn resolve_block_snap_source(
&self,
top: Handle,
model_point: DVec3,
) -> Option<(Handle, Vec<Handle>)> {
if !matches!(self.document.get_entity(top), Some(EntityType::Insert(_))) {
return None;
}
let (entity, path) = self.resolve_measurable_entity(top, model_point)?;
let handle = entity.as_entity().handle();
handle.is_valid().then_some((handle, path))
}
/// Nearest resident model wire of `viewport` to `model_point`, within
/// `aperture` model units. Wires are the same tessellation the viewport
/// draws, so this matches what the user sees.
+32 -36
View File
@@ -328,42 +328,38 @@ impl MeasurementScale {
}
}
// ─────────────────────────────────────────────────────────────────────────
// MERGE SEAM — PR1 (feat/viewport-snap) owns `Scene::viewport_frame`.
//
// PR3 needs a frame to map a resolved model point onto the sheet, but PR1 is
// not visible from this branch. This is the minimal stand-in agreed in the
// shared contract: paper_center = vp.center, model_target = vp.view_target,
// scale = `vp_effective_scale`, twist = vp.twist_angle. PR1's version is
// camera-derived (it must agree with what is actually drawn, including the
// auto-fit rescue in `Scene::camera_for_viewport` for stale saved views).
//
// To merge: delete this whole block. Nothing else in PR3 reads viewport
// fields directly — every call goes through `Scene::viewport_frame`.
// ─────────────────────────────────────────────────────────────────────────
impl crate::scene::Scene {
/// The planar paper<->model mapping of layout viewport `handle`.
///
/// `None` when the handle is not a viewport, or when its scale is
/// degenerate (a zero-height view cannot map anything).
pub fn viewport_frame(&self, handle: Handle) -> Option<ViewportFrame> {
let Some(acadrust::EntityType::Viewport(vp)) = self.document.get_entity(handle) else {
return None;
};
let scale = crate::scene::vp_effective_scale(vp.custom_scale, vp.view_height, vp.height);
if !scale.is_finite() || scale.abs() < 1e-12 {
return None;
}
Some(ViewportFrame {
viewport: handle,
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,
})
}
}
/// # 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 {
+12 -2
View File
@@ -93,8 +93,15 @@ pub struct SnapResult {
/// query, never by the engine itself. (PR1)
pub viewport: Option<Handle>,
/// Owning entity of the geometry the snap landed on, when the engine could
/// attribute the feature to a wire. Block sub-entities report the top-level
/// INSERT handle; the block path is resolved later by
/// attribute the feature to a wire.
///
/// This is whatever the *wire* carries, which for anything drawn from a
/// block is the **top-level INSERT's** handle — nested inserts included,
/// because `block_cache` stamps the host insert's name onto every
/// expanded and clipped sub-wire. Resolving that to the inner entity plus
/// 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)
pub source: Option<DimensionAssociationSource>,
}
@@ -108,6 +115,9 @@ impl SnapResult {
/// Entity handle carried by a wire (`WireModel::name` is the decimal handle
/// value). Returns `None` for preview / interim wires with a symbolic name.
///
/// For block geometry the value is the top-level INSERT handle, never the
/// inner entity: see [`SnapResult::source`].
#[inline]
pub(crate) fn wire_source(wire: &WireModel) -> Option<DimensionAssociationSource> {
wire.name