GNOME Bugzilla – Bug 436145
Map Color Range and Adjust FG-BG mapping problem
Last modified: 2007-12-16 11:16:16 UTC
"Map Color Range" and "Adjust FG-BG" do not map correctly if src1[rgb] = src2[rgb] for some rgb. Especially "Adjust FG-BG" does not map src2 to 255 (white). Reason: This filter maps [a,...,b] to [as,...,bs] more or less as follows i --> as + (bs - as) * (i - a) / (b - a) if b ≠ a, i --> as + (bs - as) * (i - a) / 1 if b = a, 0 ≤ i ≤ 255. Case two was added as a bugfix for a divide by zero error. Now it doesn't crash, but it's still buggy, since actually a --> as, ok, Adjust FG-BG: bg --> black b --> bs if b ≠ a, ok, Adjust FG-BG: fg --> white b --> as if b = a. bug! You can't map a to as and b to bs if (a = b): as <--?-- (a = b) --?--> bs (I used the meaningful identifiers :-> from the well commented source code. <g>) So, Adjust Foreground & Background will work mostly as expected, but sometimes it will set some color channel of the mapped BG color to 0. And even worse, it won't display any warning. IMHO, this filter should not accept source colors with equal values for some channel. And maybe the GUI should be changed, so that the user can select the color channels separately. Otherwise it's not obvious for the user what these color ranges really mean (and what this filter does). BTW, it's not obvious for the manual authors and translators too if you do not comment your source code... :-) Ulf ranges really mean (and what this filter does). BTW, it's not obvious for the manual authors and translators too if you do not comment your source code... :-) Ulf
Let's remove those plug-ins then if the code sucks that much. Are they in anyway useful at all?
The behvavior for the empty source range is not well-defined. Basically you are asking to map a single value to a range of values. The current behavior is definitely not very reasonable. I tried to improve it, closing as FIXED. 2007-05-06 Sven Neumann <sven@gimp.org> * plug-ins/common/mapcolor.c: try to do something reasonable for the empty source range (bug #436145).
Adjust FG-BG is still broken. It claims to map FG to black, BG to white. But this is still not true if a = b for some channel.
The implementation of the plug-in is pretty much broken as it treats the color channels independently. This simply can't do what Adjust FG-BG advertizes to achieve. I seriously suggest that we remove Adjust FG-BG then. The functionality is available in the Levels tool. Color Range mapping, the other menu entry created by this plug-in could stay if someone sees a need for it. But it suffers from a similar problem.
This doesn't need to block the release.
Since the UI team also concluded that these plugins are badly broken, I have followed the recommendation in comment #4 and removed them. If somebody fixes them, perhaps they can come back. 2007-12-14 Bill Skaggs <weskaggs@primate.ucdavis.edu> * plug-ins/common/mapcolor.c: removed * plug-ins/common/plugin-defs.pl * plug-ins/common/Makefile.am: updated accordingly See bug #436145 for the reasons for removing these plug-ins. Resolving as FIXED. Anybody who wants to work on these should feel free to reopen this bug report.
We usually keep the plug-ins and only remove their menu entry. The reason is that scripts or other plug-ins may rely on the functionality provided. So removing a plug-in is essentially an API break. You should have discussed this before removing the plug-in from SVN. But in this particular case, I think that removing it was the best choice and I support it.