Beep6581 / RawTherapee

A powerful cross-platform raw photo processing program
https://rawtherapee.com
GNU General Public License v3.0
2.74k stars 313 forks source link

New Output ICC profiles - from Elle Stone #4478

Closed Desmis closed 5 years ago

Desmis commented 6 years ago

I have slightly modified 37 ICC output profiles coming from "Elle Stone" , thanks to him

Branch : testoutputprofile

do we keep them all, or only a few, and if so, which ones?

Thank you

Jacques

Desmis commented 6 years ago

Yes, I am really a poor computer scientist

I delete old files, and make git rm rtdata/iccprofiles/output/xxx.icc

then copy all new ICC files then git rm rtdata/iccprofiles/output/RT_*.icc

then git commit -m " add new ICC " -a then git push --set-upstream origin testoutputprofile

I obtain in rtdata all new files, but when I want to compile I have an install error 1 with ACES.icc Calla stack (most recent call first)

Desmis commented 6 years ago

I have probably make a mistake !! With a new clone it's work :)

Beep6581 commented 6 years ago

https://github.com/Beep6581/RawTherapee/tree/testoutputprofile/rtdata/iccprofiles/output

The files in commit f3c4546 use curves with 4096 points. There is an ongoing discussion about whether to use 1024 or 4906 points: https://discuss.pixls.us/t/feature-request-save-as-floating-point/5696/119

Desmis commented 6 years ago

I know, and I say in forum :

"By default it is LCMS that fixe this value

define MAX_NODES_IN_CURVE 4097 in cmsgamma.c" ==> all profiles generate with LCMS have 4096 points.

Desmis commented 6 years ago

I push a commit with :

constexpr double d50_d60[3][3] = { { 1.034368, 0.016908, -0.037658}, {0.021752, 0.992183, -0.012785}, { -0.006971, 0.011377, 0.812150} };

constexpr double d60_d50[3][3] = { { 0.96743198, -0.01699717, 0.044590689}, {-0.02109893, 1.008067172, 0.014890785}, {0.008598998, -0.01426777, 1.231474467} }; The "originale" matrix define by primaries p[0] = 0.734704; // ACESc primaries p[1] = 0.265298; p[2] = -0.000004; p[3] = 0.999993; p[4] = 0.00009989; p[5] = -0.077007; temp = ColorTemp::D60; is adaptated by Bradford and gives : constexpr double ACESc_xyz[3][3] = { {0.956674714, 0.334059262, 0.033764461}, {-0.00914767, 0.719456271 , 0.020585086}, {-0.03959528, -0.11114562, 1.008891993} };

constexpr double xyz_ACESc[3][3] = { {1.039605861, -0.48655298, -0.0248649}, {0.012013027, 1.379948826 , -0.02855804}, {0.04212411, 0.132928069, 0.987064397} };

The profiles are in folder where Rawtherapee.exe (Release,...)

If you look at the results, with Histogrammar (from Guillermo Luijk) - this software is old (2012) and need to launch to change the date.

Jacques

agriggio commented 6 years ago

@Desmis Jacques, any reason why you didn't use the new facility to add working profiles using a JSON file, as explained here? This does the adaptation automatically, and there's no need to hard-code anything in the code... assuming what I implemented is correct of course :-), but if not I would greatly appreciate your input! The code is here.

Desmis commented 6 years ago

@agriggio

I know your work :) you have want to know my opinion. But I think it is good to have in "fixed working profile", this new ACES

If I calculated, my matrix - with my profiles or if I extracted from ACES.icm the matrix is //Matrix ACESc from LCMS calculation same as ACES.icm constexpr double xyz_ACESc[3][3] = { {0.99089, 0.01224, -0.03893}, {0.36189, 0.72252, -0.08441}, {-0.00272, 0.00826, 0.81937} }; and the adaptation is not current D60 D50 and not D65, this leads to constexpr double ACESc_xyz[3][3] = { {0.956674714, 0.334059262, 0.033764461}, {-0.00914767, 0.719456271 , 0.020585086}, {-0.03959528, -0.11114562, 1.008891993} }; different from you used : "matrix" : [0.7184354, 0.16578523, 0.09882643, 0.29728935, 0.66958117, 0.03571544, -0.00647622, 0.01469771, 0.66732561]

