From f5c6487cf11d7ce02b39d3063f4c86934d2dc89e Mon Sep 17 00:00:00 2001 From: Vikram Bhaskaran Date: Tue, 13 Oct 2026 00:50:56 +1100 Subject: [PATCH] Reveal enabled Time Remap on U even with no keys left (#990) anim.reveal keyframes/animation only matched keyframed top-level properties, so Time Remap vanished from U once its keys were deleted even though enabling remapping is itself the modification (and freshly enabled remaps, which get two keys, kept showing). Match the bare timeRemap prop for both kinds, as modified already did. --- crates/engine/src/commands/animation.rs | 6 +++--- crates/engine/src/tests_timeline.rs | 25 ++++++++++++++++++++++++ crates/ui-egui/src/panels/timeline.rs | 26 +++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/commands/animation.rs b/crates/engine/src/commands/animation.rs index c700123d..199cd743 100644 --- a/crates/engine/src/commands/animation.rs +++ b/crates/engine/src/commands/animation.rs @@ -361,11 +361,11 @@ fn reveal(s: &mut Session, p: &Value) -> Result { let anchor = [w as f64 / 2.0, h as f64 / 2.0]; let center = [comp.width as f64 / 2.0, comp.height as f64 / 2.0]; // Properties at the top of the layer's tree (Time Remap): enabled time remapping is a - // modification in itself. + // modification in itself, so it reveals even with no keys left on it (#990). for pr in l.props.props() { let hit = match kind.as_str() { - "keyframes" => pr.is_animated(), - "animation" => pr.is_animated() || pr.has_expression(), + "keyframes" => pr.is_animated() || pr.match_id == "timeRemap", + "animation" => pr.is_animated() || pr.has_expression() || pr.match_id == "timeRemap", _ => pr.match_id == "timeRemap" || modified(pr, "", anchor, center), }; if hit { diff --git a/crates/engine/src/tests_timeline.rs b/crates/engine/src/tests_timeline.rs index 009af9e5..9014faeb 100644 --- a/crates/engine/src/tests_timeline.rs +++ b/crates/engine/src/tests_timeline.rs @@ -750,6 +750,31 @@ fn reveal_shortcuts_take_in_time_remap() { } } +/// U (and the animation reveal) still show Time Remap after its keys are gone: enabling +/// remapping is itself the modification, so a keyless remap must not vanish from U (#990). +#[test] +fn reveal_shortcuts_take_in_keyless_time_remap() { + let (mut s, l) = precomp_setup(); + let revealed = |s: &mut Session, kind: &str| -> Vec { + let r = s.execute("anim.reveal", json!({"kind": kind})).unwrap(); + r["props"].as_array().unwrap().iter().filter(|p| p["layer"] == l).filter_map(|p| p["prop"].as_u64()).collect() + }; + s.execute("layer.enableTimeRemap", json!({})).unwrap(); + let uid = prop(&s, l, "timeRemap").uid; + assert_eq!(prop(&s, l, "timeRemap").keys.len(), 2); + // Delete both keys one at a time; remapping stays enabled. + for _ in 0..2 { + let t = prop(&s, l, "timeRemap").keys[0].time.seconds(); + s.execute("keys.select", json!({"keys": [{"layer": l, "prop": uid, "time": t}]})).unwrap(); + s.execute("edit.clear", json!({})).unwrap(); + } + let tr = prop(&s, l, "timeRemap"); + assert!(tr.keys.is_empty(), "remapping is still enabled, with no keys"); + for kind in ["keyframes", "animation", "modified"] { + assert_eq!(revealed(&mut s, kind), vec![uid], "{kind}"); + } +} + /// UU on a layer nothing was changed on shows nothing, whatever its kind: cameras, lights, text /// and nulls have their own Transform defaults, and Separate Dimensions keeps the comp centre. #[test] diff --git a/crates/ui-egui/src/panels/timeline.rs b/crates/ui-egui/src/panels/timeline.rs index 72f10429..daf50d60 100644 --- a/crates/ui-egui/src/panels/timeline.rs +++ b/crates/ui-egui/src/panels/timeline.rs @@ -4050,6 +4050,32 @@ mod tests { } } + /// U shows Time Remap while remapping is enabled even after its keys are deleted (#990): + /// enabling remapping is itself the modification. + #[test] + fn u_reveals_keyless_time_remap() { + let mut s = effectcraft_engine::Session::default(); + let inner = s.execute("comp.new", json!({"name": "Inner", "width": 32, "height": 32, "duration": 2})).unwrap()["comp"].as_u64().unwrap(); + s.execute("comp.new", json!({"name": "Main", "width": 64, "height": 64, "duration": 4})).unwrap(); + let nested = s.execute("layer.addItem", json!({"item": inner})).unwrap()["layer"].as_u64().unwrap(); + let mut app = EffectcraftApp::new(s); + let ctx = egui::Context::default(); + crate::menus::invoke(&mut app, &ctx, "layer.enableTimeRemap", json!({"layers": [nested]})).unwrap(); + let uid = app.session.active_comp().unwrap().layer(LayerId(nested)).unwrap().props.prop("timeRemap").unwrap().uid; + for _ in 0..2 { + let t = app.session.active_comp().unwrap().layer(LayerId(nested)).unwrap().props.prop("timeRemap").unwrap().keys[0].time.seconds(); + app.session.execute("keys.select", json!({"keys": [{"layer": nested, "prop": uid, "time": t}]})).unwrap(); + app.session.execute("edit.clear", json!({})).unwrap(); + } + assert!(app.session.active_comp().unwrap().layer(LayerId(nested)).unwrap().props.prop("timeRemap").unwrap().keys.is_empty()); + app.session.execute("layer.select", json!({"layers": [nested]})).unwrap(); + crate::menus::invoke(&mut app, &ctx, "anim.reveal", json!({"kind": "keyframes"})).unwrap(); + assert_eq!(app.ui.timeline.layer_reveal.get(&nested), Some(&vec!["props".to_string()])); + let rows = labels(&app); + let at = rows.iter().position(|r| r == "Inner").unwrap(); + assert_eq!(rows[at + 1].trim(), "Time Remap", "{rows:?}"); + } + /// Enabling time remapping twirls the layer open on Time Remap (#157); disabling it reveals /// nothing. #[test]