Reference : track changes to metadata on plugs

The overall goal here is to be able to promote plugs from References to Boxes
and have the metadata preserved. Previously this failed because we converted
metadata on References to be non-persistent and such metadata doesn't survive
the promotion step.

To get this working we add a mechanism that tracks edits to loaded metadata.
This allows us to determine whether metadata needs to be serialised or not so
that we can skip the step that makes all loaded metadata non-persistent.

include/Gaffer/Reference.h

@@ -65,16 +65,22 @@ class GAFFER_API Reference : public SubGraph
 		/// Emitted when a reference is loaded (or unloaded following an undo).
 		ReferenceLoadedSignal &referenceLoadedSignal();
 
+		bool hasMetadataEdit( const Plug *plug, const IECore::InternedString key ) const;
+
 	private :
 
 		void loadInternal( const std::string &fileName );
 		bool isReferencePlug( const Plug *plug ) const;
+		void transferEditedMetadata( const Plug *srcPlug, Plug *dstPlug ) const;
 
 		void convertPersistentMetadata( Plug *plug ) const;
 
 		std::string m_fileName;
 		ReferenceLoadedSignal m_referenceLoadedSignal;
 
+		class PlugEdits;
+		std::unique_ptr<PlugEdits> m_plugEdits;
+
 };
 
 IE_CORE_DECLAREPTR( Reference )

src/Gaffer/Reference.cpp

@@ -36,8 +36,10 @@
 
 #include "Gaffer/Reference.h"
 
+#include "Gaffer/BlockedConnection.h"
 #include "Gaffer/Metadata.h"
 #include "Gaffer/MetadataAlgo.h"
+#include "Gaffer/PlugAlgo.h"
 #include "Gaffer/ScriptNode.h"
 #include "Gaffer/StandardSet.h"
 #include "Gaffer/StringPlug.h"
@@ -48,6 +50,10 @@
 
 #include "boost/algorithm/string/predicate.hpp"
 #include "boost/bind.hpp"
+#include "boost/container/flat_set.hpp"
+
+#include <unordered_map>
+
 
 using namespace std;
 using namespace IECore;
@@ -137,14 +143,157 @@ void transferOutputs( Gaffer::Plug *srcPlug, Gaffer::Plug *dstPlug )
 
 } // namespace
 
+//////////////////////////////////////////////////////////////////////////
+// PlugEdits. This internal utility class is used to track where edits have
+// been applied to plugs following loading.
+//////////////////////////////////////////////////////////////////////////
+
+class Reference::PlugEdits : public boost::signals::trackable
+{
+
+	public :
+
+		PlugEdits( Reference *reference )
+			:	m_reference( reference )
+		{
+			m_connection = Metadata::plugValueChangedSignal().connect( boost::bind( &PlugEdits::plugValueChanged, this, ::_1, ::_2, ::_3, ::_4 ) );
+			m_reference->childRemovedSignal().connect( boost::bind( &PlugEdits::childRemoved, this, ::_1, ::_2 ) );
+		}
+
+		bool hasMetadataEdit( const Plug *plug, const InternedString &key ) const
+		{
+			const PlugEdit *edit = plugEdit( plug );
+
+			if( !edit )
+			{
+				return false;
+			}
+
+			return edit->m_metadataEdits.find( key ) != edit->m_metadataEdits.end();
+		}
+
+		const boost::container::flat_set<InternedString> &metadataEdits( const Plug *plug )
+		{
+			const PlugEdit *edit = plugEdit( plug );
+			if( edit )
+			{
+				return edit->m_metadataEdits;
+			}
+
+			static boost::container::flat_set<InternedString> g_emptyContainer;
+			return g_emptyContainer;
+		}
+
+		boost::signals::scoped_connection &connection()
+		{
+			return m_connection;
+		}
+
+	private :
+
+		Reference *m_reference;
+		boost::signals::scoped_connection m_connection;
+
+		struct PlugEdit
+		{
+			boost::container::flat_set<InternedString> m_metadataEdits;
+		};
+
+		std::unordered_map<const Plug*, PlugEdit> m_plugEdits;
+
+		const PlugEdit *plugEdit( const Plug *plug ) const
+		{
+			// Cheeky cast better than maintaining two near-identical functions.
+			return const_cast<PlugEdits *>( this )->plugEdit( plug, /* createIfMissing = */ false );
+		}
+
+		PlugEdit *plugEdit( const Plug *plug, bool createIfMissing )
+		{
+			if( plug->node() != m_reference )
+			{
+				return nullptr;
+			}
+
+			auto it = m_plugEdits.find( plug );
+			if( it != m_plugEdits.end() )
+			{
+				return &(it->second);
+			}
+
+			if( !m_reference->isReferencePlug( plug ) )
+			{
+				// We'll allow retrieval of existing edits on this plug, but we
+				// won't create new ones.
+				return nullptr;
+			}
+
+			if( !createIfMissing )
+			{
+				return nullptr;
+			}
+
+			return &m_plugEdits[plug];
+		}
+
+		void plugValueChanged( IECore::TypeId nodeTypeId, const IECore::StringAlgo::MatchPattern &plugPath, IECore::InternedString key, const Gaffer::Plug *plug )
+		{
+			// We only record edits to this instance. If no plug is given, the
+			// change is to generic metadata, independently of instances.
+			if( !plug )
+			{
+				return;
+			}
+
+			ScriptNode *scriptNode = m_reference->ancestor<ScriptNode>();
+			if( scriptNode && ( scriptNode->currentActionStage() == Action::Undo || scriptNode->currentActionStage() == Action::Redo ) )
+			{
+				// Our edit tracking code below utilises the undo system, so we don't need
+				// to do anything for an Undo or Redo - our action from the original Do will
+				// be replayed automatically.
+				return;
+			}
+
+			PlugEdit *edit = plugEdit( plug, /* createIfMissing = */ true );
+			if( !edit )
+			{
+				// May get a NULL edit even with createIfMissing = true,
+				// if the plug is not a reference plug on the right Reference node.
+				return;
+			}
+
+			if( edit->m_metadataEdits.find( key ) != edit->m_metadataEdits.end() )
+			{
+				return;
+			}
+
+			Action::enact(
+				m_reference,
+				[edit, key](){ edit->m_metadataEdits.insert( key ); },
+				[edit, key](){ edit->m_metadataEdits.erase( key ); }
+			);
+		}
+
+		void childRemoved( GraphComponent *parent, GraphComponent *child )
+		{
+			const Plug *plug = runTimeCast<Plug>( child );
+			if( !plug )
+			{
+				return;
+			}
+
+			m_plugEdits.erase( plug );
+		}
+
+};
+
 //////////////////////////////////////////////////////////////////////////
 // Reference
 //////////////////////////////////////////////////////////////////////////
 
 IE_CORE_DEFINERUNTIMETYPED( Reference );
 
 Reference::Reference( const std::string &name )
-	:	SubGraph( name )
+	:	SubGraph( name ), m_plugEdits( new PlugEdits( this ) )
 {
 }
 
@@ -254,6 +403,9 @@ void Reference::loadInternal( const std::string &fileName )
 	boost::filesystem::path path = sp.find( fileName );
 	if( !path.empty() )
 	{
+		// Changes made here aren't user edits and mustn't be tracked.
+		BlockedConnection blockedConnection( m_plugEdits->connection() );
+
 		errors = script->executeFile( path.string(), this, /* continueOnError = */ true );
 	}
 
@@ -265,14 +417,11 @@ void Reference::loadInternal( const std::string &fileName )
 		{
 			// Make the loaded plugs non-dynamic, because we don't want them
 			// to be serialised in the script the reference is in - the whole
-			// point is that they are referenced. For the same reason, make
-			// their instance metadata non-persistent.
+			// point is that they are referenced.
 			plug->setFlags( Plug::Dynamic, false );
-			convertPersistentMetadata( plug );
 			for( RecursivePlugIterator it( plug ); !it.done(); ++it )
 			{
 				(*it)->setFlags( Plug::Dynamic, false );
-				convertPersistentMetadata( it->get() );
 			}
 		}
 	}
@@ -307,7 +456,8 @@ void Reference::loadInternal( const std::string &fileName )
 					copyInputsAndValues( oldPlug, newPlug, /* ignoreDefaultValues = */ !versionPriorTo09 );
 				}
 				transferOutputs( oldPlug, newPlug );
-				MetadataAlgo::copy( oldPlug, newPlug );
+
+				transferEditedMetadata( oldPlug, newPlug );
 			}
 			catch( const std::exception &e )
 			{
@@ -336,6 +486,11 @@ void Reference::loadInternal( const std::string &fileName )
 
 }
 
+bool Reference::hasMetadataEdit( const Plug *plug, const IECore::InternedString key ) const
+{
+	return m_plugEdits->hasMetadataEdit( plug, key );
+}
+
 bool Reference::isReferencePlug( const Plug *plug ) const
 {
 	// If a plug is the descendant of a plug starting with
@@ -385,6 +540,28 @@ bool Reference::isReferencePlug( const Plug *plug ) const
 	return true;
 }
 
+void Reference::transferEditedMetadata( const Plug *srcPlug, Plug *dstPlug ) const
+{
+	// Transfer metadata that was edited and won't be provided by a
+	// load. Note: Adding the metadata to a new plug
+	// automatically registers a PlugEdit for that plug.
+
+	for( const InternedString &key : m_plugEdits->metadataEdits( srcPlug ) )
+	{
+		Gaffer::Metadata::registerValue( dstPlug, key, Gaffer::Metadata::value<IECore::Data>( srcPlug, key ), /* persistent =*/ true);
+	}
+
+	// Recurse
+
+	for( PlugIterator it( srcPlug ); !it.done(); ++it )
+	{
+		if( Plug *dstChildPlug = dstPlug->getChild<Plug>( (*it)->getName() ) )
+		{
+			transferEditedMetadata( it->get(), dstChildPlug );
+		}
+	}
+}
+
 void Reference::convertPersistentMetadata( Plug *plug ) const
 {
 	vector<InternedString> keys;

src/GafferModule/SubGraphBinding.cpp

@@ -289,6 +289,7 @@ void GafferModule::bindSubGraph()
 		.def( "load", &load )
 		.def( "fileName", &Reference::fileName, return_value_policy<copy_const_reference>() )
 		.def( "referenceLoadedSignal", &Reference::referenceLoadedSignal, return_internal_reference<1>() )
+		.def( "hasMetadataEdit", &Reference::hasMetadataEdit )
 	;
 
 	SignalClass<Reference::ReferenceLoadedSignal, DefaultSignalCaller<Reference::ReferenceLoadedSignal>, ReferenceLoadedSlotCaller >( "ReferenceLoadedSignal" );