Consider traits that apply to all pixel variants (where appropriate)

[antonmarsden]

I'd like to use this library to define algorithms that work for any RGB[A] pixel arrangement. However, there doesn't appear to be a generic way to achieve this without using macros. Traits could be used to provide such capability. For example:

pub trait RgbPixel<T> {
    fn red(&self) -> T;
    fn green(&self) -> T;
    fn blue(&self) -> T;
    fn set_red(&mut self, r: T);
    fn set_green(&mut self, g: T);
    fn set_blue(&mut self, b: T);

    fn rgb(&self) -> (T, T, T);
    fn set_rgb(&mut self, rgb: (T, T, T));
}

You might also have an AlphaPixel trait.

Example impl for RGBA:

/// Example impl for RGBA
impl<T : Copy> RgbPixel<T> for RGBA<T> {
    fn red(&self) -> T { self.r }
    fn green(&self) -> T { self.g }
    fn blue(&self) -> T { self.b }
    fn set_red(&mut self, r: T) { self.r = r; }
    fn set_green(&mut self, g: T) { self.g = g; }
    fn set_blue(&mut self, b: T) { self.b = b; }
    fn rgb(&self) -> (T, T, T) { (self.r, self.g, self.b) }
    fn set_rgb(&mut self, rgb: (T, T, T)) {
        self.r = rgb.0;
        self.g = rgb.1;
        self.b = rgb.2;
    }
}

I can implement this outside of the rgb crate, but was wondering whether you'd consider making it part of the core library.

[antonmarsden]

To elaborate, I could then implement functionality using the following approach:

const SRGB_LUMA: [f32; 3] = [0.2126, 0.7152, 0.0722];

pub trait LuminanceOp : RgbPixel<u8> {
    fn luminance(&self) -> u8 {
        let l = SRGB_LUMA[0] * self.red() as f32
        + SRGB_LUMA[1] * self.green() as f32
        + SRGB_LUMA[2] * self.blue() as f32;
        l as u8
    }
}

impl LuminanceOp for RGBA<u8> {}

If I wanted (hopefully) improved performance, I could try writing a more specific impl:

impl LuminanceOp for RGBA<u8> {
    fn luminance(&self) -> u8 {
        let l = SRGB_LUMA[0] * self.r as f32
        + SRGB_LUMA[1] * self.g as f32
        + SRGB_LUMA[2] * self.b as f32;
        l as u8
    }
}

[kornelski]

That's a problem indeed, but I'm not sure how to solve it.

What to do about the difference between types with and without alpha? I think generic code shouldn't just ignore existence of alpha.

What about channels that aren't present? set_green, etc. can't be meaningfully implemented for Gray pixels. RGB could have get_alpha(), but not set_alpha().

[antonmarsden]

The concrete structs would only support traits that were appropriate, e.g.,

• RGBA would have impls for Pixel, RgbPixel, and AlphaPixel traits
• GrayAlphaPixel would have impls for Pixel, GrayPixel, and AlphaPixel traits
• GrayPixel would have impls for Pixel, GrayPixel traits

(and so on)

Note that Pixel would cover things like iter(), and provide a way to refer to any pixel type.

Could be a big change, actually :(

[antonmarsden]

Assuming you simplify to having ComponentType = AlphaType, you may end up with something like the following. Note that it solves the get_alpha() problem by ensuring that Pixel always provides an impl of that method.

pub trait Pixel<T> {
    fn alpha() -> T;
}

pub trait RgbPixel<T> : Pixel<T> {

    fn red(&self) -> T;
    fn green(&self) -> T;
    fn blue(&self) -> T;
    fn set_red(&mut self, r: T);
    fn set_green(&mut self, g: T);
    fn set_blue(&mut self, b: T);

    fn rgb(&self) -> (T, T, T);
    fn set_rgb(&self, rgb: (T, T, T));

    fn rgba(&self) -> (T, T, T, T);
}

pub trait AlphaPixel<T> : Pixel<T> {
    fn set_alpha(&mut self, a: T);
}

pub trait RgbaPixel<T> : RgbPixel<T> + AlphaPixel<T> {
    fn set_rgba(&mut self, rgba: (T, T, T, T));
}

pub trait GrayPixel<T> : Pixel<T> {
    fn luminance(&self) -> T;
    fn set_luminance(&mut self);

    fn luminance_alpha(&self) -> (T, T);
}

pub trait GrayAlphaPixel<T> : GrayPixel<T> + AlphaPixel<T> {
    fn set_luminance_alpha(&mut self, ga: (T, T));
}

[kornelski]

But then how do you work with this? How do you implement a function that works with both RGB and RGBA?

It's a genuine question. I've been trying different things, like T: Into<RGBA> or creating traits for operations that I implement on all pixel types, but I haven't found an elegant solution yet.

[antonmarsden]

Pretty simple to implement your function - see the LuminanceOp above, which provides a default impl for the RgbPixel trait, and RgbaPixel trait extends RgbPixel. Then just need to ensure that you add the empty trait impls, e.g.,

impl LuminanceOp for RGB<u8> {}
impl LuminanceOp for RGBA<u8> {}
impl LuminanceOp for BGR<u8> {}
impl LuminanceOp for BGRA<u8> {}
...

[Diegovsky]

Hey, just adding my 2 cents: I thought about adding some layout information to avoid trait overhead.

To do this, we can think about separating the structs into 3 type classes (for now, could grow): grayscale, rgb and argb.

Since traits can now have associated constants, we cold store information about the layout in a trait, like this:
(using memoffset crate)

#![feature(const_ptr_offset_from, const_refs_to_cell)]

trait RGBComponents<T> {
    const RED: usize;
    const GREN: usize;
    const BLUE: usize;
    const COMPONENT_SIZE: usize = std::mem::size_of::<T>();
}

struct RGB<T> {
    r: T,
    g: T,
    b: T,
}
struct BGR<T> {
    b: T,
    g: T,
    r: T,
}

struct RGBA<T, A = T> {
    r: T,
    g: T,
    b: T,
    a: A,
}

use memoffset::{offset_of};

impl<T> RGBComponents<T> for RGB<T> {
    const RED:  usize = offset_of!(Self, r);
    const GREN: usize = offset_of!(Self, g);
    const BLUE: usize = offset_of!(Self, b);
}

impl<T> RGBComponents<T> for BGR<T> {
    const RED:  usize = offset_of!(Self, r);
    const GREN: usize = offset_of!(Self, g);
    const BLUE: usize = offset_of!(Self, b);
}

impl<T> RGBComponents<T> for RGBA<T> {
    const RED:  usize = offset_of!(Self, r);
    const GREN: usize = offset_of!(Self, g);
    const BLUE: usize = offset_of!(Self, b);
}

[ripytide]

One option would be:

pub trait AnyRgbaPixel<T> {
    fn r(&self) -> T;
    fn g(&self) -> T;
    fn b(&self) -> T;
    fn a(&self) -> Option<T>;
}

Then since a() optionally returns a T this trait could be implemented for both Rgb and Rgba and allow generic code. The only drawback is this wouldn't be implementable for grayscale pixels.

However, since this seems like fairly obscure use-case I am inclined to ignore it for v0.9 as it can always be added without breaking changes should more users request it.