Adsk Contrib - Remove dynamic properties from configs (#1205)

* Remove dynamic properties from configs

Signed-off-by: Bernard Lefebvre <bernard.lefebvre@autodesk.com>
(cherry picked from commit 58a2796713869c2803f0b921a279899807ae4318)

* Adjust dynamic property access

Signed-off-by: Bernard Lefebvre <bernard.lefebvre@autodesk.com>

* Forgot one case

Signed-off-by: Bernard Lefebvre <bernard.lefebvre@autodesk.com>

* Change also required for grading test

Signed-off-by: Bernard Lefebvre <bernard.lefebvre@autodesk.com>

Co-authored-by: Patrick Hodoul <patrick.hodoul@autodesk.com>

Author: BernardLefebvre

include/OpenColorIO/OpenColorIO.h

@@ -1934,8 +1934,8 @@ class OCIOEXPORT Processor
     /**
      * The returned pointer may be used to set the default value of any dynamic
      * properties of the requested type.  Throws if the requested property is not found.  Note
-     * that if the processor contains several ops that support the requested property, only ones
-     * for which dynamic has been enabled will be controlled.
+     * that if the processor contains several ops that support the requested property, only one
+     * can be dynamic and only this one will be controlled.
      *
      * \note The dynamic properties are a convenient way to change on-the-fly values without 
      * generating again and again a CPU or GPU processor instance. Color transformations can
@@ -1945,7 +1945,8 @@ class OCIOEXPORT Processor
      * are decoupled between the types of processor instances so that the same
      * :cpp:class:`Processor` can generate several independent CPU and/or GPU processor
      * instances i.e. changing the value of the exposure dynamic property from a CPU processor
-     * instance does not affect the corresponding GPU processor instance.
+     * instance does not affect the corresponding GPU processor instance. Processor creation will
+     * throw if there are more than one property of a given type.
      */
     DynamicPropertyRcPtr getDynamicProperty(DynamicPropertyType type) const;
     /// True if at least one dynamic property of that type exists.
@@ -2064,12 +2065,11 @@ class OCIOEXPORT CPUProcessor
 
     /* The returned pointer may be used to set the value of any dynamic properties
      * of the requested type.  Throws if the requested property is not found.  Note that if the
-     * processor contains several ops that support the requested property, only ones for which
-     * dynamic has been enabled will be controlled.
+     * processor contains several ops that support the requested property, only one can be dynamic.
      *
      * \note The dynamic properties in this object are decoupled from the ones in the
      * \ref Processor it was generated from. For each dynamic property in the Processor,
-     * there is one ine the CPU processor.
+     * there is one in the CPU processor.
      */
     DynamicPropertyRcPtr getDynamicProperty(DynamicPropertyType type) const;
 

include/OpenColorIO/OpenColorTransforms.h

@@ -893,39 +893,45 @@ class OCIOEXPORT ExposureContrastTransform : public Transform
     virtual bool equals(const ExposureContrastTransform & other) const noexcept = 0;
 
     virtual ExposureContrastStyle getStyle() const = 0;
-    /**
-     * Select the algorithm for linear, video
-     * or log color spaces.
-     */
+    /// Select the algorithm for linear, video or log color spaces.
     virtual void setStyle(ExposureContrastStyle style) = 0;
 
     virtual double getExposure() const = 0;
     /**
-     * Applies an exposure adjustment.  The value is in
-     * units of stops (regardless of style), for example, a value of -1
-     * would be equivalent to reducing the lighting by one half.
+     * Applies an exposure adjustment.  The value is in units of stops (regardless of style), for
+     * example, a value of -1  would be equivalent to reducing the lighting by one half.
      */
     virtual void setExposure(double exposure) = 0;
+    /**
+     * Exposure can be made dynamic so the value can be changed through the CPU or GPU processor,
+     * but if there are several ExposureContrastTransform only one can have a dynamic exposure.
+     */
     virtual bool isExposureDynamic() const = 0;
     virtual void makeExposureDynamic() = 0;
     virtual void makeExposureNonDynamic() = 0;
 
     virtual double getContrast() const = 0;
     /**
-     * Applies a contrast/gamma adjustment around a pivot
-     * point.  The contrast and gamma are mathematically the same, but two
-     * controls are provided to enable the use of separate dynamic
-     * parameters.  Contrast is usually a scene-referred adjustment that
-     * pivots around gray whereas gamma is usually a display-referred
-     * adjustment that pivots around white.
+     * Applies a contrast/gamma adjustment around a pivot point.  The contrast and gamma are
+     * mathematically the same, but two controls are provided to enable the use of separate
+     * dynamic parameters.  Contrast is usually a scene-referred adjustment that pivots around
+     * gray whereas gamma is usually a display-referred adjustment that pivots around white.
      */
     virtual void setContrast(double contrast) = 0;
+    /**
+     * Contrast can be made dynamic so the value can be changed through the CPU or GPU processor,
+     * but if there are several ExposureContrastTransform only one can have a dynamic contrast.
+     */
     virtual bool isContrastDynamic() const = 0;
     virtual void makeContrastDynamic() = 0;
     virtual void makeContrastNonDynamic() = 0;
 
     virtual double getGamma() const = 0;
     virtual void setGamma(double gamma) = 0;
+    /**
+     * Gamma can be made dynamic so the value can be changed through the CPU or GPU processor,
+     * but if there are several ExposureContrastTransform only one can have a dynamic gamma.
+     */
     virtual bool isGammaDynamic() const = 0;
     virtual void makeGammaDynamic() = 0;
     virtual void makeGammaNonDynamic() = 0;
@@ -1116,6 +1122,10 @@ class OCIOEXPORT GradingPrimaryTransform : public Transform
     /// Throws if value is not valid.
     virtual void setValue(const GradingPrimary & values) = 0;
 
+    /**
+     * Parameters can be made dynamic so the values can be changed through the CPU or GPU processor,
+     * but if there are several GradingPrimaryTransform only one can have dynamic parameters.
+     */
     virtual bool isDynamic() const noexcept = 0;
     virtual void makeDynamic() noexcept = 0;
     virtual void makeNonDynamic() noexcept = 0;
@@ -1176,6 +1186,10 @@ class OCIOEXPORT GradingRGBCurveTransform : public Transform
     virtual bool getBypassLinToLog() const = 0;
     virtual void setBypassLinToLog(bool bypass) = 0;
 
+    /**
+     * Parameters can be made dynamic so the values can be changed through the CPU or GPU processor,
+     * but if there are several GradingRGBCurveTransform only one can have dynamic parameters.
+     */
     virtual bool isDynamic() const noexcept = 0;
     virtual void makeDynamic() noexcept = 0;
     virtual void makeNonDynamic() noexcept = 0;
@@ -1236,6 +1250,10 @@ class OCIOEXPORT GradingToneTransform : public Transform
     virtual const GradingTone & getValue() const = 0;
     virtual void setValue(const GradingTone & values) = 0;
 
+    /**
+     * Parameters can be made dynamic so the values can be changed through the CPU or GPU processor,
+     * but if there are several GradingToneTransform only one can have dynamic parameters.
+     */
     virtual bool isDynamic() const noexcept = 0;
     virtual void makeDynamic() noexcept = 0;
     virtual void makeNonDynamic() noexcept = 0;

src/OpenColorIO/CPUProcessor.cpp

@@ -297,22 +297,10 @@ void FinalizeOpsForCPU(OpRcPtrVec & ops, const OpRcPtrVec & rawOps,
 
     if (!((oFlags & OPTIMIZATION_NO_DYNAMIC_PROPERTIES) == OPTIMIZATION_NO_DYNAMIC_PROPERTIES))
     {
-        ops.unifyDynamicProperties();
+        ops.validateDynamicProperties();
     }
 }
 
-namespace
-{
-template<typename T>
-void UnifyDynamicProperty(ConstOpCPURcPtr & op, std::shared_ptr<T> & prop, DynamicPropertyType type)
-{
-    if (op->hasDynamicProperty(type))
-    {
-        op->unifyDynamicProperty(type, prop);
-    }
-}
-}
-
 void CPUProcessor::Impl::finalize(const OpRcPtrVec & rawOps,
                                   BitDepth in, BitDepth out,
                                   OptimizationFlags oFlags)
@@ -338,30 +326,6 @@ void CPUProcessor::Impl::finalize(const OpRcPtrVec & rawOps,
     m_outBitDepthOp = nullptr;
     CreateCPUEngine(ops, in, out, oFlags, m_inBitDepthOp, m_cpuOps, m_outBitDepthOp);
 
-    // This ensures that dynamic properties are decoupled from processor and that when a dynamic
-    // property on a CPU processor is modified, all ops that respond to that property (and which
-    // are enabled) are synchronized.
-    DynamicPropertyDoubleImplRcPtr dpExposure;
-    DynamicPropertyDoubleImplRcPtr dpContrast;
-    DynamicPropertyDoubleImplRcPtr dpGamma;
-    DynamicPropertyGradingPrimaryImplRcPtr dpGradingPrimary;
-    DynamicPropertyGradingRGBCurveImplRcPtr dpGradingRGBCurve;
-    DynamicPropertyGradingToneImplRcPtr dpGradingTone;
-
-    ConstOpCPURcPtrVec allCpuOps = m_cpuOps;
-    allCpuOps.push_back(m_inBitDepthOp);
-    allCpuOps.push_back(m_outBitDepthOp);
-    for (auto op : allCpuOps)
-    {
-        // Duplicate & initialize or share property.
-        UnifyDynamicProperty(op, dpExposure, DYNAMIC_PROPERTY_EXPOSURE);
-        UnifyDynamicProperty(op, dpContrast, DYNAMIC_PROPERTY_CONTRAST);
-        UnifyDynamicProperty(op, dpGamma, DYNAMIC_PROPERTY_GAMMA);
-        UnifyDynamicProperty(op, dpGradingPrimary, DYNAMIC_PROPERTY_GRADING_PRIMARY);
-        UnifyDynamicProperty(op, dpGradingRGBCurve, DYNAMIC_PROPERTY_GRADING_RGBCURVE);
-        UnifyDynamicProperty(op, dpGradingTone, DYNAMIC_PROPERTY_GRADING_TONE);
-    }
-
     // Compute the cache id.
 
     std::stringstream ss;

src/OpenColorIO/Config.cpp

@@ -4055,32 +4055,101 @@ void Config::Impl::checkVersionConsistency(ConstTransformRcPtr & transform) cons
 {
     if (transform)
     {
-        if (ConstExponentTransformRcPtr ex = DynamicPtrCast<const ExponentTransform>(transform))
+        if (ConstBuiltinTransformRcPtr ex = DynamicPtrCast<const BuiltinTransform>(transform))
+        {
+            if (m_majorVersion < 2)
+            {
+                throw Exception("Only config version 2 (or higher) can have BuiltinInTransform.");
+            }
+        }
+        else if (ConstCDLTransformRcPtr cdl = DynamicPtrCast<const CDLTransform>(transform))
+        {
+            if (m_majorVersion < 2 && cdl->getStyle() != CDL_TRANSFORM_DEFAULT)
+            {
+                throw Exception("Only config version 2 (or higher) can have style for "
+                               "CDLTransform.");
+            }
+        }
+        else if (DynamicPtrCast<const DisplayViewTransform>(transform))
+        {
+            if (m_majorVersion < 2)
+            {
+                throw Exception("Only config version 2 (or higher) can have DisplayViewTransform.");
+            }
+        }
+        else if (ConstExponentTransformRcPtr ex =
+                 DynamicPtrCast<const ExponentTransform>(transform))
         {
             if (m_majorVersion < 2 && ex->getNegativeStyle() != NEGATIVE_CLAMP)
             {
-                throw Exception("Config version 1 only supports ExponentTransform clamping negative values.");
+                throw Exception("Config version 1 only supports ExponentTransform clamping "
+                                "negative values.");
             }
         }
         else if (DynamicPtrCast<const ExponentWithLinearTransform>(transform))
         {
             if (m_majorVersion < 2)
             {
-                throw Exception("Only config version 2 (or higher) can have ExponentWithLinearTransform.");
+                throw Exception("Only config version 2 (or higher) can have "
+                                "ExponentWithLinearTransform.");
             }
         }
         else if (DynamicPtrCast<const ExposureContrastTransform>(transform))
         {
             if (m_majorVersion < 2)
             {
-                throw Exception("Only config version 2 (or higher) can have ExposureContrastTransform.");
+                throw Exception("Only config version 2 (or higher) can have "
+                                "ExposureContrastTransform.");
+            }
+        }
+        else if (ConstFileTransformRcPtr ft = DynamicPtrCast<const FileTransform>(transform))
+        {
+            if (m_majorVersion < 2)
+            {
+                if (ft->getInterpolation() == INTERP_CUBIC)
+                {
+                    throw Exception("Only config version 2 (or higher) can use 'cubic' "
+                                    "interpolation with FileTransform.");
+
+                }
+                if (ft->getCDLStyle() != CDL_TRANSFORM_DEFAULT)
+                {
+                    throw Exception("Only config version 2 (or higher) can use CDL style' "
+                                    "for FileTransform.");
+
+                }
             }
         }
         else if (DynamicPtrCast<const FixedFunctionTransform>(transform))
         {
             if (m_majorVersion < 2)
             {
-                throw Exception("Only config version 2 (or higher) can have FixedFunctionTransform.");
+                throw Exception("Only config version 2 (or higher) can have "
+                                "FixedFunctionTransform.");
+            }
+        }
+        else if (DynamicPtrCast<const GradingPrimaryTransform>(transform))
+        {
+            if (m_majorVersion < 2)
+            {
+                throw Exception("Only config version 2 (or higher) can have "
+                                "GradingPrimaryTransform.");
+            }
+        }
+        else if (DynamicPtrCast<const GradingRGBCurveTransform>(transform))
+        {
+            if (m_majorVersion < 2)
+            {
+                throw Exception("Only config version 2 (or higher) can have "
+                                "GradingRGBCurveTransform.");
+            }
+        }
+        else if (DynamicPtrCast<const GradingToneTransform>(transform))
+        {
+            if (m_majorVersion < 2)
+            {
+                throw Exception("Only config version 2 (or higher) can have "
+                                "GradingToneTransform.");
             }
         }
         else if (DynamicPtrCast<const LogAffineTransform>(transform))

src/OpenColorIO/GPUProcessor.cpp

@@ -104,7 +104,7 @@ void GPUProcessor::Impl::finalize(const OpRcPtrVec & rawOps,
     m_ops = rawOps;
 
     m_ops.finalize(oFlags);
-    m_ops.unifyDynamicProperties();
+    m_ops.validateDynamicProperties();
 
     // Is NoOp ?
     m_isNoOp  = m_ops.isNoOp();