Integrating matrix multiplication fix from OSL #2121
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
See AcademySoftwareFoundation/OpenShadingLanguage#1513 for more details. Signed-off-by: Jerry Gamache <jerry.gamache@autodesk.com>
JGamache-autodesk
force-pushed
the
gamaj/integrate_matrix_math_fix_from_osl
branch
from
b909967 to
87f5a94
Compare
doug-walker
approved these changes
Mar 10, 2025
cozdas
approved these changes
Mar 10, 2025
Collaborator
cozdas
left a comment
cozdas
left a comment
There was a problem hiding this comment.
Verified that the transform() function in OSL multiplies by the transposed matrix.
Approved.
doug-walker
added a commit
to autodesk-forks/OpenColorIO
that referenced
this pull request
Mar 15, 2025
…tion#2121) See AcademySoftwareFoundation/OpenShadingLanguage#1513 for more details. Signed-off-by: Jerry Gamache <jerry.gamache@autodesk.com> Co-authored-by: Doug Walker <doug.walker@autodesk.com> (cherry picked from commit fed973f) Signed-off-by: Doug Walker <doug.walker@autodesk.com>
doug-walker
added a commit
that referenced
this pull request
Mar 19, 2025
* Add support for Windows ARM64 (#2089) * Add support for Windows ARM64 Signed-off-by: Anthony Roberts <anthony.roberts@linaro.org> * Fix improper compiler flag check Signed-off-by: Anthony Roberts <anthony.roberts@linaro.org> * Fix sse2neon issues on Windows ARM64 Signed-off-by: Anthony Roberts <anthony.roberts@linaro.org> * Fix cross-compilation on Windows for X64 -> ARM64 Signed-off-by: Anthony Roberts <anthony.roberts@linaro.org> * Fix comment to match with corresponding if directive Signed-off-by: Anthony Roberts <anthony.roberts@linaro.org> * Check for MSVC before setting MSVC-style flag Signed-off-by: Anthony Roberts <anthony.roberts@linaro.org> * Fix comment to resolve ambiguity Signed-off-by: Anthony Roberts <anthony.roberts@linaro.org> --------- Signed-off-by: Anthony Roberts <anthony.roberts@linaro.org> Co-authored-by: Doug Walker <doug.walker@autodesk.com> (cherry picked from commit c09951e) Signed-off-by: Doug Walker <doug.walker@autodesk.com> * Fix issue with ocio_depts handling spaces in file paths (#2109) Signed-off-by: Taegyun Ha <taegyun.ha@disguise.one> (cherry picked from commit c5c85b0) Signed-off-by: Doug Walker <doug.walker@autodesk.com> * Issue #2116 : Fixes Metal backend's generated shaders with float/int constant Array Performance (#2117) * Issue #2116 : Improves Metal Backend Perf. moves the constant float/int declaration to constant space so it doesnt get initialized per thread. This improved color correction performance on M4 Max 3-4 times better. Signed-off-by: Morteza <smostajabodaveh@apple.com> * Tiny refactoring to improve code maintainability Signed-off-by: Morteza <smostajabodaveh@apple.com> --------- Signed-off-by: Morteza <smostajabodaveh@apple.com> (cherry picked from commit d807b38) Signed-off-by: Doug Walker <doug.walker@autodesk.com> * Adsk Contrib - Issue #2111 Absolute paths not working through proxy (#2112) * Ticket #2111 - Do not use config proxy for absolute paths while computing file hash or loading LUT data. - Added the unit test provided in the ticket. Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com> * - Changing the logic so that for abs paths we first try the configProxy and if that fails fall back to file system. For relative paths, we don't fall back to file system though, proxy is expected to handle those. - Removed the unnecessary closeLutStream() function. We're using unique pointers, that means RAII is in place. The whole idea behind RAII is we don't need to worry about the cleanup or the type of the object wrapped by the RAII handler (unique_ptr in this case). - Cleaned up some unnecessary conversions, type shuffling and copies around the code I touched. - Cleaned up some unsafe type casts which are prone to dereferencing null pointers. Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com> * - Ah! make_unique is a c++14 feature and we support C++11. I wonder why windows build is configured to use c++14+ while other platforms use C++11. Replacing make_unique with the new syntax to make the other platforms happy too. Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com> * - Minor cleanup - Added a test for absolute path to inexistent file. Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com> --------- Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com> Co-authored-by: Doug Walker <doug.walker@autodesk.com> (cherry picked from commit af69f39) Signed-off-by: Doug Walker <doug.walker@autodesk.com> * Change recommended Imath version to 3.1.12. This should fix Issue #1764. (#2120) Signed-off-by: Mark Titchener <mark.titchener@foundry.com> Co-authored-by: Doug Walker <doug.walker@autodesk.com> (cherry picked from commit 7237eaa) Signed-off-by: Doug Walker <doug.walker@autodesk.com> * Integrating matrix multiplication fix from OSL (#2121) See AcademySoftwareFoundation/OpenShadingLanguage#1513 for more details. Signed-off-by: Jerry Gamache <jerry.gamache@autodesk.com> Co-authored-by: Doug Walker <doug.walker@autodesk.com> (cherry picked from commit fed973f) Signed-off-by: Doug Walker <doug.walker@autodesk.com> * Add missing setConfigIOProxy call to the Python API (#2128) * Add missing setConfigIOProxy call to the Python API Signed-off-by: Rémi Achard <remiachard@gmail.com> * Restore a clean cache for other unit tests Signed-off-by: Rémi Achard <remiachard@gmail.com> --------- Signed-off-by: Rémi Achard <remiachard@gmail.com> Co-authored-by: Doug Walker <doug.walker@autodesk.com> (cherry picked from commit 30db204) Signed-off-by: Doug Walker <doug.walker@autodesk.com> * ACES 2.0 Output Transform performance optimisation (#2127) * ACES 2.0 Output Transform performance optimisation (#2119) * Extend ocioperf to take config file parameter on CLI Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Extend ocioconvert to take config on command line Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Extract tonescale_fwd function Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Extract inverse tonescale function Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Combine c and Z variables in J calculation exponent replace 100.0 entries when referring to the scale of J Extract calculation of nonlinear compression into functions Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Split RGB<->JMh function into two parts to expose opponent intermediate values Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Use function to compute matrix multiply for LMS calculations Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Remove unused member variable from JMhParams structure Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Combine chromatic adaptation weights into LMS matrix (and inverse) - CHANGES PIXEL OUTPUT Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Use matrix form for transforming cone responses to Aab Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Normalise the F_L parameter Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Remove ra and ba related variables to avoid them being out of sync with opponent calculation Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Make A<->J conversion function generic Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Deduplicate Y<->J conversions Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Factor JMh scaling parameters into Aab matrices Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * factor our references to PI, 360 and 180 constants Avoid looking up cusp twice during inverse Whilst searching for the cusp we have already constrained the search so we do not need to clamp Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Add functions to explain some of the calculations Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Further clarify when 100 means reference luminance Migrate rescaling into tonescale s_2 parameter Rename model_gamma to reflect it is actually the inverse Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * migrate init steps performed within other init functions to the top level to avoid repeat init of precomputed values. Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * extract some of the fixed values that only depend on the hue to reduce recomputation during inverse gamut mapping Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Avoid double lookup for reachMaxM value by resolving once the hue is known. also reduces size of object on stack by not passing the whole table. Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Push wrapping of hues to the boundary, mark up conversion points from external inputs etc Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Store gamma values as reciprocals move more magic constants into const variables factor some of the complex expressions into function (temporarily makes things slower) Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Add some missing includes to headers Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * minor cleanup to use std::array instead of plain array for test samples Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Inline reach boundary finding restructure find_gamut_boundary_intersection to highlight common patterns. Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Extract gamut mapper compression function rework get_focus_gain to directly computer the slope_gain Share calculation of analytical thereshold Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Rework gamut mapper to compress absolute M then only recalculate calculate J Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Precalculate maximum search range for cusp lookup next steps would be to factor hue into separate table to improve cache hits followed by redistribution to more uniform hues which should narrow search range Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Experiment with reusing slope calculations in gamut mapper presmooth cusp values Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Add a collection of TODO's Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Restore function mapping table index to hue Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Minor tweaks to tonescale inverse clamp Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Remove duplicate table whilst calculating upper hull gamma Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Add some additional sample points for the upper hull gamma finder Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Slight tidy up of gamma fitting code Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Experiment with alternate smin implementation Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Remove unused function and tidy up comments Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Extract hue search into separate function Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Extract hues into separate table, merge gamma values into their place (gamma values now sampled on cusp hue intervals). Removes extra texture from GPU path. Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Simplify upper hull gamma hue lookup to avoid unneeded lerping as we are sampling the table entries directly Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Split out tonescale function, minor tweaks to Aab->JMh Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Build tables more uniformly, needs some clean up and lots of testing Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Speed up reach corner finding by switching to testing against the Achromatic rather than J limit Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Speed up hull gamma finding by computing values which depend only on the test points and not the gamma values themselves Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Adjust GPU hue lookup to take advantage of more uniform distribution Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Fix GLSL compatibility with hue lookup Remove compiler warnings for unused parameters Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Attempt to simplify table generation code Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Explicilty allow GCC to perform additional optimisations - Needs some discussion Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Add extra entries to reach table to avoid needing to clamp to range during pixel processing Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * GPU move reach Max M sampling to avoid looking it up multiple times per pixel Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Remove smoothing from GPU path, it is baked into the csup Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Fix bug with reach lookup Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Try only wrap hues on input to the shaders Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * rework GPU camut compressor to follow the same algorithm as CPU. Not 100% the same GPU still recalculates some values Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Rework solve_J_intersect to have fewer div instructions Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Adjust GPU code to better align with CPU code's structure, some additional precomputation is now applied during shader generation Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Precompute more scaling factors into matrices and nonlinear functions Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Experiment with unsigned integers for array access Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Bypass one J-> A conversion by saving the Aab computed earlier Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Test intrinsics for compression Norm calculation Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Attempt to calculate sin/cos only once per pixel. Some minor micro optimisations. Further alignment of GPU with CPU code, Tests values need evaluating Some GPU results are different - TBD Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Remove unused parameters Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Try tree vectoriser for gcc Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Add Vectorise option for MSVC Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Remove unused function Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Constexpr std::max is only available in C++ 14 for now avoid the call to it Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Try to fir intrinsic based errors on osome build configurations Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Another C++ 14 usage fix Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Remove check for CLANG left over from testing Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> --------- Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> * Update ACES2 CPU non-SIMD path (#2122) * - Commenting out the ACES2 SIMD implementation for now to focus on validity of the scalar math. For SIMD we need to do implement run-time switching logic too. - Slight improvements to the unit tests so that we print out the computed error metric as well as the actual and expected values. Helps to see the magnitude of the error. - FixedFunctionOpCPU and BuiltinTransform tests now produce error lines with the same structure & syntax, including the computed error. - Updated the expected values for ACES2 tests with the values the new optimized code produces, this makes all of the of CPU tests pass now. - For ACES2 ops and builtin transforms, the error threshold is increased to 1e-4 - added few, temporary code snippets that dumps the currently produced results, making it easier to update the golden values if needed again. Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com> * - Fixing Linux build Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com> * Making Linux build happy is never easy. Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com> --------- Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com> * Address GPU unit test failures (#2123) * - Weights for cos(3h) and sin(h) in chroma_compress_norm() looks wrong. Fixing the weights makes the GPU tests pass now (except of the inverse output transform which seems to have a separate issue). - If the new weights are correct, I'll need to update the CPU test target values too. Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com> * - Updating the expected values in the CPU tests Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com> * - The remaining GPU test failures were caused by a simple typo where we were passing h instead of J to ocio_tonescale_inv() function. With the fix all the unit tests are happy now. - Since we decided not to include any SIMD implementation in this version, I removed the conditional code paths and left the current SSE & AVX implementations as commented out for future guidence. Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com> --------- Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com> * Remove unused code for old gamut table calculations (#2124) Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com> * Minor code cleanup Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com> * Adding negative A trap on Aab_to_JMh_Shader() per code review Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com> * Adding copysign to tonescale to make it aligned with the CPU implementation. It's possible that on GPU we may never receive negative J due to prior guarding, but for now aligning with the CPU to be on the safer side. Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com> * Add built-in transform round-trip test Signed-off-by: Doug Walker <doug.walker@autodesk.com> * Loosen tolerance for other machines Signed-off-by: Doug Walker <doug.walker@autodesk.com> * Add GPU round-trip tests Signed-off-by: Doug Walker <doug.walker@autodesk.com> * Loosen tolerances for other GPUs Signed-off-by: Doug Walker <doug.walker@autodesk.com> --------- Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com> Signed-off-by: Doug Walker <doug.walker@autodesk.com> Co-authored-by: Kevin Wheatley <kevin.wheatley@framestore.com> Co-authored-by: Doug Walker <doug.walker@autodesk.com> (cherry picked from commit 1931542) Signed-off-by: Doug Walker <doug.walker@autodesk.com> * Increment library version to 2.4.2 Signed-off-by: Doug Walker <doug.walker@autodesk.com> * Propose NaN fix for the ACES2 inverse output transforms (#2132) * Propose Aab_to_RGB NaN fix Signed-off-by: Doug Walker <doug.walker@autodesk.com> * Fix for test on ARM Signed-off-by: Doug Walker <doug.walker@autodesk.com> * Fix for tests on Linux/Windows Signed-off-by: Doug Walker <doug.walker@autodesk.com> * Fix for GPU test on Linux Signed-off-by: Doug Walker <doug.walker@autodesk.com> * NaN fix for gamma and double log fixed functions Signed-off-by: Doug Walker <doug.walker@autodesk.com> * Remove commented-out code Signed-off-by: Doug Walker <doug.walker@autodesk.com> --------- Signed-off-by: Doug Walker <doug.walker@autodesk.com> (cherry picked from commit 0546612) Signed-off-by: Doug Walker <doug.walker@autodesk.com> --------- Signed-off-by: Anthony Roberts <anthony.roberts@linaro.org> Signed-off-by: Doug Walker <doug.walker@autodesk.com> Signed-off-by: Taegyun Ha <taegyun.ha@disguise.one> Signed-off-by: Morteza <smostajabodaveh@apple.com> Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com> Signed-off-by: Mark Titchener <mark.titchener@foundry.com> Signed-off-by: Jerry Gamache <jerry.gamache@autodesk.com> Signed-off-by: Rémi Achard <remiachard@gmail.com> Signed-off-by: Kevin Wheatley <kevin.wheatley@framestore.com> Co-authored-by: Anthony Roberts <anthony.roberts@linaro.org> Co-authored-by: Taegyun Ha <110908525+DevTGHa@users.noreply.github.com> Co-authored-by: Morteza Mostajab <92918486+Morteeza@users.noreply.github.com> Co-authored-by: Cuneyt Ozdas <cuneyt.ozdas@autodesk.com> Co-authored-by: Mark Titchener <mark.titchener@foundry.com> Co-authored-by: JGamache-autodesk <56274617+JGamache-autodesk@users.noreply.github.com> Co-authored-by: Rémi Achard <remiachard@gmail.com> Co-authored-by: Kevin Wheatley <kevin.wheatley@framestore.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See AcademySoftwareFoundation/OpenShadingLanguage#1513 for more details.
As requested in review of MaterialX AcademySoftwareFoundation/MaterialX#1917
See similar fix in MaterialX here: AcademySoftwareFoundation/MaterialX#972