fix: xterm-256 cube uses 0xd7 not 0xdf for level-4 color (palette typo) - #30
Merged
loki5512344 merged 1 commit intoAug 29, 2026
Conversation
The xterm-256 color cube (indices 16..231) is a 6x6x6 cube with per-channel levels [0x00, 0x5f, 0x87, 0xaf, 0xd7, 0xff]; index = 16 + 36*r + 6*g + b. The COLORS_256 array had 29 mis-typed entries using 0xdf instead of 0xd7 for the level-4 component. 0xdf is never a valid xterm-256 value (cube levels don't include it; gray tops at 0xee; ANSI uses 00/aa/55/ff only). The fix replaces df with d7 inside COLORS_256 for the affected cube rows (R=4 high-G and R=5 high-G/B). Verified by diffing against the canonical 256-entry table (cube via formula 16+36*r+6*g+b, gray via 8+10*i packed, ANSI via standard 16) — 29 cube diffs before, 0 remaining after df->d7. Indexes fixed include 178 (R=4,G=3,B=0), 188 (R=4,G=4,B=4), 220 (R=5,G=4,B=0), 230 (R=5,G=5,B=4). Grayscale (232..255) and ANSI (0..15) left untouched. This is a data-array fix with no logic changes. Add TerminalColorsTest asserting canonical indices and verifying no df remains in cube region. Revert-and-fail spot-check shows failing test if df restored in any cube entry. Refs: §36 m7 Assisted by: GLM (syn:large:text on synthetic.new) — code generation and review.
pocketprobe
added a commit
to pocketprobe/OC2R
that referenced
this pull request
Aug 30, 2026
Resolve SpotBugs MS_MUTABLE_ARRAY by design (not suppression) and build a real VT feature on top of it. Rebased onto work after DivByDiamond#30 merged; adapts DivByDiamond#30's follow-up test (942e1f9) to the per-instance accessors. Per-instance palette: the palette statics (COLORS/BRIGHT_COLORS/ COLORS_256) were public static final int[] — a process-global mutable shared across every Terminal. Privatized; each Terminal holds its own palette256 copy, initialized from the immutable default in RIS. A palette change never bleeds across terminals. Revert-and-fail proven: drop the clone and osc4DoesNotMutateStaticDefault / osc4PerTerminalIsolation fail. OSC 4 / 104: OSCManager was a pure stub. Real dispatch now — bounded char[] buffer, ST/BEL terminator, routes by numeric code to a per-code handler. The OSCHandler/OSC4/OSC104/OSCParse split mirrors CSISequenceHandler/CSIManager: a new OSC is a class + one registration. OSC 4 sets/queries a palette entry; OSC 104 resets one or all. Leaves OSC 0/2/8/10/11 as addable follow-ups. Palette sync across the network seam (Kimi K3 review F1): the palette never crossed the server->client diff — a server-side OSC 4 redefinition was invisible to players (the render path reads the client's palette256, which stayed default). Terminal now tracks paletteRevision (bumped on every palette write) + lastSentPaletteRevision; Snapshot carries int[] palette (nullable — null = unchanged, zero steady-state cost, ~1 KiB on change) when the revision moved, and captureFull forces it so a RIS reset snapshot carries the default palette (a client holding a prior change can't keep a stale one across reset). apply clones it onto the client. Cross-seam tests (osc4PaletteChangeSyncsToClientAcrossTheDiff etc.) exercise the server->client path the single-instance suite couldn't. OSC 4 reply xterm parity (F2): replies 16-bit per channel (rgb:%04x, duplicating the 8-bit value as xterm's UnMaskIt2 does, misc.c:2768) and reuses the query's own terminator (BEL query -> BEL reply, ST query -> ST reply), matching xterm unparseputc1(xw, final) (misc.c:2655). OSC 4 batched pairs (F3): a single OSC 4 may carry c;spec;c;spec pairs (xterm ChangeAnsiColorRequest, misc.c:2981); the loop stops on the first malformed entry. OSC 104 already looped correctly. OSC code parse (F4): the routing code is parsed without the 0-255 palette clamp (parseCode), so future OSC 777/1337 dispatch instead of silently dying at the palette edge; parseClampIndex stays for palette entries. Render path: indexed colors read terminal.palette256 and track OSC 4. DEFAULT_FOREGROUND/BACKGROUND + cursor read fixed-default accessors (xterm reserves those for OSC 10/11/12). Computed dim (SGR 2): replaces the fixed DIM_COLORS table with a computeFaint tail modifier (xterm's mechanism, util.c:5386) — scale the resolved color after mode/bold/blink resolution, so dim composes with bold and applies to bright/256/truecolor (the table only covered SIXTEEN_COLOR and silently ignored dim elsewhere). The 1/2 factor reproduces the old table exactly (0xAA0000 -> 0x550000) and is correct for our palette: xterm's 2/3 would collapse faint-bright onto normal (2/3*0xFF=0xAA), losing the fourth shade. Dim is foreground-only in xterm (getXtermBackground doesn't consume ATR_FAINT); the prior code dimmed the SIXTEEN_COLOR background, a divergence dropped. DIM_COLORS deleted — the last MS_MUTABLE_ARRAY, loop fully closed. Known divergence (separate follow-up): the ST-string managers (OSC/DCS/APC) swallow an ESC not followed by '\' rather than aborting (xterm illegal_parse aborts and re-enters the escape state). Harmless for well-formed OSC 4/104; matters for OSC 2 (title)/DCS sixel. Pre- existing across all three; a separate small PR will fix them together. Note on width changes: palette init lives in RIS only, not setWidth/resetRendition — a width change today is destructive (wipes screen/margins/cursor and would wipe the palette). DECSCPP is the non-destructive column primitive that separates "change columns" from "DECCOLM destructive reset"; the palette's setWidth-avoidance raises the urgency of that refactor promised in prior work. Verified: clean build (Error Prone on), 281 tests / 0 fail, Checkstyle 0, PMD 0, SpotBugs clean (all four palette MS_MUTABLE_ARRAY gone by design; Snapshot.palette EI_EXPOSE_REP/2 baselined, matching rowData's precedent; OSCHandler EI_EXPOSE_REP2 baselined, matching CSISequenceHandler; stale DIM_COLORS baseline entry pruned). Cross-seam tests + revert-and-fail proven. In-game confirmed (256color.sh full chart, ansi.sh --color-table four-shade + both reverse-video). Refs: §36 m7 Assisted by: GLM (syn:large:text on synthetic.new) — code generation and review. A Kimi K3 adversarial review caught the network-seam miss and three xterm-parity gaps; all addressed.
pocketprobe
added a commit
to pocketprobe/OC2R
that referenced
this pull request
Aug 30, 2026
Resolve SpotBugs MS_MUTABLE_ARRAY by design (not suppression) and build a real VT feature on top of it. Rebased onto work after DivByDiamond#30 merged; adapts DivByDiamond#30's follow-up test (942e1f9) to the per-instance accessors. Per-instance palette: the palette statics (COLORS/BRIGHT_COLORS/ COLORS_256) were public static final int[] — a process-global mutable shared across every Terminal. Privatized; each Terminal holds its own palette256 copy, initialized from the immutable default in RIS. A palette change never bleeds across terminals. Revert-and-fail proven: drop the clone and osc4DoesNotMutateStaticDefault / osc4PerTerminalIsolation fail. OSC 4 / 104: OSCManager was a pure stub. Real dispatch now — bounded char[] buffer, ST/BEL terminator, routes by numeric code to a per-code handler. The OSCHandler/OSC4/OSC104/OSCParse split mirrors CSISequenceHandler/CSIManager: a new OSC is a class + one registration. OSC 4 sets/queries a palette entry; OSC 104 resets one or all. Leaves OSC 0/2/8/10/11 as addable follow-ups. Palette sync across the network seam (Kimi K3 review F1): the palette never crossed the server->client diff — a server-side OSC 4 redefinition was invisible to players (the render path reads the client's palette256, which stayed default). Terminal now tracks paletteRevision (bumped on every palette write) + lastSentPaletteRevision; Snapshot carries int[] palette (nullable — null = unchanged, zero steady-state cost, ~1 KiB on change) when the revision moved, and captureFull forces it so a RIS reset snapshot carries the default palette (a client holding a prior change can't keep a stale one across reset). apply clones it onto the client. Cross-seam tests (osc4PaletteChangeSyncsToClientAcrossTheDiff etc.) exercise the server->client path the single-instance suite couldn't. OSC 4 reply xterm parity (F2): replies 16-bit per channel (rgb:%04x, duplicating the 8-bit value as xterm's UnMaskIt2 does, misc.c:2768) and reuses the query's own terminator (BEL query -> BEL reply, ST query -> ST reply), matching xterm unparseputc1(xw, final) (misc.c:2655). OSC 4 batched pairs (F3): a single OSC 4 may carry c;spec;c;spec pairs (xterm ChangeAnsiColorRequest, misc.c:2981); the loop stops on the first malformed entry. OSC 104 already looped correctly. OSC code parse (F4): the routing code is parsed without the 0-255 palette clamp (parseCode), so future OSC 777/1337 dispatch instead of silently dying at the palette edge; parseClampIndex stays for palette entries. Render path: indexed colors read terminal.palette256 and track OSC 4. DEFAULT_FOREGROUND/BACKGROUND + cursor read fixed-default accessors (xterm reserves those for OSC 10/11/12). Computed dim (SGR 2): replaces the fixed DIM_COLORS table with a computeFaint tail modifier (xterm's mechanism, util.c:5386) — scale the resolved color after mode/bold/blink resolution, so dim composes with bold and applies to bright/256/truecolor (the table only covered SIXTEEN_COLOR and silently ignored dim elsewhere). The 1/2 factor reproduces the old table exactly (0xAA0000 -> 0x550000) and is correct for our palette: xterm's 2/3 would collapse faint-bright onto normal (2/3*0xFF=0xAA), losing the fourth shade. Dim is foreground-only in xterm (getXtermBackground doesn't consume ATR_FAINT); the prior code dimmed the SIXTEEN_COLOR background, a divergence dropped. DIM_COLORS deleted — the last MS_MUTABLE_ARRAY, loop fully closed. Known divergence (separate follow-up): the ST-string managers (OSC/DCS/APC) swallow an ESC not followed by '\' rather than aborting (xterm illegal_parse aborts and re-enters the escape state). Harmless for well-formed OSC 4/104; matters for OSC 2 (title)/DCS sixel. Pre- existing across all three; a separate small PR will fix them together. Note on width changes: palette init lives in RIS only, not setWidth/resetRendition — a width change today is destructive (wipes screen/margins/cursor and would wipe the palette). DECSCPP is the non-destructive column primitive that separates "change columns" from "DECCOLM destructive reset"; the palette's setWidth-avoidance raises the urgency of that refactor promised in prior work. Verified: clean build (Error Prone on), 281 tests / 0 fail, Checkstyle 0, PMD 0, SpotBugs clean (all four palette MS_MUTABLE_ARRAY gone by design; Snapshot.palette EI_EXPOSE_REP/2 baselined, matching rowData's precedent; OSCHandler EI_EXPOSE_REP2 baselined, matching CSISequenceHandler; stale DIM_COLORS baseline entry pruned). Cross-seam tests + revert-and-fail proven. In-game confirmed (256color.sh full chart, ansi.sh --color-table four-shade + both reverse-video). Refs: §36 m7 Assisted by: GLM (syn:large:text on synthetic.new) — code generation and review. A Kimi K3 adversarial review caught the network-seam miss and three xterm-parity gaps; all addressed.
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
2 participants
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.
The xterm-256 color cube (
COLORS_256, indices 16–231) is a 6×6×6 cube with per-channel levels[0x00, 0x5f, 0x87, 0xaf, 0xd7, 0xff]. 29 cube entries used0xdffor the level-4 component where the canonical value is0xd7—0xdfis never a valid xterm-256 value (cube levels don't include it; the grayscale ramp tops out at0xee), so every0xdfin the array is a typo.What this does
df→d7in the affectedCOLORS_256rows. Verified against the canonical 256-entry table generated fromindex = 16 + 36*r + 6*g + bwith the level set above: 29 entries differed before (all in the cube, all containingdf); 0 after. The grayscale ramp (232–255) and the ANSI 0–15 are untouched.TerminalColorsTest.xterm256PaletteMatchesCanonicalFormula: asserts four canonical indices (188=0xd7d7d7, 178=0xd7af00, 230=0xffffd7, 220=0xffd700) and scans the whole cube region asserting no entry containsdf.Verification
compileJava+compileTestJavaran, not cached); full suite green;TerminalColorsTest1/0.0xdfdfdffails the test; restoring passes.Context
This is the base of a stack — a follow-up PR branches off this to make the palette a real per-instance feature (resolving the
MS_MUTABLE_ARRAYSpotBugs finding on the palette arrays by design, not suppression) and implementOSC 4/OSC 104for runtime palette set/query/reset. Folding the typo into that restructure was considered, but landing it separately as the corrected-defaults base keeps the feature PR's diff focused on the architecture change.Assisted by: GLM (syn:large:text on synthetic.new) — code generation and review.