Merge pull request #5474 from johnhaddon/renderPurposes

Encapsulate : Use correct rendering options

Changes.md

@@ -17,6 +17,9 @@ Fixes
 - StringPlugValueWidget : Fixed bug handling <kbd>Esc</kbd>.
 - Arnold : Fixed unnecessary `opaque` attribute deprecation warnings. These are now only emitted in the case that `opaque` has been explicitly turned off.
 - ShaderUI : Fixed bug causing identical but independent shaders in a shader network from being included in the shader browser.
+- Encapsulate :
+  - Fixed bug where global attributes (from the point of encapsulation) were baked into the contents of the capsule instead of being inherited naturally (from the node being rendered).
+  - Fixed motion blur so that global settings are now taken from the downstream node being rendered, not from the input to the Encapsulate node.
 
 API
 ---

include/GafferScene/Capsule.h

@@ -37,6 +37,7 @@
 #pragma once
 
 #include "GafferScene/Private/IECoreScenePreview/Procedural.h"
+#include "GafferScene/Private/RendererAlgo.h"
 #include "GafferScene/ScenePlug.h"
 
 #include "Gaffer/Context.h"
@@ -90,6 +91,11 @@ class GAFFERSCENE_API Capsule : public IECoreScenePreview::Procedural
 		const ScenePlug::ScenePath &root() const;
 		const Gaffer::Context *context() const;
 
+		/// Used to apply the correct render settings to the capsule before rendering it.
+		/// For internal use only.
+		void setRenderOptions( const GafferScene::Private::RendererAlgo::RenderOptions &renderOptions );
+		std::optional<GafferScene::Private::RendererAlgo::RenderOptions> getRenderOptions() const;
+
 	private :
 
 		void throwIfNoScene() const;

include/GafferScene/Private/RendererAlgo.h

@@ -63,14 +63,34 @@ namespace Private
 namespace RendererAlgo
 {
 
+struct GAFFERSCENE_API RenderOptions
+{
+	RenderOptions();
+	RenderOptions( const ScenePlug *scene );
+	RenderOptions( const RenderOptions &other ) = default;
+	RenderOptions& operator=( const RenderOptions &other ) = default;
+	bool operator==( const RenderOptions &other ) const;
+	/// The globals from the scene.
+	IECore::ConstCompoundObjectPtr globals;
+	/// Convenient access to specific properties, taking into account default
+	/// values if they have not been specified in the scene.
+	bool transformBlur;
+	bool deformationBlur;
+	Imath::V2f shutter;
+	IECore::ConstStringVectorDataPtr includedPurposes;
+	/// Returns true if `includedPurposes` includes the purpose defined by
+	/// `attributes`.
+	bool purposeIncluded( const IECore::CompoundObject *attributes ) const;
+};
+
 /// Creates the directories necessary to receive the outputs defined in globals.
 GAFFERSCENE_API void createOutputDirectories( const IECore::CompoundObject *globals );
 
-/// Set the "times" to a list of times to sample the transform or deformation of a location at, based on the
-/// "motionBlur" enable coming from the options, a shutter, and location attributes.  Returns a boolean for
-/// whether times has been altered ( returns false if times was already set correctly ).
-GAFFERSCENE_API bool transformMotionTimes( bool motionBlur, const Imath::V2f &shutter, const IECore::CompoundObject *attributes, std::vector<float> &times );
-GAFFERSCENE_API bool deformationMotionTimes( bool motionBlur, const Imath::V2f &shutter, const IECore::CompoundObject *attributes, std::vector<float> &times );
+/// Sets `times` to a list of times to sample the transform or deformation of a
+/// location at, based on the render options and location attributes. Returns `true`
+/// if `times` was altered and `false` if it was already set correctly.
+GAFFERSCENE_API bool transformMotionTimes( const RenderOptions &renderOptions, const IECore::CompoundObject *attributes, std::vector<float> &times );
+GAFFERSCENE_API bool deformationMotionTimes( const RenderOptions &renderOptions, const IECore::CompoundObject *attributes, std::vector<float> &times );
 
 /// Samples the local transform from the current location in preparation for output to the renderer.
 /// "samples" will be set to contain one sample for each sampleTime, unless the samples are all identical,
@@ -259,10 +279,10 @@ class GAFFERSCENE_API LightLinks : boost::noncopyable
 
 };
 
