ACES 2.0 Output Transform performance optimisation

[KevinJW]

As discussed in the meeting this pull request is not complete, but we have a deadline and so I am putting in the request in the current state, I would encourage others to check the code.

There are some test failures, some come from not recalculating the golden values in the CPU path,.

There must still be some lingering differences between the GPU and CPU code paths as the GPU/CPU comparisons fail with larger deltas than typical.

Overall the main algorithm changes come from:

1. Precomputing a number of the scaling factors, etc during the init stage
2. Some micro optimisations, reformulating things to use fewer divisions
3. Redistribution of the sampled hues to be more linear whilst still including explicit sampling of the corners of the gamut. This allows us to have a much narrower search range for the hue table lookup.
4. Some calculations have been migrated up the "call stack" to allow the results to be reused
5. Adjustment of the upper hull slope calculation parameters to allow for the elimination of a call to pow replaced with a multiply
6. Splitting some functions to allow exposure of the Achromatic and using that in the tonescale calculation
7. The gamut compression was calculating various slopes which nominally should all have been the same value

Things not really done, but could be in the future mostly CPU side and primarily by vectorising:

1. A number of the functions could be looked at and reformulated to take/return only the attributes they change, e.g. J or M rather than full JMh, might help with a vectorised implementation
2. The 3x3 matrix uses could be much faster may want to try 3x4 and a simple vectorised code, alternatively...
3. Generally the whole transform is run per pixel, may be worth looking at running smaller parts of the code on lines of pixels and using intermediate buffers for partial results.
4. May be worth reforming the pow() calls into appropriate logarithm/exponent form
5. The Hue lookup binary search whilst heavily improved could probably be unrolled for many of the common gamuts as the search range is often within 3 positions

General observations

The algorithm makes heavy use pow(), so it is heavily influenced by the system C library implementations.

We need finer grained testing for C++ functions rather than at an "Op/Transform" level

GPU shader generation notes:

We may want to add wrappers for compatibility to the shader generator

• log10()
• atan2()

We have a sign() wrapper but none of the code uses it.

[alexfry]

Getting some build errors on my side, Doug thinks it might related to AVX intrinsics? :

