fix(gif): compose sub-rectangle frames so the animation actually plays

crates/rustmotion-components/src/gif.rs

@@ -38,6 +38,131 @@ rustmotion_core::impl_traits!(Gif {
     Styled => style,
 });
 
+/// The frame's rectangle, clamped to the logical canvas.
+///
+/// A malformed GIF can declare a sub-rectangle that runs past the canvas;
+/// clamping keeps the blit in bounds instead of panicking on the slice.
+fn frame_rect(canvas_w: u32, canvas_h: u32, frame: &gif::Frame<'_>) -> (u32, u32, u32, u32) {
+    let left = frame.left.min(canvas_w as u16) as u32;
+    let top = frame.top.min(canvas_h as u16) as u32;
+    let w = (frame.width as u32).min(canvas_w.saturating_sub(left));
+    let h = (frame.height as u32).min(canvas_h.saturating_sub(top));
+    (left, top, w, h)
+}
+
+/// Composite one decoded frame onto the persistent canvas.
+///
+/// GIF transparency is an index, not a channel: the decoder emits alpha 0 for
+/// transparent pixels, and those must leave what is underneath alone — that is
+/// the whole point of the sub-rectangle encoding.
+fn blit_frame(composed: &mut [u8], canvas_w: u32, canvas_h: u32, frame: &gif::Frame<'_>) {
+    let (left, top, w, h) = frame_rect(canvas_w, canvas_h, frame);
+    for y in 0..h {
+        for x in 0..w {
+            let src = ((y * frame.width as u32 + x) * 4) as usize;
+            let Some(px) = frame.buffer.get(src..src + 4) else {
+                return; // truncated buffer: keep what we have rather than panic
+            };
+            if px[3] == 0 {
+                continue;
+            }
+            let dst = (((top + y) * canvas_w + (left + x)) * 4) as usize;
+            composed[dst..dst + 4].copy_from_slice(px);
+        }
+    }
+}
+
+/// `DisposalMethod::Background`: clear this frame's rectangle before the next.
+fn clear_rect(composed: &mut [u8], canvas_w: u32, canvas_h: u32, frame: &gif::Frame<'_>) {
+    let (left, top, w, h) = frame_rect(canvas_w, canvas_h, frame);
+    for y in 0..h {
+        let row = (((top + y) * canvas_w + left) * 4) as usize;
+        composed[row..row + (w as usize * 4)].fill(0);
+    }
+}
+
+/// One decoded GIF: full-canvas RGBA frames with their dimensions, the
+/// cumulative end time of each, and the total duration. Mirrors what
+/// `gif_cache` stores.
+type DecodedGif = (Vec<(Vec<u8>, u32, u32)>, Vec<f64>, f64);
+
+/// Decode a GIF into full-canvas RGBA frames, their cumulative end times, and
+/// the total duration.
+///
+/// Every frame after the first is usually a *sub-rectangle* holding only the
+/// pixels that changed, so frames must be composed onto a persistent canvas
+/// rather than used as images in their own right. Storing `frame.buffer` with
+/// the canvas dimensions produced a buffer shorter than `width * height * 4`;
+/// `raster_from_data` then returned `None` and the paint was skipped — which is
+/// why only the first frame ever appeared (issue #185).
+///
+/// `None` means nothing can be drawn, and the reason has already been reported.
+fn decode_composed_frames(src: &str) -> Option<DecodedGif> {
+    let file = match std::fs::File::open(src) {
+        Ok(f) => f,
+        Err(e) => {
+            if crate::warn_once_for(&format!("gif-open:{src}")) {
+                eprintln!("rustmotion: gif '{src}' could not be opened: {e}");
+            }
+            return None;
+        }
+    };
+
+    let mut options = gif::DecodeOptions::new();
+    options.set_color_output(gif::ColorOutput::RGBA);
+    let mut decoder = match options.read_info(file) {
+        Ok(d) => d,
+        Err(e) => {
+            if crate::warn_once_for(&format!("gif-decode:{src}")) {
+                eprintln!("rustmotion: gif '{src}' could not be decoded: {e}");
+            }
+            return None;
+        }
+    };
+
+    let canvas_w = decoder.width() as u32;
+    let canvas_h = decoder.height() as u32;
+
+    let mut frames: Vec<(Vec<u8>, u32, u32)> = Vec::new();
+    let mut cumulative_times: Vec<f64> = Vec::new();
+    let mut accumulated = 0.0;
+    let mut composed = vec![0u8; canvas_w as usize * canvas_h as usize * 4];
+
+    while let Ok(Some(frame)) = decoder.read_next_frame() {
+        // `Previous` disposal restores what was there before this frame, so it
+        // has to be captured before compositing.
+        let restore = (frame.dispose == gif::DisposalMethod::Previous).then(|| composed.clone());
+
+        blit_frame(&mut composed, canvas_w, canvas_h, frame);
+
+        let delay = frame.delay as f64 / 100.0;
+        let delay = if delay < 0.01 { 0.1 } else { delay };
+        accumulated += delay;
+        frames.push((composed.clone(), canvas_w, canvas_h));
+        cumulative_times.push(accumulated);
+
+        match frame.dispose {
+            gif::DisposalMethod::Background => clear_rect(&mut composed, canvas_w, canvas_h, frame),
+            gif::DisposalMethod::Previous => {
+                if let Some(prev) = restore {
+                    composed = prev;
+                }
+            }
+            // `Any` and `Keep` both leave the canvas as it stands.
+            _ => {}
+        }
+    }
+
+    if frames.is_empty() {
+        if crate::warn_once_for(&format!("gif-empty:{src}")) {
+            eprintln!("rustmotion: gif '{src}' decoded to zero frames");
+        }
+        return None;
+    }
+
+    Some((frames, cumulative_times, accumulated))
+}
+
 impl Painter for Gif {
     fn paint_content(
         &self,
@@ -51,33 +176,10 @@ impl Painter for Gif {
         let cached = if let Some(cached) = gcache.get(&self.src) {
             cached.clone()
         } else {
-            let Ok(file) = std::fs::File::open(&self.src) else {
-                return;
-            };
-
-            let mut decoder = gif::DecodeOptions::new();
-            decoder.set_color_output(gif::ColorOutput::RGBA);
-            let Ok(mut decoder) = decoder.read_info(file) else {
+            let Some(decoded) = decode_composed_frames(&self.src) else {
                 return;
             };
-
-            let gif_width = decoder.width() as u32;
-            let gif_height = decoder.height() as u32;
-
-            let mut frames: Vec<(Vec<u8>, u32, u32)> = Vec::new();
-            let mut cumulative_times: Vec<f64> = Vec::new();
-            let mut accumulated = 0.0;
-
-            while let Ok(Some(frame)) = decoder.read_next_frame() {
-                let delay = frame.delay as f64 / 100.0;
-                let delay = if delay < 0.01 { 0.1 } else { delay };
-                accumulated += delay;
-                frames.push((frame.buffer.to_vec(), gif_width, gif_height));
-                cumulative_times.push(accumulated);
-            }
-
-            let total_duration = accumulated;
-            let cached = Arc::new((frames, cumulative_times, total_duration));
+            let cached = Arc::new(decoded);
             gcache.insert(self.src.clone(), cached.clone());
             cached
         };
@@ -102,7 +204,7 @@ impl Painter for Gif {
         let img_info = ImageInfo::new(
             (gif_width as i32, gif_height as i32),
             ColorType::RGBA8888,
-            skia_safe::AlphaType::Premul,
+            skia_safe::AlphaType::Unpremul,
             None,
         );
         let row_bytes = gif_width as usize * 4;
@@ -114,3 +216,90 @@ impl Painter for Gif {
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    /// Write a 4x2 GIF: frame 0 fills the canvas red, frame 1 paints a 2x2 blue
+    /// sub-rectangle at (2,0) and keeps what is under it — the shape every
+    /// optimising encoder produces, and the one that used to render nothing.
+    fn write_two_frame_gif(path: &std::path::Path) {
+        let palette: &[u8] = &[0xFF, 0x00, 0x00, 0x00, 0x00, 0xFF];
+        let mut file = std::fs::File::create(path).expect("create gif fixture");
+        let mut encoder = gif::Encoder::new(&mut file, 4, 2, palette).expect("gif encoder");
+
+        let mut full = gif::Frame::from_indexed_pixels(4, 2, vec![0; 8], None);
+        full.delay = 10;
+        encoder.write_frame(&full).expect("write frame 0");
+
+        let mut patch = gif::Frame::from_indexed_pixels(2, 2, vec![1; 4], None);
+        patch.left = 2;
+        patch.top = 0;
+        patch.delay = 10;
+        patch.dispose = gif::DisposalMethod::Keep;
+        encoder.write_frame(&patch).expect("write frame 1");
+    }
+
+    fn px(frame: &[u8], w: u32, x: u32, y: u32) -> [u8; 4] {
+        let i = ((y * w + x) * 4) as usize;
+        frame[i..i + 4].try_into().expect("pixel")
+    }
+
+    /// The regression: an optimised GIF must yield as many drawable frames as
+    /// it has, all full-canvas, and they must differ.
+    #[test]
+    fn a_subrectangle_frame_is_composed_onto_the_previous_one() {
+        let path = std::env::temp_dir().join(format!(
+            "rustmotion_gif_{}.gif",
+            std::process::id() as u64 * 31 + 7
+        ));
+        write_two_frame_gif(&path);
+
+        let (frames, times, total) =
+            decode_composed_frames(path.to_str().expect("utf-8 path")).expect("gif must decode");
+        std::fs::remove_file(&path).ok();
+
+        assert_eq!(frames.len(), 2, "both frames must be drawable");
+        for (buf, w, h) in &frames {
+            assert_eq!(
+                buf.len(),
+                (*w as usize) * (*h as usize) * 4,
+                "every stored frame must be full-canvas, or raster_from_data \
+                 silently returns None and nothing is painted"
+            );
+        }
+        assert_ne!(frames[0].0, frames[1].0, "the two frames must differ");
+
+        // Frame 1 patches the right half and keeps the left: the whole point of
+        // composing rather than drawing the sub-rectangle alone.
+        assert_eq!(px(&frames[1].0, 4, 0, 0), [0xFF, 0x00, 0x00, 0xFF], "kept");
+        assert_eq!(
+            px(&frames[1].0, 4, 3, 0),
+            [0x00, 0x00, 0xFF, 0xFF],
+            "patched"
+        );
+
+        assert_eq!(times.len(), 2);
+        assert!((total - 0.2).abs() < 1e-9, "0.1s per frame, got {total}");
+    }
+
+    #[test]
+    fn a_missing_file_reports_instead_of_returning_nothing() {
+        let missing = std::env::temp_dir().join("rustmotion_gif_absent_xyz.gif");
+        assert!(decode_composed_frames(missing.to_str().expect("utf-8")).is_none());
+        // The warn-once slot must have been claimed — silence is the bug.
+        assert!(
+            !crate::warn_once_for(&format!("gif-open:{}", missing.to_str().expect("utf-8"))),
+            "the open failure must have reported once"
+        );
+    }
+
+    #[test]
+    fn a_frame_rect_running_past_the_canvas_is_clamped() {
+        let mut frame = gif::Frame::from_indexed_pixels(4, 4, vec![0; 16], None);
+        frame.left = 3;
+        frame.top = 3;
+        assert_eq!(frame_rect(4, 4, &frame), (3, 3, 1, 1));
+    }
+}

crates/rustmotion-components/src/lib.rs

@@ -186,7 +186,7 @@ impl<'de> Deserialize<'de> for PositionMode {
 /// frame: over a thousand times on a 1200-frame render, drowning out anything
 /// else on stderr. Keyed by the value rather than a plain `Once` so a scenario
 /// with several distinct bad values still hears about each of them.
-fn warn_once_for(value: &str) -> bool {
+pub(crate) fn warn_once_for(value: &str) -> bool {
     use std::collections::HashSet;
     use std::sync::{Mutex, OnceLock};
     static SEEN: OnceLock<Mutex<HashSet<String>>> = OnceLock::new();