fix(render): keep hardcoded white text readable on a light background - #370
Conversation
A truecolor CLI hardcodes a dark-terminal palette. Claude Code emits a literal ESC[38;2;255;255;255m for its primary text, which is 1.07:1 - and so invisible - against a light theme's paper floor. A 24-bit color bypasses the theme and palette layers entirely, so the only place to rescue it is per cell at paint time. ColorUtils.legibleOnLightBackground adapts a failing glyph color in two steps: mirror its HSL lightness about 0.5, preserving hue and saturation, then clamp toward black for whatever the mirror could not fix. White becomes near-black so primary text reads as ink rather than a washed-out grey, #999999 stays a grey, and a brand orange stays orange. A fully saturated color already sits near lightness 0.5, so the mirror barely moves it; that is the case the clamp exists for. Scoped to light backgrounds, gated on the WCAG 0.1791 pivot rather than a flat 0.5 so a mid-grey floor still counts. Every dark theme therefore renders byte-identically, and a cell carrying its own dark background - Claude Code's ESC[48;2;0;0;0m blocks - keeps its white text. Measured against the background pass 1 actually painted, so an INVERSE cell is corrected against the right floor too. Applied after DIM on purpose: plain white mirrors all the way to #000000 while dim white only reaches the floor at ~#727272, so ESC[2m still reads as secondary without applyDimColor needing to change. The guard runs per cell per frame and a contrast check costs six pow() calls, so results are memoised behind a single-entry memo and an LRU, both keyed by the active floor. The luminance work sits behind the cache rather than in front of it: on a dark theme every cell bails out, and caching the identity result makes that a field read. Exposed as lightBackgroundMinContrast (default 4.5, 1.0 = off). Verified with 13 unit tests over the color math and 3 that render through renderTerminal and read the bitmap back - reverting the guard fails 7 of the former and 1 of the latter. Full compose-ui suite: 1082 tests green.
|
Review (part 1 of 2). Strong PR. The problem statement is the best part: a captured SGR dump, a real repro, and the observation that a 24-bit color bypasses every layer that could remap it. Measuring against the background Pass 1 actually painted ( I could not run 1. Selection and search backgrounds are painted before the text pass, and the guard does not see them
On a light theme with a dark selection color:
Selecting was the one workaround that made this text readable, and the guard removes it. Solarized Light, the only light builtin, has
2. The guard rewrites Solarized Lights own ANSI palette - an unstated behaviour change The guard runs on every foreground, including indexed colors the theme layer does control. On Solarized Light (
Those are deliberate: Solarizeds light variant maps ANSI black/brightBlack to base2/base3 so programs setting a black background get the light tone. After this PR, Either gate on |
|
Review (part 2 of 2). 3. Hot path: a global monitor and an allocation per cell on every memo miss On a miss, The stated reason for putting the luminance check behind the cache does not quite hold:
The bail-out does not need a contrast ratio, only 4. if (cachedMinRatio != minRatio) {
guardCache.clear()
cachedMinRatio = minRatio
}Two reachable ways to thrash this:
Folding 5. The guard runs for cells that paint nothing It is computed at 6. The unreachable-floor case is silent The slider goes to 7:1, but for a background just above the pivot (L about 0.18) even pure black is only about 4.6:1, so 7. The de-duplication the PR describes is not finished
8. Minor
Summary: 1 is the one I would want resolved before merge - it is the same "measure what was actually painted" principle the PR gets right in Pass 1 and misses in Pass 1.5, and it regresses a case that used to work. 2 is a scope/documentation decision that would also make 3 mostly moot. 3 and 4 are worth fixing while the code is fresh; the rest is polish. |
The web viewer had the same bug the renderer just fixed: on a light theme, a CLI that hardcodes truecolor white paints at 1.07:1 and vanishes. The renderer's guard cannot reach it. Two independent reasons, both checked rather than assumed: - The viewer renders through xterm.js, whose glyph pipeline has no per-cell foreground hook. registerDecoration is per-marker and registerCharacterJoiner carries no color, so there is nowhere to run mirror-and-clamp client side. - Correcting server side would only fix snapshots. TerminalSnapshotEncoder handles PaneSnapshot and screen repaints, but live text is the raw pty string forwarded verbatim by MirrorShare's output tap, so the truecolor a CLI emits never passes through anything host-side that could rewrite it. Rewriting the live stream would mean re-parsing SGR in the mirror path and fighting the graphics filter. What xterm.js does have is its own minimumContrastRatio, which takes a floor and corrects per cell. So the host sends the floor instead of colors: ServerMessage.Theme gains minimumContrastRatio, set from the user's lightBackgroundMinContrast when the terminal floor is light and 1.0 (off) otherwise, via the new ColorUtils.lightBackgroundGuardRatio so the light-vs-dark pivot is not restated. Both theme producers send it - MirrorShare for GUI tabs and DaemonShareServer for headless sessions. The correction style still differs from the in-app renderer: xterm.js clamps to the floor rather than mirroring lightness, so white lands on a strong grey in the viewer where the app paints it as ink. Both are readable, and this keeps one well-maintained implementation instead of a second copy of the algorithm that could drift. The floor is validated in viewer-logic.js rather than trusted: it crosses the wire, and xterm.js throws on some out-of-range option values, so a stale or hostile host must not be able to take the viewer down. Clamped to [1, 21] at one decimal, matching xterm.js's own normalisation, with anything unparseable meaning off. The option is assigned unconditionally so switching back to a dark theme disarms the guard instead of latching the previous floor. Additive on the wire: the field is defaulted and ShareProtocol decodes with ignoreUnknownKeys, so an older host that never sends it leaves the guard off rather than failing to decode a theme frame. Verified by two new scenarios in the fake-browser harness, which run the shipped viewer.js under Node and assert the option that actually reaches the terminal - including a pane created after the theme frame. Removing the one-line wiring fails them. Plus 12 wire-validation cases under Node, a protocol round-trip with a legacy-decode case, and a test pinning that the viewer's floor and the renderer's own gate agree for every builtin theme. Full suite: 1084 green.
Follow-up commit: the share viewer
The renderer's guard could not be reused there, for two reasons I checked rather than assumed:
What xterm.js does have is its own Known difference, called out deliberatelyxterm.js clamps to the floor rather than mirroring lightness, so white lands on a strong RobustnessThe floor is validated in Additive on the wire: the field is defaulted and Verification
|
Review: light-background legibility guardStrong PR. The diagnosis holds up (truecolor bypasses the palette layer, so per-cell paint time really is the only hook), mirror-then-clamp is well motivated, the guard is measured against the background pass 1 actually painted rather than the theme, and the pixel-level tests through the real I could not run 1. Duplicated dangling comment in
|
The synthesized light terminal theme hardcoded white = #D1D5DA, which is 1.27:1 against the Blueprint Light floor: ESC[37m painted nothing readable. It was also inconsistent with brightWhite two lines below, which already resolved to the host foreground, so ANSI 7 and 15 disagreed about which direction "white" meant. ANSI 7/15 invert on a light floor, the way Solarized Light maps them to its two darkest inks rather than to greys. white now comes from the host's own TextSecondary token, so it stays correct for Daylight as well as Blueprint Light instead of pinning another literal. The new test sweeps all 16 ANSI slots against the floor rather than pinning this one value, so the next hardcoded light grey cannot slip back in. Held to 3:1 rather than the 4.5:1 text floor because red/green/yellow are brand-carrying status colors taken from the host tokens. BossTerm's renderer also rescues this class of color at paint time now (kshivang/BossTerm#370), but that only covers the painted glyph: this palette is what gets handed to the share-viewer browser mirror, so it has to be right at the source too.
The bug
With a light terminal theme active, text from CLIs like Claude Code renders invisible.
Not a theme bug. Captured from a live Claude Code session under a pty:
Claude Code emits truecolor, not palette indices. Pure white on a light floor
(
#F5F7FB) is 1.07:1 contrast. And because a 24-bit color goes straight throughColorUtils.convertTerminalColor, no theme or palette remap can reach it - the onlyplace to rescue it is per cell at paint time.
claude -pemits no color at all, so print mode proves nothing here. Repro:The fix
ColorUtils.legibleOnLightBackground(fg, bg, minRatio)- the first public contrast util inthe repo; the math was otherwise duplicated privately in
UiTheme.Companionand two testfiles. Called from the two text-painting sites in
TerminalCanvasRenderer: Pass 2renderTextandrenderZWJSequence.A failing color is corrected in two steps:
#F5F7FB#FFFFFF#000000#999999#666666#D77757#FFC107Both steps are needed: clamping alone lands white on a mid-grey (~
#727272) and collapsesthe CLI's own hierarchy, while a fully saturated color already sits near lightness 0.5, so
the mirror barely moves
#FFC107.New setting
lightBackgroundMinContrast(default 4.5,1.0= off) with a slider in Visualsettings. Purely additive: both loaders use
ignoreUnknownKeys, andloadFromFile()already re-writes when the round-trip differs, so existing
settings.jsonself-migrates.Decisions worth reviewing
Each of these had a plausible-but-wrong alternative:
if (isInverse) baseFg else baseBg),never the theme default. That is what leaves Claude Code's own
ESC[48;2;0;0;0mblocksalone and still fixes white-on-white INVERSE cells.
baseFg/baseBgsites inthe renderer paint backgrounds and are deliberately untouched.
0.1791pivot rather than a flat0.5so a mid-grey floor still counts. Every dark theme therefore renders byte-identically - a
symmetric guard would have brightened Claude Code's
#505050separators on the defaultdark theme.
#000000while dim white only reaches the floor at ~#727272, soESC[2mstill reads assecondary and
applyDimColorneeds no change.styleMatchesalready includesbatchFgColor == fgColorandthe guard runs before that comparison, so runs fragment only where colors genuinely differ.
Performance
The guard runs per cell per frame and a contrast check is six
powcalls. Results arememoised behind a single-entry memo (runs of identical style hit it) and an LRU, both keyed
by the active floor. The luminance work sits behind the cache rather than in front: on a
dark theme every cell bails out, so caching the identity result makes that path a field read.
Verification
util/ColorUtilsContrastTest).renderTerminalinto anImageBitmapand read thepixels back (
rendering/LightBackgroundLegibilityTest). AGENTS.md forbids launching theapp or screenshotting, and
CanvasDrawScope+toPixelMapis the in-repo way to get realpainted pixels - same approach as
rendering/CursorOverlayTest.are shown to catch the bug rather than assumed to.
Notes
Color.hsl()does not exist in Compose Multiplatform desktopui-graphics(Android-only),hence the hand-rolled HSL pair.
luminance()does exist and is whatUiThemealready uses.share/TerminalSnapshotEncoder.ktre-encodes cell stylesto SGR for the browser mirror and has the same truecolor-white problem on a light theme.
flushBatch()'s two unreachable fallbacks disagree -:824usesdefaultForegroundColor,:846usesColor.White. Harmless today; the white one wouldpaint white if
fgColorever became nullable.terminal-tab, whose synthesized light palettehardcoded
white = #D1D5DA(1.27:1 on paper). This guard rescues the painted glyph, butthe palette is what goes to the share-viewer mirror, so it needs fixing at the source too.