Extract out Color module into a separate crate?

[ripytide]

I really like the value-based color space information you've got here with the Color enum and all the types it is made up of. I'd like to use/contribute to that type and the related color types but I'm not such that I need the Canvas type and related behavior. Would you be open to splitting the crate into two crates, one for the color-related things, and one for the Texel/storage layout things?

One of the main things I'd like to add is a method of converting between any two pixel types using this Color enum as we have discussed previously in the image crate. If all goes well I can imagine it could be quite useful for the image crate also.

[kornelski]

I could use these color definitions in the yuv crate: https://docs.rs/yuv/latest/yuv/color/index.html

[HeroicKatora]

Revive https://github.com/image-rs/image-core with this, sure.

Developing the Color type here allowed me a sort of reality-check, is it sufficiently well-defined to describe all parameters necessary for a full transformation of a buffer. This is the reason why planar descriptions are still missing, no attempt yielded very well-defined inputs that fit well into the texture sampling pipeline (the semblence of the code here to GPU execution units is quite nice). That is the only concern moving them out, that new variants added in a separate crate might accidentally miss similar design problems.

The non-exhaustive nature shouldn't be a conceptual problem; while the current conversion code isn't fallible in the API, it really should be for instance for certain invalid encodings.

With regards to the conversion implementation, I'm not so sure whether it's a good idea to use it for the pixel types. Indeed, the 'path' (as per definition in the documentation) between two color variants is far from unique. The optimal method for conversion depends a lot on the environment: whether it be SIMD availability or lookup-table caches you may have read from disk first or even offloading / gpu / apu accelleration, there may be multiple bit-for-bit equivalent solutions to that numerical problem. The type conversion however doesn't lend itself well for the opaque environment parameters you'd need to solve this decision problem efficiently and transparently, though. That's at least my thought process on avoiding some mistakes you can observe in practice in C's locale design conflicts, it seems quite analogous. Not that it needs to stop us from extracting the types, there are re-uses enabled by this which would benefit without running into such design troubles.

[ripytide]

Thanks for the insight, I'd imagine if there were multiple types of conversions we could make a trait per conversion type such as:
GpuColorConvert, ViaXyzColorConvert, SimdColorConvert and similar though to begin with I was going to start with simply routing all colors into Xyz and then back out again.

I don't have any permissions in the image-rs org but could we perhaps create a new repo specifically for the color stuff that I can contribute towards? I'm hesitant to try and put it into the image-core repo for the same reasons I described above about de-coupling different responsibilities.

Perhaps the name pixel_color would be appropriate?

[kornelski]

I've tried to tackle the conversion problem generically in the yuv crate.
It works okay-ish, but requires user to match on an enum themselves for monomorphic code.

Definitely agree that there are common pairs of formats that should have SIMD.

Handling the superset of all color formats unfortunately brings in a century worth of dead analog TV signals that have been immortalized in H.26x and AV1.

[ripytide]

I think I've got an idea for both type-level pixel conversion and enum-value-based pixel conversion in a single interface:

trait HasColorSpace {
    type ColorSpace;
}

trait IntoXyz: HasColorSpace {
    fn into_xyz(self, color_space: Self::ColorSpace);
}

trait FromXyz: HasColorSpace {
    fn from_xyz(self, color_space: Self::ColorSpace);
}

impl<T, U> ColorConvert for T
    where
        T: IntoXyz,
        U: FromXyz,
{
    fn convert(self, source_colorspace: T::ColorSpace, destination_colorspace: U::ColorSpace) -> U;
}

// Strongly-typed pixel using a type-level implementation:
pub struct Srbg(rgb::Rgb);
impl HasColorSpace for Srgb {
    // it needs no extra run-time color space information
    type ColorSpace = ();
}

// Impl for a Weakly-typed enum-based pixel:
pub struct RgbColorSpace {
        primary: Primaries,
        transfer: Transfer,
        whitepoint: Whitepoint,
        luminance: Luminance,
}
impl HasColorSpace for rgb::Rgb {
    // this type requires run-time colorspace information
    type ColorSpace = RgbColorSpace;
}

// Maybe even a generic-based Rgb type:
pub struct GenericRgb<Primary, Transfer, Whitepoint, Lumniance> {
    inner: rgb::Rgb,
    primary: PhantomData,
    transfer: PhantomData,
    whitepoint: PhantomData,
    lumniance: PhantomData,
};
impl HasColorSpace for GenericRgb {
    // it still needs no extra run-time color space information
    type ColorSpace = ();
}

// and so on and so forth...

We could even provide specialized conversions that skip Xyz (and maybe use Simd) if we use per-conversion trait implementations rather than that blanket ColorConvert trait impl.

Would this work as a unified interface for both type-level and enum-level pixel colorspaced types?

[kornelski]

How does this interface handle preparation of lookup tables? Some conversions are computationally expensive. Some like gamma change are simple lookups, and some multi-dimensional changes only have LUTs for part of the calculation.

For better SIMDification, conversion could take slices.

There's also YUV to RGB conversion that involves chroma subsampling and chroma sample position, which is painful to design API for. YUV may be planar with a padding between the lines.

[ripytide]

That's a good point, there's also the possible optimization for large enum-based color-space conversions when doing bulk amounts of the same run-time color space conversion that you don't want to keep running the match statements for each color-space parameter (like luminance) if you can assert that all your pixels have the same colorspace.

Perhaps for these scenarios such a preperation based API could also be provided that allows things like lookup tables to be implemented also?

Something like:

trait HasColorSpace {
    type ColorSpace;
}

trait FromPixel<T>: HasColorSpace {
    fn convert(pixel: T, source_colorspace: T::ColorSpace, destination_colorspace: Self::ColorSpace) -> Self;
    fn get_converter(source_colorspace: T::ColorSpace, destination_colorspace: Self::ColorSpace) -> impl FnMut(pixel: T) -> Self;
}

So for this trait convert() allows one-off conversions so wouldn't be able to create LUT but get_converter() would as it could create a LUT and use it for multiple conversions in a row.

[kornelski]

It may be best to separate color definition enums from color conversion implementations, because the implementations can be pretty messy.
Fn(T) -> U is insufficient — with subsampled YUV to RGB you need 4 luma samples per 1 chroma sample, and that's just for box scaling. If you want to implement smooth upsampling or precisely apply chroma sample location, you need also neighboring pixels.
Single-pixel function doesn't allow SIMD code that performs aligned reads or shuffles 4/8/16 pixels. Matching libjpeg-turbo chroma upsampling algorithm (the official reference for JPEG) requires knowing the scanline number, because it applies a simple dithering.

[ripytide]

Great points. Perhaps then the conversion traits should work on an image-level rather than a pixel-level which would allow using the full [Rgb<u8>] slice context which should allow using SIMD, aligned reads, and neighboring pixels? Something like:

trait HasColorSpace {
    type ColorSpace;
}

type Image<P> = Vec<P>;

trait ImagePixelConvert<P>: HasColorSpace
    where P: HasColorSpace
{
    fn convert(image: Image<P>, source_colorspace: P::ColorSpace, destination_colorspace: Self::ColorSpace) -> Image<Self>;
}

impl ImagePixelConvert<Rgb> for Yuv {
    fn convert(..) -> Image<Self> {
        //simd voodoo magic
    }
}