From 189f31d6d1aab25a495c54afd646fb13e6e24e73 Mon Sep 17 00:00:00 2001 From: pravdomil <2387356+pravdomil@users.noreply.github.com> Date: Tue, 13 Oct 2026 14:52:17 +0200 Subject: [PATCH 1/2] Shortcuts: panel and context scopes, one chord per scope A shortcut belongs to the application, a panel or a context (editing text, a paint or camera tool active, a mask selected). The narrowest active scope runs a chord: an active context, the focused panel, the panel sharing its shortcuts (Timeline and Composition), the application. A context's shortcuts never run outside it. - shortcuts.rs: scope table for engine commands and menu entries, scope_rank, scope_label; UiCommand can state its scope. - Conflicts: the same chord in two scopes is an override (both keep working), reported with overrides: true; shortcuts.conflicts lists same-scope pairs only. - Scoped now, keys unchanged: Remember / Apply Interpretation and Delete Without Confirmation (Project), Duplicate Render Item (Render Queue). Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/nav_more.rs | 12 +++ crates/engine/src/settings/shortcuts.rs | 98 +++++++++++++++++-- crates/engine/src/tests/settings.rs | 4 +- crates/ui-egui/src/app.rs | 2 +- crates/ui-egui/src/menus/shortcuts.rs | 54 ++++++++-- crates/ui-egui/src/panels/shortcut_editor.rs | 6 +- crates/ui-egui/tests/it/main.rs | 1 + crates/ui-egui/tests/it/ui_shortcut_scopes.rs | 86 ++++++++++++++++ docs/preferences.md | 11 ++- 9 files changed, 247 insertions(+), 27 deletions(-) create mode 100644 crates/ui-egui/tests/it/ui_shortcut_scopes.rs diff --git a/crates/engine/src/cmd/nav_more.rs b/crates/engine/src/cmd/nav_more.rs index d008d1ef6..18165bf48 100644 --- a/crates/engine/src/cmd/nav_more.rs +++ b/crates/engine/src/cmd/nav_more.rs @@ -39,6 +39,18 @@ fn deselect_keys(s: &mut Session, _: &Value) -> Result { Ok(Value::Null) } +impl Session { + /// The selected mask (its group, or a property inside it) of a selected layer, the first + /// one: (layer, mask group). + pub fn selected_mask(&self) -> Option<(effectcraft_project::LayerId, effectcraft_project::Uid)> { + let comp = self.active_comp()?; + self.state.selected_props.iter().find_map(|(lid, uid)| { + let masks = comp.layer(*lid)?.masks()?; + masks.groups().find(|g| g.uid == *uid || g.find_group(*uid).is_some() || g.find(*uid).is_some()).map(|g| (*lid, g.uid)) + }) + } +} + /// Alt+` / Alt+Shift+`: select the next (previous) mask of the layer, wrapping around; the first /// (last) one when none is selected. fn step_mask(s: &mut Session, p: &Value, dir: i64) -> Result { diff --git a/crates/engine/src/settings/shortcuts.rs b/crates/engine/src/settings/shortcuts.rs index 1d60971ac..95858823b 100644 --- a/crates/engine/src/settings/shortcuts.rs +++ b/crates/engine/src/settings/shortcuts.rs @@ -9,6 +9,11 @@ //! (as After Effects asks you to "Save As" a custom set). The shortcut dispatcher and the menus //! read the active preset through [`ShortcutTable`]. //! +//! Scopes, narrowest first: a *context* ([`CONTEXT_SCOPES`]: a state of the app such as editing +//! text; its shortcuts work only while it is active), a *panel* (`Project`, `Timeline`, +//! `RenderQueue`…: the panel with the keyboard) and the application. One chord can be bound once +//! per scope; the narrowest active scope runs it ([`scope_rank`]). +//! //! Shortcut text is `Ctrl+Cmd+Alt+Shift+Key` (`Cmd` is ⌘ on macOS and Ctrl elsewhere). use std::collections::{BTreeMap, HashMap}; @@ -29,6 +34,69 @@ pub struct UiCommand { pub id: String, pub label: String, pub shortcut: Option, + /// Its scope; none: from its id (`timeline.*` → Timeline, `view.*` → Composition). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub scope: Option, +} + +/// Scopes that are a state of the app rather than a panel: editing text in the viewer, a paint +/// tool active, a mask selected, a camera tool active. +pub const CONTEXT_SCOPES: &[&str] = &["TextEditing", "Paint", "Mask", "Camera"]; + +/// Engine commands and menu entries whose shortcuts belong to one panel or context (binding key +/// → scope). Every other one is application-wide; frontend commands state their own. +const SCOPES: &[(&str, &str)] = &[ + ("file.rememberInterpretation", "Project"), + ("file.applyInterpretation", "Project"), + ("project.deleteWithoutConfirmation", "Project"), + ("renderQueue.duplicate", "RenderQueue"), +]; + +/// Scope of an engine command or menu entry from its binding key. +pub fn scope_of(key: &str) -> &'static str { + SCOPES.iter().find(|(k, _)| *k == key).map_or(APP_SCOPE, |(_, s)| s) +} + +pub fn is_context(scope: &str) -> bool { + CONTEXT_SCOPES.contains(&scope) +} + +/// The Timeline and the Composition panel show the same composition and share their shortcuts: +/// the layer keys work with either one focused. +pub fn panels_share(a: &str, b: &str) -> bool { + matches!((a, b), ("Timeline", "Composition") | ("Composition", "Timeline")) +} + +/// A scope as shown to people (`RenderQueue` → `Render Queue`). +pub fn scope_label(scope: &str) -> String { + let mut out = String::with_capacity(scope.len() + 2); + for (i, c) in scope.chars().enumerate() { + if i > 0 && c.is_uppercase() { + out.push(' '); + } + out.push(c); + } + out +} + +/// How strongly a binding in `scope` claims a chord while the panel `focused` has the keyboard +/// and `contexts` are active: lower wins, `None` doesn't run at all. An active context, then the +/// focused panel, then the panel sharing its shortcuts, then the application. A shortcut of +/// another panel still runs when nothing nearer claims the chord (the reveal keys work from the +/// Effect Controls panel too); a context's shortcuts never run outside it. +pub fn scope_rank(scope: &str, focused: &str, contexts: &[&str]) -> Option { + if is_context(scope) { + return contexts.contains(&scope).then_some(0); + } + Some(if scope == focused { + 1 + } else if panels_share(scope, focused) { + 2 + } else if scope == APP_SCOPE { + 3 + } else { + 4 + }) } /// One thing a shortcut can run. @@ -95,12 +163,12 @@ pub fn bindables(ui: &[UiCommand]) -> Vec { e.shortcut.as_deref().into_iter().chain(e.alt_shortcuts.iter().map(String::as_str)).chain(alt.iter().copied()).filter_map(normalize).collect(); push( Bindable { + scope: scope_of(&key).into(), key, command: e.command.clone(), params: e.params.clone(), label: e.label.clone(), path: path.clone(), - scope: APP_SCOPE.into(), defaults, }, &mut out, @@ -133,22 +201,22 @@ pub fn bindables(ui: &[UiCommand]) -> Vec { params: Value::Null, label: c.label.to_string(), path: vec!["Commands".into()], - scope: APP_SCOPE.into(), + scope: scope_of(c.id).into(), defaults, }, &mut out, ); } for c in ui { - let scope = ui_scope(&c.id); - let cat = if scope == APP_SCOPE { "Tools & Panels" } else { scope }; + let scope = c.scope.as_deref().unwrap_or_else(|| ui_scope(&c.id)); + let cat = if scope == APP_SCOPE { "Tools & Panels".to_string() } else { scope_label(scope) }; push( Bindable { key: c.id.clone(), command: c.id.clone(), params: Value::Null, label: c.label.clone(), - path: vec![cat.into()], + path: vec![cat], scope: scope.into(), defaults: c.shortcut.as_deref().and_then(normalize).into_iter().collect(), }, @@ -290,10 +358,16 @@ pub struct Conflict { pub key: String, pub label: String, pub scope: String, + /// The two are in different scopes, so both keep working: the narrower one runs while its + /// panel or context is active and the other one everywhere else. `false`: the same scope, + /// where only one of them can run. + pub overrides: bool, } +/// Whether shortcuts of these scopes can be active at once. Two different panels can't, except +/// the pair sharing theirs; a context can be active in any panel. fn scopes_overlap(a: &str, b: &str) -> bool { - a == b || a == APP_SCOPE || b == APP_SCOPE + a == b || a == APP_SCOPE || b == APP_SCOPE || panels_share(a, b) || is_context(a) || is_context(b) } impl Keymaps { @@ -492,11 +566,12 @@ impl ShortcutTable { self.bindings() .into_iter() .filter(|(k, o)| *k == keys && o.key != b.key && scopes_overlap(&o.scope, &b.scope)) - .map(|(k, o)| Conflict { keys: k.to_string(), key: o.key.clone(), label: o.label.clone(), scope: o.scope.clone() }) + .map(|(k, o)| Conflict { keys: k.to_string(), key: o.key.clone(), label: o.label.clone(), scope: o.scope.clone(), overrides: o.scope != b.scope }) .collect() } - /// Every pair of commands sharing keys in an overlapping scope. + /// Every pair of commands sharing keys in the same scope: only one of the two can run. + /// (Pairs in different scopes are intended, see [`Conflict::overrides`].) pub fn all_conflicts(&self) -> Vec<(String, Conflict)> { let mut by_keys: BTreeMap<&str, Vec<&Bindable>> = BTreeMap::new(); for (k, b) in self.bindings() { @@ -508,8 +583,11 @@ impl ShortcutTable { for (k, bs) in by_keys { for (i, a) in bs.iter().enumerate() { for b in &bs[i + 1..] { - if scopes_overlap(&a.scope, &b.scope) { - out.push((a.key.clone(), Conflict { keys: k.to_string(), key: b.key.clone(), label: b.label.clone(), scope: b.scope.clone() })); + if a.scope == b.scope { + out.push(( + a.key.clone(), + Conflict { keys: k.to_string(), key: b.key.clone(), label: b.label.clone(), scope: b.scope.clone(), overrides: false }, + )); } } } diff --git a/crates/engine/src/tests/settings.rs b/crates/engine/src/tests/settings.rs index 79e9fa727..61177d499 100644 --- a/crates/engine/src/tests/settings.rs +++ b/crates/engine/src/tests/settings.rs @@ -597,8 +597,8 @@ fn shortcut_presets_duplicate_rename_delete_export_import() { fn frontend_commands_are_bindable_with_panel_scope() { let mut s = Session::default(); s.set_ui_commands(vec![ - crate::shortcuts::UiCommand { id: "timeline.zoomIn".into(), label: "Zoom In Time".into(), shortcut: Some("=".into()) }, - crate::shortcuts::UiCommand { id: "tool.hand".into(), label: "Hand Tool".into(), shortcut: Some("H".into()) }, + crate::shortcuts::UiCommand { id: "timeline.zoomIn".into(), label: "Zoom In Time".into(), shortcut: Some("=".into()), scope: None }, + crate::shortcuts::UiCommand { id: "tool.hand".into(), label: "Hand Tool".into(), shortcut: Some("H".into()), scope: None }, ]); let b = s.shortcuts().find("timeline.zoomIn").unwrap().clone(); assert_eq!(b.scope, "Timeline"); diff --git a/crates/ui-egui/src/app.rs b/crates/ui-egui/src/app.rs index 79c077b2a..2480d0117 100644 --- a/crates/ui-egui/src/app.rs +++ b/crates/ui-egui/src/app.rs @@ -114,7 +114,7 @@ impl EffectcraftApp { fn with_ui_commands(mut self) -> Self { let cmds = menus::UI_COMMANDS .iter() - .map(|c| effectcraft_engine::shortcuts::UiCommand { id: c.id.into(), label: c.label.into(), shortcut: c.shortcut.map(str::to_string) }) + .map(|c| effectcraft_engine::shortcuts::UiCommand { id: c.id.into(), label: c.label.into(), shortcut: c.shortcut.map(str::to_string), scope: None }) .collect(); self.session.set_ui_commands(cmds); self diff --git a/crates/ui-egui/src/menus/shortcuts.rs b/crates/ui-egui/src/menus/shortcuts.rs index 92e4d5d2e..f82accf4d 100644 --- a/crates/ui-egui/src/menus/shortcuts.rs +++ b/crates/ui-egui/src/menus/shortcuts.rs @@ -1,6 +1,7 @@ use super::frontend::run_engine; use super::invoke::invoke; use super::{EffectcraftApp, PanelKind, Value, json}; +use crate::state::Tool; /// Shortcut text in this OS's notation (`⇧⌘K` on macOS, `Ctrl+Shift+K` elsewhere). pub fn shortcut_text(s: &str) -> String { @@ -179,6 +180,24 @@ pub(super) fn modifiers_at_start(ctx: &egui::Context) -> Option }) } +/// The shortcut contexts active now (`effectcraft_engine::shortcuts::CONTEXT_SCOPES`): text +/// being edited in the viewer, a paint tool or a camera tool active, a mask selected. +pub fn active_contexts(app: &EffectcraftApp) -> Vec<&'static str> { + let mut out = vec![]; + if app.session.state.text_edit.is_some() { + out.push("TextEditing"); + } + match app.ui.tool { + Tool::Brush | Tool::Clone | Tool::Eraser => out.push("Paint"), + Tool::Orbit | Tool::PanCamera | Tool::Dolly => out.push("Camera"), + _ => {} + } + if app.session.selected_mask().is_some() { + out.push("Mask"); + } + out +} + /// Only Shift held (Shift+Delete, not Ctrl/Alt+Shift+Delete). fn only_shift(m: egui::Modifiers) -> bool { m.shift && !m.alt && !m.ctrl && !m.command && !m.mac_cmd @@ -361,6 +380,13 @@ fn handle_shortcuts_impl(app: &mut EffectcraftApp, ctx: &egui::Context, tab_only } let binds = bindings(&app.session); for (key, mods, pressed, physical_key) in events { + // (Per event: a shortcut can change the tool, the selection or the focused panel.) + let (focused, contexts) = (app.ui.focused.id(), active_contexts(app)); + let rank = |id: &str, params: &Value| { + let key = effectcraft_engine::shortcuts::binding_key(id, params); + let scope = app.session.shortcuts().find(&key).map_or(effectcraft_engine::shortcuts::APP_SCOPE, |b| b.scope.as_str()); + effectcraft_engine::shortcuts::scope_rank(scope, &focused, &contexts) + }; // Plain Spacebar runs its shortcut (Play Current Preview) on the release: held, it is // the Hand tool (After Effects), and a drag with it must not start or stop playback. if key == egui::Key::Space && (!pressed || !mods.any()) { @@ -408,10 +434,17 @@ fn handle_shortcuts_impl(app: &mut EffectcraftApp, ctx: &egui::Context, tab_only // Explicit physical bindings distinguish keys that a layout reports with the same // logical name (for example AZERTY's minus and right parenthesis). let physical = physical_key.and_then(|key| { - app.session.shortcuts().bindings().into_iter().find_map(|(sc, b)| { - let (m, k) = parse_shortcut(sc.strip_prefix("Physical+")?)?; - (k == key && mods_match(m, mods)).then(|| (b.command.clone(), b.params.clone())) - }) + app.session + .shortcuts() + .bindings() + .into_iter() + .filter_map(|(sc, b)| { + let (m, k) = parse_shortcut(sc.strip_prefix("Physical+")?)?; + (k == key && mods_match(m, mods)).then_some(b) + }) + .filter_map(|b| rank(&b.command, &b.params).map(|r| (r, b))) + .min_by_key(|(r, _)| *r) + .map(|(_, b)| (b.command.clone(), b.params.clone())) }); if let Some((id, params)) = physical { if let Err(e) = invoke(app, ctx, &id, if params.is_null() { json!({}) } else { params }) { @@ -419,11 +452,14 @@ fn handle_shortcuts_impl(app: &mut EffectcraftApp, ctx: &egui::Context, tab_only } continue; } - if let Some((_, _, id, params)) = binds.iter().filter(|(m, k, ..)| *k == key && mods_match(*m, mods)).min_by_key(|(_, _, id, params)| { - // The focused panel wins over an application command sharing its chord. - let key = effectcraft_engine::shortcuts::binding_key(id, params); - app.session.shortcuts().find(&key).is_none_or(|b| b.scope != app.ui.focused.id()) - }) { + // The narrowest active scope wins a chord: an active context, the focused panel, the + // application. A context's shortcut doesn't run outside it. + let chosen = binds + .iter() + .filter(|(m, k, ..)| *k == key && mods_match(*m, mods)) + .filter_map(|(_, _, id, params)| rank(id, params).map(|r| (r, id, params))) + .min_by_key(|(r, ..)| *r); + if let Some((_, id, params)) = chosen { let (id, params) = (id.clone(), if params.is_null() { json!({}) } else { params.clone() }); if id == "edit.paste" && let Some(t) = &pasted diff --git a/crates/ui-egui/src/panels/shortcut_editor.rs b/crates/ui-egui/src/panels/shortcut_editor.rs index 60b908069..e4bf313ac 100644 --- a/crates/ui-egui/src/panels/shortcut_editor.rs +++ b/crates/ui-egui/src/panels/shortcut_editor.rs @@ -17,7 +17,7 @@ //! `shortcuts.resetPreset`, `shortcuts.close`. use crate::i18n::{tr, tr_args}; -use effectcraft_engine::shortcuts::{APP_SCOPE, Bindable, DEFAULT_PRESET, normalize}; +use effectcraft_engine::shortcuts::{APP_SCOPE, Bindable, DEFAULT_PRESET, normalize, scope_label}; use egui::{Align2, Color32, Rect, RichText, Sense, Stroke, StrokeKind, pos2, vec2}; use serde_json::{Value, json}; @@ -517,7 +517,7 @@ pub fn show(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { return; }; ui.label(RichText::new(&b.label).font(Tokens::semibold(13.0)).color(t.tab_text_active)); - ui.label(RichText::new(format!("{} • {}", b.command, b.scope)).color(t.text_faint)); + ui.label(RichText::new(format!("{} • {}", b.command, scope_label(&b.scope))).color(t.text_faint)); ui.add_space(6.0); let keys = table.keys.get(&b.key).cloned().unwrap_or_default(); if keys.is_empty() { @@ -565,7 +565,7 @@ pub fn show(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { ui.add_space(4.0); ui.label(RichText::new(tr_args("⚠ {} is used by:", &[&(crate::menus::shortcut_text(&k))])).color(t.warning)); for c in conflicts.iter().take(4) { - ui.label(RichText::new(format!(" {} ({})", c.label, c.scope)).color(t.warning)); + ui.label(RichText::new(format!(" {} ({})", c.label, scope_label(&c.scope))).color(t.warning)); } } } diff --git a/crates/ui-egui/tests/it/main.rs b/crates/ui-egui/tests/it/main.rs index 4fc34d9ff..0f3b7cea3 100644 --- a/crates/ui-egui/tests/it/main.rs +++ b/crates/ui-egui/tests/it/main.rs @@ -77,6 +77,7 @@ mod ui_scrolling; mod ui_self_drop; mod ui_settings_info; mod ui_settings_keyboard; +mod ui_shortcut_scopes; mod ui_tabs; mod ui_tail; mod ui_text_edit; diff --git a/crates/ui-egui/tests/it/ui_shortcut_scopes.rs b/crates/ui-egui/tests/it/ui_shortcut_scopes.rs new file mode 100644 index 000000000..6c79fe72a --- /dev/null +++ b/crates/ui-egui/tests/it/ui_shortcut_scopes.rs @@ -0,0 +1,86 @@ +//! Shortcut scopes: one chord can run a different command in a panel than in the rest of the +//! application, and the Timeline's keys work with the Composition panel focused. + +use effectcraft_engine::Session; +use effectcraft_ui_egui::EffectcraftApp; +use effectcraft_ui_egui::dock::PanelKind; +use egui::{Key, Modifiers}; +use serde_json::json; + +/// A comp with one solid selected at 1 s, and one render item. +fn app() -> EffectcraftApp { + let mut s = Session::default(); + s.execute("comp.new", json!({"name": "Main", "width": 160, "height": 90, "frameRate": 30, "duration": 4})).unwrap(); + s.execute("layer.newSolid", json!({"color": "#406080"})).unwrap(); + s.execute("renderQueue.add", json!({"output": "/out/main.mov"})).unwrap(); + s.execute("time.set", json!({"time": 1.0})).unwrap(); + EffectcraftApp::new(s) +} + +/// Press `key` with `modifiers` and run the shortcut dispatcher on it. +fn press(app: &mut EffectcraftApp, key: Key, modifiers: Modifiers) { + let ctx = egui::Context::default(); + let raw = egui::RawInput { events: vec![egui::Event::Key { key, physical_key: None, pressed: true, repeat: false, modifiers }], ..Default::default() }; + let mut output = ctx.run_ui(raw, |ui| effectcraft_ui_egui::menus::handle_shortcuts(app, ui.ctx())); + output.textures_delta.clear(); +} + +fn layers(app: &EffectcraftApp) -> usize { + app.session.active_comp().map_or(0, |c| c.layers.len()) +} + +fn render_items(app: &EffectcraftApp) -> usize { + app.session.project.render_queue.len() +} + +#[test] +fn a_panels_shortcut_wins_in_its_panel_and_the_applications_everywhere_else() { + let mut app = app(); + // Duplicate Render Item (Render Queue) on Split Layer's chord. + app.session.execute("shortcuts.set", json!({"command": "renderQueue.duplicate", "keys": "Cmd+Shift+D"})).unwrap(); + let chord = Modifiers::COMMAND | Modifiers::SHIFT; + + app.ui.focused = PanelKind::Timeline; + press(&mut app, Key::D, chord); + assert_eq!((layers(&app), render_items(&app)), (2, 1), "in the Timeline the chord splits the layer"); + + app.ui.focused = PanelKind::RenderQueue; + press(&mut app, Key::D, chord); + assert_eq!((layers(&app), render_items(&app)), (2, 2), "in the Render Queue it duplicates the render item"); + + app.ui.focused = PanelKind::Project; + app.session.execute("edit.selectAll", json!({})).unwrap(); + app.session.execute("time.set", json!({"time": 2.0})).unwrap(); + press(&mut app, Key::D, chord); + assert_eq!((layers(&app), render_items(&app)), (3, 2), "and in any other panel it is Split Layer again"); +} + +#[test] +fn a_panels_shortcut_that_nothing_else_claims_runs_from_any_panel() { + let mut app = app(); + let unclaimed = "Cmd+Alt+Shift+F12"; + assert!(app.session.shortcuts().bindings().iter().all(|(sc, _)| *sc != unclaimed), "{unclaimed} has a default now; pick a free chord"); + app.session.execute("shortcuts.set", json!({"command": "renderQueue.duplicate", "keys": unclaimed})).unwrap(); + app.ui.focused = PanelKind::Timeline; + press(&mut app, Key::F12, Modifiers::COMMAND | Modifiers::ALT | Modifiers::SHIFT); + assert_eq!(render_items(&app), 2); +} + +#[test] +fn the_timelines_keys_work_with_the_composition_panel_focused() { + for focused in [PanelKind::Timeline, PanelKind::Composition] { + let mut app = app(); + app.ui.focused = focused; + // B: the work area begins at the current time (a Timeline shortcut). + press(&mut app, Key::B, Modifiers::NONE); + let start = app.session.active_comp().map(|c| c.work_area.0.seconds()); + assert_eq!(start, Some(1.0), "{focused:?}"); + } +} + +#[test] +fn the_default_preset_has_no_two_commands_on_one_chord_in_one_scope() { + let app = app(); + let c = app.session.shortcuts().all_conflicts(); + assert!(c.is_empty(), "{c:?}"); +} diff --git a/docs/preferences.md b/docs/preferences.md index 2b80ffa0f..5dd911858 100644 --- a/docs/preferences.md +++ b/docs/preferences.md @@ -203,8 +203,15 @@ commands), and a preset menu. - **Assigning.** Select a command, then press keys in the shortcut field (or type them), or click a key on the keyboard with the modifiers toggled. Conflicts with other commands in an overlapping scope are listed before you assign. -- **Scope.** Menu and engine commands are application-wide; timeline commands work in the - Timeline and viewer commands in the Composition panel. +- **Scope.** A shortcut belongs to the application, to a panel (Project, Timeline, Composition, + Render Queue…) or to a context (editing text, a paint tool or a camera tool active, a mask + selected). One chord can be bound once per scope, and the narrowest active scope runs it: an + active context, then the panel with the keyboard, then the application. The Timeline and the + Composition panel share their shortcuts. A context's shortcuts work only while it is active; + a panel's shortcut that nothing nearer claims still works from the other panels. +- **Conflicts.** Two commands of one scope on one chord are a conflict (`shortcuts.conflicts` + lists them; the default preset has none). The same chord in two scopes is an override: both + keep working, and `shortcuts.set` reports it with `overrides: true`. Commands: `shortcuts.list {query?, keys?, assigned?}`, `shortcuts.set {command, params?, keys}`, `shortcuts.reset {command?}`, `shortcuts.preset {op: select|new|duplicate|delete|rename, name?, From aa149d7bbc4dac71c58fb90b2325af6570bbc77f Mon Sep 17 00:00:00 2001 From: pravdomil <2387356+pravdomil@users.noreply.github.com> Date: Sat, 31 Oct 2026 22:35:33 +0100 Subject: [PATCH 2/2] Shortcuts: deterministic dispatch between panels, physical and logical ranked together - shortcuts::pick chooses the binding for a key press: narrowest scope, then a physical-key binding, then the first. When two other panels (or two active contexts) claim the chord at the same rank, neither runs and the status bar says which panels own it. - Physical and logical bindings compete by scope rank in one pass, so a narrower logical binding beats a physical one of an unrelated panel. - An additional viewer (PanelKind::Viewer) ranks as the Composition panel. - Two contexts on the same keys are reported as a conflict. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/settings/shortcuts.rs | 45 ++++++++++++++++--- crates/engine/src/tests/settings.rs | 43 ++++++++++++++++++ crates/ui-egui/src/dock.rs | 7 +++ crates/ui-egui/src/menus/shortcuts.rs | 58 ++++++++++--------------- 4 files changed, 113 insertions(+), 40 deletions(-) diff --git a/crates/engine/src/settings/shortcuts.rs b/crates/engine/src/settings/shortcuts.rs index 95858823b..44e2261a2 100644 --- a/crates/engine/src/settings/shortcuts.rs +++ b/crates/engine/src/settings/shortcuts.rs @@ -83,7 +83,8 @@ pub fn scope_label(scope: &str) -> String { /// and `contexts` are active: lower wins, `None` doesn't run at all. An active context, then the /// focused panel, then the panel sharing its shortcuts, then the application. A shortcut of /// another panel still runs when nothing nearer claims the chord (the reveal keys work from the -/// Effect Controls panel too); a context's shortcuts never run outside it. +/// Effect Controls panel too); a context's shortcuts never run outside it. Two other panels (or +/// two active contexts) can claim a chord at the same rank: [`pick`] runs neither. pub fn scope_rank(scope: &str, focused: &str, contexts: &[&str]) -> Option { if is_context(scope) { return contexts.contains(&scope).then_some(0); @@ -99,6 +100,33 @@ pub fn scope_rank(scope: &str, focused: &str, contexts: &[&str]) -> Option { }) } +/// Which of the bindings matching one key press runs: `candidates` are (scope, physical, +/// binding), in the dispatcher's order. The narrowest scope ([`scope_rank`]) wins; at the same +/// rank an explicit physical-key binding wins, then the first one. `Err`: the best candidates +/// belong to different scopes of the same rank (two other panels, or two active contexts), so +/// none of them runs; it lists those scopes. +pub fn pick<'a, T>(candidates: impl IntoIterator, focused: &str, contexts: &[&str]) -> Result, Vec> { + let mut best: Option<((u8, bool), &str, T)> = None; + let mut tied: Vec = vec![]; + for (scope, physical, b) in candidates { + let Some(rank) = scope_rank(scope, focused, contexts) else { continue }; + let at = (rank, !physical); + match &best { + Some((r, s, _)) if *r < at || (*r == at && *s == scope) => {} + Some((r, _, _)) if *r == at => { + if !tied.iter().any(|t| t == scope) { + tied.push(scope.to_string()); + } + } + _ => { + tied = vec![scope.to_string()]; + best = Some((at, scope, b)); + } + } + } + if tied.len() > 1 { Err(tied) } else { Ok(best.map(|(.., b)| b)) } +} + /// One thing a shortcut can run. #[derive(Clone, Debug, PartialEq, Serialize)] pub struct Bindable { @@ -566,12 +594,19 @@ impl ShortcutTable { self.bindings() .into_iter() .filter(|(k, o)| *k == keys && o.key != b.key && scopes_overlap(&o.scope, &b.scope)) - .map(|(k, o)| Conflict { keys: k.to_string(), key: o.key.clone(), label: o.label.clone(), scope: o.scope.clone(), overrides: o.scope != b.scope }) + .map(|(k, o)| Conflict { + keys: k.to_string(), + key: o.key.clone(), + label: o.label.clone(), + scope: o.scope.clone(), + overrides: o.scope != b.scope && !(is_context(&o.scope) && is_context(&b.scope)), + }) .collect() } - /// Every pair of commands sharing keys in the same scope: only one of the two can run. - /// (Pairs in different scopes are intended, see [`Conflict::overrides`].) + /// Every pair of commands sharing keys in the same scope, or in two contexts (which can be + /// active at once): only one of the two can run, or neither ([`pick`]). (Other pairs in + /// different scopes are intended, see [`Conflict::overrides`].) pub fn all_conflicts(&self) -> Vec<(String, Conflict)> { let mut by_keys: BTreeMap<&str, Vec<&Bindable>> = BTreeMap::new(); for (k, b) in self.bindings() { @@ -583,7 +618,7 @@ impl ShortcutTable { for (k, bs) in by_keys { for (i, a) in bs.iter().enumerate() { for b in &bs[i + 1..] { - if a.scope == b.scope { + if a.scope == b.scope || (is_context(&a.scope) && is_context(&b.scope)) { out.push(( a.key.clone(), Conflict { keys: k.to_string(), key: b.key.clone(), label: b.label.clone(), scope: b.scope.clone(), overrides: false }, diff --git a/crates/engine/src/tests/settings.rs b/crates/engine/src/tests/settings.rs index 61177d499..caf6feb84 100644 --- a/crates/engine/src/tests/settings.rs +++ b/crates/engine/src/tests/settings.rs @@ -606,6 +606,49 @@ fn frontend_commands_are_bindable_with_panel_scope() { // A panel shortcut conflicts with an application shortcut on the same keys. let c = s.shortcuts().conflicts(&b, "H"); assert!(c.iter().any(|c| c.key == "tool.hand")); + // A registry command that only the Project panel runs is scoped to it. + assert_eq!(s.shortcuts().find("project.deleteWithoutConfirmation").unwrap().scope, "Project"); +} + +/// The binding that runs a key press: the narrowest scope; a physical binding at the same rank; +/// nothing when two other panels (or two active contexts) claim it at the same rank. +#[test] +fn shortcut_dispatch_picks_the_narrowest_scope_deterministically() { + use crate::shortcuts::pick; + // Two unrelated panels, neither focused: neither runs, in either order. + let both = [("Project", false, "delete"), ("RenderQueue", false, "duplicate")]; + assert_eq!(pick(both, "Timeline", &[]), Err(vec!["Project".to_string(), "RenderQueue".into()])); + assert_eq!(pick([both[1], both[0]], "Timeline", &[]), Err(vec!["RenderQueue".to_string(), "Project".into()])); + // The focused one of them wins. + assert_eq!(pick(both, "RenderQueue", &[]), Ok(Some("duplicate"))); + // A single other panel's shortcut runs when nothing nearer claims the chord. + assert_eq!(pick([("Project", false, "delete")], "Timeline", &[]), Ok(Some("delete"))); + // An application binding beats an unrelated panel's binding, also a physical one. + assert_eq!(pick([("RenderQueue", true, "duplicate"), ("Application", false, "app")], "Timeline", &[]), Ok(Some("app"))); + // At the same rank a physical binding wins, in either order. + assert_eq!(pick([("Application", false, "logical"), ("Application", true, "physical")], "Timeline", &[]), Ok(Some("physical"))); + assert_eq!(pick([("Application", true, "physical"), ("Application", false, "logical")], "Timeline", &[]), Ok(Some("physical"))); + // A Composition binding runs in the Composition panel over the application's. + assert_eq!(pick([("Application", false, "app"), ("Composition", false, "comp")], "Composition", &[]), Ok(Some("comp"))); + // Two active contexts tie; an inactive one doesn't run. + assert_eq!(pick([("Mask", false, "mask"), ("Paint", false, "paint")], "Composition", &["Paint", "Mask"]), Err(vec!["Mask".to_string(), "Paint".into()])); + assert_eq!(pick([("Mask", false, "mask"), ("Paint", false, "paint")], "Composition", &["Paint"]), Ok(Some("paint"))); + // The same scope twice (a reported conflict): the first one. + assert_eq!(pick([("Timeline", false, "a"), ("Timeline", false, "b")], "Timeline", &[]), Ok(Some("a"))); +} + +/// Two contexts can be active at once, so their shortcuts on the same keys conflict. +#[test] +fn context_shortcuts_on_the_same_keys_conflict() { + let mut s = Session::default(); + s.set_ui_commands(vec![ + crate::shortcuts::UiCommand { id: "test.mask".into(), label: "Mask".into(), shortcut: Some("F9".into()), scope: Some("Mask".into()) }, + crate::shortcuts::UiCommand { id: "test.paint".into(), label: "Paint".into(), shortcut: Some("F9".into()), scope: Some("Paint".into()) }, + ]); + let t = s.shortcuts(); + assert!(t.all_conflicts().iter().any(|(a, c)| a == "test.mask" && c.key == "test.paint" && !c.overrides), "{:?}", t.all_conflicts()); + let b = t.find("test.mask").unwrap(); + assert!(t.conflicts(b, "F9").iter().any(|c| c.key == "test.paint" && !c.overrides)); } // ---------------------------------------------------------------- auto-save / recovery diff --git a/crates/ui-egui/src/dock.rs b/crates/ui-egui/src/dock.rs index 8e8fd5eca..c8ada9bf8 100644 --- a/crates/ui-egui/src/dock.rs +++ b/crates/ui-egui/src/dock.rs @@ -137,6 +137,13 @@ impl PanelKind { pub fn id(self) -> String { format!("{self:?}") } + /// The shortcut scope of the panel: an additional viewer is a Composition panel. + pub fn shortcut_scope(self) -> String { + match self { + PanelKind::Viewer(_) => PanelKind::Composition.id(), + p => p.id(), + } + } pub fn from_name(s: &str) -> Option { let n = s.to_ascii_lowercase().replace([' ', '_', '-', '&'], ""); Self::ALL.iter().copied().find(|p| format!("{p:?}").to_ascii_lowercase() == n || p.title().to_ascii_lowercase().replace([' ', '&', '-'], "") == n) diff --git a/crates/ui-egui/src/menus/shortcuts.rs b/crates/ui-egui/src/menus/shortcuts.rs index f82accf4d..4ef1e64fb 100644 --- a/crates/ui-egui/src/menus/shortcuts.rs +++ b/crates/ui-egui/src/menus/shortcuts.rs @@ -381,12 +381,7 @@ fn handle_shortcuts_impl(app: &mut EffectcraftApp, ctx: &egui::Context, tab_only let binds = bindings(&app.session); for (key, mods, pressed, physical_key) in events { // (Per event: a shortcut can change the tool, the selection or the focused panel.) - let (focused, contexts) = (app.ui.focused.id(), active_contexts(app)); - let rank = |id: &str, params: &Value| { - let key = effectcraft_engine::shortcuts::binding_key(id, params); - let scope = app.session.shortcuts().find(&key).map_or(effectcraft_engine::shortcuts::APP_SCOPE, |b| b.scope.as_str()); - effectcraft_engine::shortcuts::scope_rank(scope, &focused, &contexts) - }; + let (focused, contexts) = (app.ui.focused.shortcut_scope(), active_contexts(app)); // Plain Spacebar runs its shortcut (Play Current Preview) on the release: held, it is // the Hand tool (After Effects), and a drag with it must not start or stop playback. if key == egui::Key::Space && (!pressed || !mods.any()) { @@ -431,36 +426,29 @@ fn handle_shortcuts_impl(app: &mut EffectcraftApp, ctx: &egui::Context, tab_only let _ = invoke(app, ctx, "edit.clear", json!({})); continue; } - // Explicit physical bindings distinguish keys that a layout reports with the same - // logical name (for example AZERTY's minus and right parenthesis). - let physical = physical_key.and_then(|key| { - app.session - .shortcuts() - .bindings() - .into_iter() - .filter_map(|(sc, b)| { - let (m, k) = parse_shortcut(sc.strip_prefix("Physical+")?)?; - (k == key && mods_match(m, mods)).then_some(b) - }) - .filter_map(|b| rank(&b.command, &b.params).map(|r| (r, b))) - .min_by_key(|(r, _)| *r) - .map(|(_, b)| (b.command.clone(), b.params.clone())) - }); - if let Some((id, params)) = physical { - if let Err(e) = invoke(app, ctx, &id, if params.is_null() { json!({}) } else { params }) { - app.ui.status = e; - } - continue; - } // The narrowest active scope wins a chord: an active context, the focused panel, the - // application. A context's shortcut doesn't run outside it. - let chosen = binds - .iter() - .filter(|(m, k, ..)| *k == key && mods_match(*m, mods)) - .filter_map(|(_, _, id, params)| rank(id, params).map(|r| (r, id, params))) - .min_by_key(|(r, ..)| *r); - if let Some((_, id, params)) = chosen { - let (id, params) = (id.clone(), if params.is_null() { json!({}) } else { params.clone() }); + // application. A context's shortcut doesn't run outside it. Explicit physical bindings + // distinguish keys that a layout reports with the same logical name (for example AZERTY's + // minus and right parenthesis); they win over logical ones of the same scope rank. + let table = app.session.shortcuts(); + let scope_of = |id: &str, params: &Value| { + let key = effectcraft_engine::shortcuts::binding_key(id, params); + table.find(&key).map_or(effectcraft_engine::shortcuts::APP_SCOPE, |b| b.scope.as_str()) + }; + let physical = table.bindings().into_iter().filter_map(|(sc, b)| { + let (m, k) = parse_shortcut(sc.strip_prefix("Physical+")?)?; + (Some(k) == physical_key && mods_match(m, mods)).then_some((b.scope.as_str(), true, (&b.command, &b.params))) + }); + let logical = binds.iter().filter(|(m, k, ..)| *k == key && mods_match(*m, mods)).map(|(_, _, id, params)| (scope_of(id, params), false, (id, params))); + let chosen = match effectcraft_engine::shortcuts::pick(physical.chain(logical), &focused, &contexts) { + Ok(c) => c.map(|(id, params)| (id.clone(), if params.is_null() { json!({}) } else { params.clone() })), + Err(scopes) => { + let panels: Vec = scopes.iter().map(|s| effectcraft_engine::shortcuts::scope_label(s)).collect(); + app.ui.status = format!("This shortcut belongs to {}: focus one of them", panels.join(" and ")); + None + } + }; + if let Some((id, params)) = chosen { if id == "edit.paste" && let Some(t) = &pasted {