Skip to content

fix(hcg): wrap negative hue into [0, 360) for rgb -> hcg#68

Open
spokodev wants to merge 1 commit into
colorjs:masterfrom
spokodev:fix-hcg-negative-hue
Open

fix(hcg): wrap negative hue into [0, 360) for rgb -> hcg#68
spokodev wants to merge 1 commit into
colorjs:masterfrom
spokodev:fix-hcg-negative-hue

Conversation

@spokodev

Copy link
Copy Markdown

Problem

rgb.hcg can return a hue outside its declared [0, 360) range. When red is the maximum channel and blue > green, the red sextant branch computes a negative value:

if (max === r) {
    hue = ((g - b) / chroma % 6); // negative when b > g
}
...
hue = (hue % 1); // JS modulo is sign-preserving, so this stays negative

hcg declares min: [0, 0, 0], max: [360, 100, 100], so a negative hue violates the space's own range, and it breaks the rgb -> hcg -> rgb round-trip:

const space = require('color-space');

space.rgb.hcg([255, 0, 55]);
// actual:   [ -12.94, 100, 0 ]   <- hue out of range
// expected: [ 347.06, 100, 0 ]

space.hcg.rgb(space.rgb.hcg([255, 0, 55]));
// actual:   [ 255, 0, 310 ]      <- blue channel impossible
// expected: [ 255, 0, 55  ]

The sibling spaces agree the hue should be ~347: rgb.hsi([255, 0, 55]) is 348.17 and rgb.hsp([255, 0, 55]) is 347. Only hcg returns a negative.

This affects the whole red-dominant, blue > green region of the RGB cube.

Fix

Add the same negative-hue wrap that rgb.hsv and rgb.hsl already use (if (h < 0) { h += ... }), applied here in the fractional domain before scaling to degrees:

hue = (hue % 1);
if (hue < 0) {
    hue += 1;
}

Tests

Extended the existing hcg: rgb -> hcg test with the red-dominant/blue > green case and a round-trip assertion. Both fail on master and pass with the fix. Full suite: 1353 pass, 0 fail.

When red is the maximum channel and blue > green, the red sextant
branch `((g - b) / chroma % 6)` is negative, and the final `hue % 1`
keeps the sign (JavaScript modulo is sign-preserving). The result is a
negative hue that falls outside hcg's declared [0, 360) range and breaks
the rgb -> hcg -> rgb round-trip (e.g. rgb [255, 0, 55] returned hue
-12.94 and round-tripped to a blue channel of 310).

Add the same negative-hue wrap already used by rgb -> hsv and rgb -> hsl.
@dy

dy commented Jun 30, 2026

Copy link
Copy Markdown
Member

Thanks @spokodev — this is a real bug and a clean fix. v3 already lands the identical fix: hcg.js wraps negative hues with if (hue < 0) hue += 1, and your exact case rgb [255,0,55] → hcg [347,100,0] passes (0 out-of-range hues across a full RGB sweep). I'm porting your targeted test assertion into the v3 hcg test as a regression pin, with credit. Since v3 (#67) is a full rewrite that supersedes master, this PR will be closed when v3 merges — but the fix and your test live on. 🙏

@dy dy mentioned this pull request Jun 30, 2026
dy added a commit that referenced this pull request Jul 2, 2026
… not -13)

Ports the targeted assertion from #68 (spokodev). v3's hcg.js already wraps negative hues; this pins it so it can't regress.
@spokodev

spokodev commented Jul 3, 2026

Copy link
Copy Markdown
Author

Thanks, glad it helped, and thanks for porting the test into v3 with credit. Fine to close this once v3 lands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants