Rawtherapee: Pink highlights in output file (HDR image) with unclipped HL in Exposure

[Hombre57]

To reproduce, generate the output file of the "Fenetre" DNG and pp3 file. As you can see, the sun is a pink highlights, while the preview in RT show it white.

I'm using No ICM: sRGB output for output profile, and my monitor profile is set to sRGB as well.

Enabling Clip out-if-gamut colours solve the problem in the output file.

I don't know enough about Color Management to know if this is to be expected (difference between preview and output), so I tag it as a question for now.

Version: 5.4-240-ge518e792
Branch: dev
Commit: e518e792
Commit date: 2018-04-19
Compiler: gcc 7.3.0
Processor: undefined
System: Windows
Bit depth: 64 bits
Gtkmm: V3.22.0
Lensfun: V0.3.2.0
Build type: Release
Build flags: -mthreads -mwin32 -m64 -msse2 -mfpmath=sse -std=c++11 -march=native -Werror=unused-label -fopenmp -Werror=unknown-pragmas -Wall -Wno-unused-result -Wno-deprecated-declarations -mwindows -O3
Link flags: -mthreads -march=native -mwindows -s -O3
OpenMP support: ON
MMAP support: ON

[Hombre57]

Forgot to mention : something for you @agriggio ?

[RawConvert]

The sun is not pink on my system (RT 5.4-79) with your PP3. However is it possible the white point in camconst.json is too agressive? I say this because when I just tried the DNG with Neutral profile, the sun was white, but changing the white point correction in the Raw tools from 1 to just .99 was enough to make it go pink. (But it isn't pink at value 1).

[agriggio]

@Hombre57 I'd say this is a bug: regardless of the settings, the preview should match the output here -- either you get the pink blob in both, or in none of the two. I'll take a look (when time permits)

[iliasg]

@agriggio Alberto, it must be the "unbounded process" which turns clipped highlights to magenta .. I just tried with an older version (5.4-84) which is before unbounded process commit and there is no problem.
With the latest RT versions the clipped highlights get magenta_colorized.

A tip .. with RT5.4-127 I get no coloration If I choose "no profile" at color management
Looks like something is missing .. I mean a recalibration of the level the channels are clipped after color profiling ..

Also .. at the current stage (unbounded is on) the best way to decolorize is to use "highlight reconstruction" .. so I think this should be on by default .. but I am affraid that this also gives artefacts occasionally :(

BTW .. this coloration makes evaluation of raw WL values difficult ;)

[agriggio]

@iliasg thanks for the feedback. The point of the "unbounded mode" is to try to leave the "out of gamut" channels untouched, and only clip at the end if needed (ie. when saving to jpg or 16-bit tiff). This however can create artifacts, this is sort of expected :-)
"Unbounded" must be considered an advanced setting, for people who know what they are doing. That's why I put a big tooltip initially that was telling exactly this...
What I consider a bug, however, is the fact that you get the pink blob in the output but not in the preview. This has to be fixed: if you get a pink blob in output, you should see it also in the preview.

[Beep6581]

Fenetre.dng using the neutral profile shows magenta in the light blob in 5.4-243-g5374afe6 but not in 5.4, regardless whether "Clip out-of-gamut colors" is enabled or not.

[agriggio]

Here's a patch to restore the 5.4 behaviour when "clip OOG" is checked (I forgot to restore this behaviour when I introduced the checkbox):

diff --git a/rtengine/rawimagesource.cc b/rtengine/rawimagesource.cc
--- a/rtengine/rawimagesource.cc
+++ b/rtengine/rawimagesource.cc
@@ -154,7 +154,7 @@
     }
 }

-void transLineD1x (const float* const red, const float* const green, const float* const blue, const int i, rtengine::Imagefloat* const image, const int tran, const int imwidth, const int imheight, const bool oddHeight)
+void transLineD1x (const float* const red, const float* const green, const float* const blue, const int i, rtengine::Imagefloat* const image, const int tran, const int imwidth, const int imheight, const bool oddHeight, const bool clip)
 {
     // Nikon D1X has an uncommon sensor with 4028 x 1324 sensels.
     // Vertical sensel size is 2x horizontal sensel size
@@ -223,6 +223,12 @@
                     image->r(row, col) = MAX(0.f, -0.0625f * (red[j] + image->r(row + 3, col)) + 0.5625f * (image->r(row - 1, col) + image->r(row + 1, col)));
                     image->g(row, col) = MAX(0.f, -0.0625f * (green[j] + image->g(row + 3, col)) + 0.5625f * (image->g(row - 1, col) + image->g(row + 1, col)));
                     image->b(row, col) = MAX(0.f, -0.0625f * (blue[j] + image->b(row + 3, col)) + 0.5625f * (image->b(row - 1, col) + image->b(row + 1, col)));
+
+                    if(clip) {
+                        image->r(row, col) = MIN(image->r(row, col), rtengine::MAXVALF);
+                        image->g(row, col) = MIN(image->g(row, col), rtengine::MAXVALF);
+                        image->b(row, col) = MIN(image->b(row, col), rtengine::MAXVALF);
+                    }
                 }
             }

@@ -280,6 +286,12 @@
                     image->r(j, col) = MAX(0.f, -0.0625f * (red[j] + image->r(j, col + 3)) + 0.5625f * (image->r(j, col - 1) + image->r(j, col + 1)));
                     image->g(j, col) = MAX(0.f, -0.0625f * (green[j] + image->g(j, col + 3)) + 0.5625f * (image->g(j, col - 1) + image->g(j, col + 1)));
                     image->b(j, col) = MAX(0.f, -0.0625f * (blue[j] + image->b(j, col + 3)) + 0.5625f * (image->b(j, col - 1) + image->b(j, col + 1)));
+
+                    if(clip) {
+                        image->r(j, col) = MIN(image->r(j, col), rtengine::MAXVALF);
+                        image->g(j, col) = MIN(image->g(j, col), rtengine::MAXVALF);
+                        image->b(j, col) = MIN(image->b(j, col), rtengine::MAXVALF);
+                    }
                 }
             }

@@ -307,6 +319,12 @@
                     image->g(row, 2 * i - 3) = MAX(0.f, -0.0625f * (green[j] + image->g(row, 2 * i - 6)) + 0.5625f * (image->g(row, 2 * i - 2) + image->g(row, 2 * i - 4)));
                     image->b(row, 2 * i - 3) = MAX(0.f, -0.0625f * (blue[j] + image->b(row, 2 * i - 6)) + 0.5625f * (image->b(row, 2 * i - 2) + image->b(row, 2 * i - 4)));

+                    if(clip) {
+                        image->r(row, 2 * i - 3) = MIN(image->r(row, 2 * i - 3), rtengine::MAXVALF);
+                        image->g(row, 2 * i - 3) = MIN(image->g(row, 2 * i - 3), rtengine::MAXVALF);
+                        image->b(row, 2 * i - 3) = MIN(image->b(row, 2 * i - 3), rtengine::MAXVALF);
+                    }
+
                     image->r(row, 2 * i) = red[j];
                     image->g(row, 2 * i) = green[j];
                     image->b(row, 2 * i) = blue[j];
@@ -319,6 +337,12 @@
                     image->g(row, 2 * i - 1) = MAX(0.f, -0.0625f * (green[j] + image->g(row, 2 * i - 4)) + 0.5625f * (image->g(row, 2 * i) + image->g(row, 2 * i - 2)));
                     image->b(row, 2 * i - 1) = MAX(0.f, -0.0625f * (blue[j] + image->b(row, 2 * i - 4)) + 0.5625f * (image->b(row, 2 * i) + image->b(row, 2 * i - 2)));

