diff --git a/Changes.md b/Changes.md index 927854660c1..e3a0591c8c5 100644 --- a/Changes.md +++ b/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) ======= diff --git a/include/Gaffer/Metadata.h b/include/Gaffer/Metadata.h index 1e7a554b83f..4aed04039f8 100644 --- a/include/Gaffer/Metadata.h +++ b/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 &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 registeredValues( const GraphComponent *target, unsigned registrationTypes = RegistrationTypes::All ); + /// \deprecated Pass RegistrationTypes instead. static void registeredValues( const GraphComponent *target, std::vector &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 static typename T::ConstPtr value( IECore::InternedString target, IECore::InternedString key ); + /// Ignores any values not included in `registrationTypes`. + template + 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 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 ); }; diff --git a/include/Gaffer/Metadata.inl b/include/Gaffer/Metadata.inl index b2ba65e9dc6..e7af38e0c96 100644 --- a/include/Gaffer/Metadata.inl +++ b/include/Gaffer/Metadata.inl @@ -45,6 +45,12 @@ typename T::ConstPtr Metadata::value( IECore::InternedString target, IECore::Int return IECore::runTimeCast( valueInternal( target, key ) ); } +template +typename T::ConstPtr Metadata::value( const GraphComponent *target, IECore::InternedString key, unsigned registrationTypes ) +{ + return IECore::runTimeCast( valueInternal( target, key, registrationTypes ) ); +} + template typename T::ConstPtr Metadata::value( const GraphComponent *target, IECore::InternedString key, bool instanceOnly ) { diff --git a/include/Gaffer/MetadataAlgo.h b/include/Gaffer/MetadataAlgo.h index dd83ab9a49e..20e1c4d5757 100644 --- a/include/Gaffer/MetadataAlgo.h +++ b/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 diff --git a/include/Gaffer/MetadataAlgo.inl b/include/Gaffer/MetadataAlgo.inl index 7e430004976..3ea90a20394 100644 --- a/include/Gaffer/MetadataAlgo.inl +++ b/include/Gaffer/MetadataAlgo.inl @@ -48,8 +48,7 @@ namespace MetadataAlgo template void copyIf( const GraphComponent *from, GraphComponent *to, Predicate &&predicate, bool persistent ) { - std::vector names; - Metadata::registeredValues( from, names, /* instanceOnly = */ false, /* persistentOnly = */ false ); + const std::vector names = Metadata::registeredValues( from ); for( const auto &name : names ) { if( predicate( from, const_cast( to ), name ) ) diff --git a/python/Gaffer/ExtensionAlgo.py b/python/Gaffer/ExtensionAlgo.py index b7b16ef2271..66013576832 100644 --- a/python/Gaffer/ExtensionAlgo.py +++ b/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( diff --git a/python/GafferSceneTest/GroupTest.py b/python/GafferSceneTest/GroupTest.py index c4664873915..480119874ce 100644 --- a/python/GafferSceneTest/GroupTest.py +++ b/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 ) : diff --git a/python/GafferSceneTest/SceneAlgoTest.py b/python/GafferSceneTest/SceneAlgoTest.py index 68b938d79ca..1d95b28dc7e 100644 --- a/python/GafferSceneTest/SceneAlgoTest.py +++ b/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() diff --git a/python/GafferTest/BoxInTest.py b/python/GafferTest/BoxInTest.py index 5c33bf300fc..8f0f161dd1f 100644 --- a/python/GafferTest/BoxInTest.py +++ b/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 ) : diff --git a/python/GafferTest/BoxOutTest.py b/python/GafferTest/BoxOutTest.py index ed1d90ed02d..1d702c3e87f 100644 --- a/python/GafferTest/BoxOutTest.py +++ b/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 ) : diff --git a/python/GafferTest/ExtensionAlgoTest.py b/python/GafferTest/ExtensionAlgoTest.py index 0431adb162b..889cb320f88 100644 --- a/python/GafferTest/ExtensionAlgoTest.py +++ b/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" ) diff --git a/python/GafferTest/MetadataAlgoTest.py b/python/GafferTest/MetadataAlgoTest.py index 947ae75dfaa..f6de9e96aaf 100644 --- a/python/GafferTest/MetadataAlgoTest.py +++ b/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 ) : diff --git a/python/GafferTest/MetadataTest.py b/python/GafferTest/MetadataTest.py index bb506687411..6873027e627 100644 --- a/python/GafferTest/MetadataTest.py +++ b/python/GafferTest/MetadataTest.py @@ -352,8 +352,8 @@ def testRegisteredValues( self ) : self.assertTrue( "ri" in Gaffer.Metadata.registeredValues( n ) ) self.assertTrue( "rpi" in Gaffer.Metadata.registeredValues( n["op1"] ) ) - self.assertTrue( "r" not in Gaffer.Metadata.registeredValues( n, instanceOnly=True ) ) - self.assertTrue( "rp" not in Gaffer.Metadata.registeredValues( n["op1"], instanceOnly=True ) ) + self.assertTrue( "r" not in Gaffer.Metadata.registeredValues( n, Gaffer.Metadata.RegistrationTypes.Instance ) ) + self.assertTrue( "rp" not in Gaffer.Metadata.registeredValues( n["op1"], Gaffer.Metadata.RegistrationTypes.Instance ) ) self.assertTrue( "ri" in Gaffer.Metadata.registeredValues( n ) ) self.assertTrue( "rpi" in Gaffer.Metadata.registeredValues( n["op1"] ) ) @@ -626,19 +626,19 @@ def assertNonExistent() : def assertPersistent() : - self.assertEqual( Gaffer.Metadata.registeredValues( s["n"], instanceOnly = True ), [ "a" ] ) - self.assertEqual( Gaffer.Metadata.registeredValues( s["n"]["op1"], instanceOnly = True ), [ "b" ] ) - self.assertEqual( Gaffer.Metadata.registeredValues( s["n"], instanceOnly = True, persistentOnly = True ), [ "a" ] ) - self.assertEqual( Gaffer.Metadata.registeredValues( s["n"]["op1"], instanceOnly = True, persistentOnly = True ), [ "b" ] ) + self.assertEqual( Gaffer.Metadata.registeredValues( s["n"], Gaffer.Metadata.RegistrationTypes.Instance ), [ "a" ] ) + self.assertEqual( Gaffer.Metadata.registeredValues( s["n"]["op1"], Gaffer.Metadata.RegistrationTypes.Instance ), [ "b" ] ) + self.assertEqual( Gaffer.Metadata.registeredValues( s["n"], Gaffer.Metadata.RegistrationTypes.InstancePersistent ), [ "a" ] ) + self.assertEqual( Gaffer.Metadata.registeredValues( s["n"]["op1"], Gaffer.Metadata.RegistrationTypes.InstancePersistent ), [ "b" ] ) self.assertEqual( Gaffer.Metadata.value( s["n"], "a" ), 1 ) self.assertEqual( Gaffer.Metadata.value( s["n"]["op1"], "b" ), 2 ) def assertNonPersistent() : - self.assertEqual( Gaffer.Metadata.registeredValues( s["n"], instanceOnly = True ), [ "a" ] ) - self.assertEqual( Gaffer.Metadata.registeredValues( s["n"]["op1"], instanceOnly = True ), [ "b" ] ) - self.assertEqual( Gaffer.Metadata.registeredValues( s["n"], instanceOnly = True, persistentOnly = True ), [] ) - self.assertEqual( Gaffer.Metadata.registeredValues( s["n"]["op1"], instanceOnly = True, persistentOnly = True ), [] ) + self.assertEqual( Gaffer.Metadata.registeredValues( s["n"], Gaffer.Metadata.RegistrationTypes.Instance ), [ "a" ] ) + self.assertEqual( Gaffer.Metadata.registeredValues( s["n"]["op1"], Gaffer.Metadata.RegistrationTypes.Instance ), [ "b" ] ) + self.assertEqual( Gaffer.Metadata.registeredValues( s["n"], Gaffer.Metadata.RegistrationTypes.InstancePersistent ), [] ) + self.assertEqual( Gaffer.Metadata.registeredValues( s["n"]["op1"], Gaffer.Metadata.RegistrationTypes.InstancePersistent ), [] ) self.assertEqual( Gaffer.Metadata.value( s["n"], "a" ), 1 ) self.assertEqual( Gaffer.Metadata.value( s["n"]["op1"], "b" ), 2 ) @@ -1043,8 +1043,8 @@ def testOverloadedMethods( self ) : Gaffer.Metadata.registerValue( n, "one", 1 ) Gaffer.Metadata.registerValue( n["op1"], "two", 2 ) - self.assertEqual( Gaffer.Metadata.registeredValues( n, instanceOnly = True ), [ "one" ] ) - self.assertEqual( Gaffer.Metadata.registeredValues( n["op1"], instanceOnly = True ), [ "two" ] ) + self.assertEqual( Gaffer.Metadata.registeredValues( n, Gaffer.Metadata.RegistrationTypes.Instance), [ "one" ] ) + self.assertEqual( Gaffer.Metadata.registeredValues( n["op1"], Gaffer.Metadata.RegistrationTypes.Instance ), [ "two" ] ) self.assertEqual( Gaffer.Metadata.value( n, "one" ), 1 ) self.assertEqual( Gaffer.Metadata.value( n["op1"], "two" ), 2 ) @@ -1052,8 +1052,45 @@ def testOverloadedMethods( self ) : Gaffer.Metadata.deregisterValue( n, "one" ) Gaffer.Metadata.deregisterValue( n["op1"], "two" ) - self.assertEqual( Gaffer.Metadata.registeredValues( n, instanceOnly = True ), [] ) - self.assertEqual( Gaffer.Metadata.registeredValues( n["op1"], instanceOnly = True ), [] ) + self.assertEqual( Gaffer.Metadata.registeredValues( n, Gaffer.Metadata.RegistrationTypes.Instance ), [] ) + self.assertEqual( Gaffer.Metadata.registeredValues( n["op1"], Gaffer.Metadata.RegistrationTypes.Instance ), [] ) + + def testQueryTypeIdRegistrationIgnoringInstanceRegistration( self ) : + + Gaffer.Metadata.registerValue( GafferTest.AddNode, "testMetadata", "typeIdValue" ) + self.addCleanup( Gaffer.Metadata.deregisterValue, GafferTest.AddNode, "testMetadata" ) + + node = GafferTest.AddNode() + Gaffer.Metadata.registerValue( node, "testMetadata", "instanceValue" ) + + self.assertEqual( Gaffer.Metadata.value( node, "testMetadata" ), "instanceValue" ) + self.assertEqual( + Gaffer.Metadata.value( node, "testMetadata", registrationTypes = Gaffer.Metadata.RegistrationTypes.TypeId ), + "typeIdValue" + ) + + def testRegistrationTypeVsInstanceOnly( self ) : + + # Check that we resolve the deprecated and non-deprecated function overloads + # correctly. Can be removed when we remove the deprecated ones. + + n = GafferTest.AddNode() + Gaffer.Metadata.registerValue( n["op1"], "persistent", 2 ) + Gaffer.Metadata.registerValue( n["op1"], "nonPersistent", 3, persistent = False ) + + # Keyword arguments should allow us to distinguish old and new overloads. + + self.assertEqual( Gaffer.Metadata.registeredValues( n["op1"], instanceOnly = True ), [ "persistent" , "nonPersistent" ] ) + self.assertEqual( Gaffer.Metadata.registeredValues( n["op1"], instanceOnly = True, persistentOnly = True ), [ "persistent" ] ) + self.assertEqual( Gaffer.Metadata.registeredValues( n["op1"], registrationTypes = Gaffer.Metadata.RegistrationTypes.Instance ), [ "persistent" , "nonPersistent" ] ) + self.assertEqual( Gaffer.Metadata.registeredValues( n["op1"], registrationTypes = Gaffer.Metadata.RegistrationTypes.InstanceNonPersistent ), [ "nonPersistent" ] ) + + # And even if we don't use keyword arguments, the argument types should allow us to disambiguate. + + self.assertEqual( Gaffer.Metadata.registeredValues( n["op1"], True ), [ "persistent" , "nonPersistent" ] ) + self.assertEqual( Gaffer.Metadata.registeredValues( n["op1"], True, True ), [ "persistent" ] ) + self.assertEqual( Gaffer.Metadata.registeredValues( n["op1"], Gaffer.Metadata.RegistrationTypes.Instance ), [ "persistent" , "nonPersistent" ] ) + self.assertEqual( Gaffer.Metadata.registeredValues( n["op1"], Gaffer.Metadata.RegistrationTypes.InstanceNonPersistent ), [ "nonPersistent" ] ) @staticmethod def testPythonUnload() : @@ -1245,5 +1282,26 @@ def changed( node, key, reason ) : Gaffer.Metadata.registerValue( node, "test", 2 ) self.assertEqual( Gaffer.Metadata.value( node, "test" ), 1 ) + def tearDown( self ) : + + GafferTest.TestCase.tearDown( self ) + + Gaffer.Metadata.deregisterValue( GafferTest.AddNode, "aKey" ) + Gaffer.Metadata.deregisterValue( GafferTest.AddNode, "iKey" ) + Gaffer.Metadata.deregisterValue( GafferTest.AddNode, "k" ) + Gaffer.Metadata.deregisterValue( GafferTest.AddNode, "imt" ) + Gaffer.Metadata.deregisterValue( GafferTest.AddNode, "maskTest" ) + Gaffer.Metadata.deregisterValue( GafferTest.AddNode, "deleteMe" ) + Gaffer.Metadata.deregisterValue( GafferTest.AddNode, "nodeData3" ) + + Gaffer.Metadata.deregisterValue( GafferTest.AddNode, "op1", "description" ) + Gaffer.Metadata.deregisterValue( GafferTest.AddNode, "op1", "iKey" ) + Gaffer.Metadata.deregisterValue( GafferTest.AddNode, "op1", "imt" ) + Gaffer.Metadata.deregisterValue( GafferTest.AddNode, "op1", "k" ) + Gaffer.Metadata.deregisterValue( GafferTest.AddNode, "op1", "deleteMe" ) + Gaffer.Metadata.deregisterValue( GafferTest.AddNode, "op1", "plugData3" ) + Gaffer.Metadata.deregisterValue( GafferTest.AddNode, "op1", "rp" ) + Gaffer.Metadata.deregisterValue( GafferTest.AddNode, "op*", "aKey" ) + if __name__ == "__main__": unittest.main() diff --git a/python/GafferTest/MonitorAlgoTest.py b/python/GafferTest/MonitorAlgoTest.py index 7e5f58858c3..68ba46a76f2 100644 --- a/python/GafferTest/MonitorAlgoTest.py +++ b/python/GafferTest/MonitorAlgoTest.py @@ -94,7 +94,7 @@ def testAnnotate( self ) : Gaffer.MonitorAlgo.removePerformanceAnnotations( s ) for node in Gaffer.Node.RecursiveRange( s ) : self.assertEqual( - Gaffer.Metadata.registeredValues( node, instanceOnly = True ), + Gaffer.Metadata.registeredValues( node, Gaffer.Metadata.RegistrationTypes.Instance ), [] ) diff --git a/python/GafferTest/PlugAlgoTest.py b/python/GafferTest/PlugAlgoTest.py index 08f26092664..d6ec26a5b2e 100644 --- a/python/GafferTest/PlugAlgoTest.py +++ b/python/GafferTest/PlugAlgoTest.py @@ -49,6 +49,7 @@ def tearDown( self ) : GafferTest.TestCase.tearDown( self ) Gaffer.Metadata.deregisterValue( GafferTest.AddNode, "op1", "plugAlgoTest:a" ) + Gaffer.Metadata.deregisterValue( Gaffer.IntPlug, "plugAlgoTest:b" ) def testPromote( self ) : @@ -474,7 +475,7 @@ def testPromotedNonBoxMetadataIsNonPersistent( self ) : p = Gaffer.PlugAlgo.promote( n["a"]["op1"] ) self.assertEqual( Gaffer.Metadata.value( p, "testPersistence" ), 10 ) self.assertTrue( "testPersistence" in Gaffer.Metadata.registeredValues( p ) ) - self.assertTrue( "testPersistence" not in Gaffer.Metadata.registeredValues( p, persistentOnly = True ) ) + self.assertTrue( "testPersistence" not in Gaffer.Metadata.registeredValues( p, Gaffer.Metadata.RegistrationTypes.InstancePersistent ) ) def testPromoteWithName( self ) : @@ -577,14 +578,14 @@ def testPromoteInNodeConstructor( self ) : self.assertEqual( Gaffer.Metadata.value( script["box"]["n"]["in"], "plugAlgoTest:a" ), "a" ) self.assertIn( "plugAlgoTest:a", Gaffer.Metadata.registeredValues( script["box"]["n"]["in"] ) ) - self.assertNotIn( "plugAlgoTest:a", Gaffer.Metadata.registeredValues( script["box"]["n"]["in"], persistentOnly = True ) ) + self.assertNotIn( "plugAlgoTest:a", Gaffer.Metadata.registeredValues( script["box"]["n"]["in"], Gaffer.Metadata.RegistrationTypes.InstancePersistent ) ) # And if we promote up one more level, we want that to work, and we want the # new metadata to be persistent so that it will be serialised and restored. Gaffer.PlugAlgo.promote( script["box"]["n"]["in"] ) self.assertIn( "plugAlgoTest:a", Gaffer.Metadata.registeredValues( script["box"]["in"] ) ) - self.assertIn( "plugAlgoTest:a", Gaffer.Metadata.registeredValues( script["box"]["in"], persistentOnly = True ) ) + self.assertIn( "plugAlgoTest:a", Gaffer.Metadata.registeredValues( script["box"]["in"], Gaffer.Metadata.RegistrationTypes.InstancePersistent ) ) # After serialisation and loading, everything should look the same. @@ -593,9 +594,9 @@ def testPromoteInNodeConstructor( self ) : self.assertEqual( Gaffer.Metadata.value( script2["box"]["n"]["in"], "plugAlgoTest:a" ), "a" ) self.assertIn( "plugAlgoTest:a", Gaffer.Metadata.registeredValues( script2["box"]["n"]["in"] ) ) - self.assertNotIn( "plugAlgoTest:a", Gaffer.Metadata.registeredValues( script2["box"]["n"]["in"], persistentOnly = True ) ) + self.assertNotIn( "plugAlgoTest:a", Gaffer.Metadata.registeredValues( script2["box"]["n"]["in"], Gaffer.Metadata.RegistrationTypes.InstancePersistent ) ) self.assertIn( "plugAlgoTest:a", Gaffer.Metadata.registeredValues( script2["box"]["in"] ) ) - self.assertIn( "plugAlgoTest:a", Gaffer.Metadata.registeredValues( script2["box"]["in"], persistentOnly = True ) ) + self.assertIn( "plugAlgoTest:a", Gaffer.Metadata.registeredValues( script2["box"]["in"], Gaffer.Metadata.RegistrationTypes.InstancePersistent ) ) def testPromotableMetadata( self ) : @@ -619,6 +620,39 @@ def testPromotableMetadata( self ) : self.assertIsNone( Gaffer.Metadata.value( p, "b:promotable" ) ) self.assertNotIn( "c:promotable", Gaffer.Metadata.registeredValues( p ) ) + def testPromoteDoesntMakeRedundantMetadata( self ) : + + # Make plug with metadata registered statically against its type. + + Gaffer.Metadata.registerValue( Gaffer.IntPlug, "plugAlgoTest:b", "testValueB" ) + self.addCleanup( Gaffer.Metadata.deregisterValue, Gaffer.IntPlug, "plugAlgoTest:b" ) + + box = Gaffer.Box() + box["node"] = Gaffer.Node() + box["node"]["plug"] = Gaffer.IntPlug( flags = Gaffer.Plug.Flags.Default | Gaffer.Plug.Flags.Dynamic ) + self.assertEqual( Gaffer.Metadata.value( box["node"]["plug"], "plugAlgoTest:b" ), "testValueB" ) + + # When promoting, we shouldn't need to make a per-instance copy of the + # static metadata, because the static registration also applies to the + # promoted plug. + + promoted = Gaffer.PlugAlgo.promote( box["node"]["plug"] ) + self.assertTrue( promoted.node().isSame( box ) ) + self.assertEqual( Gaffer.Metadata.value( promoted, "plugAlgoTest:b" ), "testValueB" ) + self.assertIsNone( Gaffer.Metadata.value( promoted, "plugAlgoTest:b", Gaffer.Metadata.RegistrationTypes.Instance ) ) + + box.removeChild( promoted ) + Gaffer.Metadata.registerValue( box["node"]["plug"], "plugAlgoTest:b", "testValueC" ) + self.assertEqual( Gaffer.Metadata.value( box["node"]["plug"], "plugAlgoTest:b" ), "testValueC" ) + + # But if the plug to be promoted has a unique per-instance value, then + # we'll need to promote that explicitly. + + promoted = Gaffer.PlugAlgo.promote( box["node"]["plug"] ) + self.assertTrue( promoted.node().isSame( box ) ) + self.assertEqual( Gaffer.Metadata.value( promoted, "plugAlgoTest:b" ), "testValueC" ) + self.assertEqual( Gaffer.Metadata.value( promoted, "plugAlgoTest:b", Gaffer.Metadata.RegistrationTypes.Instance ), "testValueC" ) + def testGetValueFromNameValuePlug( self ) : withEnabled = Gaffer.NameValuePlug( "test", 10, defaultEnabled = True ) diff --git a/python/GafferTest/ReferenceTest.py b/python/GafferTest/ReferenceTest.py index 734290f2f49..dd6aa200613 100644 --- a/python/GafferTest/ReferenceTest.py +++ b/python/GafferTest/ReferenceTest.py @@ -799,10 +799,10 @@ def testVersionMetadata( self ) : self.assertEqual( Gaffer.Metadata.value( s["r"], "serialiser:minorVersion" ), Gaffer.About.minorVersion() ) self.assertEqual( Gaffer.Metadata.value( s["r"], "serialiser:patchVersion" ), Gaffer.About.patchVersion() ) - self.assertTrue( "serialiser:milestoneVersion" not in Gaffer.Metadata.registeredValues( s["r"], persistentOnly = True ) ) - self.assertTrue( "serialiser:majorVersion" not in Gaffer.Metadata.registeredValues( s["r"], persistentOnly = True ) ) - self.assertTrue( "serialiser:minorVersion" not in Gaffer.Metadata.registeredValues( s["r"], persistentOnly = True ) ) - self.assertTrue( "serialiser:patchVersion" not in Gaffer.Metadata.registeredValues( s["r"], persistentOnly = True ) ) + self.assertNotIn( "serialiser:milestoneVersion", Gaffer.Metadata.registeredValues( s["r"], Gaffer.Metadata.RegistrationTypes.InstancePersistent ) ) + self.assertNotIn( "serialiser:majorVersion", Gaffer.Metadata.registeredValues( s["r"], Gaffer.Metadata.RegistrationTypes.InstancePersistent ) ) + self.assertNotIn( "serialiser:minorVersion", Gaffer.Metadata.registeredValues( s["r"], Gaffer.Metadata.RegistrationTypes.InstancePersistent ) ) + self.assertNotIn( "serialiser:patchVersion", Gaffer.Metadata.registeredValues( s["r"], Gaffer.Metadata.RegistrationTypes.InstancePersistent ) ) def testSerialiseWithoutLoading( self ) : diff --git a/python/GafferUI/BoxUI.py b/python/GafferUI/BoxUI.py index f48096659b9..ae4e0b112b0 100644 --- a/python/GafferUI/BoxUI.py +++ b/python/GafferUI/BoxUI.py @@ -164,7 +164,7 @@ def __nonDefaultPlugs( box ) : def __nonDefaultWalk( plug ) : - if Gaffer.Metadata.value( plug, "userDefault", instanceOnly = True ) is not None : + if Gaffer.Metadata.value( plug, "userDefault", Gaffer.Metadata.RegistrationTypes.Instance ) is not None : return True if len( plug ) == 0 : diff --git a/python/GafferUI/NodeUI.py b/python/GafferUI/NodeUI.py index c1b3b7ca51e..25d08ceb0fc 100644 --- a/python/GafferUI/NodeUI.py +++ b/python/GafferUI/NodeUI.py @@ -95,6 +95,15 @@ def __documentationURL( node ) : ), + "..." : ( + + # Just because a plug is renameable and/or deletable on one node + # does not mean it should still be when promoted to another. + "renameable:promotable", False, + "deletable:promotable", False, + + ), + } ) diff --git a/python/GafferUI/SpreadsheetUI/_Metadata.py b/python/GafferUI/SpreadsheetUI/_Metadata.py index c62eb766524..a95330b0c50 100644 --- a/python/GafferUI/SpreadsheetUI/_Metadata.py +++ b/python/GafferUI/SpreadsheetUI/_Metadata.py @@ -252,6 +252,7 @@ def __defaultCellMetadata( plug, key ) : return Gaffer.Metadata.value( __correspondingDefaultPlug( plug ), key ) for key in [ + "description", "spreadsheet:columnLabel", "spreadsheet:columnWidth", "plugValueWidget:type", diff --git a/python/GafferUI/SpreadsheetUI/_SectionChooser.py b/python/GafferUI/SpreadsheetUI/_SectionChooser.py index 474708c4fb3..1ee83a29af1 100644 --- a/python/GafferUI/SpreadsheetUI/_SectionChooser.py +++ b/python/GafferUI/SpreadsheetUI/_SectionChooser.py @@ -154,7 +154,7 @@ def __assignSectionOrder( cls, rowsPlug, sectionNames ) : # Remove metadata for sections that no longer exist - registeredValues = Gaffer.Metadata.registeredValues( rowsPlug, instanceOnly = True ) + registeredValues = Gaffer.Metadata.registeredValues( rowsPlug, Gaffer.Metadata.RegistrationTypes.Instance ) for key in registeredValues : m = re.match( "spreadsheet:section:(.+):index", key ) if m and m.group( 1 ) not in sectionNames : diff --git a/python/GafferUI/UIEditor.py b/python/GafferUI/UIEditor.py index 0a18ab90b98..9b05e93521d 100644 --- a/python/GafferUI/UIEditor.py +++ b/python/GafferUI/UIEditor.py @@ -1153,7 +1153,7 @@ def __updatePath( self ) : d = self.__pathListing.getPath().dict() d.clear() if self.__plug is not None : - for name in Gaffer.Metadata.registeredValues( self.__plug, instanceOnly = True, persistentOnly = True ) : + for name in Gaffer.Metadata.registeredValues( self.__plug, Gaffer.Metadata.RegistrationTypes.InstancePersistent ) : if name.startswith( "preset:" ) : d[name[7:]] = Gaffer.Metadata.value( self.__plug, name ) @@ -1699,7 +1699,7 @@ def newSection( oldSection ) : emptySections[i] = newSection( emptySections[i] ) Gaffer.Metadata.registerValue( self.getPlugParent(), "uiEditor:emptySections", emptySections ) - for name in Gaffer.Metadata.registeredValues( self.getPlugParent(), instanceOnly = True, persistentOnly = True ) : + for name in Gaffer.Metadata.registeredValues( self.getPlugParent(), Gaffer.Metadata.RegistrationTypes.InstancePersistent ) : m = re.match( "(layout:section:)(.*)(:.*)", name ) if m : if newSection( m.group( 2 ) ) != m.group( 2 ) : diff --git a/python/GafferUITest/BoxIOUITest.py b/python/GafferUITest/BoxIOUITest.py new file mode 100644 index 00000000000..6c9ce98df01 --- /dev/null +++ b/python/GafferUITest/BoxIOUITest.py @@ -0,0 +1,81 @@ +########################################################################## +# +# Copyright (c) 2023, Cinesite VFX Ltd. All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +# * Redistributions of source code must retain the above +# copyright notice, this list of conditions and the following +# disclaimer. +# +# * Redistributions in binary form must reproduce the above +# copyright notice, this list of conditions and the following +# disclaimer in the documentation and/or other materials provided with +# the distribution. +# +# * Neither the name of John Haddon nor the names of +# any other contributors to this software may be used to endorse or +# promote products derived from this software without specific prior +# written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS +# IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, +# THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR +# PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR +# CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, +# EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, +# PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR +# PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF +# LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING +# NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS +# SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +# +########################################################################## + +import unittest + +import imath + +import Gaffer +import GafferTest +import GafferUI +import GafferUITest + +class BoxIOUITest( GafferUITest.TestCase ) : + + def testNoRedundantMetadata( self ) : + + box = Gaffer.Box() + + box["add"] = GafferTest.AddNode() + + box["switch"] = Gaffer.Switch() + box["switch"].setup( box["add"]["sum"] ) + box["switch"]["in"][0].setInput( box["add"]["sum"] ) + + Gaffer.PlugAlgo.promote( box["switch"]["in"][1] ) + Gaffer.BoxIO.insert( box ) + self.assertEqual( Gaffer.Metadata.registeredValues( box["BoxIn"]["__in"], Gaffer.Metadata.RegistrationTypes.Instance ), [] ) + + oldColor = GafferUI.Metadata.value( box["add"]["op1"], "nodule:color", Gaffer.Metadata.RegistrationTypes.TypeId ) + self.addCleanup( Gaffer.Metadata.registerValue, Gaffer.IntPlug, "nodule:color", oldColor ) + + newColor = imath.Color3f( 1, 0, 0 ) + Gaffer.Metadata.registerValue( Gaffer.IntPlug, "nodule:color", newColor ) + self.assertEqual( Gaffer.Metadata.value( box["add"]["op1"], "nodule:color" ), newColor ) + + promoted = Gaffer.BoxIO.promote( box["switch"]["out"] ) + self.assertEqual( + set( Gaffer.Metadata.registeredValues( box["BoxOut"]["__out"], Gaffer.Metadata.RegistrationTypes.Instance ) ), + # We allow `description` and `plugValueWidget:type` because we do want those to be transferred through to + # the promoted plug, even though `description` doesn't always make sense in the new context. But we definitely + # don't want `nodule:color` to be promoted to instance metadata, because it is inherited from the type anyway. + { "description", "plugValueWidget:type" } + ) + self.assertEqual( Gaffer.Metadata.value( promoted, "nodule:color" ), newColor ) + self.assertEqual( Gaffer.Metadata.value( box["BoxOut"]["__out"], "nodule:color" ), newColor ) + +if __name__ == "__main__": + unittest.main() diff --git a/python/GafferUITest/SpreadsheetUITest.py b/python/GafferUITest/SpreadsheetUITest.py index 24054e2102a..821719145b1 100644 --- a/python/GafferUITest/SpreadsheetUITest.py +++ b/python/GafferUITest/SpreadsheetUITest.py @@ -852,5 +852,67 @@ def testRowMetadata( self ) : self.assertEqual( Gaffer.Metadata.value( s["rows"][1]["cells"]["a"]["value"], "plugValueWidget:type" ), "Test2" ) self.assertEqual( Gaffer.Metadata.value( s["rows"][1]["cells"]["b"]["value"], "plugValueWidget:type" ), "Test3" ) + def testRowMetadataNotPromotedRedundantly( self ) : + + Gaffer.Metadata.registerValue( Gaffer.IntPlug, "plugAlgoTest:b", "testValueB" ) + self.addCleanup( Gaffer.Metadata.deregisterValue, Gaffer.IntPlug, "plugAlgoTest:b" ) + + # Metadata is automatically inherited from the default row to the other rows + # so that we can avoid lots of redundant registrations. + + box = Gaffer.Box() + box["spreadsheet"] = Gaffer.Spreadsheet() + box["spreadsheet"]["rows"].addColumn( Gaffer.StringPlug( "column1" ) ) + box["spreadsheet"]["rows"].addRows( 1 ) + Gaffer.Metadata.registerValue( + box["spreadsheet"]["rows"][0]["cells"]["column1"]["value"], + "plugValueWidget:type", "GafferUI.MultiLineStringPlugValueWidget" + ) + self.assertEqual( Gaffer.Metadata.value( box["spreadsheet"]["rows"][0]["cells"]["column1"]["value"], "plugValueWidget:type" ), "GafferUI.MultiLineStringPlugValueWidget" ) + self.assertEqual( Gaffer.Metadata.value( box["spreadsheet"]["rows"][1]["cells"]["column1"]["value"], "plugValueWidget:type" ), "GafferUI.MultiLineStringPlugValueWidget" ) + + # So we want to avoid promoting that metadata redundantly for the non-default rows. + + promoted = Gaffer.PlugAlgo.promote( box["spreadsheet"]["rows"] ) + self.assertEqual( Gaffer.Metadata.value( promoted[0]["cells"]["column1"]["value"], "plugValueWidget:type" ), "GafferUI.MultiLineStringPlugValueWidget" ) + self.assertEqual( Gaffer.Metadata.value( promoted[0]["cells"]["column1"]["value"], "plugValueWidget:type", Gaffer.Metadata.RegistrationTypes.Instance ), "GafferUI.MultiLineStringPlugValueWidget" ) + self.assertEqual( Gaffer.Metadata.value( promoted[1]["cells"]["column1"]["value"], "plugValueWidget:type" ), "GafferUI.MultiLineStringPlugValueWidget" ) + self.assertIsNone( Gaffer.Metadata.value( promoted[1]["cells"]["column1"]["value"], "plugValueWidget:type", Gaffer.Metadata.RegistrationTypes.Instance ) ) + + # And we don't want any other metadata to be copied for the non-default rows. + + for plug in Gaffer.Plug.RecursiveRange( promoted[1] ) : + self.assertEqual( Gaffer.Metadata.registeredValues( plug, Gaffer.Metadata.RegistrationTypes.Instance ), [] ) + + def testMetadataAlgoRemovesNonDefaultRowMetadata( self ) : + + spreadsheet = Gaffer.Spreadsheet() + spreadsheet["rows"].addColumn( Gaffer.StringPlug( "column1" ) ) + spreadsheet["rows"].addRows( 1 ) + + # Create Spreadsheet where non-default row has its own metadata that conflicts with the values + # that should be mirrored from the default row. It used to be possible for the user to get into + # this situation because we redundantly promoted metadata onto the non-default rows. + + Gaffer.Metadata.registerValue( spreadsheet["rows"][0]["cells"][0], "spreadsheet:columnWidth", 100 ) + Gaffer.Metadata.registerValue( spreadsheet["rows"][1]["cells"][0], "spreadsheet:columnWidth", 200 ) + Gaffer.Metadata.registerValue( spreadsheet["rows"][0]["cells"][0], "spreadsheet:columnLabel", "Label 1" ) + Gaffer.Metadata.registerValue( spreadsheet["rows"][1]["cells"][0], "spreadsheet:columnLabel", "Label 2" ) + + # Even though the non-default-row values are different to the default row ones, we still want + # `deregisterRedundantValues()` to clean them up, because having different column widths/labels + # on different rows is logically impossible. + + Gaffer.MetadataAlgo.deregisterRedundantValues( spreadsheet["rows"] ) + + for row in spreadsheet["rows"] : + self.assertEqual( Gaffer.Metadata.value( row["cells"][0], "spreadsheet:columnWidth" ), 100 ) + self.assertEqual( Gaffer.Metadata.value( row["cells"][0], "spreadsheet:columnLabel" ), "Label 1" ) + + self.assertEqual( + Gaffer.Metadata.registeredValues( spreadsheet["rows"][1]["cells"][0], Gaffer.Metadata.RegistrationTypes.Instance ), + [] + ) + if __name__ == "__main__": unittest.main() diff --git a/python/GafferUITest/__init__.py b/python/GafferUITest/__init__.py index 3ccc688e451..b57c06f8f1e 100644 --- a/python/GafferUITest/__init__.py +++ b/python/GafferUITest/__init__.py @@ -130,6 +130,7 @@ from .StandardNodeToolbarTest import StandardNodeToolbarTest from .LabelPlugValueWidgetTest import LabelPlugValueWidgetTest from .PythonEditorTest import PythonEditorTest +from .BoxIOUITest import BoxIOUITest if __name__ == "__main__": unittest.main() diff --git a/src/Gaffer/BoxIO.cpp b/src/Gaffer/BoxIO.cpp index 436249b2eb8..56d4f8c1767 100644 --- a/src/Gaffer/BoxIO.cpp +++ b/src/Gaffer/BoxIO.cpp @@ -226,7 +226,20 @@ void BoxIO::setup( const Plug *plug ) /// individual exclusions. return false; } - return MetadataAlgo::isPromotable( from, to, name ); + if( !MetadataAlgo::isPromotable( from, to, name ) ) + { + return false; + } + // Only copy if the destination doesn't already have the metadata. + // This avoids making unnecessary instance-level metadata when the + // same value is registered statically (against the plug type). + ConstDataPtr fromValue = Gaffer::Metadata::value( from, name ); + ConstDataPtr toValue = Gaffer::Metadata::value( to, name ); + if( fromValue && toValue ) + { + return !toValue->isEqualTo( fromValue.get() ); + } + return (bool)fromValue != (bool)toValue; } ); diff --git a/src/Gaffer/Metadata.cpp b/src/Gaffer/Metadata.cpp index 5d173b885ce..ea0ff34166e 100644 --- a/src/Gaffer/Metadata.cpp +++ b/src/Gaffer/Metadata.cpp @@ -428,7 +428,7 @@ void registerInstanceValue( GraphComponent *instance, IECore::InternedString key ); } -void registeredInstanceValues( const GraphComponent *graphComponent, std::vector &keys, bool persistentOnly ) +void registeredInstanceValues( const GraphComponent *graphComponent, std::vector &keys, unsigned registrationTypes ) { InstanceMetadataMap &m = instanceMetadataMap(); InstanceMetadataMap::const_accessor accessor; @@ -440,13 +440,34 @@ void registeredInstanceValues( const GraphComponent *graphComponent, std::vector const InstanceValues::nth_index<1>::type &index = accessor->second->get<1>(); for( InstanceValues::nth_index<1>::type::const_iterator vIt = index.begin(), veIt = index.end(); vIt != veIt; ++vIt ) { - if( !persistentOnly || vIt->persistent ) + if( + ( vIt->persistent && ( registrationTypes & Metadata::RegistrationTypes::InstancePersistent ) ) || + ( !vIt->persistent && ( registrationTypes & Metadata::RegistrationTypes::InstanceNonPersistent ) ) + ) { keys.push_back( vIt->name ); } } } +// Utilities +// ========= + +// Converts from deprecated arguments to new RegistrationTypes. +unsigned registrationTypes( bool instanceOnly, bool persistentOnly = false ) +{ + unsigned result = Metadata::RegistrationTypes::InstancePersistent; + if( !instanceOnly ) + { + result |= Metadata::RegistrationTypes::TypeId | Metadata::RegistrationTypes::TypeIdDescendant; + } + if( !persistentOnly ) + { + result |= Metadata::RegistrationTypes::InstanceNonPersistent; + } + return result; +} + } // namespace ////////////////////////////////////////////////////////////////////////// @@ -688,9 +709,11 @@ void Metadata::registerValue( GraphComponent *target, IECore::InternedString key registerInstanceValue( target, key, value, persistent ); } -void Metadata::registeredValues( const GraphComponent *target, std::vector &keys, bool instanceOnly, bool persistentOnly ) +std::vector Metadata::registeredValues( const GraphComponent *target, unsigned registrationTypes ) { - if( !instanceOnly ) + std::vector keys; + + if( registrationTypes & RegistrationTypes::TypeId ) { IECore::TypeId typeId = target->typeId(); while( typeId != InvalidTypeId ) @@ -707,7 +730,10 @@ void Metadata::registeredValues( const GraphComponent *target, std::vector( target ) ) { vector plugPathKeys; @@ -743,55 +769,61 @@ void Metadata::registeredValues( const GraphComponent *target, std::vector &keys, bool instanceOnly, bool persistentOnly ) +{ + keys = registeredValues( target, registrationTypes( instanceOnly, persistentOnly ) ); +} + +IECore::ConstDataPtr Metadata::valueInternal( const GraphComponent *target, IECore::InternedString key, unsigned registrationTypes ) { // Look for instance values first. These override // everything else. - if( OptionalData iv = instanceValue( target, key ) ) + if( registrationTypes & RegistrationTypes::Instance ) { - return *iv; - } - - if( instanceOnly ) - { - return nullptr; + bool persistent; + if( OptionalData iv = instanceValue( target, key, &persistent ) ) + { + if( + ( !persistent && ( registrationTypes & RegistrationTypes::InstanceNonPersistent ) ) || + ( persistent && ( registrationTypes & RegistrationTypes::InstancePersistent ) ) + ) + { + return *iv; + } + } } // If the target is a plug, then look for a path-based // value. These are more specific than type-based values. - if( const Plug *plug = runTimeCast( target ) ) + if( registrationTypes & RegistrationTypes::TypeIdDescendant ) { - const GraphComponent *ancestor = plug->parent(); - vector plugPath( { plug->getName() } ); - while( ancestor ) + if( const Plug *plug = runTimeCast( target ) ) { - IECore::TypeId typeId = ancestor->typeId(); - while( typeId != InvalidTypeId ) + const GraphComponent *ancestor = plug->parent(); + vector plugPath( { plug->getName() } ); + while( ancestor ) { - auto nIt = graphComponentMetadataMap().find( typeId ); - if( nIt != graphComponentMetadataMap().end() ) + IECore::TypeId typeId = ancestor->typeId(); + while( typeId != InvalidTypeId ) { - // First do a direct lookup using the plug path. - auto it = nIt->second.plugPathsToValues.find( plugPath ); - const auto eIt = nIt->second.plugPathsToValues.end(); - if( it != eIt ) - { - auto vIt = it->second.find( key ); - if( vIt != it->second.end() ) - { - return vIt->second( plug ); - } - } - // And only if the direct lookup fails, do a full search using - // wildcard matches. - for( it = nIt->second.plugPathsToValues.begin(); it != eIt; ++it ) + auto nIt = graphComponentMetadataMap().find( typeId ); + if( nIt != graphComponentMetadataMap().end() ) { - if( StringAlgo::match( plugPath, it->first ) ) + // First do a direct lookup using the plug path. + auto it = nIt->second.plugPathsToValues.find( plugPath ); + const auto eIt = nIt->second.plugPathsToValues.end(); + if( it != eIt ) { auto vIt = it->second.find( key ); if( vIt != it->second.end() ) @@ -799,36 +831,57 @@ IECore::ConstDataPtr Metadata::valueInternal( const GraphComponent *target, IECo return vIt->second( plug ); } } + // And only if the direct lookup fails, do a full search using + // wildcard matches. + for( it = nIt->second.plugPathsToValues.begin(); it != eIt; ++it ) + { + if( StringAlgo::match( plugPath, it->first ) ) + { + auto vIt = it->second.find( key ); + if( vIt != it->second.end() ) + { + return vIt->second( plug ); + } + } + } } + typeId = RunTimeTyped::baseTypeId( typeId ); } - typeId = RunTimeTyped::baseTypeId( typeId ); - } - plugPath.insert( plugPath.begin(), ancestor->getName() ); - ancestor = ancestor->parent(); + plugPath.insert( plugPath.begin(), ancestor->getName() ); + ancestor = ancestor->parent(); + } } } // Finally look for values registered to the type - IECore::TypeId typeId = target->typeId(); - while( typeId != InvalidTypeId ) + if( registrationTypes & RegistrationTypes::TypeId ) { - auto nIt = graphComponentMetadataMap().find( typeId ); - if( nIt != graphComponentMetadataMap().end() ) + IECore::TypeId typeId = target->typeId(); + while( typeId != InvalidTypeId ) { - auto vIt = nIt->second.values.find( key ); - if( vIt != nIt->second.values.end() ) + auto nIt = graphComponentMetadataMap().find( typeId ); + if( nIt != graphComponentMetadataMap().end() ) { - return vIt->second( target ); + auto vIt = nIt->second.values.find( key ); + if( vIt != nIt->second.values.end() ) + { + return vIt->second( target ); + } } + typeId = RunTimeTyped::baseTypeId( typeId ); } - typeId = RunTimeTyped::baseTypeId( typeId ); } return nullptr; } +IECore::ConstDataPtr Metadata::valueInternal( const GraphComponent *target, IECore::InternedString key, bool instanceOnly ) +{ + return Metadata::valueInternal( target, key, registrationTypes( instanceOnly ) ); +} + Metadata::ValueChangedSignal &Metadata::valueChangedSignal() { static ValueChangedSignal *s = new ValueChangedSignal; diff --git a/src/Gaffer/MetadataAlgo.cpp b/src/Gaffer/MetadataAlgo.cpp index 62b81474f67..1536064970d 100644 --- a/src/Gaffer/MetadataAlgo.cpp +++ b/src/Gaffer/MetadataAlgo.cpp @@ -42,6 +42,7 @@ #include "Gaffer/Plug.h" #include "Gaffer/Reference.h" #include "Gaffer/ScriptNode.h" +#include "Gaffer/Spreadsheet.h" #include "IECore/SimpleTypedData.h" @@ -62,6 +63,8 @@ IECore::InternedString g_connectionColorKey( "connectionGadget:color" ); IECore::InternedString g_noduleColorKey( "nodule:color" ); const std::string g_annotationPrefix( "annotation:" ); const InternedString g_annotations( "annotations" ); +const InternedString g_spreadsheetColumnWidth( "spreadsheet:columnWidth" ); +const InternedString g_spreadsheetColumnLabel( "spreadsheet:columnLabel" ); void copy( const Gaffer::GraphComponent *src , Gaffer::GraphComponent *dst , IECore::InternedString key , bool overwrite ) { @@ -436,8 +439,7 @@ Annotation getAnnotation( const Node *node, const std::string &name, bool inheri void removeAnnotation( Node *node, const std::string &name ) { const string prefix = g_annotationPrefix + name + ":"; - vector keys; - Metadata::registeredValues( node, keys ); + const vector keys = Metadata::registeredValues( node ); for( const auto &key : keys ) { @@ -450,8 +452,7 @@ void removeAnnotation( Node *node, const std::string &name ) void annotations( const Node *node, std::vector &names ) { - vector keys; - Metadata::registeredValues( node, keys ); + const vector keys = Metadata::registeredValues( node ); for( const auto &key : keys ) { @@ -695,8 +696,14 @@ void copy( const GraphComponent *from, GraphComponent *to, bool persistent ) void copy( const GraphComponent *from, GraphComponent *to, const IECore::StringAlgo::MatchPattern &exclude, bool persistentOnly, bool persistent ) { - vector keys; - Metadata::registeredValues( from, keys, /* instanceOnly = */ false, /* persistentOnly = */ persistentOnly ); + /// \todo Change function signature to take `RegistrationTypes` directly. + unsigned registrationTypes = Metadata::RegistrationTypes::TypeId | Metadata::RegistrationTypes::TypeIdDescendant | Metadata::RegistrationTypes::InstancePersistent; + if( !persistentOnly ) + { + registrationTypes |= Metadata::RegistrationTypes::InstanceNonPersistent; + } + + const vector keys = Metadata::registeredValues( from, registrationTypes ); for( vector::const_iterator it = keys.begin(), eIt = keys.end(); it != eIt; ++it ) { if( StringAlgo::matchMultiple( it->string(), exclude ) ) @@ -747,6 +754,49 @@ bool isPromotable( const GraphComponent *from, const GraphComponent *to, const I return true; } +/// Cleanup +/// ======= + +void deregisterRedundantValues( GraphComponent *graphComponent ) +{ + for( const auto &key : Metadata::registeredValues( graphComponent, Metadata::RegistrationTypes::Instance ) ) + { + ConstDataPtr instanceValue = Metadata::value( graphComponent, key, (unsigned)Metadata::RegistrationTypes::Instance ); + ConstDataPtr typeValue = Metadata::value( graphComponent, key, (unsigned)Metadata::RegistrationTypes::TypeId | Metadata::RegistrationTypes::TypeIdDescendant ); + if( + ( (bool)instanceValue == (bool)typeValue ) && + ( !instanceValue || instanceValue->isEqualTo( typeValue.get() ) ) + ) + { + // We can remove the instance value, because the lookup will fall + // back to an identical value. + Gaffer::Metadata::deregisterValue( graphComponent, key ); + } + + // Special-case for badly promoted spreadsheet metadata of old. + if( key == g_spreadsheetColumnWidth || key == g_spreadsheetColumnLabel ) + { + if( auto row = graphComponent->ancestor() ) + { + if( auto rows = row->parent() ) + { + if( row != rows->defaultRow() ) + { + // Override on non-default row doesn't agree with the value + // that should be mirrored from the default row. Remove it. + Gaffer::Metadata::deregisterValue( graphComponent, key ); + } + } + } + } + } + + for( auto &child : GraphComponent::Range( *graphComponent ) ) + { + deregisterRedundantValues( child.get() ); + } +} + } // namespace MetadataAlgo } // namespace Gaffer diff --git a/src/Gaffer/PlugAlgo.cpp b/src/Gaffer/PlugAlgo.cpp index 4d1de79afe9..e0db579dd9a 100644 --- a/src/Gaffer/PlugAlgo.cpp +++ b/src/Gaffer/PlugAlgo.cpp @@ -1216,7 +1216,20 @@ Plug *promoteWithName( Plug *plug, const InternedString &name, Plug *parent, con /// individual exclusions. return false; } - return MetadataAlgo::isPromotable( from, to, name ); + if( !MetadataAlgo::isPromotable( from, to, name ) ) + { + return false; + } + // Only copy if the destination doesn't already have the metadata. + // This avoids making unnecessary instance-level metadata when the + // same value is registered statically (against the plug type). + ConstDataPtr fromValue = Gaffer::Metadata::value( from, name ); + ConstDataPtr toValue = Gaffer::Metadata::value( to, name ); + if( fromValue && toValue ) + { + return !toValue->isEqualTo( fromValue.get() ); + } + return (bool)fromValue != (bool)toValue; }, // We use `persistent = dynamic` so that `promoteWithName()` can be used in // constructors for custom nodes, to promote a plug from an internal diff --git a/src/GafferBindings/MetadataBinding.cpp b/src/GafferBindings/MetadataBinding.cpp index e4ef351b012..e4e8968a0e4 100644 --- a/src/GafferBindings/MetadataBinding.cpp +++ b/src/GafferBindings/MetadataBinding.cpp @@ -66,8 +66,7 @@ namespace GafferBindings std::string metadataSerialisation( const Gaffer::GraphComponent *graphComponent, const std::string &identifier, Serialisation &serialisation ) { - std::vector keys; - Metadata::registeredValues( graphComponent, keys, /* instanceOnly = */ true, /* persistentOnly = */ true ); + const std::vector keys = Metadata::registeredValues( graphComponent, Metadata::RegistrationTypes::InstancePersistent ); const Plug *plug = runTimeCast( graphComponent ); const Reference *reference = plug ? runTimeCast( plug->node() ) : nullptr; diff --git a/src/GafferModule/MetadataAlgoBinding.cpp b/src/GafferModule/MetadataAlgoBinding.cpp index 9f422095f4e..a1c50d2dccc 100644 --- a/src/GafferModule/MetadataAlgoBinding.cpp +++ b/src/GafferModule/MetadataAlgoBinding.cpp @@ -205,13 +205,21 @@ void copyColorsWrapper( const Gaffer::Plug &srcPlug, Gaffer::Plug &dstPlug, bool // Promotability // ============= - bool isPromotableWrapper( const GraphComponent &from, const GraphComponent &to, const IECore::InternedString &name ) { IECorePython::ScopedGILRelease gilRelease; return isPromotable( &from, &to, name ); } +// Cleanup +// ======= + +void deregisterRedundantValuesWrapper( GraphComponent &g ) +{ + IECorePython::ScopedGILRelease gilRelease; + return deregisterRedundantValues( &g ); +} + } // namespace void GafferModule::bindMetadataAlgo() @@ -339,4 +347,9 @@ void GafferModule::bindMetadataAlgo() def( "isPromotable", &isPromotableWrapper, ( arg( "from" ), arg( "to" ), arg( "name" ) ) ); + // Cleanup + // ======= + + def( "deregisterRedundantValues", &deregisterRedundantValuesWrapper ); + } diff --git a/src/GafferModule/MetadataBinding.cpp b/src/GafferModule/MetadataBinding.cpp index b651b221939..357bc23c5fc 100644 --- a/src/GafferModule/MetadataBinding.cpp +++ b/src/GafferModule/MetadataBinding.cpp @@ -193,7 +193,13 @@ object value( IECore::InternedString target, IECore::InternedString key, bool co return dataToPython( d.get(), copy ); } -object graphComponentValue( const GraphComponent &graphComponent, IECore::InternedString key, bool instanceOnly, bool copy ) +object graphComponentValue( const GraphComponent &graphComponent, IECore::InternedString key, unsigned registrationTypes, bool copy ) +{ + ConstDataPtr d = Metadata::value( &graphComponent, key, registrationTypes ); + return dataToPython( d.get(), copy ); +} + +object graphComponentValueDeprecated( const GraphComponent &graphComponent, IECore::InternedString key, bool instanceOnly, bool copy ) { ConstDataPtr d = Metadata::value( &graphComponent, key, instanceOnly ); return dataToPython( d.get(), copy ); @@ -304,7 +310,13 @@ list registeredValues( IECore::InternedString target ) return keysToList( keys ); } -list registeredGraphComponentValues( const GraphComponent *target, bool instanceOnly, bool persistentOnly ) +list registeredGraphComponentValues( const GraphComponent *target, Metadata::RegistrationTypes registrationTypes ) +{ + const std::vector keys = Metadata::registeredValues( target, registrationTypes ); + return keysToList( keys ); +} + +list registeredGraphComponentValuesDeprecated( const GraphComponent *target, bool instanceOnly, bool persistentOnly ) { std::vector keys; Metadata::registeredValues( target, keys, instanceOnly, persistentOnly ); @@ -339,8 +351,35 @@ list nodesWithMetadata( GraphComponent *root, const std::string &key, bool insta void GafferModule::bindMetadata() { - scope s = class_( "Metadata", no_init ) + class_ metadataClass( "Metadata", no_init ); + + { + scope s = metadataClass; + enum_( "RegistrationTypes" ) + .value( "None_", Metadata::RegistrationTypes::None ) + .value( "TypeId", Metadata::RegistrationTypes::TypeId ) + .value( "TypeIdDescendant", Metadata::RegistrationTypes::TypeIdDescendant ) + .value( "InstancePersistent", Metadata::RegistrationTypes::InstancePersistent ) + .value( "InstanceNonPersistent", Metadata::RegistrationTypes::InstanceNonPersistent ) + .value( "Instance", Metadata::RegistrationTypes::Instance ) + .value( "All", Metadata::RegistrationTypes::All ) + ; + + enum_( "ValueChangedReason" ) + .value( "StaticRegistration", Metadata::ValueChangedReason::StaticRegistration ) + .value( "StaticDeregistration", Metadata::ValueChangedReason::StaticDeregistration ) + .value( "InstanceRegistration", Metadata::ValueChangedReason::InstanceRegistration ) + .value( "InstanceDeregistration", Metadata::ValueChangedReason::InstanceDeregistration ) + ; + + SignalClass, ValueChangedSlotCaller>( "ValueChangedSignal" ); + SignalClass, ValueChangedSlotCaller>( "NodeValueChangedSignal" ); + SignalClass, ValueChangedSlotCaller>( "PlugValueChangedSignal" ); + SignalClass, ValueChangedSlotCaller>( "LegacyNodeValueChangedSignal" ); + SignalClass, ValueChangedSlotCaller>( "LegacyPlugValueChangedSignal" ); + } + metadataClass .def( "registerValue", ®isterValue ) .def( "registerValue", ®isterNodeValue ) .def( "registerValue", ®isterPlugValue ) @@ -354,13 +393,19 @@ void GafferModule::bindMetadata() .staticmethod( "registerValue" ) .def( "registeredValues", ®isteredValues ) - .def( "registeredValues", ®isteredGraphComponentValues, + .def( "registeredValues", ®isteredGraphComponentValuesDeprecated, ( boost::python::arg( "target" ), boost::python::arg( "instanceOnly" ) = false, boost::python::arg( "persistentOnly" ) = false ) ) + .def( "registeredValues", ®isteredGraphComponentValues, + ( + boost::python::arg( "target" ), + boost::python::arg( "registrationTypes" ) = Metadata::RegistrationTypes::All + ) + ) .staticmethod( "registeredValues" ) .def( "value", &value, @@ -370,7 +415,7 @@ void GafferModule::bindMetadata() boost::python::arg( "_copy" ) = true ) ) - .def( "value", &graphComponentValue, + .def( "value", &graphComponentValueDeprecated, ( boost::python::arg( "target" ), boost::python::arg( "key" ), @@ -378,6 +423,13 @@ void GafferModule::bindMetadata() boost::python::arg( "_copy" ) = true ) ) + .def( "value", &graphComponentValue, + ( + boost::python::arg( "target" ), + boost::python::arg( "registrationTypes" ) = Metadata::RegistrationTypes::All, + boost::python::arg( "_copy" ) = true + ) + ) .staticmethod( "value" ) .def( "deregisterValue", (void (*)( IECore::InternedString, IECore::InternedString ) )&Metadata::deregisterValue ) @@ -419,17 +471,4 @@ void GafferModule::bindMetadata() .staticmethod( "nodesWithMetadata" ) ; - enum_( "ValueChangedReason" ) - .value( "StaticRegistration", Metadata::ValueChangedReason::StaticRegistration ) - .value( "StaticDeregistration", Metadata::ValueChangedReason::StaticDeregistration ) - .value( "InstanceRegistration", Metadata::ValueChangedReason::InstanceRegistration ) - .value( "InstanceDeregistration", Metadata::ValueChangedReason::InstanceDeregistration ) - ; - - SignalClass, ValueChangedSlotCaller>( "ValueChangedSignal" ); - SignalClass, ValueChangedSlotCaller>( "NodeValueChangedSignal" ); - SignalClass, ValueChangedSlotCaller>( "PlugValueChangedSignal" ); - SignalClass, ValueChangedSlotCaller>( "LegacyNodeValueChangedSignal" ); - SignalClass, ValueChangedSlotCaller>( "LegacyPlugValueChangedSignal" ); - } diff --git a/src/GafferScene/SceneAlgo.cpp b/src/GafferScene/SceneAlgo.cpp index 9a65e4f47d2..b308b367a33 100644 --- a/src/GafferScene/SceneAlgo.cpp +++ b/src/GafferScene/SceneAlgo.cpp @@ -400,6 +400,8 @@ struct CapturedProcess }; +const InternedString g_processedObjectPlugName( "__processedObject" ); + /// \todo Perhaps add this to the Gaffer module as a /// public class, and expose it within the stats app? /// Give a bit more thought to the CapturedProcess @@ -432,7 +434,7 @@ class CapturingMonitor : public Monitor CapturedProcess::Ptr capturedProcess; ProcessOrScope entry; - if( !p->parent() || p->getName() != m_scenePlugChildName ) + if( !shouldCapture( p ) ) { // Parents may spawn other processes in support of the requested plug. This is one // of these other plugs that isn't directly the requested plug. Instead of creating @@ -491,10 +493,7 @@ class CapturingMonitor : public Monitor bool forceMonitoring( const Plug *plug, const IECore::InternedString &processType ) override { - if( - processType == g_hashProcessType && - plug->parent() && plug->getName() == m_scenePlugChildName - ) + if( processType == g_hashProcessType && shouldCapture( plug ) ) { return true; } @@ -504,6 +503,14 @@ class CapturingMonitor : public Monitor private : + bool shouldCapture( const Plug *plug ) const + { + return + ( plug->parent() && plug->getName() == m_scenePlugChildName ) || + ( (Gaffer::TypeId)plug->typeId() == Gaffer::TypeId::ObjectPlugTypeId && plug->getName() == g_processedObjectPlugName ) + ; + } + using Mutex = tbb::spin_mutex; Mutex m_mutex; diff --git a/src/GafferUI/NoduleLayout.cpp b/src/GafferUI/NoduleLayout.cpp index d59188ad8f7..c38be2791ac 100644 --- a/src/GafferUI/NoduleLayout.cpp +++ b/src/GafferUI/NoduleLayout.cpp @@ -577,8 +577,7 @@ std::vector NoduleLayout::layoutOrder() // Then any custom gadgets specified by the metadata - vector metadata; - Metadata::registeredValues( m_parent.get(), metadata ); + const vector metadata = Metadata::registeredValues( m_parent.get() ); static boost::regex g_customGadgetRegex( "noduleLayout:customGadget:(.+):gadgetType" ); for( vector::const_iterator it = metadata.begin(), eIt = metadata.end(); it != eIt; ++it ) { diff --git a/startup/gui/menus.py b/startup/gui/menus.py index 4943bac94f5..408202d9fac 100644 --- a/startup/gui/menus.py +++ b/startup/gui/menus.py @@ -567,3 +567,31 @@ def __usdLightCreator( lightType ) : with IECore.IgnoredExceptions( ImportError ) : import GafferTractorUI + + +## Metadata cleanup +########################################################################### + +## \todo This shouldn't be necessary forever, since we've fixed the +# problems that caused redundant metadata to be produced in the first +# place. Remove the menu item when it is no longer needed. + +def __removeRedundantMetadata( menu ) : + + script = menu.ancestor( GafferUI.ScriptWindow ).scriptNode() + with Gaffer.UndoScope( script ) : + Gaffer.MetadataAlgo.deregisterRedundantValues( script ) + +def __removeRedundantMetadataActive( menu ) : + + script = menu.ancestor( GafferUI.ScriptWindow ).scriptNode() + return not Gaffer.MetadataAlgo.readOnly( script ) + +scriptWindowMenu.append( + "/Tools/Metadata/Clean Up", + { + "command" : __removeRedundantMetadata, + "active" : __removeRedundantMetadataActive, + "description" : "Optimises file size by removing redundant node metadata." + } +)