Graph Editor: preserve locked keys across gestures (#1342)

crates/engine/src/cmd/key_transform.rs

@@ -126,6 +126,9 @@ fn spatial_tangents(s: &mut Session, p: &Value) -> Result<Value> {
     let brk = b_p(p, "break").unwrap_or(false);
     s.edit("Edit Spatial Tangents", merge_p(p), |proj, _| {
         let l = super::layer_mut(proj, cid, lid)?;
+        if l.switches.locked {
+            return Err(bad("keys.setSpatialTangents", "the layer is locked"));
+        }
         let pr = l.props.find_mut(uid).ok_or_else(|| bad("keys.setSpatialTangents", "no property"))?;
         if !pr.spatial {
             return Err(bad("keys.setSpatialTangents", "not a spatial property"));

crates/engine/src/tests/delete_keys.rs

@@ -85,6 +85,35 @@ fn selection_based_key_edits_skip_a_locked_layer() {
     assert_ne!(stored(&s), before, "unlocked, Easy Ease changes the keys");
 }
 
+#[test]
+fn spatial_handles_cannot_edit_a_locked_layer() {
+    let (mut s, layer, _) = setup();
+    for (time, value) in [(0, [10, 20, 0]), (1, [80, 60, 0])] {
+        s.execute("prop.addKey", json!({"layer": layer, "path": "transform/position", "time": time, "value": value})).unwrap();
+    }
+    let prop = s.active_comp().unwrap().layer(LayerId(layer)).unwrap().props.prop("transform/position").unwrap().uid;
+    s.execute("keys.select", json!({"keys": [{"layer": layer, "prop": prop, "time": 0}]})).unwrap();
+    s.execute("layer.setSwitch", json!({"layers": [layer], "switch": "locked", "value": true})).unwrap();
+    let before = s.project.to_json();
+    let selected = s.state.selected_keys.clone();
+    let undo_len = s.history.undo.len();
+    let params = json!({"layer": layer, "prop": prop, "time": 0, "out": [15, 25, 0]});
+    let error = s.execute("keys.setSpatialTangents", params.clone()).unwrap_err().to_string();
+    assert!(error.contains("locked"), "{error}");
+    assert_eq!(s.project.to_json(), before);
+    assert_eq!(s.state.selected_keys, selected);
+    assert_eq!(s.history.undo.len(), undo_len);
+    s.execute("layer.setSwitch", json!({"layers": [layer], "switch": "locked", "value": false})).unwrap();
+    let unlocked = s.project.to_json();
+    s.execute("keys.setSpatialTangents", params).unwrap();
+    let edited = s.project.to_json();
+    assert_ne!(edited, unlocked);
+    s.execute("edit.undo", json!({})).unwrap();
+    assert_eq!(s.project.to_json(), unlocked);
+    s.execute("edit.redo", json!({})).unwrap();
+    assert_eq!(s.project.to_json(), edited);
+}
+
 #[test]
 fn key_transform_preserves_locked_keys_in_a_mixed_selection() {
     let (mut s, locked, locked_prop) = setup();

crates/ui-egui/src/header.rs

@@ -883,21 +883,41 @@ mod tests {
         let mut s = effectcraft_engine::Session::default();
         s.execute("comp.new", json!({"width": 400, "height": 300, "duration": 2})).unwrap();
         let first = s.execute("layer.newShape", json!({"kind": "rect"})).unwrap()["layer"].as_u64().unwrap();
+        s.execute("shape.fillStroke", json!({"layers": [first], "fill": "#00ff00", "stroke": false})).unwrap();
         s.execute("shape.toolOptions", json!({"fill": "#00ff00", "stroke": false})).unwrap();
         let mut app = EffectcraftApp::new(s);
         let ctx = egui::Context::default();
         assert_eq!(app.session.state.selected_layers.first().map(|l| l.0), Some(first), "a shape layer is selected");
-        comp_sized_shape(&mut app, &ctx, Tool::Star).unwrap();
+        crate::theme::install(&ctx, &app.tokens, crate::i18n::language(&app));
+        frame(&mut app, &ctx, 0.0, vec![]);
+        frame(&mut app, &ctx, 0.1, vec![]);
+        assert_eq!(app.session.state.shape_tool.fill.color, [0.0, 1.0, 0.0], "the visible toolbar paint");
+        let [x, y, w, h] = app.auto.find("tools.Rectangle").unwrap().rect;
+        let at = egui::pos2(x + w / 2.0, y + h / 2.0);
+        let pointer = |pressed| egui::Event::PointerButton { pos: at, button: egui::PointerButton::Primary, pressed, modifiers: Default::default() };
+        frame(&mut app, &ctx, 1.0, vec![egui::Event::PointerMoved(at), pointer(true)]);
+        frame(&mut app, &ctx, 1.05, vec![pointer(false)]);
+        assert_eq!(app.session.active_comp().unwrap().layers.len(), 1, "a single click only selects the tool");
+        // A separate double-click must route through the toolbar response, not just its helper.
+        for (time, pressed) in [(2.0, true), (2.05, false), (2.1, true), (2.15, false)] {
+            frame(&mut app, &ctx, time, vec![pointer(pressed)]);
+        }
         let comp = app.session.active_comp().unwrap();
         assert_eq!(comp.layers.len(), 2, "a new layer, not a group in the selected one");
         let layer = comp.layers.iter().find(|l| l.id.0 != first).unwrap();
+        assert_eq!(layer.props.prop("transform/position").unwrap().value.components(), vec![200.0, 150.0, 0.0]);
         let mut fill = None;
+        let mut size = None;
         layer.props.walk("", &mut |_, pr| {
+            if pr.name == "Size" {
+                size = Some(pr.value.clone());
+            }
             if fill.is_none() && matches!(pr.value, effectcraft_engine::keyframe::Value::Color(_)) {
                 fill = Some(pr.value.clone());
             }
         });
         let fill = fill.expect("a fill colour");
+        assert_eq!(size, Some(effectcraft_engine::keyframe::Value::Vec2([400.0, 300.0])));
         assert_eq!(fill, effectcraft_engine::keyframe::Value::Color([0.0, 1.0, 0.0, 1.0]), "the Tools bar's Fill, not the default blue");
     }
 }

crates/ui-egui/src/panels/graph.rs

@@ -456,6 +456,7 @@ pub(crate) fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painte
     let mut key_screens: Vec<(serde_json::Value, Pos2)> = vec![];
     for c in &cs {
         let (lid, uid, d) = (c.layer.id, c.prop.uid, c.dim);
+        let unlocked = !c.layer.switches.locked;
         for k in &c.keys {
             let kp = pos2(tm.x(k.t), ymap(k.v));
             // In the speed graph an end key rises with the handle being dragged; skipping it once it
@@ -477,9 +478,10 @@ pub(crate) fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painte
                 app.auto.add(&format!("timeline.graph.handle.{uid}.{d}.{}.{side}", k.idx), hr, "Bezier handle");
                 // The same side's handles of every selected key move with it, by as much as the
                 // pointer moved (After Effects), in one undo step.
-                if hresp.drag_started() {
+                if unlocked && hresp.drag_started() {
                     let grabs: Vec<HandleGrab> = cs
                         .iter()
+                        .filter(|c2| !c2.layer.switches.locked)
                         .flat_map(|c2| {
                             c2.keys.iter().flat_map(move |k2| {
                                 k2.handles.iter().filter(|h2| h2.out == h.out).map(move |h2| HandleGrab {
@@ -499,7 +501,8 @@ pub(crate) fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painte
                     let press = ui.input(|i| i.pointer.press_origin()).unwrap_or(hp);
                     ctx.data_mut(|dd| dd.insert_temp::<HandleDrag>(handle_drag_id(), (press, grabs)));
                 }
-                if hresp.dragged()
+                if unlocked
+                    && hresp.dragged()
                     && let Some(pt) = hresp.interact_pointer_pos()
                     && let Some((press, grabs)) = ctx.data(|dd| dd.get_temp::<HandleDrag>(handle_drag_id()))
                 {
@@ -530,7 +533,7 @@ pub(crate) fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painte
             app.auto.add(&format!("timeline.graph.key.{uid}.{d}.{}", k.idx), kr, &c.prop.name);
             let mods = ui.input(|i| i.modifiers);
             let key = &c.prop.keys[k.idx];
-            if kresp.clicked() && mods.command {
+            if unlocked && kresp.clicked() && mods.command {
                 // Ctrl+click: Linear ↔ Auto Bezier; Ctrl+Alt+click: Hold on / off (as in the Timeline).
                 actions.push(("keys.select".into(), json!({"keys": [kjson.clone()]})));
                 if mods.alt {
@@ -539,20 +542,22 @@ pub(crate) fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painte
                     let linear = key.in_interp == Interp::Linear && key.out_interp == Interp::Linear;
                     actions.push(("keys.interpolation".into(), json!({"interpolation": if linear { "autoBezier" } else { "linear" }})));
                 }
-            } else if kresp.clicked() && mods.shift {
+            } else if unlocked && kresp.clicked() && mods.shift {
                 actions.push(("keys.select".into(), json!({"keys": [kjson.clone()], "toggle": true})));
-            } else if (kresp.clicked() || kresp.drag_started() || kresp.secondary_clicked()) && !k.sel {
+            } else if unlocked && (kresp.clicked() || kresp.drag_started() || kresp.secondary_clicked()) && !k.sel {
                 actions.push(("keys.select".into(), json!({"keys": [kjson.clone()], "add": mods.shift})));
             }
             // A drag moves every selected key, by how far the pointer went from where the button
             // went down (the key doesn't jump under the pointer); Shift keeps it to the time or
             // the value axis, whichever moved more.
-            if kresp.drag_started()
+            if unlocked
+                && kresp.drag_started()
                 && let Some(o) = ui.input(|i| i.pointer.press_origin())
             {
                 ctx.data_mut(|dd| dd.insert_temp(key_grab_id(), KeyGrab { press: o, t: k.t }));
             }
-            if kresp.dragged()
+            if unlocked
+                && kresp.dragged()
                 && let Some(pt) = kresp.interact_pointer_pos()
                 && let Some(g) = ctx.data(|dd| dd.get_temp::<KeyGrab>(key_grab_id()))
             {

crates/ui-egui/src/panels/timeline/tests.rs

@@ -583,3 +583,34 @@ fn the_key_menu_is_translated_and_cannot_edit_a_locked_layers_keys() {
     assert!(enabled(false, true, &copy));
     crate::i18n::set_current("en");
 }
+
+#[test]
+fn a_key_menu_edit_selects_the_clicked_key_before_applying_the_command() {
+    use egui_kittest::{Harness, kittest::Queryable};
+    let mut s = effectcraft_engine::Session::default();
+    s.execute("comp.new", json!({"width": 64, "height": 64, "duration": 2})).unwrap();
+    let id = s.execute("layer.newSolid", json!({"color": "#fff"})).unwrap()["layer"].as_u64().unwrap();
+    for (time, value) in [(0.0, 0), (1.0, 50)] {
+        s.execute("prop.addKey", json!({"layer": id, "path": "transform/opacity", "time": time, "value": value})).unwrap();
+    }
+    let layer = s.active_comp().unwrap().layer(LayerId(id)).unwrap().clone();
+    let prop = layer.props.prop("transform/opacity").unwrap().clone();
+    s.execute("keys.select", json!({"keys": [{"layer": id, "prop": prop.uid, "time": 1.0}]})).unwrap();
+    let mut app = EffectcraftApp::new(s);
+    let mut actions = vec![];
+    {
+        let mut h = Harness::new_ui(|ui| key_menu(ui, &app, &layer, &prop, &prop.keys[0], false, &mut actions));
+        h.run();
+        h.get_by_label("Easy Ease").click();
+        h.run();
+    }
+    assert_eq!(actions.iter().map(|(command, _)| command.as_str()).collect::<Vec<_>>(), ["keys.select", "keys.easyEase"]);
+    for (command, params) in actions {
+        app.session.execute(&command, params).unwrap();
+    }
+    let keys = &app.session.active_comp().unwrap().layer(LayerId(id)).unwrap().props.find(prop.uid).unwrap().keys;
+    assert_ne!(keys[0], prop.keys[0], "the clicked key was eased");
+    assert_eq!(keys[1], prop.keys[1], "the previously selected key was not edited");
+    assert_eq!(app.session.state.selected_keys.len(), 1);
+    assert_eq!(app.session.state.selected_keys[0].time, prop.keys[0].time);
+}

crates/ui-egui/tests/it/ui_graph_fit.rs

@@ -77,3 +77,75 @@ fn open_graph_toggle_twice(h: &mut Harness<'_, EffectcraftApp>) {
     assert!(!h.state().ui.timeline.graph_editor);
     open_graph(h);
 }
+
+#[test]
+fn locked_graph_keys_preserve_the_existing_selection_and_keyframes() {
+    use egui::{Event, Modifiers, PointerButton, pos2, vec2};
+    for (button, modifiers, drag) in [
+        (PointerButton::Primary, Modifiers::NONE, false),
+        (PointerButton::Primary, Modifiers::SHIFT, false),
+        (PointerButton::Primary, Modifiers::COMMAND, false),
+        (PointerButton::Secondary, Modifiers::NONE, false),
+        (PointerButton::Primary, Modifiers::NONE, true),
+    ] {
+        let (mut h, layer, uid) = harness(true);
+        let s = &mut h.state_mut().session;
+        let locked = s.execute("layer.newSolid", json!({"name": "Locked", "color": "#fff"})).unwrap()["layer"].as_u64().unwrap();
+        for (time, value) in [(9.0, 100.0), (11.0, 200.0)] {
+            s.execute("prop.addKey", json!({"layer": locked, "path": "transform/rotation", "time": time, "value": value})).unwrap();
+        }
+        let locked_id = effectcraft_engine::project::LayerId(locked);
+        let locked_uid = s.active_comp().unwrap().layer(locked_id).unwrap().props.prop("transform/rotation").unwrap().uid;
+        s.execute("layer.setSwitch", json!({"layers": [locked], "switch": "locked", "value": true})).unwrap();
+        s.execute("keys.select", json!({"keys": [{"layer": layer, "prop": uid, "time": 12.0}]})).unwrap();
+        s.state.selected_props = vec![(effectcraft_engine::project::LayerId(layer), uid), (locked_id, locked_uid)];
+        let selection = s.state.selected_keys.clone();
+        let before = s.project.clone();
+        open_graph(&mut h);
+        h.state_mut().ui.timeline.graph_auto_zoom = false;
+        h.state_mut().ui.timeline.graph_range = Some((0.0, 800.0));
+        h.run_steps(3);
+        let [x, y, w, height] = h.state().auto.find(&format!("timeline.graph.key.{locked_uid}.0.0")).expect("locked key drawn").rect;
+        let point = pos2(x + w / 2.0, y + height / 2.0);
+        h.input_mut().events.push(Event::ModifiersChanged(modifiers));
+        h.input_mut().events.push(Event::PointerMoved(point));
+        h.input_mut().events.push(Event::PointerButton { pos: point, button, pressed: true, modifiers });
+        h.step();
+        let end = if drag { point + vec2(30.0, -20.0) } else { point };
+        h.input_mut().events.push(Event::PointerMoved(end));
+        h.step();
+        h.input_mut().events.push(Event::PointerButton { pos: end, button, pressed: false, modifiers });
+        h.run_steps(3);
+        assert_eq!(h.state().session.state.selected_keys, selection, "{button:?}, {modifiers:?}, drag={drag}");
+        assert_eq!(
+            serde_json::to_value(&h.state().session.project).unwrap(),
+            serde_json::to_value(&before).unwrap(),
+            "locked gestures must not edit another key"
+        );
+    }
+}
+
+#[test]
+fn an_unlocked_graph_keys_menu_edits_the_clicked_key() {
+    use egui::{Event, Modifiers, PointerButton, pos2};
+    use egui_kittest::kittest::Queryable;
+    let (mut h, layer, uid) = harness(true);
+    h.state_mut().session.execute("keys.select", json!({"keys": [{"layer": layer, "prop": uid, "time": 12.0}]})).unwrap();
+    open_graph(&mut h);
+    let [x, y, w, height] = h.state().auto.find(&format!("timeline.graph.key.{uid}.0.0")).unwrap().rect;
+    let at = pos2(x + w / 2.0, y + height / 2.0);
+    for pressed in [true, false] {
+        h.input_mut().events.push(Event::PointerMoved(at));
+        h.input_mut().events.push(Event::PointerButton { pos: at, button: PointerButton::Secondary, pressed, modifiers: Modifiers::NONE });
+        h.step();
+    }
+    h.run_steps(3);
+    let selected = &h.state().session.state.selected_keys;
+    assert_eq!(selected.len(), 1);
+    assert_eq!(selected[0].time.seconds(), 8.0);
+    h.get_by_label("Easy Ease").click();
+    h.run_steps(3);
+    let prop = h.state().session.active_comp().unwrap().layer(effectcraft_engine::project::LayerId(layer)).unwrap().props.find(uid).unwrap();
+    assert_eq!(prop.keys[0].out_interp, effectcraft_engine::keyframe::Interp::Bezier);
+    assert_eq!(prop.keys[1].out_interp, effectcraft_engine::keyframe::Interp::Linear);
+}