Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Context
I have been creating lots of animated charts lately and I have found that the performance of
cgif_addframe
is very slow for my use case.The GIFs I'm creating have large dimensions and many frames. Not very much changes from frame-to-frame. I enable the flag
CGIF_FRAME_GEN_USE_DIFF_WINDOW
to get small file sizes.I profiled it and found that most of the time was spent in the
cmpPixel
function. This function is used in two places:doWidthHeightOptim
. This is done when theCGIF_FRAME_GEN_USE_DIFF_WINDOW
flag is set.CGIF_GEN_KEEP_IDENT_FRAMES
is not set.Optimization
This pull request changes the library to use a faster comparison if both the current and previous frame use the global palette. When both use the global palette, it compares the pixel data directly. This can allow it to compare whole rows or the whole image with a single
memcmp
. I tried to keep to the project code style.Results
These changes make my program run 30 times faster.
Considering just the time spent in CGIF it's 80 times faster.
Questions
There are a couple of things I'm unsure about.
The first is a query about the original behavior of
cmpPixel
. It looks at the frame before and the current frame, to check if they use the global palette. Is there a case where both the previous frame and the current frame both use the global palette, but some frame before that has used a local palette? If so, it seems like there could be a problem with the approach used incmpPixel
. If it's not correct, maybe theCGIF
structure needs to contain a flag indicating whether a local palette has ever been used in an earlier frame, and check that instead of the previous frame. If that was a problem before, then this still PR maintains the same behavior.Second I'm not sure if my optimization is okay with
transIndex
.Any feedback gratefully received.
@dloebl If it would be helpful I could e-mail you an example of the GIFs I'm making. (They're about 50kb).