Merge pull request #6015 from johnhaddon/arrayPlugElements

ArrayPlug improvements

Author: johnhaddon

Changes.md

@@ -35,6 +35,10 @@ Fixes
   - Fixed update of custom context-sensitive labels on Dot nodes.
 - GafferCortexUI : Removed usage of legacy PlugValueWidget API.
 - Dispatcher : Fixed crashes caused by a dispatcher's `SetupPlugsFn` attempting to access the TaskNode it was being called for. Dispatchers may now introspect the TaskNode and add different plugs based on type (#915).
+- ArrayPlug :
+  - Fixed error when `resize()` removed plugs with input connections.
+  - Fixed error when `resize()` was used on an output plug.
+- CreateViews : Fixed redundant serialisation of internal connections.
 
 API
 ---
@@ -51,6 +55,9 @@ API
 - PathColumn :
   - Added `contextMenuSignal()`, allowing the creation of custom context menus.
   - Added `instanceCreatedSignal()`, providing an opportunity to connect to the signals on _any_ column, no matter how it is created.
+- ArrayPlug :
+  - It is now legal to construct an ArrayPlug with a minimum size of 0. Previously the minimum size was 1.
+  - Added `elementPrototype()` method.
 
 Breaking Changes
 ----------------
@@ -69,6 +76,9 @@ Breaking Changes
   - Deprecated `getContext()` methods. Use `context()` instead.
 - Loop : Removed `nextIterationContext()` method.
 - NodeGadget, ConnectionGadget : Removed `activeForFocusNode()` virtual methods. Override `updateFromContextTracker()` instead.
+- ArrayPlug :
+  - Renamed `element` constructor argument to `elementPrototype`.
+  - Deprecated the passing of `element = nullptr` to the constructor.
 
 1.4.x.x (relative to 1.4.11.0)
 =======

include/Gaffer/ArrayPlug.h

@@ -49,17 +49,17 @@ class GAFFER_API ArrayPlug : public Plug
 
 	public :
 
-		/// The element plug is used as the first array element,
-		/// and all new array elements are created by calling
-		/// element->createCounterpart(). Currently the element
-		/// names are derived from the name of the first element,
-		/// but this may change in the future. It is strongly
-		/// recommended that ArrayPlug children are only accessed
-		/// through numeric indexing and never via names.
+		/// All array elements are created by calling
+		/// `elementPrototype->createCounterpart()`. Currently the element names
+		/// are derived from the name of the prototype, but this may change in
+		/// the future. It is strongly recommended that ArrayPlug children are
+		/// only accessed through numeric indexing and never via names.
 		explicit ArrayPlug(
 			const std::string &name = defaultName<ArrayPlug>(),
 			Direction direction = In,
-			PlugPtr element = nullptr,
+			/// > Caution : `elementPrototype` should not be null. It only defaults
+			/// > that way to support the loading of legacy serialisations.
+			ConstPlugPtr elementPrototype = nullptr,
 			size_t minSize = 1,
 			size_t maxSize = std::numeric_limits<size_t>::max(),
 			unsigned flags = Default,
@@ -75,8 +75,10 @@ class GAFFER_API ArrayPlug : public Plug
 		void setInput( PlugPtr input ) override;
 		PlugPtr createCounterpart( const std::string &name, Direction direction ) const override;
 
+		const Plug *elementPrototype() const;
 		size_t minSize() const;
 		size_t maxSize() const;
+		/// Resizes the array. This should be preferred to `addChild()`.
 		void resize( size_t size );
 		bool resizeWhenInputsChange() const;
 		/// Returns an unconnected element at the end of the array, adding one
@@ -91,7 +93,9 @@ class GAFFER_API ArrayPlug : public Plug
 	private :
 
 		void inputChanged( Gaffer::Plug *plug );
+		void childAdded();
 
+		ConstPlugPtr m_elementPrototype;
 		size_t m_minSize;
 		size_t m_maxSize;
 		bool m_resizeWhenInputsChange;

python/GafferDispatchTest/DebugDispatcher.py

@@ -111,7 +111,7 @@ def __createNode( name ) :
 		node = Gaffer.Node()
 		node.setName( name.replace( ".", "_" ) )
 
-		node["preTasks"] = Gaffer.ArrayPlug( element = Gaffer.Plug( "preTask0" ), flags = Gaffer.Plug.Flags.Default | Gaffer.Plug.Flags.Dynamic )
+		node["preTasks"] = Gaffer.ArrayPlug( elementPrototype = Gaffer.Plug( "preTask0" ), flags = Gaffer.Plug.Flags.Default | Gaffer.Plug.Flags.Dynamic )
 		node["task"] = Gaffer.Plug( direction = Gaffer.Plug.Direction.Out, flags = Gaffer.Plug.Flags.Default | Gaffer.Plug.Flags.Dynamic )
 		node["node"] = Gaffer.StringPlug( flags = Gaffer.Plug.Flags.Default | Gaffer.Plug.Flags.Dynamic )
 		node["frames"] = Gaffer.StringPlug( flags = Gaffer.Plug.Flags.Default | Gaffer.Plug.Flags.Dynamic )

python/GafferImageTest/AnaglyphTest.py

@@ -67,10 +67,11 @@ def test( self ) :
 		right['transform']['translate'].setValue( imath.V2f( 10, 0 ) )
 
 		createViews = GafferImage.CreateViews()
-		createViews["views"].addChild( Gaffer.NameValuePlug( "left", GafferImage.ImagePlug(), True, "view0", Gaffer.Plug.Flags.Default | Gaffer.Plug.Flags.Dynamic ) )
-		createViews["views"].addChild( Gaffer.NameValuePlug( "right", GafferImage.ImagePlug(), True, "view1", Gaffer.Plug.Flags.Default | Gaffer.Plug.Flags.Dynamic ) )
+		createViews["views"].resize( 2 )
 		createViews["views"][0]["value"].setInput( left["out"] )
+		createViews["views"][0]["name"].setValue( "left" )
 		createViews["views"][1]["value"].setInput( right["out"] )
+		createViews["views"][1]["name"].setValue( "right" )
 
 		anaglyph = GafferImage.Anaglyph()
 		anaglyph["in"].setInput( createViews["out"] )

python/GafferImageTest/CatalogueTest.py

@@ -917,10 +917,11 @@ def testGenerateFileName( self ):
 
 		# Check that two multi-view images match only if all views are identical
 		createViews = GafferImage.CreateViews()
-		createViews["views"].addChild( Gaffer.NameValuePlug( "left", GafferImage.ImagePlug(), True, "view0", Gaffer.Plug.Flags.Default | Gaffer.Plug.Flags.Dynamic ) )
-		createViews["views"].addChild( Gaffer.NameValuePlug( "right", GafferImage.ImagePlug(), True, "view1", Gaffer.Plug.Flags.Default | Gaffer.Plug.Flags.Dynamic ) )
+		createViews["views"].resize( 2 )
 		createViews["views"][0]["value"].setInput( constant1["out"] )
+		createViews["views"][0]["name"].setValue( "left" )
 		createViews["views"][1]["value"].setInput( constant2["out"] )
+		createViews["views"][1]["name"].setValue( "right" )
 
 		f3 = catalogue.generateFileName( createViews["out"] )
 		self.assertNotIn( f3, [f1, f2] )

python/GafferImageTest/ContactSheetCoreTest.py

@@ -124,8 +124,9 @@ def testNoInvalidViewAccesses( self ) :
 
 		checker = GafferImage.Checkerboard()
 		createViews = GafferImage.CreateViews()
-		createViews["views"].addChild( Gaffer.NameValuePlug( "left", GafferImage.ImagePlug(), True, "left", Gaffer.Plug.Flags.Default | Gaffer.Plug.Flags.Dynamic ) )
+		createViews["views"].resize( 1 )
 		createViews["views"][0]["value"].setInput( checker["out"] )
+		createViews["views"][0]["name"].setValue( "left" )
 		self.assertEqual( createViews["out"].viewNames(), IECore.StringVectorData( [ "left" ] ) )
 
 		contactSheet = GafferImage.ContactSheetCore()

python/GafferImageTest/CopyViewsTest.py

@@ -66,8 +66,9 @@ def test( self ) :
 					name = "source%iview%i" % ( i, j )
 
 				createViews[i].addChild( constant )
-				createViews[i]["views"].addChild( Gaffer.NameValuePlug( name, GafferImage.ImagePlug(), True, "view0", Gaffer.Plug.Flags.Default | Gaffer.Plug.Flags.Dynamic ) )
-				createViews[i]["views"][-1]["value"].setInput( constant["out"] )
+				view = createViews[i]["views"].next()
+				view["name"].setValue( name )
+				view["value"].setInput( constant["out"] )
 
 		copyViews = GafferImage.CopyViews()
 		copyViews["in"][0].setInput( createViews[0]["out"] )

python/GafferImageTest/CreateViewsTest.py

@@ -37,12 +37,11 @@
 import unittest
 import imath
 import inspect
-import os
+import pathlib
 
 import IECore
 
 import Gaffer
-import GafferTest
 import GafferImage
 import GafferImageTest
 
@@ -58,8 +57,9 @@ def test( self ) :
 		script.addChild( createViews )
 
 		# Default views added by the UI
-		createViews["views"].addChild( Gaffer.NameValuePlug( "left", GafferImage.ImagePlug(), True, "view0", Gaffer.Plug.Flags.Default | Gaffer.Plug.Flags.Dynamic ) )
-		createViews["views"].addChild( Gaffer.NameValuePlug( "right", GafferImage.ImagePlug(), True, "view1", Gaffer.Plug.Flags.Default | Gaffer.Plug.Flags.Dynamic ) )
+		createViews["views"].resize( 2 )
+		createViews["views"][0]["name"].setValue( "left" )
+		createViews["views"][1]["name"].setValue( "right" )
 
 		reader = GafferImage.ImageReader()
 		script.addChild( reader )
@@ -93,9 +93,9 @@ def test( self ) :
 		self.assertImagesEqual( createViews["out"], deserialise["CreateViews"]["out"] )
 
 
-		createViews["views"].addChild( Gaffer.NameValuePlug( "custom", GafferImage.ImagePlug(), True, "view1", Gaffer.Plug.Flags.Default | Gaffer.Plug.Flags.Dynamic ) )
-		createViews["views"]["view2"]["name"].setValue( "blah" )
-		createViews["views"]["view2"]["value"].setInput( constant2["out"] )
+		createViews["views"].resize( 3 )
+		createViews["views"][2]["name"].setValue( "blah" )
+		createViews["views"][2]["value"].setInput( constant2["out"] )
 
 		self.assertEqual( createViews["out"].viewNames(), IECore.StringVectorData( [ "left", "right", "blah" ] ) )
 		self.assertEqual(
@@ -194,8 +194,9 @@ def testInputToExpressionDrivingEnabledPlug( self ) :
 
 		# `default` view with RGBA channels, and `notDefault` view with no channels
 		script["createViews"] = GafferImage.CreateViews()
-		script["createViews"]["views"].addChild( Gaffer.NameValuePlug( "default", GafferImage.ImagePlug(), True, "view0", Gaffer.Plug.Flags.Default | Gaffer.Plug.Flags.Dynamic ) )
-		script["createViews"]["views"].addChild( Gaffer.NameValuePlug( "notDefault", GafferImage.ImagePlug(), True, "view1", Gaffer.Plug.Flags.Default | Gaffer.Plug.Flags.Dynamic ) )
+		script["createViews"]["views"].resize( 2 )
+		script["createViews"]["views"][0]["name"].setValue( "default" )
+		script["createViews"]["views"][1]["name"].setValue( "notDefault" )
 		script["createViews"]["views"][0]["value"].setInput( script["checkerboard"]["out"] )
 		self.assertEqual( script["createViews"]["out"].channelNames( "default" ), IECore.StringVectorData( [ "R", "G", "B", "A" ] ) )
 		self.assertEqual( script["createViews"]["out"].channelNames( "notDefault" ), IECore.StringVectorData() )
@@ -224,5 +225,32 @@ def testInputToExpressionDrivingEnabledPlug( self ) :
 			# for this particular view.
 			self.assertFalse( script["constant"]["enabled"].getValue() )
 
+	def testNoRedundantSerialisation( self ) :
+
+		script = Gaffer.ScriptNode()
+		script["createViews"] = GafferImage.CreateViews()
+		self.assertNotIn( "setInput", script.serialise() )
+
+	def testLoadFromVersion1_4( self ) :
+
+		script = Gaffer.ScriptNode()
+		script["fileName"].setValue( pathlib.Path( __file__ ).parent / "scripts" / "createViews-1.4.10.0.gfr" )
+		script.load()
+
+		def assertLoadedOK( script ) :
+
+			self.assertEqual( len( script["CreateViews"]["views"] ), 2 )
+			self.assertEqual( script["CreateViews"]["views"][0]["name"].getValue(), "left" )
+			self.assertEqual( script["CreateViews"]["views"][1]["name"].getValue(), "right" )
+			self.assertEqual( script["CreateViews"]["views"][0]["value"].getInput(), script["CheckerboardLeft"]["out"] )
+			self.assertEqual( script["CreateViews"]["views"][1]["value"].getInput(), script["CheckerboardRight"]["out"] )
+
+		assertLoadedOK( script )
+
+		script2 = Gaffer.ScriptNode()
+		script2.execute( script.serialise() )
+
+		assertLoadedOK( script2 )
+
 if __name__ == "__main__":
 	unittest.main()

python/GafferImageTest/DeleteViewsTest.py

@@ -63,9 +63,10 @@ def test( self ) :
 
 		createViews = GafferImage.CreateViews()
 
-		createViews["views"].addChild( Gaffer.NameValuePlug( "left", GafferImage.ImagePlug(), True, "view0", Gaffer.Plug.Flags.Default | Gaffer.Plug.Flags.Dynamic ) )
-		createViews["views"].addChild( Gaffer.NameValuePlug( "right", GafferImage.ImagePlug(), True, "view1", Gaffer.Plug.Flags.Default | Gaffer.Plug.Flags.Dynamic ) )
-		createViews["views"].addChild( Gaffer.NameValuePlug( "default", GafferImage.ImagePlug(), True, "view2", Gaffer.Plug.Flags.Default | Gaffer.Plug.Flags.Dynamic ) )
+		createViews["views"].resize( 3 )
+		createViews["views"][0]["name"].setValue( "left" )
+		createViews["views"][1]["name"].setValue( "right" )
+		createViews["views"][2]["name"].setValue( "default" )
 
 		createViews["views"]["view0"]["value"].setInput( reader["out"] )
 		createViews["views"]["view1"]["value"].setInput( constant1["out"] )

python/GafferImageTest/FormatQueryTest.py

@@ -99,8 +99,9 @@ def testView( self ) :
 		reader["fileName"].setValue( self.imagesPath() / "checkerboard.100x100.exr" )
 
 		views = GafferImage.CreateViews()
-		views["views"].addChild( Gaffer.NameValuePlug( "left", GafferImage.ImagePlug(), True ) )
-		views["views"].addChild( Gaffer.NameValuePlug( "right", GafferImage.ImagePlug(), True ) )
+		views["views"].resize( 2 )
+		views["views"][0]["name"].setValue( "left" )
+		views["views"][1]["name"].setValue( "right" )
 		views["views"][0]["value"].setInput( constantSource["out"] )
 		views["views"][1]["value"].setInput( reader["out"] )