Redundant metadata cleanup

Changes.md

@@ -7,6 +7,7 @@ Features
 - LightTool :
   - Added manipulator for disk and point light radii.
   - Added manipulators for cylinder length and radius.
+- Tools Menu : Added "Metadata/Clean Up" menu item to optimise file size by removing redundant metadata.
 
 Improvements
 ------------
@@ -25,12 +26,24 @@ Fixes
 - Reference : Fixed rare reloading error.
 - PlugLayout : Fixed lack of update when `layout:customWidget:*` metadata changes.
 - Dispatch app : Removed unnecessary and misleading "Execute" button.
+- SceneAlgo : Fixed computation of `ScenePlug.object` in networks with nodes derived from `ObjectProcessor`. These include : `CameraTweaks`, `ClosestPointSampler`, `CollectPrimitiveVariables`, `CopyPrimitiveVariables`, `CurveSampler`, `DeleteCurves`, `DeleteFaces`, `DeletePoints`, `MapOffset`, `MapProjection`, `MeshDistortion`, `MeshNormals`, `MeshSegments`, `MeshTangents`, `MeshToPoints`, `MeshType`, `Orientation`, `PointsType`, `PrimitiveSampler`, `PrimitiveVariables`, `ReverseWinding`, `ShufflePrimitiveVariables` and `UVSampler` (#5406).
 
 API
 ---
 
 - Process : Added `acquireCollaborativeResult()` method, providing an improved mechanism for multiple threads to collaborate on TBB tasks spawned by a single process they all depend on.
 - ValuePlug : Added `Default` CachePolicy and deprecated `Standard`, `TaskIsolation` and `Legacy` policies.
+- Metadata : Fixed redundant copying of metadata when promoting plugs.
+
+API
+---
+
+- Metadata :
+  - Added `RegistrationTypes` enum that allows the different types of registrations to be identified.
+  - Added improved `registeredValues()` and `value()` overloads that provide finer-grained queries based on the type of registration.
+  - Deprecated `instanceOnly` and `persistentOnly` arguments in favour of new `registrationTypes` arguments.
+  - Prevented `renameable` and `deletable` metadata from being copied during plug promotion.
+- MetadataAlgo : Added `deregisterRedundantValues()` method.
 
 1.3.5.0 (relative to 1.3.4.0)
 =======

include/Gaffer/Metadata.h

@@ -94,11 +94,22 @@ class GAFFER_API Metadata
 		/// Registration queries
 		/// ====================
 
+		enum RegistrationTypes
+		{
+			None = 0,
+			TypeId = 1,
+			TypeIdDescendant = 2,
+			InstancePersistent = 4,
+			InstanceNonPersistent = 8,
+			Instance = InstancePersistent | InstanceNonPersistent,
+			All = TypeId | TypeIdDescendant | Instance
+		};
+
 		/// Fills the keys vector with keys for all values registered with the methods above.
 		static void registeredValues( IECore::InternedString target, std::vector<IECore::InternedString> &keys );
-		/// Fills the keys vector with keys for all values registered for the specified graphComponent.
-		/// If instanceOnly is true, then only the values registered for that exact instance are returned.
-		/// If persistentOnly is true, then non-persistent instance values are ignored.
+		/// Returns the keys for all values relevant to `target`, taking into account only the specified `registrationTypes`.
+		static std::vector<IECore::InternedString> registeredValues( const GraphComponent *target, unsigned registrationTypes = RegistrationTypes::All );
+		/// \deprecated Pass RegistrationTypes instead.
 		static void registeredValues( const GraphComponent *target, std::vector<IECore::InternedString> &keys, bool instanceOnly = false, bool persistentOnly = false );
 
 		/// Value retrieval
@@ -107,6 +118,11 @@ class GAFFER_API Metadata
 		/// Retrieves a value, returning null if none exists.
 		template<typename T=IECore::Data>
 		static typename T::ConstPtr value( IECore::InternedString target, IECore::InternedString key );
+		/// Ignores any values not included in `registrationTypes`.
+		template<typename T=IECore::Data>
+		static typename T::ConstPtr value( const GraphComponent *target, IECore::InternedString key, unsigned registrationTypes );
+		/// \deprecated Pass RegistrationTypes instead. When we remove this,
+		/// default `registrationTypes` to `All` in overload above.
 		template<typename T=IECore::Data>
 		static typename T::ConstPtr value( const GraphComponent *target, IECore::InternedString key, bool instanceOnly = false );
 
@@ -189,6 +205,8 @@ class GAFFER_API Metadata
 		static void instanceDestroyed( GraphComponent *graphComponent );
 
 		static IECore::ConstDataPtr valueInternal( IECore::InternedString target, IECore::InternedString key );
+		static IECore::ConstDataPtr valueInternal( const GraphComponent *target, IECore::InternedString key, unsigned registrationTypes );
+		/// \deprecated
 		static IECore::ConstDataPtr valueInternal( const GraphComponent *target, IECore::InternedString key, bool instanceOnly );
 
 };

include/Gaffer/Metadata.inl

@@ -45,6 +45,12 @@ typename T::ConstPtr Metadata::value( IECore::InternedString target, IECore::Int
 	return IECore::runTimeCast<const T>( valueInternal( target, key ) );
 }
 
+template<typename T>
+typename T::ConstPtr Metadata::value( const GraphComponent *target, IECore::InternedString key, unsigned registrationTypes )
+{
+	return IECore::runTimeCast<const T>( valueInternal( target, key, registrationTypes ) );
+}
+
 template<typename T>
 typename T::ConstPtr Metadata::value( const GraphComponent *target, IECore::InternedString key, bool instanceOnly )
 {

include/Gaffer/MetadataAlgo.h

@@ -234,6 +234,16 @@ GAFFER_API void copyColors( const Gaffer::Plug *srcPlug, Gaffer::Plug *dstPlug,
 /// Returns true if metadata can be promoted from one plug to another.
 GAFFER_API bool isPromotable( const GraphComponent *from, const GraphComponent *to, const IECore::InternedString &name );
 
+/// Cleanup
+/// =======
+
+/// Removes any redundant metadata registrations from `graphComponent` and all
+/// its descendants. By redundant we mean instance-level registrations that have
+/// the same value as an exising type-based fallback, so that removing the
+/// instance registration has no effect on the composed result.
+/// \undoable
+GAFFER_API void deregisterRedundantValues( GraphComponent *graphComponent );
+
 } // namespace MetadataAlgo
 
 } // namespace Gaffer

include/Gaffer/MetadataAlgo.inl

@@ -48,8 +48,7 @@ namespace MetadataAlgo
 template<typename Predicate>
 void copyIf( const GraphComponent *from, GraphComponent *to, Predicate &&predicate, bool persistent )
 {
-	std::vector<IECore::InternedString> names;
-	Metadata::registeredValues( from, names, /* instanceOnly = */ false, /* persistentOnly = */ false );
+	const std::vector<IECore::InternedString> names = Metadata::registeredValues( from );
 	for( const auto &name : names )
 	{
 		if( predicate( from, const_cast<const GraphComponent *>( to ), name ) )

python/Gaffer/ExtensionAlgo.py

@@ -206,7 +206,7 @@ def __uiDefinition( box, extension ) :
 def __metadata( graphComponent ) :
 
 	items = []
-	for k in Gaffer.Metadata.registeredValues( graphComponent, instanceOnly = True, persistentOnly = True ) :
+	for k in Gaffer.Metadata.registeredValues( graphComponent, Gaffer.Metadata.RegistrationTypes.InstancePersistent ) :
 
 		v = Gaffer.Metadata.value( graphComponent, k )
 		items.append(

python/GafferSceneTest/GroupTest.py

@@ -755,11 +755,8 @@ def testConnectingGroupDoesNotCopyColorMetadata( self ) :
 
 		g["in"][0].setInput( p["out"] )
 
-		noduleColor = Gaffer.Metadata.value( p, "nodule:color", instanceOnly = True )
-		connectionColor = Gaffer.Metadata.value( p, "connectionGadget:color", instanceOnly = True )
-
-		self.assertEqual( noduleColor, None )
-		self.assertEqual( noduleColor, connectionColor )
+		self.assertIsNone( Gaffer.Metadata.value( p, "nodule:color", Gaffer.Metadata.RegistrationTypes.Instance ) )
+		self.assertIsNone( Gaffer.Metadata.value( p, "connectionGadget:color", Gaffer.Metadata.RegistrationTypes.Instance ) )
 
 	def testProcessInvalidSet( self ) :
 

python/GafferSceneTest/SceneAlgoTest.py

@@ -404,6 +404,38 @@ def testHistoryPerformanceAlreadyCached( self ) :
 			h = h.predecessors[-1]
 		self.assertEqual( h.context["seed"], 5 )
 
+	def testObjectProcessorHistory( self ) :
+
+		plane = GafferScene.Plane()
+
+		meshType = GafferScene.MeshType()
+		meshType["in"].setInput( plane["out"] )
+
+		def runTest() :
+
+			history = GafferScene.SceneAlgo.history( meshType["out"]["object"], "/plane" )
+
+			for plug, scenePath in [
+				( meshType["out"], "/plane" ),
+				( meshType["in"], "/plane" ),
+				( plane["out"], "/plane" ),
+			] :
+				self.assertEqual( history.scene, plug )
+				self.assertEqual( GafferScene.ScenePlug.pathToString( history.context["scene:path"] ), scenePath )
+				history = history.predecessors[0] if history.predecessors else None
+
+			self.assertIsNone( history )
+
+		runTest()
+
+		# Test running the test while everything is already cached still works, and doesn't add any
+		# new entries to the cache
+		Gaffer.ValuePlug.clearHashCache()
+		runTest()
+		before = Gaffer.ValuePlug.hashCacheTotalUsage()
+		runTest()
+		self.assertEqual( Gaffer.ValuePlug.hashCacheTotalUsage(), before )
+
 	def testHistoryWithNoComputes( self ) :
 
 		switch = Gaffer.Switch()

python/GafferTest/BoxInTest.py

@@ -195,7 +195,7 @@ def testMetadata( self ) :
 		s["b1"]["i"].setup( s["b1"]["n"]["op1"] )
 
 		self.assertEqual( Gaffer.Metadata.value( s["b1"]["i"].promotedPlug(), "test" ), "testValue" )
-		self.assertNotIn( "layout:section", Gaffer.Metadata.registeredValues( s["b1"]["i"].promotedPlug(), instanceOnly = True ) )
+		self.assertNotIn( "layout:section", Gaffer.Metadata.registeredValues( s["b1"]["i"].promotedPlug(), Gaffer.Metadata.RegistrationTypes.Instance ) )
 
 		s["b2"] = Gaffer.Box()
 		s.execute(
@@ -204,7 +204,7 @@ def testMetadata( self ) :
 		)
 
 		self.assertEqual( Gaffer.Metadata.value( s["b2"]["i"].promotedPlug(), "test" ), "testValue" )
-		self.assertNotIn( "layout:section", Gaffer.Metadata.registeredValues( s["b2"]["i"].promotedPlug(), instanceOnly = True ) )
+		self.assertNotIn( "layout:section", Gaffer.Metadata.registeredValues( s["b2"]["i"].promotedPlug(), Gaffer.Metadata.RegistrationTypes.Instance ) )
 
 	def testNoduleSectionMetadata( self ) :
 

python/GafferTest/BoxOutTest.py

@@ -153,7 +153,7 @@ def testMetadata( self ) :
 		s["b1"]["o"].setup( s["b1"]["n"]["sum"] )
 
 		self.assertEqual( Gaffer.Metadata.value( s["b1"]["o"].promotedPlug(), "test" ), "testValue" )
-		self.assertNotIn( "layout:section", Gaffer.Metadata.registeredValues( s["b1"]["o"].promotedPlug(), instanceOnly = True ) )
+		self.assertNotIn( "layout:section", Gaffer.Metadata.registeredValues( s["b1"]["o"].promotedPlug(), Gaffer.Metadata.RegistrationTypes.Instance ) )
 
 		s["b2"] = Gaffer.Box()
 		s.execute(
@@ -162,7 +162,7 @@ def testMetadata( self ) :
 		)
 
 		self.assertEqual( Gaffer.Metadata.value( s["b2"]["o"].promotedPlug(), "test" ), "testValue" )
-		self.assertNotIn( "layout:section", Gaffer.Metadata.registeredValues( s["b2"]["o"].promotedPlug(), instanceOnly = True ) )
+		self.assertNotIn( "layout:section", Gaffer.Metadata.registeredValues( s["b2"]["o"].promotedPlug(), Gaffer.Metadata.RegistrationTypes.Instance ) )
 
 	def testNoduleSectionMetadata( self ) :
 

python/GafferTest/ExtensionAlgoTest.py

@@ -86,9 +86,9 @@ def testExport( self ) :
 
 		def assertExpectedMetadata( node ) :
 
-			self.assertEqual( Gaffer.Metadata.registeredValues( node, instanceOnly = True ), [] )
-			self.assertEqual( Gaffer.Metadata.registeredValues( node["in"], instanceOnly = True ), [] )
-			self.assertEqual( Gaffer.Metadata.registeredValues( node["out"], instanceOnly = True ), [] )
+			self.assertEqual( Gaffer.Metadata.registeredValues( node, Gaffer.Metadata.RegistrationTypes.Instance ), [] )
+			self.assertEqual( Gaffer.Metadata.registeredValues( node["in"], Gaffer.Metadata.RegistrationTypes.Instance ), [] )
+			self.assertEqual( Gaffer.Metadata.registeredValues( node["out"], Gaffer.Metadata.RegistrationTypes.Instance ), [] )
 
 			self.assertEqual( Gaffer.Metadata.value( node, "description" ), "Test" )
 			self.assertEqual( Gaffer.Metadata.value( node["in"], "description" ), "The input" )

python/GafferTest/MetadataAlgoTest.py

@@ -486,13 +486,13 @@ def testUnbookmarkedNodesDontHaveMetadata( self ) :
 
 		s = Gaffer.ScriptNode()
 		s["n"] = Gaffer.Node()
-		self.assertEqual( len( Gaffer.Metadata.registeredValues( s["n"], instanceOnly = True ) ), 0 )
+		self.assertEqual( len( Gaffer.Metadata.registeredValues( s["n"], Gaffer.Metadata.RegistrationTypes.Instance ) ), 0 )
 
 		Gaffer.MetadataAlgo.setBookmarked( s["n"], True )
-		self.assertEqual( len( Gaffer.Metadata.registeredValues( s["n"], instanceOnly = True ) ), 1 )
+		self.assertEqual( len( Gaffer.Metadata.registeredValues( s["n"], Gaffer.Metadata.RegistrationTypes.Instance ) ), 1 )
 
 		Gaffer.MetadataAlgo.setBookmarked( s["n"], False )
-		self.assertEqual( len( Gaffer.Metadata.registeredValues( s["n"], instanceOnly = True ) ), 0 )
+		self.assertEqual( len( Gaffer.Metadata.registeredValues( s["n"], Gaffer.Metadata.RegistrationTypes.Instance ) ), 0 )
 
 	def testNumericBookmarks( self ) :
 
@@ -708,21 +708,21 @@ def testAnnotationWithoutColor( self ) :
 
 		n = Gaffer.Node()
 		Gaffer.MetadataAlgo.addAnnotation( n, "test", Gaffer.MetadataAlgo.Annotation( text = "abc" ) )
-		self.assertEqual( len( Gaffer.Metadata.registeredValues( n, instanceOnly = True ) ), 1 )
+		self.assertEqual( len( Gaffer.Metadata.registeredValues( n, Gaffer.Metadata.RegistrationTypes.Instance ) ), 1 )
 		self.assertEqual(
 			Gaffer.MetadataAlgo.getAnnotation( n, "test" ),
 			Gaffer.MetadataAlgo.Annotation( text = "abc" )
 		)
 
 		Gaffer.MetadataAlgo.addAnnotation( n, "test", Gaffer.MetadataAlgo.Annotation( text = "xyz", color = imath.Color3f( 1 ) ) )
-		self.assertEqual( len( Gaffer.Metadata.registeredValues( n, instanceOnly = True ) ), 2 )
+		self.assertEqual( len( Gaffer.Metadata.registeredValues( n, Gaffer.Metadata.RegistrationTypes.Instance ) ), 2 )
 		self.assertEqual(
 			Gaffer.MetadataAlgo.getAnnotation( n, "test" ),
 			Gaffer.MetadataAlgo.Annotation( text = "xyz", color = imath.Color3f( 1 ) )
 		)
 
 		Gaffer.MetadataAlgo.addAnnotation( n, "test", Gaffer.MetadataAlgo.Annotation( text = "abc" ) )
-		self.assertEqual( len( Gaffer.Metadata.registeredValues( n, instanceOnly = True ) ), 1 )
+		self.assertEqual( len( Gaffer.Metadata.registeredValues( n, Gaffer.Metadata.RegistrationTypes.Instance ) ), 1 )
 		self.assertEqual(
 			Gaffer.MetadataAlgo.getAnnotation( n, "test" ),
 			Gaffer.MetadataAlgo.Annotation( text = "abc" )
@@ -782,6 +782,43 @@ def testNonUserAnnotationTemplates( self ) :
 		self.assertEqual( Gaffer.MetadataAlgo.annotationTemplates(), defaultTemplates + [ "test", "test2" ] )
 		self.assertEqual( Gaffer.MetadataAlgo.annotationTemplates( userOnly = True ), userOnlyDefaultTemplates + [ "test2" ] )
 
+	def testDeregisterRedundantValues( self ) :
+
+		values = [ False, None, 1, 2 ] # False means "no registration", None means "None is registered as the value"
+		for typeValue in values :
+			for instanceValue in values :
+				for nested in ( True, False ) :
+					with self.subTest( typeValue = typeValue, instanceValue = instanceValue, nested = nested ) :
+
+						node = GafferTest.AddNode()
+
+						if typeValue is not False :
+							Gaffer.Metadata.registerValue( GafferTest.AddNode, "metadataAlgoTest", typeValue )
+						else :
+							Gaffer.Metadata.deregisterValue( GafferTest.AddNode, "metadataAlgoTest" )
+
+						if instanceValue is not False :
+							Gaffer.Metadata.registerValue( node, "metadataAlgoTest", instanceValue )
+
+						if typeValue is not False :
+							self.assertEqual( Gaffer.Metadata.value( node, "metadataAlgoTest", registrationTypes = Gaffer.Metadata.RegistrationTypes.TypeId ), typeValue )
+
+						if instanceValue is not False :
+							self.assertEqual( Gaffer.Metadata.value( node, "metadataAlgoTest", registrationTypes = Gaffer.Metadata.RegistrationTypes.Instance ), instanceValue )
+
+						if nested :
+							nodeToPass = Gaffer.Box()
+							nodeToPass.addChild( node )
+						else :
+							nodeToPass = node
+
+						valueBefore = Gaffer.Metadata.value( node, "metadataAlgoTest" )
+						Gaffer.MetadataAlgo.deregisterRedundantValues( nodeToPass )
+						self.assertEqual( Gaffer.Metadata.value( node, "metadataAlgoTest" ), valueBefore )
+
+						if typeValue == instanceValue :
+							self.assertIsNone( Gaffer.Metadata.value( node, "metadataAlgoTest", registrationTypes = Gaffer.Metadata.RegistrationTypes.Instance ) )
+
 	def tearDown( self ) :
 
 		for n in ( Gaffer.Node, Gaffer.Box, GafferTest.AddNode ) :