Merge pull request #1390 from johnhaddon/transformRebase

Transform node improvements
GafferHQ · Jul 23, 2015 · 1638b84 · 1638b84
2 parents 84abf79 + c52462c
commit 1638b84
Show file tree

Hide file tree

Showing 12 changed files with 621 additions and 123 deletions.
diff --git a/include/GafferScene/SceneAlgo.h b/include/GafferScene/SceneAlgo.h
@@ -108,6 +108,11 @@ bool setExists( const ScenePlug *scene, const IECore::InternedString &setName );
 /// computations in parallel for improved performance.
 IECore::ConstCompoundDataPtr sets( const ScenePlug *scene );
 
+/// Returns a bounding box for the specified object. Typically
+/// this is provided by the VisibleRenderable::bound() method, but
+/// for other object types we must return a synthetic bound.
+Imath::Box3f bound( const IECore::Object *object );
+
 } // namespace GafferScene
 
 #include "GafferScene/SceneAlgo.inl"

diff --git a/include/GafferScene/SceneElementProcessor.h b/include/GafferScene/SceneElementProcessor.h
@@ -45,6 +45,10 @@ namespace GafferScene
 
 /// The SceneElementProcessor class provides a base class for modifying elements of an input
 /// scene while leaving the scene hierarchy unchanged.
+/// \todo This "all in one" base class for modifying bounds/transforms/attributes/objects
+/// is feeling a bit unwieldy, and it seems that typical derived classes only ever modify
+/// one thing anyway. Perhaps we'd be better off with individual TransformProcessor,
+/// AttributeProcessor, ObjectProcessor and Deformer base classes.
 class SceneElementProcessor : public FilteredSceneProcessor
 {
 
@@ -101,6 +105,8 @@ class SceneElementProcessor : public FilteredSceneProcessor
 		virtual void hashProcessedAttributes( const ScenePath &path, const Gaffer::Context *context, IECore::MurmurHash &h ) const;
 		virtual IECore::ConstCompoundObjectPtr computeProcessedAttributes( const ScenePath &path, const Gaffer::Context *context, IECore::ConstCompoundObjectPtr inputAttributes ) const;
 
+		/// Note that if you implement processesObject() in such a way as to deform the object, you /must/ also
+		/// implement processesBound() appropriately.
 		virtual bool processesObject() const;
 		virtual void hashProcessedObject( const ScenePath &path, const Gaffer::Context *context, IECore::MurmurHash &h ) const;
 		virtual IECore::ConstObjectPtr computeProcessedObject( const ScenePath &path, const Gaffer::Context *context, IECore::ConstObjectPtr inputObject ) const;
@@ -111,7 +117,7 @@ class SceneElementProcessor : public FilteredSceneProcessor
 		enum BoundMethod
 		{
 			PassThrough = 0,
-			Direct = 1,
+			Processed = 1,
 			Union = 2
 		};
 

diff --git a/include/GafferScene/SceneNode.h b/include/GafferScene/SceneNode.h
@@ -121,10 +121,12 @@ class SceneNode : public Gaffer::ComputeNode
 
 		/// Convenience function to compute the correct bounding box for a path from the bounding box and transforms of its
 		/// children. Using this from computeBound() should be a last resort, as it implies peeking inside children to determine
-		/// information about the parent - the last thing we want to be doing when defining large scenes procedurally.
-		Imath::Box3f unionOfTransformedChildBounds( const ScenePath &path, const ScenePlug *out ) const;
+		/// information about the parent - the last thing we want to be doing when defining large scenes procedurally. If
+		/// `out->childNames()` has been computed already for some reason, then it may be passed to avoid recomputing it
+		/// internally.
+		Imath::Box3f unionOfTransformedChildBounds( const ScenePath &path, const ScenePlug *out, const IECore::InternedStringVectorData *childNames = NULL ) const;
 		/// A hash for the result of the computation in unionOfTransformedChildBounds().
-		IECore::MurmurHash hashOfTransformedChildBounds( const ScenePath &path, const ScenePlug *out ) const;
+		IECore::MurmurHash hashOfTransformedChildBounds( const ScenePath &path, const ScenePlug *out, const IECore::InternedStringVectorData *childNames = NULL ) const;
 
 	private :
 

diff --git a/include/GafferScene/Transform.h b/include/GafferScene/Transform.h
@@ -56,8 +56,11 @@ class Transform : public SceneElementProcessor
 
 		enum Space
 		{
+			Local,
+			Parent,
 			World,
-			Object
+			ResetLocal,
+			ResetWorld
 		};
 
 		Gaffer::IntPlug *spacePlug();
@@ -76,6 +79,15 @@ class Transform : public SceneElementProcessor
 
 	private :
 
+		Imath::M44f fullParentTransform( const ScenePath &path ) const;
+		IECore::MurmurHash fullParentTransformHash( const ScenePath &path ) const;
+		// Returns the transform of the parent of path, either relative to an ancestor matched
+		// by the filter or to the root if no matching ancestor is found. This is useful for
+		// the world reset mode because when a matching ancestor is found we know what its output
+		// transform will be already.
+		Imath::M44f relativeParentTransform( const ScenePath &path, const Gaffer::Context *context, bool &matchingAncestorFound ) const;
+		IECore::MurmurHash relativeParentTransformHash( const ScenePath &path, const Gaffer::Context *context ) const;
+
 		static size_t g_firstPlugIndex;
 
 };

