From 446dd683284fec1bc1c9d6be2b943d7bfe785f6a Mon Sep 17 00:00:00 2001 From: LagoLunatic Date: Thu, 28 Nov 2024 23:36:58 -0500 Subject: [PATCH] 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,