[ 17%] Building CXX object src/OpenColorIO/CMakeFiles/OpenColorIO.dir/ops/fixedfunction/FixedFunctionOpCPU.cpp.o [ 17%] Building CXX object src/OpenColorIO/CMakeFiles/OpenColorIO.dir/ops/fixedfunction/FixedFunctionOpData.cpp.o [ 17%] Building CXX object src/OpenColorIO/CMakeFiles/OpenColorIO.dir/ops/fixedfunction/FixedFunctionOpGPU.cpp.o [ 17%] Building CXX object src/OpenColorIO/CMakeFiles/OpenColorIO.dir/ops/fixedfunction/FixedFunctionOp.cpp.o In file included from /Users/afry/GitHub/OpenColorIO_kevin/src/OpenColorIO/ops/fixedfunction/ACES2/Transform.cpp:4: In file included from /Users/afry/GitHub/OpenColorIO_kevin/src/OpenColorIO/ops/fixedfunction/ACES2/Transform.h:7: /Users/afry/GitHub/OpenColorIO_kevin/src/OpenColorIO/ops/fixedfunction/ACES2/Common.h:84:44: warning: taking the max of unsigned zero and a value is always equal to the other value [-Wmax-unsigned-zero] 84 | return std::min(nominal_size - 1U, std::max(0U, entry)); | ^~~~~~~~ ~~ /Users/afry/GitHub/OpenColorIO_kevin/src/OpenColorIO/ops/fixedfunction/ACES2/Common.h:84:44: note: remove call to max function and unsigned zero argument 84 | return std::min(nominal_size - 1U, std::max(0U, entry)); | ^~~~~~~~ ~~~ In file included from /Users/afry/GitHub/OpenColorIO_kevin/src/OpenColorIO/ops/fixedfunction/FixedFunctionOpCPU.cpp:10: In file included from /Users/afry/GitHub/OpenColorIO_kevin/src/OpenColorIO/ops/fixedfunction/ACES2/Transform.h:7: /Users/afry/GitHub/OpenColorIO_kevin/src/OpenColorIO/ops/fixedfunction/ACES2/Common.h:84:44: warning: taking the max of unsigned zero and a value is always equal to the other value [-Wmax-unsigned-zero] 84 | return std::min(nominal_size - 1U, std::max(0U, entry)); | ^~~~~~~~ ~~ /Users/afry/GitHub/OpenColorIO_kevin/src/OpenColorIO/ops/fixedfunction/ACES2/Common.h:84:44: note: remove call to max function and unsigned zero argument 84 | return std::min(nominal_size - 1U, std::max(0U, entry)); | ^~~~~~~~ ~~~ /Users/afry/GitHub/OpenColorIO_kevin/src/OpenColorIO/ops/fixedfunction/ACES2/Transform.cpp:273:29: error: unknown type name '__m256' 273 | inline float hsum256_ps_avx(__m256 v) { // v = [ H G | F E | D C | B A ] | ^ 1 warning and 1 error generated. [ 17%] Building CXX object src/OpenColorIO/CMakeFiles/OpenColorIO.dir/ops/gamma/GammaOpCPU.cpp.o make[2]: *** [src/OpenColorIO/CMakeFiles/OpenColorIO.dir/ops/fixedfunction/ACES2/Transform.cpp.o] Error 1 make[2]: *** Waiting for unfinished jobs.... [ 18%] Building CXX object src/OpenColorIO/CMakeFiles/OpenColorIO.dir/ops/gamma/GammaOpData.cpp.o In file included from /Users/afry/GitHub/OpenColorIO_kevin/src/OpenColorIO/ops/fixedfunction/FixedFunctionOpGPU.cpp:10: In file included from /Users/afry/GitHub/OpenColorIO_kevin/src/OpenColorIO/ops/fixedfunction/ACES2/Transform.h:7: /Users/afry/GitHub/OpenColorIO_kevin/src/OpenColorIO/ops/fixedfunction/ACES2/Common.h:84:44: warning: taking the max of unsigned zero and a value is always equal to the other value [-Wmax-unsigned-zero] 84 | return std::min(nominal_size - 1U, std::max(0U, entry)); | ^~~~~~~~ ~~ /Users/afry/GitHub/OpenColorIO_kevin/src/OpenColorIO/ops/fixedfunction/ACES2/Common.h:84:44: note: remove call to max function and unsigned zero argument 84 | return std::min(nominal_size - 1U, std::max(0U, entry)); | ^~~~~~~~ ~~~ 1 warning generated. 1 warning generated. make[1]: *** [src/OpenColorIO/CMakeFiles/OpenColorIO.dir/all] Error 2 make: *** [all] Error 2 [syd-silver0017:~/GitHub/OpenColorIO_kevin/build] afry%

[doug-walker]

Thank you for this excellent work @KevinJW !

I've created a new branch here named aces2_optimization. I'd like to propose that you make your PR against that branch rather than main. That way we can merge what you've got so far and the group of us could work in parallel to propose further PRs against that branch.

Don't worry about the AVX-related compilation failures that Alex found on macOS, Cuneyt and I will fix those ASAP and pull in the Metal fix.

[KevinJW]

I note some of the failing errors relate to C++14 features, e.g. constexpr std::min/max I thought we had set that as our assumed base requirement, but I note it is not explicitly checked for in the cmake files and I see the documentation still allows for C++ 11 https://github.com/AcademySoftwareFoundation/OpenColorIO/blob/main/docs/quick_start/installation.rst

There is a simple fix for this specific failure, but it does open up the supported matrix question.

[KevinJW]

I note some of the build failures appear to come from the build asking for certain instruction levels ,, e.g. sse2 or avx2 etc but when building, the options for flagging to the compiler to target that specific architecture e.g. -mavx2 in the case of GCC don't appear to be on the command lines for the compilation steps, so the code has pre-processor guards around the intrinsics but they are enabled via OCIO_USE_AVX2 even though the compiler has not been allowed to issue those instruction classes.

Trying to force the flags into the specific source file property doesn't seam to work, when I've tried it, perhaps somebody else with some familiarity would like a go.

Brute forcing the option into the CMAKE_CXX_FLAGS does work for my manual builds with gcc.

[doug-walker]

Thank you Kevin! I will go ahead and merge this into the aces2_optimization branch. Cuneyt will now work on getting the CPU side finished, with unit tests passing, to unblock Alex. He will then proceed to get the GPU to match the CPU.

I suggest you raise the C++ 11 question on the OCIO Slack. The last time the TSC discussed it, we were open to raising to C++ 14, if there was a good justification.