diff --git a/src/OpenColorIO/ViewingRules.cpp b/src/OpenColorIO/ViewingRules.cpp index 2db7d1e49..1de51e222 100644 --- a/src/OpenColorIO/ViewingRules.cpp +++ b/src/OpenColorIO/ViewingRules.cpp @@ -464,6 +464,118 @@ std::ostream & operator<< (std::ostream & os, const ViewingRules & vr) return os; } +bool viewingRulesAreEqual(const ConstViewingRulesRcPtr & r1, + size_t r1Idx, + const ConstViewingRulesRcPtr & r2, + size_t r2Idx) +{ + // NB: No need to compare the name of the rules, that should be done in the caller. + + // Compare color space tokens, handling the case where they may be in a different order. + + if (r1->getNumColorSpaces(r1Idx) != r2->getNumColorSpaces(r2Idx)) + { + return false; + } + + TokensManager r1ColorSpaces; + for (size_t m = 0; m < r1->getNumColorSpaces(r1Idx); m++) + { + r1ColorSpaces.addToken(r1->getColorSpace(r1Idx, m)); + } + + for (size_t m = 0; m < r2->getNumColorSpaces(r2Idx); m++) + { + if (!r1ColorSpaces.hasToken(r2->getColorSpace(r2Idx, m))) + { + return false; + } + } + + // Compare encoding tokens, handling the case where they may be in a different order. + + if (r1->getNumEncodings(r1Idx) != r2->getNumEncodings(r2Idx)) + { + return false; + } + + TokensManager r1Encodings; + for (size_t m = 0; m < r1->getNumEncodings(r1Idx); m++) + { + r1Encodings.addToken(r1->getEncoding(r1Idx, m)); + } + + for (size_t m = 0; m < r2->getNumEncodings(r2Idx); m++) + { + if(!r1Encodings.hasToken(r2->getEncoding(r2Idx, m))) + { + return false; + } + } + + // Compare the custom keys, handling the case where they may be in a different order. + + if (r1->getNumCustomKeys(r1Idx) != r2->getNumCustomKeys(r2Idx)) + { + return false; + } + + CustomKeysContainer r1CustomKeys; + for (size_t m = 0; m < r1->getNumCustomKeys(r1Idx); m++) + { + r1CustomKeys.set(r1->getCustomKeyName(r1Idx, m), r1->getCustomKeyValue(r1Idx, m)); + } + + for (size_t m = 0; m < r2->getNumCustomKeys(r2Idx); m++) + { + if (!r1CustomKeys.hasKey(r2->getCustomKeyName(r2Idx, m))) + { + return false; + } + else + { + if (Platform::Strcasecmp(r1CustomKeys.getValueForKey(r2->getCustomKeyName(r2Idx, m)), + r2->getCustomKeyValue(r2Idx, m)) != 0) + { + return false; + } + } + } + + return true; +} + +void copyViewingRule(const ConstViewingRulesRcPtr & src, + size_t srcIdx, + size_t dstIdx, + ViewingRulesRcPtr & rules) +{ + try + { + rules->insertRule(dstIdx, src->getName(srcIdx)); + + for (int j = 0; j < static_cast(src->getNumColorSpaces(srcIdx)); j++) + { + rules->addColorSpace(dstIdx, src->getColorSpace(srcIdx, j)); + } + + for (int k = 0; k < static_cast(src->getNumEncodings(srcIdx)); k++) + { + rules->addEncoding(dstIdx, src->getEncoding(srcIdx, k)); + } + + for (int l = 0; l < static_cast(src->getNumCustomKeys(srcIdx)); l++) + { + rules->setCustomKey(dstIdx, src->getCustomKeyName(srcIdx, l), src->getCustomKeyValue(srcIdx, l)); + } + } + catch(...) + { + // Don't add it if any errors. + // Continue. + } +} + bool FindRule(ConstViewingRulesRcPtr vr, const std::string & name, size_t & ruleIndex) { const auto numrules = vr->getNumEntries(); diff --git a/src/OpenColorIO/ViewingRules.h b/src/OpenColorIO/ViewingRules.h index fb853d362..ed647548b 100644 --- a/src/OpenColorIO/ViewingRules.h +++ b/src/OpenColorIO/ViewingRules.h @@ -25,6 +25,25 @@ constexpr char CustomKey[] { "custom" }; bool FindRule(ConstViewingRulesRcPtr vr, const std::string & name, size_t & ruleIndex); +// Compares the rule at r1Idx in r1 against the rule at r2Idx in r2: same colorspaces, +// encodings, and custom keys (order-independent). Does not compare the rules' names, +// callers that care about the name should compare it separately. +bool viewingRulesAreEqual(const ConstViewingRulesRcPtr & r1, + size_t r1Idx, + const ConstViewingRulesRcPtr & r2, + size_t r2Idx); + +// Copies the rule at srcIdx in src (name, colorspaces, encodings, and custom keys) into +// rules at dstIdx. Best-effort: if the copy fails for any reason (e.g. rules already has +// a rule named src's name), the exception is swallowed rather than propagated, and rules +// is left as it was before the call. src must not alias rules: if they're the same object +// and dstIdx < srcIdx, inserting shifts srcIdx before it's read, silently copying the +// wrong (freshly-inserted, empty) rule instead of throwing. +void copyViewingRule(const ConstViewingRulesRcPtr & src, + size_t srcIdx, + size_t dstIdx, + ViewingRulesRcPtr & rules); + class ViewingRule; using ViewingRuleRcPtr = OCIO_SHARED_PTR; diff --git a/src/OpenColorIO/apphelpers/mergeconfigs/SectionMerger.cpp b/src/OpenColorIO/apphelpers/mergeconfigs/SectionMerger.cpp index 33c41f293..7ff5fb4c5 100644 --- a/src/OpenColorIO/apphelpers/mergeconfigs/SectionMerger.cpp +++ b/src/OpenColorIO/apphelpers/mergeconfigs/SectionMerger.cpp @@ -19,8 +19,8 @@ #include "ParseUtils.h" #include "Platform.h" #include "SectionMerger.h" -#include "TokensManager.h" #include "utils/StringUtils.h" +#include "ViewingRules.h" namespace OCIO_NAMESPACE { @@ -728,118 +728,6 @@ void FileRulesMerger::handleRemove() namespace { -bool viewingRulesAreEqual(const ConstViewingRulesRcPtr & r1, - size_t r1Idx, - const ConstViewingRulesRcPtr & r2, - size_t r2Idx) -{ - // NB: No need to compare the name of the rules, that should be done in the caller. - - // Compare color space tokens, handling the case where they may be in a different order. - - if (r1->getNumColorSpaces(r1Idx) != r2->getNumColorSpaces(r2Idx)) - { - return false; - } - - TokensManager r1ColorSpaces; - for (size_t m = 0; m < r1->getNumColorSpaces(r1Idx); m++) - { - r1ColorSpaces.addToken(r1->getColorSpace(r1Idx, m)); - } - - for (size_t m = 0; m < r2->getNumColorSpaces(r2Idx); m++) - { - if (!r1ColorSpaces.hasToken(r2->getColorSpace(r2Idx, m))) - { - return false; - } - } - - // Compare encoding tokens, handling the case where they may be in a different order. - - if (r1->getNumEncodings(r1Idx) != r2->getNumEncodings(r2Idx)) - { - return false; - } - - TokensManager r1Encodings; - for (size_t m = 0; m < r1->getNumEncodings(r1Idx); m++) - { - r1Encodings.addToken(r1->getEncoding(r1Idx, m)); - } - - for (size_t m = 0; m < r2->getNumEncodings(r2Idx); m++) - { - if(!r1Encodings.hasToken(r2->getEncoding(r2Idx, m))) - { - return false; - } - } - - // Compare the custom keys, handling the case where they may be in a different order. - - if (r1->getNumCustomKeys(r1Idx) != r2->getNumCustomKeys(r2Idx)) - { - return false; - } - - CustomKeysContainer r1CustomKeys; - for (size_t m = 0; m < r1->getNumCustomKeys(r1Idx); m++) - { - r1CustomKeys.set(r1->getCustomKeyName(r1Idx, m), r1->getCustomKeyValue(r1Idx, m)); - } - - for (size_t m = 0; m < r2->getNumCustomKeys(r2Idx); m++) - { - if (!r1CustomKeys.hasKey(r2->getCustomKeyName(r2Idx, m))) - { - return false; - } - else - { - if (Platform::Strcasecmp(r1CustomKeys.getValueForKey(r2->getCustomKeyName(r2Idx, m)), - r2->getCustomKeyValue(r2Idx, m)) != 0) - { - return false; - } - } - } - - return true; -} - -void copyViewingRule(const ConstViewingRulesRcPtr & src, - size_t srcIdx, - size_t dstIdx, - ViewingRulesRcPtr & rules) -{ - try - { - rules->insertRule(dstIdx, src->getName(srcIdx)); - - for (int j = 0; j < static_cast(src->getNumColorSpaces(srcIdx)); j++) - { - rules->addColorSpace(dstIdx, src->getColorSpace(srcIdx, j)); - } - - for (int k = 0; k < static_cast(src->getNumEncodings(srcIdx)); k++) - { - rules->addEncoding(dstIdx, src->getEncoding(srcIdx, k)); - } - - for (int l = 0; l < static_cast(src->getNumCustomKeys(srcIdx)); l++) - { - rules->setCustomKey(dstIdx, src->getCustomKeyName(srcIdx, l), src->getCustomKeyValue(srcIdx, l)); - } - } - catch(...) - { - // Don't add it if any errors. - // Continue. - } -} - void addUniqueViewingRules(const ConstViewingRulesRcPtr & rules, ViewingRulesRcPtr & mergedRules) { diff --git a/tests/cpu/ViewingRules_tests.cpp b/tests/cpu/ViewingRules_tests.cpp index a58ffaf63..0c03cb5f7 100644 --- a/tests/cpu/ViewingRules_tests.cpp +++ b/tests/cpu/ViewingRules_tests.cpp @@ -515,3 +515,128 @@ active_views: [] OCIO_CHECK_EQUAL(std::string(configav->getView("sRGB", "c3", 0)), std::string(configav->getDefaultView("sRGB", "c3"))); } + +OCIO_ADD_TEST(ViewingRules, viewing_rules_are_equal) +{ + // Two rules with the same colorspaces (added in a different order) and the same + // custom keys (added in a different order) are equal. The rule name is not compared. + OCIO::ViewingRulesRcPtr r1 = OCIO::ViewingRules::Create(); + OCIO_CHECK_NO_THROW(r1->insertRule(0, "RuleA")); + OCIO_CHECK_NO_THROW(r1->addColorSpace(0, "cs0")); + OCIO_CHECK_NO_THROW(r1->addColorSpace(0, "cs1")); + OCIO_CHECK_NO_THROW(r1->setCustomKey(0, "key0", "value0")); + OCIO_CHECK_NO_THROW(r1->setCustomKey(0, "key1", "value1")); + + OCIO::ViewingRulesRcPtr r2 = OCIO::ViewingRules::Create(); + OCIO_CHECK_NO_THROW(r2->insertRule(0, "RuleB")); + OCIO_CHECK_NO_THROW(r2->addColorSpace(0, "cs1")); + OCIO_CHECK_NO_THROW(r2->addColorSpace(0, "cs0")); + OCIO_CHECK_NO_THROW(r2->setCustomKey(0, "key1", "value1")); + OCIO_CHECK_NO_THROW(r2->setCustomKey(0, "key0", "value0")); + + OCIO_CHECK_ASSERT(OCIO::viewingRulesAreEqual(r1, 0, r2, 0)); + OCIO_CHECK_ASSERT(OCIO::viewingRulesAreEqual(r2, 0, r1, 0)); + + // A different number of colorspaces makes the rules unequal. + OCIO::ViewingRulesRcPtr r3 = OCIO::ViewingRules::Create(); + OCIO_CHECK_NO_THROW(r3->insertRule(0, "RuleC")); + OCIO_CHECK_NO_THROW(r3->addColorSpace(0, "cs0")); + OCIO_CHECK_ASSERT(!OCIO::viewingRulesAreEqual(r1, 0, r3, 0)); + + // The same number of colorspaces but different content makes the rules unequal. + OCIO::ViewingRulesRcPtr r4 = OCIO::ViewingRules::Create(); + OCIO_CHECK_NO_THROW(r4->insertRule(0, "RuleD")); + OCIO_CHECK_NO_THROW(r4->addColorSpace(0, "cs0")); + OCIO_CHECK_NO_THROW(r4->addColorSpace(0, "cs2")); + OCIO_CHECK_ASSERT(!OCIO::viewingRulesAreEqual(r1, 0, r4, 0)); + + // Same colorspaces, but a differing custom key value makes the rules unequal. + OCIO::ViewingRulesRcPtr r5 = OCIO::ViewingRules::Create(); + OCIO_CHECK_NO_THROW(r5->insertRule(0, "RuleE")); + OCIO_CHECK_NO_THROW(r5->addColorSpace(0, "cs0")); + OCIO_CHECK_NO_THROW(r5->addColorSpace(0, "cs1")); + OCIO_CHECK_NO_THROW(r5->setCustomKey(0, "key0", "value0")); + OCIO_CHECK_NO_THROW(r5->setCustomKey(0, "key1", "different")); + OCIO_CHECK_ASSERT(!OCIO::viewingRulesAreEqual(r1, 0, r5, 0)); + + // Same colorspaces and custom-key count, but a differing custom key name (rather than + // value) also makes the rules unequal. + OCIO::ViewingRulesRcPtr r5b = OCIO::ViewingRules::Create(); + OCIO_CHECK_NO_THROW(r5b->insertRule(0, "RuleE2")); + OCIO_CHECK_NO_THROW(r5b->addColorSpace(0, "cs0")); + OCIO_CHECK_NO_THROW(r5b->addColorSpace(0, "cs1")); + OCIO_CHECK_NO_THROW(r5b->setCustomKey(0, "key0", "value0")); + OCIO_CHECK_NO_THROW(r5b->setCustomKey(0, "keyOther", "value1")); + OCIO_CHECK_ASSERT(!OCIO::viewingRulesAreEqual(r1, 0, r5b, 0)); + + // Rules built from encodings instead of colorspaces compare the same way. + OCIO::ViewingRulesRcPtr r6 = OCIO::ViewingRules::Create(); + OCIO_CHECK_NO_THROW(r6->insertRule(0, "RuleF")); + OCIO_CHECK_NO_THROW(r6->addEncoding(0, "enc0")); + OCIO_CHECK_NO_THROW(r6->addEncoding(0, "enc1")); + + OCIO::ViewingRulesRcPtr r7 = OCIO::ViewingRules::Create(); + OCIO_CHECK_NO_THROW(r7->insertRule(0, "RuleG")); + OCIO_CHECK_NO_THROW(r7->addEncoding(0, "enc1")); + OCIO_CHECK_NO_THROW(r7->addEncoding(0, "enc0")); + OCIO_CHECK_ASSERT(OCIO::viewingRulesAreEqual(r6, 0, r7, 0)); + + OCIO::ViewingRulesRcPtr r8 = OCIO::ViewingRules::Create(); + OCIO_CHECK_NO_THROW(r8->insertRule(0, "RuleH")); + OCIO_CHECK_NO_THROW(r8->addEncoding(0, "enc0")); + OCIO_CHECK_ASSERT(!OCIO::viewingRulesAreEqual(r6, 0, r8, 0)); + + // The same number of encodings but different content also makes the rules unequal. + OCIO::ViewingRulesRcPtr r9 = OCIO::ViewingRules::Create(); + OCIO_CHECK_NO_THROW(r9->insertRule(0, "RuleI")); + OCIO_CHECK_NO_THROW(r9->addEncoding(0, "enc0")); + OCIO_CHECK_NO_THROW(r9->addEncoding(0, "enc2")); + OCIO_CHECK_ASSERT(!OCIO::viewingRulesAreEqual(r6, 0, r9, 0)); + + // Comparing different rule indices within multi-rule sets works as expected. + OCIO_CHECK_NO_THROW(r1->insertRule(1, "RuleA2")); + OCIO_CHECK_NO_THROW(r1->addColorSpace(1, "cs0")); + OCIO_CHECK_ASSERT(OCIO::viewingRulesAreEqual(r1, 1, r3, 0)); + OCIO_CHECK_ASSERT(!OCIO::viewingRulesAreEqual(r1, 0, r1, 1)); +} + +OCIO_ADD_TEST(ViewingRules, copy_viewing_rule) +{ + // Copying a rule using colorspaces and custom keys reproduces it exactly (aside from + // the name, which is also copied) at the destination index. + OCIO::ViewingRulesRcPtr src = OCIO::ViewingRules::Create(); + OCIO_CHECK_NO_THROW(src->insertRule(0, "SrcRule")); + OCIO_CHECK_NO_THROW(src->addColorSpace(0, "cs0")); + OCIO_CHECK_NO_THROW(src->addColorSpace(0, "cs1")); + OCIO_CHECK_NO_THROW(src->setCustomKey(0, "key0", "value0")); + + OCIO::ViewingRulesRcPtr dst = OCIO::ViewingRules::Create(); + OCIO_CHECK_NO_THROW(OCIO::copyViewingRule(src, 0, 0, dst)); + + OCIO_REQUIRE_EQUAL(dst->getNumEntries(), 1); + OCIO_CHECK_ASSERT(OCIO::viewingRulesAreEqual(src, 0, dst, 0)); + std::string stringVal; + OCIO_CHECK_NO_THROW(stringVal = dst->getName(0)); + OCIO_CHECK_EQUAL(stringVal, "SrcRule"); + + // Copying appends at the destination index without disturbing an existing rule there. + OCIO::ViewingRulesRcPtr srcEnc = OCIO::ViewingRules::Create(); + OCIO_CHECK_NO_THROW(srcEnc->insertRule(0, "SrcEncRule")); + OCIO_CHECK_NO_THROW(srcEnc->addEncoding(0, "enc0")); + OCIO_CHECK_NO_THROW(OCIO::copyViewingRule(srcEnc, 0, dst->getNumEntries(), dst)); + + OCIO_REQUIRE_EQUAL(dst->getNumEntries(), 2); + OCIO_CHECK_ASSERT(OCIO::viewingRulesAreEqual(src, 0, dst, 0)); + OCIO_CHECK_ASSERT(OCIO::viewingRulesAreEqual(srcEnc, 0, dst, 1)); + + // The copy is best-effort: a name collision means insertRule throws immediately (before + // any colorspace/encoding/custom-key is applied), so the exception is swallowed and the + // destination is left exactly as it was before the call. + OCIO::ViewingRulesRcPtr srcDup = OCIO::ViewingRules::Create(); + OCIO_CHECK_NO_THROW(srcDup->insertRule(0, "SrcRule")); + OCIO_CHECK_NO_THROW(srcDup->addColorSpace(0, "cs2")); + OCIO_CHECK_NO_THROW(OCIO::copyViewingRule(srcDup, 0, dst->getNumEntries(), dst)); + OCIO_REQUIRE_EQUAL(dst->getNumEntries(), 2); + OCIO_CHECK_ASSERT(OCIO::viewingRulesAreEqual(src, 0, dst, 0)); + OCIO_CHECK_ASSERT(OCIO::viewingRulesAreEqual(srcEnc, 0, dst, 1)); +}