From e14e34948ec6e8f74679740c4cc9c2677998f224 Mon Sep 17 00:00:00 2001 From: Kovid Goyal Date: Wed, 31 Dec 2025 09:35:09 +0530 Subject: [PATCH] More cleanups for color parsing Using rounding when converting float to uint8 for more accuracy. Fix rgb:3 and rgbi: parsing in Go code. Various other minor cleanups. --- kitty/colors.c | 2 +- kitty_tests/datatypes.py | 40 ++++---- tools/utils/style/colorspaces.go | 49 ++++----- tools/utils/style/colorspaces_test.go | 137 +++++--------------------- tools/utils/style/wrapper.go | 108 +++++++++++++++----- tools/utils/style/wrapper_test.go | 2 +- 6 files changed, 151 insertions(+), 187 deletions(-) diff --git a/kitty/colors.c b/kitty/colors.c index fa83e6473..7dd2023a3 100644 --- a/kitty/colors.c +++ b/kitty/colors.c @@ -844,7 +844,7 @@ parse_rgb(const char *spec, size_t len) { return (PyObject*)alloc_color(r, g, b, 0); } -static unsigned char as8bit(double f) { return (unsigned char)((MAX(0., MIN(f, 1.))) * 255.); } +static unsigned char as8bit(double f) { return (unsigned char)(round(f * 255.)); } static bool parse_single_intensity(const char *s, unsigned char *out) { diff --git a/kitty_tests/datatypes.py b/kitty_tests/datatypes.py index 5bab06088..3335ba70e 100644 --- a/kitty_tests/datatypes.py +++ b/kitty_tests/datatypes.py @@ -70,18 +70,23 @@ class TestDataTypes(BaseTest): self.ae(c.blue, b, spec) self.ae(c.alpha, a, spec) - c('#eee', 0xee, 0xee, 0xee) + c('#eee # comment', 0xee, 0xee, 0xee) c('#234567', 0x23, 0x45, 0x67) c('#abcabcdef', 0xab, 0xab, 0xde) - c('rgb:e/e/e', 0xee, 0xee, 0xee) + c('rgb:e/e/e # comment', 0xee, 0xee, 0xee) c('rgb:23/45/67', 0x23, 0x45, 0x67) c('rgb:abc/abc/def', 0xab, 0xab, 0xde) - c('red', 0xff) - - # Wide gamut color formats - c('oklch(0.7 0.15 140)', 0x67, 0xb4, 0x56) # OKLCH green - c('oklch(0.9 0.05 265)', 0xcd, 0xde, 0xfe) # OKLCH light blue - c('lab(70 50 -30)', 0xea, 0x88, 0xe2) # CIE LAB purple-ish + c('red', 0xff, 0, 0) + c('alice blue # comment', 240, 248, 255) + c('oklch(1,0,0)', 255, 255, 255) + c('oklch(0,0,0)', 0, 0, 0) + c('oklch(0.5,0.1,180)', 0, 117, 101) + c('oklch(0.7 0.15 140) # comment', 0x68, 0xb4, 0x57) + c('oklch(0.9 0.05 265)', 0xce, 0xde, 0xff) + c('lab(70 50 -30)', 0xea, 0x88, 0xe2) + c('lab(50,0,0)', 199, 199, 199) + c('lab(100,0,0)', 255, 255, 255) + c('lab(0,0,0)', 0, 0, 0) self.ae(int(Color(1, 2, 3)), 0x10203) base = Color(12, 12, 12) @@ -98,9 +103,7 @@ class TestDataTypes(BaseTest): def c(spec, r=0, g=0, b=0): color = to_color(spec) self.assertIsNotNone(color, f'Failed to parse: {spec}') - self.ae(color.red, r) - self.ae(color.green, g) - self.ae(color.blue, b) + self.ae(Color(r, g, b), color, spec) def in_range(spec): """Verify color values are in valid 0-255 range""" @@ -125,8 +128,8 @@ class TestDataTypes(BaseTest): c('oklch(1 0 0)', 0xff, 0xff, 0xff) # Pure white # Achromatic colors (zero chroma) - c('oklch(0.5 0 180)', 0xbb, 0xbb, 0xbb) # Mid gray, hue irrelevant - c('oklch(0.25 0 90)', 0x88, 0x88, 0x88) # Dark gray + c('oklch(0.5 0 180)', 0xbc, 0xbc, 0xbc) # Mid gray, hue irrelevant + c('oklch(0.25 0 90)', 0x89, 0x89, 0x89) # Dark gray # Test various hues with moderate chroma in_range('oklch(0.6 0.15 0)') # Red hue @@ -155,13 +158,11 @@ class TestDataTypes(BaseTest): def c(spec, r=0, g=0, b=0): color = to_color(spec) self.assertIsNotNone(color, f'Failed to parse: {spec}') - self.ae(color.red, r) - self.ae(color.green, g) - self.ae(color.blue, b) + self.ae(Color(r, g, b), color, spec) # OKLCH with inline comment c('oklch(0.5 0.1 180) # Cyan color', 0x00, 0x75, 0x65) - c('oklch(0.7 0.15 140) # Green', 0x67, 0xb4, 0x56) + c('oklch(0.7 0.15 140) # Green', 0x68, 0xb4, 0x57) # Hex colors with inline comments c('#ff0000 # Red', 0xff, 0x00, 0x00) @@ -186,11 +187,12 @@ class TestDataTypes(BaseTest): self.ae(color.red, r) self.ae(color.green, g) self.ae(color.blue, b) + self.ae(Color(r, g, b), color, spec) # LAB basic colors c('lab(0 0 0)', 0x00, 0x00, 0x00) # LAB black - c('lab(100 0 0)', 0xfe, 0xfe, 0xfe) # LAB white (slightly clipped in conversion) - c('lab(50 0 0)', 0xc6, 0xc6, 0xc6) # LAB mid-gray + c('lab(100 0 0)', 0xff, 0xff, 0xff) # LAB white + c('lab(50 0 0)', 0xc7, 0xc7, 0xc7) # LAB mid-gray # LAB with color components c('lab(70 50 -30)', 0xea, 0x88, 0xe2) # Purple-ish diff --git a/tools/utils/style/colorspaces.go b/tools/utils/style/colorspaces.go index d194a1005..284fbf3a3 100644 --- a/tools/utils/style/colorspaces.go +++ b/tools/utils/style/colorspaces.go @@ -3,6 +3,7 @@ package style import ( + "errors" "fmt" "math" "regexp" @@ -10,6 +11,8 @@ import ( "strings" ) +var _ = fmt.Println + // Color space conversion functions for wide gamut color support // Implements OKLCH, Display P3, and CIE LAB color formats with // CSS Color Module Level 4 gamut mapping. @@ -96,14 +99,6 @@ func deltaEOk(lab1, lab2 [3]float64) float64 { // oklchToSrgbGamutMap converts OKLCH to sRGB with CSS Color Module Level 4 gamut mapping func oklchToSrgbGamutMap(l, c, h float64) (float64, float64, float64) { - // Validate for NaN and infinity - if !math.IsInf(l, 0) && !math.IsInf(c, 0) && !math.IsInf(h, 0) && - !math.IsNaN(l) && !math.IsNaN(c) && !math.IsNaN(h) { - // Valid input - } else { - return 0.0, 0.0, 0.0 // Fallback to black - } - // Constants from CSS Color Module Level 4 const jnd = 0.02 // Just Noticeable Difference threshold const minConvergence = 0.0001 // Binary search precision @@ -236,11 +231,11 @@ func labToOklch(l, a, b float64) (float64, float64, float64) { // parseOklch parses OKLCH color: oklch(l c h) or oklch(l, c, h) func parseOklch(spec string) (RGBA, error) { - spec = strings.Trim(spec, "()") + spec = strings.TrimRight(spec, ")") parts := splitColorComponents(spec) if len(parts) != 3 { - return RGBA{}, errInvalidColor + return RGBA{}, errors.New("not enough parts") } l, err := parseFloatValue(parts[0]) @@ -260,17 +255,17 @@ func parseOklch(spec string) (RGBA, error) { if math.IsNaN(l) || math.IsInf(l, 0) || math.IsNaN(c) || math.IsInf(c, 0) || math.IsNaN(h) || math.IsInf(h, 0) { - return RGBA{}, errInvalidColor + return RGBA{}, errors.New("invalid float value") } // Handle percentages for L if strings.Contains(parts[0], "%") { - l = l / 100.0 + l /= 100.0 } // Clamp to reasonable ranges - l = math.Max(0.0, math.Min(1.0, l)) - c = math.Max(0.0, c) // Chroma is unbounded + l = max(0.0, min(l, 1.0)) + c = max(0.0, c) // Chroma is unbounded h = math.Mod(h, 360) // Wrap hue to 0-360 if h < 0 { h += 360 @@ -278,21 +273,18 @@ func parseOklch(spec string) (RGBA, error) { // Convert OKLCH to sRGB with gamut mapping r, g, b := oklchToSrgbGamutMap(l, c, h) - - return RGBA{ - Red: uint8(r * 255), - Green: uint8(g * 255), - Blue: uint8(b * 255), - }, nil + return RGBA{as8bit(r), as8bit(g), as8bit(b), 0}, nil } +func as8bit(x float64) uint8 { return uint8(math.Round(x * 255)) } + // parseLab parses LAB color: lab(l a b) or lab(l, a, b) func parseLab(spec string) (RGBA, error) { - spec = strings.Trim(spec, "()") + spec = strings.TrimRight(spec, ")") parts := splitColorComponents(spec) if len(parts) != 3 { - return RGBA{}, errInvalidColor + return RGBA{}, errors.New("not enough parts") } l, err := parseFloatValue(parts[0]) @@ -312,23 +304,18 @@ func parseLab(spec string) (RGBA, error) { if math.IsNaN(l) || math.IsInf(l, 0) || math.IsNaN(a) || math.IsInf(a, 0) || math.IsNaN(b) || math.IsInf(b, 0) { - return RGBA{}, errInvalidColor + return RGBA{}, errors.New("invalid float value") } // Clamp L to 0-100 - l = math.Max(0.0, math.Min(100.0, l)) + l = max(0.0, min(l, 100.0)) // Convert LAB to OKLCH, then use gamut mapping to sRGB lOk, c, h := labToOklch(l, a, b) // Apply gamut mapping in OKLCH space r, g, bVal := oklchToSrgbGamutMap(lOk, c, h) - - return RGBA{ - Red: uint8(r * 255), - Green: uint8(g * 255), - Blue: uint8(bVal * 255), - }, nil + return RGBA{as8bit(r), as8bit(g), as8bit(bVal), 0}, nil } // splitColorComponents splits color components by comma or whitespace @@ -353,5 +340,3 @@ func parseFloatValue(s string) (float64, error) { s = strings.TrimRight(s, "%,") return strconv.ParseFloat(s, 64) } - -var errInvalidColor = fmt.Errorf("invalid color format") diff --git a/tools/utils/style/colorspaces_test.go b/tools/utils/style/colorspaces_test.go index 8b343bea4..f1d5cf4da 100644 --- a/tools/utils/style/colorspaces_test.go +++ b/tools/utils/style/colorspaces_test.go @@ -3,123 +3,40 @@ package style import ( - "math" "testing" ) -func TestParseOklch(t *testing.T) { - tests := []struct { - name string - input string - want RGBA - }{ - { - name: "basic oklch", - input: "0.5 0.1 180", - want: RGBA{Red: 0, Green: 117, Blue: 101}, // cyan-ish with gamut mapping - }, - { - name: "white", - input: "1.0 0 0", - want: RGBA{Red: 255, Green: 255, Blue: 255}, - }, - { - name: "black", - input: "0 0 0", - want: RGBA{Red: 0, Green: 0, Blue: 0}, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := parseOklch(tt.input) - if err != nil { - t.Errorf("parseOklch() error = %v", err) - return - } - // Allow some tolerance due to rounding - if math.Abs(float64(got.Red)-float64(tt.want.Red)) > 2 || - math.Abs(float64(got.Green)-float64(tt.want.Green)) > 2 || - math.Abs(float64(got.Blue)-float64(tt.want.Blue)) > 2 { - t.Errorf("parseOklch() = %v, want %v", got, tt.want) - } - }) - } -} - -func TestParseLab(t *testing.T) { - tests := []struct { - name string - input string - want RGBA - }{ - { - name: "basic lab", - input: "50 0 0", - want: RGBA{Red: 198, Green: 198, Blue: 198}, // light gray (LAB 50 is lighter than sRGB 50%) - }, - { - name: "white", - input: "100 0 0", - want: RGBA{Red: 255, Green: 255, Blue: 255}, - }, - { - name: "black", - input: "0 0 0", - want: RGBA{Red: 0, Green: 0, Blue: 0}, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := parseLab(tt.input) - if err != nil { - t.Errorf("parseLab() error = %v", err) - return - } - // Allow some tolerance due to rounding - if math.Abs(float64(got.Red)-float64(tt.want.Red)) > 2 || - math.Abs(float64(got.Green)-float64(tt.want.Green)) > 2 || - math.Abs(float64(got.Blue)-float64(tt.want.Blue)) > 2 { - t.Errorf("parseLab() = %v, want %v", got, tt.want) - } - }) - } -} - func TestParseColor(t *testing.T) { - tests := []struct { - name string - input string - wantErr bool - }{ - { - name: "oklch format", - input: "oklch(0.5 0.1 180)", - wantErr: false, - }, - { - name: "lab format", - input: "lab(50 0 0)", - wantErr: false, - }, - { - name: "with inline comment", - input: "oklch(0.5 0.1 180) # vibrant color", - wantErr: false, - }, - { - name: "hex color", - input: "#ff0000", - wantErr: false, - }, + type tr struct { + input string + expected RGBA + } + c := func(t string, r, g, b uint8) tr { return tr{t, RGBA{r, g, b, 0}} } + tests := []tr{ + c(`#eee # comment`, 0xee, 0xee, 0xee), + c(`#234567`, 0x23, 0x45, 0x67), + c(`#abcabcdef`, 0xab, 0xab, 0xde), + c(`rgb:e/e/e # comment`, 0xee, 0xee, 0xee), + c(`rgb:23/45/67`, 0x23, 0x45, 0x67), + c(`rgb:abc/abc/def`, 0xab, 0xab, 0xde), + c(`red`, 0xff, 0, 0), + c(`alice blue # comment`, 240, 248, 255), + c(`oklch(1,0,0)`, 255, 255, 255), + c(`oklch(0,0,0)`, 0, 0, 0), + c(`oklch(0.5,0.1,180)`, 0, 117, 101), + c(`oklch(0.7 0.15 140) # comment`, 0x68, 0xb4, 0x57), + c(`oklch(0.9 0.05 265)`, 0xce, 0xde, 0xff), + c(`lab(70 50 -30)`, 0xea, 0x88, 0xe2), + c(`lab(50,0,0)`, 199, 199, 199), + c(`lab(100,0,0)`, 255, 255, 255), + c(`lab(0,0,0)`, 0, 0, 0), } for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - _, err := ParseColor(tt.input) - if (err != nil) != tt.wantErr { - t.Errorf("ParseColor() error = %v, wantErr %v", err, tt.wantErr) + t.Run(tt.input, func(t *testing.T) { + actual, err := ParseColor(tt.input) + if actual != tt.expected { + t.Errorf("ParseColor(%#v) error = %v, got %v wanted %v", tt.input, err, actual, tt.expected) } }) } diff --git a/tools/utils/style/wrapper.go b/tools/utils/style/wrapper.go index 4cfcd485b..29d9558e6 100644 --- a/tools/utils/style/wrapper.go +++ b/tools/utils/style/wrapper.go @@ -65,19 +65,45 @@ func (self RGBA) AsRGBSharp() string { func (self *RGBA) parse_rgb_strings(r string, g string, b string) bool { var rv, gv, bv uint64 var err error - if rv, err = strconv.ParseUint(r, 16, 8); err != nil { + if len(r) == 1 { + r += r + g += g + b += b + } + if rv, err = strconv.ParseUint(r[:min(len(r), 2)], 16, 8); err != nil { return false } - if gv, err = strconv.ParseUint(g, 16, 8); err != nil { + if gv, err = strconv.ParseUint(g[:min(len(r), 2)], 16, 8); err != nil { return false } - if bv, err = strconv.ParseUint(b, 16, 8); err != nil { + if bv, err = strconv.ParseUint(b[:min(len(r), 2)], 16, 8); err != nil { return false } self.Red, self.Green, self.Blue = uint8(rv), uint8(gv), uint8(bv) return true } +func fas_uint8(x float64) uint8 { + x = max(0, min(x, 1)) + return uint8(x * 255) +} + +func (self *RGBA) parse_rgb_intensities(r string, g string, b string) bool { + var rv, gv, bv float64 + var err error + if rv, err = strconv.ParseFloat(r, 64); err != nil { + return false + } + if gv, err = strconv.ParseFloat(g, 64); err != nil { + return false + } + if bv, err = strconv.ParseFloat(b, 64); err != nil { + return false + } + self.Red, self.Green, self.Blue = fas_uint8(rv), fas_uint8(gv), fas_uint8(bv) + return true +} + func (self *RGBA) AsRGB() uint32 { return uint32(self.Blue) | (uint32(self.Green) << 8) | (uint32(self.Red) << 16) } @@ -123,31 +149,41 @@ type color_value struct { func parse_sharp(color string) (ans RGBA, err error) { if len(color)%3 != 0 { - return RGBA{}, fmt.Errorf("Not a valid color: #%s", color) + return RGBA{}, fmt.Errorf("length not a multiple of 3") } part_size := len(color) / 3 - r, g, b := color[:part_size], color[part_size:2*part_size], color[part_size*2:] - if part_size == 1 { - r += r - g += g - b += b - } + r, g, b := color[:part_size], color[part_size:2*part_size], color[part_size*2:part_size*3] if !ans.parse_rgb_strings(r, g, b) { - err = fmt.Errorf("Not a valid color: #%s", color) + err = fmt.Errorf("invalid rgb numbers") } return } func parse_rgb(color string) (ans RGBA, err error) { colors := strings.Split(color, "/") - if len(colors) == 3 && ans.parse_rgb_strings(colors[0], colors[1], colors[2]) { + if len(colors) != 3 { + return RGBA{}, fmt.Errorf("length not a multiple of 3") + } + if ans.parse_rgb_strings(colors[0], colors[1], colors[2]) { return } - err = fmt.Errorf("Not a valid RGB color: %#v", color) + err = fmt.Errorf("invalid rgb numbers") return } -func ParseColor(color string) (RGBA, error) { +func parse_rgbi(color string) (ans RGBA, err error) { + colors := strings.Split(color, "/") + if len(colors) != 3 { + return RGBA{}, fmt.Errorf("length not a multiple of 3") + } + if ans.parse_rgb_intensities(colors[0], colors[1], colors[2]) { + return + } + err = fmt.Errorf("invalid rgb numbers") + return +} + +func ParseColor(color string) (ans RGBA, err error) { // Strip inline comments (e.g., "oklch(...) # comment") // For hex colors like "#ff0000", preserve the first #, but strip comments after spaces color = strings.TrimSpace(color) @@ -168,19 +204,43 @@ func ParseColor(color string) (RGBA, error) { if val, ok := ColorNames[raw]; ok { return val, nil } - if strings.HasPrefix(raw, "#") { - return parse_sharp(raw[1:]) + if len(raw) < 4 { + return RGBA{}, fmt.Errorf("not a valid color name: %#v", color) } - if strings.HasPrefix(raw, "oklch(") { - return parseOklch(raw[6:]) + var parser func(string) (RGBA, error) + switch raw[0] { + case '#': + parser = parse_sharp + raw = raw[1:] + case 'o': + if strings.HasPrefix(raw, "oklch(") { + parser, raw = parseOklch, raw[6:] + } + case 'l': + if strings.HasPrefix(raw, "lab(") { + parser, raw = parseLab, raw[4:] + } + case 'r': + if len(raw) > 4 && strings.HasPrefix(raw, "rgb") { + raw = raw[3:] + switch raw[0] { + case ':': + parser, raw = parse_rgb, raw[1:] + case 'i': + if strings.HasPrefix(raw, "i:") { + parser, raw = parse_rgbi, raw[2:] + } + } + } } - if strings.HasPrefix(raw, "lab(") { - return parseLab(raw[4:]) + if parser == nil { + err = fmt.Errorf("not a valid color name: %#v", color) + } else { + if ans, err = parser(raw); err != nil { + err = fmt.Errorf("not a valid color name: %#v %w", color, err) + } } - if strings.HasPrefix(raw, "rgb:") { - return parse_rgb(raw[4:]) - } - return RGBA{}, fmt.Errorf("Not a valid color name: %#v", color) + return } type NullableColor struct { diff --git a/tools/utils/style/wrapper_test.go b/tools/utils/style/wrapper_test.go index 866be1ba5..7f2534518 100644 --- a/tools/utils/style/wrapper_test.go +++ b/tools/utils/style/wrapper_test.go @@ -39,7 +39,7 @@ func TestANSIStyleSprint(t *testing.T) { test("fg=15", "\x1b[97m", "\x1b[39m") test("bg=15", "\x1b[107m", "\x1b[49m") test("fg=#123", "\x1b[38:2:17:34:51m", "\x1b[39m") - test("fg=rgb:1/2/3", "\x1b[38:2:1:2:3m", "\x1b[39m") + test("fg=rgb:1/2/3", "\x1b[38:2:17:34:51m", "\x1b[39m") test("bg=123", "\x1b[48:5:123m", "\x1b[49m") test("uc=123", "\x1b[58:5:123m", "\x1b[59m") test("uc=1", "\x1b[58:5:1m", "\x1b[59m")