fix(colours): letter-family group boxes lost their palette colours (uppercase distinctly coloured again) - #63
Merged
Merged
Conversation
Users on the default palette saw no group colours at all - capitals were not visually distinct from lowercase (v5 report: 'they were in a separate, uniquely colored node'). Two causes: 1. Load-order clobber: new-format color.*.xml files parsed BEFORE legacy colour.*.xml, and Data/colours/colour.xml is a legacy palette also named 'Default' - ParseLegacy overwrote the new Default and dropped its group colours (uppercase #ffff00 etc). Legacy files now parse first so the maintained format wins name collisions. 2. ParseLegacy never set any letter-family groupColor and missed the bare 'uppercase'/'limitedPunctuation' colorInfoName keys the alphabets actually use, so even legacy palettes rendered all letter families identically. Restore the v6 palette design consistently: lowercase 0 / uppercase 111 (yellow) / punctuation 112 (green) / numbers 113 (red) - legacy indices that match the new-format files' groupColor values exactly. Regression test renders a frame and asserts the uppercase group box fill: Default palette and a legacy palette both must show #FFFF00 (verified failing pre-fix via a frame-command colour probe). Fixes #62 Signed-off-by: will wade <willwade@gmail.com>
The 'A' label (unique to the uppercase group at rest) must sit inside the yellow filled rect, so an unrelated yellow element can no longer satisfy the assertion (caught in review of #63). Re-verified fail-then-pass against the pre-fix sources. Signed-off-by: will wade <willwade@gmail.com>
5 tasks
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
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.
Summary
Fixes #62 — the v5-parity report: "In v5, [capitals] were placed in a separate, uniquely colored node, which made them much easier to spot and target quickly." The separate group node survived; its colour did not.
Investigation traced it with a frame-command colour probe (scan
dasher_frameoutput for filled-rect RGB per palette), which found two compounding causes:1. Load-order clobber (hits every default-palette user)
DasherInterfaceBaseparsed new-formatcolor.*.xmlbefore legacycolour.*.xml— andData/colours/colour.xmlis a legacy palette also named "Default".ParseLegacyoverwroteKnownPalettes["Default"], destroying the new-format Default that definesuppercase groupColor="#ffff00". Probe, pre-fix: on Default no group box rendered with any palette fill at all; on European/Asian the yellow uppercase box rendered fine.2. Legacy parser never set letter-family group colours
ParseLegacymapped every letter-familycolorInfoNameto one sharedGroupColorInfowith nogroupColor, and its key list missed the bare"uppercase"/"limitedPunctuation"names the alphabet files actually use — so legacy palettes rendered all letter families identically.Fix
ParseLegacyrestores group colours per v6's palette design: lowercase0/ uppercase111(yellow) / punctuation112(green) / numbers113(red) — legacy indices that exactly match the new-format files'groupColorvalues (verified against the parsed palettes; v5's own file had pink there, but v6's maintained palettes deliberately chose yellow/green/red, so the fix makes both formats consistent).Result (probe-verified)
Capitals now sit in a distinctly coloured group box on every palette — the v5 behaviour restored under v6's colour design.
Test
uppercase_group_box_has_distinct_colour(test_draw_commands.cpp): renders frames and asserts the uppercase group's fill colour on both the Default palette (post-clobber) and a legacy palette — verified failing pre-fix, passing post-fix. Local matrix suite green (capi 10, extended 17, draw 9, parameters 13, settings 7, color-math 14).Fixes #62
Greptile Summary
The PR restores distinct letter-family group-box colors across modern and legacy palettes and ensures maintained modern palettes win name collisions.
colour.*.xmlfiles before moderncolor.*.xmlfiles.Confidence Score: 5/5
The PR appears safe to merge.
No blocking failure remains.
Important Files Changed
Reviews (2): Last reviewed commit: "test: tie uppercase-box assertion to geo..." | Re-trigger Greptile