Skip to content

Commit

Permalink
fn clip: perf: Remove an assert! (#572)
Browse files Browse the repository at this point in the history
This removes an `unsafe {}` and downgrades the `assert!` to a
`debug_assert!` by using `ToPrimitive`, as the `assert!` is not always
optimized out.
  • Loading branch information
kkysen authored Nov 21, 2023
2 parents a8d6823 + 8a2f024 commit 1942173
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 11 deletions.
2 changes: 1 addition & 1 deletion include/common/bitdepth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ pub trait BitDepth: Clone + Copy {

fn iclip_pixel<T>(&self, pixel: T) -> Self::Pixel
where
T: Copy + Ord + TryInto<Self::Pixel>,
T: Copy + Ord + TryInto<Self::Pixel> + ToPrimitive<Self::Pixel>,
Self::Pixel: Into<T>,
{
clip(pixel, 0.into(), self.bitdepth_max())
Expand Down
21 changes: 11 additions & 10 deletions include/common/intops.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use crate::include::common::attributes::clz;
use crate::include::common::attributes::clzll;
use crate::include::common::bitdepth::AsPrimitive;
use crate::include::common::bitdepth::ToPrimitive;
use std::ffi::c_int;
use std::ffi::c_uint;
use std::ffi::c_ulonglong;
Expand All @@ -11,30 +13,29 @@ use std::ffi::c_ulonglong;
#[inline]
pub fn clip<T, U>(v: T, min: U, max: U) -> U
where
T: Copy + Ord + TryInto<U>,
T: Copy + Ord + TryInto<U> + ToPrimitive<U>,
U: Copy + Ord + Into<T>,
{
assert!(min <= max);
debug_assert!(min <= max);
if v < min.into() {
min
} else if v > max.into() {
max
} else {
let v = v.try_into();
// # Safety
// `min <= v <= max`, `min: U`, `max: U`,
// and for all `u: U`, `u.into().try_into() == Ok(u)`,
// so `v` must be in `U`, too.
//
// Note that `v.try_into().unwrap()` is not always optimized out.
unsafe { v.unwrap_unchecked() }
// We use `.as_()`, a truncating cast, here instead of
// `v.try_into().unwrap()`, which doesn't always optimized out,
// or `unsafe { v.try_into().unwrap_unchecked() }`, which is unsafe
// and depends on correct `{Try,}{From,Into}` `impl`s
// and `assert!(min <= max)`, which may not always get optimized out.
v.as_()
}
}

#[inline]
pub fn clip_u8<T>(v: T) -> u8
where
T: Copy + Ord + From<u8> + TryInto<u8>,
T: Copy + Ord + From<u8> + TryInto<u8> + ToPrimitive<u8>,
{
clip(v, u8::MIN, u8::MAX)
}
Expand Down

0 comments on commit 1942173

Please sign in to comment.