Bug 148382 - overexposure indication already when fully "saturated"
Summary: overexposure indication already when fully "saturated"
Status: RESOLVED FIXED
Alias: None
Product: digikam
Classification: Applications
Component: ImageEditor-Canvas (show other bugs)
Version: 0.8.2
Platform: Compiled Sources Linux
: NOR wishlist
Target Milestone: ---
Assignee: Digikam Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-07-31 05:45 UTC by Alexander Neundorf
Modified: 2017-08-08 17:07 UTC (History)
4 users (show)

See Also:
Latest Commit:
Version Fixed In: 1.6.0


Attachments
test sample for displaying over/under exposure indicators (2.34 KB, image/png)
2010-01-05 20:04 UTC, Jens Mueller
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Alexander Neundorf 2007-07-31 05:45:39 UTC
Version:           0.8.2 (using KDE KDE 3.5.5)
Installed from:    Compiled From Sources

Hi,

in the digikam color tools I can enable a "overexposure indicator", which works good so far.
But it is too optimistic, it seems to only consider pixels as overexposed if they really went beyond the 255 count due to some actions in digikam. They should already be marked as overexposed if one channel hits the 255, because this means that this channel had at least 255, so the color has been lost for this pixel.

The images coming from my camera don't go up to 255, they stop at 254 (I think leaving 0xff out as valid pixel value may be the case for many cameras), so I'd say the threshold shouldn't be 255, but 254.

It would also be nice if this would work already in the thumbnail gallery, so that you can see it already in the overview where the bad images are.

Alex
Comment 1 Mikolaj Machowski 2007-07-31 08:16:13 UTC
> Version:           0.8.2 (using KDE KDE 3.5.5)

Report bugs against latest version (0.9.2)

> The images coming from my camera don't go up to 255, they stop at 254 (I
> think leaving 0xff out as valid pixel value may be the case for many
> cameras), so I'd say the threshold shouldn't be 255, but 254.

Do you have any proof of that?

m.
Comment 2 Alexander Neundorf 2007-08-01 01:35:47 UTC
Ok, I checked again, it was in the downscaled version that the values only go to 254, in the fullsize view they reach 255.

Sorry for this.

