From aa982319c2642918182e88bdace1ef9fea9ebc4b Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sun, 19 Jul 2026 19:28:53 +0000 Subject: [PATCH] Speed up TUI Markdown layout (#34216) ## What changed - Allocate Markdown table widths in bulk while preserving the existing column shrink priorities and balancing behavior. - Reuse flattened styled-line data during adaptive wrapping and detect URLs across span boundaries. - Remap wrapped terminal hyperlinks with a single forward scan. ## Testing - Verify bulk table shrinking matches the previous one-cell-at-a-time results, including a token-heavy column with a million-character width. - Cover URLs split across styled spans and multiple links on indented Unicode lines. GitOrigin-RevId: 24a6fd6fdace6ca948e613d67c130a3a7f8d1a4e --- codex-rs/tui/src/markdown_render.rs | 277 ++++++++++++++++-------- codex-rs/tui/src/terminal_hyperlinks.rs | 60 ++++- codex-rs/tui/src/wrapping.rs | 71 ++++-- 3 files changed, 298 insertions(+), 110 deletions(-) diff --git a/codex-rs/tui/src/markdown_render.rs b/codex-rs/tui/src/markdown_render.rs index a1a976f61c..8290b83f99 100644 --- a/codex-rs/tui/src/markdown_render.rs +++ b/codex-rs/tui/src/markdown_render.rs @@ -1120,8 +1120,7 @@ where let metrics = Self::collect_table_column_metrics(&header, &rows, column_count); let available_width = self.available_table_width(column_count); - let widths = - self.compute_column_widths(&header, &rows, &table_state.alignments, available_width); + let widths = Self::compute_column_widths(&metrics, available_width); let spillover_lines: Vec = spillover_rows .into_iter() .flat_map(|spillover| spillover.lines) @@ -1236,19 +1235,15 @@ where /// Allocate column widths for aligned, row-separated table rendering. /// /// Each column starts at its natural (max cell content) width, then columns - /// are iteratively shrunk one character at a time until the total fits within - /// `available_width`. Token-heavy columns surrender excess width before - /// narrative prose; compact columns are preserved last. Returns `None` when - /// even the minimum width (3 chars per column) cannot fit. + /// are shrunk by priority until the total fits within `available_width`. + /// Token-heavy columns surrender excess width before narrative prose; compact + /// columns are preserved last. Returns `None` when even the minimum width + /// (3 chars per column) cannot fit. fn compute_column_widths( - &self, - header: &[TableCell], - rows: &[Vec], - alignments: &[Alignment], + metrics: &[TableColumnMetrics], available_width: Option, ) -> Option> { let min_column_width = 3usize; - let metrics = Self::collect_table_column_metrics(header, rows, alignments.len()); let mut widths: Vec = metrics .iter() .map(|col| col.max_width.max(min_column_width)) @@ -1257,7 +1252,7 @@ where let Some(max_width) = available_width else { return Some(widths); }; - let minimum_total = alignments.len() * min_column_width; + let minimum_total = metrics.len() * min_column_width; if max_width < minimum_total { return None; } @@ -1266,41 +1261,19 @@ where .iter() .map(|col| Self::preferred_column_floor(col, min_column_width)) .collect(); - let mut floor_total: usize = floors.iter().sum(); + let floor_total: usize = floors.iter().sum(); if floor_total > max_width { - // Relax preferred floors in wrapping priority order until the hard width budget fits. - while floor_total > max_width { - let Some((idx, _)) = floors - .iter() - .enumerate() - .filter(|(_, floor)| **floor > min_column_width) - .min_by_key(|(idx, floor)| { - ( - Self::column_shrink_priority(metrics[*idx].kind), - usize::MAX.saturating_sub(**floor), - ) - }) - else { - break; - }; - - floors[idx] -= 1; - floor_total -= 1; - } - } - - let mut total_width: usize = widths.iter().sum(); - - while total_width > max_width { - let Some(idx) = Self::next_column_to_shrink(&widths, &floors, &metrics) else { - break; - }; - widths[idx] -= 1; - total_width -= 1; + let minimums = vec![min_column_width; floors.len()]; + Self::shrink_columns(&mut floors, &minimums, metrics, floor_total - max_width); } + let total_width: usize = widths.iter().sum(); if total_width > max_width { - return None; + let remaining = + Self::shrink_columns(&mut widths, &floors, metrics, total_width - max_width); + if remaining > 0 { + return None; + } } Some(widths) @@ -1328,14 +1301,15 @@ where let cell = &row[column]; max_width = max_width.max(Self::cell_display_width(cell)); let plain = cell.plain_text(); - body_token_width = body_token_width.max(Self::longest_token_width(&plain)); - let word_count = plain.split_whitespace().count(); + let mut word_count = 0usize; + for token in plain.split_whitespace() { + let token_width = token.width(); + body_token_width = body_token_width.max(token_width); + long_body_token_count += usize::from(token_width >= 20); + word_count += 1; + } if word_count > 0 { body_token_count += word_count; - long_body_token_count += plain - .split_whitespace() - .filter(|token| token.width() >= 20) - .count(); total_words += word_count; total_cells += 1; total_cell_width += plain.width(); @@ -1389,36 +1363,87 @@ where token_target.max(min_column_width).min(metrics.max_width) } - /// Pick the next column to shrink by one character during width allocation. + /// Shrink columns in priority order, balancing slack within each priority. /// /// Priority: TokenHeavy columns are shrunk before Narrative, then Compact. - /// Within the same kind, the column with the most slack above its floor is - /// chosen so similarly-shaped columns stay balanced. - fn next_column_to_shrink( - widths: &[usize], + /// Within the same kind, columns with the most slack above their floor are + /// shrunk first so similarly-shaped columns stay balanced. This produces the + /// same result as repeatedly shrinking one display cell, without repeatedly + /// scanning every column for long tokens. + fn shrink_columns( + widths: &mut [usize], floors: &[usize], metrics: &[TableColumnMetrics], - ) -> Option { - widths - .iter() - .enumerate() - .filter(|(idx, width)| **width > floors[*idx]) - .min_by_key(|(idx, width)| { - let slack = width.saturating_sub(floors[*idx]); - ( - Self::column_shrink_priority(metrics[*idx].kind), - usize::MAX.saturating_sub(slack), - ) - }) - .map(|(idx, _)| idx) - } + mut amount: usize, + ) -> usize { + for kind in [ + TableColumnKind::TokenHeavy, + TableColumnKind::Narrative, + TableColumnKind::Compact, + ] { + let slack_total = widths + .iter() + .enumerate() + .filter(|(idx, _)| metrics[*idx].kind == kind) + .map(|(idx, width)| width.saturating_sub(floors[idx])) + .sum::(); + let to_remove = amount.min(slack_total); + if to_remove == 0 { + continue; + } - fn column_shrink_priority(kind: TableColumnKind) -> usize { - match kind { - TableColumnKind::TokenHeavy => 0, - TableColumnKind::Narrative => 1, - TableColumnKind::Compact => 2, + let mut low = 0usize; + let mut high = widths + .iter() + .enumerate() + .filter(|(idx, _)| metrics[*idx].kind == kind) + .map(|(idx, width)| width.saturating_sub(floors[idx])) + .max() + .unwrap_or(/*default*/ 0); + while low < high { + let cap = low + (high - low) / 2; + let removed = widths + .iter() + .enumerate() + .filter(|(idx, _)| metrics[*idx].kind == kind) + .map(|(idx, width)| width.saturating_sub(floors[idx]).saturating_sub(cap)) + .sum::(); + if removed > to_remove { + low = cap + 1; + } else { + high = cap; + } + } + + let cap = low; + let mut removed = 0usize; + for (idx, width) in widths.iter_mut().enumerate() { + if metrics[idx].kind != kind { + continue; + } + let reduction = width.saturating_sub(floors[idx]).saturating_sub(cap); + *width -= reduction; + removed += reduction; + } + + let mut remainder = to_remove - removed; + for (idx, width) in widths.iter_mut().enumerate() { + if remainder == 0 { + break; + } + if metrics[idx].kind == kind && width.saturating_sub(floors[idx]) == cap { + *width -= 1; + remainder -= 1; + } + } + + amount -= to_remove; + if amount == 0 { + break; + } } + + amount } fn render_table_separator( @@ -2596,38 +2621,100 @@ mod tests { } #[test] - fn next_column_to_shrink_prefers_token_heavy_then_narrative() { - let widths = [20usize, 20, 20]; - let floors = [8usize, 8, 8]; + fn bulk_column_shrink_matches_one_cell_at_a_time() { + fn priority(kind: TableColumnKind) -> usize { + match kind { + TableColumnKind::TokenHeavy => 0, + TableColumnKind::Narrative => 1, + TableColumnKind::Compact => 2, + } + } + + fn shrink_one_at_a_time( + widths: &mut [usize], + floors: &[usize], + metrics: &[TableColumnMetrics], + mut amount: usize, + ) -> usize { + while amount > 0 { + let Some(idx) = widths + .iter() + .enumerate() + .filter(|(idx, width)| **width > floors[*idx]) + .min_by_key(|(idx, width)| { + ( + priority(metrics[*idx].kind), + usize::MAX - width.saturating_sub(floors[*idx]), + ) + }) + .map(|(idx, _)| idx) + else { + break; + }; + widths[idx] -= 1; + amount -= 1; + } + amount + } + + for case in 0..64usize { + let metrics = (0..5) + .map(|idx| TableColumnMetrics { + max_width: 100, + header_token_width: 8, + body_token_width: 8, + kind: match (case + idx) % 3 { + 0 => TableColumnKind::TokenHeavy, + 1 => TableColumnKind::Narrative, + _ => TableColumnKind::Compact, + }, + }) + .collect::>(); + let floors = (0..5) + .map(|idx| 3 + (case * (idx + 1) + idx) % 14) + .collect::>(); + let initial = floors + .iter() + .enumerate() + .map(|(idx, floor)| floor + (case * (idx + 3) + idx * 7) % 41) + .collect::>(); + let slack = initial + .iter() + .zip(&floors) + .map(|(width, floor)| width - floor) + .sum::(); + + for amount in 0..=slack + 1 { + let mut expected = initial.clone(); + let expected_remaining = + shrink_one_at_a_time(&mut expected, &floors, &metrics, amount); + let mut actual = initial.clone(); + let actual_remaining = W::shrink_columns(&mut actual, &floors, &metrics, amount); + assert_eq!((actual, actual_remaining), (expected, expected_remaining)); + } + } + } + + #[test] + fn column_widths_bulk_shrink_large_token_heavy_column() { let metrics = [ TableColumnMetrics { - max_width: 30, - header_token_width: 8, - body_token_width: 6, - kind: TableColumnKind::Narrative, - }, - TableColumnMetrics { - max_width: 30, - header_token_width: 8, - body_token_width: 28, + max_width: 1_000_000, + header_token_width: 4, + body_token_width: 1_000_000, kind: TableColumnKind::TokenHeavy, }, TableColumnMetrics { - max_width: 30, - header_token_width: 8, - body_token_width: 6, + max_width: 8, + header_token_width: 5, + body_token_width: 8, kind: TableColumnKind::Compact, }, ]; - let idx = W::next_column_to_shrink(&widths, &floors, &metrics); - assert_eq!(idx, Some(1), "token-heavy column should shrink first"); - let widths = [20usize, 8, 20]; - let idx = W::next_column_to_shrink(&widths, &floors, &metrics); assert_eq!( - idx, - Some(0), - "narrative column should shrink before compact" + W::compute_column_widths(&metrics, /*available_width*/ Some(24)), + Some(vec![16, 8]) ); } diff --git a/codex-rs/tui/src/terminal_hyperlinks.rs b/codex-rs/tui/src/terminal_hyperlinks.rs index b89672c518..6fe458d9c6 100644 --- a/codex-rs/tui/src/terminal_hyperlinks.rs +++ b/codex-rs/tui/src/terminal_hyperlinks.rs @@ -207,9 +207,14 @@ pub(crate) fn remap_wrapped_line( wrapped: Vec>, ) -> Vec { let mut out = plain_hyperlink_lines(wrapped); + if source.hyperlinks.is_empty() { + return out; + } + let source_text = line_text(&source.line); let mut source_byte = 0usize; let mut source_column = 0usize; + let mut link_index = 0usize; for (index, line) in out.iter_mut().enumerate() { if index > 0 { let trimmed = source_text[source_byte..].trim_start_matches(char::is_whitespace); @@ -227,10 +232,17 @@ pub(crate) fn remap_wrapped_line( let mut output_column = rendered[..rendered_start].width(); for ch in mapped.chars() { let width = ch.width().unwrap_or(/*default*/ 0); + while source + .hyperlinks + .get(link_index) + .is_some_and(|link| link.columns.end <= source_column) + { + link_index += 1; + } if let Some(link) = source .hyperlinks - .iter() - .find(|link| link.columns.contains(&source_column)) + .get(link_index) + .filter(|link| link.columns.contains(&source_column)) { push_link_range(line, output_column..output_column + width, link); } @@ -636,6 +648,50 @@ mod tests { ); } + #[test] + fn wrapping_maps_multiple_links_across_indented_unicode_lines() { + let text = "alpha 😀here middle there end"; + let first_start = text.find("here").expect("first link"); + let second_start = text.find("there").expect("second link"); + let mut source = HyperlinkLine::new(Line::from(text)); + source.hyperlinks.push(TerminalHyperlink::web( + text[..first_start].width()..text[..first_start].width() + "here".width(), + "https://example.com/first".to_string(), + )); + source.hyperlinks.push(TerminalHyperlink::web( + text[..second_start].width()..text[..second_start].width() + "there".width(), + "https://example.com/second".to_string(), + )); + + let wrapped = remap_wrapped_line( + &source, + vec![ + Line::from(" alpha 😀here"), + Line::from(" middle there end"), + ], + ); + + assert_eq!( + wrapped, + vec![ + HyperlinkLine { + line: Line::from(" alpha 😀here"), + hyperlinks: vec![TerminalHyperlink::web( + /*columns*/ 10..14, + "https://example.com/first".to_string(), + )], + }, + HyperlinkLine { + line: Line::from(" middle there end"), + hyperlinks: vec![TerminalHyperlink::web( + /*columns*/ 11..16, + "https://example.com/second".to_string(), + )], + }, + ] + ); + } + #[test] fn buffer_hyperlinks_follow_word_wrapping() { let destination = "https://example.com/path"; diff --git a/codex-rs/tui/src/wrapping.rs b/codex-rs/tui/src/wrapping.rs index fcec7c1ae7..043b5d5319 100644 --- a/codex-rs/tui/src/wrapping.rs +++ b/codex-rs/tui/src/wrapping.rs @@ -506,14 +506,28 @@ pub(crate) fn url_preserving_wrap_options<'a>(opts: RtOptions<'a>) -> RtOptions< /// while a genuinely overlong non-URL token can still split if needed. #[must_use] pub(crate) fn adaptive_wrap_line<'a>(line: &'a Line<'a>, base: RtOptions<'a>) -> Vec> { - if !line_contains_url_like(line) { - return word_wrap_line(line, base); + let (flat, span_bounds) = flatten_line(line); + let mut saw_url = false; + let mut saw_non_url = false; + + for token in flat.split_ascii_whitespace() { + if is_url_like_token(token) { + saw_url = true; + } else if is_substantive_non_url_token(token) { + saw_non_url = true; + } + + if saw_url && saw_non_url { + break; + } } - if line_has_mixed_url_and_non_url_tokens(line) { - mixed_url_wrap_line(line, base) + if !saw_url { + word_wrap_flattened_line(line, &flat, &span_bounds, base) + } else if saw_non_url { + mixed_url_wrap_line(line, &flat, &span_bounds, base) } else { - word_wrap_line(line, url_preserving_wrap_options(base)) + word_wrap_flattened_line(line, &flat, &span_bounds, url_preserving_wrap_options(base)) } } @@ -661,8 +675,15 @@ where O: Into>, { let (flat, span_bounds) = flatten_line(line); + word_wrap_flattened_line(line, &flat, &span_bounds, width_or_options.into()) +} - let rt_opts: RtOptions<'a> = width_or_options.into(); +fn word_wrap_flattened_line<'a>( + line: &'a Line<'a>, + flat: &str, + span_bounds: &[(Range, ratatui::style::Style)], + rt_opts: RtOptions<'a>, +) -> Vec> { let opts = Options::new(rt_opts.width) .line_ending(rt_opts.line_ending) .break_words(rt_opts.break_words) @@ -677,7 +698,7 @@ where .width .saturating_sub(rt_opts.initial_indent.width()) .max(1); - let initial_wrapped = wrap_ranges_trim(&flat, opts.clone().width(initial_width_available)); + let initial_wrapped = wrap_ranges_trim(flat, opts.clone().width(initial_width_available)); let Some(first_line_range) = initial_wrapped.first() else { return vec![rt_opts.initial_indent.clone()]; }; @@ -685,7 +706,7 @@ where // Build first wrapped line with initial indent. let mut first_line = rt_opts.initial_indent.clone().style(line.style); { - let sliced = slice_line_spans(line, &span_bounds, first_line_range); + let sliced = slice_line_spans(line, span_bounds, first_line_range); let mut spans = first_line.spans; spans.append( &mut sliced @@ -713,7 +734,7 @@ where } let mut subsequent_line = rt_opts.subsequent_indent.clone().style(line.style); let offset_range = (r.start + base)..(r.end + base); - let sliced = slice_line_spans(line, &span_bounds, &offset_range); + let sliced = slice_line_spans(line, span_bounds, &offset_range); let mut spans = subsequent_line.spans; spans.append( &mut sliced @@ -741,8 +762,12 @@ impl MixedUrlWord { } } -fn mixed_url_wrap_line<'a>(line: &'a Line<'a>, rt_opts: RtOptions<'a>) -> Vec> { - let (flat, span_bounds) = flatten_line(line); +fn mixed_url_wrap_line<'a>( + line: &'a Line<'a>, + flat: &str, + span_bounds: &[(Range, ratatui::style::Style)], + rt_opts: RtOptions<'a>, +) -> Vec> { let initial_width_available = rt_opts .width .saturating_sub(rt_opts.initial_indent.width()) @@ -751,7 +776,7 @@ fn mixed_url_wrap_line<'a>(line: &'a Line<'a>, rt_opts: RtOptions<'a>) -> Vec
  • (line: &'a Line<'a>, rt_opts: RtOptions<'a>) -> Vec