Page MenuHome

OpenColorIO: use aces_interchange role to detect CIE XYZ values
ClosedPublic

Authored by Brecht Van Lommel (brecht) on Feb 1 2021, 3:58 PM.

Details

Summary

We need standard CIE XYZ values for rendering effects like blackbody emission.
The relation to the scene linear role is based on OpenColorIO configuration.

In OpenColorIO 2.0 configs roles can no longer have the same name as color
spaces, which means our XYZ role and colorspace in the configuration give an
error.

Instead use the new standard aces_interchange role, which relates scene linear
to a known scene referred color space. Compatibility with the old XYZ role is
preserved, if the configuration file has no conflicting names.

Also includes a non-functional change to the configuraton file to use an
XYZ-to-ACES matrix instead of REC709-to-ACES, makes debugging a little easier
since the matrix is the same one we have in the code now and that is also
found easily in the ACES specs.

There is a new cie_xyz_d65_interchange role that we should add support for,
for interchange of display referred colorspace (as opposed to scene referred).
I couldn't immediately understand how that is supposed to be defined, will
probably leave that for another patch unless someone knows how.

Depends on D10273

Event Timeline

I guess just carefully look at this for now? :) Seems fine for far.

intern/cycles/render/shader.cpp
851–862

Outside of review.
What's your opinion on making make_transform to receive 4 float4 ? To me it seems to be ore readable than 16 stand-alone values.

intern/opencolorio/ocio_impl.cc
445–453

Since BLI_math_matrix.h is included in this file, use unit_m3().

release/datafiles/colormanagement/config.ocio
101–103

In OCIO 2, is there a speed penalty when chaining two matrix transforms?

Brecht Van Lommel (brecht) marked 2 inline comments as done.Feb 2 2021, 6:30 PM
Brecht Van Lommel (brecht) added inline comments.
intern/cycles/render/shader.cpp
851–862

It would help now that we have clang-format, but indeed not something I want to handle as part of this code review.

release/datafiles/colormanagement/config.ocio
101–103

No, OCIO will combine these into a single matrix for CPU and GPU processing.

Evan Wilson (EAW) added inline comments.
intern/opencolorio/ocio_impl.cc
476

One two too many Xs?

intern/opencolorio/ocio_impl.cc
476

This is a good catch!

Brecht Van Lommel (brecht) marked an inline comment as done.

Address comments.

This revision is now accepted and ready to land.Feb 3 2021, 3:36 PM