diff --git a/crates/ui-egui/src/dialogs/shortcut_import.rs b/crates/ui-egui/src/dialogs/shortcut_import.rs index 7360b3a32..fd638f971 100644 --- a/crates/ui-egui/src/dialogs/shortcut_import.rs +++ b/crates/ui-egui/src/dialogs/shortcut_import.rs @@ -377,12 +377,12 @@ pub fn candidates() -> Vec { /// Keyboard Shortcuts' entry `key`. The commands of installed plug-ins are looked up in /// [`shortcut_editor::entries`] as it is now. -fn entry(key: &str) -> Option<&'static Entry> { - static BY_KEY: OnceLock> = OnceLock::new(); +fn entry(key: &str) -> Option { + static BY_KEY: OnceLock> = OnceLock::new(); if vectorcraft_engine::cmd::plugin::is_command_id(key) { - return shortcut_editor::entries().iter().find(|e| e.key == key); + return shortcut_editor::entry(key); } - BY_KEY.get_or_init(|| shortcut_editor::entries().iter().map(|e| (e.key.as_str(), e)).collect()).get(key).copied() + BY_KEY.get_or_init(|| shortcut_editor::entries().iter().map(|e| (e.key.clone(), e.clone())).collect()).get(key).cloned() } /// A tool, command or panel that Keyboard Shortcuts can give a shortcut. @@ -400,7 +400,7 @@ fn target_label(target: Option<&str>) -> String { match target { None => tl!("Choose…").to_string(), Some("") => tl!("No equivalent").to_string(), - Some(t) => entry(t).map_or_else(|| t.to_string(), shown), + Some(t) => entry(t).map_or_else(|| t.to_string(), |e| shown(&e)), } } @@ -948,22 +948,27 @@ pub(super) fn target_picker(ui: &mut egui::Ui, i: usize, r: &Row) -> Option = if q.is_empty() { + let all = shortcut_editor::entries(); + let list: Vec = if q.is_empty() { for e in r.choices.iter().filter_map(|c| entry(c)) { - picked = option(ui, &e.key, &shown(e)).or(picked); + picked = option(ui, &e.key, &shown(&e)).or(picked); } ui.separator(); - shortcut_editor::entries().iter().collect() + (0..all.len()).collect() } else { - shortcut_editor::entries().iter().filter(|e| shown(e).to_lowercase().contains(&q) || e.key.to_lowercase().contains(&q)).collect() + all.iter() + .enumerate() + .filter(|(_, e)| shown(e).to_lowercase().contains(&q) || e.key.to_lowercase().contains(&q)) + .map(|(i, _)| i) + .collect() }; ui.data_mut(|d| d.insert_temp(header_id, ui.min_rect().height())); - (list, picked) + (all, list, picked) }, ROW_H, - |(list, _)| list.len(), - |ui, range, (list, mut picked)| { - for e in range.filter_map(|k| list.get(k)) { + |(_, list, _)| list.len(), + |ui, range, (all, list, mut picked)| { + for e in range.filter_map(|k| list.get(k).and_then(|&i| all.get(i))) { picked = option(ui, &e.key, &shown(e)).or(picked); } if picked.is_some() { diff --git a/crates/ui-egui/src/keymap.rs b/crates/ui-egui/src/keymap.rs index 844db0a98..94a26f001 100644 --- a/crates/ui-egui/src/keymap.rs +++ b/crates/ui-egui/src/keymap.rs @@ -7,7 +7,7 @@ //! outside it are VectorCraft's too (`Cmd+Shift+]`, `Comma`, `Period`, `~` for the backtick key, //! `=` for Plus with modifiers). Inside an [`Overrides`] keys and texts are the crate's. -use std::sync::{Mutex, OnceLock, RwLock}; +use std::sync::{Arc, Mutex, OnceLock, RwLock}; use craft_keymap::{Bindable, Binding, Chord, CommandRef, Key, Keymap, Kind, Lookup, Mods, Overrides, Platform, Scope, Scopes}; use serde_json::{Value, json}; @@ -142,7 +142,8 @@ fn catalogue() -> Vec { 0 } }; - let mut entries: Vec<&Entry> = shortcut_editor::entries().iter().collect(); + let all = shortcut_editor::entries(); + let mut entries: Vec<&Entry> = all.iter().collect(); entries.sort_by_key(|e| rank(e)); entries .into_iter() @@ -212,25 +213,43 @@ pub(crate) fn per_platform(cells: &'static [OnceLock; 3], p: Platform, mak cell.get_or_init(make) } -/// The keymap with the default shortcuts for [`platform`], built once per platform, and again for -/// each set of plug-in commands installed ([`vectorcraft_engine::cmd::plugin::commands_generation`]). -pub fn template() -> &'static Keymap { - static TEMPLATES: [OnceLock; 3] = [OnceLock::new(), OnceLock::new(), OnceLock::new()]; - static WITH_PLUGINS: Mutex> = Mutex::new(Vec::new()); +/// Values made for one set of installed plug-in commands +/// ([`vectorcraft_engine::cmd::plugin::commands_generation`]), one for each key. Only the values of +/// the generation asked for last are kept; an older one is freed once its last holder drops it. +pub(crate) struct Latest { + made: Mutex)>>, +} + +impl Latest { + pub(crate) const fn new() -> Self { + Latest { made: Mutex::new(Vec::new()) } + } + + /// The value for `key` at `generation`, made with `make` when there is none yet. + pub(crate) fn get(&self, key: K, generation: u64, make: impl FnOnce() -> Arc) -> Arc { + let mut made = self.made.lock().unwrap_or_else(|e| e.into_inner()); + if let Some((_, _, v)) = made.iter().find(|(k, g, _)| *k == key && *g == generation) { + return v.clone(); + } + made.retain(|(_, g, _)| *g == generation); + let v = make(); + made.push((key, generation, v.clone())); + v + } +} + +/// The keymap with the default shortcuts for [`platform`], built once per platform, and again when +/// the installed plug-in commands change ([`vectorcraft_engine::cmd::plugin::commands_generation`]). +/// Only the keymaps of the current plug-in commands are kept ([`Latest`]). +pub fn template() -> Arc { + static TEMPLATES: [OnceLock>; 3] = [OnceLock::new(), OnceLock::new(), OnceLock::new()]; + static WITH_PLUGINS: Latest = Latest::new(); let p = platform(); let generation = vectorcraft_engine::cmd::plugin::commands_generation(); if generation == 0 { - return per_platform(&TEMPLATES, p, || keymap_for(p)); + return per_platform(&TEMPLATES, p, || Arc::new(keymap_for(p))).clone(); } - let mut made = WITH_PLUGINS.lock().unwrap_or_else(|e| e.into_inner()); - if let Some((_, _, km)) = made.iter().find(|(q, g, _)| *q == p && *g == generation) { - return km; - } - made.retain(|(_, g, _)| *g == generation); - // Kept for the rest of the process: one keymap for each set of plug-in commands installed. - let km: &'static Keymap = Box::leak(Box::new(keymap_for(p))); - made.push((p, generation, km)); - km + WITH_PLUGINS.get(p, generation, || Arc::new(keymap_for(p))) } /// The default keymap of `p`. The chords the operating system takes can't be assigned. @@ -240,7 +259,7 @@ fn keymap_for(p: Platform) -> Keymap { /// The keymap with `overrides` applied. pub fn keymap_with(overrides: &Overrides) -> Keymap { - template().clone().with_overrides(overrides.clone()) + Keymap::clone(&template()).with_overrides(overrides.clone()) } /// The command a chord runs, the first for tools sharing it. @@ -409,7 +428,8 @@ mod tests { #[test] fn curvature_keeps_shift_tilde_and_plus_is_one_key() { - let km = template(); + let template = template(); + let km = &*template; let curvature = vc_chord("Shift+~").unwrap(); assert_eq!(curvature, Chord::parse("Shift+`").unwrap()); assert_eq!(lookup(km, &curvature), Some(CommandRef::new("tool:curvature"))); @@ -431,9 +451,17 @@ mod tests { fn the_default_shortcuts_do_not_conflict() { assert_eq!(template().conflicts(), vec![]); assert_eq!(template().problems(), vec![]); - assert_eq!(template().catalog().len(), shortcut_editor::entries().len(), "every entry is bindable"); + // Both lists for one set of plug-in commands: a test running alongside may install one. + let (km, entries) = loop { + let before = vectorcraft_engine::cmd::plugin::commands_generation(); + let both = (template(), shortcut_editor::entries()); + if vectorcraft_engine::cmd::plugin::commands_generation() == before { + break both; + } + }; + assert_eq!(km.catalog().len(), entries.len(), "every entry is bindable"); // A default the bridge can't read would show in the menus but never run. - let unbound: Vec<(&str, &str)> = shortcut_editor::entries() + let unbound: Vec<(&str, &str)> = entries .iter() .filter_map(|e| shortcut_editor::default_of(&e.key).map(|d| (e.key.as_str(), d))) .filter(|(key, d)| { @@ -445,6 +473,22 @@ mod tests { assert_eq!(unbound, vec![], "every default text becomes the entry's binding"); } + /// A value made for an older set of plug-in commands is freed once a newer one is asked for; + /// within one set each key keeps its own value, made once. + #[test] + fn only_the_latest_plugin_commands_are_kept() { + let cache: Latest = Latest::new(); + let first = cache.get(0, 1, || Arc::from("a")); + assert!(Arc::ptr_eq(&first, &cache.get(0, 1, || Arc::from("b"))), "made once"); + let other = Arc::downgrade(&cache.get(1, 1, || Arc::from("c"))); + assert!(other.upgrade().is_some(), "each key has its own"); + let old = Arc::downgrade(&first); + drop(first); + assert_eq!(&*cache.get(0, 2, || Arc::from("d")), "d"); + assert!(old.upgrade().is_none(), "the older value is freed"); + assert!(other.upgrade().is_none(), "the older value of the other key is freed"); + } + #[test] fn paint_defaults_have_two_bindings_and_reset_restores_both() { for (key, plain, shifted) in [("paint.lastColor", ",", "Shift+,"), ("paint.lastGradient", ".", "Shift+.")] { @@ -452,10 +496,10 @@ mod tests { let defaults = template().default_bindings(&command); assert_eq!(defaults, vec![vc_binding(plain).unwrap(), vc_binding(shifted).unwrap()], "{key}"); assert_eq!( - shortcut_editor::entry(key).map(|e| e.label.as_str()), - Some(if key == "paint.lastColor" { "Apply Last Color" } else { "Apply Last Gradient" }) + shortcut_editor::entry(key).map(|e| e.label), + Some(if key == "paint.lastColor" { "Apply Last Color" } else { "Apply Last Gradient" }.to_string()) ); - assert_eq!(effective_with(template(), key).as_deref(), Some(plain)); + assert_eq!(effective_with(&template(), key).as_deref(), Some(plain)); let mut overrides = Overrides::new(); set_override(&mut overrides, key, "Shift+;"); assert_eq!(keymap_with(&overrides).bindings(&command), vec![vc_binding("Shift+;").unwrap()]); @@ -494,7 +538,8 @@ mod tests { assert_eq!(entry_key(&c), "panel:layers"); assert_eq!(entry_of_stored(&stored_key("panel:layers")), "panel:layers"); assert_eq!(stored_key("tool:pen"), "tool:pen"); - let paths = &template().bindable(&c).unwrap().paths; + let template = template(); + let paths = &template.bindable(&c).unwrap().paths; assert_eq!(paths, &vec![vec!["Window".to_string(), "Layers".to_string()]], "import matches panels by name"); } @@ -506,8 +551,7 @@ mod tests { for id in TOGGLES { let label = crate::menus::dynamic_label(&app, id, ""); assert!(label.starts_with("Show ") || label.starts_with("Hide "), "{id}: {label}"); - let listed = template().bindable(&CommandRef::new(*id)); - assert!(listed.is_none_or(|b| b.is_toggle), "{id}"); + assert!(template().bindable(&CommandRef::new(*id)).is_none_or(|b| b.is_toggle), "{id}"); } } } diff --git a/crates/ui-egui/src/shortcut_editor.rs b/crates/ui-egui/src/shortcut_editor.rs index 07a6af693..39c7caf55 100644 --- a/crates/ui-egui/src/shortcut_editor.rs +++ b/crates/ui-egui/src/shortcut_editor.rs @@ -73,7 +73,7 @@ pub(crate) struct Mirror { impl Mirror { fn new(overrides: Overrides) -> Mirror { let plugins = vectorcraft_engine::cmd::plugin::commands_generation(); - let keymap = crate::keymap::template().clone().with_overrides(overrides); + let keymap = Keymap::clone(&crate::keymap::template()).with_overrides(overrides); let text = keymap .overrides() .iter() @@ -360,36 +360,30 @@ pub struct Entry { /// Every tool, command and panel the editor can give a shortcut, followed by the commands of the /// installed plug-ins; [`listed`] says which it lists. The list is made again when the plug-in -/// commands change ([`vectorcraft_engine::cmd::plugin::commands_generation`]). -pub fn entries() -> &'static [Entry] { - static WITH_PLUGINS: Mutex> = Mutex::new(None); +/// commands change ([`vectorcraft_engine::cmd::plugin::commands_generation`]). Only the list of the +/// current plug-in commands is kept ([`crate::keymap::Latest`]). +pub fn entries() -> Arc<[Entry]> { + static WITH_PLUGINS: crate::keymap::Latest<(), [Entry]> = crate::keymap::Latest::new(); let base = built_in_entries(); let generation = vectorcraft_engine::cmd::plugin::commands_generation(); if generation == 0 { - return base; + return base.clone(); } - let mut with = WITH_PLUGINS.lock().unwrap_or_else(|e| e.into_inner()); - if let Some((g, e)) = *with - && g == generation - { - return e; - } - let mut v = base.to_vec(); - v.extend(vectorcraft_engine::cmd::plugin::command_specs().into_iter().map(|c| Entry { - key: c.id.into(), - label: c.label.into(), - group: c.menu.join(" › "), - is_tool: false, - })); - // Kept for the rest of the process: one list for each set of plug-in commands installed. - let e: &'static [Entry] = Box::leak(v.into_boxed_slice()); - *with = Some((generation, e)); - e + WITH_PLUGINS.get((), generation, || { + let mut v = base.to_vec(); + v.extend(vectorcraft_engine::cmd::plugin::command_specs().into_iter().map(|c| Entry { + key: c.id.into(), + label: c.label.into(), + group: c.menu.join(" › "), + is_tool: false, + })); + v.into() + }) } /// [`entries`] without the plug-in commands. -fn built_in_entries() -> &'static [Entry] { - static E: OnceLock> = OnceLock::new(); +fn built_in_entries() -> &'static Arc<[Entry]> { + static E: OnceLock> = OnceLock::new(); E.get_or_init(|| { let mut v: Vec = vectorcraft_tools::catalog::all_tools() .map(|t| Entry { key: format!("tool:{}", t.id), label: t.label.to_string(), group: "Tools".into(), is_tool: true }) @@ -430,9 +424,8 @@ fn built_in_entries() -> &'static [Entry] { } v.push(Entry { key: id.to_string(), label: label.to_string(), group: id_group(id), is_tool: false }); } - v + v.into() }) - .as_slice() } /// One Effect menu item a shortcut can be given (#3296): the effect's id, the command its item runs @@ -621,8 +614,8 @@ fn names_a_required_field(params: &str) -> bool { false } -pub fn entry(key: &str) -> Option<&'static Entry> { - entries().iter().find(|e| e.key == key) +pub fn entry(key: &str) -> Option { + entries().iter().find(|e| e.key == key).cloned() } /// Entries (other than `except`) whose effective shortcut equals `chord`. @@ -1217,7 +1210,7 @@ fn export_from_dialog(app: &mut VectorcraftApp) { fn drop_hidden(d: &mut Dialog, overrides: &Overrides) { for field in ["__recording", "__selected", "__conflict"] { let key = d.str(field); - if !key.is_empty() && entry(&key).is_none_or(|e| !listed(overrides, e)) { + if !key.is_empty() && entry(&key).is_none_or(|e| !listed(overrides, &e)) { d.fields.insert(field.into(), json!("")); } } @@ -1259,7 +1252,7 @@ mod tests { #[test] fn format_parse_round_trip_for_every_default() { - for e in entries() { + for e in entries().iter() { if let Some(s) = default_of(&e.key) && let Some(sc) = crate::shortcuts::parse(s) { @@ -1574,7 +1567,7 @@ mod tests { #[test] fn duplicate_can_be_given_a_shortcut() { // Duplicate has no default chord (#1539): it must still be listed so one can be set. - assert_eq!(entry("edit.duplicate").map(|e| e.group.as_str()), Some("Edit")); + assert_eq!(entry("edit.duplicate").map(|e| e.group).as_deref(), Some("Edit")); let mut ov = Overrides::new(); assign(&mut ov, "edit.duplicate", Some("Cmd+Alt+Shift+F8"), false).unwrap(); assert_eq!(effective_in(&ov, "edit.duplicate").as_deref(), Some("Cmd+Alt+Shift+F8")); @@ -1647,10 +1640,11 @@ mod tests { // whose params are all optional, one the menu bar runs without params (Rotate… opens its // dialog) and one that opens a dialog for its values (New Variable). let none = Overrides::new(); - let dropped: Vec<&str> = entries().iter().filter(|e| default_of(&e.key).is_some() && !listed(&none, e)).map(|e| e.key.as_str()).collect(); + let all = entries(); + let dropped: Vec<&str> = all.iter().filter(|e| default_of(&e.key).is_some() && !listed(&none, e)).map(|e| e.key.as_str()).collect(); assert!(dropped.is_empty(), "{dropped:?}"); for id in ["edit.duplicate", "object.pathfinder.unite", "object.rotate", "variable.define", "object.distributeSpacing"] { - assert!(entry(id).is_some_and(|e| listed(&none, e)), "{id}"); + assert!(entry(id).is_some_and(|e| listed(&none, &e)), "{id}"); } for (doc, required) in [ ("{}", false), @@ -1679,12 +1673,12 @@ mod tests { let others = ["swatch.unused", "graphicStyle.unused", "document.info", "view.proofSetup", "magicWand.set", "edit.findNext"]; for id in NEEDS_A_PARAM.iter().chain(&others) { let e = entry(id).unwrap_or_else(|| panic!("{id} has an entry")); - assert!(!listed(&none, e), "{id}"); + assert!(!listed(&none, &e), "{id}"); assert_eq!(default_of(id), None, "{id}"); } let mut ov = Overrides::new(); crate::keymap::set_override(&mut ov, "transparency.set", "Cmd+Alt+Shift+F10"); - assert!(entry("transparency.set").is_some_and(|e| listed(&ov, e))); + assert!(entry("transparency.set").is_some_and(|e| listed(&ov, &e))); let mut app = VectorcraftApp::new(vectorcraft_engine::Session::new(), Default::default()); for id in ["transparency.set", "transparency.setOpacityMask"] { let list = run_command(&mut app, "shortcuts.list", &json!({ "query": id })).unwrap().unwrap(); @@ -1699,7 +1693,7 @@ mod tests { "addon.color.printFixes", "edit.findReplace", ] { - assert!(entry(id).is_some_and(|e| listed(&none, e)), "{id}"); + assert!(entry(id).is_some_and(|e| listed(&none, &e)), "{id}"); } assert_eq!(menu_target("object.path.cleanUp").map(|(id, _)| id).as_deref(), Some("ui.paramDialog")); assert_eq!( diff --git a/crates/ui-egui/src/shortcuts/tests.rs b/crates/ui-egui/src/shortcuts/tests.rs index 5fbffb533..bdbeb543c 100644 --- a/crates/ui-egui/src/shortcuts/tests.rs +++ b/crates/ui-egui/src/shortcuts/tests.rs @@ -510,7 +510,7 @@ fn cmd_alt_k_and_cmd_alt_shift_f_highlight_the_character_fields() { assert!(app.ui.dialog.is_none(), "not Preferences"); // Keyboard Shortcuts lists all three under Type; Highlight Tracking has no default key. for id in ["type.highlightFontName", "type.highlightKerning", "type.highlightTracking"] { - assert_eq!(crate::shortcut_editor::entry(id).map(|e| e.group.as_str()), Some("Type"), "{id}"); + assert_eq!(crate::shortcut_editor::entry(id).map(|e| e.group).as_deref(), Some("Type"), "{id}"); } assert_eq!(crate::menus::shortcut_of("type.highlightTracking"), None); } @@ -2347,7 +2347,7 @@ fn all_registered_shortcuts_parse() { /// dialog for the selection. #[test] fn an_effect_menu_item_takes_a_shortcut_of_its_own() { - let entry = crate::shortcut_editor::entries().iter().find(|e| e.key == "effect:distort.transform").expect("Transform is listed"); + let entry = crate::shortcut_editor::entry("effect:distort.transform").expect("Transform is listed"); assert_eq!(entry.group, "Effect › Distort & Transform"); assert_eq!(crate::keymap::command_of("effect:distort.transform").id, "effect.dialog"); let mut app = VectorcraftApp::new(vectorcraft_engine::Session::new(), Default::default()); diff --git a/crates/ui-egui/src/tests/plugins.rs b/crates/ui-egui/src/tests/plugins.rs index 3f898f49f..e6f3cf74a 100644 --- a/crates/ui-egui/src/tests/plugins.rs +++ b/crates/ui-egui/src/tests/plugins.rs @@ -220,7 +220,7 @@ fn plugin_commands_are_menu_items_and_shortcut_entries() { assert!(!entries.iter().any(|e| e.command.as_deref() == Some(size) && e.path.first().map(String::as_str) != Some("Object")), "not in File"); assert!(!menus::menu_strings().contains("Cmd Test"), "the plug-in's name is not text to translate"); - let listed = crate::shortcut_editor::entries().iter().find(|e| e.key == grow).expect("in Keyboard Shortcuts"); + let listed = crate::shortcut_editor::entry(grow).expect("in Keyboard Shortcuts"); assert_eq!((listed.label.as_str(), listed.group.as_str()), ("Grow", "Object › Plug-ins › Cmd Test")); assert_eq!(crate::shortcut_editor::default_of(grow), None, "a manifest gives no shortcut"); assert!(crate::keymap::template().bindable(&crate::keymap::command_of(grow)).is_some(), "a shortcut can be given to it"); @@ -238,6 +238,7 @@ fn plugin_commands_are_menu_items_and_shortcut_entries() { app.run("plugin.remove", json!({"id": id})).unwrap(); assert!(menus::menu_entries(&app).iter().all(|e| e.command.as_deref() != Some(grow))); assert!(crate::shortcut_editor::entries().iter().all(|e| e.key != grow)); + assert!(crate::keymap::template().bindable(&crate::keymap::command_of(grow)).is_none()); } /// A shortcut the user gives a plug-in command runs it.