OLE2FRAME: MOVE also moves the corners inside the OLE data (review on #2587)

The OLE data (310 groups) starts with the frame's four corners. MOVE shifted groups 10/20/11/21 only, so
a DXF save wrote new corner groups beside a data header with the old position. The transform now
rewrites those doubles (x, y; z kept) in the decoded hex, keeping each chunk's length; malformed or
short data is left untouched.

crates/doc/src/extents.rs

@@ -43,8 +43,62 @@ pub fn ole_frame(tags: &[crate::RawTag]) -> Option<[Vec2; 5]> {
     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;
+
+/// Move the four corners at the start of the OLE data (the 310 groups' hex, in order) by `m`,
+/// keeping z and every tag's length. Data that is not valid hex or is too short is left alone.
+fn ole_data_transform(tags: &mut [crate::RawTag], m: &cadcraft_geom::Mat3) {
+    let idx: Vec<usize> = outside_app(tags).into_iter().filter(|&i| tags.get(i).is_some_and(|t| t.code == 310)).collect();
+    let mut bytes: Vec<u8> = Vec::new();
+    let mut lens: Vec<usize> = Vec::new();
+    for &i in &idx {
+        let Some(t) = tags.get(i) else { return };
+        let s = t.value.trim();
+        if s.len() % 2 != 0 || !s.is_ascii() {
+            return;
+        }
+        for k in (0..s.len()).step_by(2) {
+            let Some(b) = s.get(k..k + 2).and_then(|h| u8::from_str_radix(h, 16).ok()) else { return };
+            bytes.push(b);
+        }
+        lens.push(s.len() / 2);
+    }
+    let end = OLE_HEADER + OLE_CORNERS * 8;
+    if bytes.len() < end {
+        return;
+    }
+    let mut out = bytes.clone();
+    for c in 0..4 {
+        let at = OLE_HEADER + c * 24;
+        let rd = |o: usize| out.get(o..o + 8).and_then(|s| <[u8; 8]>::try_from(s).ok()).map(f64::from_le_bytes).filter(|v| v.is_finite());
+        let (Some(x), Some(y)) = (rd(at), rd(at + 8)) else { return };
+        let p = m.apply(Vec2::new(x, y));
+        if !(p.x.is_finite() && p.y.is_finite()) {
+            return;
+        }
+        if let Some(s) = out.get_mut(at..at + 8) {
+            s.copy_from_slice(&p.x.to_le_bytes());
+        }
+        if let Some(s) = out.get_mut(at + 8..at + 16) {
+            s.copy_from_slice(&p.y.to_le_bytes());
+        }
+    }
+    let mut pos = 0;
+    for (&i, &n) in idx.iter().zip(&lens) {
+        let (Some(t), Some(chunk)) = (tags.get_mut(i), out.get(pos..pos + n)) else { return };
+        let lower = t.value.bytes().any(|b| b.is_ascii_lowercase());
+        t.value = chunk.iter().map(|b| if lower { format!("{b:02x}") } else { format!("{b:02X}") }).collect();
+        pos += n;
+    }
+}
+
 /// Move an OLE2FRAME by `m`, a translation: its two corners are mapped through `m` (a DWG save
-/// writes the frame's position from these groups). Other transforms do not keep a frame axis
+/// 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. Other transforms do not keep a frame axis
 /// aligned, so callers refuse them (see [`ole_frame_accepts`]). A frame with no usable corners is
 /// left alone.
 pub fn ole_frame_transform(tags: &mut [crate::RawTag], m: &cadcraft_geom::Mat3) {
@@ -58,6 +112,7 @@ pub fn ole_frame_transform(tags: &mut [crate::RawTag], m: &cadcraft_geom::Mat3)
             t.value = v.to_string();
         }
     }
+    ole_data_transform(tags, m);
 }
 
 /// Whether `m` can move an OLE2FRAME: only a translation (move, copy, stretch of the whole frame).

crates/engine/tests/ole_frame_transform.rs

@@ -55,3 +55,72 @@ fn rotate_scale_and_mirror_refuse_an_ole_frame() {
     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<String> {
+    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::<Vec<_>>())
+            } 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::<Vec<_>>(), before.iter().map(String::len).collect::<Vec<_>>(), "chunk lengths are kept");
+    let hex: String = after.concat();
+    let bytes: Vec<u8> = (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");
+    }
+}

crates/io/tests/it/ole_frame.rs

@@ -75,3 +75,22 @@ fn a_moved_ole_frame_keeps_its_position_in_a_dwg() {
     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<u8> = (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::<f64>().ok()).unwrap();
+    assert!(near(g(10), UL.0 + 100.0) && near(g(21), LR.1 - 50.0));
+}