Skip to content

Commit 8df7c43

Browse files
kyasbalkyasbal
committed
Adding color helper functions and better validation on style
registration
1 parent 8caa934 commit 8df7c43

6 files changed

Lines changed: 374 additions & 18 deletions

File tree

‎pkg/core/init/default/defaultextension.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import (
3030
"github.com/GoogleCloudPlatform/khi/pkg/core/inspection/tracing"
3131
"github.com/GoogleCloudPlatform/khi/pkg/generated"
3232
"github.com/GoogleCloudPlatform/khi/pkg/model/k8s"
33+
"github.com/GoogleCloudPlatform/khi/pkg/model/khifile/v6/style"
3334
"github.com/GoogleCloudPlatform/khi/pkg/parameters"
3435
"github.com/GoogleCloudPlatform/khi/pkg/server"
3536
"github.com/GoogleCloudPlatform/khi/pkg/server/option"
@@ -147,6 +148,7 @@ func (d *DefaultInitExtension) ConfigureInspectionTaskServer(taskServer *coreins
147148
return err
148149
}
149150
}
151+
style.LockRegistry()
150152
if *parameters.Auth.QuotaProjectID != "" {
151153
taskServer.AddRunContextOption(coreinspection.RunContextOptionArrayElementFromValue(googlecloudcommon_contract.APIClientFactoryOptionsContextKey, options.QuotaProject(*parameters.Auth.QuotaProjectID)))
152154
}

‎pkg/model/khifile/v6/style/colorutil.go‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
// Copyright 2026 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package style
16+
17+
import (
18+
"strconv"
19+
)
20+
21+
// ColorWhite represents the white color (1.0, 1.0, 1.0, 1.0).
22+
var ColorWhite = Color{R: 1, G: 1, B: 1, A: 1}
23+
24+
// ColorBlack represents the black color (0.0, 0.0, 0.0, 1.0).
25+
var ColorBlack = Color{R: 0, G: 0, B: 0, A: 1}
26+
27+
// MustForceConvertsRGBHex converts an sRGB hex color string to a Color.
28+
//
29+
// Deprecated:
30+
// The timeline style uses the display-p3 color space. Directly mapping sRGB
31+
// hex values to the 0.0-1.0 range in display-p3 is mathematically incorrect.
32+
// Ideally, you should define a Color struct directly with proper display-p3 values.
33+
func MustForceConvertsRGBHex(hex string) Color {
34+
if len(hex) != 7 && len(hex) != 9 {
35+
panic("invalid hex color length: " + hex)
36+
}
37+
r, err := strconv.ParseInt(hex[1:3], 16, 32)
38+
if err != nil {
39+
panic(err)
40+
}
41+
g, err := strconv.ParseInt(hex[3:5], 16, 32)
42+
if err != nil {
43+
panic(err)
44+
}
45+
b, err := strconv.ParseInt(hex[5:7], 16, 32)
46+
if err != nil {
47+
panic(err)
48+
}
49+
var a int64 = 255
50+
if len(hex) == 9 {
51+
a, err = strconv.ParseInt(hex[7:9], 16, 32)
52+
if err != nil {
53+
panic(err)
54+
}
55+
}
56+
return Color{
57+
R: float32(r) / 255.0,
58+
G: float32(g) / 255.0,
59+
B: float32(b) / 255.0,
60+
A: float32(a) / 255.0,
61+
}
62+
}

‎pkg/model/khifile/v6/style/colorutil_test.go‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
// Copyright 2026 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package style
16+
17+
import (
18+
"testing"
19+
20+
"github.com/google/go-cmp/cmp"
21+
)
22+
23+
func TestMustForceConvertsRGBHex(t *testing.T) {
24+
testCases := []struct {
25+
name string
26+
hex string
27+
want Color
28+
wantPanic bool
29+
}{
30+
{
31+
name: "valid 7-char hex",
32+
hex: "#CC33CC",
33+
want: Color{R: 204.0 / 255.0, G: 51.0 / 255.0, B: 204.0 / 255.0, A: 1.0},
34+
},
35+
{
36+
name: "valid 9-char hex with alpha",
37+
hex: "#0000FF80",
38+
want: Color{R: 0.0, G: 0.0, B: 1.0, A: 128.0 / 255.0},
39+
},
40+
{
41+
name: "invalid length 5-char",
42+
hex: "#FFFF",
43+
wantPanic: true,
44+
},
45+
{
46+
name: "invalid characters",
47+
hex: "#GGGGGG",
48+
wantPanic: true,
49+
},
50+
{
51+
name: "missing hash prefix",
52+
hex: "CC33CC",
53+
wantPanic: true,
54+
},
55+
}
56+
57+
for _, tc := range testCases {
58+
t.Run(tc.name, func(t *testing.T) {
59+
if tc.wantPanic {
60+
defer func() {
61+
if r := recover(); r == nil {
62+
t.Errorf("expected panic, but did not panic")
63+
}
64+
}()
65+
}
66+
got := MustForceConvertsRGBHex(tc.hex)
67+
if !tc.wantPanic {
68+
if diff := cmp.Diff(tc.want, got); diff != "" {
69+
t.Errorf("MustForceConvertsRGBHex() mismatch (-want +got):\n%s", diff)
70+
}
71+
}
72+
})
73+
}
74+
}

