From 8aeb91eccb9af70eff5af0ccb4efe6ec64769d22 Mon Sep 17 00:00:00 2001
From: John Haddon <thehaddonyoof@gmail.com>
Date: Tue, 14 Nov 2023 09:58:34 +0000
Subject: [PATCH] fixup! IECoreArnold : Support `NODE *` shader parameters

In batch renders, the `refCount()` on the shaders in the cache will always be 1, because we don't keep a hold of the ObjectInterfacePtr for the objects referencing the shaders. This meant we weren't updating NodeParameters at all during batch renders.
---
 python/IECoreArnoldTest/RendererTest.py | 41 ++++++++++++++++++++++++-
 src/IECoreArnold/Renderer.cpp           | 17 +++++-----
 2 files changed, 50 insertions(+), 8 deletions(-)

diff --git a/python/IECoreArnoldTest/RendererTest.py b/python/IECoreArnoldTest/RendererTest.py
index c032392f476..e2174a8c5f9 100644
--- a/python/IECoreArnoldTest/RendererTest.py
+++ b/python/IECoreArnoldTest/RendererTest.py
@@ -4237,7 +4237,7 @@ def testDiffuseAndSpecularDepthDefaults( self ) :
 		self.assertEqual( arnold.AiNodeGetInt( options, "GI_diffuse_depth" ), 2 )
 		self.assertEqual( arnold.AiNodeGetInt( options, "GI_specular_depth" ), 2 )
 
-	def testNodeParameters( self ) :
+	def testInteractiveNodeParameters( self ) :
 
 		# Make renderer with two cameras
 
@@ -4357,6 +4357,45 @@ def assertCameraParameter( renderer, cameraName ) :
 		del camera1, cameraAttributes, plane
 		del renderer
 
+	def testBatchNodeParameters( self ) :
+
+		# Render a camera projection setup to an ASS file.
+
+		renderer = GafferScene.Private.IECoreScenePreview.Renderer.create(
+			"Arnold",
+			GafferScene.Private.IECoreScenePreview.Renderer.RenderType.SceneDescription,
+			str( self.temporaryDirectory() / "test.ass" )
+		)
+
+		renderer.camera( "/camera", IECoreScene.Camera(), renderer.attributes( IECore.CompoundObject() ) )
+
+		shaderNetwork = IECoreScene.ShaderNetwork(
+			shaders = {
+				"output" : IECoreScene.Shader(
+					"camera_projection", "ai:surface",
+					{ "camera" : IECore.StringData( "/camera" ) }
+				)
+			},
+			output = ( "output", "" )
+		)
+
+		renderer.object(
+			"/plane",
+			IECoreScene.MeshPrimitive.createPlane( imath.Box2f( imath.V2f( -1 ), imath.V2f( 1 ) ) ),
+			renderer.attributes( IECore.CompoundObject( { "ai:surface" : shaderNetwork } ) )
+		)
+		renderer.render()
+
+		# Check we got what we wanted.
+
+		with IECoreArnold.UniverseBlock( writable = True ) as universe :
+
+			arnold.AiSceneLoad( universe, str( self.temporaryDirectory() / "test.ass" ), None )
+			planeNode = arnold.AiNodeLookUpByName( universe, "/plane" )
+			planeShader = arnold.AiNodeGetPtr( planeNode, "shader" )
+			cameraNode = arnold.AiNodeGetPtr( planeShader, "camera" )
+			self.assertEqual( arnold.AiNodeGetName( cameraNode ), "/camera" )
+
 	@staticmethod
 	def __m44f( m ) :
 
diff --git a/src/IECoreArnold/Renderer.cpp b/src/IECoreArnold/Renderer.cpp
index f8f04f5d37d..7ea6b38f96a 100644
--- a/src/IECoreArnold/Renderer.cpp
+++ b/src/IECoreArnold/Renderer.cpp
@@ -869,15 +869,16 @@ class ShaderCache : public IECore::RefCounted
 			return writeAccessor->second;
 		}
 
-		// _Must_ be called before `render()` launches Arnold, and must not be called concurrently with anything.
-		// This removed shaders that are no longer in use, and ensures all `ShaderNetworkAlgo::NodeParameters`
-		// are updated appropriately.
-		void preRender()
+		// _Must_ be called before `render()` launches Arnold, and must not be
+		// called concurrently with anything. This removed shaders that are no
+		// longer in use during interactive renders, and ensures all
+		// `ShaderNetworkAlgo::NodeParameters` are updated appropriately.
+		void preRender( IECoreScenePreview::Renderer::RenderType renderType )
 		{
 			vector<IECore::MurmurHash> toErase;
 			for( Cache::iterator it = m_cache.begin(), eIt = m_cache.end(); it != eIt; ++it )
 			{
-				if( it->second->refCount() == 1 )
+				if( renderType == IECoreScenePreview::Renderer::RenderType::Interactive && it->second->refCount() == 1 )
 				{
 					// Only one reference - this is ours, so
 					// nothing outside of the cache is using the
@@ -3273,6 +3274,8 @@ class ArnoldGlobals
 
 		AtUniverse *universe() { return m_universeBlock->universe(); }
 
+		IECoreScenePreview::Renderer::RenderType renderType() const { return m_renderType; }
+
 		void option( const IECore::InternedString &name, const IECore::Object *value )
 		{
 			AtNode *options = AiUniverseGetOptions( m_universeBlock->universe() );
@@ -3707,7 +3710,7 @@ class ArnoldGlobals
 				AiNodeResetParameter( options, g_subdivDicingCameraString );
 			}
 
-			m_shaderCache->preRender();
+			m_shaderCache->preRender( m_renderType );
 
 			// Do the appropriate render based on
 			// m_renderType.
@@ -4335,7 +4338,7 @@ class ArnoldRenderer final : public ArnoldRendererBase
 		{
 			const IECore::MessageHandler::Scope s( m_messageHandler.get() );
 
-			m_shaderCache->preRender();
+			m_shaderCache->preRender( m_globals->renderType() );
 			m_instanceCache->clearUnused();
 			m_globals->render();
 		}