Timeline: Puppet pin keyframes show in the Timeline (#273)

Moving a puppet pin keys its Position, but the Timeline didn't show
those keys where people look for them. U revealed every pin's Position
as one more flat "Position" row under the layer, indistinguishable from
the layer's own Position and from each other, and the reveal was a
snapshot: a pin placed or moved after U (or with the layer twirled open
and Effects closed) got keys that no row showed.

As in After Effects, revealed properties inside an effect now show
under the effect and its groups (Puppet > Mesh 1 > Deform > Puppet
Pin 1 > Position), twirled open by the reveal and closable like any
group; Transform and mask properties stay as they were. Placing, moving
or recording a pin in the viewer keeps its Position in view: on a
twirled-open layer its groups open, and a U reveal takes it in. A
collapsed layer stays collapsed. The keys select, move and delete like
any others.

Test: puppet_pin_keys_show_under_their_pins_in_the_timeline,
placing_a_pin_opens_its_groups_on_a_twirled_open_layer (ui_viewer).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

crates/ui-egui/src/menus.rs

@@ -173,7 +173,8 @@ pub fn reveal(app: &mut EffectcraftApp, kind: &str, now: f64, add: bool) {
     } else {
         kinds = vec![kind.to_string()];
     }
-    tl.apply_reveal(&targets, kinds);
+    tl.apply_reveal(&targets, kinds.clone());
+    crate::panels::timeline::open_revealed(app, &targets, &kinds);
 }
 
 /// Show Time Remap on the layers of a `layer.enableTimeRemap` (`layers` by id, else the
@@ -1073,6 +1074,10 @@ pub fn frontend(app: &mut EffectcraftApp, ctx: &egui::Context, id: &str, p: Valu
             if !shown.is_empty() {
                 tl.apply_reveal(&shown, vec!["props".into()]);
             }
+            // Properties inside effects (puppet pins…) show under their effect, twirled open.
+            for (layer, prop) in props.iter().filter_map(|x| Some((x.get("layer")?.as_u64()?, x.get("prop")?.as_u64()?))) {
+                crate::panels::timeline::open_effect_paths(app, layer, &[prop]);
+            }
             json!({"revealed": found.len()})
         }
         _ => return Err(format!("`{id}` is not a frontend command")),

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

@@ -82,6 +82,16 @@ pub fn overlay(app: &EffectcraftApp, ctx: &egui::Context, cid: ItemId, layer: &L
     Some(overlay)
 }
 
