From 9244ffda5d109cf1a371ade4e367552bd8f48b10 Mon Sep 17 00:00:00 2001 From: Hamza Date: Wed, 28 Oct 2026 20:50:16 +0500 Subject: [PATCH] Open a drawing on the view it was saved at (#1602) A drawing opened fitted to its extents, so one whose extents are dominated by far-off geometry opened at a view hundreds of times its content: the drawing was sub-pixel and there was no way to zoom in. Every other reader opens on the view the file was saved at. The saved view is read from HEADER (`VIEWCTR`/`VIEWSIZE`) and from the first `*ACTIVE` VPORT record, which is the newer of the two and wins -- as it already does for snap and grid spacing. A record that leaves the centre out has no view to give, so the header pair stands. The extents remain the fallback for a drawing that stores no view, and that fallback now fits the width too, through the aspect the record is written with: a drawing 100 wide and 10 tall was given a view 11 tall, a sliver of it, and a horizontal line got 1e-6. Writing keeps it, so a read-and-write round trip no longer loses the view. Rebased onto main, 310 commits on: `open_drawing` and `usable_view` are in engine/src/session/files.rs, the VPORT reading in io/src/dxf_read/tables.rs, the HEADER pair list in io/src/dxf_write/header.rs and the VPORT writing in io/src/dxf_write/layout.rs. The two io tests were already carried by their hunk, so only the engine one needed placing. --- crates/doc/src/header.rs | 17 ++++++++ crates/engine/src/session/files.rs | 14 +++++- crates/engine/src/tests.rs | 34 +++++++++++++++ crates/io/src/dxf_read/tables.rs | 12 +++++- crates/io/src/dxf_write/header.rs | 4 ++ crates/io/src/dxf_write/layout.rs | 16 ++++--- crates/io/src/tests.rs | 69 ++++++++++++++++++++++++++++++ 7 files changed, 158 insertions(+), 8 deletions(-) diff --git a/crates/doc/src/header.rs b/crates/doc/src/header.rs index c6f17fd7e..52b9d34b1 100644 --- a/crates/doc/src/header.rs +++ b/crates/doc/src/header.rs @@ -90,6 +90,23 @@ impl Header { .filter(|p| p.is_finite() && if allow_zero { p.x >= 0.0 && p.y >= 0.0 } else { p.x > 0.0 && p.y > 0.0 }) .unwrap_or(Vec2::new(0.5, 0.5)) } + /// The model view the drawing was saved at: `$VIEWCTR` (centre) and `$VIEWSIZE` (height, in + /// drawing units), when both are present and usable. A DXF file keeps the pair in HEADER and + /// repeats it in the `*ACTIVE` VPORT record (12/22 and 40); a reader opens the drawing on it + /// rather than on the extents, so a drawing saved on one detail of a large sheet opens there. + pub fn saved_view(&self) -> Option<(Vec2, f64)> { + let center = self.point("VIEWCTR")?.xy(); + let height = self.get("VIEWSIZE").and_then(HVal::as_f64)?; + (center.is_finite() && height.is_finite() && height > 1e-9).then_some((center, height)) + } + /// Record the view the drawing is saved at (see [`Header::saved_view`]). A view that cannot + /// be shown is not recorded, so a usable stored view is never replaced by an unusable one. + pub fn set_saved_view(&mut self, center: Vec2, height: f64) { + if center.is_finite() && height.is_finite() && height > 1e-9 { + self.set("VIEWCTR", HVal::Point(center.to3(0.0))); + self.set_f64("VIEWSIZE", height); + } + } pub fn set_f64(&mut self, name: &str, v: f64) { self.set(name, HVal::Real(v)); } diff --git a/crates/engine/src/session/files.rs b/crates/engine/src/session/files.rs index 95d439f8c..cf0f8a8c2 100644 --- a/crates/engine/src/session/files.rs +++ b/crates/engine/src/session/files.rs @@ -1,5 +1,10 @@ use super::{DocState, Drawing, EngineError, Result, Session, Space, Value, Vec2, View, cmd}; +/// Whether a stored view can be used: a drawing can record a degenerate or non-finite one. +fn usable_view(v: &View) -> bool { + v.center.is_finite() && v.height.is_finite() && v.height > 1e-12 +} + impl Session { /// Append a new untitled drawing (`DrawingN`, metric or imperial default view), end a running command, make it active, bump the document epoch and return its index. pub fn new_drawing(&mut self, metric: bool) -> usize { @@ -17,8 +22,15 @@ impl Session { /// Append a loaded drawing, fit the view to its model extents (aspect-corrected, minimum height 1e-6) when non-empty, end a running command, make it active, bump the epoch and return its index. pub fn open_drawing(&mut self, d: Drawing, title: &str, path: Option) -> usize { let mut st = DocState::new(d, title, path); + // The view the file was saved at is the one every other reader opens on. Fitting the + // extents instead showed a drawing whose extents are dominated by far-off geometry at a + // view hundreds of times its content, with the drawing sub-pixel and no way to zoom in + // (#1602). The extents remain the fallback for a drawing that stores no view. + let saved = st.doc.header.saved_view().map(|(center, height)| View { center, height }).filter(usable_view); let ext = st.doc.extents(&Space::Model); - if !ext.is_empty() { + if let Some(v) = saved { + st.set_view_quiet(v); + } else if !ext.is_empty() { st.set_view_quiet(View { center: ext.center(), height: (ext.height().max(ext.width() * self.viewport_px.1 / self.viewport_px.0.max(1.0)) * 1.1).max(1e-6), diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 8c590dd97..0a77680dc 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -179,6 +179,40 @@ fn json_execute_and_errors() { assert_eq!(s.state().unwrap().undo.len(), 1); } +/// A drawing is opened on the view it was saved at, as every other reader opens it. Fitting the +/// extents instead showed a drawing whose extents are dominated by far-off geometry at a view +/// hundreds of thousands of times its content: sub-pixel, and `ZOOM Extents` fits the same box +/// (#1602). The extents are still the fallback for a drawing that stores no view. +#[test] +fn a_drawing_opens_on_the_view_it_was_saved_at() { + let mut d = cadcraft_doc::Drawing::new_metric(); + for (a, b) in [([0.0, 0.0], [100.0, 60.0]), ([5e6, 5e6], [5e6 + 100.0, 5e6 + 60.0])] { + let line = cadcraft_doc::Line { a: cadcraft_geom::Vec3::new(a[0], a[1], 0.0), b: cadcraft_geom::Vec3::new(b[0], b[1], 0.0) }; + d.add(&Space::Model, cadcraft_doc::Common::default(), EntityKind::Line(line)).unwrap(); + } + d.header.set_saved_view(Vec2::new(50.0, 50.0), 20.0); + + let mut s = Session::new(); + s.open_drawing(d.clone(), "saved-view.dxf", None); + assert_eq!(s.state().unwrap().view(), View { center: Vec2::new(50.0, 50.0), height: 20.0 }); + + // No stored view: the extents, as before. They are what makes this drawing unusable without + // the stored one -- the fit is more than a hundred thousand times taller than the detail. + let mut plain = d.clone(); + plain.header.vars.remove("VIEWCTR"); + plain.header.vars.remove("VIEWSIZE"); + let mut s = Session::new(); + s.open_drawing(plain, "no-view.dxf", None); + assert!(s.state().unwrap().view().height > 5e6, "{:?}", s.state().unwrap().view()); + + // A stored view that cannot be shown is no view at all. + let mut broken = d; + broken.header.set("VIEWSIZE", cadcraft_doc::HVal::Real(0.0)); + let mut s = Session::new(); + s.open_drawing(broken, "broken-view.dxf", None); + assert!(s.state().unwrap().view().height > 5e6, "{:?}", s.state().unwrap().view()); +} + #[test] fn move_with_pickfirst_and_window_selection() { let mut s = Session::new(); diff --git a/crates/io/src/dxf_read/tables.rs b/crates/io/src/dxf_read/tables.rs index 8a57f7f9e..001944b86 100644 --- a/crates/io/src/dxf_read/tables.rs +++ b/crates/io/src/dxf_read/tables.rs @@ -276,7 +276,8 @@ pub(super) fn tables(tags: &[Tag], d: &mut Drawing, rx: &mut Rx) { _ => {} } } - // One *Active viewport is no tiling: the view is the editor's (it zooms to the extents). + // One *Active viewport is no tiling: its single tile is not a configuration to switch to. + // Its view was kept in the HEADER pair as it was read (see `vport_record`). d.vports.retain(|c| c.tiles.len() > usize::from(c.is_active())); } @@ -315,6 +316,12 @@ const MAX_VPORT_TILES: usize = 64; fn vport_record(d: &mut Drawing, t: &T) { let name = t.s(2).map(|n| n.trim().to_string()).unwrap_or_default(); let (min, max, center) = (t.p(10).xy(), t.p(11).xy(), t.p(12).xy()); + // A modern file keeps the view it was saved at in the first `*ACTIVE` record (12/22 centre, + // 40 height) as well as in the HEADER pair, and as with the snap and grid spacing the record + // is the newer of the two and wins (#1602). A record that leaves the centre out has no view + // to give -- `p` reads a missing coordinate as zero -- so the header pair stays. + let first_active = name.eq_ignore_ascii_case(VportConfig::ACTIVE) && !d.vports.iter().any(|c| c.is_active()); + let centred = t.f(12).is_some() && t.f(22).is_some(); let height = t.fd(40, 0.0); let unit = |v: f64| v.is_finite() && (-1e-9..=1.0 + 1e-9).contains(&v); let ok = [min.x, min.y, max.x, max.y].into_iter().all(unit) @@ -326,6 +333,9 @@ fn vport_record(d: &mut Drawing, t: &T) { if name.is_empty() || !ok { return; } + if first_active && centred { + d.header.set_saved_view(center, height); + } let aspect = t.f(41).filter(|a| a.is_finite() && *a > 0.0).unwrap_or(0.0); let clamp = |p: Vec2| Vec2::new(p.x.clamp(0.0, 1.0), p.y.clamp(0.0, 1.0)); let tile = VportTile { min: clamp(min), max: clamp(max), center, height, aspect }; diff --git a/crates/io/src/dxf_write/header.rs b/crates/io/src/dxf_write/header.rs index b628a5c3d..6a9c63032 100644 --- a/crates/io/src/dxf_write/header.rs +++ b/crates/io/src/dxf_write/header.rs @@ -44,6 +44,10 @@ const HEADER_VARS: &[(&str, i32)] = &[ ("INSUNITS", 70), ("MEASUREMENT", 70), ("PSLTSCALE", 70), + // The model view the drawing is saved at, as DXF keeps it in HEADER (the `*ACTIVE` VPORT + // record below repeats it). Written so a drawing opened on its saved view saves it back. + ("VIEWCTR", -2), + ("VIEWSIZE", 40), ]; /// The current dimension style's variables as header variables (`$DIMTXT`, `$DIMDEC`…), after diff --git a/crates/io/src/dxf_write/layout.rs b/crates/io/src/dxf_write/layout.rs index 4ad19ffbb..7384fb766 100644 --- a/crates/io/src/dxf_write/layout.rs +++ b/crates/io/src/dxf_write/layout.rs @@ -63,13 +63,17 @@ pub(super) fn paper_view(w: &mut W, d: &Drawing, e: &Entity, owner: &str, cx: &C /// The VPORT table: the `*Active` model space tiles (one viewport on the extents when there is no /// tiled configuration) and the saved configurations, each tile a record, the current one first. pub(super) fn vport_table(w: &mut W, d: &Drawing, ext: Bounds2) { - let single = VportTile { - min: Vec2::ZERO, - max: Vec2::new(1.0, 1.0), - center: if ext.is_empty() { Vec2::new(6.0, 4.5) } else { ext.center() }, - height: if ext.is_empty() { 9.0 } else { (ext.height() * 1.1).max(1e-6) }, - aspect: 1.6, + // The view the drawing is saved at, so that reading a drawing and writing it back keeps it; + // a drawing that stores none is written on its extents. The fallback fits the width as well, + // through the aspect the record is written with: a drawing 100 wide and 10 tall otherwise got + // a view 11 tall, which shows a sliver of it, and a horizontal line got 1e-6 (#1602). + const ASPECT: f64 = 1.6; + let (center, height) = match d.header.saved_view() { + Some(v) => v, + None if ext.is_empty() => (Vec2::new(6.0, 4.5), 9.0), + None => (ext.center(), (ext.height().max(ext.width() / ASPECT) * 1.1).max(1e-6)), }; + let single = VportTile { min: Vec2::ZERO, max: Vec2::new(1.0, 1.0), center, height, aspect: ASPECT }; let tiled = d.vports.iter().find(|c| c.is_active() && c.tiles.len() > 1); let mut records: Vec<(&str, &VportTile)> = match tiled { Some(c) => c.tiles.iter().map(|t| (VportConfig::ACTIVE, t)).collect(), diff --git a/crates/io/src/tests.rs b/crates/io/src/tests.rs index 977a7c184..230c202f7 100644 --- a/crates/io/src/tests.rs +++ b/crates/io/src/tests.rs @@ -518,6 +518,75 @@ fn pdf_hostile_options_and_empty() { } } +/// The view a drawing was saved at is the `*ACTIVE` VPORT record's centre and height, repeated in +/// the `$VIEWCTR`/`$VIEWSIZE` header pair. It used to be dropped on read and replaced on write by +/// a fit of the geometry extents, so a drawing with far-off geometry opened hundreds of times too +/// far out, and a conversion replaced wherever the drawing had been left (#1602). +#[test] +fn dxf_keeps_the_view_the_drawing_was_saved_at() { + const SAVED: &str = "\ +0\nSECTION\n2\nHEADER\n9\n$ACADVER\n1\nAC1027\n9\n$VIEWCTR\n10\n50.0\n20\n50.0\n30\n0.0\n9\n$VIEWSIZE\n40\n20.0\n0\nENDSEC\n\ +0\nSECTION\n2\nTABLES\n0\nTABLE\n2\nVPORT\n0\nVPORT\n2\n*ACTIVE\n70\n0\n10\n0.0\n20\n0.0\n11\n1.0\n21\n1.0\n\ +12\n50.0\n22\n50.0\n40\n20.0\n41\n1.5\n0\nENDTAB\n0\nENDSEC\n\ +0\nSECTION\n2\nENTITIES\n0\nLINE\n8\n0\n10\n0.0\n20\n0.0\n30\n0.0\n11\n100.0\n21\n60.0\n31\n0.0\n\ +0\nLINE\n8\n0\n10\n5000000.0\n20\n5000000.0\n30\n0.0\n11\n5000100.0\n21\n5000060.0\n31\n0.0\n0\nENDSEC\n0\nEOF\n"; + + // The stored view survives the read, far-away geometry and all: the extents here are + // 5 000 100 x 5 000 060, a fit of which is 275 000 times the height the drawing was saved at. + let d = read_dxf(SAVED.as_bytes()).unwrap(); + assert_eq!(d.header.saved_view(), Some((Vec2::new(50.0, 50.0), 20.0))); + assert!(d.extents(&Space::Model).max.x > 5e6, "the far-off line is read too"); + + // And writing the drawing back keeps it, in both places a reader looks. + let text = write_dxf(&d); + let back = read_dxf(text.as_bytes()).unwrap(); + assert_eq!(back.header.saved_view(), Some((Vec2::new(50.0, 50.0), 20.0))); + let active = text.replace("\r\n", "\n"); + let active = active.split("*Active").nth(1).unwrap_or_default(); + for group in ["\n 12\n50", "\n 22\n50", "\n 40\n20"] { + assert!(active.contains(group), "the *ACTIVE record keeps {group:?}: {}", &active[..active.len().min(400)]); + } + + // A drawing that stores no view is still written on its extents. + let mut plain = d.clone(); + plain.header.vars.remove("VIEWCTR"); + plain.header.vars.remove("VIEWSIZE"); + assert_eq!(plain.header.saved_view(), None); + let fitted = read_dxf(write_dxf(&plain).as_bytes()).unwrap(); + let (center, height) = fitted.header.saved_view().unwrap(); + assert!(center.x > 2.4e6 && height > 5e6, "the extents fit: {center:?} {height}"); + + // That fit covers the width as well: a wide, flat drawing opened on its height alone would + // show a sliver of it, and a drawing with no height at all would have no view. + let mut wide = cadcraft_doc::Drawing::new_metric(); + for (a, b) in [([0.0, 0.0], [100.0, 10.0]), ([0.0, 10.0], [100.0, 10.0])] { + let line = cadcraft_doc::Line { a: Vec3::new(a[0], a[1], 0.0), b: Vec3::new(b[0], b[1], 0.0) }; + wide.add(&Space::Model, cadcraft_doc::Common::default(), EntityKind::Line(line)).unwrap(); + } + let back = read_dxf(write_dxf(&wide).as_bytes()).unwrap(); + let (_, height) = back.header.saved_view().unwrap(); + assert!(height > 60.0, "a 100 x 10 drawing needs more than its height: {height}"); + + let mut flat = cadcraft_doc::Drawing::new_metric(); + let line = cadcraft_doc::Line { a: Vec3::ZERO, b: Vec3::new(100.0, 0.0, 0.0) }; + flat.add(&Space::Model, cadcraft_doc::Common::default(), EntityKind::Line(line)).unwrap(); + let back = read_dxf(write_dxf(&flat).as_bytes()).unwrap(); + let (_, height) = back.header.saved_view().unwrap(); + assert!(height > 60.0, "a horizontal line needs a view with width in it: {height}"); +} + +/// A `*ACTIVE` record that leaves its view centre out (no 12/22) is not a view: `$VIEWCTR` and +/// `$VIEWSIZE` stay, rather than being replaced by the zero the missing groups read as. +#[test] +fn a_viewport_record_without_a_centre_keeps_the_header_view() { + const NO_CENTRE: &str = "\ +0\nSECTION\n2\nHEADER\n9\n$VIEWCTR\n10\n50.0\n20\n50.0\n9\n$VIEWSIZE\n40\n20.0\n0\nENDSEC\n\ +0\nSECTION\n2\nTABLES\n0\nTABLE\n2\nVPORT\n0\nVPORT\n2\n*ACTIVE\n70\n0\n10\n0.0\n20\n0.0\n11\n1.0\n21\n1.0\n\ +40\n99.0\n0\nENDTAB\n0\nENDSEC\n0\nSECTION\n2\nENTITIES\n0\nLINE\n8\n0\n10\n0.0\n20\n0.0\n11\n9.0\n21\n9.0\n0\nENDSEC\n0\nEOF\n"; + let d = read_dxf(NO_CENTRE.as_bytes()).unwrap(); + assert_eq!(d.header.saved_view(), Some((Vec2::new(50.0, 50.0), 20.0)), "the header pair is kept"); +} + #[test] fn dxf_roundtrips_page_setup() { let mut d = sample();