From 3e42533b688df539acdde0d085243d6bfb28aa41 Mon Sep 17 00:00:00 2001 From: LagoLunatic Date: Wed, 27 Nov 2024 21:05:07 -0500 Subject: [PATCH 01/17] 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 8be0c75..8da8051 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 06c4d6e141f99e8c2c1a13040aa2da54444cafac Mon Sep 17 00:00:00 2001 From: LagoLunatic Date: Thu, 28 Nov 2024 16:19:17 -0500 Subject: [PATCH 02/17] Update .gitignore --- .gitignore | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index c46c7cc..e4a9258 100644 --- a/.gitignore +++ b/.gitignore @@ -18,4 +18,4 @@ android.keystore *.frag *.vert *.metal -.vscode/launch.json +.vscode/ From e448630f36294d310c492e500d96c8ce339abd1f Mon Sep 17 00:00:00 2001 From: LagoLunatic Date: Thu, 28 Nov 2024 16:26:30 -0500 Subject: [PATCH 03/17] 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 0000000..762e5db --- /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 18e6b7f..0974a00 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 224a169..152b9ef 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 b91cd62..952909f 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 168ed7e..0be5745 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 1ca66dd..1dca5de 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 76f1952cffa1831097a3a4f3e3eedd2758099208 Mon Sep 17 00:00:00 2001 From: LagoLunatic Date: Thu, 28 Nov 2024 16:32:20 -0500 Subject: [PATCH 04/17] 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 762e5db..ba2e660 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 152b9ef..74918c6 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 952909f..fa2261e 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 0be5745..b6b028a 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 1dca5de..7fa185f 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 7d677009cee209a602415935acc2cd58b424cda3 Mon Sep 17 00:00:00 2001 From: LagoLunatic Date: Thu, 28 Nov 2024 16:56:11 -0500 Subject: [PATCH 05/17] 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 ba2e660..723e4a3 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 74918c6..9aa6da6 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 fa2261e..5e84b30 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 b6b028a..42db981 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 7fa185f..eed3bbd 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 5bffaa8b5b0ac88ee6a911ec65cca2e4bc2aa8ca Mon Sep 17 00:00:00 2001 From: LagoLunatic Date: Thu, 28 Nov 2024 16:58:22 -0500 Subject: [PATCH 06/17] 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 eed3bbd..2e8ca50 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 dfa145e16c700532a315a3df262f2e4639c285c3 Mon Sep 17 00:00:00 2001 From: LagoLunatic Date: Thu, 28 Nov 2024 17:00:43 -0500 Subject: [PATCH 07/17] 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 2e8ca50..cc37edf 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 5af40e0a33d50916db1f8d137449002b5e4e1c64 Mon Sep 17 00:00:00 2001 From: LagoLunatic Date: Thu, 28 Nov 2024 17:07:14 -0500 Subject: [PATCH 08/17] 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 42db981..3344463 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 cc37edf..0041e92 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 5de087c9032fc56dfb6f3a97403485af8deb88be Mon Sep 17 00:00:00 2001 From: LagoLunatic Date: Thu, 28 Nov 2024 17:52:29 -0500 Subject: [PATCH 09/17] 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 723e4a3..4467ccb 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 0143046f3f19716705f47e92cf26f7c308fadc05 Mon Sep 17 00:00:00 2001 From: LagoLunatic Date: Thu, 28 Nov 2024 18:04:47 -0500 Subject: [PATCH 10/17] 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 4467ccb..d23529f 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 6d7fd9e..da382de 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 0041e92..0a9fb4a 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 461605810324b3d075dd6f05bce3fb43f4d8bb56 Mon Sep 17 00:00:00 2001 From: LagoLunatic Date: Thu, 28 Nov 2024 18:16:19 -0500 Subject: [PATCH 11/17] 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 d23529f..23151c0 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 98e971d0ce8288b2d4ed7086312f143824b979c5 Mon Sep 17 00:00:00 2001 From: LagoLunatic Date: Thu, 28 Nov 2024 18:30:50 -0500 Subject: [PATCH 12/17] 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 23151c0..72177b1 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 3344463..a3cdd8b 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.map_or(false, |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 f2b410f027e540b32bc364148208a6c38a55e0ee Mon Sep 17 00:00:00 2001 From: LagoLunatic Date: Thu, 28 Nov 2024 18:41:24 -0500 Subject: [PATCH 13/17] 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 a3cdd8b..fa97a65 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 d5dcc4f00f17aa618293b55c57a0cd81f1f3f558 Mon Sep 17 00:00:00 2001 From: LagoLunatic Date: Thu, 28 Nov 2024 18:55:26 -0500 Subject: [PATCH 14/17] 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 72177b1..e68571b 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 99641d2637b5c2418289d8c1168f35eb35ddc098 Mon Sep 17 00:00:00 2001 From: LagoLunatic Date: Thu, 28 Nov 2024 19:32:48 -0500 Subject: [PATCH 15/17] 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 fa97a65..d737471 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 0a9fb4a..a43cd86 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 446dd683284fec1bc1c9d6be2b943d7bfe785f6a Mon Sep 17 00:00:00 2001 From: LagoLunatic Date: Thu, 28 Nov 2024 23:36:58 -0500 Subject: [PATCH 16/17] 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 b79d1c6..059186b 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 d737471..7f1102e 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 a43cd86..356c73f 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 ef2723748af203fa0c0feeac4c86847b1faf4ddb Mon Sep 17 00:00:00 2001 From: LagoLunatic Date: Fri, 29 Nov 2024 13:10:00 -0500 Subject: [PATCH 17/17] 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 059186b..b79d1c6 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 356c73f..0d008c3 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 {