After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 637413 - Saving indexed png with transparency removes one unused color from the colormap
Saving indexed png with transparency removes one unused color from the colormap
Status: RESOLVED FIXED
Product: GIMP
Classification: Other
Component: Plugins
2.6.8
Other All
: Normal minor
: 2.10
Assigned To: GIMP Bugs
GIMP Bugs
: 670177 687309 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2010-12-16 18:35 UTC by pmoleri
Modified: 2017-12-31 23:50 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch (8.34 KB, patch)
2012-03-24 22:19 UTC, Alexis Wilhelm
none Details | Review
patch: try not to remove any unused color (1.37 KB, patch)
2012-03-26 18:51 UTC, Alexis Wilhelm
committed Details | Review
Comparison of palettes before and after opening and saving an indexed png (304.82 KB, image/x-xcf)
2012-10-13 13:45 UTC, Max Mustermann
  Details

Description pmoleri 2010-12-16 18:35:34 UTC
SO: Ubuntu Lucid Lynx

Symptoms: Saving png one and only one unused color is removed from the colormap.

Steps to reproduce:
1. New image rgb
2. Change to indexed mode with optimum palette
3. Add alpha channel
4. Erase a piece of background
5. Add a color to the colormap (example black)
6. Save the image and close it
7. Open again and black is not in the colormap

More info:
* If there are several unused colors one and only one color is removed, that's why I think this isn't an expected behavior.

* Without alpha channel no colors are removed.
Comment 1 Eduardo "Trialforce" Bonfandini 2012-03-15 11:06:07 UTC
Hi!

It happens to me too!

Steps:
1. Open a RGB image.
2. Convert it to indexed.
3. Import the generated palette to gimp palettes.
4. Apply the palette to image (works fine).
5. Close image.
6. Open image again.
7. Apply the same palette to same image. Not Work.

I an using GIMP 2.6.11 and bug it happens to.
I Try version 2.7.3 (unstable) and the bug persists.

If the indexed PNG image with alfa was saved, the same palette is not correctelly applied anymore.

It not occur with GIF.

I am using ubuntu linux too.
Comment 2 Alexis Wilhelm 2012-03-24 22:19:25 UTC
Created attachment 210535 [details] [review]
patch

It does not seem to be an error, since this behavior is documented in respin_cmap() in file-png.c.

However, I agree that it seems weird. We should either remove all unused colors or try not to remove any (if there are less than 256 colors in the palette). The proposed patch always removes all unused colors.
Comment 3 pmoleri 2012-03-25 20:22:25 UTC
I agree that the behaviour should be consistent, remove all or not remove any.

I rather not to remove any because when I first encountered this problem I was trying to save a set of frames to png files, all sharing the same colormap, that way I could achieve good compression storing only the deltas between frames. But sometimes there were different colors between frames. At that time a used the workarround of always add a dummy color that the plugin would consume, but that's why I think when you choose a colormap the saving plugin should respect it.
Comment 4 Eduardo "Trialforce" Bonfandini 2012-03-25 23:02:33 UTC
If you remove the "unnused" colors it will break the consistent/relation with the color table/color map previously created.

What the purpose to create/manage a color table, if you cannot use it again when you close/open the image?

Please do note remove the colors, without ask the user.

It not happens in previous versions of GIMP. I think it is a bug.
Comment 5 Alexis Wilhelm 2012-03-26 18:51:44 UTC
Created attachment 210654 [details] [review]
patch: try not to remove any unused color

pmoleri, that makes sense, even though I am not sure many users have the problem you described :)  This new patch does not remove any unused color if there is less than 256 colors in the palette.
Comment 6 pmoleri 2012-03-26 19:11:45 UTC
Nice.
Indeed, my use case was kind of peculiar :)
But as a general rule, is nice when the application keeps the colormap, you always have the option of generating an optimal pallete.

To "remove unused colors" would be an improvement as an option in the save dialog, buy everyone can live without it.
Comment 7 Michael Natterer 2012-08-23 07:06:58 UTC
Alexis, why do we at all fill the colormap to always contain 256 colors
(that's what the patch does, right?). Can't we simply export the colormap
as-is and done?
Comment 8 pmoleri 2012-08-23 15:58:53 UTC
(In reply to comment #7)
Michael, if I understood correctly the patch, it doesn't fill the colormap, it just looks if it's under 256 colors and adds *one* to use as transparency.
And only if the colormap is full it looks for the first unused color to use as transparency.
Comment 9 Alexis Wilhelm 2012-09-14 16:44:19 UTC
Uh, seems like I wasn't in the CC list.

Right now find_unused_ia_color():
 1) looks for an unused color and removes it (line 1831);
 2) checks if there is still room in the palette (line 1840).