this does not call into question the use of JSON

:) jacques

agriggio commented 6 years ago

@Desmis does that mean that my computation is wrong somewhere? In that case, can you tell me where exactly? the code is supposed to to the adaptation from the white point of the profile to D50 -- so in case of ACES that should be D60 to D50... or am I missing something?

agriggio commented 6 years ago

BTW:

But I think it is good to have in "fixed working profile", this new ACES

assuming we can fix the computation (if it is wrong), you can have ACES as "built-in" by simply shipping a default JSON file to be installed by default by RT...

Desmis commented 6 years ago

yes sure :)

Desmis commented 6 years ago

Where have you found your ACES profile ?

Desmis commented 6 years ago

I think you used, Elle Stone profile, that gives different XYZ values My profile has same values that ACES.icm When I used, Elle Profile and apply inverse Bradford (D50 D65) I obtain your matrix

I think, to verify, you must use direct Bradford and D60 to D50

agriggio commented 6 years ago

I'm not sure which ACES profile you are referring to... if you mean the example in the rawpedia page, it's the "ACEScg" that comes from Elle. But you can actually use any ICC profile. It would be great if you could take a look at the code, it's all there in rtengine/iccstore.cc

agriggio commented 6 years ago

I took the math from here: www.brucelindbloom.com/Eqn_RGB_XYZ_Matrix.html

agriggio commented 6 years ago

Here's the same thing in a Python script:

#!/usr/bin/env python
#
# see http://www.brucelindbloom.com/index.html?Eqn_ChromAdapt.html
#

import numpy

bradford_MA = numpy.array([
    [0.8951000,  0.2664000, -0.1614000],
    [-0.7502000,  1.7135000,  0.0367000],
    [0.0389000, -0.0685000,  1.0296000]
    ])

bradford_MA_inv = numpy.array([
    [0.9869929, -0.1470543,  0.1599627],
    [0.4323053,  0.5183603,  0.0492912],
    [-0.0085287,  0.0400428,  0.9684867]
    ])

def generate_adaptation_matrix(white1, white2):
    src = bradford_MA.dot(white1)
    dst = bradford_MA.dot(white2)
    m = numpy.array([
        [dst[0]/src[0], 0, 0],
        [0, dst[1]/src[1], 0],
        [0, 0, dst[2]/src[2]]
        ])
    res = bradford_MA_inv.dot(m).dot(bradford_MA)
    return res

def adapt_matrix(matrix, srcwhite, dstwhite):
    m = generate_adaptation_matrix(srcwhite, dstwhite)
    print 'ADAPTATION MATRIX:\n%s' % m
    res = m.dot(matrix)
    return res

# example usage

d50 = numpy.array([0.96422, 1.00000, 0.82521])
d60 = numpy.array([0.95265198, 1.00000000, 1.00881958])
d65 = numpy.array([0.95047, 1.00000, 1.08883])

# Elle's ACES D60
elle_aces_d60 = numpy.transpose(numpy.array([
    [0.99089050, 0.36189270, -0.00271606],
    [0.01223755, 0.72251892, 0.00825500],
    [-0.03892517, -0.08441162, 0.81936646]
    ]))
matrix = elle_aces_d60
srcwhite = d60

result = adapt_matrix(matrix, srcwhite, d50)
print result
Desmis commented 6 years ago

I don't know Python, but the code seems good in particular there is d60 = numpy.array([0.95265198, 1.00000000, 1.00881958]), I don't see in your. The matrix used is the same as mine at epsilon... but diffrent from those whe can read in profile Elle.

agriggio commented 6 years ago

ok, I'll try to describe what the code does later tonight... hopefully we can find the bug :-)

Desmis commented 6 years ago

And what is the result in terms of XYZ after adaptation ? A good test : when you change the working profile, image will not change or a little if there is out of gamut.

agriggio commented 6 years ago

Here are the results of running the script:

INPUT MATRIX:
[[ 0.9908905   0.01223755 -0.03892517]
 [ 0.3618927   0.72251892 -0.08441162]
 [-0.00271606  0.008255    0.81936646]]
SOURCE WHITE: [ 0.95265198  1.          1.00881958]
DEST WHITE:  [ 0.96422  1.       0.82521]
ADAPTATION MATRIX:
[[ 1.03413627  0.01679692 -0.03741888]
 [ 0.02160997  0.99222861 -0.01270332]
 [-0.00692622  0.01130544  0.81332956]]
RESULT:
[[ 1.03089612  0.02448249 -0.07233156]
 [ 0.38052791  0.71706353 -0.09500547]
 [-0.00498082  0.01479767  0.66573025]]

when you change the working profile, image will not change or a little if there is out of gamut.

I agree that there is something wrong, because I see some changes in the image. But I'd like to understand what is wrong...

Desmis commented 6 years ago

If I verify with my own calculation, that are some more complex (with openoffice) the matrix is near of [[ 1.03413627 0.01679692 -0.03741888] [ 0.02160997 0.99222861 -0.01270332] [-0.00692622 0.01130544 0.81332956]]

 { 1.034368,  0.016908, -0.037658},
{0.021752,  0.992183, -0.012785},
{ -0.006971,  0.011377,  0.812150}

But I think you must inverse this matrix and leads to { 0.96743198, -0.01699717, 0.044590689}, {-0.02109893, 1.008067172, 0.014890785}, {0.008598998, -0.01426777, 1.231474467}

Desmis commented 6 years ago

All calculation are made with Spectral datas and I can set the temp to 6005K, which can explain these smal diffrences ==> but it is details

agriggio commented 6 years ago

Jacques, I think I might simply have swapped the source and target white points -- I'll double check tonight, now I'm on my phone...

Desmis commented 6 years ago

but apart from this problem what do you think of the ICC profile generator, V2, V4, FOIP ?

agriggio commented 6 years ago

@Desmis Jacques, I found the problem! Quoting from Bruce Lindbloom's webpage (http://www.brucelindbloom.com/index.html?Eqn_RGB_XYZ_Matrix.html):

If you examine the matrices for these working spaces found inside ICC profiles (through the redColorantTag, greenColorantTag and blueColorantTag), those matrices will always be relative to D50, and therefore, the colorants have been subjected to a chromatic adaptation transformation if the working space reference white is not also D50.

So, my mistake was that I didn't read the docs about ICC profiles carefully enough :-( I thought that the matrix inside the ICC was relative to the white point, but in fact it is already adapted to D50. So, no need of further adaptation. This makes the code even simpler. So, for ACES the correct matrix (found in Elle's profile) should be:

 0.99089050  0.01223755  -0.03892517, 
 0.36189270  0.72251892  -0.08441162, 
-0.00271606  0.00825500   0.81936646
agriggio commented 6 years ago

Here's a patch that disables the chromatic adaptation when importing custom working profiles from ICC files. I'm including it here just for reference, I'll commit (to dev) in a moment:

diff --git a/rtengine/iccstore.cc b/rtengine/iccstore.cc
--- a/rtengine/iccstore.cc
+++ b/rtengine/iccstore.cc
@@ -656,45 +656,21 @@
             cmsCloseProfile(prof);
             return false;
         }
-        cmsCIEXYZ *white = static_cast<cmsCIEXYZ *>(cmsReadTag(prof, cmsSigMediaWhitePointTag));
         cmsCIEXYZ *red = static_cast<cmsCIEXYZ *>(cmsReadTag(prof, cmsSigRedMatrixColumnTag));
         cmsCIEXYZ *green  = static_cast<cmsCIEXYZ *>(cmsReadTag(prof, cmsSigGreenMatrixColumnTag));
         cmsCIEXYZ *blue  = static_cast<cmsCIEXYZ *>(cmsReadTag(prof, cmsSigBlueMatrixColumnTag));

