From db9a45a749ef9c7804fef4d71c803ebb34b1b35e Mon Sep 17 00:00:00 2001 From: Kevin Wheatley Date: Mon, 12 Jan 2026 17:08:02 +0000 Subject: [PATCH 1/6] Op.cpp: remove dependency on pystring Signed-off-by: Kevin Wheatley --- src/OpenColorIO/Op.cpp | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/src/OpenColorIO/Op.cpp b/src/OpenColorIO/Op.cpp index 7e95baecfc..ce45b3f5d6 100755 --- a/src/OpenColorIO/Op.cpp +++ b/src/OpenColorIO/Op.cpp @@ -1,11 +1,9 @@ // SPDX-License-Identifier: BSD-3-Clause // Copyright Contributors to the OpenColorIO Project. -#include +#include #include -#include - #include #include "Logging.h" @@ -473,16 +471,14 @@ std::ostream& operator<< (std::ostream & os, const Op & op) std::string SerializeOpVec(const OpRcPtrVec & ops, int indent) { std::ostringstream oss; + const std::string indentStr(indent, ' '); - for (OpRcPtrVec::size_type idx = 0, size = ops.size(); idx < size; ++idx) + OpRcPtrVec::size_type idx = 0; + for (const auto & op : ops) { - const OpRcPtr & op = ops[idx]; - - oss << pystring::mul(" ", indent); - oss << "Op " << idx << ": " << *op << " "; - oss << op->getCacheID(); - - oss << "\n"; + oss << indentStr << "Op " << idx << ": " << *op << " " + << op->getCacheID() << "\n"; + ++idx; } return oss.str(); From 3108f0a2683e934fde8ece6bce0c75a1f81b78ce Mon Sep 17 00:00:00 2001 From: Kevin Wheatley Date: Mon, 12 Jan 2026 17:12:48 +0000 Subject: [PATCH 2/6] Op.h: Remove unused sstream include Add sstream to LogUtils.cpp which was previously being included by transitive dependency in Op.h Signed-off-by: Kevin Wheatley --- src/OpenColorIO/Op.h | 1 - src/OpenColorIO/ops/log/LogUtils.cpp | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenColorIO/Op.h b/src/OpenColorIO/Op.h index 895c426d80..d323534561 100644 --- a/src/OpenColorIO/Op.h +++ b/src/OpenColorIO/Op.h @@ -5,7 +5,6 @@ #ifndef INCLUDED_OCIO_OP_H #define INCLUDED_OCIO_OP_H -#include #include #include diff --git a/src/OpenColorIO/ops/log/LogUtils.cpp b/src/OpenColorIO/ops/log/LogUtils.cpp index 77548acfe6..e023337774 100644 --- a/src/OpenColorIO/ops/log/LogUtils.cpp +++ b/src/OpenColorIO/ops/log/LogUtils.cpp @@ -3,6 +3,7 @@ #include #include +#include #include From ff859cd18c87a8e57abe70b095d3e948f344f1ec Mon Sep 17 00:00:00 2001 From: Kevin Wheatley Date: Mon, 12 Jan 2026 18:06:42 +0000 Subject: [PATCH 3/6] Add additional output during optimisation to show the results of ach stage Store the result of IsDebugLoggingEnabled() to avoid taking the internal mutex and to ensure either none or all the debugging will be output for the duration optimisation call. Avoid calling operator<< between character string literals which can be combined by the preprocessor Replace use of std::endl with "\n" Signed-off-by: Kevin Wheatley --- src/OpenColorIO/OpOptimizers.cpp | 71 ++++++++++++++++++-------------- 1 file changed, 41 insertions(+), 30 deletions(-) diff --git a/src/OpenColorIO/OpOptimizers.cpp b/src/OpenColorIO/OpOptimizers.cpp index 164123f413..5d925d66c1 100755 --- a/src/OpenColorIO/OpOptimizers.cpp +++ b/src/OpenColorIO/OpOptimizers.cpp @@ -615,13 +615,12 @@ void OpRcPtrVec::optimize(OptimizationFlags oFlags) return; } - if (IsDebugLoggingEnabled()) + const bool debugLoggingEnabled = IsDebugLoggingEnabled(); + if (debugLoggingEnabled) { std::ostringstream oss; - oss << std::endl - << "**" << std::endl - << "Optimizing Op Vec..." << std::endl - << SerializeOpVec(*this, 4) << std::endl; + oss << "\n**\nOptimizing Op Vec...\n" + << SerializeOpVec(*this, 4) << "\n"; LogDebug(oss.str()); } @@ -633,16 +632,15 @@ void OpRcPtrVec::optimize(OptimizationFlags oFlags) if (oFlags == OPTIMIZATION_NONE) { - if (IsDebugLoggingEnabled()) + if (debugLoggingEnabled) { OpRcPtrVec::size_type finalSize = size(); std::ostringstream os; - os << "**" << std::endl; - os << "Optimized "; - os << originalSize << "->" << finalSize << ", 1 pass, "; - os << total_nooptype << " no-op types removed\n"; - os << SerializeOpVec(*this, 4); + os << "**\nOptimized " + << originalSize << "->" << finalSize << ", 1 pass, " + << total_nooptype << " no-op types removed\n" + << SerializeOpVec(*this, 4); LogDebug(os.str()); } @@ -678,20 +676,30 @@ void OpRcPtrVec::optimize(OptimizationFlags oFlags) { // Remove all ops for which isNoOp is true, including identity matrices. int noops = optimizeIdentity ? RemoveNoOps(*this) : 0; + if (debugLoggingEnabled) + LogDebug(std::string("RemoveNoOps\n") + SerializeOpVec(*this, 4)); // Replace all complex ops with simpler ops (e.g., a CDL which only scales with a matrix). // Note this might increase the number of ops. int replacedOps = replaceOps ? ReplaceOps(*this) : 0; + if (debugLoggingEnabled) + LogDebug(std::string("ReplaceOps\n") + SerializeOpVec(*this, 4)); // Replace all complex identities with simpler ops (e.g., an identity Lut1D with a range). int identityops = ReplaceIdentityOps(*this, oFlags); + if (debugLoggingEnabled) + LogDebug(std::string("ReplaceIdentityOps\n") + SerializeOpVec(*this, 4)); // Remove all adjacent pairs of ops that are inverses of each other. int inverseops = RemoveInverseOps(*this, oFlags); + if (debugLoggingEnabled) + LogDebug(std::string("RemoveInverseOps\n") + SerializeOpVec(*this, 4)); // Combine a pair of ops, for example multiply two adjacent Matrix ops. // (Combines at most one pair on each iteration.) int combines = CombineOps(*this, oFlags); + if (debugLoggingEnabled) + LogDebug(std::string("CombineOps\n") + SerializeOpVec(*this, 4)); if (noops + identityops + inverseops + combines == 0) { @@ -701,6 +709,10 @@ void OpRcPtrVec::optimize(OptimizationFlags oFlags) if (fastLut) { const int inverses = ReplaceInverseLuts(*this); + if (debugLoggingEnabled) + LogDebug(std::string("ReplaceInverseLuts\n") + + SerializeOpVec(*this, 4)); + if (inverses == 0) { break; @@ -723,34 +735,33 @@ void OpRcPtrVec::optimize(OptimizationFlags oFlags) ++passes; } - if (passes == MAX_OPTIMIZATION_PASSES) + if (debugLoggingEnabled && (passes == MAX_OPTIMIZATION_PASSES)) { std::ostringstream os; - os << "The max number of passes, " << passes << ", "; - os << "was reached during optimization. This is likely a sign "; - os << "that either the complexity of the color transform is "; - os << "very high, or that some internal optimizers are in conflict "; - os << "(undo-ing / redo-ing the other's results)."; + os << "The max number of passes, " << passes << ", " + "was reached during optimization. This is likely a sign " + "that either the complexity of the color transform is " + "very high, or that some internal optimizers are in conflict " + "(undo-ing / redo-ing the other's results)."; LogDebug(os.str()); } - if (IsDebugLoggingEnabled()) + if (debugLoggingEnabled) { OpRcPtrVec::size_type finalSize = size(); std::ostringstream os; - os << "**" << std::endl; - os << "Optimized "; - os << originalSize << "->" << finalSize << ", "; - os << passes << " passes, "; - os << total_nooptype << " no-op types removed, "; - os << total_noops << " no-ops removed, "; - os << total_replacedops << " ops replaced, "; - os << total_identityops << " identity ops replaced, "; - os << total_inverseops << " inverse op pairs removed, "; - os << total_combines << " ops combined, "; - os << total_inverses << " ops inverted\n"; - os << SerializeOpVec(*this, 4); + os << "**\nOptimized " + << originalSize << "->" << finalSize << ", " + << passes << " passes, " + << total_nooptype << " no-op types removed, " + << total_noops << " no-ops removed, " + << total_replacedops << " ops replaced, " + << total_identityops << " identity ops replaced, " + << total_inverseops << " inverse op pairs removed, " + << total_combines << " ops combined, " + << total_inverses << " ops inverted\n" + << SerializeOpVec(*this, 4); LogDebug(os.str()); } } From a818bb74f38f1a3c0b4597419e6940e0ae8e7963 Mon Sep 17 00:00:00 2001 From: Kevin Wheatley Date: Mon, 12 Jan 2026 20:36:34 +0000 Subject: [PATCH 4/6] Add for memcpy to test code Signed-off-by: Kevin Wheatley --- tests/cpu/Op_tests.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/cpu/Op_tests.cpp b/tests/cpu/Op_tests.cpp index d6c1fb3c00..0db960d3ef 100644 --- a/tests/cpu/Op_tests.cpp +++ b/tests/cpu/Op_tests.cpp @@ -1,6 +1,7 @@ // SPDX-License-Identifier: BSD-3-Clause // Copyright Contributors to the OpenColorIO Project. +#include #include "Op.cpp" From 40944b6949472f5184d1deba1ac5d81fe43dd925 Mon Sep 17 00:00:00 2001 From: Kevin Wheatley Date: Tue, 23 Jun 2026 17:52:53 +0100 Subject: [PATCH 5/6] Improve uniformity of optimisation passes Output Ops list only when it changes Ensure passes count is >=1 even when optimisation cannot be done. Signed-off-by: Kevin Wheatley --- src/OpenColorIO/OpOptimizers.cpp | 176 ++++++++++++++++++------------- tests/cpu/OpOptimizers_tests.cpp | 52 ++++++++- 2 files changed, 151 insertions(+), 77 deletions(-) diff --git a/src/OpenColorIO/OpOptimizers.cpp b/src/OpenColorIO/OpOptimizers.cpp index 5d925d66c1..2e267804ab 100755 --- a/src/OpenColorIO/OpOptimizers.cpp +++ b/src/OpenColorIO/OpOptimizers.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include @@ -71,7 +72,7 @@ bool IsCombineEnabled(OpData::Type type, OptimizationFlags flags) constexpr int MAX_OPTIMIZATION_PASSES = 80; -int RemoveNoOpTypes(OpRcPtrVec & opVec) +int RemoveNoOpTypes(OpRcPtrVec & opVec, [[maybe_unused]] OptimizationFlags flags) { int count = 0; @@ -94,8 +95,15 @@ int RemoveNoOpTypes(OpRcPtrVec & opVec) } // Ops are preserved, dynamic properties are made non-dynamic. -void RemoveDynamicProperties(OpRcPtrVec & opVec) +int RemoveDynamicProperties(OpRcPtrVec & opVec, OptimizationFlags oFlags) { + int count = 0; + const auto removeDynamic = HasFlag(oFlags, OPTIMIZATION_NO_DYNAMIC_PROPERTIES); + if (!removeDynamic) + { + return count; + } + const size_t nbOps = opVec.size(); for (size_t i = 0; i < nbOps; ++i) { @@ -106,13 +114,21 @@ void RemoveDynamicProperties(OpRcPtrVec & opVec) auto replacedBy = op->clone(); replacedBy->removeDynamicProperties(); opVec[i] = replacedBy; + ++count; } } + return count; } -int RemoveNoOps(OpRcPtrVec & opVec) +int RemoveNoOps(OpRcPtrVec & opVec, OptimizationFlags oFlags) { int count = 0; + const bool optimizeIdentity = HasFlag(oFlags, OPTIMIZATION_IDENTITY); + if (!optimizeIdentity) + { + return count; + } + OpRcPtrVec::const_iterator iter = opVec.begin(); while (iter != opVec.end()) { @@ -140,9 +156,15 @@ void FinalizeOps(OpRcPtrVec & opVec) // Some rather complex ops can get replaced based on their data by simpler ops. // For instance CDL that does not use power will get replaced. -int ReplaceOps(OpRcPtrVec & opVec) +int ReplaceOps(OpRcPtrVec & opVec, [[maybe_unused]] OptimizationFlags oFlags) { int count = 0; + const bool replaceOps = HasFlag(oFlags, OPTIMIZATION_SIMPLIFY_OPS); + if (!replaceOps) + { + return count; + } + int firstindex = 0; // this must be a signed int OpRcPtrVec tmpops; @@ -181,24 +203,26 @@ int ReplaceIdentityOps(OpRcPtrVec & opVec, OptimizationFlags oFlags) // Remove identity gamma ops (handled separately to give control over negative // alpha clamping). const bool optIdGamma = HasFlag(oFlags, OPTIMIZATION_IDENTITY_GAMMA); - if (optIdentity || optIdGamma) + if (!optIdentity && !optIdGamma) + { + return count; + } + + const size_t nbOps = opVec.size(); + for (size_t i = 0; i < nbOps; ++i) { - const size_t nbOps = opVec.size(); - for (size_t i = 0; i < nbOps; ++i) + ConstOpRcPtr op = opVec[i]; + const auto type = op->data()->getType(); + if (type != OpData::RangeType && // Do not replace a range identity. + ((type == OpData::GammaType && optIdGamma) || + (type != OpData::GammaType && optIdentity)) && + op->isIdentity()) { - ConstOpRcPtr op = opVec[i]; - const auto type = op->data()->getType(); - if (type != OpData::RangeType && // Do not replace a range identity. - ((type == OpData::GammaType && optIdGamma) || - (type != OpData::GammaType && optIdentity)) && - op->isIdentity()) - { - // Optimization flag is tested before. - auto replacedBy = op->getIdentityReplacement(); - replacedBy->finalize(); - opVec[i] = replacedBy; - ++count; - } + // Optimization flag is tested before. + auto replacedBy = op->getIdentityReplacement(); + replacedBy->finalize(); + opVec[i] = replacedBy; + ++count; } } return count; @@ -366,9 +390,14 @@ int CombineOps(OpRcPtrVec & opVec, OptimizationFlags oFlags) // LUT is used and so this is quite accurate even for scene-linear values, but for Lut3D the baked // version is more of an approximation. The default optimization level uses the FAST method since // it is the only one available on both CPU and GPU. -int ReplaceInverseLuts(OpRcPtrVec & opVec) +int ReplaceInverseLuts(OpRcPtrVec & opVec, OptimizationFlags oFlags) { int count = 0; + const bool fastLut = HasFlag(oFlags, OPTIMIZATION_LUT_INV_FAST); + if (!fastLut) + { + return count; + } const size_t nbOps = opVec.size(); for (size_t i = 0; i < nbOps; ++i) @@ -404,7 +433,6 @@ int ReplaceInverseLuts(OpRcPtrVec & opVec) } } return count; - } int RemoveLeadingClampIdentity(OpRcPtrVec & opVec) @@ -417,7 +445,7 @@ int RemoveLeadingClampIdentity(OpRcPtrVec & opVec) auto oData = o->data(); if (oData->getType() == OpData::RangeType && oData->isIdentity()) { - iter++; + ++iter; ++count; } else @@ -593,6 +621,20 @@ void OptimizeSeparablePrefix(OpRcPtrVec & ops, BitDepth in) ops.insert(ops.begin(), lutOps.begin(), lutOps.end()); } + +int PerformOptimisation(int (*Operation)(OpRcPtrVec &, OptimizationFlags), OpRcPtrVec & opVec, OptimizationFlags oFlags, bool debugLoggingEnabled, std::string operationName) +{ + const int ops_removed = Operation(opVec, oFlags); + if (debugLoggingEnabled) + { + const std::string message = operationName + std::string(" - ") + std::to_string(ops_removed) + std::string(" optimisations found\n") + + ((ops_removed != 0) ? SerializeOpVec(opVec, 4) : std::string("")); + + LogDebug(message); + } + return ops_removed; +} + } // namespace void OpRcPtrVec::finalize() @@ -619,16 +661,14 @@ void OpRcPtrVec::optimize(OptimizationFlags oFlags) if (debugLoggingEnabled) { std::ostringstream oss; - oss << "\n**\nOptimizing Op Vec...\n" - << SerializeOpVec(*this, 4) << "\n"; - + oss << "\n**\nOptimizing Op Vec...\n" << SerializeOpVec(*this, 4); LogDebug(oss.str()); } const auto originalSize = size(); // NoOpType can be removed (facilitates conversion to a CPU/GPUProcessor). - const int total_nooptype = RemoveNoOpTypes(*this); + const int total_nooptype = PerformOptimisation(RemoveNoOpTypes, *this, oFlags, debugLoggingEnabled, "RemoveNoOpTypes"); if (oFlags == OPTIMIZATION_NONE) { @@ -637,8 +677,7 @@ void OpRcPtrVec::optimize(OptimizationFlags oFlags) OpRcPtrVec::size_type finalSize = size(); std::ostringstream os; - os << "**\nOptimized " - << originalSize << "->" << finalSize << ", 1 pass, " + os << "**\nOptimized " << originalSize << "->" << finalSize << ", 1 pass, " << total_nooptype << " no-op types removed\n" << SerializeOpVec(*this, 4); LogDebug(os.str()); @@ -649,10 +688,11 @@ void OpRcPtrVec::optimize(OptimizationFlags oFlags) // Keep dynamic ops using their default values. Remove the ability to modify // them dynamically. - const auto removeDynamic = HasFlag(oFlags, OPTIMIZATION_NO_DYNAMIC_PROPERTIES); - if (removeDynamic) + const int dynamicOps = RemoveDynamicProperties(*this, oFlags); + if (debugLoggingEnabled) { - RemoveDynamicProperties(*this); + const auto message = std::string("RemoveDynamicProperties - ") + std::to_string(dynamicOps) + std::string(" removed"); + LogDebug(message); } // As the input and output bit-depths represent the color processing @@ -665,73 +705,63 @@ void OpRcPtrVec::optimize(OptimizationFlags oFlags) int total_inverseops = 0; int total_combines = 0; int total_inverses = 0; - int passes = 0; - - const bool optimizeIdentity = HasFlag(oFlags, OPTIMIZATION_IDENTITY); - const bool replaceOps = HasFlag(oFlags, OPTIMIZATION_SIMPLIFY_OPS); - - const bool fastLut = HasFlag(oFlags, OPTIMIZATION_LUT_INV_FAST); + int passes = 1; while (passes <= MAX_OPTIMIZATION_PASSES) { - // Remove all ops for which isNoOp is true, including identity matrices. - int noops = optimizeIdentity ? RemoveNoOps(*this) : 0; if (debugLoggingEnabled) - LogDebug(std::string("RemoveNoOps\n") + SerializeOpVec(*this, 4)); + { + const auto message = std::string("Starting pass ") + std::to_string(passes); + LogDebug(message); + } + // Remove all ops for which isNoOp is true, including identity matrices. + const int noops = PerformOptimisation(RemoveNoOps, *this, oFlags, debugLoggingEnabled, "RemoveNoOps"); + total_noops += noops; // Replace all complex ops with simpler ops (e.g., a CDL which only scales with a matrix). // Note this might increase the number of ops. - int replacedOps = replaceOps ? ReplaceOps(*this) : 0; - if (debugLoggingEnabled) - LogDebug(std::string("ReplaceOps\n") + SerializeOpVec(*this, 4)); + const int replacedOps = PerformOptimisation(ReplaceOps, *this, oFlags, debugLoggingEnabled, "ReplaceOps"); + total_replacedops += replacedOps; // Replace all complex identities with simpler ops (e.g., an identity Lut1D with a range). - int identityops = ReplaceIdentityOps(*this, oFlags); - if (debugLoggingEnabled) - LogDebug(std::string("ReplaceIdentityOps\n") + SerializeOpVec(*this, 4)); + const int identityops = PerformOptimisation(ReplaceIdentityOps, *this, oFlags, debugLoggingEnabled, "ReplaceIdentityOps"); + total_identityops += identityops; // Remove all adjacent pairs of ops that are inverses of each other. - int inverseops = RemoveInverseOps(*this, oFlags); - if (debugLoggingEnabled) - LogDebug(std::string("RemoveInverseOps\n") + SerializeOpVec(*this, 4)); + const int inverseops = PerformOptimisation(RemoveInverseOps, *this, oFlags, debugLoggingEnabled, "RemoveInverseOps"); + total_inverseops += inverseops; // Combine a pair of ops, for example multiply two adjacent Matrix ops. // (Combines at most one pair on each iteration.) - int combines = CombineOps(*this, oFlags); - if (debugLoggingEnabled) - LogDebug(std::string("CombineOps\n") + SerializeOpVec(*this, 4)); + const int combines = PerformOptimisation(CombineOps, *this, oFlags, debugLoggingEnabled, "CombineOps"); + total_combines += combines; - if (noops + identityops + inverseops + combines == 0) + if (noops + replacedOps + identityops + inverseops + combines == 0) { // No optimization progress was made, so stop trying. If requested, replace any // inverse LUTs with faster forward LUTs and do another pass to see if more // optimization is possible. - if (fastLut) - { - const int inverses = ReplaceInverseLuts(*this); - if (debugLoggingEnabled) - LogDebug(std::string("ReplaceInverseLuts\n") - + SerializeOpVec(*this, 4)); - - if (inverses == 0) - { - break; - } + const int inverses = PerformOptimisation(ReplaceInverseLuts, *this, oFlags, debugLoggingEnabled, "ReplaceInverseLuts"); + total_inverses += inverses; - total_inverses += inverses; - } - else + if (inverses == 0) { break; } } - total_noops += noops; - total_replacedops += replacedOps; - total_identityops += identityops; - total_inverseops += inverseops; - total_combines += combines; + if (debugLoggingEnabled) + { + std::ostringstream os; + os << "Pass " << passes << " summary: " + << noops << " no-op removed, " + << replacedOps << " ops replaced, " + << identityops << " identity ops replaced, " + << inverseops << " inverse op pairs removed, " + << combines << " ops combined."; + LogDebug(os.str()); + } ++passes; } diff --git a/tests/cpu/OpOptimizers_tests.cpp b/tests/cpu/OpOptimizers_tests.cpp index 5b2f771a72..5cb81f3fc9 100644 --- a/tests/cpu/OpOptimizers_tests.cpp +++ b/tests/cpu/OpOptimizers_tests.cpp @@ -311,9 +311,12 @@ OCIO_ADD_TEST(OpOptimizers, combine_ops) OCIO_CHECK_EQUAL(ops.size(), 3); OCIO::CombineOps(ops, AllBut(OCIO::OPTIMIZATION_COMP_MATRIX)); OCIO_CHECK_EQUAL(ops.size(), 3); - OCIO::CombineOps(ops, OCIO::OPTIMIZATION_ALL); + auto count = OCIO::CombineOps(ops, OCIO::OPTIMIZATION_ALL); // CombineOps removes at most one pair on each call, repeat to combine all pairs. - OCIO::CombineOps(ops, OCIO::OPTIMIZATION_ALL); + OCIO_CHECK_EQUAL(count, 1); + OCIO_CHECK_EQUAL(ops.size(), 2); + count = OCIO::CombineOps(ops, OCIO::OPTIMIZATION_ALL); + OCIO_CHECK_EQUAL(count, 1); OCIO_CHECK_EQUAL(ops.size(), 1); } @@ -325,7 +328,9 @@ OCIO_ADD_TEST(OpOptimizers, combine_ops) OCIO_CHECK_EQUAL(ops.size(), 2); OCIO::CombineOps(ops, AllBut(OCIO::OPTIMIZATION_COMP_MATRIX)); OCIO_CHECK_EQUAL(ops.size(), 2); - OCIO::CombineOps(ops, OCIO::OPTIMIZATION_ALL); + auto count = OCIO::CombineOps(ops, OCIO::OPTIMIZATION_ALL); + // Note: the number of optimisations is 1 even though they both get removed + OCIO_CHECK_EQUAL(count, 1); OCIO_CHECK_EQUAL(ops.size(), 0); } @@ -355,8 +360,11 @@ OCIO_ADD_TEST(OpOptimizers, combine_ops) OCIO_CHECK_EQUAL(ops.size(), 5); OCIO::CombineOps(ops, OCIO::OPTIMIZATION_ALL); // CombineOps removes at most one pair on each call, repeat to combine all pairs. + OCIO_CHECK_EQUAL(ops.size(), 4); OCIO::CombineOps(ops, OCIO::OPTIMIZATION_ALL); + OCIO_CHECK_EQUAL(ops.size(), 3); OCIO::CombineOps(ops, OCIO::OPTIMIZATION_ALL); + OCIO_CHECK_EQUAL(ops.size(), 2); OCIO::CombineOps(ops, OCIO::OPTIMIZATION_ALL); OCIO_CHECK_EQUAL(ops.size(), 1); } @@ -374,6 +382,7 @@ OCIO_ADD_TEST(OpOptimizers, combine_ops) OCIO_CHECK_EQUAL(ops.size(), 4); OCIO::CombineOps(ops, OCIO::OPTIMIZATION_ALL); // CombineOps removes at most one pair on each call, repeat to combine all pairs. + OCIO_CHECK_EQUAL(ops.size(), 2); OCIO::CombineOps(ops, OCIO::OPTIMIZATION_ALL); OCIO_CHECK_EQUAL(ops.size(), 0); } @@ -1315,6 +1324,41 @@ OCIO_ADD_TEST(OpOptimizers, dynamic_ops) } // Test with dynamic exposure contrast. + { + OCIO::OpRcPtrVec ops; + OCIO_CHECK_NO_THROW(OCIO::CreateMatrixOp(ops, matrix, OCIO::TRANSFORM_DIR_FORWARD)); + OCIO_CHECK_NO_THROW(OCIO::CreateExposureContrastOp(ops, exposureDyn, + OCIO::TRANSFORM_DIR_FORWARD)); + OCIO_CHECK_NO_THROW(OCIO::CreateMatrixOp(ops, matrix, OCIO::TRANSFORM_DIR_FORWARD)); + OCIO_REQUIRE_EQUAL(ops.size(), 3); + OCIO_CHECK_ASSERT(!ops[0]->isIdentity()); + + // Exposure contrast is dynamic. + OCIO_CHECK_ASSERT(ops[1]->isDynamic()); + OCIO_CHECK_ASSERT(!ops[1]->isIdentity()); + + OCIO_CHECK_ASSERT(!ops[2]->isIdentity()); + + // It does not get optimized with default flags (OPTIMIZATION_NO_DYNAMIC_PROPERTIES off). + OCIO_CHECK_NO_THROW(ops.finalize()); + OCIO_CHECK_EQUAL(OCIO::RemoveDynamicProperties(ops, OCIO::OPTIMIZATION_DEFAULT), 0); + OCIO_REQUIRE_EQUAL(ops.size(), 3); + + OCIO_CHECK_ASSERT(ops[1]->isDynamic()); + + OCIO_CHECK_EQUAL(ops[0]->getInfo(), ""); + OCIO_CHECK_EQUAL(ops[1]->getInfo(), ""); + OCIO_CHECK_EQUAL(ops[2]->getInfo(), ""); + + // It does get optimized if flag is set. + OCIO_CHECK_ASSERT(HasFlag(OCIO::OPTIMIZATION_ALL, OCIO::OPTIMIZATION_NO_DYNAMIC_PROPERTIES)); + OCIO_CHECK_NO_THROW(ops.finalize()); + OCIO_CHECK_EQUAL(OCIO::RemoveDynamicProperties(ops, OCIO::OPTIMIZATION_ALL), 1); + OCIO_REQUIRE_EQUAL(ops.size(), 3); + + OCIO_CHECK_ASSERT(!ops[1]->isDynamic()); + } + { OCIO::OpRcPtrVec ops; OCIO_CHECK_NO_THROW(OCIO::CreateMatrixOp(ops, matrix, OCIO::TRANSFORM_DIR_FORWARD)); @@ -1554,7 +1598,7 @@ OCIO_ADD_TEST(OpOptimizers, opt_prefix_test1) // First one is the file no op. OCIO_CHECK_EQUAL(ops.size(), 12); - OCIO_CHECK_NO_THROW(OCIO::RemoveNoOpTypes(ops)); + OCIO_CHECK_NO_THROW(OCIO::RemoveNoOpTypes(ops, OCIO::OPTIMIZATION_ALL)); OCIO_CHECK_EQUAL(ops.size(), 11); From 16d396e0fae92eea8ce367eae2665bbe85a93189 Mon Sep 17 00:00:00 2001 From: Kevin Wheatley Date: Wed, 24 Jun 2026 11:07:41 +0100 Subject: [PATCH 6/6] refactor: Remove redundant comment and ensure we always use the passes variable when reporting the pass count. Signed-off-by: Kevin Wheatley --- src/OpenColorIO/OpOptimizers.cpp | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/src/OpenColorIO/OpOptimizers.cpp b/src/OpenColorIO/OpOptimizers.cpp index 2e267804ab..6324097d3e 100755 --- a/src/OpenColorIO/OpOptimizers.cpp +++ b/src/OpenColorIO/OpOptimizers.cpp @@ -666,6 +666,13 @@ void OpRcPtrVec::optimize(OptimizationFlags oFlags) } const auto originalSize = size(); + int total_noops = 0; + int total_replacedops = 0; + int total_identityops = 0; + int total_inverseops = 0; + int total_combines = 0; + int total_inverses = 0; + int passes = 1; // NoOpType can be removed (facilitates conversion to a CPU/GPUProcessor). const int total_nooptype = PerformOptimisation(RemoveNoOpTypes, *this, oFlags, debugLoggingEnabled, "RemoveNoOpTypes"); @@ -677,7 +684,7 @@ void OpRcPtrVec::optimize(OptimizationFlags oFlags) OpRcPtrVec::size_type finalSize = size(); std::ostringstream os; - os << "**\nOptimized " << originalSize << "->" << finalSize << ", 1 pass, " + os << "**\nOptimized " << originalSize << "->" << finalSize << ", " << passes << " pass, " << total_nooptype << " no-op types removed\n" << SerializeOpVec(*this, 4); LogDebug(os.str()); @@ -695,18 +702,6 @@ void OpRcPtrVec::optimize(OptimizationFlags oFlags) LogDebug(message); } - // As the input and output bit-depths represent the color processing - // request and they may be altered by the following optimizations, - // preserve their values. - - int total_noops = 0; - int total_replacedops = 0; - int total_identityops = 0; - int total_inverseops = 0; - int total_combines = 0; - int total_inverses = 0; - int passes = 1; - while (passes <= MAX_OPTIMIZATION_PASSES) { if (debugLoggingEnabled)