Merge pull request #1021 from pravdomil/claude/uu-effect-params

# Conflicts:
#	crates/ui-egui/src/panels/timeline.rs

crates/engine/src/commands/animation.rs

@@ -349,6 +349,25 @@ fn changed_from(g: &PropGroup, default: &PropGroup, out: &mut Vec<Uid>) {
     }
 }
 
+/// What of effect group `g` differs from `new`, a new instance of the effect: parameters with
+/// another value, and in full the groups a new instance lacks (puppet meshes and pins, paint
+/// strokes: what the user made).
+fn effect_changes(g: &PropGroup, new: &PropGroup, out: &mut Vec<Uid>) {
+    for c in &g.children {
+        match c {
+            Node::Prop(pr) => {
+                if new.get(&pr.match_id).is_some_and(|d| d.value != pr.value) && !out.contains(&pr.uid) {
+                    out.push(pr.uid);
+                }
+            }
+            Node::Group(sg) => match new.sub(&sg.match_id) {
+                Some(d) => effect_changes(sg, d, out),
+                None => out.push(sg.uid),
+            },
+        }
+    }
+}
+
 fn reveal(s: &mut Session, p: &Value) -> Result<Value> {
     let kind = str_p(p, "kind").unwrap_or("keyframes").to_string();
     let cid = super::comp_id(s, p)?;
@@ -389,12 +408,30 @@ fn reveal(s: &mut Session, p: &Value) -> Result<Value> {
                     found.push(pr.uid);
                 }
             });
