AcademySoftwareFoundation / rawtoaces

RAW to ACES Utility
141 stars 47 forks source link

XYZ to ACES matrix #108

Closed kisakata closed 11 months ago

kisakata commented 6 years ago

Values of XYZ to ACES matrix defined in XYZ_acesrgb was not correct. It was using value with D65 to D50 Bradford matrix applied.

Since XYZ to ACES matrix was incorrect, renderNonDNG() in acesrender.cpp was applying chromatic adaptation twice.

Since rta.cpp uses XYZ_acesrgb too, I I created a variable called XYZ_D65_acesrgb and copied the original value of XYZ_acesrgb, and updated rta.cpp to use XYZ_D65_acesrgb, so that it still uses same value.

One other thing I noticed is that converted values in XYZ colorspace is not in D50, but in D65. (LibRaw converts RAW image once to sRGB and then converts to XYZ, so the values in XYZ are in D65) I've defined variable called d65, and applyCAT uses this value instead of d50.

spatre commented 6 years ago

The entire Pull request is valid except for a typo on line 2 in the summary: "It was using value with D65 to D50 Bradford matrix applied." We've confirmed that the XYZ_acesrgb matrix is in fact using a D65 to D60 Bradford matrix. Your code changes are being verified by the admin, to be updated to the source master.

kisakata commented 6 years ago

Thanks @spatre, for catching the typo.

JamesEggleton commented 5 years ago

Is this pull request likely to be reviewed soon? It seems fairly critical to producing correct ACES renders. Many thanks.