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..9f29c07b6 100644 --- a/crates/doc/src/extents.rs +++ b/crates/doc/src/extents.rs @@ -9,6 +9,239 @@ 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(); + // Only the rewritten digits and a few after them, for the case of the nearest letter: a + // hostile 310 line can be as long as the file (the value is checked to be ASCII above). + let end = start.saturating_add(2 * n + 64).min(t.value.len()); + let hex = recase(chunk, t.value.get(start..end).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. A matrix that keeps the axes or swaps them (a move, scale, mirror or +/// quarter turn) gives the box of the two mapped corners. Any other turn would collapse that box +/// (a square turned 45° maps its two corners level), so the frame's centre is mapped and its size +/// kept, scaled by `m`'s, its sides swapped when the turn is nearer a quarter turn. 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 t = 1e-12 * (m.a.abs() + m.b.abs() + m.c.abs() + m.d.abs()); + let axes_kept = (m.b.abs() <= t && m.c.abs() <= t) || (m.a.abs() <= t && m.d.abs() <= t); + let (a, b) = if axes_kept { + (m.apply(Vec2::new(x1, y1)), m.apply(Vec2::new(x2, y2))) + } else { + let center = m.apply(Vec2::new((x1 + x2) / 2.0, (y1 + y2) / 2.0)); + // A plain turn keeps the size exactly; cos² + sin² can miss 1 by an ulp or two. + let s = (m.a * m.d - m.b * m.c).abs().sqrt(); + let scale = if (s - 1.0).abs() < 1e-12 { 1.0 } else { s }; + let (mut w, mut h) = ((x2 - x1).abs() * scale, (y2 - y1).abs() * scale); + // Where the frame's x axis goes: nearer to y than to x is nearer a quarter turn. + if m.b.abs() > m.a.abs() { + std::mem::swap(&mut w, &mut h); + } + (center - Vec2::new(w, h) / 2.0, center + Vec2::new(w, h) / 2.0) + }; + 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 +516,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 +713,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 +828,291 @@ 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 its box, 11/21 the lower-right, + /// and the OLE data holds the same box (review on #2587). A mirror keeps the axes, so the box + /// is that of the mapped corners; a 30° turn keeps the frame's own 5 x 3 about its turned + /// centre, where the corners' box would flatten it. + #[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, turned) in [(rotate, true), (mirror, false)] { + let mut tags = small_frame(); + let mapped = if turned { + let c = m.apply(Vec2::new(32.5, -20.5)); + [c - Vec2::new(2.5, 1.5), c + Vec2::new(2.5, 1.5)] + } else { + [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)); + } + + /// A frame from `(x1, y1)` (upper left) to `(x2, y2)` (lower right), its OLE header holding the + /// same corners. + fn frame_at(x1: f64, y1: f64, x2: f64, y2: f64) -> Vec { + let mut blob = vec![1u8, 0]; + for (x, y) in [(x1, y1), (x2, y1), (x2, y2), (x1, y2)] { + for v in [x, y, 0.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()), + ] + } + + fn size(tags: &[crate::RawTag]) -> (f64, f64, Vec2) { + let o = ole_frame(tags).unwrap(); + let b = Bounds2::from_points(o); + (b.width(), b.height(), b.center()) + } + + /// A frame turned by an angle that isn't a quarter turn keeps its size about its turned + /// centre instead of collapsing to the box of its two corners (a 10 x 10 square turned 45° + /// became 14.14 x 0), and the OLE header follows (#2587 review). + #[test] + fn a_frame_turned_off_the_axes_keeps_its_size() { + use cadcraft_geom::Mat3; + let close = |a: f64, b: f64| (a - b).abs() < 1e-9; + let mut tags = frame_at(0.0, 10.0, 10.0, 0.0); + ole_frame_transform(&mut tags, &Mat3::rotate(std::f64::consts::FRAC_PI_4)); + let (w, h, c) = size(&tags); + assert!(close(w, 10.0) && close(h, 10.0), "{w} x {h}"); + assert!(close(c.x, 0.0) && close(c.y, 50f64.sqrt()), "{c:?}"); + let (ul, lr) = (header_corners(&tags)[0], header_corners(&tags)[2]); + assert!(close(ul.0, -5.0) && close(lr.1, 50f64.sqrt() - 5.0), "the header follows: {ul:?} {lr:?}"); + // Nearer a quarter turn, the sides swap: a 10 wide, 4 tall frame turned 100° is 4 x 10. + let mut tags = frame_at(0.0, 4.0, 10.0, 0.0); + ole_frame_transform(&mut tags, &Mat3::rotate(100f64.to_radians())); + let (w, h, _) = size(&tags); + assert!(close(w, 4.0) && close(h, 10.0), "{w} x {h}"); + // A turn and its inverse bring the frame back. + let mut tags = frame_at(2.0, 9.0, 7.0, 3.0); + let m = Mat3::rotate(0.3); + ole_frame_transform(&mut tags, &m); + ole_frame_transform(&mut tags, &m.inverse().unwrap()); + let (w, h, c) = size(&tags); + assert!(close(w, 5.0) && close(h, 6.0) && close(c.x, 4.5) && close(c.y, 6.0), "{w} x {h} at {c:?}"); + } } 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/layout.rs b/crates/engine/src/cmd/layout.rs index f6b519a3c..e4b497efe 100644 --- a/crates/engine/src/cmd/layout.rs +++ b/crates/engine/src/cmd/layout.rs @@ -786,10 +786,28 @@ fn run_plot(s: &mut Session, p: &Value, cmd: &str) -> Result { // Checked here for a clear error; the plotter resolves the names the same way. let sheets = p.get("layouts").map(|v| plot_sheets(cmd, s.doc()?, v)).transpose()?; + // `layout` and `layouts` must name the same sheet(s), or the reply would name one sheet and the render another. + if let (Some(n), Some(names)) = (str_param(p, "layout"), sheets.as_deref()) + && !matches!(names, [only] if only.eq_ignore_ascii_case(n)) + { + return Err(bad(cmd, "`layout` and `layouts` name different sheets; give only one of them")); + } + let is_image = p.get("output").and_then(Value::as_str).is_some_and(|o| o.eq_ignore_ascii_case("png") || o.eq_ignore_ascii_case("svg")); let space = match str_param(p, "layout") { Some(n) if n.eq_ignore_ascii_case("model") => Space::Model, Some(n) => Space::Paper(layout_name(cmd, s.doc()?, n)?), - None => s.space(), + // A PNG or SVG holds one render: a single `layouts` entry names the sheet to draw (the PDF + // path resolves `layouts` itself), not whatever space the session happens to show. + None => match sheets.as_deref() { + Some([only]) if is_image => { + if only.eq_ignore_ascii_case("model") { + Space::Model + } else { + Space::Paper(only.clone()) + } + } + _ => s.space(), + }, }; let mut opts = if p.is_object() { p.clone() } else { json!({}) }; if let Some(raw) = p.get("offset") { @@ -828,12 +846,17 @@ fn run_plot(s: &mut Session, p: &Value, cmd: &str) -> Result { None => o.remove("plotTransparency"), }; } + // A PNG or SVG holds one render: several sheets would report `pages: N` but write only one. + if is_image && sheets.as_ref().is_some_and(|names| names.len() > 1) { + return Err(bad(cmd, "`layouts` with several sheets needs PDF output; PNG and SVG hold one sheet, so plot each layout separately")); + } let bytes = hook(s.doc()?, &space, &opts).map_err(|e| bad(cmd, e))?; let layout = match &sheets { Some(names) => names.first().cloned().unwrap_or_default(), None => space.tab_name().to_string(), }; let pages = sheets.as_ref().map_or(1, Vec::len); + let format = opts.get("output").and_then(Value::as_str).map(str::to_ascii_lowercase).unwrap_or_else(|| "pdf".into()); match str_param(p, "path") { Some(_path) => { #[cfg(not(target_arch = "wasm32"))] @@ -843,17 +866,17 @@ fn run_plot(s: &mut Session, p: &Value, cmd: &str) -> Result { // `.cadcraft-tmp` symlink cannot redirect the plot write (CWE-59). let folder = super::file::new_folder(path); crate::atomic_write::write_atomic_making_folders(std::path::Path::new(path), &bytes).map_err(|e| bad(cmd, format!("{path}: {e}")))?; - let mut r = json!({ "path": path, "bytes": bytes.len(), "layout": layout, "pages": pages }); + let mut r = json!({ "path": path, "bytes": bytes.len(), "layout": layout, "pages": pages, "format": format }); super::file::report_new_folder(s, &mut r, folder); Ok(r) } #[cfg(target_arch = "wasm32")] { super::exchange::deliver_file(cmd, _path, &bytes, super::file::deliver())?; - Ok(json!({ "path": _path, "bytes": bytes.len(), "layout": layout, "pages": pages })) + Ok(json!({ "path": _path, "bytes": bytes.len(), "layout": layout, "pages": pages, "format": format })) } } - None => Ok(json!({ "data": base64_encode(&bytes), "bytes": bytes.len(), "layout": layout, "pages": pages })), + None => Ok(json!({ "data": base64_encode(&bytes), "bytes": bytes.len(), "layout": layout, "pages": pages, "format": format })), } } diff --git a/crates/engine/src/cmd/layout/tests.rs b/crates/engine/src/cmd/layout/tests.rs index e8733786f..57d52a7ef 100644 --- a/crates/engine/src/cmd/layout/tests.rs +++ b/crates/engine/src/cmd/layout/tests.rs @@ -230,6 +230,24 @@ fn plot_goes_through_the_hook() { assert!(s.execute("plot", &json!({"layout": "Model", "plotArea": "window"})).is_err(), "window needs corners"); assert!(s.execute("plot", &json!({"layout": "Layout1", "plotArea": "display"})).is_err(), "layouts plot their sheet"); assert!(s.execute("plot", &json!({"layout": "Model", "scale": -1})).is_err()); + // Image output holds one sheet: several `layouts` would report pages:N but write one render. + for out in ["x.png", "x.svg"] { + let e = s.execute("plot", &json!({"path": out, "layouts": ["Layout1", "Layout2"]})).unwrap_err(); + assert!(e.to_string().contains("layouts"), "{e}"); + } + assert!(s.execute("plot", &json!({"output": "png", "layouts": ["Layout1", "Layout2"]})).is_err()); + // `layout` and `layouts` naming different sheets are refused (the reply would name one sheet and the render another); agreeing ones pass. + for p in + [json!({"output": "png", "layout": "Model", "layouts": ["Layout1"]}), json!({"output": "pdf", "layout": "Layout1", "layouts": ["Model"]})] + { + let e = s.execute("plot", &p).unwrap_err().to_string(); + assert!(e.contains("different"), "{e}"); + } + assert_eq!(s.execute("plot", &json!({"output": "png", "layout": "layout1", "layouts": ["Layout1"]})).unwrap()["layout"], "Layout1"); + let one = s.execute("plot", &json!({"output": "png", "layouts": ["Layout1"]})).unwrap(); + assert_eq!(one["layout"], "Layout1"); + let text = String::from_utf8(super::super::file::base64_decode(one["data"].as_str().unwrap()).unwrap()).unwrap(); + assert!(text.contains("Paper(\"Layout1\")"), "one sheet renders that layout, not the session space: {text}"); #[cfg(not(target_arch = "wasm32"))] { let path = std::env::temp_dir().join(format!("cadcraft-plot-test-{}.pdf", std::process::id())); @@ -307,3 +325,50 @@ fn plot_help_names_the_device_and_the_built_in_devices() { assert!(help.contains(name), "plot help lacks {name}"); } } + +/// The path's extension picks the plot format as EXPORT does, in any case; one that names no +/// format (or none) is refused instead of writing a PDF under that name, and a device that writes +/// another format is refused. +#[cfg(not(target_arch = "wasm32"))] +#[test] +fn plot_path_extension_picks_the_format_like_export() { + super::super::file::set_io(super::super::file::IoHooks { + read: |_, _| Err("no reader in tests".into()), + write: |_, _| Err("no writer in tests".into()), + plot: Some(fake_plot), + }); + if io().and_then(|h| h.plot).is_none() { + return; // another test installed hooks without a plotter first + } + let mut s = session_with_model(); + let dir = std::env::temp_dir().join(format!("cadcraft-plot-ext-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let path = |n: &str| dir.join(n).to_string_lossy().to_string(); + let upper = s.execute("plot", &json!({"path": path("X.PNG")})).unwrap(); + assert_eq!(upper["format"], "png", "an upper-case extension still picks png"); + let e = s.execute("plot", &json!({"path": path("x.txt")})).unwrap_err().to_string(); + assert!(e.contains("png") && e.contains("svg") && e.contains("pdf"), "{e}"); + assert!(!dir.join("x.txt").exists(), "nothing written under an unknown extension"); + let e = s.execute("plot", &json!({"path": path("noext")})).unwrap_err().to_string(); + assert!(e.contains("png") && e.contains("svg") && e.contains("pdf"), "{e}"); + assert!(!dir.join("noext").exists()); + let e = s.execute("plot", &json!({"path": path("y.png"), "device": "CADCraft PDF"})).unwrap_err().to_string(); + assert!(e.contains("writes pdf") && e.contains("png"), "{e}"); + // A device that resolves to none, or a null/unknown `output`, names no format: the refusal stands. + for p in [ + json!({"path": path("x.txt"), "device": "None"}), + json!({"path": path("x.txt"), "output": null}), + json!({"path": path("x.txt"), "output": "bogus"}), + ] { + let e = s.execute("plot", &p).unwrap_err().to_string(); + assert!(e.contains("png") && e.contains("svg") && e.contains("pdf"), "{e}"); + assert!(!dir.join("x.txt").exists(), "nothing written under an unknown extension"); + } + // An explicit output still decides for an odd name. + assert_eq!(s.execute("plot", &json!({"path": path("z.txt"), "output": "pdf"})).unwrap()["format"], "pdf"); + // But one that contradicts the extension is refused, as a device that does is (#2585 review). + let e = s.execute("plot", &json!({"path": path("w.png"), "output": "svg"})).unwrap_err().to_string(); + assert!(e.contains("svg") && e.contains("png"), "{e}"); + assert!(!dir.join("w.png").exists(), "nothing written when `output` and the path disagree"); + std::fs::remove_dir_all(&dir).ok(); +} 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/cmd/plotters.rs b/crates/engine/src/cmd/plotters.rs index 95ead78fc..457a098b4 100644 --- a/crates/engine/src/cmd/plotters.rs +++ b/crates/engine/src/cmd/plotters.rs @@ -265,6 +265,11 @@ pub fn page_device(cmd: &str, p: &Value, page: &mut PageSetup) -> Result { Ok(false) } +/// The output a path's extension (any case) asks for: png, svg or pdf; other extensions ask for none. +fn path_output(path: &str) -> Option { + PlotOutput::parse(&std::path::Path::new(path).extension()?.to_string_lossy()) +} + /// The options PLOT / EXPORTPDF hand the plotter with `device` and the device's papers /// resolved: `device` (else the layout's page setup device) picks the output (`output`: pdf, /// png or svg; EXPORTPDF always writes PDF) and `resolution`; a paper of the device becomes @@ -286,9 +291,42 @@ pub(crate) fn plot_options(s: &Session, p: &Value, cmd: &str) -> Result { None => layout.as_ref().and_then(|pg| find_device(&pg.device)), }; let mut out = if p.is_object() { p.clone() } else { json!({}) }; - let (Some(dev), Some(o)) = (dev, out.as_object_mut()) else { return Ok(out) }; + // The path's extension names the format, like EXPORT, unless a device is given; a given + // device that writes another format is refused. EXPORTPDF always writes PDF. + let by_path = if cmd == "exportpdf" { None } else { str_param(p, "path").and_then(path_output) }; + // A path whose extension names no format (or has none) would hold a PDF under that name: + // refuse it, as EXPORT does, unless `output` or a device says what to write. + if cmd == "plot" + && by_path.is_none() + && !p.get("output").and_then(Value::as_str).is_some_and(|o| PlotOutput::parse(o).is_some()) + && !(dev.is_some() && given.is_some_and(|n| !n.is_empty())) + && let Some(path) = str_param(p, "path") + { + return Err(bad( + cmd, + format!("can't tell the format of `{path}`: give a path ending in .png, .svg or .pdf, or set `output` to png, svg or pdf"), + )); + } + if let (Some(want), Some(dev), true) = (by_path, &dev, given.is_some_and(|n| !n.is_empty())) + && dev.output != want + { + return Err(bad(cmd, format!("plotter `{}` writes {}, but the path asks for {}", dev.name, dev.output.as_str(), want.as_str()))); + } + // An `output` that says another format than the path's extension is refused too, as a device + // that does is: either way the file would not hold what its name says. + if let (Some(want), Some(asked)) = (by_path, p.get("output").and_then(Value::as_str).and_then(PlotOutput::parse)) + && asked != want + { + return Err(bad(cmd, format!("`output` is {}, but the path asks for {}", asked.as_str(), want.as_str()))); + } + let (Some(dev), Some(o)) = (dev, out.as_object_mut()) else { + if let (Some(want), Some(o)) = (by_path, out.as_object_mut()) { + o.insert("output".into(), json!(want.as_str())); + } + return Ok(out); + }; o.remove("device"); - let output = if cmd == "exportpdf" { PlotOutput::Pdf } else { dev.output }; + let output = if cmd == "exportpdf" { PlotOutput::Pdf } else { by_path.unwrap_or(dev.output) }; o.insert("output".into(), json!(output.as_str())); o.insert("resolution".into(), json!(dev.resolution)); let paper = match str_param(p, "paper") { 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/engine/tests/plot_format.rs b/crates/engine/tests/plot_format.rs new file mode 100644 index 000000000..48a4306bb --- /dev/null +++ b/crates/engine/tests/plot_format.rs @@ -0,0 +1,69 @@ +//! PLOT chooses the output format from the extension of `path` when no device is named (#2556), +//! like EXPORT does; a device that writes another format is refused. + +use cadcraft_engine::Session; +use cadcraft_engine::cmd::file::{IoHooks, set_io}; +use cadcraft_engine::cmd::plotters::{PlotterStore, set_plotter_store}; +use serde_json::json; + +fn session() -> Session { + set_plotter_store(PlotterStore::memory()); + set_io(IoHooks { + read: |_, _| Err("no reader in this test".into()), + write: |_, _| Err("no writer in this test".into()), + plot: Some(|d, space, opts| cadcraft_io::plotter::plot(d, space, opts).map_err(|e| e.to_string())), + }); + let mut s = Session::new(); + s.cmdline("line 0,0 100,0").unwrap(); + s.cmdline("").unwrap(); + s +} + +fn magic(path: &std::path::Path) -> Vec { + std::fs::read(path).unwrap().into_iter().take(5).collect() +} + +#[test] +fn the_extension_picks_png_svg_or_pdf() { + let mut s = session(); + let dir = std::env::temp_dir().join(format!("cadcraft-plot-format-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let cases: [(&str, &[u8], &str); 4] = + [("a.png", b"\x89PNG\r", "png"), ("b.svg", b" 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] diff --git a/crates/ui-egui/src/context_menu.rs b/crates/ui-egui/src/context_menu.rs index bbaf48df3..3f1c98ebf 100644 --- a/crates/ui-egui/src/context_menu.rs +++ b/crates/ui-egui/src/context_menu.rs @@ -153,9 +153,29 @@ fn object_commands(app: &CadApp) -> Vec<(&'static str, Option<&'static str>)> { } } -/// [`object_commands`] that are registered, as menu items. +/// [`object_commands`] that are registered, as menu items. The Edit Text entry of a single +/// selected text, mtext or attribute definition opens the Edit Text window instead of the +/// TEXTEDIT command line; the double-click keeps its in-place editing (#842), and several +/// selected texts still take the command line. pub fn object_entries(app: &CadApp) -> Vec { - object_commands(app).into_iter().filter_map(|(id, label)| item(app, id, label)).collect() + object_commands(app) + .into_iter() + .filter_map(|(id, label)| if id == "textedit" { edit_text_window_entry(app).or_else(|| item(app, id, label)) } else { item(app, id, label) }) + .collect() +} + +/// The Edit Text entry as the `textedit:` dialog id, for a single selected text, mtext +/// or attribute definition; `None` otherwise, so the command line entry stands in. +fn edit_text_window_entry(app: &CadApp) -> Option { + use cadcraft_engine::doc::EntityKind; + let sel = app.session.selection(); + if sel.len() != 1 { + return None; + } + let h = *sel.first()?; + let d = app.session.doc().ok()?; + d.entity(h).filter(|e| matches!(e.kind, EntityKind::Text(_) | EntityKind::MText(_) | EntityKind::AttDef(_)))?; + Some(Entry::Item { label: "Edit Text".into(), id: format!("textedit:{}", h.hex()), shortcut: None, enabled: true }) } /// The command menu while a command runs (`None` when none does): Enter and Cancel, Recent Input, @@ -213,6 +233,9 @@ pub fn choose(app: &mut CadApp, id: &str) { } } else if let Some(s) = id.strip_prefix(SNAP) { choose_snap(app, s); + } else if id.starts_with("textedit:") { + // The Edit Text window of the single selected text object; the id carries the handle. + app.ui.dialog = Some(id.to_string()); } else { activate(app, id); } diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 0132ea219..5d04fcd08 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -32,6 +32,7 @@ pub fn show(app: &mut CadApp, ctx: &egui::Context) { "authoring" => crate::bedit::authoring_dialog(app, ctx, &mut open), "stylesmanager" => crate::plotstyles::dialog(app, ctx, &mut open), o if o == "options" || o.starts_with("options:") => crate::options::dialog(app, ctx, o, &mut open), + o if o.starts_with("textedit:") => crate::textdlg::dialog(app, ctx, o, &mut open), other => crate::managers::dialog(app, ctx, other, &mut open), } // A dialog that closed by opening another (Plot Preview back to Plot) keeps the new one. diff --git a/crates/ui-egui/src/i18n/ar.tsv b/crates/ui-egui/src/i18n/ar.tsv index cf5741cdd..3d3921b28 100644 --- a/crates/ui-egui/src/i18n/ar.tsv +++ b/crates/ui-egui/src/i18n/ar.tsv @@ -1380,3 +1380,6 @@ menu View عرض Text to find Text to find Whole words Whole words Nothing found Nothing found + Ctrl+Enter to apply Ctrl+Enter to apply + Edit Text Edit Text + The drawing changed. Close the window and open it again. تغيّر الرسم. أغلق النافذة وافتحها مرة أخرى. diff --git a/crates/ui-egui/src/i18n/bs.tsv b/crates/ui-egui/src/i18n/bs.tsv index 957c48993..7803b744d 100644 --- a/crates/ui-egui/src/i18n/bs.tsv +++ b/crates/ui-egui/src/i18n/bs.tsv @@ -1456,3 +1456,6 @@ menu Modify Izmijeni Text to find Text to find Whole words Whole words Nothing found Nothing found + Ctrl+Enter to apply Ctrl+Enter to apply + Edit Text Edit Text + The drawing changed. Close the window and open it again. Crtež se promijenio. Zatvorite prozor i otvorite ga ponovo. diff --git a/crates/ui-egui/src/i18n/cs.tsv b/crates/ui-egui/src/i18n/cs.tsv index 67a6e90f6..ee587f284 100644 --- a/crates/ui-egui/src/i18n/cs.tsv +++ b/crates/ui-egui/src/i18n/cs.tsv @@ -1236,3 +1236,6 @@ menu Modify Modifikace Text to find Text to find Whole words Whole words Nothing found Nothing found + Ctrl+Enter to apply Ctrl+Enter to apply + Edit Text Edit Text + The drawing changed. Close the window and open it again. Výkres se změnil. Zavřete okno a otevřete jej znovu. diff --git a/crates/ui-egui/src/i18n/de.tsv b/crates/ui-egui/src/i18n/de.tsv index 0925cb6af..d2ffa5620 100644 --- a/crates/ui-egui/src/i18n/de.tsv +++ b/crates/ui-egui/src/i18n/de.tsv @@ -1053,3 +1053,6 @@ Text to find Text to find Whole words Whole words Nothing found Nothing found + Ctrl+Enter to apply Ctrl+Enter to apply + Edit Text Edit Text + The drawing changed. Close the window and open it again. Die Zeichnung hat sich geändert. Schließen Sie das Fenster und öffnen Sie es erneut. diff --git a/crates/ui-egui/src/i18n/el.tsv b/crates/ui-egui/src/i18n/el.tsv index 3bcf59c0c..d24ac46b0 100644 --- a/crates/ui-egui/src/i18n/el.tsv +++ b/crates/ui-egui/src/i18n/el.tsv @@ -1347,3 +1347,6 @@ menu View Προβολή Text to find Text to find Whole words Whole words Nothing found Nothing found + Ctrl+Enter to apply Ctrl+Enter to apply + Edit Text Edit Text + The drawing changed. Close the window and open it again. Το σχέδιο άλλαξε. Κλείστε το παράθυρο και ανοίξτε το ξανά. diff --git a/crates/ui-egui/src/i18n/es.tsv b/crates/ui-egui/src/i18n/es.tsv index f0e6471ee..229156b79 100644 --- a/crates/ui-egui/src/i18n/es.tsv +++ b/crates/ui-egui/src/i18n/es.tsv @@ -1267,3 +1267,6 @@ Text to find Text to find Whole words Whole words Nothing found Nothing found + Ctrl+Enter to apply Ctrl+Enter to apply + Edit Text Edit Text + The drawing changed. Close the window and open it again. El dibujo cambió. Cierre la ventana y ábrala de nuevo. diff --git a/crates/ui-egui/src/i18n/fr.tsv b/crates/ui-egui/src/i18n/fr.tsv index 2f391809e..3b28261d6 100644 --- a/crates/ui-egui/src/i18n/fr.tsv +++ b/crates/ui-egui/src/i18n/fr.tsv @@ -1026,3 +1026,6 @@ Text to find Text to find Whole words Whole words Nothing found Nothing found + Ctrl+Enter to apply Ctrl+Enter to apply + Edit Text Edit Text + The drawing changed. Close the window and open it again. Le dessin a changé. Fermez la fenêtre et rouvrez-la. diff --git a/crates/ui-egui/src/i18n/hu.tsv b/crates/ui-egui/src/i18n/hu.tsv index b87edd3c3..0665055de 100644 --- a/crates/ui-egui/src/i18n/hu.tsv +++ b/crates/ui-egui/src/i18n/hu.tsv @@ -1358,3 +1358,6 @@ menu View Nézet Text to find Text to find Whole words Whole words Nothing found Nothing found + Ctrl+Enter to apply Ctrl+Enter to apply + Edit Text Edit Text + The drawing changed. Close the window and open it again. A rajz megváltozott. Zárja be az ablakot, és nyissa meg újra. diff --git a/crates/ui-egui/src/i18n/it.tsv b/crates/ui-egui/src/i18n/it.tsv index b298e1c98..9819ce4d7 100644 --- a/crates/ui-egui/src/i18n/it.tsv +++ b/crates/ui-egui/src/i18n/it.tsv @@ -1378,3 +1378,6 @@ menu Modify Edita Text to find Text to find Whole words Whole words Nothing found Nothing found + Ctrl+Enter to apply Ctrl+Enter to apply + Edit Text Edit Text + The drawing changed. Close the window and open it again. Il disegno è cambiato. Chiudere la finestra e riaprirla. diff --git a/crates/ui-egui/src/i18n/ko.tsv b/crates/ui-egui/src/i18n/ko.tsv index 3f01bb695..fbf059dc0 100644 --- a/crates/ui-egui/src/i18n/ko.tsv +++ b/crates/ui-egui/src/i18n/ko.tsv @@ -1256,3 +1256,6 @@ menu Modify 수정 Text to find Text to find Whole words Whole words Nothing found Nothing found + Ctrl+Enter to apply Ctrl+Enter to apply + Edit Text Edit Text + The drawing changed. Close the window and open it again. 도면이 변경되었습니다. 창을 닫고 다시 여십시오. diff --git a/crates/ui-egui/src/i18n/pt_br.tsv b/crates/ui-egui/src/i18n/pt_br.tsv index 3d1791c1b..93905715c 100644 --- a/crates/ui-egui/src/i18n/pt_br.tsv +++ b/crates/ui-egui/src/i18n/pt_br.tsv @@ -1462,3 +1462,6 @@ menu Dimension Cota menu Draw Desenho menu Modify Modificar menu View Exibir + Ctrl+Enter to apply Ctrl+Enter to apply + Edit Text Edit Text + The drawing changed. Close the window and open it again. O desenho mudou. Feche a janela e abra-a novamente. diff --git a/crates/ui-egui/src/i18n/ru.tsv b/crates/ui-egui/src/i18n/ru.tsv index f1a5124f0..a34105023 100644 --- a/crates/ui-egui/src/i18n/ru.tsv +++ b/crates/ui-egui/src/i18n/ru.tsv @@ -1027,3 +1027,6 @@ Text to find Text to find Whole words Whole words Nothing found Nothing found + Ctrl+Enter to apply Ctrl+Enter to apply + Edit Text Edit Text + The drawing changed. Close the window and open it again. Чертёж изменился. Закройте окно и откройте его снова. diff --git a/crates/ui-egui/src/i18n/sv.tsv b/crates/ui-egui/src/i18n/sv.tsv index d67d39c03..f34f32ca5 100644 --- a/crates/ui-egui/src/i18n/sv.tsv +++ b/crates/ui-egui/src/i18n/sv.tsv @@ -1480,3 +1480,6 @@ menu Modify Ändra The current file is kept as {name} until the next save. Den aktuella filen sparas som {name} till nästa sparande. This format can't keep these objects yet: {objects}. Det här formatet kan ännu inte behålla följande objekt: {objects}. {count} more types {count} typer till + Ctrl+Enter to apply Ctrl+Enter to apply + Edit Text Edit Text + The drawing changed. Close the window and open it again. Ritningen har ändrats. Stäng fönstret och öppna det igen. diff --git a/crates/ui-egui/src/i18n/tests.rs b/crates/ui-egui/src/i18n/tests.rs index b266ed369..66fe62199 100644 --- a/crates/ui-egui/src/i18n/tests.rs +++ b/crates/ui-egui/src/i18n/tests.rs @@ -114,6 +114,7 @@ fn missing_marked_ui_literals(language: &str) -> BTreeSet { "vpscale", "options/dialog", "finddlg", + "textdlg", ]; let src = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("src"); let mut files = Vec::new(); diff --git a/crates/ui-egui/src/i18n/tr.tsv b/crates/ui-egui/src/i18n/tr.tsv index 26308975f..da8aae1d2 100644 --- a/crates/ui-egui/src/i18n/tr.tsv +++ b/crates/ui-egui/src/i18n/tr.tsv @@ -1361,3 +1361,6 @@ menu View Görünüm Text to find Text to find Whole words Whole words Nothing found Nothing found + Ctrl+Enter to apply Ctrl+Enter to apply + Edit Text Edit Text + The drawing changed. Close the window and open it again. Çizim değişti. Pencereyi kapatıp yeniden açın. diff --git a/crates/ui-egui/src/i18n/uk.tsv b/crates/ui-egui/src/i18n/uk.tsv index 821f30c1b..072e6222a 100644 --- a/crates/ui-egui/src/i18n/uk.tsv +++ b/crates/ui-egui/src/i18n/uk.tsv @@ -1390,3 +1390,6 @@ Text to find Text to find Whole words Whole words Nothing found Nothing found + Ctrl+Enter to apply Ctrl+Enter to apply + Edit Text Edit Text + The drawing changed. Close the window and open it again. Креслення змінилося. Закрийте вікно та відкрийте його знову. diff --git a/crates/ui-egui/src/i18n/vi.tsv b/crates/ui-egui/src/i18n/vi.tsv index 91ed70cf7..c3cd41089 100644 --- a/crates/ui-egui/src/i18n/vi.tsv +++ b/crates/ui-egui/src/i18n/vi.tsv @@ -1461,3 +1461,6 @@ menu Modify Hiệu chỉnh Text to find Text to find Whole words Whole words Nothing found Nothing found + Ctrl+Enter to apply Ctrl+Enter to apply + Edit Text Edit Text + The drawing changed. Close the window and open it again. Bản vẽ đã thay đổi. Đóng cửa sổ và mở lại. diff --git a/crates/ui-egui/src/i18n/zh.tsv b/crates/ui-egui/src/i18n/zh.tsv index a07b4f027..5ec4cb794 100644 --- a/crates/ui-egui/src/i18n/zh.tsv +++ b/crates/ui-egui/src/i18n/zh.tsv @@ -1472,3 +1472,6 @@ menu View 视图 Clip Xref 剪裁外部参照 Xref 外部参照 Xref Clip Frame 外部参照剪裁边框 + Ctrl+Enter to apply Ctrl+Enter 应用 + Edit Text 编辑文字 + The drawing changed. Close the window and open it again. 图纸已切换。请关闭窗口后重新打开。 diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 0f43acfbd..1b68264d9 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -40,6 +40,8 @@ mod mtext_editor; mod panel_state; pub mod spacemouse; pub mod switch; +// The Edit Text window, opened from the shortcut menu's Edit Text entry. +pub mod textdlg; // The External References palette. pub mod xrefs; diff --git a/crates/ui-egui/src/textdlg.rs b/crates/ui-egui/src/textdlg.rs new file mode 100644 index 000000000..4c79d0993 --- /dev/null +++ b/crates/ui-egui/src/textdlg.rs @@ -0,0 +1,159 @@ +//! Edit Text: the window behind the shortcut menu's Edit Text entry. +//! +//! TEXTEDIT already existed, but only as a command line: it put the caret in the command bar, +//! one line, no context, and the text under the cursor was out of sight. The window shows the +//! same edit over the drawing: the object's own text, its type and handle, and OK/Cancel. The +//! engine call is unchanged (`textedit` with a handle and the new text), so undo, locked layers +//! and the DXF round trip behave exactly as before. The double-click keeps its in-place editing +//! (#842); the window is the shortcut menu's way in. + +use egui::RichText; +use serde_json::json; + +use crate::CadApp; +use crate::theme::Tokens; +use cadcraft_engine::doc::EntityKind; + +/// The text the window starts with, in the form the user edits: MText line breaks are `\P` in +/// the drawing and a newline here, and an empty dimension text means the measured value, shown +/// as `<>` the way the dimension itself does. +fn text_of(k: &EntityKind) -> Option { + match k { + EntityKind::Text(t) => Some(t.value.clone()), + EntityKind::AttDef(a) => Some(a.text.value.clone()), + EntityKind::MText(t) => Some(t.contents.replace("\\P", "\n")), + EntityKind::Dimension(d) => Some(if d.text.is_empty() { "<>".to_string() } else { d.text.clone() }), + _ => None, + } +} + +#[derive(Default, Clone)] +struct State { + handle: String, + kind: String, + text: String, + /// What the drawing holds now, to tell an edit from a no-op. + original: String, + /// The drawing the window was opened for: a handle means nothing once another drawing is + /// active, and committing to it would edit an object the user never saw. + uid: u64, + message: String, + /// Multiline text (MTEXT): plain Enter breaks the line. TEXT and ATTDEF are one line, and a + /// line break typed there would be a space once saved. + multiline: bool, + /// The UI pass the window last showed in. A window that missed a pass was closed some other + /// way (another dialog took its place), and its edit is not the user's any more. + pass: u64, +} + +/// Read the object the window is for. A handle that no longer exists, or an object that is not +/// text, leaves the window empty instead of failing: the drawing may have changed under it. +fn initial(app: &CadApp, hex: &str) -> State { + let uid = app.session.state().map(|s| s.uid).unwrap_or_default(); + let mut st = State { handle: hex.to_string(), uid, ..Default::default() }; + let Some(h) = cadcraft_doc::Handle::parse_hex(hex) else { return st }; + let Ok(d) = app.session.doc() else { return st }; + let Some(e) = d.entity(h) else { return st }; + st.kind = e.kind.type_name().to_string(); + st.multiline = matches!(e.kind, EntityKind::MText(_)); + st.text = text_of(&e.kind).unwrap_or_default(); + st.original = st.text.clone(); + st +} + +/// The Edit Text window for `spec`, which is `textedit:` so the handle survives the +/// dialog dispatch without a new field in the saved UI state. +pub fn dialog(app: &mut CadApp, ctx: &egui::Context, spec: &str, open: &mut bool) { + let t = Tokens::get(); + let hex = spec.strip_prefix("textedit:").unwrap_or_default().to_string(); + let sid = egui::Id::unique("textedit_state").with(&hex); + let cached: Option = ctx.data_mut(|d| d.get_temp(sid)); + let pass = ctx.cumulative_pass_nr(); + let mut st = match cached { + // Shown in the last pass: the same window, still open. + Some(s) if s.pass.saturating_add(1) >= pass => s, + // Closed without passing through here: start from the drawing again, with a fresh field. + Some(s) => { + let field = egui::Id::unique("textedit_field").with(s.uid).with(&s.handle); + ctx.data_mut(|d| d.remove::(field)); + initial(app, &hex) + } + None => initial(app, &hex), + }; + st.pass = pass; + // The window's own close button and the two buttons below all decide the same thing, so the + // decision stays in a local until the window is gone: `open` is borrowed for the title bar. + let mut keep = *open; + let mut commit = false; + let mut cancel = false; + // The field's undo history is keyed by drawing and handle: two edits on the same drawing + // must not share a Ctrl+Z trail, and neither must an edit on two different drawings. + let field = egui::Id::unique("textedit_field").with(st.uid).with(&st.handle); + egui::Window::new(crate::tl!("Edit Text")) + .id(egui::Id::unique("Edit Text")) + .open(&mut keep) + .collapsible(false) + .resizable(true) + .default_width(440.0) + .show(ctx, |ui| { + let head = format!("{} · {}", crate::i18n::t(&st.kind), st.handle); + ui.label(RichText::new(head).color(t.text_faint).small()); + let edit = if st.multiline { egui::TextEdit::multiline(&mut st.text).desired_rows(3) } else { egui::TextEdit::singleline(&mut st.text) }; + let r = ui.add(edit.id(field).desired_width(f32::INFINITY).hint_text(crate::tl!("Text"))); + // Ctrl+Enter commits from inside the field, so a multiline text can keep plain Enter; + // a one-line field commits on Enter too. + let enter = ui.input(|i| i.key_pressed(egui::Key::Enter)); + if (r.has_focus() && enter && ui.input(|i| i.modifiers.ctrl)) || (!st.multiline && r.lost_focus() && enter) { + commit = true; + } + ui.horizontal(|ui| { + if ui.button(crate::tl!("OK")).clicked() { + commit = true; + } + if ui.button(crate::tl!("Cancel")).clicked() { + cancel = true; + } + ui.label(RichText::new(crate::tl!("Ctrl+Enter to apply")).color(t.text_faint).small()); + }); + if !st.message.is_empty() { + ui.label(RichText::new(st.message.as_str()).color(t.warn).small()); + } + }); + if cancel { + keep = false; + } + if commit { + // Typing and undoing back to the same text is not an edit: closing without a command + // keeps the undo stack honest. + if st.text == st.original { + keep = false; + } else if app.session.state().map(|s| s.uid).ok() != Some(st.uid) { + // The drawing switched while the window sat open: the handle now names another + // drawing's object, so the commit is refused and the user reopens on the drawing + // they meant to edit. + st.message = crate::tl!("The drawing changed. Close the window and open it again.").to_string(); + } else { + let params = json!({ "handles": [st.handle.clone()], "text": st.text }); + match app.run("textedit", params) { + Ok(_) => { + st.original = st.text.clone(); + keep = false; + } + Err(e) => st.message = e, + } + } + } + *open = keep; + // The state lives exactly as long as the window does: a closed window leaves nothing + // behind, so reopening it reads the drawing's text instead of an edit the user cancelled. + // The field's own persisted state goes with it — otherwise the undo history would offer a + // previous object's text to whoever is edited next. + if keep { + ctx.data_mut(|d| d.insert_temp(sid, st)); + } else { + ctx.data_mut(|d| { + d.remove_temp::(sid); + d.remove::(field); + }); + } +} diff --git a/crates/ui-egui/tests/it/context_menus.rs b/crates/ui-egui/tests/it/context_menus.rs index 3134b8d3c..fbd39f930 100644 --- a/crates/ui-egui/tests/it/context_menus.rs +++ b/crates/ui-egui/tests/it/context_menus.rs @@ -133,7 +133,8 @@ fn idle_menu_lists_recent_commands_and_selection_commands() { #[test] fn selection_menu_has_the_object_types_edit_commands() { let cases = [ - ("text", json!({ "at": [0, 0], "text": "hi" }), "textedit"), + // A single text's Edit Text opens the Edit Text window: the id carries the handle. + ("text", json!({ "at": [0, 0], "text": "hi" }), "textedit:"), ("pline", json!({ "vertices": [[0, 0], [10, 0], [10, 5]] }), "pedit"), ("spline", json!({ "fit": [[0, 0], [5, 5], [10, 0]] }), "splinedit"), ]; @@ -142,14 +143,27 @@ fn selection_menu_has_the_object_types_edit_commands() { app.run(cmd, params).unwrap(); select_all(&mut app); let g = vec![context_menu::object_entries(&app)]; - assert!(ids(&g).iter().any(|i| i == edit), "{cmd}: {edit} missing from {:?}", labels(&g)); - assert!(ids(&context_menu::entries(&app)).iter().any(|i| i == edit)); + let hit = ids(&g).iter().find(|i| i.starts_with(edit)).cloned(); + assert!(hit.is_some(), "{cmd}: {edit} missing from {:?}", labels(&g)); + assert!(ids(&context_menu::entries(&app)).iter().any(|i| i.starts_with(edit))); + // Choosing the window's entry opens the dialog on the object; the double-click keeps its + // in-place editing (#842). + if let Some(id) = hit.filter(|i| i.starts_with("textedit:")) { + context_menu::choose(&mut app, &id); + assert_eq!(app.ui.dialog.as_deref(), Some(id.as_str())); + } } + // Several selected texts keep the TEXTEDIT command line: the window edits one object at a time. + let mut several = app(); + several.run("text", json!({ "at": [0, 0], "text": "hi" })).unwrap(); + several.run("text", json!({ "at": [0, 4], "text": "ho" })).unwrap(); + select_all(&mut several); + assert!(ids(&[context_menu::object_entries(&several)]).iter().any(|i| i == "textedit")); // Polyline Edit, not the command's bare "Polyline" label. - let mut app = app(); - app.run("pline", json!({ "vertices": [[0, 0], [10, 0]] })).unwrap(); - select_all(&mut app); - assert_eq!(labels(&[context_menu::object_entries(&app)]), ["Polyline Edit"]); + let mut pline = app(); + pline.run("pline", json!({ "vertices": [[0, 0], [10, 0]] })).unwrap(); + select_all(&mut pline); + assert_eq!(labels(&[context_menu::object_entries(&pline)]), ["Polyline Edit"]); } #[test]