From 422b02960289fd288e1670ea151375e2b629724b Mon Sep 17 00:00:00 2001
From: John Cupitt <jcupitt@gmail.com>
Date: Fri, 29 Jul 2016 12:35:55 +0100
Subject: [PATCH] better gif load via magick7

---
 TODO                          | 15 +------
 libvips/foreign/magick7load.c | 80 +++++++++++++++--------------------
 2 files changed, 35 insertions(+), 60 deletions(-)

diff --git a/TODO b/TODO
index 1b0ec729..101b3ee6 100644
--- a/TODO
+++ b/TODO
@@ -1,17 +1,6 @@
-- try:
+- try a gif with transparency
 
-	vips magickload images/cramps.gif x.png
-
-  we get a four-band png with a strange alpha ... should be 0/255, but it has
-  greyscale values in
-
-  try with convert, get a one-band png, no alpha at all
-
-  solution: cramps.gif is tagged as ImageType 4, palette image (not palette
-  alpha) ... the four channels are RGB + index, index is 0 - 15, the index
-  values for the gif
-
-  we'll need a separate path for each imagetype :-( 
+- try loading a PNG
 
 - try loading a DICOM
 
diff --git a/libvips/foreign/magick7load.c b/libvips/foreign/magick7load.c
index 6c44db31..05d75a1b 100644
--- a/libvips/foreign/magick7load.c
+++ b/libvips/foreign/magick7load.c
@@ -316,8 +316,21 @@ vips_foreign_load_magick7_parse( VipsForeignLoadMagick7 *magick7,
 	 */
 	out->Xsize = image->columns;
 	out->Ysize = image->rows;
-	out->Bands = GetPixelChannels( image ); 
 	magick7->frame_height = image->rows;
+	out->Bands = GetPixelChannels( image ); 
+
+	switch( GetImageType( image ) ) {
+	case PaletteType:
+	case PaletteAlphaType:
+	case PaletteBilevelAlphaType:
+		/* Palette images include an index channel, which we drop.
+		 */
+		out->Bands -= 1;
+		break;
+
+	default:
+		break;
+	};
 
 	/* Depth can be 'fractional'. You'd think we should use
 	 * GetImageDepth() but that seems to compute something very complex. 
@@ -451,28 +464,27 @@ vips_foreign_load_magick7_parse( VipsForeignLoadMagick7 *magick7,
 	return( 0 );
 }
 
-#define UNPACK( TYPE, Q, P ) { \
-	TYPE * restrict tq = (TYPE *) (Q); \
+/* We don't bother with GetPixelReadMask((), assume it's everywhere. Don't
+ * bother with traits, assume taht's always update.
+ *
+ * We do skip index channels. Palette images add extra index channels
+ * containing the index value from the file before colourmap lookup.
+ */
+#define UNPACK( TYPE ) { \
+	TYPE * restrict tq = (TYPE *) q; \
 	int x; \
 	int b; \
 	\
 	for( x = 0; x < r->width; x++ ) { \
-		if( GetPixelReadMask( image, (P) ) ) { \
-			for( b = 0; b < GetPixelChannels( image ); b++ ) { \
-				PixelChannel channel = \
-					GetPixelChannelChannel( image, b ); \
-				PixelTrait traits = \
-					GetPixelChannelTraits( image, channel );\
-				\
-				if( (traits & UpdatePixelTrait) == 0 ) \
-					continue; \
-				\
-				tq[b] = (P)[b]; \
-			} \
+		for( b = 0; b < GetPixelChannels( image ); b++ ) { \
+			PixelChannel channel = \
+				GetPixelChannelChannel( image, b ); \
+			\
+			if( channel != IndexPixelChannel ) \
+				*tq++ = p[b]; \
 		} \
 		\
-		(P) += GetPixelChannels( image ); \
-		tq += GetPixelChannels( image ); \
+		p += GetPixelChannels( image ); \
 	} \
 }
 
@@ -509,45 +521,19 @@ vips_foreign_load_magick7_fill_region( VipsRegion *or,
 
 		switch( im->BandFmt ) {
 		case VIPS_FORMAT_UCHAR:
-
-{
-	unsigned char * restrict tq = (unsigned char *) q; 
-	int x; 
-	int b; 
-	
-	for( x = 0; x < r->width; x++ ) { 
-		if( GetPixelReadMask( image, p ) ) { 
-			for( b = 0; b < GetPixelChannels( image ); b++ ) { 
-				PixelChannel channel = 
-					GetPixelChannelChannel( image, b ); 
-				PixelTrait traits = 
-					GetPixelChannelTraits( image, channel );
-				
-				if( (traits & UpdatePixelTrait ) == 0 ) 
-					continue; 
-				
-				tq[b] = p[b]; 
-			} 
-		} 
-		
-		p += GetPixelChannels( image ); 
-		tq += GetPixelChannels( image ); 
-	} 
-}
-
-			//UNPACK( unsigned char, q, p );
+			UNPACK( unsigned char ); 
 			break;
 
 		case VIPS_FORMAT_USHORT:
-			UNPACK( unsigned short, q, p );
+			UNPACK( unsigned short ); 
 			break;
 
 		case VIPS_FORMAT_FLOAT:
-			UNPACK( float, q, p );
+			UNPACK( float ); 
 			break;
 
 		case VIPS_FORMAT_DOUBLE:
-			UNPACK( double, q, p );
+			UNPACK( double ); 
 			break;
 
 		default: