diff --git a/Cargo.lock b/Cargo.lock index 9dfbc7854d86..afbd45d4c2e4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6597,6 +6597,7 @@ dependencies = [ "re_tracing", "serde", "serde_json", + "smallvec", "strum", "strum_macros", "sublime_fuzzy", diff --git a/crates/viewer/re_time_panel/src/time_control_ui.rs b/crates/viewer/re_time_panel/src/time_control_ui.rs index a95c9868bfca..b802aa088dea 100644 --- a/crates/viewer/re_time_panel/src/time_control_ui.rs +++ b/crates/viewer/re_time_panel/src/time_control_ui.rs @@ -237,8 +237,10 @@ You can also define your own timelines, e.g. for sensor time or camera frame num } fn toggle_playback_text(egui_ctx: &egui::Context) -> String { - if let Some(shortcut) = re_ui::UICommand::PlaybackTogglePlayPause.kb_shortcut() { - format!(" Toggle with {}", egui_ctx.format_shortcut(&shortcut)) + if let Some(shortcut_text) = + re_ui::UICommand::PlaybackTogglePlayPause.formatted_kb_shortcut(egui_ctx) + { + format!(" Toggle with {shortcut_text}") } else { Default::default() } diff --git a/crates/viewer/re_ui/Cargo.toml b/crates/viewer/re_ui/Cargo.toml index e7c2c7b87d4a..ba0b0bc3d4a9 100644 --- a/crates/viewer/re_ui/Cargo.toml +++ b/crates/viewer/re_ui/Cargo.toml @@ -37,17 +37,18 @@ re_log_types.workspace = true # syntax-highlighting for EntityPath re_tracing.workspace = true eframe = { workspace = true, default-features = false, features = ["wgpu"] } -egui.workspace = true egui_commonmark = { workspace = true, features = ["pulldown_cmark"] } egui_extras.workspace = true egui_tiles.workspace = true +egui.workspace = true once_cell.workspace = true parking_lot.workspace = true rand.workspace = true serde = { workspace = true, features = ["derive"] } serde_json.workspace = true -strum.workspace = true +smallvec.workspace = true strum_macros.workspace = true +strum.workspace = true sublime_fuzzy.workspace = true diff --git a/crates/viewer/re_ui/src/command.rs b/crates/viewer/re_ui/src/command.rs index f42fc755dde1..6781a2978feb 100644 --- a/crates/viewer/re_ui/src/command.rs +++ b/crates/viewer/re_ui/src/command.rs @@ -1,4 +1,5 @@ -use egui::{Key, KeyboardShortcut, Modifiers}; +use egui::{os::OperatingSystem, Key, KeyboardShortcut, Modifiers}; +use smallvec::{smallvec, SmallVec}; /// Interface for sending [`UICommand`] messages. pub trait UICommandSender { @@ -259,12 +260,16 @@ impl UICommand { } } - #[allow(clippy::unnecessary_wraps)] // Only on some platforms - pub fn kb_shortcut(self) -> Option { + /// All keyboard shortcuts, with the primary first. + pub fn kb_shortcuts(self, os: OperatingSystem) -> SmallVec<[KeyboardShortcut; 2]> { fn key(key: Key) -> KeyboardShortcut { KeyboardShortcut::new(Modifiers::NONE, key) } + fn ctrl(key: Key) -> KeyboardShortcut { + KeyboardShortcut::new(Modifiers::CTRL, key) + } + fn cmd(key: Key) -> KeyboardShortcut { KeyboardShortcut::new(Modifiers::COMMAND, key) } @@ -282,92 +287,113 @@ impl UICommand { } match self { - Self::SaveRecording => Some(cmd(Key::S)), - Self::SaveRecordingSelection => Some(cmd_alt(Key::S)), - Self::SaveBlueprint => None, - Self::Open => Some(cmd(Key::O)), - Self::Import => Some(cmd_shift(Key::O)), - Self::CloseCurrentRecording => None, - Self::CloseAllRecordings => None, - - Self::Undo => Some(cmd(Key::Z)), - Self::Redo => Some(cmd_shift(Key::Z)), + Self::SaveRecording => smallvec![cmd(Key::S)], + Self::SaveRecordingSelection => smallvec![cmd_alt(Key::S)], + Self::SaveBlueprint => smallvec![], + Self::Open => smallvec![cmd(Key::O)], + Self::Import => smallvec![cmd_shift(Key::O)], + Self::CloseCurrentRecording => smallvec![], + Self::CloseAllRecordings => smallvec![], + + Self::Undo => smallvec![cmd(Key::Z)], + Self::Redo => { + if os == OperatingSystem::Mac { + smallvec![cmd_shift(Key::Z), cmd(Key::Y)] + } else { + smallvec![ctrl(Key::Y), ctrl_shift(Key::Z)] + } + } #[cfg(all(not(target_arch = "wasm32"), target_os = "windows"))] - Self::Quit => Some(KeyboardShortcut::new(Modifiers::ALT, Key::F4)), + Self::Quit => smallvec![KeyboardShortcut::new(Modifiers::ALT, Key::F4)], - Self::OpenWebHelp => None, - Self::OpenRerunDiscord => None, + Self::OpenWebHelp => smallvec![], + Self::OpenRerunDiscord => smallvec![], #[cfg(all(not(target_arch = "wasm32"), not(target_os = "windows")))] - Self::Quit => Some(cmd(Key::Q)), + Self::Quit => smallvec![cmd(Key::Q)], - Self::ResetViewer => Some(ctrl_shift(Key::R)), - Self::ClearAndGenerateBlueprint => None, + Self::ResetViewer => smallvec![ctrl_shift(Key::R)], + Self::ClearAndGenerateBlueprint => smallvec![], #[cfg(not(target_arch = "wasm32"))] - Self::OpenProfiler => Some(ctrl_shift(Key::P)), - Self::ToggleMemoryPanel => Some(ctrl_shift(Key::M)), - Self::TogglePanelStateOverrides => None, - Self::ToggleTopPanel => None, - Self::ToggleBlueprintPanel => Some(ctrl_shift(Key::B)), - Self::ToggleSelectionPanel => Some(ctrl_shift(Key::S)), - Self::ToggleTimePanel => Some(ctrl_shift(Key::T)), - Self::ToggleChunkStoreBrowser => Some(ctrl_shift(Key::D)), - Self::Settings => { - if cfg!(target_os = "macos") { - Some(KeyboardShortcut::new(Modifiers::MAC_CMD, Key::Comma)) - } else { - // TODO(emilk): shortcut for web and non-mac too - None - } - } + Self::OpenProfiler => smallvec![ctrl_shift(Key::P)], + Self::ToggleMemoryPanel => smallvec![ctrl_shift(Key::M)], + Self::TogglePanelStateOverrides => smallvec![], + Self::ToggleTopPanel => smallvec![], + Self::ToggleBlueprintPanel => smallvec![ctrl_shift(Key::B)], + Self::ToggleSelectionPanel => smallvec![ctrl_shift(Key::S)], + Self::ToggleTimePanel => smallvec![ctrl_shift(Key::T)], + Self::ToggleChunkStoreBrowser => smallvec![ctrl_shift(Key::D)], + Self::Settings => smallvec![cmd(Key::Comma)], #[cfg(debug_assertions)] - Self::ToggleBlueprintInspectionPanel => Some(ctrl_shift(Key::I)), + Self::ToggleBlueprintInspectionPanel => smallvec![ctrl_shift(Key::I)], #[cfg(debug_assertions)] - Self::ToggleEguiDebugPanel => Some(ctrl_shift(Key::U)), + Self::ToggleEguiDebugPanel => smallvec![ctrl_shift(Key::U)], #[cfg(not(target_arch = "wasm32"))] - Self::ToggleFullscreen => Some(key(Key::F11)), + Self::ToggleFullscreen => smallvec![key(Key::F11)], #[cfg(target_arch = "wasm32")] - Self::ToggleFullscreen => None, + Self::ToggleFullscreen => smallvec![], #[cfg(not(target_arch = "wasm32"))] - Self::ZoomIn => Some(egui::gui_zoom::kb_shortcuts::ZOOM_IN), + Self::ZoomIn => smallvec![egui::gui_zoom::kb_shortcuts::ZOOM_IN], #[cfg(not(target_arch = "wasm32"))] - Self::ZoomOut => Some(egui::gui_zoom::kb_shortcuts::ZOOM_OUT), + Self::ZoomOut => smallvec![egui::gui_zoom::kb_shortcuts::ZOOM_OUT], #[cfg(not(target_arch = "wasm32"))] - Self::ZoomReset => Some(egui::gui_zoom::kb_shortcuts::ZOOM_RESET), + Self::ZoomReset => smallvec![egui::gui_zoom::kb_shortcuts::ZOOM_RESET], - Self::ToggleCommandPalette => Some(cmd(Key::P)), + Self::ToggleCommandPalette => smallvec![cmd(Key::P)], - Self::PlaybackTogglePlayPause => Some(key(Key::Space)), - Self::PlaybackFollow => Some(cmd(Key::ArrowRight)), - Self::PlaybackStepBack => Some(key(Key::ArrowLeft)), - Self::PlaybackStepForward => Some(key(Key::ArrowRight)), - Self::PlaybackRestart => Some(cmd(Key::ArrowLeft)), + Self::PlaybackTogglePlayPause => smallvec![key(Key::Space)], + Self::PlaybackFollow => smallvec![cmd(Key::ArrowRight)], + Self::PlaybackStepBack => smallvec![key(Key::ArrowLeft)], + Self::PlaybackStepForward => smallvec![key(Key::ArrowRight)], + Self::PlaybackRestart => smallvec![cmd(Key::ArrowLeft)], #[cfg(not(target_arch = "wasm32"))] - Self::ScreenshotWholeApp => None, + Self::ScreenshotWholeApp => smallvec![], #[cfg(not(target_arch = "wasm32"))] - Self::PrintChunkStore => None, + Self::PrintChunkStore => smallvec![], #[cfg(not(target_arch = "wasm32"))] - Self::PrintBlueprintStore => None, + Self::PrintBlueprintStore => smallvec![], #[cfg(not(target_arch = "wasm32"))] - Self::PrintPrimaryCache => None, + Self::PrintPrimaryCache => smallvec![], #[cfg(debug_assertions)] - Self::ResetEguiMemory => None, + Self::ResetEguiMemory => smallvec![], #[cfg(target_arch = "wasm32")] - Self::CopyDirectLink => None, + Self::CopyDirectLink => smallvec![], #[cfg(target_arch = "wasm32")] - Self::RestartWithWebGl => None, + Self::RestartWithWebGl => smallvec![], #[cfg(target_arch = "wasm32")] - Self::RestartWithWebGpu => None, + Self::RestartWithWebGpu => smallvec![], + } + } + + /// Primary keyboard shortcut + fn primary_kb_shortcut(self, os: OperatingSystem) -> Option { + self.kb_shortcuts(os).first().copied() + } + + /// Return the keyboard shortcut for this command, nicely formatted + pub fn formatted_kb_shortcut(self, egui_ctx: &egui::Context) -> Option { + // Note: we only show the primary shortcut to the user. + // The fallbacks are there for people who have muscle memory for the other shortcuts. + self.primary_kb_shortcut(egui_ctx.os()) + .map(|shortcut| egui_ctx.format_shortcut(&shortcut)) + } + + /// Add e.g. " (Ctrl+F11)" as a suffix + pub fn format_shortcut_tooltip_suffix(self, egui_ctx: &egui::Context) -> String { + if let Some(shortcut_text) = self.formatted_kb_shortcut(egui_ctx) { + format!(" ({shortcut_text})") + } else { + Default::default() } } @@ -393,14 +419,18 @@ impl UICommand { } let mut commands: Vec<(KeyboardShortcut, Self)> = Self::iter() - .filter_map(|cmd| cmd.kb_shortcut().map(|kb_shortcut| (kb_shortcut, cmd))) + .flat_map(|cmd| { + cmd.kb_shortcuts(egui_ctx.os()) + .into_iter() + .map(move |kb_shortcut| (kb_shortcut, cmd)) + }) .collect(); // If the user pressed `Cmd-Shift-S` then egui will match that // with both `Cmd-Shift-S` and `Cmd-S`. // The reason is that `Shift` (and `Alt`) are sometimes required to produce certain keys, // such as `+` (`Shift =` on an american keyboard). - // The result of this is that we bust check for `Cmd-Shift-S` before `Cmd-S`, etc. + // The result of this is that we must check for `Cmd-Shift-S` before `Cmd-S`, etc. // So we order the commands here so that the commands with `Shift` and `Alt` in them // are checked first. commands.sort_by_key(|(kb_shortcut, _cmd)| { @@ -453,22 +483,13 @@ impl UICommand { egui::Button::new(self.text()) }; - if let Some(shortcut) = self.kb_shortcut() { - button = button.shortcut_text(egui_ctx.format_shortcut(&shortcut)); + if let Some(shortcut_text) = self.formatted_kb_shortcut(egui_ctx) { + button = button.shortcut_text(shortcut_text); } button } - /// Add e.g. " (Ctrl+F11)" as a suffix - pub fn format_shortcut_tooltip_suffix(self, egui_ctx: &egui::Context) -> String { - if let Some(kb_shortcut) = self.kb_shortcut() { - format!(" ({})", egui_ctx.format_shortcut(&kb_shortcut)) - } else { - Default::default() - } - } - pub fn tooltip_with_shortcut(self, egui_ctx: &egui::Context) -> String { format!( "{}{}", @@ -499,19 +520,25 @@ fn check_for_clashing_command_shortcuts() { use strum::IntoEnumIterator as _; - for a_cmd in UICommand::iter() { - if let Some(a_shortcut) = a_cmd.kb_shortcut() { - for b_cmd in UICommand::iter() { - if a_cmd == b_cmd { - continue; - } - if let Some(b_shortcut) = b_cmd.kb_shortcut() { - assert!( - !clashes(a_shortcut, b_shortcut), - "Command '{a_cmd:?}' and '{b_cmd:?}' have overlapping keyboard shortcuts: {:?} vs {:?}", - a_shortcut.format(&egui::ModifierNames::NAMES, true), - b_shortcut.format(&egui::ModifierNames::NAMES, true), - ); + for os in [ + OperatingSystem::Mac, + OperatingSystem::Windows, + OperatingSystem::Nix, + ] { + for a_cmd in UICommand::iter() { + for a_shortcut in a_cmd.kb_shortcuts(os) { + for b_cmd in UICommand::iter() { + if a_cmd == b_cmd { + continue; + } + for b_shortcut in b_cmd.kb_shortcuts(os) { + assert!( + !clashes(a_shortcut, b_shortcut), + "Command '{a_cmd:?}' and '{b_cmd:?}' have overlapping keyboard shortcuts: {:?} vs {:?}", + a_shortcut.format(&egui::ModifierNames::NAMES, true), + b_shortcut.format(&egui::ModifierNames::NAMES, true), + ); + } } } } diff --git a/crates/viewer/re_ui/src/command_palette.rs b/crates/viewer/re_ui/src/command_palette.rs index 0a1c0d2c5b49..3ac097340b17 100644 --- a/crates/viewer/re_ui/src/command_palette.rs +++ b/crates/viewer/re_ui/src/command_palette.rs @@ -99,10 +99,7 @@ impl CommandPalette { for (i, fuzzy_match) in commands_that_match(&query).iter().enumerate() { let command = fuzzy_match.command; - let kb_shortcut = command - .kb_shortcut() - .map(|shortcut| ui.ctx().format_shortcut(&shortcut)) - .unwrap_or_default(); + let kb_shortcut_text = command.formatted_kb_shortcut(ui.ctx()).unwrap_or_default(); let (rect, response) = ui.allocate_at_least( egui::vec2(ui.available_width(), item_height), @@ -148,7 +145,7 @@ impl CommandPalette { ui.painter().text( rect.right_center(), Align2::RIGHT_CENTER, - kb_shortcut, + kb_shortcut_text, font_id.clone(), if selected { style.text_color()