After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 541859 - Colour Balance Range to Adjust has no Effect
Colour Balance Range to Adjust has no Effect
Status: RESOLVED FIXED
Product: GIMP
Classification: Other
Component: Tools
2.4.x
Other All
: Normal normal
: 2.8
Assigned To: GIMP Bugs
GIMP Bugs
: 593498 642800 666933 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2008-07-07 10:42 UTC by Richard Driscoll
Modified: 2012-03-07 20:53 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Originall Image (247.43 KB, image/jpeg)
2008-07-07 11:24 UTC, Richard Driscoll
  Details
+15 blue highlights (244.63 KB, image/jpeg)
2008-07-07 11:25 UTC, Richard Driscoll
  Details
+15 blue shadows (240.71 KB, image/jpeg)
2008-07-07 11:26 UTC, Richard Driscoll
  Details
test image as sent to the mailing-list (10.97 KB, image/xcf)
2008-07-13 20:31 UTC, Sven Neumann
  Details
sample picture, "color balance" test, preserve luminosity selected (29.19 KB, image/jpeg)
2011-02-22 15:40 UTC, dw
  Details
sample picture, "color balance" test, preserve luminosity deselected (29.63 KB, image/jpeg)
2011-02-22 15:41 UTC, dw
  Details
patch (3.18 KB, patch)
2012-01-09 22:41 UTC, Alexis Wilhelm
none Details | Review
patch (3.26 KB, patch)
2012-01-10 18:43 UTC, Alexis Wilhelm
none Details | Review
patch (6.92 KB, patch)
2012-01-10 21:12 UTC, Alexis Wilhelm
none Details | Review
patch (8.75 KB, patch)
2012-01-12 21:09 UTC, Alexis Wilhelm
none Details | Review
illustration of comment 27 (6.00 KB, image/png)
2012-01-12 21:26 UTC, Alexis Wilhelm
  Details
translation vs scaling (17.11 KB, image/png)
2012-01-20 17:51 UTC, Alexis Wilhelm
  Details
patch 1: translation (8.86 KB, patch)
2012-01-20 17:52 UTC, Alexis Wilhelm
none Details | Review
patch 2: scaling (8.91 KB, patch)
2012-01-20 17:53 UTC, Alexis Wilhelm
none Details | Review
patch 1: translation (8.92 KB, patch)
2012-02-15 20:19 UTC, Alexis Wilhelm
committed Details | Review

Description Richard Driscoll 2008-07-07 10:42:28 UTC
When using the Colours->Colour Balance tool, the Shadows, Midtones and Highlighs radio buttons seem to have no effect.

Specially a photograph modified with Highlights, Blue +15 looks just the same as one modified with Shadows, Blue +15.

I would have expected that Highlights, Blue +15 would make the bright sky more blue but leave dark parts of the photograph substantially unchanged in colour balance.

I can upload examples if requested.
Comment 1 Sven Neumann 2008-07-07 10:55:35 UTC
Yes, please attach a small example image that we can use to reproduce the problem.
Comment 2 Richard Driscoll 2008-07-07 11:24:31 UTC
Created attachment 114106 [details]
Originall Image
Comment 3 Richard Driscoll 2008-07-07 11:25:33 UTC
Created attachment 114107 [details]
+15 blue highlights
Comment 4 Richard Driscoll 2008-07-07 11:26:45 UTC
Created attachment 114108 [details]
+15 blue shadows
Comment 5 Richard Driscoll 2008-07-07 11:31:07 UTC
OS is Suse linux 10.3. This version of GIMP is an RPM from the Suse Packman repository.
Comment 6 Sven Neumann 2008-07-07 16:04:43 UTC
Why does noone ever understand what a small image is? A few pixels should be sufficient to illustrate such a problem.
Comment 7 Sven Neumann 2008-07-07 16:13:51 UTC
If you compare the two results you attached, you will notice that they are not equal. The differences are subtle. But your bug report is incorrect, the chosen range has an effect.

I am willing to discuss how we can improve this, but please let's have this discussion on the mailing-list. The best thing you can do is to propose a formula that works better than what is currently in use.
Comment 8 Richard Driscoll 2008-07-09 09:24:56 UTC
Sorry you found examples too large, I was trying to demonstrate how poor this tool is for a photographer.

I still think there is a real problem here. I have a synthetic example consisting of three grey patches of differing luminance values. XCF is about 20 k bytes.

