Skip to content

Commit ef0ac45

Browse files
Fix inline-code link underline hidden by run background in agent output (warpdotdev#13030)
## Description Fixes "wrong link highlighting" in agent rich output (reported in Slack): a detected link rendered as inline code (gray code background) lost all or part of its hover underline, in a width-dependent way. **Root cause:** A paint-ordering bug in `Line::paint_internal` (`crates/warpui_core/src/text_layout.rs`). The run's background/border rect was painted *after* the run's glyphs and per-glyph underline. The hyperlink underline is a filled rect in the **same paint layer** as the background, so drawing the background last painted over and hid the underline on any backgrounded run (e.g. an inline-code link). Plain-text links (no run background) were never affected, which matches the reported symptom. **Fix:** Split background/border painting into a new `paint_run_background` and call it *before* the glyph loop, so glyphs and the underline render on top of the background. `paint_run_decorations` now only handles the error underline and strikethrough (drawn after glyphs, unchanged); its now-unused `font_cache`/`baseline_position_fn` params were dropped. **Truncation correctness (from review):** Because the background is now painted before the glyph loop, it must not be drawn for glyphs that the loop then truncates. Each run's actually-drawn glyph x-span is precomputed before drawing — mirroring the same ellipsis cutoff and `remaining_width <= 0` stop conditions used by the glyph loop, for both clip directions including start-clipping's reversed iteration. A run with no visible glyphs paints no background, and a partially-truncated run's background/border is clamped to its visible glyph span (so it no longer extends past the visible glyphs or behind an ellipsis). The background is still painted before the per-glyph underline rects, preserving the underline fix. Affected file: `crates/warpui_core/src/text_layout.rs`. ## Linked Issue Slack thread: https://warpdev.slack.com/archives/C0BCE7AELJ2/p1782330944826139?thread_ts=1782330944.826139&cid=C0BCE7AELJ2 ## Testing Deterministic regression tests in `crates/warpui_core/src/text_layout_tests.rs`: - `test_run_background_painted_before_underline` — asserts (via the resulting `Scene` draw order) the background rect is drawn *before* the underline rect, so the underline renders on top. Fails on the pre-fix paint order. - `test_run_background_clamped_to_visible_glyph_span` — drives `paint_run_background` with a visible span narrower than the run and asserts the painted background rect is clamped to that span (36px), not the full run width (120px). - `test_fully_truncated_run_paints_no_background` — a leading run consumes the paint bounds so a trailing backgrounded run is fully truncated; asserts no background rect is painted for it. `cargo nextest run -p warpui_core` (298 tests) passes, `cargo clippy -p warpui_core --all-targets` is clean, and `cargo fmt -p warpui_core -- --check` is clean. > Note on the ellipsis-truncation path: the platform test `FontDB` reports a zero advance for the `…` glyph, so `ellipsis_width` is always `0` in unit tests and the *end-to-end* ellipsis branch in `paint_internal` cannot be driven from a unit test. The clamping arithmetic that fixes the ellipsis case is therefore pinned directly via `test_run_background_clamped_to_visible_glyph_span`, and the visible-span guard via `test_fully_truncated_run_paints_no_background`. - [ ] I have manually tested my changes locally with `./script/run` ## Verification The underline fix was verified on a real running Warp build by rendering a detected link as inline code (gray code background) at multiple terminal widths and comparing against a plain-text-link control: - **Before:** the inline-code link's hover underline was partially or fully missing in a width-dependent way (the gray run background was painted over it). The plain-text control link always underlined fully. - **After:** the inline-code link shows a full-width underline at every tested width (820 / 1000 / 1180 / 1360 px), matching the plain-text control. Pixel inspection of the cropped link token confirmed a continuous underline row spanning the full token width at each width, where the before-fix crops showed gaps / missing underline pixels under the backgrounded glyphs. Before/after screenshots (full-width renders at the four widths plus the plain-text control, and the zoomed token crops) are posted in the Slack thread linked above; the "after" comparison image is also attached there (`fixed_after.png`). Image embedding note: these screenshots were captured in a sandboxed environment and are shared via the Slack thread rather than embedded inline in this PR body. The behavior is additionally locked in by the regression tests above. ## Agent Mode - [x] Warp Agent Mode - This PR was created via Warp's AI Agent Mode <!-- CHANGELOG-BUG-FIX: Fixed agent rich output where an inline-code link's hover underline could be partially or fully hidden by the link's background. --> <!-- factory-client: {"source":"factory-client","slack_channel":"C0BCE7AELJ2","slack_thread_ts":"1782330944.826139","slack_permalink":"https://warpdev.slack.com/archives/C0BCE7AELJ2/p1782330944826139?thread_ts=1782330944.826139&cid=C0BCE7AELJ2","oz_run_id":"019efb33-f4d8-7b78-aaf7-cfb2e224eec9","repo":"warpdotdev/warp"} --> _Conversation: https://staging.warp.dev/conversation/cd3fc717-924c-4770-ac4d-f91f7f2e5774_ _Run: https://oz.staging.warp.dev/runs/019efb3c-3a78-772e-9788-7e10e517a52c_ _This PR was generated with [Oz](https://warp.dev/oz)._ --------- Co-authored-by: Oz <oz-agent@warp.dev>
1 parent 67f0c83 commit ef0ac45

2 files changed

Lines changed: 329 additions & 30 deletions

File tree

crates/warpui_core/src/text_layout.rs

Lines changed: 97 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1189,13 +1189,24 @@ impl Line {
11891189
self.last_index() + 1
11901190
}
11911191

1192+
/// Paints a run's background fill and border box, clamped horizontally to the
1193+
/// span of glyphs that are actually drawn (`visible_left`..`visible_right`, in
1194+
/// scene/paint coordinates). This must run BEFORE the run's glyphs and underline
1195+
/// are drawn so that text decorations (notably the hyperlink underline, which is a
1196+
/// filled rect in the same layer as the background) render on top of the
1197+
/// background instead of being covered by it.
1198+
///
1199+
/// Callers must only invoke this for a run that has at least one visible glyph
1200+
/// (i.e. `visible_right > visible_left`); a fully truncated/clipped-away run paints
1201+
/// no background at all.
11921202
#[allow(clippy::too_many_arguments)]
1193-
fn paint_run_decorations(
1203+
fn paint_run_background(
11941204
&self,
1195-
glyph_color: ColorU,
11961205
run: &Run,
11971206
origin: Vector2F,
11981207
visible_bounds: RectF,
1208+
visible_left: f32,
1209+
visible_right: f32,
11991210
font_cache: &FontCache,
12001211
scene: &mut Scene,
12011212
baseline_position_fn: &ComputeBaselinePositionFn,
@@ -1224,27 +1235,26 @@ impl Line {
12241235
})
12251236
.unwrap_or(self.line_height_ratio);
12261237

1227-
// Compute the origin of where the first glyph was rendered. The position reported
1228-
// by the glyph is along it's baseline, so we need to offset it by the baseline
1229-
// offset to get back to the top of the glyph.
1230-
// We also need to shift the position horizontally to account for kerning when bordering
1231-
// is turned on.
1232-
let rect_origin = origin + first_glyph.position_along_baseline
1233-
- vec2f(
1234-
2. * block_padding,
1235-
(baseline_position_fn)(ComputeBaselinePositionArgs {
1236-
font_cache,
1237-
font_size: self.font_size,
1238-
line_height_ratio,
1239-
baseline_ratio: self.baseline_ratio,
1240-
ascent: self.ascent,
1241-
descent: self.descent,
1242-
}) + 2. * block_padding,
1243-
);
1238+
// The position reported by the glyph is along its baseline, so we offset
1239+
// it by the baseline offset to get back to the top of the glyph. The
1240+
// horizontal extent is taken from the actually-drawn glyph span
1241+
// (`visible_left`/`visible_right`) rather than the full run width, so a
1242+
// partially-truncated run does not paint a background past its visible
1243+
// glyphs (or behind an ellipsis). The extra `block_padding` shift
1244+
// accounts for kerning when bordering is turned on.
1245+
let rect_top = origin.y() + first_glyph.position_along_baseline.y()
1246+
- ((baseline_position_fn)(ComputeBaselinePositionArgs {
1247+
font_cache,
1248+
font_size: self.font_size,
1249+
line_height_ratio,
1250+
baseline_ratio: self.baseline_ratio,
1251+
ascent: self.ascent,
1252+
descent: self.descent,
1253+
}) + 2. * block_padding);
12441254
let text_rect = RectF::new(
1245-
rect_origin,
1255+
vec2f(visible_left - 2. * block_padding, rect_top),
12461256
vec2f(
1247-
run.width + 2. * block_padding,
1257+
(visible_right - visible_left) + 2. * block_padding,
12481258
font_cache.line_height(self.font_size, line_height_ratio)
12491259
+ 2. * block_padding,
12501260
),
@@ -1272,7 +1282,16 @@ impl Line {
12721282
}
12731283
}
12741284
}
1285+
}
12751286

1287+
fn paint_run_decorations(
1288+
&self,
1289+
glyph_color: ColorU,
1290+
run: &Run,
1291+
origin: Vector2F,
1292+
visible_bounds: RectF,
1293+
scene: &mut Scene,
1294+
) {
12761295
if let Some((error_underline_color, first_glyph)) =
12771296
run.styles.error_underline_color.zip(run.glyphs.first())
12781297
{
@@ -1491,6 +1510,62 @@ impl Line {
14911510
glyph_color = foreground_color;
14921511
}
14931512

1513+
// Paint the run's background/border BEFORE its glyphs and underline (the
1514+
// hyperlink underline is a filled rect in the same layer as the background,
1515+
// so painting the background afterward would cover and hide the underline on
1516+
// backgrounded runs such as an inline-code link). Only backgrounded/bordered
1517+
// runs need this, so the extra glyph walk to compute the visible span is
1518+
// skipped entirely for normal text runs (the common hot path).
1519+
if run.styles.border.is_some() || run.styles.background_color.is_some() {
1520+
// Determine the horizontal span of this run's glyphs that will actually
1521+
// be drawn, so the background can be clamped to it. This mirrors the
1522+
// truncation/stop conditions in the glyph-drawing loop below (the
1523+
// ellipsis cutoff and the `remaining_width <= 0` stop) without mutating
1524+
// `remaining_width` or drawing anything; keep the two in sync.
1525+
let mut visible_left = f32::INFINITY;
1526+
let mut visible_right = f32::NEG_INFINITY;
1527+
let mut sim_remaining_width = remaining_width;
1528+
let sim_glyph_iter = if is_start_clipping {
1529+
itertools::Either::Left(run.glyphs.iter().rev())
1530+
} else {
1531+
itertools::Either::Right(run.glyphs.iter())
1532+
};
1533+
for glyph in sim_glyph_iter {
1534+
if clip_style == ClipStyle::Ellipsis
1535+
&& ellipsis_width > 0.
1536+
&& sim_remaining_width < glyph.width
1537+
{
1538+
break;
1539+
}
1540+
if sim_remaining_width <= 0. {
1541+
break;
1542+
}
1543+
sim_remaining_width -= glyph.width;
1544+
let glyph_x = if is_start_clipping {
1545+
line_origin.x() + sim_remaining_width + start_ellipsis_offset
1546+
} else {
1547+
line_origin.x() + glyph.position_along_baseline.x()
1548+
};
1549+
visible_left = visible_left.min(glyph_x);
1550+
visible_right = visible_right.max(glyph_x + glyph.width);
1551+
}
1552+
1553+
// Skip the background entirely when no glyphs are visible so a fully
1554+
// truncated run paints no background.
1555+
if visible_right > visible_left {
1556+
self.paint_run_background(
1557+
run,
1558+
line_origin,
1559+
bounds,
1560+
visible_left,
1561+
visible_right,
1562+
font_cache,
1563+
scene,
1564+
baseline_position_fn,
1565+
);
1566+
}
1567+
}
1568+
14941569
let glyph_iter = if is_start_clipping {
14951570
itertools::Either::Left(run.glyphs.iter().rev())
14961571
} else {
@@ -1577,15 +1652,7 @@ impl Line {
15771652
}
15781653
}
15791654

1580-
self.paint_run_decorations(
1581-
glyph_color,
1582-
run,
1583-
line_origin,
1584-
bounds,
1585-
font_cache,
1586-
scene,
1587-
baseline_position_fn,
1588-
);
1655+
self.paint_run_decorations(glyph_color, run, line_origin, bounds, scene);
15891656

15901657
if should_stop_after_run {
15911658
break;

0 commit comments

Comments
 (0)