kornelski / rust-rgb

struct RGB for sharing pixels between crates
https://lib.rs/rgb
MIT License
98 stars 19 forks source link

How to name methods for integer type conversions? #46

Open kornelski opened 2 years ago

kornelski commented 2 years ago

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

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 commented 1 year ago

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 commented 1 year ago

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

ripytide commented 3 months ago

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 commented 3 months ago
kornelski commented 3 months ago

More naming questions:

ripytide commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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

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.