Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Strange conversion results #187

Closed
wdanilo opened this issue May 1, 2020 · 5 comments · Fixed by #188
Closed

Strange conversion results #187

wdanilo opened this issue May 1, 2020 · 5 comments · Fixed by #188

Comments

@wdanilo
Copy link

wdanilo commented May 1, 2020

Hi consider this code:

let t1 : palette::Hsla = color::Hsla::new(0.0,0.0,0.03,1.0);
println!("{:?}", palette::Srgba::from(t1));

The result we got is:

Alpha { color: Rgb { red: 0.18974826, green: 0.18974826, blue: 0.18974826, standard: PhantomData }, alpha: 1.0 }

Which is of course wrong. It doesn't look like there is a space error, as Hsla is defined with Srgb space. Moreover, palette is type safe, so I'm just cannot get what is wrong here. The correct result of converting hsla(0,0,0.03,1.0) is around rgba(0.03, 0.03, 0.03). What I'm doing wrong here?

@okaneco
Copy link
Contributor

okaneco commented May 1, 2020

Hsl, Hsv, and Hwb are in Linear space in palette. This is similar to #160 and is probably unexpected behavior. The documentation lists it as "Linear HSL color space", but for users it should probably be gamma-corrected. The spaces themselves are ambiguous about their linearity.

If you convert to LinSrgba with those spaces, it will produce the desired results.

use palette::{Hsla, LinSrgba, Srgba};

fn main() {
    let t1: Hsla = Hsla::new(0.0, 0.0, 0.03, 1.0);
    println!("Srgba: {:?}", Srgba::from(t1));
    println!("LinSrgba: {:?}", LinSrgba::from(t1));
}

resulting in

Srgba: Alpha { color: Rgb { red: 0.18974826, green: 0.18974826, blue: 0.18974826, standard: PhantomData }, alpha: 1.0 }
LinSrgba: Alpha { color: Rgb { red: 0.03, green: 0.03, blue: 0.03, standard: PhantomData }, alpha: 1.0 }

@Ogeon
Copy link
Owner

Ogeon commented May 1, 2020

Yes, exactly, this is the assumption in the current implementation of what I like to call "the RGB family". It will convert to linear RGB first, and then to sRGB, but you expected a direct conversion to sRGB.

It should be possible to lift this restriction and make them all able to represent both linear and non-linear colors, just like with the Rgb type. The workaround until then is similar to what's mentioned in #160. Converting to LinRgba and transferring the values to Srgba without converting.

For example

let t1 : palette::Hsla = color::Hsla::new(0.0,0.0,0.03,1.0);
// Linear RGB -> plain (r, g, b, a) tuple -> sRGB
let srgb1 = palette::Srgba::from_components(palette::LinRgba::from(t1).into_components());
println!("{:?}", srgb1);

@wdanilo
Copy link
Author

wdanilo commented May 1, 2020

@okaneco @Ogeon thank you so much for the replies. I believe this is unfortunate that it is not protected somehow on type level. So to better understand that, it means that if I write color::Hsla::new(0.0,0.0,0.03,1.0); then this is actually not HSLA value that I would write using any other tool like this https://www.w3schools.com/colors/colors_converter.asp , because this is a manually provided value in the linear space? If so, why the Hsla type is not named LinHsla while we have this distinction in Rgba / LinRgba?

@Ogeon
Copy link
Owner

Ogeon commented May 1, 2020

That's right. It will not match that tool with the current implementation if the tool doesn't convert via linear RGB. And it's because palette uses the wrong model. Or rather a model that's incomplete. It's locked to linear RGB. This will be fixed before the next release.

It's an old mistake that hasn't been in focus until very recently. And it's named just Hsla because it's not supposed to be a special type of HSL.

@Ogeon
Copy link
Owner

Ogeon commented May 2, 2020

I took some time to fix it today. This will work after #188 is merged and the next release is out:

let output = Srgb::from_color(Hsl::new(0.0, 0.0, 0.3));
let expected = Srgb::new(0.3, 0.3, 0.3);

assert_eq!(output, expected); // or assert_relative_eq in case of float errors

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 a pull request may close this issue.

3 participants