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

Add type aliases for pixel types #119

Closed
ripytide opened this issue Jul 9, 2024 · 5 comments
Closed

Add type aliases for pixel types #119

ripytide opened this issue Jul 9, 2024 · 5 comments

Comments

@ripytide
Copy link
Contributor

ripytide commented Jul 9, 2024

From #64:

Type hints are repetitive, and other forms require turbofish.

Some more questions might be whether we provide these aliases for all pixel types or just Rgb and Rgba? and for how many integer types, u8, u16, isize? And from #25 whether we go with Rgb8 or RgbU8?

@kornelski
Copy link
Owner

For all pixel types, for consistency. I haven't seen usize/isize used. It probably doesn't appear outside of temporary calculations like pixel.map(), so it doesn't need an alias.

Just u8 would probably cover majority of uses. With u16 and f32, probably 99%.

But the naming is such a pain. RGBA8 is super common, and an old established name, everywhere outside of Rust. RgbaU8 is so unusual, that Google ignores the spelling, and DDG finds Rust crates (probably this discussion soon!)

And to make it worse, new GPU apis and shader languages have chosen to use a suffix instead, so there's RGBA32F.

So I'm torn between following Rust's naming, and having it weird for everyone else, or having a mishmash of "standard", but non-Rust names.

@ripytide
Copy link
Contributor Author

u8, u16, f32 and f64 sound good to me for aliases.

The specific naming is indeed a pain.

My one strong opinion in on RGBA vs Rgba since the latter is much nicer to use when inside a rust code-base since it looks more like a type and less like a constant.

Consistency tells me more explicit is better so RgbaU8/Rgba8U is better than Rgba8 to prevent potential future collisions such as between RgbaF16 and RgbaU16.

But I have very weak feelings about postfix vs prefix for the type letter: RgbaU8 vs Rgba8U, in fact I think I prefer the postfix for floating point but the prefix for unsigned integers which seems super odd and inconsistent: RgbaU8 and Rgba32F.

@kornelski
Copy link
Owner

Having slept on it, I think I'll keep the RGBA8 style aliases, and add RGBA32F.
This reduces code churn for existing users, and gives a choice between rusty Rgba::<u8> and industry-wide naming schemes.

@ripytide
Copy link
Contributor Author

ripytide commented Jul 13, 2024

That sounds good, strikes a balance between the two approaches.

@ripytide
Copy link
Contributor Author

This seems fixed in 30db416

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants