From 30b5c49d0b8513962c10068b015407fd3f03dd36 Mon Sep 17 00:00:00 2001
From: John Cupitt <jcupitt@gmail.com>
Date: Sat, 16 Feb 2019 17:37:23 +0000
Subject: [PATCH] set dispose in magicksave

since we always give complete new frames

revise gifload slightly too
---
 libvips/foreign/gifload.c    | 33 +++++++++++----------------------
 libvips/foreign/magicksave.c | 13 +++++++++----
 2 files changed, 20 insertions(+), 26 deletions(-)

diff --git a/libvips/foreign/gifload.c b/libvips/foreign/gifload.c
index f323bc01..fabc0871 100644
--- a/libvips/foreign/gifload.c
+++ b/libvips/foreign/gifload.c
@@ -88,10 +88,6 @@
 #endif
 
 /* Added in giflib5.
- *
- * DO_NOT 	- no dispose action after frame load
- * BACKGROUND 	- after frame load, reset to 0 (transparent)
- * PREVIOUS 	- after frame load, reset previous
  */
 #ifndef HAVE_GIFLIB_5
 #define DISPOSAL_UNSPECIFIED      0       
@@ -734,33 +730,26 @@ vips_foreign_load_gif_render( VipsForeignLoadGif *gif )
 	 */
 	vips_foreign_load_gif_build_cmap( gif );
 
-	/* DISPOSE_BACKGROUND means transparent pixels in this frame will show
-	 * the background of the web page, ie. be transparent.
-	 *
-	 * PREVIOUS means we init with the last non-previous frame.
-	 *
-	 * Otherwise, we must update previous.
+	/* BACKGROUND means we reset the frame to 0 (transparent) before we 
+	 * render the next set of pixels.
 	 */
-	if( gif->dispose == DISPOSE_BACKGROUND ) {
-		VIPS_DEBUG_MSG( "vips_foreign_load_gif_render: "
-			"dispose clear to zero\n" ); 
+	if( gif->dispose == DISPOSE_BACKGROUND ) 
 		memset( VIPS_IMAGE_ADDR( gif->frame, 0, 0 ), 0, 
 			VIPS_IMAGE_SIZEOF_IMAGE( gif->frame ) );
-	}
-	else if( gif->dispose == DISPOSE_PREVIOUS ) {
-		VIPS_DEBUG_MSG( "vips_foreign_load_gif_render: "
-			"dispose restore from previous\n" ); 
+
+	/* PREVIOUS means we init the frame with the frame before last, ie. we
+	 * undo the last render.
+	 *
+	 * Anything other than PREVIOUS, we must update the previous buffer,
+	 */
+	if( gif->dispose == DISPOSE_PREVIOUS ) 
 		memcpy( VIPS_IMAGE_ADDR( gif->frame, 0, 0 ),
 			VIPS_IMAGE_ADDR( gif->previous, 0, 0 ),
 			VIPS_IMAGE_SIZEOF_IMAGE( gif->frame ) );
-	}	
-	else {
-		VIPS_DEBUG_MSG( "vips_foreign_load_gif_render: "
-			"dispose copy to previous\n" ); 
+	else 
 		memcpy( VIPS_IMAGE_ADDR( gif->previous, 0, 0 ),
 			VIPS_IMAGE_ADDR( gif->frame, 0, 0 ),
 			VIPS_IMAGE_SIZEOF_IMAGE( gif->frame ) );
-	}
 
 	if( file->Image.Interlace ) {
 		int i;
diff --git a/libvips/foreign/magicksave.c b/libvips/foreign/magicksave.c
index 38e3799b..50c07f78 100644
--- a/libvips/foreign/magicksave.c
+++ b/libvips/foreign/magicksave.c
@@ -93,7 +93,7 @@ static int
 vips_foreign_save_magick_set_properties( VipsForeignSaveMagick *magick, 
 	Image *image, VipsImage *im )
 {
-  VipsObjectClass *class = VIPS_OBJECT_GET_CLASS( magick );
+	VipsObjectClass *class = VIPS_OBJECT_GET_CLASS( magick );
 
 	int number;
 	const char *str;
@@ -110,10 +110,15 @@ vips_foreign_save_magick_set_properties( VipsForeignSaveMagick *magick,
 		!vips_image_get_string( im, "gif-comment", &str ) )
 		magick_set_property( image, "comment", str, magick->exception );
 
-  if( magick_set_magick_profile( image, im, magick->exception ) ) {
+	/* libvips keeps animations as a set of independent frames, so we want
+	 * to clear to the background between each one.
+	 */
+	image->dispose = BackgroundDispose;
+
+	if( magick_set_magick_profile( image, im, magick->exception ) ) {
 		magick_vips_error( class->nickname, magick->exception ); 
-    return( -1 );
-  }
+		return( -1 );
+	}
 
 	return( 0 );
 }