Merge pull request #5896 from GafferHQ/pathBasedMetadataOrder

Metadata : Allow path-based registrations on nodes to override plug registrations

Changes.md

@@ -7,6 +7,8 @@ Improvements
 - CameraTweaks : Added `ignoreMissing` plug to align behaviour with the other Tweaks nodes.
 - AttributeTweaks : The `{source}` substitution for `linkedLights` now expands to `defaultLights` if the attribute doesn't exist yet. This makes tweaks such as `({source}) - unwantedLights` reliable even if no light links have been authored yet.
 - ImageReader : Non-standard "r", "g", "b" and "a" channel names are now automatically renamed to "R", "G", "B" and "A" on loading. As with other heuristics, this can be disabled by setting `channelInterpretation` to "EXR Specification".
+- Metadata : Metadata registered to a node or plug targeting a descendant plug will now override metadata registered locally to the target.
+- OptionTweaks, ContextVariableTweaks : Added `Remove` mode.
 
 Breaking Changes
 ----------------
@@ -15,6 +17,8 @@ Breaking Changes
 - TweakPlug : Remove deprecated `MissingMode::IgnoreOrReplace`.
 - AttributeTweaks : `Replace` mode no longer errors if the `linkedLights` attribute doesn't exist.
 - ImageReader : Changed handling of lower-cased "r", "g", "b" and "a" channels.
+- Metadata : Path based registrations to a Node or Plug now override equivalent registrations on its descendants.
+- TweakPlugValueWidget : Removed support for `tweakPlugValueWidget:allowCreate` and `tweakPlugValueWidget:allowRemove` metadata.
 
 1.4.x.x (relative to 1.4.7.0)
 =======

python/GafferSceneUI/AttributeTweaksUI.py

@@ -97,8 +97,6 @@
 
 		"tweaks.*" : [
 
-			"tweakPlugValueWidget:allowCreate", True,
-			"tweakPlugValueWidget:allowRemove", True,
 			"tweakPlugValueWidget:propertyType", "attribute",
 
 		],

python/GafferSceneUI/CameraTweaksUI.py

@@ -97,8 +97,6 @@
 
 		"tweaks.*" : [
 
-			"tweakPlugValueWidget:allowRemove", True,
-			"tweakPlugValueWidget:allowCreate", True,
 			"tweakPlugValueWidget:propertyType", "parameter",
 
 		],

python/GafferSceneUI/OptionTweaksUI.py

@@ -119,7 +119,6 @@ def __optionMetadataPresetValues( plug ) :
 
 		"tweaks.*" : [
 
-			"tweakPlugValueWidget:allowCreate", True,
 			"tweakPlugValueWidget:propertyType", "option",
 
 		],

python/GafferSceneUI/ShaderTweaksUI.py

@@ -126,8 +126,6 @@
 		"tweaks.*" : [
 
 			"noduleLayout:visible", False, # Can be shown individually using PlugAdder above
-			"tweakPlugValueWidget:allowCreate", True,
-			"tweakPlugValueWidget:allowRemove", True,
 			"tweakPlugValueWidget:propertyType", "parameter",
 			"plugValueWidget:type", "GafferSceneUI.ShaderTweaksUI._ShaderTweakPlugValueWidget",
 

python/GafferTest/MetadataTest.py

@@ -108,6 +108,80 @@ def testInheritance( self ) :
 		Gaffer.Metadata.registerValue( self.DerivedAddNode, "op1", "iKey", "Derived class plug value" )
 		self.assertEqual( Gaffer.Metadata.value( derivedAdd["op1"], "iKey" ), "Derived class plug value" )
 
+	def testNodeMetadataHasPriorityOverRowsPlugMetadata( self ) :
+
+		Gaffer.Metadata.registerValue( Gaffer.Spreadsheet.RowsPlug, "default.*...", "test", "plug" )
+
+		class MetadataTestNodeF( Gaffer.Node ) :
+
+			def __init__( self, name = "MetadataTestNodeF" ) :
+
+				Gaffer.Node.__init__( self, name )
+
+				self["a"] = Gaffer.Spreadsheet.RowsPlug()
+				self["a"].addColumn( Gaffer.StringPlug( "testColumn" ) )
+				self["b"] = Gaffer.Spreadsheet.RowsPlug()
+				self["b"].addColumn( Gaffer.StringPlug( "testColumn" ) )
+
+		IECore.registerRunTimeTyped( MetadataTestNodeF )
+
+		Gaffer.Metadata.registerNode(
+
+			MetadataTestNodeF,
+
+			plugs = {
+				"a.default.cells.testColumn.value" : [
+					"test", "node",
+				]
+			}
+
+		)
+
+		n = MetadataTestNodeF()
+		self.assertEqual( Gaffer.Metadata.value( n["a"]["default"]["cells"]["testColumn"]["value"], "test" ), "node" )
+		self.assertEqual( Gaffer.Metadata.value( n["b"]["default"]["cells"]["testColumn"]["value"], "test" ), "plug" )
+
+	def testAncestorMetadataPriority( self ) :
+
+		Gaffer.Metadata.registerValue( Gaffer.Color3fPlug, "[rgb]", "test", "color3fPlug" )
+		Gaffer.Metadata.registerValue( Gaffer.TweakPlug, "value.[rg]", "test", "tweakPlug" )
+
+		class MetadataTestNodeG( Gaffer.Node ) :
+
+			def __init__( self, name = "MetadataTestNodeG" ) :
+
+				Gaffer.Node.__init__( self, name )
+
+				self["a"] = Gaffer.TweakPlug( "a", imath.Color3f( 0 ) )
+				self["b"] = Gaffer.TweakPlug( "b", imath.Color3f( 0 ) )
+
+		IECore.registerRunTimeTyped( MetadataTestNodeG )
+
+		Gaffer.Metadata.registerNode(
+
+			MetadataTestNodeG,
+
+			plugs = {
+				"a.value.r" : [
+					"test", "node",
+				],
+
+				"b.value.[gb]" : [
+					"test", "node",
+				],
+			}
+
+		)
+
+		n = MetadataTestNodeG()
+		self.assertEqual( Gaffer.Metadata.value( n["a"]["value"]["r"], "test" ), "node" )
+		self.assertEqual( Gaffer.Metadata.value( n["a"]["value"]["g"], "test" ), "tweakPlug" )
+		self.assertEqual( Gaffer.Metadata.value( n["a"]["value"]["b"], "test" ), "color3fPlug" )
+
+		self.assertEqual( Gaffer.Metadata.value( n["b"]["value"]["r"], "test" ), "tweakPlug" )
+		self.assertEqual( Gaffer.Metadata.value( n["b"]["value"]["g"], "test" ), "node" )
+		self.assertEqual( Gaffer.Metadata.value( n["b"]["value"]["b"], "test" ), "node" )
+
 	def testNodeSignals( self ) :
 
 		add = GafferTest.AddNode()
@@ -1303,5 +1377,9 @@ def tearDown( self ) :
 		Gaffer.Metadata.deregisterValue( GafferTest.AddNode, "op1", "rp" )
 		Gaffer.Metadata.deregisterValue( GafferTest.AddNode, "op*", "aKey" )
 
+		Gaffer.Metadata.deregisterValue( Gaffer.Spreadsheet.RowsPlug, "default.*...", "test" )
+		Gaffer.Metadata.deregisterValue( Gaffer.Color3fPlug, "[rgb]", "test" )
+		Gaffer.Metadata.deregisterValue( Gaffer.TweakPlug, "value.[rg]", "test" )
+
 if __name__ == "__main__":
 	unittest.main()

python/GafferUI/ContextVariableTweaksUI.py

@@ -84,7 +84,6 @@
 
 		"tweaks.*" : [
 
-			"tweakPlugValueWidget:allowCreate", True,
 			"tweakPlugValueWidget:propertyType", "context variable",
 
 		]

python/GafferUI/SpreadsheetUI/_Metadata.py

@@ -279,8 +279,6 @@ def __forwardedMetadata( plug, key ) :
 	"spreadsheet:columnWidth",
 	"plugValueWidget:type",
 	"presetsPlugValueWidget:allowCustom",
-	"tweakPlugValueWidget:allowRemove",
-	"tweakPlugValueWidget:allowCreate",
 ] :
 
 	Gaffer.Metadata.registerValue(

python/GafferUI/TweakPlugValueWidget.py

@@ -47,11 +47,7 @@
 
 # Widget for TweakPlug, which is used to build tweak nodes such as ShaderTweaks
 # and CameraTweaks.  Shows a value plug that you can use to specify a tweak value, along with
-# a target parameter name, an enabled plug, and a mode.  The mode can be "Replace",
-# or "Add"/"Subtract"/"Multiply"/"Min"/"Max" if the plug is numeric,
-# or "ListAppend"/"ListPrepend"/"ListRemove" if the plug is a list or `PathMatcherPlug`,
-# or "Remove" if the metadata "tweakPlugValueWidget:allowRemove" is set,
-# or "Create" and "CreateIfMissing" if the metadata "tweakPlugValueWidget:allowCreate" is set.
+# a target parameter name, an enabled plug, and a mode.
 class TweakPlugValueWidget( GafferUI.PlugValueWidget ) :
 
 	def __init__( self, plugs ) :
@@ -150,11 +146,20 @@ def __childPlugs( plugs, childName ) :
 # Metadata
 
 Gaffer.Metadata.registerValue( Gaffer.TweakPlug, "deletable", lambda plug : plug.getFlags( Gaffer.Plug.Flags.Dynamic ) )
+Gaffer.Metadata.registerValue( Gaffer.TweakPlug, "tweakPlugValueWidget:propertyType:promotable", False )
+
+def __propertyType( plug ) :
+
+	source = Gaffer.PlugAlgo.findDestination(
+		plug,
+		lambda p : p if Gaffer.Metadata.value( p.parent(), "tweakPlugValueWidget:propertyType" ) else None
+	) or plug
+
+	return Gaffer.Metadata.value( source.parent(), "tweakPlugValueWidget:propertyType" ) or "property"
 
 def __nameDescription( plug ) :
 
-	property = Gaffer.Metadata.value( plug.parent(), "tweakPlugValueWidget:propertyType" ) or "property"
-	return f"The name of the {property} to apply the tweak to."
+	return f"The name of the {__propertyType( plug )} to apply the tweak to."
 
 Gaffer.Metadata.registerValue( Gaffer.TweakPlug, "name", "description", __nameDescription )
 
@@ -165,11 +170,13 @@ def __nameDescription( plug ) :
 
 def __validModes( plug ) :
 
-	result = []
-	if Gaffer.Metadata.value( plug.parent(), "tweakPlugValueWidget:allowCreate" ) :
-		result += [ Gaffer.TweakPlug.Mode.Create, Gaffer.TweakPlug.Mode.CreateIfMissing ]
+	result = [
+		Gaffer.TweakPlug.Mode.Create,
+		Gaffer.TweakPlug.Mode.CreateIfMissing,
+		Gaffer.TweakPlug.Mode.Replace,
+		Gaffer.TweakPlug.Mode.Remove,
+	]
 
-	result += [ Gaffer.TweakPlug.Mode.Replace ]
 	if hasattr( plug.parent()["value"], "hasMinValue" ) :
 		result += [
 			Gaffer.TweakPlug.Mode.Add,
@@ -201,9 +208,6 @@ def __validModes( plug ) :
 			Gaffer.TweakPlug.Mode.ListRemove
 		]
 
-	if Gaffer.Metadata.value( plug.parent(), "tweakPlugValueWidget:allowRemove" ) :
-		result += [ Gaffer.TweakPlug.Mode.Remove ]
-
 	return result
 
 Gaffer.Metadata.registerValue(
@@ -285,7 +289,7 @@ def __validModes( plug ) :
 
 def __modeDescription( plug ) :
 
-	property = Gaffer.Metadata.value( plug.parent(), "tweakPlugValueWidget:propertyType" ) or "property"
+	property = __propertyType( plug )
 
 	result =  "| Mode | Description |\n"
 	result += "| :--- | :---------- |\n"

src/Gaffer/Metadata.cpp

@@ -805,13 +805,19 @@ IECore::ConstDataPtr Metadata::valueInternal( const GraphComponent *target, IECo
 
 	// If the target is a plug, then look for a path-based
 	// value. These are more specific than type-based values.
+	// We allow metadata registered to higher-level components
+	// such as Nodes to take precedence over registrations to
+	// lower-level components such as Plugs, as a node may have
+	// specific needs for the presentation or behaviour of its
+	// plugs and thus have good reason to override their metadata.
 
 	if( registrationTypes & RegistrationTypes::TypeIdDescendant )
 	{
 		if( const Plug *plug = runTimeCast<const Plug>( target ) )
 		{
 			const GraphComponent *ancestor = plug->parent();
 			vector<InternedString> plugPath( { plug->getName() } );
+			Metadata::PlugValueFunction valueFn;
 			while( ancestor )
 			{
 				IECore::TypeId typeId = ancestor->typeId();
@@ -828,9 +834,11 @@ IECore::ConstDataPtr Metadata::valueInternal( const GraphComponent *target, IECo
 							auto vIt = it->second.find( key );
 							if( vIt != it->second.end() )
 							{
-								return vIt->second( plug );
+								valueFn = vIt->second;
+								break;
 							}
 						}
+
 						// And only if the direct lookup fails, do a full search using
 						// wildcard matches.
 						for( it = nIt->second.plugPathsToValues.begin(); it != eIt; ++it )
@@ -840,17 +848,28 @@ IECore::ConstDataPtr Metadata::valueInternal( const GraphComponent *target, IECo
 								auto vIt = it->second.find( key );
 								if( vIt != it->second.end() )
 								{
-									return vIt->second( plug );
+									valueFn = vIt->second;
+									break;
 								}
 							}
 						}
+
+						if( valueFn != nullptr )
+						{
+							break;
+						}
 					}
 					typeId = RunTimeTyped::baseTypeId( typeId );
 				}
 
 				plugPath.insert( plugPath.begin(), ancestor->getName() );
 				ancestor = ancestor->parent();
 			}
+
+			if( valueFn != nullptr )
+			{
+				return valueFn( plug );
+			}
 		}
 	}
 

src/GafferScene/EditScopeAlgo.cpp

@@ -799,13 +799,6 @@ TweakPlug *GafferScene::EditScopeAlgo::acquireAttributeEdit( Gaffer::EditScope *
 	attributeTweaks->tweaksPlug()->addChild( tweakPlug );
 
 	size_t columnIndex = rows->addColumn( tweakPlug.get(), columnName, /* adoptEnabledPlug */ true );
-	MetadataAlgo::copyIf(
-		tweakPlug.get(), rows->defaultRow()->cellsPlug()->getChild<Spreadsheet::CellPlug>( columnIndex )->valuePlug(),
-		[] ( const GraphComponent *from, const GraphComponent *to, const std::string &name ) {
-			return boost::starts_with( name, "tweakPlugValueWidget:" );
-		}
-	);
-
 	tweakPlug->setInput( processor->getChild<Spreadsheet>( "Spreadsheet" )->outPlug()->getChild<Plug>( columnIndex ) );
 
 	return row->cellsPlug()->getChild<Spreadsheet::CellPlug>( columnIndex )->valuePlug<TweakPlug>();
@@ -1425,13 +1418,6 @@ TweakPlug *GafferScene::EditScopeAlgo::acquireRenderPassOptionEdit( Gaffer::Edit
 	optionTweaks->tweaksPlug()->addChild( tweakPlug );
 
 	size_t columnIndex = rows->addColumn( tweakPlug.get(), columnName, /* adoptEnabledPlug */ true );
-	MetadataAlgo::copyIf(
-		tweakPlug.get(), rows->defaultRow()->cellsPlug()->getChild<Spreadsheet::CellPlug>( columnIndex )->valuePlug(),
-		[] ( const GraphComponent *from, const GraphComponent *to, const std::string &name ) {
-			return boost::starts_with( name, "tweakPlugValueWidget:" );
-		}
-	);
-
 	tweakPlug->setInput( processor->getChild<Spreadsheet>( "Spreadsheet" )->outPlug()->getChild<Plug>( columnIndex ) );
 
 	return row->cellsPlug()->getChild<Spreadsheet::CellPlug>( columnIndex )->valuePlug<TweakPlug>();