A tile's rectangle and the view it shows are judged apart

Two from review, both about one `*ACTIVE` record carrying two separate things:
where the tile sits on the screen (10/11) and the view it shows (12/22, 40).

Reading: the two were one `ok`, and the early return came before the view was
read. A first `*ACTIVE` with a usable view and an unusable rectangle therefore
lost the view, and a file whose header carries none opened on the extent fit
instead of where it was saved. They are judged apart now -- `view_ok` decides
the saved view, `tile_ok` decides the tile -- so the rectangle is still refused
while the view it holds is kept.

Writing: the tiled branch emitted the stored tiles as they stand, so updating
`header.set_saved_view` on a drawing with several tiles wrote the new view in
HEADER and the old one in the first `*ACTIVE`. The read takes its view from
that record, so the update did not survive a round trip. The first record now
carries the saved view while keeping its own place on the screen, and the tiles
after it are untouched.

Three tests: a bad rectangle still gives its view (and the rectangle is still
refused), a record with no usable view leaves the header alone, and a view set
on a two-tile drawing comes back from a round trip with both tiles still there,
the first keeping its half of the screen and the second its own view.

crates/io/src/dxf_read/tables.rs

@@ -324,18 +324,22 @@ fn vport_record(d: &mut Drawing, t: &T) {
     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)
-        && max.x > min.x + 1e-6
-        && max.y > min.y + 1e-6
-        && center.is_finite()
-        && height.is_finite()
-        && height > 0.0;
-    if name.is_empty() || !ok {
+    // One record carries two separate things: where the tile sits on the screen (10/11) and the
+    // view it shows (12/22, 40). They are judged apart, because a record can hold a usable view
+    // with a tile rectangle that is not usable -- a first `*ACTIVE` like that used to return
+    // before the view was read, so a file whose header has no view opened on the extent fit
+    // instead of where it was saved (noted in review).
+    let view_ok = center.is_finite() && height.is_finite() && height > 0.0;
+    let tile_ok = [min.x, min.y, max.x, max.y].into_iter().all(unit) && max.x > min.x + 1e-6 && max.y > min.y + 1e-6;
+    if name.is_empty() {
         return;
     }
-    if first_active && centred {
+    if first_active && centred && view_ok {
         d.header.set_saved_view(center, height);
     }
+    if !view_ok || !tile_ok {
+        return;
+    }
     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 };

crates/io/src/dxf_write/layout.rs

@@ -75,9 +75,17 @@ 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, 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(),
-        None => vec![(VportConfig::ACTIVE, &single)],
+    // The first `*ACTIVE` record is the one a reader takes the view from, so it carries the saved
+    // view while keeping its own place on the screen and the tiles after it unchanged. Writing
+    // the stored tiles as they stand put the new view in HEADER and the old one here, and the
+    // read takes this one, so updating the view of a tiled drawing did not survive a round trip
+    // (noted in review).
+    let first = tiled.and_then(|c| c.tiles.first()).map(|t| VportTile { center, height, ..*t });
+    let mut records: Vec<(&str, &VportTile)> = match (tiled, first.as_ref()) {
+        (Some(c), Some(first)) => {
+            std::iter::once((VportConfig::ACTIVE, first)).chain(c.tiles.iter().skip(1).map(|t| (VportConfig::ACTIVE, t))).collect()
+        }
+        _ => vec![(VportConfig::ACTIVE, &single)],
     };
     records.extend(d.vports.iter().filter(|c| !c.is_active() && !c.name.is_empty()).flat_map(|c| c.tiles.iter().map(|t| (c.name.as_str(), t))));
     let th = table_head(w, "VPORT", records.len());

crates/io/src/tests.rs

@@ -587,6 +587,58 @@ fn a_viewport_record_without_a_centre_keeps_the_header_view() {
     assert_eq!(d.header.saved_view(), Some((Vec2::new(50.0, 50.0), 20.0)), "the header pair is kept");
 }
 
+/// Updating the saved view of a drawing that has several model-space tiles survives a round
+/// trip. The write put the new view in HEADER but emitted the stored tiles unchanged, and the
+/// read takes its view from the first `*ACTIVE` record, so the update was read back as the old
+/// view (noted in review of #1602).
+#[test]
+fn a_saved_view_set_on_a_tiled_drawing_survives_a_round_trip() {
+    let mut d = sample();
+    // Two tiles side by side, the first the current one, each with its own old view.
+    let tile = |min: Vec2, max: Vec2, center: Vec2| VportTile { min, max, center, height: 10.0, aspect: 1.0 };
+    d.vports.push(VportConfig {
+        name: VportConfig::ACTIVE.to_string(),
+        tiles: vec![tile(Vec2::ZERO, Vec2::new(0.5, 1.0), Vec2::new(1.0, 1.0)), tile(Vec2::new(0.5, 0.0), Vec2::new(1.0, 1.0), Vec2::new(2.0, 2.0))],
+    });
+    d.header.set_saved_view(Vec2::new(70.0, 80.0), 30.0);
+
+    let back = read_dxf(write_dxf(&d).as_bytes()).unwrap();
+    assert_eq!(back.header.saved_view(), Some((Vec2::new(70.0, 80.0), 30.0)), "the view that was set is the one read back");
+    // The tiled configuration is still there, and the first tile keeps its own half of the screen.
+    let active = back.vports.iter().find(|c| c.is_active()).expect("the tiled configuration is kept");
+    assert_eq!(active.tiles.len(), 2, "both tiles are written: {:?}", active.tiles);
+    assert_eq!(active.tiles[0].max, Vec2::new(0.5, 1.0), "the first tile keeps its rectangle");
+    assert_eq!(active.tiles[1].center, Vec2::new(2.0, 2.0), "and the second tile keeps its own view");
+}
+
+/// A first `*ACTIVE` record can hold a usable view (12/22 centre, 40 height) with a tile
+/// rectangle that is not usable. The record used to be dropped whole before the view was read,
+/// so a file whose header carries no view opened on the extent fit rather than where it was
+/// saved (noted in review of #1602).
+#[test]
+fn a_viewport_record_with_a_bad_tile_still_gives_its_view() {
+    // No `$VIEWCTR`/`$VIEWSIZE`, and a tile whose corners are equal (10/11 both 0,0), so the
+    // rectangle is unusable while the view is not: centre 50,50 and height 20.
+    const BAD_TILE: &str = "\
+0\nSECTION\n2\nHEADER\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\n0.0\n21\n0.0\n\
+12\n50.0\n22\n50.0\n40\n20.0\n0\nENDTAB\n0\nENDSEC\n\
+0\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(BAD_TILE.as_bytes()).unwrap();
+    assert_eq!(d.header.saved_view(), Some((Vec2::new(50.0, 50.0), 20.0)), "the view the record holds is read");
+    // The unusable rectangle is still refused, so the split did not make the tile permissive.
+    assert!(d.vports.iter().all(|c| c.tiles.is_empty()), "the tile itself is not kept: {:?}", d.vports);
+
+    // And a record with no usable view either leaves the header alone, as before.
+    const NEITHER: &str = "\
+0\nSECTION\n2\nHEADER\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\n0.0\n21\n0.0\n\
+12\n50.0\n22\n50.0\n40\n0.0\n0\nENDTAB\n0\nENDSEC\n\
+0\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 neither = read_dxf(NEITHER.as_bytes()).unwrap();
+    assert_eq!(neither.header.saved_view(), None, "a zero height is not a view, so the header keeps none");
+}
+
 #[test]
 fn dxf_roundtrips_page_setup() {
     let mut d = sample();