+/// Keep a pin's keyframed Position in view in the Timeline after placing, moving or recording the
+/// pin in the viewer (#273).
+pub fn reveal_pin(app: &mut EffectcraftApp, layer: LayerId, pin: u64) {
+    let comp = app.session.active_comp();
+    let pos = comp.and_then(|c| c.layer(layer)?.props.find_group(pin)?.get("position")).filter(|p| p.is_animated()).map(|p| p.uid);
+    if let Some(pos) = pos {
+        super::timeline::keep_in_view(app, layer, pos);
+    }
+}
+
 /// Where a pin sits now (layer space): its Position, or (Bend / Starch / Overlap pins) its rest
 /// point carried along by the deformation.
 pub fn pin_position(mesh: &Mesh, def: &[[f64; 2]], p: &Pin) -> [f64; 2] {

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

@@ -685,8 +685,11 @@ fn build_rows(app: &EffectcraftApp, comp: &Comp) -> Vec<Row> {
 fn reveal_rows(app: &EffectcraftApp, l: &Layer, kinds: &[String], rows: &mut Vec<Row>) {
     let tl = &app.ui.timeline;
     let has = |k: &str| kinds.iter().any(|r| r == k);
-    let group_rows = |rows: &mut Vec<Row>, g: &PropGroup, fx: bool| {
+    // Groups shown with their whole contents: the revealed properties inside aren't listed again.
+    let mut shown_groups: Vec<u64> = vec![];
+    let mut group_rows = |rows: &mut Vec<Row>, g: &PropGroup, fx: bool| {
         for g in g.groups() {
+            shown_groups.push(g.uid);
             rows.push(Row {
                 layer: l.id,
                 depth: 1,
@@ -751,11 +754,6 @@ fn reveal_rows(app: &EffectcraftApp, l: &Layer, kinds: &[String], rows: &mut Vec
             wanted.extend(found);
         }
     }
-    if !wanted.is_empty() {
-        let mut found = vec![];
-        collect_props(&l.props, &mut found, &|p| wanted.contains(&p.uid));
-        rows.extend(found.into_iter().map(|uid| Row { layer: l.id, depth: 1, kind: RowKind::Prop { uid } }));
-    }
     // SS: the selected properties and groups (as Animation ▸ Reveal Properties shows them).
     let selected: std::collections::BTreeSet<u64> = app.session.state.selected_props.iter().filter(|(lid, _)| *lid == l.id).map(|(_, u)| *u).collect();
     if has("props") || has("selected") {
@@ -784,17 +782,124 @@ fn reveal_rows(app: &EffectcraftApp, l: &Layer, kinds: &[String], rows: &mut Vec
             if open {
                 push_group(rows, l, g, 2, &tl.open_groups);
             }
+            shown_groups.push(g.uid);
         }
-        let mut found = vec![];
-        collect_props(&l.props, &mut found, &|p| prop_visible(p, l) && reveal_props.contains(&p.uid) && !wanted.contains(&p.uid));
-        rows.extend(found.into_iter().map(|uid| Row { layer: l.id, depth: 1, kind: RowKind::Prop { uid } }));
+        wanted.extend(picked.iter().copied());
     }
+    revealed_rows(rows, l, &wanted, &shown_groups, &tl.open_groups);
     if has("waveform")
         && let Some(item) = super::waveform::audio_item(&app.session.project, l)
     {
         rows.push(Row { layer: l.id, depth: 1, kind: RowKind::Waveform { item: item.0 } });
     }
 }
+
+/// Rows for the revealed properties `shown` of `l`, in tree order. Those inside an effect show
+/// under the effect and its groups, as in After Effects (Puppet ▸ Mesh 1 ▸ Deform ▸ Puppet
+/// Pin 1 ▸ Position: every pin's Position is told apart, #273); the others (Transform, masks…)
+/// on their own. Groups in `skip` already show with their contents.
+fn revealed_rows(rows: &mut Vec<Row>, l: &Layer, shown: &std::collections::BTreeSet<u64>, skip: &[u64], open: &std::collections::BTreeSet<u64>) {
+    struct Tree<'a> {
+        l: &'a Layer,
+        wanted: &'a dyn Fn(&Property) -> bool,
+        skip: &'a [u64],
+        open: &'a std::collections::BTreeSet<u64>,
+    }
+    fn holds(g: &PropGroup, wanted: &dyn Fn(&Property) -> bool) -> bool {
+        g.children.iter().any(|c| match c {
+            Node::Prop(p) => wanted(p),
+            Node::Group(sg) => holds(sg, wanted),
+        })
+    }
+    // A group holding revealed properties (twirled as the user left it), then those inside.
+    fn group(rows: &mut Vec<Row>, t: &Tree<'_>, g: &PropGroup, depth: usize) {
+        if t.skip.contains(&g.uid) || !holds(g, t.wanted) {
+            return;
+        }
+        let o = t.open.contains(&g.uid);
+        let fx = matches!(g.kind, GroupKind::Effect { .. }).then_some(g.enabled);
+        rows.push(Row { layer: t.l.id, depth, kind: RowKind::Group { uid: g.uid, name: g.name.clone(), open: o, has_children: true, fx, eye: None } });
+        if !o {
+            return;
+        }
+        for c in &g.children {
+            match c {
+                Node::Prop(p) if (t.wanted)(p) => rows.push(Row { layer: t.l.id, depth: depth + 1, kind: RowKind::Prop { uid: p.uid } }),
+                Node::Group(sg) => group(rows, t, sg, depth + 1),
+                Node::Prop(_) => {}
+            }
+        }
+    }
+    let wanted = |p: &Property| shown.contains(&p.uid) && prop_visible(p, l);
+    let tree = Tree { l, wanted: &wanted, skip, open };
+    for c in &l.props.children {
+        match c {
+            Node::Group(fx) if fx.match_id == "effects" => {
+                for e in fx.groups() {
+                    group(rows, &tree, e, 1);
+                }
+            }
+            Node::Group(g) => {
+                let mut found = vec![];
+                collect_props(g, &mut found, &wanted);
+                rows.extend(found.into_iter().map(|uid| Row { layer: l.id, depth: 1, kind: RowKind::Prop { uid } }));
+            }
+            Node::Prop(p) if wanted(p) => rows.push(Row { layer: l.id, depth: 1, kind: RowKind::Prop { uid: p.uid } }),
+            Node::Prop(_) => {}
+        }
+    }
+}
+
+/// Uids of the groups that hold property `uid` of `l`, outermost first.
+fn enclosing_groups(l: &Layer, uid: u64) -> Vec<u64> {
+    let path = l.props.path_of(uid).unwrap_or_default();
+    let mut ids: Vec<u64> = path.split('/').filter_map(|s| s.strip_prefix('@')?.parse().ok()).collect();
+    ids.pop();
+    ids
+}
+
+/// Twirl open the groups holding `props` of `layer` that sit inside effects, so their rows show
+/// (a U reveal, an EE reveal, a puppet pin placed or moved in the viewer).
+pub(crate) fn open_effect_paths(app: &mut EffectcraftApp, layer: u64, props: &[u64]) {
+    let Some(l) = app.session.active_comp().and_then(|c| c.layer(LayerId(layer))) else { return };
+    let Some(fx) = l.effects().map(|g| g.uid) else { return };
+    let mut open = vec![];
+    for p in props {
+        let path = enclosing_groups(l, *p);
+        if path.first() == Some(&fx) {
+            open.extend(path);
+        }
+    }
+    app.ui.timeline.open_groups.extend(open);
+}
+
+/// After a reveal shortcut that picks properties by state (EE expressions): those inside effects
+/// on `layers` show under their effect, twirled open.
+pub(crate) fn open_revealed(app: &mut EffectcraftApp, layers: &[u64], kinds: &[String]) {
+    for kind in kinds.iter().filter(|k| matches!(k.as_str(), "animated" | "expressions")) {
+        for id in layers {
+            let Some(l) = app.session.active_comp().and_then(|c| c.layer(LayerId(*id))) else { continue };
+            let mut found = vec![];
+            collect_props(&l.props, &mut found, &|p| prop_visible(p, l) && reveal_matches(p, &[], kind));
+            open_effect_paths(app, *id, &found);
+        }
+    }
+}
+
+/// After the viewer edits a property (a puppet pin placed, moved or recorded): when `layer` is
+/// twirled open, its groups open, and a U reveal of the layer takes it in, so its keyframes show
+/// (#273). A collapsed layer stays collapsed, as in After Effects.
+pub fn keep_in_view(app: &mut EffectcraftApp, layer: LayerId, prop: u64) {
+    if !app.ui.timeline.open_layers.contains(&layer.0) {
+        return;
+    }
+    open_effect_paths(app, layer.0, &[prop]);
+    let tl = &mut app.ui.timeline;
+    if tl.layer_reveal.get(&layer.0).is_some_and(|k| k.iter().any(|k| k == "props")) {
+        tl.reveal_props.insert(prop);
+    }
+}
+
 fn collect_props(g: &PropGroup, out: &mut Vec<u64>, f: &dyn Fn(&Property) -> bool) {
     for c in &g.children {
         match c {

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

@@ -1456,10 +1456,16 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) {
                 Gesture::PuppetRecord { layer, pin, others, cti, samples, .. } => {
                     let pts: Vec<serde_json::Value> = samples.iter().map(|(t, p)| json!([t, p[0], p[1]])).collect();
                     let q = json!({"layer": layer.0, "pin": pin, "pins": others, "samples": pts, "start": cti.seconds()});
-                    if let Err(e) = app.session.execute("puppet.recordPin", q) {
-                        app.ui.status = e.to_string();
+                    match app.session.execute("puppet.recordPin", q) {
+                        Ok(_) => {
+                            for p in std::iter::once(pin).chain(others) {
+                                super::puppet_tool::reveal_pin(app, layer, p);
+                            }
+                        }
+                        Err(e) => app.ui.status = e.to_string(),
                     }
                 }
+                Gesture::PuppetPin { layer, pin, .. } => super::puppet_tool::reveal_pin(app, layer, pin),
                 Gesture::Create { tool, start } => {
                     let end = map.to_comp(pos);
                     create_shape(app, tool, start, end, mods.shift);
@@ -1555,9 +1561,13 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) {
                     && let Some(inv) = l2c(&ectx, l).0.inverse()
                 {
                     let lp = inv.apply(gv2(cpt[0], cpt[1]));
-                    let r = app.session.execute("puppet.addPin", json!({"layer": l.id.0, "kind": t.puppet_kind(), "position": [lp.x, lp.y]}));
-                    if let Err(e) = r {
-                        app.ui.status = e.to_string();
+                    match app.session.execute("puppet.addPin", json!({"layer": l.id.0, "kind": t.puppet_kind(), "position": [lp.x, lp.y]})) {
+                        Ok(r) => {
+                            if let Some(pin) = r["pin"].as_u64() {
+                                super::puppet_tool::reveal_pin(app, l.id, pin);
+                            }
+                        }
+                        Err(e) => app.ui.status = e.to_string(),
                     }
                 }
             }

crates/ui-egui/tests/ui_viewer.rs

@@ -801,6 +801,96 @@ fn puppet_pin_click_selects_and_delete_removes_only_the_pins() {
     assert!(h.state().session.active_comp().unwrap().layer(LayerId(id)).is_some());
 }
 
+/// A pin's Position property and its keys' times (seconds).
+fn pin_keys(h: &Harness<'_, EffectcraftApp>, layer: u64, pin: u64) -> (u64, Vec<f64>) {
+    let l = h.state().session.active_comp().unwrap().layer(LayerId(layer)).unwrap().clone();
+    let pos = l.props.find_group(pin).unwrap().get("position").unwrap().clone();
+    (pos.uid, pos.keys.iter().map(|k| k.time.seconds()).collect())
+}
+
+/// The keys the Timeline draws for a property, left to right.
+fn timeline_keys(h: &Harness<'_, EffectcraftApp>, prop: u64) -> Vec<Pos2> {
+    let mut ks: Vec<Pos2> =
+        h.state().auto.query(&format!("timeline.key.{prop}.")).iter().map(|e| pos2(e.rect[0] + e.rect[2] / 2.0, e.rect[1] + e.rect[3] / 2.0)).collect();
+    ks.sort_by(|a, b| a.x.total_cmp(&b.x));
+    ks
+}
+
+/// The pin the last click placed (placing a pin selects it).
+fn placed_pin(h: &Harness<'_, EffectcraftApp>) -> u64 {
+    h.state().session.state.selected_props.last().map(|(_, u)| *u).unwrap()
+}
+
+/// #273: U shows each pin's keyframed Position under its pin (Puppet ▸ Mesh 1 ▸ Deform ▸ Puppet
+/// Pin 1 ▸ Position) rather than as one more "Position"; a pin moved or placed in the viewer
+/// afterwards shows its keys there too (they were made but not shown); and pin keys select, move
+/// and delete like any others.
+#[test]
+fn puppet_pin_keys_show_under_their_pins_in_the_timeline() {
+    let (mut h, id, bend) = puppet_harness();
+    h.state_mut().ui.tool = Tool::Puppet;
+    let pins: Vec<u64> = h.state().auto.previous.iter().filter_map(|e| e.id.strip_prefix("viewer.puppetPin.")?.parse().ok()).filter(|p| *p != bend).collect();
+    let ctx = h.ctx.clone();
+    effectcraft_ui_egui::menus::invoke(h.state_mut(), &ctx, "anim.reveal", json!({"kind": "keyframes"})).unwrap();
+    h.run_steps(3);
+    assert_eq!(pins.len(), 2);
+    for pin in &pins {
+        let row = h.state().auto.find(&format!("timeline.group.{pin}.name")).map(|e| e.label.clone()).unwrap_or_default();
+        assert!(row.starts_with("Puppet Pin"), "the pin's row: {row:?}");
+        assert_eq!(timeline_keys(&h, pin_keys(&h, id, *pin).0).len(), 1, "its key at 0 s");
+    }
+    // Moved at 1 s: keyed there, in view.
+    h.state_mut().session.set_time(effectcraft_engine::time::Tick::from_seconds_f64(1.0));
+    h.run_steps(2);
+    let pa = rect(&h, &format!("viewer.puppetPin.{}", pins[0])).center();
+    drag(&mut h, pa, pa + vec2(20.0, 10.0));
+    let (pos, times) = pin_keys(&h, id, pins[0]);
+    assert_eq!(times.len(), 2);
+    assert_eq!(timeline_keys(&h, pos).len(), 2, "both keys drawn");
+    // A pin placed after U shows with its key too.
+    let at = screen(&h, [320.0, 210.0]);
+    click(&mut h, at);
+    let new = placed_pin(&h);
+    assert!(!pins.contains(&new) && new != bend);
+    assert!(h.state().auto.find(&format!("timeline.group.{new}.name")).is_some(), "the new pin's row");
+    assert_eq!(timeline_keys(&h, pin_keys(&h, id, new).0).len(), 1, "and its key");
+    // Click the 1 s key, drag it half a second later, delete it.
+    let ks = timeline_keys(&h, pos);
+    click(&mut h, ks[1]);
+    assert_eq!(h.state().session.state.selected_keys.iter().map(|k| k.prop).collect::<Vec<_>>(), [pos]);
+    let half = (ks[1].x - ks[0].x) / 2.0;
+    drag(&mut h, ks[1], ks[1] + vec2(half, 0.0));
+    let t = pin_keys(&h, id, pins[0]).1;
+    assert!((t[1] - 1.5).abs() < 0.05, "moved: {t:?}");
+    h.input_mut().events.push(Event::Key { key: egui::Key::Delete, physical_key: None, pressed: true, repeat: false, modifiers: Default::default() });
+    h.run_steps(2);
+    assert_eq!(pin_keys(&h, id, pins[0]).1, [0.0], "deleted; the pin stays");
+}
+
+/// #273: on a layer twirled open in the Timeline, placing a pin twirls open Effects ▸ Puppet ▸
+/// Mesh 1 ▸ Deform ▸ the pin, so its Position key shows; a collapsed layer stays collapsed, as
+/// in After Effects.
+#[test]
+fn placing_a_pin_opens_its_groups_on_a_twirled_open_layer() {
+    let mut h = harness();
+    let s = &mut h.state_mut().session;
+    s.execute("edit.clear", json!({"layers": ["Plate"]})).unwrap();
+    s.execute("layer.select", json!({"layers": ["Box"]})).unwrap();
+    let id = s.state.selected_layers[0].0;
+    h.state_mut().ui.tool = Tool::Puppet;
+    h.run_steps(2);
+    let at = screen(&h, [300.0, 160.0]);
+    click(&mut h, at);
+    assert!(h.state().ui.timeline.open_groups.is_empty() && h.state().ui.timeline.open_layers.is_empty(), "collapsed stays collapsed");
+    let twirl = rect(&h, &format!("timeline.layer.{id}.twirl")).center();
+    click(&mut h, twirl);
+    assert!(h.state().ui.timeline.open_layers.contains(&id));
+    let at = screen(&h, [340.0, 200.0]);
+    click(&mut h, at);
+    let pin = placed_pin(&h);
+    assert_eq!(timeline_keys(&h, pin_keys(&h, id, pin).0).len(), 1, "the new pin's Position key shows");
+}
+
 /// Filled circles painted this frame (flattening nested shape lists).
 fn circles(h: &Harness<'_, EffectcraftApp>) -> Vec<egui::epaint::CircleShape> {
     fn walk(s: &egui::Shape, out: &mut Vec<egui::epaint::CircleShape>) {