OpenColorIO : Defer expansion of context variables in OCIO vars

Previously, the OpenColorIOConfigPlug was baking in values from the current context at the point the OCIO plugs were edited on the ScriptNode. This meant that later changes to the context variable were not reflected in the OCIO variable. Among other things, this meant that the Viewer LUT would fail to
update when changing shot in certain multi-shot workflows.

Moving the variable expansion to `OpenColorIOAlgo::currentContextAndConfig()`
fixes this problem, but also enables more advanced workflows. For instance, a Wedge or ContactSheet node can now set the variable being referenced, and the OCIO config used by OpenColorIOTransform nodes will reflect that change dynamically as the graph executes.

There is one knock-on effect from this, as demonstrated in the test changes. On Windows, we must now use `/` separators in the context variables where before we could get away with `\`. We think it's unlikely that anyone is creating contexts manually like this in Windows - much more likely is usage of OpenColorIOContext and OpenColorIOConfigPlug where `/` has always been a requirement. So we think the very small risk of disruption is worth it for the benefits provided.

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
 ---

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" )
 

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__":

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 ) )

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 ) )

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()

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()

src/GafferImage/OpenColorIOAlgo.cpp

@@ -161,18 +161,21 @@ std::vector<std::string> 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<string>( n ) );
 		mutableContext->setStringVar(
 			n.c_str() + g_ocioStringVarPrefix.size(),
-			gafferContext->get<string>( 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<string>( n ) ) );
 		}
 	}
 	return result;

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<ScriptNode>() )
-	{
-		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<ScriptNode>();
 	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,15 +186,15 @@ 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;
 		}
 
 		if( auto stringPlug = variable->valuePlug<StringPlug>() )
 		{
-			OpenColorIOAlgo::addVariable( scriptNode->context(), name, substitutedValue( stringPlug ) );
+			OpenColorIOAlgo::addVariable( scriptNode->context(), name, stringPlug->getValue() );
 			validVariables.insert( name );
 		}
 		else