diff --git a/src/animation.rs b/src/animation.rs index b8c3ac8de7..c3b4bbc87d 100644 --- a/src/animation.rs +++ b/src/animation.rs @@ -1,4 +1,5 @@ use std::cmp::Ordering; +use std::num::NonZeroU32; use std::time::Duration; use crate::error::ImageResult; @@ -70,7 +71,7 @@ impl Frame { #[must_use] pub fn new(buffer: RgbaImage) -> Frame { Frame { - delay: Delay::from_ratio(Ratio { numer: 0, denom: 1 }), + delay: Delay::ZERO, left: 0, top: 0, buffer, @@ -125,22 +126,51 @@ impl Frame { } impl Delay { + /// A delay of zero. In other words, no delay. + pub const ZERO: Self = Delay { + ratio: Ratio::from_whole(0), + }; + + /// Creates a new `Delay` from the specified number of whole milliseconds. + #[must_use] + pub fn from_millis(millis: u32) -> Self { + Self { + ratio: Ratio::from_whole(millis), + } + } + + /// Returns the total number of whole milliseconds contained by this `Delay`. + #[must_use] + pub fn as_millis(self) -> u32 { + self.ratio.numer / self.ratio.denom + } + /// Create a delay from a ratio of milliseconds. /// + /// # Panics + /// + /// Panics if the denominator is zero. + /// /// # Examples /// /// ``` - /// use image::Delay; - /// let delay_10ms = Delay::from_numer_denom_ms(10, 1); + /// # use image::Delay; + /// let delay_10ms = Delay::from_millis(10); + /// assert_eq!(delay_10ms, Delay::from_numer_denom_ms(10, 1)); + /// assert_eq!(delay_10ms, Delay::from_numer_denom_ms(100, 10)); + /// assert_eq!(delay_10ms, Delay::from_numer_denom_ms(30, 3)); /// ``` #[must_use] pub fn from_numer_denom_ms(numerator: u32, denominator: u32) -> Self { + let Some(denominator) = NonZeroU32::new(denominator) else { + panic!("Denominator of a delay must be non-zero"); + }; Delay { ratio: Ratio::new(numerator, denominator), } } - /// Convert from a duration, clamped between 0 and an implemented defined maximum. + /// Convert from a duration, clamped between 0 and an implementation-defined maximum. /// /// The maximum is *at least* `i32::MAX` milliseconds. It should be noted that the accuracy of /// the result may be relative and very large delays have a coarse resolution. @@ -149,10 +179,12 @@ impl Delay { /// /// ``` /// use std::time::Duration; - /// use image::Delay; + /// # use image::Delay; /// - /// let duration = Duration::from_millis(20); - /// let delay = Delay::from_saturating_duration(duration); + /// assert_eq!( + /// Delay::from_millis(20), + /// Delay::from_saturating_duration(Duration::from_millis(20)) + /// ); /// ``` #[must_use] pub fn from_saturating_duration(duration: Duration) -> Self { @@ -186,15 +218,7 @@ impl Delay { /// `from_numer_denom_ms` constructor. #[must_use] pub fn numer_denom_ms(self) -> (u32, u32) { - (self.ratio.numer, self.ratio.denom) - } - - pub(crate) fn from_ratio(ratio: Ratio) -> Self { - Delay { ratio } - } - - pub(crate) fn into_ratio(self) -> Ratio { - self.ratio + (self.ratio.numer, self.ratio.denom.get()) } /// Given some fraction, compute an approximation with denominator bounded. @@ -218,12 +242,7 @@ impl Delay { // Computes the nominator of the absolute difference between two such fractions. fn abs_diff_nom((an, ad): (u64, u64), (bn, bd): (u64, u64)) -> u64 { - let c0 = an * bd; - let c1 = ad * bn; - - let d0 = c0.max(c1); - let d1 = c0.min(c1); - d0 - d1 + (an * bd).abs_diff(bn * ad) } let exact = (u64::from(nom), u64::from(denom)); @@ -292,11 +311,9 @@ impl Delay { impl From for Duration { fn from(delay: Delay) -> Self { - let ratio = delay.into_ratio(); - let ms = ratio.to_integer(); - let rest = ratio.numer % ratio.denom; - let nanos = (u64::from(rest) * 1_000_000) / u64::from(ratio.denom); - Duration::from_millis(ms.into()) + Duration::from_nanos(nanos) + Duration::from_nanos( + (delay.ratio.numer as u64 * 1_000_000) / delay.ratio.denom.get() as u64, + ) } } @@ -308,37 +325,34 @@ const fn gcd(mut a: u32, mut b: u32) -> u32 { a } -#[derive(Copy, Clone, Debug)] -pub(crate) struct Ratio { +/// A fully reduced fraction. +/// +/// `numer` and `denom` are guaranteed to be coprime. This ensures that each fraction has a unique +/// representation. Therefore, equality is trivial. +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +struct Ratio { numer: u32, - denom: u32, + denom: NonZeroU32, } impl Ratio { #[inline] - pub(crate) fn new(numerator: u32, denominator: u32) -> Self { - assert_ne!(denominator, 0); + fn new(numerator: u32, denominator: NonZeroU32) -> Self { + let denominator = denominator.get(); let divisor = gcd(numerator, denominator); Self { numer: numerator / divisor, - denom: denominator / divisor, + denom: NonZeroU32::new(denominator / divisor).unwrap(), } } - - #[inline] - pub(crate) fn to_integer(self) -> u32 { - self.numer / self.denom - } -} - -impl PartialEq for Ratio { - fn eq(&self, other: &Self) -> bool { - self.cmp(other) == Ordering::Equal + const fn from_whole(numerator: u32) -> Self { + Self { + numer: numerator, + denom: NonZeroU32::MIN, // == 1 + } } } -impl Eq for Ratio {} - impl PartialOrd for Ratio { fn partial_cmp(&self, other: &Self) -> Option { Some(self.cmp(other)) @@ -355,9 +369,9 @@ impl Ord for Ratio { // a * d c * b let a: u32 = self.numer; - let b: u32 = self.denom; + let b: u32 = self.denom.get(); let c: u32 = other.numer; - let d: u32 = other.denom; + let d: u32 = other.denom.get(); // We cast the types from `u32` to `u64` in order // to not overflow the multiplications. @@ -368,6 +382,8 @@ impl Ord for Ratio { #[cfg(test)] mod tests { + use std::num::NonZeroU32; + use super::{Delay, Duration, Ratio}; #[test] @@ -406,7 +422,10 @@ mod tests { Duration::from_millis(0xFFFF_FFFF / 1000) + Duration::from_micros(0xFFFF_FFFF % 1000); let delay = Delay::from_saturating_duration(fine); // Funnily, 0xFFFF_FFFF is divisble by 5, thus we compare with a `Ratio`. - assert_eq!(delay.into_ratio(), Ratio::new(0xFFFF_FFFF, 1000)); + assert_eq!( + delay.ratio, + Ratio::new(0xFFFF_FFFF, NonZeroU32::new(1000).unwrap()) + ); } #[test] @@ -426,6 +445,6 @@ mod tests { assert_eq!(duration.as_millis(), 0); // Not precisely the original but should be smaller than 0. let delay = Delay::from_saturating_duration(duration); - assert_eq!(delay.into_ratio().to_integer(), 0); + assert_eq!(delay.as_millis(), 0); } } diff --git a/src/codecs/gif.rs b/src/codecs/gif.rs index dea103e67f..407c1f86c7 100644 --- a/src/codecs/gif.rs +++ b/src/codecs/gif.rs @@ -35,7 +35,7 @@ use std::num::NonZeroU32; use gif::ColorOutput; use gif::{DisposalMethod, Frame}; -use crate::animation::{self, Ratio}; +use crate::animation; use crate::color::{ColorType, Rgba}; use crate::error::{ DecodingError, EncodingError, ImageError, ImageResult, LimitError, LimitErrorKind, @@ -440,7 +440,7 @@ impl FrameInfo { height: u32::from(frame.height), disposal_method: frame.dispose, // frame.delay is in units of 10ms so frame.delay*10 is in ms - delay: animation::Delay::from_ratio(Ratio::new(u32::from(frame.delay) * 10, 1)), + delay: animation::Delay::from_millis(u32::from(frame.delay) * 10), } } } @@ -577,7 +577,7 @@ impl GifEncoder { img_frame: animation::Frame, ) -> ImageResult> { // get the delay before converting img_frame - let frame_delay = img_frame.delay().into_ratio().to_integer(); + let frame_delay = img_frame.delay().as_millis(); // convert img_frame into RgbaImage let mut rbga_frame = img_frame.into_buffer(); let (width, height) = self.gif_dimensions(rbga_frame.width(), rbga_frame.height())?; diff --git a/src/codecs/png.rs b/src/codecs/png.rs index 4f22485044..6c5035ba59 100644 --- a/src/codecs/png.rs +++ b/src/codecs/png.rs @@ -10,7 +10,7 @@ use std::io::{BufRead, Seek, Write}; use png::{BlendOp, DeflateCompression, DisposeOp}; -use crate::animation::{Delay, Ratio}; +use crate::animation::Delay; use crate::color::{ColorType, ExtendedColorType}; use crate::error::{ DecodingError, ImageError, ImageResult, LimitError, LimitErrorKind, ParameterError, @@ -242,7 +242,7 @@ fn attributes_from_info(info: &png::Info<'_>) -> DecodedImageAttributes { d => u32::from(d), }; - Delay::from_ratio(Ratio::new(num, denom)) + Delay::from_numer_denom_ms(num, denom) }); DecodedImageAttributes { diff --git a/src/codecs/webp/decoder.rs b/src/codecs/webp/decoder.rs index 4c4b13b6c0..99c96c71c3 100644 --- a/src/codecs/webp/decoder.rs +++ b/src/codecs/webp/decoder.rs @@ -89,7 +89,7 @@ impl ImageDecoder for WebPDecoder { .inner .read_frame(buf) .map_err(ImageError::from_webp_decode)?; - Some(Delay::from_numer_denom_ms(delay, 1)) + Some(Delay::from_millis(delay)) } else { self.inner .read_image(buf) diff --git a/src/io/image_reader_type.rs b/src/io/image_reader_type.rs index 9405c10b10..10da0ea76e 100644 --- a/src/io/image_reader_type.rs +++ b/src/io/image_reader_type.rs @@ -833,8 +833,6 @@ impl<'stream> ImageReader<'stream> { SequenceControl::None => return None, } - let no_delay = Delay::from_saturating_duration(Default::default()); - let mut frame = DynamicImage::default(); let frame_decoded = match self.decode_to_dynimage(&mut frame) { Ok(frame) => frame, @@ -844,7 +842,7 @@ impl<'stream> ImageReader<'stream> { let x = frame_decoded.attributes().x; let y = frame_decoded.attributes().y; - let delay = frame_decoded.attributes().delay.unwrap_or(no_delay); + let delay = frame_decoded.attributes().delay.unwrap_or(Delay::ZERO); let frame = frame.into_rgba8(); let frame = Frame::from_parts(frame, x, y, delay);