Conversation
Three rendering fixes carried on top of the Ghostty 1.3 upgrade, plus a regression test: - Scroll boundary: floor viewportY once and use that single integer for both the scrollback/screen row comparison and the offset/screen-row math (render loop and hyperlink-hover scan). During smooth scroll viewportY is fractional; comparing rows against the raw value while indexing with the floored value read one row past the scrollback (returning null, leaving stale pixels) and dropped the top screen row, duplicating a line near the top of the viewport. Covered by renderer-viewport-boundary.test.ts. - Block elements: snap every partial-block edge to the device-pixel grid and add the quadrant blocks (U+2596-U+259F) as device-snapped rectangles. Quadrants previously fell through to font-glyph rendering, which doesn't fill the cell and left gaps in block/quadrant art (e.g. the Claude Code mascot), especially at line-height > 1. - Selection color: selected text keeps its original foreground instead of being forced to theme.selectionForeground, so editor theme colors survive selection (helper customization). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Reserve a SCROLLBAR_GUTTER (15px) on the right of the canvas so the scrollback scrollbar draws beside the text instead of painting over the last columns. Text still lays out across cols*metrics.width; the canvas is widened by the gutter and the needsResize check is updated to match. Route the scrollbar thumb and track colors through the theme (scrollbarThumb / scrollbarTrack), applying fade and idle dim via globalAlpha. The track defaults to empty so the gutter shows the terminal background, like VS Code's scrollbars, and is only drawn when a theme provides a color. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…roll blit Streaming output was spending almost all of its time re-reading the same cells out of WASM. Measured in Chromium against a real terminal: grid before after speedup 80x24 15.6 ms 1.96 ms 8.0x 120x40 56.3 ms 4.70 ms 12.0x 200x50 129.6 ms 9.37 ms 13.8x Three changes, in order of how much they matter. 1. Fetch the viewport once per frame. getLine() walks every cell of every row to return one row, so calling it per rendered row cost O(rows^2 * cols) WASM crossings a frame. Profiling a 120x40 grid: 40 getLine calls came to 58.7 ms, one bulk read came to 1.5 ms. That accounts for most of the speedup above, and for why the old path degraded quadratically as the terminal grew. Adds GhosttyTerminal.getViewportLines(), an optional member of IRenderable, and routes the renderer's row access through a lazy per-frame cache. Rows served from scrollback already used a direct per-row grid walk and are unchanged. 2. Glyph atlas (lib/glyph-atlas.ts). Rasterises each distinct glyph once and composites it with drawImage, keyed by text, colour and the bold/italic flags. Slots are sized from the glyph's ink box rather than the cell box so italics, combining marks and full-height powerline glyphs are not clipped. Shelf packed, repacks when full, and disables itself after four repacks so a working set that never fits falls back to fillText instead of thrashing. The lookup runs per painted cell, so it uses two-level buckets with the outer one memoised across a run. A single composite string key allocated thousands of strings a frame and made the atlas a net loss. 3. Scroll blit. A scroll relocates every row, which is why the buffer reports FULL dirty and the old path repainted the whole viewport. The pixels are still correct though, just in the wrong place, so move them with one copy and repaint only what the copy could not fill: 40 frames of scrolling went from 800 row paints to 80. The shift is derived, since nothing reports it. Row y always shows absolute line scrollbackLength - viewportY + y, so holding a line fixed across two frames gives: shift = (viewportY - lastViewportY) - (scrollbackLength - lastLength) Scrolling into history and streaming at the bottom both fall out of that. The result is treated as a hypothesis: each retained row is checked against two independent 32-bit hashes of what is actually on the canvas before its pixels are reused, so alternate-screen redraws and scrolling regions, which move content without growing scrollback, are caught and repainted. More than half the viewport failing verification falls back to a full repaint. An opaque background moves pixels in a single self-copy. Translucent themes stage through a scratch bitmap, because there the moved band has to replace the destination and clearing first would destroy the overlapping source. Verified by tests/e2e/10-renderer-optimisations.spec.ts, which diffs bitmaps rendered with each optimisation on and off. The blit is pixel exact. The atlas differs by at most 2/255 on 5.4% of pixels, spread evenly across rows, which is antialiasing rounding from rasterising at device scale in an unscaled context rather than CSS scale in a scaled one. Both paths sit behind the glyphAtlas and scrollBlit renderer options, defaulting on, so either can be switched off to isolate an artefact. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Follow-up to 96698f9, from grepping the remaining getLine() callers. Both of these walked the whole viewport to read a single row. renderCursor fetched the cell under a block cursor itself, which cost a full viewport walk on every frame that drew a cursor. It runs inside render(), which already has the frame's rows cached, so the row is passed in now. Viewport walks per frame drop from 2 to 1 and a 120x40 streaming frame goes from 4.70 ms to 3.18 ms, or 17.7x against the original renderer. The OSC8 link provider's backward scan called getLine() once per column while looking for the start of a link, on a row that does not change inside the loop. Scanning a long link was quadratic as a result. Hoisted out, which is how the forward scan a few lines below was already written. One more caller is left: SelectionManager.getSelection() fetches per row inside its loop over the selected range, so copying a selection costs one walk per selected screen row. It is a one-shot user action rather than a per-frame cost, and fixing it means threading a bulk read through the scrollback and screen split, so it is left for its own change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Completes the plumbing for the two renderer optimisations added in 96698f9, which until now could only be reached by constructing a CanvasRenderer directly. Embedders using Terminal got the defaults with no way to turn either one off. Adds both to ITerminalOptions, defaults them to true in TerminalCore, and forwards them when Terminal builds its renderer. handleOptionChange also handles them at runtime via the renderer's setGlyphAtlas/setScrollBlit, forcing one full repaint so the frame that follows starts from a canvas the blit can reason about. The setters landed in 72d3ac4 by accident, as they were written in a concurrent session while that commit was being staged. Committing the rest here rather than rewriting history, so the two halves are at least adjacent. Verified in Chromium: defaults reach the renderer, values passed at construction are respected, and toggling either at runtime leaves the canvas differing by at most 1/255 per channel, which is the antialiasing rounding between atlas and fillText rasterisation and nothing more. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
getLine() on the WASM terminal walks and copies every cell of every row to return one row. Three paths on the hover route called it per mouse move, so moving the pointer over the terminal cost several full viewport walks a frame. link-detector: getLinkAt() opened by fetching the row purely to read line.length for a bounds check and to null-check a cell whose value was then never used. A row is exactly cols wide and buffer.length is scrollback plus rows, so both checks are arithmetic. Measured in a real panel at 1.79ms -> 0.03ms per call, which was the entire cost of hover link detection. terminal: hover hit-testing fetched a row per mouse move to read one cell's hyperlink id. Now served from a bulk fetch cached per rendered frame, keyed on a render counter so a row can never be served from a frame that has already been replaced. buffer: buffer.active.getLine() is what the link providers call, and they scan several rows per lookup. Screen rows now come from one bulk fetch cached for the current synchronous burst and dropped on the next microtask checkpoint, so it cannot outlive the task that built it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three separate problems, all in how a selection reaches the screen. SelectionManager.requestRender() was an empty function. Its comment said the render loop would pick the change up at 60fps, but there is no such loop: Terminal schedules a frame only when something asks for one. While dragging over an idle terminal nothing did, except the 530ms cursor blink, so the highlight tracked the pointer at roughly two frames a second. Measured in a real panel: 612 mouse moves produced 17 renders in 8 seconds. It now goes through the wake callback the renderer already owned for the blink, exposed as renderer.requestRender(). The renderer then repainted every selected row on every frame regardless of what changed, which made SelectionManager's dirty-row tracking dead weight — whatever it reported, the full range was re-added anyway. It now diffs the selection against the previous frame's and repaints the rows that joined or left plus the endpoint rows whose column extent moved. SelectionManager marks the same delta instead of the whole range on each mouse move. Finally, the scroll blit was disabled outright whenever anything was selected, so every frame of a drag fell through to repainting the entire viewport. The exclusion was sound as written: the blit moves pixels with the highlight already painted into them, and the row hash covered only cell content, so a row whose highlight changed would hash identical and be retained stale. The hash now includes the row's selected column span, so a changed highlight fails verification like any other change. Hover underlines and kitty placements stay excluded, since they paint outside the hashed cell data. A zero shift is also admitted while scrolled, where the fallback repaints everything; at the bottom it still takes the cheaper dirty-flag path. Measured in a real panel across these: drag went from 2fps to 51fps, and rows painted per render from 51.6 of 52 down to 6.3. The new e2e case guards the part that removed a safety check: it drags a selection while the viewport scrolls and requires the blitted output to be pixel-identical to a full repaint. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…med at targetViewportY is set only by smoothScrollTo, but four other paths moved viewportY behind the animation's back. The next animation frame then measured its distance from a target that no longer meant anything and pulled the viewport back toward it, which read as the view jumping backwards a line or two while scrolling through arriving output. scrollLines, scrollToTop, scrollToBottom and scrollToLine are explicit jumps that supersede whatever the animation was heading for, so they now cancel it and adopt the new position as the target. The preserveScrollOnWrite branch is the one that caused the visible jitter. An animation aims at a line of content, not at a number, so when the scrollback grows underneath it the target has to move by the same amount. Otherwise every batch of output left the target one batch stale, and the effect repeated for as long as output kept arriving. Measured in the replay harness over ~2400 frames: 21-42 backwards jumps of up to 1.9 lines before, zero after, with the canvas fingerprint clean while parked in scrollback. Confirmed by hand in a real panel against 500 numbered lines. Both tests were checked against a reverted fix first — they fail without it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
readGridLine() allocated seven WASM scratch buffers, fetched the entire 256-colour palette, resolved a grid ref, then freed all seven — per row. A frame showing scrollback needs a contiguous run of rows, so that setup was paid around twenty-five times a frame. Hoisted into acquire/releaseGridScratch around a shared per-row body, with getScrollbackLines(start, count) reusing one scratch record across the run and re-aiming a single point struct rather than allocating one per row. The renderer fetches its scrollback rows in one lazy call, keeping the individual path as a fallback. Same shape as getViewportLines() for the screen. Worth less than expected, and the reason is the useful part. Measured A/B in the replay harness (&nobulkscrollback=1 strips the bulk path): bulk off p50 render 2.70ms 24.9 calls/frame 1.505 ms/frame fetching bulk on p50 render 2.30ms 0.62 calls/frame 1.260 ms/frame fetching About 15% off render p50, not the ~0.9ms/frame I predicted. Per row the bulk read is 0.05ms against 0.060ms individually, so the setup was a smaller share than assumed: what dominates is the per-cell WASM crossings inside the loop — three ghostty_cell_get calls, a grid_ref_style, and two colour resolutions per cell. Going below this needs a packed bulk cell export across the WASM boundary, which is a Zig-side change, not a JS refactor. The equivalence tests are the real guard on the refactor: the bulk result must match the one-at-a-time result cell for cell, across varied palette, truecolour and attribute styling, including runs that straddle the end of the scrollback. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The cell loops built a fresh DataView or Uint8Array for almost every field they read — six typed-array allocations per cell, so roughly twenty thousand objects per viewport read, every frame. They were rebuilt that eagerly for a real reason: growing the WASM memory detaches existing views. Comparing buffer identity catches exactly that case and costs a pointer compare instead. Views are now cached on the terminal and hoisted once per row in the scrollback walk and once per call in getViewport. Where a windowed Uint8Array was used to read style bytes, indices became absolute into the whole-heap view. Also routes the three per-cell field reads through ghostty_cell_get_multi, which upstream already exported and this library never called. In isolation that was worth about 8% and sat inside run-to-run noise; it is kept because crossings are a larger share of what remains. Measured in the replay harness, wheel mode, three runs per configuration: baseline p50 2.60ms p95 2.90 scrollback 2.23ms screen 1.98ms cell_get_multi only p50 2.40ms p95 2.70 scrollback 1.94ms screen 1.98ms + hoisted views p50 0.90ms p95 1.50 scrollback 0.50ms screen 0.76ms 2.9x on median render, 4.5x on a scrollback row read. Worth recording why this was nearly missed. The cost was assumed to be WASM crossings, and the plan was to add a packed bulk-row export on the Zig side to collapse them. Batching the crossings first, as a cheap test of that premise, produced almost nothing — which is what pointed at the allocations instead. The Zig work would have optimised the wrong thing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three callers want the screen on every frame of a drag: the renderer painting
it, the hover hit-test reading one cell's hyperlink id, and the buffer API
behind link detection. Each had its own cache, so each did its own bulk fetch —
about 2.3 viewport walks a frame, and during a drag that was most of the work
on the main thread.
They can share one. The screen only changes when something writes to the
terminal or it resizes, so a generation counter bumped in exactly those two
places is enough to know when a cached fetch is stale.
Measured in the replay harness, drag mode, three runs per configuration:
before getViewportLines 1.313 ms/frame 254 ms of instrumented work per
second of wall clock
after getViewportLines 0.615 ms/frame 175 ms/sec
31% less main-thread time during a drag. Render p50 only moves 1.50 -> 1.40ms
because two of the three callers run during mousemove handling rather than
inside render(); the saving is real but mostly lands outside the paint.
The cache hands the same cell objects to all three callers where they used to
get private copies, so this is only safe because none of them mutate a cell —
checked, and they only read. A caller that retained a row across a write would
now see it change underneath rather than go stale, which is worth knowing
before adding a fourth consumer.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A URL longer than the terminal is wide soft-wraps across rows, and the provider scanned one row at a time. At 80 columns a 130-character URL produced: row 0 -> http://…/synthetic.cast&repro=drag& (truncated at exactly 80) row 1 -> no link at all The first row was the worse half. It looked clickable, and because the cut lands mid-query-string the truncated address often still resolves, so it opened something plausible instead of failing. The continuation row started partway through the query string, matched nothing, and was simply dead. isWrapped marks a row as a continuation of the one above it, so the provider now walks back while it is set to find the logical start, walks forward while the next row has it, joins the rows, and maps match offsets back to per-row coordinates. ILink.range already carried separate start and end points, and isPositionInLink already handled multi-row ranges, so hit testing needed no change. Links are cached by their own range, so scanning the same logical line from either row yields the same key rather than a duplicate. Hard-wrapped text is deliberately unaffected. Lines broken with a real newline, such as a host indenting continuation lines, are separate logical lines and the indent is not part of any URL. Only soft wraps are followed. Two tests, both checked against a reverted fix: a wrapped URL must come back whole from either row, and two short URLs on adjacent hard-wrapped lines must not be joined. Reported downstream: a wrapped harness URL was clickable on its first line only, and led somewhere subtly wrong. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Hover was only ever computed in processMouseMove. Scrolling with the mouse held still left the link underline drawn wherever it had been, over whatever content scrolled into that spot, and it stayed there until the pointer moved. Output arriving at the bottom does the same thing without changing viewportY at all. The terminal now remembers where the pointer is and which view the hover was resolved against, keyed on floored viewportY plus the content generation, and re-resolves from the render loop when either has moved. Gated on something actually being hovered. With no link under the cursor there is no stale highlight to correct, so this stays off the hot path and does not turn every scrolled frame into hover work. Two tests. One checks that a scroll and new output each trigger exactly one re-resolve, that an unchanged view triggers none, and that driving the render loop does it too. That last assertion exists because the others still passed when the call site in renderTick was deleted and only the method kept; with it, that edit fails on Expected 3, Received 2. The second test checks that nothing happens when no link is hovered. Reported downstream alongside the wrapped-URL fix: hovering a link, holding still, and scrolling left the underline behind. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…oves Two defects, both visible as a wrapped link leaving one underlined row behind after a scroll. The highlight is stored in viewport coordinates worked out from viewportY and the scrollback length, but it was only recomputed when the hovered link itself changed. Scrolling leaves the same link under the pointer, so the update was skipped and the underline stayed pinned to a screen row while the text moved out from under it. It is now recomputed on every resolve, and setHoveredLinkRange compares by value so the extra calls do not each request a repaint. Streaming output hid this: writeInternal invalidates the link cache, so a new link object came back, the identity check passed, and the range was refreshed by accident. Only a pure scroll showed the bug, which matches how it was reported. Underneath that, buffer.ts reported isWrapped as false for every scrollback row, with a TODO saying the WASM API was missing. It is not missing: a grid ref resolves to a row handle and the row carries wrap_continuation, which is what isScrollbackRowWrapped now reads. Without it a soft-wrapped URL stopped being one link the moment it scrolled off the live screen, since the providers use that flag to find where a logical line starts. Probing a wrapped link in scrollback showed a range of 1..2 with this fix and 1..1 without it. The test scrolls rather than writing, and was checked against each fix reverted on its own: without the range recompute the highlight fails to move, and without the wrap flag the two-row link never forms. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…arry along The blitted branch of needsRender consulted blitExposedRows, cursorRows and the kitty damage set, but not hyperlinkRows. hyperlinkRows holds the rows a hover just left as well as the ones it moved onto, and the row hash covers cell content with no knowledge of whether a link underline is painted over it. That combination has a specific bad frame. Hovering a link disables the blit via hasOverlays, so the frame where the hover clears is the first frame the blit runs on. On exactly that frame the rows still carrying an underline were retained rather than repainted, and from then on the blit moved those pixels along with the text. The underline ended up sitting a row below the link and followed it down the screen on every later scroll, which is how it was reported. The two earlier fixes in this area were both real but neither could reach this: the highlight state was correct and the hover did clear, the stale part was pixels already on the canvas. The e2e case renders the transition directly, hovered underline then cleared hover on a scrolling frame, and diffs against the same sequence with the blit off. Without the fix that is 2166 differing pixels at max delta 196, the blue of the underline; with it, zero. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Opening a link switches to the browser, and that does not fire mouseleave. The hover stayed set with the pointer no longer over anything, and refreshHoverForMovedContent would go on re-resolving it from a position that had stopped meaning anything. Losing the window is the pointer leaving, as far as hover is concerned, so a window blur now runs the same teardown as mouseleave and the listener is removed alongside the others on dispose. This was only ever visible because the underline pixels were also being retained across the blit, which be245dd fixed. With the repaint correct the stale state stopped showing, but it was still there, so this closes it properly rather than leaving it to be uncovered by some later change. The test hovers a wrapped link, dispatches a window blur, and checks the link, the range and the stored pointer are all gone, then scrolls and renders once more to confirm it does not come back on its own. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
OSC8 links were underlined by comparing cell.hyperlink_id against the hovered id. Both read paths set that field to 1 for any hyperlinked cell rather than to a per-link identity, so hovering one link matched every hyperlinked cell in the viewport and lit all of them. Two URLs on screen meant hovering either one underlined both. The field is deliberately a flag rather than an identity, and the comment where it is populated says so: link detection identifies actual links by URI and position range. That range is already reported as hoveredLinkRange and already drawn, for OSC8 and plain text alike, so the id-based pass was both redundant and the only thing getting it wrong. Removed. The test renders two identical link rows and captures hovering each in turn. If both are underlined either way the two frames are byte-identical, which is exactly what the bug produced: diff 0 without the fix, 1596 with it. Reported downstream: two URLs in adjacent output, hovering one underlined both. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Six bugs in link handling, all found while using ghostty-web as the terminal in
a VS Code extension. They were reported one at a time by someone actually
clicking things, and each fix uncovered the next, so they are best read in
order.
A URL wider than the terminal was never detected whole
UrlRegexProviderscanned one row at a time. A URL that soft-wraps thereforelooked like two unrelated strings: the first row matched the regex and became a
link truncated at exactly the terminal width, and the continuation row started
partway through a query string and matched nothing.
The truncated half is the dangerous one. It looks clickable, and because the cut
usually lands mid-query-string the shortened address often still resolves, so it
opens something plausible rather than failing.
isWrappedmarks a row as a continuation of the one above, so the provider nowwalks back while it is set to find the logical start, joins the rows, and maps
match offsets back to per-row coordinates.
ILink.rangealready carriedseparate start and end points and
isPositionInLinkalready handled multi-rowranges, so hit testing needed no change.
Hard-wrapped text is deliberately untouched. Lines broken with a real newline,
such as a host indenting continuation lines, are separate logical lines and the
indent is not part of any URL. There is a test asserting two short URLs on
adjacent lines stay separate, so this does not later get "improved" into joining
everything.
Wrapped links stopped being links once they scrolled into history
buffer.tsreturnedisWrapped = falsefor every scrollback row, with a TODOsaying the WASM API was missing. It is not missing: a grid ref resolves to a row
handle, and the row carries
wrap_continuation. Without it the fix above workedon the live screen and silently stopped the moment the link scrolled off it.
Probing the same wrapped link in scrollback gives a range of rows 1 to 2 with
this, and 1 to 1 without.
The underline did not follow the link
Hover was only ever computed in
processMouseMove. Scrolling with the mouseheld still left the underline where it was, over whatever had scrolled into that
spot, until the pointer moved. Output arriving at the bottom does the same
without
viewportYchanging at all.The terminal now remembers the pointer position and the view the hover was
resolved against, keyed on floored
viewportYplus a content generation, andre-resolves from the render loop when either moves. It is gated on something
actually being hovered, so with no link under the cursor it costs nothing.
Separately, the highlight is stored in viewport coordinates derived from
viewportYand the scrollback length, but was only recomputed when the hoveredlink itself changed. Scrolling leaves the same link under the pointer, so the
update was skipped. It is now recomputed on every resolve, and
setHoveredLinkRangecompares by value so the extra calls do not each request arepaint.
Streaming output hid that second one:
writeInternalinvalidates the linkcache, so new output produced a new link object, the identity check passed, and
the range got refreshed by accident. Only a pure scroll shows it.
The blit carried a stale underline around the screen
The reported symptom was an underline landing one row below the link and then
following it down the screen on every later scroll.
Hovering a link disables the blit through
hasOverlays, so the frame where thehover clears is the first frame the blit runs on. The blitted branch of
needsRenderconsultedblitExposedRows,cursorRowsand the kitty damage setbut not
hyperlinkRows, which is exactly the set holding the rows a hover justleft. Those rows were retained instead of repainted, and from then on the blit
moved the underlined pixels along with the text.
The two earlier fixes could not reach this. The state was correct and the hover
did clear. What was left was pixels already on the canvas.
Losing the window left the hover set
Opening a link switches to the browser, and that does not fire
mouseleave, sothe hover stayed set with the pointer no longer over anything. A window blur now
runs the same teardown, and the listener is removed with the others on dispose.
This one was only visible because of the stale pixels above. Once the repaint
was correct the symptom disappeared, but the state was still wrong, so it is
closed properly rather than left for a later change to uncover.
Hovering one link underlined every link on screen
OSC8 links had a separate underline pass comparing
cell.hyperlink_idagainstthe hovered id. Both read paths set that field to 1 for any hyperlinked cell
rather than to a per-link identity, so hovering one link matched every
hyperlinked cell in the viewport.
The flag is meant to be a flag, and the comment where it is populated says as
much: real links are identified by URI and position range. That range is already
computed and already drawn for OSC8 and plain text alike, so the id-based pass
was redundant and the only thing getting it wrong. Removed.
Tests
Every test here was checked against its own fix reverted, because several
earlier attempts passed for the wrong reason. Two were vacuous: one rendered
nothing at all and reported zero differing pixels, another exercised the write
path where cache invalidation masks the bug.
The pixel-level cases are worth describing, since the assertions are not
obvious. For the stale underline, the transition frame is rendered directly and
diffed against the same sequence with the blit off: 2166 differing pixels at max
delta 196, the blue of the underline, without the fix, and zero with it. For the
last bug, two identical link rows are rendered while hovering each in turn and
the frames must differ. If both links underline either way the captures are
byte-identical, which is precisely what the bug produced: diff 0 without, 1596
with.
448 unit tests, 85 e2e,
tscclean.Stacked on #30.