-        if (!white || !red || !green || !blue) {
+        if (!red || !green || !blue) {
             cmsCloseProfile(prof);
             return false;
         }

-        // do the Bradford adaptation to D50
-        // matrices from Bruce Lindbloom's webpage
-        static constexpr CMatrix bradford_MA = {
-            CVector({0.8951000,  0.2664000, -0.1614000}),
-            CVector({-0.7502000,  1.7135000,  0.0367000}),
-            CVector({0.0389000, -0.0685000,  1.0296000})
-        };
-        static constexpr CMatrix bradford_MA_inv = {
-            CVector({0.9869929, -0.1470543,  0.1599627}),
-            CVector({0.4323053,  0.5183603,  0.0492912}),
-            CVector({-0.0085287,  0.0400428,  0.9684867})
-        };
-        static constexpr CVector bradford_MA_dot_D50 = {
-            0.99628443, 1.02042736, 0.81864437
+        CMatrix m = {
+            CVector({ red->X, green->X, blue->X }),
+            CVector({ red->Y, green->Y, blue->Y }),
+            CVector({ red->Z, green->Z, blue->Z })
         };
-
-        CVector srcw = dotProduct(bradford_MA, CVector({ white->X, white->Y, white->Z }));
-        CMatrix m = {
-            CVector({ bradford_MA_dot_D50[0]/srcw[0], 0.0, 0.0 }),
-            CVector({ 0.0, bradford_MA_dot_D50[1]/srcw[1], 0.0 }),
-            CVector({ 0.0, 0.0, bradford_MA_dot_D50[2]/srcw[2] })
-        };
-        CMatrix adapt = dotProduct(dotProduct(bradford_MA_inv, m), bradford_MA);
-
-        m[0][0] = red->X; m[0][1] = green->X; m[0][2] = blue->X;
-        m[1][0] = red->Y; m[1][1] = green->Y; m[1][2] = blue->Y;
-        m[2][0] = red->Z; m[2][1] = green->Z; m[2][2] = blue->Z;
-
-        m = dotProduct(adapt, m);
+        
         out.set(m);

         cmsCloseProfile(prof);
Desmis commented 6 years ago

@agriggio

Alberto I tried another time ACES with the matrix you gave 0.99089050 0.01223755 -0.03892517, 0.36189270 0.72251892 -0.08441162, -0.00271606 0.00825500 0.81936646 then calculate his inverse necessary in some part of the code

After compiling and test, the image and the histogram change when you select ACES in "Working profile" while that does not change with all the others working profiles (sRGB, Prophoto, Rec2020, etc.)

I have been working at this problem during one week when I was working on generation ICC V2 V4. In fact in theory you are true, XYZ in profiles are D50, but for ACES it seems a particular case. I have search, make may tests and leads to the conclusion that for ACES we need to compensate by a Bradford calculation D60 D50. With this correction, image does not change, histogram does not change

I have sligtly change the values to increase precision - differences are after 6 digits, for me without importance, but it is in theory better Here the matrix re-calculated: //with Bradford adaptation D50 D60 J.Desmis 04 2018 // ACESc_xyz = matrix ACESc * matrix d60 D50 from Matrix ACESc from LCMS calculation same as ACES.icm constexpr double ACESc_xyz[3][3] = { {0.9566756689, 0.334061821, 0.0337682}, {-0.00916399, 0.719455101 , 0.02057992}, {-0.03958909, -0.11114757, 1.008887738} };

constexpr double xyz_ACESc[3][3] = { {1.039596401, -0.48655409, -0.02487101}, {0.012036915, 1.379940825 , -0.02855178}, {0.042120199, 0.132933363, 0.987069107} }; //end modification ACES matrix I don't know why this particular case: 1) perhaps it is normal, that ACES without Bradford give this result, with color cast, it seems curious...and hasardous 2) perhaps LCMS do a wrong calculation with these specific settings : primaries, white points 3) perhaps ???

But Ithink users will see the color cast, and change (quite important) in histogram. The matrix I proposed "solved" the image differences and histogram differences.

Jacques

Beep6581 commented 6 years ago

About ACES,

The white point is approximate to the CIE D60 standard illuminant, and ACES compliant files are encoded in 16-bit half-floats, thus allowing ACES OpenEXR files to encode 30 stops of scene information.

Desmis commented 6 years ago

I just merge with dev, and curiously, now, with ACES, histogram and image change a little in function of image and output profile ??

Desmis commented 6 years ago

I checkout with dedce5d, and there are differences in some cases.

With my habitual image test, no differences...ACES and others profiles for image and histogram For others images as colorspace_flowers.pef, there are differences on the white and in histogram

With the "merge with dev" With my habitual image test, small differences...ACES and others profiles for histogram, no visible for image For others images as colorspace_flowers.pef, there are differences on the white and in histogram I re-change the matrix to put the same values as before (dedce5d), but same behaviour...curious

Question ? Is a good idea to add ACES as working profile ? For what reason there are in some images differences with others working profiles - in histogram and / or in image??

Desmis commented 6 years ago

Or is it a particularity of ACES to change "a little" some colors, in the "same way" as CIECAM ??

Desmis commented 6 years ago

I will try with the matrix found in internet ACES

agriggio commented 6 years ago

@Desmis Jacques, if I use the ACES matrix as found in Elle's profile, which is already adapted to D50, I see no colour cast nor any other strange behaviour

agriggio commented 6 years ago

@Desmis can you try with the latest dev an this workingspaces.json file? (just change the path to the ACES.icc file shipped with RT to the correct one)

{"working_spaces": [
    {
        "name" : "ACES",
        "file" : "/opt/rawtherapee/share/rawtherapee/iccprofiles/output/ACES.icc"
    },
    {
        // D60 matrix from Elle's ACEScg profile
        "name" : "ACEScg",
        "matrix" : [0.68988037, 0.14976501, 0.1245575, 0.28451538, 0.67169189, 0.04379272, -0.00604248, 0.01000977, 0.82093811]
    }
]}
Desmis commented 6 years ago

@agriggio When I used ICC profile inspector, or Exiftool, and the profile "Elle" which are on his web or in iccprofiles, the values are //from Elle Stone constexpr double xyz_ACESc[3][3] = { {0.68988, 0.14977, 0.12456}, {0.28452, 0.67169, 0.04379}, {-0.00604, 0.01001, 0.82094} };

constexpr double ACESc_xyz[3][3] = { {1.592666, -0.351803, -0.222887}, {-0.675936, 1.639273, 0.015117}, {0.0199598, -0.022576, 1.2162916} };

agriggio commented 6 years ago

@Desmis Jacques, here's what I get:

$ iccdump -v3 ~/src/elles_icc_profiles/profiles/ACES-elle-V2-g10.icc

Header:
  size         = 1132 bytes
  CMM          = 'lcms'
  Version      = 2.2.0
  Device Class = Display
  Color Space  = RGB
  Conn. Space  = XYZ
  Date, Time   = 22 Apr 2016, 21:13:34
  Platform     = *nix
  Flags        = Not Embedded Profile, Use anywhere
  Dev. Mnfctr. = 0x0
  Dev. Model   = 0x0
  Dev. Attrbts = Reflective, Glossy, Positive, Color
  Rndrng Intnt = Perceptual
  Illuminant   = 0.96420288, 1.00000000, 0.82490540    [Lab 100.000000, 0.000000, 0.000000]
  Creator      = 'lcms'

tag 0:
  sig      'cprt'
  type     'text'
  offset   264
  size     150
Text:
  No. chars = 142
    0x0000: Copyright 2016, Elle Stone (http://ninedegreesbelow.com/), CC-BY
    0x0040: -SA 3.0 Unported (https://creativecommons.org/licenses/by-sa/3.0
    0x0080: /legalcode).\000

tag 1:
  sig      'wtpt'
  type     'XYZ '
  offset   416
  size     20
XYZArray:
  No. elements = 1
    0:  0.95265198, 1.00000000, 1.00881958    [Lab 100.000000, -2.004650, -13.878165]

tag 2:
  sig      'bkpt'
  type     'XYZ '
  offset   436
  size     20
XYZArray:
  No. elements = 1
    0:  0.00000000, 0.00000000, 0.00000000    [Lab 0.000000, 0.000000, 0.000000]

tag 3:
  sig      'rXYZ'
  type     'XYZ '
  offset   456
  size     20
XYZArray:
  No. elements = 1
    0:  0.99089050, 0.36189270, -0.00271606    [Lab 66.664288, 148.259567, 120.066311]

tag 4:
  sig      'gXYZ'
  type     'XYZ '
  offset   476
  size     20
XYZArray:
  No. elements = 1
    0:  0.01223755, 0.72251892, 0.00825500    [Lab 88.089694, -332.032008, 136.365936]

tag 5:
  sig      'bXYZ'
  type     'XYZ '
  offset   496
  size     20
XYZArray:
  No. elements = 1
    0:  -0.03892517, -0.08441162, 0.81936646    [Lab -76.248704, 171.475652, -303.428428]

tag 6:
  sig      'rTRC'
  type     'curv'
  offset   516
  size     14
Curve:
  Curve is gamma of 1.00000000

tag 7:
  sig      'gTRC'
  type     'curv'
  offset   532
  size     14
Curve:
  Curve is gamma of 1.00000000

tag 8:
  sig      'bTRC'
  type     'curv'
  offset   548
  size     14
Curve:
  Curve is gamma of 1.00000000

tag 9:
  sig      'dmnd'
  type     'desc'
  offset   564
  size     404
TextDescription:
  ASCII data, length 104 chars:
    0x0000: ACES chromaticities from TB-2014-004, http://www.oscars.org/scie
    0x0040: nce-technology/aces/aces-documentation\000
  Unicode Data, Language code 0x0, length 105 chars
    0x0000: 0041 0043 0045 0053 0020 0063 0068 0072 006f 006d 0061 0074 0069 
    0x000d: 0063 0069 0074 0069 0065 0073 0020 0066 0072 006f 006d 0020 0054 
    0x001a: 0042 002d 0032 0030 0031 0034 002d 0030 0030 0034 002c 0020 0068 
    0x0027: 0074 0074 0070 003a 002f 002f 0077 0077 0077 002e 006f 0073 0063 
    0x0034: 0061 0072 0073 002e 006f 0072 0067 002f 0073 0063 0069 0065 006e 
    0x0041: 0063 0065 002d 0074 0065 0063 0068 006e 006f 006c 006f 0067 0079 
    0x004e: 002f 0061 0063 0065 0073 002f 0061 0063 0065 0073 002d 0064 006f 
    0x005b: 0063 0075 006d 0065 006e 0074 0061 0074 0069 006f 006e 0000 0000 
    0x0068: 0000 
  No ScriptCode data

tag 10:
  sig      'desc'
  type     'desc'
  offset   968
  size     164
TextDescription:
  ASCII data, length 24 chars:
    0x0000: ACES-elle-V2-g10.icc\000\000\000
  Unicode Data, Language code 0x0, length 25 chars
    0x0000: 0041 0043 0045 0053 002d 0065 006c 006c 0065 002d 0056 0032 002d 
    0x000d: 0067 0031 0030 002e 0069 0063 0063 0000 0000 0000 0000 0000 
  No ScriptCode data

The relevant tags are rXYZ, gXYZ and bXYZ, therefore:

rXYZ: 0.99089050, 0.36189270, -0.00271606
gXYZ: 0.01223755, 0.72251892, 0.00825500
bXYZ: -0.03892517, -0.08441162, 0.81936646

transposing, they give the following matrix:

 0.99089050, 0.01223755, -0.03892517
 0.36189270, 0.72251892, -0.08441162
-0.00271606, 0.00825500,  0.81936646

which looks reasonable to me...

agriggio commented 6 years ago

to cross-check, just try the same on sRGB-elle-V2-g10.icc, and you get essentially the same matrix as the one of RT (xyz_sRGB in iccmatrices.h)

Desmis commented 6 years ago

That is different with values display by utilitaries : ICC profile inspector, etc. For me, with these (yours) values (those I used before Bradford D60), much images are bad or very bad...

I just commit and push a change with the values of the normalization ACES2065-1 (I think it's good) constexpr double ACESc_xyz[3][3] = { {0.9525523959, 0.3439664498, 0.0}, {0.000000, 0.7281660966 , 0.0}, {0.0000936786, -0.0721325464, 1.0088251844} };

constexpr double xyz_ACESc[3][3] = { {1.0498110175, -0.4959030231, 0.0}, {0.0, 1.3733130458 , 0.0}, {-0.0000974845, 0.0982400361, 0.9912520182} }; And results are better, no diffrences on my test image, and quite smal on others

Desmis commented 6 years ago

I think, it's only my responsibility, that 1) for working profile, we must used the norm ACES2065-1 2) for ICC profiles V2, V4 we must work with actual profiles, those of Elle, or mine for which utilities give the same XYZ results as you :)

Desmis commented 6 years ago

But the question is, why with the same primaries, and same white, there are diffrences betwwen calculations made with LCMS and the norm ? Probably (sure) LCMS is not perfect for this "special" working space.

agriggio commented 6 years ago

Jacques, I'm not sure I understand this:

For me, with these (yours) values (those I used before Bradford D60), much images are bad or very bad...

The values I provided above should not require any Bradford adaptation -- they are already adapted to D50. I have tried with a bunch of different pictures and I see no difference wrt. ProPhoto...

Desmis commented 6 years ago

Me also I don't understand. In preview, with the matrix coming from ICC profiles, images are quasi always bad, some are acceptables

They are less bad, with D60 adaptation, even there is no need (cf B.Lindbloom ... but it does not speak of ACES... ) for current profile sRGB, Prophoto,...), but if you look at the values of the matrix, this modified by D60 adpatation is near "norm"

They are pretty good with matrix "norm"

agriggio commented 6 years ago

Jacques, are you using the dev branch? Here are a couple of screenshots (neutral profile, only changed the working space): prophoto aces prophoto2 aces2

Desmis commented 6 years ago

Effectively when I load XYZ with workingspaces.json, the result is good Why when the same matrix is in "iccmatrices.h" the result is bad ??

agriggio commented 6 years ago

Jacques, I don't know... I can try taking a look at your branch, but I can't promise that I will find the explanation :-)

Desmis commented 6 years ago

there is no special thing, only the XYZ values...

For me it is a great mysteries :)

I will suppress, it is easy, ACES from "testoutputprofie", but I don't like to not understand someting

:) Jacques

