Adsk Contrb - Fix pass-thru exponent composition issue#2154
Merged
doug-walker merged 6 commits intoMay 18, 2025
Merged
Conversation
Signed-off-by: Doug Walker <doug.walker@autodesk.com>
doug-walker
changed the title
doug-walker
added
the
Bug
Signed-off-by: Doug Walker <doug.walker@autodesk.com>
Signed-off-by: Doug Walker <doug.walker@autodesk.com>
remia
approved these changes
May 5, 2025
cozdas
approved these changes
May 9, 2025
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an issue with the pass‐thru exponent composition, ensuring that consecutive ExponentTransforms with a NEGATIVE_PASS_THRU style are correctly composed. Key changes include:
- Updating test assertions to pass line numbers using new macros (e.g. OCIO_CHECK_EQUAL_FROM).
- Adjusting the composition logic in GammaOpData.cpp to check for BASIC_PASS_THRU_REV instead of BASIC_PASS_THRU_FWD.
- Adding additional unit tests (including gamma_comp_test2) and a minor update in the CI workflow to disable docs build.
Reviewed Changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tests/cpu/ops/gamma/GammaOpData_tests.cpp | Updated test macros with line info and added new test cases for negative style handling. |
| tests/cpu/OpOptimizers_tests.cpp | Added a new test (gamma_comp_test2) to verify optimized gamma behavior and ensured proper exception handling. |
| src/OpenColorIO/ops/gamma/GammaOpData.cpp | Fixed the condition to properly check for PASS_THRU_REV, aligning the logic with the intended behavior. |
| .github/workflows/ci_workflow.yml | Disabled documentation build in CI; ensure this decision is intentional. |
Files not reviewed (1)
- tests/data/files/gamma_comp_test2.ctf: Language not supported
Comments suppressed due to low confidence (3)
src/OpenColorIO/ops/gamma/GammaOpData.cpp:734
- Ensure that switching from BASIC_PASS_THRU_FWD to BASIC_PASS_THRU_REV is validated by unit tests covering all relevant negative style scenarios.
if (styleB == BASIC_REV || styleB == BASIC_MIRROR_REV || styleB == BASIC_PASS_THRU_REV)
src/OpenColorIO/ops/gamma/GammaOpData.cpp:734
- Double-check that both conditions for handling negative pass‐thru styles are consistent across inputs to avoid unintended outcomes.
if (styleB == BASIC_REV || styleB == BASIC_MIRROR_REV || styleB == BASIC_PASS_THRU_REV)
.github/workflows/ci_workflow.yml:492
- Confirm that disabling the documentation build is intentional and consider adding a comment to explain this decision within the CI configuration.
build-docs: 'OFF'
Signed-off-by: Doug Walker <doug.walker@autodesk.com>
Signed-off-by: Doug Walker <doug.walker@autodesk.com>
michaelHADSK
pushed a commit
to autodesk-forks/OpenColorIO
that referenced
this pull request
Jul 24, 2025
…reFoundation#2154) * Fix gamma composition issue Signed-off-by: Doug Walker <doug.walker@autodesk.com> * Adjust docs build settings to fix CI Signed-off-by: Doug Walker <doug.walker@autodesk.com> * Adjust docs build settings to fix CI Signed-off-by: Doug Walker <doug.walker@autodesk.com> * Revert CI change Signed-off-by: Doug Walker <doug.walker@autodesk.com> * Minor comment tweak Signed-off-by: Doug Walker <doug.walker@autodesk.com> --------- Signed-off-by: Doug Walker <doug.walker@autodesk.com>
michaelHADSK
pushed a commit
to autodesk-forks/OpenColorIO
that referenced
this pull request
Jul 24, 2025
…reFoundation#2154) * Fix gamma composition issue Signed-off-by: Doug Walker <doug.walker@autodesk.com> * Adjust docs build settings to fix CI Signed-off-by: Doug Walker <doug.walker@autodesk.com> * Adjust docs build settings to fix CI Signed-off-by: Doug Walker <doug.walker@autodesk.com> * Revert CI change Signed-off-by: Doug Walker <doug.walker@autodesk.com> * Minor comment tweak Signed-off-by: Doug Walker <doug.walker@autodesk.com> --------- Signed-off-by: Doug Walker <doug.walker@autodesk.com> Signed-off-by: Michael Horsch <michael.horsch@autodesk.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fix a problem where consecutive ExponentTransforms with a negative value handling style of NEGATIVE_PASS_THRU were not combined correctly during optimization.
Added several additional unit tests.
The work-around for this is to use OPTIMIZATION_NONE or set OPTIMIZATION_COMP_GAMMA to false.