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

How to name methods for integer type conversions? #46

Open
kornelski opened this issue Dec 18, 2021 · 8 comments
Open

How to name methods for integer type conversions? #46

kornelski opened this issue Dec 18, 2021 · 8 comments

Comments

@kornelski
Copy link
Owner

When converting from 8 to 16 bits (or 16 to 32, etc.), there are two ways:

  • Change 0x12 to 0x1212. This is semantically the same color, but with more precision.

  • Change 0x12 to 0x0012. This keeps the same numeric value, but makes the numeric range bigger. This is useful for arithmetic, e.g. when you calculate sums and averages of pixel values.

What do you call these operations? Are there existing libraries/standards that have established names for these functions?

And similarly conversion the other way: 0x1234 can become either 0x12 or 0x34, depending on use-cases and interpretation of the values.

How to name these things? Can the different conversions to larger and smaller types have names that are clearly opposites of each other?

@jmaargh
Copy link

jmaargh commented Apr 1, 2023

I just got surprised by exactly the semantics of this in the current implementations of From traits in this crate.

Personally, if I would expect

let rgb8 = RGB8 { r: 0xff, g: 0x00, b: 0xff };
let rgbf: RGB<f32> = rgb8.into();
assert_eq!(rgbf.r, 1.0);
assert_eq!(rgbf.g, 0.0);
assert_eq!(rgbf.b, 1.0);

because, to me, a "colour" type has made itself semantically different from a simple (f32, f32, f32) or whatever. More generally, I'd expect all From trait implementations to be "approximately colour-preseving" as far as possible and (ideally) exist between all component types.

Of course, I recognise that there are assumptions about colour space being made here -- but they are incredibly minor. Sure, you could use f32 components with a different range than [0, 1], but that's by far a less common case. Even if I were using such an unusual colour space, I'd still expect conversion functions to assume that I wasn't (unless the types I had were explicit about the exact colour space, which this crate is not).

@kornelski
Copy link
Owner Author

Thanks for pointing that out. Scaling to unit range is yet another option.

@ripytide
Copy link
Contributor

Complexity is the enemy of a crate aiming to standardize other crates, as such I suggest we provide only the most simple implementation of the blanket impl<T, Q> From<Rgb<T>> for Rgb<Q> where Q: From<T>.

Other mappings can be trivially implemented by end-users for their particular semantics.

@kornelski
Copy link
Owner Author

  • For 0x12 to 0x0012 I propose to use From/Into. This can be a blanket trait implementation, and just lean on Rust's naming.
  • For 0x12 to 0x1212 I propose to use the term extend, a bit like sign-extend.

@kornelski
Copy link
Owner Author

More naming questions:

  • 0x1234 to 0x12. This will likely be used to convert high-precision 16-bit math (essentially 8.8 fixed point) to 8-bit pixels.

  • 0x1234 to 0x34. This may be used when averaging colors requires having larger temporary representation when summing them up, but going back to smaller type after division.

@ripytide
Copy link
Contributor

ripytide commented May 23, 2024

For 0x12 to 0x1212 I propose to use the term extend, a bit like sign-extend.

This seems contrary to this crates purpose in being agnostic on semantics, by providing an abstraction on a type of non-trivial conversion we are breaking our agnosticism.

0x1234 to 0x12. This will likely be used to convert high-precision 16-bit math (essentially 8.8 fixed point) to 8-bit pixels.

This also seems to be extrapolating to particular usages, something this crate is supposed to be agnostic about, I would suggest we explicitly refuse to name and provide any such conversions.

0x1234 to 0x34. This may be used when averaging colors requires having larger temporary representation when summing them up, but going back to smaller type after division.

I'm not sure I follow this one, would this not then count as an overflow since the division should happen in the larger type?

@kornelski
Copy link
Owner Author

This crate is explicitly for RGB pixels. I don't get the sense that it should be completely agnostic. Generic parameter types allow customization for special cases, but in practice the overwhelming majority of uses will be specifically on u8 and u16 types, and f32 in some domains.

I suppose the core problem here is that a specific integer truncation gets in the way of having all methods on one Pixel trait?

@ripytide
Copy link
Contributor

ripytide commented May 24, 2024

True, perhaps if these specific conversions are really required we could offer them under an optional feature per conversion trait:

  • an Extend<T> trait for the extend() conversion with an extend crate-feature.
  • a Truncate<T> trait for the truncating conversion with a truncate crate-feature.

This way regular users don't have to be exposed to these methods unless they opt-in, also it is infinitely-expandable to any number of types of conversion. And it can have specific implementations between the different integer types unlike the blanket Pixel trait which wouldn't be possible as you mentioned.

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

No branches or pull requests

3 participants