-GAFFERSCENE_API void outputCameras( const ScenePlug *scene, const IECore::CompoundObject *globals, const RenderSets &renderSets, IECoreScenePreview::Renderer *renderer );
-GAFFERSCENE_API void outputLightFilters( const ScenePlug *scene, const IECore::CompoundObject *globals, const RenderSets &renderSets, LightLinks *lightLinks, IECoreScenePreview::Renderer *renderer );
-GAFFERSCENE_API void outputLights( const ScenePlug *scene, const IECore::CompoundObject *globals, const RenderSets &renderSets, LightLinks *lightLinks, IECoreScenePreview::Renderer *renderer );
-GAFFERSCENE_API void outputObjects( const ScenePlug *scene, const IECore::CompoundObject *globals, const RenderSets &renderSets, const LightLinks *lightLinks, IECoreScenePreview::Renderer *renderer, const ScenePlug::ScenePath &root = ScenePlug::ScenePath() );
+GAFFERSCENE_API void outputCameras( const ScenePlug *scene, const RenderOptions &renderOptions, const RenderSets &renderSets, IECoreScenePreview::Renderer *renderer );
+GAFFERSCENE_API void outputLightFilters( const ScenePlug *scene, const RenderOptions &renderOptions, const RenderSets &renderSets, LightLinks *lightLinks, IECoreScenePreview::Renderer *renderer );
+GAFFERSCENE_API void outputLights( const ScenePlug *scene, const RenderOptions &renderOptions, const RenderSets &renderSets, LightLinks *lightLinks, IECoreScenePreview::Renderer *renderer );
+GAFFERSCENE_API void outputObjects( const ScenePlug *scene, const RenderOptions &renderOptions, const RenderSets &renderSets, const LightLinks *lightLinks, IECoreScenePreview::Renderer *renderer, const ScenePlug::ScenePath &root = ScenePlug::ScenePath() );
 
 } // namespace RendererAlgo
 

include/GafferScene/RenderController.h

@@ -122,14 +122,15 @@ class GAFFERSCENE_API RenderController : public Gaffer::Signals::Trackable
 			DeformationBlurGlobalComponent = 32,
 			CameraShutterGlobalComponent = 64,
 			IncludedPurposesGlobalComponent = 128,
+			CapsuleAffectingGlobalComponents = TransformBlurGlobalComponent | DeformationBlurGlobalComponent | IncludedPurposesGlobalComponent,
 			AllGlobalComponents = GlobalsGlobalComponent | SetsGlobalComponent | RenderSetsGlobalComponent | CameraOptionsGlobalComponent | TransformBlurGlobalComponent | DeformationBlurGlobalComponent | IncludedPurposesGlobalComponent
 		};
 
-		struct MotionBlurOptions
+		struct Unused
 		{
-			bool transformBlur = false;
-			bool deformationBlur = false;
-			Imath::V2f shutter = Imath::V2f( 0 );
+			bool unused1;
+			bool unused2;
+			Imath::V2f unused3;
 		};
 
 		void plugDirtied( const Gaffer::Plug *plug );
@@ -165,8 +166,8 @@ class GAFFERSCENE_API RenderController : public Gaffer::Signals::Trackable
 		std::vector<std::unique_ptr<SceneGraph> > m_sceneGraphs;
 		unsigned m_dirtyGlobalComponents;
 		unsigned m_changedGlobalComponents;
-		IECore::ConstCompoundObjectPtr m_globals;
-		MotionBlurOptions m_motionBlurOptions;
+		std::unique_ptr<Private::RendererAlgo::RenderOptions> m_renderOptions;
+		Unused m_unused;
 		Private::RendererAlgo::RenderSets m_renderSets;
 		std::unique_ptr<Private::RendererAlgo::LightLinks> m_lightLinks;
 		IECoreScenePreview::Renderer::ObjectInterfacePtr m_defaultCamera;

python/GafferSceneTest/CapsuleTest.py

@@ -34,7 +34,6 @@
 #
 ##########################################################################
 
-import inspect
 import unittest
 
 import IECore
@@ -72,5 +71,69 @@ def test( self ) :
 		self.assertEqual( capsuleCopy.bound(), sphere["out"].bound( "/" ) )
 		self.assertEqual( capsuleCopy.hash(), capsule.hash() )
 
+	def testInheritedAttributesAreNotBakedIntoCapsuleContents( self ) :
+
+		# Make a Capsule which inherits attributes from its parent
+		# and from the scene globals.
+
+		sphere = GafferScene.Sphere()
+		group = GafferScene.Group()
+		group["in"][0].setInput( sphere["out"] )
+
+		groupFilter = GafferScene.PathFilter()
+		groupFilter["paths"].setValue( IECore.StringVectorData( [ "/group" ] ) )
+
+		groupAttributes = GafferScene.CustomAttributes()
+		groupAttributes["in"].setInput( group["out"] )
+		groupAttributes["filter"].setInput( groupFilter["out"] )
+		groupAttributes["attributes"].addChild( Gaffer.NameValuePlug( "groupAttribute", 10 ) )
+
+		globalAttributes = GafferScene.CustomAttributes()
+		globalAttributes["in"].setInput( groupAttributes["out"] )
+		globalAttributes["global"].setValue( True )
+		globalAttributes["attributes"].addChild( Gaffer.NameValuePlug( "globalAttribute", 20 ) )
+
+		encapsulate = GafferScene.Encapsulate()
+		encapsulate["in"].setInput( globalAttributes["out"] )
+		encapsulate["filter"].setInput( groupFilter["out"] )
+
+		# Render it, and check that the capsule object had the inherited attributes applied to it.
+
+		renderer = GafferScene.Private.IECoreScenePreview.CapturingRenderer(
+			GafferScene.Private.IECoreScenePreview.Renderer.RenderType.Batch
+		)
+		GafferScene.Private.RendererAlgo.outputObjects(
+			encapsulate["out"], GafferScene.Private.RendererAlgo.RenderOptions( encapsulate["out"] ), GafferScene.Private.RendererAlgo.RenderSets( encapsulate["out"] ), GafferScene.Private.RendererAlgo.LightLinks(),
+			renderer
+		)
+
+		capturedGroup = renderer.capturedObject( "/group" )
+		self.assertIsInstance( capturedGroup.capturedSamples()[0], GafferScene.Capsule )
+		self.assertEqual(
+			capturedGroup.capturedAttributes().attributes(),
+			IECore.CompoundObject( {
+				"groupAttribute" : IECore.IntData( 10 ),
+				"globalAttribute" : IECore.IntData( 20 ),
+				"sets" : IECore.InternedStringVectorData(),
+			} )
+		)
+
+		# Expand the capsule, and check that it didn't bake the inherited attributes onto
+		# its contents. It is the responsibity of the Renderer itself to take care of attribute
+		# inheritance, ideally doing it "live", so that changes to inherited attributes don't
+		# require re-expansion of the capsule.
+
+		renderer = GafferScene.Private.IECoreScenePreview.CapturingRenderer(
+			GafferScene.Private.IECoreScenePreview.Renderer.RenderType.Batch
+		)
+		capturedGroup.capturedSamples()[0].render( renderer )
+		capturedSphere = renderer.capturedObject( "/sphere" )
+		self.assertEqual(
+			capturedSphere.capturedAttributes().attributes(),
+			IECore.CompoundObject( {
+				"sets" : IECore.InternedStringVectorData(),
+			} )
+		)
+
 if __name__ == "__main__":
 	unittest.main()

python/GafferSceneTest/RenderControllerTest.py

@@ -1584,5 +1584,104 @@ def testIncludedPurposes( self ) :
 		controller.update()
 		self.assertIsNotNone( renderer.capturedObject( "/group/sphere" ) )
 
