[Magick-bugs] Fwd: Bug in Windows Icon generation
Peter Hull
peterhull90 at gmail.com
Sat Apr 21 07:44:22 PDT 2007
Hello, me again.
I've had a look at the code in coders/icon.c. It seems like the
multiple image support was half done and half not done. I have got it
to work (as far as I can tell) but I haven't got access to a windows
system with a compiler to test at the moment.
I have attached a diff against the current SVN. It's quite long but
most of the changes are the same thing, repeatedly.
Could someone familiar with the workings of ImageMagick please have a
look at it? In particular I'm a bit unclear on the first parameter of
WriteBlobXXX - should it always be the 'root' image in a list of
images or what?
Thanks very much,
Peter
---------- Forwarded message ----------
From: Peter Hull <peterhull90 at gmail.com>
Date: Apr 20, 2007 12:17 PM
Subject: Bug in Windows Icon generation
To: magick-bugs at imagemagick.org
I'm trying to add multiple images to a Windows .ico file. The format
allows for multiple images, Windows then picks the best size/colour
depth for display.
I'm only an ImageMagick beginner, but, as I understand it,
convert i1.bmp i2.bmp test.ico
should create an icon containing the two images. However, what I get
is two separate files, test-0.ico and test-1.ico.
I can take a pre-existing icon and extract its sub-images or examine
it with identify, so the reading of icons seems fine, just the
writing.
Am I doing something wrong or is this a bug? I had a quick look at the
code in coders/icon.c, and the bit where it appears to want to count
the number of sub-images seems a little suspect, but I haven't had
time to understand it fully.
This is with ImageMagick-6.3.3-8-Q16-windows-dll.exe, and also from
the same (I think) version of the source compiled on OS X
Thanks for your help,
Peter
-------------- next part --------------
Index: coders/icon.c
===================================================================
--- coders/icon.c (revision 6659)
+++ coders/icon.c (working copy)
@@ -628,7 +628,7 @@
entry=SetMagickInfo("ICO");
entry->decoder=(DecoderHandler *) ReadICONImage;
entry->encoder=(EncoderHandler *) WriteICONImage;
- entry->adjoin=MagickFalse;
+ entry->adjoin=MagickTrue;
entry->seekable_stream=MagickTrue;
entry->description=ConstantString("Microsoft icon");
entry->module=ConstantString("ICON");
@@ -759,7 +759,7 @@
ThrowWriterException(ImageError,"WidthOrHeightExceedsLimit");
scene++;
next=SyncNextImageInList(next);
- } while (image_info->adjoin != MagickFalse);
+ } while (next && (image_info->adjoin != MagickFalse));
/*
Dump out a ICON header template to be properly initialized later.
*/
@@ -781,21 +781,23 @@
(void) WriteBlobLSBLong(image,icon_file.directory[scene].offset);
scene++;
next=SyncNextImageInList(next);
- } while (image_info->adjoin != MagickFalse);
+ } while (next && (image_info->adjoin != MagickFalse));
+
scene=0;
+ next = image;
do
{
/*
Initialize ICON raster file header.
*/
if (image_info->colorspace == UndefinedColorspace)
- (void) SetImageColorspace(image,RGBColorspace);
+ (void) SetImageColorspace(next,RGBColorspace);
icon_info.file_size=14+12+28;
icon_info.offset_bits=icon_info.file_size;
icon_info.compression=BI_RGB;
- if ((image->storage_class != DirectClass) && (image->colors > 256))
- (void) SetImageStorageClass(image,DirectClass);
- if (image->storage_class == DirectClass)
+ if ((next->storage_class != DirectClass) && (next->colors > 256))
+ (void) SetImageStorageClass(next,DirectClass);
+ if (next->storage_class == DirectClass)
{
/*
Full color ICON raster.
@@ -810,16 +812,16 @@
Colormapped ICON raster.
*/
icon_info.bits_per_pixel=8;
- if (image->colors <= 256)
+ if (next->colors <= 256)
icon_info.bits_per_pixel=8;
- if (image->colors <= 16)
+ if (next->colors <= 16)
icon_info.bits_per_pixel=4;
- if (image->colors <= 2)
+ if (next->colors <= 2)
icon_info.bits_per_pixel=1;
icon_info.number_colors=1 << icon_info.bits_per_pixel;
- if (icon_info.number_colors < image->colors)
+ if (icon_info.number_colors < next->colors)
{
- (void) SetImageStorageClass(image,DirectClass);
+ (void) SetImageStorageClass(next,DirectClass);
icon_info.number_colors=0;
icon_info.bits_per_pixel=(unsigned short) 24;
icon_info.compression=(unsigned long) BI_RGB;
@@ -832,12 +834,12 @@
icon_info.offset_bits+=(1UL << icon_info.bits_per_pixel);
}
}
- bytes_per_line=(((image->columns*icon_info.bits_per_pixel)+31) & ~31) >> 3;
+ bytes_per_line=(((next->columns*icon_info.bits_per_pixel)+31) & ~31) >> 3;
icon_info.ba_offset=0;
- icon_info.width=(long) image->columns;
- icon_info.height=(long) image->rows;
+ icon_info.width=(long) next->columns;
+ icon_info.height=(long) next->rows;
icon_info.planes=1;
- icon_info.image_size=bytes_per_line*image->rows;
+ icon_info.image_size=bytes_per_line*next->rows;
icon_info.size=40;
icon_info.size+=(4*icon_info.number_colors);
icon_info.size+=icon_info.image_size;
@@ -845,19 +847,19 @@
icon_info.file_size+=icon_info.image_size;
icon_info.x_pixels=75*39;
icon_info.y_pixels=75*39;
- switch (image->units)
+ switch (next->units)
{
case UndefinedResolution:
case PixelsPerInchResolution:
{
- icon_info.x_pixels=(unsigned long) (100.0*image->x_resolution/2.54);
- icon_info.y_pixels=(unsigned long) (100.0*image->y_resolution/2.54);
+ icon_info.x_pixels=(unsigned long) (100.0*next->x_resolution/2.54);
+ icon_info.y_pixels=(unsigned long) (100.0*next->y_resolution/2.54);
break;
}
case PixelsPerCentimeterResolution:
{
- icon_info.x_pixels=(unsigned long) (100.0*image->x_resolution);
- icon_info.y_pixels=(unsigned long) (100.0*image->y_resolution);
+ icon_info.x_pixels=(unsigned long) (100.0*next->x_resolution);
+ icon_info.y_pixels=(unsigned long) (100.0*next->y_resolution);
break;
}
}
@@ -880,16 +882,16 @@
/*
Convert PseudoClass image to a ICON monochrome image.
*/
- for (y=0; y < (long) image->rows; y++)
+ for (y=0; y < (long) next->rows; y++)
{
- p=AcquireImagePixels(image,0,y,image->columns,1,&image->exception);
+ p=AcquireImagePixels(next,0,y,next->columns,1,&next->exception);
if (p == (const PixelPacket *) NULL)
break;
- indexes=GetIndexes(image);
- q=pixels+(image->rows-y-1)*bytes_per_line;
+ indexes=GetIndexes(next);
+ q=pixels+(next->rows-y-1)*bytes_per_line;
bit=0;
byte=0;
- for (x=0; x < (long) image->columns; x++)
+ for (x=0; x < (long) next->columns; x++)
{
byte<<=1;
byte|=indexes[x] != 0 ? 0x01 : 0x00;
@@ -903,12 +905,12 @@
}
if (bit != 0)
*q++=(unsigned char) (byte << (8-bit));
- if (image->previous == (Image *) NULL)
- if ((image->progress_monitor != (MagickProgressMonitor) NULL) &&
- (QuantumTick(y,image->rows) != MagickFalse))
+ if (next->previous == (Image *) NULL)
+ if ((next->progress_monitor != (MagickProgressMonitor) NULL) &&
+ (QuantumTick(y,next->rows) != MagickFalse))
{
- status=image->progress_monitor(SaveImageTag,y,image->rows,
- image->client_data);
+ status=next->progress_monitor(SaveImageTag,y,next->rows,
+ next->client_data);
if (status == MagickFalse)
break;
}
@@ -924,16 +926,16 @@
/*
Convert PseudoClass image to a ICON monochrome image.
*/
- for (y=0; y < (long) image->rows; y++)
+ for (y=0; y < (long) next->rows; y++)
{
- p=AcquireImagePixels(image,0,y,image->columns,1,&image->exception);
+ p=AcquireImagePixels(next,0,y,next->columns,1,&next->exception);
if (p == (const PixelPacket *) NULL)
break;
- indexes=GetIndexes(image);
- q=pixels+(image->rows-y-1)*bytes_per_line;
+ indexes=GetIndexes(next);
+ q=pixels+(next->rows-y-1)*bytes_per_line;
nibble=0;
byte=0;
- for (x=0; x < (long) image->columns; x++)
+ for (x=0; x < (long) next->columns; x++)
{
byte<<=4;
byte|=((unsigned long) indexes[x] & 0x0f);
@@ -947,12 +949,12 @@
}
if (nibble != 0)
*q++=(unsigned char) (byte << 4);
- if (image->previous == (Image *) NULL)
- if ((image->progress_monitor != (MagickProgressMonitor) NULL) &&
- (QuantumTick(y,image->rows) != MagickFalse))
+ if (next->previous == (Image *) NULL)
+ if ((next->progress_monitor != (MagickProgressMonitor) NULL) &&
+ (QuantumTick(y,next->rows) != MagickFalse))
{
- status=image->progress_monitor(SaveImageTag,y,image->rows,
- image->client_data);
+ status=next->progress_monitor(SaveImageTag,y,next->rows,
+ next->client_data);
if (status == MagickFalse)
break;
}
@@ -964,21 +966,21 @@
/*
Convert PseudoClass packet to ICON pixel.
*/
- for (y=0; y < (long) image->rows; y++)
+ for (y=0; y < (long) next->rows; y++)
{
- p=AcquireImagePixels(image,0,y,image->columns,1,&image->exception);
+ p=AcquireImagePixels(next,0,y,next->columns,1,&next->exception);
if (p == (const PixelPacket *) NULL)
break;
- indexes=GetIndexes(image);
- q=pixels+(image->rows-y-1)*bytes_per_line;
- for (x=0; x < (long) image->columns; x++)
+ indexes=GetIndexes(next);
+ q=pixels+(next->rows-y-1)*bytes_per_line;
+ for (x=0; x < (long) next->columns; x++)
*q++=(unsigned char) indexes[x];
- if (image->previous == (Image *) NULL)
- if ((image->progress_monitor != (MagickProgressMonitor) NULL) &&
- (QuantumTick(y,image->rows) != MagickFalse))
+ if (next->previous == (Image *) NULL)
+ if ((next->progress_monitor != (MagickProgressMonitor) NULL) &&
+ (QuantumTick(y,next->rows) != MagickFalse))
{
- status=image->progress_monitor(SaveImageTag,y,image->rows,
- image->client_data);
+ status=next->progress_monitor(SaveImageTag,y,next->rows,
+ next->client_data);
if (status == MagickFalse)
break;
}
@@ -991,13 +993,13 @@
/*
Convert DirectClass packet to ICON BGR888 or BGRA8888 pixel.
*/
- for (y=0; y < (long) image->rows; y++)
+ for (y=0; y < (long) next->rows; y++)
{
- p=AcquireImagePixels(image,0,y,image->columns,1,&image->exception);
+ p=AcquireImagePixels(next,0,y,next->columns,1,&next->exception);
if (p == (const PixelPacket *) NULL)
break;
- q=pixels+(image->rows-y-1)*bytes_per_line;
- for (x=0; x < (long) image->columns; x++)
+ q=pixels+(next->rows-y-1)*bytes_per_line;
+ for (x=0; x < (long) next->columns; x++)
{
*q++=ScaleQuantumToChar(p->blue);
*q++=ScaleQuantumToChar(p->green);
@@ -1007,14 +1009,14 @@
p++;
}
if (icon_info.bits_per_pixel == 24)
- for (x=3L*(long) image->columns; x < (long) bytes_per_line; x++)
+ for (x=3L*(long) next->columns; x < (long) bytes_per_line; x++)
*q++=0x00;
- if (image->previous == (Image *) NULL)
- if ((image->progress_monitor != (MagickProgressMonitor) NULL) &&
- (QuantumTick(y,image->rows) != MagickFalse))
+ if (next->previous == (Image *) NULL)
+ if ((next->progress_monitor != (MagickProgressMonitor) NULL) &&
+ (QuantumTick(y,next->rows) != MagickFalse))
{
- status=image->progress_monitor(SaveImageTag,y,image->rows,
- image->client_data);
+ status=next->progress_monitor(SaveImageTag,y,next->rows,
+ next->client_data);
if (status == MagickFalse)
break;
}
@@ -1043,7 +1045,7 @@
(void) WriteBlobLSBLong(image,icon_info.y_pixels);
(void) WriteBlobLSBLong(image,icon_info.number_colors);
(void) WriteBlobLSBLong(image,icon_info.colors_important);
- if (image->storage_class == PseudoClass)
+ if (next->storage_class == PseudoClass)
{
unsigned char
*icon_colormap;
@@ -1056,11 +1058,11 @@
if (icon_colormap == (unsigned char *) NULL)
ThrowWriterException(ResourceLimitError,"MemoryAllocationFailed");
q=icon_colormap;
- for (i=0; i < (long) image->colors; i++)
+ for (i=0; i < (long) next->colors; i++)
{
- *q++=ScaleQuantumToChar(image->colormap[i].blue);
- *q++=ScaleQuantumToChar(image->colormap[i].green);
- *q++=ScaleQuantumToChar(image->colormap[i].red);
+ *q++=ScaleQuantumToChar(next->colormap[i].blue);
+ *q++=ScaleQuantumToChar(next->colormap[i].green);
+ *q++=ScaleQuantumToChar(next->colormap[i].red);
*q++=(unsigned char) 0x0;
}
for ( ; i < (long) (1UL << icon_info.bits_per_pixel); i++)
@@ -1079,18 +1081,18 @@
/*
Write matte mask.
*/
- scanline_pad=(((image->columns+31) & ~31)-image->columns) >> 3;
- for (y=((long) image->rows - 1); y >= 0; y--)
+ scanline_pad=(((next->columns+31) & ~31)-next->columns) >> 3;
+ for (y=((long) next->rows - 1); y >= 0; y--)
{
- p=AcquireImagePixels(image,0,y,image->columns,1,&image->exception);
+ p=AcquireImagePixels(next,0,y,next->columns,1,&next->exception);
if (p == (const PixelPacket *) NULL)
break;
bit=0;
byte=0;
- for (x=0; x < (long) image->columns; x++)
+ for (x=0; x < (long) next->columns; x++)
{
byte<<=1;
- if ((image->matte == MagickTrue) && (p->opacity == TransparentOpacity))
+ if ((next->matte == MagickTrue) && (p->opacity == TransparentOpacity))
byte|=0x01;
bit++;
if (bit == 8)
@@ -1106,18 +1108,18 @@
for (i=0; i < (long) scanline_pad; i++)
(void) WriteBlobByte(image,(unsigned char) 0);
}
- if (GetNextImageInList(image) == (Image *) NULL)
+ if (GetNextImageInList(next) == (Image *) NULL)
break;
- image=SyncNextImageInList(image);
- if (image->progress_monitor != (MagickProgressMonitor) NULL)
+ next=SyncNextImageInList(next);
+ if (next->progress_monitor != (MagickProgressMonitor) NULL)
{
- status=image->progress_monitor(SaveImagesTag,scene,
- GetImageListLength(image),image->client_data);
+ status=next->progress_monitor(SaveImagesTag,scene,
+ GetImageListLength(next),next->client_data);
if (status == MagickFalse)
break;
}
scene++;
- } while (image_info->adjoin != MagickFalse);
+ } while (next && (image_info->adjoin != MagickFalse));
offset=SeekBlob(image,0,SEEK_SET);
(void) WriteBlobLSBShort(image,0);
(void) WriteBlobLSBShort(image,1);
@@ -1136,7 +1138,7 @@
(void) WriteBlobLSBLong(image,icon_file.directory[scene].offset);
scene++;
next=SyncNextImageInList(next);
- } while (image_info->adjoin != MagickFalse);
+ } while (next && (image_info->adjoin != MagickFalse));
CloseBlob(image);
return(MagickTrue);
}
More information about the Magick-bugs
mailing list