From 7c3555b081d9b74810a0c59175958e1d225ef08e Mon Sep 17 00:00:00 2001 From: "cuneyt.ozdas" Date: Sun, 21 Sep 2025 21:15:14 -0700 Subject: [PATCH 1/3] Adding interchange attributes to ViewTransforms and Looks - Currently only the amf_transform_ids key is supported. That key is needed for ACES 2 built-in configs. Signed-off-by: cuneyt.ozdas --- include/OpenColorIO/OpenColorIO.h | 32 +++++ src/OpenColorIO/Config.cpp | 32 +++++ src/OpenColorIO/Look.cpp | 71 ++++++++++ src/OpenColorIO/OCIOYaml.cpp | 112 ++++++++++------ src/OpenColorIO/ViewTransform.cpp | 70 ++++++++++ src/bindings/python/PyLook.cpp | 8 +- src/bindings/python/PyViewTransform.cpp | 6 + tests/cpu/Config_tests.cpp | 170 ++++++++++++++++++++++++ tests/cpu/ViewTransform_tests.cpp | 4 + tests/python/LookTest.py | 20 +++ tests/python/ViewTransformTest.py | 22 +++ 11 files changed, 507 insertions(+), 40 deletions(-) diff --git a/include/OpenColorIO/OpenColorIO.h b/include/OpenColorIO/OpenColorIO.h index 915737fa08..ee02747d6d 100644 --- a/include/OpenColorIO/OpenColorIO.h +++ b/include/OpenColorIO/OpenColorIO.h @@ -2407,6 +2407,22 @@ class OCIOEXPORT Look const char * getDescription() const; void setDescription(const char * description); + /** + * Get/Set the interchange attributes. + * + * Currently the only supported attribute name is "amf_transform_ids". Using + * any other name will throw. If the attribute is not defined, it'll return + * an empty string. Setting the value to an empty string will effectively + * delete the attribute. + * + * The AMF transform IDs are used to identify specific transforms in the ACES + * Metadata File. Multiple transform IDs can be specified in a + * newline-separated string. + */ + const char *getInterchangeAttribute(const char *attrName) const; + void setInterchangeAttribute(const char* attrName, const char *value); + std::map getInterchangeAttributes() const noexcept; + Look(const Look &) = delete; Look& operator= (const Look &) = delete; /// Do not use (needed only for pybind11). @@ -2542,6 +2558,22 @@ class OCIOEXPORT ViewTransform const char * getDescription() const noexcept; void setDescription(const char * description); + /** + * Get/Set the interchange attributes. + * + * Currently the only supported attribute name is "amf_transform_ids". Using + * any other name will throw. If the attribute is not defined, it'll return + * an empty string. Setting the value to an empty string will effectively + * delete the attribute. + * + * The AMF transform IDs are used to identify specific transforms in the ACES + * Metadata File. Multiple transform IDs can be specified in a + * newline-separated string. + */ + const char *getInterchangeAttribute(const char *attrName) const; + void setInterchangeAttribute(const char* attrName, const char *value); + std::map getInterchangeAttributes() const noexcept; + /// \see ColorSpace::hasCategory bool hasCategory(const char * category) const; /// \see ColorSpace::addCategory diff --git a/src/OpenColorIO/Config.cpp b/src/OpenColorIO/Config.cpp index 1a8f819593..24d5b413fa 100644 --- a/src/OpenColorIO/Config.cpp +++ b/src/OpenColorIO/Config.cpp @@ -5904,6 +5904,38 @@ void Config::Impl::checkVersionConsistency() const throw Exception("Only version 2 (or higher) can have ViewTransforms."); } + // Check for new ViewTransform properties. + + if (hexVersion < 0x02050000) + { + for (const auto& vt : m_viewTransforms) + { + if (vt->getInterchangeAttributes().size()>0) + { + std::ostringstream os; + os << "Config failed validation. The view transform '" << vt->getName() << "' "; + os << "has non-empty interchange attributes and config version is less than 2.5."; + throw Exception(os.str().c_str()); + } + } + } + + // Check for new Look properties. + + if (hexVersion < 0x02050000) + { + for (const auto& look : m_looksList) + { + if (look->getInterchangeAttributes().size()>0) + { + std::ostringstream os; + os << "Config failed validation. The look '" << look->getName() << "' "; + os << "has non-empty interchange attributes and config version is less than 2.5."; + throw Exception(os.str().c_str()); + } + } + } + // Check for the NamedTransforms. if (m_majorVersion < 2 && m_allNamedTransforms.size() != 0) diff --git a/src/OpenColorIO/Look.cpp b/src/OpenColorIO/Look.cpp index 9830d0a460..5093b84657 100644 --- a/src/OpenColorIO/Look.cpp +++ b/src/OpenColorIO/Look.cpp @@ -9,7 +9,13 @@ #include #include "ContextVariableUtils.h" +#include "utils/StringUtils.h" +namespace +{ +const std::array knownInterchangeNames = { + "amf_transform_ids" }; +} namespace OCIO_NAMESPACE { @@ -29,6 +35,7 @@ class Look::Impl std::string m_name; std::string m_processSpace; std::string m_description; + std::map m_interchangeAttribs; TransformRcPtr m_transform; TransformRcPtr m_inverseTransform; @@ -48,6 +55,8 @@ class Look::Impl m_processSpace = rhs.m_processSpace; m_description = rhs.m_description; + m_interchangeAttribs = rhs.m_interchangeAttribs; + m_transform = rhs.m_transform? rhs.m_transform->createEditableCopy() : rhs.m_transform; @@ -129,6 +138,63 @@ void Look::setDescription(const char * description) getImpl()->m_description = description ? description : ""; } +const char * Look::getInterchangeAttribute(const char* attrName) const +{ + std::string name = attrName ? attrName : ""; + + for (auto& key : knownInterchangeNames) + { + // do case-insensitive comparison. + if (StringUtils::Compare(key, name)) + { + auto it = m_impl->m_interchangeAttribs.find(key); + if (it != m_impl->m_interchangeAttribs.end()) + { + return it->second.c_str(); + } + return ""; + } + } + + std::ostringstream oss; + oss << "Unknown attribute name '" << name << "'."; + throw Exception(oss.str().c_str()); +} + +void Look::setInterchangeAttribute(const char* attrName, const char* value) +{ + std::string name = attrName ? attrName : ""; + + for (auto& key : knownInterchangeNames) + { + // Do case-insensitive comparison. + if (StringUtils::Compare(key, name)) + { + // use key instead of name for storing in correct capitalization. + if (!value || !*value) + { + m_impl->m_interchangeAttribs.erase(key); + } + else + { + m_impl->m_interchangeAttribs[key] = value; + } + return; + } + } + + std::ostringstream oss; + oss << "Unknown attribute name '" << name << "'."; + throw Exception(oss.str().c_str()); +} + +std::map Look::getInterchangeAttributes() const noexcept +{ + return m_impl->m_interchangeAttribs; +} + + + bool CollectContextVariables(const Config & config, const Context & context, TransformDirection direction, @@ -216,6 +282,11 @@ std::ostream& operator<< (std::ostream& os, const Look& look) os << ", description=" << desc; } + for (const auto& attr : look.getInterchangeAttributes()) + { + os << ", " << attr.first << "=" << attr.second; + } + if(look.getTransform()) { os << ",\n transform="; diff --git a/src/OpenColorIO/OCIOYaml.cpp b/src/OpenColorIO/OCIOYaml.cpp index c6920c3dea..02fbcb3ed6 100644 --- a/src/OpenColorIO/OCIOYaml.cpp +++ b/src/OpenColorIO/OCIOYaml.cpp @@ -344,6 +344,65 @@ inline void loadCustomKeys(const YAML::Node& node, CustomKeysLoader & ck, const } } +// Interchange Attributes + +void saveInterchangeAttributes( + YAML::Emitter& out, + const std::map& interchangemap) +{ + if (interchangemap.empty()) + return; + + out << YAML::Key << "interchange"; + out << YAML::Value; + out << YAML::BeginMap; + for (const auto& keyval : interchangemap) + { + std::string valStr = SanitizeNewlines(keyval.second); + + out << YAML::Key << keyval.first << YAML::Value; + if (valStr.find_first_of('\n') != std::string::npos) + { + out << YAML::Literal; + } + out << valStr; + } + + out << YAML::EndMap; +} + +template +void loadInterchangeAttributes(const YAML::Node& node, T& owner) +{ + if (node.Type() != YAML::NodeType::Map) + { + std::ostringstream os; + os << "The 'interchange' content needs to be a map."; + throwError(node, os.str()); + } + + CustomKeysLoader kv; + loadCustomKeys(node, kv, "interchange"); + + for (const auto& keyval : kv.m_keyVals) + { + std::string keystr = keyval.first.as(); + std::string valstr = keyval.second.as(); + valstr = SanitizeNewlines(valstr); + + // OCIO exception means the key is not recognized. Convert that to a warning. + try + { + owner->setInterchangeAttribute(keystr.c_str(), valstr.c_str()); + } + catch (Exception &) + { + LogUnknownKeyWarning("interchange", keyval.first); + } + } +} + + // View inline void load(const YAML::Node& node, View& v) @@ -3239,25 +3298,7 @@ inline void load(const YAML::Node& node, ColorSpaceRcPtr& cs, unsigned int major } else if (key == "interchange") { - CustomKeysLoader kv; - loadCustomKeys(iter->second, kv, "ColorSpace interchange"); - - for (const auto& keyval : kv.m_keyVals) - { - std::string keystr = keyval.first.as(); - std::string valstr = keyval.second.as(); - valstr = SanitizeNewlines(valstr); - - // OCIO exception means the key is not recognized. Convert that to a warning. - try - { - cs->setInterchangeAttribute(keystr.c_str(), valstr.c_str()); - } - catch (Exception &) - { - LogUnknownKeyWarning(key, keyval.first); - } - } + loadInterchangeAttributes(iter->second, cs); } else if(key == "family") { @@ -3358,6 +3399,8 @@ inline void load(const YAML::Node& node, ColorSpaceRcPtr& cs, unsigned int major } } + + inline void save(YAML::Emitter& out, ConstColorSpaceRcPtr cs, unsigned int majorVersion) { out << YAML::VerbatimTag("ColorSpace"); @@ -3412,26 +3455,7 @@ inline void save(YAML::Emitter& out, ConstColorSpaceRcPtr cs, unsigned int major out << YAML::Value << is; } - auto interchangemap = cs->getInterchangeAttributes(); - if (interchangemap.size()) - { - out << YAML::Key << "interchange"; - out << YAML::Value; - out << YAML::BeginMap; - for (const auto& keyval : interchangemap) - { - std::string valStr = SanitizeNewlines(keyval.second); - - out << YAML::Key << keyval.first << YAML::Value; - if (valStr.find_first_of('\n') != std::string::npos) - { - out << YAML::Literal; - } - out << valStr; - } - - out << YAML::EndMap; - } + saveInterchangeAttributes(out, cs->getInterchangeAttributes()); out << YAML::Key << "allocation" << YAML::Value; save(out, cs->getAllocation()); @@ -3511,6 +3535,10 @@ inline void load(const YAML::Node& node, LookRcPtr& look) loadDescription(iter->second, stringval); look->setDescription(stringval.c_str()); } + else if(key == "interchange") + { + loadInterchangeAttributes(iter->second, look); + } else { LogUnknownKeyWarning(node, iter->first); @@ -3525,6 +3553,7 @@ inline void save(YAML::Emitter& out, ConstLookRcPtr look, unsigned int majorVers out << YAML::Key << "name" << YAML::Value << look->getName(); out << YAML::Key << "process_space" << YAML::Value << look->getProcessSpace(); saveDescription(out, look->getDescription()); + saveInterchangeAttributes(out, look->getInterchangeAttributes()); if(look->getTransform()) { @@ -3627,6 +3656,10 @@ inline void load(const YAML::Node & node, ViewTransformRcPtr & vt) loadDescription(iter->second, stringval); vt->setDescription(stringval.c_str()); } + else if (key == "interchange") + { + loadInterchangeAttributes(iter->second, vt); + } else if (key == "family") { std::string stringval; @@ -3685,6 +3718,7 @@ inline void save(YAML::Emitter & out, ConstViewTransformRcPtr & vt, unsigned int out << YAML::Key << "family" << YAML::Value << family; } saveDescription(out, vt->getDescription()); + saveInterchangeAttributes(out, vt->getInterchangeAttributes()); if (vt->getNumCategories() > 0) { diff --git a/src/OpenColorIO/ViewTransform.cpp b/src/OpenColorIO/ViewTransform.cpp index 7fd8914a34..28a32c591e 100644 --- a/src/OpenColorIO/ViewTransform.cpp +++ b/src/OpenColorIO/ViewTransform.cpp @@ -7,6 +7,12 @@ #include "TokensManager.h" +namespace +{ +const std::array knownInterchangeNames = { + "amf_transform_ids" }; +} + namespace OCIO_NAMESPACE { @@ -17,6 +23,7 @@ class ViewTransform::Impl std::string m_family; std::string m_description; ReferenceSpaceType m_referenceSpaceType{ REFERENCE_SPACE_SCENE }; + std::map m_interchangeAttribs; TransformRcPtr m_toRefTransform; TransformRcPtr m_fromRefTransform; @@ -41,6 +48,7 @@ class ViewTransform::Impl m_description = rhs.m_description; m_referenceSpaceType = rhs.m_referenceSpaceType; + m_interchangeAttribs = rhs.m_interchangeAttribs; m_toRefTransform = rhs.m_toRefTransform ? rhs.m_toRefTransform->createEditableCopy() : rhs.m_toRefTransform; @@ -114,6 +122,62 @@ void ViewTransform::setDescription(const char * description) getImpl()->m_description = description ? description : ""; } +const char * ViewTransform::getInterchangeAttribute(const char* attrName) const +{ + std::string name = attrName ? attrName : ""; + + for (auto& key : knownInterchangeNames) + { + // do case-insensitive comparison. + if (StringUtils::Compare(key, name)) + { + auto it = m_impl->m_interchangeAttribs.find(key); + if (it != m_impl->m_interchangeAttribs.end()) + { + return it->second.c_str(); + } + return ""; + } + } + + std::ostringstream oss; + oss << "Unknown attribute name '" << name << "'."; + throw Exception(oss.str().c_str()); +} + +void ViewTransform::setInterchangeAttribute(const char* attrName, const char* value) +{ + std::string name = attrName ? attrName : ""; + + for (auto& key : knownInterchangeNames) + { + // Do case-insensitive comparison. + if (StringUtils::Compare(key, name)) + { + // use key instead of name for storing in correct capitalization. + if (!value || !*value) + { + m_impl->m_interchangeAttribs.erase(key); + } + else + { + m_impl->m_interchangeAttribs[key] = value; + } + return; + } + } + + std::ostringstream oss; + oss << "Unknown attribute name '" << name << "'."; + throw Exception(oss.str().c_str()); +} + +std::map ViewTransform::getInterchangeAttributes() const noexcept +{ + return m_impl->m_interchangeAttribs; +} + + bool ViewTransform::hasCategory(const char * category) const { return getImpl()->m_categories.hasToken(category); @@ -210,6 +274,12 @@ std::ostream & operator<< (std::ostream & os, const ViewTransform & vt) { os << ", description=" << desc; } + + for (const auto& attr : vt.getInterchangeAttributes()) + { + os << ", " << attr.first << "=" << attr.second; + } + if (vt.getTransform(VIEWTRANSFORM_DIR_TO_REFERENCE)) { os << ",\n " << vt.getName() << " --> Reference"; diff --git a/src/bindings/python/PyLook.cpp b/src/bindings/python/PyLook.cpp index 05fc2814af..7900731f24 100644 --- a/src/bindings/python/PyLook.cpp +++ b/src/bindings/python/PyLook.cpp @@ -63,7 +63,13 @@ void bindPyLook(py::module & m) .def("getDescription", &Look::getDescription, DOC(Look, getDescription)) .def("setDescription", &Look::setDescription, "description"_a.none(false), - DOC(Look, setDescription)); + DOC(Look, setDescription)) + .def("getInterchangeAttribute", &Look::getInterchangeAttribute, "attrName"_a, + DOC(Look, getInterchangeAttribute)) + .def("setInterchangeAttribute", &Look::setInterchangeAttribute, "attrName"_a, "attrValue"_a, + DOC(Look, setInterchangeAttribute)) + .def("getInterchangeAttributes", &Look:: getInterchangeAttributes, + DOC(Look, getInterchangeAttributes)); defRepr(clsLook); } diff --git a/src/bindings/python/PyViewTransform.cpp b/src/bindings/python/PyViewTransform.cpp index 267a30f9f6..d3de4730e9 100644 --- a/src/bindings/python/PyViewTransform.cpp +++ b/src/bindings/python/PyViewTransform.cpp @@ -105,6 +105,12 @@ void bindPyViewTransform(py::module & m) DOC(ViewTransform, getDescription)) .def("setDescription", &ViewTransform::setDescription, "description"_a, DOC(ViewTransform, setDescription)) + .def("getInterchangeAttribute", &ViewTransform::getInterchangeAttribute, "attrName"_a, + DOC(ViewTransform, getInterchangeAttribute)) + .def("setInterchangeAttribute", &ViewTransform::setInterchangeAttribute, "attrName"_a, "attrValue"_a, + DOC(ViewTransform, setInterchangeAttribute)) + .def("getInterchangeAttributes", &ViewTransform:: getInterchangeAttributes, + DOC(ViewTransform, getInterchangeAttributes)) .def("hasCategory", &ViewTransform::hasCategory, "category"_a, DOC(ViewTransform, hasCategory)) .def("addCategory", &ViewTransform::addCategory, "category"_a, diff --git a/tests/cpu/Config_tests.cpp b/tests/cpu/Config_tests.cpp index 06ac3d743c..f0cee7f46e 100644 --- a/tests/cpu/Config_tests.cpp +++ b/tests/cpu/Config_tests.cpp @@ -10093,3 +10093,173 @@ OCIO_ADD_TEST(Config, set_config_io_proxy) OCIO::ClearAllCaches(); } } + +OCIO_ADD_TEST(Config, interchange_attributes) +{ + std::string END = {R"( +view_transforms: + - ! + name: vt1 + from_scene_reference: ! {min_in_value: 0., min_out_value: 0.})"}; + + const std::string str = PROFILE_START_V<2, 5>() + END; + + std::istringstream is; + is.str(str); + + OCIO::ConfigRcPtr config; + OCIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromStream(is)->createEditableCopy()); + OCIO_REQUIRE_ASSERT(config); + OCIO_CHECK_NO_THROW(config->validate()); + + // Color Space + + { + auto cs = config->getColorSpace("log")->createEditableCopy(); + OCIO_REQUIRE_ASSERT(cs); + + // Set amf_transform_ids attribute and validate. + + OCIO_CHECK_NO_THROW(cs->setInterchangeAttribute("amf_transform_ids", "sample amf id")); + config->addColorSpace(cs); + OCIO_CHECK_NO_THROW(config->validate()); + + // Check that the attribute is in the serialized config. + + std::stringstream ss; + config->serialize(ss); + auto cstrout = ss.str(); + OCIO_CHECK_ASSERT(cstrout.find("amf_transform_ids: sample amf id") != std::string::npos); + + // Check that loading the serialized config works with the attribute. + + OCIO::ConstConfigRcPtr cfg2; + OCIO_CHECK_NO_THROW(cfg2 = OCIO::Config::CreateFromStream(ss)); + + OCIO::ConstColorSpaceRcPtr cs2; + OCIO_CHECK_NO_THROW(cs2 = config->getColorSpace("log")); + OCIO_CHECK_EQUAL(cs2->getInterchangeAttribute("amf_transform_ids"), std::string("sample amf id")); + + // Check that the config can NOT be downgraded to 2.4 with the attribute. + + config->setVersion(2, 4); + OCIO_CHECK_THROW_WHAT( + config->validate(), + OCIO::Exception, + "Config failed validation. The color space 'log' has non-empty " + "interchange attributes and config version is less than 2.5."); + + // Remove the attribute and check that the config can be downgraded to 2.4. + + OCIO_CHECK_NO_THROW(cs->setInterchangeAttribute("amf_transform_ids", "")); + config->addColorSpace(cs); + OCIO_CHECK_NO_THROW(config->validate()); + + // Restore version 2.5. + + config->setVersion(2, 5); + } + + // View Transform + + { + auto vt = config->getViewTransform("vt1")->createEditableCopy(); + OCIO_REQUIRE_ASSERT(vt); + + // Set amf_transform_ids attribute and validate. + + OCIO_CHECK_NO_THROW(vt->setInterchangeAttribute("amf_transform_ids", "sample amf id")); + config->addViewTransform(vt); + OCIO_CHECK_NO_THROW(config->validate()); + + // Setting the icc_profile_name attribute should throw. + OCIO_CHECK_THROW_WHAT( + vt->setInterchangeAttribute("icc_profile_name", "some icc profile"), + OCIO::Exception, + "Unknown attribute name 'icc_profile_name'."); + + // Check that the attribute is in the serialized config. + + std::stringstream ss; + config->serialize(ss); + auto cstrout = ss.str(); + OCIO_CHECK_ASSERT(cstrout.find("amf_transform_ids: sample amf id") != std::string::npos); + + // Check that loading the serialized config works with the attribute. + + OCIO::ConstConfigRcPtr cfg2; + OCIO_CHECK_NO_THROW(cfg2 = OCIO::Config::CreateFromStream(ss)); + + OCIO::ConstViewTransformRcPtr vt2; + OCIO_CHECK_NO_THROW(vt2 = config->getViewTransform("vt1")); + OCIO_CHECK_EQUAL(vt2->getInterchangeAttribute("amf_transform_ids"), std::string("sample amf id")); + + // Check that the config can NOT be downgraded to 2.4 with the attribute. + + config->setVersion(2, 4); + OCIO_CHECK_THROW_WHAT( + config->validate(), + OCIO::Exception, + "Config failed validation. The ViewTransform 'vt1' has non-empty " + "interchange attributes and config version is less than 2.5."); + + // Remove the attribute and check that the config can be downgraded to 2.4. + + OCIO_CHECK_NO_THROW(vt->setInterchangeAttribute("amf_transform_ids", "")); + config->addViewTransform(vt); + OCIO_CHECK_NO_THROW(config->validate()); + + // Restore version 2.5. + + config->setVersion(2, 5); + } + + // Look + + { + auto lk = config->getLook("beauty")->createEditableCopy(); + OCIO_REQUIRE_ASSERT(lk); + + // Set amf_transform_ids attribute and validate. + + OCIO_CHECK_NO_THROW(lk->setInterchangeAttribute("amf_transform_ids", "sample amf id")); + config->addLook(lk); + OCIO_CHECK_NO_THROW(config->validate()); + + // Check that the attribute is in the serialized config. + + std::stringstream ss; + config->serialize(ss); + auto cstrout = ss.str(); + OCIO_CHECK_ASSERT(cstrout.find("amf_transform_ids: sample amf id") != std::string::npos); + + // Check that loading the serialized config works with the attribute. + + OCIO::ConstConfigRcPtr cfg2; + OCIO_CHECK_NO_THROW(cfg2 = OCIO::Config::CreateFromStream(ss)); + + OCIO::ConstLookRcPtr lk2; + OCIO_CHECK_NO_THROW(lk2 = config->getLook("beauty")); + OCIO_CHECK_EQUAL(lk2->getInterchangeAttribute("amf_transform_ids"), std::string("sample amf id")); + + // Check that the config can NOT be downgraded to 2.4 with the attribute. + + config->setVersion(2, 4); + OCIO_CHECK_THROW_WHAT( + config->validate(), + OCIO::Exception, + "Config failed validation. The Look 'beauty' has non-empty " + "interchange attributes and config version is less than 2.5."); + + // Remove the attribute and check that the config can be downgraded to 2.4. + + OCIO_CHECK_NO_THROW(lk->setInterchangeAttribute("amf_transform_ids", "")); + config->addLook(lk); + OCIO_CHECK_NO_THROW(config->validate()); + + // Restore version 2.5. + + config->setVersion(2, 5); + } +} + diff --git a/tests/cpu/ViewTransform_tests.cpp b/tests/cpu/ViewTransform_tests.cpp index 84103a3204..0055d3ecca 100644 --- a/tests/cpu/ViewTransform_tests.cpp +++ b/tests/cpu/ViewTransform_tests.cpp @@ -18,6 +18,7 @@ OCIO_ADD_TEST(ViewTransform, basic) OCIO_CHECK_EQUAL(std::string(""), vt->getFamily()); OCIO_CHECK_EQUAL(std::string(""), vt->getDescription()); OCIO_CHECK_EQUAL(0, vt->getNumCategories()); + OCIO_CHECK_EQUAL(std::string(""), vt->getInterchangeAttribute("amf_transform_ids")); vt->setName("name"); OCIO_CHECK_EQUAL(std::string("name"), vt->getName()); @@ -25,6 +26,9 @@ OCIO_ADD_TEST(ViewTransform, basic) OCIO_CHECK_EQUAL(std::string("family"), vt->getFamily()); vt->setDescription("description"); OCIO_CHECK_EQUAL(std::string("description"), vt->getDescription()); + vt->setInterchangeAttribute("amf_transform_ids", "amf_text"); + OCIO_CHECK_EQUAL(std::string("amf_text"), vt->getInterchangeAttribute("amf_transform_ids")); + OCIO_CHECK_EQUAL(1, vt->getInterchangeAttributes().size()); OCIO_CHECK_EQUAL(vt->getNumCategories(), 0); diff --git a/tests/python/LookTest.py b/tests/python/LookTest.py index 16f6e32488..91f1043a09 100644 --- a/tests/python/LookTest.py +++ b/tests/python/LookTest.py @@ -27,6 +27,7 @@ def test_copy(self): self.look.setName('test name') self.look.setProcessSpace('test space') self.look.setDescription('test description') + self.look.setInterchangeAttribute('amf_transform_ids', 'test amf id') mat = OCIO.MatrixTransform() self.look.setTransform(mat) self.look.setInverseTransform(mat) @@ -37,6 +38,9 @@ def test_copy(self): self.assertEqual(other.getName(), self.look.getName()) self.assertEqual(other.getProcessSpace(), self.look.getProcessSpace()) self.assertEqual(other.getDescription(), self.look.getDescription()) + self.assertEqual( + other.getInterchangeAttribute('amf_transform_ids'), + self.look.getInterchangeAttribute('amf_transform_ids')) self.assertTrue(other.getTransform().equals(self.look.getTransform())) self.assertTrue(other.getInverseTransform().equals(self.look.getInverseTransform())) @@ -91,6 +95,22 @@ def test_description(self): with self.assertRaises(TypeError): self.look.setDescription(invalid) + def test_interchangeAttribute(self): + """ + Test the setInterchangeAttribute() and setInterchangeAttribute() methods. + """ + + # Default initialized description value is "" + self.assertEqual(self.look.getInterchangeAttribute('amf_transform_ids'), '') + + self.look.setInterchangeAttribute('amf_transform_ids', 'sample amf') + self.assertEqual('sample amf', self.look.getInterchangeAttribute('amf_transform_ids')) + self.assertEqual(1, len(self.look.getInterchangeAttributes())) + + # Wrong key tests. + with self.assertRaises(TypeError): + self.look.getInterchangeAttribute('icc_profile_ name', 'should be rejected') + def test_transform(self): """ Test the setTransform() and getTransform() methods. diff --git a/tests/python/ViewTransformTest.py b/tests/python/ViewTransformTest.py index b36b25463b..01d96a416a 100644 --- a/tests/python/ViewTransformTest.py +++ b/tests/python/ViewTransformTest.py @@ -56,6 +56,7 @@ def test_copy(self): vt.setName('test name') vt.setFamily('test family') vt.setDescription('test description') + vt.setInterchangeAttribute('amf_transform_ids', 'test amf id') mat = OCIO.MatrixTransform() vt.setTransform(mat, OCIO.VIEWTRANSFORM_DIR_TO_REFERENCE) vt.setTransform(direction=OCIO.VIEWTRANSFORM_DIR_FROM_REFERENCE, transform=mat) @@ -67,6 +68,9 @@ def test_copy(self): self.assertEqual(other.getName(), vt.getName()) self.assertEqual(other.getFamily(), vt.getFamily()) self.assertEqual(other.getDescription(), vt.getDescription()) + self.assertEqual( + other.getInterchangeAttribute("amf_transform_ids"), + other.getInterchangeAttribute("amf_transform_ids")) self.assertTrue(other.getTransform(OCIO.VIEWTRANSFORM_DIR_TO_REFERENCE).equals(vt.getTransform(OCIO.VIEWTRANSFORM_DIR_TO_REFERENCE))) self.assertTrue(other.getTransform(OCIO.VIEWTRANSFORM_DIR_FROM_REFERENCE).equals(vt.getTransform(OCIO.VIEWTRANSFORM_DIR_FROM_REFERENCE))) self.assertEqual(list(other.getCategories()), list(vt.getCategories())) @@ -101,6 +105,24 @@ def test_description(self): vt.setDescription('test description') self.assertEqual(vt.getDescription(), 'test description') + def test_interchangeAttribute(self): + """ + Test the setInterchangeAttribute() and setInterchangeAttribute() methods. + """ + vt = OCIO.ViewTransform() + + # Default initialized description value is "" + self.assertEqual(vt.getInterchangeAttribute('amf_transform_ids'), '') + + vt.setInterchangeAttribute('amf_transform_ids', 'sample amf') + self.assertEqual('sample amf', vt.getInterchangeAttribute('amf_transform_ids')) + self.assertEqual(1, len(vt.getInterchangeAttributes())) + + # Wrong key tests. + with self.assertRaises(TypeError): + vt.getInterchangeAttribute('icc_profile_ name', 'should be rejected') + + def test_transform(self): """ Test get/setTransform. From 9f5182c405ba2398b062b2b7106bc7132c43e502 Mon Sep 17 00:00:00 2001 From: "cuneyt.ozdas" Date: Sun, 21 Sep 2025 21:43:29 -0700 Subject: [PATCH 2/3] Fixing the build by propagating the last minute wording change. Signed-off-by: cuneyt.ozdas --- tests/cpu/Config_tests.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/cpu/Config_tests.cpp b/tests/cpu/Config_tests.cpp index f0cee7f46e..163481c40a 100644 --- a/tests/cpu/Config_tests.cpp +++ b/tests/cpu/Config_tests.cpp @@ -10200,7 +10200,7 @@ OCIO_ADD_TEST(Config, interchange_attributes) OCIO_CHECK_THROW_WHAT( config->validate(), OCIO::Exception, - "Config failed validation. The ViewTransform 'vt1' has non-empty " + "Config failed validation. The view transform 'vt1' has non-empty " "interchange attributes and config version is less than 2.5."); // Remove the attribute and check that the config can be downgraded to 2.4. @@ -10248,7 +10248,7 @@ OCIO_ADD_TEST(Config, interchange_attributes) OCIO_CHECK_THROW_WHAT( config->validate(), OCIO::Exception, - "Config failed validation. The Look 'beauty' has non-empty " + "Config failed validation. The look 'beauty' has non-empty " "interchange attributes and config version is less than 2.5."); // Remove the attribute and check that the config can be downgraded to 2.4. From 25daf8323ad00518564d4e0966eaad02a4b29124 Mon Sep 17 00:00:00 2001 From: "cuneyt.ozdas" Date: Sun, 21 Sep 2025 22:36:15 -0700 Subject: [PATCH 3/3] - Fixing a bug in ViewTransform python tests - Removing the ColorSpace amf_transform_ids_serialization test as almost all of the cases are now exercised in the Config interchange_attributes test. - minor cleanup Signed-off-by: cuneyt.ozdas --- include/OpenColorIO/OpenColorIO.h | 6 ++-- src/OpenColorIO/Look.cpp | 2 +- src/OpenColorIO/ViewTransform.cpp | 2 +- tests/cpu/ColorSpace_tests.cpp | 51 ------------------------------- tests/python/ViewTransformTest.py | 2 +- 5 files changed, 6 insertions(+), 57 deletions(-) diff --git a/include/OpenColorIO/OpenColorIO.h b/include/OpenColorIO/OpenColorIO.h index ee02747d6d..8b76fb22e7 100644 --- a/include/OpenColorIO/OpenColorIO.h +++ b/include/OpenColorIO/OpenColorIO.h @@ -2044,7 +2044,7 @@ class OCIOEXPORT ColorSpace * * Currently supported attribute names are "amf_transform_ids" and * "icc_profile_name". Using any other name will throw. If the attribute is - * not defined, it'll return an empty string. Setting the value to an empty + * not defined, it will return an empty string. Setting the value to an empty * string will effectively delete the attribute. * * The AMF transform IDs are used to identify specific transforms in the @@ -2411,7 +2411,7 @@ class OCIOEXPORT Look * Get/Set the interchange attributes. * * Currently the only supported attribute name is "amf_transform_ids". Using - * any other name will throw. If the attribute is not defined, it'll return + * any other name will throw. If the attribute is not defined, it will return * an empty string. Setting the value to an empty string will effectively * delete the attribute. * @@ -2562,7 +2562,7 @@ class OCIOEXPORT ViewTransform * Get/Set the interchange attributes. * * Currently the only supported attribute name is "amf_transform_ids". Using - * any other name will throw. If the attribute is not defined, it'll return + * any other name will throw. If the attribute is not defined, it will return * an empty string. Setting the value to an empty string will effectively * delete the attribute. * diff --git a/src/OpenColorIO/Look.cpp b/src/OpenColorIO/Look.cpp index 5093b84657..14af742350 100644 --- a/src/OpenColorIO/Look.cpp +++ b/src/OpenColorIO/Look.cpp @@ -170,7 +170,7 @@ void Look::setInterchangeAttribute(const char* attrName, const char* value) // Do case-insensitive comparison. if (StringUtils::Compare(key, name)) { - // use key instead of name for storing in correct capitalization. + // Use key instead of name for storing in correct capitalization. if (!value || !*value) { m_impl->m_interchangeAttribs.erase(key); diff --git a/src/OpenColorIO/ViewTransform.cpp b/src/OpenColorIO/ViewTransform.cpp index 28a32c591e..c14d6021ac 100644 --- a/src/OpenColorIO/ViewTransform.cpp +++ b/src/OpenColorIO/ViewTransform.cpp @@ -154,7 +154,7 @@ void ViewTransform::setInterchangeAttribute(const char* attrName, const char* va // Do case-insensitive comparison. if (StringUtils::Compare(key, name)) { - // use key instead of name for storing in correct capitalization. + // Use key instead of name for storing in correct capitalization. if (!value || !*value) { m_impl->m_interchangeAttribs.erase(key); diff --git a/tests/cpu/ColorSpace_tests.cpp b/tests/cpu/ColorSpace_tests.cpp index 1e47ef8111..6bdbaa77ad 100644 --- a/tests/cpu/ColorSpace_tests.cpp +++ b/tests/cpu/ColorSpace_tests.cpp @@ -1127,57 +1127,6 @@ OCIO_ADD_TEST(ColorSpace, amf_transform_ids) OCIO_CHECK_EQUAL(std::string(copy->getInterchangeAttribute("amf_transform_ids")), singleID); } -OCIO_ADD_TEST(ColorSpace, amf_transform_ids_serialization) -{ - // Test YAML serialization and deserialization of AmfTransformIDs. - auto cfg = OCIO::Config::Create(); - auto cs = OCIO::ColorSpace::Create(); - cs->setName("test_colorspace"); - - const std::string amfIDs = - "urn:ampas:aces:transformId:v1.5:ACEScsc.Academy.ACEScc_to_ACES.a1.0.3\n" - "urn:ampas:aces:transformId:v1.5:ACEScsc.Academy.ACES_to_ACEScc.a1.0.3"; - - cs->setInterchangeAttribute("amf_transform_ids", amfIDs.c_str()); - cfg->addColorSpace(cs); - - // Serialize the Config. - std::stringstream ss; - cfg->serialize(ss); - std::string yamlStr = ss.str(); - - // Verify AmfTransformIDs appears in YAML. - OCIO_CHECK_NE(yamlStr.find("amf_transform_ids"), std::string::npos); - OCIO_CHECK_NE(yamlStr.find("ACEScsc.Academy.ACEScc_to_ACES"), std::string::npos); - - // Deserialize and verify. - std::istringstream iss(yamlStr); - OCIO::ConstConfigRcPtr deserializedCfg; - OCIO_CHECK_NO_THROW(deserializedCfg = OCIO::Config::CreateFromStream(iss)); - - // Verify AmfTransformIDs is preserved. - OCIO::ConstColorSpaceRcPtr deserializedCs = deserializedCfg->getColorSpace("test_colorspace"); - auto deserializedIDs = deserializedCs->getInterchangeAttribute("amf_transform_ids"); - OCIO_CHECK_EQUAL(deserializedIDs, amfIDs); - - // Verify that that earlier versions reject amf_transform_ids. - OCIO::ConfigRcPtr cfgCopy = cfg->createEditableCopy(); - cfgCopy->setVersion(2,4); - OCIO_CHECK_THROW_WHAT(cfgCopy->serialize(ss), - OCIO::Exception, - "has non-empty interchange attributes and config version is less than 2.5."); - - // Test with empty AmfTransformIDs (should not appear in YAML). - cs->setInterchangeAttribute("amf_transform_ids", nullptr); - cfg->addColorSpace(cs); // Replace the existing CS. - ss.str(""); - cfg->serialize(ss); - std::string yamlStr2 = ss.str(); - - // Verify empty AmfTransformIDs does not appear in YAML. - OCIO_CHECK_EQUAL(yamlStr2.find("amf_transform_ids"), std::string::npos); -} - OCIO_ADD_TEST(ColorSpace, icc_profile_name) { OCIO::ColorSpaceRcPtr cs = OCIO::ColorSpace::Create(); diff --git a/tests/python/ViewTransformTest.py b/tests/python/ViewTransformTest.py index 01d96a416a..bb9e9e3f31 100644 --- a/tests/python/ViewTransformTest.py +++ b/tests/python/ViewTransformTest.py @@ -70,7 +70,7 @@ def test_copy(self): self.assertEqual(other.getDescription(), vt.getDescription()) self.assertEqual( other.getInterchangeAttribute("amf_transform_ids"), - other.getInterchangeAttribute("amf_transform_ids")) + vt.getInterchangeAttribute("amf_transform_ids")) self.assertTrue(other.getTransform(OCIO.VIEWTRANSFORM_DIR_TO_REFERENCE).equals(vt.getTransform(OCIO.VIEWTRANSFORM_DIR_TO_REFERENCE))) self.assertTrue(other.getTransform(OCIO.VIEWTRANSFORM_DIR_FROM_REFERENCE).equals(vt.getTransform(OCIO.VIEWTRANSFORM_DIR_FROM_REFERENCE))) self.assertEqual(list(other.getCategories()), list(vt.getCategories()))