diff --git a/crates/engine/src/cmd/array.rs b/crates/engine/src/cmd/array.rs index da96ccfed..5fc16b310 100644 --- a/crates/engine/src/cmd/array.rs +++ b/crates/engine/src/cmd/array.rs @@ -247,9 +247,15 @@ 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(); + for m in &mats { + super::modify::refuse_ole_frames(s, hs, m, true)?; + } let mut n = 0; - for m in mats { - n += transform_entities(s, hs, &m, true)?.len(); + for m in &mats { + n += transform_entities(s, hs, m, true)?.len(); } Ok(n) } diff --git a/crates/engine/src/cmd/modify/transform.rs b/crates/engine/src/cmd/modify/transform.rs index d1e6a463e..2fad5ee7b 100644 --- a/crates/engine/src/cmd/modify/transform.rs +++ b/crates/engine/src/cmd/modify/transform.rs @@ -37,7 +37,7 @@ pub(crate) fn refuse_ole_frames(s: &Session, hs: &[Handle], m: &Mat3, copy: bool }; if hs.iter().any(ole) { return Err(crate::EngineError::Other( - "An OLE object can only be moved, copied or scaled uniformly; rotating and mirroring it are not supported.".into(), + "An OLE object can only be moved, copied or scaled uniformly; other transforms (rotation, mirroring, non-uniform scale, alignment) are not supported.".into(), )); } Ok(()) diff --git a/crates/engine/src/cmd/modify2/patharray.rs b/crates/engine/src/cmd/modify2/patharray.rs index 3f9445249..aff058ab5 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). + for m in &mats { + crate::cmd::modify::refuse_ole_frames(s, &objs, m, true)?; + } + 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/tests/ole_frame_transform.rs b/crates/engine/tests/ole_frame_transform.rs index 2578c4e18..3569d39e5 100644 --- a/crates/engine/tests/ole_frame_transform.rs +++ b/crates/engine/tests/ole_frame_transform.rs @@ -220,3 +220,19 @@ fn mocoro_scales_an_ole_frame() { 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); +}