api(ColorConfig): add isData, fix python isColorSpaceLinear.#5191
Conversation
* Add `ColorConfig::isData(name)` method to C++ and Python APIs. * Fix `ColorConfig.isColorSpaceLinear()` that was missing from the Python bindings. (Tests added as well.) Signed-off-by: Larry Gritz <lg@larrygritz.com>
|
@zachlewis Does this look good to you? |
zachlewis
left a comment
zachlewis
left a comment
There was a problem hiding this comment.
I think this is the right direction for sure; would prefer you check the "data" role instead of assuming "Raw" always points to a data space. Other than that, LGTM!
| ACEScg_alias); | ||
| } else if (Strutil::iequals(cs.name, "Rec709")) { | ||
| cs.setflag(CSInfo::is_Rec709, Rec709_alias); | ||
| } else if (Strutil::iequals(cs.name, "Raw")) { |
There was a problem hiding this comment.
I'm not in favor of these string-based assumptions. Aren't we catching these cases with OCIO's API in a subsequent classification stage anyway?
In any case, if you do want to have a fast path to selecting color spaces we know are data spaces, without having to actually inspect them, you can try looking up the "data" role, which is MUCH more likely to point to a data color space than assuming the current config has a color space named "Raw", and that "Raw" is a data color space.
You can try Config::getCanonicalColorSpaceName("data"), which will either return the name of a color space that can be mapped to the "data" colorInteropID (or any other data color space needs); or it will return something falsey if the "data" role isn't set (I think it usually is -- IIRC, configs fail to validate otherwise).
There was a problem hiding this comment.
I see your point here, and amended the PR with a change that uses getCanonicalColorSpaceName rather than hard-coding the name.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
2 participants
ColorConfig::isData(name)method to C++ and Python APIs.ColorConfig.isColorSpaceLinear()that was missing from the Python bindings. (Tests added as well.)