+                    if(clip) {
+                        image->r(j, 2 * i - 1) = MIN(image->r(j, 2 * i - 1), rtengine::MAXVALF);
+                        image->g(j, 2 * i - 1) = MIN(image->g(j, 2 * i - 1), rtengine::MAXVALF);
+                        image->b(j, 2 * i - 1) = MIN(image->b(j, 2 * i - 1), rtengine::MAXVALF);
+                    }
+
                     image->r(row, 2 * i + 1) = (red[j] + image->r(row, 2 * i - 1)) / 2;
                     image->g(row, 2 * i + 1) = (green[j] + image->g(row, 2 * i - 1)) / 2;
                     image->b(row, 2 * i + 1) = (blue[j] + image->b(row, 2 * i - 1)) / 2;
@@ -350,6 +374,12 @@
                     image->r(2 * i - 3, j) = MAX(0.f, -0.0625f * (red[j] + image->r(2 * i - 6, j)) + 0.5625f * (image->r(2 * i - 2, j) + image->r(2 * i - 4, j)));
                     image->g(2 * i - 3, j) = MAX(0.f, -0.0625f * (green[j] + image->g(2 * i - 6, j)) + 0.5625f * (image->g(2 * i - 2, j) + image->g(2 * i - 4, j)));
                     image->b(2 * i - 3, j) = MAX(0.f, -0.0625f * (blue[j] + image->b(2 * i - 6, j)) + 0.5625f * (image->b(2 * i - 2, j) + image->b(2 * i - 4, j)));
+
+                    if(clip) {
+                        image->r(2 * i - 3, j) = MIN(image->r(2 * i - 3, j), rtengine::MAXVALF);
+                        image->g(2 * i - 3, j) = MIN(image->g(2 * i - 3, j), rtengine::MAXVALF);
+                        image->b(2 * i - 3, j) = MIN(image->b(2 * i - 3, j), rtengine::MAXVALF);
+                    }
                 }
             }

@@ -359,6 +389,12 @@
                     image->g(2 * i - 1, j) = MAX(0.f, -0.0625f * (green[j] + image->g(2 * i - 4, j)) + 0.5625f * (image->g(2 * i, j) + image->g(2 * i - 2, j)));
                     image->b(2 * i - 1, j) = MAX(0.f, -0.0625f * (blue[j] + image->b(2 * i - 4, j)) + 0.5625f * (image->b(2 * i, j) + image->b(2 * i - 2, j)));

+                    if(clip) {
+                        image->r(2 * i - 1, j) = MIN(image->r(2 * i - 1, j), rtengine::MAXVALF);
+                        image->g(2 * i - 1, j) = MIN(image->g(2 * i - 1, j), rtengine::MAXVALF);
+                        image->b(2 * i - 1, j) = MIN(image->b(2 * i - 1, j), rtengine::MAXVALF);
+                    }
+
                     image->r(2 * i + 1, j) = (red[j] + image->r(2 * i - 1, j)) / 2;
                     image->g(2 * i + 1, j) = (green[j] + image->g(2 * i - 1, j)) / 2;
                     image->b(2 * i + 1, j) = (blue[j] + image->b(2 * i - 1, j)) / 2;
@@ -688,6 +724,8 @@
     hlmax[1] = clmax[1] * gm;
     hlmax[2] = clmax[2] * bm;

+    const bool doClip = (chmax[0] >= clmax[0] || chmax[1] >= clmax[1] || chmax[2] >= clmax[2]) && !hrp.hrenabled && hrp.clampOOG;
+
     float area = skip * skip;
     rm /= area;
     gm /= area;
@@ -730,6 +768,17 @@
                     gtot *= gm;
                     btot *= bm;

+                    if (doClip) {
+                        // note: as hlmax[] can be larger than CLIP and we can later apply negative
+                        // exposure this means that we can clip away local highlights which actually
+                        // are not clipped. We have to do that though as we only check pixel by pixel
+                        // and don't know if this will transition into a clipped area, if so we need
+                        // to clip also surrounding to make a good colour transition
+                        rtot = CLIP(rtot);
+                        gtot = CLIP(gtot);
+                        btot = CLIP(btot);
+                    }
+
                     line_red[j] = rtot;
                     line_grn[j] = gtot;
                     line_blue[j] = btot;
