search

kornelski / imgref

A trivial Rust struct for interchange of pixel buffers with width, height & stride
https://lib.rs/crates/imgref
Apache License 2.0
59 stars 6 forks source link

Make structs derive more traits #6

Closed yoanlcq closed 6 years ago

yoanlcq commented 6 years ago

Self-explanatory; I suspect there was a reason for not deriving that many traits in the first place, but it's way more convenient from my point of view as an API writer that use imgref.

Deriving Debug for Img might not be such a good idea though; Wouldn't it flood the screen ? x) Either way, thanks in advance for considering this PR.

kornelski commented 6 years ago

width/height is not allowed to be 0, so Default breaks that invariant.

kornelski commented 6 years ago

Debug definitely makes sense. Hash/Eq seems expensive. Do you have a use-case for using images as a key?

yoanlcq commented 6 years ago

Whoops! You're totally right about Default. Will fix.

I, personnally and right now, don't need Hash/Eq, however AFAIK there is no cost to deriving as long as no one is using these. I feel the right question to ask is rather "who are we to decide that we're forbidding this on behalf of other people who, in turn, might have use cases for these?". If people want to compare images for equality, I feel like we shouldn't get in their way. Performance footguns are not unsafe, and if Img doesn't satisfy the user's needs, they'll write their own, just as I did before learning about this crate.

Essentially: Vec derives Hash and Eq. ImgVec should as well. IMHO. :)

yoanlcq commented 6 years ago

(Also I must confess I find it weird that width and height are not allowed to be zero; There is such a thing as empty Vecs and slices... But it's out of scope for this PR).

kornelski commented 6 years ago

Makes sense, thanks!