GNOME Bugzilla – Bug 72878
Incorrect RGBA resampling in Convolve tool (Blur/Sharpen)
Last modified: 2005-01-01 21:56:06 UTC
If two pixels have different opacity values (alpha channel), then their colors are not averaged correctly by the Convolve tool (Blur/Sharpen). This bug causes interesting results when the blur tool is used on the second image attached to bug #70335, but this is incorrect even if it is funny. It looks like the RGB channels are resampled without taking the opacity into account. As a result, the (invisible) color of a transparent pixel can bleed into an opaque pixel. See bug #70335 for some test images and a longer description of the problem. This problem affects many other tools and plug-ins.
Problem seems to be located in convolve_region().
Changed target milestone of several bugs to 2.0 - these are not features, and don't look to me like blockers for a pre-release, but they have to be addressed before 2.0. Dave.
Bumping the milestone for this bug, and all that depend on it, to 2.2. In the absence of someone actively working on this, there is no point keeping it on the 2.0 milestone. If someone wants to work on this, fixes to this family would also be accepted on the stable branch. Dave.
Created attachment 28142 [details] [review] testing patch - weighted divisor some notes about this patch: 1. it works in these cases: - soft brush on a hard edge - soft brush on a soft edge - hard brush on a soft edge. If used with a hard brush on a hard edge, there is a strange 'bleeding' effect. It's possibly a property of the algorithm - I'm not entirely convinced that it is directly related to the weighting method. If it is, though, I don't see a way around it at the moment. 2. The patch assumes that the matrix divisor is calculated by summing the elements of the matrix, as it is in gimp_convolve_calculate_matrix(). This is not necessarily true. (?)
Created attachment 28143 [details] [review] testing patch - weighted divisor sorry, some unrelated code was left at the bottom of the last patch.
Created attachment 28164 [details] [review] fixed patch this patch should adress both issues.
Some imprecision resulting in a slight change in color with blended alpha values (somewhat noticeable with solid colors) could be avoided by using floating point matrices in convolve_region(). Since some floating point operations are necessary for this fix, the fixed point math isn't really useful anymore. The convolve tool starts out with floating point matrices and converts them to integers, so it wouldn't be difficult to implement (though I haven't yet investigated the scissors tool). Any reason not to?
If you can show that floating point math doesn't make things considerably slower on standard hardware, then it's probably fine to convert the math to floating point. Given the fact that we can assume the presence of an FPU on modern CPUs I don't expect the floating point algorithm to be noticeably slower. The patch looks good but I haven't found the time to try it yet.
The closest thing I have to standard hardware is my pentium 4. I put the current cvs and the patched version through gprof in nearly identical situations. Here is the average time spent in convolve_region() (remember that the current cvs should have a fairly constant speed regardless of alpha): current cvs: 645.19 microseconds patched on a fully opaque region: 225.0 microseconds patched on half opaque/half transparent: 385.13 microseconds It's already at least about 2 times as fast, 3 in many cases. This is probably because about 2/3 of the matrix elements used by the convolve tool are zeros, but convolve_region still bothers to multiply them through four times per pixel. In that regard I don't think using floating point will be any slower than it is now, but I'll start working on it and post a profile of that as well.
Created attachment 28181 [details] [review] floating point conversion + scissors fix Unfortunately I couldn't really get consistent measurements from gprof - they ranged from 96us to ~800us with the same size brush, but were mostly around 200-300us. It is either faster or only marginally slower, not noticeable either way. This patch also fixes the intelligent scissors which were broken with the last patch.
Thanks a lot for the patch; I've now applied it to the HEAD branch: 2004-05-31 Sven Neumann <sven@gimp.org> * app/paint/gimpconvolve.c * app/paint-funcs/paint-funcs.[ch] * app/tools/gimpiscissorstool.c: applied a patch from Philip Lafleur that fixes RGBA resampling in Convolve tool (bug #72878).
Created attachment 28187 [details] [review] darkening fix erg... very sorry to do this to you again, but I have yet another patch (definitely the last from me for this bug). It would seem that the previous patch was still not precise enough. You can see the effect by continuously blurring a solid-color region that is less than fully opaque - the color gets darker. This patch fixes the problem by not dividing the alpha value; if you repeat the test with this patch you will see that the color stays constant across the blurred areas. Somehow, though, this seems to break the scissors tool a bit. I was unable to figure out why this happens, so I added gboolean alpha_weighting to convolve_region()'s parameters (an option included in the convolution matrix plugin).
No problem. 2004-05-31 Sven Neumann <sven@gimp.org> * app/paint/gimpconvolve.c * app/paint-funcs/paint-funcs.[ch] * app/tools/gimpiscissorstool.c: reverted last change and applied new patch instead (bug #72878).