PNG coder - bug in transparent background code?

[mbalfour]

I've just started trying out the Magick++ API. I'm trying to use something roughly like this:
Image image;
image.read("IceAlpha-0.5.png");
image.writePixels(IndexQuantum, buffer);

This particular image (which can be found here - http://www.libpng.org/pub/png/img_png/IceAlpha-0.5.png ) is an 8-bit palettized PNG with a transparent background.

The problem that I'm seeing is that I'm getting an exception thrown from ExportQuantumPixels (in quantum-export.c), due to this block of code:

case IndexQuantum:
{
if (image->storage_class != PseudoClass)
{
(void) ThrowMagickException(exception,GetMagickModule(),ImageError,
"ColormappedImageRequired","`%s'",image->filename);
return(extent);
}

The storage_class is currently set to DirectClass, which I believe is the bug. The reason it's DirectClass is because of ReadOnePNGImage (in png.c), where it handles the transparent background chunk:

if (ping_info->valid & PNG_INFO_tRNS)
{
ClassType
storage_class;

/*
Image has a transparent background.
*/
storage_class=image->storage_class;
image->matte=MagickTrue;

<snip>

image->storage_class=DirectClass;
}

I'm a n00b to ImageMagick as a whole, so there's a lot going on in this function that I don't understand, but I can't see why the storage_class would get forced to DirectClass here. I would have expected it to preserve the palettized nature of the image.

On the other hand, if it's trying to convert it to direct pixels due to compositing with the background color or something similar, then I guess the bug would be that it's preserving the image colormap that was already created prior to this point, and it needs to release the colormap, reset the number of colors and bit depth, etc.

[mbalfour]

Actually, looking at this a tiny bit further, I think the logic for handling the tRNS chunk might be pretty wrong. Looking at the PNG Specs:
http://www.libpng.org/pub/png/spec/1.2/PNG-Chunks.html

It seems like the logic for handling tRNS should look something like this:

if (has tRNS)
if (PNG_COLOR_TYPE_PALETTE)
for each entry in the tRNS
copy the alpha information into the corresponding palette index
set all other palette entries to have alpha=255
else if (PNG_COLOR_TYPE_GRAY)
for each entry in the palette
if the palette entry == the tRNS gray value
set the palette alpha to 0
else
set the palette alpha to 255
else
do the current logic where you check every pixel in every row and column if it matches the tRNS color, and if it does, set the alpha to 0

I think the current logic in png.c is always going through and trying to set every pixel's transparency directly, regardless of whether or not it's palettized or truecolor.

[mbalfour]

I haven't exhaustively tested the following, but I grabbed a few PNGs from PngSuite and they all seem to work correctly, so I believe this represents the correct fix.

Code: Select all

  if (ping_info->valid & PNG_INFO_tRNS)
    {
      ClassType
        storage_class;

      /*
        Image has a transparent background.
      */
      storage_class=image->storage_class;
      image->matte=MagickTrue;

    if (storage_class == PseudoClass)
	{
		if ((int) ping_info->color_type == PNG_COLOR_TYPE_PALETTE)
		{
			for (x=0; x < ping_info->num_trans; x++)
			{
				image->colormap[x].opacity = ScaleCharToQuantum((unsigned char)(255-ping_info->trans_alpha[x]));
			}
		}
		else if (ping_info->color_type == PNG_COLOR_TYPE_GRAY)
		{
			for (x=0; x < image->colors; x++)
			{
				if (image->colormap[x].red == transparent_color.opacity)
				{
					image->colormap[x].opacity = (Quantum) TransparentOpacity;
				}
			}
		}
	}
	else
	{

      for (y=0; y < (long) image->rows; y++)
      {
        image->storage_class=storage_class;
        q=GetAuthenticPixels(image,0,y,image->columns,1,exception);
        if (q == (PixelPacket *) NULL)
          break;
        indices=GetAuthenticIndexQueue(image);

          for (x=(long) image->columns-1; x >= 0; x--)
          {
            if (ScaleQuantumToChar(q->red) == transparent_color.red &&
                ScaleQuantumToChar(q->green) == transparent_color.green &&
                ScaleQuantumToChar(q->blue) == transparent_color.blue)
               q->opacity=(Quantum) TransparentOpacity;
            else
              SetOpacityPixelComponent(q,OpaqueOpacity);
            q++;
          }
        if (SyncAuthenticPixels(image,exception) == MagickFalse)
          break;
      }
      image->storage_class=DirectClass;
	}
    }