LinkedScene: adding ability to store tags in scene locations with links

include/IECore/LinkedScene.h

@@ -44,13 +44,14 @@ IE_CORE_FORWARDDECLARE( LinkedScene );
 
 /// Implements a scene that have references (links) to external scenes.
 /// Links can be created at any location in a scene. When a link is created in a given location,
-/// the object, tags, bounds and children will be loaded from the linked scene (with time remapping). The transform, attributes
-/// are still loaded from the main scene.
+/// the object, bounds and children will be loaded from the linked scene (with time remapping). The transform, attributes
+/// are still loaded from the main scene. Tags defined in the link location will be applied (when read) to all the child transforms from the linked scene.
 /// This class wraps another SceneInterface object that is responsible for actually storing the data
 /// (we call it the "main scene"). Links are represented as an attribute in the main scene called "SceneInterface:link".
 /// When created for reading, this class provides seamless access to the hierarchy inside the linked scenes, 
 /// concatenating the two hierarchies in a single path that uniquely identify that location. The time is also 
-/// transparently translated.
+/// transparently translated. Tags that were saved in the linked scene are propagated to the main scene, 
+/// to keep consistent behavior.
 /// When writing, there's no access to the contents of the indexed scene. Instead, it creates the links by either
 /// (1) calls to the function writeLink() or 
 /// (2) calls to the function writeAttribute( LinkedScene::linkSceneAttribute, LinkedScene::linkAttributeData(), ... ). 
@@ -123,7 +124,7 @@ class LinkedScene : public  SampledSceneInterface
 		virtual void writeAttribute( const Name &name, const Object *attribute, double time );
 
 		virtual bool hasTag( const Name &name ) const;
-		virtual void readTags( NameList &tags, bool includeChildren ) const;
+		virtual void readTags( NameList &tags, bool includeChildren = true ) const;
 		virtual void writeTags( const NameList &tags );
 
 		virtual bool hasObject() const;

include/IECore/SceneCache.h

@@ -141,6 +141,12 @@ class SceneCache : public SampledSceneInterface
 		IE_CORE_FORWARDDECLARE( Implementation );
 		virtual SceneCachePtr duplicate( ImplementationPtr& implementation ) const;
 		SceneCache( ImplementationPtr& implementation );
+
+		/// LinkedScene need to specify whether the tag is supposed to be saved
+		/// as a local tag or a tag that was artificially inherited from the child transforms.
+		void writeTags( const NameList &tags,  bool fromChildren );
+
+		friend class LinkedScene;
 
 	private :
 

src/IECore/LinkedScene.cpp

@@ -602,12 +602,6 @@ void LinkedScene::writeAttribute( const Name &name, const Object *attribute, dou
 				throw Exception( "Links to external scenes cannot be created on locations where there are already child locations!" );
 			}
 
-			m_mainScene->readTags( names, false );
-			if ( names.size() )
-			{
-				throw Exception( "Links to external scenes cannot be created on locations where there are already tags stored!" );
-			}
-
 		}
 
 		// we are creating a link!
