Resolve the DIMLFAC sign convention and add the viewport measurement seams

DXF gives DIMLFAC a sign convention: a positive value scales every
dimension, a negative one applies |DIMLFAC| only to dimensions that live in
a paper-space layout (where it compensates a viewport's scale). The linear
formatter multiplied by the raw signed value, so a drawing with negative
DIMLFAC (e.g. 115448.dxf) rendered negative measurements.

Resolve the sign once, in `resolved_dimension_style`, the single place an
effective style is derived, so dimension text, alternate units, exploded and
baked text and the properties panel all agree. `extended_data` keeps the raw
signed value, so DXF/DWG export is unchanged. The properties "Measurement"
row now reports the value the text shows rather than the raw distance, while
`actual_measurement` (group 42) stays raw as the format requires.

Also adds the PR2 seams the dimension work builds on:
  * MeasurementScale::user_lfac_for_space / viewport_dimlfac_override —
    the persisted representation of viewport compensation.
  * Scene::viewport_frame and paper-point viewport resolution.
  * Model-geometry hit testing through a content viewport, including
    descent into block instances with the instance transform baked in.

Co-Authored-By: Claude Fable 5.1 <[email protected]>

src/scene/mod.rs

@@ -41,6 +41,7 @@ pub mod named_parameters;
 mod named_parameters_persist;
 mod mspace;
 pub mod viewport_ref;
+pub mod viewport_dim_seam;
 mod page_setup;
 mod paper;
 mod preview;

src/scene/viewport_dim_seam.rs

@@ -0,0 +1,421 @@
+//! PR2 seams onto the paper-space viewport world.
+//!
+//! 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.
+//!
+//! ## 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.
+
+use super::*;
+
+use crate::command::EntityTransform;
+use crate::scene::viewport_ref::ViewportFrame;
+use acadrust::types::{Matrix4, Transform};
+use glam::{DVec2, DVec3};
+
+/// A model entity resolved by clicking inside a paper-space content viewport.
+#[derive(Clone, Debug)]
+pub struct ViewportDimPick {
+    /// The viewport the click looked through.
+    pub frame: ViewportFrame,
+    /// Handle of the innermost model entity that owns the picked geometry.
+    pub entity_handle: Handle,
+    /// INSERT handles from the outermost instance down to the instance that
+    /// directly contains `entity_handle`; empty for top-level model geometry.
+    pub block_path: Vec<Handle>,
+    /// The picked geometry with the block-instance transform *and* the
+    /// viewport model→paper transform applied, so paper-space dimension
+    /// commands can consume it unchanged.
+    pub paper_entity: EntityType,
+    /// The click position on the sheet.
+    pub paper_point: DVec3,
+    /// The click position in model space.
+    pub model_point: DVec3,
+}
+
+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
+    /// inside a viewport (MSPACE), or when the point is on bare sheet.
+    pub fn content_viewport_at_paper_point(&self, paper: DVec3) -> Option<Handle> {
+        if self.current_layout == "Model" || self.active_viewport.is_some() {
+            return None;
+        }
+        let (_, _, handles) = self.paper_viewport_handles();
+        handles
+            .iter()
+            .filter_map(|handle| {
+                let Some(EntityType::Viewport(vp)) = self.document.get_entity(*handle) else {
+                    return None;
+                };
+                if !vp.status.is_on || vp.width <= 0.0 || vp.height <= 0.0 {
+                    return None;
+                }
+                let dx = (paper.x - vp.center.x).abs();
+                let dy = (paper.y - vp.center.y).abs();
+                (dx <= vp.width * 0.5 && dy <= vp.height * 0.5)
+                    .then(|| (*handle, vp.width * vp.height))
+            })
+            .min_by(|a, b| a.1.total_cmp(&b.1))
+            .map(|(handle, _)| handle)
+    }
+
+    /// [`Scene::content_viewport_at_paper_point`] resolved to a frame.
+    pub fn viewport_frame_at_paper_point(&self, paper: DVec3) -> Option<ViewportFrame> {
+        self.viewport_frame(self.content_viewport_at_paper_point(paper)?)
+    }
+
+    /// Resolve an explicit dimension object pick made on the sheet but landing
+    /// inside a content viewport, against the *model* geometry that viewport
+    /// displays.
+    ///
+    /// `aperture_paper` is the pick aperture in paper units.
+    pub fn dimension_pick_through_viewport(
+        &self,
+        paper: DVec3,
+        aperture_paper: f64,
+    ) -> Option<ViewportDimPick> {
+        let frame = self.viewport_frame_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 top = self.nearest_model_wire_handle(frame.viewport, model_point, aperture_model)?;
+        let (entity, block_path) = self.resolve_measurable_entity(top, model_point)?;
+
+        let mut paper_entity = entity;
+        crate::scene::view::dispatch::apply_transform(
+            &mut paper_entity,
+            &EntityTransform::Affine(viewport_model_to_paper_transform(&frame)),
+        );
+        let entity_handle = paper_entity.as_entity().handle();
+        Some(ViewportDimPick {
+            frame,
+            entity_handle: if entity_handle.is_valid() { entity_handle } else { top },
+            block_path,
+            paper_entity,
+            paper_point: paper,
+            model_point,
+        })
+    }
+
+    /// 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.
+    fn nearest_model_wire_handle(
+        &self,
+        viewport: Handle,
+        model_point: DVec3,
+        aperture: f64,
+    ) -> Option<Handle> {
+        let wires = self.model_wires_for_viewport_arc(viewport, 0.0);
+        let mut best: Option<(f64, Handle)> = None;
+        for wire in wires.iter() {
+            if !wire.display_visible {
+                continue;
+            }
+            let Some(handle) = Self::handle_from_wire_name(&wire.name) else {
+                continue;
+            };
+            let offset = wire
+                .render_instance
+                .as_ref()
+                .map(|inst| DVec3::from_array(inst.translation))
+                .unwrap_or(DVec3::ZERO);
+            let distance = wire_polyline_distance(wire, offset, model_point);
+            if let Some(distance) = distance {
+                if distance <= aperture && best.as_ref().is_none_or(|(d, _)| distance < *d) {
+                    best = Some((distance, handle));
+                }
+            }
+        }
+        best.map(|(_, handle)| handle)
+    }
+
+    /// Turn a picked handle into a measurable planar entity in model (WCS)
+    /// coordinates, descending through block instances and baking the
+    /// instance transform into the returned clone.
+    fn resolve_measurable_entity(
+        &self,
+        handle: Handle,
+        model_point: DVec3,
+    ) -> Option<(EntityType, Vec<Handle>)> {
+        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,
+                )?;
+                Some((found.0, found.1))
+            }
+            other => {
+                planar_pick_distance(other, model_point)?;
+                Some((other.clone(), Vec::new()))
+            }
+        }
+    }
+
+    /// Depth-first search for the nearest measurable entity inside an INSERT.
+    /// Returns the entity already transformed into WCS plus the INSERT path.
+    fn descend_block_instance(
+        &self,
+        insert_entity: EntityType,
+        outer: Transform,
+        model_point: DVec3,
+        path: &mut Vec<Handle>,
+        depth: usize,
+    ) -> Option<(EntityType, Vec<Handle>)> {
+        const MAX_DEPTH: usize = 8;
+        if depth > MAX_DEPTH {
+            return None;
+        }
+        let EntityType::Insert(insert) = &insert_entity else {
+            return None;
+        };
+        let insert_handle = insert.common.handle;
+        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<Handle> = block.entity_handles.clone();
+
+        let mut best: Option<(f64, EntityType, Vec<Handle>)> = None;
+        for child in children {
+            let Some(child_entity) = self.document.get_entity(child) else {
+                continue;
+            };
+            if matches!(child_entity, EntityType::Insert(_)) {
+                let mut nested = Vec::new();
+                if let Some((entity, mut nested_path)) = self.descend_block_instance(
+                    child_entity.clone(),
+                    combined,
+                    model_point,
+                    &mut nested,
+                    depth + 1,
+                ) {
+                    if let Some(distance) = planar_pick_distance(&entity, model_point) {
+                        if best.as_ref().is_none_or(|(d, _, _)| distance < *d) {
+                            let mut full = vec![insert_handle];
+                            full.append(&mut nested_path);
+                            best = Some((distance, entity, full));
+                        }
+                    }
+                }
+                continue;
+            }
+            let mut placed = child_entity.clone();
+            crate::scene::view::dispatch::apply_transform(
+                &mut placed,
+                &EntityTransform::Affine(combined),
+            );
+            let Some(distance) = planar_pick_distance(&placed, model_point) else {
+                continue;
+            };
+            if best.as_ref().is_none_or(|(d, _, _)| distance < *d) {
+                best = Some((distance, placed, vec![insert_handle]));
+            }
+        }
+        let (_, entity, found_path) = best?;
+        path.clear();
+        path.extend(found_path.iter().copied());
+        Some((entity, found_path))
+    }
+}
+
+/// Model→paper for a viewport, as an affine entity transform. Z passes
+/// through unchanged so planar geometry keeps its elevation.
+pub fn viewport_model_to_paper_transform(frame: &ViewportFrame) -> Transform {
+    let (sin, cos) = frame.twist.sin_cos();
+    let s = frame.scale;
+    let a = cos * s;
+    let b = sin * s;
+    // t = paper_center - R*s*model_target
+    let tx = frame.paper_center.x - (a * frame.model_target.x - b * frame.model_target.y);
+    let ty = frame.paper_center.y - (b * frame.model_target.x + a * frame.model_target.y);
+    Transform::from_matrix(Matrix4 {
+        m: [
+            [a, -b, 0.0, tx],
+            [b, a, 0.0, ty],
+            [0.0, 0.0, 1.0, 0.0],
+            [0.0, 0.0, 0.0, 1.0],
+        ],
+    })
+}
+
+/// Distance from `point` to a wire's polyline, in XY. `None` when the wire has
+/// no usable segment.
+fn wire_polyline_distance(
+    wire: &crate::scene::WireModel,
+    offset: DVec3,
+    point: DVec3,
+) -> Option<f64> {
+    let count = wire.points.len();
+    if count == 0 {
+        return None;
+    }
+    let at = |i: usize| -> DVec3 {
+        let hi = wire.points[i];
+        let lo = wire.points_low.get(i).copied().unwrap_or([0.0; 3]);
+        DVec3::new(
+            hi[0] as f64 + lo[0] as f64,
+            hi[1] as f64 + lo[1] as f64,
+            hi[2] as f64 + lo[2] as f64,
+        ) + offset
+    };
+    let mut best = f64::INFINITY;
+    let mut previous: Option<DVec3> = None;
+    for i in 0..count {
+        let current = at(i);
+        if !current.x.is_finite() || !current.y.is_finite() {
+            previous = None;
+            continue;
+        }
+        match previous {
+            Some(start) => best = best.min(segment_distance_xy(start, current, point)),
+            None => best = best.min((current.truncate() - point.truncate()).length()),
+        }
+        previous = Some(current);
+    }
+    best.is_finite().then_some(best)
+}
+
+fn segment_distance_xy(a: DVec3, b: DVec3, p: DVec3) -> f64 {
+    let ab = b.truncate() - a.truncate();
+    let ap = p.truncate() - a.truncate();
+    let len2 = ab.length_squared();
+    if len2 < 1e-24 {
+        return ap.length();
+    }
+    let t = (ap.dot(ab) / len2).clamp(0.0, 1.0);
+    (ap - ab * t).length()
+}
+
+/// Distance from `point` to the planar geometry object-pick dimensioning
+/// supports. `None` for entity types that cannot be dimensioned by picking.
+pub fn planar_pick_distance(entity: &EntityType, point: DVec3) -> Option<f64> {
+    let p = point.truncate();
+    match entity {
+        EntityType::Line(line) => Some(segment_distance_xy(
+            DVec3::new(line.start.x, line.start.y, line.start.z),
+            DVec3::new(line.end.x, line.end.y, line.end.z),
+            point,
+        )),
+        EntityType::Circle(circle) => {
+            let c = DVec2::new(circle.center.x, circle.center.y);
+            Some(((p - c).length() - circle.radius).abs())
+        }
+        EntityType::Arc(arc) => {
+            let c = DVec2::new(arc.center.x, arc.center.y);
+            Some(((p - c).length() - arc.radius).abs())
+        }
+        EntityType::LwPolyline(polyline) => polyline_distance(
+            polyline
+                .vertices
+                .iter()
+                .map(|v| DVec3::new(v.location.x, v.location.y, polyline.elevation)),
+            polyline.is_closed,
+            point,
+        ),
+        EntityType::Polyline2D(polyline) => polyline_distance(
+            polyline
+                .vertices
+                .iter()
+                .map(|v| DVec3::new(v.location.x, v.location.y, polyline.elevation)),
+            polyline.is_closed(),
+            point,
+        ),
+        _ => None,
+    }
+}
+
+fn polyline_distance(
+    points: impl IntoIterator<Item = DVec3>,
+    closed: bool,
+    point: DVec3,
+) -> Option<f64> {
+    let points: Vec<DVec3> = points.into_iter().collect();
+    if points.len() < 2 {
+        return None;
+    }
+    let count = if closed { points.len() } else { points.len() - 1 };
+    (0..count)
+        .map(|i| segment_distance_xy(points[i], points[(i + 1) % points.len()], point))
+        .reduce(f64::min)
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    fn frame() -> ViewportFrame {
+        ViewportFrame {
+            viewport: Handle::NULL,
+            paper_center: DVec2::new(100.0, 50.0),
+            model_target: DVec2::new(1000.0, 2000.0),
+            scale: 0.1,
+            twist: 0.4,
+            locked: false,
+        }
+    }
+
+    #[test]
+    fn affine_transform_matches_the_frame_mapping() {
+        let f = frame();
+        let t = viewport_model_to_paper_transform(&f);
+        for model in [
+            DVec3::new(1000.0, 2000.0, 0.0),
+            DVec3::new(1100.0, 2000.0, 0.0),
+            DVec3::new(940.5, 2113.25, 0.0),
+        ] {
+            let expected = f.model_to_paper(model);
+            let actual = t.transform_point(&acadrust::types::Vector3::new(
+                model.x, model.y, model.z,
+            ));
+            assert!(
+                (actual.x - expected.x).abs() < 1e-9 && (actual.y - expected.y).abs() < 1e-9,
+                "{actual:?} != {expected:?}"
+            );
+        }
+    }
+}

