Merge pull request #6832 from Hassan1305/fix-6809-replace-spine-source-move

Blend: moving a key of a blend with a replaced spine moves it (#6809)

crates/doc/src/blend.rs

@@ -1051,7 +1051,8 @@ fn pinned(spec: &BlendSpec, keys: usize, n: usize) -> Option<Vec<usize>> {
 /// - a pinned spine (`key_anchors`): the stored spine's first subpath with each key's anchor (and
 ///   its handles) moved onto the key's centre, so moving a key moves its end of the spine;
 /// - a spine from Replace Spine: its first subpath, `None` for the anchors (the keys spread
-///   evenly along it by arc length).
+///   evenly along it by arc length) while the keys sit there; once one is moved off, pinned as
+///   above at the anchors where they were spread.
 ///
 /// `None` for a blend made without a spine ([`BlendSpec::no_spine`]).
 pub fn blend_spine(keys: &[Arc<Node>], spec: &BlendSpec) -> Option<(PathData, Option<Vec<usize>>)> {
@@ -1077,7 +1078,24 @@ fn layout_spine(keys: &[Arc<Node>], spec: &BlendSpec) -> Option<(PathData, Optio
         return Some((PathData::single(SubPath::polyline(&centers, false)), Some((0..keys.len()).collect())));
     };
     let mut first = stored.subpaths.first()?.clone();
-    let Some(ka) = pinned(spec, keys.len(), first.anchors.len()) else { return Some((PathData::single(first), None)) };
+    let Some(ka) = pinned(spec, keys.len(), first.anchors.len()) else {
+        let mut path = PathData::single(first);
+        // A key moved off where Replace Spine spread it takes its point of the spine along, as
+        // on a pinned spine; otherwise the steps would put it back on the spine (#6809).
+        if Spine::new(&path).is_none_or(|sp| keys_spread_along(keys, &sp)) {
+            return Some((path, None));
+        }
+        let Some(mut ka) = spread_anchors(&mut path, keys.len()) else { return Some((path, None)) };
+        let mut first = path.subpaths.into_iter().next()?;
+        open_at_start(&mut first, &mut ka);
+        for (k, &ai) in keys.iter().zip(&ka) {
+            if let Some(a) = first.anchors.get_mut(ai) {
+                let d = center_of(k) - a.p;
+                a.translate(d);
+            }
+        }
+        return Some((PathData::single(first), Some(ka)));
+    };
     for (k, &ai) in keys.iter().zip(&ka) {
         let c = center_of(k);
         if let Some(a) = first.anchors.get_mut(ai) {
@@ -1091,21 +1109,62 @@ fn layout_spine(keys: &[Arc<Node>], spec: &BlendSpec) -> Option<(PathData, Optio
 /// [`blend_spine`] with every key on an anchor: a spine from Replace Spine gets anchors where its
 /// keys sit (by arc length), so editing it keeps them in place.
 pub fn pin_spine(keys: &[Arc<Node>], spec: &BlendSpec) -> Option<(PathData, Vec<usize>)> {
-    let (mut path, anchors) = blend_spine(keys, spec)?;
-    if let Some(a) = anchors {
-        return Some((path, a));
+    // A spine from Replace Spine is pinned as stored, even with a key moved off it (where
+    // [`blend_spine`] moves that key's point along): the layout is the same.
+    if keys.len() >= 2
+        && let Some(first) = stored_spine(spec).and_then(|s| s.subpaths.first())
+        && pinned(spec, keys.len(), first.anchors.len()).is_none()
+    {
+        let mut path = PathData::single(first.clone());
+        let mut out = spread_anchors(&mut path, keys.len())?;
+        // A closed spine's end keys get points of their own, so that once pinned each still
+        // moves only its own end.
+        let mut sp = path.subpaths.into_iter().next()?;
+        open_at_start(&mut sp, &mut out);
+        return Some((PathData::single(sp), out));
     }
+    let (path, anchors) = blend_spine(keys, spec)?;
+    Some((path, anchors?))
+}
+
+/// Do `keys` sit where a spine from Replace Spine spreads them: evenly along `spine` by arc length
+/// (within a hundredth of a point, as [`anchor_at_length`] matches anchors)?
+fn keys_spread_along(keys: &[Arc<Node>], spine: &Spine) -> bool {
     let k = keys.len();
-    let total = Spine::new(&path)?.length();
+    keys.iter().enumerate().all(|(i, key)| {
+        let f = if k > 1 { i as f64 / (k - 1) as f64 } else { 0.0 };
+        center_of(key).distance(spine.at(f).0) < 0.01
+    })
+}
+
+/// Open closed spine `sp` at its first anchor when the last key sits there with the first (as
+/// [`spread_anchors`] puts them): a copy of that anchor at the end takes the closing segment and
+/// the last key, so the two end keys move their ends of the spine apart.
+fn open_at_start(sp: &mut SubPath, ka: &mut [usize]) {
+    let Some(&start) = sp.anchors.first() else { return };
+    if !sp.closed || ka.len() < 2 {
+        return;
+    }
+    if let Some(last) = ka.last_mut().filter(|a| **a == 0) {
+        *last = sp.anchors.len();
+        sp.anchors.push(start);
+        sp.closed = false;
+    }
+}
+
+/// The anchors of `path`'s first subpath that `k` keys spread evenly along it by arc length sit
+/// on, splitting segments where none is.
+fn spread_anchors(path: &mut PathData, k: usize) -> Option<Vec<usize>> {
+    let total = Spine::new(path)?.length();
     let closed = path.subpaths.first().is_some_and(|s| s.closed);
     let mut out = vec![0usize];
     for i in 1..k.saturating_sub(1) {
         let s = total * i as f64 / (k - 1) as f64;
-        out.push(anchor_at_length(&mut path, s)?);
+        out.push(anchor_at_length(path, s)?);
     }
     let last = path.subpaths.first().map_or(0, |sp| if closed { 0 } else { sp.anchors.len().saturating_sub(1) });
     out.push(last);
-    Some((path, out))
+    Some(out)
 }
 
 /// The anchor of the first subpath at arc length `s`: an existing one within a hundredth of a

crates/engine/src/cmd/live/blend.rs

@@ -324,7 +324,11 @@ pub(super) fn spine_remove_anchor(s: &mut Session, p: &Value) -> Result<Value> {
         if anchors.contains(&index) {
             return Err(bad(C, "a key object sits on that point"));
         }
-        if sp.anchors.len() <= 2 {
+        // A closed spine pinned open keeps a copy of its first point at its end (#6809), so the
+        // points left must be two apart, not only two in number.
+        let mut rest = sp.anchors.iter().enumerate().filter(|(i, _)| *i != index).map(|(_, a)| a.p);
+        let first = rest.next();
+        if sp.anchors.len() <= 2 || first.is_none_or(|f| rest.all(|q| q.distance(f) < 0.01)) {
             return Err(bad(C, "a spine keeps at least two points"));
         }
         sp.anchors.remove(index);

crates/engine/src/tests/live.rs

@@ -424,6 +424,93 @@ fn blend_replace_spine_follows_path() {
     assert!(first.distance(Point::new(100.0, 300.0)) < 1.0, "{first:?}");
 }
 
+/// #6809: after Replace Spine, moving a key object moves the drawn key and its end of the spine,
+/// not only the key's selection outline.
+#[test]
+fn moving_a_key_of_a_replaced_spine_blend_moves_what_is_drawn() {
+    let mut s = session();
+    let (a, _, g) = blend(&mut s, json!({"steps": 5}));
+    let arc = id_of(&s.execute("path.create", &json!({"d": "M100 300 C100 245 145 200 200 200 C255 200 300 245 300 300"})).unwrap());
+    sel(&mut s, &[g, arc]);
+    s.execute("object.blend.replaceSpine", &json!({})).unwrap();
+    let center = |n: &Node| n.geometric_bounds().unwrap().center();
+    let before = steps_of(&s, g);
+    assert!(center(&before[0]).distance(Point::new(100.0, 300.0)) < 1e-6);
+    // Direct Selection picks the key inside the blend; the drag moves it.
+    sel(&mut s, &[a]);
+    s.execute("object.move", &json!({"dx": -50, "dy": 40})).unwrap();
+    let key = center(&node(&s, a));
+    assert!(key.distance(Point::new(50.0, 340.0)) < 1e-6, "{key:?}");
+    let after = steps_of(&s, g);
+    assert_eq!(after.len(), before.len());
+    let drawn = center(&after[0]);
+    assert!(drawn.distance(key) < 1e-6, "the key is drawn where it was moved to: {drawn:?}, outline at {key:?}");
+    assert!(center(&after[1]).distance(center(&before[1])) > 1.0, "the first step follows the moved key");
+    let last = center(&after[after.len() - 1]);
+    assert!(last.distance(Point::new(300.0, 300.0)) < 1e-6, "the other key stays put: {last:?}");
+    // Undo puts the key back on the spine as Replace Spine left it.
+    s.execute("edit.undo", &json!({})).unwrap();
+    let back = steps_of(&s, g);
+    assert!(center(&back[0]).distance(Point::new(100.0, 300.0)) < 1e-6);
+    assert!(center(&back[1]).distance(center(&before[1])) < 1e-6);
+}
+
+/// #6809 on a closed spine, where both end keys start on its first point: moving the first key
+/// moves its end of the spine without dragging the last key's end along.
+#[test]
+fn moving_the_first_key_of_a_closed_replaced_spine_blend_moves_what_is_drawn() {
+    let mut s = session();
+    let (a, _, g) = blend(&mut s, json!({"steps": 5}));
+    let square = id_of(&s.execute("path.create", &json!({"d": "M100 100 L300 100 L300 300 L100 300 Z"})).unwrap());
+    sel(&mut s, &[g, square]);
+    s.execute("object.blend.replaceSpine", &json!({})).unwrap();
+    let center = |n: &Node| n.geometric_bounds().unwrap().center();
+    let before = steps_of(&s, g);
+    assert!(center(&before[0]).distance(Point::new(100.0, 100.0)) < 1e-6);
+    sel(&mut s, &[a]);
+    s.execute("object.move", &json!({"dx": -50, "dy": -40})).unwrap();
+    let key = center(&node(&s, a));
+    assert!(key.distance(Point::new(50.0, 60.0)) < 1e-6, "{key:?}");
+    let after = steps_of(&s, g);
+    assert_eq!(after.len(), before.len());
+    let drawn = center(&after[0]);
+    assert!(drawn.distance(key) < 1e-6, "the key is drawn where it was moved to: {drawn:?}, outline at {key:?}");
+    assert!(center(&after[1]).distance(center(&before[1])) > 1.0, "the first step follows the moved key");
+    let last = center(&after[after.len() - 1]);
+    assert!(last.distance(Point::new(100.0, 100.0)) < 1e-6, "the other key stays put: {last:?}");
+    s.execute("edit.undo", &json!({})).unwrap();
+    let back = steps_of(&s, g);
+    assert!(center(&back[0]).distance(Point::new(100.0, 100.0)) < 1e-6);
+    assert!(center(&back[1]).distance(center(&before[1])) < 1e-6);
+}
+
+/// #6809 on a closed spine pinned by a spine edit: the end keys get points of their own, so
+/// moving the first key afterwards still moves only its end.
+#[test]
+fn moving_the_first_key_of_a_pinned_closed_spine_blend_moves_what_is_drawn() {
+    let mut s = session();
+    let (a, _, g) = blend(&mut s, json!({"steps": 5}));
+    let square = id_of(&s.execute("path.create", &json!({"d": "M100 100 L300 100 L300 300 L100 300 Z"})).unwrap());
+    sel(&mut s, &[g, square]);
+    s.execute("object.blend.replaceSpine", &json!({})).unwrap();
+    // Dragging a corner away from the keys pins the spine to them.
+    s.execute("object.blend.spine.moveAnchor", &json!({"id": g.0, "anchor": 2, "x": 320, "y": 320})).unwrap();
+    let NodeKind::Blend { spec, .. } = node(&s, g).kind else { panic!() };
+    assert_eq!(spec.key_anchors.len(), 2, "{:?}", spec.key_anchors);
+    let center = |n: &Node| n.geometric_bounds().unwrap().center();
+    let before = steps_of(&s, g);
+    sel(&mut s, &[a]);
+    s.execute("object.move", &json!({"dx": -50, "dy": -40})).unwrap();
+    let key = center(&node(&s, a));
+    assert!(key.distance(Point::new(50.0, 60.0)) < 1e-6, "{key:?}");
+    let after = steps_of(&s, g);
+    assert_eq!(after.len(), before.len());
+    let drawn = center(&after[0]);
+    assert!(drawn.distance(key) < 1e-6, "the key is drawn where it was moved to: {drawn:?}, outline at {key:?}");
+    let last = center(&after[after.len() - 1]);
+    assert!(last.distance(Point::new(100.0, 100.0)) < 1e-6, "the other key stays put: {last:?}");
+}
+
 #[test]
 fn blend_align_to_path_rotates_steps() {
     let mut s = session();