ADR 0039: Fix mouse click/hover drift on styled lines¶
Status¶
Accepted
Context¶
User-reported bug: clicking (or hovering) further right on a line increasingly missed the character actually under the pointer, on lines with mixed syntax-highlight styling (e.g. bold keywords next to plain identifiers). This is the same class of bug ADR 0013 already fixed for the caret — assuming a fixed per-character pixel width instead of measuring what was actually rendered — just never fixed on the mouse-hit-testing side.
EditorViewport::offsetForPoint() (used by both mousePressEvent and
the no-button branch of mouseMoveEvent, i.e. click, drag-select, and
hover) computed the target column as localX / m_charWidth, a single
project-wide average character width. Meanwhile drawLine and
xForColumn — used for actually painting text and positioning the
caret — measure each syntax-highlight run's real advance with
QFontMetrics::horizontalAdvance on that run's actual text and font
(fontForCapture/metricsForCapture), because bold/italic variants of
even a "monospace" font don't reliably share the exact same advance as
the regular weight. Any line with more than one highlight capture
accumulates a small error per run boundary; far enough right on a long
styled line, that error becomes visible as exactly what was reported —
the pointer needing to move noticeably further than a character-width
to advance the cursor by one character, or landing on the wrong one
entirely.
Decision¶
Added EditorViewport::columnForX(lineStart, lineEnd, localX), the
direct inverse of xForColumn: it walks the same per-run segmentation
xForColumn/drawLine use, measuring each run's actual rendered
width, and finds which column's glyph localX falls nearest to —
rounding to the nearest codepoint boundary rather than flooring
(same reasoning as ADR 0028's click-precision fix), and stepping by
codepoint (using the existing isUtf8ContinuationByte helper) rather
than by raw byte, so a multi-byte UTF-8 character is never measured as
a partial, invalid byte sequence.
offsetForPoint() now calls columnForX() instead of dividing by
m_charWidth. This also removes code, not just adds it: the
separate post-hoc "snap forward past any continuation byte" loop
offsetForPoint() used to need is now redundant — columnForX()
already only ever returns a codepoint boundary.
m_charWidth itself is unchanged and still used elsewhere (horizontal
scroll margin, the line-wrap continuation hint) — those are approximate
by nature already, unlike hit-testing where the pointer needs to track
a specific glyph.
Verified live, not just built¶
Launched the real GUI against a .c file with a long line mixing bold
keyword highlights and plain identifiers. Placed the caret at a known
column via keyboard navigation (End, then Left N times),
screenshotted its exact pixel position, then clicked at that exact
pixel and confirmed the reported column matched — both near column 80
on a ~100-column styled line and again near the line's end. Before
this fix, offsetForPoint's fixed-pitch math and xForColumn's
per-run measurement could only coincidentally agree; after, a mouse
click and the caret's own rendered position resolve to the same
character by construction, however far right on the line.
Consequences¶
Click, drag-select, and hover (LSP hover popup, completion popup
positioning) all route through offsetForPoint, so all three are
fixed together, not just the plain click case reported. No new O(line
length) cost class was introduced beyond what already existed:
xForColumn was already a linear scan over the line's runs for the
caret's own position, and columnForX is the same shape.