Skip to content

Commit 90c8963

Browse files
committed
fix cache sentinel
1 parent c68aea1 commit 90c8963

2 files changed

Lines changed: 55 additions & 22 deletions

File tree

frontends/rioterm/src/renderer/island.rs

Lines changed: 41 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ use crate::renderer::utils::add_span_with_fallback;
1111
use rio_backend::event::{EventProxy, ProgressReport, ProgressState};
1212
use rio_backend::sugarloaf::{Attributes, SpanStyle, Sugarloaf};
1313
use rustc_hash::FxHashMap;
14+
use std::borrow::Cow;
1415
use std::time::Instant;
1516

1617
/// Height of the tab bar in pixels
@@ -33,11 +34,14 @@ const TITLE_ELLIPSIS: char = '…';
3334

3435
/// Truncate `title` to fit within `max_width` pixels at the tab font,
3536
/// appending `…` when characters have to be dropped. Thin adapter that
36-
/// asks sugarloaf's cached glyph advance for each char.
37-
fn fit_title_to_width(sugarloaf: &mut Sugarloaf, title: &str, max_width: f32) -> String {
38-
if max_width <= 0.0 || title.is_empty() {
39-
return title.to_string();
40-
}
37+
/// asks sugarloaf's cached glyph advance for each char. Returns
38+
/// `Cow::Borrowed(title)` when the full string fits so the common
39+
/// "no truncation needed" path avoids allocating.
40+
fn fit_title_to_width<'a>(
41+
sugarloaf: &mut Sugarloaf,
42+
title: &'a str,
43+
max_width: f32,
44+
) -> Cow<'a, str> {
4145
let attrs = Attributes::default();
4246
fit_title_with_widths(title, max_width, |c| {
4347
sugarloaf.char_advance(c, attrs, TITLE_FONT_SIZE)
@@ -49,14 +53,23 @@ fn fit_title_to_width(sugarloaf: &mut Sugarloaf, title: &str, max_width: f32) ->
4953
/// running total would exceed `max_width`. Separated from sugarloaf so
5054
/// tests can feed synthetic widths without a GPU context.
5155
///
56+
/// Returns `Cow::Borrowed(title)` when the full string fits, so the
57+
/// hot "no truncation needed" path does zero allocation.
58+
///
59+
/// `max_width <= 0.0` falls through the loop naturally: the first
60+
/// char's accumulated width already exceeds the budget, `truncate_ix`
61+
/// stays 0, and we return just `"…"` — a consistent sentinel that
62+
/// at least signals "there was content here". Empty input returns
63+
/// `Cow::Borrowed("")`.
64+
///
5265
/// Approximate (isolated per-char advances — no kerning, no ligatures,
5366
/// no emoji cluster formation). Fine for short labels where a pixel or
5467
/// two of slack is invisible.
55-
fn fit_title_with_widths(
56-
title: &str,
68+
fn fit_title_with_widths<'a>(
69+
title: &'a str,
5770
max_width: f32,
5871
mut char_width: impl FnMut(char) -> f32,
59-
) -> String {
72+
) -> Cow<'a, str> {
6073
let suffix_width = char_width(TITLE_ELLIPSIS);
6174

6275
// `truncate_ix` tracks the last byte offset at which the prefix so
@@ -74,10 +87,10 @@ fn fit_title_with_widths(
7487
let mut out = String::with_capacity(truncate_ix + TITLE_ELLIPSIS.len_utf8());
7588
out.push_str(&title[..truncate_ix]);
7689
out.push(TITLE_ELLIPSIS);
77-
return out;
90+
return Cow::Owned(out);
7891
}
7992
}
80-
title.to_string()
93+
Cow::Borrowed(title)
8194
}
8295

8396
/// Color picker constants
@@ -437,7 +450,7 @@ impl Island {
437450
sugarloaf.content().sel(tab_data.text_id).clear().new_line();
438451
add_span_with_fallback(sugarloaf, &title, base_style);
439452
sugarloaf.content().build();
440-
tab_data.last_title = title.clone();
453+
tab_data.last_title = title.into_owned();
441454

442455
// Position text to measure, then re-center using actual rendered width
443456
sugarloaf.set_position(tab_data.text_id, x_position, 0.0);
@@ -1053,6 +1066,23 @@ mod tests {
10531066
assert_eq!(fit_title_with_widths("hi", 2.0, fixed_unit_width), "hi");
10541067
}
10551068

1069+
#[test]
1070+
fn title_that_fits_borrows_without_allocating() {
1071+
// Confirms the zero-allocation "no truncation" hot path: when the
1072+
// full title fits, the returned Cow must stay Borrowed so the
1073+
// render loop doesn't allocate a new String every frame.
1074+
let out = fit_title_with_widths("ok", 10.0, fixed_unit_width);
1075+
assert!(matches!(out, Cow::Borrowed(_)), "expected borrowed, got {out:?}");
1076+
}
1077+
1078+
#[test]
1079+
fn title_zero_budget_returns_ellipsis() {
1080+
// Historically this was short-circuited to return the full title;
1081+
// now it falls through the loop and returns "…" consistently with
1082+
// tiny-but-positive budgets.
1083+
assert_eq!(fit_title_with_widths("abc", 0.0, fixed_unit_width), "…");
1084+
}
1085+
10561086
#[test]
10571087
fn title_overflow_gets_ellipsized_and_fits_budget() {
10581088
// "hello world" budgeted at 5 → best we can do without exceeding

sugarloaf/src/sugarloaf.rs

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -234,9 +234,10 @@ impl Sugarloaf<'_> {
234234
/// Returns `0.0` when the font library can't produce an advance
235235
/// (font id unregistered or SFNT parse failure) — the same shape
236236
/// an OS text engine returns for an unmapped glyph, so callers
237-
/// can sum widths without branching. These failures shouldn't
238-
/// happen outside a pathological font-library state; they are
239-
/// not cached, so they re-query on every call.
237+
/// can sum widths without branching. The failure is cached as an
238+
/// `AdvanceInfo` with `units_per_em = 0` (which `scaled` already
239+
/// treats as 0), so repeated queries for the same char don't
240+
/// re-walk the font data on every frame.
240241
///
241242
/// Lazy: the glyph cache keeps the advance `None` until the first
242243
/// `char_advance` call for this `(char, attrs)`, then fills it for
@@ -257,17 +258,19 @@ impl Sugarloaf<'_> {
257258
return advance.scaled(font_size);
258259
}
259260

260-
let info = {
261+
let computed = {
261262
let font_ctx = self.state.content.font_library().inner.read();
262263
compute_advance(&font_ctx, resolved.font_id, ch)
263264
};
264-
match info {
265-
Some(info) => {
266-
self.font_cache.set_advance((ch, attrs), info);
267-
info.scaled(font_size)
268-
}
269-
None => 0.0,
270-
}
265+
// Cache both hits AND misses — misses become a zero-advance
266+
// sentinel (`units_per_em = 0`) so `scaled()` returns 0 and
267+
// next frame short-circuits instead of re-walking font data.
268+
let info = computed.unwrap_or(crate::font_cache::AdvanceInfo {
269+
advance_units: 0.0,
270+
units_per_em: 0,
271+
});
272+
self.font_cache.set_advance((ch, attrs), info);
273+
info.scaled(font_size)
271274
}
272275

273276
/// Resolve a batch of glyph queries with a single FontLibrary

0 commit comments

Comments
 (0)