From 0931bb01b46f2ee5898cd80fe6c50505bad5cec2 Mon Sep 17 00:00:00 2001 From: Michel de Graaf Date: Tue, 8 Sep 2026 10:42:20 +0200 Subject: [PATCH] perf(tui): flatten a browsed share once and draw one page of it Moving through a browsed share felt laggy for the same reason the results did: the popup re-flattened the whole expanded tree on every frame, cloning every row's name and path, then built a table row for each, and every keypress flattened it twice more. A heavy sharer's tree runs to tens of thousands of rows once a few folders are open. The flattened rows now live on the browse state and are rebuilt only when a folder opens or closes, and the popup draws the visible page alone. In the harness a 30,000-row tree went from 11ms to 0.35ms a frame and a keypress from 2.7ms to under a microsecond. The downloads pane had the same shape (every transfer became a row each frame, and a select-all can queue ten thousand), so it takes the same page window, which now lives beside page_of for every pane to share. --- soulseek-rs/src/models/browse.rs | 41 ++++++++++++++++++++-- soulseek-rs/src/ui/main_tui/browse.rs | 32 +++++++---------- soulseek-rs/src/ui/panes/browse_pane.rs | 24 +++++++++---- soulseek-rs/src/ui/panes/downloads_pane.rs | 30 +++++++++++++--- soulseek-rs/src/ui/panes/results_pane.rs | 37 ++----------------- soulseek-rs/src/ui/styles.rs | 33 +++++++++++++++++ 6 files changed, 129 insertions(+), 68 deletions(-) diff --git a/soulseek-rs/src/models/browse.rs b/soulseek-rs/src/models/browse.rs index 305558f..d56d282 100644 --- a/soulseek-rs/src/models/browse.rs +++ b/soulseek-rs/src/models/browse.rs @@ -267,7 +267,10 @@ pub struct BrowseState { pub username: String, pub status: BrowseStatus, pub tree: Vec, - pub expanded: HashSet, + expanded: HashSet, + /// The tree flattened for the current expansion, kept between frames: + /// a heavy sharer's tree runs to tens of thousands of rows. + rows: Vec, pub selected_row: usize, pub file_count: usize, pub folder_count: usize, @@ -282,6 +285,7 @@ impl BrowseState { status: BrowseStatus::Loading, tree: Vec::new(), expanded: HashSet::new(), + rows: Vec::new(), selected_row: 0, file_count: 0, folder_count: 0, @@ -310,12 +314,30 @@ impl BrowseState { self.file_count = built.file_count; self.folder_count = built.folder_count; self.selected_row = 0; + self.refresh(); } /// The flattened visible rows for the current expansion state. #[must_use] - pub fn rows(&self) -> Vec { - flatten_browse(&self.tree, &self.expanded) + pub fn rows(&self) -> &[BrowseRow] { + &self.rows + } + + /// Open or close the folder at `path`. + pub fn set_expanded(&mut self, path: &str, expanded: bool) { + if expanded { + self.expanded.insert(path.to_string()); + } else { + self.expanded.remove(path); + } + self.refresh(); + } + + /// Re-flatten after the tree or the expansion changed under `rows`. + pub fn refresh(&mut self) { + self.rows = flatten_browse(&self.tree, &self.expanded); + self.selected_row = + self.selected_row.min(self.rows.len().saturating_sub(1)); } } @@ -579,6 +601,19 @@ mod tests { assert_eq!(tabs.retry_active(), None); } + #[test] + fn expanding_and_collapsing_refresh_the_rows_and_keep_the_selection() { + let mut state = BrowseState::loading("bob".to_string()); + state.load(&[dir("music\\album", &[("a.mp3", 1), ("b.mp3", 2)])]); + assert_eq!(state.rows().len(), 2, "music open, album closed"); + state.set_expanded("music\\album", true); + assert_eq!(state.rows().len(), 4); + state.selected_row = 3; + state.set_expanded("music\\album", false); + assert_eq!(state.rows().len(), 2); + assert_eq!(state.selected_row, 1, "pulled back onto the last row"); + } + #[test] fn browse_state_load_auto_expands_top_level() { let mut state = BrowseState::loading("bob".to_string()); diff --git a/soulseek-rs/src/ui/main_tui/browse.rs b/soulseek-rs/src/ui/main_tui/browse.rs index 525a063..5d16d91 100644 --- a/soulseek-rs/src/ui/main_tui/browse.rs +++ b/soulseek-rs/src/ui/main_tui/browse.rs @@ -48,8 +48,8 @@ impl MainTui { _ => {} } - // Snapshot the flattened rows + current selection, then drop the borrow. - let (rows, sel, row) = { + // The highlighted row and where it sits, then drop the borrow. + let (len, sel, row) = { let Some(browse) = self.state.browse.active_tab() else { self.state.show_browse = false; return; @@ -62,13 +62,12 @@ impl MainTui { return; } let sel = browse.selected_row.min(rows.len() - 1); - let row = rows[sel].clone(); - (rows, sel, row) + (rows.len(), sel, rows[sel].clone()) }; if let Some(jump) = jump { if let Some(browse) = self.state.browse.active_tab_mut() { - browse.selected_row = jumped(sel, rows.len(), jump); + browse.selected_row = jumped(sel, len, jump); } self.sync_browse_selection(); return; @@ -102,38 +101,31 @@ impl MainTui { browse.selected_row = sel.saturating_sub(1); } KeyCode::Down | KeyCode::Char('j') => { - browse.selected_row = (sel + 1).min(rows.len() - 1); + browse.selected_row = (sel + 1).min(len - 1); } KeyCode::Right | KeyCode::Char('l') => { if row.is_folder && !row.expanded { - browse.expanded.insert(row.path.clone()); + browse.set_expanded(&row.path, true); } else if row.is_folder { - browse.selected_row = (sel + 1).min(rows.len() - 1); + browse.selected_row = (sel + 1).min(len - 1); } } KeyCode::Left | KeyCode::Char('h') => { if row.is_folder && row.expanded { - browse.expanded.remove(&row.path); - } else if let Some(parent) = - (0..sel).rev().find(|&i| rows[i].depth < row.depth) + browse.set_expanded(&row.path, false); + } else if let Some(parent) = (0..sel) + .rev() + .find(|&i| browse.rows()[i].depth < row.depth) { browse.selected_row = parent; } } KeyCode::Enter => { // Folder toggle (files handled above). - if row.expanded { - browse.expanded.remove(&row.path); - } else { - browse.expanded.insert(row.path.clone()); - } + browse.set_expanded(&row.path, !row.expanded); } _ => {} } - // Re-clamp against the new flattened length. - let new_len = browse.rows().len(); - browse.selected_row = - browse.selected_row.min(new_len.saturating_sub(1)); } self.sync_browse_selection(); diff --git a/soulseek-rs/src/ui/panes/browse_pane.rs b/soulseek-rs/src/ui/panes/browse_pane.rs index c5348d4..08299ad 100644 --- a/soulseek-rs/src/ui/panes/browse_pane.rs +++ b/soulseek-rs/src/ui/panes/browse_pane.rs @@ -1,8 +1,8 @@ use crate::models::{BrowseState, BrowseStatus, BrowseTabs}; use crate::ui::{ HIGHLIGHT_SYMBOL, body_style, dimmed_style, error_style, format_bytes, - get_spinner_char, highlight_style, pane_block, primary_style, - row_highlight_style, warning_style, + get_spinner_char, highlight_style, page_of, pane_block, primary_style, + row_highlight_style, visible_range, warning_style, }; use ratatui::{ Frame, @@ -106,8 +106,15 @@ fn render_browse_one( frame.render_widget(Paragraph::new(text).block(block), area); } BrowseStatus::Loaded => { - let rows: Vec = browse - .rows() + let all = browse.rows(); + let window = visible_range( + table_state.offset(), + Some(browse.selected_row), + all.len(), + page_of(Some(area), 0), + ); + let start = window.start; + let rows: Vec = all[window] .iter() .map(|row| { let indent = " ".repeat(row.depth); @@ -142,8 +149,13 @@ fn render_browse_one( .highlight_spacing(HighlightSpacing::Always) .block(block); - table_state.select(Some(browse.selected_row)); - frame.render_stateful_widget(table, area, table_state); + // The table holds one page, so the state it gets is shifted + // onto it. + let mut page_state = TableState::default().with_selected(Some( + browse.selected_row.min(all.len().saturating_sub(1)) - start, + )); + frame.render_stateful_widget(table, area, &mut page_state); + *table_state.offset_mut() = start; } } } diff --git a/soulseek-rs/src/ui/panes/downloads_pane.rs b/soulseek-rs/src/ui/panes/downloads_pane.rs index e624279..365627f 100644 --- a/soulseek-rs/src/ui/panes/downloads_pane.rs +++ b/soulseek-rs/src/ui/panes/downloads_pane.rs @@ -4,8 +4,8 @@ use crate::ui::{ GLYPH_ACTIVE, GLYPH_DONE, GLYPH_FAILED, GLYPH_QUEUED, HIGHLIGHT_SYMBOL, accent_style, body_style, dimmed_style, download_status_glyph, error_style, format_bytes, format_speed, header_style, inactive_style, info_style, - pane_block, pane_title, primary_style, row_highlight_style, success_style, - warning_style, + page_of, pane_block, pane_title, primary_style, row_highlight_style, + success_style, visible_range, warning_style, }; use ratatui::{ Frame, @@ -81,7 +81,23 @@ pub fn render_downloads_pane( .height(1); let name_width = column_width(area, &WIDTHS, NAME_COLUMN); - let mut rows: Vec = downloads + // Downloads come first, uploads after; one page of the two together. + let total = downloads.len() + uploads.len(); + if let Some(selected) = table_state.selected_mut() { + *selected = (*selected).min(total - 1); + } + let window = visible_range( + table_state.offset(), + table_state.selected(), + total, + page_of(Some(area), 1), + ); + let start = window.start; + let split = downloads.len(); + let shown_downloads = &downloads[start.min(split)..window.end.min(split)]; + let shown_uploads = + &uploads[start.saturating_sub(split)..window.end.saturating_sub(split)]; + let mut rows: Vec = shown_downloads .iter() .map(|download_entry| { let download = &download_entry.download; @@ -153,7 +169,7 @@ pub fn render_downloads_pane( }) .collect(); - rows.extend(uploads.iter().map(|upload| { + rows.extend(shown_uploads.iter().map(|upload| { let (status_icon, status_style) = match &upload.status { UploadStatus::Queued(_) => (GLYPH_QUEUED, warning_style()), UploadStatus::InProgress => (GLYPH_ACTIVE, accent_style()), @@ -213,5 +229,9 @@ pub fn render_downloads_pane( focused, ))); - frame.render_stateful_widget(table, area, table_state); + // The table holds one page, so the state it gets is shifted onto it. + let mut page_state = TableState::default() + .with_selected(table_state.selected().map(|selected| selected - start)); + frame.render_stateful_widget(table, area, &mut page_state); + *table_state.offset_mut() = start; } diff --git a/soulseek-rs/src/ui/panes/results_pane.rs b/soulseek-rs/src/ui/panes/results_pane.rs index 8361081..0c1c1d7 100644 --- a/soulseek-rs/src/ui/panes/results_pane.rs +++ b/soulseek-rs/src/ui/panes/results_pane.rs @@ -3,7 +3,7 @@ use crate::models::FileDisplayData; use crate::ui::{ BYTES_PER_MB, HIGHLIGHT_SYMBOL, body_style, dimmed_style, format_bytes, header_style, info_style, page_of, pane_block, pane_title, - row_highlight_style, success_style, warning_style, + row_highlight_style, success_style, visible_range, warning_style, }; use ratatui::{ Frame, @@ -11,7 +11,7 @@ use ratatui::{ text::{Line, Span}, widgets::{Cell, HighlightSpacing, Paragraph, Row, Table, TableState}, }; -use std::{collections::HashSet, ops::Range}; +use std::collections::HashSet; const VERSION: &str = env!("CARGO_PKG_VERSION"); @@ -85,26 +85,6 @@ fn row_is_selected( selected_indices.contains(&original) } -/// The page a `height`-tall table shows, by ratatui's rule (its -/// `visible_rows` is private): the offset, moved only as far as keeps the -/// selection in view. A selection past the end counts as the last row. -fn visible_range( - offset: usize, - selected: Option, - len: usize, - height: usize, -) -> Range { - let height = height.max(1); - let last = len.saturating_sub(1); - let start = match selected.map(|selected| selected.min(last)) { - Some(selected) => { - offset.clamp((selected + 1).saturating_sub(height), selected) - } - None => offset.min(last), - }; - start..(start + height).min(len) -} - pub fn render_results_pane( frame: &mut Frame, area: Rect, @@ -250,9 +230,7 @@ pub fn render_results_pane( #[cfg(test)] mod tests { - use super::{ - ResultsPaneParams, render_results_pane, row_is_selected, visible_range, - }; + use super::{ResultsPaneParams, render_results_pane, row_is_selected}; use crate::models::FileDisplayData; use ratatui::{Terminal, backend::TestBackend, widgets::TableState}; use std::collections::HashSet; @@ -371,15 +349,6 @@ mod tests { assert!(!screen.contains("…"), "{screen}"); } - #[test] - fn the_page_follows_the_selection_and_is_never_longer_than_the_pane() { - assert_eq!(visible_range(500, Some(505), 10_000, 10), 500..510); - assert_eq!(visible_range(0, Some(9_999), 10_000, 10), 9_990..10_000); - assert_eq!(visible_range(500, Some(499), 10_000, 10), 499..509); - assert_eq!(visible_range(9_995, None, 10_000, 10), 9_995..10_000); - assert_eq!(visible_range(0, Some(9), 3, 2), 1..3); - } - #[test] fn a_far_selection_is_drawn_from_its_own_page() { let items: Vec = diff --git a/soulseek-rs/src/ui/styles.rs b/soulseek-rs/src/ui/styles.rs index 83fa07b..35b30ca 100644 --- a/soulseek-rs/src/ui/styles.rs +++ b/soulseek-rs/src/ui/styles.rs @@ -5,6 +5,7 @@ use ratatui::style::{Color, Modifier, Style}; use ratatui::text::{Line, Span}; use ratatui::widgets::{Block, BorderType, Borders, Padding}; use soulseek_rs::DownloadStatus; +use std::ops::Range; /// One column of breathing room on each side, so content never touches the /// border. Vertical rows stay unpadded — they are too scarce in a terminal. @@ -160,6 +161,29 @@ pub fn page_of(area: Option, chrome: u16) -> usize { .max(1) } +/// The page a `height`-tall table shows, by ratatui's rule (its +/// `visible_rows` is private): the offset, moved only as far as keeps the +/// selection in view. A selection past the end counts as the last row. +/// Building rows for this page alone keeps a frame's cost flat however long +/// the list is. +#[must_use] +pub fn visible_range( + offset: usize, + selected: Option, + len: usize, + height: usize, +) -> Range { + let height = height.max(1); + let last = len.saturating_sub(1); + let start = match selected.map(|selected| selected.min(last)) { + Some(selected) => { + offset.clamp((selected + 1).saturating_sub(height), selected) + } + None => offset.min(last), + }; + start..(start + height).min(len) +} + pub fn download_status_glyph(status: &DownloadStatus) -> (&'static str, Style) { match status { DownloadStatus::Queued => (GLYPH_QUEUED, warning_style()), @@ -349,4 +373,13 @@ mod tests { assert_eq!(bar.spans[1].style.fg, Some(SIGNAL)); assert_eq!(bar.spans[2].content.chars().count(), 15); } + + #[test] + fn the_page_follows_the_selection_and_is_never_longer_than_the_pane() { + assert_eq!(visible_range(500, Some(505), 10_000, 10), 500..510); + assert_eq!(visible_range(0, Some(9_999), 10_000, 10), 9_990..10_000); + assert_eq!(visible_range(500, Some(499), 10_000, 10), 499..509); + assert_eq!(visible_range(9_995, None, 10_000, 10), 9_995..10_000); + assert_eq!(visible_range(0, Some(9), 3, 2), 1..3); + } }