From f3a9afda1475d0b3df893278913226acf11cb8a7 Mon Sep 17 00:00:00 2001 From: vangel76 Date: Sat, 7 Nov 2026 15:14:09 +0100 Subject: [PATCH 1/5] Effect Controls: lock the panel to the layer it shows (#3193) The Effect Controls tab gets the lock icon the Composition viewers and the Timeline have (panel.tab.EffectControls.lock). Locked, the panel and its tab name keep the layer they showed while that layer's comp is active, whatever is selected next, so a pick whip from another layer's property can reach its parameters. An effect dropped on the locked panel goes to its layer and leaves the selection alone; one applied to another layer says the panel is locked. Deleting the layer or its comp, or replacing the project, unlocks the panel, which follows the selection again. The lock is UI state (UiState::effect_controls_lock) and agents set it with ui.set {effectControlsLocked}. Fixes #3193 --- crates/ui-egui/src/app/frame.rs | 1 + crates/ui-egui/src/control.rs | 6 + crates/ui-egui/src/dock.rs | 4 +- crates/ui-egui/src/dock_ui.rs | 30 +++- crates/ui-egui/src/panels/effect_controls.rs | 11 +- .../src/panels/effect_controls/common.rs | 74 ++++++++- .../src/panels/effect_controls/panel.rs | 9 +- crates/ui-egui/src/state.rs | 21 +++ crates/ui-egui/tests/it/ui_effect_controls.rs | 155 ++++++++++++++++++ docs/control-protocol.md | 2 +- 10 files changed, 296 insertions(+), 17 deletions(-) diff --git a/crates/ui-egui/src/app/frame.rs b/crates/ui-egui/src/app/frame.rs index e77f07840..56794cfe4 100644 --- a/crates/ui-egui/src/app/frame.rs +++ b/crates/ui-egui/src/app/frame.rs @@ -421,6 +421,7 @@ impl EffectcraftApp { crate::color_picker::note_dialog(&ctx, self.dialog); // After the shortcuts, menus and header, which can replace the project (New, Open). self.ui.timeline.scope_links(self.session.script_ui.project_epoch); + panels::effect_controls::scope_lock(self); self.dock_area(ui, body); #[cfg(not(target_arch = "wasm32"))] prefs_live::dropped_files(self, ui, full); diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index 86bd20c93..f6c255d69 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -281,6 +281,12 @@ fn set_ui(app: &mut EffectcraftApp, ctx: &egui::Context, p: &Value) -> Outcome { { return err(e); } + // Effect Controls' tab lock: on locks it to the layer it shows. + if let Some(on) = p.get("effectControlsLocked").and_then(Value::as_bool) + && crate::panels::effect_controls::set_lock(app, on) != on + { + return err("effectControlsLocked: Effect Controls shows no layer to lock"); + } set_ui_layout(app, p) } diff --git a/crates/ui-egui/src/dock.rs b/crates/ui-egui/src/dock.rs index c8ada9bf8..5f03af14c 100644 --- a/crates/ui-egui/src/dock.rs +++ b/crates/ui-egui/src/dock.rs @@ -676,8 +676,8 @@ pub fn doc_tabs( out } -/// A viewer lock button (Composition viewers and the Timeline keep their comp while locked): a -/// closed padlock while locked, an open one while unlocked. +/// A panel lock button (Composition viewers and the Timeline keep their comp while locked, +/// Effect Controls its layer): a closed padlock while locked, an open one while unlocked. /// Returns true when clicked. Automation: `panel.tab..lock`. pub fn lock_button(ui: &mut egui::Ui, rect: Rect, panel: PanelKind, locked: bool, t: &Tokens, reg: &mut crate::automation::Registry) -> bool { let resp = ui.interact(rect, egui::Id::unique(("tab-lock", panel.id())), Sense::click()); diff --git a/crates/ui-egui/src/dock_ui.rs b/crates/ui-egui/src/dock_ui.rs index 6523ff757..1fbaa6f83 100644 --- a/crates/ui-egui/src/dock_ui.rs +++ b/crates/ui-egui/src/dock_ui.rs @@ -333,8 +333,11 @@ impl EffectcraftApp { } } let Some(comp) = self.session.active_comp() else { return out }; - if let Some(l) = self.session.state.selected_layers.first().and_then(|id| comp.layer(*id)) { + // Effect Controls names the layer it shows (the one it is locked to, if any). + if let Some(l) = panels::effect_controls::shown_layer_id(self).and_then(|id| comp.layer(id)) { out.push((PanelKind::EffectControls, format!("{} {}", crate::i18n::command_label("window.panel", "Effect Controls"), l.name))); + } + if let Some(l) = self.session.state.selected_layers.first().and_then(|id| comp.layer(*id)) { out.push((PanelKind::Properties, format!("{}: {}", crate::i18n::command_label("window.panel", "Properties"), l.name))); } out @@ -492,7 +495,13 @@ impl EffectcraftApp { } } for p in locks { - if !self.ui.locked_tabs.remove(&p.id()) { + if p == PanelKind::EffectControls { + // Locks on the layer the panel shows (nothing to lock without one). + let on = self.ui.effect_controls_lock.is_none(); + if panels::effect_controls::set_lock(self, on) != on { + self.ui.status = crate::i18n::tr("Select a layer to lock Effect Controls").to_string(); + } + } else if !self.ui.locked_tabs.remove(&p.id()) { self.ui.locked_tabs.insert(p.id().to_string()); } } @@ -638,10 +647,19 @@ fn dock_style(ui: &egui::Ui, t: &Tokens) -> DockStyle { style } -/// Panels with a viewer lock: the Composition viewers and the Timeline keep their comp while -/// locked. +/// Panels with a lock: the Composition viewers and the Timeline keep their comp while locked, +/// Effect Controls its layer. fn has_lock(panel: PanelKind) -> bool { - matches!(panel, PanelKind::Composition | PanelKind::Viewer(_) | PanelKind::Timeline) + matches!(panel, PanelKind::Composition | PanelKind::Viewer(_) | PanelKind::Timeline | PanelKind::EffectControls) +} + +/// Whether `panel`'s tab lock is on. Effect Controls keeps the layer it is locked to +/// (`UiState::effect_controls_lock`), the others are in `UiState::locked_tabs`. +fn is_locked(app: &EffectcraftApp, panel: PanelKind) -> bool { + match panel { + PanelKind::EffectControls => app.ui.effect_controls_lock.is_some(), + _ => app.ui.locked_tabs.contains(&panel.id()), + } } /// EffectCraft's side of the dock area for one frame. @@ -865,7 +883,7 @@ impl DockContent for EffectContent<'_> { let menu = Rect::from_min_max(pos2((header.max.x - MENU_W).max(header.min.x), header.min.y), header.max); if has_lock(*active) { let lock = Rect::from_center_size(pos2(menu.min.x - LOCK_W / 2.0, header.center().y), vec2(14.0, 14.0)); - let locked = self.app.ui.locked_tabs.contains(&active.id()); + let locked = is_locked(self.app, *active); if dock::lock_button(ui, lock, *active, locked, &t, &mut self.app.auto) { self.locks.push(*active); } diff --git a/crates/ui-egui/src/panels/effect_controls.rs b/crates/ui-egui/src/panels/effect_controls.rs index d9706f147..83f502988 100644 --- a/crates/ui-egui/src/panels/effect_controls.rs +++ b/crates/ui-egui/src/panels/effect_controls.rs @@ -1,6 +1,13 @@ //! Effect Controls: the selected layer's effects with all parameters, in After Effects' //! conventions: //! +//! - the tab's lock icon keeps the panel on the layer it shows whatever is selected next (a +//! pick whip from another layer's property can then reach its parameters); deleting that +//! layer or replacing the project unlocks it (see [`set_lock`]). The lock holds while the +//! locked layer's comp is the active comp: with another comp active the panel shows that +//! comp's selection (its edits address layers of the active comp), and the locked layer +//! again when its comp is back. The Effect menu of a right-click on empty space applies to +//! the selected layers, as in After Effects; //! - effect headers with the fx switch, twirl, Reset and About…; click selects the effect //! (Edit ▸ Copy / Paste / Duplicate and Delete then act on it), drag reorders the stack, the //! context menu copies, pastes, duplicates, resets and removes; @@ -34,8 +41,8 @@ mod panel; mod rows; mod viewer; -pub(crate) use common::gesture_key; -pub use common::{choose_files_after_apply, reveal_applied}; +pub use common::{choose_files_after_apply, reveal_applied, set_lock}; +pub(crate) use common::{gesture_key, scope_lock, shown_layer_id}; pub use panel::{show, start_rename}; pub(crate) use rows::toggle_pick; pub use viewer::viewer_hook; diff --git a/crates/ui-egui/src/panels/effect_controls/common.rs b/crates/ui-egui/src/panels/effect_controls/common.rs index ab46a07ac..49bb514dd 100644 --- a/crates/ui-egui/src/panels/effect_controls/common.rs +++ b/crates/ui-egui/src/panels/effect_controls/common.rs @@ -27,10 +27,66 @@ pub(super) fn effect_active(g: &PropGroup, layer: effectcraft_engine::project::L selected.iter().any(|(l, uid)| *l == layer && contains(g, *uid, 0)) } -pub(super) fn selected_layer(app: &EffectcraftApp) -> Option { +/// The layer Effect Controls shows: the layer it is locked to (its tab's lock) while that layer's +/// comp is the active comp, else the first selected layer of the active comp. +pub(crate) fn shown_layer_id(app: &EffectcraftApp) -> Option { let comp = app.session.active_comp()?; - let id = app.session.state.selected_layers.first()?; - comp.layer(*id).cloned() + if let Some(lock) = app.ui.effect_controls_lock + && app.session.active_comp_id().is_some_and(|c| c.0 == lock.comp) + && comp.layer(effectcraft_engine::project::LayerId(lock.layer)).is_some() + { + return Some(effectcraft_engine::project::LayerId(lock.layer)); + } + app.session.state.selected_layers.first().copied().filter(|id| comp.layer(*id).is_some()) +} + +pub(super) fn shown_layer(app: &EffectcraftApp) -> Option { + let id = shown_layer_id(app)?; + app.session.active_comp()?.layer(id).cloned() +} + +/// Lock Effect Controls on the layer it shows (`on`), or unlock it. Locking with no layer shown +/// does nothing; locking while already locked keeps the layer it is locked to. Returns whether +/// the panel is locked afterwards. The tab's lock icon and `ui.set {effectControlsLocked}`. +pub fn set_lock(app: &mut EffectcraftApp, on: bool) -> bool { + if !on { + app.ui.effect_controls_lock = None; + return false; + } + if app.ui.effect_controls_lock.is_some() { + return true; + } + let Some(comp) = app.session.active_comp_id() else { return false }; + let Some(layer) = shown_layer_id(app) else { return false }; + app.ui.effect_controls_lock = Some(crate::state::EffectControlsLock { comp: comp.0, layer: layer.0 }); + app.ui.effect_controls_lock_epoch = app.session.script_ui.project_epoch; + true +} + +/// Once a frame: a lock made in another project (New, Open: the ids start over) or on a layer or +/// comp since deleted is dropped, whether or not Effect Controls is on screen. +pub(crate) fn scope_lock(app: &mut EffectcraftApp) { + let epoch = app.session.script_ui.project_epoch; + if app.ui.effect_controls_lock_epoch != epoch { + app.ui.effect_controls_lock_epoch = epoch; + app.ui.effect_controls_lock = None; + } + prune_lock(app); +} + +/// A lock whose layer or comp no longer exists (deleted, or its comp deleted) is dropped, so the +/// panel follows the selection again and its tab shows it unlocked. Another comp being active +/// keeps the lock: the panel shows the locked layer again when its comp comes back. +pub(super) fn prune_lock(app: &mut EffectcraftApp) { + let Some(lock) = app.ui.effect_controls_lock else { return }; + let alive = app + .session + .project + .comp(effectcraft_engine::project::ItemId(lock.comp)) + .is_some_and(|c| c.layer(effectcraft_engine::project::LayerId(lock.layer)).is_some()); + if !alive { + app.ui.effect_controls_lock = None; + } } /// An effect was just applied (`effect.apply` / `effect.applyLast` with `params`): Effect @@ -39,10 +95,14 @@ pub(super) fn selected_layer(app: &EffectcraftApp) -> Option { pub fn reveal_applied(app: &mut EffectcraftApp, params: &serde_json::Value) { // `effect.apply` selects the new effect on (the last of) the layers it went to. let target = app.session.state.selected_props.last().map(|(l, _)| *l); + // Effect Controls locked to a layer of the active comp keeps showing that layer. + let locked = app.ui.effect_controls_lock.filter(|k| app.session.active_comp_id().is_some_and(|c| c.0 == k.comp)); if let Some(l) = target && !app.session.state.selected_layers.contains(&l) && params.get("comp").is_none() && app.session.active_comp().is_some_and(|c| c.layer(l).is_some()) + // Dropped on the locked panel: it shows that layer already, the selection stays. + && locked.is_none_or(|k| k.layer != l.0) { // Show that layer (dropped on an unselected one), keeping the new effect selected. let props = app.session.state.selected_props.clone(); @@ -51,6 +111,14 @@ pub fn reveal_applied(app: &mut EffectcraftApp, params: &serde_json::Value) { Err(e) => app.ui.status = e.to_string(), } } + // The effect went to another layer than the one the locked panel shows: say why it isn't + // there (After Effects opens another Effect Controls tab). + if let (Some(k), Some(l)) = (locked, target) + && k.layer != l.0 + && let Some(name) = app.session.active_comp().and_then(|c| c.layer(effectcraft_engine::project::LayerId(k.layer))).map(|x| x.name.clone()) + { + app.ui.status = crate::i18n::tr_args("Effect Controls is locked to {}", &[&name]); + } app.raise_panel(crate::dock::PanelKind::EffectControls); } diff --git a/crates/ui-egui/src/panels/effect_controls/panel.rs b/crates/ui-egui/src/panels/effect_controls/panel.rs index 0857dfa3f..95f810d1a 100644 --- a/crates/ui-egui/src/panels/effect_controls/panel.rs +++ b/crates/ui-egui/src/panels/effect_controls/panel.rs @@ -1,4 +1,4 @@ -use super::common::{Actions, PARAM_NAME_X, ROW, effect_active, selected_layer}; +use super::common::{Actions, PARAM_NAME_X, ROW, effect_active, prune_lock, shown_layer}; use super::editors::{curves_editor, editor_height, histogram_only, levels_editor, object_matte_editor, roto_editor, warp_editor}; use super::rows::{FxScope, group_rows, toggle_closed}; use super::{Align2, Color32, EffectcraftApp, EvalCtx, GroupKind, Icon, Layer, PropGroup, Rect, Sense, Stroke, Tokens, json, pos2, vec2, widgets}; @@ -158,7 +158,8 @@ fn about_button(app: &mut EffectcraftApp, ui: &mut egui::Ui, bp: &egui::Painter, } } -/// The Effect Controls panel for the selected layer: a header with the comp and layer names, then +/// The Effect Controls panel for the selected layer (or the layer the panel is locked to, see +/// `common::shown_layer_id`): a header with the comp and layer names, then /// one header per effect (Fx toggle, twirl, rename, Reset, About, label bar, reorder drag and its /// right-click menu) followed by that effect's custom editor and its property rows. Dragging /// effects or presets onto the panel applies them; a right-click on empty space opens the Effect @@ -168,7 +169,9 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { let t = app.tokens; let p = ui.painter().with_clip_rect(rect); let ctx = ui.ctx().clone(); - let Some(layer) = selected_layer(app) else { + // A locked layer that was deleted unlocks the panel, which then follows the selection. + prune_lock(app); + let Some(layer) = shown_layer(app) else { p.text(rect.center(), Align2::CENTER_CENTER, crate::i18n::tr("Select a layer to see its effects"), Tokens::ui(12.0), t.text_faint); return; }; diff --git a/crates/ui-egui/src/state.rs b/crates/ui-egui/src/state.rs index 60a30e97e..34336c297 100644 --- a/crates/ui-egui/src/state.rs +++ b/crates/ui-egui/src/state.rs @@ -594,6 +594,14 @@ impl Default for AnimToolsState { } } +/// The layer a locked Effect Controls panel keeps showing (the tab's lock icon, as After Effects' +/// panel lock): comp item id and layer id. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)] +pub struct EffectControlsLock { + pub comp: u64, + pub layer: u64, +} + #[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub enum ProjectStatus { @@ -651,6 +659,17 @@ pub struct UiState { /// bring a locked panel forward (the tab's lock icon, as After Effects' viewer lock). #[serde(default)] pub locked_tabs: BTreeSet, + /// Effect Controls locked to a layer (its tab's lock icon): the panel keeps showing that + /// layer whatever is selected while its comp is the active comp. Cleared when the layer or + /// its comp is deleted, and when the project is replaced (see + /// `panels::effect_controls::set_lock`). + #[serde(default)] + pub effect_controls_lock: Option, + /// The project the lock was made in (`ScriptUi::project_epoch`): a new or opened project + /// reuses comp and layer ids, so its lock is dropped (see + /// `panels::effect_controls::scope_lock`). + #[serde(skip)] + pub effect_controls_lock_epoch: u64, /// The comp each Composition viewer shows, by viewer id (0 = the Composition panel; see /// `panels::viewers`). #[serde(default)] @@ -773,6 +792,8 @@ impl Default for UiState { native_outer: Vec::new(), maximized: None, locked_tabs: BTreeSet::new(), + effect_controls_lock: None, + effect_controls_lock_epoch: 0, viewers: Default::default(), active_viewer: 0, focused: PanelKind::Composition, diff --git a/crates/ui-egui/tests/it/ui_effect_controls.rs b/crates/ui-egui/tests/it/ui_effect_controls.rs index 1db059220..d72febf01 100644 --- a/crates/ui-egui/tests/it/ui_effect_controls.rs +++ b/crates/ui-egui/tests/it/ui_effect_controls.rs @@ -912,3 +912,158 @@ fn double_click(h: &mut Harness<'_, EffectcraftApp>, pos: egui::Pos2) { } h.run_steps(2); } + +/// Two solids in one comp, `A` with a Point Control and `B` with an Angle Control, `A` selected +/// and Effect Controls open: the app, A, B and their effects. +fn two_layers() -> (EffectcraftApp, u64, u64, u64, u64) { + let mut s = Session::default(); + s.execute("comp.new", json!({"name": "Main", "width": 320, "height": 180, "frameRate": 30, "duration": 5})).unwrap(); + let a = s.execute("layer.newSolid", json!({"name": "A", "color": "#ff0000"})).unwrap()["layer"].as_u64().unwrap(); + let b = s.execute("layer.newSolid", json!({"name": "B", "color": "#0000ff"})).unwrap()["layer"].as_u64().unwrap(); + let fa = s.execute("effect.apply", json!({"layers": [a], "effect": "Point Control"})).unwrap()["effects"][0].as_u64().unwrap(); + let fb = s.execute("effect.apply", json!({"layers": [b], "effect": "Angle Control"})).unwrap()["effects"][0].as_u64().unwrap(); + s.state.selected_layers = vec![LayerId(a)]; + s.state.selected_props.clear(); + let mut app = EffectcraftApp::new(s); + app.show_panel(PanelKind::EffectControls); + (app, a, b, fa, fb) +} + +/// Whether Effect Controls drew effect `fx`'s header in the last frame (not the frame before: +/// a stale id must not pass). +fn shows_effect(h: &Harness<'_, EffectcraftApp>, fx: u64) -> bool { + let id = format!("effectControls.effect.{fx}"); + h.state().auto.elements.iter().any(|e| e.id == id) +} + +fn select_layer(h: &mut Harness<'_, EffectcraftApp>, layer: u64) { + h.state_mut().session.execute("layer.select", json!({"layers": [layer]})).unwrap(); + h.run_steps(3); +} + +/// #3193: the Effect Controls tab's lock keeps the panel (and its tab name) on the layer it showed +/// when locked whatever is selected next, through undo; deleting that layer unlocks it without a +/// crash and the panel follows the selection again. +#[test] +fn locked_effect_controls_keep_their_layer_until_it_is_deleted() { + let (app, a, b, fa, fb) = two_layers(); + let mut h = Harness::builder().with_size(egui::vec2(1600.0, 1000.0)).build_eframe(|_| app); + h.run_steps(4); + assert!(shows_effect(&h, fa) && !shows_effect(&h, fb), "A is shown before locking"); + let comp = h.state().session.active_comp_id().unwrap().0; + let lock = rect_of(&h, "panel.tab.EffectControls.lock").center(); + click(&mut h, lock); + assert_eq!(h.state().ui.effect_controls_lock, Some(effectcraft_ui_egui::state::EffectControlsLock { comp, layer: a })); + // Selecting B keeps A in the panel and its tab. + select_layer(&mut h, b); + assert!(shows_effect(&h, fa) && !shows_effect(&h, fb), "the locked panel keeps A"); + let tab = h.state().auto.elements.iter().find(|e| e.id == "panel.tab.EffectControls").map(|e| e.label.clone()).unwrap_or_default(); + assert!(tab.ends_with(" A"), "the tab names the locked layer: {tab:?}"); + // An edit and its undo leave the lock alone. + h.state_mut().session.execute("prop.set", json!({"layer": b, "path": "transform/opacity", "value": 50.0})).unwrap(); + h.state_mut().session.execute("edit.undo", json!({})).unwrap(); + h.run_steps(3); + assert!(shows_effect(&h, fa) && !shows_effect(&h, fb), "undo keeps the lock"); + // The lock state is UI state: it survives a serde round trip. + let ui = serde_json::to_value(&h.state().ui).unwrap(); + let back: effectcraft_ui_egui::state::UiState = serde_json::from_value(ui).unwrap(); + assert_eq!(back.effect_controls_lock, h.state().ui.effect_controls_lock); + // Unlocking follows the selection again. + let lock = rect_of(&h, "panel.tab.EffectControls.lock").center(); + click(&mut h, lock); + assert_eq!(h.state().ui.effect_controls_lock, None); + assert!(shows_effect(&h, fb) && !shows_effect(&h, fa), "unlocked, the panel shows the selection"); + // Lock on A again, select B, delete A: no crash, the lock is gone, B is shown. + select_layer(&mut h, a); + let lock = rect_of(&h, "panel.tab.EffectControls.lock").center(); + click(&mut h, lock); + assert_eq!(h.state().ui.effect_controls_lock.map(|l| l.layer), Some(a)); + select_layer(&mut h, b); + h.state_mut().session.execute("edit.clear", json!({"layers": [a]})).unwrap(); + h.run_steps(3); + assert_eq!(h.state().ui.effect_controls_lock, None, "deleting the locked layer unlocks the panel"); + assert!(!shows_effect(&h, fa), "A is gone from the panel"); + // Deleting clears the selection; the panel follows the next one. + select_layer(&mut h, b); + assert!(shows_effect(&h, fb), "the panel follows the selection"); + // Undoing the delete brings A back without bringing back (or breaking) the lock. + h.state_mut().session.execute("edit.undo", json!({})).unwrap(); + h.run_steps(3); + assert_eq!(h.state().ui.effect_controls_lock, None); + let selected = h.state().session.state.selected_layers.first().map(|l| l.0); + let (on, off) = if selected == Some(a) { (fa, fb) } else { (fb, fa) }; + assert!(shows_effect(&h, on) && !shows_effect(&h, off), "the panel shows the selection ({selected:?})"); +} + +/// #3193: a new or opened project reuses comp and layer ids, so a lock does not carry into it, +/// even onto a layer with the locked one's ids. +#[test] +fn replacing_the_project_unlocks_effect_controls() { + let (app, a, b, _, _) = two_layers(); + let mut h = Harness::builder().with_size(egui::vec2(1600.0, 1000.0)).build_eframe(|_| app); + h.run_steps(4); + let lock = rect_of(&h, "panel.tab.EffectControls.lock").center(); + click(&mut h, lock); + assert_eq!(h.state().ui.effect_controls_lock.map(|l| l.layer), Some(a)); + select_layer(&mut h, b); + // The same project opened again: every id is alive. + let project = (*h.state().session.project).clone(); + h.state_mut().session.replace_project(project, None); + h.run_steps(3); + assert_eq!(h.state().ui.effect_controls_lock, None, "the lock belongs to the replaced project"); +} + +/// #3193: an effect dropped on the locked panel goes to the locked layer and leaves the selection +/// alone; one applied to another layer says why the panel doesn't show it. +#[test] +fn applying_an_effect_while_locked_keeps_the_selection() { + let (app, a, b, fa, _) = two_layers(); + let mut h = Harness::builder().with_size(egui::vec2(1600.0, 1000.0)).build_eframe(|_| app); + h.run_steps(4); + let lock = rect_of(&h, "panel.tab.EffectControls.lock").center(); + click(&mut h, lock); + select_layer(&mut h, b); + let ctx = h.ctx.clone(); + effectcraft_ui_egui::menus::invoke(h.state_mut(), &ctx, "effect.apply", json!({"effect": "Color Control", "layers": [a]})).unwrap(); + h.run_steps(3); + assert_eq!(h.state().session.state.selected_layers, vec![LayerId(b)], "the selection stays on B"); + assert!(shows_effect(&h, fa), "the panel still shows A"); + assert_eq!(h.state().ui.effect_controls_lock.map(|l| l.layer), Some(a)); + // Applied to the selected B: the panel stays on A and says so. + h.state_mut().ui.status.clear(); + effectcraft_ui_egui::menus::invoke(h.state_mut(), &ctx, "effect.apply", json!({"effect": "Color Control", "layers": [b]})).unwrap(); + h.run_steps(3); + assert!(shows_effect(&h, fa)); + assert_eq!(h.state().ui.status, "Effect Controls is locked to A"); +} + +/// #3193: agents lock Effect Controls with `ui.set {effectControlsLocked}`; with no layer shown +/// there is nothing to lock. +#[test] +fn effect_controls_lock_over_the_control_channel() { + use effectcraft_ui_egui::control::{self, ControlRequest, Outcome}; + fn send(h: &mut Harness<'_, EffectcraftApp>, params: serde_json::Value) -> serde_json::Value { + let ctx = h.ctx.clone(); + let (req, _rx) = ControlRequest::new("ui.set", params); + let now = h.state().now(); + match control::handle(h.state_mut(), &ctx, &req, now) { + Outcome::Done(v) => v, + _ => panic!("ui.set: no immediate reply"), + } + } + let (app, a, b, fa, fb) = two_layers(); + let mut h = Harness::builder().with_size(egui::vec2(1600.0, 1000.0)).build_eframe(|_| app); + h.run_steps(4); + assert_eq!(send(&mut h, json!({"effectControlsLocked": true}))["ok"], true); + assert_eq!(h.state().ui.effect_controls_lock.map(|l| l.layer), Some(a)); + select_layer(&mut h, b); + assert!(shows_effect(&h, fa) && !shows_effect(&h, fb)); + assert_eq!(send(&mut h, json!({"effectControlsLocked": false}))["ok"], true); + h.run_steps(3); + assert!(shows_effect(&h, fb) && !shows_effect(&h, fa)); + // Nothing selected: nothing to lock. + h.state_mut().session.state.selected_layers.clear(); + h.run_steps(2); + assert_eq!(send(&mut h, json!({"effectControlsLocked": true}))["ok"], false); + assert_eq!(h.state().ui.effect_controls_lock, None); +} diff --git a/docs/control-protocol.md b/docs/control-protocol.md index 9f3d69ea5..8af641a3a 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -66,7 +66,7 @@ panel registers an element id; both are enforced by `crates/ui-egui/tests/it/ui_ |---|---|---| | `ui.inspect` | `{}` | Window size, pixels per point, fps, serialized `UiState`, playback, time, active comp, selection, open dialog, last render ms, `keyboardFocus`, `windowFocused`, `workspaceStorageError`. | | `ui.elements` | `{prefix?}` | `[{id, label, rect: [x, y, w, h]}]`: the widgets drawn in the last frame, in points. | -| `ui.set` | `{tool?, workspace?, theme?, focused?, viewer?, timeline?, menuBar?, home?}` | `viewer: {zoom (null = fit), res: Full\|Half\|Third\|Quarter\|Auto, pan: [x,y], grid, rulers, safeMargins, transparencyGrid}`; `timeline: {pps (null = fit), start, graphEditor, showModes, openLayers: [ids], openGroups: [uids]}`. | +| `ui.set` | `{tool?, workspace?, theme?, focused?, viewer?, effectControlsLocked?, timeline?, menuBar?, home?}` | `viewer: {zoom (null = fit), res: Full\|Half\|Third\|Quarter\|Auto, pan: [x,y], grid, rulers, safeMargins, transparencyGrid}`; `effectControlsLocked: true` locks Effect Controls on the layer it shows (an error when it shows none), `false` unlocks it (the tab's lock icon, `panel.tab.EffectControls.lock`; `ui.inspect` reports it as `effect_controls_lock: {comp, layer}`); `timeline: {pps (null = fit), start, graphEditor, showModes, openLayers: [ids], openGroups: [uids]}`. | | `ui.panel.show` / `ui.panel.close` | `{panel}` | Panels: `Project`, `Composition`, `Timeline`, `EffectControls`, `EffectsPresets`, `Info`, `Preview`, `Character`, `Paragraph`, `Align` and others. | | `ui.playback` | `{action: play\|stop\|toggle\|status}` | `{playing, time, audio, levelsDb, peaksDb}` (audio: an audio preview is running; L/R meter levels and held peaks in dBFS) | | `ui.resize` | `{width, height}` | Resizes the window (points). | From 7dabd6e6f16265218fc07248c5c0ef217bf5610e Mon Sep 17 00:00:00 2001 From: vangel76 Date: Sat, 7 Nov 2026 16:24:57 +0100 Subject: [PATCH 2/5] Effect Controls: drop a replaced project's lock before answering a lock request (#3193) A ui.set {effectControlsLocked: true} that ran after the project was replaced but before the next frame reported the old project's lock as success; the next frame then cleared it. set_lock now scopes the lock first. --- .../src/panels/effect_controls/common.rs | 3 +++ crates/ui-egui/tests/it/ui_effect_controls.rs | 26 +++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/crates/ui-egui/src/panels/effect_controls/common.rs b/crates/ui-egui/src/panels/effect_controls/common.rs index 49bb514dd..1e7d7cba1 100644 --- a/crates/ui-egui/src/panels/effect_controls/common.rs +++ b/crates/ui-egui/src/panels/effect_controls/common.rs @@ -49,6 +49,9 @@ pub(super) fn shown_layer(app: &EffectcraftApp) -> Option { /// does nothing; locking while already locked keeps the layer it is locked to. Returns whether /// the panel is locked afterwards. The tab's lock icon and `ui.set {effectControlsLocked}`. pub fn set_lock(app: &mut EffectcraftApp, on: bool) -> bool { + // A lock left from a replaced project is gone before this one is answered (a control + // request can run between the replacement and the next frame). + scope_lock(app); if !on { app.ui.effect_controls_lock = None; return false; diff --git a/crates/ui-egui/tests/it/ui_effect_controls.rs b/crates/ui-egui/tests/it/ui_effect_controls.rs index d72febf01..ce51a2b2f 100644 --- a/crates/ui-egui/tests/it/ui_effect_controls.rs +++ b/crates/ui-egui/tests/it/ui_effect_controls.rs @@ -1013,6 +1013,32 @@ fn replacing_the_project_unlocks_effect_controls() { assert_eq!(h.state().ui.effect_controls_lock, None, "the lock belongs to the replaced project"); } +/// #3193: a lock request right after the project is replaced, before the next frame, is not +/// answered with the old project's lock: the reply matches the lock the panel keeps. +#[test] +fn a_lock_request_after_a_project_replacement_does_not_report_the_old_lock() { + use effectcraft_ui_egui::control::{self, ControlRequest, Outcome}; + fn send(h: &mut Harness<'_, EffectcraftApp>) -> serde_json::Value { + let ctx = h.ctx.clone(); + let (req, _rx) = ControlRequest::new("ui.set", json!({"effectControlsLocked": true})); + let now = h.state().now(); + match control::handle(h.state_mut(), &ctx, &req, now) { + Outcome::Done(v) => v, + _ => panic!("ui.set: no immediate reply"), + } + } + let (app, a, _, _, _) = two_layers(); + let mut h = Harness::builder().with_size(egui::vec2(1600.0, 1000.0)).build_eframe(|_| app); + h.run_steps(4); + assert_eq!(send(&mut h)["ok"], true); + assert_eq!(h.state().ui.effect_controls_lock.map(|l| l.layer), Some(a)); + let project = (*h.state().session.project).clone(); + h.state_mut().session.replace_project(project, None); + let reply = send(&mut h); + h.run_steps(3); + assert_eq!(reply["ok"] == true, h.state().ui.effect_controls_lock.is_some(), "{reply}"); +} + /// #3193: an effect dropped on the locked panel goes to the locked layer and leaves the selection /// alone; one applied to another layer says why the panel doesn't show it. #[test] From 16dccbb8fa8bb0a9e37a99f17109e63d23c99da6 Mon Sep 17 00:00:00 2001 From: vangel76 Date: Sat, 7 Nov 2026 17:37:39 +0100 Subject: [PATCH 3/5] Effect Controls: a lock request with no layer shown is an error (#3193) With the lock kept for a layer of another comp and nothing selected in the active one, ui.set {effectControlsLocked: true} reported success. It now returns the documented error and leaves the kept lock alone. --- crates/ui-egui/src/control.rs | 14 +++++---- crates/ui-egui/tests/it/ui_effect_controls.rs | 30 +++++++++++++++++++ 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index f6c255d69..b9ff46c4b 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -281,11 +281,15 @@ fn set_ui(app: &mut EffectcraftApp, ctx: &egui::Context, p: &Value) -> Outcome { { return err(e); } - // Effect Controls' tab lock: on locks it to the layer it shows. - if let Some(on) = p.get("effectControlsLocked").and_then(Value::as_bool) - && crate::panels::effect_controls::set_lock(app, on) != on - { - return err("effectControlsLocked: Effect Controls shows no layer to lock"); + // Effect Controls' tab lock: on locks it to the layer it shows. With no layer shown (the + // lock kept for a layer of another comp, nothing selected here) a lock request is an error + // and the kept lock stays. + if let Some(on) = p.get("effectControlsLocked").and_then(Value::as_bool) { + crate::panels::effect_controls::scope_lock(app); + let shown = crate::panels::effect_controls::shown_layer_id(app).is_some(); + if (on && !shown) || crate::panels::effect_controls::set_lock(app, on) != on { + return err("effectControlsLocked: Effect Controls shows no layer to lock"); + } } set_ui_layout(app, p) } diff --git a/crates/ui-egui/tests/it/ui_effect_controls.rs b/crates/ui-egui/tests/it/ui_effect_controls.rs index ce51a2b2f..ffafe867d 100644 --- a/crates/ui-egui/tests/it/ui_effect_controls.rs +++ b/crates/ui-egui/tests/it/ui_effect_controls.rs @@ -1013,6 +1013,36 @@ fn replacing_the_project_unlocks_effect_controls() { assert_eq!(h.state().ui.effect_controls_lock, None, "the lock belongs to the replaced project"); } +/// #3193: with the lock kept for a layer of another comp and no layer selected in the active one, +/// Effect Controls shows no layer: a lock request is an error and the kept lock stays. +#[test] +fn a_lock_request_with_no_layer_shown_fails_and_keeps_the_other_comps_lock() { + use effectcraft_ui_egui::control::{self, ControlRequest, Outcome}; + fn send(h: &mut Harness<'_, EffectcraftApp>) -> serde_json::Value { + let ctx = h.ctx.clone(); + let (req, _rx) = ControlRequest::new("ui.set", json!({"effectControlsLocked": true})); + let now = h.state().now(); + match control::handle(h.state_mut(), &ctx, &req, now) { + Outcome::Done(v) => v, + _ => panic!("ui.set: no immediate reply"), + } + } + let (app, a, _, _, _) = two_layers(); + let mut h = Harness::builder().with_size(egui::vec2(1600.0, 1000.0)).build_eframe(|_| app); + h.run_steps(4); + assert_eq!(send(&mut h)["ok"], true); + let kept = h.state().ui.effect_controls_lock; + assert_eq!(kept.map(|l| l.layer), Some(a)); + // Another comp becomes active, nothing selected in it. + h.state_mut().session.execute("comp.new", json!({"name": "Other", "width": 320, "height": 180, "frameRate": 30, "duration": 5})).unwrap(); + h.state_mut().session.state.selected_layers.clear(); + h.run_steps(3); + assert_ne!(h.state().session.active_comp_id().map(|c| c.0), kept.map(|l| l.comp), "another comp is active"); + let reply = send(&mut h); + assert_eq!(reply["ok"], false, "{reply}"); + assert_eq!(h.state().ui.effect_controls_lock, kept, "the other comp's lock stays"); +} + /// #3193: a lock request right after the project is replaced, before the next frame, is not /// answered with the old project's lock: the reply matches the lock the panel keeps. #[test] From d1bdb701b74b5e7b6b764d66607b31af0f1ba109 Mon Sep 17 00:00:00 2001 From: vangel76 Date: Sat, 7 Nov 2026 18:22:03 +0100 Subject: [PATCH 4/5] Effect Controls: a lock request while locked in another comp is an error (#3193) Locked to a layer of another comp, the panel shows the active comp's selection and its tab lock reads locked (a click unlocks). ui.set {effectControlsLocked: true} reported success there without locking the shown layer; it now returns an error and keeps the other comp's lock. --- crates/ui-egui/src/control.rs | 5 +++ crates/ui-egui/tests/it/ui_effect_controls.rs | 31 +++++++++++++++++++ docs/control-protocol.md | 2 +- 3 files changed, 37 insertions(+), 1 deletion(-) diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index b9ff46c4b..15a10380d 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -290,6 +290,11 @@ fn set_ui(app: &mut EffectcraftApp, ctx: &egui::Context, p: &Value) -> Outcome { if (on && !shown) || crate::panels::effect_controls::set_lock(app, on) != on { return err("effectControlsLocked: Effect Controls shows no layer to lock"); } + // Locked to a layer of another comp, the panel shows this comp's selection: the tab's + // lock reads locked and a click unlocks, so a lock request does not lock what is shown. + if on && app.ui.effect_controls_lock.is_some_and(|k| app.session.active_comp_id().is_none_or(|c| c.0 != k.comp)) { + return err("effectControlsLocked: Effect Controls is locked to a layer of another composition; unlock it first"); + } } set_ui_layout(app, p) } diff --git a/crates/ui-egui/tests/it/ui_effect_controls.rs b/crates/ui-egui/tests/it/ui_effect_controls.rs index ffafe867d..ec7d5683d 100644 --- a/crates/ui-egui/tests/it/ui_effect_controls.rs +++ b/crates/ui-egui/tests/it/ui_effect_controls.rs @@ -1043,6 +1043,37 @@ fn a_lock_request_with_no_layer_shown_fails_and_keeps_the_other_comps_lock() { assert_eq!(h.state().ui.effect_controls_lock, kept, "the other comp's lock stays"); } +/// #3193: locked to a layer of another comp, the panel shows this comp's selection; a lock request +/// does not claim to lock that layer: it is an error and the other comp's lock stays. +#[test] +fn a_lock_request_while_locked_in_another_comp_fails() { + use effectcraft_ui_egui::control::{self, ControlRequest, Outcome}; + fn send(h: &mut Harness<'_, EffectcraftApp>) -> serde_json::Value { + let ctx = h.ctx.clone(); + let (req, _rx) = ControlRequest::new("ui.set", json!({"effectControlsLocked": true})); + let now = h.state().now(); + match control::handle(h.state_mut(), &ctx, &req, now) { + Outcome::Done(v) => v, + _ => panic!("ui.set: no immediate reply"), + } + } + let (app, a, _, _, _) = two_layers(); + let mut h = Harness::builder().with_size(egui::vec2(1600.0, 1000.0)).build_eframe(|_| app); + h.run_steps(4); + assert_eq!(send(&mut h)["ok"], true); + let kept = h.state().ui.effect_controls_lock; + assert_eq!(kept.map(|l| l.layer), Some(a)); + // Another comp with a selected layer becomes active: the panel shows that layer. + h.state_mut().session.execute("comp.new", json!({"name": "Other", "width": 320, "height": 180, "frameRate": 30, "duration": 5})).unwrap(); + let c = h.state_mut().session.execute("layer.newSolid", json!({"name": "C", "color": "#00ff00"})).unwrap()["layer"].as_u64().unwrap(); + h.state_mut().session.state.selected_layers = vec![LayerId(c)]; + h.run_steps(3); + assert_ne!(h.state().session.active_comp_id().map(|x| x.0), kept.map(|l| l.comp), "another comp is active"); + let reply = send(&mut h); + assert_eq!(reply["ok"], false, "{reply}"); + assert_eq!(h.state().ui.effect_controls_lock, kept, "the other comp's lock stays"); +} + /// #3193: a lock request right after the project is replaced, before the next frame, is not /// answered with the old project's lock: the reply matches the lock the panel keeps. #[test] diff --git a/docs/control-protocol.md b/docs/control-protocol.md index 8af641a3a..126b8fd02 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -66,7 +66,7 @@ panel registers an element id; both are enforced by `crates/ui-egui/tests/it/ui_ |---|---|---| | `ui.inspect` | `{}` | Window size, pixels per point, fps, serialized `UiState`, playback, time, active comp, selection, open dialog, last render ms, `keyboardFocus`, `windowFocused`, `workspaceStorageError`. | | `ui.elements` | `{prefix?}` | `[{id, label, rect: [x, y, w, h]}]`: the widgets drawn in the last frame, in points. | -| `ui.set` | `{tool?, workspace?, theme?, focused?, viewer?, effectControlsLocked?, timeline?, menuBar?, home?}` | `viewer: {zoom (null = fit), res: Full\|Half\|Third\|Quarter\|Auto, pan: [x,y], grid, rulers, safeMargins, transparencyGrid}`; `effectControlsLocked: true` locks Effect Controls on the layer it shows (an error when it shows none), `false` unlocks it (the tab's lock icon, `panel.tab.EffectControls.lock`; `ui.inspect` reports it as `effect_controls_lock: {comp, layer}`); `timeline: {pps (null = fit), start, graphEditor, showModes, openLayers: [ids], openGroups: [uids]}`. | +| `ui.set` | `{tool?, workspace?, theme?, focused?, viewer?, effectControlsLocked?, timeline?, menuBar?, home?}` | `viewer: {zoom (null = fit), res: Full\|Half\|Third\|Quarter\|Auto, pan: [x,y], grid, rulers, safeMargins, transparencyGrid}`; `effectControlsLocked: true` locks Effect Controls on the layer it shows (an error when it shows none, or while it is locked to a layer of another composition), `false` unlocks it (the tab's lock icon, `panel.tab.EffectControls.lock`; `ui.inspect` reports it as `effect_controls_lock: {comp, layer}`); `timeline: {pps (null = fit), start, graphEditor, showModes, openLayers: [ids], openGroups: [uids]}`. | | `ui.panel.show` / `ui.panel.close` | `{panel}` | Panels: `Project`, `Composition`, `Timeline`, `EffectControls`, `EffectsPresets`, `Info`, `Preview`, `Character`, `Paragraph`, `Align` and others. | | `ui.playback` | `{action: play\|stop\|toggle\|status}` | `{playing, time, audio, levelsDb, peaksDb}` (audio: an audio preview is running; L/R meter levels and held peaks in dBFS) | | `ui.resize` | `{width, height}` | Resizes the window (points). | From 12264b4e6380bb9104f03bd3a9105c03911f1316 Mon Sep 17 00:00:00 2001 From: vangel76 Date: Sat, 7 Nov 2026 19:47:55 +0100 Subject: [PATCH 5/5] Doc comments for the functions this PR touches --- crates/ui-egui/src/panels/effect_controls/common.rs | 1 + crates/ui-egui/tests/it/ui_effect_controls.rs | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/crates/ui-egui/src/panels/effect_controls/common.rs b/crates/ui-egui/src/panels/effect_controls/common.rs index 1e7d7cba1..31e884b98 100644 --- a/crates/ui-egui/src/panels/effect_controls/common.rs +++ b/crates/ui-egui/src/panels/effect_controls/common.rs @@ -40,6 +40,7 @@ pub(crate) fn shown_layer_id(app: &EffectcraftApp) -> Option Option { let id = shown_layer_id(app)?; app.session.active_comp()?.layer(id).cloned() diff --git a/crates/ui-egui/tests/it/ui_effect_controls.rs b/crates/ui-egui/tests/it/ui_effect_controls.rs index ec7d5683d..b79aade0d 100644 --- a/crates/ui-egui/tests/it/ui_effect_controls.rs +++ b/crates/ui-egui/tests/it/ui_effect_controls.rs @@ -936,6 +936,7 @@ fn shows_effect(h: &Harness<'_, EffectcraftApp>, fx: u64) -> bool { h.state().auto.elements.iter().any(|e| e.id == id) } +/// Selects `layer` through the `layer.select` command and runs a few frames. fn select_layer(h: &mut Harness<'_, EffectcraftApp>, layer: u64) { h.state_mut().session.execute("layer.select", json!({"layers": [layer]})).unwrap(); h.run_steps(3); @@ -1018,6 +1019,7 @@ fn replacing_the_project_unlocks_effect_controls() { #[test] fn a_lock_request_with_no_layer_shown_fails_and_keeps_the_other_comps_lock() { use effectcraft_ui_egui::control::{self, ControlRequest, Outcome}; + /// Sends a `ui.set` request that locks Effect Controls and returns the immediate reply. fn send(h: &mut Harness<'_, EffectcraftApp>) -> serde_json::Value { let ctx = h.ctx.clone(); let (req, _rx) = ControlRequest::new("ui.set", json!({"effectControlsLocked": true})); @@ -1048,6 +1050,7 @@ fn a_lock_request_with_no_layer_shown_fails_and_keeps_the_other_comps_lock() { #[test] fn a_lock_request_while_locked_in_another_comp_fails() { use effectcraft_ui_egui::control::{self, ControlRequest, Outcome}; + /// Sends a `ui.set` request that locks Effect Controls and returns the immediate reply. fn send(h: &mut Harness<'_, EffectcraftApp>) -> serde_json::Value { let ctx = h.ctx.clone(); let (req, _rx) = ControlRequest::new("ui.set", json!({"effectControlsLocked": true})); @@ -1079,6 +1082,7 @@ fn a_lock_request_while_locked_in_another_comp_fails() { #[test] fn a_lock_request_after_a_project_replacement_does_not_report_the_old_lock() { use effectcraft_ui_egui::control::{self, ControlRequest, Outcome}; + /// Sends a `ui.set` request that locks Effect Controls and returns the immediate reply. fn send(h: &mut Harness<'_, EffectcraftApp>) -> serde_json::Value { let ctx = h.ctx.clone(); let (req, _rx) = ControlRequest::new("ui.set", json!({"effectControlsLocked": true})); @@ -1129,6 +1133,7 @@ fn applying_an_effect_while_locked_keeps_the_selection() { #[test] fn effect_controls_lock_over_the_control_channel() { use effectcraft_ui_egui::control::{self, ControlRequest, Outcome}; + /// Sends a `ui.set` request with `params` and returns the immediate reply. fn send(h: &mut Harness<'_, EffectcraftApp>, params: serde_json::Value) -> serde_json::Value { let ctx = h.ctx.clone(); let (req, _rx) = ControlRequest::new("ui.set", params);