Bug 148382

> 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.

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

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

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...

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.

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.

Gilles, 

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

Arnd

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

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.

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.

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)

Jens,

What do you think about #4 and #11 ?

Gilles Caulier

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

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.

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

@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

>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.

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

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;
        }

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

Due to the AND, clipping of one channel remains undetected...

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