Skip to content

Commit

Permalink
Fixed the problem of Animated PNGs being decoded incorrectly (#2327)
Browse files Browse the repository at this point in the history
  • Loading branch information
higumachan authored Sep 15, 2024
1 parent 2f7eb0b commit 40940d2
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 6 deletions.
36 changes: 31 additions & 5 deletions src/codecs/png.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ use crate::error::{
ParameterError, ParameterErrorKind, UnsupportedError, UnsupportedErrorKind,
};
use crate::image::{AnimationDecoder, ImageDecoder, ImageEncoder, ImageFormat};
use crate::Limits;
use crate::{DynamicImage, GenericImage, ImageBuffer, Luma, LumaA, Rgb, Rgba, RgbaImage};
use crate::{GenericImageView, Limits};

// http://www.w3.org/TR/PNG-Structure.html
// The first eight bytes of a PNG file always contain the following (decimal) values:
Expand Down Expand Up @@ -229,6 +229,9 @@ pub struct ApngDecoder<R: BufRead + Seek> {
previous: Option<RgbaImage>,
/// The dispose op of the current frame.
dispose: DisposeOp,

/// The region to dispose of the previous frame.
dispose_region: Option<(u32, u32, u32, u32)>,
/// The number of image still expected to be able to load.
remaining: u32,
/// The next (first) image is the thumbnail.
Expand All @@ -251,6 +254,7 @@ impl<R: BufRead + Seek> ApngDecoder<R> {
current: None,
previous: None,
dispose: DisposeOp::Background,
dispose_region: None,
remaining,
has_thumbnail,
}
Expand Down Expand Up @@ -310,18 +314,37 @@ impl<R: BufRead + Seek> ApngDecoder<R> {
let current = self.current.as_mut().unwrap();

// Dispose of the previous frame.

match self.dispose {
DisposeOp::None => {
previous.clone_from(current);
}
DisposeOp::Background => {
previous.clone_from(current);
current
.pixels_mut()
.for_each(|pixel| *pixel = Rgba([0, 0, 0, 0]));
if let Some((px, py, width, height)) = self.dispose_region {
let mut region_current = current.sub_image(px, py, width, height);

// FIXME: This is a workaround for the fact that `pixels_mut` is not implemented
let pixels: Vec<_> = region_current.pixels().collect();

for (x, y, _) in &pixels {
region_current.put_pixel(*x, *y, Rgba::from([0, 0, 0, 0]));
}
} else {
// The first frame is always a background frame.
current.pixels_mut().for_each(|pixel| {
*pixel = Rgba::from([0, 0, 0, 0]);
});
}
}
DisposeOp::Previous => {
current.clone_from(previous);
let (px, py, width, height) = self
.dispose_region
.expect("The first frame must not set dispose=Previous");
let region_previous = previous.sub_image(px, py, width, height);
current
.copy_from(&region_previous.to_image(), px, py)
.unwrap();
}
}

Expand Down Expand Up @@ -362,6 +385,8 @@ impl<R: BufRead + Seek> ApngDecoder<R> {
}
};

self.dispose_region = Some((px, py, width, height));

// Turn the data into an rgba image proper.
limits.reserve_buffer(width, height, COLOR_TYPE)?;
let source = match self.inner.color_type {
Expand Down Expand Up @@ -404,6 +429,7 @@ impl<R: BufRead + Seek> ApngDecoder<R> {
// Ok, we can proceed with actually remaining images.
self.remaining = remaining;
// Return composited output buffer.

Ok(Some(self.current.as_ref().unwrap()))
}

Expand Down
1 change: 1 addition & 0 deletions src/imageops/colorops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,7 @@ where
}

/// Hue rotate the supplied image in place.
///
/// `value` is the degrees to rotate each pixel by.
/// 0 and 360 do nothing, the rest rotates by the given degree value.
/// just like the css webkit filter hue-rotate(180)
Expand Down
1 change: 0 additions & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,6 @@
//! [`ImageDecoderRect`]: trait.ImageDecoderRect.html
//! [`ImageDecoder`]: trait.ImageDecoder.html
//! [`ImageEncoder`]: trait.ImageEncoder.html
#![allow(redundant_explicit_links)]
#![warn(missing_docs)]
#![warn(unused_qualifications)]
#![deny(unreachable_pub)]
Expand Down

0 comments on commit 40940d2

Please sign in to comment.