diff --git a/docs/guides/authoring/overview.rst b/docs/guides/authoring/overview.rst index 69e886dc50..7df06e669c 100644 --- a/docs/guides/authoring/overview.rst +++ b/docs/guides/authoring/overview.rst @@ -319,7 +319,7 @@ to use to control other types of tasks not listed below. * ``color_picking`` - colors in a color-selection UI can be displayed in this space, while selecting colors in a different working space - (e.g. ``scene_linear`` or ``texture_paint``) + (e.g. ``scene_linear`` or ``texture_paint``). * ``color_timing`` - color space used for applying color corrections, e.g. user-specified grade within an image viewer (if the application @@ -327,32 +327,32 @@ to use to control other types of tasks not listed below. * ``compositing_log`` - a log color space used for certain processing operations (plate resizing, pulling keys, degrain, etc). Used by the - OCIOLogConvert Nuke node + OCIOLogConvert Nuke node. * ``data`` - used when writing data outputs such as normals, depth data, and other "non color" data. The color space in this role should typically have ``data: true`` specified, so no color transforms are - applied + applied. * ``default`` - when ``strictparsing: false``, this color space is used - as a fallback. If not defined, the ``scene_linear`` role is used + as a fallback. * ``matte_paint`` - color space which matte-paintings are created in (for more information, :ref:`see the guide on baking ICC profiles for Photoshop `, and - :ref:`config-spivfx`) + :ref:`config-spivfx`). * ``reference`` - the color space against which the other color spaces - are defined + are defined. .. note:: The reference role has sometimes been misinterpreted as being the space in which "reference art" is stored in. * ``scene_linear`` - the scene-referred linear-to-light color space, - often the same as the reference space (see:ref:`faq-terminology`) + often the same as the reference space (see:ref:`faq-terminology`). * ``texture_paint`` - similar to ``matte_paint`` but for painting textures for 3D objects (see the description of texture painting in - :ref:`SPI's pipeline `) + :ref:`SPI's pipeline `). diff --git a/docs/guides/authoring/rules.rst b/docs/guides/authoring/rules.rst index 7a106185be..3c557dbf4a 100644 --- a/docs/guides/authoring/rules.rst +++ b/docs/guides/authoring/rules.rst @@ -115,7 +115,7 @@ determine the colorspace ``lnf`` (it being the right-most substring containing a colorspace name) However, if the colorspace cannot be determined and ``strictparsing: -true``, it will produce an error. +true``, it will return an empty string. If the colorspace cannot be determined and ``strictparsing: false``, the default role will be used. This allows unhandled images to operate diff --git a/include/OpenColorIO/OpenColorIO.h b/include/OpenColorIO/OpenColorIO.h index 5fead719e9..e49243aa3f 100644 --- a/include/OpenColorIO/OpenColorIO.h +++ b/include/OpenColorIO/OpenColorIO.h @@ -980,7 +980,8 @@ class OCIOEXPORT Config */ void setFileRules(ConstFileRulesRcPtr fileRules); - /// Get the color space of the first rule that matched filePath. + /// Get the color space of the first rule that matched filePath. (For v1 configs, this is + /// equivalent to calling parseColorSpaceFromString with strictparsing set to false.) const char * getColorSpaceFromFilepath(const char * filePath) const; /** @@ -1009,7 +1010,7 @@ class OCIOEXPORT Config * * If strict parsing is disabled, return ROLE_DEFAULT (if defined). * * If the default role is not defined, return an empty string. */ - OCIO_DEPRECATED("This is now deprecated, please use Config::getColorSpaceFromFilepath().") + OCIO_DEPRECATED("This is now deprecated starting at v2.0, please use Config::getColorSpaceFromFilepath().") const char * parseColorSpaceFromString(const char * str) const; bool isStrictParsingEnabled() const; @@ -1178,6 +1179,10 @@ extern OCIOEXPORT std::ostream& operator<< (std::ostream&, const Config&); * Getters and setters are using the rule position, they will throw if the position is not * valid. If the rule at the specified position does not implement the requested property * getter will return NULL and setter will throw. + * + * When loading a v1 config, a set of FileRules are created with ColorSpaceNamePathSearch followed + * by the Default rule pointing to the default role. This allows getColorSpaceFromFilepath to emulate + * OCIO v1 code that used parseColorSpaceFromString with strictparsing set to false. */ class OCIOEXPORT FileRules diff --git a/src/OpenColorIO/Config.cpp b/src/OpenColorIO/Config.cpp index 3685413df4..dab173bcf1 100644 --- a/src/OpenColorIO/Config.cpp +++ b/src/OpenColorIO/Config.cpp @@ -192,24 +192,6 @@ void GetColorSpaceReferences(std::set & colorSpaceNames, } } -static constexpr char AddedDefault[]{ "added_default_rule_colorspace" }; - -void FindAvailableName(const ColorSpaceSetRcPtr & colorspaces, std::string & csname) -{ - int i = 0; - csname = AddedDefault; - while (true) - { - if (!colorspaces->hasColorSpace(csname.c_str())) - { - break; - } - - csname = AddedDefault + std::to_string(i); - ++i; - } -} - // Views are stored in two vectors of objects, using pointers to temporarily group them. typedef std::vector ViewPtrVec; @@ -591,56 +573,6 @@ class Config::Impl static ConstConfigRcPtr Read(std::istream & istream, const char * filename); - // Upgrade from v1 to v2. - void upgradeFromVersion1ToVersion2() noexcept - { - // V2 adds file_rules and these require a default rule. We try to initialize the default - // rule using the default role. If the default role doesn't exist, we look for a Raw - // ColorSpace with isdata true. If that is not found either, we add new ColorSpace - // named "added_default_rule_colorspace". - - m_majorVersion = 2; - m_minorVersion = 0; - - const char * rname = LookupRole(m_roles, ROLE_DEFAULT); - if (!hasColorSpace(rname)) - { - std::string defaultCS; - bool addNewDefault = true; - // The default role doesn't exist so look for a color space named "raw" - // (not case-sensitive) with isdata true. - const int csindex = getColorSpaceIndex("raw"); - if (-1 != csindex) - { - auto cs = m_allColorSpaces->getColorSpaceByIndex(csindex); - if (cs->isData()) - { - // "Raw" color space can be used for default. - addNewDefault = false; - defaultCS = cs->getName(); - } - } - - if (addNewDefault) - { - FindAvailableName(m_allColorSpaces, defaultCS); - auto newCS = ColorSpace::Create(); - newCS->setName(defaultCS.c_str()); - newCS->setIsData(true); - m_allColorSpaces->addColorSpace(newCS); - // Put the added CS in the inactive list to avoid it showing up in user menus. - if (!m_inactiveColorSpaceNamesConf.empty()) - { - m_inactiveColorSpaceNamesConf += ","; - } - m_inactiveColorSpaceNamesConf += defaultCS; - setInactiveColorSpaces(m_inactiveColorSpaceNamesConf.c_str()); - } - - m_fileRules->setColorSpace(m_fileRules->getNumEntries() - 1, defaultCS.c_str()); - } - } - // Validate view object that can be a config defined shared view or a display-defined view. void validateView(const std::string & display, const View & view, bool checkUseDisplayName) const { @@ -1269,15 +1201,19 @@ void Config::upgradeToLatestVersion() noexcept { if (wasVersion == 1) { - m_impl->upgradeFromVersion1ToVersion2(); + UpdateFileRulesFromV1ToV2(*this, m_impl->m_fileRules); + + // The instance version is now 2.0 + m_impl->m_majorVersion = 2; + m_impl->m_minorVersion = 0; } + static_assert(LastSupportedMajorVersion == 2, "Config: Handle newer versions"); setMajorVersion(LastSupportedMajorVersion); setMinorVersion(LastSupportedMinorVersion[LastSupportedMajorVersion - 1]); } } - ConfigRcPtr Config::createEditableCopy() const { ConfigRcPtr config = Config::Create(); @@ -1757,26 +1693,17 @@ void Config::validate() const ///// FileRules - // All Config objects have a fileRules object, regardless of version. This object is - // initialized to have a defaultRule with the color space set to "default" (i.e., the default - // role). The fileRules->validate call will validate that all color spaces used in rules - // exist, or if they are roles that they point to a color space that exists. Because this would - // cause validate to improperly fail on v1 configs (since they are not required to actually - // contain file rules), we don't do this check on v1 configs when there is only one rule. - if (getMajorVersion() >= 2 || getImpl()->m_fileRules->getNumEntries() != 1) + try { - try - { - getImpl()->m_fileRules->getImpl()->validate(*this); - } - catch (const Exception & e) - { - std::ostringstream os; - os << "Config failed validation. File rules failed with: "; - os << e.what(); - getImpl()->m_validationtext = os.str(); - throw Exception(getImpl()->m_validationtext.c_str()); - } + getImpl()->m_fileRules->getImpl()->validate(*this); + } + catch (const Exception & e) + { + std::ostringstream os; + os << "Config failed validation. File rules failed with: "; + os << e.what(); + getImpl()->m_validationtext = os.str(); + throw Exception(getImpl()->m_validationtext.c_str()); } ///// Resolve all file Transforms using context variables. @@ -4718,7 +4645,7 @@ void Config::Impl::checkVersionConsistency() const // Check for the file rules. - if (m_majorVersion < 2 && m_fileRules->getNumEntries() > 1) + if (m_majorVersion < 2 && m_fileRules->getNumEntries() > 2) { throw Exception("Only version 2 (or higher) can have file rules."); } diff --git a/src/OpenColorIO/FileRules.cpp b/src/OpenColorIO/FileRules.cpp index 329f54f500..8a35472ff6 100644 --- a/src/OpenColorIO/FileRules.cpp +++ b/src/OpenColorIO/FileRules.cpp @@ -11,6 +11,7 @@ #include "CustomKeys.h" #include "FileRules.h" +#include "Logging.h" #include "PathUtils.h" #include "Platform.h" #include "utils/StringUtils.h" @@ -662,9 +663,23 @@ void FileRules::Impl::moveRule(size_t ruleIndex, int offset) void FileRules::Impl::validate(const Config & cfg) const { - for (auto & rule : m_rules) + // All Config objects have a fileRules object, regardless of version. This object is + // initialized to have a defaultRule with the color space set to "default" (i.e., the default + // role). The fileRules->validate call will validate that all color spaces used in rules + // exist, or if they are roles that they point to a color space that exists. + // + // Because this would cause validate to improperly fail on v1 configs (since they are not + // required to actually contain file rules), we don't do this check on v1 configs when there is + // only two rules. In some case (e.g. load a v1 config from disk), the two expected rules are + // the 'Default' and 'ColorSpaceNamePathSearch' ones. + + if (cfg.getMajorVersion() >= 2 + || (cfg.getMajorVersion() == 1 && m_rules.size() > 2)) { - rule->validate(cfg); + for (auto & rule : m_rules) + { + rule->validate(cfg); + } } } @@ -940,5 +955,96 @@ std::ostream & operator<< (std::ostream & os, const FileRules & fr) } return os; } + +void UpdateFileRulesFromV1ToV2(const Config & config, FileRulesRcPtr & fileRules) +{ + if (config.getMajorVersion() != 1) + { + return; + } + + // In order to preserve the v1 behavior using Config:getColorSpaceFromFilepath() (i.e. + // mimic the Config::parseColorSpaceFromString() behavior) add the file path search + // rule to the list of file rules. + + try + { + // Throws if the file rule does not exist. + fileRules->getIndexForRule(FileRules::FilePathSearchRuleName); + } + catch(const Exception & /* ex */) + { + fileRules->insertPathSearchRule(0); + } + + // Now, double-check the default rule (which is using the default role) to find the + // right alternative if the default role is missing. + + // In order to always return a valid color space, the algorithm for the default rule is: + // 1. Use the default role if it exists (i.e. that's the default implementation) + // 2. Use the "raw" color space (case insensitive) if it exists & is a 'data' color space + // 3. Use the first 'data' color space if one exists + // 4. Use the first active color space + // 5. finally, fallback to the first color space. + + auto defaultCS = config.getColorSpace(ROLE_DEFAULT); + + if (!defaultCS) + { + ConstColorSpaceRcPtr cs = config.getColorSpace("raw"); + if (cs && cs->isData()) + { + fileRules->setColorSpace(1, cs->getName()); + } + else + { + const int numColorSpaces + = config.getNumColorSpaces(SEARCH_REFERENCE_SPACE_SCENE, COLORSPACE_ALL); + + bool found = false; + for (int idx = 0; idx < numColorSpaces && !found; ++idx) + { + const char * csName + = config.getColorSpaceNameByIndex(SEARCH_REFERENCE_SPACE_SCENE, + COLORSPACE_ALL, + idx); + ConstColorSpaceRcPtr cs = config.getColorSpace(csName); + + if (cs->isData()) + { + fileRules->setColorSpace(1, csName); + found = true; + } + } + + if (!found) + { + if (config.getNumColorSpaces() > 0) + { + // Take the first active color space. + fileRules->setColorSpace(1, config.getColorSpaceNameByIndex(0)); + } + else + { + static constexpr char msg[] + = "The default rule creation fallbacks to the first color space because "\ + "no suitable color space exists."; + + LogWarning(msg); + + // Take the first available color space. + const char * csName + = config.getColorSpaceNameByIndex(SEARCH_REFERENCE_SPACE_SCENE, + COLORSPACE_ALL, + 0); + + fileRules->setColorSpace(1, csName); + } + } + } + } +} + + } // namespace OCIO_NAMESPACE diff --git a/src/OpenColorIO/FileRules.h b/src/OpenColorIO/FileRules.h index 882d2e3909..9b4d81598f 100644 --- a/src/OpenColorIO/FileRules.h +++ b/src/OpenColorIO/FileRules.h @@ -69,6 +69,12 @@ class FileRules::Impl std::vector m_rules; }; + +// Helper method to build valid v2 file rules from a v1 config. Note that it does not change +// the config instance version. +void UpdateFileRulesFromV1ToV2(const Config & config, FileRulesRcPtr & fileRules); + + } // namespace OCIO_NAMESPACE #endif diff --git a/src/OpenColorIO/OCIOYaml.cpp b/src/OpenColorIO/OCIOYaml.cpp index 6b7c370419..67cafdf164 100644 --- a/src/OpenColorIO/OCIOYaml.cpp +++ b/src/OpenColorIO/OCIOYaml.cpp @@ -4657,10 +4657,23 @@ inline void load(const YAML::Node& node, ConfigRcPtr & config, const char* filen auto defaultCS = config->getColorSpace(ROLE_DEFAULT); if (!fileRulesFound) { - if (!defaultCS && config->getMajorVersion() >= 2) + if (config->getMajorVersion() >= 2) { - throwError(node, "The config must contain either a Default file rule or " - "the 'default' role."); + if (!defaultCS) + { + throwError(node, "The config must contain either a Default file rule or " + "the 'default' role."); + } + } + else + { + // In order to use Config::getColorSpaceFromFilepath() method for any version of + // config instance, the method updates the in-memory file rules created by a v1 config + // to have valid file rules and most importantly, to mimic + // Config::parseColorSpaceFromString() which is now deprecated since v2. + UpdateFileRulesFromV1ToV2(*config.get(), fileRules); + + config->setFileRules(fileRules); } } else diff --git a/src/bindings/python/CMakeLists.txt b/src/bindings/python/CMakeLists.txt index 3e9d70fd46..19c36451b0 100644 --- a/src/bindings/python/CMakeLists.txt +++ b/src/bindings/python/CMakeLists.txt @@ -134,10 +134,18 @@ if(${CMAKE_CXX_STANDARD} GREATER_EQUAL 17) set(APP_CXX_STANDARD 11) endif() +set(CUSTOM_COMPILE_FLAGS ${PLATFORM_COMPILE_FLAGS}) + +# The Python binding contains deprecated methods for backward compatibility reason, +# so disable the warning. +if(USE_GCC OR USE_CLANG) + set(CUSTOM_COMPILE_FLAGS "${CUSTOM_COMPILE_FLAGS} -Wno-deprecated-declarations") +endif() + set_target_properties(PyOpenColorIO PROPERTIES - COMPILE_FLAGS "${PLATFORM_COMPILE_FLAGS}" - CXX_STANDARD ${APP_CXX_STANDARD} + COMPILE_FLAGS ${CUSTOM_COMPILE_FLAGS} + CXX_STANDARD ${APP_CXX_STANDARD} ) if(NOT BUILD_SHARED_LIBS) diff --git a/src/bindings/python/PyConfig.cpp b/src/bindings/python/PyConfig.cpp index 07aab80257..cb5ab28eee 100644 --- a/src/bindings/python/PyConfig.cpp +++ b/src/bindings/python/PyConfig.cpp @@ -292,14 +292,8 @@ void bindPyConfig(py::module & m) DOC(Config, isColorSpaceUsed)) .def("clearColorSpaces", &Config::clearColorSpaces, DOC(Config, clearColorSpaces)) - - .def("parseColorSpaceFromString", [](ConfigRcPtr & self, const char * str) - { - return self->getColorSpaceFromFilepath(str); - }, - "str"_a, + .def("parseColorSpaceFromString", &Config::parseColorSpaceFromString, "str"_a, DOC(Config, parseColorSpaceFromString)) - .def("isStrictParsingEnabled", &Config::isStrictParsingEnabled, DOC(Config, isStrictParsingEnabled)) .def("setStrictParsingEnabled", &Config::setStrictParsingEnabled, "enabled"_a, diff --git a/tests/cpu/FileRules_tests.cpp b/tests/cpu/FileRules_tests.cpp index 5d64b33327..f5b77ee923 100644 --- a/tests/cpu/FileRules_tests.cpp +++ b/tests/cpu/FileRules_tests.cpp @@ -14,27 +14,203 @@ namespace OCIO = OCIO_NAMESPACE; OCIO_ADD_TEST(FileRules, config_v1) { - static const char CONFIG[] = - "ocio_profile_version: 1\n" - "strictparsing: false\n" - "roles:\n" - " default: raw\n" - "displays:\n" - " sRGB:\n" - " - ! {name: Raw, colorspace: raw}\n" - "colorspaces:\n" - " - !\n" - " name: raw\n"; + // From a v1 config create valid file rules. + + { + static const char CONFIG[] = + "ocio_profile_version: 1\n" + "\n" + "search_path: \"\"\n" + "strictparsing: false\n" + "luma: [0.2126, 0.7152, 0.0722]\n" + "\n" + "roles:\n" + " default: raw\n" + "\n" + "displays:\n" + " sRGB:\n" + " - ! {name: Raw, colorspace: raw}\n" + "\n" + "active_displays: []\n" + "active_views: []\n" + "\n" + "colorspaces:\n" + " - !\n" + " name: raw\n" + " family: \"\"\n" + " equalitygroup: \"\"\n" + " bitdepth: unknown\n" + " isdata: false\n" + " allocation: uniform\n"; - std::istringstream is; - is.str(CONFIG); + std::istringstream is; + is.str(CONFIG); - OCIO::ConstConfigRcPtr config; - OCIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromStream(is)); - OCIO_CHECK_NO_THROW(config->validate()); + OCIO::ConstConfigRcPtr config; + OCIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromStream(is)); + OCIO_CHECK_NO_THROW(config->validate()); + + OCIO_REQUIRE_EQUAL(config->getFileRules()->getNumEntries(), 2); + OCIO_CHECK_EQUAL(std::string(config->getFileRules()->getName(0)), OCIO::FileRules::FilePathSearchRuleName); + OCIO_CHECK_EQUAL(std::string(config->getFileRules()->getName(1)), OCIO::FileRules::DefaultRuleName); + + OCIO_CHECK_EQUAL(std::string(config->getFileRules()->getColorSpace(1)), std::string("default")); + + // Check that the file rules are not saved in a v1 config. + std::stringstream ss; + OCIO_CHECK_NO_THROW(ss << *config.get()); + OCIO_CHECK_EQUAL(ss.str(), std::string(CONFIG)); + } + + // Test fallback 1: The default role is missing and there is a data color space named 'raw'. + + { + static const char CONFIG[] = + "ocio_profile_version: 1\n" + "displays:\n" + " sRGB:\n" + " - ! {name: Raw, colorspace: raw}\n" + "colorspaces:\n" + " - !\n" + " name: cs2\n" + " - !\n" + " name: raw\n" + " isdata: true\n"; + + std::istringstream is; + is.str(CONFIG); + + OCIO::ConstConfigRcPtr config; + OCIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromStream(is)); + OCIO_CHECK_NO_THROW(config->validate()); + + OCIO_REQUIRE_EQUAL(config->getFileRules()->getNumEntries(), 2); + OCIO_CHECK_EQUAL(std::string(config->getFileRules()->getName(0)), OCIO::FileRules::FilePathSearchRuleName); + OCIO_CHECK_EQUAL(std::string(config->getFileRules()->getName(1)), OCIO::FileRules::DefaultRuleName); + + OCIO_CHECK_EQUAL(std::string(config->getFileRules()->getColorSpace(1)), std::string("raw")); + } + + // Test fallback 2: The default role is missing and there is a data color space. + // But 'raw' is not a data color space. + + { + static const char CONFIG[] = + "ocio_profile_version: 1\n" + "displays:\n" + " sRGB:\n" + " - ! {name: Raw, colorspace: raw}\n" + "colorspaces:\n" + " - !\n" + " name: cs2\n" + " - !\n" + " name: raw\n" + " - !\n" + " name: cs3\n" + " isdata: true\n"; + + std::istringstream is; + is.str(CONFIG); + + OCIO::ConstConfigRcPtr config; + OCIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromStream(is)); + OCIO_CHECK_NO_THROW(config->validate()); + + OCIO_REQUIRE_EQUAL(config->getFileRules()->getNumEntries(), 2); + OCIO_CHECK_EQUAL(std::string(config->getFileRules()->getName(0)), OCIO::FileRules::FilePathSearchRuleName); + OCIO_CHECK_EQUAL(std::string(config->getFileRules()->getName(1)), OCIO::FileRules::DefaultRuleName); + + OCIO_CHECK_EQUAL(std::string(config->getFileRules()->getColorSpace(1)), std::string("cs3")); + } + + // Test fallback 3: The default role is missing and there is no data color space but there is + // an active color space. + + { + static const char CONFIG[] = + "ocio_profile_version: 1\n" + "displays:\n" + " sRGB:\n" + " - ! {name: Raw, colorspace: raw}\n" + "colorspaces:\n" + " - !\n" + " name: cs2\n" + " - !\n" + " name: raw\n"; + + std::istringstream is; + is.str(CONFIG); + + OCIO::ConstConfigRcPtr config; + OCIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromStream(is)); + OCIO_CHECK_NO_THROW(config->validate()); + + OCIO_REQUIRE_EQUAL(config->getFileRules()->getNumEntries(), 2); + OCIO_CHECK_EQUAL(std::string(config->getFileRules()->getName(0)), OCIO::FileRules::FilePathSearchRuleName); + OCIO_CHECK_EQUAL(std::string(config->getFileRules()->getName(1)), OCIO::FileRules::DefaultRuleName); + + OCIO_CHECK_EQUAL(std::string(config->getFileRules()->getColorSpace(1)), std::string("cs2")); + } + + // Test that getColorSpaceFromFilePath works even with a v1 config (that pre-dates the + // introduction of file rules). + + { + static const char CONFIG[] = + "ocio_profile_version: 1\n" + "roles:\n" + " default: raw\n" + "displays:\n" + " sRGB:\n" + " - ! {name: Raw, colorspace: raw}\n" + "colorspaces:\n" + " - !\n" + " name: cs2\n" + " - !\n" + " name: raw\n" + " - !\n" + " name: cs3\n" + " isdata: true\n"; + + std::istringstream is; + is.str(CONFIG); - OCIO_CHECK_EQUAL(config->getFileRules()->getNumEntries(), 1); - OCIO_CHECK_EQUAL(std::string(config->getFileRules()->getName(0)), "Default"); + OCIO::ConstConfigRcPtr config; + OCIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromStream(is)); + OCIO_CHECK_NO_THROW(config->validate()); + + OCIO_REQUIRE_EQUAL(config->getFileRules()->getNumEntries(), 2); + OCIO_CHECK_EQUAL(std::string(config->getFileRules()->getName(0)), OCIO::FileRules::FilePathSearchRuleName); + OCIO_CHECK_EQUAL(std::string(config->getFileRules()->getName(1)), OCIO::FileRules::DefaultRuleName); + + OCIO_CHECK_EQUAL(std::string(config->getFileRules()->getColorSpace(1)), std::string("default")); + + // Test the file path search rule i.e. implemented using Config::parseColorSpaceFromString() + + size_t ruleIndex = size_t(-1); + + OCIO_CHECK_EQUAL(std::string(config->getColorSpaceFromFilepath("/usr/cs2_file.exr", ruleIndex)), + std::string("cs2")); + OCIO_CHECK_EQUAL(ruleIndex, 0); + OCIO_CHECK_ASSERT(!config->filepathOnlyMatchesDefaultRule("/usr/cs2_file.exr")); + + OCIO_CHECK_EQUAL(std::string(config->getColorSpaceFromFilepath("/usr/cs3/file.exr", ruleIndex)), + std::string("cs3")); + OCIO_CHECK_EQUAL(ruleIndex, 0); + OCIO_CHECK_ASSERT(!config->filepathOnlyMatchesDefaultRule("/usr/cs3/file.exr")); + + OCIO_CHECK_EQUAL(std::string(config->getColorSpaceFromFilepath("/usr/cs3/cs2_file.exr", ruleIndex)), + std::string("cs2")); + OCIO_CHECK_EQUAL(ruleIndex, 0); + OCIO_CHECK_ASSERT(!config->filepathOnlyMatchesDefaultRule("/usr/cs3/cs2_file.exr")); + + // Test that it fallbacks to the default rule when nothing found. + + OCIO_CHECK_EQUAL(std::string(config->getColorSpaceFromFilepath("/usr/file.exr", ruleIndex)), + std::string("default")); + OCIO_CHECK_EQUAL(ruleIndex, 1); + OCIO_CHECK_ASSERT(config->filepathOnlyMatchesDefaultRule("/usr/file.exr")); + } } OCIO_ADD_TEST(FileRules, config_read_only) @@ -1140,9 +1316,16 @@ strictparsing: true } -OCIO_ADD_TEST(FileRules, config_v1_to_v2) +OCIO_ADD_TEST(FileRules, config_v1_to_v2_from_file) { + // The unit test checks the file rules when loading a v1 config, the upgrade from v1 to v2 + // and finally, the use of file rules with the upgraded v2 in-memory config. + // + // Note: For now, only the file rules and the versions are impacted by the upgrade. + { + // Test the common use case i.e. read a v1 config file and upgrade it to v2. + constexpr char config_v1[] = { R"(ocio_profile_version: 1 strictparsing: true roles: @@ -1165,32 +1348,65 @@ strictparsing: true is.str(config_v1); OCIO::ConfigRcPtr config; OCIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromStream(is)->createEditableCopy()); - OCIO_CHECK_NO_THROW(config->validate()); - config->setMajorVersion(2); + // Check the version. + + OCIO_CHECK_EQUAL(config->getMajorVersion(), 1); + + // Check the file rules. + + auto rules = config->getFileRules(); + OCIO_CHECK_EQUAL(rules->getNumEntries(), 2); + OCIO_CHECK_EQUAL(std::string(rules->getName(0)), OCIO::FileRules::FilePathSearchRuleName); + OCIO_CHECK_EQUAL(std::string(rules->getName(1)), OCIO::FileRules::DefaultRuleName); + OCIO_CHECK_EQUAL(std::string(rules->getColorSpace(1)), "default"); + + // Check the v1 in-memory file rules are working. + + // It checks that the rule 'FileRules::FilePathSearchRuleName' exists. + OCIO_CHECK_EQUAL(std::string(config->getColorSpaceFromFilepath("/usr/cs2_file.exr")), "cs2"); + // It checks that the rule 'Default' exists. + OCIO_CHECK_EQUAL(std::string(config->getColorSpaceFromFilepath("/usr/file.exr")), "default"); + + // Upgrading is making sure to build a valid v2 config. + + config->upgradeToLatestVersion(); OCIO_CHECK_NO_THROW(config->validate()); - auto rules = config->getFileRules()->createEditableCopy(); - OCIO_CHECK_EQUAL(std::string(rules->getColorSpace(rules->getNumEntries() - 1)), - "default"); - rules->setDefaultRuleColorSpace("cs1"); - OCIO_CHECK_EQUAL(std::string(rules->getColorSpace(rules->getNumEntries() - 1)), - "cs1"); + // Check the new version. + + OCIO_CHECK_EQUAL(config->getMajorVersion(), 2); + + // Check the new file rules. + + rules = config->getFileRules(); + OCIO_CHECK_EQUAL(rules->getNumEntries(), 2); + OCIO_CHECK_EQUAL(std::string(rules->getName(0)), OCIO::FileRules::FilePathSearchRuleName); + OCIO_CHECK_EQUAL(std::string(rules->getName(1)), OCIO::FileRules::DefaultRuleName); + OCIO_CHECK_EQUAL(std::string(rules->getColorSpace(1)), "default"); + + // Check the v1 in-memory file rules are working. + + OCIO_CHECK_EQUAL(std::string(config->getColorSpaceFromFilepath("/usr/cs2_file.exr")), "cs2"); + OCIO_CHECK_EQUAL(std::string(config->getColorSpaceFromFilepath("/usr/file.exr")), "default"); } { - constexpr char config_v1_no_default[] = { R"(ocio_profile_version: 1 + // The default role is missing and there is a 'data' color space named rAw. + + constexpr char config_v1[] = { R"(ocio_profile_version: 1 strictparsing: true roles: role1: cs1 role2: cs2 displays: sRGB: - - ! {name: Raw, colorspace: raw} + - ! {name: Raw, colorspace: rAw} colorspaces: - ! - name: raw + name: rAw + isdata: true - ! name: cs1 - ! @@ -1198,85 +1414,301 @@ strictparsing: true )" }; std::istringstream is; - is.str(config_v1_no_default); - OCIO::ConstConfigRcPtr constConfig; - OCIO_CHECK_NO_THROW(constConfig = OCIO::Config::CreateFromStream(is)); - OCIO::ConfigRcPtr config = constConfig->createEditableCopy(); - + is.str(config_v1); + OCIO::ConfigRcPtr config; + OCIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromStream(is)->createEditableCopy()); OCIO_CHECK_NO_THROW(config->validate()); - config->setMajorVersion(2); - // Default rule is using 'Default' role that does not exist. - OCIO_CHECK_THROW_WHAT(config->validate(), OCIO::Exception, "rule named 'Default' is " - "referencing 'default' that is neither a color space nor a named " - "transform"); + // Check the version. + + OCIO_CHECK_EQUAL(config->getMajorVersion(), 1); - config = constConfig->createEditableCopy(); - OCIO_CHECK_EQUAL(config->getNumColorSpaces(), 3); + // Check the file rules. + + auto rules = config->getFileRules(); + OCIO_CHECK_EQUAL(rules->getNumEntries(), 2); + OCIO_CHECK_EQUAL(std::string(rules->getName(0)), OCIO::FileRules::FilePathSearchRuleName); + OCIO_CHECK_EQUAL(std::string(rules->getName(1)), OCIO::FileRules::DefaultRuleName); + OCIO_CHECK_EQUAL(std::string(rules->getColorSpace(1)), "rAw"); + + // Check the v1 in-memory file rules are working. + + OCIO_CHECK_EQUAL(std::string(config->getColorSpaceFromFilepath("/usr/cs2_file.exr")), "cs2"); + OCIO_CHECK_EQUAL(std::string(config->getColorSpaceFromFilepath("/usr/file.exr")), "rAw"); + + // Upgrading is making sure to build a valid v2 config. - // Upgrading is making sure that a valid v1 config will be a valid v2 config. config->upgradeToLatestVersion(); + OCIO_CHECK_NO_THROW(config->validate()); + + // Check the new version. OCIO_CHECK_EQUAL(config->getMajorVersion(), 2); - OCIO_CHECK_NO_THROW(config->validate()); - // 'Default' does not exist, 'Raw' is not a data color-space, so a new color-space has - // been created with a unique name. - OCIO_CHECK_EQUAL(config->getNumColorSpaces(), 3); - OCIO_CHECK_EQUAL(config->getNumColorSpaces(OCIO::SEARCH_REFERENCE_SPACE_ALL, - OCIO::COLORSPACE_ALL), 4); - OCIO_CHECK_EQUAL(std::string(config->getColorSpaceNameByIndex(OCIO::SEARCH_REFERENCE_SPACE_ALL, - OCIO::COLORSPACE_ALL, 3)), - "added_default_rule_colorspace"); + // Check the new file rules. + + rules = config->getFileRules(); + OCIO_CHECK_EQUAL(rules->getNumEntries(), 2); + OCIO_CHECK_EQUAL(std::string(rules->getName(0)), OCIO::FileRules::FilePathSearchRuleName); + OCIO_CHECK_EQUAL(std::string(rules->getName(1)), OCIO::FileRules::DefaultRuleName); + OCIO_CHECK_EQUAL(std::string(rules->getColorSpace(1)), "rAw"); + + OCIO_CHECK_EQUAL(std::string(config->getColorSpaceFromFilepath("/usr/cs2_file.exr")), "cs2"); + OCIO_CHECK_EQUAL(std::string(config->getColorSpaceFromFilepath("/usr/file.exr")), "rAw"); } { - constexpr char config_v1_no_default[] = { R"(ocio_profile_version: 1 + // The default role is missing and there is no 'data' color space so, the first + // color space is used in v1, and the first active color space is used in v2. + + // Note that inactive color spaces do not exist in v1 explaining why the first color + // space is used. + + constexpr char config_v1[] = { R"(ocio_profile_version: 1 strictparsing: true roles: role1: cs1 role2: cs2 displays: sRGB: - - ! {name: Raw, colorspace: raw} + - ! {name: Raw, colorspace: rAw} colorspaces: - - ! - name: rAw - isdata: true - ! name: cs1 - ! name: cs2 + - ! + name: rAw )" }; std::istringstream is; - is.str(config_v1_no_default); - OCIO::ConstConfigRcPtr constConfig; - OCIO_CHECK_NO_THROW(constConfig = OCIO::Config::CreateFromStream(is)); - OCIO::ConfigRcPtr config = constConfig->createEditableCopy(); - + is.str(config_v1); + OCIO::ConstConfigRcPtr config; + OCIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromStream(is)); OCIO_CHECK_NO_THROW(config->validate()); - config->setMajorVersion(2); + + // Check the version. + + OCIO_CHECK_EQUAL(config->getMajorVersion(), 1); + + // Check the file rules. + + auto rules = config->getFileRules(); + OCIO_CHECK_EQUAL(rules->getNumEntries(), 2); + OCIO_CHECK_EQUAL(std::string(rules->getName(0)), OCIO::FileRules::FilePathSearchRuleName); + OCIO_CHECK_EQUAL(std::string(rules->getName(1)), OCIO::FileRules::DefaultRuleName); + OCIO_CHECK_EQUAL(std::string(rules->getColorSpace(1)), "cs1"); + + // Check the v1 in-memory file rules are working. + + OCIO_CHECK_EQUAL(std::string(config->getColorSpaceFromFilepath("/usr/cs2_file.exr")), "cs2"); + OCIO_CHECK_EQUAL(std::string(config->getColorSpaceFromFilepath("/usr/file.exr")), "cs1"); + + { + // In v2, the first active color space is then used for the 'Default' rule. + + OCIO::ConfigRcPtr cfg; + OCIO_CHECK_NO_THROW(cfg = config->createEditableCopy()); + + // Upgrading is making sure to build a valid v2 config. + + cfg->setInactiveColorSpaces("cs1"); + cfg->upgradeToLatestVersion(); + OCIO_CHECK_NO_THROW(cfg->validate()); + + // Check the new version. + + OCIO_CHECK_EQUAL(cfg->getMajorVersion(), 2); + + // Check the new file rules. + + rules = cfg->getFileRules(); + OCIO_CHECK_EQUAL(rules->getNumEntries(), 2); + OCIO_CHECK_EQUAL(std::string(rules->getName(0)), OCIO::FileRules::FilePathSearchRuleName); + OCIO_CHECK_EQUAL(std::string(rules->getName(1)), OCIO::FileRules::DefaultRuleName); + OCIO_CHECK_EQUAL(std::string(rules->getColorSpace(1)), "cs2"); + + OCIO_CHECK_EQUAL(std::string(cfg->getColorSpaceFromFilepath("/usr/cs1_file.exr")), "cs1"); + OCIO_CHECK_EQUAL(std::string(cfg->getColorSpaceFromFilepath("/usr/file.exr")), "cs2"); + } + + { + // In v2, the first color space is used for the 'Default' rule because there no active + // color spaces. + + OCIO::ConfigRcPtr cfg; + OCIO_CHECK_NO_THROW(cfg = config->createEditableCopy()); + + // Upgrading is making sure to build a valid v2 config. + + cfg->setInactiveColorSpaces("cs1, cs2, raw"); + + { + OCIO::LogGuard l; + + cfg->upgradeToLatestVersion(); + + OCIO_CHECK_EQUAL( + std::string("[OpenColorIO Warning]: The default rule creation fallbacks to the"\ + " first color space because no suitable color space exists.\n"), + l.output()); + } + + OCIO_CHECK_NO_THROW(cfg->validate()); + + // Check the new version. + + OCIO_CHECK_EQUAL(cfg->getMajorVersion(), 2); + + // Check the new file rules. + + rules = cfg->getFileRules(); + OCIO_CHECK_EQUAL(rules->getNumEntries(), 2); + OCIO_CHECK_EQUAL(std::string(rules->getName(0)), OCIO::FileRules::FilePathSearchRuleName); + OCIO_CHECK_EQUAL(std::string(rules->getName(1)), OCIO::FileRules::DefaultRuleName); + OCIO_CHECK_EQUAL(std::string(rules->getColorSpace(1)), "cs1"); + + OCIO_CHECK_EQUAL(std::string(cfg->getColorSpaceFromFilepath("/usr/raw_file.exr")), "rAw"); + OCIO_CHECK_EQUAL(std::string(cfg->getColorSpaceFromFilepath("/usr/file.exr")), "cs1"); + } + } +} + +OCIO_ADD_TEST(FileRules, config_v1_to_v2_from_memory) +{ + // The unit test checks the file rules from an in-memory v1 config, the upgrade from v1 to v2, + // and finally, the file rules in the upgraded v2 in-memory config. + // + // Note: For now, only the file rules and the versions are impacted by the upgrade. + + // The following tests manually create an in-memory v1 config with faulty file rules. As the + // config file read (which automatically updates in-memory v1 file rules like in previous tests) + // is not used, only an explicit upgrade to the latest version, can fix the file rules. + + { + // The default role is missing but there is an active 'data' color space. + + OCIO::ConfigRcPtr config = OCIO::Config::Create(); + config->setMajorVersion(1); + config->addDisplayView("disp1", "view1", "cs1", nullptr); + OCIO::ColorSpaceRcPtr cs1 = OCIO::ColorSpace::Create(); + cs1->setName("cs1"); + cs1->setIsData(true); + config->addColorSpace(cs1); + OCIO::ColorSpaceRcPtr raw = OCIO::ColorSpace::Create(); + raw->setName("rAw"); + config->addColorSpace(raw); + OCIO_CHECK_NO_THROW(config->validate()); // because file rules are not validated. // Default rule is using 'Default' role that does not exist. + config->setMajorVersion(2); OCIO_CHECK_THROW_WHAT(config->validate(), OCIO::Exception, "rule named 'Default' is " "referencing 'default' that is neither a color space nor a named " "transform"); - config = constConfig->createEditableCopy(); - OCIO_CHECK_EQUAL(config->getNumColorSpaces(), 3); + // Upgrading is making sure to build a valid v2 config. + config->setMajorVersion(1); + config->upgradeToLatestVersion(); + OCIO_CHECK_NO_THROW(config->validate()); + + // Check the new version. + + OCIO_CHECK_EQUAL(config->getMajorVersion(), 2); + + // 'cs1' is an active & 'data' color space. + + auto rules = config->getFileRules(); + OCIO_CHECK_EQUAL(rules->getNumEntries(), 2); + OCIO_CHECK_EQUAL(std::string(rules->getName(0)), OCIO::FileRules::FilePathSearchRuleName); + OCIO_CHECK_EQUAL(std::string(rules->getName(1)), OCIO::FileRules::DefaultRuleName); + OCIO_CHECK_EQUAL(std::string(rules->getColorSpace(1)), "cs1"); + } + + // The default role is missing and there is no 'data' color space. + + { + OCIO::ConfigRcPtr config = OCIO::Config::Create(); + config->setMajorVersion(1); + config->addDisplayView("disp1", "view1", "cs1", nullptr); + OCIO::ColorSpaceRcPtr cs1 = OCIO::ColorSpace::Create(); + cs1->setName("cs1"); + config->addColorSpace(cs1); + OCIO::ColorSpaceRcPtr raw = OCIO::ColorSpace::Create(); + raw->setName("rAw"); + config->addColorSpace(raw); + OCIO_CHECK_NO_THROW(config->validate()); // because file rules are not validated. + + // Default rule is using 'Default' role but the associated color space does not exist. + config->setMajorVersion(2); + OCIO_CHECK_THROW_WHAT(config->validate(), OCIO::Exception, "rule named 'Default' is " + "referencing 'default' that is neither a color space nor a named " + "transform"); - // Upgrading is making sure that a valid v1 config will be a valid v2 config. + // Upgrading is making sure to build a valid v2 config. + config->setMajorVersion(1); config->upgradeToLatestVersion(); + OCIO_CHECK_NO_THROW(config->validate()); + + // Check the new version. OCIO_CHECK_EQUAL(config->getMajorVersion(), 2); + + // 'Default' role does not exist, 'Raw' is not a data color-space, so use the first active + // color space. + + auto rules = config->getFileRules(); + OCIO_CHECK_EQUAL(rules->getNumEntries(), 2); + OCIO_CHECK_EQUAL(std::string(rules->getName(0)), OCIO::FileRules::FilePathSearchRuleName); + OCIO_CHECK_EQUAL(std::string(rules->getName(1)), OCIO::FileRules::DefaultRuleName); + OCIO_CHECK_EQUAL(std::string(rules->getColorSpace(1)), "cs1"); + } + + // The default role is missing and there is no 'data' & active color space. The algorithm then + // fallbacks to the first available color space and logs a warning. + + { + OCIO::ConfigRcPtr config = OCIO::Config::Create(); + config->setMajorVersion(1); + config->addDisplayView("disp1", "view1", "cs1", nullptr); + OCIO::ColorSpaceRcPtr cs1 = OCIO::ColorSpace::Create(); + cs1->setName("cs1"); + config->addColorSpace(cs1); + OCIO_CHECK_NO_THROW(config->validate()); // because file rules are not validated. + + // Default rule is using 'Default' role but the associated color space does not exist. + config->setInactiveColorSpaces("cs1"); + config->setMajorVersion(2); + OCIO_CHECK_THROW_WHAT(config->validate(), OCIO::Exception, "rule named 'Default' is " + "referencing 'default' that is neither a color space nor a named " + "transform"); + + config->setMajorVersion(1); + + { + OCIO::LogGuard l; + + config->upgradeToLatestVersion(); + + OCIO_CHECK_EQUAL( + std::string("[OpenColorIO Warning]: The default rule creation fallbacks to the"\ + " first color space because no suitable color space exists.\n"), + l.output()); + } + OCIO_CHECK_NO_THROW(config->validate()); - // 'Default' does not exist, 'Raw' is a data color-space. - OCIO_CHECK_EQUAL(config->getNumColorSpaces(), 3); + // Check the new version. + + OCIO_CHECK_EQUAL(config->getMajorVersion(), 2); + + // Check the 'default' rule. As there is not 'data' or active color space, the default + // rule is using an inactive color space. + auto rules = config->getFileRules(); - const auto numRules = rules->getNumEntries(); - OCIO_CHECK_EQUAL(std::string(rules->getColorSpace(numRules - 1)), "rAw"); + OCIO_CHECK_EQUAL(rules->getNumEntries(), 2); + OCIO_CHECK_EQUAL(std::string(rules->getName(0)), OCIO::FileRules::FilePathSearchRuleName); + OCIO_CHECK_EQUAL(std::string(rules->getName(1)), OCIO::FileRules::DefaultRuleName); + OCIO_CHECK_EQUAL(std::string(rules->getColorSpace(1)), "cs1"); } }