Remove Second Generic for Alpha from pixel types.

[ripytide]

For example Rgba<T, A> would become Rgba<T>.

Justification: The only reason that the second generic is provided currently is that it allows using newtypes on the non-alpha pixel components in order to show color-space information such as Rgba<Linear<u8>, u8> or similar.

However, I think there is an alternative method of encoding the pixel color space that is easier to use, that being, the tagging method used by euclid::Point2D types. I'd suggest we do it slightly differently in that we start with the raw type:

struct Rgba<T> {
    r: T,
    g: T,
    b: T,
    a: T,
}

And then add a second type which adds tagging ability:

struct Tagged<T, U> {
    pub inner: T,
    phantom: PhantomData,
}

For an end user this would look like:

struct Linear;
type LinearRgba = Tagged<Rgba<u8>, Linear>;

Alternatively we don't have to provide the Tagged type since it is easy enough for an end-user to create as long as we document the method thoroughly enough. Then the added benefit with users implementing their own tagged structs is that due to the orphan rules they can also add their own From conversions between it and any other external pixel type.

Something Like:

struct Srgba(rgb::Rgba);

impl Deref for Srgba {
...
}

[ripytide]

It also makes implementing the Pixel trait much easier since arrays and slices can only contain a single type:

impl Pixel<T, const N: usize> for Rgba<T, A> {
fn as_components(self) -> [?; N] {
    //not possible
}

[kornelski]

This is why I suggested trait PixelAlpha: Pixel. This problem will keep coming up, because alpha is not a regular channel.

[ripytide]

Regardless of whether we separate alpha pixels into a AlphaPixel trait, do we not still want to enforce that alpha is the same type/size as the color components? And is that not best done via a single generic? I think that is the important question here.

For example, do we ever want to support Rgba<u8, f32>, or is the second generic only for allowing newtypes for color space tagging?

[kornelski]

There are applications where it's important to avoid mixing sRGB color with linear color, and processing color space vs device color space.

Newtype sucks for this, because Rust doesn't have delegation.

If you implement Deref, then you lose most of the type safety.

Admittedly, the extra A argument is not useful for storage. For example linear light pixels may be RGBA<f32, u8> or at least RGBA<u16, u8>, but due to alignment they take up the same amount of space, so it makes sense to convert alpha too for storage.

However, I have used RGBA<f32, u8> and RGBA<u16, u8> for temporary values, to perform color computation in higher precision, and leave alpha as-is.

[kornelski]

It may be necessary to keep the extra generic argument for backwards compatibility, and interoperability between 0.8 and 1.0 (allowing 0.8 to re-export 1.0 structs).

Maybe implement Pixel<T> on RGBA<T, T>? It's unfortunate that a trait must be all-or-nothing. Direct impls have the luxury of using either RGBA<T, T> or RGBA<T, A> where it fits.

[ripytide]

I think for doing calculations in higher precision it'd probably be fine to also shift alpha as well even if it didn't change when converting back, although technically this might be more expensive but it could go either way once optimized. But then backwards compatibility with 0.8 would be nice.

The other option I didn't mention at first would be to go full euclid and add an inherent tag to every pixel type:

pub struct Rgb<T, U> {
    r: T,
    g: T,
    b: T,
    tag: PhantomData<U>,
}

This way you get complete type-safety without needing Deref while still getting field access like .r. The downsides being constructing a new pixel is "harder" since you also have to pass a PhantomData. Unless you're using a fn new() constructor that is (which some types might not have #67).

I think this method has the least compromise overall atm.

[kornelski]

The tag would be nice for an opaque type, but it's just too weird and disruptive for an RGB struct. It forces a special case on regular uses.

Also it forces having a type with phantom data for individual components, because pixel.r loses type safety and now you need pixel.r() -> (T, tag) to preserve it.

[kornelski]

How about Pixel<T, Alpha = ()> for RGB<T>?

[ripytide]

The tag would be nice for an opaque type, but it's just too weird and disruptive for an RGB struct. It forces a special case on regular uses.

Normal uncaring users could use a crate-provided agnostics untagged type like euclid provides:

struct Unknown;
type RgbUnknown<T> = Rgba<T, Unknown>;

But you're right this is very messy and confusing for those who don't want tagged structs.

Also it forces having a type with phantom data for individual components, because pixel.r loses type safety and now you need pixel.r() -> (T, tag) to preserve it.

Good point I didn't think about that, though it does seem a rarer form of type-safety, since even euclid::Point doesn't provide this kind of type-safety.

How about Pixel<T, Alpha = ()> for RGB?

Then you'd have to add methods to the pixel trait for working on alpha which could still be called on types without alpha but would just be no-ops.

[ripytide]

I think a good case-study for this issue in particular is the Rgba type from image which shows that having a single type works pretty well for a lot of people.

[kornelski]

I don't know if just existence of that type says much about whether it's good or not. I've looked at some of their image processing functions, and they're not inspiring confidence.

• huerotate, despite being generic, is basically only for 8-bit RGBA. It assumes values in range 0..255, which is wrong for anything deeper than 8 bit, and 0..1 used on GPUs. It doesn't use the Rgba type despite working on 4 channels, and even avoids using Pixel trait.

• contrast applies contrast to all channels equally, which is a very questionable choice for the alpha channel.

• dithering uses a macro rather than generics.

When working with RGB pixels, code written in C style repeats the same formula for .r, .g, and .b separately, with 3 lines of the same thing. This is not ideal, because it's tedious, more verbose, and makes it possible to mix up the channels when editing manually. Even with editors that support multiple cursors it's still clumsy, and sometimes involves fighting rustfmt to keep the lines aligned.

With this crate, I try to have a single line for those formulas wherever possible (e.g. px1 + px2 rather than (px1.r + px2.r, px1.g + px2.g, px1.b + px2.b)). That's why I care about all these conversions, expansions, and handling of alpha. Alpha is always going to be special, even if you choose [T; 4] representation for the pixel. Color manipulations like brightness or saturation don't touch alpha, so it's not a mapping over all the components. Colorspace conversions (linear or color profiles) don't touch alpha either. Premultiplied alpha color is usually required for blending, which of course makes alpha even more special. Image resizing can't blend all four channels independently, if they're not premultiplied.

So when their code uses (k1, k2, k3, k4) or [u8; 4], rather than Rgba or the Pixel trait, I think that as a sign that their pixel types/traits are not good enough.

[ripytide]

I agree with that, conflating alpha with non-alpha components is more often than not undesirable, hence the map_c() and map_alpha() methods for separating their operations. My concern is that Rgba<T, A> where A != T will be treated a second-class citizen since it won't be able to implement many of the other methods such as map().

Wait, I suppose this is an advantage (if you know what you are doing) as you can prevent calls to map() or px + px, while still allowing map_c() and map_alpha() calls.

Ahh Okay, I think I'm understand this now, thanks for your insight.