diff --git a/Changes.md b/Changes.md index 2b212ce7463..3b032512d26 100644 --- a/Changes.md +++ b/Changes.md @@ -15,6 +15,12 @@ Improvements - Added support for dropping an Edit Scope node onto the widget to set it as the current Edit Scope. - Added support for middle-dragging from the widget to access the current Edit Scope node. - ArnoldAttributes : Added syntax highlighting and auto-complete for set expressions on the `shadowGroup` plug. +- OpenColorIO : When a script-level OpenColorIO variable contains a Gaffer `${contextVariable}` reference, its evaluation is now deferred to the point of use. This allows it to pick up overrides introduced by nodes such as ContextVariables and Wedge. + +Fixes +----- + +- Viewer : Fixed handling of Gaffer `${contextVariable}` references in OpenColorIO variable values. The Viewer now updates the Display Transform appropriately when the value of the context variable changes. API --- diff --git a/python/GafferImageTest/ColorSpaceTest.py b/python/GafferImageTest/ColorSpaceTest.py index 6591d8d1a51..9da4453f0b9 100644 --- a/python/GafferImageTest/ColorSpaceTest.py +++ b/python/GafferImageTest/ColorSpaceTest.py @@ -247,7 +247,7 @@ def testConfigFromGafferContext( self ) : with Gaffer.Context() as c : - GafferImage.OpenColorIOAlgo.setConfig( c, str( self.openColorIOPath() / "context.ocio" ) ) + GafferImage.OpenColorIOAlgo.setConfig( c, ( self.openColorIOPath() / "context.ocio" ).as_posix() ) GafferImage.OpenColorIOAlgo.addVariable( c, "LUT", "srgb.spi1d" ) GafferImage.OpenColorIOAlgo.addVariable( c, "CDL", "cineon.spi1d" ) diff --git a/python/GafferImageTest/DisplayTransformTest.py b/python/GafferImageTest/DisplayTransformTest.py index f16c24f0502..59077b7df08 100644 --- a/python/GafferImageTest/DisplayTransformTest.py +++ b/python/GafferImageTest/DisplayTransformTest.py @@ -256,7 +256,7 @@ def testDisplayAndViewDefaultToConfig( self ) : explicitDisplayTransform["view"].setValue( config.getDefaultView( config.getDefaultDisplay() ) ) with Gaffer.Context() as context : - GafferImage.OpenColorIOAlgo.setConfig( context, str( configPath ) ) + GafferImage.OpenColorIOAlgo.setConfig( context, configPath.as_posix() ) self.assertImagesEqual( defaultDisplayTransform["out"], explicitDisplayTransform["out"] ) if __name__ == "__main__": diff --git a/python/GafferImageTest/ImageReaderTest.py b/python/GafferImageTest/ImageReaderTest.py index 4f38b9a9c5d..07f86b2f469 100644 --- a/python/GafferImageTest/ImageReaderTest.py +++ b/python/GafferImageTest/ImageReaderTest.py @@ -524,7 +524,7 @@ def f( fileName, fileFormat, dataType, metadata, config ) : ( "exr", "openexr", "half", "" ), ( "dpx", "dpx", "uint12", "" ), ( "TIFF", "tiff", "float", "" ), - ( "tif", "tiff", "uint32", str( self.openColorIOPath() / "context.ocio" ) ), + ( "tif", "tiff", "uint32", ( self.openColorIOPath() / "context.ocio" ).as_posix() ), ] : w["fileName"].setValue( self.temporaryDirectory() / "{0}.{1}".format( dataType, ext ) ) diff --git a/python/GafferImageTest/ImageWriterTest.py b/python/GafferImageTest/ImageWriterTest.py index 8d79be16357..e7230a54205 100644 --- a/python/GafferImageTest/ImageWriterTest.py +++ b/python/GafferImageTest/ImageWriterTest.py @@ -1241,7 +1241,7 @@ def f( fileName, fileFormat, dataType, metadata, config ) : ( "exr", "openexr", "half", "" ), ( "dpx", "dpx", "uint12", "" ), ( "TIFF", "tiff", "float", "" ), - ( "tif", "tiff", "uint32", str( self.openColorIOPath() / "context.ocio" ) ), + ( "tif", "tiff", "uint32", ( self.openColorIOPath() / "context.ocio" ).as_posix() ), ] : w["fileName"].setValue( self.temporaryDirectory() / "{0}.{1}".format( dataType, ext ) ) diff --git a/python/GafferImageTest/OpenColorIOAlgoTest.py b/python/GafferImageTest/OpenColorIOAlgoTest.py index a7e08320144..4a3125aae3b 100644 --- a/python/GafferImageTest/OpenColorIOAlgoTest.py +++ b/python/GafferImageTest/OpenColorIOAlgoTest.py @@ -68,7 +68,7 @@ def testCurrentConfig( self ) : with Gaffer.Context() as c : - GafferImage.OpenColorIOAlgo.setConfig( c, str( self.openColorIOPath() / "context.ocio" ) ) + GafferImage.OpenColorIOAlgo.setConfig( c, ( self.openColorIOPath() / "context.ocio" ).as_posix() ) customConfig = GafferImage.OpenColorIOAlgo.currentConfig() self.assertIsInstance( customConfig, PyOpenColorIO.Config ) self.assertEqual( customConfig.getCacheID(), PyOpenColorIO.Config.CreateFromFile( c["ocio:config"] ).getCacheID() ) @@ -89,7 +89,7 @@ def testCurrentConfigAndContext( self ) : with Gaffer.Context() as c : - GafferImage.OpenColorIOAlgo.setConfig( c, str( self.openColorIOPath() / "context.ocio" ) ) + GafferImage.OpenColorIOAlgo.setConfig( c, ( self.openColorIOPath() / "context.ocio" ).as_posix() ) GafferImage.OpenColorIOAlgo.addVariable( c, "LUT", "srgb.spi1d" ) customConfig, customContext = GafferImage.OpenColorIOAlgo.currentConfigAndContext() diff --git a/python/GafferImageTest/OpenColorIOConfigPlugTest.py b/python/GafferImageTest/OpenColorIOConfigPlugTest.py index c33f471a343..daca3186da2 100644 --- a/python/GafferImageTest/OpenColorIOConfigPlugTest.py +++ b/python/GafferImageTest/OpenColorIOConfigPlugTest.py @@ -117,5 +117,34 @@ def testConfigAppliesDuringExecution( self ) : with open( script["writer"]["fileName"].getValue() ) as f : self.assertEqual( f.readlines(), [ "test.ocio, testValueA" ] ) + def testContextVariableInConfigVariable( self ) : + + script = Gaffer.ScriptNode() + script["variables"].addChild( Gaffer.NameValuePlug( "shot", "shot001", defaultEnabled = True, flags = Gaffer.Plug.Flags.Default | Gaffer.Plug.Flags.Dynamic ) ) + + plug = GafferImage.OpenColorIOConfigPlug.acquireDefaultConfigPlug( script ) + plug["config"].setValue( self.openColorIOPath() / "context.ocio" ) + plug["variables"].addChild( Gaffer.NameValuePlug( "CDL", "${shot}", defaultEnabled = True, flags = Gaffer.Plug.Flags.Default | Gaffer.Plug.Flags.Dynamic ) ) + + with script.context() : + ocioConfig, ocioContext = GafferImage.OpenColorIOAlgo.currentConfigAndContext() + self.assertEqual( ocioContext["CDL"], "shot001" ) + hashes = { GafferImage.OpenColorIOAlgo.currentConfigAndContextHash() } + + script["variables"][0]["value"].setValue( "shot002" ) + + with script.context() : + ocioConfig, ocioContext = GafferImage.OpenColorIOAlgo.currentConfigAndContext() + self.assertEqual( ocioContext["CDL"], "shot002" ) + hashes.add( GafferImage.OpenColorIOAlgo.currentConfigAndContextHash() ) + self.assertEqual( len( hashes ), 2 ) + + with Gaffer.Context( script.context() ) as c : + c["shot"] = "shot003" + ocioConfig, ocioContext = GafferImage.OpenColorIOAlgo.currentConfigAndContext() + self.assertEqual( ocioContext["CDL"], "shot003" ) + hashes.add( GafferImage.OpenColorIOAlgo.currentConfigAndContextHash() ) + self.assertEqual( len( hashes ), 3 ) + if __name__ == "__main__": unittest.main() diff --git a/src/GafferImage/OpenColorIOAlgo.cpp b/src/GafferImage/OpenColorIOAlgo.cpp index f1f0b10d281..be694aefa96 100644 --- a/src/GafferImage/OpenColorIOAlgo.cpp +++ b/src/GafferImage/OpenColorIOAlgo.cpp @@ -161,18 +161,21 @@ std::vector GafferImage::OpenColorIOAlgo::variables( const Gaffer:: OCIO_NAMESPACE::ConstConfigRcPtr GafferImage::OpenColorIOAlgo::currentConfig() { const Gaffer::Context *context = Gaffer::Context::current(); - return g_configCache.get( getConfig( context ) ); + return g_configCache.get( context->substitute( getConfig( context ) ) ); } IECore::MurmurHash GafferImage::OpenColorIOAlgo::currentConfigHash() { - return Gaffer::Context::current()->variableHash( g_ocioConfigContextName ); + const Gaffer::Context *context = Gaffer::Context::current(); + IECore::MurmurHash result; + result.append( context->substitute( getConfig( context ) ) ); + return result; } GafferImage::OpenColorIOAlgo::ConfigAndContext GafferImage::OpenColorIOAlgo::currentConfigAndContext() { const Gaffer::Context *gafferContext = Gaffer::Context::current(); - OCIO_NAMESPACE::ConstConfigRcPtr config = g_configCache.get( getConfig( gafferContext ) ); + OCIO_NAMESPACE::ConstConfigRcPtr config = g_configCache.get( gafferContext->substitute( getConfig( gafferContext ) ) ); OCIO_NAMESPACE::ConstContextRcPtr context = config->getCurrentContext(); OCIO_NAMESPACE::ContextRcPtr mutableContext; @@ -198,9 +201,10 @@ GafferImage::OpenColorIOAlgo::ConfigAndContext GafferImage::OpenColorIOAlgo::cur context = mutableContext; } + const string value = gafferContext->substitute( gafferContext->get( n ) ); mutableContext->setStringVar( n.c_str() + g_ocioStringVarPrefix.size(), - gafferContext->get( n ).c_str() + value.c_str() ); } @@ -219,7 +223,7 @@ IECore::MurmurHash GafferImage::OpenColorIOAlgo::currentConfigAndContextHash() { if( n == g_ocioConfigContextName || boost::starts_with( n.string(), g_ocioStringVarPrefix ) ) { - result.append( context->variableHash( n ) ); + result.append( context->substitute( context->get( n ) ) ); } } return result; diff --git a/src/GafferImage/OpenColorIOConfigPlug.cpp b/src/GafferImage/OpenColorIOConfigPlug.cpp index 8f62e419b03..34e8821ed01 100644 --- a/src/GafferImage/OpenColorIOConfigPlug.cpp +++ b/src/GafferImage/OpenColorIOConfigPlug.cpp @@ -54,16 +54,6 @@ using namespace GafferImage; namespace { -string substitutedValue( const StringPlug *plug ) -{ - string result = plug->getValue(); - if( auto script = plug->ancestor() ) - { - result = script->context()->substitute( result ); - } - return result; -} - const IECore::InternedString g_defaultConfigPlugName( "openColorIO" ); } // namespace @@ -180,8 +170,8 @@ void OpenColorIOConfigPlug::plugSet( Gaffer::Plug *plug ) auto *scriptNode = parent(); assert( scriptNode ); - OpenColorIOAlgo::setConfig( scriptNode->context(), substitutedValue( configPlug() ) ); - OpenColorIOAlgo::setWorkingSpace( scriptNode->context(), substitutedValue( workingSpacePlug() ) ); + OpenColorIOAlgo::setConfig( scriptNode->context(), configPlug()->getValue() ); + OpenColorIOAlgo::setWorkingSpace( scriptNode->context(), workingSpacePlug()->getValue() ); // Add variables @@ -196,7 +186,7 @@ void OpenColorIOConfigPlug::plugSet( Gaffer::Plug *plug ) } } - const string name = substitutedValue( variable->namePlug() ); + const string name = scriptNode->context()->substitute( variable->namePlug()->getValue() ); if( name.empty() ) { continue; @@ -204,7 +194,7 @@ void OpenColorIOConfigPlug::plugSet( Gaffer::Plug *plug ) if( auto stringPlug = variable->valuePlug() ) { - OpenColorIOAlgo::addVariable( scriptNode->context(), name, substitutedValue( stringPlug ) ); + OpenColorIOAlgo::addVariable( scriptNode->context(), name, stringPlug->getValue() ); validVariables.insert( name ); } else