@@ -754,6 +803,12 @@
                     gtot *= gm;
                     btot *= bm;

+                    if (doClip) {
+                        rtot = CLIP(rtot);
+                        gtot = CLIP(gtot);
+                        btot = CLIP(btot);
+                    }
+
                     line_red[j] = rtot;
                     line_grn[j] = gtot;
                     line_blue[j] = btot;
@@ -767,7 +822,7 @@
             }

             if(d1x) {
-                transLineD1x (line_red, line_grn, line_blue, ix, image, tran, imwidth, imheight, d1xHeightOdd);
+                transLineD1x (line_red, line_grn, line_blue, ix, image, tran, imwidth, imheight, d1xHeightOdd, doClip);
             } else if(fuji) {
                 transLineFuji (line_red, line_grn, line_blue, ix, image, tran, imheight, fw);
             } else {
diff --git a/rtgui/tonecurve.cc b/rtgui/tonecurve.cc
--- a/rtgui/tonecurve.cc
+++ b/rtgui/tonecurve.cc
@@ -32,7 +32,7 @@
     auto m = ProcEventMapper::getInstance();
     EvHistMatching = m->newEvent(AUTOEXP, "HISTORY_MSG_HISTMATCHING");
     EvHistMatchingBatch = m->newEvent(M_VOID, "HISTORY_MSG_HISTMATCHING");
-    EvClampOOG = m->newEvent(RGBCURVE, "HISTORY_MSG_CLAMPOOG");
+    EvClampOOG = m->newEvent(DARKFRAME, "HISTORY_MSG_CLAMPOOG");

     CurveListener::setMulti(true);

Unless there are objections I'll try to commit later (or tomorrow)

[heckflosse]

@agriggio The patch fixes the issue reported by @Beep6581 using neutral profile, but not the issue reported by @Hombre57 (preview shows white but saved shows pinkish highlights using his pp3 file)

[Beep6581]

"Clip out-of-gamut colors" now has an effect, and when using neutral the preview now generally matches the saved image with this exception (notice the band around the main blob):

When using @Hombre57 's PP3, the preview still does not match the saved image.
I noticed that the blob is clipped in the preview image (R=G=B=100%), but the navigator seems to show the same values as the saved image (R=B=100% G=~84%), so maybe the bug lies in the preview pipeline.

[agriggio]

@heckflosse yes, I confirm that the patch is only for @Beep6581's bug. I still need to find some time to look at the preview mismatch issue

[agriggio]

Here's the best I could do. It's still not completely accurate, but I'm afraid getting better could be difficult -- there are too many factors involved (with different cms transforms and different pipelines), so I wouldn't know what to do exactly... help is welcome though :-)

diff --git a/rtengine/improcfun.cc b/rtengine/improcfun.cc
--- a/rtengine/improcfun.cc
+++ b/rtengine/improcfun.cc
@@ -340,7 +340,7 @@

                 monitorTransform = cmsCreateProofingTransform (
                                        iprof, TYPE_Lab_FLT,
-                                       monitor, TYPE_RGB_8,
+                                       monitor, TYPE_RGB_FLT,
                                        oprof,
                                        monitorIntent, outIntent,
                                        flags
@@ -383,7 +383,7 @@
                 flags |= cmsFLAGS_BLACKPOINTCOMPENSATION;
             }

