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

Rethink conversion to support additional input and reusability #396

Open
Ogeon opened this issue Apr 28, 2024 · 0 comments
Open

Rethink conversion to support additional input and reusability #396

Ogeon opened this issue Apr 28, 2024 · 0 comments
Labels
definitely breaking Issues that are known before-hand to be a breaking change

Comments

@Ogeon
Copy link
Owner

Ogeon commented Apr 28, 2024

Description

This is similar to the old idea of pipelines (#197) that has gotten more relevant with the introduction of CAM16 and conversions that need additional input. It will ultimately need quite a bit of API design, but I'm writing this issue to air my ideas and keep an open discussion.

Motivation

The concept of a converter solves two problems:

  • Feeding parameters to the conversion process.
  • Pre-computing parameters, matrices, etc. to not have to repeat the process while converting a large number of values.

The first point could also be solved with an additional trait, but that doesn't solve the issue of reusability.

Design sketch

A converter would implement a trait with an input and output type. Whether they should be associated types or type parameters may change. The same with taking self as reference or by move.

trait Converter<T> {
    type Output;
    fn convert(&self, input: T) -> Self::Output;
}

struct XyzToRgb<T, S> {
    matrix: [T; 9],
    rgb_standard: PhantomData<S>,
}

impl<T, S> Converter<Xyz<S::WhitePoint, T>> for XyzToRgb<T, S>
where
    S: RgbStandard,
    // ...
{
    type Output = Rgb<S, T>;
    fn convert(&self, input: Xyz<S::WhitePoint, T>) -> Self::Output {
        // Do what FromColorUnclamped does today
    }
}

This would generally take the place of FromColorUnclamped as the base trait for conversion. We could then implement FromColorUnclamped as a default case for converters. By introducing another trait (let's call it DefaultConverterFrom), we can keep the current API intact.

trait DefaultConverterFrom<T> {
    type Converter: Converter<Self, Output = T>;

    fn new_converter() -> Self::Converter;
}

impl<T, U> FromColorUnclamped<T> for U
where
    U: DefaultConverterFrom<T>,
{
    fn from_color_unclamped(color: T) -> Self {
        Self::new_converter().convert(color)
    }
}

The tricky part is where additional input gets involved. One idea is to repeat the pattern above, but with an additional parameter.

trait ConverterFrom<T> {
    type Input;
    type Converter: Converter<Self, Output = T>;

    fn new_converter(input: Self::Input) -> Self::Converter;
}

impl<T, U> FromColorUnclampedWith<T> for U
where
    U: ConverterFrom<T>,
{
    type Input = U::Input;

    fn from_color_unclamped_with(color: T, input: Self::Input) -> Self {
        Self::new_converter(input).convert(color)
    }
}

That would double the number of conversion traits, which may be fine. It would also be nice to be able to bridge between conversions that do and those that don't require input. For example, converting from Rgb to Cam16UcsJmh would require three steps (Rgb -> Xyz -> Cam16 -> Cam16UcsJmh), but it could reasonably be expressed with a single FromColorUnclampedWith implementation. A derive macro could figure it out, as long as it's aware of the points where input is needed, and chain them in a single trait implementation.

impl<S, Wp, T> ConverterFrom<Rgb<S, T>> for Cam16<T>
where
    Xyz<Wp, T>: DefaultConverterFrom<Rgb<S, T>>,
{
    type Input = BakedParameters<T, StaticWp<Wp>>;
    type Converter = Chain<Xyz<Wp, T>::Converter, XyzToCam16<T, StaticWp<Wp>>>

    fn new_converter(input: Self::Input) -> Self::Converter { /* ... */ }
}

impl<S, T> ConverterFrom<Rgb<S, T>> for Cam16UcsJmh<T>
where
    Cam16<T>: ConverterFrom<Rgb<S, T>>,
{
    type Input = Cam16<T>::Input;
    type Converter = Chain<Cam16<T>::Converter, Cam16ToUcsJmh<T>>

    fn new_converter(input: Self::Input) -> Self::Converter { /* ... */ }
}

This is where it would be good to make a prototype.

@Ogeon Ogeon added the definitely breaking Issues that are known before-hand to be a breaking change label Apr 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
definitely breaking Issues that are known before-hand to be a breaking change
Projects
None yet
Development

No branches or pull requests

1 participant