Repository navigation
Fix various issues related to color conversion, gamut, and parsing - #71
Merged
Merged
Conversation
LUT_3D_SIZE 4194304 made size^3 wrap to 0 (mod 2^64), so valid() accepted an empty buffer and apply() read out of bounds. valid() now enforces size bounds (3D <= 512, 1D <= 65536) before the product, and the .cube parser rejects oversized headers at the source. Co-Authored-By: Claude Code <noreply@anthropic.com>
…ixes #55) The unbounded subtraction loop never terminated for inf or astronomically large hue separations (inf - range is still inf). The wrap is now bounded at 4096 turns — beyond that, or for non-finite separations, the distance collapses to the longest arc so hue comparison reports "not equal" instead of spinning forever. Co-Authored-By: Claude Code <noreply@anthropic.com>
std::clamp passes NaN through, and static_cast<uint8_t>(NaN) is UB — invisible to CI because GCC's -fsanitize=undefined omits float-cast-overflow. from_unit/from_value/clamp_to_channel now detect NaN with `x != x` (constexpr-safe in C++17) and floor it to the channel minimum. Float targets keep IEEE NaN propagation. Co-Authored-By: Claude Code <noreply@anthropic.com>
…ixes #52) The five function parsers read "/ alpha" and then discarded it with (void)av, so every rgba8/rgbaf conversion came back fully opaque. Each parser now reports the parsed alpha through an optional alpha_out sink, and the rgba8/rgbaf dispatch sites write it onto the cast result. Exact-typed parses (cielab_t etc.) keep their previous behavior via the default nullptr. Co-Authored-By: Claude Code <noreply@anthropic.com>
…ixes #53) calc()/parenthesized groups recurse once per level and evaluate_expression recurses once per operator, so ~20000 nesting levels or a ~50000-term additive chain overflowed the stack (segfault). The expression parser now keeps a shared 1024-operand budget in parse_unary — every operand passes through it — which bounds both parser recursion and AST depth. Over-budget expressions parse as invalid (nullopt) instead of crashing. Co-Authored-By: Claude Code <noreply@anthropic.com>
…<= 0 (Fixes #60) generate()/generate_max() built their sampler from a copy of the engine and never wrote the advanced state back, so every call replayed the identical palette. The engine state is now copied back after generation. Non-positive min_distance accepted every candidate, making generate_max() loop forever; the constructor and setter now fall back to the default distance for non-positive values. Co-Authored-By: Claude Code <noreply@anthropic.com>
…ixes #59) rgb()'s numeric channels are 0-255 in CSS, so writing unit floats as bare numbers produced "rgb(1.0 0 0)" which parsed back as r = 1/255. to_css_color_string(rgbaf_t) now emits percentages (0-100%), preserving the unit scale through a round-trip. Co-Authored-By: Claude Code <noreply@anthropic.com>
…and grays (Fixes #57) The saturation-mode bisection had character-identical in-gamut/out-of-gamut branches, so the gamut test never narrowed the search: lightness converged back to the original and the final cast channel-clipped the color. The in-gamut lightness range at fixed chroma/hue is an interval but not a monotonic predicate over [0, 1], so locate it with a coarse scan, then bisect from the nearest feasible sample toward the original lightness. When the full chroma is unreachable at every lightness, fall back to reducing chroma at the original lightness (hue-preserving behavior). Also: - return in-gamut colors (and grays with L in [0, 1]) unchanged instead of rebuilding them through OkLab - stop treating OkLab L as an sRGB channel value in the gray paths (gray 0.5 used to become 0.5982) - carry alpha through every OkLab rebuild (preserve.hpp and multi_colorspace.hpp); OkLab has no alpha channel, so the plain cast reset it to 1.0 Co-Authored-By: Claude Code <noreply@anthropic.com>
gamut_clip_perceptual's bisection produces linear sRGB values but stored them in rgbaf_t, which is gamma-encoded sRGB — so oklch(0.7 0.3 30) mapped to (1, 0.0976, 0.0569) instead of the CSS Color 4 reference (1, 0.3451, 0.2646). srgb.hpp already fixed the same pattern; wrap the clipped linear values in core::linear_rgbaf_t so downstream casts apply the sRGB transfer function. Add a test pinned to the CSS/color.js reference vector. Co-Authored-By: Claude Code <noreply@anthropic.com>
#48) linrgb_to_displayp3/adobergb/prophoto applied only transfer curves (after clamping to [0,1]), so every membership check passed and gamut_clip_to_gamut(c, display_p3) returned out-of-range colors unchanged. Replace them with unclamped linear-domain converters that go through XYZ with the same matrices registered in the conversion graph (Display P3 and Adobe RGB stay at D65; ProPhoto adapts D65→D50 with the Bradford cone matrices reused from algorithms/chromatic_adaptation). The wide-gamut checks now range-test the real linear values, so Rec.2020 green is correctly rejected by P3 and Adobe RGB (it remains inside ProPhoto, whose green primary is more saturated) and oklch(0.6 0.9 140) by all four gamuts. Membership tolerance for the wide gamuts widens to 2e-4: the shared 7-digit matrix coefficients put D65 white at 1.00012 in linear P3/ProPhoto, and 1e-4 would exclude white itself. Co-Authored-By: Claude Code <noreply@anthropic.com>
…p clipping (Fixes #56) Channel ranges of device-independent spaces are nominal only; the old bounds clipped real colors during conversion: - Luv u*/v* [-128,128] -> [-400,400]: sRGB red has u* = 175, ProPhoto red u* = 331, sRGB blue v* = -130. - Lab a*/b* [-128,128] -> [-400,400] and LCH C* [0,200] -> [0,400]: ProPhoto green a* = -201, C* = 263. - XYZ X/Y/Z [0,2] -> [-1,2]: Bradford adaptation of ProPhoto green (D50 -> D65) yields Z = -0.0129; clamping it to 0 corrupted every downstream device-independent space (identical round-trip error in both Lab and Luv). CSS lab() helper now sources its clamp bounds from the channel constants so they cannot drift again. Tests: reference values for sRGB primaries, wide-gamut round-trips for sRGB/P3/Rec.2020/ProPhoto, ProPhoto-green XYZ/Lab/LCH anchors, and nominal-range static_asserts updated to the new bounds. Round-trip tolerance for Luv is 5e-3 at encoded-0 baselines (float32 noise is gamma-amplified to <1/255; clip errors are >= 0.08). Co-Authored-By: Claude Code <noreply@anthropic.com>
…mped RGB (Fixes #49) Dijkstra tie-breaks favored the earlier linear_rgbf node, so equal-cost DI <-> DI routes went through linear sRGB, whose [0,1] unit clamp collapsed out-of-gamut colors: Display P3 red -> Lab -> OkLab came out as sRGB red's OkLab (0.6280, 0.2249, 0.1258 instead of 0.6486, 0.2620, 0.1449). - graph: new device_independent trait (XYZ, CIELAB, CIELCH, OkLab, OkLCh, CIELuv, xyY, Jzazbz; specialize for custom spaces). When both endpoints are device-independent, edges into any non-DI node are dropped before the search, so equal-cost ties cannot pick a clamping hop. RGB-involving routes and their pinned costs are unchanged. - functions/oklab: the XYZ <-> OkLab edge previously used a published XYZ/LMS matrix pair that disagreed with the canonical RGB-side OkLab pair at ~3e-5 in Z — invisible while DI traffic routed around it, but exposed now and amplified by Lab's cube-root to 0.02 on dark colors (nonzero cross-space Delta-E for the same color). Both edges now compose the same unclamped linear-sRGB OkLab cores. Tests: Display P3 red survives Lab/OkLab and LCh/OkLCh round trips against the XYZ reference; route static_asserts pin next_hop through the XYZ hub; OkLab-via-XYZ agrees with the canonical RGB edge (values, white point, and Lab-level agreement). Mutation-checked both ways. Co-Authored-By: Claude Code <noreply@anthropic.com>
…_t (Fixes #50) lerp and lerp_alpha_mode converted both endpoints through core::rgbaf_t, which clamps to sRGB: lerp(p3red, p3red, 0.5f) returned (0.9176, 0.2003, 0.1386) instead of (1, 0, 0). ease*, multi_lerp* and lerp_n inherited the bug through lerp. - lerp: interpolate each channel (including alpha) directly in Color's own channel space via a new details::lerp_channels helper; no hub conversion, so Display P3 / Rec. 2020 endpoints keep their gamut. Documented: hue models interpolate hue linearly — use lerp_hsl/hsv/oklch for shortest arc. - lerp_linear: new explicit "srgb-linear" option (through linear_rgbf_t) for gamma-free weighting, as offered by the issue. - lerp_alpha_mode: same own-space interpolation; only the alpha selection policy (lerp/start/end/multiply) differs from plain lerp. Tests: self-lerp of P3 red and Rec. 2020 green is an exact no-op; the P3 red/green midpoint stays at (0.5, 0.5, 0); lerp_linear(black, white, 0.5) encodes to ~0.7354; lerp_alpha_mode preserves P3 red while lerping alpha. Co-Authored-By: Claude Code <noreply@anthropic.com>
…color-mix (Fixes #51) White/black/gray carry only atan2 noise for hue — white's OkLCH hue is 90° at C ≈ 6e-8 — so every polar interpolator treated that noise as a real endpoint: lerp_oklch(white, blue, 0.5) swung through teal (H ≈ 175°, rgb(0, .77, .65)), lerp_lch through pink, and color-mix(in oklch, white, blue) produced rgb(0 197 165). Per CSS Color 4 a powerless hue (chroma/saturation ≈ 0) is missing and takes the other endpoint's hue; both missing → unchanged. - core: new shared is_achromatic(c, eps = 1e-3) checking the chroma channel against an epsilon (has_chroma_channel_v moved alongside it) - compare: delegates to the shared eps-based check (was exact zero, which missed the conversion noise entirely) - interpolate: details::substitute_powerless_hue applied in lerp_hsl, lerp_hsv, lerp_oklch, lerp_lch, lerp_chroma_preserving, and lerp_hue_locked (a powerless locked start hue falls through to the end hue instead of freezing atan2 noise) - css: mix_in_polar_space substitutes before hue-method adjustment Co-Authored-By: Claude Code <noreply@anthropic.com>
#61) Both constrained generators built an OkLab candidate, cast it to the target type — which clamps RGB — and only then called gamut_clip_preserve_lightness. The mapper saw an already-in-gamut color, returned it unchanged, and the sampled lightness was silently lost: with l_min = l_max = 0.7 the OkLab lightness was off by up to 0.103 for rgbf_t and rgb8_t, and contrast_generator's gamut_map option barely changed anything. - luminance_generator / contrast_generator: map in OkLab first, then color_cast to Color (mapping preserves L, so the constraint survives) - contrast_generator: new try_generate_n(count[, reference]) returns std::optional<std::vector<Color>> — next()/generate_n stay best-effort and can silently return colors below min_ratio when the attempt budget runs out; the batch variant reports that failure Co-Authored-By: Claude Code <noreply@anthropic.com>
…n generic lerp (Fixes #62) The generic lerp/lerp_channels treated every channel as a plain number, so hue-bearing models (hsl/hsv/oklch/lch/hwb) regressed after #50: the #51 teal bug came back, and 350 -> 10 midpointed at 180 instead of 0. Dispatch hue_tag channels through lerp_angle_deg, and run the CSS Color 4 powerless-hue substitution first when the model also has a chroma channel. RGB-family models keep #50's hub-free channel blend. This also fixes lerp_alpha_mode and the palette lerp_method::rgb paths (linear_scale, multi_scale), which all funnel through lerp_channels. Co-Authored-By: Claude Code <noreply@anthropic.com>
… once (Fixes #63) The #53 depth budget existed only in the calc() channel parser; color-mix(), light-dark(), contrast-color() and relative from() still overflowed the stack at ~20000 levels, and polar from() chains were exponential because each level resolved its origin twice (once for channels, once for alpha). Thread one shared thread_local nesting counter (cap 64) through the public parse_css_color_rgbaf/rgba8 entries and resolve_source_color, so every recursive form fails with nullopt in bounded time. Then eliminate the duplicate resolution: alpha now travels through the same single evaluation via an alpha_out parameter (hsl/hwb/lab/lch/oklab/oklch/color from), keeping the typed-channel parse for wide-gamut sources and re-parsing only atomic functions for alpha — never a second recursive evaluation. Co-Authored-By: Claude Code <noreply@anthropic.com>
…ixes #64) Follow-up to #47: the size caps landed, but on Linux std::from_chars parses "nan"/"inf", so non-finite LUT data and DOMAIN values were accepted and reached apply()'s float→size_t index cast — UB (UBSan float-cast-overflow). The Apple path already rejected them, so behavior also differed by platform. - parse_float now rejects !isfinite on every platform. - The reader requires DOMAIN_MIN < DOMAIN_MAX per channel. - clamp01 maps NaN to 0 (its first comparison is written so NaN cannot slip through), covering NaN input pixels in both apply() overloads. - fuzz_read_cube now feeds accepted LUTs through apply() with NaN/±inf/out-of- range probes, which the fuzzer previously never exercised. Co-Authored-By: Claude Code <noreply@anthropic.com>
…65) gamut_clip_to_gamut searched chroma against the correct target gamut, but rebuilt the result via color_cast<core::rgbaf_t>(oklab_t{...}), whose sRGB clamp silently re-clipped wide-gamut targets: oklch(0.6 0.9 140) clipped to Display P3 landed on the sRGB boundary (chroma 0.2052 instead of P3's 0.2275) with L drifted to 0.603. Both the gray and chromatic paths now cast the OkLab result straight to Color through details::clip_result_to, re-applying unit-space alpha — never routing through rgbaf_t. Strengthened tests assert the P3 clip result is outside sRGB, sits within tolerance of the P3 boundary (0.2275 ± 3e-3), preserves L, and keeps alpha. Co-Authored-By: Claude Code <noreply@anthropic.com>
…r integral construction (Fixes #66) hsv_to_rgb and hwb_to_rgb computed the 6-way sector with static_cast<int>(h * 6) directly on the unit hue: NaN/±inf input made the cast undefined behavior, and negative hue produced a negative i % 6 that matched no switch case (hsv(-30°) rendered black instead of the 330° sector). Both paths now normalize through details::wrap_unit_hue — non-finite hue maps to 0, finite hue wraps onto the [0, 1) ring — which also fixes unwrapped hues like hsv(390°). Direct construction of integral color types (rgb8_t{NaN, …}) cast the float argument straight to uint8_t — UB. make_storage now maps non-finite float arguments on integral channels to the channel minimum, the same policy as from_unit/from_value/clamped, and the class doc documents it. Finite values still store as-is (construction never range-validates). Verified UBSan-clean (-fsanitize=undefined,float-cast-overflow) on the issue's repro inputs. 7 new tests cover NaN/±inf/negative hue for HSV and HWB plus the construction policy; 3 mutations caught (hsv wrap removed, hwb wrap removed, raw cast restored — the last via UBSan). Co-Authored-By: Claude Code <noreply@anthropic.com>
…ns (Fixes #67) OkLab storage clipped wide-gamut and above-white values: ProPhoto blue (a ≈ -1.35, C ≈ 1.41) was clamped at ±1/1, breaking ProPhoto → OkLab → ProPhoto round-trips, and Lab L* was capped at 100 so above-white XYZ values were lost. - oklab.hpp: okl [0, 2], oka/okb [-2, 2], okc [0, 2] — nominal ranges wide enough for every supported gamut (ProPhoto blue a ≈ -1.35; XYZ (1,1,1) → L ≈ 1.0032) - cielab.hpp: lab_l [0, 200] (XYZ Y = 1.5 → L* ≈ 116.8) - is_in_gamut: compute membership through XYZ + a raw XYZ→linear-sRGB helper instead of oklab_t, so no storage range can clip a color before membership is decided (ProPhoto's own 21³ grid: 100 → 0 rejections) - random: decouple sampling domains from nominal ranges via details::sample_channel — OkLab/OkLCh, CIELAB/CIELCH, Luv and XYZ generators sample their perceptual working range (L [0,1], a/b ±0.5 / ±128, C [0,0.4]/[0,200], XYZ [0,1]) so widened storage never turns random palettes mostly out-of-gamut Co-Authored-By: Claude Code <noreply@anthropic.com>
Fixes #68) Adobe RGB's forward and inverse XYZ matrices came from different sources (the inverse even carried a transposed digit: -0.5649654), so the pair was not mutually inverse — linear white round-tripped to (0.99987, 1.00002, 1.00023) and 441 of 9261 points on Adobe's own [0,1]^3 cube were rejected as outside Adobe gamut. The transfer function also used a rounded gamma 2.2 instead of the spec exponent 563/256. - Replace the inverse in both conversion/functions/adobe_rgb.hpp and algorithms/gamut/details.hpp with the exact Gauss-Jordan inverse of the forward matrix, so the pair is mutually inverse by construction. - Use gamma 563/256 (and its exact inverse 256/563) in both transfer functions. - Precompute the Bradford D65->D50 adaptation matrix for ProPhoto membership as a constexpr constant folded from chromatic_adaptation's matrices/white points (keeps the two paths from drifting; ~8 ns -> ~3 ns per call). - Add own-cube membership tests for Adobe RGB (RED: 441 rejections) and Display P3 (characterization: 0), plus white membership, a tight linear<->XYZ round-trip pin and a spec-gamma pin. ProPhoto's own-cube rejections were already fixed by #67 (XYZ-routed membership); its grid test stays green here. Co-Authored-By: Claude Code <noreply@anthropic.com>
#69) Poisson-disk candidates were distance-checked as raw OkLab values and only converted to the output Color at the very end. For rgb8 (and any gamut- limited / quantized target) the cast clamps out-of-gamut candidates and quantizes, so raw candidates that were far apart collapsed onto identical received values: 54 of 435 pairs in a 30-color rgb8 palette were closer than min_distance 0.1, with 7 exact duplicates, and generate_max reported ~2084 "distinct" colors that collapsed after quantization. Round-trip every candidate through Color (cast to Color, map back to OkLab) before the distance check, and store/return the received values, so the documented minimum distance holds for the color the caller actually receives. generate_max also gets much faster (~673 ms -> ~8 ms in the test) because collapsing candidates now fail fast. Tests: rgb8 generate(30) and generate_max() palettes assert every received pair is >= min_distance apart in OkLab (measured on the returned rgb8 values), RED-verified pre-fix. Co-Authored-By: Claude Code <noreply@anthropic.com>
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.
No description provided.