From 40940d2a330b0c923481fc04fa06a852692224c4 Mon Sep 17 00:00:00 2001
From: Yuta Hinokuma <higumachan@users.noreply.github.com>
Date: Mon, 16 Sep 2024 06:29:41 +0900
Subject: [PATCH] Fixed the problem of Animated PNGs being decoded incorrectly
 (#2327)

---
 src/codecs/png.rs        | 36 +++++++++++++++++++++++++++++++-----
 src/imageops/colorops.rs |  1 +
 src/lib.rs               |  1 -
 3 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/src/codecs/png.rs b/src/codecs/png.rs
index 9258ab72da..8c95ba6954 100644
--- a/src/codecs/png.rs
+++ b/src/codecs/png.rs
@@ -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:
@@ -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.
@@ -251,6 +254,7 @@ impl<R: BufRead + Seek> ApngDecoder<R> {
             current: None,
             previous: None,
             dispose: DisposeOp::Background,
+            dispose_region: None,
             remaining,
             has_thumbnail,
         }
@@ -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();
             }
         }
 
@@ -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 {
@@ -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()))
     }
 
diff --git a/src/imageops/colorops.rs b/src/imageops/colorops.rs
index 0c746db109..765e6e247b 100644
--- a/src/imageops/colorops.rs
+++ b/src/imageops/colorops.rs
@@ -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)
diff --git a/src/lib.rs b/src/lib.rs
index 5563a7d2ee..e34d754709 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -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)]