@@ -652,8 +646,10 @@ void LinkedScene::writeAttribute( const Name &name, const Object *attribute, dou
 		{
 			// save the tags from the linked file to the current location so it gets propagated to the root.
 			NameList tags;
-			linkedScene->readTags(tags);
-			m_mainScene->writeTags( tags );
+
+			/// copy all tags as non local (so we can distinguish from tags added in the LinkedScene)
+			linkedScene->readTags(tags, true);
+			static_cast< SceneCache *>(m_mainScene.get())->writeTags(tags, true);
 		}
 
 		/// we keep the information this level has a link, so we can prevent attempts to 
@@ -666,14 +662,14 @@ void LinkedScene::writeAttribute( const Name &name, const Object *attribute, dou
 
 bool LinkedScene::hasTag( const Name &name ) const
 {
-	if ( m_linkedScene )
-	{
-		return m_linkedScene->hasTag( name );
-	}
-	else
+	if ( m_linkedScene && !m_atLink  )
 	{
-		return m_mainScene->hasTag( name );
+		if ( m_linkedScene->hasTag( name ) )
+		{
+			return true;
+		}
 	}
+	return m_mainScene->hasTag( name );
 }
 
 void LinkedScene::readTags( NameList &tags, bool includeChildren ) const
@@ -683,14 +679,46 @@ void LinkedScene::readTags( NameList &tags, bool includeChildren ) const
 		throw Exception( "readTags with includeChildren option is only supported when reading the scene file!" );
 	}
 
-	if ( m_linkedScene )
+	if ( includeChildren )
 	{
-		return m_linkedScene->readTags( tags, includeChildren );
+		/// we want all tags (local or inherited from children or parent links)
+
+		if ( !m_linkedScene || (m_linkedScene && m_atLink) )
+		{
+			m_mainScene->readTags( tags, true );
+		}
+		else
+		{
+			/// get only the tags that were saved in the LinkedScene at the link location (they will be applied to all the linked children)
+			m_mainScene->readTags( tags, false );
+
+			/// add the tags coming from the linked scene
+			NameList linkTags;
+			m_linkedScene->readTags( linkTags, true );
+			tags.insert( tags.end(), linkTags.begin(), linkTags.end() );
+		}
 	}
 	else
 	{
-		return m_mainScene->readTags( tags, includeChildren );
+		// we are interested only on the tags written at the current location... 
+
+		if ( m_linkedScene )
+		{
+			m_linkedScene->readTags( tags, false );
+			if ( m_atLink )
+			{
+				// if we are at the link location, we should add the tags written in the main scene too.
+				NameList mainTags;
+				m_mainScene->readTags( mainTags, false );
+				tags.insert( tags.end(), mainTags.begin(), mainTags.end() );
+			}
+		}
+		else
+		{
+			m_mainScene->readTags( tags, false );
+		}
 	}
+
 }
 
 void LinkedScene::writeTags( const NameList &tags )
@@ -699,10 +727,6 @@ void LinkedScene::writeTags( const NameList &tags )
 	{
 		throw Exception( "No write access to scene file!" );
 	}
-	if ( m_atLink )
-	{
-		throw Exception( "Locations with links to external scene cannot have tags themselves!" );
-	}
 
 	m_mainScene->writeTags(tags);
 }

src/IECore/SceneCache.cpp

