Skip to content

Better handling of small rays - #773

Open
facelessuser wants to merge 4 commits into
mainfrom
bugfix/raytrace-small-ray
Open

facelessuser wants to merge 4 commits into
mainfrom
bugfix/raytrace-small-ray

Conversation

@facelessuser

Copy link
Copy Markdown
Collaborator

Do not discount an axis just because distance is very small, but instead reject an entire ray if all axes have very small distances.

Fixes issue, most often with very small rays near white and black, that exited improperly causing the colors to not be correct.

A note was added that an offset of 1e-5 for anchor adjustment may be preferable as real world tests have shone that math functions on various systems may be more coarse, and produce more noise than 1e-6 can handle. In tests, it seems this adjustment is only needed for the offset related to moving the anchor closer to the gamut surface. The cutoff for ray distance in raytrace_box is currently still recommended to be 1e-6.

Issues found by @lloydk

Do not discount an axis just because distance is very small, but instead
reject an entire ray if all axes have very small distances.

Fixes issue, most often with very small rays near white and black, that
exited improperly causing the colors to not be correct.

A note was added that an offset of 1e-5 for anchor adjustment may be
preferable as real world tests have shone that math functions on various
systems may be more coarse, and produce more noise than 1e-6 can handle.
In tests, it seems this adjustment is only needed for the offset related
to moving the anchor closer to the gamut surface. The cutoff for ray
distance in `raytrace_box` is currently still recommended to be 1e-6.

Issues found by @lloydk
@netlify

netlify Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for colorjs ready!

Name Link
🔨 Latest commit 1dafc62
🔍 Latest deploy log https://app.netlify.com/projects/colorjs/deploys/6ad2687e7b4cafb02a1cf767
😎 Deploy Preview https://deploy-preview-773--colorjs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@facelessuser

Copy link
Copy Markdown
Collaborator Author

For more context, see the discussion between @lloydk and me over at this ColorAide issue: facelessuser/coloraide#529.

@lloydk

lloydk commented Oct 16, 2026

Copy link
Copy Markdown
Collaborator

I had Claude review the PR:

I checked this against near-black, near-white and just-outside-gamut sweeps in sRGB, P3 and Rec.2020, with the cast instrumented to flag any ray that lands outside the RGB cube. On main, rays land outside the cube for 351–406 near-black inputs per gamut, and almost all of them come out visibly wrong: saturated red or magenta instead of black. Near white, main also casts some rays outside the cube, but none of those results were visibly wrong. With this PR, no ray lands outside the cube in any sweep. Over 1,000,000 random colors plus the near-white sweep, the PR changes 225–267 results per gamut, and none gets worse by more than 1e-15.

Only one of the new tests fails without the fix: oklch(0.0001220703125 0.4 19.75) in sRGB. The other five pass on main too. These near-black cases fail on main and pass with this PR:

// sRGB
{
        args: ["oklch(0.0000951210968196392 0.41658416169229895 328.082845620811)"],
        expect: "rgb(0% 0% 0%)",
},
// P3
{
        args: ["oklch(0.0001220703125 0.4 19.5)"],
        expect: "color(display-p3 0 0 0)",
},
// Rec. 2020
{
        args: ["oklch(0.0001220703125 0.4 17)"],
        expect: "color(rec2020 0 0 0)",
},

Two small things:

  • The // 32-bit should use 1e-6 line is indented with spaces; the rest of the file uses tabs.
  • "impelmentation" in the new anchor comment should be "implementation".

@facelessuser

Copy link
Copy Markdown
Collaborator Author

I've updated whitespace, spelling, and added the additional tests that target the Color.js implementation more specifically. As there are a lot of things that influence the noise between different implementations, it is good to have tests that specifically target this implementation.

@lloydk

lloydk commented Oct 16, 2026

Copy link
Copy Markdown
Collaborator

LGTM

This branch has not been deployed

No deployments
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.

3 participants