ScenePlug : Allow serialisation of child connections/values

This implements the ScenePlug half of GafferHQ#3986.

Changes.md

@@ -39,6 +39,7 @@ Improvements
 - 3Delight :
   - Added camera overscan support.
   - NSI scene description export format is now based on file extension - `.nsi` for binary and `.nsia` for ASCII.
+- ScenePlug : Child plugs are now serialisable. Among other things, this enables them to be driven by expressions.
 
 Fixes
 -----
@@ -119,6 +120,7 @@ Breaking Changes
 - ShuffleUI : Removed `nodeMenuCreateCommand()`.
 - ImageStatsUI : Removed `postCreate()`.
 - 3Delight : Changed NSI scene description export with `.nsi` file extension from ASCII to binary (`.nsia` is used for ASCII now).
+- SceneProcessor : Subclasses no longer serialise internal connections to the `out` plug.
 
 Build
 -----

python/GafferSceneTest/SceneNodeTest.py

@@ -452,6 +452,47 @@ def testChildBoundsCancellation( self ) :
 			# cleanly if it has been fixed.
 			backgroundTask.cancelAndWait()
 
+	def testGlobalsExpression( self ) :
+
+		script = Gaffer.ScriptNode()
+
+		script["plane"] = GafferScene.Plane()
+
+		script["outputs"] = GafferScene.Outputs()
+		script["outputs"]["in"].setInput( script["plane"]["out"] )
+		script["outputs"].addOutput( "test", IECoreScene.Output( "test.exr", "exr", "rgba" ) )
+
+		script["dot"] = Gaffer.Dot()
+		script["dot"].setup( script["outputs"]["out"] )
+		script["dot"]["in"].setInput( script["outputs"]["out"] )
+
+		script["expression"] = Gaffer.Expression()
+		script["expression"].setExpression( inspect.cleandoc(
+			"""
+			g = parent["outputs"]["out"]["globals"]
+
+			for key in g.keys() :
+				if not key.startswith( "output:" ) :
+					continue
+				g[key].parameters()["myParameter"] = 10
+
+			parent["dot"]["in"]["globals"] = g
+			"""
+		) )
+
+		def assertExpectedScene( scene ) :
+
+			self.assertScenesEqual( scene, script["plane"]["out"], checks = self.allSceneChecks - { "globals" } )
+			self.assertEqual( scene.globals()["output:test"].parameters()["myParameter"], IECore.IntData( 10 ) )
+
+		assertExpectedScene( script["dot"]["out"] )
+
+		script2 = Gaffer.ScriptNode()
+		script2.execute( script.serialise() )
+
+		assertExpectedScene( script2["dot"]["out"] )
+		self.assertEqual( script2["expression"].getExpression(), script["expression"].getExpression() )
+
 	def setUp( self ) :
 
 		GafferSceneTest.SceneTestCase.setUp( self )

src/GafferScene/ScenePlug.cpp

@@ -61,11 +61,7 @@ static ContextAlgo::GlobalScope::Registration g_globalScopeRegistration(
 ScenePlug::ScenePlug( const std::string &name, Direction direction, unsigned flags )
 	:	ValuePlug( name, direction, flags )
 {
-	// we don't want the children to be serialised in any way - we always create
-	// them ourselves in this constructor so they aren't Dynamic, and we don't ever
-	// want to store their values because they are meaningless without an input
-	// connection, so they aren't Serialisable either.
-	unsigned childFlags = flags & ~(Dynamic | Serialisable);
+	const unsigned childFlags = flags & ~Dynamic;
 
 	addChild(
 		new AtomicBox3fPlug(

src/GafferSceneModule/CoreBinding.cpp

@@ -303,15 +303,23 @@ class SceneProcessorSerialiser : public NodeSerialiser
 
 	bool childNeedsSerialisation( const Gaffer::GraphComponent *child, const Serialisation &serialisation ) const override
 	{
-		if( child->parent()->typeId() == SceneProcessor::staticTypeId() )
+		const auto sceneProcessor = static_cast<const SceneProcessor *>( child->parent() );
+		const bool isSubclassed = sceneProcessor->typeId() != SceneProcessor::staticTypeId();
+
+		if( !isSubclassed && runTimeCast<const Node>( child ) )
 		{
-			// Parent is exactly a SceneProcessor, not a subclass. Since we don't add
-			// any nodes in the constructor, we know that any nodes added subsequently
-			// will need manual serialisation.
-			if( runTimeCast<const Node>( child ) )
-			{
-				return true;
-			}
+			// SceneProcessor doesn't add any nodes in the constructor, so we
+			// know that any nodes added subsequently will need manual
+			// serialisation.
+			return true;
+		}
+
+		if( isSubclassed && child == sceneProcessor->outPlug() )
+		{
+			// Internal connections shouldn't be serialised for custom node
+			// types, because that leaks their implementation, making it
+			// harder to change internals in future.
+			return false;
 		}
 
 		return NodeSerialiser::childNeedsSerialisation( child, serialisation );