#2348 animated gif optimize does not work as expected

[theozh]

When creating an animated GIF using the parameter optimize, apparently the colors will be optimized for each frame independently, which might lead to changing colors of datapoints or objects during the animation instead of sticking to the same color for all frames.

See: https://stackoverflow.com/q/64568057/7295599

Or the same code:

### animated plot with circles in 3D (only for gnuplot >=5.4)
reset session

set term gif size 400,400 animate delay 30 optimize
set output "WithCircles3D.gif"

# create some test data
set print $Data
    do for [i=1:100] {
        print sprintf("%g %g %g %g %g", rand(0), rand(0), rand(0), rand(0)*0.02+0.02, int(rand(0)*0xffffff))
    }
set print

set view equal xyz
set xyplane at 0
set border 4095
set xtics 0.2
set ytics 0.2
set ztics 0.2
set style fill solid 1.0

do for [a=5:360:10] {
    set view 60,a,1.25
    splot $Data u 1:2:3:4:5 w circles lc rgb var notitle
}
set output
### end of code

[Ethan Merritt]

I am afraid that the "optimize" option is broken in libgd and has been for a long time.
See for example:
https://sourceforge.net/p/gnuplot/bugs/2070/
https://sourceforge.net/p/gnuplot/bugs/1992/
https://github.com/libgd/libgd/issues/415#issuecomment-344618783

Since we can't fix it at the gnuplot level, I suppose the option should be disabled altogether or at least provided with a big WARNING: NOT RELIABLE message.

[Hiroki Motoyoshi]

In the libgd's online manual, we can find the following description about the function gdImageGifAnimAdd.

Setting the LocalCM flag to 1 adds a local palette for this image to the animation. Otherwise the global palette is assumed and the user must make sure the palettes match. Use gdImagePaletteCopy to do that.

And there, the following sample code is also shown.

{
gdImagePtr im, im2, im3;
int black, white, trans;
FILE *out;

im = gdImageCreate(100, 100);     // Create the image
white = gdImageColorAllocate(im, 255, 255, 255); // Allocate background
black = gdImageColorAllocate(im, 0, 0, 0); // Allocate drawing color
trans = gdImageColorAllocate(im, 1, 1, 1); // trans clr for compression
gdImageRectangle(im, 0, 0, 10, 10, black); // Draw rectangle

out = fopen("anim.gif", "wb");// Open output file in binary mode
gdImageGifAnimBegin(im, out, 1, 3);// Write GIF hdr, global clr map,loops
// Write the first frame.  No local color map.  Delay = 1s
gdImageGifAnimAdd(im, out, 0, 0, 0, 100, 1, NULL);

// construct the second frame
im2 = gdImageCreate(100, 100);
(void)gdImageColorAllocate(im2, 255, 255, 255); // White background
gdImagePaletteCopy (im2, im);  // Make sure the palette is identical
gdImageRectangle(im2, 0, 0, 15, 15, black);    // Draw something
// Allow animation compression with transparent pixels
gdImageColorTransparent (im2, trans);
gdImageGifAnimAdd(im2, out, 0, 0, 0, 100, 1, im);  // Add second frame

// construct the third frame
im3 = gdImageCreate(100, 100);
(void)gdImageColorAllocate(im3, 255, 255, 255); // white background
gdImagePaletteCopy (im3, im); // Make sure the palette is identical
gdImageRectangle(im3, 0, 0, 15, 20, black); // Draw something
// Allow animation compression with transparent pixels
gdImageColorTransparent (im3, trans);
// Add the third frame, compressing against the second one
gdImageGifAnimAdd(im3, out, 0, 0, 0, 100, 1, im2);
gdImageGifAnimEnd(out);  // End marker, same as putc(';', out);
fclose(out); // Close file

// Destroy images
gdImageDestroy(im);
gdImageDestroy(im2);
gdImageDestroy(im3);
}

These statements indicate that if we use GlobalCM in optimizations, it is likely that we will need to copy the GlobalCM palette to the new image using gdImagePaletteCopy. The current implementation of gnuplot does not appear to do this after creating image object in 'PNG_graphics()'. As an example, I attached a patch to try to implement this. Anyway, this patch resolve this issue and also the issue of Bug#2070 (basically same problem). But, to be honest, I'm not familiar with the GD library (and gnuplot's source code), so I'm not sure if it's really appropriate to call gdImagePaletteCopy from this location in the code. In particular, it is not clear whether this is a good relationship with the preceding gdImageColorAllocate.

FYI, the another issue (Bug#1992) that Ethan pointed out seems to be another problem than this issue. As for the Bug#1992, by appending nocrop option to set terminal gif in "figure8.plt", I could generate the animation file without causing a "Segmentation Fault" at least on my machine. Basically, the "nocrop" option is the default. However, in the "addcar.plt" file loaded from "figure8.plt", there is a command "set term png crop" and this "crop" option seems to have survived. So, I think that "crop" and "optimize" should not be specified at the same time.
There are many examples using "optimize" option on Intenet, but there are few reports of problems. I think it is because "nocrop" is the default.

[Ethan Merritt]

Wow. The documentation and internal comment blocks have dramatically expanded since I last looked at the libgd source. The logic doesn't make much sense to me (if the palette is global why do we have to manually copy it??) but if that's what makes it work, great.

[correction to original comment] the source file that routine gdImagePaletteCopy is in has been reorganized since the gnuplot gif driver was last revised. I didn't find it at first but it was indeed there before.

Thanks for looking in to this, and thanks for the patch.

As to bug #1992, I realize it's a different problem in detail. The larger point is that the libgd optimization is fragile (or was at the time). The upstream bug report I filed for it is still open on their tracker.

[Hiroki Motoyoshi]

Could you please add a check for png_state.animate to the patch's if condition?

Before the fix.

if ( png_state.frame_optimization && ( png_state.frame_count > 0 ) && png_state.previous_image ) {

After the fix.

if ( png_state.animate && png_state.frame_optimization && ( png_state.frame_count > 0 ) && png_state.previous_image ) {

[Ethan Merritt]

I don't think that is necesary. The frame_count can only increment inside an animation, so testing for (png_state.frame_count > 0) is already more restrictive than testing for (png_state.animate). Have I missed something?

[Hiroki Motoyoshi]

Since PNG_graphics is a general subroutine that can be used by GIF, PNG, JPEG, and SIXEL, I thought it would be better to clarify in the conditional expression that it is related to animation. If it is not necesary, please leave it as is.