Converting from a wider gamut to a smaller one should return an option

[enolan]

This is real confusing:

extern crate palette;
use palette::*;

fn main() {
    let foo = Lab {l: 1., a: -1., b: 0.};
    assert!(foo.is_valid());
    assert!(Rgb::from(foo).is_valid());
}

enolan at Behemoth in ~/mystuff/code/palettebug  
$ cargo run
     Running `target/debug/palettebug`
thread '<main>' panicked at 'assertion failed: Rgb::from(foo).is_valid()', main.rs:7
Process didn't exit successfully: `target/debug/palettebug` (exit code: 101)

I assumed it was a bug in the conversion routine, but it's just that Lab* is wider than sRGB. Returning an option type, assert!ing when the input is outside the target color space, or clamping the output value would all be preferable to the current situation.

[sidred]

Yeah the output of the conversion needs to be clamped. The example prints:

Rgb { red: -0.8116134892469634, green: 1.5417271999753697, blue: 0.9686961172534542 }

[Ogeon]

The reason for having unchecked conversion is to keep it lossless in situations where imaginary or otherwise invalid colors are acceptable for pure mathematical reasons, but I do see why this can be problematic. A simple alternative to forcing valid colors could be to provide additional conversion methods. Something like:

fn try_convert<T: From<Self> + Limited>(self) -> Option<T>;
fn convert_clamped<T: From<Self> + Limited>(self) -> T;

If we, on the other hand, would turn the table and force From conversion to be checked, what would then be the best way? Panicking isn't nice, since it may come as an unpleasant surprise, because of float inaccuracies. Returning an option will introduce an extra step where the color has to be unwrapped in some way, and suffers from the same problem as panicking. I'm also not sure if we are even allowed to implement From for Option, since we are playing with type parameters. That leaves us with clamping, which is limiting, but the colors will be within range.

A possible middle ground could be to only clamp values that are definitely invalid in almost every case, like negative RGB. That would still allow brighter than white and such, which is important in applications like 3D rendering.

[Ogeon]

Still a bit fuzzy. Adding it to the wish list for now.

[justinpombrio]

FYI, Rust nightly now has a TryFrom trait that returns a Result. From could clamp invalid colors, and TryFrom could return Err for invalid colors.

[Ogeon]

Yes, that would be perfect for this. There is still a use case for converting without clamping (in 3D rendering engines, for example), but that could perhaps get the same treatment as wrapping arithmetics and be available through some kind of non_clamping_from method (with a better name).

[Ogeon]

Seems like TryFrom and TryInto will not be stabilized for a forseeable future, so this issue will not not wait for it. Instead there will be a number of changes that implements clamping as the default conversion behavior and adds opt-in convert_unclamped and try_convert like behavior, starting with #106. It's probably the least surprising behavior for the average user.

I'll add more specific issues when I figure work tasks, but other than that it's free to come with input.

[Winseven4lyf]

If it means or changes anything, TryFrom is now stable.

[Ogeon]

That's great! I'm just not sure if it (and From/Into) are the right thing to use for this, after all. Or maybe they are, in combination with something else. The problem is that they will not be consistent when converting to a variation of the same color space. Like one RGB to another. This is because From<T> for T doesn't do anything, so there will not be any clamping if it's not a different type. It may be fine or it may be confusing. I'm not sure yet.

[Ogeon]

Just for the record, I have been working a bit on this, now and then when I haven't had other things going on. It has been tricky to get it right, but I think I have something acceptable on its way.

The idea is to have three kinds of traits (naming may change); ConvertUnclampedFrom<T>, TryConvertFrom<T> and ConvertFrom<T>, and the corresponding *Into traits. It will usually only be necessary to implement ConvertUnclampedFrom<T> + Limited and the rest are blanked implemented based on that. It should also be possible to at least derive ConvertUnclampedFrom for the standard color types.

It's still a bit unclear where From and TryFrom fits into this, because their semantics are not the same as ConvertFrom and TryConvertFrom. Converting a color to and from its own type will result in clamping with ConvertFrom, but not with From, as mentioned, and that may be too surprising in generic code (like passing an Srgb value to something like fn do_thing<T: Into<Srgb>>(color: T) may end up operating on out-of-bounds values).

[Ogeon]

There will be options, similar to what's described above, to convert with or without clamping in version 0.6. Including the Try* variants. Although, From and Into had to go, because they will cause conflicts and not go as well with the new lossy semantics of FromColor and IntoColor as they did before.