agriggio commented 6 years ago

@Desmis I agree 100% with you -- we want to understand this and not just hide the problem, so please leave ACES there, I'll try to take a look ASAP

Desmis commented 6 years ago

OK :)

jacques

Desmis commented 6 years ago

I just try to put in workingspaces.json, the values of norm, all works well...Curious :) {"working_spaces": [ { "name" : "ACES", "file" : "G:/code/rtdev/build/release/iccprofiles/output/ACES.icc" }, { "name" : "ACESNorm", "matrix" : [0.9525523959, 0.0, 0.0000936786, 0.3439664498, 0.7281660966, -0.0721325464, 0.0, 0.0, 1.0088251844] } ]}

agriggio commented 6 years ago

ok, I don't understand what's going on... @heckflosse @Floessie can you also take a look? (Just read the conversation above to get some context)

Desmis commented 6 years ago

I just tested to change the place of "ACES" in the table and the name in iccmatrice.h const double(wprofiles[])[3] = {xyz_sRGB, xyz_adobe, xyz_prophoto, xyz_widegamut, xyz_CESc, xyz_bruce, xyz_beta, xyz_best, xyz_rec2020}; const double(iwprofiles[])[3] = {sRGB_xyz, adobe_xyz, prophoto_xyz, widegamut_xyz, CESc_xyz, bruce_xyz, beta_xyz, best_xyz, rec2020_xyz}; const char* wpnames[] = {"sRGB", "Adobe RGB", "ProPhoto", "WideGamut", "ACESc", "BruceRGB", "Beta RGB", "BestRGB", "Rec2020" }; Instead of being at the end.

In this case, all works fine, including the last of the list "Rec_2020"

Curious ?? Jacques