From 567a947fd84905293f817ed7d4beab5cffc8767c Mon Sep 17 00:00:00 2001 From: bflatastic Date: Thu, 8 Oct 2026 23:41:02 +0700 Subject: [PATCH] Timeline: deleting Time Remap turns time remapping off (#290) Selecting Time Remap in the Timeline (its name, which selects all its keys) and pressing Delete removed the keys, and a second Delete then deleted the whole layer. As in After Effects, Edit > Clear with Time Remap selected now turns time remapping off (Layer > Time > Enable Time Remapping off), in one undo step, and the layer stays; one selected Time Remap key on its own is still deleted as a key. Test: deleting_time_remap_turns_time_remapping_off (engine). Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/commands/edit.rs | 8 +++++ crates/engine/src/commands/layer_time.rs | 38 ++++++++++++++++++++++-- crates/engine/src/tests_timeline.rs | 22 ++++++++++++++ 3 files changed, 66 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/commands/edit.rs b/crates/engine/src/commands/edit.rs index 5e1b7495b..45ab1395a 100644 --- a/crates/engine/src/commands/edit.rs +++ b/crates/engine/src/commands/edit.rs @@ -142,6 +142,14 @@ fn delete(s: &mut Session, p: &Value) -> Result { if s.state.text_edit.is_some() && p.get("layers").is_none() { return s.execute("text.delete", json!({})); } + // Time Remap selected (its name, which selects all its keys) → time remapping off, not the + // layer (After Effects). + if p.get("layers").is_none() { + let remapped = super::layer_time::selected_time_remap(s); + if !remapped.is_empty() { + return super::layer_time::disable_time_remap(s, &remapped); + } + } // Keyframes selected → delete keys; mask vertices → delete them; else layers. if !s.state.selected_keys.is_empty() && p.get("layers").is_none() { return s.execute("keys.delete", json!({})); diff --git a/crates/engine/src/commands/layer_time.rs b/crates/engine/src/commands/layer_time.rs index 7a0eb86b0..80c1b4208 100644 --- a/crates/engine/src/commands/layer_time.rs +++ b/crates/engine/src/commands/layer_time.rs @@ -6,12 +6,12 @@ use effectcraft_keyframe::{Keyframe, Value as KV}; use effectcraft_project::build::Ids; -use effectcraft_project::{Layer, LayerSource, Node, ParamUi, Property}; +use effectcraft_project::{Layer, LayerId, LayerSource, Node, ParamUi, Property}; use effectcraft_time::Tick; use serde_json::{Value, json}; use super::{CommandSpec, b_p, bad, f_p, has_layers, layers_p, str_p}; -use crate::{EngineError, Result, Session, cmd}; +use crate::{EngineError, KeyRef, Result, Session, cmd}; /// Layers whose source has its own time (precomps and footage) can be remapped. fn remappable(l: &Layer) -> bool { @@ -116,6 +116,40 @@ fn enable_time_remap(s: &mut Session, p: &Value) -> Result { Ok(json!(on)) } +/// Layers of the active comp whose Time Remap property is selected with all of its keys (clicking +/// its name selects them all, and a key selects its property): Edit ▸ Clear turns their time +/// remapping off instead of deleting the keys or the layer. +pub(crate) fn selected_time_remap(s: &Session) -> Vec { + let Some(comp) = s.active_comp() else { return vec![] }; + let st = &s.state; + st.selected_props + .iter() + .filter(|(lid, uid)| { + comp.layer(*lid) + .and_then(|l| l.props.get("timeRemap")) + .filter(|pr| pr.uid == *uid) + .is_some_and(|pr| pr.keys.iter().all(|k| st.selected_keys.contains(&KeyRef { layer: *lid, prop: *uid, time: k.time }))) + }) + .map(|(lid, _)| *lid) + .collect() +} + +/// Turn time remapping off on `layers` (one undo step) and drop the removed property and its +/// keys from the selection. +pub(crate) fn disable_time_remap(s: &mut Session, layers: &[LayerId]) -> Result { + let ids: Vec = layers.iter().map(|l| l.0).collect(); + let r = s.execute("layer.enableTimeRemap", json!({"layers": ids, "value": false}))?; + let comp = s.active_comp(); + let gone = |lid: LayerId, uid: u64| layers.contains(&lid) && comp.and_then(|c| c.layer(lid)).is_none_or(|l| l.props.find(uid).is_none()); + let (props, keys): (Vec<_>, Vec<_>) = ( + s.state.selected_props.iter().copied().filter(|(l, u)| !gone(*l, *u)).collect(), + s.state.selected_keys.iter().copied().filter(|k| !gone(k.layer, k.prop)).collect(), + ); + s.state.selected_props = props; + s.state.selected_keys = keys; + Ok(r) +} + /// Freeze Frame: time remapping holding the current source frame for the whole layer. fn freeze(s: &mut Session, p: &Value, last: bool) -> Result { let (cid, ids) = layers_p(s, p)?; diff --git a/crates/engine/src/tests_timeline.rs b/crates/engine/src/tests_timeline.rs index 8356aec7a..b4c1cf50c 100644 --- a/crates/engine/src/tests_timeline.rs +++ b/crates/engine/src/tests_timeline.rs @@ -551,6 +551,28 @@ fn time_remap_enable_freeze_and_disable() { assert!(s.execute("layer.enableTimeRemap", json!({"layers": [r["layer"]]})).is_err()); } +/// Edit ▸ Clear with Time Remap selected (clicking its name selects all its keys) turns time +/// remapping off instead of deleting the layer (#290); one selected key is still just a key. +#[test] +fn deleting_time_remap_turns_time_remapping_off() { + let (mut s, l) = precomp_setup(); + s.execute("layer.enableTimeRemap", json!({})).unwrap(); + let uid = prop(&s, l, "timeRemap").uid; + s.execute("keys.select", json!({"keys": [{"layer": l, "prop": uid, "time": 0.0}], "selectProperties": true})).unwrap(); + s.execute("edit.clear", json!({})).unwrap(); + assert_eq!(prop(&s, l, "timeRemap").keys.len(), 1, "only the selected key went"); + s.execute("prop.select", json!({"layer": l, "prop": uid})).unwrap(); + s.execute("edit.clear", json!({})).unwrap(); + let comp = s.active_comp().unwrap(); + assert_eq!(comp.layers.len(), 1, "the layer stays"); + assert!(comp.layers[0].props.get("timeRemap").is_none(), "time remapping is off"); + assert!(s.state.selected_keys.is_empty() && !s.state.selected_props.iter().any(|(_, u)| *u == uid)); + assert!((square_x(&s, 1.0) - 30.0).abs() < 1.5); + // One undo step brings it back. + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(prop(&s, l, "timeRemap").keys.len(), 1); +} + #[test] fn freeze_on_last_frame_extends_the_layer() { let (mut s, l) = precomp_setup();