Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Frame blending/disposal in GIF decoder branches for every pixel #2089

Open
Shnatsel opened this issue Jan 9, 2024 · 0 comments
Open

Frame blending/disposal in GIF decoder branches for every pixel #2089

Shnatsel opened this issue Jan 9, 2024 · 0 comments
Labels
kind: slow Not wrong, but still unusable

Comments

@Shnatsel
Copy link
Contributor

Shnatsel commented Jan 9, 2024

This happens in image v0.24.7 as well as the latest git on commit 96b8cee

The following function is currently called for every pixel in the image when decoding animated GIFs:

image/src/codecs/gif.rs

Lines 289 to 319 in 96b8cee

// blend the current frame with the non-disposed frame, then update
// the non-disposed frame according to the disposal method.
fn blend_and_dispose_pixel(
dispose: DisposalMethod,
previous: &mut Rgba<u8>,
current: &mut Rgba<u8>,
) {
let pixel_alpha = current.channels()[3];
if pixel_alpha == 0 {
*current = *previous;
}
match dispose {
DisposalMethod::Any | DisposalMethod::Keep => {
// do not dispose
// (keep pixels from this frame)
// note: the `Any` disposal method is underspecified in the GIF
// spec, but most viewers treat it identically to `Keep`
*previous = *current;
}
DisposalMethod::Background => {
// restore to background color
// (background shows through transparent pixels in the next frame)
*previous = Rgba([0, 0, 0, 0]);
}
DisposalMethod::Previous => {
// restore to previous
// (dispose frames leaving the last none disposal frame)
}
}
}

As you can see, it has non-trivial branches on every pixel, and no vectorization. Its performance could be considerably improved by branching once on the disposal method outside the loop, since the blending method is the same for every frame; and then calling the appropriate loop with no branches that operates on multiple elements at once, e.g. via slice::chunks_exact_mut.

However I have not checked how much time is spent with this function with a profiler. It might be that this function is Fast Enough ™️ (although I doubt that).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: slow Not wrong, but still unusable
Projects
None yet
Development

No branches or pull requests

1 participant