The patch simply reverse those two actions, so we won't remove a color if we don't need to.
Comment 10 Max Mustermann 2012-10-13 13:43:14 UTC
*** Bug 670177 has been marked as a duplicate of this bug. ***
Comment 11 Max Mustermann 2012-10-13 13:45:14 UTC
Created attachment 226377 [details]
Comparison of palettes before and after opening and saving an indexed png

Confirming this for GIMP 2.82 on Win7 and Mac.
Comment 12 Michael Northington 2012-11-01 04:37:44 UTC
*** Bug 687309 has been marked as a duplicate of this bug. ***
Comment 13 Michael Schumacher 2016-12-30 22:00:03 UTC
Comment on attachment 210654 [details] [review]
patch: try not to remove any unused color

This patch seems simple enough, and it still applies cleanly - and it likely keeps a few files from being changed more than necessary.
Comment 14 Jeremy T. Gibson 2017-04-14 06:04:38 UTC
Adding the transparent colour to the end is probably the best behaviour, but there's another interesting problem here: if the image has a full colourmap and *already* contains a transparent colour in the palette (in e.g., OpenXcom's case, this is always #0 in the palette), the current builds of GIMP still discard it even though it *is* the transparent colour, and assign a new one anyway.  Thus, the transparent colour gets discarded and a brand new transparent colour gets assigned at the end of the palette instead, shifting the entire palette one entry to the left.  In OXC this is non-fatal, simply darkening the image -- except for the near-black shades, which wind up shifting into the next row of colour information -- but GIMP has always been my go-to for spriting and it's a shame that it fails so badly here.

Is there a way to improve the behaviour such that it checks whether the transparent pixels in the image share a common colour in the palette before it decides to assign a new transparent colour?  An RGBA image which includes varying shades of alpha in a percentage of the pixels might degrade in a less than optimal manner, but any other image with a fixed palette will perform ideally.

I suppose a brute-force solution could simply scan the image for the first transparent pixel and, once found, see if the same RGB data is used anywhere else in the image in a non-transparent pixel.  If not, that colour should be yielded as the transparent colour and there is no need to assign a new one.

Since most images with transparency have the transparent pixels close to the corners this would usually be O(1) and no worse than O(n) for the number of pixels in the image.  One would have to intentionally design a PNG to have a single transparent pixel in the farthest corner to make this operation take a large number of operations and it would still be no more than a quarter-second or so for a typical image.
Comment 15 Alexis Wilhelm 2017-12-29 10:44:32 UTC
Jeremy, did you try with the proposed patch? (attachment 210654 [details] [review])
I believe it should fix the issue you describe.
If it does not, can you post an image and the steps to reproduce your bug?


Some more info I found about PNG and GIMP:

In PNG files, the transparent colors are always at the beginning of the palette.
In PNG files exported by GIMP, there is at most one transparent color, and it is always the first color of the palette.

When loading an image, load_image() (lines 1170 to 1190 of file-png.c) removes all fully transparent colors from the palette and shifts all other colors toward the beginning.
When exporting this image, find_unused_ia_color() sees that the palette is not full (because the transparent color was removed) and appends a new transparent color at the end.
Then respin_cmap() puts the transparent color at index 0 and shifts all other colors toward the end.
So the palette of the exported image should be the same as in the original image.
Comment 16 Michael Schumacher 2017-12-29 12:22:14 UTC
The patch still applies, is very localized and I got a branch based on master for it, so I could push it instantly.
Comment 17 Michael Schumacher 2017-12-31 21:06:06 UTC
Applies, builds, produces a working file-png plug-in and it's got the 2.10 milestone:

commit 8b29687353e38896bc8d5ee3e7d69ae0bf0f4471 (HEAD -> master, bug-637413)
Author: Alexis Wilhelm <alexiswilhelm@gmail.com>
Date:   Mon Mar 26 20:40:25 2012 +0200

    Bug 637413 - Saving indexed png with transparency removes one unused color from the colormap