GNOME Bugzilla – Bug 358851
RGBA to indexed with dithering of transparency causes transparent pixels at fixed locations
Last modified: 2006-11-27 16:39:42 UTC
This effect is seen on any RGBA layer that is converted to indexed mode using dithering of transparency. Steps to reproduce: * create new image * delete default layer * create new one transparent * fill with black * convert to index with Enable dithering of transparency
Several of us have confirmed this in HEAD (it affects one pixel out of each 32x32 block, so you might have to zoom in to see it), but it does not happen in 2.2. I think it was probably caused by the changes committed in version 1.166 of gimpimage-convert.c, with the ChangeLog entry: 2006-01-02 Sven Neumann <sven@gimp.org> * app/core/gimpimage-convert-data.h * app/core/gimpimage-convert.c: applied patch by Adam D. Moss that replaces the default dither matrix by a 32x32 Bayer pattern (see bug #136604). Comment #19 in bug #136604 makes the situation pretty clear.
Created attachment 73772 [details] Zoomed in screenshot Screenshot of zoomed in image with this defect.
The immediate cause for the problem seems to be this test in gimpimage-convert.c at line 3294: if ((has_alpha && (alpha_dither ? ((src[alpha_pix]) > (DM[foo][bar])) : (src[alpha_pix] > 127))) || !has_alpha) (reformatted and complex subexpressions replaced with "foo" and "bar") If the test is FALSE, the alpha of the destination pixel is set to zero. This causes the zero-alpha pixels we see. The interesting thing here is that DM contains values from 1 to 255. Thus whenever we are looking at the (only) "255" value in DM, data[ALPHA_PIX] can never be > DM[foo][bar], and the FALSE branch is taken. I certainly can't claim to understand this code, but would changing the ">" to ">=" here be a good idea? It fixes the problem in this bug, but what side-effects it has, I have no idea.
(Heh, when I say data[ALPHA_PIX] above, I meant src[alpha_pix], of course, I was looking at another place in gimpimage-convert.c and mistyped.) Actually, after pondering this in the shower, I think it probably is safer to keep the comparison operator as ">", but instead add another condition: If src[alpha_pix]==255, then don't bother checking any firther against the DM. I.e.: if ((has_alpha && (src[alpha_pix] == 255 || (alpha_dither ? ((src[alpha_pix]) > (DM[foo][bar])) : (src[alpha_pix] > 127)))) || !has_alpha) Then only the behaviour for fully opaque source pixels will change, they will always be fully opaque also in the result. For other pixels the behaviour will not change.
I re-wrote the Adam-code to make it easier to understand, and once this was done, it became clear that the suggestion in comment #3 is the correct one. (The direction of the inequalities is reversed in the rewritten code, though.) This should get some testing, but I'm pretty confident the fix is correct. For HEAD: 2006-10-03 Bill Skaggs <weskaggs@primate.ucdavis.edu> * app/core/gimpimage-convert.c (median_cut_pass2_nodestruct_dither_rgb): adjust alpha-dithering and make code more readable; fixes bug #358851 as suggested by Tor Lillqvist.
Created attachment 73982 [details] example of error (still) This image was created by making a new 64x48 image, adding transparency, and indexizing (with the alpha-dither option ON, of course.) I tested it on a larger image, and the wrongly transparent pixel is at [0,31] in each 'dither-tile' (the dithermatrix being 32x32.); so now, 1 of every 1024 pixels are wrongly transparent.
This does not happen for me. I'm betting that you weren't working with a fully up-to-date cvs.
Created attachment 74021 [details] Defect remains Tested your changes on current CVS. Black or white images don't cause problems any more. Painting some other colors causes no harm either. However, more than 256 colors... seems the defect is back. It had deeper roots, I guess. (Or maybe I should just get some sleep :)
I guess similar love needs to be applied to some of the other functions in gimpimage-convert.c, then. What got fixed was only the function that is used when the number of colours in the original RGB(A) image is less or equal to the number we want in the destination indexed image
Have to reopen this one because I can still reproduce the bug when image has more than colors than we want to have in a palette. (Image must have alpha channel, of course.) Comment 9 seems to have good solution. Quick investigation in a debugger shows that at least median_cut_pass2_no_dither_rgb should be changed too.
Created attachment 75465 [details] [review] Fix other functions in similar fashion This should fix the bug. Same love is applied to other functions that deal with alpha dithering.
Adding Adam to CC list.
This patch works fine for me. I had hoped it would incidentally fix bug #349882, but that didn't occur. I think it is likely that the cause of bug #349882 resides nearby to the areas changed in this patch, though.
Thanks a lot for the patch. 2006-11-27 Sven Neumann <sven@gimp.org> * app/core/gimpimage-convert.c: applied patch from Aurimas Juška that fixes conversion with dithering of transparency (bug #358851).