GNOME Bugzilla – Bug 586316
Levels tool does not adjust output levels correctly if input levels are changed
Last modified: 2009-07-16 20:32:23 UTC
Please describe the problem: Using the Levels tool to adjust the input levels and output levels at the same time gives incorrect results. Whenever the Output Levels sliders are moved, the tonal range (histogram) of the selected channel should be squeezed towards the center, regardless of whether the Input Levels sliders have been moved. After setting the black (white) output level, the channel should contain no black (white) pixels. However, if a user adjusts the input *and* output levels, there will be black (white) pixels unless the adjusted output level range is inside the adjusted input level range. Steps to reproduce: 1. Create a new image (grayscale or RGB, but as an example here I use grayscale). 2. Fill the image with different shades of gray, e.g. using Filters->Render->Clouds->Solid Noise. 3. Open Tools->Color Tools->Levels. 4. Set Input Levels to an arbitrary range, for example: black 120, gamma 1.0, white 136 (a tight range gives large black and white areas). 5. Without closing the Levels dialog, start dragging the black or white Output Levels sliders towards the center. Actual results: Wider gray areas appear between the black and white areas, but black stays black and white stays white. Expected results: Black (white) areas should begin to turn gray as soon as the black (white) slider is moved (or a non-zero value is inserted). In other words, the tonal range set by the Input Levels should always be squeezed between the black and white Output Levels settings. Does this happen every time? Yes. Other information: If a user needs to adjust both input and output levels, he/she can first set the input levels, click OK, reopen the Levels dialog and set the output levels, but currently the only way to do both at the same time is to use the Curves tool. I'm sure this is not an OS issue, but for the record, I'm using GIMP 2.6.6 on Windows XP SP2 and I've seen the same thing happen in 2.4.5 in Xubuntu 8.04.
I see this behavior also on Gimp 2.6.5 on Windows XP SP3 and on Gimp 2.6.1 on Ubuntu 8.10. Additionally, if you switch to the Curves dialog from the Levels dialog, the settings (and preview) is correctly updated. A recipe to illustrate this: 1. Open an image 2. Open the Levels dialog. 3. Set input black point to 120, input white point to 135. (highly contrasty as it should be). 4. Set output black point to 120, output white point to 135. (image is back to unmodified! It should be almost uniform grey). 5. Click "Edit these settings as Curves"--now in the curves dialog the curve is correct and the preview is correct (nearly uniform grey). This occurs whether the channel setting is "Value" or one of red, green, blue.
I tried to track this down in the source code, and got this far: gimp_levels_tool_config_notify() in app/tools/gimplevelstool.c controls the update of the preview image when the levels are changed in the levels dialog. gimp_levels_tool_config_notify() calls gimp_image_map_tool_preview() to update the preview. gimp_image_map_tool_preview() is in app/tools/gimpimagemaptool.c, and it calls gimp_image_map_tool_map() (also in app/tools/gimpimagemaptool.c), which in turn calls gimp_image_map_apply() (which is in app/core/gimpimagemap.c). I get lost in the code for gimp_image_map_apply(), but this seems to be the path where the levels are not being correctly applied. Hope this helps, because I will find it difficult to diagnose further.
Created attachment 138375 [details] [review] Patch to correct app/base/levels.c
Created attachment 138376 [details] [review] Patch to correct app/gegl/gimpoperationlevels.c
Comment #3 and #4 are the patches to fix this bug. The basic correction is to clip the LUT values to the new black point and white point range after applying the input value changes, but before applying gamma or output value changes. This had the side effect of removing a comparison in the gamma adjustment (since the LUT value could never be below zero after clipping). The patches affect the two files: app/base/levels.c app/gegl/gimpoperationlevels.c Please apply these bug fix patches before the next release! Thanks, -Mason
It would make our life a lot easier if you would use git and base your patches on the master branch and also format them using git. But of course we can also work with the patches you applied. We just need to know the name and email address that we should credit for this change. Let's consider these patches for 2.6. Mason, it would be great if you could redo your patches and use the CLAMP() macro for better readability.
Created attachment 138427 [details] [review] Combined patch to correct levels problem (based on git master) Updated using CLAMP macro, and based off most recent git clone.
OK, patches redone and combined into one (see comment #7). The patch is based off the most recent git master. Use my bugzilla information for email. By the way, the gimp.org page on submitting patches (http://www.gimp.org/bugs/howtos/submit-patch.html) describes using CVS...is CVS still being used by gimp? -Mason
That page is out of date, I have updated it for git now: http://git.gnome.org/cgit/gimp-web/tree/bugs/howtos/submit-patch.htrw?id=cbeed47a7932837db5e7c05e86865e072f5debd8 Note that we want patches generated with git format-patch origin/master, not a normal diff. Would be great if you could regenerate your patch with git format-patch and reattach it here.
Created attachment 138498 [details] [review] Updated patch to correct levels problem. OK, I hope this is in the correct format now. A lot of hoops to jump through for a small patch!
Pushed and merged to the gimp-2-6 branch: commit fc66ca5169aea63c0f52ff799511c644c4661eba Author: Mason Thomas <mason_thomas@hotmail.com> Date: Wed Jul 15 22:50:06 2009 -0700 Clamp levels after input changes applied. app/base/levels.c | 8 ++++---- app/gegl/gimpoperationlevels.c | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) Thanks a lot for your help with this problem. Closing as FIXED.