-
Notifications
You must be signed in to change notification settings - Fork 19
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
Remove Second Generic for Alpha from pixel types. #79
Comments
It also makes implementing the impl Pixel<T, const N: usize> for Rgba<T, A> {
fn as_components(self) -> [?; N] {
//not possible
} |
This is why I suggested |
Regardless of whether we separate alpha pixels into a For example, do we ever want to support |
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 However, I have used |
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 |
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 The other option I didn't mention at first would be to go full pub struct Rgb<T, U> {
r: T,
g: T,
b: T,
tag: PhantomData<U>,
} This way you get complete type-safety without needing I think this method has the least compromise overall atm. |
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 |
How about |
Normal uncaring users could use a crate-provided agnostics untagged type like 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.
Good point I didn't think about that, though it does seem a rarer form of type-safety, since even
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. |
I think a good case-study for this issue in particular is the |
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.
When working with RGB pixels, code written in C style repeats the same formula for With this crate, I try to have a single line for those formulas wherever possible (e.g. So when their code uses |
I agree with that, conflating alpha with non-alpha components is more often than not undesirable, hence the Wait, I suppose this is an advantage (if you know what you are doing) as you can prevent calls to Ahh Okay, I think I'm understand this now, thanks for your insight. |
For example
Rgba<T, A>
would becomeRgba<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:And then add a second type which adds tagging ability:
For an end user this would look like:
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 ownFrom
conversions between it and any other external pixel type.Something Like:
The text was updated successfully, but these errors were encountered: