Skip to content

Commit

Permalink
RendererAlgo : Default render:includedPurposes to default, render
Browse files Browse the repository at this point in the history
This is the expected default that avoids doubling up `proxy` and `render` geometry in the same image. We couldn't use that for the default in `1.3.x` as it would have been a breaking change that could affect existing renders, but we want to provide the best default behaviour in 1.4.
  • Loading branch information
johnhaddon committed Nov 17, 2023
1 parent 584a228 commit ab49809
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 13 deletions.
1 change: 1 addition & 0 deletions Changes.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ Improvements
Breaking Changes
----------------

- Render : Changed `render:includedPurposes` default to `"default", "render"`.
- ValuePlug : Removed deprecated `getObjectValue()` overload.
- Dispatcher : Removed `createMatching()` method.
- Process : Removed non-const variant of the `handleException()` method.
Expand Down
18 changes: 10 additions & 8 deletions python/GafferSceneTest/RenderControllerTest.py
Original file line number Diff line number Diff line change
Expand Up @@ -1535,36 +1535,37 @@ def testIncludedPurposes( self ) :

self.assertIsNotNone( renderer.capturedObject( "/group/sphere" ) )

# Should still be visible when we add a purpose attribute, because we haven't
# specified the `render:includedPurposes` option.
# Should still be visible when we add a purpose of `render`, because that's included
# in the default for `render:includedPurposes`.

sphereAttributes["attributes"].addChild( Gaffer.NameValuePlug( "usd:purpose", "proxy", defaultEnabled = True ) )
sphereAttributes["attributes"].addChild( Gaffer.NameValuePlug( "usd:purpose", "render", defaultEnabled = True ) )
self.assertTrue( controller.updateRequired() )
controller.update()
self.assertIsNotNone( renderer.capturedObject( "/group/sphere" ) )

# But should be hidden when we add `render:includedPurposes` to exclude it.

standardOptions["options"]["includedPurposes"]["enabled"].setValue( True )
standardOptions["options"]["includedPurposes"]["value"].setValue( IECore.StringVectorData( [ "default", "proxy" ] ) )
self.assertEqual(
standardOptions["options"]["includedPurposes"]["value"].getValue(),
IECore.StringVectorData( [ "default", "render" ] ),
IECore.StringVectorData( [ "default", "proxy" ] ),
)
self.assertTrue( controller.updateRequired() )
controller.update()
self.assertIsNone( renderer.capturedObject( "/group/sphere" ) )

# Should be shown again if we change purpose to one that is included.

sphereAttributes["attributes"][0]["value"].setValue( "render" )
sphereAttributes["attributes"][0]["value"].setValue( "proxy" )
self.assertTrue( controller.updateRequired() )
controller.update()
self.assertIsNotNone( renderer.capturedObject( "/group/sphere" ) )

# Shouldn't matter if parent has a purpose which is excluded, because local
# purpose will override that.

groupAttributes["attributes"].addChild( Gaffer.NameValuePlug( "usd:purpose", "proxy", defaultEnabled = True ) )
groupAttributes["attributes"].addChild( Gaffer.NameValuePlug( "usd:purpose", "render", defaultEnabled = True ) )
self.assertTrue( controller.updateRequired() )
controller.update()
self.assertIsNotNone( renderer.capturedObject( "/group/sphere" ) )
Expand All @@ -1577,7 +1578,8 @@ def testIncludedPurposes( self ) :
controller.update()
self.assertIsNone( renderer.capturedObject( "/group/sphere" ) )

# Reverting to no `includedPurposes` option should revert to showing everything.
# Reverting to no `includedPurposes` option should revert to showing
# just `default` and `render`.

standardOptions["options"]["includedPurposes"]["enabled"].setValue( False )
self.assertTrue( controller.updateRequired() )
Expand Down Expand Up @@ -1640,7 +1642,7 @@ def assertExpectedRenderOptions() :
# get another update.

del capture
standardOptions["options"]["includedPurposes"]["value"].setValue( IECore.StringVectorData( [ "default", "render", "proxy", "guide" ] ) )
standardOptions["options"]["includedPurposes"]["value"].setToDefault()
self.assertTrue( controller.updateRequired() )
controller.update()
assertExpectedRenderOptions()
Expand Down
4 changes: 2 additions & 2 deletions python/GafferSceneTest/RendererAlgoTest.py
Original file line number Diff line number Diff line change
Expand Up @@ -672,14 +672,14 @@ def assertIncludedObjects( scene, includedPurposes, paths ) :
else :
self.assertIsNone( renderer.capturedObject( path ) )

# If we don't specify a purpose, then we should get everything.
# If we don't specify a purpose, then we should get just "default"
# and "render".

assertIncludedObjects(
group["out"], None,
{
"/group/innerGroup1/cube",
"/group/innerGroup1/sphere",
"/group/innerGroup2/cube",
"/group/innerGroup2/sphere",
}
)
Expand Down
4 changes: 1 addition & 3 deletions src/GafferScene/RendererAlgo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,7 @@ const InternedString g_shutterOptionName( "option:render:shutter" );
const InternedString g_includedPurposesOptionName( "option:render:includedPurposes" );
const InternedString g_purposeAttributeName( "usd:purpose" );

/// \todo We should really default to `{ "default", "render" }`, but can only
/// change that on a major version update.
const ConstStringVectorDataPtr g_defaultIncludedPurposes( new StringVectorData( { "default", "render", "proxy", "guide" } ) );
const ConstStringVectorDataPtr g_defaultIncludedPurposes( new StringVectorData( { "default", "render" } ) );
const std::string g_defaultPurpose( "default" );

} // namespace
Expand Down

0 comments on commit ab49809

Please sign in to comment.