GNOME Bugzilla – Bug 163721
Smudge makes colors darker
Last modified: 2005-09-06 18:05:03 UTC
Create new file (for instance 200 width, 100 height), make half of it (for instance: the left half side) black and half of it white. Then smudge from the black area to the white area (for instance from pixel (25, 25) to pixel (175, 25)) and from the white area to the black area (for instance from pixel (25, 75) to pixel (175, 75)). The line from the black area to the white area is longer than the other. The line from the black area to the white area has more darkness than there is lightness in the line from the white area to the black area. Both lines should have the same length and the darkness of the line from the black area to the white area should equal the lightness of the line from the white area to the black area.
Created attachment 35846 [details] The result of performing the actions described in the bug report.
I think the problem is caused by the fact that when taking a (weighed) average, GIMP always rounds down.
No, the problem is caused by a wrong gamma setting and there's a bug report about it already. Please look up that bug-report and close this one as duplicate.
Applying a gamma correction to the display almost fixes the problem. The fact that it doesn't fix it completely seems to be a good indication that there's an additional problem such as the rounding error you mentioned.
You were referring to bug #158123, right? I think too it's not only caused by gamma settings. When I do a colorpick at the end of the line from white to black (pixel (25, 75)), I get rgb(0, 0, 0), that is: completely black. But whet I do a colorpick at the end of the line from black to white (pixel (175, 25)), I get rgb(241, 241, 241), whitch is not completely white. I am sorry, I had to tell that in the bug description allready.
Err, didn't I say exactly that in comment #4 already?
The smudge tool repeatedly moves image data between the canvas and a private buffer. As it does so, alpha blending is performed. There are 2 major problems with this, both sort of hinted at above. One is gamma. Unfortunately, the gimp is incapable of operating well with regard to this. 8 bits/channel is not enough to accurately handle linear images, and the gimp does not convert to sRGB for display. The other is simply the effect of repeated operations on 8-bit data. You just can't do this well on 8-bit data. 32-bit floats work pretty well. If you want to see it in action, get a recent CVS snapshot of Tux Paint. Be sure to use the --nosound option if you are not a little kid; otherwise the smudge tool will make noises. :-) Data for the canvas is still unfortunately 8-bit, but the smudge tool has a private buffer that uses 32-bit floats. Data is converted from 8-bit/channel sRGB to 32-bit/channel linear RGB as it comes off the canvas, and back again as it returns to the canvas. The conversion back to sRGB is via a 12-bit table. So, that's a reasonable hack for now: do like Tux Paint. Long term though, the gimp needs to use linear float data internally and convert to sRGB (always; it is the standard for displays now) for display.
Thank you. I will wait for the long term solution: Tux Paint is nice, but gimp is nicer :-)
There are a couple of things going on here. First, the asymmetry between black and white, which is caused by rounding. Second, the fact that smudging from black to white produces a smudge that does not decay away completely -- i.e., the smudge goes on forever. It is easy to fix the asymmetry by adding an offset before rounding, but the cost of doing it the easy way is that *both* black-to-white and white-to-black smudges produce effects that go on forever. Attached below is a patch (for paint-funcs-generic.h) that does something a bit more sophisticated: in the function blend_pixels, it causes rounding to always go toward the value from src1. This asymmetry is acceptable, I think, because the function is not used by anything except the smudge tool, and it produces the correct results there: symmetry between black and white, and smudges that always decay to nothing. If the patch is accepted, I will add a comment to the source code explaining this reasoning. I also, in the patch, changed a value from 256 to 255 because it was clearly a blunder: 256 means that blend1 + blend2 = 257, which can't possibly be right. I will wait for code review before committing. This is probably not something that should be committed to the stable branch.
Created attachment 39023 [details] [review] patch for app/paint-funcs/paint-funcs-generic.h
the patch in comment #10 seems to work for me, I'll attach an example image of a smudge after it
Created attachment 50155 [details] pic of smudging after patch this is a image made after the patch, just smudge a black line on a white image. previous to this, there would be a grey trail that would continue as long as the brush stroke would go
Setting on the 2.4 milestone since there's a patch that should be considered for inclusion.
I wonder if this code should use INT_BLEND() instead of trying to reimplement the blending algorithm. But I haven't looked at it in detail, just something that should be checked.
Doesn't look as if INT_BLEND() would work here. Well, that the function is used nowhere else, the patch fixes the bug and looks correct, please commit.
Okay, the patch is committed to HEAD: 2005-08-24 Bill Skaggs <weskaggs@primate.ucdavis.edu> * app/paint-funcs/paint-funcs-generic.h (blend_pixels): Change blending algorithm to fix misbehavior of smudge tool, should fix bug #163721.
Created attachment 51831 [details] Demonstrates brokenness for layers with alpha channel. * Load this image in * Select a biggish (13x13) brush * Select the smudge tool * Set the 'hard edge' option (this makes the artifacts more obvious) * paint a few strokes from the edge of a letter out into space, and watch the rainbow-colored excessively-solid pixels appear. I'm pretty sure this is caused by the fix here, as i could do it a few months ago without any artifacts. I think this bug should be reopened until this unwanted side-effect is fixed.
Absolutely. This needs to be fixed properly or the change needs to be backed out.
*** Bug 315323 has been marked as a duplicate of this bug. ***
Very peculiar. The problem seems to be that the arithmetic is being done in "unsigned" numbers rather than "signed" numbers. I can replicate the problem, and changing while (w--) { guint a1 = blend1 * src1[c]; guint a2 = blend2 * src2[c]; guint a = a1 + a2; to while (w--) { gint a1 = blend1 * src1[c]; gint a2 = blend2 * src2[c]; gint a = a1 + a2; seems to make things work correctly. Maybe somebody who understands compilers better than I do can comment on the proper approach to this. I think what is happening is that without the change, the line dest[b] = src1[b] + (src1[b] * a1 + src2[b] * a2 - a * src1[b]) / a; is producing the wrong result, because all the arithmetic is being done unsigned. The fix is easy, but is it correct?
I can't comment on the algorithm, since I didn't understand it in the first place. However, this kindof scenario is *exactly* the reason why using unsigned integers is bad bad bad and silly. Even if the final result fits into an unsigned variable, intermediate results may well leave the value range used. The rule of thumb is that unsigned variables should only be used where they are needed and/or make sense. "The nagative values are apparently not used here" is *no* reason in any way; and using unsigned is also not at all an optimization, even if people sometimes claim it is.
I forgot, please commit and close the bug :)
2005-09-06 Bill Skaggs <weskaggs@primate.ucdavis.edu> * app/paint-funcs/paint-funcs-generic.h (blend_pixels): change variables from unsigned to signed -- fixes problem described in comment 17 of bug #163721.