Make OCIO SupportsOpenGL and SupportsTiles logic more consistent (#28)

* Introduce OCIOPluginBase class to hold common logic. Signed-off-by: Aaron Colwell <300262+acolwell@users.noreply.github.com> * Introduce OCIOPluginBase::setSupportsOpenGLAndTileInfo() to make constructor and param changes consistent. Signed-off-by: Aaron Colwell <300262+acolwell@users.noreply.github.com> * Move enableGPU param logic to OCIOPluginBase Signed-off-by: Aaron Colwell <300262+acolwell@users.noreply.github.com> * Move premult and premultChannel parameters to OCIOPluginBase. Signed-off-by: Aaron Colwell <300262+acolwell@users.noreply.github.com> * Reworked setSupportsOpenGLAndTileInfo name/signature to be more in line with openfx naming. Signed-off-by: Aaron Colwell <300262+acolwell@users.noreply.github.com> * OCIOPluginBase.cpp: add license block * OCIOPluginBase.h: add license block --------- Signed-off-by: Aaron Colwell <300262+acolwell@users.noreply.github.com> Co-authored-by: Frédéric Devernay <devernay@users.noreply.github.com>
NatronGitHub · Apr 4, 2023 · b8a276a · b8a276a
1 parent f63c273
commit b8a276a
Show file tree

Hide file tree

Showing 9 changed files with 286 additions and 532 deletions.
diff --git a/OCIO/Makefile b/OCIO/Makefile
@@ -1,4 +1,4 @@
-PLUGINOBJECTS = ofxsThreadSuite.o tinythread.o OCIOCDLTransform.o OCIOColorSpace.o OCIODisplay.o OCIOFileTransform.o OCIOLogConvert.o OCIOLookTransform.o GenericOCIO.o $(OCIO_OPENGL_OBJS)
+PLUGINOBJECTS = ofxsThreadSuite.o tinythread.o OCIOCDLTransform.o OCIOColorSpace.o OCIODisplay.o OCIOFileTransform.o OCIOLogConvert.o OCIOLookTransform.o OCIOPluginBase.o GenericOCIO.o $(OCIO_OPENGL_OBJS)
 OCIO_OPENGL_OBJS = GenericOCIOOpenGL.o glsl.o glad.o ofxsOGLUtilities.o
 PLUGINNAME = OCIO
 

diff --git a/OCIO/OCIOCDLTransform.cpp b/OCIO/OCIOCDLTransform.cpp
@@ -36,6 +36,7 @@
 #include "ofxsThreadSuite.h"
 
 #include "GenericOCIO.h"
+#include "OCIOPluginBase.h"
 
 namespace OCIO = OCIO_NAMESPACE;
 
@@ -61,7 +62,7 @@ OFXS_NAMESPACE_ANONYMOUS_ENTER
 
 #define kPluginIdentifier "fr.inria.openfx.OCIOCDLTransform"
 #define kPluginVersionMajor 1 // Incrementing this number means that you have broken backwards compatibility of the plug-in.
-#define kPluginVersionMinor 0 // Increment this when you have fixed a bug or made it faster.
+#define kPluginVersionMinor 1 // Increment this when you have fixed a bug or made it faster.
 
 #define kSupportsTiles 1
 #define kSupportsMultiResolution 1
@@ -126,27 +127,10 @@ OFXS_NAMESPACE_ANONYMOUS_ENTER
 #define kParamExportHint "Export this grade as a ColorCorrection XML file (.cc), which can be loaded with the OCIOFileTransform, or using a FileTransform in an OCIO config. The file must not already exist."
 #define kParamExportDefault "Set filename to export this grade as .cc"
 
-#if defined(OFX_SUPPORTS_OPENGLRENDER)
-#define kParamEnableGPU "enableGPU"
-#define kParamEnableGPULabel "Enable GPU Render"
-#if OCIO_VERSION_HEX >= 0x02000000
-#define kParamEnableGPUHint_warn ""
-#else
-// OCIO1's GPU render is not accurate enough.
-// see https://github.com/imageworks/OpenColorIO/issues/394
-// and https://github.com/imageworks/OpenColorIO/issues/456
-#define kParamEnableGPUHint_warn "Note that GPU render is not as accurate as CPU render, so this should be enabled with care.\n"
-#endif
-#define kParamEnableGPUHint                                                                                                                                                              \
-    "Enable GPU-based OpenGL render (only available when \"(Un)premult\" is not checked).\n" kParamEnableGPUHint_warn                                                                    \
-    "If the checkbox is checked but is not enabled (i.e. it cannot be unchecked), GPU render can not be enabled or disabled from the plugin and is probably part of the host options.\n" \
-    "If the checkbox is not checked and is not enabled (i.e. it cannot be checked), GPU render is not available on this host."
-#endif
-
 static bool gHostIsNatron = false; // TODO: generate a CCCId choice param kParamCCCIDChoice from available IDs
 
 class OCIOCDLTransformPlugin
-    : public ImageEffect {
+    : public OCIOPluginBase {
 public:
     OCIOCDLTransformPlugin(OfxImageEffectHandle handle);
 
@@ -341,8 +325,6 @@ class OCIOCDLTransformPlugin
     IntParam* _version;
     StringParam* _cccid;
     StringParam* _export;
-    BooleanParam* _premult;
-    ChoiceParam* _premultChannel;
     DoubleParam* _mix;
     BooleanParam* _maskApply;
     BooleanParam* _maskInvert;
@@ -362,13 +344,12 @@ class OCIOCDLTransformPlugin
     int _procDirection;
 
 #if defined(OFX_SUPPORTS_OPENGLRENDER)
-    BooleanParam* _enableGPU;
     OCIOOpenGLContextData* _openGLContextData; // (OpenGL-only) - the single openGL context, in case the host does not support kNatronOfxImageEffectPropOpenGLContextData
 #endif
 };
 
 OCIOCDLTransformPlugin::OCIOCDLTransformPlugin(OfxImageEffectHandle handle)
-    : ImageEffect(handle)
+    : OCIOPluginBase(handle)
     , _dstClip(NULL)
     , _srcClip(NULL)
     , _maskClip(NULL)
@@ -383,8 +364,6 @@ OCIOCDLTransformPlugin::OCIOCDLTransformPlugin(OfxImageEffectHandle handle)
     , _version(NULL)
     , _cccid(NULL)
     , _export(NULL)
-    , _premult(NULL)
-    , _premultChannel(NULL)
     , _mix(NULL)
     , _maskApply(NULL)
     , _maskInvert(NULL)
@@ -400,7 +379,6 @@ OCIOCDLTransformPlugin::OCIOCDLTransformPlugin(OfxImageEffectHandle handle)
     , _procSaturation(-1)
     , _procDirection(-1)
 #if defined(OFX_SUPPORTS_OPENGLRENDER)
-    , _enableGPU(NULL)
     , _openGLContextData(NULL)
 #endif
 {
@@ -421,25 +399,13 @@ OCIOCDLTransformPlugin::OCIOCDLTransformPlugin(OfxImageEffectHandle handle)
     _cccid = fetchStringParam(kParamCCCID);
     _export = fetchStringParam(kParamExport);
     assert(_slope && _offset && _power && _saturation && _direction && _readFromFile && _file && _version && _cccid && _export);
-    _premult = fetchBooleanParam(kParamPremult);
-    _premultChannel = fetchChoiceParam(kParamPremultChannel);
-    assert(_premult && _premultChannel);
     _mix = fetchDoubleParam(kParamMix);
     _maskApply = paramExists(kParamMaskApply) ? fetchBooleanParam(kParamMaskApply) : 0;
     _maskInvert = fetchBooleanParam(kParamMaskInvert);
     assert(_mix && _maskInvert);
 
 #if defined(OFX_SUPPORTS_OPENGLRENDER)
-    _enableGPU = fetchBooleanParam(kParamEnableGPU);
-    assert(_enableGPU);
-    const ImageEffectHostDescription& gHostDescription = *getImageEffectHostDescription();
-    if (!gHostDescription.supportsOpenGLRender) {
-        _enableGPU->setEnabled(false);
-    }
-    // GPU rendering is wrong when (un)premult is checked
-    bool premult = _premult->getValue();
-    _enableGPU->setEnabled(!premult);
-    setSupportsOpenGLRender(!premult && _enableGPU->getValue());
+    setSupportsOpenGLAndTileInfo();
 #endif
 
     bool readFromFile;
@@ -503,8 +469,7 @@ OCIOCDLTransformPlugin::setupAndCopy(PixelProcessorFilterBase& processor,
 
     bool premult;
     int premultChannel;
-    _premult->getValueAtTime(time, premult);
-    _premultChannel->getValueAtTime(time, premultChannel);
+    getPremultAndPremultChannelAtTime(time, premult, premultChannel);
     double mix;
     _mix->getValueAtTime(time, mix);
     processor.setPremultMaskMix(premult, premultChannel, mix);
@@ -941,8 +906,7 @@ OCIOCDLTransformPlugin::render(const RenderArguments& args)
     size_t memSize = (args.renderWindow.y2 - args.renderWindow.y1) * tmpRowBytes;
     ImageMemory mem(memSize, this);
     float* tmpPixelData = (float*)mem.lock();
-    bool premult;
-    _premult->getValueAtTime(args.time, premult);
+    bool premult = getPremultValueAtTime(args.time);
 
     // copy renderWindow to the temporary image
     copyPixelData(premult, false, false, args.time, args.renderWindow, args.renderScale, srcPixelData, bounds, pixelComponents, pixelComponentCount, bitDepth, srcRowBytes, tmpPixelData, args.renderWindow, pixelComponents, pixelComponentCount, bitDepth, tmpRowBytes);
@@ -1256,13 +1220,8 @@ OCIOCDLTransformPlugin::changedParam(const InstanceChangedArgs& args,
         // reset back to default
         _export->setValue(kParamExportDefault);
 #if defined(OFX_SUPPORTS_OPENGLRENDER)
-    } else if (paramName == kParamEnableGPU || paramName == kParamPremult) {
-        // GPU rendering is wrong when (un)premult is checked
-        bool premult = _premult->getValueAtTime(args.time);
-        _enableGPU->setEnabled(!premult);
-        bool supportsGL = !premult && _enableGPU->getValueAtTime(args.time);
-        setSupportsOpenGLRender(supportsGL);
-        setSupportsTiles(!supportsGL);
+    } else if (paramEffectsOpenGLAndTileSupport(paramName) || paramName == kParamPremult) {
+        setSupportsOpenGLAndTileInfoAtTime(args.time);
 #endif
     }
 } // OCIOCDLTransformPlugin::changedParam
@@ -1272,21 +1231,7 @@ OCIOCDLTransformPlugin::changedClip(const InstanceChangedArgs& args,
                                     const string& clipName)
 {
     if ((clipName == kOfxImageEffectSimpleSourceClipName) && _srcClip && (args.reason == eChangeUserEdit)) {
-        if (_srcClip->getPixelComponents() != ePixelComponentRGBA) {
-            _premult->setValue(false);
-        } else {
-            switch (_srcClip->getPreMultiplication()) {
-            case eImageOpaque:
-                _premult->setValue(false);
-                break;
-            case eImagePreMultiplied:
-                _premult->setValue(true);
-                break;
-            case eImageUnPreMultiplied:
-                _premult->setValue(false);
-                break;
-            }
-        }
+        changedSrcClip(_srcClip);
     }
 }
 
@@ -1468,29 +1413,7 @@ OCIOCDLTransformPluginFactory::describeInContext(ImageEffectDescriptor& desc,
     }
 
 #if defined(OFX_SUPPORTS_OPENGLRENDER)
-    {
-        BooleanParamDescriptor* param = desc.defineBooleanParam(kParamEnableGPU);
-        param->setLabel(kParamEnableGPULabel);
-        param->setHint(kParamEnableGPUHint);
-        const ImageEffectHostDescription& gHostDescription = *getImageEffectHostDescription();
-        // Resolve advertises OpenGL support in its host description, but never calls render with OpenGL enabled
-        if (gHostDescription.supportsOpenGLRender && (gHostDescription.hostName != "DaVinciResolveLite")) {
-            // OCIO's GPU render is not accurate enough.
-            // see https://github.com/imageworks/OpenColorIO/issues/394
-            param->setDefault(/*true*/ false);
-            if (gHostDescription.APIVersionMajor * 100 + gHostDescription.APIVersionMinor < 104) {
-                // Switching OpenGL render from the plugin was introduced in OFX 1.4
-                param->setEnabled(false);
-            }
-        } else {
-            param->setDefault(false);
-            param->setEnabled(false);
-        }
-
-        if (page) {
-            page->addChild(*param);
-        }
-    }
+    OCIOPluginBase::defineEnableGPUParam(desc, page);
 #endif
 
     ofxsPremultDescribeParams(desc, page);