‎pkg/model/khifile/v6/style/timeline.go‎

Lines changed: 86 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ package style
1717
import (
1818
_ "embed"
1919
"encoding/json"
20+
"fmt"
2021
"slices"
2122
"sync"
2223

@@ -58,6 +59,7 @@ func GetIconAtlas() *pb.IconAtlas {
5859

5960
var (
6061
mu sync.RWMutex
62+
locked bool
6163
severities []*pb.Severity
6264
verbs []*pb.Verb
6365
logTypes []*pb.LogType
@@ -75,13 +77,40 @@ func reset() {
7577
logTypes = nil
7678
revisionStates = nil
7779
timelineTypes = nil
80+
locked = false
81+
}
82+
83+
// LockRegistry locks the timeline style registry, preventing any further style registrations.
84+
// This must be called after task registration has completed.
85+
func LockRegistry() {
86+
mu.Lock()
87+
defer mu.Unlock()
88+
locked = true
7889
}
7990

8091
// Color represents a color with high dynamic range (HDR) capabilities.
8192
type Color struct {
8293
R, G, B, A float32
8394
}
8495

96+
// Verify checks if R, G, B, and A are within [0.0, 1.0] range.
97+
// Returns an error if any channel value is invalid.
98+
func (c Color) Verify() error {
99+
if c.R < 0 || c.R > 1 {
100+
return fmt.Errorf("R channel %f is out of range [0, 1]", c.R)
101+
}
102+
if c.G < 0 || c.G > 1 {
103+
return fmt.Errorf("G channel %f is out of range [0, 1]", c.G)
104+
}
105+
if c.B < 0 || c.B > 1 {
106+
return fmt.Errorf("B channel %f is out of range [0, 1]", c.B)
107+
}
108+
if c.A < 0 || c.A > 1 {
109+
return fmt.Errorf("A channel %f is out of range [0, 1]", c.A)
110+
}
111+
return nil
112+
}
113+
85114
func (c Color) toProto() *pb.HDRColor4 {
86115
return &pb.HDRColor4{
87116
R: proto.Float32(c.R),
@@ -91,12 +120,22 @@ func (c Color) toProto() *pb.HDRColor4 {
91120
}
92121
}
93122

94-
// RegisterTimelineType registers a TimelineType, assigns a unique ID to it,
123+
// MustRegisterTimelineType registers a TimelineType, assigns a unique ID to it,
95124
// and returns the generated pointer. This allows for global inline initialization in plugins.
96-
func RegisterTimelineType(label string, description string, backgroundColor Color, foregroundColor Color, visible bool, sortPriority int32) *pb.TimelineType {
125+
func MustRegisterTimelineType(label string, description string, backgroundColor Color, foregroundColor Color, visible bool, sortPriority int32) *pb.TimelineType {
126+
if err := backgroundColor.Verify(); err != nil {
127+
panic(fmt.Sprintf("invalid background color for timeline type %q: %v", label, err))
128+
}
129+
if err := foregroundColor.Verify(); err != nil {
130+
panic(fmt.Sprintf("invalid foreground color for timeline type %q: %v", label, err))
131+
}
97132
mu.Lock()
98133
defer mu.Unlock()
99134

135+
if locked {
136+
panic(fmt.Sprintf("Failed to register timeline type style %q: style-related registrations must be done in task/inspection/**/contract packages during package initialization. Did you call it from outside of the contract or not at package initialization timing?", label))
137+
}
138+
100139
id := uint32(len(timelineTypes) + 1)
101140
t := &pb.TimelineType{
102141
Id: proto.Uint32(id),
@@ -111,12 +150,22 @@ func RegisterTimelineType(label string, description string, backgroundColor Colo
111150
return t
112151
}
113152

114-
// RegisterSeverity registers a Severity, assigns a unique ID to it,
153+
// MustRegisterSeverity registers a Severity, assigns a unique ID to it,
115154
// and returns the generated pointer.
116-
func RegisterSeverity(label string, shortLabel string, backgroundColor Color, foregroundColor Color, order int32) *pb.Severity {
155+
func MustRegisterSeverity(label string, shortLabel string, backgroundColor Color, foregroundColor Color, order int32) *pb.Severity {
156+
if err := backgroundColor.Verify(); err != nil {
157+
panic(fmt.Sprintf("invalid background color for severity %q: %v", label, err))
158+
}
159+
if err := foregroundColor.Verify(); err != nil {
160+
panic(fmt.Sprintf("invalid foreground color for severity %q: %v", label, err))
161+
}
117162
mu.Lock()
118163
defer mu.Unlock()
119164

165+
if locked {
166+
panic(fmt.Sprintf("Failed to register severity style %q: style-related registrations must be done in task/inspection/**/contract packages during package initialization. Did you call it from outside of the contract or not at package initialization timing?", label))
167+
}
168+
120169
id := uint32(len(severities) + 1)
121170
s := &pb.Severity{
122171
Id: proto.Uint32(id),
@@ -130,12 +179,22 @@ func RegisterSeverity(label string, shortLabel string, backgroundColor Color, fo
130179
return s
131180
}
132181

133-
// RegisterVerb registers a Verb, assigns a unique ID to it,
182+
// MustRegisterVerb registers a Verb, assigns a unique ID to it,
134183
// and returns the generated pointer.
135-
func RegisterVerb(label string, backgroundColor Color, foregroundColor Color, visible bool) *pb.Verb {
184+
func MustRegisterVerb(label string, backgroundColor Color, foregroundColor Color, visible bool) *pb.Verb {
185+
if err := backgroundColor.Verify(); err != nil {
186+
panic(fmt.Sprintf("invalid background color for verb %q: %v", label, err))
187+
}
188+
if err := foregroundColor.Verify(); err != nil {
189+
panic(fmt.Sprintf("invalid foreground color for verb %q: %v", label, err))
190+
}
136191
mu.Lock()
137192
defer mu.Unlock()
138193

194+
if locked {
195+
panic(fmt.Sprintf("Failed to register verb style %q: style-related registrations must be done in task/inspection/**/contract packages during package initialization. Did you call it from outside of the contract or not at package initialization timing?", label))
196+
}
197+
139198
id := uint32(len(verbs) + 1)
140199
v := &pb.Verb{
141200
Id: proto.Uint32(id),
@@ -148,12 +207,22 @@ func RegisterVerb(label string, backgroundColor Color, foregroundColor Color, vi
148207
return v
149208
}
150209

151-
// RegisterLogType registers a LogType, assigns a unique ID to it,
210+
// MustRegisterLogType registers a LogType, assigns a unique ID to it,
152211
// and returns the generated pointer.
153-
func RegisterLogType(label string, description string, backgroundColor Color, foregroundColor Color) *pb.LogType {
212+
func MustRegisterLogType(label string, description string, backgroundColor Color, foregroundColor Color) *pb.LogType {
213+
if err := backgroundColor.Verify(); err != nil {
214+
panic(fmt.Sprintf("invalid background color for log type %q: %v", label, err))
215+
}
216+
if err := foregroundColor.Verify(); err != nil {
217+
panic(fmt.Sprintf("invalid foreground color for log type %q: %v", label, err))
218+
}
154219
mu.Lock()
155220
defer mu.Unlock()
156221

222+
if locked {
223+
panic(fmt.Sprintf("Failed to register log type style %q: style-related registrations must be done in task/inspection/**/contract packages during package initialization. Did you call it from outside of the contract or not at package initialization timing?", label))
224+
}
225+
157226
id := uint32(len(logTypes) + 1)
158227
l := &pb.LogType{
159228
Id: proto.Uint32(id),
@@ -166,12 +235,19 @@ func RegisterLogType(label string, description string, backgroundColor Color, fo
166235
return l
167236
}
168237

169-
// RegisterRevisionState registers a RevisionState, assigns a unique ID to it,
238+
// MustRegisterRevisionState registers a RevisionState, assigns a unique ID to it,
170239
// and returns the generated pointer.
171-
func RegisterRevisionState(label string, icon string, description string, backgroundColor Color, style pb.RevisionStateStyle) *pb.RevisionState {
240+
func MustRegisterRevisionState(label string, icon string, description string, backgroundColor Color, style pb.RevisionStateStyle) *pb.RevisionState {
241+
if err := backgroundColor.Verify(); err != nil {
242+
panic(fmt.Sprintf("invalid background color for revision state %q: %v", label, err))
243+
}
172244
mu.Lock()
173245
defer mu.Unlock()
174246

247+
if locked {
248+
panic(fmt.Sprintf("Failed to register revision state style %q: style-related registrations must be done in task/inspection/**/contract packages during package initialization. Did you call it from outside of the contract or not at package initialization timing?", label))
249+
}
250+
175251
id := uint32(len(revisionStates) + 1)
176252
rs := &pb.RevisionState{
177253
Id: proto.Uint32(id),

0 commit comments

Comments
 (0)