Skip to content

Commit 4417298

Browse files
authored
Merge pull request #6806 from GafferHQ/renderManProgress
RenderMan : Add progress reporting option
2 parents 043922b + 9954f9f commit 4417298

File tree

6 files changed

+84
-9
lines changed

6 files changed

+84
-9
lines changed

Changes.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ Improvements
66

77
- Crop : Added `Auto` mode for `areaSource`, automatically cropping to show only non-empty pixels.
88
- GraphEditor : Improved responsiveness of select-drag, by deferring NodeEditor update until the drag ends.
9+
- RenderManOptions : Added `ri:progress` option to control logging of render progress.
910

1011
API
1112
---

python/IECoreRenderManTest/RendererTest.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,31 @@ def testObject( self ) :
167167
image = IECoreImage.ImageDisplayDriver.storedImage( "myLovelySphere" )
168168
self.assertEqual( max( image["A"] ), 1 )
169169

170+
def testProgress( self ) :
171+
172+
messageHandler = IECore.CapturingMessageHandler()
173+
renderer = GafferScene.Private.IECoreScenePreview.Renderer.create(
174+
self.renderer,
175+
GafferScene.Private.IECoreScenePreview.Renderer.RenderType.Batch,
176+
messageHandler = messageHandler
177+
)
178+
179+
renderer.output(
180+
"beauty",
181+
IECoreScene.Output(
182+
( self.temporaryDirectory() / "beauty.exr" ).as_posix(),
183+
"exr",
184+
"rgba",
185+
{
186+
}
187+
)
188+
)
189+
190+
renderer.option( "ri:progressMode", IECore.IntData( 2 ) )
191+
renderer.render()
192+
193+
self.assertRegex( "".join( [ m.message for m in messageHandler.messages ] ), "R90000.*%" )
194+
170195
def testMissingLightShader( self ) :
171196

172197
messageHandler = IECore.CapturingMessageHandler()

src/IECoreRenderMan/Globals.cpp

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,11 @@ const IECore::InternedString g_sampleFilterOption( "ri:samplefilter" );
6969
const IECore::InternedString g_pixelFilterNameOption( "ri:Ri:PixelFilterName" );
7070
const IECore::InternedString g_pixelFilterWidthOption( "ri:Ri:PixelFilterWidth" );
7171
const IECore::InternedString g_pixelVarianceOption( "ri:Ri:PixelVariance" );
72+
const IECore::InternedString g_progressModeOption( "ri:progressMode" );
7273
const IECore::InternedString g_xpuCPUConfigOption( "ri:xpuCpuConfig" );
7374
const IECore::InternedString g_xpuGPUConfigOption( "ri:xpuGpuConfig" );
7475

76+
const RtUString g_progressModeParameter( "progressMode" );
7577
const RtUString g_xpuCPUConfigParameter( "xpu:cpuconfig" );
7678
ConstDataPtr g_xpuCPUConfigDefault = new IntData( 1 );
7779
// We default GPUs to off, on the assumption that most rendering will still occur
@@ -194,6 +196,11 @@ Globals::Globals( RtUString rileyVariant, IECoreScenePreview::Renderer::RenderTy
194196
m_options.SetString( Loader::strings().k_bucket_order, RtUString( "circle" ) );
195197
}
196198

