search

imageworks / OpenColorIO-Configs

Color Configurations for OpenColorIO
opencolorio.org
441 stars 804 forks source link

PR: Fix incorrect "Canon BT.2020 Tungsten" matrix. #12

Closed KelSolaar closed 6 years ago

KelSolaar commented 6 years ago

References imageworks/OpenColorIO-Configs#11.

scoopxyz commented 6 years ago

Thanks for the PR!

Just to be clear to others, the actual diff in this PR is here:

index 0cf0df1..f692ce4 100644
--- a/aces_1.0.3/python/aces_ocio/colorspaces/canon.py
+++ b/aces_1.0.3/python/aces_ocio/colorspaces/canon.py
@@ -225,7 +225,7 @@ def create_c_log(gamut,
             'type': 'matrix',
             'matrix': [0.724488568, 0.115140904, 0.160370529, 0,
                        0.010659276, 0.839605344, 0.149735380, 0,
-                       0.014560161, 0.028562057, 1.014001897, 0,
+                       0.014560161, -0.028562057, 1.014001897, 0,
                        0, 0, 0, 1],
             'direction': 'forward'})

The large diff is due to small changes after re-running the config generation script. For example:

index 400873b..644189a 100644
--- a/aces_1.0.3/baked/lustre/Rec.709 for ACEScc Lustre.3dl
+++ b/aces_1.0.3/baked/lustre/Rec.709 for ACEScc Lustre.3dl
@@ -3192,7 +3192,7 @@ Mesh 6 12
 318 4095 2079
 318 4095 2081
 319 4095 2083
-319 4095 2086
+320 4095 2086
 320 4095 2089
 322 4095 2093
 323 4095 2097

Unfortunately, due to a lack of Unit testing at the moment, we are unable to currently validate the level of impact on imagery. Though I think we can mutually agree that these changes are within the same level of tolerance compared to an end user running the generation scripts themselves.

KelSolaar commented 6 years ago

This is quite annoying and will happen every time somebody will re-build the config with different hardware/os/architecture than the person having done the previous build.

scoopxyz commented 6 years ago

Interesting article of floating-point determinism: https://randomascii.wordpress.com/2013/07/16/floating-point-determinism/

But this honestly highlights that only the source code should be tracked by the repository, as the LUTs are simply a byproduct of the source execution (though they do need to be readily accessible). So instead, the configs source should be maintained and tested through a CI system that then deploys the resultant build artifacts as a separate release (https://docs.travis-ci.com/user/deployment/releases) This would also ensure that the build system is relatively consistent and these useless LUT diffs should be mitigated.

Related to #13

scoopxyz commented 6 years ago

Correction: the above only applies to the ACES LUTs since the SPI LUTs aren't currently derived from equations.