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 {