diff --git a/python/GafferSceneTest/TransformTest.py b/python/GafferSceneTest/TransformTest.py
@@ -35,6 +35,7 @@
 ##########################################################################
 
 import unittest
+import math
 
 import IECore
 
@@ -120,22 +121,271 @@ def testSpace( self ) :
 		filter["paths"].setValue( IECore.StringVectorData( [ "/sphere" ] ) )
 		transform["filter"].setInput( filter["out"] )
 
-		self.assertEqual( transform["space"].getValue(), GafferScene.Transform.Space.World )
-
+		self.assertEqual( transform["space"].getValue(), GafferScene.Transform.Space.Local )
 		transform["transform"]["rotate"]["y"].setValue( 90 )
+
+		self.assertSceneValid( transform["out"] )
+
+		self.assertTrue(
+			IECore.V3f( 1, 0, 0 ).equalWithAbsError(
+				IECore.V3f( 0 ) * transform["out"].fullTransform( "/sphere" ),
+				0.000001
+			)
+		)
+
+		transform["space"].setValue( GafferScene.Transform.Space.Parent )
 		self.assertTrue(
 			IECore.V3f( 0, 0, -1 ).equalWithAbsError(
 				IECore.V3f( 0 ) * transform["out"].fullTransform( "/sphere" ),
 				0.000001
 			)
 		)
 
