Skip to content

Commit

Permalink
Rely on defined saturation behavior for f32 to u8 casts (#87)
Browse files Browse the repository at this point in the history
This is a backport of a change from Raph made in the CPU sparse strips
experiment.

When converting from `f32` to `u8`, the code was clamping to be sure
that the values were within range. This was necessary in the past, but
hasn't been needed for some years now, since
<rust-lang/rust#10184>. This clamping is not
necessary and the compiler is not optimizing it away as of Rust 1.83 on
Apple Silicon.
  • Loading branch information
waywardmonkeys authored Dec 11, 2024
1 parent f82649b commit 9a7727a
Showing 1 changed file with 20 additions and 3 deletions.
23 changes: 20 additions & 3 deletions color/src/color.rs
Original file line number Diff line number Diff line change
Expand Up @@ -512,11 +512,13 @@ impl<CS: ColorSpace> AlphaColor<CS> {
/// Pack into 8 bit per component encoding.
#[must_use]
pub fn to_rgba8(self) -> Rgba8 {
// This does not need clamping as the behavior of a `f32` to `u8`
// cast in Rust is to saturate.
#[expect(clippy::cast_possible_truncation, reason = "deliberate quantization")]
let [r, g, b, a] = self
.convert::<Srgb>()
.components
.map(|x| (x.clamp(0., 1.) * 255.0).round() as u8);
.map(|x| (x * 255.0).round() as u8);
Rgba8 { r, g, b, a }
}
}
Expand Down Expand Up @@ -628,11 +630,13 @@ impl<CS: ColorSpace> PremulColor<CS> {
/// Pack into 8 bit per component encoding.
#[must_use]
pub fn to_rgba8(self) -> PremulRgba8 {
// This does not need clamping as the behavior of a `f32` to `u8`
// cast in Rust is to saturate.
#[expect(clippy::cast_possible_truncation, reason = "deliberate quantization")]
let [r, g, b, a] = self
.convert::<Srgb>()
.components
.map(|x| (x.clamp(0., 1.) * 255.0).round() as u8);
.map(|x| (x * 255.0).round() as u8);
PremulRgba8 { r, g, b, a }
}
}
Expand Down Expand Up @@ -828,7 +832,20 @@ impl<CS: ColorSpace> core::ops::Sub for PremulColor<CS> {

#[cfg(test)]
mod tests {
use super::{fixup_hue, HueDirection};
use super::{fixup_hue, AlphaColor, HueDirection, PremulColor, PremulRgba8, Rgba8, Srgb};

#[test]
fn to_rgba8_saturation() {
// This is just testing the Rust compiler behavior described in
// <https://github.com/rust-lang/rust/issues/10184>.
let (r, g, b, a) = (0, 0, 255, 255);

let ac = AlphaColor::<Srgb>::new([-1.01, -0.5, 1.01, 2.0]);
assert_eq!(ac.to_rgba8(), Rgba8 { r, g, b, a });

let pc = PremulColor::<Srgb>::new([-1.01, -0.5, 1.01, 2.0]);
assert_eq!(pc.to_rgba8(), PremulRgba8 { r, g, b, a });
}

#[test]
fn hue_fixup() {
Expand Down

0 comments on commit 9a7727a

Please sign in to comment.