XCLIP: invert flag as group 290, extents are the boundary's box, Delete with no boundary asks again; clip on main's new Insert literals

crates/doc/src/extents.rs

@@ -179,12 +179,12 @@ fn bounds_in<'a>(d: &'a Drawing, e: &Entity, depth: usize, blocks: &mut HashMap<
                     }
                 };
                 b = if inner.is_empty() { Bounds2::EMPTY } else { Bounds2::from_points(inner.corners().map(|c| m.apply(c))) };
-                // XCLIP: what shows is within the boundary (an inverted clip can show anything).
+                // XCLIP: the reference app takes the boundary's box, also where it reaches past the
+                // block or misses it (an inverted clip can show anything, so it keeps the block's).
                 if let Some(c) = ins.clip.as_ref().filter(|c| c.active() && !c.inverted)
                     && !b.is_empty()
                 {
-                    let cb = Bounds2::from_points(c.polygon().into_iter().map(|p| m.apply(p)));
-                    b = if b.intersects(&cb) { Bounds2 { min: b.min.max(cb.min), max: b.max.min(cb.max) } } else { Bounds2::EMPTY };
+                    b = Bounds2::from_points(c.polygon().into_iter().map(|p| m.apply(p)));
                 }
                 if ins.cols > 1 || ins.rows > 1 {
                     let off =
@@ -493,6 +493,7 @@ mod tests {
                     rows: 1,
                     col_spacing: 0.0,
                     row_spacing: 0.0,
+                    clip: None,
                 }),
             )
         };

crates/engine/src/cmd/blocks/xclip.rs

@@ -280,6 +280,15 @@ impl XclipM {
         }
     }
 
+    /// Delete: as the reference app, with nothing to delete it says so and asks again.
+    fn delete(&mut self, s: &mut Session) -> Result<Step> {
+        if !self.any_clipped(s) {
+            s.echo("No clip boundary found.");
+            return Ok(Step::Continue);
+        }
+        self.apply(s, json!({ "option": "delete" }))
+    }
+
     fn any_clipped(&self, s: &Session) -> bool {
         self.refs.iter().any(|h| clip_of(s, *h).is_some())
     }
@@ -352,7 +361,7 @@ impl Interactive for XclipM {
                         s.echo("Clip depth does not apply to a 2D drawing: front and back clipping planes are ignored.");
                         Ok(Step::Done)
                     }
