From ef680a5e7da1e0820461d8bec6c4738070b9817e Mon Sep 17 00:00:00 2001 From: LagoLunatic Date: Wed, 27 Nov 2024 21:05:07 -0500 Subject: [PATCH 01/18] Fix missing dependency feature for objdiff-gui --- objdiff-gui/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/objdiff-gui/Cargo.toml b/objdiff-gui/Cargo.toml index aac88736..555872f2 100644 --- a/objdiff-gui/Cargo.toml +++ b/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] From 123253c322e8eb7c5c1127caf98f77aff1a1488c Mon Sep 17 00:00:00 2001 From: LagoLunatic Date: Thu, 28 Nov 2024 16:19:17 -0500 Subject: [PATCH 02/18] Update .gitignore --- .gitignore | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index c46c7ccb..e4a92588 100644 --- a/.gitignore +++ b/.gitignore @@ -18,4 +18,4 @@ android.keystore *.frag *.vert *.metal -.vscode/launch.json +.vscode/ From acc1189150fd12a51a933bc740463ad7a7c91fe5 Mon Sep 17 00:00:00 2001 From: LagoLunatic Date: Thu, 28 Nov 2024 16:26:30 -0500 Subject: [PATCH 03/18] Add enter and back hotkeys --- objdiff-gui/src/hotkeys.rs | 11 +++++++++++ objdiff-gui/src/main.rs | 1 + objdiff-gui/src/views/data_diff.rs | 15 +++++++++------ objdiff-gui/src/views/extab_diff.rs | 19 +++++++++++-------- objdiff-gui/src/views/function_diff.rs | 17 ++++++++++------- objdiff-gui/src/views/symbol_diff.rs | 3 ++- 6 files changed, 44 insertions(+), 22 deletions(-) create mode 100644 objdiff-gui/src/hotkeys.rs diff --git a/objdiff-gui/src/hotkeys.rs b/objdiff-gui/src/hotkeys.rs new file mode 100644 index 00000000..762e5db8 --- /dev/null +++ b/objdiff-gui/src/hotkeys.rs @@ -0,0 +1,11 @@ +use egui::{Context, Key, PointerButton}; + +pub fn enter_pressed(ctx: &Context) -> bool { + ctx.input_mut(|i| i.key_pressed(Key::Enter) || i.pointer.button_pressed(PointerButton::Extra2)) +} + +pub fn back_pressed(ctx: &Context) -> bool { + ctx.input_mut(|i| { + i.key_pressed(Key::Backspace) || i.pointer.button_pressed(PointerButton::Extra1) + }) +} diff --git a/objdiff-gui/src/main.rs b/objdiff-gui/src/main.rs index 18e6b7fc..0974a001 100644 --- a/objdiff-gui/src/main.rs +++ b/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; diff --git a/objdiff-gui/src/views/data_diff.rs b/objdiff-gui/src/views/data_diff.rs index 62379f04..70ae1dcd 100644 --- a/objdiff-gui/src/views/data_diff.rs +++ b/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; @@ -224,7 +227,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())); } diff --git a/objdiff-gui/src/views/extab_diff.rs b/objdiff-gui/src/views/extab_diff.rs index b0bbada4..ce59a3ab 100644 --- a/objdiff-gui/src/views/extab_diff.rs +++ b/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(); diff --git a/objdiff-gui/src/views/function_diff.rs b/objdiff-gui/src/views/function_diff.rs index dde736e4..997a3fc0 100644 --- a/objdiff-gui/src/views/function_diff.rs +++ b/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, + }, }, }; @@ -675,7 +678,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(); diff --git a/objdiff-gui/src/views/symbol_diff.rs b/objdiff-gui/src/views/symbol_diff.rs index 1ca66dd4..1dca5ded 100644 --- a/objdiff-gui/src/views/symbol_diff.rs +++ b/objdiff-gui/src/views/symbol_diff.rs @@ -15,6 +15,7 @@ use regex::{Regex, RegexBuilder}; use crate::{ app::AppStateRef, + hotkeys, jobs::{ create_scratch::{start_create_scratch, CreateScratchConfig, CreateScratchResult}, objdiff::{BuildStatus, ObjDiffResult}, @@ -534,7 +535,7 @@ fn symbol_ui( ret = Some(DiffViewAction::Navigate(result)); } }); - if response.clicked() { + if response.clicked() || (selected && hotkeys::enter_pressed(ui.ctx())) { if let Some(section) = section { match section.kind { ObjSectionKind::Code => { From dbf86ec3cff5ca048d02d621704776d0a146d3d1 Mon Sep 17 00:00:00 2001 From: LagoLunatic Date: Thu, 28 Nov 2024 16:32:20 -0500 Subject: [PATCH 04/18] Add scroll hotkeys --- objdiff-gui/src/hotkeys.rs | 35 +++++++++++++++++++++++++- objdiff-gui/src/views/data_diff.rs | 2 ++ objdiff-gui/src/views/extab_diff.rs | 2 ++ objdiff-gui/src/views/function_diff.rs | 1 + objdiff-gui/src/views/symbol_diff.rs | 2 ++ 5 files changed, 41 insertions(+), 1 deletion(-) diff --git a/objdiff-gui/src/hotkeys.rs b/objdiff-gui/src/hotkeys.rs index 762e5db8..ba2e6609 100644 --- a/objdiff-gui/src/hotkeys.rs +++ b/objdiff-gui/src/hotkeys.rs @@ -1,4 +1,4 @@ -use egui::{Context, Key, PointerButton}; +use egui::{style::ScrollAnimation, vec2, Context, Key, PointerButton}; pub fn enter_pressed(ctx: &Context) -> bool { ctx.input_mut(|i| i.key_pressed(Key::Enter) || i.pointer.button_pressed(PointerButton::Extra2)) @@ -9,3 +9,36 @@ pub fn back_pressed(ctx: &Context) -> bool { i.key_pressed(Key::Backspace) || i.pointer.button_pressed(PointerButton::Extra1) }) } + +pub fn up_pressed(ctx: &Context) -> bool { + ctx.input_mut(|i| i.key_pressed(Key::ArrowUp) || i.key_pressed(Key::W)) +} + +pub fn down_pressed(ctx: &Context) -> bool { + 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) { + let ui_height = ui.available_rect_before_wrap().height(); + if up_pressed(ui.ctx()) { + ui.scroll_with_delta_animation(vec2(0.0, ui_height / 10.0), ScrollAnimation::none()); + } else if down_pressed(ui.ctx()) { + 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()); + } +} diff --git a/objdiff-gui/src/views/data_diff.rs b/objdiff-gui/src/views/data_diff.rs index 70ae1dcd..c08e4155 100644 --- a/objdiff-gui/src/views/data_diff.rs +++ b/objdiff-gui/src/views/data_diff.rs @@ -179,6 +179,8 @@ fn data_table_ui( let left_diffs = left_section.map(|(_, section)| split_diffs(§ion.data_diff)); let right_diffs = right_section.map(|(_, section)| split_diffs(§ion.data_diff)); + hotkeys::check_scroll_hotkeys(ui); + render_table(ui, available_width, 2, config.code_font.size, total_rows, |row, column| { let i = row.index(); let address = i * BYTES_PER_ROW; diff --git a/objdiff-gui/src/views/extab_diff.rs b/objdiff-gui/src/views/extab_diff.rs index ce59a3ab..db391e08 100644 --- a/objdiff-gui/src/views/extab_diff.rs +++ b/objdiff-gui/src/views/extab_diff.rs @@ -235,6 +235,8 @@ pub fn extab_diff_ui( } }); + hotkeys::check_scroll_hotkeys(ui); + // Table render_strips(ui, available_width, 2, |ui, column| { if column == 0 { diff --git a/objdiff-gui/src/views/function_diff.rs b/objdiff-gui/src/views/function_diff.rs index 997a3fc0..1df1152a 100644 --- a/objdiff-gui/src/views/function_diff.rs +++ b/objdiff-gui/src/views/function_diff.rs @@ -435,6 +435,7 @@ fn asm_table_ui( }; if left_len.is_some() && right_len.is_some() { // Joint view + hotkeys::check_scroll_hotkeys(ui); render_table( ui, available_width, diff --git a/objdiff-gui/src/views/symbol_diff.rs b/objdiff-gui/src/views/symbol_diff.rs index 1dca5ded..7fa185f2 100644 --- a/objdiff-gui/src/views/symbol_diff.rs +++ b/objdiff-gui/src/views/symbol_diff.rs @@ -649,6 +649,8 @@ pub fn symbol_list_ui( } } + hotkeys::check_scroll_hotkeys(ui); + ui.scope(|ui| { ui.style_mut().override_text_style = Some(egui::TextStyle::Monospace); ui.style_mut().wrap_mode = Some(egui::TextWrapMode::Extend); From b57178773215b19b94378d6119243e09608c2f1c Mon Sep 17 00:00:00 2001 From: LagoLunatic Date: Thu, 28 Nov 2024 16:56:11 -0500 Subject: [PATCH 05/18] Add hotkeys to select the next symbol above/below the current one in the listing --- objdiff-gui/src/hotkeys.rs | 20 ++++++++-- objdiff-gui/src/views/data_diff.rs | 2 +- objdiff-gui/src/views/extab_diff.rs | 2 +- objdiff-gui/src/views/function_diff.rs | 2 +- objdiff-gui/src/views/symbol_diff.rs | 52 +++++++++++++++++++++++++- 5 files changed, 69 insertions(+), 9 deletions(-) diff --git a/objdiff-gui/src/hotkeys.rs b/objdiff-gui/src/hotkeys.rs index ba2e6609..723e4a3c 100644 --- a/objdiff-gui/src/hotkeys.rs +++ b/objdiff-gui/src/hotkeys.rs @@ -1,4 +1,4 @@ -use egui::{style::ScrollAnimation, vec2, Context, Key, PointerButton}; +use egui::{style::ScrollAnimation, vec2, Context, Key, Modifiers, PointerButton}; pub fn enter_pressed(ctx: &Context) -> bool { ctx.input_mut(|i| i.key_pressed(Key::Enter) || i.pointer.button_pressed(PointerButton::Extra2)) @@ -26,11 +26,11 @@ pub fn home_pressed(ctx: &Context) -> bool { ctx.input_mut(|i| i.key_pressed(Key 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) { +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()) { + 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()) { + } 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()); @@ -42,3 +42,15 @@ pub fn check_scroll_hotkeys(ui: &mut egui::Ui) { ui.scroll_with_delta_animation(vec2(0.0, -f32::INFINITY), ScrollAnimation::none()); } } + +pub fn consume_up_key(ctx: &Context) -> bool { + 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 { + ctx.input_mut(|i| { + i.consume_key(Modifiers::NONE, Key::ArrowDown) || i.consume_key(Modifiers::NONE, Key::S) + }) +} diff --git a/objdiff-gui/src/views/data_diff.rs b/objdiff-gui/src/views/data_diff.rs index c08e4155..37f2a6ce 100644 --- a/objdiff-gui/src/views/data_diff.rs +++ b/objdiff-gui/src/views/data_diff.rs @@ -179,7 +179,7 @@ fn data_table_ui( let left_diffs = left_section.map(|(_, section)| split_diffs(§ion.data_diff)); let right_diffs = right_section.map(|(_, section)| split_diffs(§ion.data_diff)); - hotkeys::check_scroll_hotkeys(ui); + hotkeys::check_scroll_hotkeys(ui, true); render_table(ui, available_width, 2, config.code_font.size, total_rows, |row, column| { let i = row.index(); diff --git a/objdiff-gui/src/views/extab_diff.rs b/objdiff-gui/src/views/extab_diff.rs index db391e08..5d991178 100644 --- a/objdiff-gui/src/views/extab_diff.rs +++ b/objdiff-gui/src/views/extab_diff.rs @@ -235,7 +235,7 @@ pub fn extab_diff_ui( } }); - hotkeys::check_scroll_hotkeys(ui); + hotkeys::check_scroll_hotkeys(ui, true); // Table render_strips(ui, available_width, 2, |ui, column| { diff --git a/objdiff-gui/src/views/function_diff.rs b/objdiff-gui/src/views/function_diff.rs index 1df1152a..67ea175b 100644 --- a/objdiff-gui/src/views/function_diff.rs +++ b/objdiff-gui/src/views/function_diff.rs @@ -435,7 +435,7 @@ fn asm_table_ui( }; if left_len.is_some() && right_len.is_some() { // Joint view - hotkeys::check_scroll_hotkeys(ui); + hotkeys::check_scroll_hotkeys(ui, true); render_table( ui, available_width, diff --git a/objdiff-gui/src/views/symbol_diff.rs b/objdiff-gui/src/views/symbol_diff.rs index 7fa185f2..eed3bbd5 100644 --- a/objdiff-gui/src/views/symbol_diff.rs +++ b/objdiff-gui/src/views/symbol_diff.rs @@ -1,4 +1,4 @@ -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, @@ -649,7 +649,55 @@ pub fn symbol_list_ui( } } - hotkeys::check_scroll_hotkeys(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, + ) + } else { + DiffViewAction::SetSymbolHighlight( + new_symbol_diff.target_symbol, + Some(*new_sym_ref), + ) + }); + } ui.scope(|ui| { ui.style_mut().override_text_style = Some(egui::TextStyle::Monospace); From 157e99de6fbe7759b5f8cbb27856a998055cdf81 Mon Sep 17 00:00:00 2001 From: LagoLunatic Date: Thu, 28 Nov 2024 16:58:22 -0500 Subject: [PATCH 06/18] Do not clear highlighted symbol when backing out of diff view --- objdiff-gui/src/views/symbol_diff.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/objdiff-gui/src/views/symbol_diff.rs b/objdiff-gui/src/views/symbol_diff.rs index eed3bbd5..2e8ca50f 100644 --- a/objdiff-gui/src/views/symbol_diff.rs +++ b/objdiff-gui/src/views/symbol_diff.rs @@ -212,7 +212,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; }; From 2427b065843e8a080bc3c159f9dc634d748a6cd7 Mon Sep 17 00:00:00 2001 From: LagoLunatic Date: Thu, 28 Nov 2024 17:00:43 -0500 Subject: [PATCH 07/18] Do not clear highlighted symbol when hovering mouse over an unpaired symbol --- objdiff-gui/src/views/symbol_diff.rs | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/objdiff-gui/src/views/symbol_diff.rs b/objdiff-gui/src/views/symbol_diff.rs index 2e8ca50f..cc37edf8 100644 --- a/objdiff-gui/src/views/symbol_diff.rs +++ b/objdiff-gui/src/views/symbol_diff.rs @@ -561,20 +561,16 @@ 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, + ) } else { - DiffViewAction::SetSymbolHighlight(None, None) + DiffViewAction::SetSymbolHighlight( + symbol_diff.target_symbol, + Some(symbol_diff.symbol_ref), + ) }); } ret From 046f3d69993a00c74526c04665178444c9e3711c Mon Sep 17 00:00:00 2001 From: LagoLunatic Date: Thu, 28 Nov 2024 17:07:14 -0500 Subject: [PATCH 08/18] Auto-scroll the keyboard-selected symbols into view if offscreen --- objdiff-gui/src/views/function_diff.rs | 16 +++++++------ objdiff-gui/src/views/symbol_diff.rs | 32 ++++++++++++++++++-------- 2 files changed, 32 insertions(+), 16 deletions(-) diff --git a/objdiff-gui/src/views/function_diff.rs b/objdiff-gui/src/views/function_diff.rs index 67ea175b..457ef064 100644 --- a/objdiff-gui/src/views/function_diff.rs +++ b/objdiff-gui/src/views/function_diff.rs @@ -409,7 +409,7 @@ fn asm_table_ui( right_ctx: Option>, appearance: &Appearance, ins_view_state: &FunctionViewState, - symbol_state: &SymbolViewState, + symbol_state: &mut SymbolViewState, ) -> Option { let mut ret = None; let left_len = left_ctx.and_then(|ctx| { @@ -516,8 +516,9 @@ fn asm_table_ui( SymbolRefByName::new(right_symbol, right_section), )); } - DiffViewAction::SetSymbolHighlight(_, _) => { - // Ignore + DiffViewAction::SetSymbolHighlight(left, right, scroll) => { + symbol_state.highlighted_symbol = (left, right); + symbol_state.scroll_highlighted_symbol_into_view = scroll; } _ => { ret = Some(action); @@ -576,8 +577,9 @@ fn asm_table_ui( right_symbol_ref, )); } - DiffViewAction::SetSymbolHighlight(_, _) => { - // Ignore + DiffViewAction::SetSymbolHighlight(left, right, scroll) => { + symbol_state.highlighted_symbol = (left, right); + symbol_state.scroll_highlighted_symbol_into_view = scroll; } _ => { ret = Some(action); @@ -620,7 +622,7 @@ impl<'a> FunctionDiffContext<'a> { #[must_use] pub fn function_diff_ui( ui: &mut egui::Ui, - state: &DiffViewState, + state: &mut DiffViewState, appearance: &Appearance, ) -> Option { let mut ret = None; @@ -823,7 +825,7 @@ pub fn function_diff_ui( right_ctx, appearance, &state.function_state, - &state.symbol_state, + &mut state.symbol_state, ) }) .inner diff --git a/objdiff-gui/src/views/symbol_diff.rs b/objdiff-gui/src/views/symbol_diff.rs index cc37edf8..0041e92a 100644 --- a/objdiff-gui/src/views/symbol_diff.rs +++ b/objdiff-gui/src/views/symbol_diff.rs @@ -1,8 +1,8 @@ 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, @@ -57,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, Option), + /// Set the highlighted symbols in the symbols view, optionally scrolling them into view. + SetSymbolHighlight(Option, Option, bool), /// Set the symbols view search filter SetSearch(String), /// Submit the current function to decomp.me @@ -136,6 +136,7 @@ pub struct DiffViewState { #[derive(Default)] pub struct SymbolViewState { pub highlighted_symbol: (Option, Option), + pub scroll_highlighted_symbol_into_view: bool, pub left_symbol: Option, pub right_symbol: Option, pub reverse_fn_order: bool, @@ -247,8 +248,9 @@ impl DiffViewState { self.post_build_nav = Some(nav); } } - DiffViewAction::SetSymbolHighlight(left, right) => { + DiffViewAction::SetSymbolHighlight(left, right, scroll) => { self.symbol_state.highlighted_symbol = (left, right); + self.symbol_state.scroll_highlighted_symbol_into_view = scroll; } DiffViewAction::SetSearch(search) => { self.search_regex = if search.is_empty() { @@ -471,7 +473,7 @@ fn symbol_ui( symbol: &ObjSymbol, symbol_diff: &ObjSymbolDiff, section: Option<&ObjSection>, - state: &SymbolViewState, + state: &mut SymbolViewState, appearance: &Appearance, column: usize, ) -> Option { @@ -534,6 +536,14 @@ fn symbol_ui( ret = Some(DiffViewAction::Navigate(result)); } }); + if selected && state.scroll_highlighted_symbol_into_view { + // 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()); + // Then reset this flag so that we don't repeatedly scroll the view back when the user is + // trying to manually scroll away. + state.scroll_highlighted_symbol_into_view = false; + } if response.clicked() || (selected && hotkeys::enter_pressed(ui.ctx())) { if let Some(section) = section { match section.kind { @@ -565,11 +575,13 @@ fn symbol_ui( DiffViewAction::SetSymbolHighlight( Some(symbol_diff.symbol_ref), symbol_diff.target_symbol, + false, ) } else { DiffViewAction::SetSymbolHighlight( symbol_diff.target_symbol, Some(symbol_diff.symbol_ref), + false, ) }); } @@ -603,7 +615,7 @@ pub fn symbol_list_ui( ui: &mut Ui, ctx: SymbolDiffContext<'_>, other_ctx: Option>, - state: &SymbolViewState, + state: &mut SymbolViewState, filter: SymbolFilter<'_>, appearance: &Appearance, column: usize, @@ -685,11 +697,13 @@ pub fn symbol_list_ui( 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, ) }); } @@ -941,7 +955,7 @@ pub fn symbol_diff_ui( .second_obj .as_ref() .map(|(obj, diff)| SymbolDiffContext { obj, diff }), - &state.symbol_state, + &mut state.symbol_state, filter, appearance, column, @@ -965,7 +979,7 @@ pub fn symbol_diff_ui( .first_obj .as_ref() .map(|(obj, diff)| SymbolDiffContext { obj, diff }), - &state.symbol_state, + &mut state.symbol_state, filter, appearance, column, From 77c104c67bb7347d71c25a693d677303294f8e64 Mon Sep 17 00:00:00 2001 From: LagoLunatic Date: Thu, 28 Nov 2024 17:52:29 -0500 Subject: [PATCH 09/18] 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. --- objdiff-gui/src/hotkeys.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/objdiff-gui/src/hotkeys.rs b/objdiff-gui/src/hotkeys.rs index 723e4a3c..4467ccb4 100644 --- a/objdiff-gui/src/hotkeys.rs +++ b/objdiff-gui/src/hotkeys.rs @@ -1,20 +1,34 @@ use egui::{style::ScrollAnimation, vec2, Context, Key, 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.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.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)) } @@ -44,12 +58,18 @@ pub fn check_scroll_hotkeys(ui: &mut egui::Ui, include_small_increments: bool) { } 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) }) From 4fb64a3ad40a21a8150de739bfbb9672766d8fa4 Mon Sep 17 00:00:00 2001 From: LagoLunatic Date: Thu, 28 Nov 2024 18:04:47 -0500 Subject: [PATCH 10/18] Add Ctrl+F/S shortcuts for focusing the object and symbol filter text edits --- objdiff-gui/src/hotkeys.rs | 16 +++++++++++++++- objdiff-gui/src/views/config.rs | 7 ++++++- objdiff-gui/src/views/symbol_diff.rs | 6 +++++- 3 files changed, 26 insertions(+), 3 deletions(-) diff --git a/objdiff-gui/src/hotkeys.rs b/objdiff-gui/src/hotkeys.rs index 4467ccb4..d23529f9 100644 --- a/objdiff-gui/src/hotkeys.rs +++ b/objdiff-gui/src/hotkeys.rs @@ -1,4 +1,6 @@ -use egui::{style::ScrollAnimation, vec2, Context, Key, Modifiers, PointerButton}; +use egui::{ + style::ScrollAnimation, vec2, Context, Key, KeyboardShortcut, Modifiers, PointerButton, +}; fn any_widget_focused(ctx: &Context) -> bool { ctx.memory(|mem| mem.focused().is_some()) } @@ -74,3 +76,15 @@ pub fn consume_down_key(ctx: &Context) -> bool { 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)) +} diff --git a/objdiff-gui/src/views/config.rs b/objdiff-gui/src/views/config.rs index 6d7fd9e7..da382de7 100644 --- a/objdiff-gui/src/views/config.rs +++ b/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; diff --git a/objdiff-gui/src/views/symbol_diff.rs b/objdiff-gui/src/views/symbol_diff.rs index 0041e92a..0a9fb4a6 100644 --- a/objdiff-gui/src/views/symbol_diff.rs +++ b/objdiff-gui/src/views/symbol_diff.rs @@ -898,7 +898,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 { From 3f03a758252cf8dc56c317be3c1123e1635d4d54 Mon Sep 17 00:00:00 2001 From: LagoLunatic Date: Thu, 28 Nov 2024 18:16:19 -0500 Subject: [PATCH 11/18] Add space as alternative to enter hotkey This is for consistency with egui's builtint enter/space hotkey for interacting with the focused widget. --- objdiff-gui/src/hotkeys.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/objdiff-gui/src/hotkeys.rs b/objdiff-gui/src/hotkeys.rs index d23529f9..23151c03 100644 --- a/objdiff-gui/src/hotkeys.rs +++ b/objdiff-gui/src/hotkeys.rs @@ -8,7 +8,11 @@ pub fn enter_pressed(ctx: &Context) -> bool { if any_widget_focused(ctx) { return false; } - ctx.input_mut(|i| i.key_pressed(Key::Enter) || i.pointer.button_pressed(PointerButton::Extra2)) + 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 { From 28bd7182d1d7fa17b3d86122fea1be587e58b9f4 Mon Sep 17 00:00:00 2001 From: LagoLunatic Date: Thu, 28 Nov 2024 18:30:50 -0500 Subject: [PATCH 12/18] Add hotkeys to change target and base functions --- objdiff-gui/src/hotkeys.rs | 12 ++++++++++++ objdiff-gui/src/views/function_diff.rs | 4 +++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/objdiff-gui/src/hotkeys.rs b/objdiff-gui/src/hotkeys.rs index 23151c03..72177b11 100644 --- a/objdiff-gui/src/hotkeys.rs +++ b/objdiff-gui/src/hotkeys.rs @@ -92,3 +92,15 @@ const SYMBOL_FILTER_SHORTCUT: KeyboardShortcut = KeyboardShortcut::new(Modifiers 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)) +} diff --git a/objdiff-gui/src/views/function_diff.rs b/objdiff-gui/src/views/function_diff.rs index 457ef064..ba8071f0 100644 --- a/objdiff-gui/src/views/function_diff.rs +++ b/objdiff-gui/src/views/function_diff.rs @@ -714,10 +714,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())); @@ -791,6 +792,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())); From 441b30070e2fd4ce9c0cd83d0b44c3f87d2c761a Mon Sep 17 00:00:00 2001 From: LagoLunatic Date: Thu, 28 Nov 2024 18:41:24 -0500 Subject: [PATCH 13/18] Split function diff view: Enable PageUp/PageDown/Home/End for scrolling --- objdiff-gui/src/views/function_diff.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/objdiff-gui/src/views/function_diff.rs b/objdiff-gui/src/views/function_diff.rs index ba8071f0..b059b2c7 100644 --- a/objdiff-gui/src/views/function_diff.rs +++ b/objdiff-gui/src/views/function_diff.rs @@ -471,6 +471,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, @@ -532,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, From d7d7a7f14ac59c2d33c380f4e912fa1332d5c8f9 Mon Sep 17 00:00:00 2001 From: LagoLunatic Date: Thu, 28 Nov 2024 18:55:26 -0500 Subject: [PATCH 14/18] Add escape as an alternative to back hotkey --- objdiff-gui/src/hotkeys.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/objdiff-gui/src/hotkeys.rs b/objdiff-gui/src/hotkeys.rs index 72177b11..e68571b6 100644 --- a/objdiff-gui/src/hotkeys.rs +++ b/objdiff-gui/src/hotkeys.rs @@ -20,7 +20,9 @@ pub fn back_pressed(ctx: &Context) -> bool { return false; } ctx.input_mut(|i| { - i.key_pressed(Key::Backspace) || i.pointer.button_pressed(PointerButton::Extra1) + i.key_pressed(Key::Backspace) + || i.key_pressed(Key::Escape) + || i.pointer.button_pressed(PointerButton::Extra1) }) } From 45dd73f5a996d34f41131c01c8726bab1b216eb4 Mon Sep 17 00:00:00 2001 From: LagoLunatic Date: Thu, 28 Nov 2024 19:32:48 -0500 Subject: [PATCH 15/18] 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. --- objdiff-gui/src/views/function_diff.rs | 4 ++-- objdiff-gui/src/views/symbol_diff.rs | 13 +++++++++---- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/objdiff-gui/src/views/function_diff.rs b/objdiff-gui/src/views/function_diff.rs index b059b2c7..ed7ee675 100644 --- a/objdiff-gui/src/views/function_diff.rs +++ b/objdiff-gui/src/views/function_diff.rs @@ -519,7 +519,7 @@ fn asm_table_ui( } DiffViewAction::SetSymbolHighlight(left, right, scroll) => { symbol_state.highlighted_symbol = (left, right); - symbol_state.scroll_highlighted_symbol_into_view = scroll; + symbol_state.scroll_to_highlighted_symbols = (scroll, scroll); } _ => { ret = Some(action); @@ -581,7 +581,7 @@ fn asm_table_ui( } DiffViewAction::SetSymbolHighlight(left, right, scroll) => { symbol_state.highlighted_symbol = (left, right); - symbol_state.scroll_highlighted_symbol_into_view = scroll; + symbol_state.scroll_to_highlighted_symbols = (scroll, scroll); } _ => { ret = Some(action); diff --git a/objdiff-gui/src/views/symbol_diff.rs b/objdiff-gui/src/views/symbol_diff.rs index 0a9fb4a6..a43cd867 100644 --- a/objdiff-gui/src/views/symbol_diff.rs +++ b/objdiff-gui/src/views/symbol_diff.rs @@ -136,7 +136,7 @@ pub struct DiffViewState { #[derive(Default)] pub struct SymbolViewState { pub highlighted_symbol: (Option, Option), - pub scroll_highlighted_symbol_into_view: bool, + pub scroll_to_highlighted_symbols: (bool, bool), pub left_symbol: Option, pub right_symbol: Option, pub reverse_fn_order: bool, @@ -250,7 +250,7 @@ impl DiffViewState { } DiffViewAction::SetSymbolHighlight(left, right, scroll) => { self.symbol_state.highlighted_symbol = (left, right); - self.symbol_state.scroll_highlighted_symbol_into_view = scroll; + self.symbol_state.scroll_to_highlighted_symbols = (scroll, scroll); } DiffViewAction::SetSearch(search) => { self.search_regex = if search.is_empty() { @@ -536,13 +536,18 @@ fn symbol_ui( ret = Some(DiffViewAction::Navigate(result)); } }); - if selected && state.scroll_highlighted_symbol_into_view { + let should_scroll = if column == 0 { + &mut state.scroll_to_highlighted_symbols.0 + } else { + &mut state.scroll_to_highlighted_symbols.1 + }; + if selected && *should_scroll { // 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()); // Then reset this flag so that we don't repeatedly scroll the view back when the user is // trying to manually scroll away. - state.scroll_highlighted_symbol_into_view = false; + *should_scroll = false; } if response.clicked() || (selected && hotkeys::enter_pressed(ui.ctx())) { if let Some(section) = section { From 517b84e766e42e00f02d6d888389712676af7de2 Mon Sep 17 00:00:00 2001 From: LagoLunatic Date: Thu, 28 Nov 2024 23:36:58 -0500 Subject: [PATCH 16/18] Simplify clearing of the autoscroll flag, remove &mut State --- objdiff-gui/src/app.rs | 2 ++ objdiff-gui/src/views/function_diff.rs | 14 +++--------- objdiff-gui/src/views/symbol_diff.rs | 30 +++++++++++--------------- 3 files changed, 17 insertions(+), 29 deletions(-) diff --git a/objdiff-gui/src/app.rs b/objdiff-gui/src/app.rs index b79d1c6b..059186bc 100644 --- a/objdiff-gui/src/app.rs +++ b/objdiff-gui/src/app.rs @@ -831,6 +831,8 @@ impl eframe::App for App { } else { symbol_diff_ui(ui, diff_state, appearance) }; + // Clear the autoscroll flag so it doesn't scroll continuously. + diff_state.symbol_state.autoscroll_to_highlighted_symbols = false; }); project_window(ctx, state, show_project_config, config_state, appearance); diff --git a/objdiff-gui/src/views/function_diff.rs b/objdiff-gui/src/views/function_diff.rs index ed7ee675..ad11995c 100644 --- a/objdiff-gui/src/views/function_diff.rs +++ b/objdiff-gui/src/views/function_diff.rs @@ -409,7 +409,7 @@ fn asm_table_ui( right_ctx: Option>, appearance: &Appearance, ins_view_state: &FunctionViewState, - symbol_state: &mut SymbolViewState, + symbol_state: &SymbolViewState, ) -> Option { let mut ret = None; let left_len = left_ctx.and_then(|ctx| { @@ -517,10 +517,6 @@ fn asm_table_ui( SymbolRefByName::new(right_symbol, right_section), )); } - DiffViewAction::SetSymbolHighlight(left, right, scroll) => { - symbol_state.highlighted_symbol = (left, right); - symbol_state.scroll_to_highlighted_symbols = (scroll, scroll); - } _ => { ret = Some(action); } @@ -579,10 +575,6 @@ fn asm_table_ui( right_symbol_ref, )); } - DiffViewAction::SetSymbolHighlight(left, right, scroll) => { - symbol_state.highlighted_symbol = (left, right); - symbol_state.scroll_to_highlighted_symbols = (scroll, scroll); - } _ => { ret = Some(action); } @@ -624,7 +616,7 @@ impl<'a> FunctionDiffContext<'a> { #[must_use] pub fn function_diff_ui( ui: &mut egui::Ui, - state: &mut DiffViewState, + state: &DiffViewState, appearance: &Appearance, ) -> Option { let mut ret = None; @@ -829,7 +821,7 @@ pub fn function_diff_ui( right_ctx, appearance, &state.function_state, - &mut state.symbol_state, + &state.symbol_state, ) }) .inner diff --git a/objdiff-gui/src/views/symbol_diff.rs b/objdiff-gui/src/views/symbol_diff.rs index a43cd867..356c73fd 100644 --- a/objdiff-gui/src/views/symbol_diff.rs +++ b/objdiff-gui/src/views/symbol_diff.rs @@ -136,7 +136,7 @@ pub struct DiffViewState { #[derive(Default)] pub struct SymbolViewState { pub highlighted_symbol: (Option, Option), - pub scroll_to_highlighted_symbols: (bool, bool), + pub autoscroll_to_highlighted_symbols: bool, pub left_symbol: Option, pub right_symbol: Option, pub reverse_fn_order: bool, @@ -248,9 +248,9 @@ impl DiffViewState { self.post_build_nav = Some(nav); } } - DiffViewAction::SetSymbolHighlight(left, right, scroll) => { + DiffViewAction::SetSymbolHighlight(left, right, autoscroll) => { self.symbol_state.highlighted_symbol = (left, right); - self.symbol_state.scroll_to_highlighted_symbols = (scroll, scroll); + self.symbol_state.autoscroll_to_highlighted_symbols = autoscroll; } DiffViewAction::SetSearch(search) => { self.search_regex = if search.is_empty() { @@ -473,7 +473,7 @@ fn symbol_ui( symbol: &ObjSymbol, symbol_diff: &ObjSymbolDiff, section: Option<&ObjSection>, - state: &mut SymbolViewState, + state: &SymbolViewState, appearance: &Appearance, column: usize, ) -> Option { @@ -536,18 +536,12 @@ fn symbol_ui( ret = Some(DiffViewAction::Navigate(result)); } }); - let should_scroll = if column == 0 { - &mut state.scroll_to_highlighted_symbols.0 - } else { - &mut state.scroll_to_highlighted_symbols.1 - }; - if selected && *should_scroll { - // Scroll the view to encompass the selected symbol in case the user selected an offscreen - // symbol by using a keyboard shortcut. + 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()); - // Then reset this flag so that we don't repeatedly scroll the view back when the user is - // trying to manually scroll away. - *should_scroll = false; + // This state flag will be reset in App::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 { @@ -620,7 +614,7 @@ pub fn symbol_list_ui( ui: &mut Ui, ctx: SymbolDiffContext<'_>, other_ctx: Option>, - state: &mut SymbolViewState, + state: &SymbolViewState, filter: SymbolFilter<'_>, appearance: &Appearance, column: usize, @@ -964,7 +958,7 @@ pub fn symbol_diff_ui( .second_obj .as_ref() .map(|(obj, diff)| SymbolDiffContext { obj, diff }), - &mut state.symbol_state, + &state.symbol_state, filter, appearance, column, @@ -988,7 +982,7 @@ pub fn symbol_diff_ui( .first_obj .as_ref() .map(|(obj, diff)| SymbolDiffContext { obj, diff }), - &mut state.symbol_state, + &state.symbol_state, filter, appearance, column, From 95528fa8d298b4c39c0314afe2a8cf6312f3ad42 Mon Sep 17 00:00:00 2001 From: LagoLunatic Date: Fri, 29 Nov 2024 13:10:00 -0500 Subject: [PATCH 17/18] 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. --- objdiff-gui/src/app.rs | 2 -- objdiff-gui/src/views/symbol_diff.rs | 8 ++++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/objdiff-gui/src/app.rs b/objdiff-gui/src/app.rs index 059186bc..b79d1c6b 100644 --- a/objdiff-gui/src/app.rs +++ b/objdiff-gui/src/app.rs @@ -831,8 +831,6 @@ impl eframe::App for App { } else { symbol_diff_ui(ui, diff_state, appearance) }; - // Clear the autoscroll flag so it doesn't scroll continuously. - diff_state.symbol_state.autoscroll_to_highlighted_symbols = false; }); project_window(ctx, state, show_project_config, config_state, appearance); diff --git a/objdiff-gui/src/views/symbol_diff.rs b/objdiff-gui/src/views/symbol_diff.rs index 356c73fd..0d008c35 100644 --- a/objdiff-gui/src/views/symbol_diff.rs +++ b/objdiff-gui/src/views/symbol_diff.rs @@ -199,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; }; @@ -540,8 +543,9 @@ fn symbol_ui( // 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 state flag will be reset in App::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. + // 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 { From 0899e6779c05e71e4fff2888b40f1138d63b8e68 Mon Sep 17 00:00:00 2001 From: Luke Street Date: Sun, 1 Dec 2024 21:59:40 -0700 Subject: [PATCH 18/18] Add alt shortcut support to many elements --- objdiff-gui/src/app.rs | 55 +++++++----- objdiff-gui/src/hotkeys.rs | 122 ++++++++++++++++++++++++++- objdiff-gui/src/views/config.rs | 7 +- objdiff-gui/src/views/jobs.rs | 11 ++- objdiff-gui/src/views/symbol_diff.rs | 4 +- 5 files changed, 172 insertions(+), 27 deletions(-) diff --git a/objdiff-gui/src/app.rs b/objdiff-gui/src/app.rs index b79d1c6b..2a521e8f 100644 --- a/objdiff-gui/src/app.rs +++ b/objdiff-gui/src/app.rs @@ -25,6 +25,7 @@ use time::UtcOffset; use crate::{ app_config::{deserialize_config, AppConfigVersion}, config::{load_project_config, ProjectObjectNode}, + hotkeys, jobs::{ objdiff::{start_build, ObjDiffConfig}, Job, JobQueue, JobResult, JobStatus, @@ -527,6 +528,10 @@ impl App { } fn post_update(&mut self, ctx: &egui::Context, action: Option) { + if action.is_some() { + ctx.request_repaint(); + } + self.appearance.post_update(ctx); let ViewState { jobs, diff_state, config_state, graphics_state, .. } = &mut self.view_state; @@ -690,13 +695,13 @@ impl eframe::App for App { *show_side_panel = !*show_side_panel; } ui.separator(); - ui.menu_button("File", |ui| { + ui.menu_button(hotkeys::button_alt_text(ui, "_File"), |ui| { #[cfg(debug_assertions)] - if ui.button("Debug…").clicked() { + if ui.button(hotkeys::button_alt_text(ui, "_Debug…")).clicked() { *show_debug = !*show_debug; ui.close_menu(); } - if ui.button("Project…").clicked() { + if ui.button(hotkeys::button_alt_text(ui, "_Project…")).clicked() { *show_project_config = !*show_project_config; ui.close_menu(); } @@ -705,10 +710,11 @@ impl eframe::App for App { } else { vec![] }; + let recent_projects_text = hotkeys::button_alt_text(ui, "_Recent Projects…"); if recent_projects.is_empty() { - ui.add_enabled(false, egui::Button::new("Recent projects…")); + ui.add_enabled(false, egui::Button::new(recent_projects_text)); } else { - ui.menu_button("Recent Projects…", |ui| { + ui.menu_button(recent_projects_text, |ui| { if ui.button("Clear").clicked() { state.write().unwrap().config.recent_projects.clear(); }; @@ -721,36 +727,39 @@ impl eframe::App for App { } }); } - if ui.button("Appearance…").clicked() { + if ui.button(hotkeys::button_alt_text(ui, "_Appearance…")).clicked() { *show_appearance_config = !*show_appearance_config; ui.close_menu(); } - if ui.button("Graphics…").clicked() { + if ui.button(hotkeys::button_alt_text(ui, "_Graphics…")).clicked() { *show_graphics = !*show_graphics; ui.close_menu(); } - if ui.button("Quit").clicked() { + if ui.button(hotkeys::button_alt_text(ui, "_Quit")).clicked() { ctx.send_viewport_cmd(egui::ViewportCommand::Close); } }); - ui.menu_button("Tools", |ui| { - if ui.button("Demangle…").clicked() { + ui.menu_button(hotkeys::button_alt_text(ui, "_Tools"), |ui| { + if ui.button(hotkeys::button_alt_text(ui, "_Demangle…")).clicked() { *show_demangle = !*show_demangle; ui.close_menu(); } - if ui.button("Rlwinm Decoder…").clicked() { + if ui.button(hotkeys::button_alt_text(ui, "_Rlwinm Decoder…")).clicked() { *show_rlwinm_decode = !*show_rlwinm_decode; ui.close_menu(); } }); - ui.menu_button("Diff Options", |ui| { - if ui.button("Arch Settings…").clicked() { + ui.menu_button(hotkeys::button_alt_text(ui, "_Diff Options"), |ui| { + if ui.button(hotkeys::button_alt_text(ui, "_Arch Settings…")).clicked() { *show_arch_config = !*show_arch_config; ui.close_menu(); } let mut state = state.write().unwrap(); let response = ui - .checkbox(&mut state.config.rebuild_on_changes, "Rebuild on changes") + .checkbox( + &mut state.config.rebuild_on_changes, + hotkeys::button_alt_text(ui, "_Rebuild on changes"), + ) .on_hover_text("Automatically re-run the build & diff when files change."); if response.changed() { state.watcher_change = true; @@ -759,18 +768,21 @@ impl eframe::App for App { !diff_state.symbol_state.disable_reverse_fn_order, egui::Checkbox::new( &mut diff_state.symbol_state.reverse_fn_order, - "Reverse function order (-inline deferred)", + hotkeys::button_alt_text( + ui, + "Reverse function order (-inline _deferred)", + ), ), ) .on_disabled_hover_text(CONFIG_DISABLED_TEXT); ui.checkbox( &mut diff_state.symbol_state.show_hidden_symbols, - "Show hidden symbols", + hotkeys::button_alt_text(ui, "Show _hidden symbols"), ); if ui .checkbox( &mut state.config.diff_obj_config.relax_reloc_diffs, - "Relax relocation diffs", + hotkeys::button_alt_text(ui, "Rela_x relocation diffs"), ) .on_hover_text( "Ignores differences in relocation targets. (Address, name, etc)", @@ -782,7 +794,7 @@ impl eframe::App for App { if ui .checkbox( &mut state.config.diff_obj_config.space_between_args, - "Space between args", + hotkeys::button_alt_text(ui, "_Space between args"), ) .changed() { @@ -791,14 +803,17 @@ impl eframe::App for App { if ui .checkbox( &mut state.config.diff_obj_config.combine_data_sections, - "Combine data sections", + hotkeys::button_alt_text(ui, "Combine _data sections"), ) .on_hover_text("Combines data sections with equal names.") .changed() { state.queue_reload = true; } - if ui.button("Clear custom symbol mappings").clicked() { + if ui + .button(hotkeys::button_alt_text(ui, "_Clear custom symbol mappings")) + .clicked() + { state.clear_mappings(); diff_state.post_build_nav = Some(DiffViewNavigation::symbol_diff()); state.queue_reload = true; diff --git a/objdiff-gui/src/hotkeys.rs b/objdiff-gui/src/hotkeys.rs index e68571b6..92b1f43d 100644 --- a/objdiff-gui/src/hotkeys.rs +++ b/objdiff-gui/src/hotkeys.rs @@ -1,5 +1,6 @@ use egui::{ - style::ScrollAnimation, vec2, Context, Key, KeyboardShortcut, Modifiers, PointerButton, + style::ScrollAnimation, text::LayoutJob, vec2, Align, Context, FontSelection, Key, + KeyboardShortcut, Modifiers, PointerButton, RichText, Ui, WidgetText, }; fn any_widget_focused(ctx: &Context) -> bool { ctx.memory(|mem| mem.focused().is_some()) } @@ -106,3 +107,122 @@ const CHANGE_BASE_SHORTCUT: KeyboardShortcut = KeyboardShortcut::new(Modifiers:: pub fn consume_change_base_shortcut(ctx: &Context) -> bool { ctx.input_mut(|i| i.consume_shortcut(&CHANGE_BASE_SHORTCUT)) } + +fn shortcut_key(text: &str) -> (usize, char, Key) { + let i = text.chars().position(|c| c == '_').expect("No underscore in text"); + let key = text.chars().nth(i + 1).expect("No character after underscore"); + let shortcut_key = match key { + 'a' | 'A' => Key::A, + 'b' | 'B' => Key::B, + 'c' | 'C' => Key::C, + 'd' | 'D' => Key::D, + 'e' | 'E' => Key::E, + 'f' | 'F' => Key::F, + 'g' | 'G' => Key::G, + 'h' | 'H' => Key::H, + 'i' | 'I' => Key::I, + 'j' | 'J' => Key::J, + 'k' | 'K' => Key::K, + 'l' | 'L' => Key::L, + 'm' | 'M' => Key::M, + 'n' | 'N' => Key::N, + 'o' | 'O' => Key::O, + 'p' | 'P' => Key::P, + 'q' | 'Q' => Key::Q, + 'r' | 'R' => Key::R, + 's' | 'S' => Key::S, + 't' | 'T' => Key::T, + 'u' | 'U' => Key::U, + 'v' | 'V' => Key::V, + 'w' | 'W' => Key::W, + 'x' | 'X' => Key::X, + 'y' | 'Y' => Key::Y, + 'z' | 'Z' => Key::Z, + _ => panic!("Invalid key {}", key), + }; + (i, key, shortcut_key) +} + +fn alt_text_ui(ui: &Ui, text: &str, i: usize, key: char, interactive: bool) -> WidgetText { + let color = if interactive { + ui.visuals().widgets.inactive.text_color() + } else { + ui.visuals().widgets.noninteractive.text_color() + }; + let mut job = LayoutJob::default(); + if i > 0 { + let text = &text[..i]; + RichText::new(text).color(color).append_to( + &mut job, + ui.style(), + FontSelection::Default, + Align::Center, + ); + } + let mut rt = RichText::new(key).color(color); + if ui.input(|i| i.modifiers.alt) { + rt = rt.underline(); + } + rt.append_to(&mut job, ui.style(), FontSelection::Default, Align::Center); + if i < text.len() - 1 { + let text = &text[i + 2..]; + RichText::new(text).color(color).append_to( + &mut job, + ui.style(), + FontSelection::Default, + Align::Center, + ); + } + job.into() +} + +pub fn button_alt_text(ui: &Ui, text: &str) -> WidgetText { + let (n, c, key) = shortcut_key(text); + let result = alt_text_ui(ui, text, n, c, true); + if ui.input_mut(|i| check_alt_key(i, c, key)) { + let btn_id = ui.next_auto_id(); + ui.memory_mut(|m| m.request_focus(btn_id)); + ui.input_mut(|i| { + i.events.push(egui::Event::Key { + key: Key::Enter, + physical_key: None, + pressed: true, + repeat: false, + modifiers: Default::default(), + }) + }); + } + result +} + +pub fn alt_text(ui: &Ui, text: &str, enter: bool) -> WidgetText { + let (n, c, key) = shortcut_key(text); + let result = alt_text_ui(ui, text, n, c, false); + if ui.input_mut(|i| check_alt_key(i, c, key)) { + let btn_id = ui.next_auto_id(); + ui.memory_mut(|m| m.request_focus(btn_id)); + if enter { + ui.input_mut(|i| { + i.events.push(egui::Event::Key { + key: Key::Enter, + physical_key: None, + pressed: true, + repeat: false, + modifiers: Default::default(), + }) + }); + } + } + result +} + +fn check_alt_key(i: &mut egui::InputState, c: char, key: Key) -> bool { + if i.consume_key(Modifiers::ALT, key) { + // Remove any text input events that match the key + let cs = c.to_string(); + i.events.retain(|e| !matches!(e, egui::Event::Text(t) if *t == cs)); + true + } else { + false + } +} diff --git a/objdiff-gui/src/views/config.rs b/objdiff-gui/src/views/config.rs index da382de7..e4efd522 100644 --- a/objdiff-gui/src/views/config.rs +++ b/objdiff-gui/src/views/config.rs @@ -219,7 +219,7 @@ pub fn config_ui( ui.horizontal(|ui| { ui.heading("Project"); - if ui.button(RichText::new("Settings")).clicked() { + if ui.button("Settings").clicked() { *show_config_window = true; } }); @@ -255,8 +255,9 @@ pub fn config_ui( } } else { let had_search = !config_state.object_search.is_empty(); - let response = - 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(hotkeys::alt_text(ui, "Filter _objects", false)) + .ui(ui); if hotkeys::consume_object_filter_shortcut(ui.ctx()) { response.request_focus(); } diff --git a/objdiff-gui/src/views/jobs.rs b/objdiff-gui/src/views/jobs.rs index f6fc2d5f..509fc0fb 100644 --- a/objdiff-gui/src/views/jobs.rs +++ b/objdiff-gui/src/views/jobs.rs @@ -3,6 +3,7 @@ use std::cmp::Ordering; use egui::{ProgressBar, RichText, Widget}; use crate::{ + hotkeys, jobs::{JobQueue, JobStatus}, views::appearance::Appearance, }; @@ -94,7 +95,14 @@ impl From<&JobStatus> for JobStatusDisplay { } pub fn jobs_menu_ui(ui: &mut egui::Ui, jobs: &mut JobQueue, appearance: &Appearance) -> bool { - ui.label("Jobs:"); + let mut clicked = false; + if egui::Label::new(hotkeys::alt_text(ui, "_Jobs:", true)) + .sense(egui::Sense::click()) + .ui(ui) + .clicked() + { + clicked = true; + } let mut statuses = Vec::new(); for job in jobs.iter_mut() { let Ok(status) = job.context.status.read() else { @@ -105,7 +113,6 @@ pub fn jobs_menu_ui(ui: &mut egui::Ui, jobs: &mut JobQueue, appearance: &Appeara let running_jobs = statuses.iter().filter(|s| !s.error).count(); let error_jobs = statuses.iter().filter(|s| s.error).count(); - let mut clicked = false; let spinner = egui::Spinner::new().size(appearance.ui_font.size * 0.9).color(appearance.text_color); match running_jobs.cmp(&1) { diff --git a/objdiff-gui/src/views/symbol_diff.rs b/objdiff-gui/src/views/symbol_diff.rs index 0d008c35..05cc9ede 100644 --- a/objdiff-gui/src/views/symbol_diff.rs +++ b/objdiff-gui/src/views/symbol_diff.rs @@ -901,7 +901,9 @@ pub fn symbol_diff_ui( }); let mut search = state.search.clone(); - let response = TextEdit::singleline(&mut search).hint_text("Filter symbols").ui(ui); + let response = TextEdit::singleline(&mut search) + .hint_text(hotkeys::alt_text(ui, "Filter _symbols", false)) + .ui(ui); if hotkeys::consume_symbol_filter_shortcut(ui.ctx()) { response.request_focus(); }