diff --git a/Changes.md b/Changes.md index 25b79de8fc..f9ebc25394 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 : `: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 afdcf00472..071d8201c5 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 ¶meters ) 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 );