objdiff-gui: Implement keyboard shortcuts (#139)

* Fix missing dependency feature for objdiff-gui

* Update .gitignore

* Add enter and back hotkeys

* Add scroll hotkeys

* Add hotkeys to select the next symbol above/below the current one in the listing

* Do not clear highlighted symbol when backing out of diff view

* Do not clear highlighted symbol when hovering mouse over an unpaired symbol

* Auto-scroll the keyboard-selected symbols into view if offscreen

* Fix some hotkeys stealing input from focused widgets

e.g. The symbol list was stealing the W/S key presses when typing into the symbol filter text edit.

If the user actually wants to use these shortcuts while a widget is focused, they can simply press the escape key to unfocus all widgets and then press the shortcut.

* Add Ctrl+F/S shortcuts for focusing the object and symbol filter text edits

* Add space as alternative to enter hotkey

This is for consistency with egui's builtint enter/space hotkey for interacting with the focused widget.

* Add hotkeys to change target and base functions

* Split function diff view: Enable PageUp/PageDown/Home/End for scrolling

* Add escape as an alternative to back hotkey

* Fix auto-scrolling to highlighted symbol only working for the left side

The flag is cleared after one scroll to avoid doing it continuously, but this breaks when we need to scroll to both the left and the right symbol at the same time. So now each side has its own flag to keep track of this state independently.

* Simplify clearing of the autoscroll flag, remove &mut State

* Found a better place to clear the autoscroll flag

DiffViewState::post_update is where the flag gets set, so clearing it right before that at the start of the function seems to make the most sense, instead of doing it in App::update.

.gitignore

@@ -18,4 +18,4 @@ android.keystore
 *.frag
 *.vert
 *.metal
-.vscode/launch.json
+.vscode/

objdiff-gui/Cargo.toml

@@ -95,7 +95,7 @@ exec = "0.3"
 
 # native:
 [target.'cfg(not(target_arch = "wasm32"))'.dependencies]
-tracing-subscriber = "0.3"
+tracing-subscriber = { version = "0.3", features = ["env-filter"] }
 
 # web:
 [target.'cfg(target_arch = "wasm32")'.dependencies]

objdiff-gui/src/hotkeys.rs

@@ -0,0 +1,108 @@
+use egui::{
+    style::ScrollAnimation, vec2, Context, Key, KeyboardShortcut, Modifiers, PointerButton,
+};
+
+fn any_widget_focused(ctx: &Context) -> bool { ctx.memory(|mem| mem.focused().is_some()) }
+
+pub fn enter_pressed(ctx: &Context) -> bool {
+    if any_widget_focused(ctx) {
+        return false;
+    }
+    ctx.input_mut(|i| {
+        i.key_pressed(Key::Enter)
+            || i.key_pressed(Key::Space)
+            || i.pointer.button_pressed(PointerButton::Extra2)
+    })
+}
+
+pub fn back_pressed(ctx: &Context) -> bool {
+    if any_widget_focused(ctx) {
+        return false;
+    }
+    ctx.input_mut(|i| {
+        i.key_pressed(Key::Backspace)
+            || i.key_pressed(Key::Escape)
+            || i.pointer.button_pressed(PointerButton::Extra1)
+    })
+}
+
+pub fn up_pressed(ctx: &Context) -> bool {
+    if any_widget_focused(ctx) {
+        return false;
+    }
+    ctx.input_mut(|i| i.key_pressed(Key::ArrowUp) || i.key_pressed(Key::W))
+}
+
+pub fn down_pressed(ctx: &Context) -> bool {
+    if any_widget_focused(ctx) {
+        return false;
+    }
+    ctx.input_mut(|i| i.key_pressed(Key::ArrowDown) || i.key_pressed(Key::S))
+}
+
+pub fn page_up_pressed(ctx: &Context) -> bool { ctx.input_mut(|i| i.key_pressed(Key::PageUp)) }
+
+pub fn page_down_pressed(ctx: &Context) -> bool { ctx.input_mut(|i| i.key_pressed(Key::PageDown)) }
+
+pub fn home_pressed(ctx: &Context) -> bool { ctx.input_mut(|i| i.key_pressed(Key::Home)) }
+
+pub fn end_pressed(ctx: &Context) -> bool { ctx.input_mut(|i| i.key_pressed(Key::End)) }
+
+pub fn check_scroll_hotkeys(ui: &mut egui::Ui, include_small_increments: bool) {
+    let ui_height = ui.available_rect_before_wrap().height();
+    if up_pressed(ui.ctx()) && include_small_increments {
+        ui.scroll_with_delta_animation(vec2(0.0, ui_height / 10.0), ScrollAnimation::none());
+    } else if down_pressed(ui.ctx()) && include_small_increments {
+        ui.scroll_with_delta_animation(vec2(0.0, -ui_height / 10.0), ScrollAnimation::none());
+    } else if page_up_pressed(ui.ctx()) {
+        ui.scroll_with_delta_animation(vec2(0.0, ui_height), ScrollAnimation::none());
+    } else if page_down_pressed(ui.ctx()) {
+        ui.scroll_with_delta_animation(vec2(0.0, -ui_height), ScrollAnimation::none());
+    } else if home_pressed(ui.ctx()) {
+        ui.scroll_with_delta_animation(vec2(0.0, f32::INFINITY), ScrollAnimation::none());
+    } else if end_pressed(ui.ctx()) {
+        ui.scroll_with_delta_animation(vec2(0.0, -f32::INFINITY), ScrollAnimation::none());
+    }
+}
+
+pub fn consume_up_key(ctx: &Context) -> bool {
+    if any_widget_focused(ctx) {
+        return false;
+    }
+    ctx.input_mut(|i| {
+        i.consume_key(Modifiers::NONE, Key::ArrowUp) || i.consume_key(Modifiers::NONE, Key::W)
+    })
+}
+
+pub fn consume_down_key(ctx: &Context) -> bool {
+    if any_widget_focused(ctx) {
+        return false;
+    }
+    ctx.input_mut(|i| {
+        i.consume_key(Modifiers::NONE, Key::ArrowDown) || i.consume_key(Modifiers::NONE, Key::S)
+    })
+}
+
+const OBJECT_FILTER_SHORTCUT: KeyboardShortcut = KeyboardShortcut::new(Modifiers::CTRL, Key::F);
+
+pub fn consume_object_filter_shortcut(ctx: &Context) -> bool {
+    ctx.input_mut(|i| i.consume_shortcut(&OBJECT_FILTER_SHORTCUT))
+}
+
+const SYMBOL_FILTER_SHORTCUT: KeyboardShortcut = KeyboardShortcut::new(Modifiers::CTRL, Key::S);
+
+pub fn consume_symbol_filter_shortcut(ctx: &Context) -> bool {
+    ctx.input_mut(|i| i.consume_shortcut(&SYMBOL_FILTER_SHORTCUT))
+}
+
+const CHANGE_TARGET_SHORTCUT: KeyboardShortcut = KeyboardShortcut::new(Modifiers::CTRL, Key::T);
+
+pub fn consume_change_target_shortcut(ctx: &Context) -> bool {
+    ctx.input_mut(|i| i.consume_shortcut(&CHANGE_TARGET_SHORTCUT))
+}
+
+const CHANGE_BASE_SHORTCUT: KeyboardShortcut = KeyboardShortcut::new(Modifiers::CTRL, Key::B);
+
+pub fn consume_change_base_shortcut(ctx: &Context) -> bool {
+    ctx.input_mut(|i| i.consume_shortcut(&CHANGE_BASE_SHORTCUT))
+}

objdiff-gui/src/main.rs

@@ -4,6 +4,7 @@ mod app;
 mod app_config;
 mod config;
 mod fonts;
+mod hotkeys;
 mod jobs;
 mod update;
 mod views;

objdiff-gui/src/views/config.rs

@@ -21,6 +21,7 @@ use strum::{EnumMessage, VariantArray};
 use crate::{
     app::{AppConfig, AppState, AppStateRef, ObjectConfig},
     config::ProjectObjectNode,
+    hotkeys,
     jobs::{
         check_update::{start_check_update, CheckUpdateResult},
         update::start_update,
@@ -254,7 +255,11 @@ pub fn config_ui(
         }
     } else {
         let had_search = !config_state.object_search.is_empty();
-        egui::TextEdit::singleline(&mut config_state.object_search).hint_text("Filter").ui(ui);
+        let response =
+            egui::TextEdit::singleline(&mut config_state.object_search).hint_text("Filter").ui(ui);
+        if hotkeys::consume_object_filter_shortcut(ui.ctx()) {
+            response.request_focus();
+        }
 
         let mut root_open = None;
         let mut node_open = NodeOpen::Default;

objdiff-gui/src/views/data_diff.rs

@@ -7,11 +7,14 @@ use objdiff_core::{
 };
 use time::format_description;
 
-use crate::views::{
-    appearance::Appearance,
-    column_layout::{render_header, render_table},
-    symbol_diff::{DiffViewAction, DiffViewNavigation, DiffViewState},
-    write_text,
+use crate::{
+    hotkeys,
+    views::{
+        appearance::Appearance,
+        column_layout::{render_header, render_table},
+        symbol_diff::{DiffViewAction, DiffViewNavigation, DiffViewState},
+        write_text,
+    },
 };
 
 const BYTES_PER_ROW: usize = 16;
@@ -176,6 +179,8 @@ fn data_table_ui(
     let left_diffs = left_section.map(|(_, section)| split_diffs(&section.data_diff));
     let right_diffs = right_section.map(|(_, section)| split_diffs(&section.data_diff));
 
+    hotkeys::check_scroll_hotkeys(ui, true);
+
     render_table(ui, available_width, 2, config.code_font.size, total_rows, |row, column| {
         let i = row.index();
         let address = i * BYTES_PER_ROW;
@@ -224,7 +229,7 @@ pub fn data_diff_ui(
     render_header(ui, available_width, 2, |ui, column| {
         if column == 0 {
             // Left column
-            if ui.button("⏴ Back").clicked() {
+            if ui.button("⏴ Back").clicked() || hotkeys::back_pressed(ui.ctx()) {
                 ret = Some(DiffViewAction::Navigate(DiffViewNavigation::symbol_diff()));
             }
 

objdiff-gui/src/views/extab_diff.rs

@@ -5,13 +5,16 @@ use objdiff_core::{
 };
 use time::format_description;
 
-use crate::views::{
-    appearance::Appearance,
-    column_layout::{render_header, render_strips},
-    function_diff::FunctionDiffContext,
-    symbol_diff::{
-        match_color_for_symbol, DiffViewAction, DiffViewNavigation, DiffViewState, SymbolRefByName,
-        View,
+use crate::{
+    hotkeys,
+    views::{
+        appearance::Appearance,
+        column_layout::{render_header, render_strips},
+        function_diff::FunctionDiffContext,
+        symbol_diff::{
+            match_color_for_symbol, DiffViewAction, DiffViewNavigation, DiffViewState,
+            SymbolRefByName, View,
+        },
     },
 };
 
@@ -136,7 +139,7 @@ pub fn extab_diff_ui(
         if column == 0 {
             // Left column
             ui.horizontal(|ui| {
-                if ui.button("⏴ Back").clicked() {
+                if ui.button("⏴ Back").clicked() || hotkeys::back_pressed(ui.ctx()) {
                     ret = Some(DiffViewAction::Navigate(DiffViewNavigation::symbol_diff()));
                 }
                 ui.separator();
@@ -232,6 +235,8 @@ pub fn extab_diff_ui(
         }
     });
 
+    hotkeys::check_scroll_hotkeys(ui, true);
+
     // Table
     render_strips(ui, available_width, 2, |ui, column| {
         if column == 0 {

objdiff-gui/src/views/function_diff.rs

@@ -14,12 +14,15 @@ use objdiff_core::{
 };
 use time::format_description;
 
-use crate::views::{
-    appearance::Appearance,
-    column_layout::{render_header, render_strips, render_table},
-    symbol_diff::{
-        match_color_for_symbol, symbol_list_ui, DiffViewAction, DiffViewNavigation, DiffViewState,
-        SymbolDiffContext, SymbolFilter, SymbolRefByName, SymbolViewState, View,
+use crate::{
+    hotkeys,
+    views::{
+        appearance::Appearance,
+        column_layout::{render_header, render_strips, render_table},
+        symbol_diff::{
+            match_color_for_symbol, symbol_list_ui, DiffViewAction, DiffViewNavigation,
+            DiffViewState, SymbolDiffContext, SymbolFilter, SymbolRefByName, SymbolViewState, View,
+        },
     },
 };
 
@@ -149,6 +152,10 @@ fn ins_hover_ui(
                 ui.colored_label(appearance.highlight_color, "Extern".to_string());
             }
         }
+
+        if let Some(decoded) = rlwinmdec::decode(&ins.formatted) {
+            ui.colored_label(appearance.highlight_color, decoded.trim());
+        }
     });
 }
 
@@ -432,6 +439,7 @@ fn asm_table_ui(
     };
     if left_len.is_some() && right_len.is_some() {
         // Joint view
+        hotkeys::check_scroll_hotkeys(ui, true);
         render_table(
             ui,
             available_width,
@@ -467,6 +475,7 @@ fn asm_table_ui(
             if column == 0 {
                 if let Some(ctx) = left_ctx {
                     if ctx.has_symbol() {
+                        hotkeys::check_scroll_hotkeys(ui, false);
                         render_table(
                             ui,
                             available_width / 2.0,
@@ -512,9 +521,6 @@ fn asm_table_ui(
                                         SymbolRefByName::new(right_symbol, right_section),
                                     ));
                                 }
-                                DiffViewAction::SetSymbolHighlight(_, _) => {
-                                    // Ignore
-                                }
                                 _ => {
                                     ret = Some(action);
                                 }
@@ -527,6 +533,7 @@ fn asm_table_ui(
             } else if column == 1 {
                 if let Some(ctx) = right_ctx {
                     if ctx.has_symbol() {
+                        hotkeys::check_scroll_hotkeys(ui, false);
                         render_table(
                             ui,
                             available_width / 2.0,
@@ -572,9 +579,6 @@ fn asm_table_ui(
                                         right_symbol_ref,
                                     ));
                                 }
-                                DiffViewAction::SetSymbolHighlight(_, _) => {
-                                    // Ignore
-                                }
                                 _ => {
                                     ret = Some(action);
                                 }
@@ -675,7 +679,7 @@ pub fn function_diff_ui(
         if column == 0 {
             // Left column
             ui.horizontal(|ui| {
-                if ui.button("⏴ Back").clicked() {
+                if ui.button("⏴ Back").clicked() || hotkeys::back_pressed(ui.ctx()) {
                     ret = Some(DiffViewAction::Navigate(DiffViewNavigation::symbol_diff()));
                 }
                 ui.separator();
@@ -708,10 +712,11 @@ pub fn function_diff_ui(
                         .color(appearance.highlight_color),
                 );
                 if right_ctx.is_some_and(|m| m.has_symbol())
-                    && ui
+                    && (ui
                         .button("Change target")
                         .on_hover_text_at_pointer("Choose a different symbol to use as the target")
                         .clicked()
+                        || hotkeys::consume_change_target_shortcut(ui.ctx()))
                 {
                     if let Some(symbol_ref) = state.symbol_state.right_symbol.as_ref() {
                         ret = Some(DiffViewAction::SelectingLeft(symbol_ref.clone()));
@@ -785,6 +790,7 @@ pub fn function_diff_ui(
                                 "Choose a different symbol to use as the base",
                             )
                             .clicked()
+                            || hotkeys::consume_change_base_shortcut(ui.ctx())
                         {
                             if let Some(symbol_ref) = state.symbol_state.left_symbol.as_ref() {
                                 ret = Some(DiffViewAction::SelectingRight(symbol_ref.clone()));

objdiff-gui/src/views/rlwinm.rs

@@ -16,13 +16,13 @@ pub fn rlwinm_decode_window(
     egui::Window::new("Rlwinm Decoder").open(show).show(ctx, |ui| {
         ui.text_edit_singleline(&mut state.text);
         ui.add_space(10.0);
-        if let Some(demangled) = rlwinmdec::decode(&state.text) {
+        if let Some(decoded) = rlwinmdec::decode(&state.text) {
             ui.scope(|ui| {
                 ui.style_mut().override_text_style = Some(TextStyle::Monospace);
-                ui.colored_label(appearance.replace_color, &demangled);
+                ui.colored_label(appearance.replace_color, decoded.trim());
             });
             if ui.button("Copy").clicked() {
-                ui.output_mut(|output| output.copied_text = demangled);
+                ui.output_mut(|output| output.copied_text = decoded);
             }
         } else {
             ui.scope(|ui| {

objdiff-gui/src/views/symbol_diff.rs

@@ -1,8 +1,8 @@
-use std::{collections::BTreeMap, mem::take};
+use std::{collections::BTreeMap, mem::take, ops::Bound};
 
 use egui::{
-    text::LayoutJob, CollapsingHeader, Color32, Id, OpenUrl, ScrollArea, SelectableLabel, TextEdit,
-    Ui, Widget,
+    style::ScrollAnimation, text::LayoutJob, CollapsingHeader, Color32, Id, OpenUrl, ScrollArea,
+    SelectableLabel, TextEdit, Ui, Widget,
 };
 use objdiff_core::{
     arch::ObjArch,
@@ -15,6 +15,7 @@ use regex::{Regex, RegexBuilder};
 
 use crate::{
     app::AppStateRef,
+    hotkeys,
     jobs::{
         create_scratch::{start_create_scratch, CreateScratchConfig, CreateScratchResult},
         objdiff::{BuildStatus, ObjDiffResult},
@@ -56,8 +57,8 @@ pub enum DiffViewAction {
     Build,
     /// Navigate to a new diff view
     Navigate(DiffViewNavigation),
-    /// Set the highlighted symbols in the symbols view
-    SetSymbolHighlight(Option<SymbolRef>, Option<SymbolRef>),
+    /// Set the highlighted symbols in the symbols view, optionally scrolling them into view.
+    SetSymbolHighlight(Option<SymbolRef>, Option<SymbolRef>, bool),
     /// Set the symbols view search filter
     SetSearch(String),
     /// Submit the current function to decomp.me
@@ -135,6 +136,7 @@ pub struct DiffViewState {
 #[derive(Default)]
 pub struct SymbolViewState {
     pub highlighted_symbol: (Option<SymbolRef>, Option<SymbolRef>),
+    pub autoscroll_to_highlighted_symbols: bool,
     pub left_symbol: Option<SymbolRefByName>,
     pub right_symbol: Option<SymbolRefByName>,
     pub reverse_fn_order: bool,
@@ -197,6 +199,9 @@ impl DiffViewState {
             ctx.output_mut(|o| o.open_url = Some(OpenUrl::new_tab(result.scratch_url)));
         }
 
+        // Clear the autoscroll flag so that it doesn't scroll continuously.
+        self.symbol_state.autoscroll_to_highlighted_symbols = false;
+
         let Some(action) = action else {
             return;
         };
@@ -211,7 +216,6 @@ impl DiffViewState {
                     // Ignore action if we're already navigating
                     return;
                 }
-                self.symbol_state.highlighted_symbol = (None, None);
                 let Ok(mut state) = state.write() else {
                     return;
                 };
@@ -247,8 +251,9 @@ impl DiffViewState {
                     self.post_build_nav = Some(nav);
                 }
             }
-            DiffViewAction::SetSymbolHighlight(left, right) => {
+            DiffViewAction::SetSymbolHighlight(left, right, autoscroll) => {
                 self.symbol_state.highlighted_symbol = (left, right);
+                self.symbol_state.autoscroll_to_highlighted_symbols = autoscroll;
             }
             DiffViewAction::SetSearch(search) => {
                 self.search_regex = if search.is_empty() {
@@ -534,7 +539,15 @@ fn symbol_ui(
             ret = Some(DiffViewAction::Navigate(result));
         }
     });
-    if response.clicked() {
+    if selected && state.autoscroll_to_highlighted_symbols {
+        // Automatically scroll the view to encompass the selected symbol in case the user selected
+        // an offscreen symbol by using a keyboard shortcut.
+        ui.scroll_to_rect_animation(response.rect, None, ScrollAnimation::none());
+        // This autoscroll state flag will be reset in DiffViewState::post_update at the end of
+        // every frame so that we don't continuously scroll the view back when the user is trying to
+        // manually scroll away.
+    }
+    if response.clicked() || (selected && hotkeys::enter_pressed(ui.ctx())) {
         if let Some(section) = section {
             match section.kind {
                 ObjSectionKind::Code => {
@@ -561,20 +574,18 @@ fn symbol_ui(
             }
         }
     } else if response.hovered() {
-        ret = Some(if let Some(target_symbol) = symbol_diff.target_symbol {
-            if column == 0 {
-                DiffViewAction::SetSymbolHighlight(
-                    Some(symbol_diff.symbol_ref),
-                    Some(target_symbol),
-                )
-            } else {
-                DiffViewAction::SetSymbolHighlight(
-                    Some(target_symbol),
-                    Some(symbol_diff.symbol_ref),
-                )
-            }
+        ret = Some(if column == 0 {
+            DiffViewAction::SetSymbolHighlight(
+                Some(symbol_diff.symbol_ref),
+                symbol_diff.target_symbol,
+                false,
+            )
         } else {
-            DiffViewAction::SetSymbolHighlight(None, None)
+            DiffViewAction::SetSymbolHighlight(
+                symbol_diff.target_symbol,
+                Some(symbol_diff.symbol_ref),
+                false,
+            )
         });
     }
     ret
@@ -648,6 +659,58 @@ pub fn symbol_list_ui(
             }
         }
 
+        hotkeys::check_scroll_hotkeys(ui, false);
+
+        let mut new_key_value_to_highlight = None;
+        if let Some(sym_ref) =
+            if column == 0 { state.highlighted_symbol.0 } else { state.highlighted_symbol.1 }
+        {
+            let up = if hotkeys::consume_up_key(ui.ctx()) {
+                Some(true)
+            } else if hotkeys::consume_down_key(ui.ctx()) {
+                Some(false)
+            } else {
+                None
+            };
+            if let Some(mut up) = up {
+                if state.reverse_fn_order {
+                    up = !up;
+                }
+                new_key_value_to_highlight = if up {
+                    mapping.range(..sym_ref).next_back()
+                } else {
+                    mapping.range((Bound::Excluded(sym_ref), Bound::Unbounded)).next()
+                };
+            };
+        } else {
+            // No symbol is highlighted in this column. Select the topmost symbol instead.
+            // Note that we intentionally do not consume the up/down key presses in this case, but
+            // we do when a symbol is highlighted. This is so that if only one column has a symbol
+            // highlighted, that one takes precedence over the one with nothing highlighted.
+            if hotkeys::up_pressed(ui.ctx()) || hotkeys::down_pressed(ui.ctx()) {
+                new_key_value_to_highlight = if state.reverse_fn_order {
+                    mapping.last_key_value()
+                } else {
+                    mapping.first_key_value()
+                };
+            }
+        }
+        if let Some((new_sym_ref, new_symbol_diff)) = new_key_value_to_highlight {
+            ret = Some(if column == 0 {
+                DiffViewAction::SetSymbolHighlight(
+                    Some(*new_sym_ref),
+                    new_symbol_diff.target_symbol,
+                    true,
+                )
+            } else {
+                DiffViewAction::SetSymbolHighlight(
+                    new_symbol_diff.target_symbol,
+                    Some(*new_sym_ref),
+                    true,
+                )
+            });
+        }
+
         ui.scope(|ui| {
             ui.style_mut().override_text_style = Some(egui::TextStyle::Monospace);
             ui.style_mut().wrap_mode = Some(egui::TextWrapMode::Extend);
@@ -838,7 +901,11 @@ pub fn symbol_diff_ui(
             });
 
             let mut search = state.search.clone();
-            if TextEdit::singleline(&mut search).hint_text("Filter symbols").ui(ui).changed() {
+            let response = TextEdit::singleline(&mut search).hint_text("Filter symbols").ui(ui);
+            if hotkeys::consume_symbol_filter_shortcut(ui.ctx()) {
+                response.request_focus();
+            }
+            if response.changed() {
                 ret = Some(DiffViewAction::SetSearch(search));
             }
         } else if column == 1 {