-
Notifications
You must be signed in to change notification settings - Fork 3
remove clamping from ICC file transforms #34
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…ion#2206) * adding unit test for applyRGB buffer, check in the applyRGB if the buffer is C order or not, for now throw error Signed-off-by: Afsaneh Sheikhmiri <afsaneh.sheikhmiri@autodesk.com> * Adsk Contrib - Detect C/F order, if not C order : RuntimeError shown, test is still not passing Signed-off-by: Afsaneh Sheikhmiri <afsaneh.sheikhmiri@autodesk.com> * remove 1d arrays from the test, since even after Transpose, 1d arrays are still contiguous Signed-off-by: Afsaneh Sheikhmiri <afsaneh.sheikhmiri@autodesk.com> --------- Signed-off-by: Afsaneh Sheikhmiri <afsaneh.sheikhmiri@autodesk.com> Co-authored-by: Doug Walker <doug.walker@autodesk.com>
* Enable Python 3.14 wheels Signed-off-by: Rémi Achard <remiachard@gmail.com> * Replace macos-13 by macos-15-intel Signed-off-by: Rémi Achard <remiachard@gmail.com> * Allow find_package CONFIG mode with Package_ROOT Signed-off-by: Rémi Achard <remiachard@gmail.com> --------- Signed-off-by: Rémi Achard <remiachard@gmail.com>
…2217) Signed-off-by: Carol Payne <carol.alynn.payne@gmail.com> Co-authored-by: Doug Walker <doug.walker@autodesk.com>
…ng from ICC profiles. Currently the clamping is removed ONLY when the the type 0 (simple gamma) TRC is used. In this case the the mirroring around origin is used for the negative values. Other parametric curves are currently implemented as 1DLuts thus will not handle out of bound values properly. In a later wave we can either convert LUTs to half-domain ones or try to implement them with more complex ops (such as exponent with linear). Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com>
doug-walker
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple minor comments, but looks good.
Your commit comment seems to imply only type 0 para curves are affected, but it applies to single entry curv tags too (see the comment I added).
| OCIO_CHECK_NO_THROW(opsInv.optimize(OCIO::OPTIMIZATION_LOSSLESS)); | ||
| OCIO_REQUIRE_EQUAL(2, ops.size()); | ||
| OCIO_CHECK_EQUAL("<GammaOp>", ops[0]->getInfo()); | ||
| OCIO_CHECK_EQUAL("<MatrixOffsetOp>", ops[1]->getInfo()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be on opsInv and be Matrix, then Gamma.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah! That explains why my initial test of expecting matrix and then gamma failed. My tired eyes saw opsInv as ops. Good catch, thanks.
| // range 0.0 to 1.0 as per ICC specifications. For type 0, we're relaxing | ||
| // the spec to allow full range with mirroring for negative values. This | ||
| // makes it possible to use ICC profiles for linear color space conversions | ||
| // or for extended range displays. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggest the following:
// The ICC spec states that the TRC tags should clamp to [0,1]. For curves that are implemented in the ICC profile as LUTs and most parametric curves (which become LUTs in OCIO), this is the case. However, as floating-point and HDR workflows become more common, the clamping has become a critical roadblock. For example, it is now common to have ICC profiles for linear color spaces that need to pass values outside [0,1]. Therefore, OCIO now implements single entry 'curv' tags and type 0 'para' tags without clamping using an ExponentTransform which extends above 1 and mirrors below 0. (Note that gamma values of 1 do not need to be tested for here since they will be omitted as no-ops later by the optimizer.)
- More detailed explanation for the clamp removal in pure-gamma TRC cases. Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com>
* - Fix Windows Doxygen install script as Doxygen v1.16.0 changed the released zip name. Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com> * double escape maybe? Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com> --------- Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com>
Currently the clamping is removed ONLY when the the type 0 (simple gamma) TRC is used. In this case the the mirroring around origin is used for the negative values.
Other parametric curves are currently implemented as 1DLuts thus will not handle out of bound values properly. In a later wave we can either convert LUTs to half-domain ones or try to implement them with more complex ops (such as exponent with linear).