Oklch to Srgb seems to be not implemented

Diablo-D3 commented 2 years ago

Conversion to Srgb from Oklch is not implemented, Oklch from Srgb is

How To Reproduce

let oklch = Oklch::from_color(Srgb::new(0.5, 0.5, 0.5));
let srgb = Srgb::from_color(oklch);

Actual Outcome

xx |         let srgb = Srgb::from_color(oklch);
   |                    ^^^^^^^^^^^^^^^^ the trait `FromColorUnclamped<&Oklch>` is not implemented for `Rgb<palette::encoding::Srgb, _>`

Additional Details

Cargo.toml entry: palette = { version = "0.6.0", features = ["serializing"] }
Rust version(s): 1.48.0
Target platform: Linux

Ogeon commented 2 years ago

Hi, thanks for bringing this up. Before diving deeper into this, it looks from your error message like the input is &Oklch, so a reference. Is this the case in the actual code? Conversion is not implemented for references, and that could be the reason.

Diablo-D3 commented 2 years ago

Ahh, sorry. Throwing a * in fixed it. I'm new to rust, and I'm not quite used to how often I have to ref/deref compared to idiomatic C. I'm going to close this issue, but I wonder if it would benefit Palette to also implement conversion for references, given how an obvious use case would be colors stored in slices or hashmaps or similar.

Ogeon commented 2 years ago

No worries, it can take some time to get used to. The fact that it's an input type to a trait doesn't make it simpler. It's good that you asked, in case it was an issue with the library! Oklab and Oklch have some extra requirements, after all.

Implementing the conversion traits for references could definitely be possible. The downside is that it would need an implicit .clone() to create an ownend value, unless it's restricted to T: Copy. It's a minor issue in practice, since most people use u8 and f32 values, but experience has taught me to push decisions like copying to the caller to avoid nasty surprises.

For cases where there are stored colors that need to be modified (like an array), the next version of Palette will have some neat changes for colors behind &mut references. Including temporary and permanent in-place conversion of the whole array in many cases. No ETA, though!

Ogeon / palette