From d6bf1617e99df0272b231855a933a74e99b6578f Mon Sep 17 00:00:00 2001 From: glennrp Date: Fri, 17 Dec 2010 17:28:54 +0000 Subject: [PATCH] Eliminated some IsGray() tests, rewrote BUILD_PALETTE block. --- ChangeLog | 13 +- coders/png.c | 444 +++++++++++++++------------------------------------ 2 files changed, 137 insertions(+), 320 deletions(-) diff --git a/ChangeLog b/ChangeLog index 93b0e913c..9d9a1dccd 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,17 +1,18 @@ +2010-12-17 6.6.6-6 Glenn Randers-Pehrson + * Eliminated a redundant quantization step in the PNG encoder. + * Untangled a confusing but apparently harmless improper loop nesting + in the PNG encoder. + * Eliminated redundant "IsGray()" tests from the PNG encoder. + 2010-12-14 6.6.6-6 Cristy * -format "%[fx:u.p{5,5}]" no longer reports parse exception (reference http://www.imagemagick.org/discourse-server/viewtopic.php?f=3&t=17678). -2010-12-12 6.6.6-6 Glenn Randers-Pehrson - * Eliminated a redundant quantization step in the PNG encoder. - Untangled a confusing but apparently harmless improper loop nesting - in the PNG encoder. - 2010-12-10 6.6.6-5 Glenn Randers-Pehrson * Make the PNG encoder always rebuild the palette, to avoid losing transparency when it is out of sync with the pixel data (reference http://www.imagemagick.org/discourse-server/viewtopic.php?f=3&t=17655). - Eliminated CompressColormapTransFirst() since the palette is already + * Eliminated CompressColormapTransFirst() since the palette is already compressed and sorted by opacty now when it's recreated. 2010-12-10 6.6.6-5 Cristy diff --git a/coders/png.c b/coders/png.c index b6e7d4165..c7b80052b 100644 --- a/coders/png.c +++ b/coders/png.c @@ -2875,7 +2875,7 @@ static Image *ReadOnePNGImage(MngInfo *mng_info, q->opacity=(Quantum) TransparentOpacity; else - SetOpacityPixelComponent(q,OpaqueOpacity); + q->opacity=(Quantum) OpaqueOpacity; q++; } @@ -6803,6 +6803,7 @@ static MagickBooleanType WriteOnePNGImage(MngInfo *mng_info, y; MagickBooleanType + ping_have_color, ping_have_PLTE, ping_have_bKGD, ping_have_pHYs, @@ -6882,6 +6883,7 @@ static MagickBooleanType WriteOnePNGImage(MngInfo *mng_info, ping_pHYs_x_resolution = 0; ping_pHYs_y_resolution = 0; + ping_have_color=MagickTrue; ping_have_PLTE=MagickFalse; ping_have_bKGD=MagickFalse; ping_have_pHYs=MagickFalse; @@ -6916,19 +6918,23 @@ static MagickBooleanType WriteOnePNGImage(MngInfo *mng_info, #endif #ifdef PNG_BUILD_PALETTE - if (mng_info->write_png_colortype < 5) + if (mng_info->write_png_colortype < 8 /* all */) { /* * Sometimes we get DirectClass images that have 256 colors or fewer. - * This code will convert them to PseudoClass and build a colormap. + * This code will build a colormap. * * Also, sometimes we get PseudoClass images with an out-of-date * colormap. This code will replace the colormap with a new one. + * Sometimes we get PseudoClass images that have more than 256 colors. + * This code will delete the colormap and change the image to + * DirectClass. * * Also we gather some information (number of opaque, transparent, - * and semitransparent pixels) that we might need later. If the user - * wants to force GrayAlpha or RGBA (colortype 4 or 6) we don't need - * any of that. + * and semitransparent pixels, and whether the image has any non-gray + * pixels) that we might need later. If the user wants to force + * GrayAlpha or RGBA (colortype 4 or 6) we probably don't need any + * of that. */ ExceptionInfo @@ -6938,17 +6944,17 @@ static MagickBooleanType WriteOnePNGImage(MngInfo *mng_info, n; PixelPacket - colormap[1024], - opaque[256], - semitransparent[256], - transparent[256]; - - register const PixelPacket - *q; + colormap[800], + opaque[260], + semitransparent[260], + transparent[260]; register IndexPacket *indexes; + register const PixelPacket + *q; + (void) LogMagickEvent(CoderEvent,GetMagickModule(), " Enter BUILD_PALETTE:"); @@ -6993,36 +6999,38 @@ static MagickBooleanType WriteOnePNGImage(MngInfo *mng_info, " (zero means unknown)"); } - /* Sometimes SetImageType(image,PaletteMatteType) - * loses the transparency, so we replace the - * colormap afterwards. - */ - (void) LogMagickEvent(CoderEvent,GetMagickModule(), " Regenerate the colormap"); exception=(&image->exception); + ping_have_color=MagickFalse; + image_colors=0; + for (y=0; y < (ssize_t) image->rows; y++) { - q=GetAuthenticPixels(image,0,y,image->columns,1,exception); + q=GetVirtualPixels(image,0,y,image->columns,1, + exception); if (q == (PixelPacket *) NULL) break; - if (y == 0) - { - /* Initialize the colormap */ - colormap[0]=*q; + for (x=0; x < (ssize_t) image->columns; x++) + { + if (q->red != q->green || q->red != q->blue) + ping_have_color=MagickTrue; - if (image->matte == MagickFalse) - colormap[0].opacity = OpaqueOpacity; + if (image_colors == 0) + { + /* Initialize the colormap */ + colormap[0]=*q; + + if (image->matte == MagickFalse) + colormap[0].opacity = OpaqueOpacity; + + image_colors=1; + } - image_colors=1; - } - - for (x=0; x < (ssize_t) image->columns; x++) - { for (i=0; imatte == MagickFalse || @@ -7031,33 +7039,25 @@ static MagickBooleanType WriteOnePNGImage(MngInfo *mng_info, break; } - if (i < image_colors) + if (i == image_colors && image_colors < 299) { - q++; - continue; - } - if (image_colors == 299) - continue; - - image_colors++; + image_colors++; - colormap[i]=*q; + colormap[i]=*q; - if (image->matte == MagickFalse) - colormap[i].opacity = OpaqueOpacity; + if (image->matte == MagickFalse) + colormap[i].opacity = OpaqueOpacity; + } q++; } - - if (x < (ssize_t) image->columns) - break; } - if (image_colors >= 1024) + if (image_colors >= 800) (void) LogMagickEvent(CoderEvent,GetMagickModule(), - " image has more than 1024 colors"); + " image has more than 800 colors"); else (void) LogMagickEvent(CoderEvent,GetMagickModule(), @@ -7084,17 +7084,6 @@ static MagickBooleanType WriteOnePNGImage(MngInfo *mng_info, colormap[i]; } - (void) LogMagickEvent(CoderEvent,GetMagickModule(), - " number_transparent = %d", - number_transparent); - - (void) LogMagickEvent(CoderEvent,GetMagickModule(), - " number_opaque = %d", - number_opaque); - - (void) LogMagickEvent(CoderEvent,GetMagickModule(), - " number_semitransparent = %d", - number_semitransparent); n = 0; @@ -7107,21 +7096,35 @@ static MagickBooleanType WriteOnePNGImage(MngInfo *mng_info, for (i=0; ibackground_color)) break; } - if (n < 256 && i == number_opaque) + if (number_opaque < 257 && i == number_opaque) { - colormap[n]=image->background_color; - n++; + opaque[i]=image->background_color; + opaque[i].opacity = OpaqueOpacity; number_opaque++; - image_colors++; } + (void) LogMagickEvent(CoderEvent,GetMagickModule(), + " number_transparent = %d", + number_transparent); + + (void) LogMagickEvent(CoderEvent,GetMagickModule(), + " number_opaque = %d", + number_opaque); + + (void) LogMagickEvent(CoderEvent,GetMagickModule(), + " number_semitransparent = %d", + number_semitransparent); + if (((mng_info->write_png_colortype-1) == PNG_COLOR_TYPE_PALETTE) || (mng_info->write_png_colortype == 0)) @@ -7511,16 +7514,16 @@ static MagickBooleanType WriteOnePNGImage(MngInfo *mng_info, (void) LogMagickEvent(CoderEvent,GetMagickModule(), " Selecting PNG colortype:"); - ping_color_type=(png_byte) ((matte == MagickTrue)? + ping_color_type=(png_byte) ((matte != MagickFalse)? PNG_COLOR_TYPE_RGB_ALPHA:PNG_COLOR_TYPE_RGB); - if(image_info->type == TrueColorType) + if (image_info->type == TrueColorType) { ping_color_type=(png_byte) PNG_COLOR_TYPE_RGB; image_matte=MagickFalse; } - if(image_info->type == TrueColorMatteType) + if (image_info->type == TrueColorMatteType) { ping_color_type=(png_byte) PNG_COLOR_TYPE_RGB_ALPHA; image_matte=MagickTrue; @@ -7529,21 +7532,22 @@ static MagickBooleanType WriteOnePNGImage(MngInfo *mng_info, if (image_info->type == UndefinedType || image_info->type == OptimizeType) { + if ((image_info->type == GrayscaleType) && - image_matte == MagickFalse && ImageIsGray(image)) + image_matte == MagickFalse && ping_have_color != MagickFalse) { ping_color_type=(png_byte) PNG_COLOR_TYPE_GRAY; image_matte=MagickFalse; } - if ((image_info->type == GrayscaleMatteType) && - image_matte == MagickTrue && ImageIsGray(image)) + else if ((image_info->type == GrayscaleMatteType) && + image_matte == MagickTrue && ping_have_color != MagickFalse) { ping_color_type=(png_byte) PNG_COLOR_TYPE_GRAY_ALPHA; image_matte=MagickTrue; } - if (image_info->type == PaletteType || + else if (image_info->type == PaletteType || image_info->type == PaletteMatteType) ping_color_type=(png_byte) PNG_COLOR_TYPE_PALETTE; } @@ -7561,6 +7565,8 @@ static MagickBooleanType WriteOnePNGImage(MngInfo *mng_info, ping_bit_depth=8; } + old_bit_depth=ping_bit_depth; + if (ping_color_type == PNG_COLOR_TYPE_GRAY) { if (image->matte == MagickFalse && image->colors < 256) @@ -7568,9 +7574,6 @@ static MagickBooleanType WriteOnePNGImage(MngInfo *mng_info, if (ImageIsMonochrome(image)) { ping_bit_depth=1; - - if (ping_bit_depth < (int)mng_info->write_png_depth) - ping_bit_depth = mng_info->write_png_depth; } } } @@ -7588,44 +7591,23 @@ static MagickBooleanType WriteOnePNGImage(MngInfo *mng_info, "image has 0 colors", "`%s'",""); } - if (logging != MagickFalse) - (void) LogMagickEvent(CoderEvent,GetMagickModule(), - " SyncImage.2."); - while ((int) (one << ping_bit_depth) < (ssize_t) image_colors) ping_bit_depth <<= 1; + } - if (logging != MagickFalse) - { - (void) LogMagickEvent(CoderEvent,GetMagickModule(), - " Number of colors: %.20g",(double) image_colors); - - (void) LogMagickEvent(CoderEvent,GetMagickModule(), - " Tentative PNG bit depth: %d",ping_bit_depth); - } - - if (mng_info->write_png_depth) - { - old_bit_depth=ping_bit_depth; - if (ping_bit_depth < (int)mng_info->write_png_depth) - { - ping_bit_depth = mng_info->write_png_depth; + if (logging != MagickFalse) + { + (void) LogMagickEvent(CoderEvent,GetMagickModule(), + " Number of colors: %.20g",(double) image_colors); - if (ping_bit_depth > 8) - ping_bit_depth = 8; + (void) LogMagickEvent(CoderEvent,GetMagickModule(), + " Tentative PNG bit depth: %d",ping_bit_depth); + } - if (ping_bit_depth != (int) old_bit_depth) - { - if (logging != MagickFalse) - (void) LogMagickEvent(CoderEvent,GetMagickModule(), - " Colors increased to %.20g",(double) - image_colors); - } - } - } - } + if (ping_bit_depth < (int) mng_info->write_png_depth) + ping_bit_depth = mng_info->write_png_depth; } - + image_depth=ping_bit_depth; if (logging != MagickFalse) @@ -7649,54 +7631,16 @@ static MagickBooleanType WriteOnePNGImage(MngInfo *mng_info, if (matte && (mng_info->IsPalette)) { - register const PixelPacket - *p; - p=GetVirtualPixels(image,0,0,image->columns,1,&image->exception); ping_color_type=PNG_COLOR_TYPE_GRAY_ALPHA; - for (y=0; y < (ssize_t) image->rows; y++) - { - p=GetVirtualPixels(image,0,y,image->columns,1,&image->exception); - if (p == (const PixelPacket *) NULL) - break; - - for (x=(ssize_t) image->columns-1; x >= 0; x--) - { - if (IsGray(p) == MagickFalse) - { - ping_color_type=(png_byte) PNG_COLOR_TYPE_RGB_ALPHA; - break; - } - p++; - } - } + if (ping_have_color != MagickFalse) + ping_color_type=PNG_COLOR_TYPE_RGBA; /* * Determine if there is any transparent color. - * To do: simplify this using data from the BUILD_PALETTE block. */ - for (y=0; y < (ssize_t) image->rows; y++) - { - p=GetVirtualPixels(image,0,y,image->columns,1,&image->exception); - - if (p == (const PixelPacket *) NULL) - break; - - for (x=(ssize_t) image->columns-1; x >= 0; x--) - { - if (p->opacity != OpaqueOpacity) - { - break; - } - p++; - } - - if (x >= 0) - break; - } - - if ((y == (ssize_t) image->rows) && (x < 0)) + if (number_transparent + number_semitransparent == 0) { /* No transparent pixels are present. Change 4 or 6 to 0 or 2. @@ -7725,19 +7669,19 @@ static MagickBooleanType WriteOnePNGImage(MngInfo *mng_info, mask=0x0001; ping_trans_color.red=(png_uint_16) - (ScaleQuantumToShort(GetRedPixelComponent(p)) & mask); + (ScaleQuantumToShort(image->colormap[0].red) & mask); ping_trans_color.green=(png_uint_16) - (ScaleQuantumToShort(GetGreenPixelComponent(p)) & mask); + (ScaleQuantumToShort(image->colormap[0].green) & mask); ping_trans_color.blue=(png_uint_16) - (ScaleQuantumToShort(GetBluePixelComponent(p)) & mask); + (ScaleQuantumToShort(image->colormap[0].blue) & mask); ping_trans_color.gray=(png_uint_16) - (ScaleQuantumToShort(PixelIntensityToQuantum(p)) & mask); + (ScaleQuantumToShort(PixelIntensityToQuantum( + image->colormap)) & mask); - ping_trans_color.index=(png_byte) - (ScaleQuantumToChar((Quantum) (GetAlphaPixelComponent(p)))); + ping_trans_color.index=(png_byte) 0; ping_have_tRNS=MagickTrue; } @@ -7750,62 +7694,24 @@ static MagickBooleanType WriteOnePNGImage(MngInfo *mng_info, */ if (logging != MagickFalse) (void) LogMagickEvent(CoderEvent,GetMagickModule(), - " Is there a single transparent color?"); + " Is there a single fully transparent color?"); - for (y=0; y < (ssize_t) image->rows; y++) - { - p=GetVirtualPixels(image,0,y,image->columns,1, - &image->exception); - x=0; - - if (p == (const PixelPacket *) NULL) - break; - - for (x=(ssize_t) image->columns-1; x >= 0; x--) - { - if (p->opacity != OpaqueOpacity) - { - if (IsPNGColorEqual(ping_trans_color,*p) == 0) - { - break; /* Can't use RGB + tRNS for multiple - transparent colors. */ - } - - if (p->opacity != (Quantum) TransparentOpacity) - { - break; /* Can't use RGB + tRNS for - semitransparency. */ - } - } - - else - { - if (IsPNGColorEqual(ping_trans_color,*p)) - break; /* Can't use RGB + tRNS when another pixel - having the same RGB samples is - transparent. */ - } - - p++; - } - - if (x >= 0) - break; - } - - if (x >= 0) + if (number_transparent > 1 || number_semitransparent > 0) { ping_have_tRNS = MagickFalse; - if (logging != MagickFalse) (void) LogMagickEvent(CoderEvent,GetMagickModule(), - " ... No."); + " ... No."); } else { if (logging != MagickFalse) (void) LogMagickEvent(CoderEvent,GetMagickModule(), - " ... Yes."); + " ... Yes: (%d,%d,%d,%d)", + (int) ping_trans_color.red, + (int) ping_trans_color.green, + (int) ping_trans_color.blue, + (int) ping_trans_color.gray); } } @@ -7950,7 +7856,7 @@ static MagickBooleanType WriteOnePNGImage(MngInfo *mng_info, } /* color_type is PNG_COLOR_TYPE_PALETTE */ - if (!mng_info->write_png_depth) + if (mng_info->write_png_depth == 0) { size_t one; @@ -7966,112 +7872,26 @@ static MagickBooleanType WriteOnePNGImage(MngInfo *mng_info, if (matte) { - ExceptionInfo - *exception; - - register const PixelPacket - *p; - - int - trans[256]; - - register const IndexPacket - *packet_indexes; - /* - Identify which colormap entry is transparent. - */ + * Set up trans_colors array. + */ assert(number_colors <= 256); - for (i=0; i < (ssize_t) number_colors; i++) - trans[i]=256; - - exception=(&image->exception); - - for (y=0; y < (ssize_t) image->rows; y++) - { - p=GetVirtualPixels(image,0,y,image->columns,1,exception); - - if (p == (const PixelPacket *) NULL) - break; - - packet_indexes=GetVirtualIndexQueue(image); - - for (x=0; x < (ssize_t) image->columns; x++) - { - if (p->opacity != OpaqueOpacity) - { - IndexPacket - packet_index; - - packet_index=packet_indexes[x]; - - if((size_t) packet_index >= number_colors) - (void) LogMagickEvent(CoderEvent, GetMagickModule(), - "packet_index=%d, number_colors=%d", - (int) packet_index, (int) number_colors); - - assert((size_t) packet_index < number_colors); - - if (trans[(ssize_t) packet_index] != 256) - { - if (trans[(ssize_t) packet_index] != - (png_byte) (255- - ScaleQuantumToChar(GetOpacityPixelComponent(p)))) - { - ping_color_type=(png_byte) - PNG_COLOR_TYPE_RGB_ALPHA; - break; - } - } - - trans[(ssize_t) packet_index]=(png_byte) (255- - ScaleQuantumToChar(GetOpacityPixelComponent(p))); - - ping_have_tRNS=MagickTrue; - } - - p++; - } - - if ((int) ping_color_type == PNG_COLOR_TYPE_RGB_ALPHA) - { - ping_num_trans=0; - ping_have_tRNS=MagickFalse; - ping_have_PLTE=MagickFalse; - mng_info->IsPalette=MagickFalse; - (void) SyncImage(image); - - if (logging != MagickFalse) - (void) LogMagickEvent(CoderEvent, GetMagickModule(), - " Cannot write image as indexed PNG, writing RGBA."); - - break; - } - } - - if (ping_have_tRNS != MagickFalse) - { - for (i=0; i < (ssize_t) number_colors; i++) - { - if (trans[i] == 256) - trans[i]=255; - - if (trans[i] != 255) - ping_num_trans=(unsigned short) (i+1); - } - } + ping_num_trans=(unsigned short) (number_transparent + + number_semitransparent); if (ping_num_trans == 0) ping_have_tRNS=MagickFalse; - if (ping_have_tRNS == MagickFalse) - ping_num_trans=0; - - if (ping_num_trans != 0) + else { - for (i=0; i < (ssize_t) number_colors; i++) - ping_trans_alpha[i]=(png_byte) trans[i]; + ping_have_tRNS=MagickTrue; + + for (i=0; i < ping_num_trans; i++) + { + ping_trans_alpha[i]= (png_byte) (255- + ScaleQuantumToChar(image->colormap[i].opacity)); + } } } } @@ -8091,6 +7911,9 @@ static MagickBooleanType WriteOnePNGImage(MngInfo *mng_info, } } + if (ping_bit_depth < mng_info->write_png_depth) + ping_bit_depth = mng_info->write_png_depth; + /* Adjust background and transparency samples in sub-8-bit grayscale files. */ @@ -8133,15 +7956,6 @@ static MagickBooleanType WriteOnePNGImage(MngInfo *mng_info, ping_background.index=(png_byte) i; - if (i == (ssize_t) number_colors) - { - if (i < 255) - { - number_colors++; - image_colors++; - } - } - if (logging != MagickFalse) { (void) LogMagickEvent(CoderEvent,GetMagickModule(), @@ -8162,7 +7976,7 @@ static MagickBooleanType WriteOnePNGImage(MngInfo *mng_info, } } - else + else /* Can't happen */ { if (logging != MagickFalse) (void) LogMagickEvent(CoderEvent,GetMagickModule(), @@ -8371,7 +8185,7 @@ static MagickBooleanType WriteOnePNGImage(MngInfo *mng_info, /* Bail out if cannot meet defined PNG:bit-depth or PNG:color-type */ - if (mng_info->write_png_colortype) + if (mng_info->write_png_colortype != 0) { if (mng_info->write_png_colortype-1 == PNG_COLOR_TYPE_GRAY) if (ImageIsGray(image) == MagickFalse) @@ -8450,17 +8264,18 @@ static MagickBooleanType WriteOnePNGImage(MngInfo *mng_info, { for (i=0; i< (ssize_t) number_colors; i++) { - if (ping_num_trans != 0) + if (i < ping_num_trans) (void) LogMagickEvent(CoderEvent,GetMagickModule(), - " PLTE[%d] + tRNS[%d] =(%d,%d,%d,%d)", - (int) i,(int) i, + " PLTE[%d] = (%d,%d,%d), tRNS[%d] = (%d)", + (int) i, (int) palette[i].red, (int) palette[i].green, (int) palette[i].blue, + (int) i, (int) ping_trans_alpha[i]); else (void) LogMagickEvent(CoderEvent,GetMagickModule(), - " PLTE[%d] =(%d,%d,%d)", + " PLTE[%d] = (%d,%d,%d)", (int) i, (int) palette[i].red, (int) palette[i].green, @@ -8841,7 +8656,7 @@ static MagickBooleanType WriteOnePNGImage(MngInfo *mng_info, if (ping_color_type == PNG_COLOR_TYPE_GRAY) (void) ExportQuantumPixels(image,(const CacheView *) NULL, - quantum_info,GrayQuantum,png_pixels,&image->exception); + quantum_info,GrayQuantum,png_pixels,&image->exception); else if (ping_color_type == PNG_COLOR_TYPE_GRAY_ALPHA) { @@ -8850,7 +8665,8 @@ static MagickBooleanType WriteOnePNGImage(MngInfo *mng_info, " Writing GRAY_ALPHA PNG pixels (4)"); (void) ExportQuantumPixels(image,(const CacheView *) NULL, - quantum_info,GrayAlphaQuantum,png_pixels,&image->exception); + quantum_info,GrayAlphaQuantum,png_pixels, + &image->exception); } else @@ -9307,7 +9123,7 @@ static MagickBooleanType WritePNGImage(const ImageInfo *image_info, if (logging != MagickFalse) (void) LogMagickEvent(CoderEvent,GetMagickModule(), - " png:color-type=%d was defined.\n",mng_info->write_png_colortype); + " png:color-type=%d was defined.\n",mng_info->write_png_colortype-1); } status=WriteOnePNGImage(mng_info,image_info,image); -- 2.50.0