When I use the same +50 blue value for highlight and shadow, the two resulting xcf output files are *identical* as verified by the cmp command. (Midtone seems same too).

Comment 9 Sven Neumann 2008-07-10 06:32:52 UTC
Please send a mail to the gimp-developer list then. Or, if you can't do that, for whatever reason, attach the XCF file here.
Comment 10 Sven Neumann 2008-07-13 20:29:09 UTC
Let's reopen this. It looks like we should look at this for 2.6. There's a discrepancy of the legacy and the GEGL code that should be sorted out.
Comment 11 Sven Neumann 2008-07-13 20:31:08 UTC
Created attachment 114489 [details]
test image as sent to the mailing-list
Comment 12 Martin Nordholts 2008-09-06 07:54:19 UTC
Link to mailing-list thread:
https://lists.xcf.berkeley.edu/lists/gimp-developer/2008-July/020480.html
Comment 13 Michael Schumacher 2009-07-01 08:06:07 UTC
No unconfirmed bugs with milestones, please.
Comment 14 Martin Nordholts 2009-08-29 17:07:55 UTC
*** Bug 593498 has been marked as a duplicate of this bug. ***
Comment 15 Roger.Duncan59 2009-08-29 21:20:44 UTC
Seems that this obvious problem is still persistent in Version 2.6.7 and nobody cared about it for over 1 year.
Comment 16 LightningIsMyName 2011-02-22 15:10:13 UTC
*** Bug 642800 has been marked as a duplicate of this bug. ***
Comment 17 dw 2011-02-22 15:40:19 UTC
Created attachment 181595 [details]
sample picture, "color balance" test, preserve luminosity selected
Comment 18 dw 2011-02-22 15:41:39 UTC
Created attachment 181596 [details]
sample picture, "color balance" test, preserve luminosity deselected
Comment 19 dw 2011-02-22 15:41:47 UTC
The GIMP manual says regarding filter "color balance":

"Select range to adjust

Selecting one of these options will restrict the range of colors which are
changed with the sliders or input boxes for Shadows (darkest pixels), Midtones
(medium pixels) and Highlights (brightest pixels)."

However, selecting "shadows", moving the slides has little impact on the dark
pixels, but changes the bright pixels in a drastic way. This behavior, however,
is not expected with the selection of "shadows".

I added two sample picture. They differ in the selection of "preserve
luminosity". There are 4 layers. The top layer (layer A) is the origin, a white
to black gradient blend. The three layers below have been subject to the
appliance of "color balance", with "highlights", "midtones" and "shadows"
selected respectively.

As you can see, the result looks quite different from what would be expected.
Especially the "highlights" and "shadows" layers look quite the same.

Once again: The filter works faulty. Highlights are affected when selecting "shadows" and vice versa. This is a wrong behavior and constitutes a bug in the sense of a misprogrammed feature.
Comment 20 Michael Schumacher 2011-12-28 19:42:23 UTC
*** Bug 666933 has been marked as a duplicate of this bug. ***
Comment 21 Michael Natterer 2012-01-08 21:19:38 UTC
2.6 -> 2.8
Comment 22 Alexis Wilhelm 2012-01-09 22:41:38 UTC
Created attachment 204910 [details] [review]
patch

I noticed some surprisingly strange things, so maybe I'm just missing something.

In color_balance_create_lookup_tables() r_n, g_n and b_n were used as subscript for the xx_yy_transfer arrays, which I'm pretty sure is wrong, because then the corrections in the 3 ranges would not be independent.

In color_balance_transfer_init(), the formula for the 'low' variable was wrong (see this plot http://www.wolframalpha.com/input/?i=1.075+-+1+%2F+%28x+%2F+16+%2B+1%29+for+x%3D0..255) so I replaced it with a parabola just like the 'mid' variable, and then I tweaked the array subscripts until it looked right.
Comment 23 Alexis Wilhelm 2012-01-10 18:43:10 UTC
Created attachment 204974 [details] [review]
patch

Using simple linear ramps instead of parabolas gives this nice property :

  cyan_red_transfer[GIMP_SHADOWS][i] + cyan_red_transfer[GIMP_MIDTONES][i] +
  cyan_red_transfer[GIMP_HIGHLIGHTS][i] = 1 for i in 0..255.

This means that applying the same correction in the shadows and in the midtones is equivalent to applying this correction on a virtual GIMP_SHADOWS_AND_MIDTONES range.
Comment 24 Michael Natterer 2012-01-10 18:54:39 UTC
That all sounds very promising, but you are only changing the legacy code
so far ;) Can you do the same to the operation in app/gegl/?
Comment 25 Alexis Wilhelm 2012-01-10 21:12:32 UTC
Created attachment 204983 [details] [review]
patch

