diff --git a/apps/cadcraft-cli/src/output.rs b/apps/cadcraft-cli/src/output.rs index 83fa17dfd..46283753e 100644 --- a/apps/cadcraft-cli/src/output.rs +++ b/apps/cadcraft-cli/src/output.rs @@ -162,19 +162,31 @@ pub(crate) fn export_framed(s: &Session, out: &Path, window: Option, io /// as unknown objects, `Proxy` in `info`), or `None` when there are none. pub(crate) fn undrawn_message(d: &cadcraft_engine::doc::Drawing, path: &str) -> Option { let mut types: std::collections::BTreeMap<&str, usize> = std::collections::BTreeMap::new(); + let mut ole = 0usize; for e in d.model.iter() { + // An OLE2FRAME's frame outline is drawn, so it isn't an undrawn entity; its content is + // not, which gets a note of its own. if let cadcraft_engine::doc::EntityKind::Unknown(u) = &e.kind { + if u.is_ole_frame() && cadcraft_engine::doc::ole_frame(&u.tags).is_some() { + ole = ole.saturating_add(1); + continue; + } let n = types.entry(u.dxf_type.as_str()).or_insert(0); *n = n.saturating_add(1); } } let n = types.values().fold(0usize, |a, n| a.saturating_add(*n)); - if n == 0 { - return None; + let mut lines = Vec::new(); + if n > 0 { + let list: Vec = types.iter().map(|(t, n)| format!("{t}: {n}")).collect(); + let entities = if n == 1 { "entity" } else { "entities" }; + lines.push(format!("{path}: {n} {entities} not drawn ({})", list.join(", "))); } - let list: Vec = types.iter().map(|(t, n)| format!("{t}: {n}")).collect(); - let entities = if n == 1 { "entity" } else { "entities" }; - Some(format!("{path}: {n} {entities} not drawn ({})", list.join(", "))) + if ole > 0 { + let objects = if ole == 1 { "object" } else { "objects" }; + lines.push(format!("{path}: {ole} OLE {objects} shown as the frame only (the OLE content is not drawn)")); + } + if lines.is_empty() { None } else { Some(lines.join("\n")) } } /// The reader's notes on what it left out of `path` (a damaged DWG's unreadable object records, diff --git a/apps/cadcraft-cli/tests/it/undrawn_entities.rs b/apps/cadcraft-cli/tests/it/undrawn_entities.rs index 7e7189379..59762d283 100644 --- a/apps/cadcraft-cli/tests/it/undrawn_entities.rs +++ b/apps/cadcraft-cli/tests/it/undrawn_entities.rs @@ -7,6 +7,25 @@ fn cli(args: &[&str]) -> Output { Command::new(env!("CARGO_BIN_EXE_cadcraft-cli")).args(args).output().unwrap() } +/// A fresh scratch directory that is removed on drop, also when an assertion fails. +struct Scratch(std::path::PathBuf); + +impl Scratch { + fn new(tag: &str) -> Self { + let dir = std::env::temp_dir().join(format!("cc-cli-{tag}-{}", std::process::id())); + // A directory left behind by an earlier run with the same process id must not leak in. + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir(&dir).expect("create scratch dir"); + Scratch(dir) + } +} + +impl Drop for Scratch { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.0); + } +} + /// A line, two foreign REGIONs and a PDF underlay: the last three are kept as unknown entities. const PROXIES: &str = "0\nSECTION\n2\nENTITIES\n\ 0\nLINE\n8\n0\n10\n0\n20\n0\n11\n5\n21\n5\n\ @@ -17,8 +36,8 @@ const PROXIES: &str = "0\nSECTION\n2\nENTITIES\n\ #[test] fn convert_to_an_image_warns_about_entities_it_cannot_draw() { - let dir = std::env::temp_dir().join(format!("cc-cli-undrawn-{}", std::process::id())); - std::fs::create_dir_all(&dir).unwrap(); + let scratch = Scratch::new("undrawn"); + let dir = scratch.0.clone(); let input = dir.join("proxies.dxf"); std::fs::write(&input, PROXIES).unwrap(); let input = input.to_str().unwrap(); @@ -49,5 +68,37 @@ fn convert_to_an_image_warns_about_entities_it_cannot_draw() { let out = cli(&["convert", sample.to_str().unwrap(), dir.join("sample.svg").to_str().unwrap()]); assert!(out.status.success()); assert!(!String::from_utf8_lossy(&out.stderr).contains("not drawn"), "{}", String::from_utf8_lossy(&out.stderr)); - let _ = std::fs::remove_dir_all(&dir); +} + +/// An OLE2FRAME's frame outline is drawn, so an image export doesn't call it an undrawn entity (#2422); it +/// says once that the OLE content inside the frame is not drawn (review on #2587). +#[test] +fn an_ole_frame_is_noted_as_frame_only() { + let scratch = Scratch::new("ole"); + let dir = scratch.0.clone(); + let input = dir.join("ole.dxf"); + let ole = "0\nSECTION\n2\nENTITIES\n0\nOLE2FRAME\n8\n0\n100\nAcDbOle2Frame\n10\n0\n20\n0\n30\n0\n11\n5\n21\n-3\n31\n0\n0\nENDSEC\n0\nEOF\n"; + std::fs::write(&input, ole).unwrap(); + let out = cli(&["convert", input.to_str().unwrap(), dir.join("out.png").to_str().unwrap()]); + assert!(out.status.success()); + let err = String::from_utf8_lossy(&out.stderr); + assert!(!err.contains("entity not drawn") && !err.contains("entities not drawn"), "{err}"); + let notes: Vec<&str> = err.lines().filter(|l| l.contains("OLE")).collect(); + assert_eq!(notes, [format!("{}: 1 OLE object shown as the frame only (the OLE content is not drawn)", input.display())], "{err}"); +} + +/// An OLE2FRAME without usable corners (here group 11/21 missing) has no outline to draw, so it is +/// an undrawn entity, not "shown as the frame only" (review on #2587). +#[test] +fn an_ole_frame_without_corners_is_counted_as_not_drawn() { + let scratch = Scratch::new("ole-nocorner"); + let dir = scratch.0.clone(); + let input = dir.join("ole.dxf"); + let ole = "0\nSECTION\n2\nENTITIES\n0\nOLE2FRAME\n8\n0\n100\nAcDbOle2Frame\n10\n0\n20\n0\n30\n0\n0\nENDSEC\n0\nEOF\n"; + std::fs::write(&input, ole).unwrap(); + let out = cli(&["convert", input.to_str().unwrap(), dir.join("out.png").to_str().unwrap()]); + assert!(out.status.success()); + let err = String::from_utf8_lossy(&out.stderr); + assert!(err.contains(&format!("{}: 1 entity not drawn (OLE2FRAME: 1)", input.display())), "{err}"); + assert!(!err.contains("frame only"), "{err}"); } diff --git a/crates/doc/src/entity.rs b/crates/doc/src/entity.rs index 535506806..832e0925f 100644 --- a/crates/doc/src/entity.rs +++ b/crates/doc/src/entity.rs @@ -60,6 +60,13 @@ pub struct Unknown { pub tags: Vec, } +impl Unknown { + /// Whether this is an OLE2FRAME (the type name is matched without regard to case). + pub fn is_ole_frame(&self) -> bool { + self.dxf_type.eq_ignore_ascii_case("OLE2FRAME") + } +} + #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] #[serde(tag = "type", rename_all = "camelCase")] pub enum EntityKind { @@ -469,6 +476,7 @@ impl EntityKind { let a = if mirror { t.rotation() } else { t.rotation() + rot }; t.dir = Vec2::from_angle(cadcraft_geom::norm_angle(a)).to3(0.0); } + EntityKind::Unknown(u) if u.is_ole_frame() => crate::ole_frame_transform(&mut u.tags, m), EntityKind::Unknown(_) => {} } } diff --git a/crates/doc/src/extents.rs b/crates/doc/src/extents.rs index e2a748606..618da4aab 100644 --- a/crates/doc/src/extents.rs +++ b/crates/doc/src/extents.rs @@ -9,6 +9,218 @@ use crate::{Drawing, Entity, EntityKind, LwPolyline, Prim}; /// Nested block references deeper than this are ignored (cyclic or hostile files). pub const MAX_BLOCK_DEPTH: usize = 16; +/// Positions in `tags` of the groups outside application-defined `102 {` .. `102 }` groups, whose +/// codes are the application's own and not the entity's. +fn outside_app(tags: &[crate::RawTag]) -> impl Iterator + '_ { + let mut in_app = false; + tags.iter().enumerate().filter_map(move |(i, t)| { + if t.code == 102 { + in_app = t.value.starts_with('{'); + None + } else { + (!in_app).then_some(i) + } + }) +} + +/// The positions of the frame's corner groups 10, 20, 11, 21 (in that order) and their values: +/// the first of each outside application groups, found in one pass that stops at the last. +fn ole_corner_groups(tags: &[crate::RawTag]) -> Option<[(usize, f64); 4]> { + let mut found: [Option<(usize, f64)>; 4] = [None; 4]; + for i in outside_app(tags) { + let Some(t) = tags.get(i) else { continue }; + let slot = match t.code { + 10 => 0, + 20 => 1, + 11 => 2, + 21 => 3, + _ => continue, + }; + if let Some(f) = found.get_mut(slot).filter(|f| f.is_none()) { + *f = Some((i, t.value.trim().parse::().ok().filter(|v| v.is_finite())?)); + if found.iter().all(Option::is_some) { + break; + } + } + } + let [Some(a), Some(b), Some(c), Some(d)] = found else { return None }; + Some([a, b, c, d]) +} + +/// The closed outline of an OLE2FRAME from its upper-left (groups 10, 20) and lower-right +/// (groups 11, 21) corners (DXF Reference, OLE2FRAME); `None` when either is missing or not finite. +pub fn ole_frame(tags: &[crate::RawTag]) -> Option<[Vec2; 5]> { + let [(_, x1), (_, y1), (_, x2), (_, y2)] = ole_corner_groups(tags)?; + let (a, b) = (Vec2::new(x1, y1), Vec2::new(x2, y2)); + Some([a, Vec2::new(b.x, a.y), b, Vec2::new(a.x, b.y), a]) +} + +/// The bytes of the OLE data before its corners: a 2-byte marker. +const OLE_HEADER: usize = 2; +/// The corners in the OLE data: four of x, y, z as little-endian doubles (upper left, upper right, +/// lower right, lower left). +const OLE_CORNERS: usize = 12; + +/// Replace the four corners at the start of the OLE data (the 310 groups' hex, in order) by `new` +/// (upper left, upper right, lower right, lower left), keeping z and every group's length and +/// letter case. Only the groups that cover the header are decoded and rewritten; the rest of the +/// payload is not touched. Data that is not valid hex, too short, or whose corners are not the old +/// `frame`'s is left alone. +fn ole_data_transform(tags: &mut [crate::RawTag], new: [Vec2; 4], frame: [f64; 4]) { + const END: usize = OLE_HEADER + OLE_CORNERS * 8; + let mut bytes: Vec = Vec::with_capacity(END); + // The groups read, and how many leading bytes of each belong to the header. + let mut spans: Vec<(usize, usize)> = Vec::new(); + for i in outside_app(tags).filter(|&i| tags.get(i).is_some_and(|t| t.code == 310)) { + if bytes.len() >= END { + break; + } + let Some(t) = tags.get(i) else { return }; + let s = t.value.trim(); + if s.len() % 2 != 0 || !s.is_ascii() { + return; + } + let take = (s.len() / 2).min(END - bytes.len()); + for k in 0..take { + // `from_str_radix` also takes a sign, so each digit is checked first. + let Some(h) = s.get(2 * k..2 * k + 2).filter(|h| h.bytes().all(|b| b.is_ascii_hexdigit())) else { return }; + let Ok(b) = u8::from_str_radix(h, 16) else { return }; + bytes.push(b); + } + spans.push((i, take)); + } + if bytes.len() < END { + return; + } + // Only data whose corners are this frame's corners (UL, UR, LR, LL) has the layout above; any + // other header is not ours to rewrite. + let [x1, y1, x2, y2] = frame; + let want = [(x1, y1), (x2, y1), (x2, y2), (x1, y2)]; + // Two corners match within a fixed tolerance for ordinary coordinates (CORNER_EPS at most), or + // within a few ulps of the coordinate where that is larger, which is from about 1.1e9 up. + let near = |a: f64, b: f64| { + let m = a.abs().max(b.abs()); + (a - b).abs() <= (1e-9 * m.max(1.0)).min(CORNER_EPS).max(4.0 * f64::EPSILON * m) + }; + for (c, (wx, wy)) in want.iter().enumerate() { + let at = OLE_HEADER + c * 24; + let rd = |o: usize| bytes.get(o..o + 8).and_then(|s| <[u8; 8]>::try_from(s).ok()).map(f64::from_le_bytes); + let (Some(x), Some(y)) = (rd(at), rd(at + 8)) else { return }; + if !(near(x, *wx) && near(y, *wy)) { + return; + } + } + for (c, p) in new.iter().enumerate() { + let at = OLE_HEADER + c * 24; + if let Some(s) = bytes.get_mut(at..at + 8) { + s.copy_from_slice(&p.x.to_le_bytes()); + } + if let Some(s) = bytes.get_mut(at + 8..at + 16) { + s.copy_from_slice(&p.y.to_le_bytes()); + } + } + let mut pos = 0; + for (i, n) in spans { + let (Some(t), Some(chunk)) = (tags.get_mut(i), bytes.get(pos..pos + n)) else { return }; + // Leave any padding around the digits, and the digits after the header, as they were. + let start = t.value.len() - t.value.trim_start().len(); + let hex = recase(chunk, t.value.get(start..).unwrap_or_default()); + t.value.replace_range(start..start + 2 * n, &hex); + pos += n; + } +} + +/// The hex of `chunk`, each digit in the case of the digit at the same place in `old`. A digit that +/// was a number and becomes a letter takes the case of the nearest letter in `old`, upper when there +/// is none, so bytes that did not change keep their exact text. +fn recase(chunk: &[u8], old: &str) -> String { + let old = old.as_bytes(); + // For each place, whether the letter nearest to it (the one at it, else the closest before or + // after; the one before on a tie) is lower case: one pass for the next letter at or after each + // place, one for the previous, then the nearer of the two. + let mut next: Vec> = vec![None; old.len()]; + let mut seen: Option<(usize, bool)> = None; + for (i, c) in old.iter().enumerate().rev() { + if c.is_ascii_alphabetic() { + seen = Some((i, c.is_ascii_lowercase())); + } + if let Some(slot) = next.get_mut(i) { + *slot = seen; + } + } + let mut lower_at = vec![false; old.len()]; + let mut prev: Option<(usize, bool)> = None; + for (i, c) in old.iter().enumerate() { + if c.is_ascii_alphabetic() { + prev = Some((i, c.is_ascii_lowercase())); + } + let after = next.get(i).copied().flatten(); + let lower = match (prev, after) { + (Some((p, lp)), Some((q, lq))) => { + if i - p <= q - i { + lp + } else { + lq + } + } + (Some((_, l)), None) | (None, Some((_, l))) => l, + (None, None) => false, + }; + if let Some(slot) = lower_at.get_mut(i) { + *slot = lower; + } + } + let mut out = String::with_capacity(chunk.len() * 2); + for (k, b) in chunk.iter().enumerate() { + for (j, d) in [b >> 4, b & 15].into_iter().enumerate() { + let digit = char::from_digit(d.into(), 16).unwrap_or('0'); + out.push(if lower_at.get(2 * k + j).copied().unwrap_or(false) { digit } else { digit.to_ascii_uppercase() }); + } + } + out +} + +/// The fixed part of the tolerance when matching the corners in the OLE data to the frame's. The +/// header holds the same f64 the frame groups print, so they agree to a few ulps. Up to about 1.1e9 +/// a fixed tolerance of at most this decides, so that distinct geometry does not match; above that, +/// four ulps of the coordinate (`4 * EPSILON * |x|`) exceed it and decide instead. +const CORNER_EPS: f64 = 1e-6; + +/// Transform an OLE2FRAME by `m`: its two corners are mapped through `m` (a DWG save writes the +/// frame's position from these groups), and so are the corners at the start of its OLE data (310 +/// groups), which a reader of the saved file may trust instead. The frame stays axis aligned with +/// its upper-left corner first: the result is the box of the two mapped corners. Commands refuse a +/// rotation or mirror (see [`ole_frame_accepts`]), but internal callers such as EXPLODE, REFEDIT and +/// paste still apply them, and must leave a consistent frame. A frame with no usable corners is +/// left alone. +pub fn ole_frame_transform(tags: &mut [crate::RawTag], m: &cadcraft_geom::Mat3) { + let Some([(ix1, x1), (iy1, y1), (ix2, x2), (iy2, y2)]) = ole_corner_groups(tags) else { return }; + let frame = [x1, y1, x2, y2]; + let (a, b) = (m.apply(Vec2::new(x1, y1)), m.apply(Vec2::new(x2, y2))); + if !(a.x.is_finite() && a.y.is_finite() && b.x.is_finite() && b.y.is_finite()) { + return; + } + let (lo, hi) = (Vec2::new(a.x.min(b.x), a.y.min(b.y)), Vec2::new(a.x.max(b.x), a.y.max(b.y))); + let (ul, lr) = (Vec2::new(lo.x, hi.y), Vec2::new(hi.x, lo.y)); + for (i, v) in [(ix1, ul.x), (iy1, ul.y), (ix2, lr.x), (iy2, lr.y)] { + if let Some(t) = tags.get_mut(i) { + t.value = v.to_string(); + } + } + ole_data_transform(tags, [ul, Vec2::new(lr.x, ul.y), lr, Vec2::new(ul.x, lr.y)], frame); +} + +/// Whether `m` can move an OLE2FRAME: a translation (move, copy, stretch of the whole frame) or a +/// uniform scale with a positive factor, which keep the frame axis aligned and its upper-left corner +/// upper-left. +pub fn ole_frame_accepts(m: &cadcraft_geom::Mat3) -> bool { + const T: f64 = 1e-12; + // The tolerance grows with the scale: a matrix built from two angles (ALIGN) leaves rounding noise + // in b and c of about `scale * 1e-16`. + let t = T * m.a.abs(); + m.b.abs() < t && m.c.abs() < t && m.a > 0.0 && (m.a - m.d).abs() <= t +} + /// Bounds of a single-line TEXT / ATTDEF / ATTRIB of `len` characters, following its justification /// the same way the renderer places it (the stroke font is about 0.88 of the height per character). fn text_box(t: &crate::Text, len: usize) -> Bounds2 { @@ -283,6 +495,8 @@ fn bounds_in<'a>(d: &'a Drawing, e: &Entity, depth: usize, blocks: &mut HashMap< Bounds2::from_points([o, o + u, o + v, o + u + v]) } EntityKind::LwPolyline(p) => lwpolyline_bounds(p), + // An OLE object's content is not drawn, but its frame is, so the frame counts (#2422). + EntityKind::Unknown(u) if u.is_ole_frame() => ole_frame(&u.tags).map_or(Bounds2::EMPTY, Bounds2::from_points), kind => { let mut b = Bounds2::EMPTY; for p in kind.prims() { @@ -478,6 +692,41 @@ mod tests { assert!(bounds(EntityKind::MLeader(empty)).max.x < 26.0); } + /// An OLE2FRAME counts in the extents through its corner tags (#2422); a missing or + /// non-finite corner counts as nothing. + #[test] + fn ole_frame_counts_in_extents() { + let tag = |code: i32, value: &str| crate::RawTag { code, value: value.into() }; + let frame = |tags: Vec| EntityKind::Unknown(crate::Unknown { dxf_type: "OLE2FRAME".into(), tags }); + let b = bounds(frame(vec![tag(10, "30"), tag(20, "-19"), tag(11, "35"), tag(21, "-22")])); + assert_eq!((b.min, b.max), (Vec2::new(30.0, -22.0), Vec2::new(35.0, -19.0))); + assert!(bounds(frame(vec![tag(10, "1"), tag(20, "2")])).is_empty(), "one corner is not a frame"); + assert!(bounds(frame(vec![tag(10, "NaN"), tag(20, "0"), tag(11, "1"), tag(21, "inf")])).is_empty()); + assert!(bounds(EntityKind::Unknown(crate::Unknown { dxf_type: "OTHER".into(), tags: vec![tag(10, "5"), tag(20, "5")] })).is_empty()); + } + + /// Groups 10/20 inside an application-defined `102 {...}` group belong to the application, not + /// to the frame: the real corners come after it (review on #2587). + #[test] + fn ole_frame_skips_application_groups() { + let tag = |code: i32, value: &str| crate::RawTag { code, value: value.into() }; + let tags = vec![ + tag(102, "{ACAD_REACTORS"), + tag(10, "900"), + tag(20, "900"), + tag(11, "901"), + tag(21, "901"), + tag(102, "}"), + tag(10, "30"), + tag(20, "-19"), + tag(11, "35"), + tag(21, "-22"), + ]; + let q = ole_frame(&tags).expect("frame"); + assert_eq!((q[0], q[2]), (Vec2::new(30.0, -19.0), Vec2::new(35.0, -22.0))); + assert!(ole_frame(&tags[..6]).is_none(), "corners only inside the group are not a frame"); + } + #[test] fn a_shared_block_cache_measures_as_a_fresh_one_for_each_entity() { let line = |a: (f64, f64), b: (f64, f64), layer: &str| { @@ -558,4 +807,231 @@ mod tests { let b = entity_bounds(&d, &array(u32::MAX, 1, 0.0), 0); assert!((b.max.x - (10.0 * 9_999.0 + 1.0)).abs() < 1e-6, "{b:?}"); } + + /// An OLE2FRAME at large coordinates, with its OLE data header holding the frame's corners + /// shifted by `dx` in x. + fn big_frame(dx: f64) -> Vec { + let (x1, y1, x2, y2) = (1e9, 1e9, 1e9 + 10.0, 1e9 + 5.0); + let mut blob = vec![1u8, 0]; + for (x, y) in [(x1, y1), (x2, y1), (x2, y2), (x1, y2)] { + for v in [x + dx, y, 7.0] { + blob.extend_from_slice(&v.to_le_bytes()); + } + } + let tag = |code: i32, value: String| crate::RawTag { code, value }; + vec![ + tag(10, x1.to_string()), + tag(20, y1.to_string()), + tag(11, x2.to_string()), + tag(21, y2.to_string()), + tag(310, blob.iter().map(|b| format!("{b:02X}")).collect()), + ] + } + + /// The x of the first corner in the OLE data of a [`big_frame`]. + fn data_x(tags: &[crate::RawTag]) -> f64 { + let hex = tags.last().map(|t| t.value.as_str()).unwrap_or_default(); + let bytes: Vec = (0..hex.len() / 2).filter_map(|i| u8::from_str_radix(hex.get(2 * i..2 * i + 2)?, 16).ok()).collect(); + bytes.get(2..10).and_then(|s| <[u8; 8]>::try_from(s).ok()).map(f64::from_le_bytes).unwrap_or(f64::NAN) + } + + /// The match between the frame and the corners in its OLE data does not loosen with the + /// coordinate: half a unit off at 1e9 is not the frame, so the bytes stay as they are (review + /// on #2587). + #[test] + fn a_half_unit_offset_at_large_coordinates_is_not_the_frame() { + let mut tags = big_frame(0.5); + let before = tags.last().cloned(); + ole_frame_transform(&mut tags, &cadcraft_geom::Mat3::translate(Vec2::new(1.0, 1.0))); + assert_eq!(tags.first().map(|t| t.value.as_str()), Some("1000000001"), "the frame tags move"); + assert_eq!(tags.last().map(|t| &t.value), before.as_ref().map(|t| &t.value), "the 310 bytes stay identical"); + } + + #[test] + fn exact_corners_at_large_coordinates_still_move() { + let mut tags = big_frame(0.0); + ole_frame_transform(&mut tags, &cadcraft_geom::Mat3::translate(Vec2::new(1.0, 1.0))); + assert_eq!(data_x(&tags), 1e9 + 1.0); + } + + /// A translation, and a uniform positive scale about any point, keep a frame axis aligned and + /// upper-left; a rotation, a mirror, a non-uniform or non-positive scale do not. + #[test] + fn ole_frame_accepts_translations_and_uniform_positive_scales_only() { + use cadcraft_geom::Mat3; + let p = Vec2::new(3.0, -4.0); + assert!(ole_frame_accepts(&Mat3::translate(p))); + assert!(ole_frame_accepts(&Mat3::scale_about(p, 2.5))); + assert!(ole_frame_accepts(&Mat3::scale_about(p, 1e-3))); + assert!(!ole_frame_accepts(&Mat3::scale(2.0, 3.0))); + assert!(!ole_frame_accepts(&Mat3::scale_about(p, -1.0))); + assert!(!ole_frame_accepts(&Mat3::scale_about(p, 0.0))); + assert!(!ole_frame_accepts(&Mat3::rotate_about(p, 0.5))); + assert!(!ole_frame_accepts(&Mat3::mirror(Vec2::new(0.0, 0.0), Vec2::new(1.0, 0.0)))); + } + /// ALIGN with a scale builds its matrix from two angles that agree only to rounding, so a large + /// scale leaves a rotation part of about `scale * 1e-16`. That is a scale, not a turn: the + /// tolerance for it grows with the scale, while a real turn is still refused (review on #2587). + #[test] + fn ole_frame_accepts_a_large_scale_with_rounding_noise_in_its_rotation() { + use cadcraft_geom::Mat3; + let p = Vec2::new(3.0, -4.0); + let noisy = Mat3::scale_about(p, 1e6).then_before(Mat3::rotate_about(p, 2e-16)); + assert!(noisy.b.abs() > 1e-12, "the setup must exceed an absolute tolerance: {}", noisy.b); + assert!(ole_frame_accepts(&noisy)); + let turned = Mat3::scale_about(p, 1e6).then_before(Mat3::rotate_about(p, 1e-6)); + assert!(!ole_frame_accepts(&turned)); + } + + /// The corners (x, y) in the OLE data header of a frame built by [`big_frame`]-style tags. + fn header_corners(tags: &[crate::RawTag]) -> Vec<(f64, f64)> { + let hex: String = tags.iter().filter(|t| t.code == 310).map(|t| t.value.as_str()).collect(); + let bytes: Vec = (0..hex.len() / 2).filter_map(|i| u8::from_str_radix(hex.get(2 * i..2 * i + 2)?, 16).ok()).collect(); + let f = |o: usize| bytes.get(o..o + 8).and_then(|s| <[u8; 8]>::try_from(s).ok()).map(f64::from_le_bytes).unwrap_or(f64::NAN); + (0..4).map(|c| (f(2 + c * 24), f(10 + c * 24))).collect() + } + + fn small_frame() -> Vec { + let mut blob = vec![1u8, 0]; + for (x, y) in [(30.0f64, -19.0f64), (35.0, -19.0), (35.0, -22.0), (30.0, -22.0)] { + for v in [x, y, 7.0] { + blob.extend_from_slice(&v.to_le_bytes()); + } + } + let tag = |code: i32, value: &str| crate::RawTag { code, value: value.into() }; + vec![ + tag(10, "30"), + tag(20, "-19"), + tag(11, "35"), + tag(21, "-22"), + crate::RawTag { code: 310, value: blob.iter().map(|b| format!("{b:02X}")).collect() }, + ] + } + + /// Internal callers (EXPLODE, REFEDIT, paste, array placement) transform a frame by any matrix. + /// It stays axis aligned: groups 10/20 are the upper-left of the box of the mapped corners, + /// 11/21 the lower-right, and the OLE data holds the same box (review on #2587). + #[test] + fn a_rotated_frame_stays_axis_aligned_and_consistent() { + use cadcraft_geom::Mat3; + let rotate = Mat3::rotate_about(Vec2::new(1.0, 2.0), 30f64.to_radians()); + let mirror = Mat3::mirror(Vec2::new(0.0, 0.0), Vec2::new(1.0, 0.0)); + for m in [rotate, mirror] { + let mut tags = small_frame(); + let mapped = [m.apply(Vec2::new(30.0, -19.0)), m.apply(Vec2::new(35.0, -22.0))]; + let (minx, maxx) = (mapped[0].x.min(mapped[1].x), mapped[0].x.max(mapped[1].x)); + let (miny, maxy) = (mapped[0].y.min(mapped[1].y), mapped[0].y.max(mapped[1].y)); + ole_frame_transform(&mut tags, &m); + let g = |code: i32| tags.iter().find(|t| t.code == code).and_then(|t| t.value.parse::().ok()).unwrap_or(f64::NAN); + assert_eq!((g(10), g(20), g(11), g(21)), (minx, maxy, maxx, miny)); + assert_eq!(header_corners(&tags), vec![(minx, maxy), (maxx, maxy), (maxx, miny), (minx, miny)]); + } + } + /// `from_str_radix` accepts a leading sign, so "+A" must not count as a hex byte (review on + /// #2587): data with one is not valid and MOVE leaves its bytes alone. + #[test] + fn a_plus_sign_in_ole_data_is_not_hex() { + let mut tags = small_frame(); + let hex = tags.last().map(|t| t.value.clone()).unwrap_or_default(); + // The marker's first byte is 01: "+1" would decode to the same byte. + let signed = format!("+1{}", hex.get(2..).unwrap_or_default()); + if let Some(t) = tags.last_mut() { + t.value = signed.clone(); + } + ole_frame_transform(&mut tags, &cadcraft_geom::Mat3::translate(Vec2::new(10.0, 5.0))); + assert_eq!(tags.first().map(|t| t.value.as_str()), Some("40"), "the frame tags still move"); + assert_eq!(tags.last().map(|t| t.value.as_str()), Some(signed.as_str())); + } + + /// A payload of 1,000 groups of 127 bytes: only the group(s) holding the 98-byte header are + /// rewritten. Later groups stay as they were, even ones that are not valid hex or have padding + /// (the old code re-encoded the whole payload and gave up on any bad group). + #[test] + fn a_large_ole_payload_only_rewrites_its_leading_groups() { + let mut tags = small_frame(); + let first = tags.pop().map(|t| t.value).unwrap_or_default(); + let mut head = first.clone(); + // Fill the first group up to 127 bytes. + head.push_str(&"ab".repeat(127 - first.len() / 2)); + tags.push(crate::RawTag { code: 310, value: head }); + for g in 1..1000 { + let value = match g { + 2 => format!(" {} ", "Ab".repeat(127)), + 500 => "ZZ".repeat(127), + _ => "ab".repeat(127), + }; + tags.push(crate::RawTag { code: 310, value }); + } + let rest: Vec = tags.iter().skip(5).map(|t| t.value.clone()).collect(); + ole_frame_transform(&mut tags, &cadcraft_geom::Mat3::translate(Vec2::new(10.0, 5.0))); + assert_eq!(header_corners(&tags[..5]), vec![(40.0, -14.0), (45.0, -14.0), (45.0, -17.0), (40.0, -17.0)]); + let after: Vec = tags.iter().skip(5).map(|t| t.value.clone()).collect(); + assert_eq!(after, rest, "every group after the first is string-identical"); + assert_eq!(tags.get(4).map(|t| t.value.len()), Some(254), "the first group keeps its length"); + } + + /// Above about 8.6e9 one ulp of an f64 is more than the fixed ceiling on the corner match, so a + /// text corner one ulp from the header double must still count as the same corner (review on + /// #2587). + #[test] + fn ole_frame_moves_at_2e10_with_a_one_ulp_text_corner() { + let mut tags = big_frame(0.0); + // Rebuild the frame at 2e10: the header keeps the exact corners, the text is one ulp off. + let (x1, y1, x2, y2) = (2e10f64, 2e10f64, 2e10 + 10.0, 2e10 + 5.0); + let mut blob = vec![1u8, 0]; + for (x, y) in [(x1, y1), (x2, y1), (x2, y2), (x1, y2)] { + for v in [x, y, 7.0] { + blob.extend_from_slice(&v.to_le_bytes()); + } + } + let text_x1 = f64::from_bits(x1.to_bits() + 1); + tags[0].value = text_x1.to_string(); + tags[1].value = y1.to_string(); + tags[2].value = x2.to_string(); + tags[3].value = y2.to_string(); + if let Some(t) = tags.last_mut() { + t.value = blob.iter().map(|b| format!("{b:02X}")).collect(); + } + ole_frame_transform(&mut tags, &cadcraft_geom::Mat3::translate(Vec2::new(1e4, 1e4))); + assert_eq!(header_corners(&tags)[0].0, text_x1 + 1e4, "the header moved with the frame"); + } + + /// Below scale 1 the rotation bound is still relative to the scale: a small real turn at a tiny + /// scale is refused, and so is a non-uniform scale whose two factors differ by more than a + /// relative 1e-12 (review on #2587). + #[test] + fn ole_frame_refuses_a_small_turn_at_a_tiny_scale() { + use cadcraft_geom::Mat3; + let p = Vec2::new(3.0, -4.0); + let turned = Mat3::scale_about(p, 1e-9).then_before(Mat3::rotate_about(p, 0.05f64.to_radians())); + assert!(!ole_frame_accepts(&turned)); + assert!(ole_frame_accepts(&Mat3::scale_about(p, 1e-9))); + assert!(!ole_frame_accepts(&Mat3::scale(1e-6, 1e-6 + 5e-13))); + } + + /// Hex digits keep the case they had: bytes the transform does not change come back as the + /// identical text, also in a group that mixes upper and lower case (review on #2587). + #[test] + fn moving_an_ole_frame_keeps_each_hex_digits_case() { + let mut tags = small_frame(); + let old = tags.last().map(|t| t.value.clone()).unwrap_or_default(); + let mixed: String = old.chars().enumerate().map(|(i, c)| if i < 24 { c.to_ascii_lowercase() } else { c }).collect(); + assert_ne!(mixed, old); + if let Some(t) = tags.last_mut() { + t.value = mixed.clone(); + } + ole_frame_transform(&mut tags, &cadcraft_geom::Mat3::translate(Vec2::new(10.0, 5.0))); + let new = tags.last().map(|t| t.value.clone()).unwrap_or_default(); + assert_eq!(new.len(), mixed.len()); + let byte = |s: &str, i: usize| s.get(2 * i..2 * i + 2).and_then(|h| u8::from_str_radix(h, 16).ok()); + let mut same = 0; + for i in 0..mixed.len() / 2 { + if byte(&mixed, i) == byte(&new, i) { + same += 1; + assert_eq!(mixed.get(2 * i..2 * i + 2), new.get(2 * i..2 * i + 2), "byte {i}"); + } + } + assert!(same > 40, "most of the header is unchanged: {same}"); + assert_eq!(header_corners(&tags)[0], (40.0, -14.0)); + } } diff --git a/crates/doc/src/lib.rs b/crates/doc/src/lib.rs index 2b11a5bd3..873e83b83 100644 --- a/crates/doc/src/lib.rs +++ b/crates/doc/src/lib.rs @@ -29,7 +29,10 @@ pub use cadcraft_color as color; pub use cadcraft_geom as geom; pub use constraint::*; pub use entity::*; -pub use extents::{BlockBoundsCache, MAX_BLOCK_DEPTH, entity_bounds, entity_bounds_cached, extent_bounds, extent_bounds_cached}; +pub use extents::{ + BlockBoundsCache, MAX_BLOCK_DEPTH, entity_bounds, entity_bounds_cached, extent_bounds, extent_bounds_cached, ole_frame, ole_frame_accepts, + ole_frame_transform, +}; pub use format::Format; pub use gdt::*; pub use header::{HVal, Header}; diff --git a/crates/engine/src/cmd/array.rs b/crates/engine/src/cmd/array.rs index 452cf176f..a443e03f8 100644 --- a/crates/engine/src/cmd/array.rs +++ b/crates/engine/src/cmd/array.rs @@ -248,9 +248,16 @@ fn max_count(s: &Session, others: u64, objs: usize) -> u64 { } fn copy_all(s: &mut Session, hs: &[Handle], mats: impl Iterator) -> Result { + // Every copy is checked before the first is made: a refusal part way would leave the copies + // already added in the drawing (a failed command is not rolled back). + let mats: Vec = mats.collect(); + // A matrix every frame accepts needs no look at the objects; otherwise they are looked at once. + if mats.iter().any(|m| !cadcraft_doc::ole_frame_accepts(m)) && super::modify::holds_ole_frame(s, hs, true)? { + return Err(super::modify::ole_refusal()); + } let mut n = 0; - for m in mats { - n += transform_entities(s, hs, &m, true)?.len(); + for m in &mats { + n += transform_entities(s, hs, m, true)?.len(); } Ok(n) } diff --git a/crates/engine/src/cmd/modify.rs b/crates/engine/src/cmd/modify.rs index 7e31cdebe..367dacc65 100644 --- a/crates/engine/src/cmd/modify.rs +++ b/crates/engine/src/cmd/modify.rs @@ -35,7 +35,7 @@ use oops::run_oops; pub(crate) use oops::{Erased, erase_kept}; use select_then::{DisplaceM, Op, SelectThen}; use stretch::{StretchM, run_stretch}; -pub(crate) use transform::transform_entities; +pub(crate) use transform::{holds_ole_frame, ole_refusal, refuse_ole_frames, transform_entities}; use transform::{run_copy, run_erase, run_mirror, run_move, run_rotate, run_scale}; pub(crate) use trim::{Edges, extend, trim}; diff --git a/crates/engine/src/cmd/modify/transform.rs b/crates/engine/src/cmd/modify/transform.rs index 11e52ea23..df43f782c 100644 --- a/crates/engine/src/cmd/modify/transform.rs +++ b/crates/engine/src/cmd/modify/transform.rs @@ -5,6 +5,7 @@ use super::{Entity, Handle, Mat3, Result, Session, Value, Vec2, bad, bool_or, er // ---------------- core operations ---------------- pub(crate) fn transform_entities(s: &mut Session, hs: &[Handle], m: &Mat3, copy: bool) -> Result> { + refuse_ole_frames(s, hs, m, copy)?; edit_entities(s, hs, copy, |e| e.kind.transform(m)) } @@ -12,6 +13,7 @@ pub(crate) fn transform_entities(s: &mut Session, hs: &[Handle], m: &Mat3, copy: /// and every height of the objects about its Z, their thickness too. pub(crate) fn scale_entities(s: &mut Session, hs: &[Handle], base: Vec3, f: f64, copy: bool) -> Result> { let m = Mat3::scale_about(base.xy(), f); + refuse_ole_frames(s, hs, &m, copy)?; edit_entities(s, hs, copy, |e| { e.kind.transform(&m); e.kind.scale_heights(base.z, f); @@ -19,6 +21,34 @@ pub(crate) fn scale_entities(s: &mut Session, hs: &[Handle], base: Vec3, f: f64, }) } +/// An OLE2FRAME only translates or scales uniformly (its corners stay axis aligned): refuse any other `m` before anything +/// changes, instead of reporting success and leaving the frame where it was. +pub(crate) fn refuse_ole_frames(s: &Session, hs: &[Handle], m: &Mat3, copy: bool) -> Result<()> { + if cadcraft_doc::ole_frame_accepts(m) || !holds_ole_frame(s, hs, copy)? { + return Ok(()); + } + Err(ole_refusal()) +} + +/// The refusal for a transform an OLE2FRAME cannot take. +pub(crate) fn ole_refusal() -> crate::EngineError { + crate::EngineError::Other( + "An OLE object can only be moved, copied or scaled uniformly; other transforms (rotation, mirroring, non-uniform scale, alignment) are not supported.".into(), + ) +} + +/// Whether `hs` holds an OLE2FRAME a transform would touch. An in-place edit (`copy` false) skips +/// an object on a locked layer (see edit_entities), so such a frame cannot be refused. +pub(crate) fn holds_ole_frame(s: &Session, hs: &[Handle], copy: bool) -> Result { + let d = s.doc()?; + Ok(hs.iter().any(|h| { + d.entity(*h).is_some_and(|e| { + let locked = !copy && d.layer(&e.common.layer).is_some_and(|l| l.locked); + !locked && matches!(&e.kind, cadcraft_doc::EntityKind::Unknown(u) if u.is_ole_frame()) + }) + })) +} + /// `p` at the current elevation (ELEVATION), the Z AutoCAD gives a point entered without one. pub(crate) fn at_elevation(s: &Session, p: Vec2) -> Vec3 { let z = s.doc().map(|d| d.header.f64("ELEVATION", 0.0)).ok().filter(|z| z.is_finite()); diff --git a/crates/engine/src/cmd/modify2/mocoro.rs b/crates/engine/src/cmd/modify2/mocoro.rs index c701e24ae..47d6833fb 100644 --- a/crates/engine/src/cmd/modify2/mocoro.rs +++ b/crates/engine/src/cmd/modify2/mocoro.rs @@ -16,7 +16,7 @@ use super::other; use crate::cmd::curves::KW; use crate::cmd::helpers::line; use crate::cmd::machines::{SelOutcome, SelectPhase, number}; -use crate::cmd::modify::transform_entities; +use crate::cmd::modify::{refuse_ole_frames, transform_entities}; use crate::cmd::{bad, point_req, point_value, targets}; use crate::prompt::Drag; use crate::{Accept, Input, Interactive, Prompt, Result, Session, Step}; @@ -108,6 +108,15 @@ pub(super) fn run_mocoro(s: &mut Session, p: &Value) -> Result { } // Every option is checked before the drawing changes. let ops = list.iter().enumerate().map(|(i, v)| parse_op(i, v)).collect::>>()?; + // An OLE frame only translates or scales uniformly: a rotate step (or any step) that would refuse it must do so before an + // earlier move or copy changes the drawing (a refused command is not rolled back). + for op in &ops { + match *op { + Op::Rotate(a) => refuse_ole_frames(s, &objs, &Mat3::rotate_about(base, a), false)?, + Op::Scale(f) => refuse_ole_frames(s, &objs, &Mat3::scale_about(base, f), false)?, + _ => {} + } + } let mut copies = Vec::new(); for op in ops { copies.extend(apply(s, &objs, &mut base, op)?); diff --git a/crates/engine/src/cmd/modify2/patharray.rs b/crates/engine/src/cmd/modify2/patharray.rs index 3f9445249..dd9597177 100644 --- a/crates/engine/src/cmd/modify2/patharray.rs +++ b/crates/engine/src/cmd/modify2/patharray.rs @@ -35,14 +35,23 @@ pub(crate) fn arraypath( }; crate::cmd::array::check_size(s, n as u64, objs.len())?; let Some((p0, t0)) = c.at_length(0.0) else { return Ok(Vec::new()) }; - let mut out = Vec::new(); + let mut mats = Vec::new(); for i in 1..n { let Some((p, t)) = c.at_length(step * i as f64) else { continue }; let mut m = Mat3::translate(p - p0); if align { m = Mat3::rotate_about(p, t.angle() - t0.angle()).then_before(m); } - out.extend(crate::cmd::modify::transform_entities(s, &objs, &m, true)?); + mats.push(m); + } + // Every copy is checked before the first is made: a refusal part way would leave the copies + // already added in the drawing (a failed command is not rolled back). + if mats.iter().any(|m| !cadcraft_doc::ole_frame_accepts(m)) && crate::cmd::modify::holds_ole_frame(s, &objs, true)? { + return Err(crate::cmd::modify::ole_refusal()); + } + let mut out = Vec::new(); + for m in &mats { + out.extend(crate::cmd::modify::transform_entities(s, &objs, m, true)?); } Ok(out) } diff --git a/crates/engine/src/select.rs b/crates/engine/src/select.rs index b92c6b14f..b5ef2dc9f 100644 --- a/crates/engine/src/select.rs +++ b/crates/engine/src/select.rs @@ -36,6 +36,13 @@ pub fn hit_polylines(d: &Drawing, e: &Entity, tol: f64) -> Vec> { } v } + // An OLE object's frame is drawn, so it is picked on its outline (the corners the extents and + // the renderer use); a frame with no width or height is drawn as a line, so it is picked as + // one. It stays out of prims(), which TRIM, object snap and the dimension commands read. + EntityKind::Unknown(u) if u.is_ole_frame() => match cadcraft_doc::ole_frame(&u.tags) { + Some(q) => vec![q.to_vec()], + None => Vec::new(), + }, kind => { let mut out = Vec::new(); for p in kind.prims() { @@ -507,6 +514,23 @@ mod tests { assert_eq!(pick(&d, &Space::Model, Vec2::new(41.3, 50.0), 1.0), Some(line)); } + /// An OLE2FRAME draws as its frame, so a click on an edge picks it (review on #2587); a click in + /// the middle of the frame, or on a frame without both corners, does not. + #[test] + fn clicking_an_ole_frame_edge_selects_it() { + let tag = |code: i32, value: &str| cadcraft_doc::RawTag { code, value: value.into() }; + let frame = |tags: Vec| EntityKind::Unknown(cadcraft_doc::Unknown { dxf_type: "OLE2FRAME".into(), tags }); + let mut d = Drawing::new_metric(); + let h = d.add(&Space::Model, Common::default(), frame(vec![tag(10, "30"), tag(20, "-19"), tag(11, "35"), tag(21, "-22")])).unwrap(); + d.add(&Space::Model, Common::default(), frame(vec![tag(10, "80"), tag(20, "80")])).unwrap(); + let line = d.add(&Space::Model, Common::default(), frame(vec![tag(10, "60"), tag(20, "-19"), tag(11, "60"), tag(21, "-22")])).unwrap(); + assert_eq!(pick(&d, &Space::Model, Vec2::new(32.5, -19.0), 0.2), Some(h), "top edge"); + assert_eq!(pick(&d, &Space::Model, Vec2::new(35.0, -20.5), 0.2), Some(h), "right edge"); + assert_eq!(pick(&d, &Space::Model, Vec2::new(32.5, -20.5), 0.2), None, "inside the frame"); + assert_eq!(pick(&d, &Space::Model, Vec2::new(80.0, 80.0), 0.2), None, "a frame without a second corner"); + assert_eq!(pick(&d, &Space::Model, Vec2::new(60.0, -20.0), 0.2), Some(line), "a frame with no width is drawn as a line and picked as one"); + } + /// A block holding a diagonal dashed line, inserted at `scale`. fn dashed_block(length: f64, scale: f64) -> (Drawing, Handle) { let mut d = Drawing::new_imperial(); diff --git a/crates/engine/tests/ole_frame_transform.rs b/crates/engine/tests/ole_frame_transform.rs new file mode 100644 index 000000000..171af556c --- /dev/null +++ b/crates/engine/tests/ole_frame_transform.rs @@ -0,0 +1,285 @@ +//! An OLE2FRAME is window-selectable (it counts in the extents, #2422): MOVE and COPY carry it by +//! the offset, and the transforms that would not keep it axis aligned are refused before anything +//! reports success (review on #2587). + +use cadcraft_engine::Session; +use cadcraft_engine::doc::{Common, Drawing, EntityKind, RawTag, Space, Unknown}; +use serde_json::json; + +fn session() -> (Session, String) { + let tag = |code: i32, v: &str| RawTag { code, value: v.into() }; + let mut d = Drawing::new_imperial(); + let kind = + EntityKind::Unknown(Unknown { dxf_type: "OLE2FRAME".into(), tags: vec![tag(10, "30"), tag(20, "-19"), tag(11, "35"), tag(21, "-22")] }); + let h = d.add(&Space::Model, Common::default(), kind).unwrap(); + let mut s = Session::empty(); + s.open_drawing(d, "ole.dxf", None); + (s, h.hex()) +} + +fn corners(s: &Session) -> Vec<(i32, String)> { + let d = s.doc().unwrap(); + d.model + .iter() + .filter_map(|e| { + if let EntityKind::Unknown(u) = &e.kind { Some(u.tags.iter().map(|t| (t.code, t.value.clone())).collect::>()) } else { None } + }) + .flatten() + .collect() +} + +#[test] +fn move_and_copy_carry_an_ole_frame() { + let (mut s, h) = session(); + s.execute("move", &json!({ "handles": [h], "delta": [10, 5] })).unwrap(); + let want: Vec<(i32, String)> = [(10, "40"), (20, "-14"), (11, "45"), (21, "-17")].iter().map(|(c, v)| (*c, v.to_string())).collect(); + assert_eq!(corners(&s), want); + s.execute("copy", &json!({ "handles": [h], "delta": [10, 5] })).unwrap(); + let all = corners(&s); + assert_eq!(all.len(), 8); + assert_eq!(all[4..].iter().map(|(_, v)| v.as_str()).collect::>(), ["50", "-9", "55", "-12"]); +} + +#[test] +fn rotate_and_mirror_refuse_an_ole_frame() { + let (mut s, h) = session(); + let before = corners(&s); + for (cmd, p) in + [("rotate", json!({ "handles": [h], "base": [0, 0], "angle": 30 })), ("mirror", json!({ "handles": [h], "p1": [0, 0], "p2": [1, 0] }))] + { + let e = s.execute(cmd, &p).expect_err(cmd); + assert!(e.to_string().contains("OLE object"), "{cmd}: {e}"); + } + assert_eq!(corners(&s), before); + assert_eq!(s.doc().unwrap().model.len(), 1, "nothing was copied"); +} + +/// The OLE data's marker and four corners (x, y, z doubles each), as hex split into chunks. +fn data_session(chunks: &[usize], hex_override: Option<&str>) -> (Session, String) { + let tag = |code: i32, v: &str| RawTag { code, value: v.into() }; + let mut blob = vec![1u8, 0]; + for (x, y) in [(30.0f64, -19.0f64), (35.0, -19.0), (35.0, -22.0), (30.0, -22.0)] { + for v in [x, y, 7.0] { + blob.extend_from_slice(&v.to_le_bytes()); + } + } + let hex: String = hex_override.map(str::to_string).unwrap_or_else(|| blob.iter().map(|b| format!("{b:02X}")).collect()); + let mut tags = vec![tag(10, "30"), tag(20, "-19"), tag(11, "35"), tag(21, "-22")]; + let mut at = 0; + for &n in chunks { + let end = (at + n).min(hex.len()); + tags.push(tag(310, &hex[at..end])); + at = end; + } + if at < hex.len() { + tags.push(tag(310, &hex[at..])); + } + let mut d = Drawing::new_imperial(); + let h = d.add(&Space::Model, Common::default(), EntityKind::Unknown(Unknown { dxf_type: "OLE2FRAME".into(), tags })).unwrap(); + let mut s = Session::empty(); + s.open_drawing(d, "ole.dxf", None); + (s, h.hex()) +} + +fn data_tags(s: &Session) -> Vec { + let d = s.doc().unwrap(); + d.model + .iter() + .filter_map(|e| { + if let EntityKind::Unknown(u) = &e.kind { + Some(u.tags.iter().filter(|t| t.code == 310).map(|t| t.value.clone()).collect::>()) + } else { + None + } + }) + .flatten() + .collect() +} + +#[test] +fn move_also_moves_the_corners_in_the_ole_data() { + let (mut s, h) = data_session(&[10, 100, 40], None); + let before = data_tags(&s); + s.execute("move", &json!({ "handles": [h], "delta": [10, 5] })).unwrap(); + let after = data_tags(&s); + assert_eq!(after.iter().map(String::len).collect::>(), before.iter().map(String::len).collect::>(), "chunk lengths are kept"); + let hex: String = after.concat(); + let bytes: Vec = (0..hex.len() / 2).map(|i| u8::from_str_radix(&hex[2 * i..2 * i + 2], 16).unwrap()).collect(); + let f = |o: usize| f64::from_le_bytes(bytes[o..o + 8].try_into().unwrap()); + let want = [(40.0, -14.0), (45.0, -14.0), (45.0, -17.0), (40.0, -17.0)]; + for (c, (x, y)) in want.iter().enumerate() { + assert_eq!((f(2 + c * 24), f(10 + c * 24), f(18 + c * 24)), (*x, *y, 7.0), "corner {c}"); + } +} + +#[test] +fn move_leaves_malformed_or_short_ole_data_alone() { + for (chunks, hex) in [(vec![4], "0100"), (vec![3, 3], "ABCDE"), (vec![10], "ZZ0100ZZ0100"), (vec![2], "é")] { + let (mut s, h) = data_session(&chunks, Some(hex)); + let before = data_tags(&s); + s.execute("move", &json!({ "handles": [h], "delta": [10, 5] })).unwrap(); + assert_eq!(data_tags(&s), before, "{hex}"); + assert_eq!(corners(&s)[0].1, "40", "the corner groups still move"); + } +} + +/// OLE data whose bytes are not this frame's corners (an unknown header, so the layout does not +/// apply) is not rewritten by MOVE; the frame's own corner groups still move (review on #2587). +#[test] +fn move_leaves_ole_data_that_is_not_the_frames_corners_alone() { + let mut blob = vec![9u8, 9]; + for (x, y) in [(1.0f64, 2.0f64), (3.0, 2.0), (3.0, 4.0), (1.0, 4.0)] { + for v in [x, y, 7.0] { + blob.extend_from_slice(&v.to_le_bytes()); + } + } + let hex: String = blob.iter().map(|b| format!("{b:02X}")).collect(); + let (mut s, h) = data_session(&[40, 100], Some(&hex)); + let before = data_tags(&s); + s.execute("move", &json!({ "handles": [h], "delta": [10, 5] })).unwrap(); + assert_eq!(data_tags(&s), before); + assert_eq!(corners(&s)[0].1, "40", "the corner groups still move"); +} + +#[test] +fn mocoro_refuses_before_it_changes_anything() { + let (mut s, h) = session(); + let before = corners(&s); + let undo = s.state().unwrap().undo.len(); + let ops = json!([{ "move": [10, 5] }, { "copy": [20, 0] }, { "rotate": 30 }]); + let e = s.execute("mocoro", &json!({ "handles": [h], "base": [0, 0], "ops": ops })).expect_err("rotate"); + assert!(e.to_string().contains("OLE object"), "{e}"); + assert_eq!(corners(&s), before, "the earlier move and copy were not applied"); + assert_eq!(s.doc().unwrap().model.len(), 1); + assert_eq!(s.state().unwrap().undo.len(), undo); +} + +#[test] +fn a_locked_ole_frame_does_not_block_an_in_place_rotation() { + let tag = |code: i32, v: &str| RawTag { code, value: v.into() }; + let mut d = Drawing::new_imperial(); + d.ensure_layer("L"); + let common = Common { layer: "L".into(), ..Common::default() }; + let kind = + EntityKind::Unknown(Unknown { dxf_type: "OLE2FRAME".into(), tags: vec![tag(10, "30"), tag(20, "-19"), tag(11, "35"), tag(21, "-22")] }); + let ole = d.add(&Space::Model, common, kind).unwrap(); + if let Some(l) = d.layer_mut("L") { + l.locked = true; + } + let mut s = Session::empty(); + s.open_drawing(d, "ole.dxf", None); + let ln = s.execute("line", &json!({ "points": [[0, 0], [10, 0]] })).unwrap(); + let ln = ln["handle"].as_str().or_else(|| ln["handles"][0].as_str()).unwrap().to_string(); + let before = corners(&s); + s.execute("rotate", &json!({ "handles": [ln.clone(), ole.hex()], "base": [0, 0], "angle": 90 })).unwrap(); + assert_eq!(corners(&s), before, "the locked frame is left alone"); + let d = s.doc().unwrap(); + let e = d.entity(cadcraft_engine::doc::Handle::parse_hex(&ln).unwrap()).unwrap(); + let cadcraft_engine::doc::EntityKind::Line(l) = &e.kind else { panic!("line") }; + assert!((l.b.y - 10.0).abs() < 1e-9 && l.b.x.abs() < 1e-9, "{l:?}"); +} + +/// The four corners (x, y) in the OLE data header, read back from the 310 groups. +fn data_corners(s: &Session) -> Vec<(f64, f64)> { + let hex: String = data_tags(s).concat(); + let bytes: Vec = (0..hex.len() / 2).map(|i| u8::from_str_radix(&hex[2 * i..2 * i + 2], 16).unwrap()).collect(); + let f = |o: usize| f64::from_le_bytes(bytes[o..o + 8].try_into().unwrap()); + (0..4).map(|c| (f(2 + c * 24), f(10 + c * 24))).collect() +} + +fn line_in(s: &mut Session) -> String { + let ln = s.execute("line", &json!({ "points": [[1, 2], [4, 6]] })).unwrap(); + ln["handle"].as_str().or_else(|| ln["handles"][0].as_str()).unwrap().to_string() +} + +fn line_ends(s: &Session, h: &str) -> ((f64, f64), (f64, f64)) { + let e = s.doc().unwrap().entity(cadcraft_engine::doc::Handle::parse_hex(h).unwrap()).unwrap(); + let EntityKind::Line(l) = &e.kind else { panic!("line") }; + ((l.a.x, l.a.y), (l.b.x, l.b.y)) +} + +/// SCALE with a uniform positive factor carries an OLE frame with the rest of the selection: the +/// corner groups and the corners in the OLE data are both mapped (review on #2587). +#[test] +fn scale_carries_an_ole_frame_with_the_rest_of_the_selection() { + let (mut s, h) = data_session(&[10, 100, 40], None); + let ln = line_in(&mut s); + s.execute("scale", &json!({ "handles": [ln.clone(), h], "base": [0, 0], "factor": 2 })).unwrap(); + assert_eq!(line_ends(&s, &ln), ((2.0, 4.0), (8.0, 12.0))); + let want: Vec<(i32, String)> = [(10, "60"), (20, "-38"), (11, "70"), (21, "-44")].iter().map(|(c, v)| (*c, v.to_string())).collect(); + let tags: Vec<(i32, String)> = corners(&s).into_iter().filter(|(c, _)| *c != 310).collect(); + assert_eq!(tags, want); + assert_eq!(data_corners(&s), vec![(60.0, -38.0), (70.0, -38.0), (70.0, -44.0), (60.0, -44.0)]); +} + +/// MOCORO with a scale step carries the frame too, like SCALE. +#[test] +fn mocoro_scales_an_ole_frame() { + let (mut s, h) = data_session(&[10, 100, 40], None); + s.execute("mocoro", &json!({ "handles": [h], "base": [0, 0], "ops": [{ "scale": 3 }] })).unwrap(); + assert_eq!(data_corners(&s)[0], (90.0, -57.0)); + assert_eq!(corners(&s)[0].1, "90"); +} + +/// ARRAYPATH with align rotates each copy, which an OLE frame refuses. The refusal comes before +/// the first copy is added: an error does not roll the drawing back (review on #2587). +#[test] +fn arraypath_refuses_before_adding_any_copy() { + let (mut s, h) = session(); + // Straight for 30 units, then a right-angle bend: the first copies only move, a later one turns. + let path = s.execute("pline", &json!({ "points": [[0, 0], [30, 0], [30, 30]] })).unwrap(); + let path = path["handle"].as_str().or_else(|| path["handles"][0].as_str()).unwrap().to_string(); + let count = s.doc().unwrap().model.len(); + let before = corners(&s); + let e = s.execute("arraypath", &json!({ "handles": [h], "path": path, "count": 7, "align": true })).expect_err("align"); + assert!(e.to_string().contains("OLE object"), "{e}"); + assert_eq!(s.doc().unwrap().model.len(), count, "no copy was added"); + assert_eq!(corners(&s), before); +} + +/// A refused command leaves the drawing and its undo history alone: UNDO afterwards takes back the +/// earlier MOVE, not a phantom step (review on #2587). +#[test] +fn a_refused_rotation_adds_no_undo_step() { + let (mut s, h) = session(); + let original = corners(&s); + s.execute("move", &json!({ "handles": [h.clone()], "delta": [10, 5] })).unwrap(); + let undo = s.state().unwrap().undo.len(); + s.execute("rotate", &json!({ "handles": [h], "base": [0, 0], "angle": 30 })).expect_err("rotate"); + assert_eq!(s.state().unwrap().undo.len(), undo); + s.execute("undo", &json!({})).unwrap(); + assert_eq!(corners(&s), original); +} + +/// The classic -ARRAY makes separate copies: a polar array that turns its items refuses an OLE +/// frame before the first copy, and one that keeps the items upright carries it. +#[test] +fn classic_polar_array_refuses_a_turned_frame_but_not_an_upright_one() { + let (mut s, h) = session(); + let before = corners(&s); + let turned = json!({ "handles": [h.clone()], "type": "polar", "center": [0, 0], "count": 4, "rotate": true }); + let e = s.execute("-array", &turned).expect_err("turned items"); + assert!(e.to_string().contains("OLE object"), "{e}"); + assert_eq!((s.doc().unwrap().model.len(), corners(&s)), (1, before)); + let upright = json!({ "handles": [h], "type": "polar", "center": [0, 0], "count": 4, "rotate": false }); + s.execute("-array", &upright).unwrap(); + assert_eq!(s.doc().unwrap().model.len(), 4); +} + +/// The frame is picked by its outline but is not geometry for other commands: a line across it is +/// not cut at its edges by TRIM, and object snap near its corner offers no frame point (review on +/// #2587). +#[test] +fn ole_frame_is_not_a_trim_edge_or_snap_target() { + use cadcraft_engine::snap::{mode, osnap}; + let (mut s, _frame) = session(); + let ln = s.execute("line", &json!({ "points": [[28, -20.5], [37, -20.5]] })).unwrap(); + let ln = ln["handle"].as_str().or_else(|| ln["handles"][0].as_str()).unwrap().to_string(); + // No cutting edge is named, so every object but the line itself is one. + let _ = s.execute("trim", &json!({ "handle": ln.clone(), "pick": [32, -20.5] })); + assert_eq!(line_ends(&s, &ln), ((28.0, -20.5), (37.0, -20.5)), "the line is not cut at the frame"); + assert_eq!(s.doc().unwrap().model.len(), 2); + let all = mode::END | mode::MID | mode::INT | mode::PER | mode::CEN | mode::QUA | mode::NOD; + let hit = osnap(s.doc().unwrap(), &Space::Model, cadcraft_engine::geom::Vec2::new(30.0, -19.0), 0.5, all, None, false); + assert!(hit.is_none(), "no snap point on the frame: {hit:?}"); +} diff --git a/crates/io/tests/it/main.rs b/crates/io/tests/it/main.rs index 2e6c894a9..57081f97a 100644 --- a/crates/io/tests/it/main.rs +++ b/crates/io/tests/it/main.rs @@ -49,6 +49,8 @@ mod mtext_line_spacing; mod multiline_attributes; #[cfg(not(target_arch = "wasm32"))] mod ole2frame_roundtrip; +#[cfg(not(target_arch = "wasm32"))] +mod ole_frame; mod paperview_marker; mod passthrough_image_roundtrip; mod pdf_followups; diff --git a/crates/io/tests/it/ole_frame.rs b/crates/io/tests/it/ole_frame.rs new file mode 100644 index 000000000..188987db5 --- /dev/null +++ b/crates/io/tests/it/ole_frame.rs @@ -0,0 +1,96 @@ +//! An OLE2FRAME is kept as an unknown entity; a DWG save carries it with its data and frame +//! position (#2422). A DWG's OLE2FRAME object has no corner fields: the position travels in the +//! start of the OLE data (a 2-byte marker, then the frame's four corners as three little-endian +//! doubles each: upper left, upper right, lower right, lower left), which is how the DXF's +//! groups 10 and 11 come back. That layout is read from the acadrust dependency's decoder. + +use cadcraft_doc::EntityKind; +use cadcraft_io::{SaveOptions, Version, read, read_dxf, write_with}; + +const UL: (f64, f64) = (30.14, -18.99); +const LR: (f64, f64) = (35.27, -22.39); + +/// The OLE data: the envelope with the frame's corners. +fn blob() -> Vec { + let mut b = vec![1u8, 0]; + for (x, y) in [UL, (LR.0, UL.1), LR, (UL.0, LR.1)] { + for v in [x, y, 0.0] { + b.extend_from_slice(&v.to_le_bytes()); + } + } + b +} + +/// A 2000 DXF with one OLE2FRAME: corners, the binary data and the end marker (DXF Reference). +fn dxf() -> String { + let data = blob(); + let hex: String = data.iter().map(|b| format!("{b:02X}")).collect(); + format!( + "0\nSECTION\n2\nHEADER\n9\n$ACADVER\n1\nAC1015\n0\nENDSEC\n0\nSECTION\n2\nENTITIES\n\ + 0\nOLE2FRAME\n5\n2B\n100\nAcDbEntity\n8\n0\n100\nAcDbOle2Frame\n70\n2\n3\nOLE Object\n\ + 10\n{}\n20\n{}\n30\n0\n11\n{}\n21\n{}\n31\n0\n71\n2\n72\n0\n90\n{}\n310\n{hex}\n1\nOLE\n\ + 0\nENDSEC\n0\nEOF\n", + UL.0, + UL.1, + LR.0, + LR.1, + data.len() + ) +} + +fn ole(d: &cadcraft_doc::Drawing) -> Option { + d.model.iter().find_map(|e| match &e.kind { + EntityKind::Unknown(u) if u.dxf_type == "OLE2FRAME" => Some(u.clone()), + _ => None, + }) +} + +#[test] +fn a_dwg_keeps_an_ole_frame_with_its_data_and_position() { + let d = read_dxf(dxf().as_bytes()).unwrap(); + assert!(ole(&d).is_some()); + let want: String = blob().iter().map(|b| format!("{b:02X}")).collect(); + for v in [Version::R2000, Version::R2010, Version::R2018] { + let dwg = write_with(&d, "x.dwg", &SaveOptions { version: Some(v), binary: false }).unwrap(); + let back = read(&dwg, "x.dwg").unwrap(); + let u = ole(&back).unwrap_or_else(|| panic!("{v:?}: the OLE2FRAME is gone")); + let data: String = u.tags.iter().filter(|t| t.code == 310).map(|t| t.value.to_ascii_uppercase()).collect(); + let corner = |code: i32| u.tags.iter().find(|t| t.code == code).and_then(|t| t.value.trim().parse::().ok()); + assert_eq!(data, want, "{v:?}: data"); + assert_eq!((corner(10), corner(20), corner(11), corner(21)), (Some(UL.0), Some(UL.1), Some(LR.0), Some(LR.1)), "{v:?}: frame corners"); + } +} + +/// A frame moved through its corner tags (what MOVE does) keeps the new position in a DWG save +/// and read-back (review on #2587). +#[test] +fn a_moved_ole_frame_keeps_its_position_in_a_dwg() { + let mut d = read_dxf(dxf().as_bytes()).unwrap(); + for h in d.model.handles() { + d.model.modify(h, |e| e.kind.transform(&cadcraft_geom::Mat3::translate(cadcraft_geom::Vec2::new(100.0, -50.0)))); + } + let dwg = write_with(&d, "x.dwg", &SaveOptions { version: Some(Version::R2018), binary: false }).unwrap(); + let u = ole(&read(&dwg, "x.dwg").unwrap()).unwrap(); + let c = |code: i32| u.tags.iter().find(|t| t.code == code).and_then(|t| t.value.trim().parse::().ok()).unwrap(); + let near = |a: f64, b: f64| (a - b).abs() < 1e-9; + assert!(near(c(10), UL.0 + 100.0) && near(c(20), UL.1 - 50.0) && near(c(11), LR.0 + 100.0) && near(c(21), LR.1 - 50.0), "{:?}", u.tags); +} + +/// After a move, a DXF save and read-back carries the moved corners both in groups 10/11 and in +/// the OLE data header (review on #2587). +#[test] +fn a_moved_ole_frame_keeps_its_data_header_in_a_dxf() { + let mut d = read_dxf(dxf().as_bytes()).unwrap(); + for h in d.model.handles() { + d.model.modify(h, |e| e.kind.transform(&cadcraft_geom::Mat3::translate(cadcraft_geom::Vec2::new(100.0, -50.0)))); + } + let bytes = write_with(&d, "x.dxf", &SaveOptions { version: None, binary: false }).unwrap(); + let u = ole(&read_dxf(&bytes).unwrap()).unwrap(); + let hex: String = u.tags.iter().filter(|t| t.code == 310).map(|t| t.value.clone()).collect(); + let b: Vec = (0..hex.len() / 2).map(|i| u8::from_str_radix(&hex[2 * i..2 * i + 2], 16).unwrap()).collect(); + let f = |o: usize| f64::from_le_bytes(b[o..o + 8].try_into().unwrap()); + let near = |a: f64, c: f64| (a - c).abs() < 1e-9; + assert!(near(f(2), UL.0 + 100.0) && near(f(10), UL.1 - 50.0) && near(f(2 + 48), LR.0 + 100.0) && near(f(10 + 48), LR.1 - 50.0)); + let g = |code: i32| u.tags.iter().find(|t| t.code == code).and_then(|t| t.value.trim().parse::().ok()).unwrap(); + assert!(near(g(10), UL.0 + 100.0) && near(g(21), LR.1 - 50.0)); +} diff --git a/crates/render/src/draw.rs b/crates/render/src/draw.rs index f779bff5b..5e362e5fc 100644 --- a/crates/render/src/draw.rs +++ b/crates/render/src/draw.rs @@ -227,6 +227,12 @@ fn entity_parts(b: &mut Builder, ctx: &Ctx, e: &Entity) { b.polyline(ctx, rgb, lw, l); } } + // An OLE object's content is not drawn; its frame, from its two corner points, is. + EntityKind::Unknown(u) if u.is_ole_frame() => { + if let Some(q) = cadcraft_doc::ole_frame(&u.tags) { + b.polyline(ctx, rgb, lw, &q); + } + } kind => { for prim in kind.prims() { match prim { diff --git a/crates/render/src/tests.rs b/crates/render/src/tests.rs index 757259583..cbbc83ca9 100644 --- a/crates/render/src/tests.rs +++ b/crates/render/src/tests.rs @@ -1127,6 +1127,20 @@ fn raster_hatch_pattern_matches_one_by_one() { } } +/// An OLE2FRAME draws its frame, the rectangle from its two corner points (#2422), and nothing +/// when a corner is missing. +#[test] +fn ole_frame_draws_its_outline() { + let tag = |code: i32, value: &str| cadcraft_doc::RawTag { code, value: value.into() }; + let frame = |tags: Vec| EntityKind::Unknown(cadcraft_doc::Unknown { dxf_type: "OLE2FRAME".into(), tags }); + let mut d = Drawing::new_imperial(); + d.add(&Space::Model, Common::default(), frame(vec![tag(10, "30"), tag(20, "-19"), tag(11, "35"), tag(21, "-22")])).unwrap(); + d.add(&Space::Model, Common::default(), frame(vec![tag(10, "1"), tag(20, "2")])).unwrap(); + let l = build(&d, &Space::Model, &Options { tolerance: 0.01, ..Default::default() }); + assert_eq!(l.prims.len(), 1, "only the frame with both corners is drawn"); + assert_eq!(l.prims[0].len, 5, "a closed rectangle: five points"); +} + /// `Options::linetypes` off draws a dashed line, a dashed wide polyline and a linetype on a layer /// as the same whole line a Continuous one draws: what hit tests look at. #[test]