From 26dc20e1df0aa6da8190f7584d49d606f53ea6c4 Mon Sep 17 00:00:00 2001 From: Doug Walker Date: Thu, 10 Sep 2026 00:28:26 -0400 Subject: [PATCH 1/4] Implement display and view aliases Signed-off-by: Doug Walker --- docs/guides/authoring/displays_views.rst | 19 + include/OpenColorIO/OpenColorIO.h | 109 ++- src/OpenColorIO/Config.cpp | 408 ++++++++- src/OpenColorIO/OCIOYaml.cpp | 32 + src/OpenColorIO/ViewTransform.cpp | 75 ++ .../apphelpers/LegacyViewingPipeline.cpp | 41 +- .../transforms/DisplayViewTransform.cpp | 70 +- src/apps/ocioconvert/main.cpp | 5 +- src/bindings/python/PyConfig.cpp | 13 + src/bindings/python/PyViewTransform.cpp | 67 +- tests/cpu/Config_tests.cpp | 786 +++++++++++++++++- tests/cpu/ViewTransform_tests.cpp | 71 ++ .../LegacyViewingPipeline_tests.cpp | 132 +++ .../transforms/DisplayViewTransform_tests.cpp | 373 +++++++++ tests/python/ConfigTest.py | 123 +++ tests/python/ViewTransformTest.py | 64 ++ 16 files changed, 2326 insertions(+), 62 deletions(-) diff --git a/docs/guides/authoring/displays_views.rst b/docs/guides/authoring/displays_views.rst index 1cdc8c51bf..adede1c874 100644 --- a/docs/guides/authoring/displays_views.rst +++ b/docs/guides/authoring/displays_views.rst @@ -132,6 +132,25 @@ A View Transform may use the following keys: .. TODO: Good spot for an example in a future revision. + +``use_display_view_aliases`` +^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +Optional. Activates aliases for display and view names. + +By default, the arguments to DisplayViewTransform must be the exact strings found +in the display / view section of the config. However, if ``ocio_profile_version`` +is 2.6 or higher, ``use_display_view_aliases`` may be set to true. This allows a +display to be referred to by the name or aliases of its corresponding display +ColorSpace and the view to be referred to by the name or aliases of its +corresponding ViewTransform. This config-level attribute defaults to false and +must be omitted from the config file if its value is not "true". + +.. code-block:: yaml + + use_display_view_aliases: true + + ``default_view_transform`` ^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/include/OpenColorIO/OpenColorIO.h b/include/OpenColorIO/OpenColorIO.h index ad294fc805..8e10024d40 100644 --- a/include/OpenColorIO/OpenColorIO.h +++ b/include/OpenColorIO/OpenColorIO.h @@ -656,10 +656,9 @@ class OCIOEXPORT Config void removeColorSpace(const char * name); /** - * Return true if the color space is used by a transform, a role, or a look. - * - * \note - * Name must be the canonical name. + * Return true if the color space is used by a transform, a role, a look, a (display, view) + * pair, or a file rule. The argument may be either an alias or the canonical name. While + * searching the config, aliases are always resolve to their canonical names for comparison. */ bool isColorSpaceUsed(const char * name) const noexcept; @@ -918,6 +917,8 @@ class OCIOEXPORT Config /** * Returns the colorspace attribute of the (display, view) pair. * (Note that this may be either a color space or a display color space.) + * See \ref Config::getResolvedDisplayViewColorSpaceName to first resolve + * any display or view aliases. */ const char * getDisplayViewColorSpaceName(const char * display, const char * view) const; /// Returns the looks attribute of a (display, view) pair. @@ -985,6 +986,85 @@ class OCIOEXPORT Config /// Clear all the displays. void clearDisplays(); + /** + * Methods related to display and view aliases. + * + */ + + /** + * \brief This property on the Config object allows config authors to use aliases for + * display or view names. This feature is off by default. + * + * Corresponds to the "use_display_view_aliases" config file attribute, which is only + * written to the file when true. Requires config version 2.6 or higher (validation + * will fail if this is enabled on an older config). + */ + bool getUseDisplayViewAliases() const noexcept; + void setUseDisplayViewAliases(bool enabled) noexcept; + + /** + * \brief Resolve display name aliases. + * + * If the argument does not match an existing display, a fallback checks if getColorSpace + * returns a display color space. If so, it checks to see if there is a display whose + * name matches that color space name or one of its aliases. + * + * This fallback is only performed if \ref Config::getUseDisplayViewAliases is true. + * + * Returns "" if no display can be found, even with the fallback. + */ + const char * getCanonicalDisplayName(const char * displayName) const; + + /** + * \brief Resolve view name aliases. + * + * If the arguments do not directly match an existing (display, view) pair, a fallback + * checks if getViewTransform or getNamedTransform returns a result for viewName. If so, + * it checks if the display has a view whose view_transform matches the name or an alias + * of that transform. + * + * This fallback is only performed if \ref Config::getUseDisplayViewAliases is true. + * + * The displayName is first resolved via \ref Config::getCanonicalDisplayName. + * + * Returns "" if no display and view can be found, even with the fallback, or if the + * arguments are null or empty. + */ + const char * getCanonicalViewName(const char * displayName, const char * viewName) const; + + /** + * \brief Returns the name of the color space that a (display, view) pair uses. + * + * This is similar to \ref Config::getDisplayViewColorSpaceName, but it first attempts + * to resolve displayName and viewName (which could be aliases) to their canonical names. + * And unlike that function, the displayName may not be empty. The alias resolution is + * gated by \ref Config::getUseDisplayViewAliases. + * + * In addition, if the display_colorspace of a shared view is , that + * is resolved to the name of the view's display. + * + * Note that, as with getDisplayViewColorSpaceName, the returned name may be that of a + * named transform rather than a color space (this is allowed for views that have no + * view_transform). + * + * Returns either the canonical name of the view's color space or, if that does not + * find a result, the raw color space string (which would likely be used in an error + * message). If the (display, view) pair cannot even be resolved, it returns "". + */ + const char * getResolvedDisplayViewColorSpaceName(const char * displayName, + const char * viewName) const; + + /** + * \brief Return the description of the display color space associated with displayName. + * + * If displayName matches the canonical name of a display color space, its description is + * returned. If \ref Config::getUseDisplayViewAliases is true, the search is broadened to + * include display color spaces that have displayName as an alias. + * + * Returns "" if no such display color space can be found. + */ + const char * getDisplayDescription(const char * displayName) const; + /** * Methods related to the Virtual Display. * @@ -2555,14 +2635,35 @@ class OCIOEXPORT ViewTransform ViewTransformRcPtr createEditableCopy() const; const char * getName() const noexcept; + /// \see ColorSpace::setName void setName(const char * name) noexcept; + /** + * \note + * ViewTransform aliases are available in config file versions 2.6 or higher. They + * are availaable regardless of how \ref Config::getUseDisplayViewAliases is set. + */ + + /// \see ColorSpace::getNumAliases + size_t getNumAliases() const noexcept; + /// \see ColorSpace::getAlias + const char * getAlias(size_t idx) const noexcept; + /// \see ColorSpace::hasAlias + bool hasAlias(const char * alias) const noexcept; + /// \see ColorSpace::addAlias + void addAlias(const char * alias) noexcept; + /// \see ColorSpace::removeAlias + void removeAlias(const char * alias) noexcept; + /// \see ColorSpace::clearAliases + void clearAliases() noexcept; + /// \see ColorSpace::getFamily const char * getFamily() const noexcept; /// \see ColorSpace::setFamily void setFamily(const char * family); const char * getDescription() const noexcept; + /// \see ColorSpace::setDescription void setDescription(const char * description); /** diff --git a/src/OpenColorIO/Config.cpp b/src/OpenColorIO/Config.cpp index a8f25d5cad..2900984cbe 100644 --- a/src/OpenColorIO/Config.cpp +++ b/src/OpenColorIO/Config.cpp @@ -248,7 +248,7 @@ static constexpr unsigned LastSupportedMajorVersion = OCIO_VERSION_MAJOR; // For each major version keep the most recent minor. static const unsigned int LastSupportedMinorVersion[] = {0, // Version 1 - 5 // Version 2 + 6 // Version 2 }; } // namespace @@ -321,6 +321,7 @@ class Config::Impl // Misc std::vector m_defaultLumaCoefs; bool m_strictParsing; + bool m_useDisplayViewAliases{ false }; mutable Validation m_validation; mutable std::string m_validationtext; @@ -440,6 +441,7 @@ class Config::Impl m_defaultViewTransform = rhs.m_defaultViewTransform; m_defaultLumaCoefs = rhs.m_defaultLumaCoefs; m_strictParsing = rhs.m_strictParsing; + m_useDisplayViewAliases = rhs.m_useDisplayViewAliases; m_validation = rhs.m_validation; m_validationtext = rhs.m_validationtext; @@ -539,6 +541,15 @@ class Config::Impl } } + // Not found by name, so look for an alias. + for (const auto & vt : m_viewTransforms) + { + if (vt->hasAlias(name)) + { + return vt; + } + } + return ConstViewTransformRcPtr(); } @@ -1955,7 +1966,8 @@ void Config::validate() const if (!getImpl()->m_defaultViewTransform.empty()) { const auto vt = getDefaultSceneToDisplayViewTransform(); - if (!vt || !StringUtils::Compare(vt->getName(), getImpl()->m_defaultViewTransform)) + if (!vt || !(StringUtils::Compare(vt->getName(), getImpl()->m_defaultViewTransform) || + vt->hasAlias(getImpl()->m_defaultViewTransform.c_str()))) { std::ostringstream os; os << "Config failed validation. Default view transform is defined as: '"; @@ -2674,6 +2686,21 @@ bool Config::isColorSpaceUsed(const char * name) const noexcept if (!name || !*name) return false; + // Resolve any aliases, so both source and target are comparing the canonical names. + auto resolveName = [this](const char * csName) -> std::string + { + const char * canonicalName = getCanonicalName(csName); + return (canonicalName && *canonicalName) ? std::string(canonicalName) + : std::string(csName ? csName : ""); + }; + + const std::string searchName = resolveName(name); + + auto isMatch = [&resolveName, &searchName](const char * csName) + { + return csName && *csName && StringUtils::Compare(resolveName(csName), searchName); + }; + // Check for all color spaces, looks and view transforms. ConstTransformVec allTransforms; @@ -2688,7 +2715,7 @@ bool Config::isColorSpaceUsed(const char * name) const noexcept for (const auto & csName : colorSpaceNames) { - if (0 == Platform::Strcasecmp(name, csName.c_str())) + if (isMatch(csName.c_str())) { return true; } @@ -2701,7 +2728,7 @@ bool Config::isColorSpaceUsed(const char * name) const noexcept { const char * roleName = getRoleName(idx); const char * csName = LookupRole(getImpl()->m_roles, roleName); - if (0 == Platform::Strcasecmp(csName, name)) + if (isMatch(csName)) { return true; } @@ -2712,7 +2739,7 @@ bool Config::isColorSpaceUsed(const char * name) const noexcept for (const auto & view : getImpl()->m_sharedViews) { const char * csName = view.m_colorspace.c_str(); - if (0 == Platform::Strcasecmp(csName, name)) + if (isMatch(csName)) { return true; } @@ -2727,7 +2754,7 @@ bool Config::isColorSpaceUsed(const char * name) const noexcept { const char * viewName = view.m_name.c_str(); const char * csName = getDisplayViewColorSpaceName(dispName, viewName); - if (0 == Platform::Strcasecmp(csName, name)) + if (isMatch(csName)) { return true; } @@ -2740,7 +2767,8 @@ bool Config::isColorSpaceUsed(const char * name) const noexcept if (!((*viewIt).m_viewTransform.empty()) && (*viewIt).useDisplayNameForColorspace()) { - if (0 == Platform::Strcasecmp(dispName, name)) + // The view uses the display color space named after the display. + if (isMatch(dispName)) { return true; } @@ -2757,7 +2785,7 @@ bool Config::isColorSpaceUsed(const char * name) const noexcept const char * lookName = getLookNameByIndex(idx); ConstLookRcPtr l = getLook(lookName); - if (0 == Platform::Strcasecmp(l->getProcessSpace(), name)) + if (isMatch(l->getProcessSpace())) { return true; } @@ -2771,7 +2799,7 @@ bool Config::isColorSpaceUsed(const char * name) const noexcept for (size_t idx = 0; idx < numRules; ++idx) { const char * csName = rules->getColorSpace(idx); - if (0 == Platform::Strcasecmp(csName, name)) + if (isMatch(csName)) { return true; } @@ -3608,6 +3636,8 @@ const char * Config::getDisplayViewTransformName(const char * display, const cha return viewPtr->m_viewTransform.c_str(); } +// NB: See getResolvedDisplayViewColorSpaceName for a version of this that resolves display aliases. + const char * Config::getDisplayViewColorSpaceName(const char * display, const char * view) const { const View * viewPtr = getImpl()->getView(display, view); @@ -3823,6 +3853,275 @@ void Config::clearDisplays() getImpl()->resetCacheIDs(); } +/////////////////////////////////////////////////////////////////////////// +// Unlike the above functions, these are set up to work with display +// and view aliases. + +bool Config::getUseDisplayViewAliases() const noexcept +{ + return getImpl()->m_useDisplayViewAliases; +} + +void Config::setUseDisplayViewAliases(bool enabled) noexcept +{ + getImpl()->m_useDisplayViewAliases = enabled; + + AutoMutex lock(getImpl()->m_cacheidMutex); + getImpl()->resetCacheIDs(); +} + +const char * Config::getCanonicalDisplayName(const char * displayName) const +{ + if (!displayName || !*displayName) + { + return ""; + } + + // This is the normal case, there is a display called displayName. + DisplayMap::const_iterator iter = FindDisplay(getImpl()->m_displays, displayName); + if (iter != getImpl()->m_displays.end()) + { + return iter->first.c_str(); + } + + // Use of the fallback requires a config-level opt-in, which is false by default. + if (!getImpl()->m_useDisplayViewAliases) + { + return ""; + } + + // Normally, getColorSpace searches roles, but that is not the intended use-case and + // could be confusing, so don't resolve against them. + if (hasRole(displayName)) + { + return ""; + } + + // Look for a display color space that has displayName as its name or an alias. + ConstColorSpaceRcPtr cs = getColorSpace(displayName); + + // Only consider display-referred color spaces. + if (!cs || cs->getReferenceSpaceType() == REFERENCE_SPACE_SCENE) + { + return ""; + } + + iter = FindDisplay(getImpl()->m_displays, cs->getName()); + if (iter != getImpl()->m_displays.end()) + { + // A display exists with the name of the color space. In this case, displayName + // was an alias of the color space. + return iter->first.c_str(); + } + + const size_t numAliases = cs->getNumAliases(); + for (size_t i = 0; i < numAliases; ++i) + { + iter = FindDisplay(getImpl()->m_displays, cs->getAlias(i)); + if (iter != getImpl()->m_displays.end()) + { + // A display exists with the name of an alias of the color space. In this case, + // displayName was either the color space name or one of the other aliases. + return iter->first.c_str(); + } + } + + return ""; +} + +const char * Config::getResolvedDisplayViewColorSpaceName(const char * display, + const char * view) const +{ + if (!display || !*display || !view || !*view) + { + return ""; + } + + const char * resolvedDisplay = getCanonicalDisplayName(display); + if (!resolvedDisplay || !*resolvedDisplay) + { + return ""; + } + + const char * resolvedView = getCanonicalViewName(resolvedDisplay, view); + if (!resolvedView || !*resolvedView) + { + return ""; + } + + const View * viewPtr = getImpl()->getView(resolvedDisplay, resolvedView); + if (!viewPtr) return ""; + + // A shared view that has a view transform may set its display_colorspace to + // , meaning that the display color space named after the display is used. + const char * csName = viewPtr->useDisplayNameForColorspace() ? resolvedDisplay + : viewPtr->m_colorspace.c_str(); + + // Return the canonical name, since csName may be a role or an alias. (Note that the view's + // colorspace attribute is allowed to name a named transform rather than a color space, if + // the view has no view_transform, and getCanonicalName handles both.) If it doesn't name + // anything at all, return it as-is so callers can report what they were unable to find. + const char * canonicalName = getCanonicalName(csName); + return (canonicalName && *canonicalName) ? canonicalName : csName; +} + +const char * Config::getDisplayDescription(const char * display) const +{ + if (!display || !*display) + { + return ""; + } + + // Getting a description requires that the display name matches the name or alias of + // a display color space in the config. + ConstColorSpaceRcPtr cs = getColorSpace(display); + + // Only support display color spaces. + if (!cs || cs->getReferenceSpaceType() != REFERENCE_SPACE_DISPLAY) + { + return ""; + } + + // A display color space named exactly "display" always works. If it was only found via + // one of its aliases, the fallback is opt-in. + if (!StringUtils::Compare(cs->getName(), display) && !getImpl()->m_useDisplayViewAliases) + { + return ""; + } + + return cs->getDescription(); +} + +const char * Config::getCanonicalViewName(const char * displayName, const char * viewName) const +{ + if (!displayName || !*displayName || !viewName || !*viewName) + { + return ""; + } + + // Resolve the displayName first, since it may itself be an alias. (The display + // resolution function's fallback is likewise gated by m_useDisplayViewAliases.) + const char * resolvedDisplay = displayName; + const char * canonicalDisplay = getCanonicalDisplayName(displayName); + if (canonicalDisplay && *canonicalDisplay) + { + resolvedDisplay = canonicalDisplay; + } + + // This is the normal case, the display has a view named viewName. + if (getImpl()->getView(resolvedDisplay, viewName)) + { + return viewName; + } + + // Use of the fallback requires a config-level opt-in, which is false by default. + if (!getImpl()->m_useDisplayViewAliases) + { + return ""; + } + + // The view's view_transform attribute may point to a ViewTransform or NamedTransform, + // both of which support alias names. Resolve both source and target to the canonical + // name for all comparisons. + auto resolveVTOrNT = [this](const std::string & name) -> std::string + { + if (name.empty()) + { + return std::string(); + } + // If there is both a VT and NT with that name, the ViewTransform takes priority. + ConstViewTransformRcPtr vt = getViewTransform(name.c_str()); + if (vt) + { + return std::string(vt->getName()); + } + ConstNamedTransformRcPtr nt = getNamedTransform(name.c_str()); + if (nt) + { + return std::string(nt->getName()); + } + return std::string(); + }; + + // Get the canonical name of a ViewTransform or NamedTransform responding to viewName. + const std::string transformName = resolveVTOrNT(viewName); + if (transformName.empty()) + { + // There are no ViewTransforms or NamedTransforms that respond to viewName as + // either a name or alias. No fallbacks are possible. + return ""; + } + + DisplayMap::const_iterator iter = FindDisplay(getImpl()->m_displays, resolvedDisplay); + if (iter == getImpl()->m_displays.end()) + { + // The requested display does not exist. + return ""; + } + + // Consider both display-defined views and shared views used by this display, and both + // active and inactive views. + const ViewPtrVec views = getImpl()->getViews(iter->second); + + // If there is a display color space corresponding to this display, get its pointer. As with + // Config::getCanonicalDisplayName's own alias-based resolution, a match found via one of the + // color space's aliases (rather than its own current name) is accepted. + ConstColorSpaceRcPtr resolvedDisplayCs = getColorSpace(resolvedDisplay); + + // There's nothing that prevents there from being a scene-referred color space that matches + // a display name. In that case, don't try to use this color space to validate the + // display_colorspace of the view candidates. + if (!resolvedDisplayCs || resolvedDisplayCs->getReferenceSpaceType() != REFERENCE_SPACE_DISPLAY) + { + resolvedDisplayCs = ConstColorSpaceRcPtr(); + } + + // Iterate over all views for this display, testing each candidate. + for (const auto * candidate : views) + { + // Does this candidate's view_transform (which may be a NT) resolve to the same one as + // viewName? If not, this candidate is unrelated to viewName and can be skipped outright. + const std::string resolvedCandidate = resolveVTOrNT(candidate->m_viewTransform); + if (resolvedCandidate.empty() || !StringUtils::Compare(resolvedCandidate, transformName)) + { + continue; + } + + // At this point, we've found a view in this display where its view_transform + // corresponds to viewName (either by name or alias). However, don't return it + // if it uses a display_colorspace that does not match the display color space + // corresponding to this display, if one exists. + + if (candidate->useDisplayNameForColorspace()) + { + // The candidate is using for its display_colorspace, so it + // goes with the display, by definition. + return candidate->m_name.c_str(); + } + + if (!resolvedDisplayCs) + { + // There is no display color space in the config corresponding to this display, + // so regardless of what the candidate's display_colorspace is, there is + // nothing to check it against. Accept the match. + return candidate->m_name.c_str(); + } + + // There is a display color space for this display, so only accept this candidate + // if its display_colorspace resolves to that same color space (by name or alias). + // Otherwise keep looking -- another candidate might still satisfy this check. + ConstColorSpaceRcPtr candidateCs = getColorSpace(candidate->m_colorspace.c_str()); + if (candidateCs && StringUtils::Compare(candidateCs->getName(), resolvedDisplayCs->getName())) + { + return candidate->m_name.c_str(); + } + } + + return ""; +} + +/////////////////////////////////////////////////////////////////////////// + bool Config::hasVirtualView(const char * viewName) const { const char * cs = getVirtualDisplayViewColorSpaceName(viewName); @@ -4140,6 +4439,8 @@ int Config::instantiateDisplayFromICCProfile(const char * ICCProfileFilepath) return getImpl()->instantiateDisplay("", monitorDescription, ICCProfileFilepath); } +/////////////////////////////////////////////////////////////////////////// + void Config::setActiveDisplays(const char * displays) { getImpl()->m_activeDisplays.clear(); @@ -4340,6 +4641,8 @@ int Config::getNumActiveViews() const return static_cast(getImpl()->m_activeViews.size()); } +/////////////////////////////////////////////////////////////////////////// + int Config::getNumDisplaysAll() const noexcept { return static_cast(getImpl()->m_displays.size()); @@ -4617,6 +4920,36 @@ void Config::addViewTransform(const ConstViewTransformRcPtr & viewTransform) const std::string namelower = StringUtils::Lower(name); + // The name and aliases must not collide with a different, existing view transform. + for (const auto & vt : getImpl()->m_viewTransforms) + { + if (StringUtils::Lower(vt->getName()) == namelower) + { + continue; + } + + if (vt->hasAlias(name.c_str())) + { + std::ostringstream os; + os << "Cannot add '" << name << "' view transform, existing view transform '"; + os << vt->getName() << "' is using this name as an alias."; + throw Exception(os.str().c_str()); + } + + const size_t numAliases = viewTransform->getNumAliases(); + for (size_t aidx = 0; aidx < numAliases; ++aidx) + { + const char * alias = viewTransform->getAlias(aidx); + if (StringUtils::Compare(vt->getName(), alias) || vt->hasAlias(alias)) + { + std::ostringstream os; + os << "Cannot add '" << name << "' view transform, it has an alias '" << alias; + os << "' that is already used by view transform '" << vt->getName() << "'."; + throw Exception(os.str().c_str()); + } + } + } + bool addIt = true; // If the view transform exists, replace it. @@ -5133,15 +5466,8 @@ ConstProcessorRcPtr Config::GetProcessorFromConfigs(const ConstContextRcPtr & sr "the source color space."); } - const char* csName = dstConfig->getDisplayViewColorSpaceName(dstDisplay, dstView); - const char* displayColorSpaceName = View::UseDisplayName(csName) ? dstDisplay : csName; - ConstColorSpaceRcPtr displayColorSpace = dstConfig->getColorSpace(displayColorSpaceName); - if (!displayColorSpace) - { - throw Exception("Can't create the processor for the destination config: " - "display color space not found."); - } - + // This creates a DisplayViewTransform and uses the standard BuildDisplayOps to build it, + // handle aliases, and do any necessary error handling. auto p2 = dstConfig->getProcessor(dstContext, dstInterchangeName, dstDisplay, dstView, direction); if (!p2) { @@ -5149,13 +5475,35 @@ ConstProcessorRcPtr Config::GetProcessorFromConfigs(const ConstContextRcPtr & sr "and the destination display view transform."); } + // Although we now have a valid processor, we still need to get the view's color space + // to check if it's actually a data space. This resolution process handles aliases and + // the case. + bool viewIsData = false; + const char * csName = dstConfig->getResolvedDisplayViewColorSpaceName(dstDisplay, dstView); + ConstColorSpaceRcPtr viewColorSpace = dstConfig->getColorSpace(csName); + if (!viewColorSpace) + { + // A view's color space could be a Named Transform. In this case, viewIsData + // should remain false. + ConstNamedTransformRcPtr nt = dstConfig->getNamedTransform(csName); + if (!nt) + { + // Given that the getProcessor call succeeded above, this should never happen. + throw Exception("Can't create the processor for the destination config."); + } + } + else + { + viewIsData = viewColorSpace->isData(); + } + ProcessorRcPtr processor = Processor::Create(); processor->getImpl()->setProcessorCacheFlags(srcConfig->getImpl()->m_cacheFlags); // If either of the color spaces are data spaces, its corresponding processor // will be empty, but need to make sure the entire result is also empty to // better match the semantics of how data spaces are handled. - if (!srcColorSpace->isData() && !displayColorSpace->isData()) + if (!srcColorSpace->isData() && !viewIsData) { if (direction == TRANSFORM_DIR_INVERSE) { @@ -5978,6 +6326,28 @@ void Config::Impl::checkVersionConsistency() const } } + if (hexVersion < 0x02060000) + { + for (const auto& vt : m_viewTransforms) + { + if (vt->getNumAliases() > 0) + { + std::ostringstream os; + os << "Config failed validation. The view transform '" << vt->getName() << "' "; + os << "has aliases and config version is less than 2.6."; + throw Exception(os.str().c_str()); + } + } + } + + // Check for use_display_view_aliases. + + if (hexVersion < 0x02060000 && m_useDisplayViewAliases) + { + throw Exception("Config failed validation: use_display_view_aliases is true and config " + "version is less than 2.6."); + } + // Check for new Look properties. if (hexVersion < 0x02050000) diff --git a/src/OpenColorIO/OCIOYaml.cpp b/src/OpenColorIO/OCIOYaml.cpp index 9010dcb606..8f2ad05e40 100644 --- a/src/OpenColorIO/OCIOYaml.cpp +++ b/src/OpenColorIO/OCIOYaml.cpp @@ -3822,6 +3822,15 @@ inline void load(const YAML::Node & node, ViewTransformRcPtr & vt) load(iter->second, stringval); vt->setName(stringval.c_str()); } + else if (key == "aliases") + { + StringUtils::StringVec aliases; + load(iter->second, aliases); + for (const auto & alias : aliases) + { + vt->addAlias(alias.c_str()); + } + } else if (key == "description") { std::string stringval; @@ -3884,6 +3893,18 @@ inline void save(YAML::Emitter & out, ConstViewTransformRcPtr & vt, unsigned int out << YAML::BeginMap; out << YAML::Key << "name" << YAML::Value << vt->getName(); + const size_t numAliases = vt->getNumAliases(); + if (numAliases) + { + out << YAML::Key << "aliases"; + StringUtils::StringVec aliases; + for (size_t aidx = 0; aidx < numAliases; ++aidx) + { + aliases.push_back(vt->getAlias(aidx)); + } + out << YAML::Flow << YAML::Value << aliases; + } + const char * family = vt->getFamily(); if (family && *family) { @@ -4754,6 +4775,11 @@ inline void load(const YAML::Node& node, ConfigRcPtr & config, const char* filen } } } + else if (key == "use_display_view_aliases") + { + load(iter->second, boolval); + config->setUseDisplayViewAliases(boolval); + } else if(key == "active_displays") { StringUtils::StringVec display; @@ -5272,6 +5298,12 @@ inline void save(YAML::Emitter & out, const Config & config) out << YAML::Newline; out << YAML::Newline; + + if (config.getUseDisplayViewAliases()) + { + out << YAML::Key << "use_display_view_aliases" << YAML::Value << true; + } + out << YAML::Key << "active_displays"; StringUtils::StringVec active_displays; int nDisplays = config.getNumActiveDisplays(); diff --git a/src/OpenColorIO/ViewTransform.cpp b/src/OpenColorIO/ViewTransform.cpp index c14d6021ac..c784ca8ebd 100644 --- a/src/OpenColorIO/ViewTransform.cpp +++ b/src/OpenColorIO/ViewTransform.cpp @@ -5,7 +5,9 @@ #include +#include "Platform.h" #include "TokensManager.h" +#include "utils/StringUtils.h" namespace { @@ -20,6 +22,7 @@ class ViewTransform::Impl { public: std::string m_name; + StringUtils::StringVec m_aliases; std::string m_family; std::string m_description; ReferenceSpaceType m_referenceSpaceType{ REFERENCE_SPACE_SCENE }; @@ -44,6 +47,7 @@ class ViewTransform::Impl if (this != &rhs) { m_name = rhs.m_name; + m_aliases = rhs.m_aliases; m_family = rhs.m_family; m_description = rhs.m_description; @@ -100,6 +104,63 @@ const char * ViewTransform::getName() const noexcept void ViewTransform::setName(const char * name) noexcept { getImpl()->m_name = name ? name : ""; + // Name can no longer be an alias. + StringUtils::Remove(getImpl()->m_aliases, getImpl()->m_name); +} + +size_t ViewTransform::getNumAliases() const noexcept +{ + return getImpl()->m_aliases.size(); +} + +const char * ViewTransform::getAlias(size_t idx) const noexcept +{ + if (idx < getImpl()->m_aliases.size()) + { + return getImpl()->m_aliases[idx].c_str(); + } + return ""; +} + +bool ViewTransform::hasAlias(const char * alias) const noexcept +{ + if (!alias) return false; + for (size_t idx = 0; idx < getImpl()->m_aliases.size(); ++idx) + { + if (0 == Platform::Strcasecmp(getImpl()->m_aliases[idx].c_str(), alias)) + { + return true; + } + } + return false; +} + +void ViewTransform::addAlias(const char * alias) noexcept +{ + if (alias && *alias) + { + if (!StringUtils::Compare(alias, getImpl()->m_name)) + { + if (!StringUtils::Contain(getImpl()->m_aliases, alias)) + { + getImpl()->m_aliases.push_back(alias); + } + } + } +} + +void ViewTransform::removeAlias(const char * name) noexcept +{ + if (name && *name) + { + const std::string alias{ name }; + StringUtils::Remove(getImpl()->m_aliases, alias); + } +} + +void ViewTransform::clearAliases() noexcept +{ + getImpl()->m_aliases.clear(); } const char * ViewTransform::getFamily() const noexcept @@ -267,6 +328,20 @@ std::ostream & operator<< (std::ostream & os, const ViewTransform & vt) { os << " 1) + { + os << "aliases=[" << vt.getAlias(0); + for (size_t aidx = 1; aidx < numAliases; ++aidx) + { + os << ", " << vt.getAlias(aidx); + } + os << "], "; + } os << "family=" << vt.getFamily() << ", "; os << "referenceSpaceType=" << ReferenceSpaceTypeToString(vt.getReferenceSpaceType()); const std::string desc{ vt.getDescription() }; diff --git a/src/OpenColorIO/apphelpers/LegacyViewingPipeline.cpp b/src/OpenColorIO/apphelpers/LegacyViewingPipeline.cpp index 71fe1941fd..89491f80f4 100644 --- a/src/OpenColorIO/apphelpers/LegacyViewingPipeline.cpp +++ b/src/OpenColorIO/apphelpers/LegacyViewingPipeline.cpp @@ -205,24 +205,37 @@ ConstProcessorRcPtr LegacyViewingPipelineImpl::getProcessor(const ConstConfigRcP throw Exception(os.str().c_str()); } - const std::string display = m_displayViewTransform->getDisplay(); - const std::string view = m_displayViewTransform->getView(); + // Resolve the display and view the same way BuildDisplayOps does, so that a display or view + // name that is aliased still finds the right color space and looks. + // + // Note this is not merely duplicating what the DisplayViewTransform appended below will do + // for itself. The looks lookup further down uses Config::getDisplayViewLooks, which reports + // what the config contains and so needs an up-to-date view name, and the color space found + // here decides whether color space conversions are skipped at all. The looks matter most: + // setDisplayViewTransform forces LooksBypass on the stored transform, so the looks are + // applied by this function or not at all. + std::string display = m_displayViewTransform->getDisplay(); + const char * canonicalDisplay = config->getCanonicalDisplayName(display.c_str()); + if (canonicalDisplay && *canonicalDisplay) + { + display = canonicalDisplay; + } - const std::string viewTransformName = config->getDisplayViewTransformName(display.c_str(), - view.c_str()); - ConstViewTransformRcPtr viewTransform; - if (!viewTransformName.empty()) + // Ensure the view name is resolved (needed for getDisplayViewLooks below). The + // DisplayViewTransform will do the same name resolution, so need to stay in sync. + std::string view = m_displayViewTransform->getView(); + const char * canonicalView = config->getCanonicalViewName(display.c_str(), view.c_str()); + if (canonicalView && *canonicalView) { - viewTransform = config->getViewTransform(viewTransformName.c_str()); + view = canonicalView; } - // NB: If the viewTransform is present, then displayColorSpace is a true display color space - // rather than a traditional color space. - const std::string name{ config->getDisplayViewColorSpaceName(display.c_str(), view.c_str()) }; - // A shared view containing a view transform may set the color space to USE_DISPLAY_NAME, - // in which case we look for a display color space with the same name as the display. - const bool nameFromDisplay = (0 == strcmp(name.c_str(), OCIO_VIEW_USE_DISPLAY_NAME)); - const std::string displayColorSpaceName{ nameFromDisplay ? display : name }; + // NB: If the view has a view transform, then displayColorSpace is a true display color space + // rather than a traditional color space. Note that this also handles a shared view that + // sets its display color space to USE_DISPLAY_NAME, by looking for a display color space + // with the same name as the display. + const std::string displayColorSpaceName{ + config->getResolvedDisplayViewColorSpaceName(display.c_str(), view.c_str()) }; ConstColorSpaceRcPtr displayColorSpace = config->getColorSpace(displayColorSpaceName.c_str()); // If this is not a color space it can be a named transform. Error handling (missing color // space or named transform) is handled by display view transform. diff --git a/src/OpenColorIO/transforms/DisplayViewTransform.cpp b/src/OpenColorIO/transforms/DisplayViewTransform.cpp index 629008cf3b..aa490d61c6 100644 --- a/src/OpenColorIO/transforms/DisplayViewTransform.cpp +++ b/src/OpenColorIO/transforms/DisplayViewTransform.cpp @@ -324,7 +324,17 @@ void BuildDisplayOps(OpRcPtrVec & ops, } const std::string display = displayViewTransform.getDisplay(); - if (config.getNumViews(display.c_str()) == 0) + + // Config authors may opt-in to using display color space names/aliases as aliases for + // display names. Resolve those to the actual display name. + std::string resolvedDisplay = display; + const char * canonicalDisplay = config.getCanonicalDisplayName(display.c_str()); + if (canonicalDisplay && *canonicalDisplay) + { + resolvedDisplay = canonicalDisplay; + } + + if (config.getNumViews(resolvedDisplay.c_str()) == 0) { std::ostringstream os; os << "DisplayViewTransform error."; @@ -333,13 +343,23 @@ void BuildDisplayOps(OpRcPtrVec & ops, } const std::string view = displayViewTransform.getView(); + // Config authors may opt-in to using view transform names/aliases as aliases for + // view names. Resolve those to the actual view name. + std::string resolvedView = view; + const char * canonicalView = config.getCanonicalViewName(resolvedDisplay.c_str(), view.c_str()); + if (canonicalView && *canonicalView) + { + resolvedView = canonicalView; + } + // Get the view transform if any: if it exists, it can be a view transform or a named transform. - const std::string viewTransformName = config.getDisplayViewTransformName(display.c_str(), - view.c_str()); + const std::string viewTransformName = config.getDisplayViewTransformName(resolvedDisplay.c_str(), + resolvedView.c_str()); ConstViewTransformRcPtr viewTransform; ConstNamedTransformRcPtr viewNamedTransform; if (!viewTransformName.empty()) { + // If there is both a VT and NT with that name, the ViewTransform takes priority. viewTransform = config.getViewTransform(viewTransformName.c_str()); if (!viewTransform) { @@ -355,19 +375,18 @@ void BuildDisplayOps(OpRcPtrVec & ops, } } - // Get the color space associated to the (display, view) pair. + // Get the color space associated to the (display, view) pair. This also takes care of the + // case of a shared view containing a view transform that sets the color space to + // , by looking for a display color space with the same name as the display. // (Returns an empty string if the view does not exist. This is trapped below.) - const char * csName = config.getDisplayViewColorSpaceName(display.c_str(), view.c_str()); - - // A shared view containing a view transform may set the color space to , - // in which case we look for a display color space with the same name as the display. - const std::string displayColorSpaceName = View::UseDisplayName(csName) ? display : csName; + const std::string displayColorSpaceName + = config.getResolvedDisplayViewColorSpaceName(resolvedDisplay.c_str(), + resolvedView.c_str()); // At this point, displayColorSpaceName is typically one of the following strings: // 1. The "colorspace" attribute of the View, if there is no "view_transform". - // 2. The name of the View's display, if it's a shared_view and the "display_colorspace" - // is "". It is expected this will also be the name of a display - // color space in the config. + // 2. The display color space named after the View's display, if it's a shared_view and the + // "display_colorspace" is "". // 3. Else, the "display_colorspace" string if it's a View with a "view_transform". // // (Though, in the implementation, both the "colorspace" and "display_colorspace" of @@ -427,7 +446,7 @@ void BuildDisplayOps(OpRcPtrVec & ops, LookParseResult looks; if (!displayViewTransform.getLooksBypass()) { - looks.parse(config.getDisplayViewLooks(display.c_str(), view.c_str())); + looks.parse(config.getDisplayViewLooks(resolvedDisplay.c_str(), resolvedView.c_str())); } // Now that all the inputs are found and validated, the following code builds the list of ops @@ -543,7 +562,26 @@ bool CollectContextVariables(const Config & config, foundContextVars = true; } - const char * csName = config.getDisplayViewColorSpaceName(tr.getDisplay(), tr.getView()); + // Resolve the display and view the same way BuildDisplayOps does, so that a + // DisplayViewTransform using an old display or view name still finds the right color space, + // view transform, and looks. + std::string display{ tr.getDisplay() }; + const char * canonicalDisplay = config.getCanonicalDisplayName(display.c_str()); + if (canonicalDisplay && *canonicalDisplay) + { + display = canonicalDisplay; + } + + std::string view{ tr.getView() }; + const char * canonicalView = config.getCanonicalViewName(display.c_str(), view.c_str()); + if (canonicalView && *canonicalView) + { + view = canonicalView; + } + + // Note that this also handles a shared view whose display color space is . + const char * csName = config.getResolvedDisplayViewColorSpaceName(display.c_str(), + view.c_str()); if (csName && *csName) { src = config.getColorSpace(csName); @@ -553,7 +591,7 @@ bool CollectContextVariables(const Config & config, } } - const char * vtName = config.getDisplayViewTransformName(tr.getDisplay(), tr.getView()); + const char * vtName = config.getDisplayViewTransformName(display.c_str(), view.c_str()); if (vtName && *vtName) { ConstViewTransformRcPtr vt = config.getViewTransform(vtName); @@ -576,7 +614,7 @@ bool CollectContextVariables(const Config & config, // TODO: The LooksBypass must be a DynamicProperty to allow live on/off. if (!tr.getLooksBypass()) { - const std::string looksStr = config.getDisplayViewLooks(tr.getDisplay(), tr.getView()); + const std::string looksStr = config.getDisplayViewLooks(display.c_str(), view.c_str()); LookParseResult looks; looks.parse(looksStr); diff --git a/src/apps/ocioconvert/main.cpp b/src/apps/ocioconvert/main.cpp index 92bf188acb..1d0fa6280f 100644 --- a/src/apps/ocioconvert/main.cpp +++ b/src/apps/ocioconvert/main.cpp @@ -654,7 +654,10 @@ int main(int argc, const char **argv) { if (useDisplayView) { - outputcolorspace = config->getDisplayViewColorSpaceName(display, view); + // Note that this resolves the (display, view) pair the same way the processor above + // did, and yields the name of an actual color space even for a shared view that uses + // . + outputcolorspace = config->getResolvedDisplayViewColorSpaceName(display, view); } if (outputcolorspace) diff --git a/src/bindings/python/PyConfig.cpp b/src/bindings/python/PyConfig.cpp index 6e7971844f..aa85845ad4 100644 --- a/src/bindings/python/PyConfig.cpp +++ b/src/bindings/python/PyConfig.cpp @@ -346,6 +346,10 @@ void bindPyConfig(py::module & m) DOC(Config, isStrictParsingEnabled)) .def("setStrictParsingEnabled", &Config::setStrictParsingEnabled, "enabled"_a, DOC(Config, setStrictParsingEnabled)) + .def("getUseDisplayViewAliases", &Config::getUseDisplayViewAliases, + DOC(Config, getUseDisplayViewAliases)) + .def("setUseDisplayViewAliases", &Config::setUseDisplayViewAliases, "enabled"_a, + DOC(Config, setUseDisplayViewAliases)) .def("setInactiveColorSpaces", &Config::setInactiveColorSpaces, "inactiveColorSpaces"_a, DOC(Config, setInactiveColorSpaces)) .def("getInactiveColorSpaces", &Config::getInactiveColorSpaces, @@ -432,6 +436,12 @@ void bindPyConfig(py::module & m) { return DisplayAllIterator(self); }) + .def("getCanonicalDisplayName", &Config::getCanonicalDisplayName, "display"_a, + DOC(Config, getCanonicalDisplayName)) + .def("getDisplayDescription", &Config::getDisplayDescription, "display"_a, + DOC(Config, getDisplayDescription)) + .def("getCanonicalViewName", &Config::getCanonicalViewName, "display"_a, "view"_a, + DOC(Config, getCanonicalViewName)) .def("getDefaultView", (const char * (Config::*)(const char *) const) &Config::getDefaultView, "display"_a, @@ -463,6 +473,9 @@ void bindPyConfig(py::module & m) .def("getDisplayViewColorSpaceName", &Config::getDisplayViewColorSpaceName, "display"_a, "view"_a, DOC(Config, getDisplayViewColorSpaceName)) + .def("getResolvedDisplayViewColorSpaceName", &Config::getResolvedDisplayViewColorSpaceName, + "display"_a, "view"_a, + DOC(Config, getResolvedDisplayViewColorSpaceName)) .def("getDisplayViewLooks", &Config::getDisplayViewLooks, "display"_a, "view"_a, DOC(Config, getDisplayViewLooks)) .def("getDisplayViewRule", &Config::getDisplayViewRule, "display"_a, "view"_a, diff --git a/src/bindings/python/PyViewTransform.cpp b/src/bindings/python/PyViewTransform.cpp index d3de4730e9..564a6df785 100644 --- a/src/bindings/python/PyViewTransform.cpp +++ b/src/bindings/python/PyViewTransform.cpp @@ -11,11 +11,13 @@ namespace enum ViewTransformIterator { - IT_VIEW_TRANSFORM_CATEGORY = 0 + IT_VIEW_TRANSFORM_CATEGORY = 0, + IT_VIEW_TRANSFORM_ALIAS }; using ViewTransformCategoryIterator = PyIterator; +using ViewTransformAliasIterator = PyIterator; std::vector getCategoriesStdVec(const ViewTransformRcPtr & p) { std::vector categories; @@ -27,6 +29,17 @@ std::vector getCategoriesStdVec(const ViewTransformRcPtr & p) { return categories; } +std::vector getAliasesStdVec(const ViewTransformRcPtr & p) +{ + std::vector aliases; + aliases.reserve(p->getNumAliases()); + for (size_t i = 0; i < p->getNumAliases(); i++) + { + aliases.push_back(p->getAlias(i)); + } + return aliases; +} + } // namespace void bindPyViewTransform(py::module & m) @@ -41,6 +54,10 @@ void bindPyViewTransform(py::module & m) py::class_( clsViewTransform, "ViewTransformCategoryIterator"); + auto clsViewTransformAliasIterator = + py::class_( + clsViewTransform, "ViewTransformAliasIterator"); + clsViewTransform .def(py::init([](ReferenceSpaceType referenceSpace) { @@ -54,9 +71,19 @@ void bindPyViewTransform(py::module & m) const std::string & description, const TransformRcPtr & toReference, const TransformRcPtr & fromReference, - const std::vector & categories) + const std::vector & categories, + const std::vector & aliases) { ViewTransformRcPtr p = ViewTransform::Create(referenceSpace); + if (!aliases.empty()) + { + p->clearAliases(); + for (size_t i = 0; i < aliases.size(); i++) + { + p->addAlias(aliases[i].c_str()); + } + } + // Setting the name will remove alias named the same, so set name after. if (!name.empty()) { p->setName(name.c_str()); } if (!family.empty()) { p->setFamily(family.c_str()); } if (!description.empty()) { p->setDescription(description.c_str()); } @@ -85,6 +112,7 @@ void bindPyViewTransform(py::module & m) "toReference"_a = DEFAULT->getTransform(VIEWTRANSFORM_DIR_TO_REFERENCE), "fromReference"_a = DEFAULT->getTransform(VIEWTRANSFORM_DIR_FROM_REFERENCE), "categories"_a = getCategoriesStdVec(DEFAULT), + "aliases"_a = getAliasesStdVec(DEFAULT), DOC(ViewTransform, Create)) .def("__deepcopy__", [](const ConstViewTransformRcPtr & self, py::dict) @@ -97,6 +125,21 @@ void bindPyViewTransform(py::module & m) DOC(ViewTransform, getName)) .def("setName", &ViewTransform::setName, "name"_a, DOC(ViewTransform, setName)) + + // Aliases. + .def("hasAlias", &ViewTransform::hasAlias, "alias"_a, + DOC(ViewTransform, hasAlias)) + .def("addAlias", &ViewTransform::addAlias, "alias"_a.none(false), + DOC(ViewTransform, addAlias)) + .def("removeAlias", &ViewTransform::removeAlias, "alias"_a.none(false), + DOC(ViewTransform, removeAlias)) + .def("getAliases", [](ViewTransformRcPtr & self) + { + return ViewTransformAliasIterator(self); + }) + .def("clearAliases", &ViewTransform::clearAliases, + DOC(ViewTransform, clearAliases)) + .def("getFamily", &ViewTransform::getFamily, DOC(ViewTransform, getFamily)) .def("setFamily", &ViewTransform::setFamily, "family"_a, @@ -151,6 +194,26 @@ void bindPyViewTransform(py::module & m) int i = it.nextIndex(it.m_obj->getNumCategories()); return it.m_obj->getCategory(i); }); + + clsViewTransformAliasIterator + .def("__len__", [](ViewTransformAliasIterator & it) + { + return it.m_obj->getNumAliases(); + }) + .def("__getitem__", [](ViewTransformAliasIterator & it, int i) + { + it.checkIndex(i, (int)it.m_obj->getNumAliases()); + return it.m_obj->getAlias(i); + }) + .def("__iter__", [](ViewTransformAliasIterator & it) -> ViewTransformAliasIterator & + { + return it; + }) + .def("__next__", [](ViewTransformAliasIterator & it) + { + int i = it.nextIndex((int)it.m_obj->getNumAliases()); + return it.m_obj->getAlias(i); + }); } } // namespace OCIO_NAMESPACE diff --git a/tests/cpu/Config_tests.cpp b/tests/cpu/Config_tests.cpp index 28d5e430b1..1722f7e7e8 100644 --- a/tests/cpu/Config_tests.cpp +++ b/tests/cpu/Config_tests.cpp @@ -2093,12 +2093,12 @@ OCIO_ADD_TEST(Config, version) { OCIO_CHECK_THROW_WHAT(config->setVersion(2, 9), OCIO::Exception, "The minor version 9 is not supported for major version 2. " - "Maximum minor version is 5"); + "Maximum minor version is 6"); OCIO_CHECK_NO_THROW(config->setMajorVersion(2)); OCIO_CHECK_THROW_WHAT(config->setMinorVersion(9), OCIO::Exception, "The minor version 9 is not supported for major version 2. " - "Maximum minor version is 5"); + "Maximum minor version is 6"); } { @@ -6578,7 +6578,9 @@ OCIO_ADD_TEST(Config, inactive_color_space_read_write) OCIO_ADD_TEST(Config, get_processor_from_two_configs) { constexpr const char * SIMPLE_CONFIG1{ R"( -ocio_profile_version: 2 +ocio_profile_version: 2.6 + +use_display_view_aliases: true environment: {} @@ -6587,18 +6589,28 @@ ocio_profile_version: 2 default: raw1 aces_interchange: aces1 cie_xyz_d65_interchange: display1 + color_timing: raw1 + compositing_log: raw1 + scene_linear: aces1 displays: displayname: - ! {name: view1, colorspace: displaytest1} - - ! {name: view2, view_transform: vt1, display_colorspace: display2} + - ! {name: view2, view_transform: vt1, display_colorspace: displayname} - ! {name: view3, colorspace: data_space} + - ! {name: view4, view_transform: vt2, display_colorspace: } view_transforms: - ! name: vt1 + aliases: [oldvt1] from_scene_reference: ! {min_in_value: 0., min_out_value: 0.} + - ! + name: vt2 + aliases: [oldvt2] + from_scene_reference: ! {offset: [0.02, 0.03, 0.04, 0]} + colorspaces: - ! name: raw1 @@ -6634,6 +6646,12 @@ ocio_profile_version: 2 allocation: uniform from_display_reference: ! {style: ACES_RedMod03} + - ! + name: displayname + aliases: [olddisplayname] + allocation: uniform + from_display_reference: ! {style: linear, exposure: 1.5} + )" }; constexpr const char * SIMPLE_CONFIG2{ R"( @@ -6832,10 +6850,74 @@ ocio_profile_version: 2 r3 = OCIO_DYNAMIC_POINTER_CAST(t3); OCIO_CHECK_ASSERT(r3); t4 = group->getTransform(4); - auto ff4 = OCIO_DYNAMIC_POINTER_CAST(t4); - OCIO_CHECK_ASSERT(ff4); + auto l4 = OCIO_DYNAMIC_POINTER_CAST(t4); + OCIO_CHECK_ASSERT(l4); + + // Test that aliases for display and view names work. + + // Basic test using aliased display and view names. + OCIO_CHECK_NO_THROW(p = OCIO::Config::GetProcessorFromConfigs( + config2, "test2", "aces2", config1, "olddisplayname", "oldvt1", "aces1", + OCIO::TRANSFORM_DIR_FORWARD)); + OCIO_REQUIRE_ASSERT(p); + group = p->createGroupTransform(); + OCIO_REQUIRE_EQUAL(group->getNumTransforms(), 5); + + // Test that the fallback works with just the single config getProcessor too. The result + // goes through "displayname" itself (a ECTransform), confirming the resolved view actually + // belongs with the resolved display. + OCIO_CHECK_NO_THROW(p = config1->getProcessor("aces1", "olddisplayname", "oldvt1", + OCIO::TRANSFORM_DIR_FORWARD)); + OCIO_REQUIRE_ASSERT(p); + group = p->createGroupTransform(); + OCIO_REQUIRE_EQUAL(group->getNumTransforms(), 3); + t0 = group->getTransform(0); + OCIO_CHECK_ASSERT(OCIO_DYNAMIC_POINTER_CAST(t0)); + t1 = group->getTransform(1); + OCIO_CHECK_ASSERT(OCIO_DYNAMIC_POINTER_CAST(t1)); + t2 = group->getTransform(2); + OCIO_CHECK_ASSERT(OCIO_DYNAMIC_POINTER_CAST(t2)); + + // Verify that substitution happens. + OCIO_CHECK_NO_THROW(p = config1->getProcessor("aces1", "displayname", "oldvt2", + OCIO::TRANSFORM_DIR_FORWARD)); + OCIO_REQUIRE_ASSERT(p); + group = p->createGroupTransform(); + OCIO_REQUIRE_EQUAL(group->getNumTransforms(), 3); + t0 = group->getTransform(0); + OCIO_CHECK_ASSERT(OCIO_DYNAMIC_POINTER_CAST(t0)); + t1 = group->getTransform(1); + OCIO_CHECK_ASSERT(OCIO_DYNAMIC_POINTER_CAST(t1)); + t2 = group->getTransform(2); + OCIO_CHECK_ASSERT(OCIO_DYNAMIC_POINTER_CAST(t2)); + + // Turn off display/view aliasing and ensure the result now throws. + { + OCIO::ConfigRcPtr config1NoFallback = config1->createEditableCopy(); + config1NoFallback->setUseDisplayViewAliases(false); + OCIO_CHECK_THROW_WHAT(OCIO::Config::GetProcessorFromConfigs( + config2, "test2", "aces2", config1NoFallback, "olddisplayname", "oldvt1", "aces1", + OCIO::TRANSFORM_DIR_FORWARD), + OCIO::Exception, + "DisplayViewTransform error. Display 'olddisplayname' not found."); + } + + // Modify view2 to point to a display color space that is different from displayname. + // This should throw since the view aliasing will not accept that as a viable match. + { + OCIO::ConfigRcPtr config1Mismatch = config1->createEditableCopy(); + OCIO_CHECK_NO_THROW(config1Mismatch->addDisplayView("displayname", "view2", "vt1", + "display2", "", "", "")); + OCIO_CHECK_THROW_WHAT(OCIO::Config::GetProcessorFromConfigs( + config2, "test2", "aces2", config1Mismatch, "olddisplayname", "oldvt1", "aces1", + OCIO::TRANSFORM_DIR_FORWARD), + OCIO::Exception, + "DisplayViewTransform error. The display 'olddisplayname' does not have view " + "'oldvt1'."); + } // If one of the spaces is a data space, the whole result must be a no-op. + OCIO_CHECK_NO_THROW(p = OCIO::Config::GetProcessorFromConfigs( config2, "test2", config1, "displayname", "view3", OCIO::TRANSFORM_DIR_FORWARD)); OCIO_REQUIRE_ASSERT(p); @@ -7131,6 +7213,103 @@ OCIO_ADD_TEST(Config, view_transforms) OCIO_CHECK_EQUAL(std::string("NotFirst"), configEdit->getDefaultViewTransformName()); } +OCIO_ADD_TEST(Config, view_transform_alias) +{ + OCIO::ConfigRcPtr config = OCIO::Config::CreateFromBuiltinConfig( + "cg-config-v2.2.0_aces-v1.3_ocio-v2.4")->createEditableCopy(); + // ViewTransform aliases require config version 2.6 or higher. + config->setVersion(2, 6); + // Aliases work even if display/view aliasing is off. + OCIO_REQUIRE_ASSERT(!config->getUseDisplayViewAliases()); + + auto sceneVT = OCIO::ViewTransform::Create(OCIO::REFERENCE_SPACE_SCENE); + sceneVT->setName("scene_vt"); + sceneVT->addAlias("old_scene_vt"); + OCIO_CHECK_NO_THROW(sceneVT->setTransform(OCIO::MatrixTransform::Create(), + OCIO::VIEWTRANSFORM_DIR_FROM_REFERENCE)); + OCIO_CHECK_NO_THROW(config->addViewTransform(sceneVT)); + + auto displayVT = OCIO::ViewTransform::Create(OCIO::REFERENCE_SPACE_DISPLAY); + displayVT->setName("display_vt"); + OCIO_CHECK_NO_THROW(displayVT->setTransform(OCIO::MatrixTransform::Create(), + OCIO::VIEWTRANSFORM_DIR_FROM_REFERENCE)); + OCIO_CHECK_NO_THROW(config->addViewTransform(displayVT)); + + OCIO_CHECK_NO_THROW(config->validate()); + + // Validate getViewTransform resolves the alias. + OCIO_CHECK_ASSERT(config->getViewTransform("old_scene_vt")); + OCIO_CHECK_EQUAL(std::string(config->getViewTransform("old_scene_vt")->getName()), "scene_vt"); + + // Check setDefaultViewTransformName also accepts an alias. + config->setDefaultViewTransformName("old_scene_vt"); + OCIO_CHECK_NO_THROW(config->validate()); + OCIO_REQUIRE_ASSERT(config->getDefaultSceneToDisplayViewTransform()); + OCIO_CHECK_EQUAL(std::string(config->getDefaultSceneToDisplayViewTransform()->getName()), + "scene_vt"); + config->setDefaultViewTransformName(""); + + // Validate addViewTransform is blocked if it would create any collisions. + + // An alias must not collide with another view transform's name. + auto conflict = OCIO::ViewTransform::Create(OCIO::REFERENCE_SPACE_SCENE); + conflict->setName("display_vt"); + conflict->addAlias("scene_vt"); + OCIO_CHECK_NO_THROW(conflict->setTransform(OCIO::MatrixTransform::Create(), + OCIO::VIEWTRANSFORM_DIR_FROM_REFERENCE)); + OCIO_CHECK_THROW_WHAT(config->addViewTransform(conflict), OCIO::Exception, + "it has an alias 'scene_vt' that is already used by view " + "transform 'scene_vt'"); + + conflict->setName("another_vt"); + OCIO_CHECK_THROW_WHAT(config->addViewTransform(conflict), OCIO::Exception, + "it has an alias 'scene_vt' that is already used by view " + "transform 'scene_vt'"); + + // An alias must not collide with another view transform's alias. + conflict->clearAliases(); + conflict->addAlias("old_scene_vt"); + OCIO_CHECK_THROW_WHAT(config->addViewTransform(conflict), OCIO::Exception, + "it has an alias 'old_scene_vt' that is already used by view " + "transform 'scene_vt'"); + + // A name must not collide with another view transform's alias. + conflict->clearAliases(); + conflict->setName("old_scene_vt"); + OCIO_CHECK_THROW_WHAT(config->addViewTransform(conflict), OCIO::Exception, + "existing view transform 'scene_vt' is using this name as an alias"); + + // Aliases require config version 2.6 or higher. + + config->setVersion(2, 5); + OCIO_CHECK_THROW_WHAT(config->validate(), OCIO::Exception, + "The view transform 'scene_vt' has aliases and config version is " + "less than 2.6"); + config->setVersion(2, 6); + OCIO_CHECK_NO_THROW(config->validate()); + + // Aliases survive a serialize / reload round-trip. + + std::ostringstream os; + config->serialize(os); + + std::istringstream is; + is.str(os.str()); + + OCIO::ConstConfigRcPtr configReloaded; + OCIO_CHECK_NO_THROW(configReloaded = OCIO::Config::CreateFromStream(is)); + OCIO_CHECK_NO_THROW(configReloaded->validate()); + + auto reloadedVT = configReloaded->getViewTransform("scene_vt"); + OCIO_REQUIRE_ASSERT(reloadedVT); + OCIO_REQUIRE_EQUAL(reloadedVT->getNumAliases(), 1); + OCIO_CHECK_EQUAL(std::string(reloadedVT->getAlias(0)), "old_scene_vt"); + + OCIO_CHECK_ASSERT(configReloaded->getViewTransform("old_scene_vt")); + OCIO_CHECK_EQUAL(std::string(configReloaded->getViewTransform("old_scene_vt")->getName()), + "scene_vt"); +} + OCIO_ADD_TEST(Config, display_view) { // Create a config with a display that has 2 kinds of views. @@ -7299,6 +7478,470 @@ default_view_transform: view_transform OCIO::Exception, "a non-empty color space name is needed"); } +OCIO_ADD_TEST(Config, aliased_view_name) +{ + constexpr const char * SIMPLE_CONFIG{ R"( +ocio_profile_version: 2.6 + +use_display_view_aliases: true + +environment: + {} +roles: + default: raw1 + aces_interchange: raw1 + cie_xyz_d65_interchange: display_cs + color_timing: raw1 + compositing_log: raw1 + scene_linear: raw1 + +view_transforms: + - ! + name: vt_new + aliases: [vt_old, vt_older] + from_scene_reference: ! {} + + - ! + name: vt_shared_new + aliases: [vt_shared_old] + from_scene_reference: ! {} + + - ! + name: vt_udn + aliases: [vt_udn_old] + from_scene_reference: ! {} + + - ! + name: vt_match + from_scene_reference: ! {} + +named_transforms: + - ! + name: nt_new + aliases: [nt_old, nt_older] + transform: ! {} + +colorspaces: + - ! + name: display_scene + + - ! + name: raw1 + +display_colorspaces: + - ! + name: display_cs + aliases: [old_display_name, display_alias_cs] + + - ! + name: dcs2 + aliases: [dcs_shared] + + - ! + name: dcs3 + +shared_views: + - ! {name: view_shared, view_transform: vt_shared_old, display_colorspace: dcs_shared} + - ! {name: view_udn, view_transform: vt_udn_old, display_colorspace: } + +displays: + display_cs: + - ! {name: view_act, colorspace: raw1} + - ! {name: view2, view_transform: vt_old, display_colorspace: dcs2} + - ! {name: view2b, view_transform: vt_old, display_colorspace: display_cs, + looks: lk1, rule: r1, description: foo} + - ! [view_shared, view_udn] + - ! {name: view_match_alias, view_transform: vt_match, + display_colorspace: display_alias_cs} + display_no_cs: + - ! {name: view_match, view_transform: vt_match, display_colorspace: display_cs} + - ! {name: view2, view_transform: vt_old, display_colorspace: dcs2} + - ! {name: view_nt, view_transform: nt_old, display_colorspace: dcs3} + - ! [view_shared] + display_scene: + - ! {name: view3, view_transform: vt_old, display_colorspace: dcs3} + +active_views: [view_act] +)" }; + + std::istringstream is; + is.str(SIMPLE_CONFIG); + OCIO::ConstConfigRcPtr config; + OCIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromStream(is)); + + // Most views are inactive, the tests confirm the fallback must still find it. + // Like Config::hasView, it must work regardless of whether the view is active. + OCIO_CHECK_EQUAL(config->getNumViews("display_cs"), 1); + OCIO_CHECK_ASSERT(config->hasView("display_cs", "view2b")); + + // The fallback is opt-in and disabled by default (exact matches still work). + { + OCIO::ConfigRcPtr configNoAliases = config->createEditableCopy(); + OCIO_CHECK_NO_THROW(configNoAliases->setUseDisplayViewAliases(false)); + + OCIO_CHECK_ASSERT(!configNoAliases->getUseDisplayViewAliases()); + OCIO_CHECK_EQUAL(std::string( + configNoAliases->getCanonicalViewName("display_cs", "vt_new")), ""); + OCIO_CHECK_NO_THROW(configNoAliases->setUseDisplayViewAliases(true)); + OCIO_CHECK_EQUAL(std::string( + configNoAliases->getCanonicalViewName("display_cs", "vt_new")), "view2b"); + } + OCIO_CHECK_ASSERT(config->getUseDisplayViewAliases()); + + // Normal case: "view_act" is already a view name. + OCIO_CHECK_EQUAL(std::string( + config->getCanonicalViewName("display_cs", "view_act")), "view_act"); + + // Test without a display color space corresponding to the display. + { + // The "vt_match" is not a view name, but it's a view_transform name. + OCIO_CHECK_EQUAL(std::string( + config->getCanonicalViewName("display_no_cs", "vt_match")), "view_match"); + + // The "view2" has "view_transform: vt_old", which is an alias for "vt_new". + OCIO_CHECK_EQUAL(std::string( + config->getCanonicalViewName("display_no_cs", "vt_new")), "view2"); + + // Test where both the requested view and the view's view_transform are aliases. + OCIO_CHECK_EQUAL(std::string( + config->getCanonicalViewName("display_no_cs", "vt_older")), "view2"); + + // Test where the view_transform is a NamedTransform. + OCIO_CHECK_EQUAL(std::string( + config->getCanonicalViewName("display_no_cs", "nt_new")), "view_nt"); + + // Test referring to an alias of a NamedTransform. + OCIO_CHECK_EQUAL(std::string( + config->getCanonicalViewName("display_no_cs", "nt_older")), "view_nt"); + + // Test where the view is a shared view. + OCIO_CHECK_EQUAL(std::string( + config->getCanonicalViewName("display_no_cs", "vt_shared_new")), "view_shared"); + } + + // Now test when there is a display color space that corresponds to the display. In + // this case, the function does additional checking to ensure that it does not return + // a view that has a display_colorspace that differs from the display's color space + + // Repeat the test from above and confirm that "view2" is no longer an option (even + // though it appears earlier in the config) because its display_colorspace is different + // from that of the display. + OCIO_CHECK_EQUAL(std::string( + config->getCanonicalViewName("display_cs", "vt_older")), "view2b"); + + // Now remove view2b and confirm that no match is found. + { + OCIO::ConfigRcPtr configNoView2b = config->createEditableCopy(); + OCIO_CHECK_NO_THROW(configNoView2b->removeDisplayView("display_cs", "view2b")); + + OCIO_CHECK_EQUAL(std::string( + configNoView2b->getCanonicalViewName("display_cs", "vt_older")), ""); + } + + // Verify that a display name alias is resolved as well. + OCIO_CHECK_EQUAL(std::string( + config->getCanonicalViewName("old_display_name", "vt_older")), "view2b"); + + // Test that the display_colorspace resolution succeeds, even if it's an alias. + OCIO_CHECK_EQUAL(std::string( + config->getCanonicalViewName("display_cs", "vt_match")), "view_match_alias"); + + // A candidate using as its display_colorspace is always accepted: by + // definition, its color space is whichever one is named after the display. + OCIO_CHECK_EQUAL(std::string( + config->getCanonicalViewName("display_cs", "vt_udn")), "view_udn"); + + // The display_scene is both a display name and scene-referred color space, which is something + // that could happen (e.g. "sRGB"). In this case, avoid the comparison against the view's + // display_colorspace. There should be no comparison of display_scene and dcs3. + OCIO_CHECK_EQUAL(std::string( + config->getCanonicalViewName("display_scene", "vt_new")), "view3"); + + // No match at all. + OCIO_CHECK_EQUAL(std::string(config->getCanonicalViewName("display_cs", "not a view")), ""); + + // Null/empty display or view. + OCIO_CHECK_EQUAL(std::string(config->getCanonicalViewName("display_cs", "")), ""); + OCIO_CHECK_EQUAL(std::string(config->getCanonicalViewName("display_cs", nullptr)), ""); + OCIO_CHECK_EQUAL(std::string(config->getCanonicalViewName("", "view1")), ""); + OCIO_CHECK_EQUAL(std::string(config->getCanonicalViewName(nullptr, "view1")), ""); + + // Unlike getCanonicalViewName, the other Config methods that take a view name are not meant + // to resolve it against a view transform alias. These functions are meant to tell exactly + // what the Config object contains, which would become less clear if they resolved aliases. + OCIO_CHECK_EQUAL(std::string(config->getCanonicalViewName("display_cs", "vt_old")), "view2b"); + OCIO_CHECK_ASSERT(config->hasView("display_cs", "view2")); + OCIO_CHECK_ASSERT(!config->hasView("display_cs", "vt_new")); + OCIO_CHECK_ASSERT(!config->hasView("display_cs", "vt_old")); + OCIO_CHECK_ASSERT(!config->hasView("display_cs", "vt_older")); + + OCIO_CHECK_EQUAL(std::string(config->getDisplayViewTransformName("display_cs", "vt_old")), ""); + OCIO_CHECK_EQUAL(std::string(config->getDisplayViewColorSpaceName("display_cs", "vt_old")), ""); + OCIO_CHECK_EQUAL(std::string(config->getDisplayViewLooks("display_cs", "vt_old")), ""); + OCIO_CHECK_EQUAL(std::string(config->getDisplayViewRule("display_cs", "vt_old")), ""); + OCIO_CHECK_EQUAL(std::string(config->getDisplayViewDescription("display_cs", "vt_old")), ""); + + OCIO::ConfigRcPtr configEdit = config->createEditableCopy(); + OCIO_CHECK_THROW_WHAT(configEdit->removeDisplayView("display_cs", "vt_old"), OCIO::Exception, + "Could not find a view named 'vt_old"); +} + +OCIO_ADD_TEST(Config, aliased_display_name) +{ + OCIO::ConfigRcPtr config = OCIO::Config::Create(); + config->setVersion(2, 6); + + auto raw = OCIO::ColorSpace::Create(); + raw->setName("raw"); + config->addColorSpace(raw); + + // The display color space associated with the "sRGB - Display" display keeps the display's + // old name, "sRGB", as an alias. + auto dcs = OCIO::ColorSpace::Create(OCIO::REFERENCE_SPACE_DISPLAY); + dcs->setName("sRGB - Display"); + dcs->addAlias("sRGB"); + config->addColorSpace(dcs); + + OCIO_CHECK_NO_THROW(config->addDisplayView("sRGB - Display", "view1", "raw", "")); + + // The fallback is opt-in and disabled by default. + OCIO_CHECK_ASSERT(!config->getUseDisplayViewAliases()); + OCIO_CHECK_EQUAL(std::string(config->getCanonicalDisplayName("sRGB")), ""); + + config->setUseDisplayViewAliases(true); + + // Exact, case-insensitive match. + OCIO_CHECK_EQUAL(std::string(config->getCanonicalDisplayName("sRGB - Display")), + "sRGB - Display"); + OCIO_CHECK_EQUAL(std::string(config->getCanonicalDisplayName("srgb - display")), + "sRGB - Display"); + + // Resolved via the display color space's alias. + OCIO_CHECK_EQUAL(std::string(config->getCanonicalDisplayName("sRGB")), "sRGB - Display"); + + // If there is a display named "sRGB" added, make sure it returns that one. + OCIO_CHECK_NO_THROW(config->addDisplayView("sRGB", "view1", "raw", "")); + OCIO_CHECK_EQUAL(std::string(config->getCanonicalDisplayName("sRGB")), "sRGB"); + OCIO_CHECK_NO_THROW(config->removeDisplayView("sRGB", "view1")); + OCIO_CHECK_EQUAL(std::string(config->getCanonicalDisplayName("sRGB")), "sRGB - Display"); + + // If the resolved color space's own name doesn't match any display, its aliases are tried + // too. Here "other_dcs" is a different display color space than the one associated with + // "sRGB - Display": its own name matches no display, but one of its aliases does. + OCIO_CHECK_NO_THROW(config->addDisplayView("AliasedDisplay", "view1", "raw", "")); + auto otherDcs = OCIO::ColorSpace::Create(OCIO::REFERENCE_SPACE_DISPLAY); + otherDcs->setName("other_dcs"); + otherDcs->addAlias("AliasedDisplay"); + otherDcs->addAlias("other_alias"); + OCIO_CHECK_NO_THROW(config->addColorSpace(otherDcs)); + OCIO_CHECK_EQUAL(std::string(config->getCanonicalDisplayName("other_dcs")), "AliasedDisplay"); + + // Similarly any alias of other_dcs will find the AliasedDisplay. + OCIO_CHECK_EQUAL(std::string(config->getCanonicalDisplayName("other_alias")), "AliasedDisplay"); + + // Don't resolve against role names. + OCIO_CHECK_NO_THROW(config->setRole("display_role", "sRGB - Display")); + OCIO_CHECK_EQUAL(std::string(config->getCanonicalDisplayName("display_role")), ""); + config->setRole("display_role", nullptr); + + // Replace the display color space with a scene-referred one of the same name, keeping the + // same "sRGB" alias. Since it is no longer display-referred, "sRGB" must no longer resolve + // to the display, even though the color space's canonical name still matches it exactly. + auto scs = OCIO::ColorSpace::Create(OCIO::REFERENCE_SPACE_SCENE); + scs->setName("sRGB - Display"); + scs->addAlias("sRGB"); + config->addColorSpace(scs); + + OCIO_CHECK_EQUAL(std::string(config->getCanonicalDisplayName("sRGB")), ""); + + // No match at all, and null/empty input. + OCIO_CHECK_EQUAL(std::string(config->getCanonicalDisplayName("does not exist")), ""); + OCIO_CHECK_EQUAL(std::string(config->getCanonicalDisplayName("")), ""); + OCIO_CHECK_EQUAL(std::string(config->getCanonicalDisplayName(nullptr)), ""); + + // Unlike getCanonicalDisplayName, the other Config methods that take a display name are not + // meant to resolve it via the display color space alias fallback. These functions are meant + // to tell exactly what the Config object contains, which would become less clear if they + // resolved aliases. + OCIO_CHECK_EQUAL(config->getNumViews("sRGB"), 0); + OCIO_CHECK_ASSERT(!config->hasView("sRGB", "view1")); + OCIO_CHECK_EQUAL(std::string(config->getDefaultView("sRGB")), ""); + OCIO_CHECK_EQUAL(std::string(config->getView("sRGB", 0)), ""); + OCIO_CHECK_EQUAL(std::string(config->getDisplayViewColorSpaceName("sRGB", "view1")), ""); + OCIO_CHECK_EQUAL(std::string(config->getDisplayViewTransformName("sRGB", "view1")), ""); + + OCIO_CHECK_THROW_WHAT(config->removeDisplayView("sRGB", "view1"), OCIO::Exception, + "Could not find a display named 'sRGB'"); +} + +OCIO_ADD_TEST(Config, display_description) +{ + OCIO::ConfigRcPtr config = OCIO::Config::Create(); + config->setVersion(2, 6); + + auto dcs = OCIO::ColorSpace::Create(OCIO::REFERENCE_SPACE_DISPLAY); + dcs->setName("sRGB - Display"); + dcs->addAlias("sRGB"); + dcs->setDescription("The sRGB display."); + config->addColorSpace(dcs); + + // Exact match works even though the fallback is disabled by default. + OCIO_CHECK_ASSERT(!config->getUseDisplayViewAliases()); + OCIO_CHECK_EQUAL(std::string(config->getDisplayDescription("sRGB - Display")), + "The sRGB display."); + + // Matching via the color space's alias requires the switch. + OCIO_CHECK_EQUAL(std::string(config->getDisplayDescription("sRGB")), ""); + config->setUseDisplayViewAliases(true); + OCIO_CHECK_EQUAL(std::string(config->getDisplayDescription("sRGB")), "The sRGB display."); + + // A color space that isn't display-referred doesn't count, even with a matching name. + auto scs = OCIO::ColorSpace::Create(OCIO::REFERENCE_SPACE_SCENE); + scs->setName("scene_cs"); + scs->setDescription("A scene color space."); + config->addColorSpace(scs); + OCIO_CHECK_EQUAL(std::string(config->getDisplayDescription("scene_cs")), ""); + + // No color space at all, and null/empty input. + OCIO_CHECK_EQUAL(std::string(config->getDisplayDescription("does not exist")), ""); + OCIO_CHECK_EQUAL(std::string(config->getDisplayDescription("")), ""); + OCIO_CHECK_EQUAL(std::string(config->getDisplayDescription(nullptr)), ""); +} + +OCIO_ADD_TEST(Config, resolved_display_view_color_space_name) +{ + OCIO::ConfigRcPtr config = OCIO::Config::Create(); + config->setVersion(2, 6); + + auto raw = OCIO::ColorSpace::Create(); + raw->setName("raw"); + config->addColorSpace(raw); + + // The display color space associated with the "sRGB - Display" display keeps the display's + // old name, "sRGB", as an alias. + auto dcs = OCIO::ColorSpace::Create(OCIO::REFERENCE_SPACE_DISPLAY); + dcs->setName("sRGB - Display"); + dcs->addAlias("sRGB"); + config->addColorSpace(dcs); + + auto vt = OCIO::ViewTransform::Create(OCIO::REFERENCE_SPACE_SCENE); + vt->setName("vt_new"); + vt->addAlias("vt_old"); + OCIO_CHECK_NO_THROW(vt->setTransform(OCIO::MatrixTransform::Create(), + OCIO::VIEWTRANSFORM_DIR_FROM_REFERENCE)); + OCIO_CHECK_NO_THROW(config->addViewTransform(vt)); + + // A plain view, with just a color space. + OCIO_CHECK_NO_THROW(config->addDisplayView("sRGB - Display", "view1", "raw", "")); + + // A view with a view transform, whose display color space is . It refers to + // the view transform by its old alias, so getCanonicalViewName can find this view from the + // view transform's current name. + OCIO_CHECK_NO_THROW(config->addDisplayView("sRGB - Display", "view2", "vt_old", + "", "", "", "")); + + // The plain case behaves like getDisplayViewColorSpaceName. + OCIO_CHECK_EQUAL(std::string(config->getResolvedDisplayViewColorSpaceName("sRGB - Display", + "view1")), "raw"); + + // is resolved to the display's own display color space, rather than being + // returned as the placeholder string. This does not require getUseDisplayViewAliases. + OCIO_CHECK_ASSERT(!config->getUseDisplayViewAliases()); + OCIO_CHECK_EQUAL(std::string(config->getDisplayViewColorSpaceName("sRGB - Display", "view2")), + ""); + OCIO_CHECK_EQUAL(std::string(config->getResolvedDisplayViewColorSpaceName("sRGB - Display", + "view2")), + "sRGB - Display"); + + // Resolving the display and the view is opt-in, like getCanonicalDisplayName and + // getCanonicalViewName themselves. + OCIO_CHECK_EQUAL(std::string(config->getResolvedDisplayViewColorSpaceName("sRGB", "view1")), + ""); + OCIO_CHECK_EQUAL(std::string(config->getResolvedDisplayViewColorSpaceName("sRGB - Display", + "vt_new")), ""); + + config->setUseDisplayViewAliases(true); + + // The display is resolved from the old name kept as an alias on its display color space. + OCIO_CHECK_EQUAL(std::string(config->getResolvedDisplayViewColorSpaceName("sRGB", "view1")), + "raw"); + + // The view is resolved from the view transform it uses, and the display may be out of date at + // the same time. + OCIO_CHECK_EQUAL(std::string(config->getResolvedDisplayViewColorSpaceName("sRGB - Display", + "vt_new")), + "sRGB - Display"); + OCIO_CHECK_EQUAL(std::string(config->getResolvedDisplayViewColorSpaceName("sRGB", "vt_new")), + "sRGB - Display"); + + // A view whose colorspace attribute is an alias of a color space returns the canonical name + // of that color space. + auto aliased = OCIO::ColorSpace::Create(); + aliased->setName("cs_new"); + aliased->addAlias("cs_old"); + config->addColorSpace(aliased); + OCIO_CHECK_NO_THROW(config->addDisplayView("sRGB - Display", "view3", "cs_old", "")); + OCIO_CHECK_EQUAL(std::string(config->getDisplayViewColorSpaceName("sRGB - Display", "view3")), + "cs_old"); + OCIO_CHECK_EQUAL(std::string(config->getResolvedDisplayViewColorSpaceName("sRGB - Display", + "view3")), "cs_new"); + + // Likewise for a role. + OCIO_CHECK_NO_THROW(config->setRole("cs_role", "cs_new")); + OCIO_CHECK_NO_THROW(config->addDisplayView("sRGB - Display", "view4", "cs_role", "")); + OCIO_CHECK_EQUAL(std::string(config->getResolvedDisplayViewColorSpaceName("sRGB - Display", + "view4")), "cs_new"); + + // A view with no view transform may use a named transform in place of a color space, and + // named transforms have aliases too. + auto nt = OCIO::NamedTransform::Create(); + nt->setName("nt_new"); + nt->addAlias("nt_old"); + nt->setTransform(OCIO::MatrixTransform::Create(), OCIO::TRANSFORM_DIR_FORWARD); + OCIO_CHECK_NO_THROW(config->addNamedTransform(nt)); + OCIO_CHECK_NO_THROW(config->addDisplayView("sRGB - Display", "view5", "nt_old", "")); + OCIO_CHECK_EQUAL(std::string(config->getResolvedDisplayViewColorSpaceName("sRGB - Display", + "view5")), "nt_new"); + + // A shared view attached to the display is found too, and its resolves to + // the display's own color space. + OCIO_CHECK_NO_THROW(config->addSharedView("shared1", "vt_new", "", + "", "", "")); + OCIO_CHECK_NO_THROW(config->addDisplaySharedView("sRGB - Display", "shared1")); + OCIO_CHECK_EQUAL(std::string(config->getResolvedDisplayViewColorSpaceName("sRGB - Display", + "shared1")), + "sRGB - Display"); + OCIO_CHECK_EQUAL(std::string(config->getResolvedDisplayViewColorSpaceName("sRGB", "shared1")), + "sRGB - Display"); + + // If the color space named after the display doesn't exist, the display name is returned + // as-is, so that callers can report which color space they were unable to find. + OCIO_CHECK_NO_THROW(config->addDisplayView("no_cs_display", "view1", "vt_new", + "", "", "", "")); + OCIO_CHECK_EQUAL(std::string(config->getResolvedDisplayViewColorSpaceName("no_cs_display", + "view1")), + "no_cs_display"); + + // Nonexistent display or view, and null/empty input. + OCIO_CHECK_EQUAL(std::string(config->getResolvedDisplayViewColorSpaceName("sRGB - Display", + "not a view")), ""); + OCIO_CHECK_EQUAL(std::string(config->getResolvedDisplayViewColorSpaceName("not a display", + "view1")), ""); + OCIO_CHECK_EQUAL(std::string(config->getResolvedDisplayViewColorSpaceName("sRGB - Display", + "")), ""); + OCIO_CHECK_EQUAL(std::string(config->getResolvedDisplayViewColorSpaceName("sRGB - Display", + nullptr)), ""); + + // Unlike getDisplayViewColorSpaceName, an empty display does not mean "look at the + // config-level shared views", since a shared view's color space is only meaningful in the + // context of a display. + OCIO_CHECK_EQUAL(std::string(config->getDisplayViewColorSpaceName("", "shared1")), + ""); + OCIO_CHECK_EQUAL(std::string(config->getResolvedDisplayViewColorSpaceName("", "shared1")), ""); + OCIO_CHECK_EQUAL(std::string(config->getResolvedDisplayViewColorSpaceName(nullptr, + "shared1")), ""); +} + OCIO_ADD_TEST(Config, not_case_sensitive) { // Validate that the color spaces and roles are case insensitive. @@ -7740,6 +8383,137 @@ OCIO_ADD_TEST(Config, is_colorspace_used) OCIO_CHECK_ASSERT(!config->isColorSpaceUsed("cs65")); // Unknown color spaces are not used. } +OCIO_ADD_TEST(Config, is_colorspace_used_aliases) +{ + // Config::isColorSpaceUsed must resolve both the name it is given and each of the names it finds + // in the config down to a canonical name before comparing them, since either side may be an + // alias or a role rather than the color space's name. + + constexpr char CONFIG[]{ R"( +ocio_profile_version: 2.6 + +roles: + aces_interchange: raw + cie_xyz_d65_interchange: cie_xyz_d65 + color_timing: raw + compositing_log: raw + default: default_old + scene_linear: raw + some_role: role_old + +file_rules: + - ! {name: rule1, colorspace: fr_old, pattern: "*", extension: "*"} + - ! {name: Default, colorspace: default} + +shared_views: + - ! {name: shared1, view_transform: vt1, display_colorspace: } + +displays: + disp1: + - ! {name: view1, colorspace: view_old} + AliasedDisplay: + - ! [shared1] + +view_transforms: + - ! + name: vt1 + from_scene_reference: ! {offset: [0.1, 0.1, 0.1, 0]} + +display_colorspaces: + - ! + name: other_dcs + aliases: [AliasedDisplay] + to_display_reference: ! {offset: [0.25, 0.15, 0.35, 0]} + + - ! + name: cie_xyz_d65 + +colorspaces: + - ! + name: raw + isdata: true + + - ! + name: default_new + aliases: [default_old] + + - ! + name: view_new + aliases: [view_old] + + - ! + name: role_new + aliases: [role_old] + + - ! + name: fr_new + aliases: [fr_old] + + - ! + name: unused_new + aliases: [unused_old] +)" }; + + std::istringstream iss; + iss.str(CONFIG); + + OCIO::ConstConfigRcPtr config; + OCIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromStream(iss)); + OCIO_CHECK_NO_THROW(config->validate()); + + // The display-defined view "view1" refers to "view_new" by its alias. Nothing else in the + // config mentions this color space, so the view is the only thing that can make it used. + OCIO_CHECK_ASSERT(config->isColorSpaceUsed("view_new")); + OCIO_CHECK_ASSERT(config->isColorSpaceUsed("view_old")); + + OCIO_CHECK_ASSERT(config->isColorSpaceUsed("other_dcs")); + OCIO_CHECK_ASSERT(config->isColorSpaceUsed("AliasedDisplay")); + + OCIO_CHECK_ASSERT(config->isColorSpaceUsed("role_new")); + OCIO_CHECK_ASSERT(config->isColorSpaceUsed("role_old")); + OCIO_CHECK_ASSERT(config->isColorSpaceUsed("some_role")); + + OCIO_CHECK_ASSERT(config->isColorSpaceUsed("fr_new")); + OCIO_CHECK_ASSERT(config->isColorSpaceUsed("fr_old")); + + // The Default rule's color space is written as the "default" role, which in turn refers to + // "default_new" by its alias, so resolving the rule takes two steps. + // + // Note that this path is not independently observable: the roles are checked separately, and + // any color space reachable through the Default rule's role is necessarily reachable through + // the role itself. It is the "rule1" case above that exercises the file rule comparison on + // its own. + OCIO_CHECK_ASSERT(config->isColorSpaceUsed("default_new")); + OCIO_CHECK_ASSERT(config->isColorSpaceUsed("default_old")); + + // A color space that really isn't used anywhere is still reported as unused, whether it is + // named by its canonical name or by an alias. + OCIO_CHECK_ASSERT(!config->isColorSpaceUsed("unused_new")); + OCIO_CHECK_ASSERT(!config->isColorSpaceUsed("unused_old")); + + // None of the above depends on Config::getUseDisplayViewAliases, which was off. + // Turning it on must not change any result. + OCIO::ConfigRcPtr editableConfig = config->createEditableCopy(); + OCIO_CHECK_ASSERT(!editableConfig->getUseDisplayViewAliases()); + OCIO_CHECK_NO_THROW(editableConfig->setUseDisplayViewAliases(true)); + OCIO_CHECK_NO_THROW(editableConfig->validate()); + + OCIO_CHECK_ASSERT(editableConfig->isColorSpaceUsed("view_new")); + OCIO_CHECK_ASSERT(editableConfig->isColorSpaceUsed("view_old")); + OCIO_CHECK_ASSERT(editableConfig->isColorSpaceUsed("role_new")); + OCIO_CHECK_ASSERT(editableConfig->isColorSpaceUsed("role_old")); + OCIO_CHECK_ASSERT(editableConfig->isColorSpaceUsed("some_role")); + OCIO_CHECK_ASSERT(editableConfig->isColorSpaceUsed("fr_new")); + OCIO_CHECK_ASSERT(editableConfig->isColorSpaceUsed("fr_old")); + OCIO_CHECK_ASSERT(editableConfig->isColorSpaceUsed("default_new")); + OCIO_CHECK_ASSERT(editableConfig->isColorSpaceUsed("default_old")); + OCIO_CHECK_ASSERT(editableConfig->isColorSpaceUsed("other_dcs")); + OCIO_CHECK_ASSERT(editableConfig->isColorSpaceUsed("AliasedDisplay")); + + OCIO_CHECK_ASSERT(!editableConfig->isColorSpaceUsed("unused_new")); + OCIO_CHECK_ASSERT(!editableConfig->isColorSpaceUsed("unused_old")); +} + OCIO_ADD_TEST(Config, transform_versions) { // Saving a v1 config containing v2 transforms must fail. diff --git a/tests/cpu/ViewTransform_tests.cpp b/tests/cpu/ViewTransform_tests.cpp index 0055d3ecca..c61f3ce4b4 100644 --- a/tests/cpu/ViewTransform_tests.cpp +++ b/tests/cpu/ViewTransform_tests.cpp @@ -68,3 +68,74 @@ OCIO_ADD_TEST(ViewTransform, basic) OCIO_REQUIRE_ASSERT(vtd); OCIO_CHECK_EQUAL(OCIO::REFERENCE_SPACE_DISPLAY, vtd->getReferenceSpaceType()); } + +OCIO_ADD_TEST(ViewTransform, aliases) +{ + OCIO::ViewTransformRcPtr vt = OCIO::ViewTransform::Create(OCIO::REFERENCE_SPACE_SCENE); + OCIO_REQUIRE_ASSERT(vt); + OCIO_CHECK_EQUAL(vt->getNumAliases(), 0); + constexpr char AliasA[]{ "aliasA" }; + constexpr char AliasAAlt[]{ "aLiaSa" }; + constexpr char AliasB[]{ "aliasB" }; + vt->addAlias(AliasA); + OCIO_CHECK_EQUAL(vt->getNumAliases(), 1); + OCIO_CHECK_ASSERT(vt->hasAlias(AliasA)); + OCIO_CHECK_ASSERT(vt->hasAlias(AliasAAlt)); + OCIO_CHECK_ASSERT(!vt->hasAlias(AliasB)); + vt->addAlias(AliasB); + OCIO_CHECK_EQUAL(vt->getNumAliases(), 2); + OCIO_CHECK_EQUAL(std::string(vt->getAlias(0)), AliasA); + OCIO_CHECK_EQUAL(std::string(vt->getAlias(1)), AliasB); + OCIO_CHECK_ASSERT(vt->hasAlias(AliasB)); + + // Alias with same name (different case) already exists, do nothing. + + vt->addAlias(AliasAAlt); + OCIO_CHECK_EQUAL(vt->getNumAliases(), 2); + OCIO_CHECK_EQUAL(std::string(vt->getAlias(0)), AliasA); + OCIO_CHECK_EQUAL(std::string(vt->getAlias(1)), AliasB); + + // Remove alias. + + vt->removeAlias(AliasAAlt); + OCIO_CHECK_EQUAL(vt->getNumAliases(), 1); + OCIO_CHECK_EQUAL(std::string(vt->getAlias(0)), AliasB); + OCIO_CHECK_ASSERT(!vt->hasAlias(AliasA)); + OCIO_CHECK_ASSERT(!vt->hasAlias(AliasAAlt)); + + // Add with new case. + + vt->addAlias(AliasAAlt); + OCIO_CHECK_EQUAL(vt->getNumAliases(), 2); + OCIO_CHECK_EQUAL(std::string(vt->getAlias(0)), AliasB); + OCIO_CHECK_EQUAL(std::string(vt->getAlias(1)), AliasAAlt); + OCIO_CHECK_ASSERT(vt->hasAlias(AliasA)); + OCIO_CHECK_ASSERT(vt->hasAlias(AliasAAlt)); + + // Setting the name of the view transform to one of its aliases removes the alias. + + vt->setName(AliasA); + OCIO_CHECK_EQUAL(std::string(vt->getName()), AliasA); + OCIO_CHECK_EQUAL(vt->getNumAliases(), 1); + OCIO_CHECK_EQUAL(std::string(vt->getAlias(0)), AliasB); + OCIO_CHECK_ASSERT(!vt->hasAlias(AliasA)); + OCIO_CHECK_ASSERT(!vt->hasAlias(AliasAAlt)); + + // Alias is not added if it is already the view transform name. + + vt->addAlias(AliasAAlt); + OCIO_CHECK_EQUAL(std::string(vt->getName()), AliasA); + OCIO_CHECK_EQUAL(vt->getNumAliases(), 1); + OCIO_CHECK_EQUAL(std::string(vt->getAlias(0)), AliasB); + OCIO_CHECK_ASSERT(!vt->hasAlias(AliasAAlt)); + + // Remove all aliases. + + vt->addAlias("other"); + OCIO_CHECK_EQUAL(vt->getNumAliases(), 2); + OCIO_CHECK_ASSERT(vt->hasAlias("other")); + vt->clearAliases(); + OCIO_CHECK_EQUAL(vt->getNumAliases(), 0); + OCIO_CHECK_ASSERT(!vt->hasAlias(AliasB)); + OCIO_CHECK_ASSERT(!vt->hasAlias("other")); +} diff --git a/tests/cpu/apphelpers/LegacyViewingPipeline_tests.cpp b/tests/cpu/apphelpers/LegacyViewingPipeline_tests.cpp index 1c41d9dd9c..e0bd834a8a 100644 --- a/tests/cpu/apphelpers/LegacyViewingPipeline_tests.cpp +++ b/tests/cpu/apphelpers/LegacyViewingPipeline_tests.cpp @@ -953,3 +953,135 @@ OCIO_ADD_TEST(LegacyViewingPipeline, processorWithNoOpLook) OCIO_REQUIRE_ASSERT(groupTransform); OCIO_CHECK_NO_THROW(groupTransform->validate()); } + +OCIO_ADD_TEST(LegacyViewingPipeline, display_view_alias_fallback) +{ + // A LegacyViewingPipeline resolves the display and view of its DisplayViewTransform, so that + // names that are out of date still produce the same pipeline. + // + // Note that this is not simply duplicating what the DisplayViewTransform does for itself when + // it is appended to the group. It matters most for the looks: setDisplayViewTransform forces + // LooksBypass on the stored transform, so the view's looks are applied by the pipeline itself + // rather than by BuildDisplayOps. If the view name did not resolve here, the looks lookup + // would come back empty and the looks would be silently dropped. + + constexpr char CONFIG[]{ R"( +ocio_profile_version: 2.6 + +use_display_view_aliases: true + +roles: + default: raw + aces_interchange: raw + cie_xyz_d65_interchange: sRGB - Display + color_timing: raw + compositing_log: raw + scene_linear: source + +displays: + sRGB - Display: + - ! {name: view, view_transform: display_vt, display_colorspace: sRGB - Display, looks: look1} + +looks: + - ! + name: look1 + process_space: source + transform: ! {slope: [1.1, 1.2, 1.3]} + +view_transforms: + - ! + name: display_vt + aliases: [old_vt] + to_scene_reference: ! {offset: [0.3, 0.1, 0.1, 0]} + +display_colorspaces: + - ! + name: sRGB - Display + aliases: [sRGB] + to_display_reference: ! {offset: [0.25, 0.15, 0.35, 0]} + +colorspaces: + - ! + name: raw + isdata: true + + - ! + name: source + to_scene_reference: ! {offset: [0, 0.1, 0.2, 0]} +)" }; + + std::istringstream is(CONFIG); + + OCIO::ConstConfigRcPtr cfg; + OCIO_CHECK_NO_THROW(cfg = OCIO::Config::CreateFromStream(is)); + OCIO_CHECK_NO_THROW(cfg->validate()); + + // Build the pipeline for a (display, view) pair and run one pixel through it. Comparing the + // resulting values catches a dropped look, which a structural check on the transform list + // could easily miss. + auto applyPipeline = [&cfg](const char * display, const char * view, bool bypassLooks, + float * rgb) + { + OCIO::DisplayViewTransformRcPtr dt = OCIO::DisplayViewTransform::Create(); + dt->setSrc("source"); + dt->setDisplay(display); + dt->setView(view); + dt->setLooksBypass(bypassLooks); + + OCIO::LegacyViewingPipelineRcPtr vp = OCIO::LegacyViewingPipeline::Create(); + vp->setDisplayViewTransform(dt); + + OCIO::ConstProcessorRcPtr proc = vp->getProcessor(cfg, cfg->getCurrentContext()); + proc->getDefaultCPUProcessor()->applyRGB(rgb); + }; + + constexpr float srcPixel[3]{ 0.3f, 0.5f, 0.7f }; + constexpr float tolerance = 1e-6f; + + // The reference result, using the display's and the view's current names. + float ref[3]{ srcPixel[0], srcPixel[1], srcPixel[2] }; + OCIO_CHECK_NO_THROW(applyPipeline("sRGB - Display", "view", false, ref)); + + // Confirm the look actually changes the result, so that the comparisons below can't pass + // merely because the look happens to be a no-op. Setting LooksBypass before handing the + // transform to the pipeline is what makes it skip the looks (see m_dtOriginalLooksBypass). + { + float noLook[3]{ srcPixel[0], srcPixel[1], srcPixel[2] }; + OCIO_CHECK_NO_THROW(applyPipeline("sRGB - Display", "view", true, noLook)); + + OCIO_CHECK_ASSERT(std::abs(noLook[0] - ref[0]) > 1e-4f || + std::abs(noLook[1] - ref[1]) > 1e-4f || + std::abs(noLook[2] - ref[2]) > 1e-4f); + } + + // The display's old, alias-only name must give the same result. + { + float aliasedDisplay[3]{ srcPixel[0], srcPixel[1], srcPixel[2] }; + OCIO_CHECK_NO_THROW(applyPipeline("sRGB", "view", false, aliasedDisplay)); + + OCIO_CHECK_CLOSE(aliasedDisplay[0], ref[0], tolerance); + OCIO_CHECK_CLOSE(aliasedDisplay[1], ref[1], tolerance); + OCIO_CHECK_CLOSE(aliasedDisplay[2], ref[2], tolerance); + } + + // So must the view transform's old, alias-only name used as the view. This is the case that + // would silently lose the look if the view were not resolved. + { + float aliasedView[3]{ srcPixel[0], srcPixel[1], srcPixel[2] }; + OCIO_CHECK_NO_THROW(applyPipeline("sRGB - Display", "old_vt", false, aliasedView)); + + OCIO_CHECK_CLOSE(aliasedView[0], ref[0], tolerance); + OCIO_CHECK_CLOSE(aliasedView[1], ref[1], tolerance); + OCIO_CHECK_CLOSE(aliasedView[2], ref[2], tolerance); + } + + // And both being out of date at once, since the view is resolved against the resolved display. + { + float bothAliased[3]{ srcPixel[0], srcPixel[1], srcPixel[2] }; + OCIO_CHECK_NO_THROW(applyPipeline("sRGB", "old_vt", false, bothAliased)); + + OCIO_CHECK_CLOSE(bothAliased[0], ref[0], tolerance); + OCIO_CHECK_CLOSE(bothAliased[1], ref[1], tolerance); + OCIO_CHECK_CLOSE(bothAliased[2], ref[2], tolerance); + } +} diff --git a/tests/cpu/transforms/DisplayViewTransform_tests.cpp b/tests/cpu/transforms/DisplayViewTransform_tests.cpp index 082ea3ee0e..66cd32e952 100644 --- a/tests/cpu/transforms/DisplayViewTransform_tests.cpp +++ b/tests/cpu/transforms/DisplayViewTransform_tests.cpp @@ -1491,3 +1491,376 @@ environment: { FILE: cdl_test1.cc } dt->setView("View18"); OCIO_CHECK_ASSERT(!CollectContextVariables(*cfg, *cfg->getCurrentContext(), *dt, usedContextVars)); } + +OCIO_ADD_TEST(DisplayViewTransform, use_display_name_alias) +{ + // Test that USE_DISPLAY_NAME will find a display color space where the display name + // is an alias. + + constexpr const char * SIMPLE_CONFIG{ R"( +ocio_profile_version: 2 + +roles: + default: raw + +shared_views: + - ! {name: view1, view_transform: display_vt, display_colorspace: } + +displays: + sRGB - Display: + - ! [view1] + sRGB: + - ! [view1] + +view_transforms: + - ! + name: display_vt + to_scene_reference: ! {offset: [0.3, 0.1, 0.1, 0]} + +display_colorspaces: + - ! + name: sRGB - Display + aliases: [sRGB] + to_display_reference: ! {offset: [0.25, 0.15, 0.35, 0]} + +colorspaces: + - ! + name: raw + isdata: true + + - ! + name: source + to_scene_reference: ! {offset: [0.11, 0.12, 0.13, 0]} +)" }; + + std::istringstream is; + is.str(SIMPLE_CONFIG); + OCIO::ConstConfigRcPtr config; + OCIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromStream(is)); + OCIO_CHECK_NO_THROW(config->validate()); + + const std::string display{ "sRGB - Display" }; + const std::string aliasDisplay{ "sRGB" }; + const std::string view{ "view1" }; + + auto dt = OCIO::DisplayViewTransform::Create(); + dt->setSrc("source"); + dt->setView(view.c_str()); + + // Build once with the display name matching the display color space name. + dt->setDisplay(display.c_str()); + OCIO::OpRcPtrVec currentOps; + OCIO_CHECK_NO_THROW(OCIO::BuildDisplayOps(currentOps, *config, config->getCurrentContext(), + *dt, OCIO::TRANSFORM_DIR_FORWARD)); + OCIO_CHECK_NO_THROW(currentOps.validate()); + OCIO_REQUIRE_EQUAL(currentOps.size(), 5); // (includes gpu allocation no-ops) + + // Build again with the display name matching an alias. The result must be identical. + dt->setDisplay(aliasDisplay.c_str()); + OCIO::OpRcPtrVec aliasedOps; + OCIO_CHECK_NO_THROW(OCIO::BuildDisplayOps(aliasedOps, *config, config->getCurrentContext(), + *dt, OCIO::TRANSFORM_DIR_FORWARD)); + OCIO_CHECK_NO_THROW(aliasedOps.validate()); + OCIO_REQUIRE_EQUAL(aliasedOps.size(), currentOps.size()); + + for (size_t i = 0; i < aliasedOps.size(); ++i) + { + auto opA = OCIO_DYNAMIC_POINTER_CAST(aliasedOps[i]); + auto opB = OCIO_DYNAMIC_POINTER_CAST(currentOps[i]); + OCIO_CHECK_ASSERT(*opA->data() == *opB->data()); + } +} + +OCIO_ADD_TEST(DisplayViewTransform, display_alias_fallback) +{ + // Validate that BuildDisplayOps resolves a display using Config::getCanonicalDisplayName, + // so that renaming a display in a config (while keeping the old name as an alias on the + // associated display color space) doesn't break a DisplayViewTransform still using the old + // name. + + constexpr char CONFIG[]{ R"( +ocio_profile_version: 2.6 + +# Opt-in to display aliasing. +use_display_view_aliases: true + +roles: + default: raw + aces_interchange: raw + cie_xyz_d65_interchange: sRGB - Display + color_timing: raw + compositing_log: raw + scene_linear: raw + +shared_views: + - ! {name: view, view_transform: display_vt, display_colorspace: } + +displays: + sRGB - Display: + - ! [view] + +view_transforms: + - ! + name: display_vt + to_scene_reference: ! {offset: [0.3, 0.1, 0.1, 0]} + +display_colorspaces: + - ! + name: sRGB - Display + aliases: [sRGB] + to_display_reference: ! {offset: [0.25, 0.15, 0.35, 0]} + +colorspaces: + - ! + name: raw + isdata: true + + - ! + name: source + to_scene_reference: ! {offset: [0, 0.1, 0.2, 0]} +)" }; + + std::istringstream is; + is.str(CONFIG); + + OCIO::ConstConfigRcPtr config; + OCIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromStream(is)); + OCIO_CHECK_NO_THROW(config->validate()); + + const std::string display{ "sRGB - Display" }; + const std::string oldDisplayName{ "sRGB" }; + const std::string view{ "view" }; + + OCIO_CHECK_EQUAL(std::string(config->getCanonicalDisplayName(oldDisplayName.c_str())), display); + + auto dt = OCIO::DisplayViewTransform::Create(); + dt->setSrc("source"); + dt->setView(view.c_str()); + + // Build once using the display's current name. + dt->setDisplay(display.c_str()); + OCIO::OpRcPtrVec currentOps; + OCIO_CHECK_NO_THROW(OCIO::BuildDisplayOps(currentOps, *config, config->getCurrentContext(), + *dt, OCIO::TRANSFORM_DIR_FORWARD)); + OCIO_CHECK_NO_THROW(currentOps.validate()); + OCIO_REQUIRE_EQUAL(currentOps.size(), 5); // (includes gpu allocation no-ops) + + // Build again using only the display's old, alias-only name. The result must be identical. + dt->setDisplay(oldDisplayName.c_str()); + OCIO::OpRcPtrVec aliasedOps; + OCIO_CHECK_NO_THROW(OCIO::BuildDisplayOps(aliasedOps, *config, config->getCurrentContext(), + *dt, OCIO::TRANSFORM_DIR_FORWARD)); + OCIO_CHECK_NO_THROW(aliasedOps.validate()); + OCIO_REQUIRE_EQUAL(aliasedOps.size(), currentOps.size()); + + for (size_t i = 0; i < aliasedOps.size(); ++i) + { + auto opA = OCIO_DYNAMIC_POINTER_CAST(aliasedOps[i]); + auto opB = OCIO_DYNAMIC_POINTER_CAST(currentOps[i]); + OCIO_CHECK_ASSERT(*opA->data() == *opB->data()); + } + + // A display name that cannot be resolved at all -- not an existing display, and not a + // display color space name or alias either -- must still throw, referencing the name the + // caller actually provided. + dt->setDisplay("not a display"); + OCIO::OpRcPtrVec badOps; + OCIO_CHECK_THROW_WHAT(OCIO::BuildDisplayOps(badOps, *config, config->getCurrentContext(), + *dt, OCIO::TRANSFORM_DIR_FORWARD), + OCIO::Exception, + "DisplayViewTransform error. Display 'not a display' not found."); +} + +OCIO_ADD_TEST(DisplayViewTransform, view_alias_fallback) +{ + // Validate that BuildDisplayOps resolves a view using Config::getCanonicalViewName, so + // that renaming a view in a config (while keeping the old name as an alias) doesn't + // break a DisplayViewTransform still using the old name as its "view". + + constexpr char CONFIG[]{ R"( +ocio_profile_version: 2.6 + +# Opt-in to view aliasing. +use_display_view_aliases: true + +roles: + default: raw + aces_interchange: raw + cie_xyz_d65_interchange: sRGB - Display + color_timing: raw + compositing_log: raw + scene_linear: raw + +displays: + sRGB - Display: + - ! {name: view, view_transform: display_vt, display_colorspace: sRGB - Display, looks: look1} + +looks: + - ! + name: look1 + process_space: source + transform: ! {slope: [1.1, 1.2, 1.3]} + +view_transforms: + - ! + name: display_vt + aliases: [old_vt] + to_scene_reference: ! {offset: [0.3, 0.1, 0.1, 0]} + +display_colorspaces: + - ! + name: sRGB - Display + to_display_reference: ! {offset: [0.25, 0.15, 0.35, 0]} + +colorspaces: + - ! + name: raw + isdata: true + + - ! + name: source + to_scene_reference: ! {offset: [0, 0.1, 0.2, 0]} +)" }; + + std::istringstream is; + is.str(CONFIG); + + OCIO::ConstConfigRcPtr config; + OCIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromStream(is)); + OCIO_CHECK_NO_THROW(config->validate()); + + const std::string display{ "sRGB - Display" }; + const std::string view{ "view" }; + const std::string oldViewTransformName{ "old_vt" }; + + OCIO_CHECK_EQUAL(std::string(config->getCanonicalViewName(display.c_str(), + oldViewTransformName.c_str())), + view); + + auto dt = OCIO::DisplayViewTransform::Create(); + dt->setSrc("source"); + dt->setDisplay(display.c_str()); + + // Build once using the view's current name. + dt->setView(view.c_str()); + OCIO::OpRcPtrVec currentOps; + OCIO_CHECK_NO_THROW(OCIO::BuildDisplayOps(currentOps, *config, config->getCurrentContext(), + *dt, OCIO::TRANSFORM_DIR_FORWARD)); + OCIO_CHECK_NO_THROW(currentOps.validate()); + OCIO_REQUIRE_EQUAL(currentOps.size(), 7); // (includes gpu allocation no-ops) + + // Build again using only the view transform's old, alias-only name as the "view". The result + // must be identical. Note that the view has a look, so this also verifies that the resolved + // view name is used to look up the view's looks, not just its color space and view transform. + dt->setView(oldViewTransformName.c_str()); + OCIO::OpRcPtrVec aliasedOps; + OCIO_CHECK_NO_THROW(OCIO::BuildDisplayOps(aliasedOps, *config, config->getCurrentContext(), + *dt, OCIO::TRANSFORM_DIR_FORWARD)); + OCIO_CHECK_NO_THROW(aliasedOps.validate()); + OCIO_REQUIRE_EQUAL(aliasedOps.size(), currentOps.size()); + + for (size_t i = 0; i < aliasedOps.size(); ++i) + { + auto opA = OCIO_DYNAMIC_POINTER_CAST(aliasedOps[i]); + auto opB = OCIO_DYNAMIC_POINTER_CAST(currentOps[i]); + OCIO_CHECK_ASSERT(*opA->data() == *opB->data()); + } + + // A view name that cannot be resolved at all -- not an existing view, and not a view + // transform or named transform name or alias used by this display either -- must still + // throw, referencing the name the caller actually provided. + dt->setView("not a view"); + OCIO::OpRcPtrVec badOps; + OCIO_CHECK_THROW_WHAT(OCIO::BuildDisplayOps(badOps, *config, config->getCurrentContext(), + *dt, OCIO::TRANSFORM_DIR_FORWARD), + OCIO::Exception, + "DisplayViewTransform error. The display 'sRGB - Display' does not " + "have view 'not a view'."); +} + +OCIO_ADD_TEST(DisplayViewTransform, context_variables_with_resolved_display_view) +{ + // Validate that CollectContextVariables resolves the (display, view) pair the same way + // BuildDisplayOps does, so that the context variables of the display color space are found + // even when the view uses or when the display name is aliased. + + constexpr const char * OCIO_CONFIG{ R"( +ocio_profile_version: 2.6 + +use_display_view_aliases: true + +environment: { FILE: cdl_test1.cc } + +roles: + default: raw + aces_interchange: raw + cie_xyz_d65_interchange: sRGB - Display + color_timing: raw + compositing_log: raw + scene_linear: source + +file_rules: + - ! {name: Default, colorspace: default} + +shared_views: + - ! {name: view, view_transform: display_vt, display_colorspace: } + +displays: + sRGB - Display: + - ! [view] + - ! {name: plain_view, colorspace: source} + +view_transforms: + - ! + name: display_vt + to_scene_reference: ! {offset: [0.3, 0.1, 0.1, 0]} + +display_colorspaces: + - ! + name: sRGB - Display + aliases: [sRGB] + to_display_reference: ! {src: $FILE} + +colorspaces: + - ! + name: raw + isdata: true + + - ! + name: source + allocation: uniform +)" }; + + std::istringstream is; + is.str(OCIO_CONFIG); + + OCIO::ConfigRcPtr cfg; + OCIO_CHECK_NO_THROW(cfg = OCIO::Config::CreateFromStream(is)->createEditableCopy()); + cfg->setSearchPath(OCIO::GetTestFilesDir().c_str()); + OCIO_CHECK_NO_THROW(cfg->validate()); + + OCIO::ContextRcPtr usedContextVars = OCIO::Context::Create(); + + auto dt = OCIO::DisplayViewTransform::Create(); + dt->setSrc("source"); + + // A view with no context variables anywhere: neither the source nor the color space it + // targets uses one. Returns false. + dt->setDisplay("sRGB - Display"); + dt->setView("plain_view"); + OCIO_CHECK_ASSERT(!CollectContextVariables(*cfg, *cfg->getCurrentContext(), *dt, + usedContextVars)); + + // The shared view's display color space is , so finding the context + // variable requires resolving that to the display color space named after the display. + // The context variable is found: returns true. + dt->setView("view"); + OCIO_CHECK_ASSERT(CollectContextVariables(*cfg, *cfg->getCurrentContext(), *dt, + usedContextVars)); + + // The display's old, alias-only name is resolved too, so the same context variable is found. + // The context variable is found: returns true. + dt->setDisplay("sRGB"); + OCIO_CHECK_ASSERT(CollectContextVariables(*cfg, *cfg->getCurrentContext(), *dt, + usedContextVars)); +} diff --git a/tests/python/ConfigTest.py b/tests/python/ConfigTest.py index 62c2c7d043..f0992fd1f4 100644 --- a/tests/python/ConfigTest.py +++ b/tests/python/ConfigTest.py @@ -798,6 +798,129 @@ def test_canonical_name(self): self.assertEqual(cfg.getCanonicalName('Alias1'), 'nt1') self.assertEqual(cfg.getCanonicalName('Test1'), 'nt1') + def test_use_display_view_aliases(self): + # Test the getUseDisplayViewAliases/setUseDisplayViewAliases methods, and that they gate + # the getCanonicalDisplayName/getCanonicalViewName fallbacks. + + cfg = OCIO.Config() + self.assertFalse(cfg.getUseDisplayViewAliases()) + + cfg.setUseDisplayViewAliases(True) + self.assertTrue(cfg.getUseDisplayViewAliases()) + cfg.setUseDisplayViewAliases(False) + self.assertFalse(cfg.getUseDisplayViewAliases()) + + # Build a config where a display and a view transform have both been renamed, keeping + # their old names alive as aliases. + cfg.setVersion(2, 6) + + dcs = OCIO.ColorSpace( + referenceSpace=OCIO.REFERENCE_SPACE_DISPLAY, + name='sRGB - Display', + aliases=['sRGB']) + dcs.setTransform(OCIO.MatrixTransform(), OCIO.COLORSPACE_DIR_FROM_REFERENCE) + cfg.addColorSpace(dcs) + + vt = OCIO.ViewTransform( + referenceSpace=OCIO.REFERENCE_SPACE_SCENE, + name='vt_new', + aliases=['vt_old']) + vt.setTransform(OCIO.MatrixTransform(), OCIO.VIEWTRANSFORM_DIR_FROM_REFERENCE) + cfg.addViewTransform(vt) + + cfg.addDisplayView('sRGB - Display', 'view', viewTransform='vt_new', + displayColorSpaceName='sRGB - Display') + + # The fallback is disabled by default: only exact matches resolve. + self.assertEqual(cfg.getCanonicalDisplayName('sRGB - Display'), 'sRGB - Display') + self.assertEqual(cfg.getCanonicalDisplayName('sRGB'), '') + self.assertEqual(cfg.getCanonicalViewName('sRGB - Display', 'view'), 'view') + self.assertEqual(cfg.getCanonicalViewName('sRGB - Display', 'vt_old'), '') + + # Once enabled, both the display and view transform aliases resolve. + cfg.setUseDisplayViewAliases(True) + self.assertEqual(cfg.getCanonicalDisplayName('sRGB'), 'sRGB - Display') + self.assertEqual(cfg.getCanonicalViewName('sRGB - Display', 'vt_old'), 'view') + + def test_display_description(self): + # Test that getDisplayDescription borrows the description of the display's associated + # display color space: an exact name match always works, but matching only via one of + # the color space's aliases requires getUseDisplayViewAliases. + + cfg = OCIO.Config() + cfg.setVersion(2, 6) + + dcs = OCIO.ColorSpace( + referenceSpace=OCIO.REFERENCE_SPACE_DISPLAY, + name='sRGB - Display', + aliases=['sRGB'], + description='The sRGB display.') + dcs.setTransform(OCIO.MatrixTransform(), OCIO.COLORSPACE_DIR_FROM_REFERENCE) + cfg.addColorSpace(dcs) + + self.assertFalse(cfg.getUseDisplayViewAliases()) + self.assertEqual(cfg.getDisplayDescription('sRGB - Display'), 'The sRGB display.') + self.assertEqual(cfg.getDisplayDescription('sRGB'), '') + + cfg.setUseDisplayViewAliases(True) + self.assertEqual(cfg.getDisplayDescription('sRGB'), 'The sRGB display.') + + self.assertEqual(cfg.getDisplayDescription('does not exist'), '') + + def test_resolved_display_view_color_space_name(self): + # Test that getResolvedDisplayViewColorSpaceName returns the color space actually used by + # a (display, view) pair, rather than the colorspace attribute as written in the config. + + cfg = OCIO.Config() + cfg.setVersion(2, 6) + + cfg.addColorSpace(OCIO.ColorSpace(name='raw')) + + dcs = OCIO.ColorSpace( + referenceSpace=OCIO.REFERENCE_SPACE_DISPLAY, + name='sRGB - Display', + aliases=['sRGB']) + dcs.setTransform(OCIO.MatrixTransform(), OCIO.COLORSPACE_DIR_FROM_REFERENCE) + cfg.addColorSpace(dcs) + + vt = OCIO.ViewTransform( + referenceSpace=OCIO.REFERENCE_SPACE_SCENE, + name='vt_new', + aliases=['vt_old']) + vt.setTransform(OCIO.MatrixTransform(), OCIO.VIEWTRANSFORM_DIR_FROM_REFERENCE) + cfg.addViewTransform(vt) + + cfg.addDisplayView('sRGB - Display', 'view1', 'raw') + cfg.addDisplayView('sRGB - Display', 'view2', viewTransform='vt_old', + displayColorSpaceName='') + + # A plain view behaves like getDisplayViewColorSpaceName. + self.assertEqual( + cfg.getResolvedDisplayViewColorSpaceName('sRGB - Display', 'view1'), 'raw') + + # resolves to the display's own color space, without needing the + # display/view alias fallback to be enabled. + self.assertFalse(cfg.getUseDisplayViewAliases()) + self.assertEqual( + cfg.getDisplayViewColorSpaceName('sRGB - Display', 'view2'), '') + self.assertEqual( + cfg.getResolvedDisplayViewColorSpaceName('sRGB - Display', 'view2'), 'sRGB - Display') + + # Resolving an out-of-date display or view name is opt-in. + self.assertEqual(cfg.getResolvedDisplayViewColorSpaceName('sRGB', 'view1'), '') + self.assertEqual(cfg.getResolvedDisplayViewColorSpaceName('sRGB - Display', 'vt_new'), '') + + cfg.setUseDisplayViewAliases(True) + self.assertEqual(cfg.getResolvedDisplayViewColorSpaceName('sRGB', 'view1'), 'raw') + self.assertEqual( + cfg.getResolvedDisplayViewColorSpaceName('sRGB', 'vt_new'), 'sRGB - Display') + + # Nonexistent display or view. + self.assertEqual( + cfg.getResolvedDisplayViewColorSpaceName('sRGB - Display', 'not a view'), '') + self.assertEqual(cfg.getResolvedDisplayViewColorSpaceName('not a display', 'view1'), '') + self.assertEqual(cfg.getResolvedDisplayViewColorSpaceName('', 'view1'), '') + def test_virtual_display(self): # Test platform agnostic virtual display interface. diff --git a/tests/python/ViewTransformTest.py b/tests/python/ViewTransformTest.py index bb9e9e3f31..cb6df5fafe 100644 --- a/tests/python/ViewTransformTest.py +++ b/tests/python/ViewTransformTest.py @@ -61,11 +61,13 @@ def test_copy(self): vt.setTransform(mat, OCIO.VIEWTRANSFORM_DIR_TO_REFERENCE) vt.setTransform(direction=OCIO.VIEWTRANSFORM_DIR_FROM_REFERENCE, transform=mat) vt.addCategory('cat1') + vt.addAlias('alias1') other = copy.deepcopy(vt) self.assertFalse(other is vt) self.assertEqual(other.getName(), vt.getName()) + self.assertEqual(list(other.getAliases()), list(vt.getAliases())) self.assertEqual(other.getFamily(), vt.getFamily()) self.assertEqual(other.getDescription(), vt.getDescription()) self.assertEqual( @@ -85,6 +87,68 @@ def test_name(self): vt.setName('test name') self.assertEqual(vt.getName(), 'test name') + def test_aliases(self): + """ + Test ViewTransform aliases. + """ + + vt = OCIO.ViewTransform() + self.assertEqual(vt.getName(), '') + aliases = vt.getAliases() + self.assertEqual(len(aliases), 0) + + vt.addAlias('alias1') + aliases = vt.getAliases() + self.assertEqual(len(aliases), 1) + self.assertEqual(aliases[0], 'alias1') + self.assertTrue(vt.hasAlias('alias1')) + self.assertTrue(vt.hasAlias('aLiaS1')) + self.assertFalse(vt.hasAlias('alias2')) + + vt.addAlias('alias2') + aliases = vt.getAliases() + self.assertEqual(len(aliases), 2) + self.assertEqual(aliases[0], 'alias1') + self.assertEqual(aliases[1], 'alias2') + self.assertTrue(vt.hasAlias('alias2')) + + # Alias is already there, not added. + + vt.addAlias('Alias2') + aliases = vt.getAliases() + self.assertEqual(len(aliases), 2) + self.assertEqual(aliases[0], 'alias1') + self.assertEqual(aliases[1], 'alias2') + + # Name might remove an alias. + + vt.setName('name') + aliases = vt.getAliases() + self.assertEqual(len(aliases), 2) + + vt.setName('alias2') + aliases = vt.getAliases() + self.assertEqual(len(aliases), 1) + self.assertEqual(aliases[0], 'alias1') + + vt.removeAlias('alias1') + aliases = vt.getAliases() + self.assertEqual(len(aliases), 0) + + vt.addAlias('alias3') + vt.addAlias('alias4') + self.assertEqual(len(vt.getAliases()), 2) + vt.clearAliases() + self.assertEqual(len(vt.getAliases()), 0) + + # Aliases may also be set via the constructor. + + vt = OCIO.ViewTransform(name='vt_name', aliases=['a1', 'a2']) + aliases = vt.getAliases() + self.assertEqual(len(aliases), 2) + self.assertEqual(aliases[0], 'a1') + self.assertEqual(aliases[1], 'a2') + def test_family(self): """ Test get/setFamily. From 5a51361b8e32d37e7bd79e1f2f7b51e70cd62c14 Mon Sep 17 00:00:00 2001 From: Doug Walker Date: Mon, 28 Sep 2026 02:20:02 -0400 Subject: [PATCH 2/4] Rework view aliases Signed-off-by: Doug Walker --- docs/guides/authoring/displays_views.rst | 29 +- include/OpenColorIO/OpenColorIO.h | 99 +-- src/OpenColorIO/Config.cpp | 412 +++++++----- src/OpenColorIO/Display.cpp | 7 +- src/OpenColorIO/Display.h | 33 +- src/OpenColorIO/OCIOYaml.cpp | 64 +- src/OpenColorIO/ViewTransform.cpp | 75 --- .../transforms/DisplayViewTransform.cpp | 4 +- src/bindings/python/PyConfig.cpp | 41 +- src/bindings/python/PyViewTransform.cpp | 67 +- tests/cpu/Config_tests.cpp | 631 +++++++++--------- tests/cpu/ViewTransform_tests.cpp | 71 -- .../LegacyViewingPipeline_tests.cpp | 14 +- .../transforms/DisplayViewTransform_tests.cpp | 44 +- tests/python/ConfigTest.py | 130 ++-- tests/python/ViewTransformTest.py | 64 -- 16 files changed, 870 insertions(+), 915 deletions(-) diff --git a/docs/guides/authoring/displays_views.rst b/docs/guides/authoring/displays_views.rst index adede1c874..6953710378 100644 --- a/docs/guides/authoring/displays_views.rst +++ b/docs/guides/authoring/displays_views.rst @@ -89,6 +89,11 @@ The keys allowed with a View are: and the '-' character to apply in reverse. See :ref:`config-looks` * ``rule``: The viewing rule to be used with this View. See :ref:`config-viewing-rules` * ``description``: A description string for this View. +* ``aliases``: Alternative names that can be used to refer to this View. Unlike display + aliases, resolving a view by one of its aliases is always active (it does not depend + on ``use_display_aliases``). An alias must not collide with the name or an alias of + another view used by the same display (whether display-defined or a referenced + shared view). Requires ``ocio_profile_version`` 2.6 or higher. Note that a View may use either the colorspace key or it may use both the view_transform and dispay_colorspace keys. No other combinations @@ -133,22 +138,24 @@ A View Transform may use the following keys: .. TODO: Good spot for an example in a future revision. -``use_display_view_aliases`` -^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +``use_display_aliases`` +^^^^^^^^^^^^^^^^^^^^^^^ -Optional. Activates aliases for display and view names. +Optional. Activates aliases for display names. -By default, the arguments to DisplayViewTransform must be the exact strings found -in the display / view section of the config. However, if ``ocio_profile_version`` -is 2.6 or higher, ``use_display_view_aliases`` may be set to true. This allows a -display to be referred to by the name or aliases of its corresponding display -ColorSpace and the view to be referred to by the name or aliases of its -corresponding ViewTransform. This config-level attribute defaults to false and -must be omitted from the config file if its value is not "true". +By default, the arguments to DisplayViewTransform must be the exact display name found +in the display section of the config. However, if ``ocio_profile_version`` is 2.6 or +higher, ``use_display_aliases`` may be set to true. This allows a display to be +referred to by the name or aliases of its corresponding display ColorSpace. This +config-level attribute defaults to false and must be omitted from the config file if +its value is not "true". + +Note that this does not affect resolving a view by one of its ``aliases`` (see the View +keys above), which is always active regardless of this setting. .. code-block:: yaml - use_display_view_aliases: true + use_display_aliases: true ``default_view_transform`` diff --git a/include/OpenColorIO/OpenColorIO.h b/include/OpenColorIO/OpenColorIO.h index 8e10024d40..7f101037cb 100644 --- a/include/OpenColorIO/OpenColorIO.h +++ b/include/OpenColorIO/OpenColorIO.h @@ -862,6 +862,17 @@ class OCIOEXPORT Config void addSharedView(const char * view, const char * viewTransformName, const char * colorSpaceName, const char * looks, const char * ruleName, const char * description); + /** + * \brief As above, but also sets the view's aliases (see \ref Config::getDisplayViewAliases + * for the format). + * + * Will throw if view or colorSpaceName are null or empty, or if an alias collides with the + * name or an alias of another shared view. + */ + void addSharedView(const char * view, const char * viewTransformName, + const char * colorSpaceName, const char * looks, + const char * ruleName, const char * description, + const char * aliases); /// Remove a shared view. Will throw if the view does not exist. void removeSharedView(const char * view); @@ -928,6 +939,23 @@ class OCIOEXPORT Config /// Returns the description attribute of a (display, view) pair. const char * getDisplayViewDescription(const char * display, const char * view) const noexcept; + /** + * \brief Get the aliases of a (display, view) pair, as a comma-delimited string (as it + * would appear in a config file). If display is null or empty, config shared views are used. + * + * If an alias itself contains a comma, it is enclosed in quotes, similar to active_views. + * + * Returns "" if the (display, view) pair does not exist or has no aliases. + */ + std::string getDisplayViewAliases(const char * display, const char * view) const; + + /** + * \brief Convenience method to check whether a (display, view) pair has a specific alias. + * If display is null or empty, config shared views are used. + */ + bool hasDisplayViewAlias(const char * display, const char * view, + const char * alias) const noexcept; + /** * \brief Determine if a display and view exist. * @@ -962,6 +990,20 @@ class OCIOEXPORT Config const char * colorSpaceName, const char * looks, const char * ruleName, const char * description); + /** + * \brief As above, but also sets the view's aliases (see \ref Config::getDisplayViewAliases + * for the format). + * + * Will throw if: + * * Display, view or colorSpace are null or empty. + * * Display already has a shared view with the same name. + * * An alias collides with the name or an alias of another view in this display, whether + * display-defined or a shared view referenced by this display. + */ + void addDisplayView(const char * display, const char * view, const char * viewTransformName, + const char * colorSpaceName, const char * looks, + const char * ruleName, const char * description, const char * aliases); + /** * \brief Add a (reference to a) shared view to a display. * @@ -987,29 +1029,29 @@ class OCIOEXPORT Config void clearDisplays(); /** - * Methods related to display and view aliases. + * Methods that involve resolving display and view aliases. * */ /** * \brief This property on the Config object allows config authors to use aliases for - * display or view names. This feature is off by default. + * display names. This feature is off by default. * - * Corresponds to the "use_display_view_aliases" config file attribute, which is only + * Corresponds to the "use_display_aliases" config file attribute, which is only * written to the file when true. Requires config version 2.6 or higher (validation * will fail if this is enabled on an older config). */ - bool getUseDisplayViewAliases() const noexcept; - void setUseDisplayViewAliases(bool enabled) noexcept; + bool getUseDisplayAliases() const noexcept; + void setUseDisplayAliases(bool enabled) noexcept; /** * \brief Resolve display name aliases. * * If the argument does not match an existing display, a fallback checks if getColorSpace - * returns a display color space. If so, it checks to see if there is a display whose + * returns a display color space. If so, it checks to see if there is a display whose * name matches that color space name or one of its aliases. * - * This fallback is only performed if \ref Config::getUseDisplayViewAliases is true. + * This fallback is only performed if \ref Config::getUseDisplayAliases is true. * * Returns "" if no display can be found, even with the fallback. */ @@ -1018,12 +1060,10 @@ class OCIOEXPORT Config /** * \brief Resolve view name aliases. * - * If the arguments do not directly match an existing (display, view) pair, a fallback - * checks if getViewTransform or getNamedTransform returns a result for viewName. If so, - * it checks if the display has a view whose view_transform matches the name or an alias - * of that transform. - * - * This fallback is only performed if \ref Config::getUseDisplayViewAliases is true. + * If the arguments do not directly match an existing (display, view) pair, this looks for + * a view used by the display (whether display-defined or a referenced shared view, active + * or inactive) that has viewName as one of its aliases (see \ref Config::addDisplayView + * and \ref Config::addSharedView). * * The displayName is first resolved via \ref Config::getCanonicalDisplayName. * @@ -1034,18 +1074,18 @@ class OCIOEXPORT Config /** * \brief Returns the name of the color space that a (display, view) pair uses. - * + * * This is similar to \ref Config::getDisplayViewColorSpaceName, but it first attempts - * to resolve displayName and viewName (which could be aliases) to their canonical names. - * And unlike that function, the displayName may not be empty. The alias resolution is - * gated by \ref Config::getUseDisplayViewAliases. - * + * to resolve displayName and viewName (which could be aliases) to their canonical names, + * via \ref Config::getCanonicalDisplayName and \ref Config::getCanonicalViewName. And + * unlike that function, the displayName may not be empty. + * * In addition, if the display_colorspace of a shared view is , that * is resolved to the name of the view's display. * * Note that, as with getDisplayViewColorSpaceName, the returned name may be that of a * named transform rather than a color space (this is allowed for views that have no - * view_transform). + * view_transform). * * Returns either the canonical name of the view's color space or, if that does not * find a result, the raw color space string (which would likely be used in an error @@ -1058,7 +1098,7 @@ class OCIOEXPORT Config * \brief Return the description of the display color space associated with displayName. * * If displayName matches the canonical name of a display color space, its description is - * returned. If \ref Config::getUseDisplayViewAliases is true, the search is broadened to + * returned. If \ref Config::getUseDisplayAliases is true, the search is broadened to * include display color spaces that have displayName as an alias. * * Returns "" if no such display color space can be found. @@ -2638,25 +2678,6 @@ class OCIOEXPORT ViewTransform /// \see ColorSpace::setName void setName(const char * name) noexcept; - /** - * \note - * ViewTransform aliases are available in config file versions 2.6 or higher. They - * are availaable regardless of how \ref Config::getUseDisplayViewAliases is set. - */ - - /// \see ColorSpace::getNumAliases - size_t getNumAliases() const noexcept; - /// \see ColorSpace::getAlias - const char * getAlias(size_t idx) const noexcept; - /// \see ColorSpace::hasAlias - bool hasAlias(const char * alias) const noexcept; - /// \see ColorSpace::addAlias - void addAlias(const char * alias) noexcept; - /// \see ColorSpace::removeAlias - void removeAlias(const char * alias) noexcept; - /// \see ColorSpace::clearAliases - void clearAliases() noexcept; - /// \see ColorSpace::getFamily const char * getFamily() const noexcept; /// \see ColorSpace::setFamily diff --git a/src/OpenColorIO/Config.cpp b/src/OpenColorIO/Config.cpp index cd0410d1f2..dae802e747 100644 --- a/src/OpenColorIO/Config.cpp +++ b/src/OpenColorIO/Config.cpp @@ -321,7 +321,7 @@ class Config::Impl // Misc std::vector m_defaultLumaCoefs; bool m_strictParsing; - bool m_useDisplayViewAliases{ false }; + bool m_useDisplayAliases{ false }; mutable Validation m_validation; mutable std::string m_validationtext; @@ -441,7 +441,7 @@ class Config::Impl m_defaultViewTransform = rhs.m_defaultViewTransform; m_defaultLumaCoefs = rhs.m_defaultLumaCoefs; m_strictParsing = rhs.m_strictParsing; - m_useDisplayViewAliases = rhs.m_useDisplayViewAliases; + m_useDisplayAliases = rhs.m_useDisplayAliases; m_validation = rhs.m_validation; m_validationtext = rhs.m_validationtext; @@ -541,15 +541,6 @@ class Config::Impl } } - // Not found by name, so look for an alias. - for (const auto & vt : m_viewTransforms) - { - if (vt->hasAlias(name)) - { - return vt; - } - } - return ConstViewTransformRcPtr(); } @@ -744,7 +735,32 @@ class Config::Impl m_validationtext = os.str(); throw Exception(m_validationtext.c_str()); } - else if (checkUseDisplayName) + else + { + // Does the shared view's name or an alias collide with the name or an alias of a + // display-defined view in this display? This is normally already prevented by + // addDisplayView and addDisplaySharedView at the point a view is added to (or + // linked to) the display. However, addSharedView may replace an already-linked + // shared view's aliases afterwards, and that call does not check if any other + // displays currently reference it, so this check acts as the defensive backstop + // for that case. + for (const auto & view : viewsOfDisplay) + { + const std::string collision = sharedViewIt->FindNamingCollision(view); + if (!collision.empty()) + { + std::ostringstream os; + os << "Config failed view validation. The display '" << display << "' "; + os << "contains a shared view '" << sharedViewIt->m_name << "' whose name "; + os << "or alias '" << collision << "' collides with the name or alias of "; + os << "the view '" << view.m_name << "' in this display."; + m_validationtext = os.str(); + throw Exception(m_validationtext.c_str()); + } + } + } + + if (checkUseDisplayName) { const auto view = *sharedViewIt; if (!view.m_viewTransform.empty() && view.useDisplayNameForColorspace()) @@ -1966,8 +1982,7 @@ void Config::validate() const if (!getImpl()->m_defaultViewTransform.empty()) { const auto vt = getDefaultSceneToDisplayViewTransform(); - if (!vt || !(StringUtils::Compare(vt->getName(), getImpl()->m_defaultViewTransform) || - vt->hasAlias(getImpl()->m_defaultViewTransform.c_str()))) + if (!vt || !StringUtils::Compare(vt->getName(), getImpl()->m_defaultViewTransform)) { std::ostringstream os; os << "Config failed validation. Default view transform is defined as: '"; @@ -3399,6 +3414,14 @@ bool Config::isViewShared(const char * dispName, const char * viewName) const void Config::addSharedView(const char * view, const char * viewTransform, const char * colorSpace, const char * looks, const char * rule, const char * description) +{ + addSharedView(view, viewTransform, colorSpace, looks, rule, description, nullptr); +} + +void Config::addSharedView(const char * view, const char * viewTransform, + const char * colorSpace, const char * looks, + const char * rule, const char * description, + const char * aliases) { if (!view || !*view) { @@ -3412,8 +3435,42 @@ void Config::addSharedView(const char * view, const char * viewTransform, "non-empty name."); } + StringUtils::StringVec aliasVec = SplitStringEnvStyle(aliases ? aliases : ""); + if (aliasVec.size() == 1 && aliasVec[0].empty()) + { + aliasVec.clear(); + } + ViewVec & views = getImpl()->m_sharedViews; - AddView(views, view, viewTransform, colorSpace, looks, rule, description); + + const View candidate(view, viewTransform, colorSpace, looks, rule, description, aliasVec); + + // Keep shared views unambiguous among themselves (independent of which displays end up + // referencing them), by checking the candidate's name/aliases against every sibling shared + // view (other than the one being replaced, if this call is updating an existing one). + // + // Note: this does not check the candidate against the views of displays that may already + // reference an existing shared view of this name. That case (redefining an already-linked + // shared view's aliases so that they collide with that display's own views) is instead + // caught defensively by validateSharedView. + for (const auto & existing : views) + { + if (StringUtils::Compare(existing.m_name, view)) + { + continue; + } + const std::string collision = candidate.FindNamingCollision(existing); + if (!collision.empty()) + { + std::ostringstream os; + os << "Shared view '" << view << "' could not be added to config: '" << collision; + os << "' is already used as the name or alias of the shared view '"; + os << existing.m_name << "'."; + throw Exception(os.str().c_str()); + } + } + + AddView(views, view, viewTransform, colorSpace, looks, rule, description, aliasVec); getImpl()->m_displayCache.clear(); @@ -3668,6 +3725,22 @@ const char * Config::getDisplayViewDescription(const char * display, const char return viewPtr ? viewPtr->m_description.c_str() : ""; } +std::string Config::getDisplayViewAliases(const char * display, const char * view) const +{ + // Follows the same display=null/empty convention as above: look up view among + // the config's shared views if display is null or empty. + const View * viewPtr = getImpl()->getView(display, view); + + return viewPtr ? JoinStringEnvStyle(viewPtr->m_aliases) : std::string(); +} + +bool Config::hasDisplayViewAlias(const char * display, const char * view, + const char * alias) const noexcept +{ + const View * viewPtr = getImpl()->getView(display, view); + return viewPtr && viewPtr->hasAlias(alias); +} + bool Config::hasView(const char * dispName, const char * viewName) const { // This returns null if either the display or view doesn't exist. @@ -3717,6 +3790,27 @@ void Config::addDisplaySharedView(const char * display, const char * sharedView) throw Exception(os.str().c_str()); } + // If the shared view is already defined, make sure its aliases don't collide with this + // display's own views either (a plain name collision is already excluded above, since + // shared view names and display-defined view names share one namespace per display). + const auto sharedViewIt = FindView(getImpl()->m_sharedViews, sharedView); + if (sharedViewIt != getImpl()->m_sharedViews.end()) + { + for (const auto & existing : existingViews) + { + const std::string collision = sharedViewIt->FindNamingCollision(existing); + if (!collision.empty()) + { + std::ostringstream os; + os << "Shared view '" << sharedView << "' could not be added to display '"; + os << display << "': '" << collision; + os << "' is already used as the name or alias of the view '"; + os << existing.m_name << "' in this display."; + throw Exception(os.str().c_str()); + } + } + } + StringUtils::StringVec & views = iter->second.m_sharedViews; if (StringUtils::Contain(views, sharedView)) { @@ -3737,12 +3831,19 @@ void Config::addDisplaySharedView(const char * display, const char * sharedView) void Config::addDisplayView(const char * display, const char * view, const char * colorSpace, const char * looks) { - addDisplayView(display, view, nullptr, colorSpace, looks, nullptr, nullptr); + addDisplayView(display, view, nullptr, colorSpace, looks, nullptr, nullptr, nullptr); } void Config::addDisplayView(const char * display, const char * view, const char * viewTransform, const char * colorSpace, const char * looks, const char * rule, const char * description) +{ + addDisplayView(display, view, viewTransform, colorSpace, looks, rule, description, nullptr); +} + +void Config::addDisplayView(const char * display, const char * view, const char * viewTransform, + const char * colorSpace, const char * looks, + const char * rule, const char * description, const char * aliases) { if (!display || !*display) { @@ -3760,6 +3861,12 @@ void Config::addDisplayView(const char * display, const char * view, const char "name is needed."); } + StringUtils::StringVec aliasVec = SplitStringEnvStyle(aliases ? aliases : ""); + if (aliasVec.size() == 1 && aliasVec[0].empty()) + { + aliasVec.clear(); + } + DisplayMap::iterator iter = FindDisplay(getImpl()->m_displays, display); if (iter == getImpl()->m_displays.end()) { @@ -3768,7 +3875,7 @@ void Config::addDisplayView(const char * display, const char * view, const char getImpl()->m_displays[curSize].first = display; getImpl()->m_displays[curSize].second.m_views.push_back(View(view, viewTransform, colorSpace, looks, rule, - description)); + description, aliasVec)); getImpl()->m_displayCache.clear(); } else @@ -3781,8 +3888,48 @@ void Config::addDisplayView(const char * display, const char * view, const char throw Exception(os.str().c_str()); } + const View candidate(view, viewTransform, colorSpace, looks, rule, description, aliasVec); + + // Check the candidate's name/aliases against sibling views in this display (other than + // the one being replaced, if this call is updating an existing view by that name). + for (const auto & existing : iter->second.m_views) + { + if (StringUtils::Compare(existing.m_name, view)) + { + continue; + } + const std::string collision = candidate.FindNamingCollision(existing); + if (!collision.empty()) + { + std::ostringstream os; + os << "View '" << view << "' could not be added to display '" << display; + os << "': '" << collision << "' is already used as the name or alias of the "; + os << "view '" << existing.m_name << "' in this display."; + throw Exception(os.str().c_str()); + } + } + + // Check the candidate's name/aliases against the shared views this display references. + for (const auto & sharedViewName : iter->second.m_sharedViews) + { + const auto sharedViewIt = FindView(getImpl()->m_sharedViews, sharedViewName); + if (sharedViewIt == getImpl()->m_sharedViews.end()) + { + continue; + } + const std::string collision = candidate.FindNamingCollision(*sharedViewIt); + if (!collision.empty()) + { + std::ostringstream os; + os << "View '" << view << "' could not be added to display '" << display; + os << "': '" << collision << "' is already used as the name or alias of the "; + os << "shared view '" << sharedViewIt->m_name << "' referenced by this display."; + throw Exception(os.str().c_str()); + } + } + ViewVec & views = iter->second.m_views; - AddView(views, view, viewTransform, colorSpace, looks, rule, description); + AddView(views, view, viewTransform, colorSpace, looks, rule, description, aliasVec); } AutoMutex lock(getImpl()->m_cacheidMutex); @@ -3857,14 +4004,14 @@ void Config::clearDisplays() // Unlike the above functions, these are set up to work with display // and view aliases. -bool Config::getUseDisplayViewAliases() const noexcept +bool Config::getUseDisplayAliases() const noexcept { - return getImpl()->m_useDisplayViewAliases; + return getImpl()->m_useDisplayAliases; } -void Config::setUseDisplayViewAliases(bool enabled) noexcept +void Config::setUseDisplayAliases(bool enabled) noexcept { - getImpl()->m_useDisplayViewAliases = enabled; + getImpl()->m_useDisplayAliases = enabled; AutoMutex lock(getImpl()->m_cacheidMutex); getImpl()->resetCacheIDs(); @@ -3885,7 +4032,7 @@ const char * Config::getCanonicalDisplayName(const char * displayName) const } // Use of the fallback requires a config-level opt-in, which is false by default. - if (!getImpl()->m_useDisplayViewAliases) + if (!getImpl()->m_useDisplayAliases) { return ""; } @@ -3906,8 +4053,39 @@ const char * Config::getCanonicalDisplayName(const char * displayName) const return ""; } + // A candidate display found by name match alone isn't enough: it must also actually use + // this color space, i.e. have a view whose display_colorspace is or + // that resolves (by name or alias) to this same color space. Otherwise, a display that + // merely happens to share a name with an unrelated color space would incorrectly match. + // + // Only requiring one (rather than all) the display's views to match since sometimes a + // display will have utility views such as "Raw" that don't rely on a display color space. + // + auto usesColorSpace = [this, &cs](DisplayMap::const_iterator candidateIter) -> bool + { + // Consider both display-defined views and shared views used by this display, and both + // active and inactive views. + const ViewPtrVec views = getImpl()->getViews(candidateIter->second); + for (const auto * view : views) + { + if (view->useDisplayNameForColorspace()) + { + // THe display_colorspace is . + return true; + } + ConstColorSpaceRcPtr viewCs = getColorSpace(view->m_colorspace.c_str()); + if (viewCs && StringUtils::Compare(viewCs->getName(), cs->getName())) + { + // The canonical name of the view's display_colorspace (or colorspace) + // equals that of the cs that matched displayName. + return true; + } + } + return false; + }; + iter = FindDisplay(getImpl()->m_displays, cs->getName()); - if (iter != getImpl()->m_displays.end()) + if (iter != getImpl()->m_displays.end() && usesColorSpace(iter)) { // A display exists with the name of the color space. In this case, displayName // was an alias of the color space. @@ -3918,10 +4096,19 @@ const char * Config::getCanonicalDisplayName(const char * displayName) const for (size_t i = 0; i < numAliases; ++i) { iter = FindDisplay(getImpl()->m_displays, cs->getAlias(i)); - if (iter != getImpl()->m_displays.end()) + if (iter != getImpl()->m_displays.end() && usesColorSpace(iter)) { // A display exists with the name of an alias of the color space. In this case, // displayName was either the color space name or one of the other aliases. + // + // The reason to allow this match is because resolves via + // getColorSpace(displanName), which works via color space aliases, independent + // of display aliases. Therefore, configs may already contain display color + // spaces that are set up to only match the display name via their aliases. + // In addition, this allows for renaming a display but keeping the display + // color space name unchanged, if that were desired for some reason. + // + // See the aliased_display_name test in Config_tests.cpp. return iter->first.c_str(); } } @@ -3984,7 +4171,7 @@ const char * Config::getDisplayDescription(const char * display) const // A display color space named exactly "display" always works. If it was only found via // one of its aliases, the fallback is opt-in. - if (!StringUtils::Compare(cs->getName(), display) && !getImpl()->m_useDisplayViewAliases) + if (!StringUtils::Compare(cs->getName(), display) && !getImpl()->m_useDisplayAliases) { return ""; } @@ -3999,8 +4186,9 @@ const char * Config::getCanonicalViewName(const char * displayName, const char * return ""; } - // Resolve the displayName first, since it may itself be an alias. (The display - // resolution function's fallback is likewise gated by m_useDisplayViewAliases.) + // Resolve the displayName first, since it may itself be an alias. (That fallback is gated + // by m_useDisplayAliases; view alias resolution below is not, since view aliases are + // an explicit attribute of the view rather than a heuristic fallback.) const char * resolvedDisplay = displayName; const char * canonicalDisplay = getCanonicalDisplayName(displayName); if (canonicalDisplay && *canonicalDisplay) @@ -4014,44 +4202,8 @@ const char * Config::getCanonicalViewName(const char * displayName, const char * return viewName; } - // Use of the fallback requires a config-level opt-in, which is false by default. - if (!getImpl()->m_useDisplayViewAliases) - { - return ""; - } - - // The view's view_transform attribute may point to a ViewTransform or NamedTransform, - // both of which support alias names. Resolve both source and target to the canonical - // name for all comparisons. - auto resolveVTOrNT = [this](const std::string & name) -> std::string - { - if (name.empty()) - { - return std::string(); - } - // If there is both a VT and NT with that name, the ViewTransform takes priority. - ConstViewTransformRcPtr vt = getViewTransform(name.c_str()); - if (vt) - { - return std::string(vt->getName()); - } - ConstNamedTransformRcPtr nt = getNamedTransform(name.c_str()); - if (nt) - { - return std::string(nt->getName()); - } - return std::string(); - }; - - // Get the canonical name of a ViewTransform or NamedTransform responding to viewName. - const std::string transformName = resolveVTOrNT(viewName); - if (transformName.empty()) - { - // There are no ViewTransforms or NamedTransforms that respond to viewName as - // either a name or alias. No fallbacks are possible. - return ""; - } - + // Otherwise, look for a view used by this display (whether display-defined or a + // referenced shared view, active or inactive) that has viewName as an alias. DisplayMap::const_iterator iter = FindDisplay(getImpl()->m_displays, resolvedDisplay); if (iter == getImpl()->m_displays.end()) { @@ -4059,59 +4211,10 @@ const char * Config::getCanonicalViewName(const char * displayName, const char * return ""; } - // Consider both display-defined views and shared views used by this display, and both - // active and inactive views. const ViewPtrVec views = getImpl()->getViews(iter->second); - - // If there is a display color space corresponding to this display, get its pointer. As with - // Config::getCanonicalDisplayName's own alias-based resolution, a match found via one of the - // color space's aliases (rather than its own current name) is accepted. - ConstColorSpaceRcPtr resolvedDisplayCs = getColorSpace(resolvedDisplay); - - // There's nothing that prevents there from being a scene-referred color space that matches - // a display name. In that case, don't try to use this color space to validate the - // display_colorspace of the view candidates. - if (!resolvedDisplayCs || resolvedDisplayCs->getReferenceSpaceType() != REFERENCE_SPACE_DISPLAY) - { - resolvedDisplayCs = ConstColorSpaceRcPtr(); - } - - // Iterate over all views for this display, testing each candidate. for (const auto * candidate : views) { - // Does this candidate's view_transform (which may be a NT) resolve to the same one as - // viewName? If not, this candidate is unrelated to viewName and can be skipped outright. - const std::string resolvedCandidate = resolveVTOrNT(candidate->m_viewTransform); - if (resolvedCandidate.empty() || !StringUtils::Compare(resolvedCandidate, transformName)) - { - continue; - } - - // At this point, we've found a view in this display where its view_transform - // corresponds to viewName (either by name or alias). However, don't return it - // if it uses a display_colorspace that does not match the display color space - // corresponding to this display, if one exists. - - if (candidate->useDisplayNameForColorspace()) - { - // The candidate is using for its display_colorspace, so it - // goes with the display, by definition. - return candidate->m_name.c_str(); - } - - if (!resolvedDisplayCs) - { - // There is no display color space in the config corresponding to this display, - // so regardless of what the candidate's display_colorspace is, there is - // nothing to check it against. Accept the match. - return candidate->m_name.c_str(); - } - - // There is a display color space for this display, so only accept this candidate - // if its display_colorspace resolves to that same color space (by name or alias). - // Otherwise keep looking -- another candidate might still satisfy this check. - ConstColorSpaceRcPtr candidateCs = getColorSpace(candidate->m_colorspace.c_str()); - if (candidateCs && StringUtils::Compare(candidateCs->getName(), resolvedDisplayCs->getName())) + if (candidate->hasAlias(viewName)) { return candidate->m_name.c_str(); } @@ -4920,36 +5023,6 @@ void Config::addViewTransform(const ConstViewTransformRcPtr & viewTransform) const std::string namelower = StringUtils::Lower(name); - // The name and aliases must not collide with a different, existing view transform. - for (const auto & vt : getImpl()->m_viewTransforms) - { - if (StringUtils::Lower(vt->getName()) == namelower) - { - continue; - } - - if (vt->hasAlias(name.c_str())) - { - std::ostringstream os; - os << "Cannot add '" << name << "' view transform, existing view transform '"; - os << vt->getName() << "' is using this name as an alias."; - throw Exception(os.str().c_str()); - } - - const size_t numAliases = viewTransform->getNumAliases(); - for (size_t aidx = 0; aidx < numAliases; ++aidx) - { - const char * alias = viewTransform->getAlias(aidx); - if (StringUtils::Compare(vt->getName(), alias) || vt->hasAlias(alias)) - { - std::ostringstream os; - os << "Cannot add '" << name << "' view transform, it has an alias '" << alias; - os << "' that is already used by view transform '" << vt->getName() << "'."; - throw Exception(os.str().c_str()); - } - } - } - bool addIt = true; // If the view transform exists, replace it. @@ -6358,28 +6431,51 @@ void Config::Impl::checkVersionConsistency() const } } + // Check for use_display_aliases. + + if (hexVersion < 0x02060000 && m_useDisplayAliases) + { + throw Exception("Config failed validation: use_display_aliases is true and config " + "version is less than 2.6."); + } + + // Check for view aliases. + if (hexVersion < 0x02060000) { - for (const auto& vt : m_viewTransforms) + auto checkViewAliases = [](const ViewVec & views) -> const View * { - if (vt->getNumAliases() > 0) + for (const auto & view : views) + { + if (!view.m_aliases.empty()) + { + return &view; + } + } + return nullptr; + }; + + if (const View * view = checkViewAliases(m_sharedViews)) + { + std::ostringstream os; + os << "Config failed validation. The shared view '" << view->m_name << "' "; + os << "has aliases and config version is less than 2.6."; + throw Exception(os.str().c_str()); + } + // Note: the virtual display's views are not checked here, since there is currently no + // way to set aliases on them (addVirtualDisplayView has no aliases argument). + for (const auto & display : m_displays) + { + if (const View * view = checkViewAliases(display.second.m_views)) { std::ostringstream os; - os << "Config failed validation. The view transform '" << vt->getName() << "' "; - os << "has aliases and config version is less than 2.6."; + os << "Config failed validation. The view '" << view->m_name << "' in display '"; + os << display.first << "' has aliases and config version is less than 2.6."; throw Exception(os.str().c_str()); } } } - // Check for use_display_view_aliases. - - if (hexVersion < 0x02060000 && m_useDisplayViewAliases) - { - throw Exception("Config failed validation: use_display_view_aliases is true and config " - "version is less than 2.6."); - } - // Check for new Look properties. if (hexVersion < 0x02050000) diff --git a/src/OpenColorIO/Display.cpp b/src/OpenColorIO/Display.cpp index aebc37fb08..05f0d4984f 100644 --- a/src/OpenColorIO/Display.cpp +++ b/src/OpenColorIO/Display.cpp @@ -50,7 +50,8 @@ ViewVec::iterator FindView(ViewVec & vec, const std::string & name) void AddView(ViewVec & views, const char * name, const char * viewTransform, const char * displayColorSpace, const char * looks, - const char * rule, const char * description) + const char * rule, const char * description, + const StringUtils::StringVec & aliases) { if (displayColorSpace && 0 == Platform::Strcasecmp(displayColorSpace, OCIO_VIEW_USE_DISPLAY_NAME)) { @@ -59,7 +60,8 @@ void AddView(ViewVec & views, const char * name, const char * viewTransform, auto view = FindView(views, name); if (view == views.end()) { - views.push_back(View(name, viewTransform, displayColorSpace, looks, rule, description)); + views.push_back(View(name, viewTransform, displayColorSpace, looks, rule, description, + aliases)); } else { @@ -68,6 +70,7 @@ void AddView(ViewVec & views, const char * name, const char * viewTransform, (*view).m_looks = looks ? looks : ""; (*view).m_rule = rule ? rule : ""; (*view).m_description = description ? description : ""; + (*view).m_aliases = aliases; } } diff --git a/src/OpenColorIO/Display.h b/src/OpenColorIO/Display.h index b2ffd85b50..8b129a3301 100644 --- a/src/OpenColorIO/Display.h +++ b/src/OpenColorIO/Display.h @@ -28,6 +28,7 @@ struct View std::string m_looks; // Might be empty. std::string m_rule; // Might be empty. std::string m_description; // Might be empty. + StringUtils::StringVec m_aliases; // Might be empty. View() = default; @@ -36,13 +37,15 @@ struct View const char * colorspace, const char * looks, const char * rule, - const char * description) + const char * description, + const StringUtils::StringVec & aliases = StringUtils::StringVec()) : m_name(name ? name : "") , m_viewTransform(viewTransform ? viewTransform : "") , m_colorspace(colorspace ? colorspace : "") , m_looks(looks ? looks : "") , m_rule(rule ? rule : "") , m_description(description ? description : "") + , m_aliases(aliases) { } // Make sure that csname is not null. @@ -54,6 +57,31 @@ struct View { return UseDisplayName(m_colorspace.c_str()); } + + bool hasAlias(const char * alias) const + { + if (!alias || !*alias) return false; + return StringUtils::Contain(m_aliases, alias); + } + + // Returns the string (name or alias) of 'this' that collides with the name or an alias of + // 'other', or an empty string if the two views' identities do not overlap. Comparisons are + // case-insensitive. Used to keep view names/aliases unambiguous within whatever scope two + // views can both be resolved in (e.g. the same display, or the config's shared views). + std::string FindNamingCollision(const View & other) const + { + if (StringUtils::Compare(m_name, other.m_name)) return m_name; + for (const auto & alias : m_aliases) + { + if (StringUtils::Compare(alias, other.m_name)) return alias; + } + if (other.hasAlias(m_name.c_str())) return m_name; + for (const auto & alias : m_aliases) + { + if (other.hasAlias(alias.c_str())) return alias; + } + return std::string(); + } }; typedef std::vector ViewVec; @@ -63,7 +91,8 @@ ViewVec::iterator FindView(ViewVec & vec, const std::string & name); void AddView(ViewVec & views, const char * name, const char * viewTransform, const char * displayColorSpace, const char * looks, - const char * rule, const char * description); + const char * rule, const char * description, + const StringUtils::StringVec & aliases = StringUtils::StringVec()); // Display can be part of the list of displays (DisplayMap) of a config. struct Display diff --git a/src/OpenColorIO/OCIOYaml.cpp b/src/OpenColorIO/OCIOYaml.cpp index 8f2ad05e40..5bfaa8962b 100644 --- a/src/OpenColorIO/OCIOYaml.cpp +++ b/src/OpenColorIO/OCIOYaml.cpp @@ -453,6 +453,10 @@ inline void load(const YAML::Node& node, View& v) { load(iter->second, v.m_description); } + else if (key == "aliases") + { + load(iter->second, v.m_aliases); + } else { LogUnknownKeyWarning(node, iter->first); @@ -500,6 +504,10 @@ inline void save(YAML::Emitter& out, const View & view) { out << YAML::Key << "rule" << YAML::Value << view.m_rule; } + if (!view.m_aliases.empty()) + { + out << YAML::Key << "aliases" << YAML::Value << view.m_aliases; + } saveDescription(out, view.m_description.c_str()); out << YAML::EndMap; } @@ -3822,15 +3830,6 @@ inline void load(const YAML::Node & node, ViewTransformRcPtr & vt) load(iter->second, stringval); vt->setName(stringval.c_str()); } - else if (key == "aliases") - { - StringUtils::StringVec aliases; - load(iter->second, aliases); - for (const auto & alias : aliases) - { - vt->addAlias(alias.c_str()); - } - } else if (key == "description") { std::string stringval; @@ -3893,18 +3892,6 @@ inline void save(YAML::Emitter & out, ConstViewTransformRcPtr & vt, unsigned int out << YAML::BeginMap; out << YAML::Key << "name" << YAML::Value << vt->getName(); - const size_t numAliases = vt->getNumAliases(); - if (numAliases) - { - out << YAML::Key << "aliases"; - StringUtils::StringVec aliases; - for (size_t aidx = 0; aidx < numAliases; ++aidx) - { - aliases.push_back(vt->getAlias(aidx)); - } - out << YAML::Flow << YAML::Value << aliases; - } - const char * family = vt->getFamily(); if (family && *family) { @@ -4688,10 +4675,11 @@ inline void load(const YAML::Node& node, ConfigRcPtr & config, const char* filen View view; load(val, view); + const std::string aliases = JoinStringEnvStyle(view.m_aliases); config->addSharedView(view.m_name.c_str(), view.m_viewTransform.c_str(), view.m_colorspace.c_str(), view.m_looks.c_str(), view.m_rule.c_str(), - view.m_description.c_str()); + view.m_description.c_str(), aliases.c_str()); } } else if (key == "displays") @@ -4718,10 +4706,11 @@ inline void load(const YAML::Node& node, ConfigRcPtr & config, const char* filen { View view; load(node, view); + const std::string aliases = JoinStringEnvStyle(view.m_aliases); config->addDisplayView(display.c_str(), view.m_name.c_str(), view.m_viewTransform.c_str(), view.m_colorspace.c_str(), view.m_looks.c_str(), view.m_rule.c_str(), - view.m_description.c_str()); + view.m_description.c_str(), aliases.c_str()); } else if (node.Tag() == "Views") { @@ -4775,10 +4764,10 @@ inline void load(const YAML::Node& node, ConfigRcPtr & config, const char* filen } } } - else if (key == "use_display_view_aliases") + else if (key == "use_display_aliases") { load(iter->second, boolval); - config->setUseDisplayViewAliases(boolval); + config->setUseDisplayAliases(boolval); } else if(key == "active_displays") { @@ -5056,6 +5045,21 @@ inline void load(const YAML::Node& node, ConfigRcPtr & config, const char* filen } } +// Config only exposes a view's aliases as a single comma-delimited string (see +// Config::getDisplayViewAliases), so split it back into a vector for the View struct used +// below to build up what gets passed to save(YAML::Emitter&, const View&). +StringUtils::StringVec GetViewAliasVec(const Config & config, const char * display, + const char * name) +{ + StringUtils::StringVec aliases = SplitStringEnvStyle(config.getDisplayViewAliases(display, + name)); + if (aliases.size() == 1 && aliases[0].empty()) + { + aliases.clear(); + } + return aliases; +} + inline void save(YAML::Emitter & out, const Config & config) { std::stringstream ss; @@ -5210,7 +5214,8 @@ inline void save(YAML::Emitter & out, const Config & config) config.getDisplayViewColorSpaceName(nullptr, name), config.getDisplayViewLooks(nullptr, name), config.getDisplayViewRule(nullptr, name), - config.getDisplayViewDescription(nullptr, name) }; + config.getDisplayViewDescription(nullptr, name), + GetViewAliasVec(config, nullptr, name) }; save(out, dview); } out << YAML::EndSeq; @@ -5239,7 +5244,8 @@ inline void save(YAML::Emitter & out, const Config & config) config.getDisplayViewColorSpaceName(display, name), config.getDisplayViewLooks(display, name), config.getDisplayViewRule(display, name), - config.getDisplayViewDescription(display, name) }; + config.getDisplayViewDescription(display, name), + GetViewAliasVec(config, display, name) }; save(out, dview); } @@ -5299,9 +5305,9 @@ inline void save(YAML::Emitter & out, const Config & config) out << YAML::Newline; out << YAML::Newline; - if (config.getUseDisplayViewAliases()) + if (config.getUseDisplayAliases()) { - out << YAML::Key << "use_display_view_aliases" << YAML::Value << true; + out << YAML::Key << "use_display_aliases" << YAML::Value << true; } out << YAML::Key << "active_displays"; diff --git a/src/OpenColorIO/ViewTransform.cpp b/src/OpenColorIO/ViewTransform.cpp index c784ca8ebd..c14d6021ac 100644 --- a/src/OpenColorIO/ViewTransform.cpp +++ b/src/OpenColorIO/ViewTransform.cpp @@ -5,9 +5,7 @@ #include -#include "Platform.h" #include "TokensManager.h" -#include "utils/StringUtils.h" namespace { @@ -22,7 +20,6 @@ class ViewTransform::Impl { public: std::string m_name; - StringUtils::StringVec m_aliases; std::string m_family; std::string m_description; ReferenceSpaceType m_referenceSpaceType{ REFERENCE_SPACE_SCENE }; @@ -47,7 +44,6 @@ class ViewTransform::Impl if (this != &rhs) { m_name = rhs.m_name; - m_aliases = rhs.m_aliases; m_family = rhs.m_family; m_description = rhs.m_description; @@ -104,63 +100,6 @@ const char * ViewTransform::getName() const noexcept void ViewTransform::setName(const char * name) noexcept { getImpl()->m_name = name ? name : ""; - // Name can no longer be an alias. - StringUtils::Remove(getImpl()->m_aliases, getImpl()->m_name); -} - -size_t ViewTransform::getNumAliases() const noexcept -{ - return getImpl()->m_aliases.size(); -} - -const char * ViewTransform::getAlias(size_t idx) const noexcept -{ - if (idx < getImpl()->m_aliases.size()) - { - return getImpl()->m_aliases[idx].c_str(); - } - return ""; -} - -bool ViewTransform::hasAlias(const char * alias) const noexcept -{ - if (!alias) return false; - for (size_t idx = 0; idx < getImpl()->m_aliases.size(); ++idx) - { - if (0 == Platform::Strcasecmp(getImpl()->m_aliases[idx].c_str(), alias)) - { - return true; - } - } - return false; -} - -void ViewTransform::addAlias(const char * alias) noexcept -{ - if (alias && *alias) - { - if (!StringUtils::Compare(alias, getImpl()->m_name)) - { - if (!StringUtils::Contain(getImpl()->m_aliases, alias)) - { - getImpl()->m_aliases.push_back(alias); - } - } - } -} - -void ViewTransform::removeAlias(const char * name) noexcept -{ - if (name && *name) - { - const std::string alias{ name }; - StringUtils::Remove(getImpl()->m_aliases, alias); - } -} - -void ViewTransform::clearAliases() noexcept -{ - getImpl()->m_aliases.clear(); } const char * ViewTransform::getFamily() const noexcept @@ -328,20 +267,6 @@ std::ostream & operator<< (std::ostream & os, const ViewTransform & vt) { os << " 1) - { - os << "aliases=[" << vt.getAlias(0); - for (size_t aidx = 1; aidx < numAliases; ++aidx) - { - os << ", " << vt.getAlias(aidx); - } - os << "], "; - } os << "family=" << vt.getFamily() << ", "; os << "referenceSpaceType=" << ReferenceSpaceTypeToString(vt.getReferenceSpaceType()); const std::string desc{ vt.getDescription() }; diff --git a/src/OpenColorIO/transforms/DisplayViewTransform.cpp b/src/OpenColorIO/transforms/DisplayViewTransform.cpp index aa490d61c6..3e416cb9cd 100644 --- a/src/OpenColorIO/transforms/DisplayViewTransform.cpp +++ b/src/OpenColorIO/transforms/DisplayViewTransform.cpp @@ -343,8 +343,8 @@ void BuildDisplayOps(OpRcPtrVec & ops, } const std::string view = displayViewTransform.getView(); - // Config authors may opt-in to using view transform names/aliases as aliases for - // view names. Resolve those to the actual view name. + // A view may have its own aliases. Resolving a view by one of its aliases is always + // active. Resolve those to the actual view name. std::string resolvedView = view; const char * canonicalView = config.getCanonicalViewName(resolvedDisplay.c_str(), view.c_str()); if (canonicalView && *canonicalView) diff --git a/src/bindings/python/PyConfig.cpp b/src/bindings/python/PyConfig.cpp index aa85845ad4..7d9d683aba 100644 --- a/src/bindings/python/PyConfig.cpp +++ b/src/bindings/python/PyConfig.cpp @@ -346,10 +346,10 @@ void bindPyConfig(py::module & m) DOC(Config, isStrictParsingEnabled)) .def("setStrictParsingEnabled", &Config::setStrictParsingEnabled, "enabled"_a, DOC(Config, setStrictParsingEnabled)) - .def("getUseDisplayViewAliases", &Config::getUseDisplayViewAliases, - DOC(Config, getUseDisplayViewAliases)) - .def("setUseDisplayViewAliases", &Config::setUseDisplayViewAliases, "enabled"_a, - DOC(Config, setUseDisplayViewAliases)) + .def("getUseDisplayAliases", &Config::getUseDisplayAliases, + DOC(Config, getUseDisplayAliases)) + .def("setUseDisplayAliases", &Config::setUseDisplayAliases, "enabled"_a, + DOC(Config, setUseDisplayAliases)) .def("setInactiveColorSpaces", &Config::setInactiveColorSpaces, "inactiveColorSpaces"_a, DOC(Config, setInactiveColorSpaces)) .def("getInactiveColorSpaces", &Config::getInactiveColorSpaces, @@ -412,11 +412,13 @@ void bindPyConfig(py::module & m) const char *, const char *, const char *, + const char *, const char *)) &Config::addSharedView, - "view"_a, "viewTransformName"_a, "colorSpaceName"_a, + "view"_a, "viewTransformName"_a, "colorSpaceName"_a, "looks"_a = "", - "ruleName"_a = "", - "description"_a = "", + "ruleName"_a = "", + "description"_a = "", + "aliases"_a = "", DOC(Config, addSharedView)) .def("removeSharedView", &Config::removeSharedView, "view"_a, DOC(Config, removeSharedView)) @@ -476,6 +478,11 @@ void bindPyConfig(py::module & m) .def("getResolvedDisplayViewColorSpaceName", &Config::getResolvedDisplayViewColorSpaceName, "display"_a, "view"_a, DOC(Config, getResolvedDisplayViewColorSpaceName)) + .def("getDisplayViewAliases", &Config::getDisplayViewAliases, "display"_a, "view"_a, + DOC(Config, getDisplayViewAliases)) + .def("hasDisplayViewAlias", &Config::hasDisplayViewAlias, + "display"_a, "view"_a, "alias"_a, + DOC(Config, hasDisplayViewAlias)) .def("getDisplayViewLooks", &Config::getDisplayViewLooks, "display"_a, "view"_a, DOC(Config, getDisplayViewLooks)) .def("getDisplayViewRule", &Config::getDisplayViewRule, "display"_a, "view"_a, @@ -492,18 +499,20 @@ void bindPyConfig(py::module & m) "display"_a, "view"_a, "colorSpaceName"_a, "looks"_a = "", DOC(Config, addDisplayView)) - .def("addDisplayView", - (void (Config::*)(const char *, - const char *, - const char *, - const char *, + .def("addDisplayView", + (void (Config::*)(const char *, const char *, const char *, - const char *)) &Config::addDisplayView, - "display"_a, "view"_a, "viewTransform"_a, "displayColorSpaceName"_a, + const char *, + const char *, + const char *, + const char *, + const char *)) &Config::addDisplayView, + "display"_a, "view"_a, "viewTransform"_a, "displayColorSpaceName"_a, "looks"_a = "", - "ruleName"_a = "", - "description"_a = "", + "ruleName"_a = "", + "description"_a = "", + "aliases"_a = "", DOC(Config, addDisplayView)) .def("isViewShared", &Config::isViewShared, "display"_a, "view"_a, DOC(Config, isViewShared)) diff --git a/src/bindings/python/PyViewTransform.cpp b/src/bindings/python/PyViewTransform.cpp index 564a6df785..d3de4730e9 100644 --- a/src/bindings/python/PyViewTransform.cpp +++ b/src/bindings/python/PyViewTransform.cpp @@ -11,13 +11,11 @@ namespace enum ViewTransformIterator { - IT_VIEW_TRANSFORM_CATEGORY = 0, - IT_VIEW_TRANSFORM_ALIAS + IT_VIEW_TRANSFORM_CATEGORY = 0 }; using ViewTransformCategoryIterator = PyIterator; -using ViewTransformAliasIterator = PyIterator; std::vector getCategoriesStdVec(const ViewTransformRcPtr & p) { std::vector categories; @@ -29,17 +27,6 @@ std::vector getCategoriesStdVec(const ViewTransformRcPtr & p) { return categories; } -std::vector getAliasesStdVec(const ViewTransformRcPtr & p) -{ - std::vector aliases; - aliases.reserve(p->getNumAliases()); - for (size_t i = 0; i < p->getNumAliases(); i++) - { - aliases.push_back(p->getAlias(i)); - } - return aliases; -} - } // namespace void bindPyViewTransform(py::module & m) @@ -54,10 +41,6 @@ void bindPyViewTransform(py::module & m) py::class_( clsViewTransform, "ViewTransformCategoryIterator"); - auto clsViewTransformAliasIterator = - py::class_( - clsViewTransform, "ViewTransformAliasIterator"); - clsViewTransform .def(py::init([](ReferenceSpaceType referenceSpace) { @@ -71,19 +54,9 @@ void bindPyViewTransform(py::module & m) const std::string & description, const TransformRcPtr & toReference, const TransformRcPtr & fromReference, - const std::vector & categories, - const std::vector & aliases) + const std::vector & categories) { ViewTransformRcPtr p = ViewTransform::Create(referenceSpace); - if (!aliases.empty()) - { - p->clearAliases(); - for (size_t i = 0; i < aliases.size(); i++) - { - p->addAlias(aliases[i].c_str()); - } - } - // Setting the name will remove alias named the same, so set name after. if (!name.empty()) { p->setName(name.c_str()); } if (!family.empty()) { p->setFamily(family.c_str()); } if (!description.empty()) { p->setDescription(description.c_str()); } @@ -112,7 +85,6 @@ void bindPyViewTransform(py::module & m) "toReference"_a = DEFAULT->getTransform(VIEWTRANSFORM_DIR_TO_REFERENCE), "fromReference"_a = DEFAULT->getTransform(VIEWTRANSFORM_DIR_FROM_REFERENCE), "categories"_a = getCategoriesStdVec(DEFAULT), - "aliases"_a = getAliasesStdVec(DEFAULT), DOC(ViewTransform, Create)) .def("__deepcopy__", [](const ConstViewTransformRcPtr & self, py::dict) @@ -125,21 +97,6 @@ void bindPyViewTransform(py::module & m) DOC(ViewTransform, getName)) .def("setName", &ViewTransform::setName, "name"_a, DOC(ViewTransform, setName)) - - // Aliases. - .def("hasAlias", &ViewTransform::hasAlias, "alias"_a, - DOC(ViewTransform, hasAlias)) - .def("addAlias", &ViewTransform::addAlias, "alias"_a.none(false), - DOC(ViewTransform, addAlias)) - .def("removeAlias", &ViewTransform::removeAlias, "alias"_a.none(false), - DOC(ViewTransform, removeAlias)) - .def("getAliases", [](ViewTransformRcPtr & self) - { - return ViewTransformAliasIterator(self); - }) - .def("clearAliases", &ViewTransform::clearAliases, - DOC(ViewTransform, clearAliases)) - .def("getFamily", &ViewTransform::getFamily, DOC(ViewTransform, getFamily)) .def("setFamily", &ViewTransform::setFamily, "family"_a, @@ -194,26 +151,6 @@ void bindPyViewTransform(py::module & m) int i = it.nextIndex(it.m_obj->getNumCategories()); return it.m_obj->getCategory(i); }); - - clsViewTransformAliasIterator - .def("__len__", [](ViewTransformAliasIterator & it) - { - return it.m_obj->getNumAliases(); - }) - .def("__getitem__", [](ViewTransformAliasIterator & it, int i) - { - it.checkIndex(i, (int)it.m_obj->getNumAliases()); - return it.m_obj->getAlias(i); - }) - .def("__iter__", [](ViewTransformAliasIterator & it) -> ViewTransformAliasIterator & - { - return it; - }) - .def("__next__", [](ViewTransformAliasIterator & it) - { - int i = it.nextIndex((int)it.m_obj->getNumAliases()); - return it.m_obj->getAlias(i); - }); } } // namespace OCIO_NAMESPACE diff --git a/tests/cpu/Config_tests.cpp b/tests/cpu/Config_tests.cpp index e8cebc8fa9..c1b3ce4056 100644 --- a/tests/cpu/Config_tests.cpp +++ b/tests/cpu/Config_tests.cpp @@ -6624,7 +6624,7 @@ OCIO_ADD_TEST(Config, get_processor_from_two_configs) constexpr const char * SIMPLE_CONFIG1{ R"( ocio_profile_version: 2.6 -use_display_view_aliases: true +use_display_aliases: true environment: {} @@ -6640,19 +6640,19 @@ use_display_view_aliases: true displays: displayname: - ! {name: view1, colorspace: displaytest1} - - ! {name: view2, view_transform: vt1, display_colorspace: displayname} + - ! {name: view2, view_transform: vt1, display_colorspace: displayname, + aliases: [oldview2]} - ! {name: view3, colorspace: data_space} - - ! {name: view4, view_transform: vt2, display_colorspace: } + - ! {name: view4, view_transform: vt2, display_colorspace: , + aliases: [oldview4]} view_transforms: - ! name: vt1 - aliases: [oldvt1] from_scene_reference: ! {min_in_value: 0., min_out_value: 0.} - ! name: vt2 - aliases: [oldvt2] from_scene_reference: ! {offset: [0.02, 0.03, 0.04, 0]} colorspaces: @@ -6901,7 +6901,7 @@ ocio_profile_version: 2 // Basic test using aliased display and view names. OCIO_CHECK_NO_THROW(p = OCIO::Config::GetProcessorFromConfigs( - config2, "test2", "aces2", config1, "olddisplayname", "oldvt1", "aces1", + config2, "test2", "aces2", config1, "olddisplayname", "oldview2", "aces1", OCIO::TRANSFORM_DIR_FORWARD)); OCIO_REQUIRE_ASSERT(p); group = p->createGroupTransform(); @@ -6910,7 +6910,7 @@ ocio_profile_version: 2 // Test that the fallback works with just the single config getProcessor too. The result // goes through "displayname" itself (a ECTransform), confirming the resolved view actually // belongs with the resolved display. - OCIO_CHECK_NO_THROW(p = config1->getProcessor("aces1", "olddisplayname", "oldvt1", + OCIO_CHECK_NO_THROW(p = config1->getProcessor("aces1", "olddisplayname", "oldview2", OCIO::TRANSFORM_DIR_FORWARD)); OCIO_REQUIRE_ASSERT(p); group = p->createGroupTransform(); @@ -6923,7 +6923,7 @@ ocio_profile_version: 2 OCIO_CHECK_ASSERT(OCIO_DYNAMIC_POINTER_CAST(t2)); // Verify that substitution happens. - OCIO_CHECK_NO_THROW(p = config1->getProcessor("aces1", "displayname", "oldvt2", + OCIO_CHECK_NO_THROW(p = config1->getProcessor("aces1", "displayname", "oldview4", OCIO::TRANSFORM_DIR_FORWARD)); OCIO_REQUIRE_ASSERT(p); group = p->createGroupTransform(); @@ -6935,31 +6935,18 @@ ocio_profile_version: 2 t2 = group->getTransform(2); OCIO_CHECK_ASSERT(OCIO_DYNAMIC_POINTER_CAST(t2)); - // Turn off display/view aliasing and ensure the result now throws. + // Turn off display aliasing and ensure the result now throws. (View alias resolution is + // always active, but the display fails to resolve.) { OCIO::ConfigRcPtr config1NoFallback = config1->createEditableCopy(); - config1NoFallback->setUseDisplayViewAliases(false); + config1NoFallback->setUseDisplayAliases(false); OCIO_CHECK_THROW_WHAT(OCIO::Config::GetProcessorFromConfigs( - config2, "test2", "aces2", config1NoFallback, "olddisplayname", "oldvt1", "aces1", + config2, "test2", "aces2", config1NoFallback, "olddisplayname", "oldview2", "aces1", OCIO::TRANSFORM_DIR_FORWARD), OCIO::Exception, "DisplayViewTransform error. Display 'olddisplayname' not found."); } - // Modify view2 to point to a display color space that is different from displayname. - // This should throw since the view aliasing will not accept that as a viable match. - { - OCIO::ConfigRcPtr config1Mismatch = config1->createEditableCopy(); - OCIO_CHECK_NO_THROW(config1Mismatch->addDisplayView("displayname", "view2", "vt1", - "display2", "", "", "")); - OCIO_CHECK_THROW_WHAT(OCIO::Config::GetProcessorFromConfigs( - config2, "test2", "aces2", config1Mismatch, "olddisplayname", "oldvt1", "aces1", - OCIO::TRANSFORM_DIR_FORWARD), - OCIO::Exception, - "DisplayViewTransform error. The display 'olddisplayname' does not have view " - "'oldvt1'."); - } - // If one of the spaces is a data space, the whole result must be a no-op. OCIO_CHECK_NO_THROW(p = OCIO::Config::GetProcessorFromConfigs( @@ -7257,103 +7244,6 @@ OCIO_ADD_TEST(Config, view_transforms) OCIO_CHECK_EQUAL(std::string("NotFirst"), configEdit->getDefaultViewTransformName()); } -OCIO_ADD_TEST(Config, view_transform_alias) -{ - OCIO::ConfigRcPtr config = OCIO::Config::CreateFromBuiltinConfig( - "cg-config-v2.2.0_aces-v1.3_ocio-v2.4")->createEditableCopy(); - // ViewTransform aliases require config version 2.6 or higher. - config->setVersion(2, 6); - // Aliases work even if display/view aliasing is off. - OCIO_REQUIRE_ASSERT(!config->getUseDisplayViewAliases()); - - auto sceneVT = OCIO::ViewTransform::Create(OCIO::REFERENCE_SPACE_SCENE); - sceneVT->setName("scene_vt"); - sceneVT->addAlias("old_scene_vt"); - OCIO_CHECK_NO_THROW(sceneVT->setTransform(OCIO::MatrixTransform::Create(), - OCIO::VIEWTRANSFORM_DIR_FROM_REFERENCE)); - OCIO_CHECK_NO_THROW(config->addViewTransform(sceneVT)); - - auto displayVT = OCIO::ViewTransform::Create(OCIO::REFERENCE_SPACE_DISPLAY); - displayVT->setName("display_vt"); - OCIO_CHECK_NO_THROW(displayVT->setTransform(OCIO::MatrixTransform::Create(), - OCIO::VIEWTRANSFORM_DIR_FROM_REFERENCE)); - OCIO_CHECK_NO_THROW(config->addViewTransform(displayVT)); - - OCIO_CHECK_NO_THROW(config->validate()); - - // Validate getViewTransform resolves the alias. - OCIO_CHECK_ASSERT(config->getViewTransform("old_scene_vt")); - OCIO_CHECK_EQUAL(std::string(config->getViewTransform("old_scene_vt")->getName()), "scene_vt"); - - // Check setDefaultViewTransformName also accepts an alias. - config->setDefaultViewTransformName("old_scene_vt"); - OCIO_CHECK_NO_THROW(config->validate()); - OCIO_REQUIRE_ASSERT(config->getDefaultSceneToDisplayViewTransform()); - OCIO_CHECK_EQUAL(std::string(config->getDefaultSceneToDisplayViewTransform()->getName()), - "scene_vt"); - config->setDefaultViewTransformName(""); - - // Validate addViewTransform is blocked if it would create any collisions. - - // An alias must not collide with another view transform's name. - auto conflict = OCIO::ViewTransform::Create(OCIO::REFERENCE_SPACE_SCENE); - conflict->setName("display_vt"); - conflict->addAlias("scene_vt"); - OCIO_CHECK_NO_THROW(conflict->setTransform(OCIO::MatrixTransform::Create(), - OCIO::VIEWTRANSFORM_DIR_FROM_REFERENCE)); - OCIO_CHECK_THROW_WHAT(config->addViewTransform(conflict), OCIO::Exception, - "it has an alias 'scene_vt' that is already used by view " - "transform 'scene_vt'"); - - conflict->setName("another_vt"); - OCIO_CHECK_THROW_WHAT(config->addViewTransform(conflict), OCIO::Exception, - "it has an alias 'scene_vt' that is already used by view " - "transform 'scene_vt'"); - - // An alias must not collide with another view transform's alias. - conflict->clearAliases(); - conflict->addAlias("old_scene_vt"); - OCIO_CHECK_THROW_WHAT(config->addViewTransform(conflict), OCIO::Exception, - "it has an alias 'old_scene_vt' that is already used by view " - "transform 'scene_vt'"); - - // A name must not collide with another view transform's alias. - conflict->clearAliases(); - conflict->setName("old_scene_vt"); - OCIO_CHECK_THROW_WHAT(config->addViewTransform(conflict), OCIO::Exception, - "existing view transform 'scene_vt' is using this name as an alias"); - - // Aliases require config version 2.6 or higher. - - config->setVersion(2, 5); - OCIO_CHECK_THROW_WHAT(config->validate(), OCIO::Exception, - "The view transform 'scene_vt' has aliases and config version is " - "less than 2.6"); - config->setVersion(2, 6); - OCIO_CHECK_NO_THROW(config->validate()); - - // Aliases survive a serialize / reload round-trip. - - std::ostringstream os; - config->serialize(os); - - std::istringstream is; - is.str(os.str()); - - OCIO::ConstConfigRcPtr configReloaded; - OCIO_CHECK_NO_THROW(configReloaded = OCIO::Config::CreateFromStream(is)); - OCIO_CHECK_NO_THROW(configReloaded->validate()); - - auto reloadedVT = configReloaded->getViewTransform("scene_vt"); - OCIO_REQUIRE_ASSERT(reloadedVT); - OCIO_REQUIRE_EQUAL(reloadedVT->getNumAliases(), 1); - OCIO_CHECK_EQUAL(std::string(reloadedVT->getAlias(0)), "old_scene_vt"); - - OCIO_CHECK_ASSERT(configReloaded->getViewTransform("old_scene_vt")); - OCIO_CHECK_EQUAL(std::string(configReloaded->getViewTransform("old_scene_vt")->getName()), - "scene_vt"); -} - OCIO_ADD_TEST(Config, display_view) { // Create a config with a display that has 2 kinds of views. @@ -7522,90 +7412,43 @@ default_view_transform: view_transform OCIO::Exception, "a non-empty color space name is needed"); } -OCIO_ADD_TEST(Config, aliased_view_name) +OCIO_ADD_TEST(Config, view_aliases) { + // Views (display-defined or shared) may have their own aliases, set directly via + // Config::addDisplayView/addSharedView, and queried via Config::getDisplayViewAliases/ + // Config::hasDisplayViewAlias. This test covers that machinery: setting aliases, querying + // them, keeping them unambiguous, requiring config version 2.6, and surviving a YAML + // serialize/reload round-trip. See the aliased_view_name test below for how aliases are + // resolved by Config::getCanonicalViewName. + constexpr const char * SIMPLE_CONFIG{ R"( ocio_profile_version: 2.6 -use_display_view_aliases: true - -environment: - {} roles: - default: raw1 - aces_interchange: raw1 - cie_xyz_d65_interchange: display_cs - color_timing: raw1 - compositing_log: raw1 - scene_linear: raw1 - -view_transforms: - - ! - name: vt_new - aliases: [vt_old, vt_older] - from_scene_reference: ! {} + default: ref1 + aces_interchange: ref1 + color_timing: ref1 + compositing_log: ref1 + scene_linear: ref1 - - ! - name: vt_shared_new - aliases: [vt_shared_old] - from_scene_reference: ! {} - - - ! - name: vt_udn - aliases: [vt_udn_old] - from_scene_reference: ! {} - - - ! - name: vt_match - from_scene_reference: ! {} +shared_views: + - ! {name: sview, colorspace: ref1, aliases: [sv_alias]} -named_transforms: - - ! - name: nt_new - aliases: [nt_old, nt_older] - transform: ! {} +displays: + display1: + - ! {name: view1, colorspace: ref1, aliases: [v1_alias2, v1_alias]} + - ! {name: view2, colorspace: cs1} + - ! [sview] + display2: + - ! {name: view3, colorspace: ref1, aliases: [v1_alias]} + - ! [sview] colorspaces: - ! - name: display_scene - - - ! - name: raw1 - -display_colorspaces: - - ! - name: display_cs - aliases: [old_display_name, display_alias_cs] + name: ref1 - ! - name: dcs2 - aliases: [dcs_shared] - - - ! - name: dcs3 - -shared_views: - - ! {name: view_shared, view_transform: vt_shared_old, display_colorspace: dcs_shared} - - ! {name: view_udn, view_transform: vt_udn_old, display_colorspace: } - -displays: - display_cs: - - ! {name: view_act, colorspace: raw1} - - ! {name: view2, view_transform: vt_old, display_colorspace: dcs2} - - ! {name: view2b, view_transform: vt_old, display_colorspace: display_cs, - looks: lk1, rule: r1, description: foo} - - ! [view_shared, view_udn] - - ! {name: view_match_alias, view_transform: vt_match, - display_colorspace: display_alias_cs} - display_no_cs: - - ! {name: view_match, view_transform: vt_match, display_colorspace: display_cs} - - ! {name: view2, view_transform: vt_old, display_colorspace: dcs2} - - ! {name: view_nt, view_transform: nt_old, display_colorspace: dcs3} - - ! [view_shared] - display_scene: - - ! {name: view3, view_transform: vt_old, display_colorspace: dcs3} - -active_views: [view_act] + name: cs1 )" }; std::istringstream is; @@ -7613,121 +7456,246 @@ active_views: [view_act] OCIO::ConstConfigRcPtr config; OCIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromStream(is)); - // Most views are inactive, the tests confirm the fallback must still find it. - // Like Config::hasView, it must work regardless of whether the view is active. - OCIO_CHECK_EQUAL(config->getNumViews("display_cs"), 1); - OCIO_CHECK_ASSERT(config->hasView("display_cs", "view2b")); + // Test getDisplayViewAliases returns the aliases as a comma-delimited string (in view order). + OCIO_CHECK_EQUAL(std::string(config->getDisplayViewAliases("display1", "view1")), + "v1_alias2, v1_alias"); + // A view with no aliases, an unknown view, or an unknown display all return "". + OCIO_CHECK_EQUAL(std::string(config->getDisplayViewAliases("display1", "view2")), ""); + OCIO_CHECK_EQUAL(std::string(config->getDisplayViewAliases("display1", "not_a_view")), ""); + OCIO_CHECK_EQUAL(std::string(config->getDisplayViewAliases("not_a_display", "view1")), ""); + + // A null/empty display means look up the view among the config's shared views, following + // the same convention as some of the other getters. + OCIO_CHECK_EQUAL(std::string(config->getDisplayViewAliases(nullptr, "sview")), "sv_alias"); + OCIO_CHECK_EQUAL(std::string(config->getDisplayViewAliases("", "sview")), "sv_alias"); + + // Test hasDisplayViewAlias convenience method. + OCIO_CHECK_ASSERT(config->hasDisplayViewAlias("display1", "view1", "v1_alias")); + OCIO_CHECK_ASSERT(!config->hasDisplayViewAlias("display1", "view1", "not_an_alias")); + OCIO_CHECK_ASSERT(config->hasDisplayViewAlias(nullptr, "sview", "sv_alias")); + + // Aliases only need to be unique among the views used by a single display, so the same + // alias string ("v1_alias") can be reused by an unrelated view in a different display. + OCIO_CHECK_ASSERT(config->hasDisplayViewAlias("display2", "view3", "v1_alias")); + + // Aliases containing a comma must be quoted, following the same convention used for e.g. + // Config::setActiveViews. + { + OCIO::ConfigRcPtr edit = config->createEditableCopy(); + OCIO_CHECK_NO_THROW(edit->addDisplayView("display1", "view4", nullptr, "ref1", "", "", "", + R"("a,b",plain)")); + OCIO_CHECK_EQUAL(std::string(edit->getDisplayViewAliases("display1", "view4")), + R"("a,b", plain)"); + OCIO_CHECK_ASSERT(edit->hasDisplayViewAlias("display1", "view4", "a,b")); + OCIO_CHECK_ASSERT(edit->hasDisplayViewAlias("display1", "view4", "plain")); + + // The comma-containing alias survives a YAML serialize / reload round-trip, quoted as + // needed by the YAML emitter so that it re-parses back into a single alias rather than + // being split into two. + std::ostringstream os; + edit->serialize(os); + + std::istringstream reloadStream; + reloadStream.str(os.str()); + OCIO::ConstConfigRcPtr reloaded; + OCIO_CHECK_NO_THROW(reloaded = OCIO::Config::CreateFromStream(reloadStream)); + OCIO_CHECK_NO_THROW(reloaded->validate()); - // The fallback is opt-in and disabled by default (exact matches still work). - { - OCIO::ConfigRcPtr configNoAliases = config->createEditableCopy(); - OCIO_CHECK_NO_THROW(configNoAliases->setUseDisplayViewAliases(false)); + OCIO_CHECK_EQUAL(std::string(reloaded->getDisplayViewAliases("display1", "view4")), + R"("a,b", plain)"); + OCIO_CHECK_ASSERT(reloaded->hasDisplayViewAlias("display1", "view4", "a,b")); + OCIO_CHECK_ASSERT(reloaded->hasDisplayViewAlias("display1", "view4", "plain")); - OCIO_CHECK_ASSERT(!configNoAliases->getUseDisplayViewAliases()); - OCIO_CHECK_EQUAL(std::string( - configNoAliases->getCanonicalViewName("display_cs", "vt_new")), ""); - OCIO_CHECK_NO_THROW(configNoAliases->setUseDisplayViewAliases(true)); - OCIO_CHECK_EQUAL(std::string( - configNoAliases->getCanonicalViewName("display_cs", "vt_new")), "view2b"); + // The other, comma-free aliases also survive the round-trip. + OCIO_CHECK_EQUAL(std::string(reloaded->getDisplayViewAliases("display1", "view1")), + "v1_alias2, v1_alias"); + OCIO_CHECK_EQUAL(std::string(reloaded->getDisplayViewAliases(nullptr, "sview")), + "sv_alias"); } - OCIO_CHECK_ASSERT(config->getUseDisplayViewAliases()); - - // Normal case: "view_act" is already a view name. - OCIO_CHECK_EQUAL(std::string( - config->getCanonicalViewName("display_cs", "view_act")), "view_act"); - // Test without a display color space corresponding to the display. + // A view's alias must not collide with the name or alias of another view used by the same + // display, whether display-defined or a referenced shared view. This is checked as soon as + // the collision would occur. { - // The "vt_match" is not a view name, but it's a view_transform name. - OCIO_CHECK_EQUAL(std::string( - config->getCanonicalViewName("display_no_cs", "vt_match")), "view_match"); + OCIO::ConfigRcPtr edit = config->createEditableCopy(); - // The "view2" has "view_transform: vt_old", which is an alias for "vt_new". - OCIO_CHECK_EQUAL(std::string( - config->getCanonicalViewName("display_no_cs", "vt_new")), "view2"); + // Collides with view1's alias in display1. + OCIO_CHECK_THROW_WHAT( + edit->addDisplayView("display1", "view4", nullptr, "ref1", "", "", "", "v1_alias"), + OCIO::Exception, "already used as the name or alias"); - // Test where both the requested view and the view's view_transform are aliases. - OCIO_CHECK_EQUAL(std::string( - config->getCanonicalViewName("display_no_cs", "vt_older")), "view2"); + // Collides with the "sview" shared view's alias, which display1 references. + OCIO_CHECK_THROW_WHAT( + edit->addDisplayView("display1", "view4", nullptr, "ref1", "", "", "", "sv_alias"), + OCIO::Exception, "already used as the name or alias"); - // Test where the view_transform is a NamedTransform. - OCIO_CHECK_EQUAL(std::string( - config->getCanonicalViewName("display_no_cs", "nt_new")), "view_nt"); + // No collision in display2, since display2 does not use view1 or its aliases. + OCIO_CHECK_NO_THROW( + edit->addDisplayView("display2", "view4", nullptr, "ref1", "", "", "", "v1_alias2")); - // Test referring to an alias of a NamedTransform. - OCIO_CHECK_EQUAL(std::string( - config->getCanonicalViewName("display_no_cs", "nt_older")), "view_nt"); + // A shared view's alias must not collide with another shared view's name or alias, + // regardless of which displays reference either of them (this keeps shared views + // unambiguous when looked up directly, e.g. via a null display argument). + OCIO_CHECK_THROW_WHAT(edit->addSharedView("sview2", nullptr, "ref1", "", "", "", + "sv_alias"), + OCIO::Exception, "already used as the name or alias"); - // Test where the view is a shared view. - OCIO_CHECK_EQUAL(std::string( - config->getCanonicalViewName("display_no_cs", "vt_shared_new")), "view_shared"); + // Linking an existing shared view to a display is also checked against that display's + // own views. + OCIO_CHECK_NO_THROW(edit->addDisplayView("display3", "view5", "ref1", "")); + OCIO_CHECK_NO_THROW( + edit->addDisplayView("display3", "view5", nullptr, "ref1", "", "", "", "sv_alias")); + OCIO_CHECK_THROW_WHAT(edit->addDisplaySharedView("display3", "sview"), OCIO::Exception, + "already used as the name or alias"); } - // Now test when there is a display color space that corresponds to the display. In - // this case, the function does additional checking to ensure that it does not return - // a view that has a display_colorspace that differs from the display's color space + // Defensive backstop: addSharedView can replace an already-linked shared view's aliases, + // and that call has no way to know which displays currently reference it, so this + // particular collision is only caught later, when the config is validated. + { + OCIO::ConfigRcPtr edit = config->createEditableCopy(); + OCIO_CHECK_NO_THROW(edit->addDisplayView("display3", "view5", "ref1", "")); + OCIO_CHECK_NO_THROW(edit->addDisplaySharedView("display3", "sview")); + + // No collision yet: "view5" has no alias, and "sview"'s alias is "sv_alias". + OCIO_CHECK_NO_THROW(edit->validate()); - // Repeat the test from above and confirm that "view2" is no longer an option (even - // though it appears earlier in the config) because its display_colorspace is different - // from that of the display. - OCIO_CHECK_EQUAL(std::string( - config->getCanonicalViewName("display_cs", "vt_older")), "view2b"); + // Redefine "sview" so its alias now collides with "view5" in display3. addSharedView + // does not throw, since it only checks against sibling shared views. + OCIO_CHECK_NO_THROW(edit->addSharedView("sview", nullptr, "ref1", "", "", "", "view5")); + OCIO_CHECK_THROW_WHAT(edit->validate(), OCIO::Exception, "collides with the name or " + "alias"); + } - // Now remove view2b and confirm that no match is found. + // View aliases require ocio_profile_version 2.6 or higher. { - OCIO::ConfigRcPtr configNoView2b = config->createEditableCopy(); - OCIO_CHECK_NO_THROW(configNoView2b->removeDisplayView("display_cs", "view2b")); + constexpr const char * OLD_CONFIG{ R"( +ocio_profile_version: 2.5 - OCIO_CHECK_EQUAL(std::string( - configNoView2b->getCanonicalViewName("display_cs", "vt_older")), ""); +roles: + default: ref1 + aces_interchange: ref1 + color_timing: ref1 + compositing_log: ref1 + scene_linear: ref1 + +colorspaces: + - ! + name: ref1 + +displays: + disp: + - ! {name: view1, colorspace: ref1} +)" }; + std::istringstream isOld; + isOld.str(OLD_CONFIG); + OCIO::ConfigRcPtr oldConfig; + OCIO_CHECK_NO_THROW( + oldConfig = OCIO::Config::CreateFromStream(isOld)->createEditableCopy()); + OCIO_CHECK_NO_THROW( + oldConfig->addDisplayView("disp", "view1", nullptr, "ref1", "", "", "", "alias1")); + OCIO_CHECK_THROW_WHAT(oldConfig->validate(), OCIO::Exception, "less than 2.6"); } +} + +OCIO_ADD_TEST(Config, aliased_view_name) +{ + // Test that Config::getCanonicalViewName resolves a view by one of its aliases. Unlike + // display name aliasing (see the aliased_display_name test below), this resolution is + // always active, i.e. it does not depend on Config::getUseDisplayAliases. See the + // view_aliases test above for the machinery of setting and querying view aliases. + + constexpr const char * SIMPLE_CONFIG{ R"( +ocio_profile_version: 2.6 + +roles: + default: ref1 + aces_interchange: ref1 + color_timing: ref1 + compositing_log: ref1 + scene_linear: ref1 + +shared_views: + - ! {name: sview, colorspace: ref1, aliases: [sv_alias]} + +displays: + display1: + - ! {name: view1, colorspace: ref1, aliases: [v1_alias, v1_alias2]} + - ! {name: view2, colorspace: cs1} + - ! [sview] + display2: + - ! {name: view3, colorspace: ref1, aliases: [v1_alias]} + - ! [sview] + act_display: + - ! {name: act_view, colorspace: cs1} + +# The view aliasing works even if the display and/or view are inactive. +active_displays: [act_display] +active_views: [act_view] + +colorspaces: + - ! + name: ref1 + + - ! + name: cs1 +)" }; - // Verify that a display name alias is resolved as well. - OCIO_CHECK_EQUAL(std::string( - config->getCanonicalViewName("old_display_name", "vt_older")), "view2b"); + std::istringstream is; + is.str(SIMPLE_CONFIG); + OCIO::ConstConfigRcPtr config; + OCIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromStream(is)); - // Test that the display_colorspace resolution succeeds, even if it's an alias. - OCIO_CHECK_EQUAL(std::string( - config->getCanonicalViewName("display_cs", "vt_match")), "view_match_alias"); + // Alias resolution is always active, regardless of getUseDisplayAliases (which only + // gates display name aliasing). + OCIO_CHECK_ASSERT(!config->getUseDisplayAliases()); - // A candidate using as its display_colorspace is always accepted: by - // definition, its color space is whichever one is named after the display. - OCIO_CHECK_EQUAL(std::string( - config->getCanonicalViewName("display_cs", "vt_udn")), "view_udn"); + // Resolve a display-defined view by one of its aliases. + OCIO_CHECK_EQUAL(std::string(config->getCanonicalViewName("display1", "v1_alias")), "view1"); + OCIO_CHECK_EQUAL(std::string(config->getCanonicalViewName("display1", "v1_alias2")), "view1"); - // The display_scene is both a display name and scene-referred color space, which is something - // that could happen (e.g. "sRGB"). In this case, avoid the comparison against the view's - // display_colorspace. There should be no comparison of display_scene and dcs3. - OCIO_CHECK_EQUAL(std::string( - config->getCanonicalViewName("display_scene", "vt_new")), "view3"); + // Resolve a shared view by its alias, through either display that references it. + OCIO_CHECK_EQUAL(std::string(config->getCanonicalViewName("display1", "sv_alias")), "sview"); + OCIO_CHECK_EQUAL(std::string(config->getCanonicalViewName("display2", "sv_alias")), "sview"); - // No match at all. - OCIO_CHECK_EQUAL(std::string(config->getCanonicalViewName("display_cs", "not a view")), ""); + // Aliases only need to be unique among the views used by a single display, so the same + // alias string ("v1_alias") can be reused by an unrelated view in a different display. + OCIO_CHECK_EQUAL(std::string(config->getCanonicalViewName("display2", "v1_alias")), "view3"); - // Null/empty display or view. - OCIO_CHECK_EQUAL(std::string(config->getCanonicalViewName("display_cs", "")), ""); - OCIO_CHECK_EQUAL(std::string(config->getCanonicalViewName("display_cs", nullptr)), ""); + // An exact view name always takes priority and needs no alias resolution. + OCIO_CHECK_EQUAL(std::string(config->getCanonicalViewName("display1", "view2")), "view2"); + + // No match at all: unknown alias, unknown display, or empty/null arguments. + OCIO_CHECK_EQUAL(std::string(config->getCanonicalViewName("display1", "not_a_view")), ""); + OCIO_CHECK_EQUAL(std::string(config->getCanonicalViewName("not_a_display", "v1_alias")), ""); + OCIO_CHECK_EQUAL(std::string(config->getCanonicalViewName("display1", "")), ""); + OCIO_CHECK_EQUAL(std::string(config->getCanonicalViewName("display1", nullptr)), ""); OCIO_CHECK_EQUAL(std::string(config->getCanonicalViewName("", "view1")), ""); OCIO_CHECK_EQUAL(std::string(config->getCanonicalViewName(nullptr, "view1")), ""); + // A comma-containing alias also resolves. + { + OCIO::ConfigRcPtr edit = config->createEditableCopy(); + OCIO_CHECK_NO_THROW(edit->addDisplayView("display1", "view4", nullptr, "ref1", "", "", "", + R"("a,b",plain)")); + OCIO_CHECK_EQUAL(std::string(edit->getCanonicalViewName("display1", "a,b")), "view4"); + } + // Unlike getCanonicalViewName, the other Config methods that take a view name are not meant - // to resolve it against a view transform alias. These functions are meant to tell exactly - // what the Config object contains, which would become less clear if they resolved aliases. - OCIO_CHECK_EQUAL(std::string(config->getCanonicalViewName("display_cs", "vt_old")), "view2b"); - OCIO_CHECK_ASSERT(config->hasView("display_cs", "view2")); - OCIO_CHECK_ASSERT(!config->hasView("display_cs", "vt_new")); - OCIO_CHECK_ASSERT(!config->hasView("display_cs", "vt_old")); - OCIO_CHECK_ASSERT(!config->hasView("display_cs", "vt_older")); - - OCIO_CHECK_EQUAL(std::string(config->getDisplayViewTransformName("display_cs", "vt_old")), ""); - OCIO_CHECK_EQUAL(std::string(config->getDisplayViewColorSpaceName("display_cs", "vt_old")), ""); - OCIO_CHECK_EQUAL(std::string(config->getDisplayViewLooks("display_cs", "vt_old")), ""); - OCIO_CHECK_EQUAL(std::string(config->getDisplayViewRule("display_cs", "vt_old")), ""); - OCIO_CHECK_EQUAL(std::string(config->getDisplayViewDescription("display_cs", "vt_old")), ""); + // to resolve it against a view alias. These functions are meant to tell exactly what the + // Config object contains, which would become less clear if they resolved aliases. + OCIO_CHECK_ASSERT(config->hasView("display1", "view1")); + OCIO_CHECK_ASSERT(!config->hasView("display1", "v1_alias")); + + OCIO_CHECK_EQUAL(std::string(config->getDisplayViewTransformName("display1", "v1_alias")), ""); + OCIO_CHECK_EQUAL(std::string(config->getDisplayViewColorSpaceName("display1", "v1_alias")), ""); + OCIO_CHECK_EQUAL(std::string(config->getDisplayViewLooks("display1", "v1_alias")), ""); + OCIO_CHECK_EQUAL(std::string(config->getDisplayViewRule("display1", "v1_alias")), ""); + OCIO_CHECK_EQUAL(std::string(config->getDisplayViewDescription("display1", "v1_alias")), ""); OCIO::ConfigRcPtr configEdit = config->createEditableCopy(); - OCIO_CHECK_THROW_WHAT(configEdit->removeDisplayView("display_cs", "vt_old"), OCIO::Exception, - "Could not find a view named 'vt_old"); + OCIO_CHECK_THROW_WHAT(configEdit->removeDisplayView("display1", "v1_alias"), OCIO::Exception, + "Could not find a view named 'v1_alias"); } OCIO_ADD_TEST(Config, aliased_display_name) @@ -7748,19 +7716,28 @@ OCIO_ADD_TEST(Config, aliased_display_name) OCIO_CHECK_NO_THROW(config->addDisplayView("sRGB - Display", "view1", "raw", "")); - // The fallback is opt-in and disabled by default. - OCIO_CHECK_ASSERT(!config->getUseDisplayViewAliases()); - OCIO_CHECK_EQUAL(std::string(config->getCanonicalDisplayName("sRGB")), ""); - - config->setUseDisplayViewAliases(true); - // Exact, case-insensitive match. OCIO_CHECK_EQUAL(std::string(config->getCanonicalDisplayName("sRGB - Display")), "sRGB - Display"); OCIO_CHECK_EQUAL(std::string(config->getCanonicalDisplayName("srgb - display")), "sRGB - Display"); - // Resolved via the display color space's alias. + // Display aliasing is opt-in and disabled by default. + OCIO_CHECK_ASSERT(!config->getUseDisplayAliases()); + config->setUseDisplayAliases(true); + + // Resolving via the display color space's alias also requires that the display actually use + // that color space in one of its views. So far "sRGB - Display" only has view1, whose color + // space is "raw", so there is no connection to the display color space and the match fails. + OCIO_CHECK_EQUAL(std::string(config->getCanonicalDisplayName("sRGB")), ""); + + // Add a view that uses the display color space itself, satisfying that requirement. + OCIO_CHECK_NO_THROW(config->addDisplayView("sRGB - Display", "view2", "sRGB - Display", "")); + // Now the alias works. + OCIO_CHECK_EQUAL(std::string(config->getCanonicalDisplayName("sRGB")), "sRGB - Display"); + + // Setting display_colorspace to satisfies the requirement as well. + OCIO_CHECK_NO_THROW(config->addDisplayView("sRGB - Display", "view2", "", "")); OCIO_CHECK_EQUAL(std::string(config->getCanonicalDisplayName("sRGB")), "sRGB - Display"); // If there is a display named "sRGB" added, make sure it returns that one. @@ -7769,25 +7746,51 @@ OCIO_ADD_TEST(Config, aliased_display_name) OCIO_CHECK_NO_THROW(config->removeDisplayView("sRGB", "view1")); OCIO_CHECK_EQUAL(std::string(config->getCanonicalDisplayName("sRGB")), "sRGB - Display"); - // If the resolved color space's own name doesn't match any display, its aliases are tried - // too. Here "other_dcs" is a different display color space than the one associated with - // "sRGB - Display": its own name matches no display, but one of its aliases does. + // If the display color space's own name doesn't match any display, its aliases are tried + // too. Here the name of "other_dcs" matches no display, but one of its aliases does. OCIO_CHECK_NO_THROW(config->addDisplayView("AliasedDisplay", "view1", "raw", "")); auto otherDcs = OCIO::ColorSpace::Create(OCIO::REFERENCE_SPACE_DISPLAY); otherDcs->setName("other_dcs"); otherDcs->addAlias("AliasedDisplay"); otherDcs->addAlias("other_alias"); + otherDcs->addAlias("another_alias"); OCIO_CHECK_NO_THROW(config->addColorSpace(otherDcs)); + + // As above, the name match alone isn't enough: "AliasedDisplay" only has view1, whose + // colorspace is "raw", so it isn't yet found to be using "other_dcs". + OCIO_CHECK_EQUAL(std::string(config->getCanonicalDisplayName("other_dcs")), ""); + + // Add a view that uses the display color space, satisfying that requirement. Note that + // the display_colorspace may resolve via an alias of the display color space. + OCIO_CHECK_NO_THROW(config->addDisplayView("AliasedDisplay", "view2", "other_alias", "")); OCIO_CHECK_EQUAL(std::string(config->getCanonicalDisplayName("other_dcs")), "AliasedDisplay"); - // Similarly any alias of other_dcs will find the AliasedDisplay. - OCIO_CHECK_EQUAL(std::string(config->getCanonicalDisplayName("other_alias")), "AliasedDisplay"); + // Any alias of other_dcs will find the AliasedDisplay. + OCIO_CHECK_EQUAL(std::string(config->getCanonicalDisplayName("another_alias")), "AliasedDisplay"); // Don't resolve against role names. OCIO_CHECK_NO_THROW(config->setRole("display_role", "sRGB - Display")); OCIO_CHECK_EQUAL(std::string(config->getCanonicalDisplayName("display_role")), ""); config->setRole("display_role", nullptr); + // The fallback is opt-in. + config->setUseDisplayAliases(false); + OCIO_CHECK_EQUAL(std::string(config->getCanonicalDisplayName("sRGB")), ""); + config->setUseDisplayAliases(true); + + // The alias resolution still works even if the display being found is not part of the + // active_displays list. + OCIO_CHECK_NO_THROW(config->setActiveDisplays("not_a_real_display")); + OCIO_CHECK_EQUAL(std::string(config->getCanonicalDisplayName("sRGB")), "sRGB - Display"); + OCIO_CHECK_EQUAL(std::string(config->getCanonicalDisplayName("other_dcs")), "AliasedDisplay"); + config->clearActiveDisplays(); + + // It also still works even if the display color space is in the inactive_colorspaces list. + OCIO_CHECK_NO_THROW(config->setInactiveColorSpaces("sRGB - Display, other_dcs")); + OCIO_CHECK_EQUAL(std::string(config->getCanonicalDisplayName("sRGB")), "sRGB - Display"); + OCIO_CHECK_EQUAL(std::string(config->getCanonicalDisplayName("other_dcs")), "AliasedDisplay"); + config->setInactiveColorSpaces(""); + // Replace the display color space with a scene-referred one of the same name, keeping the // same "sRGB" alias. Since it is no longer display-referred, "sRGB" must no longer resolve // to the display, even though the color space's canonical name still matches it exactly. @@ -7830,13 +7833,13 @@ OCIO_ADD_TEST(Config, display_description) config->addColorSpace(dcs); // Exact match works even though the fallback is disabled by default. - OCIO_CHECK_ASSERT(!config->getUseDisplayViewAliases()); + OCIO_CHECK_ASSERT(!config->getUseDisplayAliases()); OCIO_CHECK_EQUAL(std::string(config->getDisplayDescription("sRGB - Display")), "The sRGB display."); // Matching via the color space's alias requires the switch. OCIO_CHECK_EQUAL(std::string(config->getDisplayDescription("sRGB")), ""); - config->setUseDisplayViewAliases(true); + config->setUseDisplayAliases(true); OCIO_CHECK_EQUAL(std::string(config->getDisplayDescription("sRGB")), "The sRGB display."); // A color space that isn't display-referred doesn't count, even with a matching name. @@ -7870,7 +7873,6 @@ OCIO_ADD_TEST(Config, resolved_display_view_color_space_name) auto vt = OCIO::ViewTransform::Create(OCIO::REFERENCE_SPACE_SCENE); vt->setName("vt_new"); - vt->addAlias("vt_old"); OCIO_CHECK_NO_THROW(vt->setTransform(OCIO::MatrixTransform::Create(), OCIO::VIEWTRANSFORM_DIR_FROM_REFERENCE)); OCIO_CHECK_NO_THROW(config->addViewTransform(vt)); @@ -7878,44 +7880,47 @@ OCIO_ADD_TEST(Config, resolved_display_view_color_space_name) // A plain view, with just a color space. OCIO_CHECK_NO_THROW(config->addDisplayView("sRGB - Display", "view1", "raw", "")); - // A view with a view transform, whose display color space is . It refers to - // the view transform by its old alias, so getCanonicalViewName can find this view from the - // view transform's current name. - OCIO_CHECK_NO_THROW(config->addDisplayView("sRGB - Display", "view2", "vt_old", - "", "", "", "")); + // A view with a view transform, whose display color space is . It has its + // own alias, "view2_old", so getCanonicalViewName can find it from that old name. + OCIO_CHECK_NO_THROW(config->addDisplayView("sRGB - Display", "view2", "vt_new", + "", "", "", "", "view2_old")); // The plain case behaves like getDisplayViewColorSpaceName. OCIO_CHECK_EQUAL(std::string(config->getResolvedDisplayViewColorSpaceName("sRGB - Display", "view1")), "raw"); // is resolved to the display's own display color space, rather than being - // returned as the placeholder string. This does not require getUseDisplayViewAliases. - OCIO_CHECK_ASSERT(!config->getUseDisplayViewAliases()); + // returned as the placeholder string. This does not require getUseDisplayAliases. + OCIO_CHECK_ASSERT(!config->getUseDisplayAliases()); OCIO_CHECK_EQUAL(std::string(config->getDisplayViewColorSpaceName("sRGB - Display", "view2")), ""); OCIO_CHECK_EQUAL(std::string(config->getResolvedDisplayViewColorSpaceName("sRGB - Display", "view2")), "sRGB - Display"); - // Resolving the display and the view is opt-in, like getCanonicalDisplayName and - // getCanonicalViewName themselves. + // Resolving a view by one of its own aliases is always active, unlike resolving the display + // name, which is opt-in (see getCanonicalDisplayName/getCanonicalViewName). + OCIO_CHECK_EQUAL(std::string(config->getResolvedDisplayViewColorSpaceName("sRGB - Display", + "view2_old")), + "sRGB - Display"); + + // Resolving the display is opt-in. OCIO_CHECK_EQUAL(std::string(config->getResolvedDisplayViewColorSpaceName("sRGB", "view1")), ""); - OCIO_CHECK_EQUAL(std::string(config->getResolvedDisplayViewColorSpaceName("sRGB - Display", - "vt_new")), ""); - config->setUseDisplayViewAliases(true); + config->setUseDisplayAliases(true); // The display is resolved from the old name kept as an alias on its display color space. OCIO_CHECK_EQUAL(std::string(config->getResolvedDisplayViewColorSpaceName("sRGB", "view1")), "raw"); - // The view is resolved from the view transform it uses, and the display may be out of date at + // The view is still resolved from its own alias, and the display may be out of date at // the same time. OCIO_CHECK_EQUAL(std::string(config->getResolvedDisplayViewColorSpaceName("sRGB - Display", - "vt_new")), + "view2_old")), "sRGB - Display"); - OCIO_CHECK_EQUAL(std::string(config->getResolvedDisplayViewColorSpaceName("sRGB", "vt_new")), + OCIO_CHECK_EQUAL(std::string(config->getResolvedDisplayViewColorSpaceName("sRGB", + "view2_old")), "sRGB - Display"); // A view whose colorspace attribute is an alias of a color space returns the canonical name @@ -8535,11 +8540,11 @@ ocio_profile_version: 2.6 OCIO_CHECK_ASSERT(!config->isColorSpaceUsed("unused_new")); OCIO_CHECK_ASSERT(!config->isColorSpaceUsed("unused_old")); - // None of the above depends on Config::getUseDisplayViewAliases, which was off. + // None of the above depends on Config::getUseDisplayAliases, which was off. // Turning it on must not change any result. OCIO::ConfigRcPtr editableConfig = config->createEditableCopy(); - OCIO_CHECK_ASSERT(!editableConfig->getUseDisplayViewAliases()); - OCIO_CHECK_NO_THROW(editableConfig->setUseDisplayViewAliases(true)); + OCIO_CHECK_ASSERT(!editableConfig->getUseDisplayAliases()); + OCIO_CHECK_NO_THROW(editableConfig->setUseDisplayAliases(true)); OCIO_CHECK_NO_THROW(editableConfig->validate()); OCIO_CHECK_ASSERT(editableConfig->isColorSpaceUsed("view_new")); diff --git a/tests/cpu/ViewTransform_tests.cpp b/tests/cpu/ViewTransform_tests.cpp index c61f3ce4b4..0055d3ecca 100644 --- a/tests/cpu/ViewTransform_tests.cpp +++ b/tests/cpu/ViewTransform_tests.cpp @@ -68,74 +68,3 @@ OCIO_ADD_TEST(ViewTransform, basic) OCIO_REQUIRE_ASSERT(vtd); OCIO_CHECK_EQUAL(OCIO::REFERENCE_SPACE_DISPLAY, vtd->getReferenceSpaceType()); } - -OCIO_ADD_TEST(ViewTransform, aliases) -{ - OCIO::ViewTransformRcPtr vt = OCIO::ViewTransform::Create(OCIO::REFERENCE_SPACE_SCENE); - OCIO_REQUIRE_ASSERT(vt); - OCIO_CHECK_EQUAL(vt->getNumAliases(), 0); - constexpr char AliasA[]{ "aliasA" }; - constexpr char AliasAAlt[]{ "aLiaSa" }; - constexpr char AliasB[]{ "aliasB" }; - vt->addAlias(AliasA); - OCIO_CHECK_EQUAL(vt->getNumAliases(), 1); - OCIO_CHECK_ASSERT(vt->hasAlias(AliasA)); - OCIO_CHECK_ASSERT(vt->hasAlias(AliasAAlt)); - OCIO_CHECK_ASSERT(!vt->hasAlias(AliasB)); - vt->addAlias(AliasB); - OCIO_CHECK_EQUAL(vt->getNumAliases(), 2); - OCIO_CHECK_EQUAL(std::string(vt->getAlias(0)), AliasA); - OCIO_CHECK_EQUAL(std::string(vt->getAlias(1)), AliasB); - OCIO_CHECK_ASSERT(vt->hasAlias(AliasB)); - - // Alias with same name (different case) already exists, do nothing. - - vt->addAlias(AliasAAlt); - OCIO_CHECK_EQUAL(vt->getNumAliases(), 2); - OCIO_CHECK_EQUAL(std::string(vt->getAlias(0)), AliasA); - OCIO_CHECK_EQUAL(std::string(vt->getAlias(1)), AliasB); - - // Remove alias. - - vt->removeAlias(AliasAAlt); - OCIO_CHECK_EQUAL(vt->getNumAliases(), 1); - OCIO_CHECK_EQUAL(std::string(vt->getAlias(0)), AliasB); - OCIO_CHECK_ASSERT(!vt->hasAlias(AliasA)); - OCIO_CHECK_ASSERT(!vt->hasAlias(AliasAAlt)); - - // Add with new case. - - vt->addAlias(AliasAAlt); - OCIO_CHECK_EQUAL(vt->getNumAliases(), 2); - OCIO_CHECK_EQUAL(std::string(vt->getAlias(0)), AliasB); - OCIO_CHECK_EQUAL(std::string(vt->getAlias(1)), AliasAAlt); - OCIO_CHECK_ASSERT(vt->hasAlias(AliasA)); - OCIO_CHECK_ASSERT(vt->hasAlias(AliasAAlt)); - - // Setting the name of the view transform to one of its aliases removes the alias. - - vt->setName(AliasA); - OCIO_CHECK_EQUAL(std::string(vt->getName()), AliasA); - OCIO_CHECK_EQUAL(vt->getNumAliases(), 1); - OCIO_CHECK_EQUAL(std::string(vt->getAlias(0)), AliasB); - OCIO_CHECK_ASSERT(!vt->hasAlias(AliasA)); - OCIO_CHECK_ASSERT(!vt->hasAlias(AliasAAlt)); - - // Alias is not added if it is already the view transform name. - - vt->addAlias(AliasAAlt); - OCIO_CHECK_EQUAL(std::string(vt->getName()), AliasA); - OCIO_CHECK_EQUAL(vt->getNumAliases(), 1); - OCIO_CHECK_EQUAL(std::string(vt->getAlias(0)), AliasB); - OCIO_CHECK_ASSERT(!vt->hasAlias(AliasAAlt)); - - // Remove all aliases. - - vt->addAlias("other"); - OCIO_CHECK_EQUAL(vt->getNumAliases(), 2); - OCIO_CHECK_ASSERT(vt->hasAlias("other")); - vt->clearAliases(); - OCIO_CHECK_EQUAL(vt->getNumAliases(), 0); - OCIO_CHECK_ASSERT(!vt->hasAlias(AliasB)); - OCIO_CHECK_ASSERT(!vt->hasAlias("other")); -} diff --git a/tests/cpu/apphelpers/LegacyViewingPipeline_tests.cpp b/tests/cpu/apphelpers/LegacyViewingPipeline_tests.cpp index e0bd834a8a..34ddb80346 100644 --- a/tests/cpu/apphelpers/LegacyViewingPipeline_tests.cpp +++ b/tests/cpu/apphelpers/LegacyViewingPipeline_tests.cpp @@ -968,7 +968,7 @@ OCIO_ADD_TEST(LegacyViewingPipeline, display_view_alias_fallback) constexpr char CONFIG[]{ R"( ocio_profile_version: 2.6 -use_display_view_aliases: true +use_display_aliases: true roles: default: raw @@ -980,7 +980,8 @@ use_display_view_aliases: true displays: sRGB - Display: - - ! {name: view, view_transform: display_vt, display_colorspace: sRGB - Display, looks: look1} + - ! {name: view, view_transform: display_vt, display_colorspace: sRGB - Display, + looks: look1, aliases: [old_view]} looks: - ! @@ -991,7 +992,6 @@ use_display_view_aliases: true view_transforms: - ! name: display_vt - aliases: [old_vt] to_scene_reference: ! {offset: [0.3, 0.1, 0.1, 0]} display_colorspaces: @@ -1064,11 +1064,11 @@ use_display_view_aliases: true OCIO_CHECK_CLOSE(aliasedDisplay[2], ref[2], tolerance); } - // So must the view transform's old, alias-only name used as the view. This is the case that - // would silently lose the look if the view were not resolved. + // So must the view's old, alias-only name used as the view. This is the case that would + // silently lose the look if the view were not resolved. { float aliasedView[3]{ srcPixel[0], srcPixel[1], srcPixel[2] }; - OCIO_CHECK_NO_THROW(applyPipeline("sRGB - Display", "old_vt", false, aliasedView)); + OCIO_CHECK_NO_THROW(applyPipeline("sRGB - Display", "old_view", false, aliasedView)); OCIO_CHECK_CLOSE(aliasedView[0], ref[0], tolerance); OCIO_CHECK_CLOSE(aliasedView[1], ref[1], tolerance); @@ -1078,7 +1078,7 @@ use_display_view_aliases: true // And both being out of date at once, since the view is resolved against the resolved display. { float bothAliased[3]{ srcPixel[0], srcPixel[1], srcPixel[2] }; - OCIO_CHECK_NO_THROW(applyPipeline("sRGB", "old_vt", false, bothAliased)); + OCIO_CHECK_NO_THROW(applyPipeline("sRGB", "old_view", false, bothAliased)); OCIO_CHECK_CLOSE(bothAliased[0], ref[0], tolerance); OCIO_CHECK_CLOSE(bothAliased[1], ref[1], tolerance); diff --git a/tests/cpu/transforms/DisplayViewTransform_tests.cpp b/tests/cpu/transforms/DisplayViewTransform_tests.cpp index 66cd32e952..be61ad0094 100644 --- a/tests/cpu/transforms/DisplayViewTransform_tests.cpp +++ b/tests/cpu/transforms/DisplayViewTransform_tests.cpp @@ -1495,7 +1495,9 @@ environment: { FILE: cdl_test1.cc } OCIO_ADD_TEST(DisplayViewTransform, use_display_name_alias) { // Test that USE_DISPLAY_NAME will find a display color space where the display name - // is an alias. + // is an alias. Note that this has nothing to do with whether use_display_aliases + // is set. It is simply a consequence of the fact that USE_DISPLAY_NAME is resolved + // via a call to config->getColorSpace(displayName), which handles aliases. constexpr const char * SIMPLE_CONFIG{ R"( ocio_profile_version: 2 @@ -1509,6 +1511,8 @@ ocio_profile_version: 2 displays: sRGB - Display: - ! [view1] + # Note that the "sRGB" display needs to be present. This test relies on color space aliases + # rather than display aliases. sRGB: - ! [view1] @@ -1582,7 +1586,7 @@ OCIO_ADD_TEST(DisplayViewTransform, display_alias_fallback) ocio_profile_version: 2.6 # Opt-in to display aliasing. -use_display_view_aliases: true +use_display_aliases: true roles: default: raw @@ -1674,15 +1678,12 @@ use_display_view_aliases: true OCIO_ADD_TEST(DisplayViewTransform, view_alias_fallback) { // Validate that BuildDisplayOps resolves a view using Config::getCanonicalViewName, so - // that renaming a view in a config (while keeping the old name as an alias) doesn't - // break a DisplayViewTransform still using the old name as its "view". + // that renaming a view in a config (while keeping the old name as one of the view's + // aliases) doesn't break a DisplayViewTransform still using the old name as its "view". constexpr char CONFIG[]{ R"( ocio_profile_version: 2.6 -# Opt-in to view aliasing. -use_display_view_aliases: true - roles: default: raw aces_interchange: raw @@ -1693,7 +1694,8 @@ use_display_view_aliases: true displays: sRGB - Display: - - ! {name: view, view_transform: display_vt, display_colorspace: sRGB - Display, looks: look1} + - ! {name: view, view_transform: display_vt, display_colorspace: sRGB - Display, + looks: look1, aliases: [old_view]} looks: - ! @@ -1704,7 +1706,6 @@ use_display_view_aliases: true view_transforms: - ! name: display_vt - aliases: [old_vt] to_scene_reference: ! {offset: [0.3, 0.1, 0.1, 0]} display_colorspaces: @@ -1731,10 +1732,10 @@ use_display_view_aliases: true const std::string display{ "sRGB - Display" }; const std::string view{ "view" }; - const std::string oldViewTransformName{ "old_vt" }; + const std::string oldViewName{ "old_view" }; OCIO_CHECK_EQUAL(std::string(config->getCanonicalViewName(display.c_str(), - oldViewTransformName.c_str())), + oldViewName.c_str())), view); auto dt = OCIO::DisplayViewTransform::Create(); @@ -1749,10 +1750,10 @@ use_display_view_aliases: true OCIO_CHECK_NO_THROW(currentOps.validate()); OCIO_REQUIRE_EQUAL(currentOps.size(), 7); // (includes gpu allocation no-ops) - // Build again using only the view transform's old, alias-only name as the "view". The result - // must be identical. Note that the view has a look, so this also verifies that the resolved - // view name is used to look up the view's looks, not just its color space and view transform. - dt->setView(oldViewTransformName.c_str()); + // Build again using only the view's old, alias-only name as the "view". The result must be + // identical. Note that the view has a look, so this also verifies that the resolved view + // name is used to look up the view's looks, not just its color space and view transform. + dt->setView(oldViewName.c_str()); OCIO::OpRcPtrVec aliasedOps; OCIO_CHECK_NO_THROW(OCIO::BuildDisplayOps(aliasedOps, *config, config->getCurrentContext(), *dt, OCIO::TRANSFORM_DIR_FORWARD)); @@ -1766,9 +1767,9 @@ use_display_view_aliases: true OCIO_CHECK_ASSERT(*opA->data() == *opB->data()); } - // A view name that cannot be resolved at all -- not an existing view, and not a view - // transform or named transform name or alias used by this display either -- must still - // throw, referencing the name the caller actually provided. + // A view name that cannot be resolved at all -- not an existing view, and not an alias of + // a view used by this display either -- must still throw, referencing the name the caller + // actually provided. dt->setView("not a view"); OCIO::OpRcPtrVec badOps; OCIO_CHECK_THROW_WHAT(OCIO::BuildDisplayOps(badOps, *config, config->getCurrentContext(), @@ -1787,7 +1788,7 @@ OCIO_ADD_TEST(DisplayViewTransform, context_variables_with_resolved_display_view constexpr const char * OCIO_CONFIG{ R"( ocio_profile_version: 2.6 -use_display_view_aliases: true +use_display_aliases: true environment: { FILE: cdl_test1.cc } @@ -1850,6 +1851,7 @@ environment: { FILE: cdl_test1.cc } dt->setView("plain_view"); OCIO_CHECK_ASSERT(!CollectContextVariables(*cfg, *cfg->getCurrentContext(), *dt, usedContextVars)); + OCIO_CHECK_EQUAL(0, usedContextVars->getNumStringVars()); // The shared view's display color space is , so finding the context // variable requires resolving that to the display color space named after the display. @@ -1857,10 +1859,14 @@ environment: { FILE: cdl_test1.cc } dt->setView("view"); OCIO_CHECK_ASSERT(CollectContextVariables(*cfg, *cfg->getCurrentContext(), *dt, usedContextVars)); + OCIO_CHECK_EQUAL(1, usedContextVars->getNumStringVars()); + OCIO_CHECK_EQUAL(std::string("FILE"), usedContextVars->getStringVarNameByIndex(0)); // The display's old, alias-only name is resolved too, so the same context variable is found. // The context variable is found: returns true. dt->setDisplay("sRGB"); OCIO_CHECK_ASSERT(CollectContextVariables(*cfg, *cfg->getCurrentContext(), *dt, usedContextVars)); + OCIO_CHECK_EQUAL(1, usedContextVars->getNumStringVars()); + OCIO_CHECK_EQUAL(std::string("FILE"), usedContextVars->getStringVarNameByIndex(0)); } diff --git a/tests/python/ConfigTest.py b/tests/python/ConfigTest.py index f0992fd1f4..3b069c8b95 100644 --- a/tests/python/ConfigTest.py +++ b/tests/python/ConfigTest.py @@ -798,20 +798,22 @@ def test_canonical_name(self): self.assertEqual(cfg.getCanonicalName('Alias1'), 'nt1') self.assertEqual(cfg.getCanonicalName('Test1'), 'nt1') - def test_use_display_view_aliases(self): - # Test the getUseDisplayViewAliases/setUseDisplayViewAliases methods, and that they gate - # the getCanonicalDisplayName/getCanonicalViewName fallbacks. + def test_use_display_aliases(self): + # Test the getUseDisplayAliases/setUseDisplayAliases methods, and that they gate + # the getCanonicalDisplayName fallback, but not view alias resolution, which is + # always active since a view's aliases are an explicit attribute of the view. cfg = OCIO.Config() - self.assertFalse(cfg.getUseDisplayViewAliases()) + self.assertFalse(cfg.getUseDisplayAliases()) - cfg.setUseDisplayViewAliases(True) - self.assertTrue(cfg.getUseDisplayViewAliases()) - cfg.setUseDisplayViewAliases(False) - self.assertFalse(cfg.getUseDisplayViewAliases()) + cfg.setUseDisplayAliases(True) + self.assertTrue(cfg.getUseDisplayAliases()) + cfg.setUseDisplayAliases(False) + self.assertFalse(cfg.getUseDisplayAliases()) - # Build a config where a display and a view transform have both been renamed, keeping - # their old names alive as aliases. + # Build a config where a display has been renamed, keeping its old name available as + # an alias, and a view that has been renamed, keeping its old name available as one + # of the view's aliases. cfg.setVersion(2, 6) dcs = OCIO.ColorSpace( @@ -821,31 +823,78 @@ def test_use_display_view_aliases(self): dcs.setTransform(OCIO.MatrixTransform(), OCIO.COLORSPACE_DIR_FROM_REFERENCE) cfg.addColorSpace(dcs) - vt = OCIO.ViewTransform( - referenceSpace=OCIO.REFERENCE_SPACE_SCENE, - name='vt_new', - aliases=['vt_old']) - vt.setTransform(OCIO.MatrixTransform(), OCIO.VIEWTRANSFORM_DIR_FROM_REFERENCE) - cfg.addViewTransform(vt) + cfg.addDisplayView('sRGB - Display', 'view', viewTransform='', + displayColorSpaceName='sRGB - Display', looks='', ruleName='', + description='', aliases='view_old') - cfg.addDisplayView('sRGB - Display', 'view', viewTransform='vt_new', - displayColorSpaceName='sRGB - Display') - - # The fallback is disabled by default: only exact matches resolve. + # The display alias fallback is disabled by default: only an exact match resolves. self.assertEqual(cfg.getCanonicalDisplayName('sRGB - Display'), 'sRGB - Display') self.assertEqual(cfg.getCanonicalDisplayName('sRGB'), '') + + # But view alias resolution is always active, regardless of getUseDisplayAliases. self.assertEqual(cfg.getCanonicalViewName('sRGB - Display', 'view'), 'view') - self.assertEqual(cfg.getCanonicalViewName('sRGB - Display', 'vt_old'), '') + self.assertEqual(cfg.getCanonicalViewName('sRGB - Display', 'view_old'), 'view') - # Once enabled, both the display and view transform aliases resolve. - cfg.setUseDisplayViewAliases(True) + # Once enabled, the display alias resolves too. + cfg.setUseDisplayAliases(True) self.assertEqual(cfg.getCanonicalDisplayName('sRGB'), 'sRGB - Display') - self.assertEqual(cfg.getCanonicalViewName('sRGB - Display', 'vt_old'), 'view') + self.assertEqual(cfg.getCanonicalViewName('sRGB - Display', 'view_old'), 'view') + + def test_display_view_aliases(self): + # Test Config.getDisplayViewAliases and Config.hasDisplayViewAlias, for both a + # display-defined view and a shared view. + + cfg = OCIO.Config() + cfg.setVersion(2, 6) + cfg.addColorSpace(OCIO.ColorSpace(name='raw')) + + cfg.addDisplayView('display1', 'view1', viewTransform='', + displayColorSpaceName='raw', looks='', ruleName='', + description='', aliases='alias1, alias2') + + self.assertEqual(cfg.getDisplayViewAliases('display1', 'view1'), 'alias1, alias2') + self.assertTrue(cfg.hasDisplayViewAlias('display1', 'view1', 'alias1')) + self.assertTrue(cfg.hasDisplayViewAlias('display1', 'view1', 'ALIAS2')) + self.assertFalse(cfg.hasDisplayViewAlias('display1', 'view1', 'alias3')) + + # A view with no aliases. + cfg.addDisplayView('display1', 'view2', 'raw') + self.assertEqual(cfg.getDisplayViewAliases('display1', 'view2'), '') + self.assertFalse(cfg.hasDisplayViewAlias('display1', 'view2', 'alias1')) + + # A shared view, looked up the same way as Config.hasView (an empty display finds it + # among the config's shared views). + cfg.addSharedView('shared1', '', 'raw', aliases='shared_alias') + cfg.addDisplaySharedView('display1', 'shared1') + + self.assertEqual(cfg.getDisplayViewAliases('', 'shared1'), 'shared_alias') + self.assertTrue(cfg.hasDisplayViewAlias('', 'shared1', 'shared_alias')) + self.assertFalse(cfg.hasDisplayViewAlias('display1', 'shared1', 'unknown')) + + # Unknown display, view, or display/view combination. + self.assertEqual(cfg.getDisplayViewAliases('display1', 'not_a_view'), '') + self.assertFalse(cfg.hasDisplayViewAlias('display1', 'not_a_view', 'alias1')) + self.assertFalse(cfg.hasDisplayViewAlias('not_a_display', 'view1', 'alias1')) + + # An alias may itself contain a comma, as long as it is surrounded by quotes, so + # that the comma isn't mistaken for the separator between aliases. + cfg.addDisplayView('display1', 'view3', viewTransform='', + displayColorSpaceName='raw', looks='', ruleName='', + description='', aliases='"alias,with,comma", alias4') + + self.assertTrue(cfg.hasDisplayViewAlias('display1', 'view3', 'alias,with,comma')) + self.assertTrue(cfg.hasDisplayViewAlias('display1', 'view3', 'alias4')) + self.assertFalse(cfg.hasDisplayViewAlias('display1', 'view3', 'alias')) + + # The comma-containing alias is quoted again on the way out, so that the result can be + # split back apart the same way. + self.assertEqual( + cfg.getDisplayViewAliases('display1', 'view3'), '"alias,with,comma", alias4') def test_display_description(self): # Test that getDisplayDescription borrows the description of the display's associated # display color space: an exact name match always works, but matching only via one of - # the color space's aliases requires getUseDisplayViewAliases. + # the color space's aliases requires getUseDisplayAliases. cfg = OCIO.Config() cfg.setVersion(2, 6) @@ -858,11 +907,11 @@ def test_display_description(self): dcs.setTransform(OCIO.MatrixTransform(), OCIO.COLORSPACE_DIR_FROM_REFERENCE) cfg.addColorSpace(dcs) - self.assertFalse(cfg.getUseDisplayViewAliases()) + self.assertFalse(cfg.getUseDisplayAliases()) self.assertEqual(cfg.getDisplayDescription('sRGB - Display'), 'The sRGB display.') self.assertEqual(cfg.getDisplayDescription('sRGB'), '') - cfg.setUseDisplayViewAliases(True) + cfg.setUseDisplayAliases(True) self.assertEqual(cfg.getDisplayDescription('sRGB'), 'The sRGB display.') self.assertEqual(cfg.getDisplayDescription('does not exist'), '') @@ -883,37 +932,34 @@ def test_resolved_display_view_color_space_name(self): dcs.setTransform(OCIO.MatrixTransform(), OCIO.COLORSPACE_DIR_FROM_REFERENCE) cfg.addColorSpace(dcs) - vt = OCIO.ViewTransform( - referenceSpace=OCIO.REFERENCE_SPACE_SCENE, - name='vt_new', - aliases=['vt_old']) - vt.setTransform(OCIO.MatrixTransform(), OCIO.VIEWTRANSFORM_DIR_FROM_REFERENCE) - cfg.addViewTransform(vt) - cfg.addDisplayView('sRGB - Display', 'view1', 'raw') - cfg.addDisplayView('sRGB - Display', 'view2', viewTransform='vt_old', - displayColorSpaceName='') + cfg.addDisplayView('sRGB - Display', 'view2', viewTransform='', + displayColorSpaceName='', looks='', ruleName='', + description='', aliases='view2_old') # A plain view behaves like getDisplayViewColorSpaceName. self.assertEqual( cfg.getResolvedDisplayViewColorSpaceName('sRGB - Display', 'view1'), 'raw') # resolves to the display's own color space, without needing the - # display/view alias fallback to be enabled. - self.assertFalse(cfg.getUseDisplayViewAliases()) + # display alias fallback to be enabled. + self.assertFalse(cfg.getUseDisplayAliases()) self.assertEqual( cfg.getDisplayViewColorSpaceName('sRGB - Display', 'view2'), '') self.assertEqual( cfg.getResolvedDisplayViewColorSpaceName('sRGB - Display', 'view2'), 'sRGB - Display') - # Resolving an out-of-date display or view name is opt-in. + # Resolving a view by one of its own aliases is always active, unlike resolving an + # out-of-date display name, which requires getUseDisplayAliases. self.assertEqual(cfg.getResolvedDisplayViewColorSpaceName('sRGB', 'view1'), '') - self.assertEqual(cfg.getResolvedDisplayViewColorSpaceName('sRGB - Display', 'vt_new'), '') + self.assertEqual( + cfg.getResolvedDisplayViewColorSpaceName('sRGB - Display', 'view2_old'), + 'sRGB - Display') - cfg.setUseDisplayViewAliases(True) + cfg.setUseDisplayAliases(True) self.assertEqual(cfg.getResolvedDisplayViewColorSpaceName('sRGB', 'view1'), 'raw') self.assertEqual( - cfg.getResolvedDisplayViewColorSpaceName('sRGB', 'vt_new'), 'sRGB - Display') + cfg.getResolvedDisplayViewColorSpaceName('sRGB', 'view2_old'), 'sRGB - Display') # Nonexistent display or view. self.assertEqual( diff --git a/tests/python/ViewTransformTest.py b/tests/python/ViewTransformTest.py index cb6df5fafe..bb9e9e3f31 100644 --- a/tests/python/ViewTransformTest.py +++ b/tests/python/ViewTransformTest.py @@ -61,13 +61,11 @@ def test_copy(self): vt.setTransform(mat, OCIO.VIEWTRANSFORM_DIR_TO_REFERENCE) vt.setTransform(direction=OCIO.VIEWTRANSFORM_DIR_FROM_REFERENCE, transform=mat) vt.addCategory('cat1') - vt.addAlias('alias1') other = copy.deepcopy(vt) self.assertFalse(other is vt) self.assertEqual(other.getName(), vt.getName()) - self.assertEqual(list(other.getAliases()), list(vt.getAliases())) self.assertEqual(other.getFamily(), vt.getFamily()) self.assertEqual(other.getDescription(), vt.getDescription()) self.assertEqual( @@ -87,68 +85,6 @@ def test_name(self): vt.setName('test name') self.assertEqual(vt.getName(), 'test name') - def test_aliases(self): - """ - Test ViewTransform aliases. - """ - - vt = OCIO.ViewTransform() - self.assertEqual(vt.getName(), '') - aliases = vt.getAliases() - self.assertEqual(len(aliases), 0) - - vt.addAlias('alias1') - aliases = vt.getAliases() - self.assertEqual(len(aliases), 1) - self.assertEqual(aliases[0], 'alias1') - self.assertTrue(vt.hasAlias('alias1')) - self.assertTrue(vt.hasAlias('aLiaS1')) - self.assertFalse(vt.hasAlias('alias2')) - - vt.addAlias('alias2') - aliases = vt.getAliases() - self.assertEqual(len(aliases), 2) - self.assertEqual(aliases[0], 'alias1') - self.assertEqual(aliases[1], 'alias2') - self.assertTrue(vt.hasAlias('alias2')) - - # Alias is already there, not added. - - vt.addAlias('Alias2') - aliases = vt.getAliases() - self.assertEqual(len(aliases), 2) - self.assertEqual(aliases[0], 'alias1') - self.assertEqual(aliases[1], 'alias2') - - # Name might remove an alias. - - vt.setName('name') - aliases = vt.getAliases() - self.assertEqual(len(aliases), 2) - - vt.setName('alias2') - aliases = vt.getAliases() - self.assertEqual(len(aliases), 1) - self.assertEqual(aliases[0], 'alias1') - - vt.removeAlias('alias1') - aliases = vt.getAliases() - self.assertEqual(len(aliases), 0) - - vt.addAlias('alias3') - vt.addAlias('alias4') - self.assertEqual(len(vt.getAliases()), 2) - vt.clearAliases() - self.assertEqual(len(vt.getAliases()), 0) - - # Aliases may also be set via the constructor. - - vt = OCIO.ViewTransform(name='vt_name', aliases=['a1', 'a2']) - aliases = vt.getAliases() - self.assertEqual(len(aliases), 2) - self.assertEqual(aliases[0], 'a1') - self.assertEqual(aliases[1], 'a2') - def test_family(self): """ Test get/setFamily. From 8e26546ad0a773ee429896a05ed995abc9bdf03a Mon Sep 17 00:00:00 2001 From: Doug Walker Date: Tue, 29 Sep 2026 02:33:08 -0400 Subject: [PATCH 3/4] Respond to review requests Signed-off-by: Doug Walker --- include/OpenColorIO/OpenColorIO.h | 37 ++++--- src/OpenColorIO/Config.cpp | 122 +++++++++++++--------- src/OpenColorIO/OCIOYaml.cpp | 30 ++++-- src/bindings/python/PyConfig.cpp | 48 +++++++-- tests/cpu/Config_tests.cpp | 162 ++++++++++++++++++++++-------- tests/python/ConfigTest.py | 45 ++++++--- 6 files changed, 306 insertions(+), 138 deletions(-) diff --git a/include/OpenColorIO/OpenColorIO.h b/include/OpenColorIO/OpenColorIO.h index 7f101037cb..ee2cbefeed 100644 --- a/include/OpenColorIO/OpenColorIO.h +++ b/include/OpenColorIO/OpenColorIO.h @@ -658,7 +658,7 @@ class OCIOEXPORT Config /** * Return true if the color space is used by a transform, a role, a look, a (display, view) * pair, or a file rule. The argument may be either an alias or the canonical name. While - * searching the config, aliases are always resolve to their canonical names for comparison. + * searching the config, aliases are always resolved to their canonical names for comparison. */ bool isColorSpaceUsed(const char * name) const noexcept; @@ -863,8 +863,7 @@ class OCIOEXPORT Config const char * colorSpaceName, const char * looks, const char * ruleName, const char * description); /** - * \brief As above, but also sets the view's aliases (see \ref Config::getDisplayViewAliases - * for the format). + * \brief As above, but also sets the view's aliases. * * Will throw if view or colorSpaceName are null or empty, or if an alias collides with the * name or an alias of another shared view. @@ -872,7 +871,8 @@ class OCIOEXPORT Config void addSharedView(const char * view, const char * viewTransformName, const char * colorSpaceName, const char * looks, const char * ruleName, const char * description, - const char * aliases); + const std::vector & aliases); + /// Remove a shared view. Will throw if the view does not exist. void removeSharedView(const char * view); @@ -940,14 +940,19 @@ class OCIOEXPORT Config const char * getDisplayViewDescription(const char * display, const char * view) const noexcept; /** - * \brief Get the aliases of a (display, view) pair, as a comma-delimited string (as it - * would appear in a config file). If display is null or empty, config shared views are used. - * - * If an alias itself contains a comma, it is enclosed in quotes, similar to active_views. + * \brief Get the number of aliases of a (display, view) pair. If display is null or + * empty, config shared views are used. + */ + int getNumDisplayViewAliases(const char * display, const char * view) const noexcept; + + /** + * \brief Get an alias of a (display, view) pair, by index. If display is null or empty, + * config shared views are used. * - * Returns "" if the (display, view) pair does not exist or has no aliases. + * Returns "" if the (display, view) pair does not exist or index is out of range. */ - std::string getDisplayViewAliases(const char * display, const char * view) const; + const char * getDisplayViewAlias(const char * display, const char * view, + int index) const noexcept; /** * \brief Convenience method to check whether a (display, view) pair has a specific alias. @@ -991,8 +996,7 @@ class OCIOEXPORT Config const char * ruleName, const char * description); /** - * \brief As above, but also sets the view's aliases (see \ref Config::getDisplayViewAliases - * for the format). + * \brief As above, but also sets the view's aliases. * * Will throw if: * * Display, view or colorSpace are null or empty. @@ -1002,7 +1006,8 @@ class OCIOEXPORT Config */ void addDisplayView(const char * display, const char * view, const char * viewTransformName, const char * colorSpaceName, const char * looks, - const char * ruleName, const char * description, const char * aliases); + const char * ruleName, const char * description, + const std::vector & aliases); /** * \brief Add a (reference to a) shared view to a display. @@ -1234,6 +1239,9 @@ class OCIOEXPORT Config * the config file as well as any modifications made by the client app. These functions * only get and set what is in the config object and do not take into account the override * and thus may not represent the actual user experience. + * + * Display aliases may not be used in the active list, use \ref Config::getCanonicalDisplayName + * to convert any aliases to their canonical name. */ /// Set all active displays at once as a comma or colon delimited string. This replaces any /// previous contents of the list. @@ -1272,6 +1280,9 @@ class OCIOEXPORT Config * the config file as well as any modifications made by the client app. These functions * only get and set what is in the config object and do not take into account the override * and thus may not represent the actual user experience. + * + * View aliases may not be used in the active list, use \ref Config::getCanonicalViewName + * to convert any aliases to their canonical name. */ /// Set all active views at once as a comma or colon delimited string. This replaces any /// previous contents of the list. diff --git a/src/OpenColorIO/Config.cpp b/src/OpenColorIO/Config.cpp index dae802e747..edca6c2ea7 100644 --- a/src/OpenColorIO/Config.cpp +++ b/src/OpenColorIO/Config.cpp @@ -3415,13 +3415,13 @@ void Config::addSharedView(const char * view, const char * viewTransform, const char * colorSpace, const char * looks, const char * rule, const char * description) { - addSharedView(view, viewTransform, colorSpace, looks, rule, description, nullptr); + addSharedView(view, viewTransform, colorSpace, looks, rule, description, {}); } void Config::addSharedView(const char * view, const char * viewTransform, const char * colorSpace, const char * looks, const char * rule, const char * description, - const char * aliases) + const std::vector & aliases) { if (!view || !*view) { @@ -3435,15 +3435,9 @@ void Config::addSharedView(const char * view, const char * viewTransform, "non-empty name."); } - StringUtils::StringVec aliasVec = SplitStringEnvStyle(aliases ? aliases : ""); - if (aliasVec.size() == 1 && aliasVec[0].empty()) - { - aliasVec.clear(); - } - ViewVec & views = getImpl()->m_sharedViews; - const View candidate(view, viewTransform, colorSpace, looks, rule, description, aliasVec); + const View candidate(view, viewTransform, colorSpace, looks, rule, description, aliases); // Keep shared views unambiguous among themselves (independent of which displays end up // referencing them), by checking the candidate's name/aliases against every sibling shared @@ -3470,7 +3464,7 @@ void Config::addSharedView(const char * view, const char * viewTransform, } } - AddView(views, view, viewTransform, colorSpace, looks, rule, description, aliasVec); + AddView(views, view, viewTransform, colorSpace, looks, rule, description, aliases); getImpl()->m_displayCache.clear(); @@ -3725,13 +3719,21 @@ const char * Config::getDisplayViewDescription(const char * display, const char return viewPtr ? viewPtr->m_description.c_str() : ""; } -std::string Config::getDisplayViewAliases(const char * display, const char * view) const +int Config::getNumDisplayViewAliases(const char * display, const char * view) const noexcept { - // Follows the same display=null/empty convention as above: look up view among - // the config's shared views if display is null or empty. const View * viewPtr = getImpl()->getView(display, view); + return viewPtr ? static_cast(viewPtr->m_aliases.size()) : 0; +} - return viewPtr ? JoinStringEnvStyle(viewPtr->m_aliases) : std::string(); +const char * Config::getDisplayViewAlias(const char * display, const char * view, + int index) const noexcept +{ + const View * viewPtr = getImpl()->getView(display, view); + if (!viewPtr || index < 0 || static_cast(index) >= viewPtr->m_aliases.size()) + { + return ""; + } + return viewPtr->m_aliases[index].c_str(); } bool Config::hasDisplayViewAlias(const char * display, const char * view, @@ -3831,19 +3833,20 @@ void Config::addDisplaySharedView(const char * display, const char * sharedView) void Config::addDisplayView(const char * display, const char * view, const char * colorSpace, const char * looks) { - addDisplayView(display, view, nullptr, colorSpace, looks, nullptr, nullptr, nullptr); + addDisplayView(display, view, nullptr, colorSpace, looks, nullptr, nullptr, {}); } void Config::addDisplayView(const char * display, const char * view, const char * viewTransform, const char * colorSpace, const char * looks, const char * rule, const char * description) { - addDisplayView(display, view, viewTransform, colorSpace, looks, rule, description, nullptr); + addDisplayView(display, view, viewTransform, colorSpace, looks, rule, description, {}); } void Config::addDisplayView(const char * display, const char * view, const char * viewTransform, const char * colorSpace, const char * looks, - const char * rule, const char * description, const char * aliases) + const char * rule, const char * description, + const std::vector & aliases) { if (!display || !*display) { @@ -3861,12 +3864,6 @@ void Config::addDisplayView(const char * display, const char * view, const char "name is needed."); } - StringUtils::StringVec aliasVec = SplitStringEnvStyle(aliases ? aliases : ""); - if (aliasVec.size() == 1 && aliasVec[0].empty()) - { - aliasVec.clear(); - } - DisplayMap::iterator iter = FindDisplay(getImpl()->m_displays, display); if (iter == getImpl()->m_displays.end()) { @@ -3875,7 +3872,7 @@ void Config::addDisplayView(const char * display, const char * view, const char getImpl()->m_displays[curSize].first = display; getImpl()->m_displays[curSize].second.m_views.push_back(View(view, viewTransform, colorSpace, looks, rule, - description, aliasVec)); + description, aliases)); getImpl()->m_displayCache.clear(); } else @@ -3888,7 +3885,7 @@ void Config::addDisplayView(const char * display, const char * view, const char throw Exception(os.str().c_str()); } - const View candidate(view, viewTransform, colorSpace, looks, rule, description, aliasVec); + const View candidate(view, viewTransform, colorSpace, looks, rule, description, aliases); // Check the candidate's name/aliases against sibling views in this display (other than // the one being replaced, if this call is updating an existing view by that name). @@ -3929,7 +3926,7 @@ void Config::addDisplayView(const char * display, const char * view, const char } ViewVec & views = iter->second.m_views; - AddView(views, view, viewTransform, colorSpace, looks, rule, description, aliasVec); + AddView(views, view, viewTransform, colorSpace, looks, rule, description, aliases); } AutoMutex lock(getImpl()->m_cacheidMutex); @@ -4017,6 +4014,38 @@ void Config::setUseDisplayAliases(bool enabled) noexcept getImpl()->resetCacheIDs(); } +namespace +{ + +// A display might have a display color space with a matching name purely by coincidence. +// This checks whether one of the display's views (display-defined or shared, active or +// inactive) actually resolves, directly or via , to the given color space. +bool DisplayUsesColorSpace(const Config & config, const ViewPtrVec & views, + const ConstColorSpaceRcPtr & cs) +{ + for (const auto * view : views) + { + // Only requiring one (rather than all) the display's views to match since sometimes a + // display will have utility views such as "Raw" that don't rely on a display color space. + + if (view->useDisplayNameForColorspace()) + { + // The display_colorspace is . + return true; + } + ConstColorSpaceRcPtr viewCs = config.getColorSpace(view->m_colorspace.c_str()); + if (viewCs && StringUtils::Compare(viewCs->getName(), cs->getName())) + { + // The canonical name of the view's display_colorspace (or colorspace) + // equals that of cs. + return true; + } + } + return false; +} + +} // namespace + const char * Config::getCanonicalDisplayName(const char * displayName) const { if (!displayName || !*displayName) @@ -4057,31 +4086,11 @@ const char * Config::getCanonicalDisplayName(const char * displayName) const // this color space, i.e. have a view whose display_colorspace is or // that resolves (by name or alias) to this same color space. Otherwise, a display that // merely happens to share a name with an unrelated color space would incorrectly match. - // - // Only requiring one (rather than all) the display's views to match since sometimes a - // display will have utility views such as "Raw" that don't rely on a display color space. - // auto usesColorSpace = [this, &cs](DisplayMap::const_iterator candidateIter) -> bool { // Consider both display-defined views and shared views used by this display, and both // active and inactive views. - const ViewPtrVec views = getImpl()->getViews(candidateIter->second); - for (const auto * view : views) - { - if (view->useDisplayNameForColorspace()) - { - // THe display_colorspace is . - return true; - } - ConstColorSpaceRcPtr viewCs = getColorSpace(view->m_colorspace.c_str()); - if (viewCs && StringUtils::Compare(viewCs->getName(), cs->getName())) - { - // The canonical name of the view's display_colorspace (or colorspace) - // equals that of the cs that matched displayName. - return true; - } - } - return false; + return DisplayUsesColorSpace(*this, getImpl()->getViews(candidateIter->second), cs); }; iter = FindDisplay(getImpl()->m_displays, cs->getName()); @@ -4169,13 +4178,28 @@ const char * Config::getDisplayDescription(const char * display) const return ""; } - // A display color space named exactly "display" always works. If it was only found via - // one of its aliases, the fallback is opt-in. + // If the color space was found via its aliases, exit now if display alias support is off. if (!StringUtils::Compare(cs->getName(), display) && !getImpl()->m_useDisplayAliases) { return ""; } + // Check that there is an actual display with this name. + const char * canonicalDisplay = getCanonicalDisplayName(cs->getName()); + if (!canonicalDisplay || !*canonicalDisplay) + { + return ""; + } + + // Confirm that the display has a view that references this display color space. + // This rules out matches that coincidentally share the same name but are unrelated. + DisplayMap::const_iterator iter = FindDisplay(getImpl()->m_displays, canonicalDisplay); + if (iter == getImpl()->m_displays.end() || + !DisplayUsesColorSpace(*this, getImpl()->getViews(iter->second), cs)) + { + return ""; + } + return cs->getDescription(); } diff --git a/src/OpenColorIO/OCIOYaml.cpp b/src/OpenColorIO/OCIOYaml.cpp index 5bfaa8962b..8209bf773b 100644 --- a/src/OpenColorIO/OCIOYaml.cpp +++ b/src/OpenColorIO/OCIOYaml.cpp @@ -455,6 +455,9 @@ inline void load(const YAML::Node& node, View& v) } else if (key == "aliases") { + // This uses load(const YAML::Node & node, StringUtils::StringVec & x), so the + // Yaml parser handles unquoting view names with embedded quotes, symmetric with + // the save function below, rather than using SplitStringEnvStyle. load(iter->second, v.m_aliases); } else @@ -506,6 +509,8 @@ inline void save(YAML::Emitter& out, const View & view) } if (!view.m_aliases.empty()) { + // The Yaml parser automatically quotes view name aliases that contain commas, so + // they are not confused with separators. No need to use JoinStringEnvStyle here. out << YAML::Key << "aliases" << YAML::Value << view.m_aliases; } saveDescription(out, view.m_description.c_str()); @@ -4675,11 +4680,10 @@ inline void load(const YAML::Node& node, ConfigRcPtr & config, const char* filen View view; load(val, view); - const std::string aliases = JoinStringEnvStyle(view.m_aliases); config->addSharedView(view.m_name.c_str(), view.m_viewTransform.c_str(), view.m_colorspace.c_str(), view.m_looks.c_str(), view.m_rule.c_str(), - view.m_description.c_str(), aliases.c_str()); + view.m_description.c_str(), view.m_aliases); } } else if (key == "displays") @@ -4706,11 +4710,10 @@ inline void load(const YAML::Node& node, ConfigRcPtr & config, const char* filen { View view; load(node, view); - const std::string aliases = JoinStringEnvStyle(view.m_aliases); config->addDisplayView(display.c_str(), view.m_name.c_str(), view.m_viewTransform.c_str(), view.m_colorspace.c_str(), view.m_looks.c_str(), view.m_rule.c_str(), - view.m_description.c_str(), aliases.c_str()); + view.m_description.c_str(), view.m_aliases); } else if (node.Tag() == "Views") { @@ -4739,6 +4742,11 @@ inline void load(const YAML::Node& node, ConfigRcPtr & config, const char* filen { View view; load(val, view); + if (!view.m_aliases.empty()) + { + throwValueError(node.Tag(), iter->first, + "Aliases are not supported for virtual display views."); + } config->addVirtualDisplayView(view.m_name.c_str(), view.m_viewTransform.c_str(), view.m_colorspace.c_str(), @@ -5045,17 +5053,17 @@ inline void load(const YAML::Node& node, ConfigRcPtr & config, const char* filen } } -// Config only exposes a view's aliases as a single comma-delimited string (see -// Config::getDisplayViewAliases), so split it back into a vector for the View struct used -// below to build up what gets passed to save(YAML::Emitter&, const View&). +// Build the vector for the View struct used below to build up what gets passed to +// save(YAML::Emitter&, const View&). StringUtils::StringVec GetViewAliasVec(const Config & config, const char * display, const char * name) { - StringUtils::StringVec aliases = SplitStringEnvStyle(config.getDisplayViewAliases(display, - name)); - if (aliases.size() == 1 && aliases[0].empty()) + StringUtils::StringVec aliases; + const int numAliases = config.getNumDisplayViewAliases(display, name); + aliases.reserve(numAliases); + for (int i = 0; i < numAliases; i++) { - aliases.clear(); + aliases.push_back(config.getDisplayViewAlias(display, name, i)); } return aliases; } diff --git a/src/bindings/python/PyConfig.cpp b/src/bindings/python/PyConfig.cpp index 7d9d683aba..81feec7255 100644 --- a/src/bindings/python/PyConfig.cpp +++ b/src/bindings/python/PyConfig.cpp @@ -39,6 +39,7 @@ enum ConfigIterator IT_DISPLAY_ALL, IT_DISPLAY_VIEW_TYPE, IT_VIRTUAL_DISPLAY_VIEW, + IT_DISPLAY_VIEW_ALIAS, }; using EnvironmentVarNameIterator = PyIterator; @@ -65,6 +66,8 @@ using ViewForColorSpaceIterator = PyIterator; using ViewForViewTypeIterator = PyIterator; +using DisplayViewAliasIterator = PyIterator; using ActiveDisplaysListIterator = PyIterator; using ActiveViewsListIterator = PyIterator; using LookNameIterator = PyIterator; @@ -146,7 +149,11 @@ void bindPyConfig(py::module & m) py::class_( clsConfig, "ViewForViewTypeIterator"); - auto clsActiveDisplaysListIterator = + auto clsDisplayViewAliasIterator = + py::class_( + clsConfig, "DisplayViewAliasIterator"); + + auto clsActiveDisplaysListIterator = py::class_( clsConfig, "ActiveDisplaysListIterator"); @@ -413,12 +420,12 @@ void bindPyConfig(py::module & m) const char *, const char *, const char *, - const char *)) &Config::addSharedView, + const std::vector &)) &Config::addSharedView, "view"_a, "viewTransformName"_a, "colorSpaceName"_a, "looks"_a = "", "ruleName"_a = "", "description"_a = "", - "aliases"_a = "", + "aliases"_a = std::vector(), DOC(Config, addSharedView)) .def("removeSharedView", &Config::removeSharedView, "view"_a, DOC(Config, removeSharedView)) @@ -478,8 +485,12 @@ void bindPyConfig(py::module & m) .def("getResolvedDisplayViewColorSpaceName", &Config::getResolvedDisplayViewColorSpaceName, "display"_a, "view"_a, DOC(Config, getResolvedDisplayViewColorSpaceName)) - .def("getDisplayViewAliases", &Config::getDisplayViewAliases, "display"_a, "view"_a, - DOC(Config, getDisplayViewAliases)) + .def("getDisplayViewAliases", [](ConfigRcPtr & self, + const std::string & display, const std::string & view) + { + return DisplayViewAliasIterator(self, display, view); + }, + "display"_a, "view"_a) .def("hasDisplayViewAlias", &Config::hasDisplayViewAlias, "display"_a, "view"_a, "alias"_a, DOC(Config, hasDisplayViewAlias)) @@ -507,12 +518,12 @@ void bindPyConfig(py::module & m) const char *, const char *, const char *, - const char *)) &Config::addDisplayView, + const std::vector &)) &Config::addDisplayView, "display"_a, "view"_a, "viewTransform"_a, "displayColorSpaceName"_a, "looks"_a = "", "ruleName"_a = "", "description"_a = "", - "aliases"_a = "", + "aliases"_a = std::vector(), DOC(Config, addDisplayView)) .def("isViewShared", &Config::isViewShared, "display"_a, "view"_a, DOC(Config, isViewShared)) @@ -1316,6 +1327,29 @@ void bindPyConfig(py::module & m) std::get<1>(it.m_args).c_str(), i); }); + clsDisplayViewAliasIterator + .def("__len__", [](DisplayViewAliasIterator & it) + { return it.m_obj->getNumDisplayViewAliases(std::get<0>(it.m_args).c_str(), + std::get<1>(it.m_args).c_str()); }) + .def("__getitem__", [](DisplayViewAliasIterator & it, int i) + { + it.checkIndex(i, it.m_obj->getNumDisplayViewAliases(std::get<0>(it.m_args).c_str(), + std::get<1>(it.m_args).c_str())); + return it.m_obj->getDisplayViewAlias(std::get<0>(it.m_args).c_str(), + std::get<1>(it.m_args).c_str(), i); + }) + .def("__iter__", [](DisplayViewAliasIterator & it) -> DisplayViewAliasIterator & + { + return it; + }) + .def("__next__", [](DisplayViewAliasIterator & it) + { + int i = it.nextIndex(it.m_obj->getNumDisplayViewAliases(std::get<0>(it.m_args).c_str(), + std::get<1>(it.m_args).c_str())); + return it.m_obj->getDisplayViewAlias(std::get<0>(it.m_args).c_str(), + std::get<1>(it.m_args).c_str(), i); + }); + clsActiveDisplaysListIterator .def("__len__", [](ActiveDisplaysListIterator & it) { return it.m_obj->getNumActiveDisplays(); }) .def("__getitem__", [](ActiveDisplaysListIterator & it, int i) diff --git a/tests/cpu/Config_tests.cpp b/tests/cpu/Config_tests.cpp index c1b3ce4056..4a21fffd12 100644 --- a/tests/cpu/Config_tests.cpp +++ b/tests/cpu/Config_tests.cpp @@ -7412,10 +7412,28 @@ default_view_transform: view_transform OCIO::Exception, "a non-empty color space name is needed"); } +namespace +{ +// Checks that a (display, view) pair's aliases, as returned by Config::getDisplayViewAlias +// match the expected value, preserving order. +void CheckDisplayViewAliases(const OCIO::ConstConfigRcPtr & config, const char * display, + const char * view, const StringUtils::StringVec & expected, + int line) +{ + OCIO_REQUIRE_EQUAL_FROM(config->getNumDisplayViewAliases(display, view), + (int)expected.size(), line); + for (size_t i = 0; i < expected.size(); i++) + { + OCIO_CHECK_EQUAL_FROM(std::string(config->getDisplayViewAlias(display, view, (int)i)), + expected[i], line); + } +} +} // namespace + OCIO_ADD_TEST(Config, view_aliases) { // Views (display-defined or shared) may have their own aliases, set directly via - // Config::addDisplayView/addSharedView, and queried via Config::getDisplayViewAliases/ + // Config::addDisplayView/addSharedView, and queried via Config::getDisplayViewAlias and // Config::hasDisplayViewAlias. This test covers that machinery: setting aliases, querying // them, keeping them unambiguous, requiring config version 2.6, and surviving a YAML // serialize/reload round-trip. See the aliased_view_name test below for how aliases are @@ -7456,18 +7474,17 @@ ocio_profile_version: 2.6 OCIO::ConstConfigRcPtr config; OCIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromStream(is)); - // Test getDisplayViewAliases returns the aliases as a comma-delimited string (in view order). - OCIO_CHECK_EQUAL(std::string(config->getDisplayViewAliases("display1", "view1")), - "v1_alias2, v1_alias"); - // A view with no aliases, an unknown view, or an unknown display all return "". - OCIO_CHECK_EQUAL(std::string(config->getDisplayViewAliases("display1", "view2")), ""); - OCIO_CHECK_EQUAL(std::string(config->getDisplayViewAliases("display1", "not_a_view")), ""); - OCIO_CHECK_EQUAL(std::string(config->getDisplayViewAliases("not_a_display", "view1")), ""); + // Test getNumDisplayViewAliases/getDisplayViewAlias return the aliases in view order. + CheckDisplayViewAliases(config, "display1", "view1", { "v1_alias2", "v1_alias" }, __LINE__); + // A view with no aliases, an unknown view, or an unknown display all return none. + CheckDisplayViewAliases(config, "display1", "view2", {}, __LINE__); + CheckDisplayViewAliases(config, "display1", "not_a_view", {}, __LINE__); + CheckDisplayViewAliases(config, "not_a_display", "view1", {}, __LINE__); // A null/empty display means look up the view among the config's shared views, following // the same convention as some of the other getters. - OCIO_CHECK_EQUAL(std::string(config->getDisplayViewAliases(nullptr, "sview")), "sv_alias"); - OCIO_CHECK_EQUAL(std::string(config->getDisplayViewAliases("", "sview")), "sv_alias"); + CheckDisplayViewAliases(config, nullptr, "sview", { "sv_alias" }, __LINE__); + CheckDisplayViewAliases(config, "", "sview", { "sv_alias" }, __LINE__); // Test hasDisplayViewAlias convenience method. OCIO_CHECK_ASSERT(config->hasDisplayViewAlias("display1", "view1", "v1_alias")); @@ -7478,19 +7495,18 @@ ocio_profile_version: 2.6 // alias string ("v1_alias") can be reused by an unrelated view in a different display. OCIO_CHECK_ASSERT(config->hasDisplayViewAlias("display2", "view3", "v1_alias")); - // Aliases containing a comma must be quoted, following the same convention used for e.g. - // Config::setActiveViews. + // Aliases containing a comma must be quoted in the Yaml, e.g., similar to how the + // active_views list works. { OCIO::ConfigRcPtr edit = config->createEditableCopy(); OCIO_CHECK_NO_THROW(edit->addDisplayView("display1", "view4", nullptr, "ref1", "", "", "", - R"("a,b",plain)")); - OCIO_CHECK_EQUAL(std::string(edit->getDisplayViewAliases("display1", "view4")), - R"("a,b", plain)"); + { "a,b", "plain" })); + CheckDisplayViewAliases(edit, "display1", "view4", { "a,b", "plain" }, __LINE__); OCIO_CHECK_ASSERT(edit->hasDisplayViewAlias("display1", "view4", "a,b")); OCIO_CHECK_ASSERT(edit->hasDisplayViewAlias("display1", "view4", "plain")); // The comma-containing alias survives a YAML serialize / reload round-trip, quoted as - // needed by the YAML emitter so that it re-parses back into a single alias rather than + // needed by the YAML emitter so that it stays a single flow-sequence entry rather than // being split into two. std::ostringstream os; edit->serialize(os); @@ -7501,16 +7517,14 @@ ocio_profile_version: 2.6 OCIO_CHECK_NO_THROW(reloaded = OCIO::Config::CreateFromStream(reloadStream)); OCIO_CHECK_NO_THROW(reloaded->validate()); - OCIO_CHECK_EQUAL(std::string(reloaded->getDisplayViewAliases("display1", "view4")), - R"("a,b", plain)"); + CheckDisplayViewAliases(reloaded, "display1", "view4", { "a,b", "plain" }, __LINE__); OCIO_CHECK_ASSERT(reloaded->hasDisplayViewAlias("display1", "view4", "a,b")); OCIO_CHECK_ASSERT(reloaded->hasDisplayViewAlias("display1", "view4", "plain")); // The other, comma-free aliases also survive the round-trip. - OCIO_CHECK_EQUAL(std::string(reloaded->getDisplayViewAliases("display1", "view1")), - "v1_alias2, v1_alias"); - OCIO_CHECK_EQUAL(std::string(reloaded->getDisplayViewAliases(nullptr, "sview")), - "sv_alias"); + CheckDisplayViewAliases(reloaded, "display1", "view1", { "v1_alias2", "v1_alias" }, + __LINE__); + CheckDisplayViewAliases(reloaded, nullptr, "sview", { "sv_alias" }, __LINE__); } // A view's alias must not collide with the name or alias of another view used by the same @@ -7521,30 +7535,30 @@ ocio_profile_version: 2.6 // Collides with view1's alias in display1. OCIO_CHECK_THROW_WHAT( - edit->addDisplayView("display1", "view4", nullptr, "ref1", "", "", "", "v1_alias"), + edit->addDisplayView("display1", "view4", nullptr, "ref1", "", "", "", { "v1_alias" }), OCIO::Exception, "already used as the name or alias"); // Collides with the "sview" shared view's alias, which display1 references. OCIO_CHECK_THROW_WHAT( - edit->addDisplayView("display1", "view4", nullptr, "ref1", "", "", "", "sv_alias"), + edit->addDisplayView("display1", "view4", nullptr, "ref1", "", "", "", { "sv_alias" }), OCIO::Exception, "already used as the name or alias"); // No collision in display2, since display2 does not use view1 or its aliases. OCIO_CHECK_NO_THROW( - edit->addDisplayView("display2", "view4", nullptr, "ref1", "", "", "", "v1_alias2")); + edit->addDisplayView("display2", "view4", nullptr, "ref1", "", "", "", { "v1_alias2" })); // A shared view's alias must not collide with another shared view's name or alias, // regardless of which displays reference either of them (this keeps shared views // unambiguous when looked up directly, e.g. via a null display argument). OCIO_CHECK_THROW_WHAT(edit->addSharedView("sview2", nullptr, "ref1", "", "", "", - "sv_alias"), + { "sv_alias" }), OCIO::Exception, "already used as the name or alias"); // Linking an existing shared view to a display is also checked against that display's // own views. OCIO_CHECK_NO_THROW(edit->addDisplayView("display3", "view5", "ref1", "")); OCIO_CHECK_NO_THROW( - edit->addDisplayView("display3", "view5", nullptr, "ref1", "", "", "", "sv_alias")); + edit->addDisplayView("display3", "view5", nullptr, "ref1", "", "", "", { "sv_alias" })); OCIO_CHECK_THROW_WHAT(edit->addDisplaySharedView("display3", "sview"), OCIO::Exception, "already used as the name or alias"); } @@ -7562,7 +7576,7 @@ ocio_profile_version: 2.6 // Redefine "sview" so its alias now collides with "view5" in display3. addSharedView // does not throw, since it only checks against sibling shared views. - OCIO_CHECK_NO_THROW(edit->addSharedView("sview", nullptr, "ref1", "", "", "", "view5")); + OCIO_CHECK_NO_THROW(edit->addSharedView("sview", nullptr, "ref1", "", "", "", { "view5" })); OCIO_CHECK_THROW_WHAT(edit->validate(), OCIO::Exception, "collides with the name or " "alias"); } @@ -7593,9 +7607,28 @@ ocio_profile_version: 2.5 OCIO_CHECK_NO_THROW( oldConfig = OCIO::Config::CreateFromStream(isOld)->createEditableCopy()); OCIO_CHECK_NO_THROW( - oldConfig->addDisplayView("disp", "view1", nullptr, "ref1", "", "", "", "alias1")); + oldConfig->addDisplayView("disp", "view1", nullptr, "ref1", "", "", "", { "alias1" })); OCIO_CHECK_THROW_WHAT(oldConfig->validate(), OCIO::Exception, "less than 2.6"); } + + // Aliases are not supported for virtual display views. Config::addVirtualDisplayView has + // no aliases argument, so an alias is injected into the serialized YAML to exercise the + // parser's check. + { + OCIO::ConfigRcPtr edit = config->createEditableCopy(); + OCIO_CHECK_NO_THROW(edit->addVirtualDisplayView("vview", nullptr, "ref1", "", "", "")); + + std::ostringstream os; + edit->serialize(os); + const std::string configStr = StringUtils::Replace( + os.str(), "name: vview, colorspace: ref1}", + "name: vview, colorspace: ref1, aliases: [vv_alias]}"); + + std::istringstream reloadStream; + reloadStream.str(configStr); + OCIO_CHECK_THROW_WHAT(OCIO::Config::CreateFromStream(reloadStream), OCIO::Exception, + "Aliases are not supported for virtual display views."); + } } OCIO_ADD_TEST(Config, aliased_view_name) @@ -7662,6 +7695,18 @@ active_views: [act_view] // alias string ("v1_alias") can be reused by an unrelated view in a different display. OCIO_CHECK_EQUAL(std::string(config->getCanonicalViewName("display2", "v1_alias")), "view3"); + // Aliases may not be used in the active_views list. + { + OCIO::ConfigRcPtr edit = config->createEditableCopy(); + OCIO_REQUIRE_EQUAL(edit->getNumViews("display1"), 3); + OCIO_CHECK_NO_THROW(edit->setActiveViews("v1_alias")); + // Has no effect. + OCIO_REQUIRE_EQUAL(edit->getNumViews("display1"), 3); + OCIO_CHECK_EQUAL(std::string(edit->getView("display1", 0)), "view1"); + OCIO_CHECK_EQUAL(std::string(edit->getView("display1", 1)), "view2"); + OCIO_CHECK_EQUAL(std::string(edit->getView("display1", 2)), "sview"); + } + // An exact view name always takes priority and needs no alias resolution. OCIO_CHECK_EQUAL(std::string(config->getCanonicalViewName("display1", "view2")), "view2"); @@ -7677,7 +7722,7 @@ active_views: [act_view] { OCIO::ConfigRcPtr edit = config->createEditableCopy(); OCIO_CHECK_NO_THROW(edit->addDisplayView("display1", "view4", nullptr, "ref1", "", "", "", - R"("a,b",plain)")); + { "a,b", "plain" })); OCIO_CHECK_EQUAL(std::string(edit->getCanonicalViewName("display1", "a,b")), "view4"); } @@ -7737,6 +7782,9 @@ OCIO_ADD_TEST(Config, aliased_display_name) OCIO_CHECK_EQUAL(std::string(config->getCanonicalDisplayName("sRGB")), "sRGB - Display"); // Setting display_colorspace to satisfies the requirement as well. + // (Note: is only legal for a shared view with a view transform, so this + // particular config would fail Config::validate() as-is; that's fine here since this test + // never calls validate() on this "config" object.) OCIO_CHECK_NO_THROW(config->addDisplayView("sRGB - Display", "view2", "", "")); OCIO_CHECK_EQUAL(std::string(config->getCanonicalDisplayName("sRGB")), "sRGB - Display"); @@ -7791,15 +7839,17 @@ OCIO_ADD_TEST(Config, aliased_display_name) OCIO_CHECK_EQUAL(std::string(config->getCanonicalDisplayName("other_dcs")), "AliasedDisplay"); config->setInactiveColorSpaces(""); - // Replace the display color space with a scene-referred one of the same name, keeping the - // same "sRGB" alias. Since it is no longer display-referred, "sRGB" must no longer resolve - // to the display, even though the color space's canonical name still matches it exactly. - auto scs = OCIO::ColorSpace::Create(OCIO::REFERENCE_SPACE_SCENE); - scs->setName("sRGB - Display"); - scs->addAlias("sRGB"); - config->addColorSpace(scs); - - OCIO_CHECK_EQUAL(std::string(config->getCanonicalDisplayName("sRGB")), ""); + // Aliases may not be used in the active_displays list. + { + OCIO::ConfigRcPtr edit = config->createEditableCopy(); + OCIO_REQUIRE_EQUAL(edit->getNumDisplays(), 2); + OCIO_CHECK_NO_THROW(edit->setActiveDisplays("sRGB")); + // It has no effect, their are still two displays. + OCIO_REQUIRE_EQUAL(edit->getNumDisplays(), 2); + // TODO: The validate doesn't pass for other reasons. + //OCIO_CHECK_THROW_WHAT(edit->validate(), OCIO::Exception, + // "The list of active displays [sRGB] from the config file is invalid."); + } // No match at all, and null/empty input. OCIO_CHECK_EQUAL(std::string(config->getCanonicalDisplayName("does not exist")), ""); @@ -7819,6 +7869,16 @@ OCIO_ADD_TEST(Config, aliased_display_name) OCIO_CHECK_THROW_WHAT(config->removeDisplayView("sRGB", "view1"), OCIO::Exception, "Could not find a display named 'sRGB'"); + + // Replace the display color space with a scene-referred one of the same name, keeping the + // same "sRGB" alias. Since it is no longer display-referred, "sRGB" must no longer resolve + // to the display, even though the color space's canonical name still matches it exactly. + auto scs = OCIO::ColorSpace::Create(OCIO::REFERENCE_SPACE_SCENE); + scs->setName("sRGB - Display"); + scs->addAlias("sRGB"); + config->addColorSpace(scs); + + OCIO_CHECK_EQUAL(std::string(config->getCanonicalDisplayName("sRGB")), ""); } OCIO_ADD_TEST(Config, display_description) @@ -7826,14 +7886,29 @@ OCIO_ADD_TEST(Config, display_description) OCIO::ConfigRcPtr config = OCIO::Config::Create(); config->setVersion(2, 6); + auto raw = OCIO::ColorSpace::Create(); + raw->setName("raw"); + config->addColorSpace(raw); + auto dcs = OCIO::ColorSpace::Create(OCIO::REFERENCE_SPACE_DISPLAY); dcs->setName("sRGB - Display"); dcs->addAlias("sRGB"); dcs->setDescription("The sRGB display."); config->addColorSpace(dcs); - // Exact match works even though the fallback is disabled by default. + OCIO_CHECK_NO_THROW(config->addDisplayView("sRGB - Display", "view1", "raw", "")); + + // The name match alone isn't enough: the display must also actually use that color space + // in one of its views. So far "sRGB - Display" only has view1, whose color space is "raw", + // so there is no connection to the display color space and the match fails, even though it + // is an exact name match. OCIO_CHECK_ASSERT(!config->getUseDisplayAliases()); + OCIO_CHECK_EQUAL(std::string(config->getDisplayDescription("sRGB - Display")), ""); + + // Add a view that uses the display color space itself, satisfying that requirement. + OCIO_CHECK_NO_THROW(config->addDisplayView("sRGB - Display", "view2", "sRGB - Display", "")); + + // Exact match works even though the fallback is disabled by default. OCIO_CHECK_EQUAL(std::string(config->getDisplayDescription("sRGB - Display")), "The sRGB display."); @@ -7842,11 +7917,14 @@ OCIO_ADD_TEST(Config, display_description) config->setUseDisplayAliases(true); OCIO_CHECK_EQUAL(std::string(config->getDisplayDescription("sRGB")), "The sRGB display."); - // A color space that isn't display-referred doesn't count, even with a matching name. + // A color space that isn't display-referred doesn't count, even with a matching name and + // even though it is actually used by a display's view (so this fails specifically because + // it isn't display-referred, not merely because it goes unused). auto scs = OCIO::ColorSpace::Create(OCIO::REFERENCE_SPACE_SCENE); scs->setName("scene_cs"); scs->setDescription("A scene color space."); config->addColorSpace(scs); + OCIO_CHECK_NO_THROW(config->addDisplayView("scene_cs", "view1", "scene_cs", "")); OCIO_CHECK_EQUAL(std::string(config->getDisplayDescription("scene_cs")), ""); // No color space at all, and null/empty input. @@ -7883,7 +7961,7 @@ OCIO_ADD_TEST(Config, resolved_display_view_color_space_name) // A view with a view transform, whose display color space is . It has its // own alias, "view2_old", so getCanonicalViewName can find it from that old name. OCIO_CHECK_NO_THROW(config->addDisplayView("sRGB - Display", "view2", "vt_new", - "", "", "", "", "view2_old")); + "", "", "", "", { "view2_old" })); // The plain case behaves like getDisplayViewColorSpaceName. OCIO_CHECK_EQUAL(std::string(config->getResolvedDisplayViewColorSpaceName("sRGB - Display", diff --git a/tests/python/ConfigTest.py b/tests/python/ConfigTest.py index 3b069c8b95..48529abae6 100644 --- a/tests/python/ConfigTest.py +++ b/tests/python/ConfigTest.py @@ -825,7 +825,7 @@ def test_use_display_aliases(self): cfg.addDisplayView('sRGB - Display', 'view', viewTransform='', displayColorSpaceName='sRGB - Display', looks='', ruleName='', - description='', aliases='view_old') + description='', aliases=['view_old']) # The display alias fallback is disabled by default: only an exact match resolves. self.assertEqual(cfg.getCanonicalDisplayName('sRGB - Display'), 'sRGB - Display') @@ -850,55 +850,59 @@ def test_display_view_aliases(self): cfg.addDisplayView('display1', 'view1', viewTransform='', displayColorSpaceName='raw', looks='', ruleName='', - description='', aliases='alias1, alias2') + description='', aliases=['alias1', 'alias2']) - self.assertEqual(cfg.getDisplayViewAliases('display1', 'view1'), 'alias1, alias2') + self.assertEqual(list(cfg.getDisplayViewAliases('display1', 'view1')), + ['alias1', 'alias2']) self.assertTrue(cfg.hasDisplayViewAlias('display1', 'view1', 'alias1')) self.assertTrue(cfg.hasDisplayViewAlias('display1', 'view1', 'ALIAS2')) self.assertFalse(cfg.hasDisplayViewAlias('display1', 'view1', 'alias3')) # A view with no aliases. cfg.addDisplayView('display1', 'view2', 'raw') - self.assertEqual(cfg.getDisplayViewAliases('display1', 'view2'), '') + self.assertEqual(list(cfg.getDisplayViewAliases('display1', 'view2')), []) self.assertFalse(cfg.hasDisplayViewAlias('display1', 'view2', 'alias1')) # A shared view, looked up the same way as Config.hasView (an empty display finds it # among the config's shared views). - cfg.addSharedView('shared1', '', 'raw', aliases='shared_alias') + cfg.addSharedView('shared1', '', 'raw', aliases=['shared_alias']) cfg.addDisplaySharedView('display1', 'shared1') - self.assertEqual(cfg.getDisplayViewAliases('', 'shared1'), 'shared_alias') + self.assertEqual(list(cfg.getDisplayViewAliases('', 'shared1')), ['shared_alias']) self.assertTrue(cfg.hasDisplayViewAlias('', 'shared1', 'shared_alias')) self.assertFalse(cfg.hasDisplayViewAlias('display1', 'shared1', 'unknown')) # Unknown display, view, or display/view combination. - self.assertEqual(cfg.getDisplayViewAliases('display1', 'not_a_view'), '') + self.assertEqual(list(cfg.getDisplayViewAliases('display1', 'not_a_view')), []) self.assertFalse(cfg.hasDisplayViewAlias('display1', 'not_a_view', 'alias1')) self.assertFalse(cfg.hasDisplayViewAlias('not_a_display', 'view1', 'alias1')) - # An alias may itself contain a comma, as long as it is surrounded by quotes, so - # that the comma isn't mistaken for the separator between aliases. + # An alias may itself contain a comma; since aliases are set as a real list, no + # quoting is needed (unlike the comma-delimited Config.setActiveViews string). cfg.addDisplayView('display1', 'view3', viewTransform='', displayColorSpaceName='raw', looks='', ruleName='', - description='', aliases='"alias,with,comma", alias4') + description='', aliases=['alias,with,comma', 'alias4']) self.assertTrue(cfg.hasDisplayViewAlias('display1', 'view3', 'alias,with,comma')) self.assertTrue(cfg.hasDisplayViewAlias('display1', 'view3', 'alias4')) self.assertFalse(cfg.hasDisplayViewAlias('display1', 'view3', 'alias')) - # The comma-containing alias is quoted again on the way out, so that the result can be - # split back apart the same way. - self.assertEqual( - cfg.getDisplayViewAliases('display1', 'view3'), '"alias,with,comma", alias4') + self.assertEqual(list(cfg.getDisplayViewAliases('display1', 'view3')), + ['alias,with,comma', 'alias4']) def test_display_description(self): # Test that getDisplayDescription borrows the description of the display's associated # display color space: an exact name match always works, but matching only via one of - # the color space's aliases requires getUseDisplayAliases. + # the color space's aliases requires getUseDisplayAliases. Either way, the display must + # actually use that color space in one of its views, or the match fails (a display + # color space that merely happens to share a name with an unrelated display doesn't + # match). cfg = OCIO.Config() cfg.setVersion(2, 6) + cfg.addColorSpace(OCIO.ColorSpace(name='raw')) + dcs = OCIO.ColorSpace( referenceSpace=OCIO.REFERENCE_SPACE_DISPLAY, name='sRGB - Display', @@ -907,7 +911,16 @@ def test_display_description(self): dcs.setTransform(OCIO.MatrixTransform(), OCIO.COLORSPACE_DIR_FROM_REFERENCE) cfg.addColorSpace(dcs) + cfg.addDisplayView('sRGB - Display', 'view1', 'raw') + + # The name match alone isn't enough: so far "sRGB - Display" only has view1, whose + # color space is "raw", so the match fails even though it is an exact name match. self.assertFalse(cfg.getUseDisplayAliases()) + self.assertEqual(cfg.getDisplayDescription('sRGB - Display'), '') + + # Add a view that uses the display color space itself, satisfying that requirement. + cfg.addDisplayView('sRGB - Display', 'view2', 'sRGB - Display', '') + self.assertEqual(cfg.getDisplayDescription('sRGB - Display'), 'The sRGB display.') self.assertEqual(cfg.getDisplayDescription('sRGB'), '') @@ -935,7 +948,7 @@ def test_resolved_display_view_color_space_name(self): cfg.addDisplayView('sRGB - Display', 'view1', 'raw') cfg.addDisplayView('sRGB - Display', 'view2', viewTransform='', displayColorSpaceName='', looks='', ruleName='', - description='', aliases='view2_old') + description='', aliases=['view2_old']) # A plain view behaves like getDisplayViewColorSpaceName. self.assertEqual( From f7a1930836665ed7a31b838e6fec5d686d225710 Mon Sep 17 00:00:00 2001 From: Doug Walker Date: Tue, 29 Sep 2026 18:46:22 -0400 Subject: [PATCH 4/4] Fine tune tests Signed-off-by: Doug Walker --- docs/releases/ocio_2_6.rst | 75 +++++++++++++++++++++++++ tests/cpu/Config_tests.cpp | 111 ++++++++++++++++++++++++++----------- 2 files changed, 154 insertions(+), 32 deletions(-) diff --git a/docs/releases/ocio_2_6.rst b/docs/releases/ocio_2_6.rst index d2c023bde2..035a69f85c 100644 --- a/docs/releases/ocio_2_6.rst +++ b/docs/releases/ocio_2_6.rst @@ -15,6 +15,81 @@ calendar year 2027. New Feature Guide ================= +Display and View Aliases +************************ + +For Config Authors +++++++++++++++++++ + +Config authors may now define alias names for displays and views that will be recognized +in a ``DisplayViewTransform`` as equivalent to the canonical names. Similar to color space +aliases, this allows config authors to evolve naming of display and views over time while +still providing backwards compatibility for the older names. + +Display aliasing is opt-in and the config author must set the new config-level attribute +``use_display_aliases: true``. With that enabled, the name or aliases of the display color +space for the display will be considered synonyms for that display. + +View aliasing is allowed via a new ``aliases`` attribute on a view or shared view. These are +always active, independent of whether ``use_display_aliases`` is enabled. Similar to other +Yaml lists, these are separated by a comma. Names that contain an embedded comma are +enclosed in quotes to prevent it from being used as a separator. + +Please note that the ``active_displays`` and ``active_views`` lists must use the canonical names +rather than aliases. Similarly, view aliases in a shared view may not be used when referring to +the shared view in a display's views. + +For virtual displays, aliases may be used with shared views but are +not suppored for display-defined virtual views. + +As an example, in the following config file excerpt, "srgb_rec709_display" could be used as +a display alias and "aces2_sdr_view" could be used as a view alias when creating a +``DisplayViewTransform``. + +.. code-block:: yaml + + use_display_aliases: true + + shared_views: + - ! {name: ACES 2.0 - SDR, view_transform: ACES 2.0 - SDR, + display_colorspace: , aliases: [aces2_sdr_view]} + + displays: + sRGB - Display: + - ! [ACES 2.0 - SDR] + + display_colorspaces: + - ! + name: sRGB - Display + aliases: [srgb_rec709_display] + +For Developers +++++++++++++++ + +If application code is currently calling ``Config::getDisplayViewColorSpaceName``, you will +probably want to change that to ``Config::getResolvedDisplayViewColorSpaceName`` so that it +will handle aliases. Note that this resolves the ```` token, as well. + +Any existing calls to ``DisplayViewTransform`` should automatically work with aliases, without +any changes. + +The new functions ``Config::getCanonicalDisplayName`` and ``Config::getCanonicalViewName`` may +be used to convert aliases back to the primary name used in the config. + + +Display Descriptions +******************** + +For Developers +++++++++++++++ + +On a related note, the new ``Config::getDisplayDescription`` allows applications to get a +description for a display. This is sourced from the description attribute of the display +color space that implements the display. (Views already have a description attribute +available for config authors to set.) This enables applications to provide tool-tips or +similar help text for both displays and views. + + New Fixed Function Transforms ***************************** diff --git a/tests/cpu/Config_tests.cpp b/tests/cpu/Config_tests.cpp index 4a21fffd12..721c2cbf67 100644 --- a/tests/cpu/Config_tests.cpp +++ b/tests/cpu/Config_tests.cpp @@ -7611,11 +7611,31 @@ ocio_profile_version: 2.5 OCIO_CHECK_THROW_WHAT(oldConfig->validate(), OCIO::Exception, "less than 2.6"); } - // Aliases are not supported for virtual display views. Config::addVirtualDisplayView has - // no aliases argument, so an alias is injected into the serialized YAML to exercise the - // parser's check. + // Test aliases in virtual displays. { OCIO::ConfigRcPtr edit = config->createEditableCopy(); + + // A virtual display may reference a shared view that has aliases ("sview" has the + // alias "sv_alias"). + OCIO_CHECK_NO_THROW(edit->addVirtualDisplaySharedView("sview")); + OCIO_REQUIRE_EQUAL(edit->getVirtualDisplayNumViews(OCIO::VIEW_SHARED), 1); + OCIO_CHECK_EQUAL(std::string(edit->getVirtualDisplayView(OCIO::VIEW_SHARED, 0)), "sview"); + OCIO_CHECK_ASSERT(edit->hasDisplayViewAlias(nullptr, "sview", "sv_alias")); + OCIO_CHECK_NO_THROW(edit->validate()); + + // The virtual display's reference to a shared view with an alias survives a + // serialize/reload round-trip without throwing. + std::ostringstream osRoundTrip; + edit->serialize(osRoundTrip); + std::istringstream roundTripStream; + roundTripStream.str(osRoundTrip.str()); + OCIO_CHECK_NO_THROW(OCIO::Config::CreateFromStream(roundTripStream)); + + // But aliases on display-defined views on a virtual display are not supported. + // Validate that a hand-edited config file will throw if it tries to use them. + + // Config::addVirtualDisplayView has no aliases argument, so an alias is injected into + // the serialized YAML to exercise the parser's check. OCIO_CHECK_NO_THROW(edit->addVirtualDisplayView("vview", nullptr, "ref1", "", "", "")); std::ostringstream os; @@ -7695,18 +7715,6 @@ active_views: [act_view] // alias string ("v1_alias") can be reused by an unrelated view in a different display. OCIO_CHECK_EQUAL(std::string(config->getCanonicalViewName("display2", "v1_alias")), "view3"); - // Aliases may not be used in the active_views list. - { - OCIO::ConfigRcPtr edit = config->createEditableCopy(); - OCIO_REQUIRE_EQUAL(edit->getNumViews("display1"), 3); - OCIO_CHECK_NO_THROW(edit->setActiveViews("v1_alias")); - // Has no effect. - OCIO_REQUIRE_EQUAL(edit->getNumViews("display1"), 3); - OCIO_CHECK_EQUAL(std::string(edit->getView("display1", 0)), "view1"); - OCIO_CHECK_EQUAL(std::string(edit->getView("display1", 1)), "view2"); - OCIO_CHECK_EQUAL(std::string(edit->getView("display1", 2)), "sview"); - } - // An exact view name always takes priority and needs no alias resolution. OCIO_CHECK_EQUAL(std::string(config->getCanonicalViewName("display1", "view2")), "view2"); @@ -7741,6 +7749,20 @@ active_views: [act_view] OCIO::ConfigRcPtr configEdit = config->createEditableCopy(); OCIO_CHECK_THROW_WHAT(configEdit->removeDisplayView("display1", "v1_alias"), OCIO::Exception, "Could not find a view named 'v1_alias"); + + // Aliases may not be used in the active_views list. + { + OCIO::ConfigRcPtr edit = config->createEditableCopy(); + OCIO_REQUIRE_EQUAL(edit->getNumViews("display1"), 3); + OCIO_CHECK_NO_THROW(edit->setActiveViews("v1_alias")); + // Has no effect. + OCIO_REQUIRE_EQUAL(edit->getNumViews("display1"), 3); + OCIO_CHECK_EQUAL(std::string(edit->getView("display1", 0)), "view1"); + OCIO_CHECK_EQUAL(std::string(edit->getView("display1", 1)), "view2"); + OCIO_CHECK_EQUAL(std::string(edit->getView("display1", 2)), "sview"); + // Note: Config validation currently does not validate active_views, + // so that is not something that could be tested here. + } } OCIO_ADD_TEST(Config, aliased_display_name) @@ -7781,11 +7803,20 @@ OCIO_ADD_TEST(Config, aliased_display_name) // Now the alias works. OCIO_CHECK_EQUAL(std::string(config->getCanonicalDisplayName("sRGB")), "sRGB - Display"); - // Setting display_colorspace to satisfies the requirement as well. - // (Note: is only legal for a shared view with a view transform, so this - // particular config would fail Config::validate() as-is; that's fine here since this test - // never calls validate() on this "config" object.) - OCIO_CHECK_NO_THROW(config->addDisplayView("sRGB - Display", "view2", "", "")); + // Remove that view, the alias no longer works. + OCIO_CHECK_NO_THROW(config->removeDisplayView("sRGB - Display", "view2")); + OCIO_CHECK_EQUAL(std::string(config->getCanonicalDisplayName("sRGB")), ""); + + // Now test that a shared view that uses is sufficient to establish + // a connection between the display and a display color space. + auto vt = OCIO::ViewTransform::Create(OCIO::REFERENCE_SPACE_SCENE); + vt->setName("vt1"); + OCIO_CHECK_NO_THROW(vt->setTransform(OCIO::MatrixTransform::Create(), + OCIO::VIEWTRANSFORM_DIR_FROM_REFERENCE)); + OCIO_CHECK_NO_THROW(config->addViewTransform(vt)); + OCIO_CHECK_NO_THROW(config->addSharedView("sview", "vt1", "", "", "", "")); + OCIO_CHECK_NO_THROW(config->addDisplaySharedView("sRGB - Display", "sview")); + // The alias works again. OCIO_CHECK_EQUAL(std::string(config->getCanonicalDisplayName("sRGB")), "sRGB - Display"); // If there is a display named "sRGB" added, make sure it returns that one. @@ -7839,18 +7870,6 @@ OCIO_ADD_TEST(Config, aliased_display_name) OCIO_CHECK_EQUAL(std::string(config->getCanonicalDisplayName("other_dcs")), "AliasedDisplay"); config->setInactiveColorSpaces(""); - // Aliases may not be used in the active_displays list. - { - OCIO::ConfigRcPtr edit = config->createEditableCopy(); - OCIO_REQUIRE_EQUAL(edit->getNumDisplays(), 2); - OCIO_CHECK_NO_THROW(edit->setActiveDisplays("sRGB")); - // It has no effect, their are still two displays. - OCIO_REQUIRE_EQUAL(edit->getNumDisplays(), 2); - // TODO: The validate doesn't pass for other reasons. - //OCIO_CHECK_THROW_WHAT(edit->validate(), OCIO::Exception, - // "The list of active displays [sRGB] from the config file is invalid."); - } - // No match at all, and null/empty input. OCIO_CHECK_EQUAL(std::string(config->getCanonicalDisplayName("does not exist")), ""); OCIO_CHECK_EQUAL(std::string(config->getCanonicalDisplayName("")), ""); @@ -7881,6 +7900,34 @@ OCIO_ADD_TEST(Config, aliased_display_name) OCIO_CHECK_EQUAL(std::string(config->getCanonicalDisplayName("sRGB")), ""); } +OCIO_ADD_TEST(Config, active_display_alias) +{ + // Aliases may not be used in the active_displays list. + + OCIO::ConstConfigRcPtr config; + OCIO_CHECK_NO_THROW( + config = OCIO::Config::CreateFromBuiltinConfig("cg-config-v4.0.0_aces-v2.0_ocio-v2.5") + ); + OCIO_REQUIRE_ASSERT(config); + + OCIO::ConfigRcPtr edit = config->createEditableCopy(); + edit->setVersion(2, 6); + edit->setUseDisplayAliases(true); + + // "srgb_rec709_display" is an alias of the "sRGB - Display" display color space, and + // resolves to the "sRGB - Display" display. + OCIO_CHECK_EQUAL(std::string(edit->getCanonicalDisplayName("srgb_rec709_display")), + "sRGB - Display"); + + OCIO_REQUIRE_EQUAL(edit->getNumDisplays(), 8); + OCIO_CHECK_NO_THROW(edit->setActiveDisplays("srgb_rec709_display")); + // It has no effect, there are still eight displays. + OCIO_REQUIRE_EQUAL(edit->getNumDisplays(), 8); + OCIO_CHECK_THROW_WHAT(edit->validate(), OCIO::Exception, + "The list of active displays [srgb_rec709_display] from the config " + "file is invalid."); +} + OCIO_ADD_TEST(Config, display_description) { OCIO::ConfigRcPtr config = OCIO::Config::Create();