Keyframes: moving keys keeps subframe spacing (#1012)

keys.move and the nudges snapped each key's destination to the nearest
frame, so moving keys a Quick Offset had staggered onto subframes put
them back on frames: four keys at 0, 1/3, 2/3 and 1 frame, moved one
frame, came out at 1, 1, 2 and 2. Layer bars kept theirs, because
layer.timing snaps the move, not the times.

The move is now rounded to whole frames and every key translated by it.
layer_shift turns a comp-time move into a layer-time move through the
layer's start and stretch (reversed too) without a round trip through
comp time; keys.quickOffset uses it as well.

Test: moving_staggered_keys_keeps_their_subframe_spacing (Quick Offset,
then keys.move and keys.nudgeForward; fails before: 1/3 frame lands on
a frame).

crates/engine/src/commands/prop.rs

@@ -928,9 +928,16 @@ pub(crate) fn move_key_refs(comp: &mut effectcraft_project::Comp, from: &[KeyRef
     Ok(from.iter().filter_map(|k| remapped.get(k).copied()).filter(|k| seen.insert(*k)).collect())
 }
 
-/// Move the selected keys by `d` comp seconds, snapped to the comp's frames in comp time (keys
-/// of time-stretched layers move with the pointer, not by the layer-time distance). Keys of
-/// locked layers stay. A key landing on another key of its property replaces it.
+/// A move of `d` in comp time as the move of `l`'s layer times (through its stretch, reversed
+/// too), without a round trip through comp time.
+pub(crate) fn layer_shift(l: &Layer, d: Tick) -> Tick {
+    l.layer_time(l.start_time + d)
+}
+
+/// Move the selected keys by `d` comp seconds, rounded to whole frames: the keys keep their
+/// spacing, subframe offsets (Quick Offset) included, and keys of time-stretched layers move with
+/// the pointer, not by the layer-time distance. Keys of locked layers stay. A key landing on
+/// another key of its property replaces it.
 pub(crate) fn shift_keys(s: &mut Session, label: &str, merge: Option<&str>, d: Tick) -> Result<Value> {
     let cid = s.active_comp_id().ok_or(EngineError::NoComp)?;
     let (from, total, base) = key_gesture(s, merge, d, false);
@@ -940,8 +947,8 @@ pub(crate) fn shift_keys(s: &mut Session, label: &str, merge: Option<&str>, d: T
             *proj = (*b).clone();
         }
         let comp = proj.comp_mut(cid).ok_or(EngineError::NoComp)?;
-        let fr = comp.frame_rate;
-        st.selected_keys = move_key_refs(comp, &from, |l, t| Ok(l.layer_time(fr.snap_nearest(l.comp_time(t) + total))))?;
+        let d = comp.frame_rate.snap_nearest(total);
+        st.selected_keys = move_key_refs(comp, &from, |l, t| Ok(t + layer_shift(l, d)))?;
         Ok(json!(st.selected_keys.len()))
     })
 }

crates/engine/src/commands/quick_offset.rs

@@ -9,7 +9,7 @@ use effectcraft_project::LayerId;
 use effectcraft_time::{TICKS_PER_SECOND, Tick};
 use serde_json::{Value, json};
 
-use super::prop::{key_gesture, move_key_refs};
+use super::prop::{key_gesture, layer_shift, move_key_refs};
 use super::{CommandSpec, b_p, bad, f_p, has_keys, has_layers, layers_p, merge_p};
 use crate::{EngineError, Result, Session, cmd};
 
@@ -110,11 +110,8 @@ fn keys(s: &mut Session, p: &Value) -> Result<Value> {
         st.selected_keys = move_key_refs(comp, &from, |l, t| {
             let Some(d) = by_layer.get(&l.id) else { return Ok(t) };
             let what = format!("a key of \"{}\"", l.name);
-            let at = l.layer_time(shifted(l.comp_time(t), *d, NAME, &what)?);
-            if at < Tick::MIN || at > Tick::MAX {
-                return Err(bad(NAME, format!("{what} would move out of range")));
-            }
-            Ok(at)
+            shifted(l.start_time, *d, NAME, &what)?;
+            shifted(t, layer_shift(l, *d), NAME, &what)
         })?;
         Ok(json!({"keys": st.selected_keys.len(), "layers": order.iter().map(|id| json!({"layer": id.0, "offset": by_layer.get(id).copied().unwrap_or_default().seconds()})).collect::<Vec<_>>()}))
     })

crates/engine/src/tests_quick_offset.rs

@@ -252,3 +252,19 @@ fn roving_keys_follow_and_the_selection_stays_on_the_moved_keys() {
     }
     assert_eq!(s.state.selected_keys.iter().map(|k| k.layer.0).collect::<Vec<_>>(), vec![ids[1], ids[0]]);
 }
+
+#[test]
+fn moving_staggered_keys_keeps_their_subframe_spacing() {
+    let (mut s, ids) = setup(4);
+    for id in &ids {
+        add_key(&mut s, *id, "transform/opacity", 0.0, json!(0));
+    }
+    select_keys(&mut s, &ids.iter().map(|id| (*id, "transform/opacity", 0.0)).collect::<Vec<_>>());
+    s.execute_checked("keys.quickOffset", json!({"delta": 1.0 / 30.0})).unwrap();
+    // A plain move (and a nudge) translates them: it used to put each key on its nearest frame.
+    s.execute_checked("keys.move", json!({"delta": 1.0 / 30.0})).unwrap();
+    s.execute_checked("keys.nudgeForward", json!({})).unwrap();
+    for (i, id) in ids.iter().enumerate() {
+        assert_eq!(key_times(&s, *id, "transform/opacity"), vec![2 * F + F * i as i64 / 3], "layer {i}");
+    }
+}