diff --git a/crates/doc/src/extents.rs b/crates/doc/src/extents.rs index 548998389..4f40a148c 100644 --- a/crates/doc/src/extents.rs +++ b/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, }), ) }; diff --git a/crates/engine/src/cmd/blocks/xclip.rs b/crates/engine/src/cmd/blocks/xclip.rs index b9e1c0e4f..82ee26fb4 100644 --- a/crates/engine/src/cmd/blocks/xclip.rs +++ b/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 { + 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 }; diff --git a/crates/engine/src/select.rs b/crates/engine/src/select.rs index 6721cdad8..b92c6b14f 100644 --- a/crates/engine/src/select.rs +++ b/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) diff --git a/crates/engine/tests/it/xclip.rs b/crates/engine/tests/it/xclip.rs index 0b1d64443..664acd0bd 100644 --- a/crates/engine/tests/it/xclip.rs +++ b/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(); diff --git a/crates/io/src/dxf_clip.rs b/crates/io/src/dxf_clip.rs index fc3d56316..02614b784 100644 --- a/crates/io/src/dxf_clip.rs +++ b/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 { } 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 = 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 { @@ -328,20 +296,13 @@ pub(crate) fn prepare(d: &Drawing) -> Option { } }; 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); } } diff --git a/crates/io/tests/it/block_expansion.rs b/crates/io/tests/it/block_expansion.rs index 64fca189e..8a6e142a7 100644 --- a/crates/io/tests/it/block_expansion.rs +++ b/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 { diff --git a/crates/io/tests/it/dynblock_read.rs b/crates/io/tests/it/dynblock_read.rs index dd47c43c9..9c4943350 100644 --- a/crates/io/tests/it/dynblock_read.rs +++ b/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) diff --git a/crates/io/tests/it/roundtrip/cases/blocks.rs b/crates/io/tests/it/roundtrip/cases/blocks.rs index a1af6218a..51a8ba0c0 100644 --- a/crates/io/tests/it/roundtrip/cases/blocks.rs +++ b/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, } } diff --git a/crates/io/tests/it/roundtrip/compare.rs b/crates/io/tests/it/roundtrip/compare.rs index d4bd1541a..e263f47f8 100644 --- a/crates/io/tests/it/roundtrip/compare.rs +++ b/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}]")); diff --git a/crates/io/tests/it/xclip_roundtrip.rs b/crates/io/tests/it/xclip_roundtrip.rs index 1e07ecf23..762ac1700 100644 --- a/crates/io/tests/it/xclip_roundtrip.rs +++ b/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.