Fix motion-tween first-keyframe: anchor the clip's start

Creating the first transform keyframe for a clip instance at frame N left
the curve with a single keyframe, which Hold-extrapolates backward — so
moving it at frame N also moved it on every earlier frame (frame 1).

When SetKeyframeAction creates a brand-new curve for a clip instance and
the clip already existed before `time`, also anchor a keyframe at the
clip's start (its group visibility start, or timeline_start for movie
clips) with the original value. Earlier frames now hold the original
position and the move produces a proper tween from start to N.

Also capture the clip instance's actual on-stage value when keying (its
base transform), instead of a generic 0/identity default, so a new
keyframe doesn't snap the clip to the origin.

Adds VectorLayer::group_visibility_start and tests covering the anchor and
the no-double-anchor case (keying at the clip's own start).

lightningbeam-ui/lightningbeam-core/src/actions/set_keyframe.rs

@@ -8,6 +8,7 @@ use crate::action::Action;
 use crate::animation::{AnimationCurve, AnimationTarget, Keyframe, TransformProperty};
 use crate::document::Document;
 use crate::layer::{AnyLayer, ShapeKeyframe};
+use crate::object::Transform;
 use uuid::Uuid;
 
 /// Undo info for a clip animation curve
@@ -54,11 +55,17 @@ const TRANSFORM_PROPERTIES: &[TransformProperty] = &[
     TransformProperty::Opacity,
 ];
 
-fn transform_default(prop: &TransformProperty) -> f64 {
+/// The clip instance's own value for a property (its base transform / opacity).
+fn transform_prop_value(t: &Transform, opacity: f64, prop: &TransformProperty) -> f64 {
     match prop {
-        TransformProperty::ScaleX | TransformProperty::ScaleY => 1.0,
-        TransformProperty::Opacity => 1.0,
-        _ => 0.0,
+        TransformProperty::X => t.x,
+        TransformProperty::Y => t.y,
+        TransformProperty::Rotation => t.rotation,
+        TransformProperty::ScaleX => t.scale_x,
+        TransformProperty::ScaleY => t.scale_y,
+        TransformProperty::SkewX => t.skew_x,
+        TransformProperty::SkewY => t.skew_y,
+        TransformProperty::Opacity => opacity,
     }
 }
 
@@ -67,6 +74,21 @@ impl Action for SetKeyframeAction {
         self.clip_undo_entries.clear();
         self.shape_keyframe_created = false;
 
+        // Phase 1 (immutable): for each clip instance, gather its base transform and the
+        // start time of its visibility region, so a brand-new curve can be anchored there.
+        let mut clip_info: std::collections::HashMap<Uuid, (Transform, f64, f64)> =
+            std::collections::HashMap::new(); // id -> (base transform, opacity, start time)
+        if let Some(AnyLayer::Vector(vl)) = document.get_layer(&self.layer_id) {
+            for clip_id in &self.clip_instance_ids {
+                if let Some(ci) = vl.clip_instances.iter().find(|c| c.id == *clip_id) {
+                    let start = vl
+                        .group_visibility_start(clip_id, self.time)
+                        .unwrap_or(ci.timeline_start);
+                    clip_info.insert(*clip_id, (ci.transform.clone(), ci.opacity, start));
+                }
+            }
+        }
+
         let layer = document
             .get_layer_mut(&self.layer_id)
             .ok_or_else(|| format!("Layer {} not found", self.layer_id))?;
@@ -82,23 +104,37 @@ impl Action for SetKeyframeAction {
 
             // Add clip animation keyframes
             for clip_id in &self.clip_instance_ids {
+                let (base_transform, base_opacity, start) = clip_info
+                    .get(clip_id)
+                    .cloned()
+                    .unwrap_or((Transform::new(), 1.0, 0.0));
                 for prop in TRANSFORM_PROPERTIES {
                     let target = AnimationTarget::Object {
                         id: *clip_id,
                         property: *prop,
                     };
-                    let default = transform_default(prop);
-                    let value = vl.layer.animation_data.eval(&target, self.time, default);
+                    // Fall back to the clip's OWN value (not a generic default) so a brand-new
+                    // keyframe captures the actual on-stage position, not (0,0)/identity.
+                    let base = transform_prop_value(&base_transform, base_opacity, prop);
+                    let value = vl.layer.animation_data.eval(&target, self.time, base);
 
                     let curve_created = vl.layer.animation_data.get_curve(&target).is_none();
                     if curve_created {
                         vl.layer
                             .animation_data
-                            .set_curve(AnimationCurve::new(target.clone(), default));
+                            .set_curve(AnimationCurve::new(target.clone(), base));
                     }
 
                     let curve = vl.layer.animation_data.get_curve_mut(&target).unwrap();
                     let old_keyframe = curve.get_keyframe_at(self.time, 0.001).cloned();
+
+                    // When this is the first keyframe of the curve and the clip already existed
+                    // before `time`, anchor a keyframe at its start with the original value.
+                    // Otherwise a single keyframe would Hold-extrapolate backward and move the
+                    // clip on every earlier frame too (the motion-tween first-keyframe bug).
+                    if curve_created && start < self.time - 0.001 {
+                        curve.set_keyframe(Keyframe::linear(start, base));
+                    }
                     curve.set_keyframe(Keyframe::linear(self.time, value));
 
                     self.clip_undo_entries.push(ClipUndoEntry {
@@ -145,3 +181,86 @@ impl Action for SetKeyframeAction {
         "New keyframe".to_string()
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::actions::TransformClipInstancesAction;
+    use crate::clip::ClipInstance;
+    use crate::layer::VectorLayer;
+    use std::collections::HashMap;
+
+    fn x_curve_eval(document: &Document, layer_id: Uuid, instance_id: Uuid, time: f64) -> f64 {
+        let target = AnimationTarget::Object { id: instance_id, property: TransformProperty::X };
+        match document.get_layer(&layer_id) {
+            Some(AnyLayer::Vector(vl)) => vl.layer.animation_data.eval(&target, time, f64::NAN),
+            _ => panic!("no layer"),
+        }
+    }
+
+    #[test]
+    fn first_keyframe_then_move_does_not_disturb_earlier_frames() {
+        // Group created at frame 0 (clip at x=50), keyframe + move at frame 10 → x=200.
+        // Frame 0 must keep x=50 (the motion-tween first-keyframe bug: it used to become 200).
+        let mut document = Document::new("Test");
+        let mut layer = VectorLayer::new("Layer");
+        let clip_id = Uuid::new_v4();
+        let instance_id = Uuid::new_v4();
+        let mut instance = ClipInstance::with_id(instance_id, clip_id);
+        instance.transform = Transform::with_position(50.0, 50.0);
+        layer.clip_instances.push(instance);
+        // The group's visibility starts at a keyframe at time 0 containing the instance.
+        layer.ensure_keyframe_at(0.0).clip_instance_ids.push(instance_id);
+        let layer_id = document.root_mut().add_child(AnyLayer::Vector(layer));
+
+        // Create a keyframe at frame 10.
+        SetKeyframeAction::new(layer_id, 10.0, vec![instance_id])
+            .execute(&mut document)
+            .unwrap();
+
+        // The new curve must be anchored at the start (two keyframes, both at x=50 so far).
+        assert!((x_curve_eval(&document, layer_id, instance_id, 0.0) - 50.0).abs() < 1e-6);
+        assert!((x_curve_eval(&document, layer_id, instance_id, 10.0) - 50.0).abs() < 1e-6);
+
+        // Move the clip at frame 10 to x=200.
+        let mut transforms = HashMap::new();
+        transforms.insert(
+            instance_id,
+            (Transform::with_position(50.0, 50.0), Transform::with_position(200.0, 200.0)),
+        );
+        TransformClipInstancesAction::new(layer_id, 10.0, transforms)
+            .execute(&mut document)
+            .unwrap();
+
+        // Frame 0 unchanged; frame 10 moved; midpoint tweens.
+        assert!((x_curve_eval(&document, layer_id, instance_id, 0.0) - 50.0).abs() < 1e-6, "frame 0 must stay 50");
+        assert!((x_curve_eval(&document, layer_id, instance_id, 10.0) - 200.0).abs() < 1e-6, "frame 10 must be 200");
+        assert!((x_curve_eval(&document, layer_id, instance_id, 5.0) - 125.0).abs() < 1e-6, "midpoint tweens");
+    }
+
+    #[test]
+    fn first_keyframe_at_clip_start_is_not_double_anchored() {
+        // When the keyframe is created at the clip's own start, there's nothing earlier to
+        // anchor — a single keyframe is correct.
+        let mut document = Document::new("Test");
+        let mut layer = VectorLayer::new("Layer");
+        let clip_id = Uuid::new_v4();
+        let instance_id = Uuid::new_v4();
+        let mut instance = ClipInstance::with_id(instance_id, clip_id);
+        instance.transform = Transform::with_position(10.0, 0.0);
+        layer.clip_instances.push(instance);
+        layer.ensure_keyframe_at(0.0).clip_instance_ids.push(instance_id);
+        let layer_id = document.root_mut().add_child(AnyLayer::Vector(layer));
+
+        SetKeyframeAction::new(layer_id, 0.0, vec![instance_id])
+            .execute(&mut document)
+            .unwrap();
+
+        let target = AnimationTarget::Object { id: instance_id, property: TransformProperty::X };
+        if let Some(AnyLayer::Vector(vl)) = document.get_layer(&layer_id) {
+            let curve = vl.layer.animation_data.get_curve(&target).unwrap();
+            assert_eq!(curve.keyframes.len(), 1, "keyframe at clip start needs no anchor");
+            assert!((curve.eval(0.0) - 10.0).abs() < 1e-6);
+        }
+    }
+}

lightningbeam-ui/lightningbeam-core/src/layer.rs

@@ -433,6 +433,25 @@ impl VectorLayer {
         time + frame_duration
     }
 
+    /// Start time of the group clip instance's visibility region that contains `time`:
+    /// the time of the earliest keyframe reachable by walking back through consecutive
+    /// keyframes that all contain the clip instance. Returns `None` if the clip instance
+    /// isn't a keyframe-gated group visible at `time` (e.g. a movie clip).
+    pub fn group_visibility_start(&self, clip_instance_id: &Uuid, time: f64) -> Option<f64> {
+        let after = self.keyframes.partition_point(|kf| kf.time <= time);
+        if after == 0 {
+            return None;
+        }
+        let mut idx = after - 1; // keyframe at-or-before `time`
+        if !self.keyframes[idx].clip_instance_ids.contains(clip_instance_id) {
+            return None;
+        }
+        while idx > 0 && self.keyframes[idx - 1].clip_instance_ids.contains(clip_instance_id) {
+            idx -= 1;
+        }
+        Some(self.keyframes[idx].time)
+    }
+
     // Shape-based methods removed — use DCEL methods instead.
     // - shapes_at_time_mut → graph_at_time_mut
     // - get_shape_in_keyframe → use DCEL vertex/edge/face accessors