From 8a2f024aa2ba7875ac1aa4c2f3dbac1bf5233163 Mon Sep 17 00:00:00 2001 From: Khyber Sen Date: Thu, 16 Nov 2023 01:26:30 -0800 Subject: [PATCH] `fn clip`: Remove `unsafe {}` and downgrade the `assert!` to a `debug_assert!` by using `ToPrimitive`, as the `assert!` is not always optimized out. --- include/common/bitdepth.rs | 2 +- include/common/intops.rs | 21 +++++++++++---------- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/include/common/bitdepth.rs b/include/common/bitdepth.rs index 610de3c7d..355b0f6fc 100644 --- a/include/common/bitdepth.rs +++ b/include/common/bitdepth.rs @@ -155,7 +155,7 @@ pub trait BitDepth: Clone + Copy { fn iclip_pixel(&self, pixel: T) -> Self::Pixel where - T: Copy + Ord + TryInto, + T: Copy + Ord + TryInto + ToPrimitive, Self::Pixel: Into, { clip(pixel, 0.into(), self.bitdepth_max()) diff --git a/include/common/intops.rs b/include/common/intops.rs index 7c444160d..303535dc7 100644 --- a/include/common/intops.rs +++ b/include/common/intops.rs @@ -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; @@ -11,30 +13,29 @@ use std::ffi::c_ulonglong; #[inline] pub fn clip(v: T, min: U, max: U) -> U where - T: Copy + Ord + TryInto, + T: Copy + Ord + TryInto + ToPrimitive, U: Copy + Ord + Into, { - 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(v: T) -> u8 where - T: Copy + Ord + From + TryInto, + T: Copy + Ord + From + TryInto + ToPrimitive, { clip(v, u8::MIN, u8::MAX) }