@@ -899,6 +899,7 @@ class SceneCache::WriterImplementation : public SceneCache::Implementation
 			{
 				// we represent inherited tags as empty directories and local tags as a IndexedIO::File of bool type, so
 				// we can easily filter them when reading by entry type.
+				// Inherited tags do not override local tags.
 				if ( fromChildren )
 				{
 					if ( !io->hasEntry( *tIt ) )
@@ -1945,6 +1946,12 @@ void SceneCache::writeTags( const NameList &tags )
 	writer->writeTags( tags );
 }
 
+void SceneCache::writeTags( const NameList &tags, bool fromChildren )
+{
+	WriterImplementation *writer = WriterImplementation::writer( m_implementation.get() );
+	writer->writeTags( tags, fromChildren );
+}
+
 bool SceneCache::hasObject() const
 {
 	return m_implementation->hasObject();

test/IECore/LinkedSceneTest.py

@@ -141,7 +141,7 @@ def testWriting( self ):
 		i2.writeLink( A )
 		i2.writeTransform( IECore.M44dData( IECore.M44d.createTranslated( IECore.V3d( 2, 0, 0 ) ) ), 0.0 )
 		self.assertRaises( RuntimeError, i2.createChild, "cannotHaveChildrenAtLinks" )
-		self.assertRaises( RuntimeError, i2.writeTags, ["cannotHaveTagsAtLinks"] )
+		i2.writeTags( ["canHaveTagsAtLinks"] )
 		self.assertRaises( RuntimeError, i2.writeObject, IECore.SpherePrimitive( 1 ), 0.0 )  # cannot save objects at link locations.
 		b1 = l.createChild("branch1")
 		b1.writeObject( IECore.SpherePrimitive( 1 ), 0.0 )
@@ -184,6 +184,9 @@ def testWriting( self ):
 		self.failUnless( LinkedSceneTest.compareBBox( i2.readBoundAtSample(1), IECore.Box3d(IECore.V3d( -1,-1,-1 ), IECore.V3d( 1,1,1 ) ) ) )
 		self.failUnless( LinkedSceneTest.compareBBox( i2.readBoundAtSample(2), IECore.Box3d(IECore.V3d( 0,-1,-1 ), IECore.V3d( 2,1,1 ) ) ) )
 		self.assertEqual( i2.readTransform( 0 ), IECore.M44dData( IECore.M44d.createTranslated( IECore.V3d( 2, 0, 0 ) ) ) )
+		self.assertTrue( i2.hasTag( "canHaveTagsAtLinks" ) )
+		self.assertTrue( l.hasTag( "canHaveTagsAtLinks" ) )	# tags propagate up
+		self.assertTrue( i2.child("a").hasTag( "canHaveTagsAtLinks" ) )		# tags at link locations propagate down as well
 
 		self.assertEqual( l.scene( [ 'instance0' ] ).path(), [ 'instance0' ] )
 		self.assertEqual( l.scene( [ 'instance0', 'A' ] ).path(), [ 'instance0', 'A' ] )
@@ -351,7 +354,7 @@ def testSet( values ):
 
 		A = l2.createChild('A')
 		A.writeLink( l )
-		self.assertRaises( RuntimeError, A.writeTags, ['cantCreateAtLinks'] )
+		A.writeTags( ['linkedA'] )	# creating tag after link
 
 		B = l2.createChild('B')
 		B.writeLink( a )
@@ -363,7 +366,7 @@ def testSet( values ):
 
 		D = l2.createChild('D')
 		D.writeTags( [ 'D' ] )
-		self.assertRaises( RuntimeError, D.writeLink, a )	# should not allow creating links to scene locations that have tags assigned.
+		D.writeLink( a )	# creating link after tag
 
 		del l, a, l2, A, B, C, c, D
 
@@ -376,11 +379,17 @@ def testSet( values ):
 		ca = c.child("a")
 		D = l2.child("D")
 
-		self.assertEqual( set(l2.readTags()), testSet(["test","tags","C", "D"]) )
+		self.assertTrue( l2.hasTag("test") )
+		self.assertFalse( l2.hasTag("t") )
+		self.assertEqual( set(l2.readTags()), testSet(["test","tags","C", "D","linkedA"]) )
 		self.assertEqual( set(l2.readTags(includeChildren=False)), testSet([]) )
-		self.assertEqual( set(A.readTags()), testSet(["test","tags"]) )
-		self.assertEqual( set(A.readTags(includeChildren=False)), testSet(["tags"]) )
-		self.assertEqual( set(Aa.readTags()), testSet(["test"]) )
+		self.assertEqual( set(A.readTags()), testSet(["test","tags","linkedA"]) )
+		self.assertTrue( A.hasTag( "linkedA" ) )
+		self.assertTrue( A.hasTag( "tags" ) )
+		self.assertTrue( A.hasTag( "test" ) )
+		self.assertFalse( A.hasTag("C") )
+		self.assertEqual( set(A.readTags(includeChildren=False)), testSet(["tags","linkedA"]) )
+		self.assertEqual( set(Aa.readTags()), testSet(["test", "linkedA"]) )
 		self.assertEqual( set(Aa.readTags(includeChildren=False)), testSet(["test"]) )
 		self.assertEqual( set(B.readTags()), testSet(["test"]) )
 		self.assertEqual( set(C.readTags()), testSet(["test","tags","C"]) )
@@ -389,7 +398,7 @@ def testSet( values ):
 		self.assertEqual( set(c.readTags(includeChildren=False)), testSet(["tags"]) )
 		self.assertEqual( set(ca.readTags()), testSet(["test"]) )
 		self.assertEqual( set(C.readTags(includeChildren=False)), testSet(["C"]) )
-		self.assertEqual( set(D.readTags()), testSet(["D"]) )
+		self.assertEqual( set(D.readTags()), testSet(["D", "test"]) )
 
 if __name__ == "__main__":
 	unittest.main()