From 8c9bd561c44bc55d2a0f201dfcf059139664e5f9 Mon Sep 17 00:00:00 2001 From: Patrick Hodoul Date: Mon, 10 May 2021 10:21:29 -0400 Subject: [PATCH 1/2] Adsk Contrib - Fix some bugs found by Maya and SonarCloud Signed-off-by: Patrick Hodoul --- src/OpenColorIO/Config.cpp | 2 +- src/OpenColorIO/DynamicProperty.cpp | 8 ++++---- src/OpenColorIO/DynamicProperty.h | 2 +- src/OpenColorIO/OpOptimizers.cpp | 4 ++-- src/OpenColorIO/ops/allocation/AllocationOp.cpp | 1 - .../ops/gradingprimary/GradingPrimary.cpp | 12 ++++++------ src/OpenColorIO/ops/gradingprimary/GradingPrimary.h | 4 +++- .../cpu/ops/gradingprimary/GradingPrimary_tests.cpp | 7 +++++++ 8 files changed, 24 insertions(+), 16 deletions(-) diff --git a/src/OpenColorIO/Config.cpp b/src/OpenColorIO/Config.cpp index 3685413df4..3e20ce49f1 100644 --- a/src/OpenColorIO/Config.cpp +++ b/src/OpenColorIO/Config.cpp @@ -1075,7 +1075,7 @@ class Config::Impl m_allColorSpaces->removeColorSpace(colorSpaceName.c_str()); - throw ex; + throw; } // The display must be active. diff --git a/src/OpenColorIO/DynamicProperty.cpp b/src/OpenColorIO/DynamicProperty.cpp index 808fba4930..5885492cfe 100644 --- a/src/OpenColorIO/DynamicProperty.cpp +++ b/src/OpenColorIO/DynamicProperty.cpp @@ -76,25 +76,25 @@ bool DynamicPropertyImpl::equals(const DynamicPropertyImpl & rhs) const { auto lhst = dynamic_cast(this); auto rhst = dynamic_cast(&rhs); - return (lhst->getValue() == rhst->getValue()); + return lhst && rhst && (lhst->getValue() == rhst->getValue()); } case DYNAMIC_PROPERTY_GRADING_PRIMARY: { auto lhst = dynamic_cast(this); auto rhst = dynamic_cast(&rhs); - return (lhst->getValue() == rhst->getValue()); + return lhst && rhst && (lhst->getValue() == rhst->getValue()); } case DYNAMIC_PROPERTY_GRADING_RGBCURVE: { auto lhst = dynamic_cast(this); auto rhst = dynamic_cast(&rhs); - return (*lhst->getValue() == *rhst->getValue()); + return lhst && rhst && (*lhst->getValue() == *rhst->getValue()); } case DYNAMIC_PROPERTY_GRADING_TONE: { auto lhst = dynamic_cast(this); auto rhst = dynamic_cast(&rhs); - return (lhst->getValue() == rhst->getValue()); + return lhst && rhst && (lhst->getValue() == rhst->getValue()); } } // Different values. diff --git a/src/OpenColorIO/DynamicProperty.h b/src/OpenColorIO/DynamicProperty.h index 4bf49ea3c6..0e398d9b30 100644 --- a/src/OpenColorIO/DynamicProperty.h +++ b/src/OpenColorIO/DynamicProperty.h @@ -123,7 +123,7 @@ class DynamicPropertyGradingPrimaryImpl : public DynamicPropertyImpl, const Float3 & getSlope() const { return m_preRenderValues.getSlope(); } // Do not apply the op if all params are identity. - bool getLocalBypass() const { return m_preRenderValues.m_localBypass; } + bool getLocalBypass() const { return m_preRenderValues.getLocalBypass(); } DynamicPropertyGradingPrimaryImplRcPtr createEditableCopy() const; diff --git a/src/OpenColorIO/OpOptimizers.cpp b/src/OpenColorIO/OpOptimizers.cpp index 727e29674a..8b7e2c2fc7 100755 --- a/src/OpenColorIO/OpOptimizers.cpp +++ b/src/OpenColorIO/OpOptimizers.cpp @@ -207,7 +207,7 @@ int RemoveInverseOps(OpRcPtrVec & opVec, OptimizationFlags oFlags) int count = 0; int firstindex = 0; // this must be a signed int - while (firstindex < static_cast(opVec.size() - 1)) + while (firstindex < (static_cast(opVec.size()) - 1)) { ConstOpRcPtr op1 = opVec[firstindex]; ConstOpRcPtr op2 = opVec[firstindex + 1]; @@ -273,7 +273,7 @@ int CombineOps(OpRcPtrVec & opVec, OptimizationFlags oFlags) OpRcPtrVec tmpops; - while (firstindex < static_cast(opVec.size() - 1)) + while (firstindex < (static_cast(opVec.size()) - 1)) { ConstOpRcPtr op1 = opVec[firstindex]; ConstOpRcPtr op2 = opVec[firstindex + 1]; diff --git a/src/OpenColorIO/ops/allocation/AllocationOp.cpp b/src/OpenColorIO/ops/allocation/AllocationOp.cpp index 01fe03d8d2..e8141e38fb 100644 --- a/src/OpenColorIO/ops/allocation/AllocationOp.cpp +++ b/src/OpenColorIO/ops/allocation/AllocationOp.cpp @@ -113,7 +113,6 @@ void CreateAllocationOps(OpRcPtrVec & ops, } case ALLOCATION_UNKNOWN: throw Exception("Unsupported Allocation Type."); - break; } } diff --git a/src/OpenColorIO/ops/gradingprimary/GradingPrimary.cpp b/src/OpenColorIO/ops/gradingprimary/GradingPrimary.cpp index 98a500cf84..8f70916b22 100644 --- a/src/OpenColorIO/ops/gradingprimary/GradingPrimary.cpp +++ b/src/OpenColorIO/ops/gradingprimary/GradingPrimary.cpp @@ -150,8 +150,8 @@ void GradingPrimaryPreRender::update(GradingStyle style, m_isPowerIdentity = m_gamma[0] == 1.0f && m_gamma[1] == 1.0f && m_gamma[2] == 1.0f; m_pivot = 0.5 + v.m_pivot * 0.5; m_localBypass = m_localBypass && m_isPowerIdentity && - m_brightness[0] == 0.f && m_brightness[2] == 0.f && m_brightness[2] == 0.f && - m_contrast[0] == 1.f && m_contrast[2] == 1.f && m_contrast[2] == 1.f; + m_brightness[0] == 0.f && m_brightness[1] == 0.f && m_brightness[2] == 0.f && + m_contrast[0] == 1.f && m_contrast[1] == 1.f && m_contrast[2] == 1.f; break; } case GRADING_LIN: @@ -193,8 +193,8 @@ void GradingPrimaryPreRender::update(GradingStyle style, m_contrast[2] == 1.0f; m_pivot = 0.18 * std::pow(2., v.m_pivot); m_localBypass = m_localBypass && m_isPowerIdentity && - m_exposure[0] == 1.f && m_exposure[2] == 1.f && m_exposure[2] == 1.f && - m_offset[0] == 0.f && m_offset[2] == 0.f && m_offset[2] == 0.f; + m_exposure[0] == 1.f && m_exposure[1] == 1.f && m_exposure[2] == 1.f && + m_offset[0] == 0.f && m_offset[1] == 0.f && m_offset[2] == 0.f; break; } case GRADING_VIDEO: @@ -251,8 +251,8 @@ void GradingPrimaryPreRender::update(GradingStyle style, } m_isPowerIdentity = m_gamma[0] == 1.0f || m_gamma[1] == 1.0f || m_gamma[2] == 1.0f; m_localBypass = m_localBypass && m_isPowerIdentity && - m_slope[0] == 1.f && m_slope[2] == 1.f && m_slope[2] == 1.f && - m_offset[0] == 0.f && m_offset[2] == 0.f && m_offset[2] == 0.f; + m_slope[0] == 1.f && m_slope[1] == 1.f && m_slope[2] == 1.f && + m_offset[0] == 0.f && m_offset[1] == 0.f && m_offset[2] == 0.f; break; } } diff --git a/src/OpenColorIO/ops/gradingprimary/GradingPrimary.h b/src/OpenColorIO/ops/gradingprimary/GradingPrimary.h index 8fbda9a2a5..9a47be5a53 100644 --- a/src/OpenColorIO/ops/gradingprimary/GradingPrimary.h +++ b/src/OpenColorIO/ops/gradingprimary/GradingPrimary.h @@ -24,7 +24,7 @@ struct GradingPrimaryPreRender void update(GradingStyle style, TransformDirection dir, const GradingPrimary & v) noexcept; // Do not apply the op if all params are identity. - bool m_localBypass{ false }; + bool getLocalBypass() const { return m_localBypass; } // Access to the precomputed values. Note that values are already inversed based on the // direction so that no computation is required before using them. @@ -58,6 +58,8 @@ struct GradingPrimaryPreRender double m_pivot{ 0. }; bool m_isPowerIdentity{ false }; + + bool m_localBypass{ false }; }; bool operator==(const GradingRGBM & lhs, const GradingRGBM & rhs); diff --git a/tests/cpu/ops/gradingprimary/GradingPrimary_tests.cpp b/tests/cpu/ops/gradingprimary/GradingPrimary_tests.cpp index 16dca08f5f..a9d0f3e641 100644 --- a/tests/cpu/ops/gradingprimary/GradingPrimary_tests.cpp +++ b/tests/cpu/ops/gradingprimary/GradingPrimary_tests.cpp @@ -90,7 +90,14 @@ OCIO_ADD_TEST(GradingPrimary, precompute) OCIO_CHECK_CLOSE(comp.getPivot(), 0.4f, 1.e-6f); OCIO_CHECK_ASSERT(comp.isGammaIdentity()); + gp.m_brightness.m_green = 0.1 * 1023. / 6.25; + comp.update(OCIO::GRADING_LOG, OCIO::TRANSFORM_DIR_FORWARD, gp); + OCIO_CHECK_ASSERT(comp.getBrightness() == OCIO::Float3({ 0.f, 0.1f, 0.f })); + OCIO_CHECK_ASSERT(!comp.getLocalBypass()); + OCIO_CHECK_ASSERT(comp.isGammaIdentity()); + gp.m_brightness.m_red = 0.1 * 1023. / 6.25; + gp.m_brightness.m_green = 0.; gp.m_contrast.m_red = 0.; // Inverse will be 1. gp.m_contrast.m_green = 1.25; gp.m_gamma.m_blue = 0.8; From 15f448c69550134b2b22cef38dafc9bf1966f3f8 Mon Sep 17 00:00:00 2001 From: Patrick Hodoul Date: Tue, 11 May 2021 09:38:24 -0400 Subject: [PATCH 2/2] Fix a Windows warning Signed-off-by: Patrick Hodoul --- src/OpenColorIO/Config.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenColorIO/Config.cpp b/src/OpenColorIO/Config.cpp index 3e20ce49f1..c21519819f 100644 --- a/src/OpenColorIO/Config.cpp +++ b/src/OpenColorIO/Config.cpp @@ -1065,7 +1065,7 @@ class Config::Impl // Note that it adds it or updates the existing one. m_allColorSpaces->addColorSpace(cs); } - catch(const Exception & ex) + catch(const Exception & /* ex */) { DisplayMap::iterator iter = FindDisplay(m_displays, colorSpaceName); if (iter!=m_displays.end())