Skip to content

Conversation

@ickshonpe
Copy link
Contributor

@ickshonpe ickshonpe commented Jul 9, 2024

Problem

Division by zero in crates/bevy_color/src/hsva.rs when blackness is 1:

impl From<Hwba> for Hsva {
    fn from(
        Hwba {
            hue,
            whiteness,
            blackness,
            alpha,
        }: Hwba,
    ) -> Self {
        // Based on https://en.wikipedia.org/wiki/HWB_color_model#Conversion
        let value = 1. - blackness;
        let saturation = 1. - (whiteness / value);

        Hsva::new(hue, saturation, value, alpha)
    }
}

Solution

With Hsva colors if the value component is set to 0. the output will be pure black regardless of the values of the hue or saturation components.

So if value is 0, we don't need to calculate a saturation value and can just set it to 0:

impl From<Hwba> for Hsva {
    fn from(
        Hwba {
            hue,
            whiteness,
            blackness,
            alpha,
        }: Hwba,
    ) -> Self {
        // Based on https://en.wikipedia.org/wiki/HWB_color_model#Conversion
        let value = 1. - blackness;
        let saturation = if value != 0. {
            1. - (whiteness / value)
        } else {
            0.
        };

        Hsva::new(hue, saturation, value, alpha)
    }
}

@ickshonpe ickshonpe changed the title Change the Hwba to Hsva conversion so it doesn't divide by zero Prevent division by zero in HWBA to HSVA conversion Jul 9, 2024
@ickshonpe ickshonpe added C-Bug An unexpected or incorrect behavior A-Color Color spaces and color math labels Jul 9, 2024
@ickshonpe ickshonpe changed the title Prevent division by zero in HWBA to HSVA conversion Prevent division by zero in HWBA to HSVA conversions Jul 9, 2024
@alice-i-cecile alice-i-cecile added this to the 0.15 milestone Jul 9, 2024
Copy link

@Bergerserkkeri Bergerserkkeri left a comment

Choose a reason for hiding this comment

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

Seems good to me

@janhohenheim janhohenheim added D-Trivial Nice and easy! A great choice to get started with Bevy 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 17, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jul 22, 2024
Merged via the queue into bevyengine:main with commit 453e0e4 Jul 22, 2024
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-Trivial Nice and easy! A great choice to get started with Bevy 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.

5 participants