Re: unneeded buggy code in gdk-pixbuf PGN saving

[Owen Taylor <otaylor redhat com>]

Michael Natterer <mitch gimp org> writes:

> Hi,
> 
> gdk_pixbuf__png_image_save() in io-png.c does voodoo things to RGBA
> pixbufs, resulting in _BGRA_ PNGs being saved. Also, the endian
> conversion seems useless. At least I didn't find any of the statements
> removed in the GIMP png plug-in:

It looks to me like this code was copied from some other place where
the in-memory format of data was ARGB in native-endian, rather than
gdk-pixbuf's fixed big-endian RGBA. (Which is also libpng's default
format.) I think the lines in questions can safely be removed.

Maybe you could extend testpixbuf-save to test the alpha case? Also, 
if you are using this code, you probably want to fix the silly:

                       for (j = 0, x = 0; x < w; x++)
                               memcpy (&(data[x*3]), &(ptr[x*3]), 3);

I don't think there is any reason for the code to have to rewrite
the data, either for the RGBA or the RGB code.

Thanks,
                                        Owen

> Index: gdk-pixbuf/io-png.c
> ===================================================================
> RCS file: /cvs/gnome/gtk+/gdk-pixbuf/io-png.c,v
> retrieving revision 1.42
> diff -u -p -r1.42 io-png.c
> --- gdk-pixbuf/io-png.c 2001/10/05 18:51:47     1.42
> +++ gdk-pixbuf/io-png.c 2001/12/10 11:35:28
> @@ -823,11 +823,6 @@ gdk_pixbuf__png_image_save (FILE        
>                 png_set_IHDR (png_ptr, info_ptr, w, h, bpc,
>                               PNG_COLOR_TYPE_RGB_ALPHA, PNG_INTERLACE_NONE,
>                               PNG_COMPRESSION_TYPE_BASE, PNG_FILTER_TYPE_BASE);
> -#ifdef WORDS_BIGENDIAN
> -               png_set_swap_alpha (png_ptr);
> -#else
> -               png_set_bgr (png_ptr);
> -#endif
>         } else {
>                 png_set_IHDR (png_ptr, info_ptr, w, h, bpc,
>                               PNG_COLOR_TYPE_RGB, PNG_INTERLACE_NONE,
> 
> 
> Removing the #ifdef stuff produces the expected results (and endian
> conversion should be the job of libpng itself)...