GNOME Bugzilla – Bug 762443
Levels tool Output Level sliders works incorrectly
Last modified: 2018-01-02 17:51:04 UTC
Output Levels sliders does not do what it should. Steps to reproduce: 1. Create a new image 2. Fill the image with Filters->Render->Noise->Solid Noise. 3. Open Colours->Levels. 4. Set Input Levels: black 50, gamma 1.0, white 200 (a tight range gives large black and white areas). 5. Set Output Levels: black 50, gamma 1.0, white 200. What should happen: This operation should work as follows. First editing Input Levels should create big splotches of pure white and pure black. Then, editing Output Levels should make those pure black and white splotches dark grey and light grey. What happens: If you turn Preview on and off you will see that both look the same, Output levels cancels input levels edits. That should happen. Output levels are for trimming min and max values. Side note: Current Output levels works like Lightness from Hue-Saturation tool, but on current value scale. This is actually pretty handy sometimes. My suggestion would be to rename current Output Levels to Lightness or something similar, move it to bottom and create proper Output Levels slider.
* That shouldn't happen. typo in "What happens" section :(
It seems that this CLAMP https://git.gnome.org/browse/gimp/tree/app/base/levels.c?h=gimp-2-8#n93 has been lost when ported to GEGL: https://git.gnome.org/browse/gimp/tree/app/operations/gimpoperationlevels.c#n90 I don't know whether that was intentional or not (git log on that file shows a commit mentioning HDR images suggesting values < 0 have been considered). Anyway, if the old behaviour is desired, the fix is simple
(In reply to Massimo from comment #2) > Anyway, if the old behaviour is desired, the fix is simple Yes, the clamp behaviour is how it should work. Although, as I mentioned in side note, this new type of slider in Levels is also useful tool and could be attractive new addition to plain old levels :)
So anyone who has a clue about what this operation is meant to do is welcome to step in. :-) Was this removed CLAMP a bug in the end? Or quite the opposite, is it an improvement (for HDR images or whatever)? If you want to see this in 2.10, please react soon. Personally I have no idea what is expected in this op.
CCing Elle as the CLAMP()ing expert in residence.
(Elle was trying to comment and hit a collision :) ) Please do *not* add 2.8-style clamping code back into 2.9 Levels as any kind of default behavior. Unclamped Levels is incredibly useful for general and also HDR editing. 1. At integer precision, the old behavior can be duplicated in 2.9 by applying the upper slider values, closing and then reopening Levels, and then applying the lower slider values. 2. At floating point precision, the old behavior can be duplicated in 2.9 by: applying the upper slider values closing Levels applying RGB clip opening Levels applying the lower slider values closing Levels applying RGB clip Yes 1 and 2 are both more cumbersome than having the code clamp as it does in 2.8, if one's goal is to duplicate 2.8 behavior. But it's doable. If the old Levels clamp is put back in the code, then please make this old behavior *optional*: * The old clamping behavior makes it impossible to properly white balance an image using GIMP 2.8 (though currently even in GIMP 2.9 this actually still isn't possible unless you use the gamma hack, because proper white balancing requires linear RGB - see Bug 740153 and Bug 757444 ). * The old clamping behavior would entirely negate HDR editing using Levels. * Personally I use the unclamped capability of Levels a lot, all the time. In fact my entire workflow with GIMP 2.9 would be undermined if Levels were suddenly clamped. It's the first new thing I discovered about 2.9 back in 2012/2013, and I've been using it ever since. I even wrote a GIMP 2.9 tutorial on using Levels to pull shadow details up while preserving highlight details (https://www.gimp.org/tutorials/Tone_Mapping_Using_GIMP_Levels/ - the tutorial actually uses Exposure because GIMP 2.9 Levels still operates on perceptually uniform RGB instead of operating on linear RGB as it should by default to avoid gamma artifacts). A checkbox to optionally clamp results of the upper slider before apply the lower slider might be nice to have. A checkbox to also optionally clamp results of the the lower slider, might also be nice. In fact I would love to see the Colors/RGB clip operation be incorporated into the Levels UI as there are many times at 32f when clipping (usually shadows but sometimes also/instead highlights) is necessary for various editing purposes.
(In reply to Elle Stone from comment #6) Short version: * Please don't clamp Levels by default. * Perhaps add two checkboxes: * To optionally clamp results of the upper slider slider before performing the lower slider * To optionally clamp results of the entire Levels operation. These two checkboxes would operate individually, and at floating point precision both would need to be checked to produce 2.8 clamping behavior. Perhaps better yet, incorporate the "Colors/RGB Clip" behavior as UI options within the Levels dialog. But this would require a more complicated UI than simple "highlights and shadows both clipped" clamping via checkboxes.
That sounds like a pretty good plan. "RGB Clip" is so trivial, it wouldn't need to be used by levels, we'd just use a more variable CLAMP() or MIN/MAX instruction. In any case, we need to restore the traditional functionality for the plug-in interface, or scripts will get different results.
For info, Elle, if you feel like it, do not hesitate to write a patch implementing comments 7 and 8. Let's try and get 2.10 roll! :-)
This changes behavior of a very common PDB function and should be addressed for 2.10.
This should do it. I'm not totally happy with the toggle buttons, but wanted to place them according to the flow of pixels through the operation, while not making the dialog bigger... maybe not really feasible without looking creepy :) commit dce93c7d7e012ba9b85766e13d4f6dd62e21ec8d Author: Michael Natterer <mitch@gimp.org> Date: Tue Jan 2 18:42:21 2018 +0100 Bug 762443 - Levels tool Output Level sliders works incorrectly Add "clamp-input" (which clamps the input values to [0..1]) and "clamp-output" (which clips the final result to [0..1]), properties, parameters and GUI to: - GimpLevelsConfig - GimpOperationLevels - The levels tool dialog - The gimp_drawable_levels() PDB API The old deprecated gimp_levels() PDB API now sets both clamping options to TRUE which restores the 2.8 behavior. Also reorder some stuff in GimpLevelsConfig and elsewhere so the levels parameters are always in the same order. app/operations/gimplevelsconfig.c | 91 +++++++++++++++++++++++++++--------- app/operations/gimplevelsconfig.h | 8 +++- app/operations/gimpoperationlevels.c | 32 +++++++++---- app/pdb/color-cmds.c | 12 +++-- app/pdb/drawable-color-cmds.c | 34 ++++++++++---- app/tools/gimplevelstool.c | 19 +++++++- libgimp/gimpdrawablecolor_pdb.c | 8 +++- libgimp/gimpdrawablecolor_pdb.h | 4 +- pdb/groups/color.pdb | 12 +++-- pdb/groups/drawable_color.pdb | 18 ++++--- 10 files changed, 176 insertions(+), 62 deletions(-)