Deleted lots of obsolete code. Feels good :)

Also, now it works with GEGL.
Comment 26 Michael Natterer 2012-01-11 08:09:02 UTC
And it looks good :) You rock (given the code is correct), will have to try...
Comment 27 Alexis Wilhelm 2012-01-12 21:09:49 UTC
Created attachment 205142 [details] [review]
patch

My previous patch introduced (or at least made clearly apparent) a weird behavior when suppressing the red channel for highlights in a black -> red -> white gradient: the gradient became black -> mid-red -> dark-red -> mid-red -> cyan, but it should be black -> red -> cyan. This is because we subtract a uniform value. It is hard to explain with words, and even harder when those words are in English, so you will have to plot the curves or test it.

This new patch fixes this bug in the GEGL operation. When the correction is negative, it truncates the low, mid and high masks so that they stay under the y = x line. This means that, when the correction is negative, the correction is proportional to the value, whereas when the correction is positive it is uniform.

Also, now it decides if a pixel is in the shadows, midtones or highlights based on its lightness rather than on its individual channels values. It seems more natural that way.

I could not apply the same fix in color-balance.c, because it uses a static lookup table.
Comment 28 Alexis Wilhelm 2012-01-12 21:26:09 UTC
Created attachment 205145 [details]
illustration of comment 27

cyan_red[GIMP_HIGHLIGHTS] = -100, preserve_luminosity = false.
Comment 29 Alexis Wilhelm 2012-01-20 17:51:47 UTC
Created attachment 205712 [details]
translation vs scaling

After a whole week, I still cannot say clearly why subtracting a uniform correction to the value looks wrong, nor why scaling it looks better, so I am not sure anymore if it is indeed better.

The attached image compares the effect of a translation and of a scaling, with a positive or a negative correction, in the midtones, in the red channel. What bugs me is that with a translation the correction seems to be shifted left.
Comment 30 Alexis Wilhelm 2012-01-20 17:52:40 UTC
Created attachment 205713 [details] [review]
patch 1: translation
Comment 31 Alexis Wilhelm 2012-01-20 17:53:15 UTC
Created attachment 205714 [details] [review]
patch 2: scaling
Comment 32 Michael Natterer 2012-02-12 20:27:38 UTC
I totally forgot about these patches. Alexis, you should really come to
IRC, reminding works best where all the action happens ;)

Did you come to a conclusion about which method is more "correct" here?
Comment 33 Alexis Wilhelm 2012-02-13 18:12:50 UTC
The truth is I never used this tool before, so I do not know what it is supposed to do (I mostly use levels, masks and layer modes). I looked at the GUI and the code and inferred what the original authors probably wanted to do, but it would feel arrogant to say exactly what it was. Is there a real-life equivalent of this tool in photography/cinema?

I am no photograph, but if I were to do it with a 198x camera, I would put colored filters on the lens, so the colors in my image would be translated.

Also, the current code does a translation, so in doubt I would keep the same method.
Comment 34 Alexis Wilhelm 2012-02-15 20:19:16 UTC
Created attachment 207699 [details] [review]
patch 1: translation

In this new patch, the masks are scaled up in the legacy code and down in the GEGL code, so now both are equally powerful.
Comment 35 Przemysław Gołąb (n-pigeon) 2012-03-05 23:33:46 UTC
I've tested new patches. Patch 1: translation, works nice, in my opinion of course. 

Legacy and GEGL modes also work as expected.
Comment 36 Michael Natterer 2012-03-07 20:53:15 UTC
Fixed in master, thanks for the great work!

commit c7c752f5df77ecce57779e4caa3b683192c22435
Author: Alexis Wilhelm <alexiswilhelm@gmail.com>
Date:   Wed Feb 15 21:00:27 2012 +0100

    Bug 541859 - Colour Balance Range to Adjust has no Effect
    
    This makes the different ranges have a clearly different effect,
    plus speeds up the gegl op quite a lot.
    
    NOTE: this might affect the result of scripts using color-balance

 app/base/color-balance.c             |  105 +++++++---------------------------
 app/gegl/gimpoperationcolorbalance.c |   62 ++++++++++----------
 2 files changed, 52 insertions(+), 115 deletions(-)