-            monitorTransform = cmsCreateTransform (iprof, TYPE_Lab_FLT, monitor, TYPE_RGB_8, monitorIntent, flags);
+            monitorTransform = cmsCreateTransform (iprof, TYPE_Lab_FLT, monitor, TYPE_RGB_FLT, monitorIntent, flags);
         }

         if (gamutCheck && gamutprof) {
diff --git a/rtengine/iplab2rgb.cc b/rtengine/iplab2rgb.cc
--- a/rtengine/iplab2rgb.cc
+++ b/rtengine/iplab2rgb.cc
@@ -32,64 +32,61 @@

 extern void filmlike_clip(float *r, float *g, float *b);

+extern const Settings* settings;
+
 namespace {

-inline void clipLAB(float iL, float ia, float ib, float &oL, float &oa, float &ob, const float scale, const float wp[3][3], const float wip[3][3])
+inline void copyAndClampLine(const float *src, unsigned char *dst, const int W)
 {
-    if (iL < 0.f) {
-        oL = oa = ob = 0.f;
-    } else if (iL > 32768.f || min(ia, ib) < -42000.f || max(ia, ib) > 42000.f) {
-        
-        float X, Y, Z;
-        float r, g, b;
-        Color::Lab2XYZ(iL, ia, ib, X, Y, Z);
-        Color::xyz2rgb(X, Y, Z, r, g, b, wip);
-        filmlike_clip(&r, &g, &b);
-        Color::rgbxyz(r, g, b, X, Y, Z, wp);
-        Color::XYZ2Lab(X, Y, Z, oL, oa, ob);
-        oL /= scale;
-        oa /= scale;
-        ob /= scale;
-        
-        // oL = 32768.f / scale;
-        // oa = ob = 0.f;
-    } else {
-        oL = iL / scale;
-        oa = ia / scale;
-        ob = ib / scale;
+    for (int j = 0, iy = 0; j < W; ++j) {
+        float r = src[iy] * MAXVALF;
+        float g = src[iy+1] * MAXVALF;
+        float b = src[iy+2] * MAXVALF;
+        if (r > MAXVALF || g > MAXVALF || b > MAXVALF) {
+            filmlike_clip(&r, &g, &b);
+        }
+        dst[iy] = uint16ToUint8Rounded(CLIP(r));
+        dst[iy+1] = uint16ToUint8Rounded(CLIP(g));
+        dst[iy+2] = uint16ToUint8Rounded(CLIP(b));
+        iy += 3;
     }
 }


-inline void clipLAB(float iL, float ia, float ib, double &oL, double &oa, double &ob, const float scale, const float wp[3][3], const float wip[3][3])
+inline void copyAndClamp(const LabImage *src, unsigned char *dst, const double rgb_xyz[3][3], bool multiThread)
 {
-    float tL, ta, tb;
-    clipLAB(iL, ia, ib, tL, ta, tb, scale, wp, wip);
-    oL = tL;
-    oa = ta;
-    ob = tb;
+    int W = src->W;
+    int H = src->H;
+
+#ifdef _OPENMP
+        #pragma omp parallel for schedule(dynamic,16) if (multiThread)
+#endif
+    for (int i = 0; i < H; ++i) {
+        float* rL = src->L[i];
+        float* ra = src->a[i];
+        float* rb = src->b[i];
+        int ix = i * 3 * W;
+
+        float R, G, B;
+        float x_, y_, z_;
+
+        for (int j = 0; j < W; ++j) {
+            Color::Lab2XYZ(rL[j], ra[j], rb[j], x_, y_, z_ );
+            Color::xyz2rgb(x_, y_, z_, R, G, B, rgb_xyz);
+
+            if (R > MAXVALF || G > MAXVALF || B > MAXVALF) {
+                filmlike_clip(&R, &G, &B);
+            }
+
+            dst[ix++] = uint16ToUint8Rounded(Color::gamma2curve[R]);
+            dst[ix++] = uint16ToUint8Rounded(Color::gamma2curve[G]);
+            dst[ix++] = uint16ToUint8Rounded(Color::gamma2curve[B]);
+        }
+    }
 }

 } // namespace

-extern const Settings* settings;
-
-#define DECLARE_WORKING_MATRICES_(space) \
-    TMatrix wprof = ICCStore::getInstance()->workingSpaceMatrix ( space ); \
-    const float wp[3][3] = {                                            \
-        {static_cast<float> (wprof[0][0]), static_cast<float> (wprof[0][1]), static_cast<float> (wprof[0][2])}, \
-        {static_cast<float> (wprof[1][0]), static_cast<float> (wprof[1][1]), static_cast<float> (wprof[1][2])}, \
-        {static_cast<float> (wprof[2][0]), static_cast<float> (wprof[2][1]), static_cast<float> (wprof[2][2])} \
-    };                                                                  \
-                                                                        \
-    TMatrix wiprof = ICCStore::getInstance()->workingSpaceInverseMatrix ( space ); \
-    const float wip[3][3] = {                                           \
-        {static_cast<float> (wiprof[0][0]), static_cast<float> (wiprof[0][1]), static_cast<float> (wiprof[0][2])}, \
-        {static_cast<float> (wiprof[1][0]), static_cast<float> (wiprof[1][1]), static_cast<float> (wiprof[1][2])}, \
-        {static_cast<float> (wiprof[2][0]), static_cast<float> (wiprof[2][1]), static_cast<float> (wiprof[2][2])} \
-    }
-    
-
 // Used in ImProcCoordinator::updatePreviewImage  (rtengine/improccoordinator.cc)
 //         Crop::update                           (rtengine/dcrop.cc)
 //         Thumbnail::processImage                (rtengine/rtthumbnail.cc)
@@ -98,8 +95,6 @@
 // otherwise divide by 327.68, convert to xyz and apply the sRGB transform, before converting with gamma2curve
 void ImProcFunctions::lab2monitorRgb (LabImage* lab, Image8* image)
 {
-    DECLARE_WORKING_MATRICES_(params->icm.working);
-    
     if (monitorTransform) {

         int W = lab->W;
@@ -112,6 +107,7 @@
 #endif
         {
             AlignedBuffer<float> pBuf(3 * lab->W);
+            AlignedBuffer<float> mBuf(3 * lab->W);

             AlignedBuffer<float> gwBuf1;
             AlignedBuffer<float> gwBuf2;
@@ -121,6 +117,7 @@
             }

             float *buffer = pBuf.data;
+            float *outbuffer = mBuf.data;

 #ifdef _OPENMP
             #pragma omp for schedule(dynamic,16)
@@ -136,11 +133,13 @@
                 float* rb = lab->b[i];

                 for (int j = 0; j < W; j++) {
-                    clipLAB(rL[j], ra[j], rb[j], buffer[iy], buffer[iy+1], buffer[iy+2], 327.68f, wp, wip);
-                    iy += 3;
+                    buffer[iy++] = rL[j] / 327.68f;
+                    buffer[iy++] = ra[j] / 327.68f;
+                    buffer[iy++] = rb[j] / 327.68f;
                 }

-                cmsDoTransform (monitorTransform, buffer, data + ix, W);
+                cmsDoTransform (monitorTransform, buffer, outbuffer, W);
+                copyAndClampLine(outbuffer, data + ix, W);

                 if (gamutWarning) {
                     gamutWarning->markLine(image, i, buffer, gwBuf1.data, gwBuf2.data);
@@ -148,41 +147,7 @@
             }
         } // End of parallelization
     } else {
-
-        int W = lab->W;
-        int H = lab->H;
-        unsigned char * data = image->data;
-
-#ifdef _OPENMP
-        #pragma omp parallel for schedule(dynamic,16) if (multiThread)
-#endif
-
-        for (int i = 0; i < H; ++i) {
-            float* rL = lab->L[i];
-            float* ra = lab->a[i];
-            float* rb = lab->b[i];
-            int ix = i * 3 * W;
-
-            float R, G, B;
-            float x_, y_, z_;
-            float L, a, b;
-
-            for (int j = 0; j < W; ++j) {
-
-                //float L1=rL[j],a1=ra[j],b1=rb[j];//for testing
-                clipLAB(rL[j], ra[j], rb[j], L, a, b, 1.f, wp, wip);
-
-                Color::Lab2XYZ(L, a, b, x_, y_, z_ );
-
-                Color::xyz2srgb(x_, y_, z_, R, G, B);
-
-                /* copy RGB */
-                //int R1=((int)gamma2curve[(R)])
-                data[ix++] = uint16ToUint8Rounded(Color::gamma2curve[R]);
-                data[ix++] = uint16ToUint8Rounded(Color::gamma2curve[G]);
-                data[ix++] = uint16ToUint8Rounded(Color::gamma2curve[B]);
-            }
-        }
+        copyAndClamp(lab, image->data, sRGB_xyz, multiThread);
     }
 }

@@ -197,8 +162,6 @@
 // otherwise divide by 327.68, convert to xyz and apply the RGB transform, before converting with gamma2curve
 Image8* ImProcFunctions::lab2rgb (LabImage* lab, int cx, int cy, int cw, int ch, const procparams::ColorManagementParams &icm, bool consider_histogram_settings)
 {
-    DECLARE_WORKING_MATRICES_(icm.working);
-    
     //gamutmap(lab);

     if (cx < 0) {
@@ -248,7 +211,7 @@
         }
         lcmsMutex->lock ();
         cmsHPROFILE LabIProf  = cmsCreateLab4Profile(nullptr);
-        cmsHTRANSFORM hTransform = cmsCreateTransform (LabIProf, TYPE_Lab_DBL, oprofG, TYPE_RGB_8, icm.outputIntent, flags);  // NOCACHE is important for thread safety
+        cmsHTRANSFORM hTransform = cmsCreateTransform (LabIProf, TYPE_Lab_DBL, oprofG, TYPE_RGB_FLT, icm.outputIntent, flags);  // NOCACHE is important for thread safety
         cmsCloseProfile(LabIProf);
         lcmsMutex->unlock ();

@@ -260,7 +223,9 @@
 #endif
         {
             AlignedBuffer<double> pBuf(3 * cw);
+            AlignedBuffer<float> oBuf(3 * cw);
             double *buffer = pBuf.data;
+            float *outbuffer = oBuf.data;
             int condition = cy + ch;

 #ifdef _OPENMP
@@ -275,11 +240,13 @@
                 float* rb = lab->b[i];

                 for (int j = cx; j < cx + cw; j++) {
-                    clipLAB(rL[j], ra[j], rb[j], buffer[iy], buffer[iy+1], buffer[iy+2], 327.68f, wp, wip);
-                    iy += 3;
+                    buffer[iy++] = rL[j] / 327.68f;
+                    buffer[iy++] = ra[j] / 327.68f;
+                    buffer[iy++] = rb[j] / 327.68f;
                 }

-                cmsDoTransform (hTransform, buffer, data + ix, cw);
+                cmsDoTransform (hTransform, buffer, outbuffer, cw);
+                copyAndClampLine(outbuffer, data + ix, cw);
             }
         } // End of parallelization

@@ -289,34 +256,8 @@
             cmsCloseProfile(oprofG);
         }
     } else {
-
         const auto xyz_rgb = ICCStore::getInstance()->workingSpaceInverseMatrix (profile);
-
-#ifdef _OPENMP
-        #pragma omp parallel for schedule(dynamic,16) if (multiThread)
-#endif
-
-        for (int i = cy; i < cy + ch; ++i) {
-            float* rL = lab->L[i];
-            float* ra = lab->a[i];
-            float* rb = lab->b[i];
-            int ix = 3 * i * cw;
-
-            float R, G, B;
-            float x_, y_, z_;
-            float L, a, b;
-
-            for (int j = cx; j < cx + cw; ++j) {
-                clipLAB(rL[j], ra[j], rb[j], L, a, b, 1.f, wp, wip);
-                Color::Lab2XYZ(rL[j], ra[j], rb[j], x_, y_, z_);
-
-                Color::xyz2rgb(x_, y_, z_, R, G, B, xyz_rgb);
-
-                image->data[ix++] = uint16ToUint8Rounded(Color::gamma2curve[R]);
-                image->data[ix++] = uint16ToUint8Rounded(Color::gamma2curve[G]);
-                image->data[ix++] = uint16ToUint8Rounded(Color::gamma2curve[B]);
-            }
-        }
+        copyAndClamp(lab, image->data, xyz_rgb, multiThread);
     }

     return image;
