Normalize the hue after a negative rotation - #118
Open
youdie006 wants to merge 1 commit into
Open
Conversation
Go's % keeps the sign of the dividend, so Hue produced a negative hue for a
negative change. HSLToRGB documents h as "range is from 0 to 360" and its
hueFn wraps only once, so it absorbs a hue down to -240 and returns wrong
channels below that.
adjust.Hue(src, -360) on RGB(192,128,64)
before: {192 128 0 255}
after: {192 128 64 255}
RGBToHSL already normalizes with "if h < 0 { h += 360 }" right after the
arithmetic that can go negative, and so does RGBToHSV. Hue was the only
place producing a hue without it.
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.
Huecomputed(int(h) + change) % 360. Go's%keeps the sign of the dividend, so a negativechangeyields a negative hue.HSLToRGBdocumentshas "range is from 0 to 360 degrees" (util/colormodel.go:52) and its internalhueFnwraps only once (if v < 0 { v++ }), so it absorbs a hue down to-240and returns wrong channels below that.-240is easy to cross inside the documented parameter range —Hue's own doc sayschangeis-360 to 360, so any source hue under 120 degrees reaches it.The asymmetry is visible at a full rotation, which should be identity in both directions.
adjust.HueonRGB(192,128,64):The boundary is exact: wrong at a raw hue of
-241, correct at-240.Reachable from the CLI too —
cmd/adjust.go:90binds--changewith a plainIntVarPand no range check, sobild adjust hue --change -360 in.png out.pngreturns a changed image.The precedent in this repo
RGBToHSL(util/colormodel.go:42-44) already normalizes withif h < 0 { h += 360 }immediately after the arithmetic that can go negative, andRGBToHSV(:135-137) does the same.adjust.Huewas the only place producing a hue that skipped it.Testing
TestHueFullRotationadded next to the existingTestHue, with apositiveand anegativesubtest.TestHuealready varieschangeand already asserts360is identity — it stops one short of the negative extreme. Its existing-67row passes on master because those pixels are saturated; a saturated colour clamps the out-of-range channel back onto the right value and hides this, so the new test uses desaturated samples.negativesubtest fails (0X40expected,0X0actual) andpositivepasses.% 360fails onlynegative, while dropping the outer reduction (%360 + 360) fails onlypositive, since an unreduced hue above 600 overrunshueFn's single wrap on the other side.make build,make test,make race,go vet ./...andgofmtare all clean.Every asserted channel sits exactly
0.5from its rounding boundary, so the arm64 CI leg has the maximum possible margin against float drift.I left
CHANGELOG.mdalone since this is a fix rather than a feature.This patch was found and written with Claude Code. The numbers above are verbatim from running it against master and against this branch.