199+
m_renderParameters.SetString(
200+
RtUString( "renderMode" ),
201+
renderType == IECoreScenePreview::Renderer::RenderType::Interactive ? RtUString( "interactive" ) : RtUString( "batch" )
202+
);
203+
197204
for( const auto &[name, value] : g_lpeLobeDefaults )
198205
{
199206
// Set up default lobe definitions.
@@ -337,6 +344,25 @@ void Globals::option( const IECore::InternedString &name, const IECore::Object *
337344
auto data = optionCast<const Data>( value, name );
338345
ParamListAlgo::convertParameter( g_xpuGPUConfigParameter, data ? data : g_xpuGPUConfigDefault.get(), m_rileyParameters );
339346
}
347+
else if( name == g_progressModeOption )
348+
{
349+
if( auto *d = optionCast<const IntData>( value, name ) )
350+
{
351+
// This is the XPU way of requesting progress reporting - an undocumented
352+
// parameter passed to the `Riley::Render()` call.
353+
m_renderParameters.SetInteger( RtUString( "progressMode" ), d->readable() );
354+
// But RIS wants progress specified as a "command line argument" to
355+
// `PRManRenderBegin()` instead, so we smuggle the value into `Session.cpp`
356+
// via `m_options` as well.
357+
/// \todo Remove when RIS is no longer.
358+
m_options.SetInteger( RtUString( "progressMode" ), d->readable() );
359+
}
360+
else
361+
{
362+
m_renderParameters.Remove( RtUString( "progressMode" ) );
363+
m_options.Remove( RtUString( "progressMode" ) );
364+
}
365+
}
340366
else if( boost::starts_with( name.c_str(), "ri:lpe:" ) )
341367
{
342368
const RtUString renderManName( name.c_str() + g_renderManPrefix.size() );
@@ -532,18 +558,14 @@ void Globals::render()
532558
switch( m_session->renderType )
533559
{
534560
case IECoreScenePreview::Renderer::Batch : {
535-
RtParamList renderOptions;
536-
renderOptions.SetString( RtUString( "renderMode" ), RtUString( "batch" ) );
537-
m_session->riley->Render( { 1, &m_renderView }, renderOptions );
561+
m_session->riley->Render( { 1, &m_renderView }, m_renderParameters );
538562
break;
539563
}
540564
case IECoreScenePreview::Renderer::Interactive :
541565
/// \todo Would it reduce latency if we reused the same thread?
542566
m_interactiveRenderThread = std::thread(
543567
[this] {
544-
RtParamList renderOptions;
545-
renderOptions.SetString( RtUString( "renderMode" ), RtUString( "interactive" ) );
546-
m_session->riley->Render( { 1, &m_renderView }, renderOptions );
568+
m_session->riley->Render( { 1, &m_renderView }, m_renderParameters );
547569
}
548570
);
549571
break;

src/IECoreRenderMan/Globals.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ class Globals : public boost::noncopyable
9898

9999
RtParamList m_rileyParameters;
100100
RtParamList m_options;
101+
RtParamList m_renderParameters;
101102
std::string m_cameraOption;
102103
IECoreScene::ConstShaderPtr m_integratorToConvert;
103104
IECoreScene::ConstShaderNetworkPtr m_displayFilterToConvert;

src/IECoreRenderMan/Session.cpp

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ const RtUString g_lightColorUStr( "lightColor" );
5959
const RtUString g_lightColorMapUStr( "lightColorMap" );
6060
const RtUString g_portalNameUStr( "portalName" );
6161
const RtUString g_portalToDomeUStr( "portalToDome" );
62+
const RtUString g_progressModeUStr( "progressMode" );
6263
const RtUString g_pxrDomeLightUStr( "PxrDomeLight" );
6364
const RtUString g_pxrPortalLightUStr( "PxrPortalLight" );
6465
const RtUString g_tintUStr( "tint" );
@@ -167,7 +168,14 @@ Session::Session( RtUString rileyVariant, const RtParamList &rileyParameters, IE
167168
args.push_back( "-recover" );
168169
args.push_back( "1" );
169170
}
170-
171+
int32_t progressMode = 0;
172+
if( options.GetInteger( g_progressModeUStr, progressMode ) && progressMode )
173+
{
174+
// This is needed for RIS, but doesn't do anything for XPU. For XPU
175+
// we have to pass `progressMode` to the `Riley::Render()` call - see
176+
// `Globals.cpp`.
177+
args.push_back( progressMode == 1 ? "-Progress" : "-progress" );
178+
}
171179
m_riCtl->PRManRenderBegin( args.size(), args.data() );
172180

173181
if( messageHandler )

startup/GafferScene/renderManOptions.py

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -259,8 +259,9 @@
259259

260260
},
261261

262-
# Add an option to allow checkpoint recovery - this is handled by `IECoreRenderMan::Session::Session()`
263-
# since it is not an official RenderMan option.
262+
# Add options to allow checkpoint recovery and progress logging - these
263+
# are not official RenderMan options so have special handling in
264+
# IECoreRenderMan.
264265

265266
"option:ri:checkpoint:recover" : {
266267

@@ -272,6 +273,23 @@
272273

273274
},
274275

276+
"option:ri:progressMode" : {
277+
278+
"defaultValue" : 0,
279+
"description" : "Reports render progress percentage.",
280+
"label" : "Report Progress",
281+
# The statistics section isn't ideal for this, but of the available
282+
# sections it's the one that makes most sense. It seems daft to make
283+
# a new section for this one thing.
284+
"layout:section" : Gaffer.Metadata.value( "option:ri:statistics:jsonFilename", "layout:section" ),
285+
# In theory RenderMan accepts values of 0, 1 and 2, but the
286+
# distinction (whether or not a linefeed is used) is lost by the
287+
# time it is passed through our MessageHandlers. So just present as
288+
# a simple boolean option.
289+
"plugValueWidget:type" : "GafferUI.BoolPlugValueWidget",
290+
291+
},
292+
275293
# Add options to specify which devices XPU should use. Since there don't
276294
# seem to be any names for this specified in the USD schemas shipping
277295
# with RenderMan, we're using the names from HdPrmanLoaderTokens in the

0 commit comments

Comments
 (0)