-		transform["space"].setValue( GafferScene.Transform.Space.Object )
+		transform["space"].setValue( GafferScene.Transform.Space.World )
 		self.assertTrue(
-			IECore.V3f( 1, 0, 0 ).equalWithAbsError(
+			IECore.V3f( 0, 0, -1 ).equalWithAbsError(
 				IECore.V3f( 0 ) * transform["out"].fullTransform( "/sphere" ),
 				0.000001
 			)
 		)
+
+	def testSpaceWithNestedHierarchy( self ) :
+
+		sphere = GafferScene.Sphere()
+
+		group = GafferScene.Group()
+		group["in"].setInput( sphere["out"] )
+		group["transform"]["translate"].setValue( IECore.V3f( 1, 0, 0 ) )
+
+		transform = GafferScene.Transform()
+		transform["in"].setInput( group["out"] )
+
+		filter = GafferScene.PathFilter()
+		filter["paths"].setValue( IECore.StringVectorData( [ "/group/sphere" ] ) )
+		transform["filter"].setInput( filter["out"] )
+
+		self.assertEqual( transform["space"].getValue(), GafferScene.Transform.Space.Local )
+		self.assertSceneValid( transform["out"] )
+		self.assertTrue(
+			IECore.V3f( 1, 0, 0 ).equalWithAbsError(
+				IECore.V3f( 0 ) * transform["out"].fullTransform( "/group/sphere" ),
+				0.000001
+			)
+		)
+
+		transform["space"].setValue( GafferScene.Transform.Space.Parent )
+		self.assertSceneValid( transform["out"] )
+		self.assertTrue(
+			IECore.V3f( 1, 0, 0 ).equalWithAbsError(
+				IECore.V3f( 0 ) * transform["out"].fullTransform( "/group/sphere" ),
+				0.000001
+			)
+		)
+
+		transform["space"].setValue( GafferScene.Transform.Space.World )
+		transform["transform"]["rotate"]["y"].setValue( 90 )
+		self.assertTrue(
+			IECore.V3f( 0, 0, -1 ).equalWithAbsError(
+				IECore.V3f( 0 ) * transform["out"].fullTransform( "/group/sphere" ),
+				0.000001
+			)
+		)
+
+	def testResetLocal( self ) :
+
+		sphere = GafferScene.Sphere()
+		sphere["transform"]["translate"].setValue( IECore.V3f( 1, 0, 0 ) )
+
+		group = GafferScene.Group()
+		group["in"].setInput( sphere["out"] )
+		group["transform"]["translate"].setValue( IECore.V3f( 1, 0, 0 ) )
+
+		transform = GafferScene.Transform()
+		transform["in"].setInput( group["out"] )
+		transform["transform"]["rotate"]["y"].setValue( 90 )
+
+		filter = GafferScene.PathFilter()
+		filter["paths"].setValue( IECore.StringVectorData( [ "/group/sphere" ] ) )
+		transform["filter"].setInput( filter["out"] )
+
+		transform["space"].setValue( GafferScene.Transform.Space.ResetLocal )
+		self.assertSceneValid( transform["out"] )
+		self.assertTrue(
+			IECore.V3f( 1, 0, 0 ).equalWithAbsError(
+				IECore.V3f( 0 ) * transform["out"].fullTransform( "/group/sphere" ),
+				0.000001
+			)
+		)
+		self.assertTrue(
+			IECore.V3f( 2, 0, 0 ).equalWithAbsError(
+				IECore.V3f( 0, 0, 1 ) * transform["out"].fullTransform( "/group/sphere" ),
+				0.000001
+			)
+		)
+		self.assertEqual(
+			transform["out"].transform( "/group/sphere" ),
+			IECore.M44f.createRotated( IECore.V3f( 0, math.radians( 90 ), 0 ) )
+		)
+
+	def testResetWorld( self ) :
+
+		sphere = GafferScene.Sphere()
+		sphere["transform"]["translate"].setValue( IECore.V3f( 1, 0, 0 ) )
+
+		group = GafferScene.Group()
+		group["in"].setInput( sphere["out"] )
+		group["transform"]["translate"].setValue( IECore.V3f( 1, 0, 0 ) )
+
+		transform = GafferScene.Transform()
+		transform["in"].setInput( group["out"] )
+		transform["transform"]["rotate"]["y"].setValue( 90 )
+
+		filter = GafferScene.PathFilter()
+		filter["paths"].setValue( IECore.StringVectorData( [ "/group/sphere" ] ) )
+		transform["filter"].setInput( filter["out"] )
+
+		transform["space"].setValue( GafferScene.Transform.Space.ResetWorld )
+		self.assertSceneValid( transform["out"] )
+		self.assertTrue(
+			IECore.V3f( 0, 0, 0 ).equalWithAbsError(
+				IECore.V3f( 0 ) * transform["out"].fullTransform( "/group/sphere" ),
+				0.000001
+			)
+		)
+		self.assertTrue(
+			IECore.V3f( 1, 0, 0 ).equalWithAbsError(
+				IECore.V3f( 0, 0, 1 ) * transform["out"].fullTransform( "/group/sphere" ),
+				0.000001
+			)
+		)
+		self.assertEqual(
+			transform["out"].fullTransform( "/group/sphere" ),
+			IECore.M44f.createRotated( IECore.V3f( 0, math.radians( 90 ), 0 ) )
+		)
+
+	def testWorldWithMatchingAncestors( self ) :
+
+		b = GafferScene.Sphere()
+		b["name"].setValue( "b" )
+
+		a = GafferScene.Group()
+		a["in"].setInput( b["out"] )
+		a["name"].setValue( "a" )
+
+		t = GafferScene.Transform()
+		t["in"].setInput( b["out"] )
+		t["transform"]["translate"].setValue( IECore.V3f( 1, 2, 3 ) )
+		t["space"].setValue( t.Space.World )
+
+		f = GafferScene.PathFilter()
+		f["paths"].setValue( IECore.StringVectorData( [ "/a", "/a/b" ] ) )
+		t["filter"].setInput( f["out"] )
+
+		self.assertSceneValid( t["out"] )
+		self.assertEqual(
+			t["out"].fullTransform( "/a" ),
+			IECore.M44f.createTranslated( IECore.V3f( 1, 2, 3 ) )
+		)
+
+		# We want it to be as if /a/b has been transformed
+		# independently in world space, and not inherit the
+		# additional transform also applied to /a.
+
+		self.assertEqual(
+			t["out"].fullTransform( "/a/b" ),
+			IECore.M44f.createTranslated( IECore.V3f( 1, 2, 3 ) )
+		)
+
+		b["transform"]["translate"].setValue( IECore.V3f( 4, 5, 6 ) )
+		self.assertSceneValid( t["out"] )
+		self.assertEqual(
+			t["out"].fullTransform( "/a/b" ),
+			IECore.M44f.createTranslated( IECore.V3f( 5, 7, 9 ) )
+		)
+
+	def testResetWorldWithMatchingAncestors( self ) :
+
+		c = GafferScene.Sphere()
+		c["name"].setValue( "c" )
+
+		b = GafferScene.Group()
+		b["in"].setInput( c["out"] )
+		b["name"].setValue( "b" )
+
+		a = GafferScene.Group()
+		a["in"].setInput( b["out"] )
+		a["name"].setValue( "a" )
+
+		t = GafferScene.Transform()
+		t["in"].setInput( a["out"] )
+		t["transform"]["translate"].setValue( IECore.V3f( 1, 2, 3 ) )
+		t["space"].setValue( t.Space.ResetWorld )
+
+		# Apply to /a and /a/b/c so that we must take into
+		# account the changing parent transform of /a/b/c
+		# to get it's absolute position in world space
+		# right.
+
+		f = GafferScene.PathFilter()
+		f["paths"].setValue( IECore.StringVectorData( [ "/a", "/a/b/c" ] ) )
+		t["filter"].setInput( f["out"] )
+
+		# Check that we're good.
+
+		self.assertSceneValid( t["out"] )
+
+		self.assertEqual(
+			t["out"].fullTransform( "/a" ),
+			IECore.M44f.createTranslated( IECore.V3f( 1, 2, 3 ) )
+		)
+
+		self.assertEqual(
+			t["out"].fullTransform( "/a/b" ),
+			IECore.M44f.createTranslated( IECore.V3f( 1, 2, 3 ) )
+		)
+
+		self.assertEqual(
+			t["out"].fullTransform( "/a/b/c" ),
+			IECore.M44f.createTranslated( IECore.V3f( 1, 2, 3 ) )
+		)
+
+		# Change the transform on /a/b, and check that it is
+		# retained, but that /a/b/c adjusts for it and maintains
+		# the required absolute transform.
+
+		b["transform"]["translate"].setValue( IECore.V3f( 9, 7, 5 ) )
+
+		self.assertSceneValid( t["out"] )
+
+		self.assertEqual(
+			t["out"].fullTransform( "/a" ),
+			IECore.M44f.createTranslated( IECore.V3f( 1, 2, 3 ) )
+		)
+
+		self.assertEqual(
+			t["out"].fullTransform( "/a/b" ),
+			IECore.M44f.createTranslated( IECore.V3f( 10, 9, 8 ) )
+		)
+
+		self.assertEqual(
+			t["out"].fullTransform( "/a/b/c" ),
+			IECore.M44f.createTranslated( IECore.V3f( 1, 2, 3 ) )
+		)
+
+	def testObjectBoundIncludedWhenDescendantsMatch( self ) :
+
+		s = GafferScene.Cube()
+
+		f = GafferScene.PathFilter()
+		f["paths"].setValue( IECore.StringVectorData( [ "/..." ] ) ) # the dread ellipsis!
+
+		t = GafferScene.Transform()
+		t["in"].setInput( s["out"] )
+		t["filter"].setInput( f["out"] )
+		t["transform"]["translate"].setValue( IECore.V3f( 1 ) )
+
+		self.assertSceneValid( t["out"] )
+		self.assertEqual( t["out"].bound( "/" ), IECore.Box3f( IECore.V3f( 0.5 ), IECore.V3f( 1.5 ) ) )
+
 if __name__ == "__main__":
 	unittest.main()