The other point is still valid (but I didn't check with current svn). Can you point me to the source file where overexposure is detected ?

Alex
Comment 3 Marcel Wiesweg 2007-08-01 16:57:03 UTC
In trunk/ it's line 1204, QImage DImg::pureColorMask(ExposureSettingsContainer *expoSettings) of libs/dimg/dimg.cpp.
The method is called from void DImgInterface::paintOnDevice, utilities/imageeditor/canvas/dimginterface.cpp
Comment 4 Alexander Neundorf 2007-08-02 04:39:06 UTC
I should really try current svn...

Anyway, changing the following line from:
 if (expoSettings->overExposureIndicator && 
     pix.red() == max && pix.green() == max && pix.blue() == max)

to 

 if (expoSettings->overExposureIndicator && 
     pix.red() == max || pix.green() == max || pix.blue() == max)

might be a good idea, so you will also catch areas where only one channel is overexposed, which also leads to bad colors, e.g. if you have an area where green would go from 200 to 400 and red and blue from 125 to 250, which would be a bright green gradient, you will instead get a change from bright green to white, since green can't go beyond the 255. Detecting these areas too would show this problem.

Not quite sure about underexposure...
Comment 5 Alexander Neundorf 2007-08-02 04:41:13 UTC
Does this actually work on the full image data or on the image which is scaled down for display ?

In the scaled down image white areas here are only 254, 254, 254 instead of full 255, 255, 255.

Comment 6 Marcel Wiesweg 2007-08-02 17:56:54 UTC
It operates on the full image. If you look at paintOnDevice(), which draws a part of the image on a specified region of a paint device, first the image is smoothScaled, then converted to pixmap (with color management applied if enabled) and drawn.
The mask is then created from the relevant part of the full image, scaled, converted to pixmap and drawn.

I am no expert on this, for any changes I suggest to wait until Gilles is back from his holidays.
Comment 7 Arnd Baecker 2007-09-01 02:02:53 UTC
Gilles, 

any feedback from your side on this issue?
In particular #4 does sound very reasonable to me.

Arnd
Comment 8 caulier.gilles 2007-09-01 09:22:40 UTC
Arnd,

Well no. Overexposure for me is to have a real white color (R= 255, G=255, B=255 for 8 bits color depth). For ex. (R=255, G=125, B= 25) is not and overexposed color...

Perhaps we need to add an overage around the limit, for exemple if R > 250 && G < 250 && B > 250

What do you think about ?

Gilles

Comment 9 Mikolaj Machowski 2007-09-01 11:42:24 UTC
Maybe two types of overexposure indicator with different colors.
Example: when red is 255 show red zebra, when green is 255 show green
zebra, etc. Only solid black (or white) when all colors are maxed.
Behaviour could be configurable.
Comment 10 Arnd Baecker 2007-09-01 14:33:48 UTC
Currently it is already  possible
to change the colors for over/under exposure.
So this would mean to add two more colors for over-exposure indication?
(do we need the same for under-exposure?, presumably not ...)

Gilles, I do think that individual R, G, B channels can be
overexposed, see e.g. 
""" You need to have a histogram that shows each of the R, G and B
channels. Single channel histograms don't show when just one color 
is overexposed."""
(from http://www.kenrockwell.com/tech/yrgb.htm)
Therefore I am not sure whether averaging is the right approach.
Any value 255 could mean that this was 256 or more, clipped to 255.

I still find #4 convincing.
Comment 11 Dik Takken 2008-10-05 14:03:30 UTC
Overexposure == clipping. When any one channel gets clipped, you have overexposure.  IMHO, #4 is the way to go.

The same thing is true for clipping on the bottom end. Take a bright color, like (250,10,10). When this color is darkened by the amount of 50, it changes to (200,0,0). That color looks quite different, due to clipping. And because it is a fairly bright color, this form of clipping is very well visible. Therefore, it should be indicated as being underexposed.

So, I'd say:

if (expoSettings->underExposureIndicator && 
     pix.red() == 0 || pix.green() == 0 || pix.blue() == 0)

Comment 12 caulier.gilles 2010-01-03 22:38:58 UTC
Jens,

What do you think about #4 and #11 ?

Gilles Caulier
Comment 13 Jens Mueller 2010-01-04 19:46:24 UTC
Hmm, quite a good question. My thoughts on this:

When we are looking at HSV color interpretation we have Color, Saturation and Value. Value (between 0.0 and 1.0) shows the lightness of a pixel, so I think this is the one. A Value of 0.0 meens underexposed and a value of 1.0 meens overexposed. 
When will Value be 0.0? If all RGB components are 0.
When will Value be 1.0? If one of the RGB components is max.

Following this argumentation we should have (R=0 && G=0 && B=0) for underexposure and (R=max || G=max || B=max) for overexposure.

But I dont know yet how other applications handle this.

Jens
Comment 14 Johannes Wienke 2010-01-04 19:57:55 UTC
I found this rather interesting discussion about clipping and over- / underexposure:
http://www.luminous-landscape.com/forum/lofiversion/index.php/t37527.html

In basic they say that luminance clipping appears if all 3 color channels reach their max value, whereas saturation clipping is reached if on color channel reaches its max value.

To my mind it looks like a good idea to provide indicators for both clipping types. The luminance version as it is now and a new saturation clipping indicator. Both can be of value for editing photos.
Comment 15 caulier.gilles 2010-01-05 07:44:34 UTC
Jens,

HSV is not exactly HSL, but in this case, we don't care.

http://en.wikipedia.org/wiki/HSL_color_space

H is not color component but the Hue.

HSL or HSL are not RGB. It's different color space.

>When will Value be 0.0? If all RGB components are 0.

yes

>When will Value be 1.0? If one of the RGB components is max.

no. R = max && G = max && B = max

Go to gimp, edit foreground color properties and select V component (Set  components H=0 and S=0). Change V from min to max, and look RGB values.

This is want mean that we don't need to convert for RGB to HSV to check under/over exposed pixels... ouf...

>Following this argumentation we should have (R=0 && G=0 && B=0) for >underexposure 

yes

>and (R=max || G=max || B=max) for overexposure.

no (R=max && G=max && B=max) 

This is the current implementation do.

Gilles
Comment 16 Jens Mueller 2010-01-05 08:29:57 UTC
@Gilles:

I know there is a difference between HSL and HSV. 

From http://en.wikipedia.org/wiki/HSL_color_space (Conversion from RGB to HSL or HSV):
in HSL L is computated as 0.5*(max(R, G, B) + min(R, G, B))
in HSV V is computated as max(R, G, B)

For my argumentation I mean exactly HSV, because this is what we implemented in histogram luminosity channel http://lxr.kde.org/source/extragear/graphics/digikam/libs/histogram/imagehistogram.cpp#244

Beside from if HSV is right this means what I say in https://bugs.kde.org/show_bug.cgi?id=148382#c13

@Johannes:
the link is quite interessting but I think the mantra should be keep it simple for users

Jens
Comment 17 Dik Takken 2010-01-05 08:54:56 UTC
>When we are looking at HSV color interpretation we have Color, Saturation and
Value. Value (between 0.0 and 1.0) shows the lightness of a pixel, so I think
this is the one.

Clipping can happen to ANY channel, no matter what color space is used. The
saturation channel can also get clipped, leading to patches of uniform
saturation in the image. This is not as bad as clipping the Value channel, but
the pixel has been damaged. Period.

It all comes down to the same thing: When *ANY* value gets clipped, the image
has been damaged. Whenever this happens during editing, I want to know
immediately. And this is exactly what an exposure indicator is for.

IMHO, it's really quite simple. Clipping == Damage. Any code that does

     if (Pixel[Channel].Value > MAX) Pixel[Channel].Value = MAX

or

     if (Pixel[Channel]Value < MIN) Pixel[Channel].Value = MIN

has caused damage to the pixel. Therefore, it should indicate overexposure:

     if (Pixel[Channel].Value > MAX) 
     {
       Pixel[Channel].Value = MAX
       Pixel.Overexposed = TRUE
     }

     if (Pixel[Channel].Value < MIN) 
     {
       Pixel[Channel].Value = MIN
       Pixel.Overexposed = TRUE
     }

Yes, there are several different types of overexposure (luminance, saturation,
...). Personally, I don't care what kind of overexposure happened. Any kind of
damage is bad and should be indicated. Quite simple.
Comment 18 Jens Mueller 2010-01-05 20:04:40 UTC
Created attachment 39595 [details]
test sample for displaying over/under exposure indicators

I added a test sample showing the given values of 0, 1, 2, 253, 244 and 255 for channels red (r), green (g), blue (b) & luminosity (l) to test other applications behaviour on this. current digikam marks l0 as underexposure and l254 & l255 as overexposure. the l254 thing is something to fix in future, Gilles added this as workarround for some other problem.

Jens
Comment 19 caulier.gilles 2010-10-15 15:42:24 UTC
SVN commit 1186207 by cgilles:

fix endianess with over/under exposure indicator.
set over exposure limit to more than 1% of max histogram segment.
set under exposure limit to less than 1% of min histogram segment.
BUGS: 253702



 M  +52 -9     dimg.cpp


--- trunk/extragear/graphics/digikam/libs/dimg/dimg.cpp #1186206:1186207
@@ -1559,8 +1559,10 @@
    // alpha channel is auto-detected during QImage->QPixmap conversion

    uchar *bits = img.bits();
+
    // Using DImgScale before to compute Mask clamp to 65534 | 254. Why ?
-    int    max  = sixteenBit() ? 65534 : 254;
+    int    max  = sixteenBit() ? 64880 : 252;     // max histogram segment -1%
+    int    min  = sixteenBit() ? 655   : 3;       // min histogram segment +1%

    // --------------------------------------------------------

@@ -1580,6 +1582,7 @@

    uint dim    = m_priv->width * m_priv->height;
    uchar* dptr = bits;
+    int s_blue, s_green, s_red;

    if (sixteenBit())
    {
@@ -1587,26 +1590,46 @@

        for (uint i = 0; i < dim; ++i)
        {
-            int s_blue  = *sptr++;
-            int s_green = *sptr++;
-            int s_red   = *sptr++;
+            s_blue  = *sptr++;
+            s_green = *sptr++;
+            s_red   = *sptr++;
            sptr++;

-            if ((under) && (s_red == 0) && (s_green == 0) && (s_blue == 0))
+            if ((under) && (s_red <= min) && (s_green <= min) && (s_blue <= min))
            {
+                if (QSysInfo::ByteOrder == QSysInfo::BigEndian)
+                {
+                    dptr[0] = 0xFF;
+                    dptr[1] = u_red;
+                    dptr[2] = u_green;
+                    dptr[3] = u_blue;
+                }
+                else
+                {
                dptr[0] = u_blue;
                dptr[1] = u_green;
                dptr[2] = u_red;
                dptr[3] = 0xFF;
            }
+            }

            if ((over) && (s_red >= max) && (s_green >= max) && (s_blue >= max))
            {
+                if (QSysInfo::ByteOrder == QSysInfo::BigEndian)
+                {
+                    dptr[0] = 0xFF;
+                    dptr[1] = o_red;
+                    dptr[2] = o_green;
+                    dptr[3] = o_blue;
+                }
+                else
+                {
                dptr[0] = o_blue;
                dptr[1] = o_green;
                dptr[2] = o_red;
                dptr[3] = 0xFF;
            }
+            }

            dptr += 4;
        }
@@ -1617,26 +1640,46 @@

       for (uint i = 0; i < dim; ++i)
        {
-            int s_blue  = *sptr++;
-            int s_green = *sptr++;
-            int s_red   = *sptr++;
+            s_blue  = *sptr++;
+            s_green = *sptr++;
+            s_red   = *sptr++;
            sptr++;

-            if ((under) && (s_red == 0) && (s_green == 0) && (s_blue == 0))
+            if ((under) && (s_red <= min) && (s_green <= min) && (s_blue <= min))
            {
+                if (QSysInfo::ByteOrder == QSysInfo::BigEndian)
+                {
+                    dptr[0] = 0xFF;
+                    dptr[1] = u_red;
+                    dptr[2] = u_green;
+                    dptr[3] = u_blue;
+                }
+                else
+                {
                dptr[0] = u_blue;
                dptr[1] = u_green;
                dptr[2] = u_red;
                dptr[3] = 0xFF;
            }
+            }

            if ((over) && (s_red >= max) && (s_green >= max) && (s_blue >= max))
            {
+                if (QSysInfo::ByteOrder == QSysInfo::BigEndian)
+                {
+                    dptr[0] = 0xFF;
+                    dptr[1] = o_red;
+                    dptr[2] = o_green;
+                    dptr[3] = o_blue;
+                }
+                else
+                {
                dptr[0] = o_blue;
                dptr[1] = o_green;
                dptr[2] = o_red;
                dptr[3] = 0xFF;
            }
+            }

            dptr += 4;
        }
Comment 20 caulier.gilles 2010-10-15 15:46:11 UTC
Look my commit #1186207. Now, i compute :

- over-exposure if value is >= max histogram segments - 1% 
- under-exposure if value is <= min histogram segments + 1%

It still AND operator between color component channels.

Gilles Caulier
Comment 21 Dik Takken 2010-10-15 22:32:33 UTC
Due to the AND, clipping of one channel remains undetected...
Comment 22 caulier.gilles 2010-10-17 22:45:56 UTC
SVN commit 1186895 by cgilles:

New option to display over/under exposure indicators if pure colors is detected
or if only one color component match the condition.
BUGS: 148382


 M  +4 -1      NEWS  
 M  +16 -4     libs/dimg/dimg.cpp  
 M  +9 -1      libs/dimg/exposurecontainer.h  
 M  +1 -2      utilities/imageeditor/editor/editorwindow.cpp  
 M  +2 -0      utilities/imageeditor/editor/editorwindow_p.h  
 M  +13 -0     utilities/setup/setupeditor.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1186895