@@ -383,6 +324,7 @@
         cmsDeleteTransform(hTransform);
         image->normalizeFloatTo65535();
     } else {
+        
 #ifdef _OPENMP
         #pragma omp parallel for schedule(dynamic,16) if (multiThread)
 #endif

[aferrero2707]

I have been also confronted with the problem of adapting the RAW processing steps to the unbounded case, and I came up with the following steps that seem to yield optimal results in all cases:

• decode de RAW data, subtract the black point, and normalize according to the white level (I will assume a normalization to the [0..1] range for simplicity)
• multiply the normalized RAW values by the WB coefficients
• the next step depends on the highlights reconstruction strategy:
  
  
  
  • if the HL reco is set to clip, then the RAW values after WB are immediately clipped to the [0..1] range. This mitigates the purple fringes around dark/bright transitions (as explained here)
  
  
  • in all other cases, the HL reco is deferred to after the demosaicing
  
  
• demosaicing
• HL reco, if not yet applied
• lens corrections
• exposure compensation
• color conversion to the working colorspace, either using the standard Adobe matrices or a DCP profile
• optionally, clip either the negaive values or the values above 1, or both

I hope this helps...

[RawConvert]

hi @aferrero2707 , I guess the steps above are for Photoflow. To check my understanding, is everything prior to "color conversion..." done with linear gamma?
Suppose this was in Rawtherapee and you applied a camera DCP, and you had the working space set to say Prophoto. Would a conversion to Prophoto take place after the last bullet point?

[aferrero2707]

Yes, everything before "color conversion" happens in the linear camera colorspace.

Regarding the second question, the answer depends on the definition of "working RGB colorspace" in RT... does the conversion to ProPhoto happen before or after the the one to Lab colorspace?

In fact, in the case of a conversion from camera to Lab the clipping is maybe not needed. The most critical values are negative RGB channels, because of the way they can interfere with certain operations in RGB colorspace. In Lab colorspace things are quite different...

[Hombre57]

@agriggio Your 2 patch gives more consistent result. If no one objects, could you commit them ?

[agriggio]

@Hombre57 sure, committing now

[Hombre57]

@agriggio I'm reopening the issue since the pink HL is still there, and I have new informations (feel free to close again if nothing can be done in RT) : when using the RT_ACEScg-V4-g22 output profile of the testoutputprofile branch (might be the case with ACES output profile of dev branch, if available), then the pink HL is gone in the output file (not the gray "crown" though), meaning that a clipping occurs in RT when sRGB output is used (w/ or w/o ICC profile attached), or a narrow gamut profile is selected, generally speaking.

(ping @heckflosse @aferrero2707 @Beep6581 @RawConvert)

[agriggio]

@Hombre57 I'm not sure I understand what you are saying... do you mean that the preview and the output are different?

[Hombre57]

@agriggio Yes, specifically with ACES output profile, with output image seen in IrfanView, CMM enabled.

[agriggio]

@Hombre57 what I don't understand of your comment is this part:

meaning that a clipping occurs in RT when sRGB output is used (w/ or w/o ICC profile attached), or a narrow gamut profile is selected, generally speaking.

what do you mean exactly? Clipping definitely occurs when generating the output image for display, because we convert to 8 bits. Some clipping also occurs internally, even when "clip OOG" is not set to true. This happens to allow "smooth" transitions between unclipped and clipped colours. It might also be possible that some tools are still clipping internally, because they have not yet been made "unbounded-processing aware". If you have evidence of this third case, please do report it and I'll see if I can do something about it.

Regarding the mismatch between output and preview -- unfortunately as I wrote above I haven't been able to fully fix it, as the two pipelines are just different. Regardless of the OOG setting, though, there are other things that you won't see in the preview, for example the posterization that occurs when you save the image above with an ACES output profile. I'm not saying this is not an issue, I'm just saying I don't know how to fix it at the moment... :-/

[agriggio]

EDIT to my comment above: it seems that the posterization I was talking about depends on the viewer... It occurs in Geeqie, but not in GIMP 2.10. Both are colour managed, use the same screen profile and the same rendering intent. Here are a couple of screenshots:

• Geeqie

• Gimp 2.10