Skip to content

Fix NaN okhsl/okhsv values when lightness approximates 0 or 1#24691

Merged
alice-i-cecile merged 6 commits into
bevyengine:mainfrom
beicause:fix-okhsl-okhsv-nan-lightness-0-1
Jun 22, 2026
Merged

Fix NaN okhsl/okhsv values when lightness approximates 0 or 1#24691
alice-i-cecile merged 6 commits into
bevyengine:mainfrom
beicause:fix-okhsl-okhsv-nan-lightness-0-1

Conversation

@beicause

@beicause beicause commented Jun 21, 2026

Copy link
Copy Markdown
Member

Objective

Fixes the issues found in #24678 (comment) because LinearRgba::new(1.0, 1.0, 0.9999996, 1.0) converted to Okhsla { hue: 104.03624, saturation: NaN, lightness: 1.0, alpha: 1.0 } which contains NaN.

This happens when oklab_l == 0 or 1 while C != 0, e.g. white minus epsilon

Solution

Early return okhsl/okhsv when oklab lightness approximates 0 or 1.

Testing

Added more color tests.

With this fix the okhsl lightness slider looks correct:
屏幕截图_20260621_135530

@beicause beicause force-pushed the fix-okhsl-okhsv-nan-lightness-0-1 branch 2 times, most recently from 336af6e to b14b5bb Compare June 21, 2026 05:50
@beicause beicause changed the title Fix NaN and arbitrary okhsl/okhsv values when lightness approximates 0 or 1 Fix NaN okhsl/okhsv values when lightness approximates 0 or 1 Jun 21, 2026
@beicause beicause force-pushed the fix-okhsl-okhsv-nan-lightness-0-1 branch from a75b37f to 86d1a53 Compare June 21, 2026 13:49
@beicause beicause force-pushed the fix-okhsl-okhsv-nan-lightness-0-1 branch from 86d1a53 to 3ef7846 Compare June 21, 2026 14:04
@kfc35 kfc35 added C-Bug An unexpected or incorrect behavior D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward A-Color Color spaces and color math labels Jun 21, 2026

@kfc35 kfc35 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This happens when oklab_l == 0 or 1 while C != 0,

May be a stupid question, but I noticed you also changed the if to check whether C == 0. Would that suffice instead, meaning that you won’t have to early out for oklab_l == 0 or 1? Or is it that just an unrelated clarification

@kfc35 kfc35 added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jun 22, 2026
@beicause

beicause commented Jun 22, 2026

Copy link
Copy Markdown
Member Author

This happens when oklab_l == 0 or 1 while C != 0,

May be a stupid question, but I noticed you also changed the if to check whether C == 0. Would that suffice instead, meaning that you won’t have to early out for oklab_l == 0 or 1? Or is it that just an unrelated clarification

Both are needed.

We already checked C isn't 0. I just removed f32::EPSILON as it's arbitrary and doesn't have a good reason.

@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jun 22, 2026
Merged via the queue into bevyengine:main with commit 4516a9f Jun 22, 2026
40 checks passed
@beicause beicause deleted the fix-okhsl-okhsv-nan-lightness-0-1 branch June 23, 2026 04:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Color Color spaces and color math C-Bug An unexpected or incorrect behavior D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants