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 109161 - Improve Histogram with Luminance Channel
Improve Histogram with Luminance Channel
Status: RESOLVED FIXED
Product: GIMP
Classification: Other
Component: General
2.8.0
Other All
: Normal enhancement
: 2.10
Assigned To: GIMP Bugs
GIMP Bugs
: 315616 496102 674819 (view as bug list)
Depends on:
Blocks: 750940
 
 
Reported: 2003-03-25 16:07 UTC by Jack Zagaja
Modified: 2017-02-15 08:49 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
replace histogram Value channel by a Luminosity channel (41.82 KB, patch)
2015-03-26 17:56 UTC, Thomas Manni
none Details | Review
add Luminance histogram channel (20.51 KB, patch)
2015-06-24 15:03 UTC, Thomas Manni
none Details | Review
add Luminance histogram channel (22.39 KB, patch)
2015-11-02 11:10 UTC, Thomas Manni
committed Details | Review

Description Jack Zagaja 2003-03-25 16:07:03 UTC
The main property of histogram in those days is an exposure determination, 
and tone mapping visualization. If GIMP uses a value method then all 
calculations (Mean, Median) are different from what photographers need. 
Photographers are familiar with tone zones and linear histogram works in 
this respect as it should.

The main purpose of this post is not a mathematical proving of what we 
want to do. We just request for quick implementation. Below we cite a 
voice form colour expert – prof. Gernot Hoffmann (http://www.fho-
emden.de/~hoffmann/):

“"Value" belongs to the HSV color model. Really bad. You may
use the HLS Lightness or the CieLab Lightness. The latter requires
much more calculations.

The log. presentation doesn’t make much sense in image processing,
IMO. The purpose of a Histogram is: visualize a good or bad distribution.

For example in my digital camera Nikon D100, I can immediately
check the exposure. Because a CCD array works primarily linearly,
a log. conversion wouldn’t  be very helpful.”

Regards – JZ.
Comment 1 Alan Horkan 2003-07-23 18:38:11 UTC
Changes at the request of Dave Neary on the developer mailing list.  
I am changing many of the bugzilla reports that have not specified a target
milestone to Future milestone.  Hope that is acceptable.  
Comment 2 Sven Neumann 2005-09-13 21:21:28 UTC
*** Bug 315616 has been marked as a duplicate of this bug. ***
Comment 3 s5t1e3v4e3m11 2006-09-19 15:03:07 UTC
>The log. presentation doesn't make much sense in image processing, IMO.

Maybe photographers don't need it; I sometimes have to look at the difference of two (very similar) pictures and the logarithmic scale helps showing where the remainders are which otherwise would be difficult to find in a linear scale.
So for me this feature is VERY useful.

Although not a photographer I took a look at Irfanview with the histogram plugin from www.reindeergraphics.com (or look at
http://www.irfanview-online.com/downloads.php?view=detail&id=6 )
Their Hue and Saturation histogram are also useful for me with my "difference pictures".

As the original post is very old I guess in the past GIMP didn't offer a linear scale?
Comment 4 weskaggs 2006-09-19 16:34:28 UTC
I think comment #3 misunderstands what this bug report is about -- it is about the difference between Value and Luminance (two different ways of measuring pixel brightness), not the difference between linear and logarithmic.  
Comment 5 s5t1e3v4e3m11 2006-09-20 06:40:39 UTC
Re-reading my post I have to admit that I obviously was reading too much between the lines: I latched onto "The log. presentation doesn't make much sense in image processing, IMO" and saw the request for removal of this (for me very useful) feature (but the original poster didn't write that).

When I saw "HLS" I read between the lines that this bug also calls for implementing the Hue and Saturation Channel as well so I won't need to create a new bug (which then might be marked as duplicate; but again the original poster didn't write that).

So maybe you better ignore my comment here and I should create a new feature request calling for implementing a Hue and a Saturation Channel in the Histogram.
Comment 6 Sven Neumann 2007-11-12 12:36:22 UTC
*** Bug 496102 has been marked as a duplicate of this bug. ***
Comment 7 Michael Natterer 2012-05-07 23:54:18 UTC
*** Bug 674819 has been marked as a duplicate of this bug. ***
Comment 8 Michael Natterer 2012-05-07 23:55:41 UTC
This is really a joke to implement, let's consider it for 2.10.
Comment 9 Thomas Manni 2015-03-26 17:56:40 UTC
Created attachment 300380 [details] [review]
replace histogram Value channel by a Luminosity channel

Patch content:

- replace the histogram "Value" channel by a "Luminosity" channel
- compute luminosity channel via GIMP_RGB_LUMINANCE
- rename GIMP_HISTOGRAM_VALUE enum by GIMP_HISTOGRAM_LUMINOSITY
- update app/... and plug-ins files impacted by the enum change
Comment 10 Michael Natterer 2015-03-26 21:50:46 UTC
Nice patch, but we cannot change the meaning and name of enum values, we can
only append to enums :/, at least as long this is GIMP 2.x.
Comment 11 Thomas Manni 2015-05-03 21:31:28 UTC
So, what's next ? I see 3 possibilities :

1) create an additional GimpHistogramChannel to compute luminance, appended a GIMP_HISTOGRAM_LUMINOSITY after GIMP_HISTOGRAM_RGB ;) (and, maybe, set this channel as default choice for levels and curves tools)

2) modify the way the GIMP_HISTOGRAM_VALUE is computed (now, using luminance) and replacing "Value" by "Luminosity" in the combobox channels of the tools.

3) do nothing and set this bug report to milestone 3.0
Comment 12 Elle Stone 2015-05-20 13:08:54 UTC
Thomas Manni's patch replaces the Value histogram with the luminance histogram calculated the same as what you would get choosing Colors/desaturate/luminosity.

In terms of communicating to the user the actual distribution of relative luminance, the luminance histogram is exactly what's needed, as neither minimum nor maximum convey truthful information about luminance.

I find the Value histogram to be useful precisely because it does show me the maximum of the R, G, and B channels. I think it should be kept, and relabelled "Maximum". This histogram allows the user to adjust the image white point without worrying about clipping any channels.

It would be also be useful to add a histogram for the minimum of the R, G, and B channels, labelled "Minimum". If you use the Value channel to set the image black point, you risk clipping a lot of pixels in two of their three channels, and you have to check the R, G, and B channels individually to see what the maximum "no clip" black point really is.

It might be nice to also add the Lightness histogram from LAB/LCH.

So altogether that would make Maximum (same as Value), Minimum, Luminance, maybe Lightness, Red, Green, Blue, and Alpha histograms, with the default being Luminance.

A terminology note: Many people talk about luminance and luminosity as if they were the same thing. But they aren't: http://en.wikipedia.org/wiki/Luminosity_%28disambiguation%29

GIMP terminology would be considerably more correct than PhotoShop terminology if anything that really is a calculation of relative luminance were called "luminance" instead of "luminosity".
Comment 13 Elle Stone 2015-06-14 17:01:10 UTC
(In reply to Elle Stone from comment #12)

> I find the Value histogram to be useful precisely because it does show me
> the maximum of the R, G, and B channels. I think it should be kept, and
> relabelled "Maximum". This histogram allows the user to adjust the image
> white point without worrying about clipping any channels.
> 

Actually, as per https://bugzilla.gnome.org/show_bug.cgi?id=750940, seeing the RGB histogram - all three channels at once - is a lot more useful and simple to use than seeing the Max (Value) (and Min, if it were added) as individually displayed channels. The Luminance histogram would make a nice back-drop to the RGB histogram. But when modifying all three channels by the same amount, it's really important to be able to see precisely which individual channels might be clipped by the adjustment, and by how much.
Comment 14 Michael Natterer 2015-06-14 19:19:47 UTC
Re comment 11: we can only add an additional luminance enum value,
at the end of the enum. That would be a nice addition. Then we can
look at elle's patch in bug 750940.
Comment 15 Michael Natterer 2015-06-14 19:26:32 UTC
Actually, it's more complicated: the new value needs to be added BEFORE
GIMP_HISTOGRAM_RGB, because that one only exists in the core and is
not explorted to plug-ins. Then ALL places where GimpHistogramChannel
is used (and abused) in the core need to be checked...
Comment 16 Thomas Manni 2015-06-24 15:03:13 UTC
Created attachment 306011 [details] [review]
add Luminance histogram channel

Add luminance histogram channel as asked in comment 15.
Comment 17 Michael Schumacher 2015-11-01 01:34:31 UTC
There are two instances of trailing whitespace
(both lines "+      channel != GIMP_HISTOGRAM_LUMINANCE && ")

Easily fixed after applying the patch. Is there anything else?
Comment 18 Elle Stone 2015-11-01 10:28:28 UTC
After applying the patch, it's nice to see "Luminance" instead of Value.

(In reply to Elle Stone from comment #12)
> It might be nice to also add the Lightness histogram from LAB/LCH.

Sorry, my mistake. Lightness converted to RGB *is* Luminance. I was relying on visual feedback from back when I used PhotoShop, and didn't realize until working with GIMP that PhotoShop's Channel View of the LAB channels changes with the user's chosen "gamma" for gray channels (http://ninedegreesbelow.com/photography/lab-lightness-to-black-and-white-gimp29-photoshop.html#photoshop-tutorial-mathematical-mistakes).
Comment 19 Michael Schumacher 2015-11-01 22:33:46 UTC
I get a warning there:

gimplevelsconfig.c: In function ‘gimp_levels_config_input_from_color’:
gimplevelsconfig.c:513:3: warning: enumeration value ‘GIMP_HISTOGRAM_LUMINANCE’ not handled in switch [-Wswitch]
   switch (channel)
   ^
Comment 20 Michael Schumacher 2015-11-01 22:56:03 UTC
And 

gimpcurvestool.c: In function ‘gimp_curves_tool_update_channel’:
gimpcurvestool.c:670:3: warning: enumeration value ‘GIMP_HISTOGRAM_LUMINANCE’ not handled in switch [-Wswitch]
   switch (config->channel)
   ^
gimpcurvestool.c: In function ‘curves_menu_sensitivity’:
gimpcurvestool.c:783:3: warning: enumeration value ‘GIMP_HISTOGRAM_LUMINANCE’ not handled in switch [-Wswitch]
   switch (channel)
   ^


gimplevelstool.c: In function ‘levels_update_input_bar’:
gimplevelstool.c:822:3: warning: enumeration value ‘GIMP_HISTOGRAM_LUMINANCE’ not handled in switch [-Wswitch]
   switch (config->channel)
   ^
gimplevelstool.c: In function ‘levels_menu_sensitivity’:
gimplevelstool.c:909:3: warning: enumeration value ‘GIMP_HISTOGRAM_LUMINANCE’ not handled in switch [-Wswitch]
   switch (channel)
   ^
Comment 21 Thomas Manni 2015-11-02 11:10:56 UTC
Created attachment 314625 [details] [review]
add Luminance histogram channel

fix trailing whitespace and unhandled switches in levels and curves tools.
Comment 22 Michael Natterer 2016-04-18 18:37:13 UTC
Thanks, pushed to master:

commit f4cb2dd881a107c0ac02493ef8baf1ff4a620e66
Author: Thomas Manni <thomas.manni@free.fr>
Date:   Mon Nov 2 08:53:50 2015 +0100

    Bug 109161 - Improve Histogram with Luminance Channel
    
    Add a luminance channel to GimpHistogram

 app/core/core-enums.c               |  2 ++
 app/core/core-enums.h               | 13 +++++-----
 app/core/gimphistogram.c            | 76 +++++++++++++++++++++++++++++++++++++++----------------
 app/operations/gimplevelsconfig.c   |  3 +++
 app/pdb/color-cmds.c                |  7 +++--
 app/pdb/drawable-color-cmds.c       |  9 ++++---
 app/tools/gimpcurvestool.c          |  4 +++
 app/tools/gimplevelstool.c          |  4 +++
 app/widgets/gimpcolorbar.c          |  1 +
 app/widgets/gimphistogrameditor.c   |  1 +
 libgimp/gimpenums.h                 |  3 ++-
 tools/pdbgen/enums.pl               |  5 ++--
 tools/pdbgen/pdb/color.pdb          |  7 +++--
 tools/pdbgen/pdb/drawable_color.pdb |  9 ++++---
 14 files changed, 103 insertions(+), 41 deletions(-)