Fix encoding of transparency

gifski-api/Cargo.toml

@@ -10,7 +10,7 @@ license = "AGPL-3.0+"
 name = "gifski"
 readme = "README.md"
 repository = "https://github.com/ImageOptim/gifski"
-version = "1.6.3"
+version = "1.6.4"
 autobins = false
 edition = "2018"
 
@@ -23,9 +23,9 @@ gifsicle = { version = "1.92.5", optional = true }
 clap = "2.34.0"
 gif = "0.11.3"
 gif-dispose = "3.1.1"
-imagequant = "4.0.0-beta.6"
+imagequant = "4.0.0-beta.7"
 imgref = "1.9.1"
-lodepng = "3.4.7"
+lodepng = "3.5.0"
 pbr = "1.0.4"
 resize = "0.7.2"
 rgb = "0.8.31"

gifski-api/src/lib.rs

@@ -140,6 +140,7 @@ struct DiffMessage {
     dispose: gif::DisposalMethod,
     image: ImgVec<RGBA8>,
     importance_map: Vec<u8>,
+    needs_transparency: bool,
 }
 
 /// Frame post quantization, before remap
@@ -299,32 +300,41 @@ impl Writer {
     /// Avoids wasting palette on pixels identical to the background.
     ///
     /// `background` is the previous frame.
-    fn quantize(image: ImgRef<'_, RGBA8>, importance_map: &[u8], has_prev_frame: bool, settings: &Settings) -> CatResult<(Attributes, QuantizationResult, Image<'static>)> {
+    fn quantize(image: ImgVec<RGBA8>, importance_map: &[u8], first_frame: bool, needs_transparency: bool, prev_frame_keeps: bool, settings: &Settings) -> CatResult<(Attributes, QuantizationResult, Image<'static>)> {
         let mut liq = Attributes::new();
         if settings.fast {
-            liq.set_speed(10);
+            liq.set_speed(10)?;
         }
-        let quality = if has_prev_frame {
+        let quality = if !first_frame {
             settings.color_quality().into()
         } else {
             100 // the first frame is too important to ruin it
         };
-        liq.set_quality(0, quality);
-        let mut img = liq.new_image_stride(image.buf(), image.width(), image.height(), image.stride(), 0.)?;
-        img.set_importance_map(importance_map)?;
-        if has_prev_frame {
-            img.add_fixed_color(RGBA8::new(0, 0, 0, 0));
+        liq.set_quality(0, quality)?;
+        let (buf, width, height) = image.into_contiguous_buf();
+        let mut img = liq.new_image(buf, width, height, 0.)?;
+        // only later remapping tracks which area has been damanged by transparency
+        // so for previous-transparent background frame the importance map may be invalid
+        // because there's a transparent hole in the background not taken into account,
+        // and palette may lack colors to fill that hole
+        if first_frame || prev_frame_keeps {
+            img.set_importance_map(importance_map)?;
+        }
+        // first frame may be transparent too, so it's not just for diffs
+        if needs_transparency {
+            img.add_fixed_color(RGBA8::new(0, 0, 0, 0))?;
         }
         let res = liq.quantize(&mut img)?;
         Ok((liq, res, img))
     }
 
     fn remap(liq: Attributes, mut res: QuantizationResult, mut img: Image<'static>, background: Option<ImgRef<'_, RGBA8>>, settings: &Settings) -> CatResult<(ImgVec<u8>, Vec<RGBA8>)> {
         if let Some(bg) = background {
-            img.set_background(liq.new_image_stride(bg.buf(), bg.width(), bg.height(), bg.stride(), 0.)?)?;
+            let (buf, width, height) = bg.to_contiguous_buf();
+            img.set_background(liq.new_image(buf, width, height, 0.)?)?;
         }
 
-        res.set_dithering_level((settings.quality as f32 / 50.0 - 1.).max(0.));
+        res.set_dithering_level((settings.quality as f32 / 50.0 - 1.).max(0.))?;
 
         let (pal, pal_img) = res.remapped(&mut img)?;
         debug_assert_eq!(img.width() * img.height(), pal_img.len());
@@ -407,7 +417,7 @@ impl Writer {
 
     fn make_diffs(mut inputs: OrdQueueIter<DecodedImage>, quant_queue: Sender<DiffMessage>, settings: &Settings) -> CatResult<()> {
         let (first_frame, first_frame_pts) = inputs.next().transpose()?.ok_or(Error::NoFrames)?;
-        let mut prev_frame_pts = f64::NAN;
+        let mut prev_frame_pts = 0.;
 
         let mut denoiser = Denoiser::new(first_frame.width(), first_frame.height(), settings.quality);
 
@@ -469,9 +479,8 @@ impl Writer {
                     // shifts the whole anim and is the delay of the last frame
                     pts + first_frame_pts
                 } else {
-                    debug_assert!(prev_frame_pts.is_finite());
                     // otherwise assume steady framerate
-                    pts + (pts - prev_frame_pts)
+                    (pts + (pts - prev_frame_pts)).max(1./100.)
                 };
                 debug_assert!(end_pts > 0.);
                 prev_frame_pts = pts;
@@ -499,6 +508,7 @@ impl Writer {
                 dispose,
                 importance_map,
                 ordinal_frame_number,
+                needs_transparency: ordinal_frame_number > 0 || (ordinal_frame_number == 0 && first_frame_has_transparency),
                 image,
                 end_pts,
             })?;
@@ -510,7 +520,7 @@ impl Writer {
     fn quantize_frames(inputs: Receiver<DiffMessage>, remap_queue: Sender<RemapMessage>, settings: &Settings) -> CatResult<()> {
         let mut prev_frame_keeps = false;
         let mut consecutive_frame_num = 0;
-        while let Some(DiffMessage {mut image, end_pts, dispose, ordinal_frame_number, mut importance_map}) = inputs.recv().ok() {
+        while let Some(DiffMessage {mut image, end_pts, dispose, ordinal_frame_number, needs_transparency, mut importance_map}) = inputs.recv().ok() {
             if !prev_frame_keeps || importance_map.iter().any(|&px| px > 0) {
 
                 if prev_frame_keeps {
@@ -522,7 +532,7 @@ impl Writer {
                     }
                 }
 
-                let (liq, remap, liq_image) = Self::quantize(image.as_ref(), &importance_map, ordinal_frame_number > 1, settings)?;
+                let (liq, remap, liq_image) = Self::quantize(image, &importance_map, consecutive_frame_num == 0, needs_transparency, prev_frame_keeps, settings)?;
                 let max_loss = settings.gifsicle_loss();
                 for imp in &mut importance_map {
                     // encoding assumes rgba background looks like encoded background, which is not true for lossy