From 51c4ad19d21294a076c5c723246cfb214900462a Mon Sep 17 00:00:00 2001
From: Murray Stevenson <50844517+murraystevenson@users.noreply.github.com>
Date: Tue, 11 Aug 2026 17:29:39 -0700
Subject: [PATCH] Cycles Renderer : Update pause() to wait until the render has
 paused

We've run into intermittent test failures on CI where renders in tests such as InteractiveRenderTest.testLights fail to produce the expected image after a pause, edit, resume. One recent clue from the new assertEventually reporting shows those failing renders having early iterations representing the previous scene state.

I'm able to reproduce the failures locally, and a bit of instrumentation showed that in those failing cases, the session reset in `Renderer::render()` never occurs as `m_scene->need_reset()` returns false. It seems like we're running into a timing issue where we ask Cycles to pause in `Renderer::pause()` but then make scene edits before the render has actually paused. Meanwhile Cycles clears the update flags, pauses, and then our later `m_scene->need_reset()` check returns false when resuming the render.

As an experiment, I've made `Renderer::pause()` poll the session status waiting for it to report that the render is paused before returning. I'm not super happy with this, but I've yet to come up with a better alternative...
---
 Changes.md                                    |  1 +
 .../IECoreCyclesPreview/Renderer.cpp          | 31 ++++++++++++++++---
 2 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/Changes.md b/Changes.md
index 25b79de8fcf..f9ebc253946 100644
--- a/Changes.md
+++ b/Changes.md
@@ -16,6 +16,7 @@ Fixes
 - SetExpressionAlgo : Fixed invalid set expressions returned by `exclude()` when the set expression to be excluded contains only whitespace [^1].
 - PlugLayout : `<layoutName>:width` metadata is now correctly reapplied to widgets with labels when a PlugLayout is rebuilt.
 - GraphEditor : Fixed bug with history back and forward buttons when a node in the history has been deleted (#7071) [^1].
+- Cycles : Fixed potential interactive rendering errors caused by scene edits being made before the render has paused.
 
 API
 ---
diff --git a/src/GafferCycles/IECoreCyclesPreview/Renderer.cpp b/src/GafferCycles/IECoreCyclesPreview/Renderer.cpp
index afdcf00472d..071d8201c59 100644
--- a/src/GafferCycles/IECoreCyclesPreview/Renderer.cpp
+++ b/src/GafferCycles/IECoreCyclesPreview/Renderer.cpp
@@ -2451,6 +2451,8 @@ IECore::InternedString g_dicingCameraOptionName( "cycles:dicing_camera" );
 // Cryptomatte
 IECore::InternedString g_cryptomatteDepthOptionName( "cycles:film:cryptomatte_depth");
 
+const string g_renderPausedStatus( "Rendering Paused" );
+
 IE_CORE_FORWARDDECLARE( CyclesRenderer )
 
 class CyclesRenderer final : public IECoreScenePreview::Renderer
@@ -2650,10 +2652,32 @@ class CyclesRenderer final : public IECoreScenePreview::Renderer
 		void pause() override
 		{
 			const IECore::MessageHandler::Scope s( m_messageHandler.get() );
-			if( m_rendering )
+			if( !m_rendering || m_renderType != Interactive )
 			{
-				m_session->set_pause( true );
+				return;
 			}
+
+			m_session->set_pause( true );
+
+			// m_session->set_pause() requests an eventual pause, but doesn't block until the render
+			// has actually paused. Our workaround is to monitor the session until it reports that
+			// the render has paused or has been cancelled.
+			string status, subStatus;
+			const auto timeout = std::chrono::steady_clock::now() + std::chrono::seconds( 30 );
+			while( std::chrono::steady_clock::now() < timeout )
+			{
+				m_session->progress.get_status( status, subStatus );
+				if( status == g_renderPausedStatus || m_session->progress.get_cancel() )
+				{
+					return;
+				}
+				std::this_thread::sleep_for( std::chrono::milliseconds( 10 ) );
+			}
+
+			IECore::msg(
+				IECore::Msg::Warning, "CyclesRenderer::pause",
+				"Timed out waiting for Cycles to pause render (last status \"{}\")", status
+			);
 		}
 
 		IECore::DataPtr command( const IECore::InternedString name, const IECore::CompoundDataMap &parameters ) override
@@ -3202,9 +3226,6 @@ class CyclesRenderer final : public IECoreScenePreview::Renderer
 			// `set_output_driver()`, because otherwise the rendering threads
 			// may try to send data to an output driver that was just destroyed
 			// on the main thread.
-			/// \todo `Renderer::pause()` really shouldn't return until after
-			/// the PathTrace has been cancelled, so we shouldn't need to worry
-			/// about that here.
 			m_session->reset( m_session->params, m_bufferParams );
 
 			film->set_cryptomatte_passes( crypto );