From db9a45a749ef9c7804fef4d71c803ebb34b1b35e Mon Sep 17 00:00:00 2001 From: Kevin Wheatley Date: Mon, 12 Jan 2026 17:08:02 +0000 Subject: [PATCH 01/20] 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 02/20] 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 03/20] 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 04/20] 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 05/20] 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 06/20] 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) From 95b2c190269272bf854d404a91fde28a57b088c9 Mon Sep 17 00:00:00 2001 From: Kevin Wheatley Date: Thu, 25 Jun 2026 19:56:12 +0100 Subject: [PATCH 07/20] refactor: Try replace manual iterator handling with std algorithms Signed-off-by: Kevin Wheatley --- src/OpenColorIO/OpOptimizers.cpp | 392 ++++++++++++++----------------- 1 file changed, 176 insertions(+), 216 deletions(-) diff --git a/src/OpenColorIO/OpOptimizers.cpp b/src/OpenColorIO/OpOptimizers.cpp index 6324097d3e..ab963b688d 100755 --- a/src/OpenColorIO/OpOptimizers.cpp +++ b/src/OpenColorIO/OpOptimizers.cpp @@ -74,23 +74,13 @@ constexpr int MAX_OPTIMIZATION_PASSES = 80; int RemoveNoOpTypes(OpRcPtrVec & opVec, [[maybe_unused]] OptimizationFlags flags) { - int count = 0; - - OpRcPtrVec::const_iterator iter = opVec.begin(); - while (iter != opVec.end()) - { - ConstOpRcPtr o = (*iter); - if (o->data()->getType() == OpData::NoOpType) - { - iter = opVec.erase(iter); - ++count; - } - else - { - ++iter; - } - } - + // TODO: with c++20 could use std::erase_if ? + auto newEnd = std::remove_if(opVec.begin(), opVec.end(), [](const auto & o) { + return o->isNoOpType(); + }); + + int count = static_cast(std::distance(newEnd, opVec.end())); + opVec.erase(newEnd, opVec.end()); return count; } @@ -104,50 +94,41 @@ int RemoveDynamicProperties(OpRcPtrVec & opVec, OptimizationFlags oFlags) return count; } - const size_t nbOps = opVec.size(); - for (size_t i = 0; i < nbOps; ++i) - { - auto & op = opVec[i]; + std::for_each(opVec.begin(), opVec.end(), [&count](auto & op) { if (op->isDynamic()) { // Optimization flag is tested before. auto replacedBy = op->clone(); replacedBy->removeDynamicProperties(); - opVec[i] = replacedBy; + op = std::move(replacedBy); ++count; } - } + }); return count; } int RemoveNoOps(OpRcPtrVec & opVec, OptimizationFlags oFlags) { - int count = 0; const bool optimizeIdentity = HasFlag(oFlags, OPTIMIZATION_IDENTITY); if (!optimizeIdentity) { - return count; + return 0; } - OpRcPtrVec::const_iterator iter = opVec.begin(); - while (iter != opVec.end()) + // TODO: with c++20 could use std::erase_if ? + auto newEnd = std::remove_if(opVec.begin(), opVec.end(), [](const auto& op) { - if ((*iter)->isNoOp()) - { - iter = opVec.erase(iter); - ++count; - } - else - { - ++iter; - } - } + return op->isNoOp(); + }); + + int count = static_cast(std::distance(newEnd, opVec.end())); + opVec.erase(newEnd, opVec.end()); return count; } void FinalizeOps(OpRcPtrVec & opVec) { - for (auto op : opVec) + for (const auto &op : opVec) { // Prepare LUT 1D for inversion and ensure Matrix & Range are forward. op->finalize(); @@ -167,6 +148,9 @@ int ReplaceOps(OpRcPtrVec & opVec, [[maybe_unused]] OptimizationFlags oFlags) int firstindex = 0; // this must be a signed int + // Note this erase/insert is potentially O(N^2), an alternative is to build a newOpVec at least as big as the current + // and as we pass throughand append the replacement or push_back the original. + // If the count is non-zero then std::move the newOpVec to OpVec OpRcPtrVec tmpops; while (firstindex < static_cast(opVec.size())) @@ -185,10 +169,16 @@ int ReplaceOps(OpRcPtrVec & opVec, [[maybe_unused]] OptimizationFlags oFlags) // Insert the new ops at this location. opVec.insert(opVec.begin() + firstindex, tmpops.begin(), tmpops.end()); - // We've done something so increment the count! + // Advance index by the number of inserted elements to skip re-evaluating them, + // or add 0 if you want to recursively simplify newly inserted ops. + firstindex += static_cast(tmpops.size()); ++count; } - ++firstindex; + else + { + ++firstindex; + } + } return count; @@ -208,11 +198,10 @@ int ReplaceIdentityOps(OpRcPtrVec & opVec, OptimizationFlags oFlags) return count; } - const size_t nbOps = opVec.size(); - for (size_t i = 0; i < nbOps; ++i) + for (auto & op : opVec) { - ConstOpRcPtr op = opVec[i]; - const auto type = op->data()->getType(); + ConstOpRcPtr constOp = op; // const hackery to be able call getType() vie const member function + const auto type = constOp->data()->getType(); if (type != OpData::RangeType && // Do not replace a range identity. ((type == OpData::GammaType && optIdGamma) || (type != OpData::GammaType && optIdentity)) && @@ -221,7 +210,7 @@ int ReplaceIdentityOps(OpRcPtrVec & opVec, OptimizationFlags oFlags) // Optimization flag is tested before. auto replacedBy = op->getIdentityReplacement(); replacedBy->finalize(); - opVec[i] = replacedBy; + op = std::move(replacedBy); ++count; } } @@ -231,38 +220,30 @@ int ReplaceIdentityOps(OpRcPtrVec & opVec, OptimizationFlags oFlags) int RemoveInverseOps(OpRcPtrVec & opVec, OptimizationFlags oFlags) { int count = 0; - int firstindex = 0; // this must be a signed int + int writeIdx = 0; + const int numOps = static_cast(opVec.size()); - while (firstindex < (static_cast(opVec.size()) - 1)) + for (int readIdx = 0; readIdx < numOps; ++readIdx) { - ConstOpRcPtr op1 = opVec[firstindex]; - ConstOpRcPtr op2 = opVec[firstindex + 1]; - const auto type1 = op1->data()->getType(); - const auto type2 = op2->data()->getType(); + auto & op = opVec[readIdx]; + + if (writeIdx > 0) + { + ConstOpRcPtr lastOp = opVec[writeIdx - 1]; + ConstOpRcPtr constOp = op; + const auto type1 = lastOp->data()->getType(); + const auto type2 = constOp->data()->getType(); + // The common case of inverse ops is to have a deep nesting: // ..., A, B, B', A', ... // - // Consider the above, when firstindex reaches B: - // - // | - // ..., A, B, B', A', ... - // - // We will remove B and B'. - // Firstindex remains pointing at the original location: - // - // | - // ..., A, A', ... - // - // We then decrement firstindex by 1, - // to backstep and reconsider the A, A' case: - // - // | <-- firstindex decremented - // ..., A, A', ... - // + // By treating the processed portion of the vector as a stack (`writeIdx`), + // popping an element automatically exposes `A` to be reconsidered against `A'` + // on the next loop iteration. if (type1 == type2 && IsPairInverseEnabled(type1, oFlags) && - op1->isInverse(op2)) + lastOp->isInverse(constOp)) { // When a pair of inverse ops is removed, we want the optimized ops to give the // same result as the original. For certain ops such as Lut1D or Log this may @@ -274,8 +255,8 @@ int RemoveInverseOps(OpRcPtrVec & opVec, OptimizationFlags oFlags) // Lut1D gets special handling so that both halfs of the pair are available. // Only the inverse LUT has the values needed to generate the replacement. - ConstLut1DOpDataRcPtr lut1 = OCIO_DYNAMIC_POINTER_CAST(op1->data()); - ConstLut1DOpDataRcPtr lut2 = OCIO_DYNAMIC_POINTER_CAST(op2->data()); + ConstLut1DOpDataRcPtr lut1 = OCIO_DYNAMIC_POINTER_CAST(lastOp->data()); + ConstLut1DOpDataRcPtr lut2 = OCIO_DYNAMIC_POINTER_CAST(constOp->data()); OpDataRcPtr opData = lut1->getPairIdentityReplacement(lut2); @@ -296,28 +277,37 @@ int RemoveInverseOps(OpRcPtrVec & opVec, OptimizationFlags oFlags) } else { - replacedBy = op1->getIdentityReplacement(); + replacedBy = lastOp->getIdentityReplacement(); } replacedBy->finalize(); if (replacedBy->isNoOp()) { - opVec.erase(opVec.begin() + firstindex, opVec.begin() + firstindex + 2); - firstindex = std::max(0, firstindex - 1); + // Pop the last element off the stack to naturally backstep + --writeIdx; } else { // Forward + inverse does clamp. - opVec[firstindex] = replacedBy; - opVec.erase(opVec.begin() + firstindex + 1); - ++firstindex; + opVec[writeIdx - 1] = std::move(replacedBy); } ++count; + continue; } - else + } + + // Push the current op to the stack if it wasn't cancelled out + if (writeIdx != readIdx) { - ++firstindex; + opVec[writeIdx] = std::move(op); } + ++writeIdx; + } + + if (count > 0) + { + // Drop any unused elements at the tail in a single O(N) pass + opVec.erase(opVec.begin() + writeIdx, opVec.end()); } return count; @@ -325,61 +315,46 @@ int RemoveInverseOps(OpRcPtrVec & opVec, OptimizationFlags oFlags) int CombineOps(OpRcPtrVec & opVec, OptimizationFlags oFlags) { - int count = 0; - int firstindex = 0; // this must be a signed int - - OpRcPtrVec tmpops; - - while (firstindex < (static_cast(opVec.size()) - 1)) - { - ConstOpRcPtr op1 = opVec[firstindex]; - ConstOpRcPtr op2 = opVec[firstindex + 1]; - const auto type1 = op1->data()->getType(); - - if (IsCombineEnabled(type1, oFlags) && op1->canCombineWith(op2)) - { - tmpops.clear(); - op1->combineWith(tmpops, op2); - FinalizeOps(tmpops); - - // The tmpops may have any number of ops in it: (0, 1, 2, ...). - // (Size 0 would occur only if the combination results in a no-op, - // for example, a pair of matrices that compose into a no-op are - // returned as empty rather than as an identity matrix.) - // - // No matter the number, we need to swap them in for the original ops. - - // Erase the initial two ops we've combined. - opVec.erase(opVec.begin() + firstindex, opVec.begin() + firstindex + 2); - - // Insert the new ops (which may be empty) at this location. - opVec.insert(opVec.begin() + firstindex, tmpops.begin(), tmpops.end()); - - // Decrement firstindex by 1, - // to backstep and reconsider the A, A' case. - // See RemoveInverseOps for the full discussion of - // why this is appropriate. - firstindex = std::max(0, firstindex - 1); + auto it = std::adjacent_find(opVec.begin(), opVec.end(), + [oFlags](const auto & ptr1, const auto & ptr2) { + ConstOpRcPtr op1 = ptr1; + ConstOpRcPtr op2 = ptr2; + return IsCombineEnabled(op1->data()->getType(), oFlags) && op1->canCombineWith(op2); + }); + + if (it != opVec.end()) + { + OpRcPtrVec tmpops; + ConstOpRcPtr op1 = *it; + ConstOpRcPtr op2 = *(it + 1); + + op1->combineWith(tmpops, op2); + FinalizeOps(tmpops); + + // The tmpops may have any number of ops in it: (0, 1, 2, ...). + // (Size 0 would occur only if the combination results in a no-op, + // for example, a pair of matrices that compose into a no-op are + // returned as empty rather than as an identity matrix.) + // + // No matter the number, we need to swap them in for the original ops. - // We've done something so increment the count! - ++count; + // Erase the initial two ops we've combined. + it = opVec.erase(it, it + 2); + + // Insert the new ops (which may be empty) at this location. + opVec.insert(it, tmpops.begin(), tmpops.end()); - // Break, since combining ops is less desirable than other optimization options. - // For example, it is preferable to remove a pair of ops using RemoveInverseOps - // rather than combining them. Consider this example: - // Lut1D A --> Matrix B --> Matrix C --> Lut1D Ainv - // If Matrix B & C are not pair inverses but do combine into an identity, then - // CombineOps would compose Lut1D A & Ainv, into a new Lut1D rather than - // allowing another round of optimization which would remove them as inverses. - break; - } - else - { - ++firstindex; - } + // Return 1 since combining ops is less desirable than other optimization options. + // For example, it is preferable to remove a pair of ops using RemoveInverseOps + // rather than combining them. Consider this example: + // Lut1D A --> Matrix B --> Matrix C --> Lut1D Ainv + // If Matrix B & C are not pair inverses but do combine into an identity, then + // CombineOps would compose Lut1D A & Ainv, into a new Lut1D rather than + // allowing another round of optimization which would remove them as inverses. + return 1; } - return count; + return 0; } // Replace any Lut1D or Lut3D that specify inverse evaluation with a faster forward approximation. @@ -399,11 +374,10 @@ int ReplaceInverseLuts(OpRcPtrVec & opVec, OptimizationFlags oFlags) return count; } - const size_t nbOps = opVec.size(); - for (size_t i = 0; i < nbOps; ++i) + for (auto & op : opVec) { - ConstOpRcPtr op = opVec[i]; - auto opData = op->data(); + ConstOpRcPtr constOp = op; + auto opData = constOp->data(); const auto type = opData->getType(); if (type == OpData::Lut1DType) { @@ -414,7 +388,7 @@ int ReplaceInverseLuts(OpRcPtrVec & opVec, OptimizationFlags oFlags) OpRcPtrVec tmpops; CreateLut1DOp(tmpops, invLutData, TRANSFORM_DIR_FORWARD); FinalizeOps(tmpops); - opVec[i] = tmpops[0]; + op = std::move(tmpops[0]); ++count; } } @@ -427,63 +401,42 @@ int ReplaceInverseLuts(OpRcPtrVec & opVec, OptimizationFlags oFlags) OpRcPtrVec tmpops; CreateLut3DOp(tmpops, invLutData, TRANSFORM_DIR_FORWARD); FinalizeOps(tmpops); - opVec[i] = tmpops[0]; + op = std::move(tmpops[0]); ++count; } } } - return count; -} + return count;} int RemoveLeadingClampIdentity(OpRcPtrVec & opVec) { - int count = 0; - OpRcPtrVec::const_iterator iter = opVec.begin(); - while (iter != opVec.end()) - { - ConstOpRcPtr o = (*iter); - auto oData = o->data(); - if (oData->getType() == OpData::RangeType && oData->isIdentity()) - { - ++iter; - ++count; - } - else - { - break; - } - } - if (count != 0) + auto it = std::find_if_not(opVec.begin(), opVec.end(), [](const auto & op) { + ConstOpRcPtr constOp = op; + auto oData = constOp->data(); + return oData->getType() == OpData::RangeType && oData->isIdentity(); + }); + + int count = static_cast(std::distance(opVec.begin(), it)); + if (count > 0) { - OpRcPtrVec::const_iterator iter = opVec.begin() + count; - opVec.erase(opVec.begin(), iter); + opVec.erase(opVec.begin(), it); } return count; } int RemoveTrailingClampIdentity(OpRcPtrVec & opVec) { - int count = 0; - int current = static_cast(opVec.size()) - 1; - while (current >= 0) - { - ConstOpRcPtr o = opVec[current]; - auto oData = o->data(); - if (oData->getType() == OpData::RangeType && oData->isIdentity()) - { - ++count; - --current; - } - else - { - break; - } - } + // Note the use of reverse iterators + auto rit = std::find_if_not(opVec.rbegin(), opVec.rend(), [](const auto & op) { + ConstOpRcPtr constOp = op; + auto oData = constOp->data(); + return oData->getType() == OpData::RangeType && oData->isIdentity(); + }); - if (count != 0) + int count = static_cast(std::distance(opVec.rbegin(), rit)); + if (count > 0) { - OpRcPtrVec::const_iterator iter = opVec.begin() + (current + 1); - opVec.erase(iter, opVec.end()); + opVec.erase(rit.base(), opVec.end()); } return count; } @@ -500,23 +453,16 @@ int RemoveTrailingClampIdentity(OpRcPtrVec & opVec) // unsigned FindSeparablePrefix(const OpRcPtrVec & ops) { - unsigned prefixLen = 0; - // Loop over the ops until we get to one that cannot be combined. // // Note: For some ops such as Matrix and CDL, the separability depends upon // the parameters. - for (const auto & op : ops) - { + auto it = std::find_if(ops.begin(), ops.end(), [](const auto & op) { // In OCIO, the hasChannelCrosstalk method returns false for separable ops. - if (op->hasChannelCrosstalk() || op->isDynamic()) - { - break; - } + return op->hasChannelCrosstalk() || op->isDynamic(); + }); - // Op is separable, keep going. - prefixLen++; - } + unsigned prefixLen = static_cast(std::distance(ops.begin(), it)); // If the only op is a 1D LUT, there is actually nothing to optimize // so set the length to 0. (This also avoids an infinite loop.) @@ -538,11 +484,7 @@ unsigned FindSeparablePrefix(const OpRcPtrVec & ops) // Some ops are so fast that it may not make sense to replace just one of those. // E.g., if it's just a single matrix, it may not be faster to replace it with a LUT. // So make sure there are some more expensive ops to combine. - unsigned expensiveOps = 0U; - for (unsigned i = 0; i < prefixLen; ++i) - { - auto op = ops[i]; - + auto expensiveOps = std::count_if(ops.begin(), ops.begin() + prefixLen, [](const auto & op) { if (op->hasChannelCrosstalk()) { // Non-separable ops (should never get here). @@ -550,17 +492,13 @@ unsigned FindSeparablePrefix(const OpRcPtrVec & ops) } ConstOpRcPtr constOp = op; - if (constOp->data()->getType() == OpData::MatrixType - || constOp->data()->getType() == OpData::RangeType) - { - // Potentially separable, but inexpensive ops. - // TODO: Perhaps a LUT is faster once the conversion to float is considered? - } - else - { - expensiveOps++; - } - } + const auto type = constOp->data()->getType(); + + // Potentially separable, but inexpensive ops are not counted. + // TODO: Perhaps a LUT is faster once the conversion to float is considered? + + return type != OpData::MatrixType && type != OpData::RangeType; + }); if (expensiveOps == 0) { @@ -599,10 +537,9 @@ void OptimizeSeparablePrefix(OpRcPtrVec & ops, BitDepth in) } OpRcPtrVec prefixOps; - for (unsigned i = 0; i < prefixLen; ++i) - { - prefixOps.push_back(ops[i]->clone()); - } + // TODO: add a function to reserve on the type: prefixOps.reserve(prefixLen); + std::transform(ops.begin(), ops.begin() + prefixLen, std::back_inserter(prefixOps), + [](const auto & op) { return op->clone(); }); // Make a domain for the LUT. (Will be half-domain for target == 16f.) Lut1DOpDataRcPtr newDomain = Lut1DOpData::MakeLookupDomain(in); @@ -611,26 +548,49 @@ void OptimizeSeparablePrefix(OpRcPtrVec & ops, BitDepth in) // Note: This sets the outBitDepth of newDomain to match prefixOps. Lut1DOpData::ComposeVec(newDomain, prefixOps); - // Remove the prefix ops. - ops.erase(ops.begin(), ops.begin() + prefixLen); - // Insert the new LUT to replace the prefix ops. OpRcPtrVec lutOps; CreateLut1DOp(lutOps, newDomain, TRANSFORM_DIR_FORWARD); FinalizeOps(lutOps); - ops.insert(ops.begin(), lutOps.begin(), lutOps.end()); + const size_t numNewOps = lutOps.size(); + if (numNewOps <= prefixLen) + { + for (size_t i = 0; i < numNewOps; ++i) + { + ops[i] = std::move(lutOps[i]); + } + if (numNewOps < prefixLen) + { + ops.erase(ops.begin() + numNewOps, ops.begin() + prefixLen); + } + } + else + { + for (size_t i = 0; i < prefixLen; ++i) + { + ops[i] = std::move(lutOps[i]); + } + ops.insert(ops.begin() + prefixLen, + lutOps.begin() + prefixLen, + lutOps.end()); + // TODO if we were compliance std::make_move_iterator(lutOps.begin() + prefixLen), + // std::make_move_iterator(lutOps.end())); + } } -int PerformOptimisation(int (*Operation)(OpRcPtrVec &, OptimizationFlags), OpRcPtrVec & opVec, OptimizationFlags oFlags, bool debugLoggingEnabled, std::string operationName) +int PerformOptimisation(int (*Operation)(OpRcPtrVec &, OptimizationFlags), OpRcPtrVec & opVec, OptimizationFlags oFlags, bool debugLoggingEnabled, const char * const 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); + std::ostringstream os; + os << operationName << " - " << ops_removed << " optimisations found\n"; + if (ops_removed != 0) + { + os << SerializeOpVec(opVec, 4); + } + LogDebug(os.str()); } return ops_removed; } From b9595b2bd1a0a555ad3e08011f3eb3df1ef80199 Mon Sep 17 00:00:00 2001 From: Kevin Wheatley Date: Thu, 25 Jun 2026 20:14:15 +0100 Subject: [PATCH 08/20] style: fix some typos Signed-off-by: Kevin Wheatley --- src/OpenColorIO/OpOptimizers.cpp | 109 ++++++++++++++++--------------- 1 file changed, 55 insertions(+), 54 deletions(-) diff --git a/src/OpenColorIO/OpOptimizers.cpp b/src/OpenColorIO/OpOptimizers.cpp index ab963b688d..4e41807647 100755 --- a/src/OpenColorIO/OpOptimizers.cpp +++ b/src/OpenColorIO/OpOptimizers.cpp @@ -149,7 +149,7 @@ int ReplaceOps(OpRcPtrVec & opVec, [[maybe_unused]] OptimizationFlags oFlags) int firstindex = 0; // this must be a signed int // Note this erase/insert is potentially O(N^2), an alternative is to build a newOpVec at least as big as the current - // and as we pass throughand append the replacement or push_back the original. + // and as we pass through and append the replacement or push_back the original. // If the count is non-zero then std::move the newOpVec to OpVec OpRcPtrVec tmpops; @@ -234,66 +234,66 @@ int RemoveInverseOps(OpRcPtrVec & opVec, OptimizationFlags oFlags) const auto type1 = lastOp->data()->getType(); const auto type2 = constOp->data()->getType(); - // The common case of inverse ops is to have a deep nesting: - // ..., A, B, B', A', ... - // - // By treating the processed portion of the vector as a stack (`writeIdx`), - // popping an element automatically exposes `A` to be reconsidered against `A'` - // on the next loop iteration. - - if (type1 == type2 && - IsPairInverseEnabled(type1, oFlags) && - lastOp->isInverse(constOp)) - { - // When a pair of inverse ops is removed, we want the optimized ops to give the - // same result as the original. For certain ops such as Lut1D or Log this may - // mean inserting a Range to emulate the clamping done by the original ops. + // The common case of inverse ops is to have a deep nesting: + // ..., A, B, B', A', ... + // + // By treating the processed portion of the vector as a stack (`writeIdx`), + // popping an element automatically exposes `A` to be reconsidered against `A'` + // on the next loop iteration. - OpRcPtr replacedBy; - if (type1 == OpData::Lut1DType) + if (type1 == type2 && + IsPairInverseEnabled(type1, oFlags) && + lastOp->isInverse(constOp)) { - // Lut1D gets special handling so that both halfs of the pair are available. - // Only the inverse LUT has the values needed to generate the replacement. + // When a pair of inverse ops is removed, we want the optimized ops to give the + // same result as the original. For certain ops such as Lut1D or Log this may + // mean inserting a Range to emulate the clamping done by the original ops. - ConstLut1DOpDataRcPtr lut1 = OCIO_DYNAMIC_POINTER_CAST(lastOp->data()); - ConstLut1DOpDataRcPtr lut2 = OCIO_DYNAMIC_POINTER_CAST(constOp->data()); - - OpDataRcPtr opData = lut1->getPairIdentityReplacement(lut2); - - OpRcPtrVec ops; - if (opData->getType() == OpData::MatrixType) + OpRcPtr replacedBy; + if (type1 == OpData::Lut1DType) { - // No-op that will be optimized. - auto mat = OCIO_DYNAMIC_POINTER_CAST(opData); - CreateMatrixOp(ops, mat, TRANSFORM_DIR_FORWARD); + // Lut1D gets special handling so that both halfs of the pair are available. + // Only the inverse LUT has the values needed to generate the replacement. + + ConstLut1DOpDataRcPtr lut1 = OCIO_DYNAMIC_POINTER_CAST(lastOp->data()); + ConstLut1DOpDataRcPtr lut2 = OCIO_DYNAMIC_POINTER_CAST(constOp->data()); + + OpDataRcPtr opData = lut1->getPairIdentityReplacement(lut2); + + OpRcPtrVec ops; + if (opData->getType() == OpData::MatrixType) + { + // No-op that will be optimized. + auto mat = OCIO_DYNAMIC_POINTER_CAST(opData); + CreateMatrixOp(ops, mat, TRANSFORM_DIR_FORWARD); + } + else if (opData->getType() == OpData::RangeType) + { + // Clamping op. + auto range = OCIO_DYNAMIC_POINTER_CAST(opData); + CreateRangeOp(ops, range, TRANSFORM_DIR_FORWARD); + } + replacedBy = ops[0]; } - else if (opData->getType() == OpData::RangeType) + else { - // Clamping op. - auto range = OCIO_DYNAMIC_POINTER_CAST(opData); - CreateRangeOp(ops, range, TRANSFORM_DIR_FORWARD); + replacedBy = lastOp->getIdentityReplacement(); } - replacedBy = ops[0]; - } - else - { - replacedBy = lastOp->getIdentityReplacement(); - } - replacedBy->finalize(); - if (replacedBy->isNoOp()) - { - // Pop the last element off the stack to naturally backstep - --writeIdx; - } - else - { - // Forward + inverse does clamp. - opVec[writeIdx - 1] = std::move(replacedBy); + replacedBy->finalize(); + if (replacedBy->isNoOp()) + { + // Pop the last element off the stack to naturally backstep + --writeIdx; + } + else + { + // Forward + inverse does clamp. + opVec[writeIdx - 1] = std::move(replacedBy); + } + ++count; + continue; } - ++count; - continue; - } } // Push the current op to the stack if it wasn't cancelled out @@ -406,7 +406,8 @@ int ReplaceInverseLuts(OpRcPtrVec & opVec, OptimizationFlags oFlags) } } } - return count;} + return count; +} int RemoveLeadingClampIdentity(OpRcPtrVec & opVec) { @@ -574,7 +575,7 @@ void OptimizeSeparablePrefix(OpRcPtrVec & ops, BitDepth in) ops.insert(ops.begin() + prefixLen, lutOps.begin() + prefixLen, lutOps.end()); - // TODO if we were compliance std::make_move_iterator(lutOps.begin() + prefixLen), + // TODO if we were compliant std::make_move_iterator(lutOps.begin() + prefixLen), // std::make_move_iterator(lutOps.end())); } } From 8ca63cc33e4535900d83e3f48de4efb3dd755b3c Mon Sep 17 00:00:00 2001 From: Kevin Wheatley Date: Fri, 26 Jun 2026 12:26:56 +0100 Subject: [PATCH 09/20] refactor: Avoid shuffling the Ops vector entries unnecessarily Signed-off-by: Kevin Wheatley --- src/OpenColorIO/OpOptimizers.cpp | 44 ++++++++++++++++++++++++-------- 1 file changed, 34 insertions(+), 10 deletions(-) diff --git a/src/OpenColorIO/OpOptimizers.cpp b/src/OpenColorIO/OpOptimizers.cpp index 4e41807647..48b5fc9200 100755 --- a/src/OpenColorIO/OpOptimizers.cpp +++ b/src/OpenColorIO/OpOptimizers.cpp @@ -163,15 +163,22 @@ int ReplaceOps(OpRcPtrVec & opVec, [[maybe_unused]] OptimizationFlags oFlags) { FinalizeOps(tmpops); - // Erase the initial op we've replaced. - opVec.erase(opVec.begin() + firstindex, opVec.begin() + firstindex + 1); + auto it = opVec.begin() + firstindex; + const size_t numNewOps = tmpops.size(); - // Insert the new ops at this location. - opVec.insert(opVec.begin() + firstindex, tmpops.begin(), tmpops.end()); + if (numNewOps == 1) + { + *it = std::move(tmpops[0]); + } + else + { + *it = std::move(tmpops[0]); + opVec.insert(it + 1, tmpops.begin() + 1, tmpops.end()); + } // Advance index by the number of inserted elements to skip re-evaluating them, // or add 0 if you want to recursively simplify newly inserted ops. - firstindex += static_cast(tmpops.size()); + firstindex += static_cast(numNewOps); ++count; } else @@ -338,11 +345,28 @@ int CombineOps(OpRcPtrVec & opVec, OptimizationFlags oFlags) // // No matter the number, we need to swap them in for the original ops. - // Erase the initial two ops we've combined. - it = opVec.erase(it, it + 2); - - // Insert the new ops (which may be empty) at this location. - opVec.insert(it, tmpops.begin(), tmpops.end()); + const size_t numNewOps = tmpops.size(); + if (numNewOps == 0) + { + opVec.erase(it, it + 2); + } + else if (numNewOps == 1) + { + *it = std::move(tmpops[0]); + opVec.erase(it + 1); + } + else if (numNewOps == 2) + { + *it = std::move(tmpops[0]); + *(it + 1) = std::move(tmpops[1]); + } + else + { + *it = std::move(tmpops[0]); + *(it + 1) = std::move(tmpops[1]); + opVec.insert(it + 2, tmpops.begin() + 2, tmpops.end()); + } + // Return 1 since combining ops is less desirable than other optimization options. // For example, it is preferable to remove a pair of ops using RemoveInverseOps From 37d863b73efc44eb26277ff75d1133dfe2c1340a Mon Sep 17 00:00:00 2001 From: Kevin Wheatley Date: Fri, 26 Jun 2026 12:42:02 +0100 Subject: [PATCH 10/20] refactor: Avoid constructing temporary strings and use string_view to pass the length of char* stings Signed-off-by: Kevin Wheatley --- src/OpenColorIO/OpOptimizers.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/OpenColorIO/OpOptimizers.cpp b/src/OpenColorIO/OpOptimizers.cpp index 48b5fc9200..9b960c93f3 100755 --- a/src/OpenColorIO/OpOptimizers.cpp +++ b/src/OpenColorIO/OpOptimizers.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include @@ -604,7 +605,7 @@ void OptimizeSeparablePrefix(OpRcPtrVec & ops, BitDepth in) } } -int PerformOptimisation(int (*Operation)(OpRcPtrVec &, OptimizationFlags), OpRcPtrVec & opVec, OptimizationFlags oFlags, bool debugLoggingEnabled, const char * const operationName) +int PerformOptimisation(int (*Operation)(OpRcPtrVec &, OptimizationFlags), OpRcPtrVec & opVec, OptimizationFlags oFlags, bool debugLoggingEnabled, std::string_view operationName) { const int ops_removed = Operation(opVec, oFlags); if (debugLoggingEnabled) @@ -683,7 +684,7 @@ void OpRcPtrVec::optimize(OptimizationFlags oFlags) const int dynamicOps = RemoveDynamicProperties(*this, oFlags); if (debugLoggingEnabled) { - const auto message = std::string("RemoveDynamicProperties - ") + std::to_string(dynamicOps) + std::string(" removed"); + const auto message = "RemoveDynamicProperties - " + std::to_string(dynamicOps) + " removed"; LogDebug(message); } @@ -691,7 +692,7 @@ void OpRcPtrVec::optimize(OptimizationFlags oFlags) { if (debugLoggingEnabled) { - const auto message = std::string("Starting pass ") + std::to_string(passes); + const auto message = "Starting pass " + std::to_string(passes); LogDebug(message); } // Remove all ops for which isNoOp is true, including identity matrices. From a29f518f869598fb9cedc4a48f6361a328bc5b12 Mon Sep 17 00:00:00 2001 From: Kevin Wheatley Date: Fri, 26 Jun 2026 13:08:25 +0100 Subject: [PATCH 11/20] refactor: Simplify OptimizeSeparablePrefix to avoid duplicated loops Signed-off-by: Kevin Wheatley --- src/OpenColorIO/OpOptimizers.cpp | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/src/OpenColorIO/OpOptimizers.cpp b/src/OpenColorIO/OpOptimizers.cpp index 9b960c93f3..59af26e96d 100755 --- a/src/OpenColorIO/OpOptimizers.cpp +++ b/src/OpenColorIO/OpOptimizers.cpp @@ -580,23 +580,19 @@ void OptimizeSeparablePrefix(OpRcPtrVec & ops, BitDepth in) FinalizeOps(lutOps); const size_t numNewOps = lutOps.size(); - if (numNewOps <= prefixLen) + const size_t elementsToOverwrite = std::min(numNewOps, static_cast(prefixLen)); + + for (size_t i = 0; i < elementsToOverwrite; ++i) { - for (size_t i = 0; i < numNewOps; ++i) - { - ops[i] = std::move(lutOps[i]); - } - if (numNewOps < prefixLen) - { - ops.erase(ops.begin() + numNewOps, ops.begin() + prefixLen); - } + ops[i] = std::move(lutOps[i]); } - else + + if (numNewOps < prefixLen) + { + ops.erase(ops.begin() + numNewOps, ops.begin() + prefixLen); + } + else if (numNewOps > prefixLen) { - for (size_t i = 0; i < prefixLen; ++i) - { - ops[i] = std::move(lutOps[i]); - } ops.insert(ops.begin() + prefixLen, lutOps.begin() + prefixLen, lutOps.end()); From 6e8ccca42b0222e2ff6b7bb118b8ae5060b66e19 Mon Sep 17 00:00:00 2001 From: Kevin Wheatley Date: Fri, 26 Jun 2026 13:11:59 +0100 Subject: [PATCH 12/20] refactor: Reuse PerformOptimisation() to call RemoveDynamicProperties and report in the summary Signed-off-by: Kevin Wheatley --- src/OpenColorIO/OpOptimizers.cpp | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/OpenColorIO/OpOptimizers.cpp b/src/OpenColorIO/OpOptimizers.cpp index 59af26e96d..259f5ff76d 100755 --- a/src/OpenColorIO/OpOptimizers.cpp +++ b/src/OpenColorIO/OpOptimizers.cpp @@ -677,12 +677,7 @@ void OpRcPtrVec::optimize(OptimizationFlags oFlags) // Keep dynamic ops using their default values. Remove the ability to modify // them dynamically. - const int dynamicOps = RemoveDynamicProperties(*this, oFlags); - if (debugLoggingEnabled) - { - const auto message = "RemoveDynamicProperties - " + std::to_string(dynamicOps) + " removed"; - LogDebug(message); - } + const int total_dynamicOps = PerformOptimisation(RemoveDynamicProperties, *this, oFlags, debugLoggingEnabled, "RemoveDynamicProperties"); while (passes <= MAX_OPTIMIZATION_PASSES) { @@ -762,6 +757,7 @@ void OpRcPtrVec::optimize(OptimizationFlags oFlags) << originalSize << "->" << finalSize << ", " << passes << " passes, " << total_nooptype << " no-op types removed, " + << total_dynamicOps << " dynamic-ops made static, " << total_noops << " no-ops removed, " << total_replacedops << " ops replaced, " << total_identityops << " identity ops replaced, " From 50b9652a333ccc25bc1a8691c74f55877ea1d3cb Mon Sep 17 00:00:00 2001 From: Kevin Wheatley Date: Fri, 26 Jun 2026 14:00:53 +0100 Subject: [PATCH 13/20] refactor: avoid casting to int now we no longer manually managing indices Signed-off-by: Kevin Wheatley --- src/OpenColorIO/OpOptimizers.cpp | 103 +++++++++++++++---------------- 1 file changed, 51 insertions(+), 52 deletions(-) diff --git a/src/OpenColorIO/OpOptimizers.cpp b/src/OpenColorIO/OpOptimizers.cpp index 259f5ff76d..e83d9b0fc0 100755 --- a/src/OpenColorIO/OpOptimizers.cpp +++ b/src/OpenColorIO/OpOptimizers.cpp @@ -73,22 +73,22 @@ bool IsCombineEnabled(OpData::Type type, OptimizationFlags flags) constexpr int MAX_OPTIMIZATION_PASSES = 80; -int RemoveNoOpTypes(OpRcPtrVec & opVec, [[maybe_unused]] OptimizationFlags flags) +size_t RemoveNoOpTypes(OpRcPtrVec & opVec, [[maybe_unused]] OptimizationFlags flags) { // TODO: with c++20 could use std::erase_if ? auto newEnd = std::remove_if(opVec.begin(), opVec.end(), [](const auto & o) { return o->isNoOpType(); }); - int count = static_cast(std::distance(newEnd, opVec.end())); + size_t count = std::distance(newEnd, opVec.end()); opVec.erase(newEnd, opVec.end()); return count; } // Ops are preserved, dynamic properties are made non-dynamic. -int RemoveDynamicProperties(OpRcPtrVec & opVec, OptimizationFlags oFlags) +size_t RemoveDynamicProperties(OpRcPtrVec & opVec, OptimizationFlags oFlags) { - int count = 0; + size_t count = 0; const auto removeDynamic = HasFlag(oFlags, OPTIMIZATION_NO_DYNAMIC_PROPERTIES); if (!removeDynamic) { @@ -108,7 +108,7 @@ int RemoveDynamicProperties(OpRcPtrVec & opVec, OptimizationFlags oFlags) return count; } -int RemoveNoOps(OpRcPtrVec & opVec, OptimizationFlags oFlags) +size_t RemoveNoOps(OpRcPtrVec & opVec, OptimizationFlags oFlags) { const bool optimizeIdentity = HasFlag(oFlags, OPTIMIZATION_IDENTITY); if (!optimizeIdentity) @@ -122,7 +122,7 @@ int RemoveNoOps(OpRcPtrVec & opVec, OptimizationFlags oFlags) return op->isNoOp(); }); - int count = static_cast(std::distance(newEnd, opVec.end())); + size_t count = std::distance(newEnd, opVec.end()); opVec.erase(newEnd, opVec.end()); return count; } @@ -138,23 +138,23 @@ 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, [[maybe_unused]] OptimizationFlags oFlags) +size_t ReplaceOps(OpRcPtrVec & opVec, [[maybe_unused]] OptimizationFlags oFlags) { - int count = 0; + size_t count = 0; const bool replaceOps = HasFlag(oFlags, OPTIMIZATION_SIMPLIFY_OPS); if (!replaceOps) { return count; } - int firstindex = 0; // this must be a signed int + size_t firstindex = 0; // Note this erase/insert is potentially O(N^2), an alternative is to build a newOpVec at least as big as the current // and as we pass through and append the replacement or push_back the original. // If the count is non-zero then std::move the newOpVec to OpVec OpRcPtrVec tmpops; - while (firstindex < static_cast(opVec.size())) + while (firstindex < opVec.size()) { tmpops.clear(); ConstOpRcPtr op = opVec[firstindex]; @@ -179,7 +179,7 @@ int ReplaceOps(OpRcPtrVec & opVec, [[maybe_unused]] OptimizationFlags oFlags) // Advance index by the number of inserted elements to skip re-evaluating them, // or add 0 if you want to recursively simplify newly inserted ops. - firstindex += static_cast(numNewOps); + firstindex += numNewOps; ++count; } else @@ -192,9 +192,9 @@ int ReplaceOps(OpRcPtrVec & opVec, [[maybe_unused]] OptimizationFlags oFlags) return count; } -int ReplaceIdentityOps(OpRcPtrVec & opVec, OptimizationFlags oFlags) +size_t ReplaceIdentityOps(OpRcPtrVec & opVec, OptimizationFlags oFlags) { - int count = 0; + size_t count = 0; // Remove any identity ops (other than gamma). const bool optIdentity = HasFlag(oFlags, OPTIMIZATION_IDENTITY); @@ -225,13 +225,13 @@ int ReplaceIdentityOps(OpRcPtrVec & opVec, OptimizationFlags oFlags) return count; } -int RemoveInverseOps(OpRcPtrVec & opVec, OptimizationFlags oFlags) +size_t RemoveInverseOps(OpRcPtrVec & opVec, OptimizationFlags oFlags) { - int count = 0; - int writeIdx = 0; - const int numOps = static_cast(opVec.size()); + size_t count = 0; + size_t writeIdx = 0; + const size_t numOps = opVec.size(); - for (int readIdx = 0; readIdx < numOps; ++readIdx) + for (size_t readIdx = 0; readIdx < numOps; ++readIdx) { auto & op = opVec[readIdx]; @@ -321,7 +321,7 @@ int RemoveInverseOps(OpRcPtrVec & opVec, OptimizationFlags oFlags) return count; } -int CombineOps(OpRcPtrVec & opVec, OptimizationFlags oFlags) +size_t CombineOps(OpRcPtrVec & opVec, OptimizationFlags oFlags) { auto it = std::adjacent_find(opVec.begin(), opVec.end(), [oFlags](const auto & ptr1, const auto & ptr2) { @@ -390,9 +390,9 @@ 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, OptimizationFlags oFlags) +size_t ReplaceInverseLuts(OpRcPtrVec & opVec, OptimizationFlags oFlags) { - int count = 0; + size_t count = 0; const bool fastLut = HasFlag(oFlags, OPTIMIZATION_LUT_INV_FAST); if (!fastLut) { @@ -434,7 +434,7 @@ int ReplaceInverseLuts(OpRcPtrVec & opVec, OptimizationFlags oFlags) return count; } -int RemoveLeadingClampIdentity(OpRcPtrVec & opVec) +size_t RemoveLeadingClampIdentity(OpRcPtrVec & opVec) { auto it = std::find_if_not(opVec.begin(), opVec.end(), [](const auto & op) { ConstOpRcPtr constOp = op; @@ -442,7 +442,7 @@ int RemoveLeadingClampIdentity(OpRcPtrVec & opVec) return oData->getType() == OpData::RangeType && oData->isIdentity(); }); - int count = static_cast(std::distance(opVec.begin(), it)); + size_t count = std::distance(opVec.begin(), it); if (count > 0) { opVec.erase(opVec.begin(), it); @@ -450,7 +450,7 @@ int RemoveLeadingClampIdentity(OpRcPtrVec & opVec) return count; } -int RemoveTrailingClampIdentity(OpRcPtrVec & opVec) +size_t RemoveTrailingClampIdentity(OpRcPtrVec & opVec) { // Note the use of reverse iterators auto rit = std::find_if_not(opVec.rbegin(), opVec.rend(), [](const auto & op) { @@ -459,7 +459,7 @@ int RemoveTrailingClampIdentity(OpRcPtrVec & opVec) return oData->getType() == OpData::RangeType && oData->isIdentity(); }); - int count = static_cast(std::distance(opVec.rbegin(), rit)); + size_t count = std::distance(opVec.rbegin(), rit); if (count > 0) { opVec.erase(rit.base(), opVec.end()); @@ -477,7 +477,7 @@ int RemoveTrailingClampIdentity(OpRcPtrVec & opVec) // pixels. Rather than convert to float and apply the power function on each // pixel, it's better to build a 1024 entry LUT and just do a look-up. // -unsigned FindSeparablePrefix(const OpRcPtrVec & ops) +size_t FindSeparablePrefix(const OpRcPtrVec & ops) { // Loop over the ops until we get to one that cannot be combined. // @@ -488,7 +488,7 @@ unsigned FindSeparablePrefix(const OpRcPtrVec & ops) return op->hasChannelCrosstalk() || op->isDynamic(); }); - unsigned prefixLen = static_cast(std::distance(ops.begin(), it)); + size_t prefixLen = std::distance(ops.begin(), it); // If the only op is a 1D LUT, there is actually nothing to optimize // so set the length to 0. (This also avoids an infinite loop.) @@ -556,7 +556,7 @@ void OptimizeSeparablePrefix(OpRcPtrVec & ops, BitDepth in) return; } - const unsigned prefixLen = FindSeparablePrefix(ops); + const auto prefixLen = FindSeparablePrefix(ops); if (prefixLen == 0) { return; // Nothing to do. @@ -579,8 +579,8 @@ void OptimizeSeparablePrefix(OpRcPtrVec & ops, BitDepth in) CreateLut1DOp(lutOps, newDomain, TRANSFORM_DIR_FORWARD); FinalizeOps(lutOps); - const size_t numNewOps = lutOps.size(); - const size_t elementsToOverwrite = std::min(numNewOps, static_cast(prefixLen)); + const auto numNewOps = lutOps.size(); + const auto elementsToOverwrite = std::min(numNewOps, prefixLen); for (size_t i = 0; i < elementsToOverwrite; ++i) { @@ -601,9 +601,9 @@ void OptimizeSeparablePrefix(OpRcPtrVec & ops, BitDepth in) } } -int PerformOptimisation(int (*Operation)(OpRcPtrVec &, OptimizationFlags), OpRcPtrVec & opVec, OptimizationFlags oFlags, bool debugLoggingEnabled, std::string_view operationName) +size_t PerformOptimisation(size_t (*Operation)(OpRcPtrVec &, OptimizationFlags), OpRcPtrVec & opVec, OptimizationFlags oFlags, bool debugLoggingEnabled, std::string_view operationName) { - const int ops_removed = Operation(opVec, oFlags); + const size_t ops_removed = Operation(opVec, oFlags); if (debugLoggingEnabled) { std::ostringstream os; @@ -642,28 +642,27 @@ void OpRcPtrVec::optimize(OptimizationFlags oFlags) const bool debugLoggingEnabled = IsDebugLoggingEnabled(); if (debugLoggingEnabled) { - std::ostringstream oss; - oss << "\n**\nOptimizing Op Vec...\n" << SerializeOpVec(*this, 4); - LogDebug(oss.str()); + const std::string message = "\n**\nOptimizing Op Vec...\n" + SerializeOpVec(*this, 4); + LogDebug(message); } 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; + size_t total_noops = 0; + size_t total_replacedops = 0; + size_t total_identityops = 0; + size_t total_inverseops = 0; + size_t total_combines = 0; + size_t 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"); + const auto total_nooptype = PerformOptimisation(RemoveNoOpTypes, *this, oFlags, debugLoggingEnabled, "RemoveNoOpTypes"); if (oFlags == OPTIMIZATION_NONE) { if (debugLoggingEnabled) { - OpRcPtrVec::size_type finalSize = size(); + const auto finalSize = size(); std::ostringstream os; os << "**\nOptimized " << originalSize << "->" << finalSize << ", " << passes << " pass, " @@ -677,7 +676,7 @@ void OpRcPtrVec::optimize(OptimizationFlags oFlags) // Keep dynamic ops using their default values. Remove the ability to modify // them dynamically. - const int total_dynamicOps = PerformOptimisation(RemoveDynamicProperties, *this, oFlags, debugLoggingEnabled, "RemoveDynamicProperties"); + const auto total_dynamicOps = PerformOptimisation(RemoveDynamicProperties, *this, oFlags, debugLoggingEnabled, "RemoveDynamicProperties"); while (passes <= MAX_OPTIMIZATION_PASSES) { @@ -687,25 +686,25 @@ void OpRcPtrVec::optimize(OptimizationFlags oFlags) LogDebug(message); } // Remove all ops for which isNoOp is true, including identity matrices. - const int noops = PerformOptimisation(RemoveNoOps, *this, oFlags, debugLoggingEnabled, "RemoveNoOps"); + const auto 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. - const int replacedOps = PerformOptimisation(ReplaceOps, *this, oFlags, debugLoggingEnabled, "ReplaceOps"); + const auto 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). - const int identityops = PerformOptimisation(ReplaceIdentityOps, *this, oFlags, debugLoggingEnabled, "ReplaceIdentityOps"); + const auto identityops = PerformOptimisation(ReplaceIdentityOps, *this, oFlags, debugLoggingEnabled, "ReplaceIdentityOps"); total_identityops += identityops; // Remove all adjacent pairs of ops that are inverses of each other. - const int inverseops = PerformOptimisation(RemoveInverseOps, *this, oFlags, debugLoggingEnabled, "RemoveInverseOps"); + const auto 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.) - const int combines = PerformOptimisation(CombineOps, *this, oFlags, debugLoggingEnabled, "CombineOps"); + const auto combines = PerformOptimisation(CombineOps, *this, oFlags, debugLoggingEnabled, "CombineOps"); total_combines += combines; if (noops + replacedOps + identityops + inverseops + combines == 0) @@ -713,7 +712,7 @@ void OpRcPtrVec::optimize(OptimizationFlags oFlags) // 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. - const int inverses = PerformOptimisation(ReplaceInverseLuts, *this, oFlags, debugLoggingEnabled, "ReplaceInverseLuts"); + const auto inverses = PerformOptimisation(ReplaceInverseLuts, *this, oFlags, debugLoggingEnabled, "ReplaceInverseLuts"); total_inverses += inverses; if (inverses == 0) @@ -750,7 +749,7 @@ void OpRcPtrVec::optimize(OptimizationFlags oFlags) if (debugLoggingEnabled) { - OpRcPtrVec::size_type finalSize = size(); + const auto finalSize = size(); std::ostringstream os; os << "**\nOptimized " From e494d58cd9613c70f16fb8596588867079bc09f8 Mon Sep 17 00:00:00 2001 From: Kevin Wheatley Date: Fri, 26 Jun 2026 14:13:02 +0100 Subject: [PATCH 14/20] refactor: Avoid scanning whole OpVec when looking for expensive ops As we only care about there being a single expensive Op switch to std::any_of should return early once first matching Op is found. Signed-off-by: Kevin Wheatley --- src/OpenColorIO/OpOptimizers.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/OpenColorIO/OpOptimizers.cpp b/src/OpenColorIO/OpOptimizers.cpp index e83d9b0fc0..29856e8a51 100755 --- a/src/OpenColorIO/OpOptimizers.cpp +++ b/src/OpenColorIO/OpOptimizers.cpp @@ -510,7 +510,7 @@ size_t FindSeparablePrefix(const OpRcPtrVec & ops) // Some ops are so fast that it may not make sense to replace just one of those. // E.g., if it's just a single matrix, it may not be faster to replace it with a LUT. // So make sure there are some more expensive ops to combine. - auto expensiveOps = std::count_if(ops.begin(), ops.begin() + prefixLen, [](const auto & op) { + bool hasExpensiveOps = std::any_of(ops.begin(), ops.begin() + prefixLen, [](const auto & op) { if (op->hasChannelCrosstalk()) { // Non-separable ops (should never get here). @@ -526,7 +526,7 @@ size_t FindSeparablePrefix(const OpRcPtrVec & ops) return type != OpData::MatrixType && type != OpData::RangeType; }); - if (expensiveOps == 0) + if (!hasExpensiveOps) { return 0; } From 93f893456562762e6144f7265f1c80a91be5ced7 Mon Sep 17 00:00:00 2001 From: Kevin Wheatley Date: Fri, 26 Jun 2026 14:19:48 +0100 Subject: [PATCH 15/20] refactor: Avoiod reallocating temporary during ReplaceInverseLuts Hoist it out of the loop and clear() on demand Signed-off-by: Kevin Wheatley --- src/OpenColorIO/OpOptimizers.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/OpenColorIO/OpOptimizers.cpp b/src/OpenColorIO/OpOptimizers.cpp index 29856e8a51..9b2bb34913 100755 --- a/src/OpenColorIO/OpOptimizers.cpp +++ b/src/OpenColorIO/OpOptimizers.cpp @@ -399,6 +399,8 @@ size_t ReplaceInverseLuts(OpRcPtrVec & opVec, OptimizationFlags oFlags) return count; } + OpRcPtrVec tmpops; + for (auto & op : opVec) { ConstOpRcPtr constOp = op; @@ -410,7 +412,7 @@ size_t ReplaceInverseLuts(OpRcPtrVec & opVec, OptimizationFlags oFlags) if (lutData->getDirection() == TRANSFORM_DIR_INVERSE) { auto invLutData = MakeFastLut1DFromInverse(lutData); - OpRcPtrVec tmpops; + tmpops.clear(); CreateLut1DOp(tmpops, invLutData, TRANSFORM_DIR_FORWARD); FinalizeOps(tmpops); op = std::move(tmpops[0]); @@ -423,7 +425,7 @@ size_t ReplaceInverseLuts(OpRcPtrVec & opVec, OptimizationFlags oFlags) if (lutData->getDirection() == TRANSFORM_DIR_INVERSE) { auto invLutData = MakeFastLut3DFromInverse(lutData); - OpRcPtrVec tmpops; + tmpops.clear(); CreateLut3DOp(tmpops, invLutData, TRANSFORM_DIR_FORWARD); FinalizeOps(tmpops); op = std::move(tmpops[0]); From db4bfbea8188dc264cfdf3eb4172ae17a7839db3 Mon Sep 17 00:00:00 2001 From: Kevin Wheatley Date: Fri, 26 Jun 2026 14:39:15 +0100 Subject: [PATCH 16/20] refactor: markup simple uses of using const pointers to gain access to function calls using std::as_const Signed-off-by: Kevin Wheatley --- src/OpenColorIO/OpOptimizers.cpp | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/src/OpenColorIO/OpOptimizers.cpp b/src/OpenColorIO/OpOptimizers.cpp index 9b2bb34913..1a20e227a0 100755 --- a/src/OpenColorIO/OpOptimizers.cpp +++ b/src/OpenColorIO/OpOptimizers.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include @@ -208,8 +209,7 @@ size_t ReplaceIdentityOps(OpRcPtrVec & opVec, OptimizationFlags oFlags) for (auto & op : opVec) { - ConstOpRcPtr constOp = op; // const hackery to be able call getType() vie const member function - const auto type = constOp->data()->getType(); + const auto type = std::as_const(*op).data()->getType(); if (type != OpData::RangeType && // Do not replace a range identity. ((type == OpData::GammaType && optIdGamma) || (type != OpData::GammaType && optIdentity)) && @@ -403,8 +403,7 @@ size_t ReplaceInverseLuts(OpRcPtrVec & opVec, OptimizationFlags oFlags) for (auto & op : opVec) { - ConstOpRcPtr constOp = op; - auto opData = constOp->data(); + auto opData = std::as_const(*op).data(); const auto type = opData->getType(); if (type == OpData::Lut1DType) { @@ -439,8 +438,7 @@ size_t ReplaceInverseLuts(OpRcPtrVec & opVec, OptimizationFlags oFlags) size_t RemoveLeadingClampIdentity(OpRcPtrVec & opVec) { auto it = std::find_if_not(opVec.begin(), opVec.end(), [](const auto & op) { - ConstOpRcPtr constOp = op; - auto oData = constOp->data(); + auto oData = std::as_const(*op).data(); return oData->getType() == OpData::RangeType && oData->isIdentity(); }); @@ -456,8 +454,7 @@ size_t RemoveTrailingClampIdentity(OpRcPtrVec & opVec) { // Note the use of reverse iterators auto rit = std::find_if_not(opVec.rbegin(), opVec.rend(), [](const auto & op) { - ConstOpRcPtr constOp = op; - auto oData = constOp->data(); + auto oData = std::as_const(*op).data(); return oData->getType() == OpData::RangeType && oData->isIdentity(); }); @@ -497,8 +494,7 @@ size_t FindSeparablePrefix(const OpRcPtrVec & ops) // (If it is an inverse 1D LUT, proceed since we want to replace it with a 1D LUT.) if (prefixLen == 1) { - ConstOpRcPtr constOp0 = ops[0]; - auto opData = constOp0->data(); + auto opData = std::as_const(*ops[0]).data(); if (opData->getType() == OpData::Lut1DType) { auto lutData = OCIO_DYNAMIC_POINTER_CAST(opData); @@ -519,8 +515,7 @@ size_t FindSeparablePrefix(const OpRcPtrVec & ops) throw Exception("Non-separable op."); } - ConstOpRcPtr constOp = op; - const auto type = constOp->data()->getType(); + const auto type = std::as_const(*op).data()->getType(); // Potentially separable, but inexpensive ops are not counted. // TODO: Perhaps a LUT is faster once the conversion to float is considered? From 695309c7821d21c0aef32f137168435cf8b83a69 Mon Sep 17 00:00:00 2001 From: Kevin Wheatley Date: Fri, 26 Jun 2026 14:49:00 +0100 Subject: [PATCH 17/20] refactor: remove some unnecassary variables replacing with std::as_const() usage Signed-off-by: Kevin Wheatley --- src/OpenColorIO/OpOptimizers.cpp | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/OpenColorIO/OpOptimizers.cpp b/src/OpenColorIO/OpOptimizers.cpp index 1a20e227a0..facee553a9 100755 --- a/src/OpenColorIO/OpOptimizers.cpp +++ b/src/OpenColorIO/OpOptimizers.cpp @@ -158,8 +158,7 @@ size_t ReplaceOps(OpRcPtrVec & opVec, [[maybe_unused]] OptimizationFlags oFlags) while (firstindex < opVec.size()) { tmpops.clear(); - ConstOpRcPtr op = opVec[firstindex]; - op->getSimplerReplacement(tmpops); + opVec[firstindex]->getSimplerReplacement(tmpops); if (!tmpops.empty()) { @@ -325,18 +324,16 @@ size_t CombineOps(OpRcPtrVec & opVec, OptimizationFlags oFlags) { auto it = std::adjacent_find(opVec.begin(), opVec.end(), [oFlags](const auto & ptr1, const auto & ptr2) { - ConstOpRcPtr op1 = ptr1; ConstOpRcPtr op2 = ptr2; - return IsCombineEnabled(op1->data()->getType(), oFlags) && op1->canCombineWith(op2); + return IsCombineEnabled(std::as_const(*ptr1).data()->getType(), oFlags) && ptr1->canCombineWith(op2); }); if (it != opVec.end()) { OpRcPtrVec tmpops; - ConstOpRcPtr op1 = *it; ConstOpRcPtr op2 = *(it + 1); - op1->combineWith(tmpops, op2); + (*it)->combineWith(tmpops, op2); FinalizeOps(tmpops); // The tmpops may have any number of ops in it: (0, 1, 2, ...). @@ -787,4 +784,3 @@ void OpRcPtrVec::optimizeForBitdepth(const BitDepth & inBitDepth, } } // namespace OCIO_NAMESPACE - From 2d423289c8c53e97d65a4ff9ab5baaa0ce614772 Mon Sep 17 00:00:00 2001 From: Kevin Wheatley Date: Fri, 26 Jun 2026 18:10:47 +0100 Subject: [PATCH 18/20] refactor: Add some of the missing functions to make OpVec more std compliant Signed-off-by: Kevin Wheatley --- src/OpenColorIO/Op.cpp | 76 +++++++------------------------- src/OpenColorIO/Op.h | 39 ++++++++++++---- src/OpenColorIO/OpOptimizers.cpp | 71 ++++++++++++++++------------- 3 files changed, 87 insertions(+), 99 deletions(-) diff --git a/src/OpenColorIO/Op.cpp b/src/OpenColorIO/Op.cpp index ce45b3f5d6..ae013d7905 100755 --- a/src/OpenColorIO/Op.cpp +++ b/src/OpenColorIO/Op.cpp @@ -1,6 +1,7 @@ // SPDX-License-Identifier: BSD-3-Clause // Copyright Contributors to the OpenColorIO Project. +#include #include #include @@ -24,16 +25,6 @@ namespace OCIO_NAMESPACE { -bool OpCPU::isDynamic() const -{ - return false; -} - -bool OpCPU::hasDynamicProperty(DynamicPropertyType /* type */) const -{ - return false; -} - DynamicPropertyRcPtr OpCPU::getDynamicProperty(DynamicPropertyType /* type */) const { throw Exception("Op does not implement dynamic property."); @@ -177,6 +168,8 @@ OpRcPtr Op::getIdentityReplacement() const { auto opData = m_data->getIdentityReplacement(); OpRcPtrVec ops; + ops.reserve(1); + if (opData->getType() == OpData::MatrixType) { // No-op that will be optimized. @@ -196,13 +189,14 @@ OpRcPtr Op::getIdentityReplacement() const << std::string(GetTypeName(opData->getType())) << "."; throw Exception(oss.str().c_str()); } - return ops[0]; + return std::move(ops[0]); } void Op::getSimplerReplacement(OpRcPtrVec & ops) const { OpDataVec opDataVec; m_data->getSimplerReplacement(opDataVec); + ops.reserve(ops.size() + opDataVec.size()); for (const auto & opData : opDataVec) { CreateOpVecFromOpData(ops, opData, TRANSFORM_DIR_FORWARD); @@ -246,68 +240,28 @@ OpRcPtrVec & OpRcPtrVec::operator+=(const OpRcPtrVec & v) } } -OpRcPtrVec::iterator OpRcPtrVec::erase(OpRcPtrVec::const_iterator position) -{ - return m_ops.erase(position); -} - -OpRcPtrVec::iterator OpRcPtrVec::erase(OpRcPtrVec::const_iterator first, - OpRcPtrVec::const_iterator last) -{ - return m_ops.erase(first, last); -} - -void OpRcPtrVec::insert(OpRcPtrVec::const_iterator position, - OpRcPtrVec::const_iterator first, - OpRcPtrVec::const_iterator last) -{ - m_ops.insert(position, first, last); -} - -void OpRcPtrVec::push_back(const OpRcPtrVec::value_type & val) -{ - m_ops.push_back(val); -} - -OpRcPtrVec::const_reference OpRcPtrVec::back() const -{ - return m_ops.back(); -} - -OpRcPtrVec::const_reference OpRcPtrVec::front() const -{ - return m_ops.front(); -} - bool OpRcPtrVec::isNoOp() const noexcept { - for (const auto & op : m_ops) - { - if(!op->isNoOp()) return false; - } - - return true; + return std::all_of(m_ops.begin(), m_ops.end(), + [](const auto & op) { return op->isNoOp(); }); } bool OpRcPtrVec::hasChannelCrosstalk() const noexcept { - return m_ops.end() != std::find_if(m_ops.begin(), - m_ops.end(), - [](const OpRcPtr & op) { return op->hasChannelCrosstalk(); } ); + return std::any_of(m_ops.begin(), m_ops.end(), + [](const auto & op) { return op->hasChannelCrosstalk(); }); } bool OpRcPtrVec::isDynamic() const noexcept { - return m_ops.end() != std::find_if(m_ops.begin(), - m_ops.end(), - [](const OpRcPtr & op) { return op->isDynamic(); } ); + return std::any_of(m_ops.begin(), m_ops.end(), + [](const auto & op) { return op->isDynamic(); }); } bool OpRcPtrVec::hasDynamicProperty(DynamicPropertyType type) const noexcept { - return m_ops.end() != std::find_if(m_ops.begin(), - m_ops.end(), - [type](const OpRcPtr & op) { return op->hasDynamicProperty(type); } ); + return std::any_of(m_ops.begin(), m_ops.end(), + [type](const auto & op) { return op->hasDynamicProperty(type); }); } DynamicPropertyRcPtr OpRcPtrVec::getDynamicProperty(DynamicPropertyType type) const @@ -326,10 +280,11 @@ DynamicPropertyRcPtr OpRcPtrVec::getDynamicProperty(DynamicPropertyType type) co OpRcPtrVec OpRcPtrVec::clone() const { OpRcPtrVec cloned; + cloned.reserve(m_ops.size()); for (const auto & op : m_ops) { - cloned.push_back(op->clone()); + cloned.emplace_back(op->clone()); } return cloned; @@ -338,6 +293,7 @@ OpRcPtrVec OpRcPtrVec::clone() const OpRcPtrVec OpRcPtrVec::invert() const { OpRcPtrVec inverted; + inverted.reserve(m_ops.size()); OpRcPtrVec::const_reverse_iterator iter = m_ops.rbegin(); OpRcPtrVec::const_reverse_iterator end = m_ops.rend(); diff --git a/src/OpenColorIO/Op.h b/src/OpenColorIO/Op.h index d323534561..bb669e8e8c 100644 --- a/src/OpenColorIO/Op.h +++ b/src/OpenColorIO/Op.h @@ -6,6 +6,7 @@ #define INCLUDED_OCIO_OP_H #include +#include #include @@ -44,8 +45,8 @@ class OpCPU // the 1D LUT CPU Op where the finalization depends on input and output bit depths. virtual void apply(const void * inImg, void * outImg, long numPixels) const = 0; - virtual bool isDynamic() const; - virtual bool hasDynamicProperty(DynamicPropertyType type) const; + virtual bool isDynamic() const { return false; } + virtual bool hasDynamicProperty([[maybe_unused]] DynamicPropertyType type) const { return false; } virtual DynamicPropertyRcPtr getDynamicProperty(DynamicPropertyType type) const; }; @@ -332,22 +333,28 @@ class OpRcPtrVec OpRcPtrVec & operator+=(const OpRcPtrVec & v); size_type size() const { return m_ops.size(); } + size_type capacity() const noexcept { return m_ops.capacity(); } + size_type max_size() const noexcept { return m_ops.max_size(); } iterator begin() noexcept { return m_ops.begin(); } const_iterator begin() const noexcept { return m_ops.begin(); } + const_iterator cbegin() const noexcept { return m_ops.cbegin(); } iterator end() noexcept { return m_ops.end(); } const_iterator end() const noexcept { return m_ops.end(); } + const_iterator cend() const noexcept { return m_ops.cend(); } reverse_iterator rbegin() noexcept { return m_ops.rbegin(); } const_reverse_iterator rbegin() const noexcept { return m_ops.rbegin(); } + const_reverse_iterator crbegin() const noexcept { return m_ops.crbegin(); } reverse_iterator rend() noexcept { return m_ops.rend(); } const_reverse_iterator rend() const noexcept { return m_ops.rend(); } + const_reverse_iterator crend() const noexcept { return m_ops.crend(); } const OpRcPtr & operator[](size_type idx) const { return m_ops[idx]; } OpRcPtr & operator[](size_type idx) { return m_ops[idx]; } - iterator erase(const_iterator position); - iterator erase(const_iterator first, const_iterator last); + iterator erase(const_iterator position) { return m_ops.erase(position); } + iterator erase(const_iterator first, const_iterator last) { return m_ops.erase(first, last); } // Insert at the 'position' the elements from the range ['first', 'last'[ // respecting the element's order. Inserting elements at a given position @@ -357,15 +364,31 @@ class OpRcPtrVec // in an empty list appends elements from the range ['first', 'last'[. // // Note: It copies elements i.e. no clone. - void insert(const_iterator position, const_iterator first, const_iterator last); + template + void insert(const_iterator position, InputIt first, InputIt last) + { + m_ops.insert(position, first, last); + } void clear() noexcept { m_ops.clear(); } bool empty() const noexcept { return m_ops.empty(); } - void push_back(const value_type & val); + void reserve(size_type n) { m_ops.reserve(n); } + void shrink_to_fit() { m_ops.shrink_to_fit(); } + void resize(size_type count) { m_ops.resize(count); } + void resize(size_type count, const value_type & value) { m_ops.resize(count, value); } + + void push_back(const value_type & val) { m_ops.push_back(val); } + void push_back(value_type && val) { m_ops.push_back(std::move(val)); } + + template + reference emplace_back(Args&&... args) + { + return m_ops.emplace_back(std::forward(args)...); + } - const_reference back() const; - const_reference front() const; + const_reference back() const { return m_ops.back(); } + const_reference front() const { return m_ops.front(); } // The following methods provide helpers for basic Op behaviors. diff --git a/src/OpenColorIO/OpOptimizers.cpp b/src/OpenColorIO/OpOptimizers.cpp index facee553a9..d8fc7724ff 100755 --- a/src/OpenColorIO/OpOptimizers.cpp +++ b/src/OpenColorIO/OpOptimizers.cpp @@ -148,47 +148,54 @@ size_t ReplaceOps(OpRcPtrVec & opVec, [[maybe_unused]] OptimizationFlags oFlags) return count; } - size_t firstindex = 0; - // Note this erase/insert is potentially O(N^2), an alternative is to build a newOpVec at least as big as the current - // and as we pass through and append the replacement or push_back the original. - // If the count is non-zero then std::move the newOpVec to OpVec OpRcPtrVec tmpops; + OpRcPtrVec newOpVec; + bool rebuilding = false; - while (firstindex < opVec.size()) + for (size_t i = 0; i < opVec.size(); ++i) { tmpops.clear(); - opVec[firstindex]->getSimplerReplacement(tmpops); + opVec[i]->getSimplerReplacement(tmpops); if (!tmpops.empty()) { - FinalizeOps(tmpops); - - auto it = opVec.begin() + firstindex; - const size_t numNewOps = tmpops.size(); - - if (numNewOps == 1) + if (!rebuilding) { - *it = std::move(tmpops[0]); + rebuilding = true; + // Defer allocation until the first replacement to prevent + // large, unnecessary heap allocations on no-op optimizer passes. + newOpVec.reserve(opVec.size()); + + // Catch up any unmodified elements prior to this replacement + for (size_t j = 0; j < i; ++j) + { + newOpVec.emplace_back(std::move(opVec[j])); + } } - else + + FinalizeOps(tmpops); + for (auto & newOp : tmpops) { - *it = std::move(tmpops[0]); - opVec.insert(it + 1, tmpops.begin() + 1, tmpops.end()); + newOpVec.emplace_back(std::move(newOp)); } - - // Advance index by the number of inserted elements to skip re-evaluating them, - // or add 0 if you want to recursively simplify newly inserted ops. - firstindex += numNewOps; ++count; } else { - ++firstindex; + if (rebuilding) + { + newOpVec.emplace_back(std::move(opVec[i])); + } } } + if (rebuilding) + { + opVec = std::move(newOpVec); + } + return count; } @@ -280,7 +287,7 @@ size_t RemoveInverseOps(OpRcPtrVec & opVec, OptimizationFlags oFlags) auto range = OCIO_DYNAMIC_POINTER_CAST(opData); CreateRangeOp(ops, range, TRANSFORM_DIR_FORWARD); } - replacedBy = ops[0]; + replacedBy = std::move(ops[0]); } else { @@ -362,8 +369,9 @@ size_t CombineOps(OpRcPtrVec & opVec, OptimizationFlags oFlags) { *it = std::move(tmpops[0]); *(it + 1) = std::move(tmpops[1]); - opVec.insert(it + 2, tmpops.begin() + 2, tmpops.end()); - } + opVec.insert(it + 2, + std::make_move_iterator(tmpops.begin() + 2), + std::make_move_iterator(tmpops.end())); } // Return 1 since combining ops is less desirable than other optimization options. @@ -557,9 +565,11 @@ void OptimizeSeparablePrefix(OpRcPtrVec & ops, BitDepth in) } OpRcPtrVec prefixOps; - // TODO: add a function to reserve on the type: prefixOps.reserve(prefixLen); - std::transform(ops.begin(), ops.begin() + prefixLen, std::back_inserter(prefixOps), - [](const auto & op) { return op->clone(); }); + prefixOps.reserve(prefixLen); + for (size_t i = 0; i < prefixLen; ++i) + { + prefixOps.emplace_back(ops[i]->clone()); + } // Make a domain for the LUT. (Will be half-domain for target == 16f.) Lut1DOpDataRcPtr newDomain = Lut1DOpData::MakeLookupDomain(in); @@ -570,6 +580,7 @@ void OptimizeSeparablePrefix(OpRcPtrVec & ops, BitDepth in) // Insert the new LUT to replace the prefix ops. OpRcPtrVec lutOps; + lutOps.reserve(1); CreateLut1DOp(lutOps, newDomain, TRANSFORM_DIR_FORWARD); FinalizeOps(lutOps); @@ -588,10 +599,8 @@ void OptimizeSeparablePrefix(OpRcPtrVec & ops, BitDepth in) else if (numNewOps > prefixLen) { ops.insert(ops.begin() + prefixLen, - lutOps.begin() + prefixLen, - lutOps.end()); - // TODO if we were compliant std::make_move_iterator(lutOps.begin() + prefixLen), - // std::make_move_iterator(lutOps.end())); + std::make_move_iterator(lutOps.begin() + prefixLen), + std::make_move_iterator(lutOps.end())); } } From 9113131f264b20936215d8e3728e3d633f85d727 Mon Sep 17 00:00:00 2001 From: Kevin Wheatley Date: Sat, 27 Jun 2026 18:22:49 +0100 Subject: [PATCH 19/20] refactor: Move implementation into header for 'simple' functions that just forward on to the vector. Signed-off-by: Kevin Wheatley --- src/OpenColorIO/Op.cpp | 107 ----------------------------------------- src/OpenColorIO/Op.h | 38 ++++++++------- 2 files changed, 21 insertions(+), 124 deletions(-) diff --git a/src/OpenColorIO/Op.cpp b/src/OpenColorIO/Op.cpp index ae013d7905..6bc32061d7 100755 --- a/src/OpenColorIO/Op.cpp +++ b/src/OpenColorIO/Op.cpp @@ -30,26 +30,6 @@ DynamicPropertyRcPtr OpCPU::getDynamicProperty(DynamicPropertyType /* type */) c throw Exception("Op does not implement dynamic property."); } -OpData::OpData() - : m_metadata() -{ } - -OpData::OpData(const OpData & rhs) - : m_metadata() -{ - *this = rhs; -} - -OpData & OpData::operator=(const OpData & rhs) -{ - if (this != &rhs) - { - m_metadata = rhs.m_metadata; - } - - return *this; -} - OpDataRcPtr OpData::getIdentityReplacement() const { return std::make_shared(); @@ -59,39 +39,6 @@ void OpData::getSimplerReplacement(OpDataVec & /* ops */) const { } -bool OpData::equals(const OpData & other) const -{ - if (this == &other) return true; - - // Ignore metadata. - return getType() == other.getType(); -} - -const std::string & OpData::getID() const -{ - return m_metadata.getAttributeValueString(METADATA_ID); -} - -void OpData::setID(const std::string & id) -{ - return m_metadata.setID(id.c_str()); -} - -const std::string & OpData::getName() const -{ - return m_metadata.getAttributeValueString(METADATA_NAME); -} - -void OpData::setName(const std::string & name) -{ - return m_metadata.setName(name.c_str()); -} - -bool operator==(const OpData & lhs, const OpData & rhs) -{ - return lhs.equals(rhs); -} - const char * GetTypeName(OpData::Type type) { switch (type) @@ -203,28 +150,6 @@ void Op::getSimplerReplacement(OpRcPtrVec & ops) const } } -OpRcPtrVec::OpRcPtrVec() - : m_metadata() -{ -} - -OpRcPtrVec::OpRcPtrVec(const OpRcPtrVec & v) - : OpRcPtrVec() -{ - *this = v; -} - -OpRcPtrVec & OpRcPtrVec::operator=(const OpRcPtrVec & v) -{ - if(this!=&v) - { - m_ops = v.m_ops; - m_metadata = v.m_metadata; - } - - return *this; -} - OpRcPtrVec & OpRcPtrVec::operator+=(const OpRcPtrVec & v) { if (this != &v) @@ -240,30 +165,6 @@ OpRcPtrVec & OpRcPtrVec::operator+=(const OpRcPtrVec & v) } } -bool OpRcPtrVec::isNoOp() const noexcept -{ - return std::all_of(m_ops.begin(), m_ops.end(), - [](const auto & op) { return op->isNoOp(); }); -} - -bool OpRcPtrVec::hasChannelCrosstalk() const noexcept -{ - return std::any_of(m_ops.begin(), m_ops.end(), - [](const auto & op) { return op->hasChannelCrosstalk(); }); -} - -bool OpRcPtrVec::isDynamic() const noexcept -{ - return std::any_of(m_ops.begin(), m_ops.end(), - [](const auto & op) { return op->isDynamic(); }); -} - -bool OpRcPtrVec::hasDynamicProperty(DynamicPropertyType type) const noexcept -{ - return std::any_of(m_ops.begin(), m_ops.end(), - [type](const auto & op) { return op->hasDynamicProperty(type); }); -} - DynamicPropertyRcPtr OpRcPtrVec::getDynamicProperty(DynamicPropertyType type) const { for (const auto & op : m_ops) @@ -315,14 +216,6 @@ OpRcPtrVec OpRcPtrVec::invert() const return inverted; } -void OpRcPtrVec::validate() const -{ - for (auto & op : m_ops) - { - op->validate(); - } -} - namespace { template diff --git a/src/OpenColorIO/Op.h b/src/OpenColorIO/Op.h index bb669e8e8c..6dce4109b9 100644 --- a/src/OpenColorIO/Op.h +++ b/src/OpenColorIO/Op.h @@ -5,6 +5,7 @@ #ifndef INCLUDED_OCIO_OP_H #define INCLUDED_OCIO_OP_H +#include #include #include @@ -118,18 +119,18 @@ class OpData }; public: - OpData(); - OpData(const OpData & rhs); + OpData() = default; + OpData(const OpData & rhs) : m_metadata(rhs.m_metadata) {} OpData(OpData && rhs) = delete; - OpData & operator=(const OpData & rhs); + OpData & operator=(const OpData & rhs) { if (this != &rhs) { m_metadata = rhs.m_metadata; } return *this; } OpData & operator=(OpData && rhs) = delete; virtual ~OpData() = default; - const std::string & getID() const; - void setID(const std::string & id); + const std::string & getID() const { return m_metadata.getAttributeValueString(METADATA_ID); } + void setID(const std::string & id) { m_metadata.setID(id.c_str()); } - const std::string & getName() const; - void setName(const std::string & name); + const std::string & getName() const { return m_metadata.getAttributeValueString(METADATA_NAME); } + void setName(const std::string & name) { m_metadata.setName(name.c_str()); } virtual void validate() const = 0; @@ -154,7 +155,7 @@ class OpData // returns true if the op's output does not combine input channels virtual bool hasChannelCrosstalk() const = 0; - virtual bool equals(const OpData & other) const; + virtual bool equals(const OpData & other) const { if (this == &other) return true; return getType() == other.getType(); } // This should yield a string of not unreasonable length. virtual std::string getCacheID() const = 0; @@ -170,7 +171,10 @@ class OpData FormatMetadataImpl m_metadata; }; -bool operator==(const OpData & lhs, const OpData & rhs); +inline bool operator==(const OpData & lhs, const OpData & rhs) +{ + return lhs.equals(rhs); +} const char * GetTypeName(OpData::Type type); @@ -324,11 +328,11 @@ class OpRcPtrVec typedef Type::reference reference; typedef Type::const_reference const_reference; - OpRcPtrVec(); + OpRcPtrVec() = default; ~OpRcPtrVec() {} - OpRcPtrVec(const OpRcPtrVec & v); - OpRcPtrVec & operator=(const OpRcPtrVec & v); + OpRcPtrVec(const OpRcPtrVec & v) = default; + OpRcPtrVec & operator=(const OpRcPtrVec & v) = default; // Note: It copies elements i.e. no clone. OpRcPtrVec & operator+=(const OpRcPtrVec & v); @@ -395,11 +399,11 @@ class OpRcPtrVec FormatMetadataImpl & getFormatMetadata() { return m_metadata; } const FormatMetadataImpl & getFormatMetadata() const { return m_metadata; } - bool isNoOp() const noexcept; - bool hasChannelCrosstalk() const noexcept; + bool isNoOp() const noexcept { return std::all_of(m_ops.begin(), m_ops.end(), [](const auto & op) { return op->isNoOp(); }); } + bool hasChannelCrosstalk() const noexcept { return std::any_of(m_ops.begin(), m_ops.end(), [](const auto & op) { return op->hasChannelCrosstalk(); }); } - bool isDynamic() const noexcept; - bool hasDynamicProperty(DynamicPropertyType type) const noexcept; + bool isDynamic() const noexcept { return std::any_of(m_ops.begin(), m_ops.end(), [](const auto & op) { return op->isDynamic(); }); } + bool hasDynamicProperty(DynamicPropertyType type) const noexcept { return std::any_of(m_ops.begin(), m_ops.end(), [type](const auto & op) { return op->hasDynamicProperty(type); }); } DynamicPropertyRcPtr getDynamicProperty(DynamicPropertyType type) const; void validateDynamicProperties(); @@ -408,7 +412,7 @@ class OpRcPtrVec // Note: The elements are cloned. OpRcPtrVec invert() const; - void validate() const; + void validate() const { for (auto & op : m_ops) { op->validate(); } } std::string getCacheID() const; From 5ad9b0bc93bf916239be8b1c8133bcb522b10029 Mon Sep 17 00:00:00 2001 From: Kevin Wheatley Date: Sat, 27 Jun 2026 18:29:28 +0100 Subject: [PATCH 20/20] refactor: Try avoid copying temporary shared_ptr using emplace_back Now we have added the functions to the op vector, this should act as a micro optimisation. Signed-off-by: Kevin Wheatley --- src/OpenColorIO/NamedTransform.cpp | 4 ++-- src/OpenColorIO/ParseUtils.cpp | 6 +++--- src/OpenColorIO/Processor.cpp | 2 +- src/OpenColorIO/ViewingRules.cpp | 7 +++---- src/OpenColorIO/ops/cdl/CDLOp.cpp | 2 +- src/OpenColorIO/ops/cdl/CDLOpData.cpp | 6 ++---- src/OpenColorIO/ops/exponent/ExponentOp.cpp | 4 ++-- .../ops/exposurecontrast/ExposureContrastOp.cpp | 4 ++-- .../ops/fixedfunction/FixedFunctionOp.cpp | 2 +- src/OpenColorIO/ops/gamma/GammaOp.cpp | 2 +- .../ops/gradinghuecurve/GradingHueCurveOp.cpp | 2 +- .../ops/gradingprimary/GradingPrimaryOp.cpp | 2 +- .../ops/gradingrgbcurve/GradingRGBCurveOp.cpp | 2 +- src/OpenColorIO/ops/gradingtone/GradingToneOp.cpp | 2 +- src/OpenColorIO/ops/log/LogOp.cpp | 6 +++--- src/OpenColorIO/ops/lut1d/Lut1DOp.cpp | 5 ++--- src/OpenColorIO/ops/lut3d/Lut3DOp.cpp | 5 ++--- src/OpenColorIO/ops/matrix/MatrixOp.cpp | 6 +++--- src/OpenColorIO/ops/noop/NoOps.cpp | 14 +++++++------- src/OpenColorIO/ops/range/RangeOp.cpp | 2 +- 20 files changed, 40 insertions(+), 45 deletions(-) diff --git a/src/OpenColorIO/NamedTransform.cpp b/src/OpenColorIO/NamedTransform.cpp index 19c6d8d67c..45bd1298bc 100644 --- a/src/OpenColorIO/NamedTransform.cpp +++ b/src/OpenColorIO/NamedTransform.cpp @@ -86,7 +86,7 @@ void NamedTransformImpl::addAlias(const char * alias) noexcept { if (!StringUtils::Contain(m_aliases, alias)) { - m_aliases.push_back(alias); + m_aliases.emplace_back(alias); } } } @@ -279,7 +279,7 @@ std::ostream & operator<< (std::ostream & os, const NamedTransform & t) StringUtils::StringVec categories; for (int i = 0; i < t.getNumCategories(); ++i) { - categories.push_back(t.getCategory(i)); + categories.emplace_back(t.getCategory(i)); } os << ", categories=[" << StringUtils::Join(categories, ',') << "]"; } diff --git a/src/OpenColorIO/ParseUtils.cpp b/src/OpenColorIO/ParseUtils.cpp index 47d4f2641a..6759b2d0d4 100644 --- a/src/OpenColorIO/ParseUtils.cpp +++ b/src/OpenColorIO/ParseUtils.cpp @@ -771,12 +771,12 @@ StringUtils::StringVec SplitStringEnvStyle(const std::string & str) foundComma != std::string::npos ? ',' : ':' ); if(nameEndPos > currentPos) { - outputvec.push_back(s.substr(currentPos, nameEndPos - currentPos)); + outputvec.emplace_back(s.substr(currentPos, nameEndPos - currentPos)); currentPos = nameEndPos + 1; } else { - outputvec.push_back(""); + outputvec.emplace_back(""); currentPos += 1; } } @@ -784,7 +784,7 @@ StringUtils::StringVec SplitStringEnvStyle(const std::string & str) else { // If there is no comma or colon, consider the string as a single element. - outputvec.push_back(s); + outputvec.emplace_back(s); } for ( auto & val : outputvec ) diff --git a/src/OpenColorIO/Processor.cpp b/src/OpenColorIO/Processor.cpp index 992701855f..ea4b441ca7 100755 --- a/src/OpenColorIO/Processor.cpp +++ b/src/OpenColorIO/Processor.cpp @@ -93,7 +93,7 @@ const char * ProcessorMetadata::getLook(int index) const void ProcessorMetadata::addLook(const char * look) { - getImpl()->looks.push_back(look); + getImpl()->looks.emplace_back(look); } ////////////////////////////////////////////////////////////////////////// diff --git a/src/OpenColorIO/ViewingRules.cpp b/src/OpenColorIO/ViewingRules.cpp index 2db7d1e499..543681e26f 100644 --- a/src/OpenColorIO/ViewingRules.cpp +++ b/src/OpenColorIO/ViewingRules.cpp @@ -156,7 +156,7 @@ ViewingRules::Impl & ViewingRules::Impl::operator=(const ViewingRules::Impl & rh for (const auto & rule : rhs.m_rules) { - m_rules.push_back(rule->clone()); + m_rules.emplace_back(rule->clone()); } } @@ -387,15 +387,14 @@ void ViewingRules::insertRule(size_t ruleIndex, const char * name) m_impl->validateNewRule(ruleName.c_str()); - auto newRule = std::make_shared(ruleName.c_str()); if (ruleIndex == getNumEntries()) { - m_impl->m_rules.push_back(newRule); + m_impl->m_rules.emplace_back(std::make_shared(ruleName.c_str())); } else { m_impl->validatePosition(ruleIndex); - m_impl->m_rules.insert(m_impl->m_rules.begin() + ruleIndex, newRule); + m_impl->m_rules.insert(m_impl->m_rules.begin() + ruleIndex, std::make_shared(ruleName.c_str())); } } diff --git a/src/OpenColorIO/ops/cdl/CDLOp.cpp b/src/OpenColorIO/ops/cdl/CDLOp.cpp index bd0511ea43..b6d96df8a2 100644 --- a/src/OpenColorIO/ops/cdl/CDLOp.cpp +++ b/src/OpenColorIO/ops/cdl/CDLOp.cpp @@ -177,7 +177,7 @@ void CreateCDLOp(OpRcPtrVec & ops, cdl = cdl->inverse(); } - ops.push_back(std::make_shared(cdl)); + ops.emplace_back(std::make_shared(cdl)); } /////////////////////////////////////////////////////////////////////////// diff --git a/src/OpenColorIO/ops/cdl/CDLOpData.cpp b/src/OpenColorIO/ops/cdl/CDLOpData.cpp index fa214be887..cca445ccd4 100644 --- a/src/OpenColorIO/ops/cdl/CDLOpData.cpp +++ b/src/OpenColorIO/ops/cdl/CDLOpData.cpp @@ -361,8 +361,7 @@ void CDLOpData::getSimplerReplacement(OpDataVec & tmpops) const if (isClamping()) { // Same in both directions. - auto range = std::make_shared(0., 1., 0., 1.); - tmpops.push_back(range); + tmpops.emplace_back(std::make_shared(0., 1., 0., 1.)); } static constexpr double lumaCoef3[3]{ 0.2126, 0.7152, 0.0722 }; @@ -383,8 +382,7 @@ void CDLOpData::getSimplerReplacement(OpDataVec & tmpops) const // Clamping if (isClamping()) { - auto range = std::make_shared(0., 1., 0., 1.); - tmpops.push_back(range); + tmpops.emplace_back( std::make_shared(0., 1., 0., 1.)); } if (getDirection() == TRANSFORM_DIR_INVERSE) diff --git a/src/OpenColorIO/ops/exponent/ExponentOp.cpp b/src/OpenColorIO/ops/exponent/ExponentOp.cpp index e10eb56b71..c5142a763c 100644 --- a/src/OpenColorIO/ops/exponent/ExponentOp.cpp +++ b/src/OpenColorIO/ops/exponent/ExponentOp.cpp @@ -312,7 +312,7 @@ void CreateExponentOp(OpRcPtrVec & ops, { case TRANSFORM_DIR_FORWARD: { - ops.push_back(std::make_shared(expData)); + ops.emplace_back(std::make_shared(expData)); break; } case TRANSFORM_DIR_INVERSE: @@ -330,7 +330,7 @@ void CreateExponentOp(OpRcPtrVec & ops, } } ExponentOpDataRcPtr expInv = std::make_shared(values); - ops.push_back(std::make_shared(expInv)); + ops.emplace_back(std::make_shared(expInv)); break; } } diff --git a/src/OpenColorIO/ops/exposurecontrast/ExposureContrastOp.cpp b/src/OpenColorIO/ops/exposurecontrast/ExposureContrastOp.cpp index fc5d1cd4e4..1462ae336d 100644 --- a/src/OpenColorIO/ops/exposurecontrast/ExposureContrastOp.cpp +++ b/src/OpenColorIO/ops/exposurecontrast/ExposureContrastOp.cpp @@ -180,13 +180,13 @@ void CreateExposureContrastOp(OpRcPtrVec & ops, { case TRANSFORM_DIR_FORWARD: { - ops.push_back(std::make_shared(data)); + ops.emplace_back(std::make_shared(data)); break; } case TRANSFORM_DIR_INVERSE: { ExposureContrastOpDataRcPtr dataInv = data->inverse(); - ops.push_back(std::make_shared(dataInv)); + ops.emplace_back(std::make_shared(dataInv)); break; } } diff --git a/src/OpenColorIO/ops/fixedfunction/FixedFunctionOp.cpp b/src/OpenColorIO/ops/fixedfunction/FixedFunctionOp.cpp index 4f9569ef1c..5694bdfeb1 100644 --- a/src/OpenColorIO/ops/fixedfunction/FixedFunctionOp.cpp +++ b/src/OpenColorIO/ops/fixedfunction/FixedFunctionOp.cpp @@ -157,7 +157,7 @@ void CreateFixedFunctionOp(OpRcPtrVec & ops, func = func->inverse(); } - ops.push_back(std::make_shared(func)); + ops.emplace_back(std::make_shared(func)); } /////////////////////////////////////////////////////////////////////////// diff --git a/src/OpenColorIO/ops/gamma/GammaOp.cpp b/src/OpenColorIO/ops/gamma/GammaOp.cpp index 9d6afccc7d..a0c7c63e27 100644 --- a/src/OpenColorIO/ops/gamma/GammaOp.cpp +++ b/src/OpenColorIO/ops/gamma/GammaOp.cpp @@ -141,7 +141,7 @@ void CreateGammaOp(OpRcPtrVec & ops, gamma = gamma->inverse(); } - ops.push_back(std::make_shared(gamma)); + ops.emplace_back(std::make_shared(gamma)); } /////////////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/src/OpenColorIO/ops/gradinghuecurve/GradingHueCurveOp.cpp b/src/OpenColorIO/ops/gradinghuecurve/GradingHueCurveOp.cpp index 1915301319..c59e1abd21 100644 --- a/src/OpenColorIO/ops/gradinghuecurve/GradingHueCurveOp.cpp +++ b/src/OpenColorIO/ops/gradinghuecurve/GradingHueCurveOp.cpp @@ -211,7 +211,7 @@ void CreateGradingHueCurveOp(OpRcPtrVec & ops, curve = curve->inverse(); } - ops.push_back(std::make_shared(curve)); + ops.emplace_back(std::make_shared(curve)); } /////////////////////////////////////////////////////////////////////////// diff --git a/src/OpenColorIO/ops/gradingprimary/GradingPrimaryOp.cpp b/src/OpenColorIO/ops/gradingprimary/GradingPrimaryOp.cpp index 5bb2f960bf..f1cdea8b8f 100644 --- a/src/OpenColorIO/ops/gradingprimary/GradingPrimaryOp.cpp +++ b/src/OpenColorIO/ops/gradingprimary/GradingPrimaryOp.cpp @@ -215,7 +215,7 @@ void CreateGradingPrimaryOp(OpRcPtrVec & ops, prim = prim->inverse(); } - ops.push_back(std::make_shared(prim)); + ops.emplace_back(std::make_shared(prim)); } /////////////////////////////////////////////////////////////////////////// diff --git a/src/OpenColorIO/ops/gradingrgbcurve/GradingRGBCurveOp.cpp b/src/OpenColorIO/ops/gradingrgbcurve/GradingRGBCurveOp.cpp index 598ae28e67..52b5c3f271 100644 --- a/src/OpenColorIO/ops/gradingrgbcurve/GradingRGBCurveOp.cpp +++ b/src/OpenColorIO/ops/gradingrgbcurve/GradingRGBCurveOp.cpp @@ -215,7 +215,7 @@ void CreateGradingRGBCurveOp(OpRcPtrVec & ops, curve = curve->inverse(); } - ops.push_back(std::make_shared(curve)); + ops.emplace_back(std::make_shared(curve)); } /////////////////////////////////////////////////////////////////////////// diff --git a/src/OpenColorIO/ops/gradingtone/GradingToneOp.cpp b/src/OpenColorIO/ops/gradingtone/GradingToneOp.cpp index 5968210e71..f09278a209 100644 --- a/src/OpenColorIO/ops/gradingtone/GradingToneOp.cpp +++ b/src/OpenColorIO/ops/gradingtone/GradingToneOp.cpp @@ -209,7 +209,7 @@ void CreateGradingToneOp(OpRcPtrVec & ops, tone = tone->inverse(); } - ops.push_back(std::make_shared(tone)); + ops.emplace_back(std::make_shared(tone)); } /////////////////////////////////////////////////////////////////////////// diff --git a/src/OpenColorIO/ops/log/LogOp.cpp b/src/OpenColorIO/ops/log/LogOp.cpp index 8eeff9b3b2..f15691ad5b 100644 --- a/src/OpenColorIO/ops/log/LogOp.cpp +++ b/src/OpenColorIO/ops/log/LogOp.cpp @@ -130,13 +130,13 @@ void CreateLogOp(OpRcPtrVec & ops, { auto opData = std::make_shared(base, logSlope, logOffset, linSlope, linOffset, direction); - ops.push_back(std::make_shared(opData)); + ops.emplace_back(std::make_shared(opData)); } void CreateLogOp(OpRcPtrVec & ops, double base, TransformDirection direction) { auto opData = std::make_shared(base, direction); - ops.push_back(std::make_shared(opData)); + ops.emplace_back(std::make_shared(opData)); } void CreateLogOp(OpRcPtrVec & ops, @@ -149,7 +149,7 @@ void CreateLogOp(OpRcPtrVec & ops, log = log->inverse(); } - ops.push_back(std::make_shared(log)); + ops.emplace_back(std::make_shared(log)); } diff --git a/src/OpenColorIO/ops/lut1d/Lut1DOp.cpp b/src/OpenColorIO/ops/lut1d/Lut1DOp.cpp index 412462414b..8e5737baf4 100644 --- a/src/OpenColorIO/ops/lut1d/Lut1DOp.cpp +++ b/src/OpenColorIO/ops/lut1d/Lut1DOp.cpp @@ -123,8 +123,7 @@ void Lut1DOp::combineWith(OpRcPtrVec & ops, ConstOpRcPtr & secondOp) const const auto compFlag = Lut1DOpData::COMPOSE_RESAMPLE_BIG; auto thisLut = lut1DData(); Lut1DOpDataRcPtr result = Lut1DOpData::Compose(thisLut, secondLut, compFlag); - auto composedOp = std::make_shared(result); - ops.push_back(composedOp); + ops.emplace_back(std::make_shared(result)); } bool Lut1DOp::hasChannelCrosstalk() const @@ -188,7 +187,7 @@ void CreateLut1DOp(OpRcPtrVec & ops, lutData = lut->inverse(); } - ops.push_back(std::make_shared(lutData)); + ops.emplace_back(std::make_shared(lutData)); } void GenerateIdentityLut1D(float* img, int numElements, int numChannels) diff --git a/src/OpenColorIO/ops/lut3d/Lut3DOp.cpp b/src/OpenColorIO/ops/lut3d/Lut3DOp.cpp index 4f0de76f4d..741f95a699 100644 --- a/src/OpenColorIO/ops/lut3d/Lut3DOp.cpp +++ b/src/OpenColorIO/ops/lut3d/Lut3DOp.cpp @@ -175,8 +175,7 @@ void Lut3DOp::combineWith(OpRcPtrVec & ops, ConstOpRcPtr & secondOp) const auto secondLut = typedRcPtr->lut3DData(); auto thisLut = lut3DData(); auto composed = Lut3DOpData::Compose(thisLut, secondLut); - auto composedOp = std::make_shared(composed); - ops.push_back(composedOp); + ops.emplace_back(std::make_shared(composed)); } bool Lut3DOp::hasChannelCrosstalk() const @@ -229,7 +228,7 @@ void CreateLut3DOp(OpRcPtrVec & ops, Lut3DOpDataRcPtr & lut, TransformDirection lutData = lut->inverse(); } - ops.push_back(std::make_shared(lutData)); + ops.emplace_back(std::make_shared(lutData)); } void CreateLut3DTransform(GroupTransformRcPtr & group, ConstOpRcPtr & op) diff --git a/src/OpenColorIO/ops/matrix/MatrixOp.cpp b/src/OpenColorIO/ops/matrix/MatrixOp.cpp index b593983128..1928700632 100644 --- a/src/OpenColorIO/ops/matrix/MatrixOp.cpp +++ b/src/OpenColorIO/ops/matrix/MatrixOp.cpp @@ -288,7 +288,7 @@ void CreateIdentityMatrixOp(OpRcPtrVec & ops, TransformDirection direction) matrix[15] = 1.0; const double offset[4] = { 0.0, 0.0, 0.0, 0.0 }; - ops.push_back(std::make_shared(matrix, + ops.emplace_back(std::make_shared(matrix, offset, direction)); } @@ -348,14 +348,14 @@ void CreateMatrixOp(OpRcPtrVec & ops, MatrixOpDataRcPtr & matrix, TransformDirec mat->setDirection(newDir); } - ops.push_back(std::make_shared(mat)); + ops.emplace_back(std::make_shared(mat)); } void CreateIdentityMatrixOp(OpRcPtrVec & ops) { MatrixOpDataRcPtr mat = MatrixOpData::CreateDiagonalMatrix(1.0); - ops.push_back(std::make_shared(mat)); + ops.emplace_back(std::make_shared(mat)); } /////////////////////////////////////////////////////////////////////////// diff --git a/src/OpenColorIO/ops/noop/NoOps.cpp b/src/OpenColorIO/ops/noop/NoOps.cpp index cf71b1fde5..690c0b9a08 100644 --- a/src/OpenColorIO/ops/noop/NoOps.cpp +++ b/src/OpenColorIO/ops/noop/NoOps.cpp @@ -103,7 +103,7 @@ bool DefinesGpuAllocation(const OpRcPtr & op) void CreateGpuAllocationNoOp(OpRcPtrVec & ops, const AllocationData & allocationData) { - ops.push_back( std::make_shared(allocationData) ); + ops.emplace_back( std::make_shared(allocationData) ); } @@ -233,7 +233,7 @@ void PartitionGPUOps(OpRcPtrVec & gpuPreOps, { for(unsigned int i=0; iclone() ); + gpuPreOps.emplace_back( ops[i]->clone() ); } } // Analytical -> 3D LUT -> analytical @@ -242,7 +242,7 @@ void PartitionGPUOps(OpRcPtrVec & gpuPreOps, // Handle analytical shader block before start index. for(int i=0; iclone() ); + gpuPreOps.emplace_back( ops[i]->clone() ); } // Get the GPU Allocation at the cross-over point @@ -275,13 +275,13 @@ void PartitionGPUOps(OpRcPtrVec & gpuPreOps, // Handle cpu lattice processing for(int i=gpuLut3DOpStartIndex; i<=gpuLut3DOpEndIndex; ++i) { - gpuLatticeOps.push_back( ops[i]->clone() ); + gpuLatticeOps.emplace_back( ops[i]->clone() ); } // And then handle the gpu post processing for(int i=gpuLut3DOpEndIndex+1; i<(int)ops.size(); ++i) { - gpuPostOps.push_back( ops[i]->clone() ); + gpuPostOps.emplace_back( ops[i]->clone() ); } } } @@ -365,7 +365,7 @@ std::string FileNoOp::getCacheID() const void CreateFileNoOp(OpRcPtrVec & ops, const std::string & fileReference) { - ops.push_back( std::make_shared(fileReference) ); + ops.emplace_back( std::make_shared(fileReference) ); } @@ -449,7 +449,7 @@ std::string LookNoOp::getCacheID() const void CreateLookNoOp(OpRcPtrVec & ops, const std::string & look) { - ops.push_back( std::make_shared(look) ); + ops.emplace_back( std::make_shared(look) ); } } // namespace OCIO_NAMESPACE diff --git a/src/OpenColorIO/ops/range/RangeOp.cpp b/src/OpenColorIO/ops/range/RangeOp.cpp index f708908fbd..0dd63f9b19 100644 --- a/src/OpenColorIO/ops/range/RangeOp.cpp +++ b/src/OpenColorIO/ops/range/RangeOp.cpp @@ -238,7 +238,7 @@ void CreateRangeOp(OpRcPtrVec & ops, RangeOpDataRcPtr & rangeData, TransformDire range->setDirection(newDir); } - ops.push_back(std::make_shared(range)); + ops.emplace_back(std::make_shared(range)); } ///////////////////////////////////////////////////////////////////////////