image-webp icon indicating copy to clipboard operation
image-webp copied to clipboard

Revert `clamp` in favor of `max(0).min(255)` in `vp8.rs`

Open okaneco opened this issue 1 year ago • 0 comments

A recent clippy lint suggests using clamp instead of manual clamping. However, using Ord::clamp produces worse vectorized code than the manual clamp.

Prefer using manual clamps in loops and especially when doing saturating truncating casts such as from i32 to u8.

https://rust.godbolt.org/z/veGzv1dPx https://rust.godbolt.org/z/sf4v6ceGM


I reported this issue upstream to clippy before it was stable (closed and moved to rustc), rustc, and today to LLVM https://github.com/rust-lang/rust/issues/125738

Not sure if it makes sense to add an allow in the lib root for this lint. clamp usually produces identical code for scalars or when the min/max aren't integer limits, so flat-out denying the method seems a little overkill.

However, I don't like having to think about whether or not a convenience method I'm using is producing non-trivially worse code 🤷

okaneco avatar Aug 20 '24 04:08 okaneco