-                    Some(k) if k.starts_with('d') => self.apply(s, json!({ "option": "delete" })),
+                    Some(k) if k.starts_with('d') => self.delete(s),
                     Some(k) if k.starts_with('p') || k.starts_with('g') => self.apply(s, json!({ "option": "polyline" })),
                     Some(k) if k.starts_with('n') => {
                         self.stage = if self.any_clipped(s) { Stage::DeleteOld } else { Stage::Boundary };

crates/engine/src/select.rs

@@ -527,6 +527,7 @@ mod tests {
             rows: 1,
             col_spacing: 0.0,
             row_spacing: 0.0,
+            clip: None,
         };
         let h = d.add(&Space::Model, Common::default(), EntityKind::Insert(insert)).unwrap();
         (d, h)

crates/engine/tests/it/xclip.rs

@@ -80,6 +80,23 @@ fn rectangular_clip_cuts_only_that_reference() {
     assert!(clip(&s, &r1).is_some());
 }
 
+#[test]
+fn extents_are_the_boundary_box_as_in_the_reference_app() {
+    let (mut s, r1, _) = setup();
+    let bounds = |s: &Session| {
+        let d = s.doc().unwrap();
+        cadcraft_engine::doc::entity_bounds(d, d.entity(Handle::parse_hex(&r1).unwrap()).unwrap(), 0)
+    };
+    // A boundary reaching past the block (0..100 x -50..50): its own box, not the overlap.
+    s.execute("xclip", &json!({ "handle": r1, "rectangle": [[50, -20], [150, 30]] })).unwrap();
+    let b = bounds(&s);
+    assert!((b.min - Vec2::new(50.0, -20.0)).length() < 1e-9 && (b.max - Vec2::new(150.0, 30.0)).length() < 1e-9, "{b:?}");
+    // One that misses the block entirely: still its box.
+    s.execute("xclip", &json!({ "handle": r1, "rectangle": [[200, 200], [300, 300]] })).unwrap();
+    let b = bounds(&s);
+    assert!((b.min - Vec2::new(200.0, 200.0)).length() < 1e-9 && (b.max - Vec2::new(300.0, 300.0)).length() < 1e-9, "{b:?}");
+}
+
 #[test]
 fn rotated_and_scaled_reference_gets_a_polygon_in_block_coordinates() {
     let (mut s, _, r2) = setup();
@@ -189,6 +206,14 @@ fn typed_xclip_follows_the_prompts() {
     s.input(Input::Enter).unwrap();
     s.cmdline("d").unwrap();
     assert!(clip(&s, &r1).is_none() && clip(&s, &r2).is_none());
+    // Delete with no boundary left says so and asks again.
+    s.start("xclip").unwrap();
+    s.input(pick(&r1)).unwrap();
+    s.input(Input::Enter).unwrap();
+    s.cmdline("d").unwrap();
+    assert!(s.log.iter().any(|l| l.contains("No clip boundary found.")), "{:?}", s.log);
+    assert!(s.prompt_text().contains("Enter clipping option"), "{}", s.prompt_text());
+    s.input(Input::Cancel).unwrap();
     // XCLIPFRAME typed.
     s.cmdline("xclipframe").unwrap();
     s.cmdline("1").unwrap();

crates/io/src/dxf_clip.rs

@@ -4,10 +4,7 @@
 //! 10/20 the vertices), whether clipping is on (71), the front and back clip (72/73, 40/41) and
 //! two 3×4 matrices (group 40, row by row): the inverse of the reference's transform, which takes
 //! the boundary into the block's coordinates, and the boundary's own transform (identity here).
-//!
-//! An inverted clip is CADCraft's own entry `CADCRAFT_XCLIP` beside `ACAD_FILTER` in the same
-//! extension dictionary: an XRECORD with group 70 = 1. Other programs keep it as unknown data and
-//! clip normally.
+//! Group 290 is 1 for an inverted clip ("Invert clip"), as the reference app writes it.
 //!
 //! Reading takes the planar filters the model can hold out of the kept foreign data and puts them
 //! on the references ([`read`]); others (front/back clipping, a skewed boundary transform) stay
@@ -21,8 +18,6 @@ use cadcraft_doc::foreign::{Foreign, ForeignObject};
 use cadcraft_doc::{ClipBoundary, Drawing, EntityKind, Handle, MAX_CLIP_VERTICES, RawTag};
 use cadcraft_geom::{Mat3, Vec2};
 
-/// The extension-dictionary entry of CADCraft's inverted-clip flag.
-const INVERT_KEY: &str = "CADCRAFT_XCLIP";
 /// Handles above this are not handed out (the DXF handle space is 64-bit, keep a margin).
 const MAX_HANDLE: u64 = 1 << 48;
 
@@ -125,18 +120,10 @@ fn parse_filter(o: &ForeignObject) -> Option<ClipBoundary> {
         }
         points.push(p);
     }
-    let clip = ClipBoundary { points, enabled: number(&o.tags, 71).unwrap_or(1.0) != 0.0, inverted: false };
+    let clip = ClipBoundary { points, enabled: number(&o.tags, 71).unwrap_or(1.0) != 0.0, inverted: number(&o.tags, 290).is_some_and(|v| v != 0.0) };
     (clip.points.len() == count && cadcraft_render::clip::PolyClip::new(&clip.polygon()).is_some()).then_some(clip)
 }
 
-/// Whether the extension dictionary `xd` marks the clip inverted.
-fn inverted(f: &Foreign, xd: Handle) -> bool {
-    object(f, xd)
-        .and_then(|o| entry(&o.tags, INVERT_KEY))
-        .and_then(|h| object(f, h))
-        .is_some_and(|o| o.kind.eq_ignore_ascii_case("XRECORD") && number(&o.tags, 70).is_some_and(|v| v != 0.0))
-}
-
 /// Put the planar filters of the file read into `d` on their references, taking them out of
 /// the kept foreign data (the model writes them from now on).
 pub(crate) fn read(d: &mut Drawing) {
@@ -148,10 +135,8 @@ pub(crate) fn read(d: &mut Drawing) {
         .entities
         .keys()
         .filter_map(|h| {
-            let (xd, _, o) = chain(&d.foreign, *h)?;
-            let mut clip = parse_filter(o)?;
-            clip.inverted = inverted(&d.foreign, xd);
-            Some((*h, clip))
+            let (_, _, o) = chain(&d.foreign, *h)?;
+            Some((*h, parse_filter(o)?))
         })
         .collect();
     if found.is_empty() {
@@ -188,14 +173,13 @@ pub(crate) fn read(d: &mut Drawing) {
 }
 
 /// Take the clip of entity `h` out of the foreign data: the ACAD_FILTER entry and what it holds,
-/// CADCraft's inverted flag, and the extension dictionary when nothing else is left in it.
+/// and the extension dictionary when nothing else is left in it.
 fn forget(f: &mut Foreign, h: Handle) {
     let Some(xd) = f.entities.get(&h).and_then(|x| x.xdict) else { return };
     let mut gone: Vec<Handle> = Vec::new();
     let mut empty = false;
     if let Some(o) = f.objects.iter_mut().find(|o| o.handle == xd) {
         gone.extend(remove_entry(&mut o.tags, "ACAD_FILTER"));
-        gone.extend(remove_entry(&mut o.tags, INVERT_KEY));
         empty = !o.tags.iter().any(|t| t.code == 3);
     }
     // The filter dictionary's entries go with it.
@@ -264,26 +248,10 @@ fn spatial_filter(h: Handle, owner: Handle, clip: &ClipBoundary, to_world: &Mat3
     for v in matrix_values(inverse).into_iter().chain(matrix_values(&Mat3::IDENTITY)) {
         tags.push(raw(40, v));
     }
+    tags.push(raw(290, i32::from(clip.inverted)));
     ForeignObject { handle: h, kind: "SPATIAL_FILTER".into(), tags }
 }
 
-fn invert_record(h: Handle, owner: Handle) -> ForeignObject {
-    ForeignObject {
-        handle: h,
-        kind: "XRECORD".into(),
-        tags: vec![
-            raw(5, h.hex()),
-            raw(102, "{ACAD_REACTORS"),
-            raw(330, owner.hex()),
-            raw(102, "}"),
-            raw(330, owner.hex()),
-            raw(100, "AcDbXrecord"),
-            raw(280, 1),
-            raw(70, 1),
-        ],
-    }
-}
-
 /// A copy of `d` whose foreign data holds the SPATIAL_FILTER of every clipped reference (any
 /// filter kept from the file for it is replaced). `None` when no reference is clipped.
 pub(crate) fn prepare(d: &Drawing) -> Option<Drawing> {
@@ -328,20 +296,13 @@ pub(crate) fn prepare(d: &Drawing) -> Option<Drawing> {
             }
         };
         let (fd, sf) = (fresh(&mut next), fresh(&mut next));
-        let flag = clip.inverted.then(|| fresh(&mut next));
         if let Some(o) = f.objects.iter_mut().find(|o| o.handle == xd) {
             o.tags.extend([raw(3, "ACAD_FILTER"), raw(360, fd.hex())]);
-            if let Some(x) = flag {
-                o.tags.extend([raw(3, INVERT_KEY), raw(360, x.hex())]);
-            }
         }
         let mut filters = dictionary(fd, xd);
         filters.tags.extend([raw(3, "SPATIAL"), raw(360, sf.hex())]);
         f.objects.push(filters);
         f.objects.push(spatial_filter(sf, fd, &clip, &to_world, &inverse));
-        if let Some(x) = flag {
-            f.objects.push(invert_record(x, xd));
-        }
     }
     Some(out)
 }
@@ -369,6 +330,19 @@ mod tests {
         assert!(parse_filter(&o(vec![raw(70, 2), raw(10, "nan"), raw(20, 0), raw(10, 1), raw(20, 1)])).is_none());
         assert!(parse_filter(&o(vec![raw(70, 2), raw(10, 0), raw(20, 0), raw(10, 1), raw(20, 1), raw(72, 1)])).is_none());
         let ok = parse_filter(&o(vec![raw(70, 2), raw(10, 0), raw(20, 0), raw(10, 1), raw(20, 1)])).unwrap();
-        assert_eq!(ok.points.len(), 2);
+        assert_eq!((ok.points.len(), ok.inverted), (2, false));
+    }
+
+    #[test]
+    fn group_290_is_the_inverted_flag() {
+        let tags = |invert: &str| vec![raw(70, 2), raw(10, 0), raw(20, 0), raw(10, 1), raw(20, 1), raw(290, invert)];
+        let o = |invert| ForeignObject { handle: Handle(1), kind: "SPATIAL_FILTER".into(), tags: tags(invert) };
+        assert!(parse_filter(&o("1")).unwrap().inverted);
+        assert!(!parse_filter(&o("0")).unwrap().inverted);
+        assert!(!parse_filter(&o("junk")).unwrap().inverted);
+        let clip = ClipBoundary { inverted: true, ..ClipBoundary::new(vec![Vec2::ZERO, Vec2::new(1.0, 1.0)]) };
+        let written = spatial_filter(Handle(2), Handle(1), &clip, &Mat3::IDENTITY, &Mat3::IDENTITY);
+        assert!(written.tags.iter().any(|t| t.code == 290 && t.value == "1"));
+        assert!(parse_filter(&written).unwrap().inverted);
     }
 }

crates/io/tests/it/block_expansion.rs

@@ -103,6 +103,7 @@ fn an_empty_followup_viewport_does_not_mark_an_exact_budget_truncated() {
         rows: 1_000,
         col_spacing: 0.0,
         row_spacing: 0.0,
+        clip: None,
     };
     d.add(&Space::Model, Common::default(), EntityKind::Insert(insert)).unwrap();
     let viewport = |center: Vec3, view_center: Vec2, frozen_layers| Viewport {

crates/io/tests/it/dynblock_read.rs

@@ -462,6 +462,7 @@ fn door_reference(src: &str) -> (cadcraft_doc::Drawing, Handle) {
         rows: 1,
         col_spacing: 0.0,
         row_spacing: 0.0,
+        clip: None,
     };
     let r = d.add(&cadcraft_doc::Space::Model, cadcraft_doc::Common::default(), cadcraft_doc::EntityKind::Insert(ins)).unwrap();
     (d, r)

crates/io/tests/it/roundtrip/cases/blocks.rs

@@ -20,6 +20,7 @@ pub(crate) fn insert(block: &str) -> Insert {
         rows: 1,
         col_spacing: 0.0,
         row_spacing: 0.0,
+        clip: None,
     }
 }
 

crates/io/tests/it/roundtrip/compare.rs

@@ -263,7 +263,7 @@ fn attrib(expected: &Attrib, actual: &Attrib, context: &str) {
 }
 
 fn insert(expected: &Insert, actual: &Insert, context: &str) {
-    let Insert { block, insert, scale, rotation, attribs, cols, rows, col_spacing, row_spacing } = expected;
+    let Insert { block, insert, scale, rotation, attribs, cols, rows, col_spacing, row_spacing, clip } = expected;
     equal(block, &actual.block, context, "insert.block");
     equal(insert, &actual.insert, context, "insert.insert");
     equal(scale, &actual.scale, context, "insert.scale");
@@ -272,6 +272,7 @@ fn insert(expected: &Insert, actual: &Insert, context: &str) {
     equal(rows, &actual.rows, context, "insert.rows");
     equal(col_spacing, &actual.col_spacing, context, "insert.col_spacing");
     equal(row_spacing, &actual.row_spacing, context, "insert.row_spacing");
+    equal(clip, &actual.clip, context, "insert.clip");
     equal(&attribs.len(), &actual.attribs.len(), context, "insert.attribs.len");
     for (index, (a, b)) in attribs.iter().zip(&actual.attribs).enumerate() {
         attrib(a, b, &format!("{context}.attrib[{index}]"));

crates/io/tests/it/xclip_roundtrip.rs

@@ -77,7 +77,11 @@ fn clips_round_trip_through_dxf() {
     let bytes = save(&d, "a.dxf", Version::R2018);
     let text = String::from_utf8(bytes.clone()).unwrap();
     assert_eq!(text.matches("SPATIAL_FILTER").count(), 3, "one filter per clipped reference");
-    assert!(text.contains("ACAD_FILTER") && text.contains("CADCRAFT_XCLIP"));
+    assert!(text.contains("ACAD_FILTER") && !text.contains("CADCRAFT_XCLIP"));
+    // The inverted triangle is marked on its filter (group 290), as the reference app writes it.
+    let filters: Vec<&str> = text.split("\r\nSPATIAL_FILTER\r\n").skip(1).map(|f| f.split("\r\n  0\r\n").next().unwrap_or("")).collect();
+    assert!(filters.iter().all(|f| f.contains("\r\n290\r\n")), "every filter says whether it is inverted");
+    assert_eq!(filters.iter().filter(|f| f.contains("\r\n290\r\n1\r\n")).count(), 1);
     let first = read(&bytes, "a.dxf").unwrap();
     same(&clips(&first), &clips(&d));
     // The model owns the filters now: nothing of them is left as foreign data.