-            // Modified: effects and masks count as modifications in full (AE reveals them).
-            if kind == "modified" && matches!(top.match_id.as_str(), "effects" | "masks") {
+            // Modified: masks count as modifications in full (AE reveals them).
+            if kind == "modified" && top.match_id == "masks" {
                 for g in top.groups() {
                     found.push(g.uid);
                 }
             }
+            // Modified: an effect's parameters that differ from a new instance's.
+            if kind == "modified" && top.match_id == "effects" {
+                let size = effectcraft_render::effect_bounds(&s.project, comp, l).0;
+                for g in top.groups() {
+                    let spec = match &g.kind {
+                        GroupKind::Effect { effect } => effectcraft_effects::find(effect),
+                        _ => None,
+                    };
+                    // A missing plug-in's defaults aren't known: the effect shows in full.
+                    let Some(spec) = spec else {
+                        found.push(g.uid);
+                        continue;
+                    };
+                    let mut next = 0;
+                    let new = effectcraft_effects::instantiate(spec, &mut Ids(&mut next), "", size);
+                    effect_changes(g, &new, &mut found);
+                }
+            }
             if kind == "modified" && top.match_id == "text" {
                 for g in top.group("animators").into_iter().flat_map(|a| a.groups()) {
                     found.push(g.uid);

crates/engine/src/tests_timeline.rs

@@ -866,6 +866,27 @@ fn modified_reveal_takes_in_options_and_layer_styles() {
     assert_eq!(modified(&mut s, light), ["/lightOptions/intensity"]);
 }
 
+/// UU shows the parameters of an effect that differ from a new instance's, not the whole effect:
+/// a just-applied effect shows nothing, and a reset parameter goes again.
+#[test]
+fn modified_reveal_shows_changed_effect_parameters_only() {
+    let (mut s, l) = setup();
+    s.execute("layer.select", json!({"layers": [l]})).unwrap();
+    s.execute("effect.apply", json!({"effect": "ec.blur.gaussian"})).unwrap();
+    let modified = |s: &mut Session| -> Vec<String> {
+        let r = s.execute("anim.reveal", json!({"kind": "modified"})).unwrap();
+        r["props"].as_array().unwrap().iter().filter_map(|p| p["prop"].as_u64()).map(|u| path_of(s, l, u)).collect()
+    };
+    assert!(modified(&mut s).is_empty(), "{:?}", modified(&mut s));
+    s.execute("prop.set", json!({"layer": l, "path": "effects/#1/blurriness", "value": 12})).unwrap();
+    assert_eq!(modified(&mut s), ["/effects/ec.blur.gaussian/blurriness"]);
+    // Keyframed at its default value: animated, so still shown.
+    s.execute("prop.set", json!({"layer": l, "path": "effects/#1/blurriness", "value": 0})).unwrap();
+    assert!(modified(&mut s).is_empty(), "{:?}", modified(&mut s));
+    s.execute("prop.toggleAnimation", json!({"layer": l, "path": "effects/#1/blurriness"})).unwrap();
+    assert_eq!(modified(&mut s), ["/effects/ec.blur.gaussian/blurriness"]);
+}
+
 /// Enable Time Remapping on a layer extended past its source's end (its last frame frozen) puts
 /// the end key at the source's end, not at the extended out point (#290).
 #[test]

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

@@ -1030,7 +1030,7 @@ fn reveal_rows(app: &EffectcraftApp, l: &Layer, kinds: &[String], rows: &mut Vec
         }
         wanted.extend(picked.iter().copied());
     }
-    revealed_rows(rows, l, &wanted, &shown_groups, &tl.open_groups);
+    revealed_rows(rows, l, &wanted, &shown_groups, &tl.open_groups, tl.full_groups.get(&l.id.0));
     if has("waveform")
         && let Some(item) = super::waveform::audio_item(&app.session.project, l)
     {
@@ -1041,13 +1041,22 @@ fn reveal_rows(app: &EffectcraftApp, l: &Layer, kinds: &[String], rows: &mut Vec
 /// 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>) {
+/// on their own. Groups in `skip` already show with their contents; those in `full` (twirled
+/// open by hand since the reveal) show all of theirs.
+fn revealed_rows(
+    rows: &mut Vec<Row>,
+    l: &Layer,
+    shown: &std::collections::BTreeSet<u64>,
+    skip: &[u64],
+    open: &std::collections::BTreeSet<u64>,
+    full: Option<&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>,
+        full: Option<&'a std::collections::BTreeSet<u64>>,
     }
     fn holds(g: &PropGroup, wanted: &dyn Fn(&Property) -> bool) -> bool {
         g.children.iter().any(|c| match c {
@@ -1066,6 +1075,10 @@ fn revealed_rows(rows: &mut Vec<Row>, l: &Layer, shown: &std::collections::BTree
         if !o {
             return;
         }
+        if t.full.is_some_and(|f| f.contains(&g.uid)) {
+            push_group(rows, t.l, g, depth + 1, t.open);
+            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 } }),
@@ -1075,7 +1088,7 @@ fn revealed_rows(rows: &mut Vec<Row>, l: &Layer, shown: &std::collections::BTree
         }
     }
     let wanted = |p: &Property| shown.contains(&p.uid) && prop_visible(p, l);
-    let tree = Tree { l, wanted: &wanted, skip, open };
+    let tree = Tree { l, wanted: &wanted, skip, open, full };
     for c in &l.props.children {
         match c {
             Node::Group(fx) if fx.match_id == "effects" => {
@@ -3350,6 +3363,7 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) {
                 // Twirled by hand, the layer shows its whole tree again.
                 tl.reveal.clear();
                 tl.layer_reveal.remove(&id);
+                tl.full_groups.remove(&id);
                 let open = !tl.open_layers.contains(&id);
                 if open {
                     tl.open_layers.insert(id);
@@ -3370,10 +3384,8 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) {
             }
             UiAct::ToggleGroup(layer, uid, all) => {
                 let tl = &mut app.ui.timeline;
-                let open = !tl.open_groups.remove(&uid);
-                if open {
-                    tl.open_groups.insert(uid);
-                }
+                tl.toggle_group(layer, uid);
+                let open = tl.open_groups.contains(&uid);
                 if all && let Some(g) = comp.layer(LayerId(layer)).and_then(|l| find_group(&l.props, uid)) {
                     let mut uids = vec![];
                     walk_groups(g, &mut uids);
@@ -4338,6 +4350,8 @@ mod tests {
         let n_keys = app.ui.timeline.reveal_props.len();
         assert_eq!(app.ui.timeline.reveal, vec!["props"]);
         assert!(n_keys >= 1);
+        let bx = app.session.state.selected_layers[0].0;
+        app.session.execute("prop.set", json!({"layer": bx, "path": "effects/#1/blurriness", "value": 12})).unwrap();
         crate::menus::invoke(&mut app, &ctx, "anim.reveal", json!({"kind": "keyframes"})).unwrap();
         assert!(app.ui.timeline.reveal_props.len() > n_keys, "UU reveals modified properties too");
     }
@@ -4376,9 +4390,24 @@ mod tests {
         let at = |n: &str| rows.iter().position(|r| r == n).unwrap();
         assert_eq!(rows[at("Plate") + 1].trim(), "Scale", "{rows:?}");
         assert_eq!(rows[at("Box") + 1].trim(), "Position", "{rows:?}");
-        // UU (modified), then U later: Box shows its keyframed properties only again.
+        // UU (modified): the effect's changed parameter only, under the effect twirled open.
+        // Then U later: Box shows its keyframed properties only again.
+        app.session.execute("prop.set", json!({"layer": bx, "path": "effects/#1/blurriness", "value": 12})).unwrap();
         crate::menus::invoke(&mut app, &ctx, "anim.reveal", json!({"kind": "keyframes"})).unwrap();
-        assert!(labels(&app).iter().any(|r| r.trim() == ">Gaussian Blur"), "UU shows the effect");
+        let rows = labels(&app);
+        let fx = rows.iter().position(|r| r.trim().ends_with("Gaussian Blur")).unwrap_or_else(|| panic!("UU shows the effect: {rows:?}"));
+        assert_eq!(rows[fx + 1].trim(), "Blurriness", "{rows:?}");
+        assert!(!rows.iter().any(|r| r.trim() == "Blur Dimensions"), "unchanged parameters stay hidden: {rows:?}");
+        // Collapsed and expanded by hand, the effect shows all its parameters; UU filters again.
+        let fx_uid = app.session.active_comp().unwrap().layer(LayerId(bx)).unwrap().effects().unwrap().groups().next().unwrap().uid;
+        app.ui.timeline.toggle_group(bx, fx_uid);
+        assert!(!labels(&app).iter().any(|r| r.trim() == "Blurriness"), "collapsed");
+        app.ui.timeline.toggle_group(bx, fx_uid);
+        let rows = labels(&app);
+        assert!(rows.iter().any(|r| r.trim() == "Blurriness") && rows.iter().any(|r| r.trim() == "Blur Dimensions"), "{rows:?}");
+        app.last_reveal = Some(("keyframes".into(), f64::MAX));
+        crate::menus::invoke(&mut app, &ctx, "anim.reveal", json!({"kind": "keyframes"})).unwrap();
+        assert!(!labels(&app).iter().any(|r| r.trim() == "Blur Dimensions"), "UU again shows the changed ones only");
         app.last_reveal = Some(("keyframes".into(), -10.0));
         crate::menus::invoke(&mut app, &ctx, "anim.reveal", json!({"kind": "keyframes"})).unwrap();
         app.last_reveal = Some(("keyframes".into(), -10.0));

crates/ui-egui/src/state.rs

@@ -310,6 +310,7 @@ impl TimelineState {
     /// reveal. Other layers keep theirs.
     pub fn apply_reveal(&mut self, layers: &[u64], kinds: Vec<String>) {
         for id in layers {
+            self.full_groups.remove(id);
             if kinds.is_empty() {
                 self.open_layers.remove(id);
                 self.layer_reveal.remove(id);
@@ -320,6 +321,18 @@ impl TimelineState {
         }
         self.reveal = kinds;
     }
+
+    /// Twirl group `uid` of `layer` by hand. Opened under a reveal, it shows all its properties
+    /// (collapse and expand an effect that shows only its revealed parameters to see them all).
+    pub fn toggle_group(&mut self, layer: u64, uid: u64) {
+        if self.open_groups.remove(&uid) {
+            return;
+        }
+        self.open_groups.insert(uid);
+        if self.layer_reveal.contains_key(&layer) {
+            self.full_groups.entry(layer).or_default().insert(uid);
+        }
+    }
 }
 
 /// Where a Timeline shortcut scrolls to.
@@ -361,6 +374,10 @@ pub struct TimelineState {
     /// Properties / groups (uids) shown by the `props` reveal (Animation ▸ Reveal Properties…).
     #[serde(default)]
     pub reveal_props: BTreeSet<u64>,
+    /// Groups of a revealed layer (by layer id) twirled open by hand since the reveal: they show
+    /// their whole contents, not just the revealed properties.
+    #[serde(default)]
+    pub full_groups: BTreeMap<u64, BTreeSet<u64>>,
     /// Graph Editor: `auto` (Auto-Select Graph Type, the default: the speed graph when every
     /// property shown is spatial, else the value graph), `value` or `speed`.
     #[serde(default = "auto_graph")]
@@ -435,6 +452,7 @@ impl Default for TimelineState {
             reveal: vec![],
             layer_reveal: BTreeMap::new(),
             reveal_props: BTreeSet::new(),
+            full_groups: BTreeMap::new(),
             graph_mode: auto_graph(),
             graph_show_selected: true,
             graph_auto_zoom: true,