+	def testCapsuleRenderOptions( self ) :
+
+		rootFilter = GafferScene.PathFilter()
+		rootFilter["paths"].setValue( IECore.StringVectorData( [ "*" ] ) )
+
+		cube = GafferScene.Cube()
+
+		encapsulate = GafferScene.Encapsulate()
+		encapsulate["in"].setInput( cube["out"] )
+		encapsulate["filter"].setInput( rootFilter["out"] )
+
+		standardOptions = GafferScene.StandardOptions()
+		standardOptions["in"].setInput( encapsulate["out"] )
+		standardOptions["options"]["includedPurposes"]["enabled"].setValue( True )
+
+		renderer = GafferScene.Private.IECoreScenePreview.CapturingRenderer()
+		controller = GafferScene.RenderController( standardOptions["out"], Gaffer.Context(), renderer )
+		controller.setMinimumExpansionDepth( 2 )
+
+		def assertExpectedRenderOptions() :
+
+			captured = renderer.capturedObject( "/cube" )
+			self.assertIsNotNone( captured )
+			self.assertEqual( len( captured.capturedSamples() ), 1 )
+			self.assertIsInstance( captured.capturedSamples()[0], GafferScene.Capsule )
+			self.assertEqual(
+				captured.capturedSamples()[0].getRenderOptions(),
+				GafferScene.Private.RendererAlgo.RenderOptions( standardOptions["out"] )
+			)
+
+		# Check that a capsule has the initial RenderOptions we expect.
+
+		self.assertTrue( controller.updateRequired() )
+		controller.update()
+		assertExpectedRenderOptions()
+
+		# Check that the capsule is updated when the RenderOptions change.
+
+		standardOptions["options"]["includedPurposes"]["value"].setValue( IECore.StringVectorData( [ "default", "proxy" ] ) )
+		self.assertTrue( controller.updateRequired() )
+		controller.update()
+		assertExpectedRenderOptions()
+
+		# Check that the capsule is not updated when the globals change
+		# but the RenderOptions that the capsule uses aren't affected.
+
+		capture = renderer.capturedObject( "/cube" )
+		standardOptions["options"]["performanceMonitor"]["enabled"].setValue( True )
+		self.assertTrue( controller.updateRequired() )
+		controller.update()
+		self.assertTrue( renderer.capturedObject( "/cube" ).isSame( capture ) )
+
+		# Change RenderOptions again, this time to the default, and check we
+		# get another update.
+
+		del capture
+		standardOptions["options"]["includedPurposes"]["value"].setValue( IECore.StringVectorData( [ "default", "render", "proxy", "guide" ] ) )
+		self.assertTrue( controller.updateRequired() )
+		controller.update()
+		assertExpectedRenderOptions()
+
+		# Remove `includedPurposes` option, so it's not in the globals. The
+		# fallback is the same as the previous value, so we should get no
+		# update.
+
+		capture = renderer.capturedObject( "/cube" )
+		standardOptions["options"]["includedPurposes"]["enabled"].setValue( False )
+		self.assertTrue( controller.updateRequired() )
+		controller.update()
+		self.assertTrue( renderer.capturedObject( "/cube" ).isSame( capture ) )
+
+	def testNoUnnecessaryObjectUpdatesOnPurposeChange( self ) :
+
+		cube = GafferScene.Cube()
+
+		standardOptions = GafferScene.StandardOptions()
+		standardOptions["in"].setInput( cube["out"] )
+
+		renderer = GafferScene.Private.IECoreScenePreview.CapturingRenderer()
+		controller = GafferScene.RenderController( standardOptions["out"], Gaffer.Context(), renderer )
+		controller.setMinimumExpansionDepth( 2 )
+
+		# Check initial capture
+
+		self.assertTrue( controller.updateRequired() )
+		controller.update()
+		capture = renderer.capturedObject( "/cube" )
+		self.assertIsNotNone( capture )
+
+		# Check that changing the purposes doesn't make an unnecessary edit for
+		# the object. It was included before and it is still included, so we
+		# want to reuse the old object.
+
+		standardOptions["options"]["includedPurposes"]["enabled"].setValue( True )
+		standardOptions["options"]["includedPurposes"]["value"].setValue( IECore.StringVectorData( [ "default", "proxy" ] ) )
+		self.assertTrue( controller.updateRequired() )
+		controller.update()
+		self.assertTrue( capture.isSame( renderer.capturedObject( "/cube" ) ) )
+
 if __name__ == "__main__":
 	unittest.main()