src/scene/viewport_ref.rs

@@ -267,6 +267,65 @@ impl MeasurementScale {
     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 (see
+    /// [`MeasurementScale::viewport_dimlfac_override`]). This is the same rule
+    /// as [`MeasurementScale::from_dimlfac`] with `viewport.is_some()`
+    /// replaced by `paper_space`.
+    pub fn user_lfac_for_space(dimlfac: f64, paper_space: bool) -> f64 {
+        if dimlfac == 0.0 {
+            1.0
+        } else if dimlfac > 0.0 {
+            dimlfac
+        } else if paper_space {
+            -dimlfac
+        } else {
+            1.0
+        }
+    }
+
+    /// The persisted representation of viewport compensation, PR2's single
+    /// source of truth.
+    ///
+    /// A paper-space dimension that measures model geometry through a viewport
+    /// keeps its definition points in **paper** coordinates and carries a
+    /// **negative DIMLFAC override** whose magnitude is
+    /// `user_lfac * viewport_compensation`. Because a viewport frame is a
+    /// similarity (uniform scale + in-plane twist), the raw paper measurement
+    /// times that magnitude is exactly the model measurement times the user's
+    /// linear factor — so the compensation is applied exactly once, at
+    /// formatting time, and round-trips through DXF/DWG unchanged (AutoCAD
+    /// applies |DIMLFAC| to paper-space dimensions by the same rule).
+    ///
+    /// Returns `None` when no override is needed:
+    /// * the compensation is 1:1, or
+    /// * the drawing/style DIMLFAC is already negative with exactly this
+    ///   magnitude (the drawing-wide DIMLFAC is set for this purpose).
+    ///
+    /// `style_dimlfac` is the effective DIMLFAC the dimension would inherit
+    /// *without* an override.
+    pub fn viewport_dimlfac_override(style_dimlfac: f64, compensation: f64) -> Option<f64> {
+        if !compensation.is_finite() || compensation.abs() < 1e-12 {
+            return None;
+        }
+        if (compensation - 1.0).abs() < 1e-12 {
+            return None;
+        }
+        if style_dimlfac < 0.0
+            && ((-style_dimlfac) - compensation).abs() <= 1e-9 * compensation.abs().max(1.0)
+        {
+            return None;
+        }
+        // A positive style DIMLFAC is the user's own multiplier and must be
+        // preserved; a negative one of a *different* magnitude was meant for
+        // another viewport scale, so the user multiplier there is 1.0.
+        let user = if style_dimlfac > 0.0 { style_dimlfac } else { 1.0 };
+        Some(-(user * compensation))
+    }
 }
 
 #[cfg(test)]
@@ -318,4 +377,44 @@ mod tests {
         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
+            );
+        }
+    }
+
+    #[test]
+    fn override_encodes_compensation_once() {
+        // 1:10 viewport, no user factor -> -10, and a 10-unit paper span then
+        // formats as 100.
+        let ovr = MeasurementScale::viewport_dimlfac_override(1.0, 10.0).unwrap();
+        assert!((ovr + 10.0).abs() < 1e-12);
+        let lfac = MeasurementScale::user_lfac_for_space(ovr, true);
+        assert!((10.0 * lfac - 100.0).abs() < 1e-9);
+        // Same dimension read back in model space must not scale.
+        assert_eq!(MeasurementScale::user_lfac_for_space(ovr, false), 1.0);
+    }
+
+    #[test]
+    fn override_preserves_user_multiplier_and_skips_redundant_cases() {
+        assert_eq!(MeasurementScale::viewport_dimlfac_override(1.0, 1.0), None);
+        // Drawing-wide negative DIMLFAC already carries this compensation.
+        assert_eq!(MeasurementScale::viewport_dimlfac_override(-10.0, 10.0), None);
+        // Positive user factor is folded in.
+        let ovr = MeasurementScale::viewport_dimlfac_override(25.4, 10.0).unwrap();
+        assert!((ovr + 254.0).abs() < 1e-9);
+        // Negative but for a different scale -> user multiplier is 1.0.
+        let ovr = MeasurementScale::viewport_dimlfac_override(-2.0, 10.0).unwrap();
+        assert!((ovr + 10.0).abs() < 1e-12);
+    }
 }