Skip to content

Conversation

ickshonpe
Copy link
Contributor

@ickshonpe ickshonpe commented Jul 7, 2025

Objective

The false and true arguments for the select statement in lerp_hue_long are misordered, resulting in it taking the wrong hue path:

oklch-long-wrong-path

Solution

Swap the arguments around.

Also fixed another case I found during testing. The hue was interpolated even when it is undefined for one of the endpoints (for example in a gradient from black to yellow). In those cases it shouldn't interpolate, instead it should return the hue of the other end point.

Testing

I added a linear_gradient module to the testbed ui example, run with:

cargo run --example testbed_ui

In the linear gradients screen (press space to switch) it shows a column of red to yellow linear gradients. The last gradient in the column uses the OKLCH long path, which should look like this:

okchlong-red-yellow

matching the same gradient in CSS:

https://jsfiddle.net/fevshkdy/14/

if the correct hue path is chosen.

…were misordered, causing the wrong path to be chosen.

Swapped them around so that it takes the correct hue path.
@ickshonpe ickshonpe added C-Bug An unexpected or incorrect behavior A-Rendering Drawing game state to the screen A-UI Graphical user interfaces, styles, layouts, and widgets D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward D-Shaders This code uses GPU shader languages A-Color Color spaces and color math labels Jul 7, 2025
@ickshonpe ickshonpe added the M-Deliberate-Rendering-Change An intentional change to how tests and examples are rendered label Jul 7, 2025
Copy link
Member

@janhohenheim janhohenheim left a comment

Choose a reason for hiding this comment

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

Good catch! Code makes sense to me :)

@janhohenheim janhohenheim 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 Jul 7, 2025
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jul 7, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Jul 7, 2025
@janhohenheim janhohenheim added S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged and removed S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it labels Jul 7, 2025
@janhohenheim
Copy link
Member

Marking as Waiting-on-Author due to merge conflicts

@janhohenheim janhohenheim removed the S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged label Jul 7, 2025
@janhohenheim janhohenheim added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Jul 7, 2025
@alice-i-cecile alice-i-cecile enabled auto-merge July 7, 2025 22:06
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jul 7, 2025
Merged via the queue into bevyengine:main with commit 5e3927b Jul 7, 2025
32 checks passed
@github-project-automation github-project-automation bot moved this to Done in Rendering Jul 7, 2025
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 A-Rendering Drawing game state to the screen A-UI Graphical user interfaces, styles, layouts, and widgets C-Bug An unexpected or incorrect behavior D-Shaders This code uses GPU shader languages D-Straightforward Simple bug fixes and API improvements, docs, test and examples M-Deliberate-Rendering-Change An intentional change to how tests and examples are rendered S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants