From b0e01637bb90d2cfacfdee1452afdc026eef642b Mon Sep 17 00:00:00 2001
From: John Haddon <thehaddonyoof@gmail.com>
Date: Wed, 25 Oct 2023 14:14:38 +0100
Subject: [PATCH] BoxIO : Don't create redundant metadata in `setup()`

---
 python/GafferUITest/BoxIOUITest.py | 20 ++++++++++++++++++++
 src/Gaffer/BoxIO.cpp               | 15 ++++++++++++++-
 2 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/python/GafferUITest/BoxIOUITest.py b/python/GafferUITest/BoxIOUITest.py
index 6b803868fdd..6c9ce98df01 100644
--- a/python/GafferUITest/BoxIOUITest.py
+++ b/python/GafferUITest/BoxIOUITest.py
@@ -36,6 +36,8 @@
 
 import unittest
 
+import imath
+
 import Gaffer
 import GafferTest
 import GafferUI
@@ -57,5 +59,23 @@ def testNoRedundantMetadata( self ) :
 		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/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;
 		}
 	);