Skip to content

Error in colorspace conversion from RGBA to HSLA #4382

Closed

Description

@fgiordana

Bevy version

commit 5e6400a

Operating system & version

macOS 12.2.1 on MBP 2021, Apple M1 Max

What you did

#[cfg(test)]
mod test {
    use bevy::prelude::*;

    const EPSILON: f32 = 0.0001;
    fn are_equal(a: &[f32; 4], b: &[f32; 4]) -> bool {
        for i in 0..4 {
            if (a[i] - b[i]).abs() > EPSILON {
                return false;
            }
        }
        true
    }

    #[test]
    fn test_hsl() {
        for r in 0..255 {
            for g in 0..255 {
                for b in 0..255 {
                    let c_rgba = Color::rgba(r as f32 / 255.0, g as f32 / 255.0, b as f32 / 255.0, 1.0);
                    let c_hsla = c_rgba.as_hsla();
                    if !are_equal(&(c_rgba.as_rgba_f32()), &(c_hsla.as_rgba_f32())) {
                        panic!("colors are different: {:?}, {:?}\n{:?}, {:?}", c_rgba, c_hsla, c_rgba.as_rgba_f32(), c_hsla.as_rgba_f32());
                    }
                }
            }
        }
    }
}

What you expected to happen

The test should be successful. There is an error in the conversion formula when the R and G components are the same

What actually happened

thread 'scene::test::test_hsl' panicked at 'colors are different: Rgba { red: 0.003921569, green: 0.003921569, blue: 0.0, alpha: 1.0 }, Hsla { hue: 240.0, saturation: 1.0, lightness: 0.0019607844, alpha: 1.0 }
[0.003921569, 0.003921569, 0.0, 1.0], [2.3283064e-10, 2.3283064e-10, 0.0039215684, 1.0]'

Additional information

There is an error in the conversion formula, it should actually be:

/// converts a color in sRGB space to HLS space
#[inline]
pub fn nonlinear_srgb_to_hsl([red, green, blue]: [f32; 3]) -> (f32, f32, f32) {
    // https://en.wikipedia.org/wiki/HSL_and_HSV#From_RGB
    let x_max = red.max(green.max(blue));
    let x_min = red.min(green.min(blue));
    let chroma = x_max - x_min;
    let lightness = (x_max + x_min) / 2.0;
    let hue = if chroma == 0.0 {
        0.0
    } else if red == x_max {
        60.0 * (green - blue) / chroma
    } else if green == x_max {
        60.0 * (2.0 + (blue - red) / chroma)
    } else {
        60.0 * (4.0 + (red - green) / chroma)
    };
    let hue = if hue < 0.0 { 360.0 + hue } else { hue };
    let saturation = if lightness <= 0.0 || lightness >= 1.0 {
        0.0
    } else {
        (x_max - lightness) / lightness.min(1.0 - lightness)
    };

    (hue, saturation, lightness)
}

Activity

  1. added
    C-BugAn unexpected or incorrect behavior
    S-Needs-TriageThis issue needs to be labelled
    on Mar 31, 2022
  2. added
    A-RenderingDrawing game state to the screen
    and removed
    S-Needs-TriageThis issue needs to be labelled
    on Mar 31, 2022
  3. added a commit that references this issue on Apr 4, 2022
    dbd5e7a
  4. added a commit that references this issue on Jun 7, 2022
    6a09ddd
  5. added a commit that references this issue on Feb 1, 2023
    52e8d20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-RenderingDrawing game state to the screenC-BugAn unexpected or incorrect behavior

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions