Skip to content

Commit ae0b2d3

Browse files
committed
Centre the avatar letter with a 1.0 line-height ratio, not a custom baseline
The requester found the simple fix locally. The Text box carried the default 1.2 line-height's leading, so centring that box never centred the glyph inside it. At a ratio of 1.0 the box is the glyph, and a plain `Align` puts it where it belongs. That retires the whole apparatus built around the old approach: - the custom `with_compute_baseline_position_fn` hook and its ink-bounds computation - the provisional whole-pixel raster lift - `avatar_letter_ink_height` and the font-cache plumbing feeding it, plus the AVATAR_GLYPH_RATIO and FALLBACK_CAP_HEIGHT_RATIO constants - the nested Flex that `glyph_centered` used, now just `Align` The pin glyph was sized off that cap-height helper, so it now derives from the disc directly via PIN_GLYPH_RATIO, calibrated to the same size the helper produced. One constant instead of a helper plus two constants plus font lookups. Diff against the branch point drops from +329/-101 to +204/-112. Co-Authored-By: Warp Agent <agent@warp.dev>
1 parent d5198b6 commit ae0b2d3

1 file changed

Lines changed: 35 additions & 171 deletions

File tree

app/src/ai/blocklist/agent_view/orchestration_pill_bar.rs

Lines changed: 35 additions & 171 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,10 @@ use warpui::elements::{
2626
PositionedElementOffsetBounds, PositioningAxis, Radius, SavePosition, ScrollbarWidth, Stack,
2727
Text, XAxisAnchor, YAxisAnchor,
2828
};
29-
use warpui::fonts::{FamilyId, Properties, Weight};
29+
use warpui::fonts::{Properties, Weight};
3030
use warpui::platform::{Cursor, LineStyle};
3131
use warpui::text_layout::{
32-
ClipConfig, ClipDirection, ClipStyle, ComputeBaselinePositionFn, DEFAULT_TOP_BOTTOM_RATIO,
33-
StyleAndFont, TextStyle,
32+
ClipConfig, ClipDirection, ClipStyle, DEFAULT_TOP_BOTTOM_RATIO, StyleAndFont, TextStyle,
3433
};
3534
use warpui::{
3635
AppContext, Entity, EntityId, ModelHandle, SingletonEntity, TypedActionView, View, ViewContext,
@@ -83,11 +82,6 @@ const PILL_AVATAR_VERTICAL_PADDING: f32 = (PILL_HEIGHT - PILL_AVATAR_DISC_SIZE)
8382
/// size the avatar disc (that is [`PILL_AVATAR_DISC_SIZE`]) — it only reserves
8483
/// the square whose bottom-right corner the badge hangs off.
8584
const AVATAR_WITH_STATUS_TOTAL_SIZE: f32 = PILL_AVATAR_SLOT_SIZE;
86-
/// Letter/icon size inside an avatar disc, as a fraction of the disc.
87-
const AVATAR_GLYPH_RATIO: f32 = 0.625;
88-
/// Cap-height fallback used only when the font reports no bounds for `H`.
89-
/// Roboto's is 0.711; this is close enough to keep a glyph sane in that case.
90-
const FALLBACK_CAP_HEIGHT_RATIO: f32 = 0.711;
9185
const PILL_LABEL_MAX_WIDTH: f32 = 83.;
9286
const PILL_ROW_GAP: f32 = 8.;
9387
const PILL_CONTENT_GAP: f32 = 2.;
@@ -1936,48 +1930,6 @@ fn render_pinned_divider(app: &AppContext) -> Box<dyn Element> {
19361930
.finish()
19371931
}
19381932

1939-
/// Ink height of an avatar letter on a disc of `disc_size` — the UI font's cap
1940-
/// height at the size [`render_avatar_disc`] draws letters at. `H` is the
1941-
/// reference capital because its flat top and bottom make its ink box exactly
1942-
/// the cap height.
1943-
///
1944-
/// The pin glyph is sized off this so hovering swaps one mark for another of
1945-
/// the same visual weight instead of jumping to something much heavier.
1946-
fn avatar_letter_ink_height(disc_size: f32, appearance: &Appearance, app: &AppContext) -> f32 {
1947-
let glyph_size = disc_size * AVATAR_GLYPH_RATIO;
1948-
let font_cache = app.font_cache();
1949-
let font_id = font_cache.select_font(
1950-
appearance.ui_font_family(),
1951-
Properties {
1952-
weight: Weight::Bold,
1953-
..Default::default()
1954-
},
1955-
);
1956-
font_cache
1957-
.glyph_for_char(font_id, 'H', false)
1958-
.and_then(|(glyph_id, glyph_font_id)| {
1959-
font_cache
1960-
.glyph_typographic_bounds(glyph_font_id, glyph_size, glyph_id)
1961-
.ok()
1962-
})
1963-
.map(|ink| ink.max_y() - ink.min_y())
1964-
.unwrap_or(glyph_size * FALLBACK_CAP_HEIGHT_RATIO)
1965-
}
1966-
1967-
/// The pin glyph itself, sized so its box matches `ink_height`. Solid when
1968-
/// pinned, outline when not.
1969-
fn render_pin_glyph(is_pinned: bool, icon_color: ColorU, ink_height: f32) -> Box<dyn Element> {
1970-
let icon_variant = if is_pinned {
1971-
Icon::PinFilled
1972-
} else {
1973-
Icon::Pin
1974-
};
1975-
ConstrainedBox::new(icon_variant.to_warpui_icon(icon_color.into()).finish())
1976-
.with_width(ink_height)
1977-
.with_height(ink_height)
1978-
.finish()
1979-
}
1980-
19811933
/// The clickable pin button a child pill shows in place of its avatar while
19821934
/// hovered: a circle occupying exactly the avatar disc's rect.
19831935
///
@@ -1990,14 +1942,11 @@ fn render_pin_glyph(is_pinned: bool, icon_color: ColorU, ink_height: f32) -> Box
19901942
/// The glyph is centered by equal padding rather than by a centering
19911943
/// container, which keeps it exact regardless of how the surrounding box
19921944
/// behaves.
1993-
#[allow(clippy::too_many_arguments)]
19941945
fn render_pin_button(
19951946
is_pinned: bool,
19961947
icon_color: ColorU,
19971948
mouse_state: MouseStateHandle,
19981949
conversation_id: AIConversationId,
1999-
appearance: &Appearance,
2000-
app: &AppContext,
20011950
) -> Box<dyn Element> {
20021951
// Tint with the pill's own contrasting colour rather than a fixed
20031952
// foreground overlay. `fg_overlay_1` is the foreground at 5% opacity, and a
@@ -2006,11 +1955,19 @@ fn render_pin_button(
20061955
// selected chip — which, since the bar anchors on the parent of whatever
20071956
// leaf you are viewing, is the common case rather than an edge case.
20081957
let hover_background = coloru_with_opacity(icon_color, PIN_BUTTON_HOVER_OPACITY);
2009-
let glyph_ink_height =
2010-
avatar_letter_ink_height(PILL_AVATAR_DISC_SIZE, appearance, app) + PIN_GLYPH_INK_BOOST;
1958+
let glyph_size = PILL_AVATAR_DISC_SIZE * PIN_GLYPH_RATIO;
1959+
let icon = if is_pinned {
1960+
Icon::PinFilled
1961+
} else {
1962+
Icon::Pin
1963+
};
20111964
let button = Hoverable::new(mouse_state, move |hover_state| {
2012-
let mut circle = Container::new(render_pin_glyph(is_pinned, icon_color, glyph_ink_height))
2013-
.with_uniform_padding((PILL_AVATAR_DISC_SIZE - glyph_ink_height) / 2.)
1965+
let glyph = ConstrainedBox::new(icon.to_warpui_icon(icon_color.into()).finish())
1966+
.with_width(glyph_size)
1967+
.with_height(glyph_size)
1968+
.finish();
1969+
let mut circle = Container::new(glyph)
1970+
.with_uniform_padding((PILL_AVATAR_DISC_SIZE - glyph_size) / 2.)
20141971
.with_corner_radius(CornerRadius::with_all(Radius::Pixels(
20151972
PILL_AVATAR_DISC_SIZE / 2.,
20161973
)));
@@ -2183,8 +2140,6 @@ fn render_pill(
21832140
text_color,
21842141
pin_button_mouse_state.clone(),
21852142
conversation_id,
2186-
appearance,
2187-
app,
21882143
)
21892144
} else if let Some(ref status) = status {
21902145
render_avatar_with_status_overlay(
@@ -2396,11 +2351,12 @@ fn render_overflow_button(
23962351
SavePosition::new(button, &overflow_button_position_id(conversation_id)).finish()
23972352
}
23982353

2399-
/// How much taller the pin glyph is than the avatar letter's ink. Matching the
2400-
/// two exactly made the pin read *smaller* than the letter it replaces: a
2401-
/// thin, busy outline carries less visual weight than a solid letterform at
2402-
/// the same height. This is the knob to nudge if it still reads wrong.
2403-
const PIN_GLYPH_INK_BOOST: f32 = 4.;
2354+
/// Pin glyph size as a fraction of the avatar disc it sits in. Calibrated
2355+
/// against the letter it replaces: matching the letter's ink height exactly
2356+
/// read too *small*, because a thin outline carries less visual weight than a
2357+
/// solid letterform, so design asked for roughly 4px more. This is the knob to
2358+
/// nudge if it still reads wrong.
2359+
const PIN_GLYPH_RATIO: f32 = 0.71;
24042360

24052361
/// Opacity of the pin button's hover tint, over the pill's contrasting colour.
24062362
/// A little stronger than the 5% `fg_overlay_1` used to apply, because that
@@ -2535,80 +2491,6 @@ fn render_avatar_with_status_overlay(
25352491
render_avatar_slot(lockup)
25362492
}
25372493

2538-
/// Whole-pixel lift applied to the avatar letter's baseline.
2539-
///
2540-
/// Centering the ink arithmetically still paints it low, measured at 0.93px on
2541-
/// a 15px disc across H, O and S. Three mechanisms were tested against the
2542-
/// pixels and all three predicted the wrong direction or magnitude:
2543-
/// antialiasing fringe (disproved -- the coverage profile has hard zeros
2544-
/// either side of the ink), rasterisation at the line height rather than the
2545-
/// font size, and centering on the layout rect where the disc paints on a
2546-
/// snapped one. What is left behaves like a rounding step in the glyph
2547-
/// rasteriser's vertical placement, which is an absolute pixel effect rather
2548-
/// than a proportional one -- so this is expressed in pixels deliberately, and
2549-
/// a whole one so the baseline's fractional part is unchanged.
2550-
///
2551-
/// Measured on a 15px disc, coverage-weighted ink centroid against the disc's
2552-
/// centre, three letters: before the lift +0.82 / +0.89 / +0.89 low; after it
2553-
/// -0.18 / 0.00 / -0.04. So the lift translates the glyph ~1:1 and lands the
2554-
/// ink centred to within a fifth of a pixel, comfortably inside antialiasing
2555-
/// noise.
2556-
///
2557-
/// Only verified at the 15px chip avatar. If it is the rounding effect it
2558-
/// appears to be, it is absolute and should hold at the 16px hover-card and
2559-
/// 1.25x transcript avatars too, but that has not been measured.
2560-
const AVATAR_GLYPH_RASTER_LIFT: f32 = 1.;
2561-
2562-
/// Baseline placement that centers an avatar letter's *ink* on its disc.
2563-
///
2564-
/// Two separate things push the letter off center by default, and this fixes
2565-
/// both at once by choosing the baseline outright:
2566-
/// * A line box spans ascent + descent, but a capital only occupies
2567-
/// baseline-to-cap-height, so centering the box still leaves the ink high.
2568-
/// * The glyph is painted at the *top* of the `disc_size`-square box
2569-
/// [`render_avatar_disc`] wraps it in rather than centered in it. Measured
2570-
/// off real frames: with the default baseline the ink sat 2px above center
2571-
/// on a 15px disc. The returned baseline is therefore measured from the
2572-
/// disc's top edge, not from the middle of the line box.
2573-
///
2574-
/// Everything here is derived from the font's own metrics at the size actually
2575-
/// being drawn, so the 15px chip avatar, the 16px hover-card avatar and the
2576-
/// 1.25x transcript avatar all land correctly without a per-size nudge.
2577-
fn center_glyph_ink_baseline_position_fn(
2578-
letter: char,
2579-
family_id: FamilyId,
2580-
properties: Properties,
2581-
disc_size: f32,
2582-
) -> ComputeBaselinePositionFn {
2583-
Box::new(move |args| {
2584-
let font_id = args.font_cache.select_font(family_id, properties);
2585-
// Typographic bounds are y-up from the baseline: `max_y` is the ink's
2586-
// top above it and `min_y` its bottom. A glyph with no bounds — a
2587-
// blank, or a stub font in tests — just puts the baseline on the
2588-
// disc's center line.
2589-
let Some(ink) = args
2590-
.font_cache
2591-
.glyph_for_char(font_id, letter, false)
2592-
.and_then(|(glyph_id, glyph_font_id)| {
2593-
args.font_cache
2594-
.glyph_typographic_bounds(glyph_font_id, args.font_size, glyph_id)
2595-
.ok()
2596-
})
2597-
else {
2598-
return disc_size / 2.;
2599-
};
2600-
// Centering the ink exactly leaves both of its edges on fractional
2601-
// device pixels, and the rasterizer's coverage then fringes further
2602-
// down than up, which reads as the letter sitting low. Snapping the
2603-
// ink's top edge to a whole pixel gives it a crisp edge to sit on.
2604-
// The snap is applied after the size-scaled computation, so it holds
2605-
// at every avatar size rather than being tuned to one.
2606-
let ink_height = ink.max_y() - ink.min_y();
2607-
let ink_top = ((disc_size - ink_height) / 2.).round();
2608-
ink_top + ink.max_y() - AVATAR_GLYPH_RASTER_LIFT
2609-
})
2610-
}
2611-
26122494
/// Renders the avatar circle as a colored disc with a centered glyph (letter
26132495
/// or icon) on top. Uses `Stack` so the disc is a clean rounded square that
26142496
/// composites cleanly over the pill's own background without visual seams.
@@ -2628,21 +2510,21 @@ fn render_avatar_disc(
26282510
.with_width(size)
26292511
.with_height(size)
26302512
.finish();
2631-
let glyph_size = size * AVATAR_GLYPH_RATIO;
2513+
let glyph_size = size * 0.625;
26322514

26332515
let glyph_element: Box<dyn Element> = match glyph {
26342516
AvatarGlyph::Letter(letter) => {
2635-
let family_id = appearance.ui_font_family();
2636-
let properties = Properties {
2637-
weight: Weight::Bold,
2638-
..Default::default()
2639-
};
2640-
Text::new(letter.to_string(), family_id, glyph_size)
2517+
Text::new(letter.to_string(), appearance.ui_font_family(), glyph_size)
26412518
.with_color(theme.background().into_solid())
2642-
.with_style(properties)
2643-
.with_compute_baseline_position_fn(center_glyph_ink_baseline_position_fn(
2644-
letter, family_id, properties, size,
2645-
))
2519+
.with_style(Properties {
2520+
weight: Weight::Bold,
2521+
..Default::default()
2522+
})
2523+
// The default 1.2 ratio pads the text box with leading, so
2524+
// centering the box leaves the letter's ink sitting high in
2525+
// the disc. At 1.0 the box is the glyph, and centering it
2526+
// centers what you can see.
2527+
.with_line_height_ratio(1.)
26462528
.finish()
26472529
}
26482530
AvatarGlyph::Icon(icon) => {
@@ -2653,28 +2535,10 @@ fn render_avatar_disc(
26532535
}
26542536
};
26552537

2656-
// Horizontal centering only. The nested `Flex` centers the glyph across
2657-
// the disc, but does not move it vertically — letters are positioned by
2658-
// `center_glyph_ink_baseline_position_fn` instead, and the icon variant is
2659-
// already square and centered.
2660-
let glyph_centered = ConstrainedBox::new(
2661-
Flex::column()
2662-
.with_main_axis_size(MainAxisSize::Max)
2663-
.with_main_axis_alignment(MainAxisAlignment::Center)
2664-
.with_cross_axis_alignment(CrossAxisAlignment::Center)
2665-
.with_child(
2666-
Flex::row()
2667-
.with_main_axis_size(MainAxisSize::Max)
2668-
.with_main_axis_alignment(MainAxisAlignment::Center)
2669-
.with_cross_axis_alignment(CrossAxisAlignment::Center)
2670-
.with_child(glyph_element)
2671-
.finish(),
2672-
)
2673-
.finish(),
2674-
)
2675-
.with_width(size)
2676-
.with_height(size)
2677-
.finish();
2538+
let glyph_centered = ConstrainedBox::new(Align::new(glyph_element).finish())
2539+
.with_width(size)
2540+
.with_height(size)
2541+
.finish();
26782542

26792543
Stack::new()
26802544
.with_child(disc)

0 commit comments

Comments
 (0)