ImagePlug : Allow serialisation of child connections/values

This means we also need to disable serialisation for `ImageProcessor.out` to avoid unwanted serialisation of internal connections. This _could_ have been achieved by removing the Serialisable flag, but that caused problems for things like `Dot::setup()`, which _do_ force the added plug to be serialisable, but _not_ its children. We could have modified all those locations to set flags recursively, but since we want to remove flags entirely in future it makes more sense to use a custom serialiser instead. The Dot setup problem demonstrates the problem with flags in the first place - they get copied from place to place, but they may only have made sense on the node they originated from.

In conjunction with the previous commit, this fixes #3986.

Changes.md

@@ -48,7 +48,7 @@ Improvements
   - Added `importanceSampleFilter` plug to DelightOptions, providing denoiser-compatible output.
   - Matched DelightOptions default values for `oversampling` and `shadingSamples` to 3Delight's own default values.
 - GraphEditor : Improved logic used to connect a newly created node to the selected nodes.
-- ScenePlug : Child plugs are now serialisable. Among other things, this enables them to be driven by expressions.
+- ScenePlug, ImagePlug : Child plugs are now serialisable. Among other things, this enables them to be driven by expressions (#3986).
 
 Fixes
 -----
@@ -134,6 +134,7 @@ Breaking Changes
 - OSLShader : Output parameters are now loaded onto the `out` plug for all types (`surface`, `displacement` etc), not just `shader`.
 - DelightOptions : Changed default values for `oversampling` and `shadingSamples` plugs.
 - SceneProcessor : Subclasses no longer serialise internal connections to the `out` plug.
+- ImageProcessor : Internal connections to the `out` plug are no longer serialised.
 
 Build
 -----

python/GafferImageTest/ImageNodeTest.py

@@ -34,7 +34,7 @@
 #
 ##########################################################################
 
-import os
+import inspect
 import unittest
 import threading
 import imath
@@ -106,6 +106,44 @@ def testNodesConstructWithDefaultValues( self ) :
 		self.assertNodesConstructWithDefaultValues( GafferImage )
 		self.assertNodesConstructWithDefaultValues( GafferImageTest )
 
+	def testMetadataExpression( self ) :
+
+		script = Gaffer.ScriptNode()
+
+		script["checker"] = GafferImage.Checkerboard()
+
+		script["metadata"] = GafferImage.ImageMetadata()
+		script["metadata"]["in"].setInput( script["checker"]["out"] )
+		script["metadata"]["metadata"].addChild( Gaffer.NameValuePlug( "test", "testValue", flags = Gaffer.Plug.Flags.Default | Gaffer.Plug.Flags.Dynamic ) )
+
+		script["dot"] = Gaffer.Dot()
+		script["dot"].setup( script["metadata"]["out"] )
+		script["dot"]["in"].setInput( script["metadata"]["out"] )
+
+		script["expression"] = Gaffer.Expression()
+		script["expression"].setExpression( inspect.cleandoc(
+			"""
+			m = parent["metadata"]["out"]["metadata"]
+			m["test"] = m["test"].value + "Modified"
+			parent["dot"]["in"]["metadata"] = m
+			"""
+		) )
+
+		def assertExpectedImage( image ) :
+
+			self.assertImagesEqual( image, script["metadata"]["out"], ignoreMetadata = True )
+			self.assertEqual( image.metadata()["test"], IECore.StringData( "testValueModified" ) )
+
+		assertExpectedImage( script["dot"]["out"] )
+
+		script2 = Gaffer.ScriptNode()
+		script2.execute( script.serialise() )
+
+		print( script2["dot"]["out"]["dataWindow"].source().fullName() )
+
+		assertExpectedImage( script2["dot"]["out"] )
+		self.assertEqual( script2["expression"].getExpression(), script["expression"].getExpression() )
+
 	def setUp( self ) :
 
 		GafferImageTest.ImageTestCase.setUp( self )

src/GafferImage/ImagePlug.cpp

@@ -77,11 +77,7 @@ ImagePlug::ImagePlug( const std::string &name, Direction direction, unsigned fla
 {
 	storeIndexOfNextChild( g_firstPlugIndex );
 
-	// 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 StringVectorDataPlug(

src/GafferImage/Shape.cpp

@@ -112,7 +112,6 @@ Shape::Shape( const std::string &name )
 	merge->operationPlug()->setValue( Merge::Over );
 
 	outPlug()->setInput( merge->outPlug() );
-	outPlug()->setFlags( Gaffer::Plug::Serialisable, false );
 }
 
 Shape::~Shape()

src/GafferImageModule/ImageProcessorBinding.cpp

@@ -57,6 +57,29 @@ using namespace Gaffer;
 using namespace GafferBindings;
 using namespace GafferImage;
 
+namespace
+{
+
+class ImageProcessorSerialiser : public GafferBindings::NodeSerialiser
+{
+
+	public :
+
+		bool childNeedsSerialisation( const Gaffer::GraphComponent *child, const Serialisation &serialisation ) const override
+		{
+			auto imageProcessor = static_cast<const ImageProcessor *>( child->parent() );
+			if( child == imageProcessor->outPlug() )
+			{
+				return false;
+			}
+
+			return NodeSerialiser::childNeedsSerialisation( child, serialisation );
+		}
+
+};
+
+} // namespace
+
 void GafferImageModule::bindImageProcessor()
 {
 
@@ -72,6 +95,8 @@ void GafferImageModule::bindImageProcessor()
 		)
 	;
 
+	Serialisation::registerSerialiser( ImageProcessor::staticTypeId(), new ImageProcessorSerialiser );
+
 	GafferBindings::DependencyNodeClass<FlatImageProcessor>();
 
 	DependencyNodeClass<CollectImages>();