Handle a palette with fewer than two colors in Palette.At - #7
Merged
peterhellberg merged 1 commit intoSep 7, 2026
Merged
Conversation
At interpolates between p[i] and p[i+1] using n-1 as the divisor. With a single color that divisor is zero, so any t strictly between 0 and 1 indexes p[1] on a length-1 slice and panics. An empty palette panics at p[0] for every t. Color(n) and Convert already guard the degenerate cases on the same type; At was the one that did not.
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.
Palette.Atis documented as taking a value inrange 0-1, and it guardst <= 0,t >= 1andNaN, so it looks total over that range. It is not, for a palette with fewer than two colors.Both endpoints return correctly and only the interior of the documented range panics, which is why it survives a smoke test. The cause is at
palette.go:99-102:n-1is the divisor, so withn == 1it is0,sbecomes+Inf,istays0, and the last line readsp[i+1]. An empty palette is worse -- it panics atp[0]for everyt.Why I picked these return values
Rather than invent a convention I followed the two sibling methods on the same type:
Color(n)(palette.go:15) bounds-checks and returnscolor.NRGBA{}Convert(palette.go:48) guardslen(p) == 0and returnscolor.RGBA{}So the empty case returns the same transparent zero value the rest of the type already returns for "nothing to give", and the single-color case returns that color, which is what both existing endpoint guards already do for
t <= 0andt >= 1.Convertbeing guarded is not hypothetical --palette_test.go:65constructsPalette{}and passes it toConvert, so an empty palette is already an expected input to this type.CmplxPhaseAt(cmplx.go:56) forwards a[0,1]value straight intoAt.Behaviour change
Only on inputs that previously panicked. No existing test row changes.
Verification
go test -count=1 ./...passes both packages;gofmt -l .empty;go vet ./...clean. The workflow runsgo test -v ./...on Go 1.25.x/1.26.x; I ran go1.26.3.I checked the boundary from both sides rather than only confirming the new rows go green:
TestPaletteAtfails with the panic above, so the test reaches the bugn == 1ton <= 2-- the new{Palette{ColorBlack, ColorWhite}, 0.5, ...}row fails withr = 0, want 32767The two-color row is there specifically to pin that side, so the boundary is exactly 1 rather than merely "not the old behaviour".
Disclosure: AI-assisted. I found and prepared this with an AI assistant, and I ran and verified everything above myself.